[Libguestfs] [PATCH nbdkit 9/9] eval, sh: Define $nbdkit_safe_stdio = 0|1 in scripts.

[Richard W.M. Jones]

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