From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <s.sterz@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 AE75E918ED
 for <pbs-devel@lists.proxmox.com>; Thu, 15 Feb 2024 16:20:18 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 3CF2316729
 for <pbs-devel@lists.proxmox.com>; Thu, 15 Feb 2024 16:20:18 +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>; Thu, 15 Feb 2024 16:20:16 +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 D143748418
 for <pbs-devel@lists.proxmox.com>; Thu, 15 Feb 2024 16:20:15 +0100 (CET)
From: Stefan Sterz <s.sterz@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Thu, 15 Feb 2024 16:20:00 +0100
Message-Id: <20240215152001.269490-12-s.sterz@proxmox.com>
X-Mailer: git-send-email 2.39.2
In-Reply-To: <20240215152001.269490-1-s.sterz@proxmox.com>
References: <20240215152001.269490-1-s.sterz@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.082 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: [pbs-devel] [PATCH proxmox-backup 11/12] auth/manager: add manager
 command to upgrade hashes
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: Thu, 15 Feb 2024 15:20:18 -0000

this uses proxmox-sys' `upgrade_hash` function to upgrade all stored
hashes that don't already use the latest hash. this allows admins to
upgrade their user's password hashes even if the user's don't log in.

however, as this requires a newer version of proxmox-sys'
`verify_crypt_pw` function to work, make this a command that admin's
can optional perform instead of automatically upgrading all hashes.
admins can reset their users password to avoid issues when
downgrading pbs or similar to work around potential issues here, but
doing this automatically may cause problems.

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
note that once an admin has upgraded a hash, downgrading
proxmox-backup-server will break logging in for all users with upgraded
passwords. an admin would then need to manually reset the password via
`proxmox-backup-manager user update <user> --password <pw>`.

 src/auth_helpers.rs                    | 38 +++++++++++++++++++++++++-
 src/bin/proxmox_backup_manager/user.rs | 34 ++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/src/auth_helpers.rs b/src/auth_helpers.rs
index 1a483d84..375ce190 100644
--- a/src/auth_helpers.rs
+++ b/src/auth_helpers.rs
@@ -1,7 +1,8 @@
+use std::collections::HashMap;
 use std::path::PathBuf;
 use std::sync::OnceLock;

-use anyhow::Error;
+use anyhow::{format_err, Error};
 use lazy_static::lazy_static;
 use openssl::pkey::{PKey, Private, Public};
 use openssl::rsa::Rsa;
@@ -16,6 +17,41 @@ use serde_json::json;
 pub use crate::auth::setup_auth_context;
 pub use proxmox_auth_api::api::assemble_csrf_prevention_token;

+pub fn upgrade_password_hashes() -> Result<(), Error> {
+    let data =
+        proxmox_sys::fs::file_get_json(crate::auth::SHADOW_CONFIG_FILENAME, Some(json!({})))?;
+    let mut new = HashMap::new();
+
+    for (username, password) in data
+        .as_object()
+        .ok_or_else(|| format_err!("shadow file has an unexpected format!"))?
+    {
+        let upgraded_password = proxmox_sys::crypt::upgrade_hash(
+            password
+                .as_str()
+                .ok_or_else(|| format_err!("user without password found!"))?,
+        )?;
+
+        new.insert(username.as_str(), upgraded_password);
+    }
+
+    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
+    let options = proxmox_sys::fs::CreateOptions::new()
+        .perm(mode)
+        .owner(nix::unistd::ROOT)
+        .group(nix::unistd::Gid::from_raw(0));
+
+    let new_data = serde_json::to_vec_pretty(&new)?;
+    proxmox_sys::fs::replace_file(
+        crate::auth::SHADOW_CONFIG_FILENAME,
+        &new_data,
+        options,
+        true,
+    )?;
+
+    Ok(())
+}
+
 pub fn generate_csrf_key() -> Result<(), Error> {
     let path = PathBuf::from(configdir!("/csrf.key"));

diff --git a/src/bin/proxmox_backup_manager/user.rs b/src/bin/proxmox_backup_manager/user.rs
index 743c5d16..2daf2db7 100644
--- a/src/bin/proxmox_backup_manager/user.rs
+++ b/src/bin/proxmox_backup_manager/user.rs
@@ -1,7 +1,9 @@
-use anyhow::Error;
+use anyhow::{bail, Error};
 use serde_json::Value;

 use std::collections::HashMap;
+use std::io::IsTerminal;
+use std::io::Write;

 use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
 use proxmox_schema::api;
@@ -65,6 +67,32 @@ fn list_users(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Er
     Ok(Value::Null)
 }

+#[api]
+/// Updates the hashes of all users that don't use the current hashing method to the current
+/// hashing method. This is a stop-gap meassure to avoid keeping insecure hashes when migrating
+/// between password hash function.
+fn update_password_hashes() -> Result<(), Error> {
+    // If we're on a TTY, query the user
+    if std::io::stdin().is_terminal() {
+        println!("You are about to update all password hashes that are managed by Proxmox Backup Server!");
+        print!("Are you sure you want to continue? (y/N): ");
+        let _ = std::io::stdout().flush();
+        use std::io::{BufRead, BufReader};
+        let mut line = String::new();
+        match BufReader::new(std::io::stdin()).read_line(&mut line) {
+            Ok(_) => match line.trim() {
+                "y" | "Y" => (), // continue
+                _ => bail!("Aborting."),
+            },
+            Err(err) => bail!("Failed to read line - {err}."),
+        }
+    }
+
+    proxmox_backup::auth_helpers::upgrade_password_hashes()?;
+
+    Ok(())
+}
+
 #[api(
     input: {
         properties: {
@@ -194,6 +222,10 @@ fn list_user_tfa(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value,
 pub fn user_commands() -> CommandLineInterface {
     let cmd_def = CliCommandMap::new()
         .insert("list", CliCommand::new(&API_METHOD_LIST_USERS))
+        .insert(
+            "update-hashes",
+            CliCommand::new(&API_METHOD_UPDATE_PASSWORD_HASHES),
+        )
         .insert(
             "create",
             // fixme: howto handle password parameter?
--
2.39.2