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

[Eric Blake]

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.

> +  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