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 A8B2997DB2 for ; Wed, 6 Mar 2024 13:36:52 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D15A017018 for ; Wed, 6 Mar 2024 13:36:21 +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 ; Wed, 6 Mar 2024 13:36:19 +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 344404884E for ; Wed, 6 Mar 2024 13:36:19 +0100 (CET) From: Stefan Sterz To: pbs-devel@lists.proxmox.com Date: Wed, 6 Mar 2024 13:36:06 +0100 Message-Id: <20240306123609.164021-10-s.sterz@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240306123609.164021-1-s.sterz@proxmox.com> References: <20240306123609.164021-1-s.sterz@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.124 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 v2 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: Wed, 06 Mar 2024 12:36:52 -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 --- 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