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

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