[Libguestfs] [PATCH nbdkit 5/5] vddk: Munge password parameters when we reexec (RHBZ#1842440).
Richard W.M. Jones
rjones at redhat.com
Tue Jun 2 12:27:06 UTC 2020
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
More information about the Libguestfs
mailing list