[pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v4 00/11] token-shadow: reduce api token verification overhead

[pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v4 00/11] token-shadow: reduce api token verification overhead

[Samuel Rufinatscha]

Hi,

this series improves the performance of token-based API authentication
in PBS (pbs-config) and in PDM (underlying proxmox-access-control
crate), addressing the API token verification hotspot reported in our
bugtracker #7017 [1].

When profiling PBS /status endpoint with cargo flamegraph [2],
token-based authentication showed up as a dominant hotspot via
proxmox_sys::crypt::verify_crypt_pw. Applying this series removes that
path from the hot section of the flamegraph. The same performance issue
was measured [2] for PDM. PDM uses the underlying shared
proxmox-access-control library for token handling, which is a
factored out version of the token.shadow handling code from PBS.

While this series fixes the immediate performance issue both in PBS
(pbs-config) and in the shared proxmox-access-control crate used by
PDM, PBS should eventually, ideally be refactored, in a separate
effort, to use proxmox-access-control for token handling instead of its
local implementation.

Approach

The goal is to reduce the cost of token-based authentication preserving
the existing token handling semantics (including detecting manual edits
to token.shadow) and be consistent between PBS (pbs-config) and
PDM (proxmox-access-control). For both sites, this series proposes to:

1. Introduce an in-memory cache for verified token secrets and
invalidate it through a shared ConfigVersionCache generation. Note, a
shared generation is required to keep privileged and unprivileged
daemon in sync to avoid caching inconsistencies across processes.
2. Invalidate on token.shadow API changes (set_secret,
delete_secret)
3. Invalidate on direct/manual token.shadow file changes (mtime +
length)
4. Avoid per-request file stat calls using a TTL window

Testing

To verify the effect in PBS (pbs-config changes), I:
1. Set up test environment based on latest PBS ISO, installed Rust
   toolchain, cloned proxmox-backup repository to use with cargo
   flamegraph. Reproduced bug #7017 [1] by profiling the /status
   endpoint with token-based authentication using cargo flamegraph [2].
2. Built PBS with pbs-config patches and re-ran the same workload and
   profiling setup. Confirmed that
   proxmox_sys::crypt::verify_crypt_pw path no longer appears in the
   hot section of the flamegraph. CPU usage is now dominated by TLS
   overhead.
3. Functionally-wise, I verified that:
   * valid tokens authenticate correctly when used in API requests
   * invalid secrets are rejected as before
   * generating a new token secret via dashboard (create token for
   user, regenerate existing secret) works and authenticates correctly

To verify the effect in PDM (proxmox-access-control changes), instead
of PBS’ /status, I profiled the /version endpoint with cargo flamegraph
[2] and verified that the expensive hashing path disappears from the
hot section after introducing caching. Functionally-wise, I verified
that:
   * valid tokens authenticate correctly when used in API requests
   * invalid secrets are rejected as before
   * generating a new token secret via dashboard (create token for user,
   regenerate existing secret) works and authenticates correctly

Benchmarks

Two different benchmarks have been run to measure caching effects
and RwLock contention:

(1) Requests per second for PBS /status endpoint (E2E)

Benchmarked parallel token auth requests for
/status?verbose=0 on top of the datastore lookup cache series [3]
to check throughput impact. With datastores=1, repeat=5000, parallel=16
this series gives ~172 req/s compared to ~65 req/s without it.
This is a ~2.6x improvement (and aligns with the ~179 req/s from the
previous series, which used per-process cache invalidation).

(2) RwLock contention for token create/delete under heavy load of
token-authenticated requests

The previous version of the series compared std::sync::RwLock and
parking_lot::RwLock contention for token create/delete under heavy
parallel token-authenticated readers. parking_lot::RwLock has been
chosen for the added fairness guarantees.

Patch summary

pbs-config:
0001 – pbs-config: add token.shadow generation to ConfigVersionCache
0002 – pbs-config: cache verified API token secrets
0003 – pbs-config: invalidate token-secret cache on token.shadow
changes
0004 – pbs-config: add TTL window to token-secret cache

proxmox-access-control:
0005 – access-control: extend AccessControlConfig for token.shadow invalidation
0006 – access-control: cache verified API token secrets
0007 – access-control: invalidate token-secret cache on token.shadow changes
0008 – access-control: add TTL window to token-secret cache

proxmox-datacenter-manager:
0009 – pdm-config: add token.shadow generation to ConfigVersionCache
0010 – docs: document API token-cache TTL effects
0011 – pdm-config: wire user+acl cache generation

Maintainer notes
* proxmox-access-control trait split: permissions now live in
 AccessControlPermissions, and AccessControlConfig now requires
 fn permissions(&self) -> &dyn AccessControlPermissions ->
 version bump
* Renames ConfigVersionCache`s pub user_cache_generation and
 increase_user_cache_generation -> version bump
* Adds parking_lot::RwLock dependency in PBS and proxmox-access-control

Kind regards,
Samuel Rufinatscha

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=7017
[2] attachment 1767 [1]: Flamegraph showing the proxmox_sys::crypt::verify_crypt_pw stack
[3] https://bugzilla.proxmox.com/show_bug.cgi?id=6049

proxmox-backup:

Samuel Rufinatscha (4):
  pbs-config: add token.shadow generation to ConfigVersionCache
  pbs-config: cache verified API token secrets
  pbs-config: invalidate token-secret cache on token.shadow changes
  pbs-config: add TTL window to token secret cache

 Cargo.toml                             |   1 +
 docs/user-management.rst               |   4 +
 pbs-config/Cargo.toml                  |   1 +
 pbs-config/src/config_version_cache.rs |  18 ++
 pbs-config/src/token_shadow.rs         | 302 ++++++++++++++++++++++++-
 5 files changed, 323 insertions(+), 3 deletions(-)


proxmox:

Samuel Rufinatscha (4):
  proxmox-access-control: split AccessControlConfig and add token.shadow
    gen
  proxmox-access-control: cache verified API token secrets
  proxmox-access-control: invalidate token-secret cache on token.shadow
    changes
  proxmox-access-control: add TTL window to token secret cache

 Cargo.toml                                 |   1 +
 proxmox-access-control/Cargo.toml          |   1 +
 proxmox-access-control/src/acl.rs          |  10 +-
 proxmox-access-control/src/init.rs         | 113 ++++++--
 proxmox-access-control/src/token_shadow.rs | 303 ++++++++++++++++++++-
 5 files changed, 401 insertions(+), 27 deletions(-)


proxmox-datacenter-manager:

Samuel Rufinatscha (3):
  pdm-config: implement token.shadow generation
  docs: document API token-cache TTL effects
  pdm-config: wire user+acl cache generation

 cli/admin/src/main.rs                      |  2 +-
 docs/access-control.rst                    |  4 +++
 lib/pdm-api-types/src/acl.rs               |  4 +--
 lib/pdm-config/Cargo.toml                  |  1 +
 lib/pdm-config/src/access_control.rs       | 31 ++++++++++++++++++++
 lib/pdm-config/src/config_version_cache.rs | 34 +++++++++++++++++-----
 lib/pdm-config/src/lib.rs                  |  2 ++
 server/src/acl.rs                          |  3 +-
 ui/src/main.rs                             | 10 ++++++-
 9 files changed, 77 insertions(+), 14 deletions(-)
 create mode 100644 lib/pdm-config/src/access_control.rs


Summary over all repositories:
  19 files changed, 801 insertions(+), 44 deletions(-)

-- 
Generated by git-murpp 0.8.1


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v4 1/4] pbs-config: add token.shadow generation to ConfigVersionCache

[Samuel Rufinatscha]

Prepares the config version cache to support token_shadow caching.

Safety: the shmem mapping is fixed to 4096 bytes via the #[repr(C)]
union padding, and the new atomic is appended to the end of the
#[repr(C)] inner struct, so all existing field offsets stay unchanged.
Old processes keep accessing the same bytes and new processes consume
previously reserved padding.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
Changes from v3 to v4:
* Rebased
* Adjusted commit message

Changes from v2 to v3:
* Rebased

Changes from v1 to v2:
* Rebased

 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 b875f7e0..399a6f79 100644
--- a/pbs-config/src/config_version_cache.rs
+++ b/pbs-config/src/config_version_cache.rs
@@ -27,6 +27,8 @@ struct ConfigVersionCacheDataInner {
     traffic_control_generation: AtomicUsize,
     // datastore (datastore.cfg) generation/version
     datastore_generation: AtomicUsize,
+    // Token shadow (token.shadow) generation/version.
+    token_shadow_generation: AtomicUsize,
     // Add further atomics here
 }
 
@@ -159,4 +161,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

[pbs-devel] [PATCH proxmox-backup v4 2/4] pbs-config: cache verified API token secrets

[Samuel Rufinatscha]

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.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
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 | 160 ++++++++++++++++++++++++++++++++-
 3 files changed, 159 insertions(+), 3 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 0da18383..aed66fe3 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..d5aa5de2 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"),
     }
 }
@@ -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,131 @@ 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(())
 }
+
+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_and_set_gen(&mut cache, 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.
+///
+/// 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 {
+        invalidate_cache_state_and_set_gen(&mut cache, 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) {
+        invalidate_cache_state_and_set_gen(&mut cache, current_gen);
+        return;
+    }
+
+    // Update to the post-mutation generation.
+    cache.shared_gen = current_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 local cache contents and sets/updates the cached generation.
+fn invalidate_cache_state_and_set_gen(cache: &mut ApiTokenSecretCache, gen: usize) {
+    cache.secrets.clear();
+    cache.shared_gen = gen;
+}
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup v4 2/4] pbs-config: cache verified API token secrets

[Christian Ebner]

one suggestion below

On 1/21/26 4:13 PM, Samuel Rufinatscha wrote:
> 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.
> 
> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
> ---
> 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 | 160 ++++++++++++++++++++++++++++++++-
>   3 files changed, 159 insertions(+), 3 deletions(-)
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index 0da18383..aed66fe3 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..d5aa5de2 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"),
>       }
>   }
> @@ -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,131 @@ 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(())
>   }
> +
> +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_and_set_gen(&mut cache, 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.
> +///
> +/// 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 {
> +        invalidate_cache_state_and_set_gen(&mut cache, 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) {
> +        invalidate_cache_state_and_set_gen(&mut cache, current_gen);
> +        return;
> +    }
> +
> +    // Update to the post-mutation generation.
> +    cache.shared_gen = current_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 local cache contents and sets/updates the cached generation.
> +fn invalidate_cache_state_and_set_gen(cache: &mut ApiTokenSecretCache, gen: usize) {
> +    cache.secrets.clear();
> +    cache.shared_gen = gen;
> +}

above function operates on the chache, so why not make it a method 
thereof? And also bundle the generation bumps, so they might not be 
forgotten.

Something along the lines of the following diff on top of this patch:

diff --git a/pbs-config/src/token_shadow.rs b/pbs-config/src/token_shadow.rs
index d5aa5de28..a8104f142 100644
--- a/pbs-config/src/token_shadow.rs
+++ b/pbs-config/src/token_shadow.rs
@@ -136,6 +136,11 @@ pub fn delete_secret(tokenid: &Authid) -> 
Result<(), Error> {
      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
@@ -147,9 +152,22 @@ struct ApiTokenSecretCache {
      shared_gen: usize,
  }

-/// Cached secret.
-struct CachedSecret {
-    secret: String,
+impl ApiTokenSecretCache {
+    /// Invalidates local cache contents and sets/updates the cached 
generation.
+    fn invalidate_state_and_set_gen(&mut self, gen: usize) {
+        self.secrets.clear();
+        self.shared_gen = gen;
+    }
+
+    fn insert_and_set_gen(&mut self, tokenid: Authid, secret: 
CachedSecret, gen: usize) {
+        self.secrets.insert(tokenid.clone(), secret);
+        self.shared_gen = gen;
+    }
+
+    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) {
@@ -163,12 +181,12 @@ fn cache_try_insert_secret(tokenid: Authid, 
secret: String, shared_gen_before: u

      // If this process missed a generation bump, its cache is stale.
      if cache.shared_gen != shared_gen_now {
-        invalidate_cache_state_and_set_gen(&mut cache, shared_gen_now);
+        cache.invalidate_state_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.secrets.insert(tokenid, CachedSecret { secret });
+        cache.insert_and_set_gen(tokenid, CachedSecret { secret }, 
shared_gen_now);
      }
  }

@@ -204,33 +222,24 @@ fn apply_api_mutation(_guard: BackupLockGuard, 
tokenid: &Authid, new_secret: Opt

      // If we cannot get the current generation, we cannot trust the cache
      let Some(current_gen) = token_shadow_shared_gen() else {
-        invalidate_cache_state_and_set_gen(&mut cache, 0);
+        cache.invalidate_state_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) {
-        invalidate_cache_state_and_set_gen(&mut cache, current_gen);
+        cache.invalidate_state_and_set_gen(current_gen);
          return;
      }

-    // Update to the post-mutation generation.
-    cache.shared_gen = current_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);
+            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),
      }
  }

@@ -248,8 +257,3 @@ fn bump_token_shadow_shared_gen() -> Option<usize> {
          .map(|cvc| cvc.increase_token_shadow_generation() + 1)
  }

-/// Invalidates local cache contents and sets/updates the cached 
generation.
-fn invalidate_cache_state_and_set_gen(cache: &mut ApiTokenSecretCache, 
gen: usize) {
-    cache.secrets.clear();
-    cache.shared_gen = gen;
-}

Re: [pbs-devel] [PATCH proxmox-backup v4 2/4] pbs-config: cache verified API token secrets

[Samuel Rufinatscha]

On 2/10/26 1:52 PM, Christian Ebner wrote:
> one suggestion below
> 
> On 1/21/26 4:13 PM, Samuel Rufinatscha wrote:
>> 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.
>>
>> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
>> ---
>> 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 | 160 ++++++++++++++++++++++++++++++++-
>>   3 files changed, 159 insertions(+), 3 deletions(-)
>>
>> diff --git a/Cargo.toml b/Cargo.toml
>> index 0da18383..aed66fe3 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..d5aa5de2 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"),
>>       }
>>   }
>> @@ -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,131 @@ 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(())
>>   }
>> +
>> +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_and_set_gen(&mut cache, 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.
>> +///
>> +/// 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 {
>> +        invalidate_cache_state_and_set_gen(&mut cache, 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) {
>> +        invalidate_cache_state_and_set_gen(&mut cache, current_gen);
>> +        return;
>> +    }
>> +
>> +    // Update to the post-mutation generation.
>> +    cache.shared_gen = current_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 local cache contents and sets/updates the cached 
>> generation.
>> +fn invalidate_cache_state_and_set_gen(cache: &mut 
>> ApiTokenSecretCache, gen: usize) {
>> +    cache.secrets.clear();
>> +    cache.shared_gen = gen;
>> +}
> 
> above function operates on the chache, so why not make it a method 
> thereof? And also bundle the generation bumps, so they might not be 
> forgotten.

Good catch, makes sense. I’ll adjust!

> 
> Something along the lines of the following diff on top of this patch:
>

Thanks for the code suggestion below, Christian.

> diff --git a/pbs-config/src/token_shadow.rs b/pbs-config/src/ 
> token_shadow.rs
> index d5aa5de28..a8104f142 100644
> --- a/pbs-config/src/token_shadow.rs
> +++ b/pbs-config/src/token_shadow.rs
> @@ -136,6 +136,11 @@ pub fn delete_secret(tokenid: &Authid) -> 
> Result<(), Error> {
>       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
> @@ -147,9 +152,22 @@ struct ApiTokenSecretCache {
>       shared_gen: usize,
>   }
> 
> -/// Cached secret.
> -struct CachedSecret {
> -    secret: String,
> +impl ApiTokenSecretCache {
> +    /// Invalidates local cache contents and sets/updates the cached 
> generation.
> +    fn invalidate_state_and_set_gen(&mut self, gen: usize) {
> +        self.secrets.clear();
> +        self.shared_gen = gen;
> +    }
> +
> +    fn insert_and_set_gen(&mut self, tokenid: Authid, secret: 
> CachedSecret, gen: usize) {
> +        self.secrets.insert(tokenid.clone(), secret);
> +        self.shared_gen = gen;
> +    }
> +
> +    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) {
> @@ -163,12 +181,12 @@ fn cache_try_insert_secret(tokenid: Authid, 
> secret: String, shared_gen_before: u
> 
>       // If this process missed a generation bump, its cache is stale.
>       if cache.shared_gen != shared_gen_now {
> -        invalidate_cache_state_and_set_gen(&mut cache, shared_gen_now);
> +        cache.invalidate_state_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.secrets.insert(tokenid, CachedSecret { secret });
> +        cache.insert_and_set_gen(tokenid, CachedSecret { secret }, 
> shared_gen_now);
>       }
>   }
> 
> @@ -204,33 +222,24 @@ fn apply_api_mutation(_guard: BackupLockGuard, 
> tokenid: &Authid, new_secret: Opt
> 
>       // If we cannot get the current generation, we cannot trust the cache
>       let Some(current_gen) = token_shadow_shared_gen() else {
> -        invalidate_cache_state_and_set_gen(&mut cache, 0);
> +        cache.invalidate_state_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) {
> -        invalidate_cache_state_and_set_gen(&mut cache, current_gen);
> +        cache.invalidate_state_and_set_gen(current_gen);
>           return;
>       }
> 
> -    // Update to the post-mutation generation.
> -    cache.shared_gen = current_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);
> +            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),
>       }
>   }
> 
> @@ -248,8 +257,3 @@ fn bump_token_shadow_shared_gen() -> Option<usize> {
>           .map(|cvc| cvc.increase_token_shadow_generation() + 1)
>   }
> 
> -/// Invalidates local cache contents and sets/updates the cached 
> generation.
> -fn invalidate_cache_state_and_set_gen(cache: &mut ApiTokenSecretCache, 
> gen: usize) {
> -    cache.secrets.clear();
> -    cache.shared_gen = gen;
> -}
> 

[pbs-devel] [PATCH proxmox-backup v4 3/4] pbs-config: invalidate token-secret cache on token.shadow changes

[Samuel Rufinatscha]

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 <s.rufinatscha@proxmox.com>
---
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.

 pbs-config/src/token_shadow.rs | 123 +++++++++++++++++++++++++++++++--
 1 file changed, 119 insertions(+), 4 deletions(-)

diff --git a/pbs-config/src/token_shadow.rs b/pbs-config/src/token_shadow.rs
index d5aa5de2..a5bd1525 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,7 @@ static TOKEN_SECRET_CACHE: LazyLock<RwLock<ApiTokenSecretCache>> = LazyLock::new
     RwLock::new(ApiTokenSecretCache {
         secrets: HashMap::new(),
         shared_gen: 0,
+        shadow: None,
     })
 });
 
@@ -62,6 +67,56 @@ 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_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() {
@@ -69,7 +124,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 +164,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(guard, tokenid, Some(secret));
+    apply_api_mutation(guard, tokenid, Some(secret), pre_meta);
 
     Ok(())
 }
@@ -127,11 +185,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(())
 }
@@ -145,6 +206,8 @@ struct ApiTokenSecretCache {
     secrets: HashMap<Authid, CachedSecret>,
     /// Shared generation to detect mutations of the underlying token.shadow file.
     shared_gen: usize,
+    /// Shadow file info to detect changes
+    shadow: Option<ShadowFileInfo>,
 }
 
 /// Cached secret.
@@ -152,6 +215,16 @@ struct CachedSecret {
     secret: String,
 }
 
+/// Shadow file info
+struct ShadowFileInfo {
+    // shadow file mtime to detect changes
+    mtime: Option<SystemTime>,
+    // shadow file length to detect changes
+    len: Option<u64>,
+    // 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;
@@ -196,7 +269,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: BackupLockGuard,
+    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 bumped_gen = bump_token_shadow_shared_gen();
 
@@ -215,6 +295,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;
 
@@ -232,6 +322,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.
@@ -252,4 +358,13 @@ fn bump_token_shadow_shared_gen() -> Option<usize> {
 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<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

[pbs-devel] [PATCH proxmox-backup v4 4/4] pbs-config: add TTL window to token secret cache

[Samuel Rufinatscha]

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.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
Changes from v3 to 4:
* Adjusted commit message

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.

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.

 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 a5bd1525..24633f6e 100644
--- a/pbs-config/src/token_shadow.rs
+++ b/pbs-config/src/token_shadow.rs
@@ -31,6 +31,8 @@ static TOKEN_SECRET_CACHE: LazyLock<RwLock<ApiTokenSecretCache>> = LazyLock::new
         shadow: 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")]
@@ -72,11 +74,29 @@ 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.shadow.as_ref().is_some_and(|cached| {
+                now >= cached.last_checked
+                    && (now - cached.last_checked) < 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;
     };
@@ -86,6 +106,13 @@ fn refresh_cache_if_file_changed() -> bool {
         invalidate_cache_state_and_set_gen(&mut cache, shared_gen_now);
     }
 
+    // TTL check again after acquiring the lock
+    if cache.shadow.as_ref().is_some_and(|cached| {
+        now >= cached.last_checked && (now - cached.last_checked) < 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

Re: [pbs-devel] [PATCH proxmox-backup v4 4/4] pbs-config: add TTL window to token secret cache

[Christian Ebner]

one suggestion inline

On 1/21/26 4:13 PM, Samuel Rufinatscha wrote:
> 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.
> 
> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
> ---
> Changes from v3 to 4:
> * Adjusted commit message
> 
> 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.
> 
> 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.
> 
>   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 a5bd1525..24633f6e 100644
> --- a/pbs-config/src/token_shadow.rs
> +++ b/pbs-config/src/token_shadow.rs
> @@ -31,6 +31,8 @@ static TOKEN_SECRET_CACHE: LazyLock<RwLock<ApiTokenSecretCache>> = LazyLock::new
>           shadow: 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")]
> @@ -72,11 +74,29 @@ 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

starting here ..

> +            && cache.shadow.as_ref().is_some_and(|cached| {
> +                now >= cached.last_checked
> +                    && (now - cached.last_checked) < TOKEN_SECRET_CACHE_TTL_SECS

... to here is the same as ...

> +            })
> +        {
> +            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;
>       };
> @@ -86,6 +106,13 @@ fn refresh_cache_if_file_changed() -> bool {
>           invalidate_cache_state_and_set_gen(&mut cache, shared_gen_now);
>       }
>   
> +    // TTL check again after acquiring the lock
> +    if cache.shadow.as_ref().is_some_and(|cached| {
> +        now >= cached.last_checked && (now - cached.last_checked) < TOKEN_SECRET_CACHE_TTL_SECS

... this check above. I think this could be defined as method on the 
cache object so it can be easily reused and the code is more readable.

> +    }) {
> +        return true;
> +    }
> +
>       // Stat the file to detect manual edits.
>       let Ok((new_mtime, new_len)) = shadow_mtime_len() else {
>           return false;

Re: [pbs-devel] [PATCH proxmox-backup v4 4/4] pbs-config: add TTL window to token secret cache

[Samuel Rufinatscha]

On 2/10/26 1:57 PM, Christian Ebner wrote:
> one suggestion inline
> 
> On 1/21/26 4:13 PM, Samuel Rufinatscha wrote:
>> 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.
>>
>> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
>> ---
>> Changes from v3 to 4:
>> * Adjusted commit message
>>
>> 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.
>>
>> 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.
>>
>>   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 a5bd1525..24633f6e 100644
>> --- a/pbs-config/src/token_shadow.rs
>> +++ b/pbs-config/src/token_shadow.rs
>> @@ -31,6 +31,8 @@ static TOKEN_SECRET_CACHE: 
>> LazyLock<RwLock<ApiTokenSecretCache>> = LazyLock::new
>>           shadow: 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")]
>> @@ -72,11 +74,29 @@ 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
> 
> starting here ..
> 
>> +            && cache.shadow.as_ref().is_some_and(|cached| {
>> +                now >= cached.last_checked
>> +                    && (now - cached.last_checked) < 
>> TOKEN_SECRET_CACHE_TTL_SECS
> 
> ... to here is the same as ...
> 
>> +            })
>> +        {
>> +            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;
>>       };
>> @@ -86,6 +106,13 @@ fn refresh_cache_if_file_changed() -> bool {
>>           invalidate_cache_state_and_set_gen(&mut cache, shared_gen_now);
>>       }
>> +    // TTL check again after acquiring the lock
>> +    if cache.shadow.as_ref().is_some_and(|cached| {
>> +        now >= cached.last_checked && (now - cached.last_checked) < 
>> TOKEN_SECRET_CACHE_TTL_SECS
> 
> ... this check above. I think this could be defined as method on the 
> cache object so it can be easily reused and the code is more readable.

Makes sense, I’ll update it to match this. Thanks!

> 
>> +    }) {
>> +        return true;
>> +    }
>> +
>>       // Stat the file to detect manual edits.
>>       let Ok((new_mtime, new_len)) = shadow_mtime_len() else {
>>           return false;
> 

[pbs-devel] [PATCH proxmox v4 1/4] proxmox-access-control: split AccessControlConfig and add token.shadow gen

[Samuel Rufinatscha]

Splits AccessControlConfig trait into AccessControlPermissions and
AccessControlConfig traits and adds token.shadow generation support
to AccessControlConfig (provides default impl).

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
Changes from v3 to 4:
* Split AccessControlConfig: introduced AccessControlPermissions to
provide permissions for AccessControlConfig
* Adjusted commit message

 proxmox-access-control/src/acl.rs  |  10 ++-
 proxmox-access-control/src/init.rs | 113 +++++++++++++++++++++++------
 2 files changed, 99 insertions(+), 24 deletions(-)

diff --git a/proxmox-access-control/src/acl.rs b/proxmox-access-control/src/acl.rs
index 38cb7edf..4b4eac09 100644
--- a/proxmox-access-control/src/acl.rs
+++ b/proxmox-access-control/src/acl.rs
@@ -763,7 +763,7 @@ fn privs_to_priv_names(privs: u64) -> Vec<&'static str> {
 mod test {
     use std::{collections::HashMap, sync::OnceLock};
 
-    use crate::init::{init_access_config, AccessControlConfig};
+    use crate::init::{init_access_config, AccessControlConfig, AccessControlPermissions};
 
     use super::AclTree;
     use anyhow::Error;
@@ -775,7 +775,7 @@ mod test {
         roles: HashMap<&'a str, (u64, &'a str)>,
     }
 
-    impl AccessControlConfig for TestAcmConfig<'_> {
+    impl AccessControlPermissions for TestAcmConfig<'_> {
         fn roles(&self) -> &HashMap<&str, (u64, &str)> {
             &self.roles
         }
@@ -793,6 +793,12 @@ mod test {
         }
     }
 
+    impl AccessControlConfig for TestAcmConfig<'_> {
+        fn permissions(&self) -> &dyn AccessControlPermissions {
+            self
+        }
+    }
+
     fn setup_acl_tree_config() {
         static ACL_CONFIG: OnceLock<TestAcmConfig> = OnceLock::new();
         let config = ACL_CONFIG.get_or_init(|| {
diff --git a/proxmox-access-control/src/init.rs b/proxmox-access-control/src/init.rs
index e64398e8..dfd7784b 100644
--- a/proxmox-access-control/src/init.rs
+++ b/proxmox-access-control/src/init.rs
@@ -8,9 +8,8 @@ use proxmox_section_config::SectionConfigData;
 
 static ACCESS_CONF: OnceLock<&'static dyn AccessControlConfig> = OnceLock::new();
 
-/// This trait specifies the functions a product needs to implement to get ACL tree based access
-/// control management from this plugin.
-pub trait AccessControlConfig: Send + Sync {
+/// Provides permission metadata used by access control.
+pub trait AccessControlPermissions: Send + Sync {
     /// Returns a mapping of all recognized privileges and their corresponding `u64` value.
     fn privileges(&self) -> &HashMap<&str, u64>;
 
@@ -32,25 +31,6 @@ pub trait AccessControlConfig: Send + Sync {
         false
     }
 
-    /// Returns the current cache generation of the user and acl configs. If the generation was
-    /// incremented since the last time the cache was queried, the configs are loaded again from
-    /// disk.
-    ///
-    /// Returning `None` will always reload the cache.
-    ///
-    /// Default: Always returns `None`.
-    fn cache_generation(&self) -> Option<usize> {
-        None
-    }
-
-    /// Increment the cache generation of user and acl configs. This indicates that they were
-    /// changed on disk.
-    ///
-    /// Default: Does nothing.
-    fn increment_cache_generation(&self) -> Result<(), Error> {
-        Ok(())
-    }
-
     /// Optionally returns a role that has no access to any resource.
     ///
     /// Default: Returns `None`.
@@ -103,6 +83,95 @@ pub trait AccessControlConfig: Send + Sync {
     }
 }
 
+/// This trait specifies the functions a product needs to implement to get ACL tree based access
+/// control management from this plugin.
+pub trait AccessControlConfig: Send + Sync {
+    /// Return the permissions provider.
+    fn permissions(&self) -> &dyn AccessControlPermissions;
+
+    fn privileges(&self) -> &HashMap<&str, u64> {
+        self.permissions().privileges()
+    }
+
+    fn roles(&self) -> &HashMap<&str, (u64, &str)> {
+        self.permissions().roles()
+    }
+
+    fn is_superuser(&self, auth_id: &Authid) -> bool {
+        self.permissions().is_superuser(auth_id)
+    }
+
+    fn is_group_member(&self, user_id: &Userid, group: &str) -> bool {
+        self.permissions().is_group_member(user_id, group)
+    }
+
+    fn role_no_access(&self) -> Option<&str> {
+        self.permissions().role_no_access()
+    }
+
+    fn role_admin(&self) -> Option<&str> {
+        self.permissions().role_admin()
+    }
+
+    fn init_user_config(&self, config: &mut SectionConfigData) -> Result<(), Error> {
+        self.permissions().init_user_config(config)
+    }
+
+    fn acl_audit_privileges(&self) -> u64 {
+        self.permissions().acl_audit_privileges()
+    }
+
+    fn acl_modify_privileges(&self) -> u64 {
+        self.permissions().acl_modify_privileges()
+    }
+
+    fn check_acl_path(&self, path: &str) -> Result<(), Error> {
+        self.permissions().check_acl_path(path)
+    }
+
+    fn allow_partial_permission_match(&self) -> bool {
+        self.permissions().allow_partial_permission_match()
+    }
+
+    // Cache hooks
+
+    /// Returns the current cache generation of the user and acl configs. If the generation was
+    /// incremented since the last time the cache was queried, the configs are loaded again from
+    /// disk.
+    ///
+    /// Returning `None` will always reload the cache.
+    ///
+    /// Default: Always returns `None`.
+    fn cache_generation(&self) -> Option<usize> {
+        None
+    }
+
+    /// Increment the cache generation of user and acl configs. This indicates that they were
+    /// changed on disk.
+    ///
+    /// Default: Does nothing.
+    fn increment_cache_generation(&self) -> Result<(), Error> {
+        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");
+    }
+}
+
 pub fn init_access_config(config: &'static dyn AccessControlConfig) -> Result<(), Error> {
     ACCESS_CONF
         .set(config)
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox v4 2/4] proxmox-access-control: cache verified API token secrets

[Samuel Rufinatscha]

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.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
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 +
 proxmox-access-control/Cargo.toml          |   1 +
 proxmox-access-control/src/token_shadow.rs | 160 ++++++++++++++++++++-
 3 files changed, 159 insertions(+), 3 deletions(-)

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..e4dfab50 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"),
     }
 }
@@ -49,13 +82,15 @@ pub 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(())
 }
 
@@ -65,12 +100,14 @@ 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(())
 }
 
@@ -81,3 +118,120 @@ 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_and_set_gen(&mut cache, 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.
+///
+/// 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 {
+        invalidate_cache_state_and_set_gen(&mut cache, 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) {
+        invalidate_cache_state_and_set_gen(&mut cache, current_gen);
+        return;
+    }
+
+    // Update to the post-mutation generation.
+    cache.shared_gen = current_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 local cache contents and sets/updates the cached generation.
+fn invalidate_cache_state_and_set_gen(cache: &mut ApiTokenSecretCache, gen: usize) {
+    cache.secrets.clear();
+    cache.shared_gen = gen;
+}
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox v4 2/4] proxmox-access-control: cache verified API token secrets

[Christian Ebner]

On 1/21/26 4:13 PM, Samuel Rufinatscha wrote:
> 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.
> 
> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
> ---

IMHO the commit message should contain a concise additional statement 
specifying the purpose of the shared shadow generation for inter-process 
cache invalidation, as the code in this patch covers that to a 
significant extend.

Forgot to mention this on patch 2 for proxmox-backup, where it applies 
as well.

Re: [pbs-devel] [PATCH proxmox v4 2/4] proxmox-access-control: cache verified API token secrets

[Samuel Rufinatscha]

On 2/10/26 2:36 PM, Christian Ebner wrote:
> On 1/21/26 4:13 PM, Samuel Rufinatscha wrote:
>> 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.
>>
>> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
>> ---
> 
> IMHO the commit message should contain a concise additional statement 
> specifying the purpose of the shared shadow generation for inter-process 
> cache invalidation, as the code in this patch covers that to a 
> significant extend.
> 
> Forgot to mention this on patch 2 for proxmox-backup, where it applies 
> as well.
> 

Agreed, this should be added and will make the implementation a bit
more obvious. Will add also on patch 2, thanks!

[pbs-devel] [PATCH proxmox v4 3/4] proxmox-access-control: invalidate token-secret cache on token.shadow changes

[Samuel Rufinatscha]

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 <s.rufinatscha@proxmox.com>
---
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<RwLock<ApiTokenSecretCache>> = LazyLock::new
     RwLock::new(ApiTokenSecretCache {
         secrets: HashMap::new(),
         shared_gen: 0,
+        shadow: None,
     })
 });
 
@@ -45,6 +50,56 @@ 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_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<Authid, CachedSecret>,
     /// Shared generation to detect mutations of the underlying token.shadow file.
     shared_gen: usize,
+    /// Shadow file info to detect changes
+    shadow: Option<ShadowFileInfo>,
 }
 
 /// Cached secret.
@@ -135,6 +198,16 @@ struct CachedSecret {
     secret: String,
 }
 
+/// Shadow file info
+struct ShadowFileInfo {
+    // shadow file mtime to detect changes
+    mtime: Option<SystemTime>,
+    // shadow file length to detect changes
+    len: Option<u64>,
+    // 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<SystemTime>, Option<u64>),
+) {
+    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<usize> {
 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<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

[pbs-devel] [PATCH proxmox v4 4/4] proxmox-access-control: add TTL window to token secret cache

[Samuel Rufinatscha]

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.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
Changes from v3 to 4:
* Adjusted commit message

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.

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.

 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 05813b52..a361fd72 100644
--- a/proxmox-access-control/src/token_shadow.rs
+++ b/proxmox-access-control/src/token_shadow.rs
@@ -28,6 +28,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)
@@ -55,11 +58,29 @@ 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.shadow.as_ref().is_some_and(|cached| {
+                now >= cached.last_checked
+                    && (now - cached.last_checked) < 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;
     };
@@ -69,6 +90,13 @@ fn refresh_cache_if_file_changed() -> bool {
         invalidate_cache_state_and_set_gen(&mut cache, shared_gen_now);
     }
 
+    // TTL check again after acquiring the lock
+    if cache.shadow.as_ref().is_some_and(|cached| {
+        now >= cached.last_checked && (now - cached.last_checked) < 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

[pbs-devel] [PATCH proxmox-datacenter-manager v4 1/3] pdm-config: implement token.shadow generation

[Samuel Rufinatscha]

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 cache/invalidate
token.shadow secrets.

This patch wires AccessControlConfig to ConfigVersionCache for
token.shadow invalidation and switches server/CLI to use
pdm-config’s AccessControlConfig and UI to use
UiAccessControlConfig.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
Changes from v3 to v4:
* pdm-api-types: replace AccessControlConfig with
AccessControlPermissions and implement init::AccessControlPermissions
there
* pdm-config: add new AccessControlConfig implementing
init::AccessControlConfig
* UI: init uses a local UiAccessControlConfig for init_access_config()
* Adjusted commit message

 cli/admin/src/main.rs                      |  2 +-
 lib/pdm-api-types/src/acl.rs               |  4 ++--
 lib/pdm-config/Cargo.toml                  |  1 +
 lib/pdm-config/src/access_control.rs       | 20 ++++++++++++++++++++
 lib/pdm-config/src/config_version_cache.rs | 18 ++++++++++++++++++
 lib/pdm-config/src/lib.rs                  |  2 ++
 server/src/acl.rs                          |  3 +--
 ui/src/main.rs                             | 10 +++++++++-
 8 files changed, 54 insertions(+), 6 deletions(-)
 create mode 100644 lib/pdm-config/src/access_control.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-api-types/src/acl.rs b/lib/pdm-api-types/src/acl.rs
index 405982a..7c405a7 100644
--- a/lib/pdm-api-types/src/acl.rs
+++ b/lib/pdm-api-types/src/acl.rs
@@ -187,9 +187,9 @@ pub struct AclListItem {
     pub roleid: String,
 }
 
-pub struct AccessControlConfig;
+pub struct AccessControlPermissions;
 
-impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
+impl proxmox_access_control::init::AccessControlPermissions for AccessControlPermissions {
     fn privileges(&self) -> &HashMap<&str, u64> {
         static PRIVS: LazyLock<HashMap<&str, u64>> =
             LazyLock::new(|| PRIVILEGES.iter().copied().collect());
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.rs b/lib/pdm-config/src/access_control.rs
new file mode 100644
index 0000000..389b3f4
--- /dev/null
+++ b/lib/pdm-config/src/access_control.rs
@@ -0,0 +1,20 @@
+use anyhow::Error;
+
+pub struct AccessControlConfig;
+
+impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
+    fn permissions(&self) -> &dyn proxmox_access_control::init::AccessControlPermissions {
+        &pdm_api_types::AccessControlPermissions
+    }
+
+    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..614f7ae 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;
+pub use access_control::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");
diff --git a/ui/src/main.rs b/ui/src/main.rs
index 2bd900e..9f87505 100644
--- a/ui/src/main.rs
+++ b/ui/src/main.rs
@@ -390,10 +390,18 @@ fn main() {
     pwt::state::set_available_languages(proxmox_yew_comp::available_language_list());
 
     if let Err(e) =
-        proxmox_access_control::init::init_access_config(&pdm_api_types::AccessControlConfig)
+        proxmox_access_control::init::init_access_config(&UiAccessControlConfig)
     {
         log::error!("could not initialize access control config - {e:#}");
     }
 
     yew::Renderer::<DatacenterManagerApp>::new().render();
 }
+
+struct UiAccessControlConfig;
+
+impl proxmox_access_control::init::AccessControlConfig for UiAccessControlConfig {
+    fn permissions(&self) -> &dyn proxmox_access_control::init::AccessControlPermissions {
+        &pdm_api_types::AccessControlPermissions
+    }
+}
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-datacenter-manager v4 2/3] docs: document API token-cache TTL effects

[Samuel Rufinatscha]

Documents the effects of the added API token-cache in the
proxmox-access-control crate.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
Changes from v3 to 4:
* Adjusted commit message

 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

[pbs-devel] [PATCH proxmox-datacenter-manager v4 3/3] pdm-config: wire user+acl cache generation

[Samuel Rufinatscha]

Rename ConfigVersionCache’s user_cache_generation to
user_and_acl_generation to match AccessControlConfig::cache_generation
and increment_cache_generation semantics: it expects the same shared
generation for both user and ACL configs.

Safety: no layout change, the shared-memory size and field order remain
unchanged.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 lib/pdm-config/src/access_control.rs       | 11 +++++++++++
 lib/pdm-config/src/config_version_cache.rs | 16 ++++++++--------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/lib/pdm-config/src/access_control.rs b/lib/pdm-config/src/access_control.rs
index 389b3f4..1d498d3 100644
--- a/lib/pdm-config/src/access_control.rs
+++ b/lib/pdm-config/src/access_control.rs
@@ -7,6 +7,17 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
         &pdm_api_types::AccessControlPermissions
     }
 
+    fn cache_generation(&self) -> Option<usize> {
+        crate::ConfigVersionCache::new()
+            .ok()
+            .map(|c| c.user_and_acl_generation())
+    }
+
+    fn increment_cache_generation(&self) -> Result<(), Error> {
+        let c = crate::ConfigVersionCache::new()?;
+        Ok(c.increase_user_and_acl_generation())
+    }
+
     fn token_shadow_cache_generation(&self) -> Option<usize> {
         crate::ConfigVersionCache::new()
             .ok()
diff --git a/lib/pdm-config/src/config_version_cache.rs b/lib/pdm-config/src/config_version_cache.rs
index 933140c..f3d52a0 100644
--- a/lib/pdm-config/src/config_version_cache.rs
+++ b/lib/pdm-config/src/config_version_cache.rs
@@ -21,8 +21,8 @@ use proxmox_shared_memory::*;
 #[repr(C)]
 struct ConfigVersionCacheDataInner {
     magic: [u8; 8],
-    // User (user.cfg) cache generation/version.
-    user_cache_generation: AtomicUsize,
+    // User (user.cfg) and ACL (acl.cfg) generation/version.
+    user_and_acl_generation: AtomicUsize,
     // Traffic control (traffic-control.cfg) generation/version.
     traffic_control_generation: AtomicUsize,
     // Tracks updates to the remote/hostname/nodename mapping cache.
@@ -126,19 +126,19 @@ impl ConfigVersionCache {
         Ok(Arc::new(Self { shmem }))
     }
 
-    /// Returns the user cache generation number.
-    pub fn user_cache_generation(&self) -> usize {
+    /// Returns the user and ACL cache generation number.
+    pub fn user_and_acl_generation(&self) -> usize {
         self.shmem
             .data()
-            .user_cache_generation
+            .user_and_acl_generation
             .load(Ordering::Acquire)
     }
 
-    /// Increase the user cache generation number.
-    pub fn increase_user_cache_generation(&self) {
+    /// Increase the user and ACL cache generation number.
+    pub fn increase_user_and_acl_generation(&self) {
         self.shmem
             .data()
-            .user_cache_generation
+            .user_and_acl_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

Re: [pbs-devel] [PATCH proxmox{-backup, , -datacenter-manager} v4 00/11] token-shadow: reduce api token verification overhead

[Christian Ebner]

Hi, nice work!

Left a few minor suggestions on individual patches, code looks otherwise 
good to me.

A bit less confident when it comes to the traits and changes for PDM, so 
this definitely warrants a second pair of eyes.

With modifications folded in, please consider:

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>

[pbs-devel] superseded: [PATCH proxmox{-backup,,-datacenter-manager} v4 00/11] token-shadow: reduce api token verification overhead

[Samuel Rufinatscha]

https://lore.proxmox.com/pbs-devel/20260217111229.78661-1-s.rufinatscha@proxmox.com/T/#t

On 1/21/26 4:13 PM, Samuel Rufinatscha wrote:
> Hi,
> 
> this series improves the performance of token-based API authentication
> in PBS (pbs-config) and in PDM (underlying proxmox-access-control
> crate), addressing the API token verification hotspot reported in our
> bugtracker #7017 [1].
> 
> When profiling PBS /status endpoint with cargo flamegraph [2],
> token-based authentication showed up as a dominant hotspot via
> proxmox_sys::crypt::verify_crypt_pw. Applying this series removes that
> path from the hot section of the flamegraph. The same performance issue
> was measured [2] for PDM. PDM uses the underlying shared
> proxmox-access-control library for token handling, which is a
> factored out version of the token.shadow handling code from PBS.
> 
> While this series fixes the immediate performance issue both in PBS
> (pbs-config) and in the shared proxmox-access-control crate used by
> PDM, PBS should eventually, ideally be refactored, in a separate
> effort, to use proxmox-access-control for token handling instead of its
> local implementation.
> 
> Approach
> 
> The goal is to reduce the cost of token-based authentication preserving
> the existing token handling semantics (including detecting manual edits
> to token.shadow) and be consistent between PBS (pbs-config) and
> PDM (proxmox-access-control). For both sites, this series proposes to:
> 
> 1. Introduce an in-memory cache for verified token secrets and
> invalidate it through a shared ConfigVersionCache generation. Note, a
> shared generation is required to keep privileged and unprivileged
> daemon in sync to avoid caching inconsistencies across processes.
> 2. Invalidate on token.shadow API changes (set_secret,
> delete_secret)
> 3. Invalidate on direct/manual token.shadow file changes (mtime +
> length)
> 4. Avoid per-request file stat calls using a TTL window
> 
> Testing
> 
> To verify the effect in PBS (pbs-config changes), I:
> 1. Set up test environment based on latest PBS ISO, installed Rust
>     toolchain, cloned proxmox-backup repository to use with cargo
>     flamegraph. Reproduced bug #7017 [1] by profiling the /status
>     endpoint with token-based authentication using cargo flamegraph [2].
> 2. Built PBS with pbs-config patches and re-ran the same workload and
>     profiling setup. Confirmed that
>     proxmox_sys::crypt::verify_crypt_pw path no longer appears in the
>     hot section of the flamegraph. CPU usage is now dominated by TLS
>     overhead.
> 3. Functionally-wise, I verified that:
>     * valid tokens authenticate correctly when used in API requests
>     * invalid secrets are rejected as before
>     * generating a new token secret via dashboard (create token for
>     user, regenerate existing secret) works and authenticates correctly
> 
> To verify the effect in PDM (proxmox-access-control changes), instead
> of PBS’ /status, I profiled the /version endpoint with cargo flamegraph
> [2] and verified that the expensive hashing path disappears from the
> hot section after introducing caching. Functionally-wise, I verified
> that:
>     * valid tokens authenticate correctly when used in API requests
>     * invalid secrets are rejected as before
>     * generating a new token secret via dashboard (create token for user,
>     regenerate existing secret) works and authenticates correctly
> 
> Benchmarks
> 
> Two different benchmarks have been run to measure caching effects
> and RwLock contention:
> 
> (1) Requests per second for PBS /status endpoint (E2E)
> 
> Benchmarked parallel token auth requests for
> /status?verbose=0 on top of the datastore lookup cache series [3]
> to check throughput impact. With datastores=1, repeat=5000, parallel=16
> this series gives ~172 req/s compared to ~65 req/s without it.
> This is a ~2.6x improvement (and aligns with the ~179 req/s from the
> previous series, which used per-process cache invalidation).
> 
> (2) RwLock contention for token create/delete under heavy load of
> token-authenticated requests
> 
> The previous version of the series compared std::sync::RwLock and
> parking_lot::RwLock contention for token create/delete under heavy
> parallel token-authenticated readers. parking_lot::RwLock has been
> chosen for the added fairness guarantees.
> 
> Patch summary
> 
> pbs-config:
> 0001 – pbs-config: add token.shadow generation to ConfigVersionCache
> 0002 – pbs-config: cache verified API token secrets
> 0003 – pbs-config: invalidate token-secret cache on token.shadow
> changes
> 0004 – pbs-config: add TTL window to token-secret cache
> 
> proxmox-access-control:
> 0005 – access-control: extend AccessControlConfig for token.shadow invalidation
> 0006 – access-control: cache verified API token secrets
> 0007 – access-control: invalidate token-secret cache on token.shadow changes
> 0008 – access-control: add TTL window to token-secret cache
> 
> proxmox-datacenter-manager:
> 0009 – pdm-config: add token.shadow generation to ConfigVersionCache
> 0010 – docs: document API token-cache TTL effects
> 0011 – pdm-config: wire user+acl cache generation
> 
> Maintainer notes
> * proxmox-access-control trait split: permissions now live in
>   AccessControlPermissions, and AccessControlConfig now requires
>   fn permissions(&self) -> &dyn AccessControlPermissions ->
>   version bump
> * Renames ConfigVersionCache`s pub user_cache_generation and
>   increase_user_cache_generation -> version bump
> * Adds parking_lot::RwLock dependency in PBS and proxmox-access-control
> 
> Kind regards,
> Samuel Rufinatscha
> 
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=7017
> [2] attachment 1767 [1]: Flamegraph showing the proxmox_sys::crypt::verify_crypt_pw stack
> [3] https://bugzilla.proxmox.com/show_bug.cgi?id=6049
> 
> proxmox-backup:
> 
> Samuel Rufinatscha (4):
>    pbs-config: add token.shadow generation to ConfigVersionCache
>    pbs-config: cache verified API token secrets
>    pbs-config: invalidate token-secret cache on token.shadow changes
>    pbs-config: add TTL window to token secret cache
> 
>   Cargo.toml                             |   1 +
>   docs/user-management.rst               |   4 +
>   pbs-config/Cargo.toml                  |   1 +
>   pbs-config/src/config_version_cache.rs |  18 ++
>   pbs-config/src/token_shadow.rs         | 302 ++++++++++++++++++++++++-
>   5 files changed, 323 insertions(+), 3 deletions(-)
> 
> 
> proxmox:
> 
> Samuel Rufinatscha (4):
>    proxmox-access-control: split AccessControlConfig and add token.shadow
>      gen
>    proxmox-access-control: cache verified API token secrets
>    proxmox-access-control: invalidate token-secret cache on token.shadow
>      changes
>    proxmox-access-control: add TTL window to token secret cache
> 
>   Cargo.toml                                 |   1 +
>   proxmox-access-control/Cargo.toml          |   1 +
>   proxmox-access-control/src/acl.rs          |  10 +-
>   proxmox-access-control/src/init.rs         | 113 ++++++--
>   proxmox-access-control/src/token_shadow.rs | 303 ++++++++++++++++++++-
>   5 files changed, 401 insertions(+), 27 deletions(-)
> 
> 
> proxmox-datacenter-manager:
> 
> Samuel Rufinatscha (3):
>    pdm-config: implement token.shadow generation
>    docs: document API token-cache TTL effects
>    pdm-config: wire user+acl cache generation
> 
>   cli/admin/src/main.rs                      |  2 +-
>   docs/access-control.rst                    |  4 +++
>   lib/pdm-api-types/src/acl.rs               |  4 +--
>   lib/pdm-config/Cargo.toml                  |  1 +
>   lib/pdm-config/src/access_control.rs       | 31 ++++++++++++++++++++
>   lib/pdm-config/src/config_version_cache.rs | 34 +++++++++++++++++-----
>   lib/pdm-config/src/lib.rs                  |  2 ++
>   server/src/acl.rs                          |  3 +-
>   ui/src/main.rs                             | 10 ++++++-
>   9 files changed, 77 insertions(+), 14 deletions(-)
>   create mode 100644 lib/pdm-config/src/access_control.rs
> 
> 
> Summary over all repositories:
>    19 files changed, 801 insertions(+), 44 deletions(-)
>