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)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 6EBD4616E6 for ; Wed, 9 Feb 2022 16:55:27 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5C2D04B07 for ; Wed, 9 Feb 2022 16:54:57 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (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 DA3DD4AFB for ; Wed, 9 Feb 2022 16:54:55 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id B21C546CF4 for ; Wed, 9 Feb 2022 16:54:55 +0100 (CET) Message-ID: <5acd5a0d-d66a-4e1a-3a75-fb9ef3b33d86@proxmox.com> Date: Wed, 9 Feb 2022 16:54:54 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:97.0) Gecko/20100101 Thunderbird/97.0 Content-Language: en-US To: Stefan Sterz , Dominik Csapak , Proxmox Backup Server development discussion References: <20220207124825.1116194-1-s.sterz@proxmox.com> <10c2b794-7815-30b8-9957-a89acdafcab1@proxmox.com> <23c29e64-bc74-9a8f-7b70-bad0abdb633a@proxmox.com> From: Thomas Lamprecht In-Reply-To: <23c29e64-bc74-9a8f-7b70-bad0abdb633a@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.059 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox-backup 1/2] fix #3853: api: add force option to tape key change-passphrase X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Feb 2022 15:55:27 -0000 On 08.02.22 16:30, Stefan Sterz wrote: > On 2/8/22 16:26, Dominik Csapak wrote: >> On 2/7/22 17:14, Stefan Sterz wrote: >>> On 2/7/22 16:57, Stefan Sterz wrote: >>>> On 2/7/22 15:58, Thomas Lamprecht wrote: >>>>> On 07.02.22 13:48, Stefan Sterz wrote: >> [snip] >>>>>> +=C2=A0=C2=A0=C2=A0 // sanity checks for "password xor --force" >>>>>> +=C2=A0=C2=A0=C2=A0 if force && password.is_some() { >>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 bail!("password is not= allowed when using force") >>>>>> +=C2=A0=C2=A0=C2=A0 } >>>>>> + >>>>>> +=C2=A0=C2=A0=C2=A0 if !force && password.is_none() { >>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 bail!("missing paramet= er: password") >>>>>> +=C2=A0=C2=A0=C2=A0 } >>>>> Above two if's could be written slightly shorter while IMO even imp= roving readability >>>>> >>>>> match (force, password) { >>>>> =C2=A0=C2=A0=C2=A0=C2=A0 (true, Some(_)) =3D> bail!("password is no= t allowed when using force"), >>>>> =C2=A0=C2=A0=C2=A0=C2=A0 (false, None) =3D> bail!("missing paramete= r: password"), >>>>> =C2=A0=C2=A0=C2=A0=C2=A0 _ =3D> (), // OK >>>>> } >>>> This does not work, because here password is moved into the match ex= pression. The borrow checker will complain about it being used later on w= hen trying to decrypt the key configuration. You could clone password her= e, but this solution strikes me as rather "inelegant". Yeah I just wrote that in the MTA from top of my head so that you get the= rough idea, using a reference here like Dominik also suggests is the obvi= ous way to make it work as intended.. ;-) The whole thing, actually working, meaning it compiles ^^: let (key, created, fingerprint) =3D match (force, &password) { (true, Some(_)) =3D> bail!("password is not allowed when using force"= ), (false, None) =3D> bail!("missing parameter: password"), (false, Some(pass)) =3D> key_config.decrypt(&|| Ok(pass.as_bytes().to= _vec()))?, (true, None) =3D> { let (keys, _) =3D load_keys()?; let key_info =3D keys.get(&fingerprint).ok_or_else(|| format_err!= ("..."))?; (key_info.key, key_config.created, fingerprint) } }; IMO having this as match where we know we got all cases covered has it's = value and it's not that bad to read, but I also do not want to spent to much time n= itpicking on this one, so your call if it's the whole match or just the checks in t= he match.