From nobody Wed May 8 03:00:05 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 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=1569846897; cv=none; d=zoho.com; s=zohoarc; b=eiJQzUMovMdIdUJ8Z6avehezvdoU1H3iEGRXJ7Cu2cS0F5mI/h4zLr5WrzH3vLJ+d2rxRLnAwg2MJTBBUBaEYaJmSqAawbcTds6HvdUiqrOVDfJO7RZXLg/u8Fqu4nAD2J4F4g9KXdIOcMsu9ahXt0KZYcDqTvKcxTKjcOtBvG0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1569846897; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To:ARC-Authentication-Results; bh=xIfZIU8CQuYoqHb1swE77O0b+SvfP7S4zI58BMXM3p8=; b=dXVTD8MJdwugv3of7yQWbA03BlLdaKMwWNigxfXFLfEQMjz03Rm3kkVI9f9831StmRmPhPGwsck//mE2N5W/fUVgKF6x0Cu5iEC4PmBcBa2hm8jJPVVvb4wk1PnTqjzQeYwwvjixvewVSCljjVUVGGukOL1pWIDIA1ozEhySDZk= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1569846897353291.84651100234237; Mon, 30 Sep 2019 05:34:57 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 26C3510CC1F9; Mon, 30 Sep 2019 12:34:54 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D7DAB60559; Mon, 30 Sep 2019 12:34:50 +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 22A9383542; Mon, 30 Sep 2019 12:34:48 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x8UCYkej007536 for ; Mon, 30 Sep 2019 08:34:46 -0400 Received: by smtp.corp.redhat.com (Postfix) id A48854B8; Mon, 30 Sep 2019 12:34:46 +0000 (UTC) Received: from dhcp-17-55.lcy.redhat.com (unknown [10.42.17.55]) by smtp.corp.redhat.com (Postfix) with ESMTP id B541C4DC; Mon, 30 Sep 2019 12:34:39 +0000 (UTC) From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Date: Mon, 30 Sep 2019 13:34:31 +0100 Message-Id: <20190930123431.3098-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: =?UTF-8?q?Guido=20G=C3=BCnther?= Subject: [libvirt] [PATCH] rpc: fix escaping of shell path for netcat binary 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: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.65]); Mon, 30 Sep 2019 12:34:55 +0000 (UTC) Consider having a nc binary in the path with a space in its name, for example '/tmp/fo o/nc' This results in libvirt running SSH with the following arg value "'if ''/tmp/fo o/nc'' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then ARG=3D-q0; else ARG=3D;fi;''/tmp/fo o/nc'' $ARG -U /var/run/libvirt/libvirt-sock'" The use of the single quote escaping was introduced by commit 6ac6238de33fc74e7545b245ae273d1bfd658808 Author: Guido G=C3=BCnther Date: Thu Oct 13 21:49:01 2011 +0200 Use virBufferEscapeShell in virNetSocketNewConnectSSH to escape the netcat command since it's passed to the shell. Adjust expected test case output accordingly. While the intention of this change was good, the result is broken as it is still underquoted. On the SSH server side, SSH itself runs the command via the shell. Our command is then invoking the shell again. Thus we see $ virsh -c qemu+ssh://root@domokun/system?netcat=3D%2Ftmp%2Ffo%20o%2Fnc list error: failed to connect to the hypervisor error: End of file while reading data: sh: /tmp/fo: No such file or directo= ry: Input/output error With the second level of escaping added we can now successfully use a nc binary with a space in the path. The original test case added was misleading as it illustrated using a binary path of 'nc -4' which is not a path, it is a command with a separate argument, which is getting interpreted as a path. Signed-off-by: Daniel P. Berrang=C3=A9 Reviewed-by: Eric Blake --- src/rpc/virnetclient.c | 8 ++++++++ src/rpc/virnetsocket.c | 9 +++++++++ tests/virnetsockettest.c | 6 +++--- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 64855fb8d6..53d8b219ea 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -490,6 +490,10 @@ virNetClientPtr virNetClientNewLibSSH2(const char *hos= t, DEFAULT_VALUE(knownHostsVerify, "normal"); =20 virBufferEscapeShell(&buf, netcatPath); + if (!(nc =3D virBufferContentAndReset(&buf))) + goto no_memory; + virBufferEscapeShell(&buf, nc); + VIR_FREE(nc); if (!(nc =3D virBufferContentAndReset(&buf))) goto no_memory; =20 @@ -596,6 +600,10 @@ virNetClientPtr virNetClientNewLibssh(const char *host, DEFAULT_VALUE(knownHostsVerify, "normal"); =20 virBufferEscapeShell(&buf, netcatPath); + if (!(nc =3D virBufferContentAndReset(&buf))) + goto no_memory; + virBufferEscapeShell(&buf, nc); + VIR_FREE(nc); if (!(nc =3D virBufferContentAndReset(&buf))) goto no_memory; =20 diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index ebd304707a..a469907779 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -903,6 +903,15 @@ int virNetSocketNewConnectSSH(const char *nodename, return -1; } quoted =3D virBufferContentAndReset(&buf); + + virBufferEscapeShell(&buf, quoted); + VIR_FREE(quoted); + if (virBufferCheckError(&buf) < 0) { + virCommandFree(cmd); + return -1; + } + 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 diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index bb8357f7cd..8cad351605 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -661,15 +661,15 @@ mymain(void) =20 struct testSSHData sshData7 =3D { .nodename =3D "somehost", - .netcat =3D "nc -4", + .netcat =3D "/tmp/fo o/nc", .path =3D "/tmp/socket", .expectOut =3D "-T -e none -- somehost sh -c '" - "if ''nc -4'' -q 2>&1 | grep \"requires an argument\"= >/dev/null 2>&1; then " + "if \'''\\''/tmp/fo o/nc'\\'''' -q 2>&1 | grep \"requ= ires an argument\" >/dev/null 2>&1; then " "ARG=3D-q0;" "else " "ARG=3D;" "fi;" - "''nc -4'' $ARG -U /tmp/socket'\n", + "'''\\''/tmp/fo o/nc'\\'''' $ARG -U /tmp/socket'\n", }; if (virTestRun("SSH test 7", testSocketSSH, &sshData7) < 0) ret =3D -1; --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list