On 6/1/20 5:31 AM, Richard W.M. Jones wrote:
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.
Makes sense.
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.
I agree with that decision.
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(-)
+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)) {
Could spell it STDIN_FILENO if desired, but that's trivial.
+ /* 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");
Is this actually an error? EOF on a tty merely means that the password
is the empty string (perhaps because the user intentionally pressed ^D
instead of enter to stop further input)...
+ 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';
...especially since you already allow an interactive user to pass an
empty password by pressing solely enter.
The rest of the patch looks fine; I'll leave it up to you what you want
to do about handling an empty password interactively.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs
