From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 7146B1FF13F for ; Thu, 09 Apr 2026 17:54:37 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B3680ABC1; Thu, 9 Apr 2026 17:55:19 +0200 (CEST) From: Samuel Rufinatscha To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox v8 2/6] token shadow: cache verified API token secrets Date: Thu, 9 Apr 2026 17:54:22 +0200 Message-ID: <20260409155437.312760-3-s.rufinatscha@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260409155437.312760-1-s.rufinatscha@proxmox.com> References: <20260409155437.312760-1-s.rufinatscha@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775750013632 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.230 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: YU4GSCGFEGUOIC6RLYSAN3K5KYWZXJ7X X-Message-ID-Hash: YU4GSCGFEGUOIC6RLYSAN3K5KYWZXJ7X X-MailFrom: s.rufinatscha@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Adds an in-memory cache of successfully verified token secrets. Subsequent requests for the same token+secret combination only perform a comparison using openssl::memcmp::eq and avoid re-running the password hash. The cache is updated when a token secret is set and cleared when a token is deleted. A shared generation counter (via ConfigVersionCache) is used to invalidate caches across processes when token secrets are modified or deleted. This keeps privileged and unprivileged daemons in sync. Signed-off-by: Samuel Rufinatscha --- Changes from v7 to v8: * Rename shared_gen -> cached_gen * Rename token_shadow_shared_gen() -> token_shadow_generation() * Rename bump_token_shadow_shared_gen() -> bump_token_shadow_generation() * Use access_backend() instead of access_conf() for generation hooks Changes from v6 to v7: * Rebased * Rename "gen" variables to be compatible with Rust 2024 keyword changes Changes from v5 to v6: * Rebased * Check that the input byte lengths are equal before calling openssl::memcmp::eq(..). Changes from v4 to v5: * Rebased * Fix wrong type compilation issue; replaced with ApiLockGuard * Move invalidate_cache_state_and_set_gen into cache object impl rename to reset_and_set_gen * Add additional insert/remove helpers which set/update the generation directly * Clarified the usage of shared generation counter in the commit message Changes from v3 to v4: * Add gen param to invalidate_cache_state() * Validates the generation bump after obtaining write lock in apply_api_mutation * Pass lock to apply_api_mutation * Remove unnecessary gen check cache_try_secret_matches * Adjusted commit message Changes from v2 to v3: * Replaced process-local cache invalidation (AtomicU64 API_MUTATION_GENERATION) with a cross-process shared generation via ConfigVersionCache. * Validate shared generation before/after the constant-time secret compare; only insert into cache if the generation is unchanged. * invalidate_cache_state() on insert if shared generation changed. Changes from v1 to v2: * Replace OnceCell with LazyLock, and std::sync::RwLock with parking_lot::RwLock. * Add API_MUTATION_GENERATION and guard cache inserts to prevent “zombie inserts” across concurrent set/delete. * Refactor cache operations into cache_try_secret_matches, cache_try_insert_secret, and centralize write-side behavior in apply_api_mutation. * Switch fast-path cache access to try_read/try_write (best-effort). Cargo.toml | 1 + proxmox-access-control/Cargo.toml | 1 + proxmox-access-control/src/token_shadow.rs | 170 ++++++++++++++++++++- 3 files changed, 169 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 02ff7f81..cf55653f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,6 +115,7 @@ native-tls = "0.2" nix = "0.29" openssl = "0.10" pam-sys = "0.5" +parking_lot = "0.12" percent-encoding = "2.1" pin-utils = "0.1.0" proc-macro2 = "1.0" diff --git a/proxmox-access-control/Cargo.toml b/proxmox-access-control/Cargo.toml index ec189664..1de2842c 100644 --- a/proxmox-access-control/Cargo.toml +++ b/proxmox-access-control/Cargo.toml @@ -16,6 +16,7 @@ anyhow.workspace = true const_format.workspace = true nix = { workspace = true, optional = true } openssl = { workspace = true, optional = true } +parking_lot.workspace = true regex.workspace = true hex = { workspace = true, optional = true } serde.workspace = true diff --git a/proxmox-access-control/src/token_shadow.rs b/proxmox-access-control/src/token_shadow.rs index c586d834..d0bf43d7 100644 --- a/proxmox-access-control/src/token_shadow.rs +++ b/proxmox-access-control/src/token_shadow.rs @@ -1,13 +1,28 @@ use std::collections::HashMap; +use std::sync::LazyLock; use anyhow::{bail, format_err, Error}; +use parking_lot::RwLock; use serde_json::{from_value, Value}; use proxmox_auth_api::types::Authid; use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockGuard}; +use crate::init::access_backend; use crate::init::impl_feature::{token_shadow, token_shadow_lock}; +/// Global in-memory cache for successfully verified API token secrets. +/// The cache stores plain text secrets for token Authids that have already been +/// verified against the hashed values in `token.shadow`. This allows for cheap +/// subsequent authentications for the same token+secret combination, avoiding +/// recomputing the password hash on every request. +static TOKEN_SECRET_CACHE: LazyLock> = LazyLock::new(|| { + RwLock::new(ApiTokenSecretCache { + secrets: HashMap::new(), + cached_gen: 0, + }) +}); + // Get exclusive lock fn lock_config() -> Result { open_api_lockfile(token_shadow_lock(), None, true) @@ -36,9 +51,27 @@ pub fn verify_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> { bail!("not an API token ID"); } + // Fast path + if cache_try_secret_matches(tokenid, secret) { + return Ok(()); + } + + // Slow path + // First, capture the generation before doing the hash verification. + let gen_before = token_shadow_generation(); + let data = read_file()?; match data.get(tokenid) { - Some(hashed_secret) => proxmox_sys::crypt::verify_crypt_pw(secret, hashed_secret), + Some(hashed_secret) => { + proxmox_sys::crypt::verify_crypt_pw(secret, hashed_secret)?; + + // Try to cache only if nothing changed while verifying the secret. + if let Some(gen_before) = gen_before { + cache_try_insert_secret(tokenid.clone(), secret.to_owned(), gen_before); + } + + Ok(()) + } None => bail!("invalid API token"), } } @@ -49,13 +82,15 @@ pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> { bail!("not an API token ID"); } - let _guard = lock_config()?; + let guard = lock_config()?; let mut data = read_file()?; let hashed_secret = proxmox_sys::crypt::encrypt_pw(secret)?; data.insert(tokenid.clone(), hashed_secret); write_file(data)?; + apply_api_mutation(guard, tokenid, Some(secret)); + Ok(()) } @@ -65,12 +100,14 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> { bail!("not an API token ID"); } - let _guard = lock_config()?; + let guard = lock_config()?; let mut data = read_file()?; data.remove(tokenid); write_file(data)?; + apply_api_mutation(guard, tokenid, None); + Ok(()) } @@ -81,3 +118,130 @@ pub fn generate_and_set_secret(tokenid: &Authid) -> Result { set_secret(tokenid, &secret)?; Ok(secret) } + +/// Cached secret. +struct CachedSecret { + secret: String, +} + +struct ApiTokenSecretCache { + /// Keys are token Authids, values are the corresponding plain text secrets. + /// Entries are added after a successful on-disk verification in + /// `verify_secret` or when a new token secret is generated by + /// `generate_and_set_secret`. Used to avoid repeated + /// password-hash computation on subsequent authentications. + secrets: HashMap, + /// token.shadow generation of cached secrets. + cached_gen: usize, +} + +impl ApiTokenSecretCache { + /// Resets all local cache contents and sets/updates the cached generation. + fn reset_and_set_gen(&mut self, new_gen: usize) { + self.secrets.clear(); + self.cached_gen = new_gen; + } + + /// Caches a secret and sets/updates the cache generation. + fn insert_and_set_gen(&mut self, tokenid: Authid, secret: CachedSecret, new_gen: usize) { + self.secrets.insert(tokenid, secret); + self.cached_gen = new_gen; + } + + /// Evicts a cached secret and sets/updates the cached generation. + fn evict_and_set_gen(&mut self, tokenid: &Authid, new_gen: usize) { + self.secrets.remove(tokenid); + self.cached_gen = new_gen; + } +} + +fn cache_try_insert_secret(tokenid: Authid, secret: String, gen_before: usize) { + let Some(mut cache) = TOKEN_SECRET_CACHE.try_write() else { + return; + }; + + let Some(gen_now) = token_shadow_generation() else { + return; + }; + + // If this process missed a generation bump, its cache is stale. + if cache.cached_gen != gen_now { + cache.reset_and_set_gen(gen_now); + } + + // If a mutation happened while we were verifying the secret, do not insert. + if gen_now == gen_before { + cache.insert_and_set_gen(tokenid, CachedSecret { secret }, gen_now); + } +} + +/// Tries to match the given token secret against the cached secret. +/// +/// Verifies the generation/version before doing the constant-time +/// comparison to reduce TOCTOU risk. During token rotation or deletion +/// tokens for in-flight requests may still validate against the previous +/// generation. +fn cache_try_secret_matches(tokenid: &Authid, secret: &str) -> bool { + let Some(cache) = TOKEN_SECRET_CACHE.try_read() else { + return false; + }; + let Some(entry) = cache.secrets.get(tokenid) else { + return false; + }; + let Some(current_gen) = token_shadow_generation() else { + return false; + }; + + if current_gen == cache.cached_gen { + let cached_secret_bytes = entry.secret.as_bytes(); + let secret_bytes = secret.as_bytes(); + + return cached_secret_bytes.len() == secret_bytes.len() + && openssl::memcmp::eq(cached_secret_bytes, secret_bytes); + } + + false +} + +fn apply_api_mutation(_guard: ApiLockGuard, tokenid: &Authid, secret: Option<&str>) { + // Signal cache invalidation to other processes (best-effort). + let bumped_gen = bump_token_shadow_generation(); + let mut cache = TOKEN_SECRET_CACHE.write(); + + // If we cannot get the current generation, we cannot trust the cache + let Some(current_gen) = token_shadow_generation() else { + cache.reset_and_set_gen(0); + return; + }; + + // If we cannot bump the generation, or if it changed after + // obtaining the cache write lock, we cannot trust the cache + if bumped_gen != Some(current_gen) { + cache.reset_and_set_gen(current_gen); + return; + } + + // Apply the new mutation. + match secret { + Some(secret) => { + let cached_secret = CachedSecret { + secret: secret.to_owned(), + }; + cache.insert_and_set_gen(tokenid.clone(), cached_secret, current_gen); + } + None => cache.evict_and_set_gen(tokenid, current_gen), + } +} + +/// Get the current generation. +fn token_shadow_generation() -> Option { + access_backend().token_shadow_cache_generation() +} + +/// Bump and return the new generation. +fn bump_token_shadow_generation() -> Option { + access_backend() + .increment_token_shadow_cache_generation() + .ok() + .map(|prev| prev + 1) +} -- 2.47.3