5:31 a.m.
This command fails with an incorrect error message:
$ nbdkit ssh host=localhost /nosuchfile password=- --run 'qemu-img info $nbd'
</dev/null
password:
nbdkit: error: could not read password from stdin: Inappropriate ioctl for device
The error (ENOTTY Inappropriate ioctl for device) is actually a
leftover errno from the previous isatty call. This happens because
getline(3) can return -1 both for error and EOF. In the EOF case it
does not set errno so we get the previous random value.
Also:
$ echo -n '' | nbdkit ssh host=localhost /nosuchfile password=- --run
'qemu-img info $nbd'
password:
nbdkit: error: could not read password from stdin: Inappropriate ioctl for device
$ echo -n '1' | nbdkit ssh host=localhost /nosuchfile password=- --run
'qemu-img info $nbd'
password:
[password is read with no error]
This raises the question of what password=- actually means. It's
documented as "read a password interactively", with the word
"interactively" going back to at least nbdkit 1.2, and therefore I
think we should reject attempts to use password=- from non-ttys.
Since at least nbdkit 1.2 we have allowed passwords to be read from
files (password=+FILENAME), and since nbdkit 1.16 you can read
passwords from arbitrary file descriptors (password=-FD).
Another justification for the interactive-only nature of password=- is
that it prints a “password: ” prompt.
So I believe it is fair to ban password=- unless the input is a tty.
This commit also fixes the error message by handling the case where
getline returns -1 without setting errno.
After this change:
$ ./nbdkit ssh host=localhost /nosuchfile password=- --run 'qemu-img info $nbd'
password: <--- press ^D
nbdkit: error: could not read password from stdin: end of file
$ echo -n '' | ./nbdkit ssh host=localhost /nosuchfile password=- --run
'qemu-img info $nbd'
nbdkit: error: stdin is not a tty, cannot read password interactively
$ echo -n '1' | ./nbdkit ssh host=localhost /nosuchfile password=- --run
'qemu-img info $nbd'
nbdkit: error: stdin is not a tty, cannot read password interactively
$ ./nbdkit ssh host=localhost /nosuchfile password=- --run 'qemu-img info $nbd'
password: <--- press return key
[zero length password is read]
Thanks: Ming Xie, Pino Toscano.
---
docs/nbdkit-plugin.pod | 8 ++-
server/public.c | 107 ++++++++++++++++++++++++++---------------
2 files changed, 74 insertions(+), 41 deletions(-)
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index f5e6dd01..612688ab 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -1242,8 +1242,12 @@ or from a file descriptor inherited by nbdkit:
nbdkit myplugin password=-99
-(If the password begins with a C<-> or C<+> character then it must be
-passed in a file).
+=head3 Notes on reading passwords
+
+If the password begins with a C<-> or C<+> character then it must be
+passed in a file.
+
+C<password=-> can only be used when stdin is a terminal.
=head2 Safely interacting with stdin and stdout
diff --git a/server/public.c b/server/public.c
index bcf1a3a2..dafdfbae 100644
--- a/server/public.c
+++ b/server/public.c
@@ -413,53 +413,18 @@ nbdkit_stdio_safe (void)
}
/* Read a password from configuration value. */
+static int read_password_interactive (char **password);
static int read_password_from_fd (const char *what, int fd, char **password);
int
nbdkit_read_password (const char *value, char **password)
{
- int tty, err;
- struct termios orig, temp;
- ssize_t r;
- size_t n;
-
*password = NULL;
- /* Read from stdin. */
+ /* Read from stdin interactively. */
if (strcmp (value, "-") == 0) {
- if (!nbdkit_stdio_safe ()) {
- nbdkit_error ("stdin is not available for reading password");
+ if (read_password_interactive (password) == -1)
return -1;
- }
-
- printf ("password: ");
-
- /* Set no echo. */
- tty = isatty (0);
- if (tty) {
- tcgetattr (0, &orig);
- temp = orig;
- temp.c_lflag &= ~ECHO;
- tcsetattr (0, TCSAFLUSH, &temp);
- }
-
- r = getline (password, &n, stdin);
- err = errno;
-
- /* Restore echo. */
- if (tty)
- tcsetattr (0, TCSAFLUSH, &orig);
-
- /* Complete the printf above. */
- printf ("\n");
-
- if (r == -1) {
- errno = err;
- nbdkit_error ("could not read password from stdin: %m");
- return -1;
- }
- if (*password && r > 0 && (*password)[r-1] == '\n')
- (*password)[r-1] = '\0';
}
/* Read from numbered file descriptor. */
@@ -501,6 +466,62 @@ nbdkit_read_password (const char *value, char **password)
return 0;
}
+static int
+read_password_interactive (char **password)
+{
+ int err;
+ struct termios orig, temp;
+ ssize_t r;
+ size_t n;
+
+ if (!nbdkit_stdio_safe ()) {
+ nbdkit_error ("stdin is not available for reading password");
+ return -1;
+ }
+
+ if (!isatty (0)) {
+ nbdkit_error ("stdin is not a tty, cannot read password interactively");
+ return -1;
+ }
+
+ printf ("password: ");
+
+ /* Set no echo. This is best-effort, ignore errors. */
+ tcgetattr (0, &orig);
+ temp = orig;
+ temp.c_lflag &= ~ECHO;
+ tcsetattr (0, TCSAFLUSH, &temp);
+
+ /* To distinguish between error and EOF we have to check errno.
+ * getline can return -1 and errno = 0 which means we got end of
+ * file.
+ */
+ errno = 0;
+ r = getline (password, &n, stdin);
+ err = errno;
+
+ /* Restore echo. */
+ tcsetattr (0, TCSAFLUSH, &orig);
+
+ /* Complete the printf above. */
+ printf ("\n");
+
+ if (r == -1) {
+ if (err == 0)
+ nbdkit_error ("could not read password from stdin: end of file");
+ else {
+ errno = err;
+ nbdkit_error ("could not read password from stdin: %m");
+ }
+ return -1;
+ }
+
+ if (*password && r > 0 && (*password)[r-1] == '\n')
+ (*password)[r-1] = '\0';
+
+ return 0;
+}
+
static int
read_password_from_fd (const char *what, int fd, char **password)
{
@@ -515,10 +536,18 @@ read_password_from_fd (const char *what, int fd, char **password)
close (fd);
return -1;
}
+
+ /* To distinguish between error and EOF we have to check errno.
+ * getline can return -1 and errno = 0 which means we got end of
+ * file, which is simply a zero length password.
+ */
+ errno = 0;
r = getline (password, &n, fp);
err = errno;
+
fclose (fp);
- if (r == -1) {
+
+ if (r == -1 && err != 0) {
errno = err;
nbdkit_error ("could not read password from %s: %m", what);
return -1;
--
2.25.0