Re: [pbs-devel] [PATCH proxmox{-backup, } 0/6] Reduce token.shadow verification overhead

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

Thank you for your great review, Shannon, and for your feedback.

I agree, it would be a good to publicly document the TTL window.

Where would you add this best?

Thanks!

On 12/5/25 3:05 PM, Shannon Sterz wrote:
> thank you for this series and the extensive documentation in it. it was
> very easy to follow. the changes look good to me for the most part, see
> the comments on the first patch. one top level question, though:
> 
> should we publicly document that manually editing the token.shadow will
> now not instantly make requests by tokens invalid, but changes will take
> up to one minute to take effect? i don't think that this is necessarily
> and issue, but imo we shouldn't make such a change without informing
> users.
> 
> other than this and the comments in-line, consider this:
> 
> Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
> 
> On Fri Dec 5, 2025 at 2:25 PM CET, 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 #6049 [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 [3] 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.
>>
>> Problem
>>
>> For token-based API requests, both PBS’s pbs-config token.shadow
>> handling and PDM proxmox-access-control’s token.shadow handling
>> currently:
>>
>> 1. read the token.shadow file on each request
>> 2. deserialize it into a HashMap<Authid, String>
>> 3. run password hash verification via
>>     proxmox_sys::crypt::verify_crypt_pw for the provided token secret
>>
>> Under load, this results in significant CPU usage spent in repeated
>> password hash computations for the same token+secret pairs. The
>> attached flamegraphs for PBS [2] and PDM [3] show
>> proxmox_sys::crypt::verify_crypt_pw dominating the hot path.
>>
>> 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, the series proposes
>> following approach:
>>
>> 1. Introduce an in-memory cache for verified token secrets
>> 2. Invalidate the cache when token.shadow changes (detect manual edits)
>> 3. Control metadata checks with a TTL window
>>
>> Testing
>>
>> *PBS (pbs-config)*
>>
>> To verify the effect in PBS, 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 #6049 [1] by profiling the /status
>>     endpoint with token-based authentication using cargo flamegraph [2].
>>     The flamegraph showed proxmox_sys::crypt::verify_crypt_pw is the
>>     hotspot.
>> 2. Built PBS with pbs-config patches and re-ran the same workload and
>>     profiling setup.
>> 3. Confirmed that the 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.
>> 4. Functionally verified that:
>>     * token-based API authentication still works for valid tokens
>>     * invalid secrets are rejected as before
>>     * generating a new token secret via dashboard works and
>>     authenticates correctly
>>
>> *PDM (proxmox-access-control)*
>>
>> To verify the effect in PDM, I followed a similar testing approach.
>> Instead of /status, I profiled the /version endpoint with cargo
>> flamegraph [3] and verified that the token hashing path disappears
>> from the hot section after applying the proxmox-access-control patches.
>>
>> Functionally I verified that:
>>     * token-based API authentication still works for valid tokens
>>     * invalid secrets are rejected as before
>>     * generating a new token secret via dashboard works and
>>     authenticates correctly
>>
>> Patch summary
>>
>> pbs-config:
>>
>> 0001 – pbs-config: cache verified API token secrets
>> Adds an in-memory cache keyed by Authid that stores plain text token
>> secrets after a successful verification or generation and uses
>> openssl’s memcmp constant-time for comparison.
>>
>> 0002 – pbs-config: invalidate token-secret cache on token.shadow changes
>> Tracks token.shadow mtime and length and clears the in-memory cache
>> when the file changes.
>>
>> 0003 – pbs-config: add TTL window to token-secret cache
>> Introduces a TTL (TOKEN_SECRET_CACHE_TTL_SECS, default 60) for metadata checks so
>> that fs::metadata is only called periodically.
>>
>> proxmox-access-control:
>>
>> 0004 – access-control: cache verified API token secrets
>> Mirrors PBS patch 0001.
>>
>> 0005 – access-control: invalidate token-secret cache on token.shadow changes
>> Mirrors PBS patch 0002.
>>
>> 0006 – access-control: add TTL window to token-secret cache
>> Mirrors PBS patch 0003.
>>
>> Thanks for considering this patch series, I look forward to your
>> feedback.
>>
>> Best,
>> Samuel Rufinatscha
>>
>> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=6049
>> [2] Flamegraph illustrating the`proxmox_sys::crypt::verify_crypt_pw
>> hotspot before this series (attached to [1])
>>
>> proxmox-backup:
>>
>> Samuel Rufinatscha (3):
>>    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
>>
>>   pbs-config/src/token_shadow.rs | 109 ++++++++++++++++++++++++++++++++-
>>   1 file changed, 108 insertions(+), 1 deletion(-)
>>
>>
>> proxmox:
>>
>> Samuel Rufinatscha (3):
>>    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
>>
>>   proxmox-access-control/src/token_shadow.rs | 108 ++++++++++++++++++++-
>>   1 file changed, 107 insertions(+), 1 deletion(-)
>>
>>
>> Summary over all repositories:
>>    2 files changed, 215 insertions(+), 2 deletions(-)
> 



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