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