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

["Shannon Sterz" <s.sterz@proxmox.com>]

On Tue Feb 17, 2026 at 12:12 PM CET, 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. A shared generation counter (via
> ConfigVersionCache) is used to invalidate caches across processes when
> token secrets are modified or deleted. This keeps privileged and
> unprivileged daemons in sync.
>
> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
> ---
> Changes from v4 to v5:
> * Rebased
> * Move invalidate_cache_state_and_set_gen into cache object impl
> rename to reset_and_set_gen
> * Add additional insert/remove helpers which set/update the generation
> directly
> * Clarified the  usage of shared generation counter in the commit
> message
>
> 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 | 167 ++++++++++++++++++++++++++++++++-
>  3 files changed, 166 insertions(+), 3 deletions(-)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index dd8af85f..469538bb 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -144,6 +144,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..ad766671 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,138 @@ 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(())
>  }
> +
> +/// 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
> +    /// `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,
> +}
> +
> +impl ApiTokenSecretCache {
> +    /// Resets all local cache contents and sets/updates the cached generation.
> +    fn reset_and_set_gen(&mut self, gen: usize) {
> +        self.secrets.clear();
> +        self.shared_gen = gen;
> +    }
> +
> +    /// Caches a secret and sets/updates the cache generation.
> +    fn insert_and_set_gen(&mut self, tokenid: Authid, secret: CachedSecret, gen: usize) {
> +        self.secrets.insert(tokenid, secret);
> +        self.shared_gen = gen;
> +    }
> +
> +    /// Evicts a cached secret and sets/updates the cached generation.
> +    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) {
> +    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 {
> +        cache.reset_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.insert_and_set_gen(tokenid, CachedSecret { secret }, shared_gen_now);
> +    }
> +}
> +
> +/// 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());

tiny comment here: if we ever allow secrets to have different lengths
this could panic:

> This function will panic the current task if a and b do not have the
> same length.
> - https://docs.rs/openssl/latest/openssl/memcmp/fn.eq.html

might be worth guarding against that or at least documenting that we
expect these to always have the same length.

> +    }
> +
> +    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 {
> +        cache.reset_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) {
> +        cache.reset_and_set_gen(current_gen);
> +        return;
> +    }
> +
> +    // Apply the new mutation.
> +    match new_secret {
> +        Some(secret) => {
> +            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),
> +    }
> +}
> +
> +/// 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)
> +}