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 843271FF137 for ; Tue, 17 Feb 2026 12:11:56 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B286433EAA; Tue, 17 Feb 2026 12:12:46 +0100 (CET) From: Samuel Rufinatscha To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox v5 3/4] proxmox-access-control: invalidate token-secret cache on token.shadow changes Date: Tue, 17 Feb 2026 12:12:24 +0100 Message-ID: <20260217111229.78661-8-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-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1771326748406 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.254 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: 6EKKQMFFJTFEXZXXQNAGDDIX7UJJ7F36 X-Message-ID-Hash: 6EKKQMFFJTFEXZXXQNAGDDIX7UJJ7F36 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: This patch adds manual/direct file change detection by tracking the mtime and length of token.shadow and clears the in-memory token secret cache whenever these values change. Signed-off-by: Samuel Rufinatscha --- Changes from v4 to v5: * Rebased Changes from v3 to v4: * make use of .replace() in refresh_cache_if_file_changed to get previous state * Group file stats with ShadowFileInfo * Return false in refresh_cache_if_file_changed to avoid unnecessary cache queries * Adjusted commit message Changes from v2 to v3: * Cache now tracks last_checked (epoch seconds). * Simplified refresh_cache_if_file_changed, removed FILE_GENERATION logic * On first load, initializes file metadata and keeps empty cache. Changes from v1 to v2: * Add file metadata tracking (file_mtime, file_len) and FILE_GENERATION. * Store file_gen in CachedSecret and verify it against the current FILE_GENERATION to ensure cached entries belong to the current file state. * Add shadow_mtime_len() helper and convert refresh to best-effort (try_write, returns bool). * Pass a pre-write metadata snapshot into apply_api_mutation and clear/bump generation if the cache metadata indicates missed external edits. proxmox-access-control/src/token_shadow.rs | 123 ++++++++++++++++++++- 1 file changed, 119 insertions(+), 4 deletions(-) diff --git a/proxmox-access-control/src/token_shadow.rs b/proxmox-access-control/src/token_shadow.rs index ba5fb3f8..d1b7d4cb 100644 --- a/proxmox-access-control/src/token_shadow.rs +++ b/proxmox-access-control/src/token_shadow.rs @@ -1,5 +1,8 @@ use std::collections::HashMap; +use std::fs; +use std::io::ErrorKind; use std::sync::LazyLock; +use std::time::SystemTime; use anyhow::{bail, format_err, Error}; use parking_lot::RwLock; @@ -7,6 +10,7 @@ use serde_json::{from_value, Value}; use proxmox_auth_api::types::Authid; use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockGuard}; +use proxmox_time::epoch_i64; use crate::init::access_conf; use crate::init::impl_feature::{token_shadow, token_shadow_lock}; @@ -20,6 +24,7 @@ static TOKEN_SECRET_CACHE: LazyLock> = LazyLock::new RwLock::new(ApiTokenSecretCache { secrets: HashMap::new(), shared_gen: 0, + shadow: None, }) }); @@ -45,6 +50,56 @@ fn write_file(data: HashMap) -> Result<(), Error> { replace_config(token_shadow(), &json) } +/// Refreshes the in-memory cache if the on-disk token.shadow file changed. +/// Returns true if the cache is valid to use, false if not. +fn refresh_cache_if_file_changed() -> bool { + let now = epoch_i64(); + + // Best-effort refresh under write lock. + let Some(mut cache) = TOKEN_SECRET_CACHE.try_write() else { + return false; + }; + + let Some(shared_gen_now) = token_shadow_shared_gen() else { + return false; + }; + + // If another process bumped the generation, we don't know what changed -> clear cache + if cache.shared_gen != shared_gen_now { + cache.reset_and_set_gen(shared_gen_now); + } + + // Stat the file to detect manual edits. + let Ok((new_mtime, new_len)) = shadow_mtime_len() else { + return false; + }; + + // If the file didn't change, only update last_checked + if let Some(shadow) = cache.shadow.as_mut() { + if shadow.mtime == new_mtime && shadow.len == new_len { + shadow.last_checked = now; + return true; + } + } + + cache.secrets.clear(); + + let prev = cache.shadow.replace(ShadowFileInfo { + mtime: new_mtime, + len: new_len, + last_checked: now, + }); + + if prev.is_some() { + // Best-effort propagation to other processes if a change was detected + if let Some(shared_gen_new) = bump_token_shadow_shared_gen() { + cache.shared_gen = shared_gen_new; + } + } + + false +} + /// Verifies that an entry for given tokenid / API token secret exists pub fn verify_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> { if !tokenid.is_token() { @@ -52,7 +107,7 @@ pub fn verify_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> { } // Fast path - if cache_try_secret_matches(tokenid, secret) { + if refresh_cache_if_file_changed() && cache_try_secret_matches(tokenid, secret) { return Ok(()); } @@ -84,12 +139,15 @@ pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> { let guard = lock_config()?; + // Capture state before we write to detect external edits. + let pre_meta = shadow_mtime_len().unwrap_or((None, None)); + 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)); + apply_api_mutation(guard, tokenid, Some(secret), pre_meta); Ok(()) } @@ -102,11 +160,14 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> { let guard = lock_config()?; + // Capture state before we write to detect external edits. + let pre_meta = shadow_mtime_len().unwrap_or((None, None)); + let mut data = read_file()?; data.remove(tokenid); write_file(data)?; - apply_api_mutation(guard, tokenid, None); + apply_api_mutation(guard, tokenid, None, pre_meta); Ok(()) } @@ -133,6 +194,8 @@ struct ApiTokenSecretCache { secrets: HashMap, /// Shared generation to detect mutations of the underlying token.shadow file. shared_gen: usize, + /// Shadow file info to detect changes + shadow: Option, } impl ApiTokenSecretCache { @@ -140,6 +203,7 @@ impl ApiTokenSecretCache { fn reset_and_set_gen(&mut self, gen: usize) { self.secrets.clear(); self.shared_gen = gen; + self.shadow = None; } /// Caches a secret and sets/updates the cache generation. @@ -155,6 +219,16 @@ impl ApiTokenSecretCache { } } +/// Shadow file info +struct ShadowFileInfo { + // shadow file mtime to detect changes + mtime: Option, + // shadow file length to detect changes + len: Option, + // last time the file metadata was checked + last_checked: i64, +} + fn cache_try_insert_secret(tokenid: Authid, secret: String, shared_gen_before: usize) { let Some(mut cache) = TOKEN_SECRET_CACHE.try_write() else { return; @@ -199,7 +273,14 @@ fn cache_try_secret_matches(tokenid: &Authid, secret: &str) -> bool { false } -fn apply_api_mutation(_guard: ApiLockGuard, tokenid: &Authid, new_secret: Option<&str>) { +fn apply_api_mutation( + _guard: ApiLockGuard, + tokenid: &Authid, + new_secret: Option<&str>, + pre_write_meta: (Option, Option), +) { + let now = epoch_i64(); + // Signal cache invalidation to other processes (best-effort). let bumped_gen = bump_token_shadow_shared_gen(); @@ -218,6 +299,16 @@ fn apply_api_mutation(_guard: ApiLockGuard, tokenid: &Authid, new_secret: Option return; } + // If our cached file metadata does not match the on-disk state before our write, + // we likely missed an external/manual edit. We can no longer trust any cached secrets. + if cache + .shadow + .as_ref() + .is_some_and(|s| (s.mtime, s.len) != pre_write_meta) + { + cache.secrets.clear(); + } + // Apply the new mutation. match new_secret { Some(secret) => { @@ -228,6 +319,22 @@ fn apply_api_mutation(_guard: ApiLockGuard, tokenid: &Authid, new_secret: Option } None => cache.evict_and_set_gen(tokenid, current_gen), } + + // Update our view of the file metadata to the post-write state (best-effort). + // (If this fails, drop local cache so callers fall back to slow path until refreshed.) + match shadow_mtime_len() { + Ok((mtime, len)) => { + cache.shadow = Some(ShadowFileInfo { + mtime, + len, + last_checked: now, + }); + } + Err(_) => { + // If we cannot validate state, do not trust cache. + cache.reset_and_set_gen(current_gen); + } + } } /// Get the current shared generation. @@ -242,3 +349,11 @@ fn bump_token_shadow_shared_gen() -> Option { .ok() .map(|prev| prev + 1) } + +fn shadow_mtime_len() -> Result<(Option, Option), Error> { + match fs::metadata(token_shadow()) { + Ok(meta) => Ok((meta.modified().ok(), Some(meta.len()))), + Err(e) if e.kind() == ErrorKind::NotFound => Ok((None, None)), + Err(e) => Err(e.into()), + } +} -- 2.47.3