5:02 p.m.
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(a)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