From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <m.carrara@proxmox.com>
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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <m.carrara@proxmox.com>
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
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=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 <s.sterz@proxmox.com>
> ---
>  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<Box<dyn Future<Output = Result<(), Error>> + 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(())
>          })