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 26C7F1FF15E for ; Wed, 21 Jan 2026 16:14:35 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 60EA22714; Wed, 21 Jan 2026 16:14:53 +0100 (CET) From: Samuel Rufinatscha To: pbs-devel@lists.proxmox.com Date: Wed, 21 Jan 2026 16:14:03 +0100 Message-ID: <20260121151408.731516-8-s.rufinatscha@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260121151408.731516-1-s.rufinatscha@proxmox.com> References: <20260121151408.731516-1-s.rufinatscha@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1769008397213 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.252 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 Subject: [pbs-devel] [PATCH proxmox v4 3/4] proxmox-access-control: invalidate token-secret cache on token.shadow changes 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: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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 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 e4dfab50..05813b52 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 { + invalidate_cache_state_and_set_gen(&mut cache, 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(()) } @@ -128,6 +189,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, } /// Cached secret. @@ -135,6 +198,16 @@ struct CachedSecret { secret: String, } +/// 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; @@ -179,7 +252,14 @@ fn cache_try_secret_matches(tokenid: &Authid, secret: &str) -> bool { false } -fn apply_api_mutation(_guard: BackupLockGuard, 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(); @@ -198,6 +278,16 @@ fn apply_api_mutation(_guard: BackupLockGuard, tokenid: &Authid, new_secret: Opt 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(); + } + // Update to the post-mutation generation. cache.shared_gen = current_gen; @@ -215,6 +305,22 @@ fn apply_api_mutation(_guard: BackupLockGuard, tokenid: &Authid, new_secret: Opt cache.secrets.remove(tokenid); } } + + // 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. + invalidate_cache_state_and_set_gen(&mut cache, current_gen); + } + } } /// Get the current shared generation. @@ -234,4 +340,13 @@ fn bump_token_shadow_shared_gen() -> Option { fn invalidate_cache_state_and_set_gen(cache: &mut ApiTokenSecretCache, gen: usize) { cache.secrets.clear(); cache.shared_gen = gen; + cache.shadow = None; +} + +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 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel