[pbs-devel] [PATCH proxmox v3 2/4] proxmox-access-control: cache verified API token secrets
Samuel Rufinatscha
s.rufinatscha at proxmox.com
Fri Jan 2 17:07:45 CET 2026
Currently, every token-based API request reads the token.shadow file and
runs the expensive password hash verification for the given token
secret. This issue was first observed as part of profiling the PBS
/status endpoint (see bug #7017 [1]) and is required for the factored
out proxmox_access_control token_shadow implementation too.
This patch introduces an in-memory cache of successfully verified token
secrets. Subsequent requests for the same token+secret combination only
perform a comparison using openssl::memcmp::eq and avoid re-running the
password hash. The cache is updated when a token secret is set and
cleared when a token is deleted. Note, this does NOT include manual
config changes, which will be covered in a subsequent patch.
This patch is part of the series which fixes bug #7017 [1].
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=7017
Signed-off-by: Samuel Rufinatscha <s.rufinatscha at proxmox.com>
---
Changes from v1 to v2:
* Replace OnceCell with LazyLock, and std::sync::RwLock with
parking_lot::RwLock.
* Add API_MUTATION_GENERATION and guard cache inserts
to prevent “zombie inserts” across concurrent set/delete.
* Refactor cache operations into cache_try_secret_matches,
cache_try_insert_secret, and centralize write-side behavior in
apply_api_mutation.
* Switch fast-path cache access to try_read/try_write (best-effort).
Changes from v2 to v3:
* Replaced process-local cache invalidation (AtomicU64
API_MUTATION_GENERATION) with a cross-process shared generation via
ConfigVersionCache.
* Validate shared generation before/after the constant-time secret
compare; only insert into cache if the generation is unchanged.
* invalidate_cache_state() on insert if shared generation changed.
Cargo.toml | 1 +
proxmox-access-control/Cargo.toml | 1 +
proxmox-access-control/src/token_shadow.rs | 154 ++++++++++++++++++++-
3 files changed, 155 insertions(+), 1 deletion(-)
diff --git a/Cargo.toml b/Cargo.toml
index 27a69afa..59a2ec93 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -112,6 +112,7 @@ native-tls = "0.2"
nix = "0.29"
openssl = "0.10"
pam-sys = "0.5"
+parking_lot = "0.12"
percent-encoding = "2.1"
pin-utils = "0.1.0"
proc-macro2 = "1.0"
diff --git a/proxmox-access-control/Cargo.toml b/proxmox-access-control/Cargo.toml
index ec189664..1de2842c 100644
--- a/proxmox-access-control/Cargo.toml
+++ b/proxmox-access-control/Cargo.toml
@@ -16,6 +16,7 @@ anyhow.workspace = true
const_format.workspace = true
nix = { workspace = true, optional = true }
openssl = { workspace = true, optional = true }
+parking_lot.workspace = true
regex.workspace = true
hex = { workspace = true, optional = true }
serde.workspace = true
diff --git a/proxmox-access-control/src/token_shadow.rs b/proxmox-access-control/src/token_shadow.rs
index c586d834..895309d2 100644
--- a/proxmox-access-control/src/token_shadow.rs
+++ b/proxmox-access-control/src/token_shadow.rs
@@ -1,13 +1,28 @@
use std::collections::HashMap;
+use std::sync::LazyLock;
use anyhow::{bail, format_err, Error};
+use parking_lot::RwLock;
use serde_json::{from_value, Value};
use proxmox_auth_api::types::Authid;
use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockGuard};
+use crate::init::access_conf;
use crate::init::impl_feature::{token_shadow, token_shadow_lock};
+/// Global in-memory cache for successfully verified API token secrets.
+/// The cache stores plain text secrets for token Authids that have already been
+/// verified against the hashed values in `token.shadow`. This allows for cheap
+/// subsequent authentications for the same token+secret combination, avoiding
+/// recomputing the password hash on every request.
+static TOKEN_SECRET_CACHE: LazyLock<RwLock<ApiTokenSecretCache>> = LazyLock::new(|| {
+ RwLock::new(ApiTokenSecretCache {
+ secrets: HashMap::new(),
+ shared_gen: 0,
+ })
+});
+
// Get exclusive lock
fn lock_config() -> Result<ApiLockGuard, Error> {
open_api_lockfile(token_shadow_lock(), None, true)
@@ -36,9 +51,27 @@ pub fn verify_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
bail!("not an API token ID");
}
+ // Fast path
+ if cache_try_secret_matches(tokenid, secret) {
+ return Ok(());
+ }
+
+ // Slow path
+ // First, capture the shared generation before doing the hash verification.
+ let gen_before = token_shadow_shared_gen();
+
let data = read_file()?;
match data.get(tokenid) {
- Some(hashed_secret) => proxmox_sys::crypt::verify_crypt_pw(secret, hashed_secret),
+ Some(hashed_secret) => {
+ proxmox_sys::crypt::verify_crypt_pw(secret, hashed_secret)?;
+
+ // Try to cache only if nothing changed while verifying the secret.
+ if let Some(gen) = gen_before {
+ cache_try_insert_secret(tokenid.clone(), secret.to_owned(), gen);
+ }
+
+ Ok(())
+ }
None => bail!("invalid API token"),
}
}
@@ -56,6 +89,8 @@ pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
data.insert(tokenid.clone(), hashed_secret);
write_file(data)?;
+ apply_api_mutation(tokenid, Some(secret));
+
Ok(())
}
@@ -71,6 +106,8 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> {
data.remove(tokenid);
write_file(data)?;
+ apply_api_mutation(tokenid, None);
+
Ok(())
}
@@ -81,3 +118,118 @@ pub fn generate_and_set_secret(tokenid: &Authid) -> Result<String, Error> {
set_secret(tokenid, &secret)?;
Ok(secret)
}
+
+struct ApiTokenSecretCache {
+ /// Keys are token Authids, values are the corresponding plain text secrets.
+ /// Entries are added after a successful on-disk verification in
+ /// `verify_secret` or when a new token secret is generated by
+ /// `generate_and_set_secret`. Used to avoid repeated
+ /// password-hash computation on subsequent authentications.
+ secrets: HashMap<Authid, CachedSecret>,
+ /// Shared generation to detect mutations of the underlying token.shadow file.
+ shared_gen: usize,
+}
+
+/// Cached secret.
+struct CachedSecret {
+ secret: String,
+}
+
+fn cache_try_insert_secret(tokenid: Authid, secret: String, shared_gen_before: usize) {
+ let Some(mut cache) = TOKEN_SECRET_CACHE.try_write() else {
+ return;
+ };
+
+ let Some(shared_gen_now) = token_shadow_shared_gen() else {
+ return;
+ };
+
+ // If this process missed a generation bump, its cache is stale.
+ if cache.shared_gen != shared_gen_now {
+ invalidate_cache_state(&mut cache);
+ cache.shared_gen = shared_gen_now;
+ }
+
+ // If a mutation happened while we were verifying the secret, do not insert.
+ if shared_gen_now == shared_gen_before {
+ cache.secrets.insert(tokenid, CachedSecret { secret });
+ }
+}
+
+// Tries to match the given token secret against the cached secret.
+// Checks the generation before and after the constant-time compare to avoid a
+// TOCTOU window. If another process rotates/deletes a token while we're validating
+// the cached secret, the generation will change, and we
+// must not trust the cache for this request.
+fn cache_try_secret_matches(tokenid: &Authid, secret: &str) -> bool {
+ let Some(cache) = TOKEN_SECRET_CACHE.try_read() else {
+ return false;
+ };
+ let Some(entry) = cache.secrets.get(tokenid) else {
+ return false;
+ };
+
+ let cache_gen = cache.shared_gen;
+
+ let Some(gen1) = token_shadow_shared_gen() else {
+ return false;
+ };
+ if gen1 != cache_gen {
+ return false;
+ }
+
+ let eq = openssl::memcmp::eq(entry.secret.as_bytes(), secret.as_bytes());
+
+ let Some(gen2) = token_shadow_shared_gen() else {
+ return false;
+ };
+
+ eq && gen2 == cache_gen
+}
+
+fn apply_api_mutation(tokenid: &Authid, new_secret: Option<&str>) {
+ // Signal cache invalidation to other processes (best-effort).
+ let new_shared_gen = bump_token_shadow_shared_gen();
+
+ let mut cache = TOKEN_SECRET_CACHE.write();
+
+ // If we cannot read/bump the shared generation, we cannot safely trust the cache.
+ let Some(gen) = new_shared_gen else {
+ invalidate_cache_state(&mut cache);
+ cache.shared_gen = 0;
+ return;
+ };
+
+ // Update to the post-mutation generation.
+ cache.shared_gen = gen;
+
+ // Apply the new mutation.
+ match new_secret {
+ Some(secret) => {
+ cache.secrets.insert(
+ tokenid.clone(),
+ CachedSecret {
+ secret: secret.to_owned(),
+ },
+ );
+ }
+ None => {
+ cache.secrets.remove(tokenid);
+ }
+ }
+}
+
+/// Get the current shared generation.
+fn token_shadow_shared_gen() -> Option<usize> {
+ access_conf().token_shadow_cache_generation()
+}
+
+/// Bump and return the new shared generation.
+fn bump_token_shadow_shared_gen() -> Option<usize> {
+ access_conf().increment_token_shadow_cache_generation().ok().map(|prev| prev + 1)
+}
+
+/// Invalidates the cache state and only keeps the shared generation.
+fn invalidate_cache_state(cache: &mut ApiTokenSecretCache) {
+ cache.secrets.clear();
+}
\ No newline at end of file
--
2.47.3
More information about the pbs-devel
mailing list