On Wed, Apr 15, 2020 at 01:09:21PM -0500, Eric Blake wrote:
On 4/15/20 11:16 AM, Richard W.M. Jones wrote:
In the subject, you describe $nbdkit_safe_stdio, but in the patch body...
>---
> plugins/eval/nbdkit-eval-plugin.pod | 11 +++--------
> plugins/sh/nbdkit-sh-plugin.pod | 18 +++++++++++++++++-
> plugins/sh/call.c | 8 ++++++--
> tests/test-single-sh.sh | 4 ++++
> 4 files changed, 30 insertions(+), 11 deletions(-)
>
The patch does not work as intended as currently written: when we
invoke /path/to/script config $key $value, we have already set up
stdin/stdout to our own pipes [1].
Urgghh ... very true.
I think patches 1-8 are still worthwhile though :-) And with the same
mechanism we can pass other environment variables in if we think of
any in future ($nbdkit_peer_name?).
For .config and
.config_complete, reading will always see EOF (no other callback
needs to interact with the original stdin, and callbacks like
.pwrite actually use the pipe for data). If we want to allow a
script to read a password from stdin, we need to preserve the
original fd to .config and .config_complete rather than passing in
an empty pipe (but those are the only two callbacks where it makes
sense, and even then, only when we did not use script=- or when -s
is in effect).
[1] ./nbdkit eval config='ls -l /proc/$$/fd >/dev/tty' a=b
total 0
lr-x------. 1 eblake eblake 64 Apr 15 13:03 0 -> 'pipe:[1669308]'
l-wx------. 1 eblake eblake 64 Apr 15 13:03 1 -> 'pipe:[1669309]'
l-wx------. 1 eblake eblake 64 Apr 15 13:03 2 -> 'pipe:[1669310]'
lr-x------. 1 eblake eblake 64 Apr 15 13:03 255 ->
/tmp/nbdkitevalVUNZ1F/config
Yes we might I suppose duplicate original stdin to another file
descriptor and pass the FD in via an environment variable.
[...]
Side thought: Both the eval and sh plugins already pass on all
unrecognized key=value pairs through to the .config callback, and
error out if the config callback returns missing. But right now, if
a script wants to set up an environment variable that will still be
present to affect later calls, it has to track things itself
(perhaps by having the .config callback append to $tmpdir/vars, then
all other callbacks start by 'source $tmpdir/vars'). Would it make
sense to reserve a special exit value from the .config callback for
the case where the script wants the current key=value passed to
config to be preserved to all future callbacks? Or even state that
the config callback exiting with status 2 (for missing) is NOT an
error, but does the key=value preservation automatically?
Would it be secure, given that plausibly the command line could be
supplied from a different place than the script. eg. if an untrusted
user sets $PATH ...
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages.
http://libguestfs.org