[pbs-devel] [PATCH proxmox-backup v3 3/4] pbs-config: invalidate token-secret cache on token.shadow changes
Samuel Rufinatscha
s.rufinatscha at proxmox.com
Fri Jan 2 17:07:42 CET 2026
Previously the in-memory token-secret cache was only updated via
set_secret() and delete_secret(), so manual edits to token.shadow were
not reflected.
This patch adds file change detection to the cache. It tracks the mtime
and length of token.shadow and clears the in-memory token secret cache
whenever these values change.
Note, this patch fetches file stats on every request. An TTL-based
optimization will be covered in a subsequent patch of the series.
This patch is part of the series which fixes bug #7017 [1].
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=7017
Signed-off-by: Samuel Rufinatscha <s.rufinatscha at proxmox.com>
---
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.
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.
pbs-config/src/token_shadow.rs | 122 +++++++++++++++++++++++++++++++--
1 file changed, 118 insertions(+), 4 deletions(-)
diff --git a/pbs-config/src/token_shadow.rs b/pbs-config/src/token_shadow.rs
index fa84aee5..02fb191b 100644
--- a/pbs-config/src/token_shadow.rs
+++ b/pbs-config/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::{Deserialize, Serialize};
use serde_json::{from_value, Value};
use proxmox_sys::fs::CreateOptions;
+use proxmox_time::epoch_i64;
use pbs_api_types::Authid;
//use crate::auth;
@@ -24,6 +28,9 @@ static TOKEN_SECRET_CACHE: LazyLock<RwLock<ApiTokenSecretCache>> = LazyLock::new
RwLock::new(ApiTokenSecretCache {
secrets: HashMap::new(),
shared_gen: 0,
+ file_mtime: None,
+ file_len: None,
+ last_checked: None,
})
});
@@ -62,6 +69,63 @@ fn write_file(data: HashMap<Authid, String>) -> Result<(), Error> {
proxmox_sys::fs::replace_file(CONF_FILE, &json, options, true)
}
+/// 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(&mut cache);
+ cache.shared_gen = shared_gen_now;
+ }
+
+ // Stat the file to detect manual edits.
+ let Ok((new_mtime, new_len)) = shadow_mtime_len() else {
+ return false;
+ };
+
+ // Initialize file stats if we have no prior state.
+ if cache.last_checked.is_none() {
+ cache.secrets.clear(); // ensure cache is empty on first load
+ cache.file_mtime = new_mtime;
+ cache.file_len = new_len;
+ cache.last_checked = Some(now);
+ return true;
+ }
+
+ // No change detected.
+ if cache.file_mtime == new_mtime && cache.file_len == new_len {
+ cache.last_checked = Some(now);
+ return true;
+ }
+
+ // Manual edit detected -> invalidate cache and update stat.
+ cache.secrets.clear();
+ cache.file_mtime = new_mtime;
+ cache.file_len = new_len;
+ cache.last_checked = Some(now);
+
+ // Best-effort propagation to other processes + update local view.
+ if let Some(shared_gen_new) = bump_token_shadow_shared_gen() {
+ cache.shared_gen = shared_gen_new;
+ } else {
+ // Do not fail: local cache is already safe as we cleared it above.
+ // Keep local shared_gen as-is to avoid repeated failed attempts.
+ }
+
+ true
+}
+
/// 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() {
@@ -69,7 +133,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(());
}
@@ -109,12 +173,15 @@ 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(tokenid, Some(secret));
+ apply_api_mutation(tokenid, Some(secret), pre_meta);
Ok(())
}
@@ -127,11 +194,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(tokenid, None);
+ apply_api_mutation(tokenid, None, pre_meta);
Ok(())
}
@@ -145,6 +215,12 @@ struct ApiTokenSecretCache {
secrets: HashMap<Authid, CachedSecret>,
/// Shared generation to detect mutations of the underlying token.shadow file.
shared_gen: usize,
+ // shadow file mtime to detect changes
+ file_mtime: Option<SystemTime>,
+ // shadow file length to detect changes
+ file_len: Option<u64>,
+ // last time the file metadata was checked
+ last_checked: Option<i64>,
}
/// Cached secret.
@@ -204,7 +280,13 @@ fn cache_try_secret_matches(tokenid: &Authid, secret: &str) -> bool {
eq && gen2 == cache_gen
}
-fn apply_api_mutation(tokenid: &Authid, new_secret: Option<&str>) {
+fn apply_api_mutation(
+ tokenid: &Authid,
+ new_secret: Option<&str>,
+ pre_write_meta: (Option<SystemTime>, Option<u64>),
+) {
+ let now = epoch_i64();
+
// Signal cache invalidation to other processes (best-effort).
let new_shared_gen = bump_token_shadow_shared_gen();
@@ -220,6 +302,13 @@ fn apply_api_mutation(tokenid: &Authid, new_secret: Option<&str>) {
// Update to the post-mutation generation.
cache.shared_gen = gen;
+ // 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.
+ let (pre_mtime, pre_len) = pre_write_meta;
+ if cache.file_mtime != pre_mtime || cache.file_len != pre_len {
+ cache.secrets.clear();
+ }
+
// Apply the new mutation.
match new_secret {
Some(secret) => {
@@ -234,6 +323,20 @@ fn apply_api_mutation(tokenid: &Authid, new_secret: Option<&str>) {
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.file_mtime = mtime;
+ cache.file_len = len;
+ cache.last_checked = Some(now);
+ }
+ Err(_) => {
+ // If we cannot validate state, do not trust cache.
+ invalidate_cache_state(&mut cache);
+ }
+ }
}
/// Get the current shared generation.
@@ -253,4 +356,15 @@ fn bump_token_shadow_shared_gen() -> Option<usize> {
/// Invalidates the cache state and only keeps the shared generation.
fn invalidate_cache_state(cache: &mut ApiTokenSecretCache) {
cache.secrets.clear();
+ cache.file_mtime = None;
+ cache.file_len = None;
+ cache.last_checked = None;
+}
+
+fn shadow_mtime_len() -> Result<(Option<SystemTime>, Option<u64>), Error> {
+ match fs::metadata(CONF_FILE) {
+ 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
More information about the pbs-devel
mailing list