[libvirt PATCH v2 4/6] virnetclient: Don't unnecessarily quote user-provided values
Andrea Bolognani
abologna at redhat.com
Mon Feb 14 16:03:33 UTC 2022
If the output of virNetClientDoubleEscapeShell() matches its
input, then no escaping actually happened and quoting the value
in the generated script is unnecessary.
With this change, awkward use of quotes such as
sh -c 'if 'nc' -q'
is completely gone when using the default settings.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/273
Signed-off-by: Andrea Bolognani <abologna at redhat.com>
---
src/rpc/virnetclient.c | 36 +++++++++++++++++++++++++++---------
tests/virnetsockettest.c | 28 ++++++++++++++--------------
2 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 9c7047c7f8..cd92af1669 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -420,10 +420,12 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
const char *driverURI,
bool readonly)
{
- g_autofree char *netcatPathSafe = virNetClientDoubleEscapeShell(netcatPath ? netcatPath : "nc");
- g_autofree char *driverURISafe = virNetClientDoubleEscapeShell(driverURI);
+ g_autofree char *netcatPathSafe = NULL;
+ g_autofree char *driverURISafe = NULL;
g_autofree char *nccmd = NULL;
g_autofree char *helpercmd = NULL;
+ const char *netcatPathQuotes = "";
+ const char *driverURIQuotes = "";
if (netcatPath) {
if (proxy == VIR_NET_CLIENT_PROXY_AUTO) {
@@ -436,20 +438,36 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
_("netcat path not valid with native proxy mode"));
return NULL;
}
+ } else {
+ netcatPath = "nc";
+ }
+
+ /* Escape user-provided values so that they're safe for use as part
+ * of our generated shell snippet. If escaping was necessary, we
+ * will also need to add quotes around all uses of each value */
+ netcatPathSafe = virNetClientDoubleEscapeShell(netcatPath);
+ if (STRNEQ(netcatPathSafe, netcatPath)) {
+ netcatPathQuotes = "'";
+ }
+ driverURISafe = virNetClientDoubleEscapeShell(driverURI);
+ if (STRNEQ(driverURISafe, driverURI)) {
+ driverURIQuotes = "'";
}
nccmd = g_strdup_printf(
- "if '%s' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
+ "if %s%s%s -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
- "'%s' $ARG -U %s",
- netcatPathSafe, netcatPathSafe, socketPath);
-
- helpercmd = g_strdup_printf("virt-ssh-helper%s'%s'",
- readonly ? " -r " : " ",
- driverURISafe);
+ "%s%s%s $ARG -U %s",
+ netcatPathQuotes, netcatPathSafe, netcatPathQuotes,
+ netcatPathQuotes, netcatPathSafe, netcatPathQuotes,
+ socketPath);
+
+ helpercmd = g_strdup_printf("virt-ssh-helper%s %s%s%s",
+ readonly ? " -r" : "",
+ driverURIQuotes, driverURISafe, driverURIQuotes);
switch (proxy) {
case VIR_NET_CLIENT_PROXY_AUTO:
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index adff6a0f9e..09c3ba13ad 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -571,12 +571,12 @@ mymain(void)
.path = "/tmp/socket",
.netcat = "nc",
.expectOut = "-T -e none -- somehost sh -c '"
- "if 'nc' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
+ "if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
- "'nc' $ARG -U /tmp/socket"
+ "nc $ARG -U /tmp/socket"
"'\n",
};
if (virTestRun("SSH test 1", testSocketSSH, &sshData1) < 0)
@@ -591,12 +591,12 @@ mymain(void)
.noVerify = false,
.path = "/tmp/socket",
.expectOut = "-p 9000 -l fred -T -e none -o BatchMode=yes -- somehost sh -c '"
- "if 'netcat' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
+ "if netcat -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
- "'netcat' $ARG -U /tmp/socket"
+ "netcat $ARG -U /tmp/socket"
"'\n",
};
if (virTestRun("SSH test 2", testSocketSSH, &sshData2) < 0)
@@ -611,12 +611,12 @@ mymain(void)
.noVerify = true,
.path = "/tmp/socket",
.expectOut = "-p 9000 -l fred -T -e none -o StrictHostKeyChecking=no -- somehost sh -c '"
- "if 'netcat' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
+ "if netcat -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
- "'netcat' $ARG -U /tmp/socket"
+ "netcat $ARG -U /tmp/socket"
"'\n",
};
if (virTestRun("SSH test 3", testSocketSSH, &sshData3) < 0)
@@ -635,12 +635,12 @@ mymain(void)
.path = "/tmp/socket",
.netcat = "nc",
.expectOut = "-T -e none -- crashyhost sh -c '"
- "if 'nc' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
+ "if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
- "'nc' $ARG -U /tmp/socket"
+ "nc $ARG -U /tmp/socket"
"'\n",
.dieEarly = true,
};
@@ -654,12 +654,12 @@ mymain(void)
.keyfile = "/root/.ssh/example_key",
.noVerify = true,
.expectOut = "-i /root/.ssh/example_key -T -e none -o StrictHostKeyChecking=no -- example.com sh -c '"
- "if 'nc' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
+ "if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
- "'nc' $ARG -U /tmp/socket"
+ "nc $ARG -U /tmp/socket"
"'\n",
};
if (virTestRun("SSH test 6", testSocketSSH, &sshData6) < 0)
@@ -718,14 +718,14 @@ mymain(void)
.path = "/tmp/socket",
.expectOut = "-T -e none -- somehost sh -c '"
"if which virt-ssh-helper >/dev/null 2>&1; then "
- "virt-ssh-helper -r 'qemu:///session'; "
+ "virt-ssh-helper -r qemu:///session; "
"else "
- "if 'nc' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
+ "if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
- "'nc' $ARG -U /tmp/socket; "
+ "nc $ARG -U /tmp/socket; "
"fi"
"'\n"
};
@@ -736,7 +736,7 @@ mymain(void)
.nodename = "somehost",
.proxy = VIR_NET_CLIENT_PROXY_NATIVE,
.expectOut = "-T -e none -- somehost sh -c '"
- "virt-ssh-helper -r 'qemu:///session'"
+ "virt-ssh-helper -r qemu:///session"
"'\n"
};
if (virTestRun("SSH test 11", testSocketSSH, &sshData11) < 0)
--
2.35.1
More information about the libvir-list
mailing list