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 AD0C7934EA for ; Mon, 19 Feb 2024 19:59:01 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8F1A234A63 for ; Mon, 19 Feb 2024 19:59:01 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 19 Feb 2024 19:59:00 +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 5A547439AB for ; Mon, 19 Feb 2024 19:59:00 +0100 (CET) Message-ID: <0434cd4f-d0fe-4c1d-9d70-fbf7bac4f239@proxmox.com> Date: Mon, 19 Feb 2024 19:58:59 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: pbs-devel@lists.proxmox.com References: <20240215152001.269490-1-s.sterz@proxmox.com> <20240215152001.269490-11-s.sterz@proxmox.com> From: Max Carrara In-Reply-To: <20240215152001.269490-11-s.sterz@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.006 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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 10/12] auth: upgrade hashes on user log in 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: Mon, 19 Feb 2024 18:59:01 -0000 On 2/15/24 16:19, Stefan Sterz wrote: > if a users password is not hashed with the latest password hashing > function, re-hash the password with the newest hashing function. we > can only do this on login and after the password has been validated, > as this is the only point at which we have access to the plain text > password and also know that it matched the original password. > > Signed-off-by: Stefan Sterz > --- > src/auth.rs | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/src/auth.rs b/src/auth.rs > index c89314f5..3379577f 100644 > --- a/src/auth.rs > +++ b/src/auth.rs > @@ -28,20 +28,30 @@ pub const TERM_PREFIX: &str = "PBSTERM"; > > struct PbsAuthenticator; > > -const SHADOW_CONFIG_FILENAME: &str = configdir!("/shadow.json"); > +pub(crate) const SHADOW_CONFIG_FILENAME: &str = configdir!("/shadow.json"); > > impl Authenticator for PbsAuthenticator { > fn authenticate_user<'a>( > - &self, > + &'a self, > username: &'a UsernameRef, > password: &'a str, > - _client_ip: Option<&'a IpAddr>, > + client_ip: Option<&'a IpAddr>, > ) -> Pin> + Send + 'a>> { > Box::pin(async move { > let data = proxmox_sys::fs::file_get_json(SHADOW_CONFIG_FILENAME, Some(json!({})))?; > match data[username.as_str()].as_str() { > None => bail!("no password set"), > - Some(enc_password) => proxmox_sys::crypt::verify_crypt_pw(password, enc_password)?, > + Some(enc_password) => { > + proxmox_sys::crypt::verify_crypt_pw(password, enc_password)?; > + > + // if the password hash is not based on the current hashing function (as > + // identified by its prefix), rehash the password. > + if !enc_password.starts_with(proxmox_sys::crypt::HASH_PREFIX) { > + // ignore errors here, we already authenticated the user, re-hashing the > + // password should not prevent them from logging in. > + let _ = self.store_password(username, password, client_ip); IMO this should be logged somewhere instead of just swallowing the error silently, possibly even warning the user or admin that re-hashing failed (while letting them log on anyways). The point of this series is to move away from the old stuff, so we should ensure that we actually do. > + } > + } > } > Ok(()) > })