[Libguestfs] [PATCH nbdkit 5/5] vddk: Munge password parameters when we reexec (RHBZ#1842440).

Richard W.M. Jones
2020-06-02T12:27:06Z

See this thread:
https://www.redhat.com/archives/libguestfs/2020-June/thread.html#00012

This commit also adds a regression test of vddk password=- and
password=-FD.
---
 tests/Makefile.am                       |  4 ++
 plugins/vddk/vddk.h                     |  1 +
 plugins/vddk/reexec.c                   | 43 ++++++++++++-
 plugins/vddk/vddk.c                     |  2 +-
 tests/test-vddk-password-fd.sh          | 86 +++++++++++++++++++++++++
 tests/test-vddk-password-interactive.sh | 77 ++++++++++++++++++++++
 tests/dummy-vddk.c                      |  6 ++
 7 files changed, 217 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2b16122b..7cecf873 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -860,6 +860,8 @@ noinst_LTLIBRARIES += libvixDiskLib.la
 TESTS += \
 	test-vddk.sh \
 	test-vddk-dump-plugin.sh \
+	test-vddk-password-fd.sh \
+	test-vddk-password-interactive.sh \
 	test-vddk-real.sh \
 	test-vddk-real-dump-plugin.sh \
 	$(NULL)
@@ -881,6 +883,8 @@ endif HAVE_VDDK
 EXTRA_DIST += \
 	test-vddk.sh \
 	test-vddk-dump-plugin.sh \
+	test-vddk-password-fd.sh \
+	test-vddk-password-interactive.sh \
 	test-vddk-real.sh \
 	test-vddk-real-dump-plugin.sh \
 	$(NULL)
diff --git a/plugins/vddk/vddk.h b/plugins/vddk/vddk.h
index e229e55e..0f2d0050 100644
--- a/plugins/vddk/vddk.h
+++ b/plugins/vddk/vddk.h
@@ -34,6 +34,7 @@
 #define NBDKIT_VDDK_H

 extern char *libdir;
+extern char *password;
 extern char *reexeced;

 extern void reexec_if_needed (const char *prepend);
diff --git a/plugins/vddk/reexec.c b/plugins/vddk/reexec.c
index 9641ee8c..43d4e1b7 100644
--- a/plugins/vddk/reexec.c
+++ b/plugins/vddk/reexec.c
@@ -37,6 +37,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <sys/types.h>

 #define NBDKIT_API_VERSION 2
 #include <nbdkit-plugin.h>
@@ -122,7 +123,47 @@ perform_reexec (const char *env, const char *prepend)

   /* Split cmdline into argv, then append one more arg. */
   for (len = 0; len < buf.size; len += strlen (buf.ptr + len) + 1) {
-    if (string_vector_append (&argv, buf.ptr + len) == -1) {
+    char *arg = buf.ptr + len;  /* Next \0-terminated argument. */
+
+    /* password parameter requires special handling for reexec.  For
+     * password=- and password=-FD, after reexec we might try to
+     * reread these, but stdin has gone away and FD has been consumed
+     * already so that won't work.  Even password=+FILE is a little
+     * problematic since the file will be read twice, which may break
+     * for special files.
+     *
+     * However we may write the password to a temporary file and
+     * substitute password=-<FD> of the opened temporary file here.
+     * The trick is described by Eric Blake here:
+     * https://www.redhat.com/archives/libguestfs/2020-June/msg00021.html
+     *
+     * (RHBZ#1842440)
+     */
+    if (strncmp (arg, "password=", 9) == 0) {
+      char tmpfile[] = "/tmp/XXXXXX";
+      char *password_fd;
+
+      /* These operations should never fail, so exit on error. */
+      fd = mkstemp (tmpfile);
+      if (fd == -1) {
+        nbdkit_error ("mkstemp: %m");
+        exit (EXIT_FAILURE);
+      }
+      unlink (tmpfile);
+      if (write (fd, password, strlen (password)) != strlen (password)) {
+        nbdkit_error ("write: %m");
+        exit (EXIT_FAILURE);
+      }
+      lseek (fd, 0, SEEK_SET);
+      if (asprintf (&password_fd, "password=-%d", fd) == -1) {
+        nbdkit_error ("asprintf: %m");
+        exit (EXIT_FAILURE);
+      }
+      /* Leaked here but cleaned up when we exec below. */
+      arg = password_fd;
+    }
+
+    if (string_vector_append (&argv, arg) == -1) {
     argv_realloc_fail:
       nbdkit_debug ("argv: realloc: %m");
       return;
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index eb865cc2..f2696fa6 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -82,7 +82,7 @@ static const char *cookie;                 /* cookie */
 static const char *filename;               /* file */
 char *libdir;                              /* libdir */
 static uint16_t nfc_host_port;             /* nfchostport */
-static char *password;                     /* password */
+char *password;                            /* password */
 static uint16_t port;                      /* port */
 static const char *server_name;            /* server */
 static bool single_link;                   /* single-link */
diff --git a/tests/test-vddk-password-fd.sh b/tests/test-vddk-password-fd.sh
new file mode 100755
index 00000000..6386fa11
--- /dev/null
+++ b/tests/test-vddk-password-fd.sh
@@ -0,0 +1,86 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# password=-FD was broken in the VDDK plugin in nbdkit 1.18 through
+# 1.20.2.  This was because the reexec code caused
+# nbdkit_read_password to be called twice, second time with stdin
+# reopened on /dev/null.  Since we have now fixed this bug, this is a
+# regression test.
+
+source ./functions.sh
+set -e
+set -x
+
+# Setting LD_LIBRARY_PATH breaks valgrind.
+if [ "x$NBDKIT_VALGRIND" = "x1" ]; then
+    echo "$0: skipped under valgrind"
+    exit 77
+fi
+
+requires qemu-img --version
+
+f=test-vddk-password-fd.file
+out=test-vddk-password-fd.out
+cleanup_fn rm -f $f $out
+
+# Get dummy-vddk to print the password to stderr.
+export DUMMY_VDDK_PRINT_PASSWORD=1
+
+# Password -FD.
+echo 123 > $f
+exec 3< $f
+nbdkit -fv -U - vddk \
+       libdir=.libs \
+       server=noserver.example.com thumbprint=ab \
+       vm=novm /nofile \
+       user=root password=-3 \
+       --run 'qemu-img info $nbd' \
+       >&$out ||:
+exec 3<&-
+cat $out
+
+grep "password=123$" $out
+
+# Password -FD, zero length.
+: > $f
+exec 3< $f
+nbdkit -fv -U - vddk \
+       libdir=.libs \
+       server=noserver.example.com thumbprint=ab \
+       vm=novm /nofile \
+       user=root password=-3 \
+       --run 'qemu-img info $nbd' \
+       >&$out ||:
+exec 3<&-
+cat $out
+
+grep "password=$" $out
diff --git a/tests/test-vddk-password-interactive.sh
b/tests/test-vddk-password-interactive.sh
new file mode 100755
index 00000000..b7e0af26
--- /dev/null
+++ b/tests/test-vddk-password-interactive.sh
@@ -0,0 +1,77 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# password=- was broken in the VDDK plugin in nbdkit 1.18 through
+# 1.20.2.  This was because the reexec code caused
+# nbdkit_read_password to be called twice, second time with stdin
+# reopened on /dev/null.  Since we have now fixed this bug, this is a
+# regression test.
+
+source ./functions.sh
+set -e
+set -x
+
+# The tests break under valgrind for a couple of reasons:
+# (1) Setting LD_LIBRARY_PATH breaks valgrind.
+# (2) Valgrinded nbdkit hangs under expect.
+if [ "x$NBDKIT_VALGRIND" = "x1" ]; then
+    echo "$0: skipped under valgrind"
+    exit 77
+fi
+
+requires qemu-img --version
+requires expect -v
+
+out=test-vddk-password-interactive.out
+cleanup_fn rm -f $out
+
+# Get dummy-vddk to print the password to stderr.
+export DUMMY_VDDK_PRINT_PASSWORD=1
+
+export out
+expect -f - <<'EOF'
+  spawn sh -c "
+    nbdkit -fv -U - \
+           vddk \
+           libdir=.libs \
+           server=noserver.example.com thumbprint=ab \
+           vm=novm /nofile \
+           user=root password=- \
+           --run 'qemu-img info \$nbd' \
+           2>$env(out)"
+  expect "ssword:"
+  send "abc\r"
+  wait
+EOF
+cat $out
+
+grep "password=abc$" $out
diff --git a/tests/dummy-vddk.c b/tests/dummy-vddk.c
index 13be492d..a2377944 100644
--- a/tests/dummy-vddk.c
+++ b/tests/dummy-vddk.c
@@ -91,6 +91,12 @@ VixDiskLib_ConnectEx (const VixDiskLibConnectParams *params,
                       const char *transport_modes,
                       VixDiskLibConnection *connection)
 {
+  /* Used when regression testing password= parameter. */
+  if (getenv ("DUMMY_VDDK_PRINT_PASSWORD") &&
+      params->credType == VIXDISKLIB_CRED_UID &&
+      params->creds.uid.password != NULL)
+    fprintf (stderr, "dummy-vddk: password=%s\n",
params->creds.uid.password);
+
   return VIX_OK;
 }

--
2.25.0