[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

[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

[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

[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