10:03 a.m.
Changes from [v1]:
* most patches have been pushed;
* reduce quoting for most users by only using it when it's
actually necessary to do so.
[v1] https://listman.redhat.com/archives/libvir-list/2022-February/msg00477.html
Andrea Bolognani (6):
virnetclient: Improve spacing of ssh script
virnetclient: Use 'if' consistently
virnetclient: Handle netcatPath presence all at once
virnetclient: Don't unnecessarily quote user-provided values
virnetsockettest: Test more socket paths
virnetclient: Escape socket path
src/rpc/virnetclient.c | 78 +++++++++++++++++++------------
tests/virnetsockettest.c | 99 ++++++++++++++++++++--------------------
2 files changed, 99 insertions(+), 78 deletions(-)
--
2.35.1
10:03 a.m.
New subject: [libvirt PATCH v2 1/6] virnetclient: Improve spacing of ssh script
This results in the generated script having consistent spacing
throughout, instead of having repeated whitespace in some parts
and commands that are separated by a semicolon and no spacing at
all in others.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/rpc/virnetclient.c | 12 ++++----
tests/virnetsockettest.c | 60 ++++++++++++++++++++--------------------
2 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 2998551cd8..cbefa8f11f 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -436,10 +436,10 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
nccmd = g_strdup_printf(
"if '%s' -q 2>&1 | grep \"requires an argument\"
>/dev/null 2>&1; then "
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'%s' $ARG -U %s",
netcatPathSafe, netcatPathSafe, socketPath);
@@ -451,9 +451,9 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
case VIR_NET_CLIENT_PROXY_AUTO:
return g_strdup_printf("sh -c 'which virt-ssh-helper 1>/dev/null
2>&1; "
"if test $? = 0; then "
- " %s; "
- "else"
- " %s; "
+ "%s; "
+ "else "
+ "%s; "
"fi'", helpercmd, nccmd);
case VIR_NET_CLIENT_PROXY_NETCAT:
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index 4e8edb05c3..1bd5255d90 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -572,10 +572,10 @@ mymain(void)
.netcat = "nc",
.expectOut = "-T -e none -- somehost sh -c '"
"if 'nc' -q 2>&1 | grep \"requires an
argument\" >/dev/null 2>&1; then "
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'nc' $ARG -U /tmp/socket"
"'\n",
};
@@ -592,10 +592,10 @@ mymain(void)
.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 "
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'netcat' $ARG -U /tmp/socket"
"'\n",
};
@@ -612,10 +612,10 @@ mymain(void)
.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 "
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'netcat' $ARG -U /tmp/socket"
"'\n",
};
@@ -636,10 +636,10 @@ mymain(void)
.netcat = "nc",
.expectOut = "-T -e none -- crashyhost sh -c '"
"if 'nc' -q 2>&1 | grep \"requires an
argument\" >/dev/null 2>&1; then "
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'nc' $ARG -U /tmp/socket"
"'\n",
.dieEarly = true,
@@ -655,10 +655,10 @@ mymain(void)
.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 "
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'nc' $ARG -U /tmp/socket"
"'\n",
};
@@ -671,10 +671,10 @@ mymain(void)
.path = "/tmp/socket",
.expectOut = "-T -e none -- somehost sh -c '"
"if '''\\''n
c'\\'''' -q 2>&1 | grep \"requires an argument\"
>/dev/null 2>&1; then "
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'''\\''n c'\\''''
$ARG -U /tmp/socket"
"'\n",
};
@@ -687,10 +687,10 @@ mymain(void)
.path = "/tmp/socket",
.expectOut = "-T -e none -- somehost sh -c '"
"if
'''\\''n'\\''\\'\\'''\\''c'\\''''
-q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then
"
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'''\\''n'\\''\\'\\'''\\''c'\\''''
$ARG -U /tmp/socket"
"'\n",
};
@@ -703,10 +703,10 @@ mymain(void)
.path = "/tmp/socket",
.expectOut = "-T -e none -- somehost sh -c '"
"if
'''\\''n\"c'\\'''' -q 2>&1 | grep
\"requires an argument\" >/dev/null 2>&1; then "
- "ARG=-q0;"
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'''\\''n\"c'\\'''' $ARG -U
/tmp/socket"
"'\n",
};
@@ -719,13 +719,13 @@ mymain(void)
.expectOut = "-T -e none -- somehost sh -c '"
"which virt-ssh-helper 1>/dev/null 2>&1; "
"if test $? = 0; then "
- " virt-ssh-helper -r 'qemu:///session'; "
- "else"
- " if 'nc' -q 2>&1 | grep \"requires
an argument\" >/dev/null 2>&1; then "
- "ARG=-q0;"
+ "virt-ssh-helper -r 'qemu:///session'; "
+ "else "
+ "if 'nc' -q 2>&1 | grep \"requires
an argument\" >/dev/null 2>&1; then "
+ "ARG=-q0; "
"else "
- "ARG=;"
- "fi;"
+ "ARG=; "
+ "fi; "
"'nc' $ARG -U /tmp/socket; "
"fi"
"'\n"
--
2.35.1
10:03 a.m.
New subject: [libvirt PATCH v2 2/6] virnetclient: Use 'if' consistently
This makes the generated script a bit shorter and removes an
unnecessary call to test.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/rpc/virnetclient.c | 3 +--
tests/virnetsockettest.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index cbefa8f11f..7e7e9d52a6 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -449,8 +449,7 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
switch (proxy) {
case VIR_NET_CLIENT_PROXY_AUTO:
- return g_strdup_printf("sh -c 'which virt-ssh-helper 1>/dev/null
2>&1; "
- "if test $? = 0; then "
+ return g_strdup_printf("sh -c 'if which virt-ssh-helper >/dev/null
2>&1; then "
"%s; "
"else "
"%s; "
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index 1bd5255d90..adff6a0f9e 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -717,8 +717,7 @@ mymain(void)
.nodename = "somehost",
.path = "/tmp/socket",
.expectOut = "-T -e none -- somehost sh -c '"
- "which virt-ssh-helper 1>/dev/null 2>&1; "
- "if test $? = 0; then "
+ "if which virt-ssh-helper >/dev/null 2>&1; then
"
"virt-ssh-helper -r 'qemu:///session'; "
"else "
"if 'nc' -q 2>&1 | grep \"requires
an argument\" >/dev/null 2>&1; then "
--
2.35.1
10:03 a.m.
New subject: [libvirt PATCH v2 3/6] virnetclient: Handle netcatPath presence all at once
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/rpc/virnetclient.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 7e7e9d52a6..9c7047c7f8 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -425,13 +425,17 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
g_autofree char *nccmd = NULL;
g_autofree char *helpercmd = NULL;
- /* If user gave a 'netcat' path in the URI, we must
- * assume they want the legacy 'nc' based proxy, not
- * our new virt-ssh-helper
- */
- if (proxy == VIR_NET_CLIENT_PROXY_AUTO &&
- netcatPath != NULL) {
- proxy = VIR_NET_CLIENT_PROXY_NETCAT;
+ if (netcatPath) {
+ if (proxy == VIR_NET_CLIENT_PROXY_AUTO) {
+ /* If user gave a 'netcat' path in the URI, we must
+ * assume they want the legacy 'nc' based proxy, not
+ * our new virt-ssh-helper */
+ proxy = VIR_NET_CLIENT_PROXY_NETCAT;
+ } else if (proxy == VIR_NET_CLIENT_PROXY_NATIVE) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("netcat path not valid with native proxy mode"));
+ return NULL;
+ }
}
nccmd = g_strdup_printf(
@@ -459,11 +463,6 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
return g_strdup_printf("sh -c '%s'", nccmd);
case VIR_NET_CLIENT_PROXY_NATIVE:
- if (netcatPath) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("netcat path not valid with native proxy mode"));
- return NULL;
- }
return g_strdup_printf("sh -c '%s'", helpercmd);
case VIR_NET_CLIENT_PROXY_LAST:
--
2.35.1
10:03 a.m.
New subject: [libvirt PATCH v2 4/6] virnetclient: Don't unnecessarily quote user-provided values
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(a)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
10:03 a.m.
New subject: [libvirt PATCH v2 5/6] virnetsockettest: Test more socket paths
While unlikely, the socket path might also contain unusual
characters that need special handling. Adjust test cases so that
these scenarios are covered.
Note that none of these inputs will actually work outside of the
test suite at the moment, but we're going to address that in the
next commit.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
tests/virnetsockettest.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index 09c3ba13ad..ef7b5c6f59 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -668,14 +668,14 @@ mymain(void)
struct testSSHData sshData7 = {
.nodename = "somehost",
.netcat = "n c",
- .path = "/tmp/socket",
+ .path = "/tmp/sock et",
.expectOut = "-T -e none -- somehost sh -c '"
"if '''\\''n
c'\\'''' -q 2>&1 | grep \"requires an argument\"
>/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
- "'''\\''n c'\\''''
$ARG -U /tmp/socket"
+ "'''\\''n c'\\''''
$ARG -U /tmp/sock et"
"'\n",
};
if (virTestRun("SSH test 7", testSocketSSH, &sshData7) < 0)
@@ -684,14 +684,14 @@ mymain(void)
struct testSSHData sshData8 = {
.nodename = "somehost",
.netcat = "n'c",
- .path = "/tmp/socket",
+ .path = "/tmp/sock'et",
.expectOut = "-T -e none -- somehost sh -c '"
"if
'''\\''n'\\''\\'\\'''\\''c'\\''''
-q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then
"
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
-
"'''\\''n'\\''\\'\\'''\\''c'\\''''
$ARG -U /tmp/socket"
+
"'''\\''n'\\''\\'\\'''\\''c'\\''''
$ARG -U /tmp/sock'et"
"'\n",
};
if (virTestRun("SSH test 8", testSocketSSH, &sshData8) < 0)
@@ -700,14 +700,14 @@ mymain(void)
struct testSSHData sshData9 = {
.nodename = "somehost",
.netcat = "n\"c",
- .path = "/tmp/socket",
+ .path = "/tmp/sock\"et",
.expectOut = "-T -e none -- somehost sh -c '"
"if
'''\\''n\"c'\\'''' -q 2>&1 | grep
\"requires an argument\" >/dev/null 2>&1; then "
"ARG=-q0; "
"else "
"ARG=; "
"fi; "
-
"'''\\''n\"c'\\'''' $ARG -U
/tmp/socket"
+
"'''\\''n\"c'\\'''' $ARG -U
/tmp/sock\"et"
"'\n",
};
if (virTestRun("SSH test 9", testSocketSSH, &sshData9) < 0)
--
2.35.1
10:03 a.m.
New subject: [libvirt PATCH v2 6/6] virnetclient: Escape socket path
Just like the name of the netcat command and the connection URI,
the socket path is a user-provided piece of information that
might contain characters that have special meaning for the
shell, and as such should be escaped.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/rpc/virnetclient.c | 10 ++++++++--
tests/virnetsockettest.c | 6 +++---
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index cd92af1669..2d31429161 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -422,10 +422,12 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
{
g_autofree char *netcatPathSafe = NULL;
g_autofree char *driverURISafe = NULL;
+ g_autofree char *socketPathSafe = NULL;
g_autofree char *nccmd = NULL;
g_autofree char *helpercmd = NULL;
const char *netcatPathQuotes = "";
const char *driverURIQuotes = "";
+ const char *socketPathQuotes = "";
if (netcatPath) {
if (proxy == VIR_NET_CLIENT_PROXY_AUTO) {
@@ -453,6 +455,10 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
if (STRNEQ(driverURISafe, driverURI)) {
driverURIQuotes = "'";
}
+ socketPathSafe = virNetClientDoubleEscapeShell(socketPath);
+ if (STRNEQ_NULLABLE(socketPathSafe, socketPath)) {
+ socketPathQuotes = "'";
+ }
nccmd = g_strdup_printf(
"if %s%s%s -q 2>&1 | grep \"requires an argument\"
>/dev/null 2>&1; then "
@@ -460,10 +466,10 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
"else "
"ARG=; "
"fi; "
- "%s%s%s $ARG -U %s",
+ "%s%s%s $ARG -U %s%s%s",
netcatPathQuotes, netcatPathSafe, netcatPathQuotes,
netcatPathQuotes, netcatPathSafe, netcatPathQuotes,
- socketPath);
+ socketPathQuotes, socketPathSafe, socketPathQuotes);
helpercmd = g_strdup_printf("virt-ssh-helper%s %s%s%s",
readonly ? " -r" : "",
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index ef7b5c6f59..ff75de5a85 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -675,7 +675,7 @@ mymain(void)
"else "
"ARG=; "
"fi; "
- "'''\\''n c'\\''''
$ARG -U /tmp/sock et"
+ "'''\\''n c'\\''''
$ARG -U '''\\''/tmp/sock et'\\''''"
"'\n",
};
if (virTestRun("SSH test 7", testSocketSSH, &sshData7) < 0)
@@ -691,7 +691,7 @@ mymain(void)
"else "
"ARG=; "
"fi; "
-
"'''\\''n'\\''\\'\\'''\\''c'\\''''
$ARG -U /tmp/sock'et"
+
"'''\\''n'\\''\\'\\'''\\''c'\\''''
$ARG -U
'''\\''/tmp/sock'\\''\\'\\'''\\''et'\\''''"
"'\n",
};
if (virTestRun("SSH test 8", testSocketSSH, &sshData8) < 0)
@@ -707,7 +707,7 @@ mymain(void)
"else "
"ARG=; "
"fi; "
-
"'''\\''n\"c'\\'''' $ARG -U
/tmp/sock\"et"
+
"'''\\''n\"c'\\'''' $ARG -U
'''\\''/tmp/sock\"et'\\''''"
"'\n",
};
if (virTestRun("SSH test 9", testSocketSSH, &sshData9) < 0)
--
2.35.1
10:59 a.m.
On Mon, Feb 14, 2022 at 05:03:29PM +0100, Andrea Bolognani wrote:
Changes from [v1]:
* most patches have been pushed;
* reduce quoting for most users by only using it when it's
actually necessary to do so.
This still has the back compatibility problems I mentioned
previously and has introduced a further one by reducing
quoting. I'm not in favour of knowingly introducing back
compatibility problems when it doesn't solve functional
problems on the libvirt side.
If we want to support whitespace in the socket path, it
is possible todo that in a back compatible manner, by only
changing our quoting of the socket path when we know it has
whitespace, and not touching anything else.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1004
days inactive
1004
days old
7 comments
2 participants
participants (2)
-
Andrea Bolognani -
Daniel P. Berrangé