[Libguestfs] [PATCH nbdkit] server: Allow file descriptors to be passed to nbdkit_read_password.

[Richard W.M. Jones]

On Thu, Oct 17, 2019 at 04:50:20PM -0500, Eric Blake wrote:
> On 10/17/19 4:38 PM, Richard W.M. Jones wrote:
> >Allow password parameters such as ‘password=-FD’ where FD is a file
> >descriptor number inherited by nbdkit from the parent process.  This
> >is another way to allow programs to hand passwords to nbdkit in a very
> >secure way, for example over a pipe so they never touch the
> >filesystem.
> >
> >Previously nbdkit allowed you to use literal passwords on the command
> >line if they began with a ‘-’ (but were not just that single
> >character).  However that was contrary to the documentation, and this
> >commit now prevents that.
> >---
> 
> >+static int
> >+read_password_from_fd (const char *what, int fd, char **password)
> >+{
> >+  FILE *fp;
> >+  size_t n;
> >+  ssize_t r;
> >+  int err;
> >+
> >+  fp = fdopen (fd, "r");
> >+  if (fp == NULL) {
> >+    nbdkit_error ("fdopen %s: %m", what);
> >+    close (fd);
> >+    return -1;
> >+  }
> >+  r = getline (password, &n, fp);
> 
> This prevents a password from containing a newline.  Is that a
> problem? Can a password contain a literal newline when passed
> literally through the command line?  If so, that feels inconsistent.

I believe that's also a problem with the current code, as I simply
factored out this function from the existing code for "+file".  Anyone
who has a password containing a newline presumably also has a problem
logging in (to any reasonable server)?

Rich.

> >+  err = errno;
> >+  fclose (fp);
> >+  if (r == -1) {
> >+    errno = err;
> >+    nbdkit_error ("could not read password from %s: %m", what);
> >+    return -1;
> >+  }
> >+
> >+  if (*password && r > 0 && (*password)[r-1] == '\n')
> >+    (*password)[r-1] = '\0';
> >+
> >+  return 0;
> >+}
> >+
> 
> >+++ b/server/test-public.c
> >@@ -335,6 +335,8 @@ test_nbdkit_read_password (void)
> >  {
> >    bool pass = true;
> >    char template[] = "+/tmp/nbdkit_testpw_XXXXXX";
> >+  char template2[] = "/tmp/nbdkit_testpw2_XXXXXX";
> >+  char fdbuf[16];
> >    char *pw = template;
> >    int fd;
> >@@ -391,6 +393,35 @@ test_nbdkit_read_password (void)
> >      unlink (&template[1]);
> >    }
> >+  /* Test reading password from file descriptor. */
> >+  fd = mkstemp (template2);
> >+  if (fd < 0) {
> >+    perror ("mkstemp");
> >+    pass = false;
> >+  }
> >+  else if (write (fd, "abc\n", 4) != 4) {
> >+    fprintf (stderr, "Failed to write to file %s\n", template2);
> >+    pass = false;
> >+  }
> 
> But at least you test that newlines are stripped when using a file
> descriptor, so the addition seems self-consistent (even if we need
> more documentation on how newlines in passwords are handled).
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/