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

[Samuel Rufinatscha <s.rufinatscha@proxmox.com>]

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

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