* [pbs-devel] [PATCH proxmox-backup v3 1/4] pbs-config: add token.shadow generation to ConfigVersionCache
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-backup v3 2/4] pbs-config: cache verified API token secrets Samuel Rufinatscha
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
Currently, every token-based API request reads the token.shadow file and
runs the expensive password hash verification for the given token
secret. This shows up as a hotspot in /status profiling (see
bug #7017 [1]).
To solve the issue, this patch prepares the config version cache,
so that token_shadow_generation config caching can be built on
top of it.
This patch specifically:
(1) implements increment function in order to invalidate generations
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@proxmox.com>
---
pbs-config/src/config_version_cache.rs | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/pbs-config/src/config_version_cache.rs b/pbs-config/src/config_version_cache.rs
index e8fb994f..1376b11d 100644
--- a/pbs-config/src/config_version_cache.rs
+++ b/pbs-config/src/config_version_cache.rs
@@ -28,6 +28,8 @@ struct ConfigVersionCacheDataInner {
// datastore (datastore.cfg) generation/version
// FIXME: remove with PBS 3.0
datastore_generation: AtomicUsize,
+ // Token shadow (token.shadow) generation/version.
+ token_shadow_generation: AtomicUsize,
// Add further atomics here
}
@@ -153,4 +155,20 @@ impl ConfigVersionCache {
.datastore_generation
.fetch_add(1, Ordering::AcqRel)
}
+
+ /// Returns the token shadow generation number.
+ pub fn token_shadow_generation(&self) -> usize {
+ self.shmem
+ .data()
+ .token_shadow_generation
+ .load(Ordering::Acquire)
+ }
+
+ /// Increase the token shadow generation number.
+ pub fn increase_token_shadow_generation(&self) -> usize {
+ self.shmem
+ .data()
+ .token_shadow_generation
+ .fetch_add(1, Ordering::AcqRel)
+ }
}
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 2/4] pbs-config: cache verified API token secrets
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-backup v3 1/4] pbs-config: add token.shadow generation to ConfigVersionCache Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-backup v3 3/4] pbs-config: invalidate token-secret cache on token.shadow changes Samuel Rufinatscha
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
Currently, every token-based API request reads the token.shadow file and
runs the expensive password hash verification for the given token
secret. This shows up as a hotspot in /status profiling (see
bug #7017 [1]).
This patch introduces 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. Note, this does NOT include manual
config changes, which will be covered in a subsequent patch.
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@proxmox.com>
---
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).
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.
Cargo.toml | 1 +
pbs-config/Cargo.toml | 1 +
pbs-config/src/token_shadow.rs | 157 ++++++++++++++++++++++++++++++++-
3 files changed, 158 insertions(+), 1 deletion(-)
diff --git a/Cargo.toml b/Cargo.toml
index 1aa57ae5..821b63b7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -143,6 +143,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..fa84aee5 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<RwLock<ApiTokenSecretCache>> = 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"),
}
}
@@ -82,6 +114,8 @@ fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
data.insert(tokenid.clone(), hashed_secret);
write_file(data)?;
+ apply_api_mutation(tokenid, Some(secret));
+
Ok(())
}
@@ -97,5 +131,126 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> {
data.remove(tokenid);
write_file(data)?;
+ apply_api_mutation(tokenid, None);
+
Ok(())
}
+
+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<Authid, CachedSecret>,
+ /// Shared generation to detect mutations of the underlying token.shadow file.
+ shared_gen: usize,
+}
+
+/// Cached secret.
+struct CachedSecret {
+ secret: String,
+}
+
+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 {
+ invalidate_cache_state(&mut cache);
+ cache.shared_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.secrets.insert(tokenid, CachedSecret { secret });
+ }
+}
+
+// Tries to match the given token secret against the cached secret.
+// Checks the generation before and after the constant-time compare to avoid a
+// TOCTOU window. If another process rotates/deletes a token while we're validating
+// the cached secret, the generation will change, and we
+// must not trust the cache for this request.
+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 cache_gen = cache.shared_gen;
+
+ let Some(gen1) = token_shadow_shared_gen() else {
+ return false;
+ };
+ if gen1 != cache_gen {
+ return false;
+ }
+
+ let eq = openssl::memcmp::eq(entry.secret.as_bytes(), secret.as_bytes());
+
+ let Some(gen2) = token_shadow_shared_gen() else {
+ return false;
+ };
+
+ eq && gen2 == cache_gen
+}
+
+fn apply_api_mutation(tokenid: &Authid, new_secret: Option<&str>) {
+ // Signal cache invalidation to other processes (best-effort).
+ let new_shared_gen = bump_token_shadow_shared_gen();
+
+ let mut cache = TOKEN_SECRET_CACHE.write();
+
+ // If we cannot read/bump the shared generation, we cannot safely trust the cache.
+ let Some(gen) = new_shared_gen else {
+ invalidate_cache_state(&mut cache);
+ cache.shared_gen = 0;
+ return;
+ };
+
+ // Update to the post-mutation generation.
+ cache.shared_gen = gen;
+
+ // Apply the new mutation.
+ match new_secret {
+ Some(secret) => {
+ cache.secrets.insert(
+ tokenid.clone(),
+ CachedSecret {
+ secret: secret.to_owned(),
+ },
+ );
+ }
+ None => {
+ cache.secrets.remove(tokenid);
+ }
+ }
+}
+
+/// Get the current shared generation.
+fn token_shadow_shared_gen() -> Option<usize> {
+ crate::ConfigVersionCache::new()
+ .ok()
+ .map(|cvc| cvc.token_shadow_generation())
+}
+
+/// Bump and return the new shared generation.
+fn bump_token_shadow_shared_gen() -> Option<usize> {
+ crate::ConfigVersionCache::new()
+ .ok()
+ .map(|cvc| cvc.increase_token_shadow_generation() + 1)
+}
+
+/// Invalidates the cache state and only keeps the shared generation.
+fn invalidate_cache_state(cache: &mut ApiTokenSecretCache) {
+ cache.secrets.clear();
+}
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 3/4] pbs-config: invalidate token-secret cache on token.shadow changes
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-backup v3 1/4] pbs-config: add token.shadow generation to ConfigVersionCache Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-backup v3 2/4] pbs-config: cache verified API token secrets Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-backup v3 4/4] pbs-config: add TTL window to token secret cache Samuel Rufinatscha
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
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@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
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 4/4] pbs-config: add TTL window to token secret cache
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
` (2 preceding siblings ...)
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-backup v3 3/4] pbs-config: invalidate token-secret cache on token.shadow changes Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox v3 1/4] proxmox-access-control: extend AccessControlConfig for token.shadow invalidation Samuel Rufinatscha
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
Verify_secret() currently calls refresh_cache_if_file_changed() on every
request, which performs a metadata() call on token.shadow each time.
Under load this adds unnecessary overhead, considering also the file
usually should rarely change.
This patch introduces a TTL boundary, controlled by
TOKEN_SECRET_CACHE_TTL_SECS. File metadata is only re-loaded once the
TTL has expired. Documents TTL effects.
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@proxmox.com>
---
Changes from v1 to v2:
* Add TOKEN_SECRET_CACHE_TTL_SECS and last_checked.
* Implement double-checked TTL: check with try_read first; only attempt
refresh with try_write if expired/unknown.
* Fix TTL bookkeeping: update last_checked on the “file unchanged” path
and after API mutations.
* Add documentation warning about TTL-delayed effect of manual
token.shadow edits.
Changes from v2 to v3:
* Refactored refresh_cache_if_file_changed TTL logic.
* Remove had_prior_state check (replaced by last_checked logic).
* Improve TTL bound checks.
* Reword documentation warning for clarity.
docs/user-management.rst | 4 ++++
pbs-config/src/token_shadow.rs | 29 ++++++++++++++++++++++++++++-
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/docs/user-management.rst b/docs/user-management.rst
index 41b43d60..8dfae528 100644
--- a/docs/user-management.rst
+++ b/docs/user-management.rst
@@ -156,6 +156,10 @@ metadata:
Similarly, the ``user delete-token`` subcommand can be used to delete a token
again.
+.. WARNING:: Direct/manual edits to ``token.shadow`` may take up to 60 seconds (or
+ longer in edge cases) to take effect due to caching. Restart services for
+ immediate effect of manual edits.
+
Newly generated API tokens don't have any permissions. Please read the next
section to learn how to set access permissions.
diff --git a/pbs-config/src/token_shadow.rs b/pbs-config/src/token_shadow.rs
index 02fb191b..e3529b40 100644
--- a/pbs-config/src/token_shadow.rs
+++ b/pbs-config/src/token_shadow.rs
@@ -33,6 +33,8 @@ static TOKEN_SECRET_CACHE: LazyLock<RwLock<ApiTokenSecretCache>> = LazyLock::new
last_checked: None,
})
});
+/// Max age in seconds of the token secret cache before checking for file changes.
+const TOKEN_SECRET_CACHE_TTL_SECS: i64 = 60;
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
@@ -74,11 +76,28 @@ fn write_file(data: HashMap<Authid, String>) -> Result<(), Error> {
fn refresh_cache_if_file_changed() -> bool {
let now = epoch_i64();
- // Best-effort refresh under write lock.
+ // Fast path: cache is fresh if shared-gen matches and TTL not expired.
+ if let (Some(cache), Some(shared_gen_read)) =
+ (TOKEN_SECRET_CACHE.try_read(), token_shadow_shared_gen())
+ {
+ if cache.shared_gen == shared_gen_read
+ && cache
+ .last_checked
+ .is_some_and(|last| now >= last && (now - last) < TOKEN_SECRET_CACHE_TTL_SECS)
+ {
+ return true;
+ }
+ // read lock drops here
+ } else {
+ return false;
+ }
+
+ // Slow path: best-effort refresh under write lock.
let Some(mut cache) = TOKEN_SECRET_CACHE.try_write() else {
return false;
};
+ // Re-read generation after acquiring the lock (may have changed meanwhile).
let Some(shared_gen_now) = token_shadow_shared_gen() else {
return false;
};
@@ -89,6 +108,14 @@ fn refresh_cache_if_file_changed() -> bool {
cache.shared_gen = shared_gen_now;
}
+ // TTL check again after acquiring the lock
+ if cache
+ .last_checked
+ .is_some_and(|last| now >= last && (now - last) < TOKEN_SECRET_CACHE_TTL_SECS)
+ {
+ return true;
+ }
+
// Stat the file to detect manual edits.
let Ok((new_mtime, new_len)) = shadow_mtime_len() else {
return false;
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox v3 1/4] proxmox-access-control: extend AccessControlConfig for token.shadow invalidation
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
` (3 preceding siblings ...)
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-backup v3 4/4] pbs-config: add TTL window to token secret cache Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox v3 2/4] proxmox-access-control: cache verified API token secrets Samuel Rufinatscha
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
Add token_shadow_cache_generation() and
increment_token_shadow_cache_generation()
hooks to AccessControlConfig. This lets products provide a cross-process
invalidation signal for token.shadow so proxmox-access-control can cache
verified API token secrets and invalidate that cache on token
rotation/deletion.
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@proxmox.com>
---
proxmox-access-control/src/init.rs | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/proxmox-access-control/src/init.rs b/proxmox-access-control/src/init.rs
index e64398e8..0ba1a526 100644
--- a/proxmox-access-control/src/init.rs
+++ b/proxmox-access-control/src/init.rs
@@ -51,6 +51,23 @@ pub trait AccessControlConfig: Send + Sync {
Ok(())
}
+ /// Returns the current cache generation of the token shadow cache. If the generation was
+ /// incremented since the last time the cache was queried, the token shadow cache is reloaded
+ /// from disk.
+ ///
+ /// Default: Always returns `None`.
+ fn token_shadow_cache_generation(&self) -> Option<usize> {
+ None
+ }
+
+ /// Increment the cache generation of the token shadow cache. This indicates that it was
+ /// changed on disk.
+ ///
+ /// Default: Returns an error as token shadow generation is not supported.
+ fn increment_token_shadow_cache_generation(&self) -> Result<usize, Error> {
+ anyhow::bail!("token shadow generation not supported");
+ }
+
/// Optionally returns a role that has no access to any resource.
///
/// Default: Returns `None`.
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox v3 2/4] proxmox-access-control: cache verified API token secrets
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
` (4 preceding siblings ...)
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox v3 1/4] proxmox-access-control: extend AccessControlConfig for token.shadow invalidation Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox v3 3/4] proxmox-access-control: invalidate token-secret cache on token.shadow changes Samuel Rufinatscha
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
Currently, every token-based API request reads the token.shadow file and
runs the expensive password hash verification for the given token
secret. This issue was first observed as part of profiling the PBS
/status endpoint (see bug #7017 [1]) and is required for the factored
out proxmox_access_control token_shadow implementation too.
This patch introduces 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. Note, this does NOT include manual
config changes, which will be covered in a subsequent patch.
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@proxmox.com>
---
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).
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.
Cargo.toml | 1 +
proxmox-access-control/Cargo.toml | 1 +
proxmox-access-control/src/token_shadow.rs | 154 ++++++++++++++++++++-
3 files changed, 155 insertions(+), 1 deletion(-)
diff --git a/Cargo.toml b/Cargo.toml
index 27a69afa..59a2ec93 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -112,6 +112,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..895309d2 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_conf;
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<RwLock<ApiTokenSecretCache>> = LazyLock::new(|| {
+ RwLock::new(ApiTokenSecretCache {
+ secrets: HashMap::new(),
+ shared_gen: 0,
+ })
+});
+
// Get exclusive lock
fn lock_config() -> Result<ApiLockGuard, Error> {
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 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"),
}
}
@@ -56,6 +89,8 @@ pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
data.insert(tokenid.clone(), hashed_secret);
write_file(data)?;
+ apply_api_mutation(tokenid, Some(secret));
+
Ok(())
}
@@ -71,6 +106,8 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> {
data.remove(tokenid);
write_file(data)?;
+ apply_api_mutation(tokenid, None);
+
Ok(())
}
@@ -81,3 +118,118 @@ pub fn generate_and_set_secret(tokenid: &Authid) -> Result<String, Error> {
set_secret(tokenid, &secret)?;
Ok(secret)
}
+
+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<Authid, CachedSecret>,
+ /// Shared generation to detect mutations of the underlying token.shadow file.
+ shared_gen: usize,
+}
+
+/// Cached secret.
+struct CachedSecret {
+ secret: String,
+}
+
+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 {
+ invalidate_cache_state(&mut cache);
+ cache.shared_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.secrets.insert(tokenid, CachedSecret { secret });
+ }
+}
+
+// Tries to match the given token secret against the cached secret.
+// Checks the generation before and after the constant-time compare to avoid a
+// TOCTOU window. If another process rotates/deletes a token while we're validating
+// the cached secret, the generation will change, and we
+// must not trust the cache for this request.
+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 cache_gen = cache.shared_gen;
+
+ let Some(gen1) = token_shadow_shared_gen() else {
+ return false;
+ };
+ if gen1 != cache_gen {
+ return false;
+ }
+
+ let eq = openssl::memcmp::eq(entry.secret.as_bytes(), secret.as_bytes());
+
+ let Some(gen2) = token_shadow_shared_gen() else {
+ return false;
+ };
+
+ eq && gen2 == cache_gen
+}
+
+fn apply_api_mutation(tokenid: &Authid, new_secret: Option<&str>) {
+ // Signal cache invalidation to other processes (best-effort).
+ let new_shared_gen = bump_token_shadow_shared_gen();
+
+ let mut cache = TOKEN_SECRET_CACHE.write();
+
+ // If we cannot read/bump the shared generation, we cannot safely trust the cache.
+ let Some(gen) = new_shared_gen else {
+ invalidate_cache_state(&mut cache);
+ cache.shared_gen = 0;
+ return;
+ };
+
+ // Update to the post-mutation generation.
+ cache.shared_gen = gen;
+
+ // Apply the new mutation.
+ match new_secret {
+ Some(secret) => {
+ cache.secrets.insert(
+ tokenid.clone(),
+ CachedSecret {
+ secret: secret.to_owned(),
+ },
+ );
+ }
+ None => {
+ cache.secrets.remove(tokenid);
+ }
+ }
+}
+
+/// Get the current shared generation.
+fn token_shadow_shared_gen() -> Option<usize> {
+ access_conf().token_shadow_cache_generation()
+}
+
+/// Bump and return the new shared generation.
+fn bump_token_shadow_shared_gen() -> Option<usize> {
+ access_conf().increment_token_shadow_cache_generation().ok().map(|prev| prev + 1)
+}
+
+/// Invalidates the cache state and only keeps the shared generation.
+fn invalidate_cache_state(cache: &mut ApiTokenSecretCache) {
+ cache.secrets.clear();
+}
\ No newline at end of file
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox v3 3/4] proxmox-access-control: invalidate token-secret cache on token.shadow changes
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
` (5 preceding siblings ...)
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox v3 2/4] proxmox-access-control: cache verified API token secrets Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox v3 4/4] proxmox-access-control: add TTL window to token secret cache Samuel Rufinatscha
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
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@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.
proxmox-access-control/src/token_shadow.rs | 129 ++++++++++++++++++++-
1 file changed, 123 insertions(+), 6 deletions(-)
diff --git a/proxmox-access-control/src/token_shadow.rs b/proxmox-access-control/src/token_shadow.rs
index 895309d2..f30c8ed5 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,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,
})
});
@@ -45,6 +52,63 @@ fn write_file(data: HashMap<Authid, String>) -> 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(&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() {
@@ -52,7 +116,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 +148,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(tokenid, Some(secret));
+ apply_api_mutation(tokenid, Some(secret), pre_meta);
Ok(())
}
@@ -102,11 +169,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(())
}
@@ -128,6 +198,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.
@@ -187,7 +263,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();
@@ -203,6 +285,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) => {
@@ -217,6 +306,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.
@@ -226,10 +329,24 @@ fn token_shadow_shared_gen() -> Option<usize> {
/// Bump and return the new shared generation.
fn bump_token_shadow_shared_gen() -> Option<usize> {
- access_conf().increment_token_shadow_cache_generation().ok().map(|prev| prev + 1)
+ access_conf()
+ .increment_token_shadow_cache_generation()
+ .ok()
+ .map(|prev| prev + 1)
}
/// Invalidates the cache state and only keeps the shared generation.
fn invalidate_cache_state(cache: &mut ApiTokenSecretCache) {
cache.secrets.clear();
-}
\ No newline at end of file
+ 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(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
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox v3 4/4] proxmox-access-control: add TTL window to token secret cache
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
` (6 preceding siblings ...)
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox v3 3/4] proxmox-access-control: invalidate token-secret cache on token.shadow changes Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-datacenter-manager v3 1/2] pdm-config: implement token.shadow generation Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-datacenter-manager v3 2/2] docs: document API token-cache TTL effects Samuel Rufinatscha
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
Verify_secret() currently calls refresh_cache_if_file_changed() on every
request, which performs a metadata() call on token.shadow each time.
Under load this adds unnecessary overhead, considering also the file
should rarely change.
This patch introduces a TTL boundary, controlled by
TOKEN_SECRET_CACHE_TTL_SECS. File metadata is only re-loaded once the
TTL has expired.
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@proxmox.com>
---
Changes from v1 to v2:
* Add TOKEN_SECRET_CACHE_TTL_SECS and last_checked.
* Implement double-checked TTL: check with try_read first; only attempt
refresh with try_write if expired/unknown.
* Fix TTL bookkeeping: update last_checked on the “file unchanged” path
and after API mutations.
* Add documentation warning about TTL-delayed effect of manual
token.shadow edits.
Changes from v2 to v3:
* Refactored refresh_cache_if_file_changed TTL logic.
* Remove had_prior_state check (replaced by last_checked logic).
* Improve TTL bound checks.
* Reword documentation warning for clarity.
proxmox-access-control/src/token_shadow.rs | 30 +++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/proxmox-access-control/src/token_shadow.rs b/proxmox-access-control/src/token_shadow.rs
index f30c8ed5..14eea560 100644
--- a/proxmox-access-control/src/token_shadow.rs
+++ b/proxmox-access-control/src/token_shadow.rs
@@ -30,6 +30,9 @@ static TOKEN_SECRET_CACHE: LazyLock<RwLock<ApiTokenSecretCache>> = LazyLock::new
})
});
+/// Max age in seconds of the token secret cache before checking for file changes.
+const TOKEN_SECRET_CACHE_TTL_SECS: i64 = 60;
+
// Get exclusive lock
fn lock_config() -> Result<ApiLockGuard, Error> {
open_api_lockfile(token_shadow_lock(), None, true)
@@ -57,11 +60,28 @@ fn write_file(data: HashMap<Authid, String>) -> Result<(), Error> {
fn refresh_cache_if_file_changed() -> bool {
let now = epoch_i64();
- // Best-effort refresh under write lock.
+ // Fast path: cache is fresh if shared-gen matches and TTL not expired.
+ if let (Some(cache), Some(shared_gen_read)) =
+ (TOKEN_SECRET_CACHE.try_read(), token_shadow_shared_gen())
+ {
+ if cache.shared_gen == shared_gen_read
+ && cache
+ .last_checked
+ .is_some_and(|last| now >= last && (now - last) < TOKEN_SECRET_CACHE_TTL_SECS)
+ {
+ return true;
+ }
+ // read lock drops here
+ } else {
+ return false;
+ }
+
+ // Slow path: best-effort refresh under write lock.
let Some(mut cache) = TOKEN_SECRET_CACHE.try_write() else {
return false;
};
+ // Re-read generation after acquiring the lock (may have changed meanwhile).
let Some(shared_gen_now) = token_shadow_shared_gen() else {
return false;
};
@@ -72,6 +92,14 @@ fn refresh_cache_if_file_changed() -> bool {
cache.shared_gen = shared_gen_now;
}
+ // TTL check again after acquiring the lock
+ if cache
+ .last_checked
+ .is_some_and(|last| now >= last && (now - last) < TOKEN_SECRET_CACHE_TTL_SECS)
+ {
+ return true;
+ }
+
// Stat the file to detect manual edits.
let Ok((new_mtime, new_len)) = shadow_mtime_len() else {
return false;
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox-datacenter-manager v3 1/2] pdm-config: implement token.shadow generation
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
` (7 preceding siblings ...)
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox v3 4/4] proxmox-access-control: add TTL window to token secret cache Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-datacenter-manager v3 2/2] docs: document API token-cache TTL effects Samuel Rufinatscha
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
PDM depends on the shared proxmox/proxmox-access-control crate for
token.shadow handling, which expects the product to provide a
cross-process invalidation signal so it can safely cache verified API
token secrets and invalidate them when token.shadow is changed.
This patch
* adds a token_shadow_generation to PDM’s shared-memory
ConfigVersionCache
* implements proxmox_access_control::init::AccessControlConfig
for pdm_config::AccessControlConfig, which
- delegates roles/privs/path checks to the existing
pdm_api_types::AccessControlConfig implementation
- implements the shadow cache generation trait functions
* switches the AccessControlConfig init paths (server + CLI) to use
pdm_config::AccessControlConfig instead of
pdm_api_types::AccessControlConfig
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@proxmox.com>
---
cli/admin/src/main.rs | 2 +-
lib/pdm-config/Cargo.toml | 1 +
lib/pdm-config/src/access_control_config.rs | 73 +++++++++++++++++++++
lib/pdm-config/src/config_version_cache.rs | 18 +++++
lib/pdm-config/src/lib.rs | 2 +
server/src/acl.rs | 3 +-
6 files changed, 96 insertions(+), 3 deletions(-)
create mode 100644 lib/pdm-config/src/access_control_config.rs
diff --git a/cli/admin/src/main.rs b/cli/admin/src/main.rs
index f698fa2..916c633 100644
--- a/cli/admin/src/main.rs
+++ b/cli/admin/src/main.rs
@@ -19,7 +19,7 @@ fn main() {
proxmox_product_config::init(api_user, priv_user);
proxmox_access_control::init::init(
- &pdm_api_types::AccessControlConfig,
+ &pdm_config::AccessControlConfig,
pdm_buildcfg::configdir!("/access"),
)
.expect("failed to setup access control config");
diff --git a/lib/pdm-config/Cargo.toml b/lib/pdm-config/Cargo.toml
index d39c2ad..19781d2 100644
--- a/lib/pdm-config/Cargo.toml
+++ b/lib/pdm-config/Cargo.toml
@@ -13,6 +13,7 @@ once_cell.workspace = true
openssl.workspace = true
serde.workspace = true
+proxmox-access-control.workspace = true
proxmox-config-digest = { workspace = true, features = [ "openssl" ] }
proxmox-http = { workspace = true, features = [ "http-helpers" ] }
proxmox-ldap = { workspace = true, features = [ "types" ]}
diff --git a/lib/pdm-config/src/access_control_config.rs b/lib/pdm-config/src/access_control_config.rs
new file mode 100644
index 0000000..6f2e6b3
--- /dev/null
+++ b/lib/pdm-config/src/access_control_config.rs
@@ -0,0 +1,73 @@
+// e.g. in src/main.rs or server::context mod, wherever convenient
+
+use anyhow::Error;
+use pdm_api_types::{Authid, Userid};
+use proxmox_section_config::SectionConfigData;
+use std::collections::HashMap;
+
+pub struct AccessControlConfig;
+
+impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
+ fn privileges(&self) -> &HashMap<&str, u64> {
+ pdm_api_types::AccessControlConfig.privileges()
+ }
+
+ fn roles(&self) -> &HashMap<&str, (u64, &str)> {
+ pdm_api_types::AccessControlConfig.roles()
+ }
+
+ fn is_superuser(&self, auth_id: &Authid) -> bool {
+ pdm_api_types::AccessControlConfig.is_superuser(auth_id)
+ }
+
+ fn is_group_member(&self, user_id: &Userid, group: &str) -> bool {
+ pdm_api_types::AccessControlConfig.is_group_member(user_id, group)
+ }
+
+ fn role_admin(&self) -> Option<&str> {
+ pdm_api_types::AccessControlConfig.role_admin()
+ }
+
+ fn role_no_access(&self) -> Option<&str> {
+ pdm_api_types::AccessControlConfig.role_no_access()
+ }
+
+ fn init_user_config(&self, config: &mut SectionConfigData) -> Result<(), Error> {
+ pdm_api_types::AccessControlConfig.init_user_config(config)
+ }
+
+ fn acl_audit_privileges(&self) -> u64 {
+ pdm_api_types::AccessControlConfig.acl_audit_privileges()
+ }
+
+ fn acl_modify_privileges(&self) -> u64 {
+ pdm_api_types::AccessControlConfig.acl_modify_privileges()
+ }
+
+ fn check_acl_path(&self, path: &str) -> Result<(), Error> {
+ pdm_api_types::AccessControlConfig.check_acl_path(path)
+ }
+
+ fn allow_partial_permission_match(&self) -> bool {
+ pdm_api_types::AccessControlConfig.allow_partial_permission_match()
+ }
+
+ fn cache_generation(&self) -> Option<usize> {
+ pdm_api_types::AccessControlConfig.cache_generation()
+ }
+
+ fn increment_cache_generation(&self) -> Result<(), Error> {
+ pdm_api_types::AccessControlConfig.increment_cache_generation()
+ }
+
+ fn token_shadow_cache_generation(&self) -> Option<usize> {
+ crate::ConfigVersionCache::new()
+ .ok()
+ .map(|c| c.token_shadow_generation())
+ }
+
+ fn increment_token_shadow_cache_generation(&self) -> Result<usize, Error> {
+ let c = crate::ConfigVersionCache::new()?;
+ Ok(c.increase_token_shadow_generation())
+ }
+}
diff --git a/lib/pdm-config/src/config_version_cache.rs b/lib/pdm-config/src/config_version_cache.rs
index 36a6a77..933140c 100644
--- a/lib/pdm-config/src/config_version_cache.rs
+++ b/lib/pdm-config/src/config_version_cache.rs
@@ -27,6 +27,8 @@ struct ConfigVersionCacheDataInner {
traffic_control_generation: AtomicUsize,
// Tracks updates to the remote/hostname/nodename mapping cache.
remote_mapping_cache: AtomicUsize,
+ // Token shadow (token.shadow) generation/version.
+ token_shadow_generation: AtomicUsize,
// Add further atomics here
}
@@ -172,4 +174,20 @@ impl ConfigVersionCache {
.fetch_add(1, Ordering::Relaxed)
+ 1
}
+
+ /// Returns the token shadow generation number.
+ pub fn token_shadow_generation(&self) -> usize {
+ self.shmem
+ .data()
+ .token_shadow_generation
+ .load(Ordering::Acquire)
+ }
+
+ /// Increase the token shadow generation number.
+ pub fn increase_token_shadow_generation(&self) -> usize {
+ self.shmem
+ .data()
+ .token_shadow_generation
+ .fetch_add(1, Ordering::AcqRel)
+ }
}
diff --git a/lib/pdm-config/src/lib.rs b/lib/pdm-config/src/lib.rs
index 4c49054..a15a006 100644
--- a/lib/pdm-config/src/lib.rs
+++ b/lib/pdm-config/src/lib.rs
@@ -9,6 +9,8 @@ pub mod remotes;
pub mod setup;
pub mod views;
+mod access_control_config;
+pub use access_control_config::AccessControlConfig;
mod config_version_cache;
pub use config_version_cache::ConfigVersionCache;
diff --git a/server/src/acl.rs b/server/src/acl.rs
index f421814..e6e007b 100644
--- a/server/src/acl.rs
+++ b/server/src/acl.rs
@@ -1,6 +1,5 @@
pub(crate) fn init() {
- static ACCESS_CONTROL_CONFIG: pdm_api_types::AccessControlConfig =
- pdm_api_types::AccessControlConfig;
+ static ACCESS_CONTROL_CONFIG: pdm_config::AccessControlConfig = pdm_config::AccessControlConfig;
proxmox_access_control::init::init(&ACCESS_CONTROL_CONFIG, pdm_buildcfg::configdir!("/access"))
.expect("failed to setup access control config");
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread* [pbs-devel] [PATCH proxmox-datacenter-manager v3 2/2] docs: document API token-cache TTL effects
2026-01-02 16:07 [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v3 00/10] token-shadow: reduce api token verification overhead Samuel Rufinatscha
` (8 preceding siblings ...)
2026-01-02 16:07 ` [pbs-devel] [PATCH proxmox-datacenter-manager v3 1/2] pdm-config: implement token.shadow generation Samuel Rufinatscha
@ 2026-01-02 16:07 ` Samuel Rufinatscha
9 siblings, 0 replies; 11+ messages in thread
From: Samuel Rufinatscha @ 2026-01-02 16:07 UTC (permalink / raw)
To: pbs-devel
Documents the effects of the added API token-cache in the
proxmox-access-control crate. This patch is part of the
series that fixes bug #7017 [1].
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@proxmox.com>
---
Changes from v2 to v3:
* Reword documentation warning for clarity.
docs/access-control.rst | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/docs/access-control.rst b/docs/access-control.rst
index adf26cd..18e57a2 100644
--- a/docs/access-control.rst
+++ b/docs/access-control.rst
@@ -47,6 +47,10 @@ place of the user ID (``user@realm``) and the user password, respectively.
The API token is passed from the client to the server by setting the ``Authorization`` HTTP header
with method ``PDMAPIToken`` to the value ``TOKENID:TOKENSECRET``.
+.. WARNING:: Direct/manual edits to ``token.shadow`` may take up to 60 seconds (or
+ longer in edge cases) to take effect due to caching. Restart services for
+ immediate effect of manual edits.
+
.. _access_control:
Access Control
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread