[Libguestfs] [nbdkit PATCH 1/2] server: Add nbdkit_stdio_safe

Eric Blake eblake at redhat.com
Sat Apr 4 22:02:38 UTC 2020


Trying to read a password or inline script from stdin when the -s
option was used to connect to our client over stdin is not going to
work.  It would be nice to fail such usage as soon as possible, by
giving plugins a means to decide if it is safe to use stdio during
.config.  Update nbdkit_read_password and the sh plugin to take
advantage of the new entry point.

Also, treat 'nbdkit -s --dump-plugin' as an error, as .dump_plugin is
supposed to interact with stdout.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 docs/nbdkit-plugin.pod          | 23 ++++++++++++++++++++++-
 plugins/sh/nbdkit-sh-plugin.pod |  4 +++-
 include/nbdkit-common.h         |  2 ++
 server/main.c                   |  5 +++--
 server/nbdkit.syms              |  1 +
 server/public.c                 | 18 +++++++++++++++++-
 server/test-public.c            | 23 +++++++++++++++++++++--
 plugins/sh/sh.c                 |  7 ++++++-
 8 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 6c1311eb..822e19df 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -388,6 +388,10 @@ should probably look at other plugins and follow the same conventions.
 If the value is a relative path, then note that the server changes
 directory when it starts up.  See L</FILENAMES AND PATHS> above.

+If C<nbdkit_stdio_safe> returns true, the value of the configuration
+parameter may be used to trigger reading additional data through stdin
+(such as a password or inline script).
+
 If the C<.config> callback is not provided by the plugin, and the user
 tries to specify any C<key=value> arguments, then nbdkit will exit
 with an error.
@@ -1148,6 +1152,23 @@ C<str> can be a string containing a case-insensitive form of various
 common toggle values.  The function returns 0 or 1 if the parse was
 successful.  If there was an error, it returns C<-1>.

+=head2 Interacting with stdin and stdout
+
+Use the C<nbdkit_stdio_safe> utility function to determine if it is
+safe to interact with stdin and stdout during C<.config>.  When nbdkit
+is started under the single connection mode (C<-s>), the plugin must
+not directly interact with stdin, because that would interfere with
+the client.  The result of this function only matters prior to
+C<.config_complete>; once nbdkit reaches C<.get_ready>, the plugin
+should assume that nbdkit may have closed the original stdin and
+stdout in order to become a daemon.
+
+ bool nbdkit_stdio_safe (void);
+
+As an example, the L<nbdkit-sh-plugin(3)> uses this function to
+determine whether it is safe to support C<script=-> to read a script
+from stdin.
+
 =head2 Reading passwords

 The C<nbdkit_read_password> utility function can be used to read
@@ -1180,7 +1201,7 @@ used directly on the command line, eg:
  nbdkit myplugin password=mostsecret

 But more securely this function can also read a password
-interactively:
+interactively (if C<nbdkit_stdio_safe> returns true):

  nbdkit myplugin password=-

diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod
index ffd0310f..20a2b785 100644
--- a/plugins/sh/nbdkit-sh-plugin.pod
+++ b/plugins/sh/nbdkit-sh-plugin.pod
@@ -48,7 +48,9 @@ as the name of the script, like this:
  EOF

 By default the inline script runs under F</bin/sh>.  You can add a
-shebang (C<#!>) to use other scripting languages.
+shebang (C<#!>) to use other scripting languages.  Of course, reading
+an inline script from stdin is incompatible with the C<-s>
+(C<--single>) mode of nbdkit that connects a client on stdin.

 =head1 WRITING AN NBDKIT SH PLUGIN

diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
index 9d1d89d0..9b10500a 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -38,6 +38,7 @@
 #endif

 #include <stdarg.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <errno.h>
 #include <sys/socket.h>
@@ -106,6 +107,7 @@ extern int nbdkit_parse_int64_t (const char *what, const char *str,
                                  int64_t *r);
 extern int nbdkit_parse_uint64_t (const char *what, const char *str,
                                   uint64_t *r);
+extern bool nbdkit_stdio_safe (void);
 extern int nbdkit_read_password (const char *value, char **password);
 extern char *nbdkit_realpath (const char *path);
 extern int nbdkit_nanosleep (unsigned sec, unsigned nsec);
diff --git a/server/main.c b/server/main.c
index 62598b81..748122fc 100644
--- a/server/main.c
+++ b/server/main.c
@@ -529,12 +529,13 @@ main (int argc, char *argv[])
       (port && listen_stdin) ||
       (unixsocket && listen_stdin) ||
       (listen_stdin && run) ||
+      (listen_stdin && dump_plugin) ||
       (vsock && unixsocket) ||
       (vsock && listen_stdin) ||
       (vsock && run)) {
     fprintf (stderr,
-             "%s: -p, --run, -s, -U or --vsock options cannot be used "
-             "in this combination\n",
+             "%s: --dump-plugin, -p, --run, -s, -U or --vsock options "
+             "cannot be used in this combination\n",
              program_name);
     exit (EXIT_FAILURE);
   }
diff --git a/server/nbdkit.syms b/server/nbdkit.syms
index 111223f2..20c390a9 100644
--- a/server/nbdkit.syms
+++ b/server/nbdkit.syms
@@ -65,6 +65,7 @@
     nbdkit_realpath;
     nbdkit_set_error;
     nbdkit_shutdown;
+    nbdkit_stdio_safe;
     nbdkit_vdebug;
     nbdkit_verror;

diff --git a/server/public.c b/server/public.c
index 3fd11253..f19791bc 100644
--- a/server/public.c
+++ b/server/public.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -404,6 +404,13 @@ nbdkit_parse_bool (const char *str)
   return -1;
 }

+/* Return true if it is safe to read from stdin during configuration. */
+bool
+nbdkit_stdio_safe (void)
+{
+  return !listen_stdin;
+}
+
 /* Read a password from configuration value. */
 static int read_password_from_fd (const char *what, int fd, char **password);

@@ -419,6 +426,11 @@ nbdkit_read_password (const char *value, char **password)

   /* Read from stdin. */
   if (strcmp (value, "-") == 0) {
+    if (!nbdkit_stdio_safe ()) {
+      nbdkit_error ("stdin is not available for reading password");
+      return -1;
+    }
+
     printf ("password: ");

     /* Set no echo. */
@@ -455,6 +467,10 @@ nbdkit_read_password (const char *value, char **password)

     if (nbdkit_parse_int ("password file descriptor", &value[1], &fd) == -1)
       return -1;
+    if (!nbdkit_stdio_safe () && fd < STDERR_FILENO) {
+      nbdkit_error ("stdin/out is not available for reading password");
+      return -1;
+    }
     if (read_password_from_fd (&value[1], fd, password) == -1)
       return -1;
   }
diff --git a/server/test-public.c b/server/test-public.c
index fe347d44..d4903420 100644
--- a/server/test-public.c
+++ b/server/test-public.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2018-2019 Red Hat Inc.
+ * Copyright (C) 2018-2020 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -53,6 +53,8 @@ nbdkit_error (const char *fs, ...)
   error_flagged = true;
 }

+bool listen_stdin;
+
 volatile int quit;
 int quit_fd = -1;

@@ -427,7 +429,24 @@ test_nbdkit_read_password (void)
     pass = false;
   }

-  /* XXX Testing reading from stdin would require setting up a pty */
+  /* XXX Testing reading from stdin would require setting up a pty. But
+   * we can test that it is forbidden with -s.
+   */
+  listen_stdin = true;
+  if (nbdkit_read_password ("-", &pw) != -1) {
+    fprintf (stderr, "Failed to diagnose failed password from stdin with -s\n");
+    pass = false;
+  }
+  else if (pw != NULL) {
+    fprintf (stderr, "Failed to set password to NULL on failure\n");
+    pass = false;
+  }
+  else if (!error_flagged) {
+    fprintf (stderr, "Wrong error message handling\n");
+    pass = false;
+  }
+  error_flagged = false;
+
   return pass;
 }

diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c
index 736b8ef0..c8a321f1 100644
--- a/plugins/sh/sh.c
+++ b/plugins/sh/sh.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2018-2019 Red Hat Inc.
+ * Copyright (C) 2018-2020 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -116,6 +116,11 @@ inline_script (void)
   char *filename = NULL;
   CLEANUP_FREE char *cmd = NULL;

+  if (!nbdkit_stdio_safe ()) {
+    nbdkit_error ("inline script is incompatible with -s");
+    return NULL;
+  }
+
   if (asprintf (&filename, "%s/%s", tmpdir, scriptname) == -1) {
     nbdkit_error ("asprintf: %m");
     goto err;
-- 
2.26.0.rc2




More information about the Libguestfs mailing list