server: Disallow password=- from non-tty and fix error message (RHBZ#…

…1842440).

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.  Since stdin is
/dev/null, isatty returns 0 and sets errno = ENOTTY.  The reason why
this error turns up in the error message is because getline(3) can
return -1 either for an error or for EOF.  In the EOF case it does not
set errno so we get the previous errno value which happens to be the
one from isatty.

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 OK]

All of this raises the question of what password=- actually means.

It's documented as "read a password interactively", with the word
"interactively" appearing in the documentation at least as far back as
nbdkit 1.2.  Also since at least 1.2 we have allowed passwords to be
read from files (password=+FILENAME), and since 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.  It doesn't try to suppress this
prompt if stdin is not a tty.

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.  Additionally we have to
deal with possible undefined behaviour in this case (see
https://stackoverflow.com/a/47067633).

After this change:

  $ ./nbdkit ssh host=localhost /nosuchfile password=- --run 'qemu-img info $nbd'
  password:      <--- press return key
  [zero length password is read]

  $ ./nbdkit ssh host=localhost /nosuchfile password=- --run 'qemu-img info $nbd'
  password:      <--- press ^D
  [zero length password is read]

  $ 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

Thanks: Ming Xie, Pino Toscano, Eric Blake.

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
 

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");
-      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");
+    if (read_password_interactive (password) == -1)
       return -1;
-    }
-    if (*password && r > 0 && (*password)[r-1] == '\n')
-      (*password)[r-1] = '\0';
   }
 
   /* Read from numbered file descriptor. */
@@ -501,6 +466,68 @@ 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 (STDIN_FILENO)) {
+    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 (STDIN_FILENO, &orig);
+  temp = orig;
+  temp.c_lflag &= ~ECHO;
+  tcsetattr (STDIN_FILENO, 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, which is simply a zero length password.
+   */
+  errno = 0;
+  r = getline (password, &n, stdin);
+  err = errno;
+
+  /* Restore echo. */
+  tcsetattr (STDIN_FILENO, TCSAFLUSH, &orig);
+
+  /* Complete the printf above. */
+  printf ("\n");
+
+  if (r == -1 && err != 0) {
+    if (err == 0) {             /* EOF, not an error. */
+      free (*password);         /* State of linebuf is undefined. */
+      *password = strdup ("");
+      if (*password == NULL) {
+        nbdkit_error ("strdup: %m");
+        return -1;
+      }
+    }
+    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,13 +542,31 @@ 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) {
-    errno = err;
-    nbdkit_error ("could not read password from %s: %m", what);
-    return -1;
+    if (err == 0) {             /* EOF, not an error. */
+      free (*password);         /* State of linebuf is undefined. */
+      *password = strdup ("");
+      if (*password == NULL) {
+        nbdkit_error ("strdup: %m");
+        return -1;
+      }
+    }
+    else {
+      errno = err;
+      nbdkit_error ("could not read password from %s: %m", what);
+      return -1;
+    }
   }
 
   if (*password && r > 0 && (*password)[r-1] == '\n')