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 D923F1FF137 for ; Tue, 17 Feb 2026 12:11:53 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5880833E05; Tue, 17 Feb 2026 12:12:42 +0100 (CET) From: Samuel Rufinatscha To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v5 2/4] pbs-config: cache verified API token secrets Date: Tue, 17 Feb 2026 12:12:19 +0100 Message-ID: <20260217111229.78661-3-s.rufinatscha@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260217111229.78661-1-s.rufinatscha@proxmox.com> References: <20260217111229.78661-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: 1771326748098 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.259 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 Message-ID-Hash: PSXQ42K37LNPWQQJNA7XFVO3HELJX62S X-Message-ID-Hash: PSXQ42K37LNPWQQJNA7XFVO3HELJX62S 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 v4 to v5: * Rebased * 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 + pbs-config/Cargo.toml | 1 + pbs-config/src/token_shadow.rs | 167 ++++++++++++++++++++++++++++++++- 3 files changed, 166 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dd8af85f..469538bb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -144,6 +144,7 @@ nom = "7" num-traits = "0.2" once_cell = "1.3.1" openssl = "0.10.40" +parking_lot = "0.12" percent-encoding = "2.1" pin-project-lite = "0.2" regex = "1.5.5" diff --git a/pbs-config/Cargo.toml b/pbs-config/Cargo.toml index 74afb3c6..eb81ce00 100644 --- a/pbs-config/Cargo.toml +++ b/pbs-config/Cargo.toml @@ -13,6 +13,7 @@ libc.workspace = true nix.workspace = true once_cell.workspace = true openssl.workspace = true +parking_lot.workspace = true regex.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/pbs-config/src/token_shadow.rs b/pbs-config/src/token_shadow.rs index 640fabbf..ad766671 100644 --- a/pbs-config/src/token_shadow.rs +++ b/pbs-config/src/token_shadow.rs @@ -1,6 +1,8 @@ use std::collections::HashMap; +use std::sync::LazyLock; use anyhow::{bail, format_err, Error}; +use parking_lot::RwLock; use serde::{Deserialize, Serialize}; use serde_json::{from_value, Value}; @@ -13,6 +15,18 @@ use crate::{open_backup_lockfile, BackupLockGuard}; const LOCK_FILE: &str = pbs_buildcfg::configdir!("/token.shadow.lock"); const CONF_FILE: &str = pbs_buildcfg::configdir!("/token.shadow"); +/// 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(), + shared_gen: 0, + }) +}); + #[derive(Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] /// ApiToken id / secret pair @@ -54,9 +68,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 shared generation before doing the hash verification. + let gen_before = token_shadow_shared_gen(); + 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) = gen_before { + cache_try_insert_secret(tokenid.clone(), secret.to_owned(), gen); + } + + Ok(()) + } None => bail!("invalid API token"), } } @@ -75,13 +107,15 @@ 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(()) } @@ -91,11 +125,138 @@ 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(()) } + +/// 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, + /// Shared generation to detect mutations of the underlying token.shadow file. + shared_gen: usize, +} + +impl ApiTokenSecretCache { + /// Resets all local cache contents and sets/updates the cached generation. + fn reset_and_set_gen(&mut self, gen: usize) { + self.secrets.clear(); + self.shared_gen = gen; + } + + /// Caches a secret and sets/updates the cache generation. + fn insert_and_set_gen(&mut self, tokenid: Authid, secret: CachedSecret, gen: usize) { + self.secrets.insert(tokenid, secret); + self.shared_gen = gen; + } + + /// Evicts a cached secret and sets/updates the cached generation. + fn evict_and_set_gen(&mut self, tokenid: &Authid, gen: usize) { + self.secrets.remove(tokenid); + self.shared_gen = gen; + } +} + +fn cache_try_insert_secret(tokenid: Authid, secret: String, shared_gen_before: usize) { + let Some(mut cache) = TOKEN_SECRET_CACHE.try_write() else { + return; + }; + + let Some(shared_gen_now) = token_shadow_shared_gen() else { + return; + }; + + // If this process missed a generation bump, its cache is stale. + if cache.shared_gen != shared_gen_now { + cache.reset_and_set_gen(shared_gen_now); + } + + // If a mutation happened while we were verifying the secret, do not insert. + if shared_gen_now == shared_gen_before { + cache.insert_and_set_gen(tokenid, CachedSecret { secret }, shared_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_shared_gen() else { + return false; + }; + + if current_gen == cache.shared_gen { + return openssl::memcmp::eq(entry.secret.as_bytes(), secret.as_bytes()); + } + + false +} + +fn apply_api_mutation(_guard: BackupLockGuard, tokenid: &Authid, new_secret: Option<&str>) { + // Signal cache invalidation to other processes (best-effort). + let bumped_gen = bump_token_shadow_shared_gen(); + + 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_shared_gen() else { + cache.reset_and_set_gen(0); + return; + }; + + // If we cannot bump the shared 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 new_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 shared generation. +fn token_shadow_shared_gen() -> Option { + crate::ConfigVersionCache::new() + .ok() + .map(|cvc| cvc.token_shadow_generation()) +} + +/// Bump and return the new shared generation. +fn bump_token_shadow_shared_gen() -> Option { + crate::ConfigVersionCache::new() + .ok() + .map(|cvc| cvc.increase_token_shadow_generation() + 1) +} -- 2.47.3