From nobody Sat Feb 7 11:04:53 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 207.211.31.81 as permitted sender) client-ip=207.211.31.81; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.81 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1594319826; cv=none; d=zohomail.com; s=zohoarc; b=jvm9Shid2QHjjTUdgu/+/poTJ/0hIPtRgNJE0NdE6rIeN4KTCS4P0pzRt4XeRoqcks+DHq/OShJ0lJQ6wPFX6Q6wWB9QlmbEirTBfaTGNZ/yX5s8gTHQrj4qZAesFm1ZlzE2WGP4VfB3yZN75dvoE7y4vbQuSaycwueMlFv7Bec= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1594319826; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=W1f0+hkY7klQuVteXxhdFuGfgIZHA3YQxQWGZbTPaWA=; b=lGVMzwpk+GUBm56AF1bDQg5eN7QS0cMlu3ftl8iScaCCgmVbANA8ffpcTKzwJcxfAwFzblNCDxTRHuTTT1mtKJGAZR3JO7dXI60S789cSRNAaZ/GDesO0ijBW/UjmEge5DDqqVt7RJentbdRDuOprmKmEYrAs1FimnaQvXb5dfE= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.81 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by mx.zohomail.com with SMTPS id 1594319826471962.73275987648; Thu, 9 Jul 2020 11:37:06 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-487-OZnRGVDBPU-Zem6g_ntIoQ-1; Thu, 09 Jul 2020 14:37:02 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 1977D8005B0; Thu, 9 Jul 2020 18:36:56 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E978619D7C; Thu, 9 Jul 2020 18:36:55 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id B9C2A1809563; Thu, 9 Jul 2020 18:36:55 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 069Iarra023326 for ; Thu, 9 Jul 2020 14:36:53 -0400 Received: by smtp.corp.redhat.com (Postfix) id 70A8C60E1C; Thu, 9 Jul 2020 18:36:53 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.36.110.45]) by smtp.corp.redhat.com (Postfix) with ESMTP id 316A760F8D; Thu, 9 Jul 2020 18:36:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594319825; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=W1f0+hkY7klQuVteXxhdFuGfgIZHA3YQxQWGZbTPaWA=; b=gRpqDLK3ZjhFXifDpeUMKJCKdvorm5NT8Ye9JvSyonXwk79eN2DHzQniEoaAbSMI+Ne66J 4q9yOzCuCUdMkweFywWTSBgpsLvbHcs1WY1XQ0fcK1RlijlQ0i7xkIE5NEs3UQL9scJXNo X7x6qwCkuqCrhWxGlyyaxrmbqhZXnGQ= X-MC-Unique: OZnRGVDBPU-Zem6g_ntIoQ-1 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [libvirt PATCH 1/9] rpc: merge logic for generating remote SSH shell script Date: Thu, 9 Jul 2020 19:36:38 +0100 Message-Id: <20200709183646.4016586-2-berrange@redhat.com> In-Reply-To: <20200709183646.4016586-1-berrange@redhat.com> References: <20200709183646.4016586-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Three parts of the code all build up the same SSH shell script snippet for remote tunneling the RPC protocol, but in slightly different ways. Combine them all into one helper method in the virNetClient code, since this logic doesn't really belong in the virNetSocket code. Note that the this change means the shell snippet is passed to the SSH binary as a single arg, instead of three separate args, but this is functionally identical, as the three separate args were combined into one already when passed to the remote system. Signed-off-by: Daniel P. Berrang=C3=A9 --- src/libvirt_remote.syms | 1 + src/rpc/virnetclient.c | 105 +++++++++++++++++++++------------------ src/rpc/virnetclient.h | 3 ++ src/rpc/virnetsocket.c | 37 +------------- src/rpc/virnetsocket.h | 3 +- tests/virnetsockettest.c | 9 +++- 6 files changed, 70 insertions(+), 88 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 0018a0c41d..0b00bce1fa 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -42,6 +42,7 @@ virNetClientSendStream; virNetClientSendWithReply; virNetClientSetCloseCallback; virNetClientSetTLSSession; +virNetClientSSHHelperCommand; =20 =20 # rpc/virnetclientprogram.h diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 1c5bef86a1..aee2b52bf6 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -391,28 +391,75 @@ virNetClientPtr virNetClientNewTCP(const char *nodena= me, return virNetClientNew(sock, nodename); } =20 + +/* + * The SSH Server uses shell to spawn the command we give + * it. Our command then invokes shell again. Thus we need + * to apply two levels of escaping, so that commands with + * whitespace in their path get correctly interpreted. + */ +static char * +virNetClientDoubleEscapeShell(const char *str) +{ + virBuffer buf =3D VIR_BUFFER_INITIALIZER; + g_autofree char *tmp =3D NULL; + + virBufferEscapeShell(&buf, str); + + tmp =3D virBufferContentAndReset(&buf); + + virBufferEscapeShell(&buf, tmp); + + return virBufferContentAndReset(&buf); +} + +char * +virNetClientSSHHelperCommand(const char *netcatPath, + const char *socketPath) +{ + g_autofree char *netcatPathSafe =3D virNetClientDoubleEscapeShell(netc= atPath); + + return g_strdup_printf( + "sh -c " + "'if '%s' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1;= then " + "ARG=3D-q0;" + "else " + "ARG=3D;" + "fi;" + "'%s' $ARG -U %s'", + netcatPathSafe, netcatPathSafe, socketPath); +} + + +#define DEFAULT_VALUE(VAR, VAL) \ + if (!VAR) \ + VAR =3D VAL; + virNetClientPtr virNetClientNewSSH(const char *nodename, const char *service, const char *binary, const char *username, bool noTTY, bool noVerify, - const char *netcat, + const char *netcatPath, const char *keyfile, - const char *path) + const char *socketPath) { virNetSocketPtr sock; =20 + g_autofree char *command =3D NULL; + + DEFAULT_VALUE(netcatPath, "nc"); + + command =3D virNetClientSSHHelperCommand(netcatPath, socketPath); + if (virNetSocketNewConnectSSH(nodename, service, binary, username, noT= TY, - noVerify, netcat, keyfile, path, &sock) = < 0) + noVerify, keyfile, command, &sock) < 0) return NULL; =20 return virNetClientNew(sock, NULL); } =20 -#define DEFAULT_VALUE(VAR, VAL) \ - if (!VAR) \ - VAR =3D VAL; virNetClientPtr virNetClientNewLibSSH2(const char *host, const char *port, int family, @@ -428,8 +475,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, { virNetSocketPtr sock =3D NULL; =20 - virBuffer buf =3D VIR_BUFFER_INITIALIZER; - g_autofree char *nc =3D NULL; g_autofree char *command =3D NULL; =20 g_autofree char *homedir =3D NULL; @@ -442,9 +487,7 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, knownhosts =3D g_strdup(knownHostsPath); } else { confdir =3D virGetUserConfigDirectory(); - virBufferAsprintf(&buf, "%s/known_hosts", confdir); - if (!(knownhosts =3D virBufferContentAndReset(&buf))) - return NULL; + knownhosts =3D g_strdup_printf("%s/known_hosts", confdir); } =20 if (privkeyPath) { @@ -468,26 +511,7 @@ virNetClientPtr virNetClientNewLibSSH2(const char *hos= t, DEFAULT_VALUE(netcatPath, "nc"); DEFAULT_VALUE(knownHostsVerify, "normal"); =20 - virBufferEscapeShell(&buf, netcatPath); - if (!(nc =3D virBufferContentAndReset(&buf))) - return NULL; - virBufferEscapeShell(&buf, nc); - VIR_FREE(nc); - if (!(nc =3D virBufferContentAndReset(&buf))) - return NULL; - - virBufferAsprintf(&buf, - "sh -c " - "'if '%s' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1= ; then " - "ARG=3D-q0;" - "else " - "ARG=3D;" - "fi;" - "'%s' $ARG -U %s'", - nc, nc, socketPath); - - if (!(command =3D virBufferContentAndReset(&buf))) - return NULL; + command =3D virNetClientSSHHelperCommand(netcatPath, socketPath); =20 if (virNetSocketNewConnectLibSSH2(host, port, family, @@ -498,11 +522,7 @@ virNetClientPtr virNetClientNewLibSSH2(const char *hos= t, =20 return virNetClientNew(sock, NULL); } -#undef DEFAULT_VALUE =20 -#define DEFAULT_VALUE(VAR, VAL) \ - if (!VAR) \ - VAR =3D VAL; virNetClientPtr virNetClientNewLibssh(const char *host, const char *port, int family, @@ -518,8 +538,6 @@ virNetClientPtr virNetClientNewLibssh(const char *host, { virNetSocketPtr sock =3D NULL; =20 - virBuffer buf =3D VIR_BUFFER_INITIALIZER; - g_autofree char *nc =3D NULL; g_autofree char *command =3D NULL; =20 g_autofree char *homedir =3D NULL; @@ -556,18 +574,7 @@ virNetClientPtr virNetClientNewLibssh(const char *host, DEFAULT_VALUE(netcatPath, "nc"); DEFAULT_VALUE(knownHostsVerify, "normal"); =20 - virBufferEscapeShell(&buf, netcatPath); - if (!(nc =3D virBufferContentAndReset(&buf))) - return NULL; - virBufferEscapeShell(&buf, nc); - VIR_FREE(nc); - if (!(nc =3D virBufferContentAndReset(&buf))) - return NULL; - - command =3D g_strdup_printf("sh -c " - "'if '%s' -q 2>&1 | grep \"requires an argum= ent\" >/dev/null 2>&1; then " - "ARG=3D-q0;" "else " "ARG=3D;" "fi;" "'%s' $= ARG -U %s'", nc, nc, - socketPath); + command =3D virNetClientSSHHelperCommand(netcatPath, socketPath); =20 if (virNetSocketNewConnectLibssh(host, port, family, diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 778910b575..0005de46f3 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -30,6 +30,9 @@ #include "virobject.h" #include "viruri.h" =20 +char * +virNetClientSSHHelperCommand(const char *netcatPath, + const char *socketPath); =20 virNetClientPtr virNetClientNewUNIX(const char *path, bool spawnDaemon, diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 3ea863f625..6909a92a93 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -842,14 +842,11 @@ int virNetSocketNewConnectSSH(const char *nodename, const char *username, bool noTTY, bool noVerify, - const char *netcat, const char *keyfile, - const char *path, + const char *command, virNetSocketPtr *retsock) { - char *quoted; virCommandPtr cmd; - virBuffer buf =3D VIR_BUFFER_INITIALIZER; =20 *retsock =3D NULL; =20 @@ -874,38 +871,8 @@ int virNetSocketNewConnectSSH(const char *nodename, if (noVerify) virCommandAddArgList(cmd, "-o", "StrictHostKeyChecking=3Dno", NULL= ); =20 - if (!netcat) - netcat =3D "nc"; - - virCommandAddArgList(cmd, "--", nodename, "sh", "-c", NULL); - - virBufferEscapeShell(&buf, netcat); - quoted =3D virBufferContentAndReset(&buf); + virCommandAddArgList(cmd, "--", nodename, command, NULL); =20 - virBufferEscapeShell(&buf, quoted); - VIR_FREE(quoted); - quoted =3D virBufferContentAndReset(&buf); - - /* - * This ugly thing is a shell script to detect availability of - * the -q option for 'nc': debian and suse based distros need this - * flag to ensure the remote nc will exit on EOF, so it will go away - * when we close the connection tunnel. If it doesn't go away, subsequ= ent - * connection attempts will hang. - * - * Fedora's 'nc' doesn't have this option, and defaults to the desired - * behavior. - */ - virCommandAddArgFormat(cmd, - "'if '%s' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1= ; then " - "ARG=3D-q0;" - "else " - "ARG=3D;" - "fi;" - "'%s' $ARG -U %s'", - quoted, quoted, path); - - VIR_FREE(quoted); return virNetSocketNewConnectCommand(cmd, retsock); } =20 diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index f2b74f3ccb..d39b270480 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -78,9 +78,8 @@ int virNetSocketNewConnectSSH(const char *nodename, const char *username, bool noTTY, bool noVerify, - const char *netcat, const char *keyfile, - const char *path, + const char *command, virNetSocketPtr *addr); =20 int virNetSocketNewConnectLibSSH2(const char *host, diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 78fb9cbffd..842eb1bcfc 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -32,6 +32,7 @@ #include "virstring.h" =20 #include "rpc/virnetsocket.h" +#include "rpc/virnetclient.h" =20 #define VIR_FROM_THIS VIR_FROM_RPC =20 @@ -463,6 +464,8 @@ static int testSocketSSH(const void *opaque) virNetSocketPtr csock =3D NULL; /* Client socket */ int ret =3D -1; char buf[1024]; + g_autofree char *command =3D virNetClientSSHHelperCommand(data->netcat, + data->path); =20 if (virNetSocketNewConnectSSH(data->nodename, data->service, @@ -470,9 +473,8 @@ static int testSocketSSH(const void *opaque) data->username, data->noTTY, data->noVerify, - data->netcat, data->keyfile, - data->path, + command, &csock) < 0) goto cleanup; =20 @@ -570,6 +572,7 @@ mymain(void) struct testSSHData sshData1 =3D { .nodename =3D "somehost", .path =3D "/tmp/socket", + .netcat =3D "nc", .expectOut =3D "-T -e none -- somehost sh -c '" "if 'nc' -q 2>&1 | grep \"requires an argument\" >/de= v/null 2>&1; then " "ARG=3D-q0;" @@ -630,6 +633,7 @@ mymain(void) struct testSSHData sshData5 =3D { .nodename =3D "crashyhost", .path =3D "/tmp/socket", + .netcat =3D "nc", .expectOut =3D "-T -e none -- crashyhost sh -c " "'if 'nc' -q 2>&1 | grep \"requires an argument\" >/d= ev/null 2>&1; then " "ARG=3D-q0;" @@ -645,6 +649,7 @@ mymain(void) struct testSSHData sshData6 =3D { .nodename =3D "example.com", .path =3D "/tmp/socket", + .netcat =3D "nc", .keyfile =3D "/root/.ssh/example_key", .noVerify =3D true, .expectOut =3D "-i /root/.ssh/example_key -T -e none -o StrictHost= KeyChecking=3Dno -- example.com sh -c '" --=20 2.26.2