From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 688FB60BA1 for ; Fri, 9 Oct 2020 19:09:33 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5615718014 for ; Fri, 9 Oct 2020 19:09:03 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 76DFA18007 for ; Fri, 9 Oct 2020 19:09:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 40E9345433; Fri, 9 Oct 2020 19:09:01 +0200 (CEST) Date: Fri, 9 Oct 2020 19:08:59 +0200 From: Stoiko Ivanov To: Daniel Berteaud Cc: Proxmox VE development discussion Message-ID: <20201009190859.67a49c2a@rosa.proxmox.com> In-Reply-To: <1253861784.220180.1602258262225.JavaMail.zimbra@fws.fr> References: <20201009151344.8999-1-s.ivanov@proxmox.com> <1663712196.220058.1602257198078.JavaMail.zimbra@fws.fr> <1253861784.220180.1602258262225.JavaMail.zimbra@fws.fr> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.050 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [open3.pm, lio.pm, proxmox.com, zfsplugin.pm] Subject: Re: [pve-devel] [PATCH storage] ZFSPlugin: untaint lun number X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Oct 2020 17:09:33 -0000 On Fri, 9 Oct 2020 17:44:22 +0200 (CEST) Daniel Berteaud wrote: > ----- Le 9 Oct 20, =C3=A0 17:26, Daniel Berteaud daniel@firewall-services= .com a =C3=A9crit : >=20 > > ----- Le 9 Oct 20, =C3=A0 17:13, Stoiko Ivanov s.ivanov@proxmox.com a = =C3=A9crit : > > =20 > >> ZFS over iSCSI fetches information about the disk-images via ssh, thus > >> the obtainted data is tainted (perlsec (1)). > >>=20 > >> Since pvedaemon runs with '-T' enabled trying to start a VM via GUI/AP= I failed, > >> while it still worked via `qm` or `pvesh`. > >>=20 > >> The issue surfaced after commit cb9db10c1a9855cf40ff13e81f9dd97d6a9b26= 98 in > >> pve-common ('run_command: improve performance for logging and long lin= es'), > >> and results from concatenating the original (tainted) buffer to a vari= able, > >> instead of a captured subgroup. > >>=20 > >> Untainting the value in ZFSPlugin should not cause any regressiosn, si= nce the > >> other 3 target providers already have a match on '\d+' for retrieving = the > >> lun number. > >>=20 > >> reported via pve-user [0]. > >>=20 > >> reproduced and tested by setting up a LIO-target (on top of a virtual = PVE), > >> adding it as storage and trying to start a guest (with a disk on the > >> ZFS over iSCSI storage) with `perl -T /usr/sbin/qm start $vmid` > >>=20 > >> [0] https://lists.proxmox.com/pipermail/pve-user/2020-October/172055.h= tml > >>=20 > >> Signed-off-by: Stoiko Ivanov > >> --- > >> PVE/Storage/ZFSPlugin.pm | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >>=20 > >> diff --git a/PVE/Storage/ZFSPlugin.pm b/PVE/Storage/ZFSPlugin.pm > >> index 383f0a0..63b9551 100644 > >> --- a/PVE/Storage/ZFSPlugin.pm > >> +++ b/PVE/Storage/ZFSPlugin.pm > >> @@ -159,7 +159,11 @@ sub zfs_get_lun_number { > >>=20 > >> die "could not find lun_number for guid $guid" if !$guid; > >>=20 > >> - return $class->zfs_request($scfg, undef, 'list_view', $guid); > >> + if ($class->zfs_request($scfg, undef, 'list_view', $guid) =3D~ /^= (\d+)$/) { > >> + return $1; > >> + } > >> + > >> + die "lun_number for guid $guid is not a number"; > >> } =20 > > =20 >=20 >=20 > I can confirm this is fixing the issue of the VM not starting from the we= b interface. There's still a (probably) related issue : ZFS over iSCSI disk= s can't be removed (eg, at the end of a live disk move) : Thanks for testing it so quickly! >=20 > Use of uninitialized value in die at /usr/share/perl5/PVE/Storage/LunCmd/= LIO.pm line 337. > error during cfs-locked 'storage-iscsi-zfs-zol1-prd' operation: command '= /usr/bin/ssh -o 'BatchMode=3Dyes' -i /etc/pve/priv/zfs/10.29.255.252_id_rsa= root@10.29.255.252 -- /usr/bin/targetcli /iscsi/iqn.2019-10.fr.fws.pve:pro= d/tpg1/luns/ delete lun1' failed: Insecure dependency in exec while running= with -T switch at /usr/share/perl/5.28/IPC/Open3.pm line 178. > ...propagated at /usr/share/perl5/PVE/Storage/LunCmd/LIO.pm line 337. You're right - I'll come up with a patch for the other places wher the LIO Backend needs untainting and will send a patch. Best Regards, stoiko >=20 > It might be only affecting the LIO backend though >=20 > Cheers, > Daniel >=20 >=20