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 74D9691A16 for ; Thu, 15 Feb 2024 16:20:49 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CF7201673B for ; 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 ; Thu, 15 Feb 2024 16:20:15 +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 A9AA04841F for ; Thu, 15 Feb 2024 16:20:15 +0100 (CET) From: Stefan Sterz To: pbs-devel@lists.proxmox.com Date: Thu, 15 Feb 2024 16:19:58 +0100 Message-Id: <20240215152001.269490-10-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.131 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 PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far 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 09/12] auth: move to hmac keys for csrf tokens 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: Thu, 15 Feb 2024 15:20:49 -0000 previously we used a self-rolled implementation for csrf tokens. while it's unlikely to cause issues in reality, as csrf tokens are only valid for a given tickets lifetime, there are still theoretical attacks on our implementation. so move all of this code into the proxmox-auth-api crate and use hmac instead. this change should not impact existing installations for now, as this falls back to the old implementation if a key is already present. hmac keys will only be used for new installations and if users manually remove the old key and Signed-off-by: Stefan Sterz --- note that the fallbacks here and in `proxmox-auth-api` should be removed with the next (major) version if possible. simply removing the csrf key before restarting the services should be enough here. it is possible that this will break some sessions, possibly forcing a user to log back in. so only do this when we can assume that an admin would have a reasonable expectation that something like this will happen (i.e.: possibly document this in the upgrade notes etc.). src/auth.rs | 10 ++--- src/auth_helpers.rs | 97 ++++++++++----------------------------------- 2 files changed, 26 insertions(+), 81 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index ec0bc41f..c89314f5 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -15,7 +15,7 @@ use serde_json::json; use proxmox_auth_api::api::{Authenticator, LockedTfaConfig}; use proxmox_auth_api::ticket::{Empty, Ticket}; use proxmox_auth_api::types::Authid; -use proxmox_auth_api::Keyring; +use proxmox_auth_api::{HMACKey, Keyring}; use proxmox_ldap::{Config, Connection, ConnectionMode}; use proxmox_tfa::api::{OpenUserChallengeData, TfaConfig}; @@ -267,7 +267,7 @@ pub fn setup_auth_context(use_private_key: bool) { AUTH_CONTEXT .set(PbsAuthContext { keyring, - csrf_secret: crate::auth_helpers::csrf_secret().to_vec(), + csrf_secret: crate::auth_helpers::csrf_secret(), }) .map_err(drop) .expect("auth context setup twice"); @@ -285,7 +285,7 @@ pub(crate) fn public_auth_keyring() -> &'static Keyring { struct PbsAuthContext { keyring: &'static Keyring, - csrf_secret: Vec, + csrf_secret: &'static HMACKey, } impl proxmox_auth_api::api::AuthContext for PbsAuthContext { @@ -327,8 +327,8 @@ impl proxmox_auth_api::api::AuthContext for PbsAuthContext { } /// CSRF prevention token secret data. - fn csrf_secret(&self) -> &[u8] { - &self.csrf_secret + fn csrf_secret(&self) -> &'static HMACKey { + self.csrf_secret } /// Verify a token secret. diff --git a/src/auth_helpers.rs b/src/auth_helpers.rs index c2eaaef1..1a483d84 100644 --- a/src/auth_helpers.rs +++ b/src/auth_helpers.rs @@ -1,81 +1,20 @@ use std::path::PathBuf; +use std::sync::OnceLock; -use anyhow::{bail, format_err, Error}; +use anyhow::Error; use lazy_static::lazy_static; use openssl::pkey::{PKey, Private, Public}; use openssl::rsa::Rsa; -use openssl::sha; use pbs_config::BackupLockGuard; -use proxmox_lang::try_block; +use proxmox_auth_api::HMACKey; use proxmox_sys::fs::{file_get_contents, replace_file, CreateOptions}; -use pbs_api_types::Userid; use pbs_buildcfg::configdir; use serde_json::json; pub use crate::auth::setup_auth_context; - -fn compute_csrf_secret_digest(timestamp: i64, secret: &[u8], userid: &Userid) -> String { - let mut hasher = sha::Sha256::new(); - let data = format!("{:08X}:{}:", timestamp, userid); - hasher.update(data.as_bytes()); - hasher.update(secret); - - base64::encode_config(hasher.finish(), base64::STANDARD_NO_PAD) -} - -pub fn assemble_csrf_prevention_token(secret: &[u8], userid: &Userid) -> String { - let epoch = proxmox_time::epoch_i64(); - - let digest = compute_csrf_secret_digest(epoch, secret, userid); - - format!("{:08X}:{}", epoch, digest) -} - -pub fn verify_csrf_prevention_token( - secret: &[u8], - userid: &Userid, - token: &str, - min_age: i64, - max_age: i64, -) -> Result { - use std::collections::VecDeque; - - let mut parts: VecDeque<&str> = token.split(':').collect(); - - try_block!({ - if parts.len() != 2 { - bail!("format error - wrong number of parts."); - } - - let timestamp = parts.pop_front().unwrap(); - let sig = parts.pop_front().unwrap(); - - let ttime = i64::from_str_radix(timestamp, 16) - .map_err(|err| format_err!("timestamp format error - {}", err))?; - - let digest = compute_csrf_secret_digest(ttime, secret, userid); - - if digest != sig { - bail!("invalid signature."); - } - - let now = proxmox_time::epoch_i64(); - - let age = now - ttime; - if age < min_age { - bail!("timestamp newer than expected."); - } - - if age > max_age { - bail!("timestamp too old."); - } - - Ok(age) - }) - .map_err(|err| format_err!("invalid csrf token - {}", err)) -} +pub use proxmox_auth_api::api::assemble_csrf_prevention_token; pub fn generate_csrf_key() -> Result<(), Error> { let path = PathBuf::from(configdir!("/csrf.key")); @@ -84,17 +23,14 @@ pub fn generate_csrf_key() -> Result<(), Error> { return Ok(()); } - let rsa = Rsa::generate(2048).unwrap(); - - let pem = rsa.private_key_to_pem()?; + let key = HMACKey::generate()?.to_base64()?; use nix::sys::stat::Mode; - let backup_user = pbs_config::backup_user()?; replace_file( &path, - &pem, + key.as_bytes(), CreateOptions::new() .perm(Mode::from_bits_truncate(0o0640)) .owner(nix::unistd::ROOT) @@ -145,12 +81,21 @@ pub fn generate_auth_key() -> Result<(), Error> { Ok(()) } -pub fn csrf_secret() -> &'static [u8] { - lazy_static! { - static ref SECRET: Vec = file_get_contents(configdir!("/csrf.key")).unwrap(); - } - - &SECRET +pub fn csrf_secret() -> &'static HMACKey { + static SECRET: OnceLock = OnceLock::new(); + + SECRET.get_or_init(|| { + let bytes = file_get_contents(configdir!("/csrf.key")).unwrap(); + std::str::from_utf8(&bytes) + .map_err(anyhow::Error::new) + .and_then(HMACKey::from_base64) + // legacy fall back to load legacy csrf secrets + // TODO: remove once we move away from legacy token verification + .unwrap_or_else(|_| { + let key_as_b64 = base64::encode_config(bytes, base64::STANDARD_NO_PAD); + HMACKey::from_base64(&key_as_b64).unwrap() + }) + }) } fn load_public_auth_key() -> Result, Error> { -- 2.39.2