Re: [pbs-devel] [PATCH proxmox-backup 1/3] pbs-config: cache verified API token secrets

[Christian Ebner <c.ebner@proxmox.com>]

On 12/9/25 2:29 PM, Samuel Rufinatscha wrote:
> On 12/5/25 3:03 PM, Shannon Sterz wrote:
>> On Fri Dec 5, 2025 at 2:25 PM CET, Samuel Rufinatscha wrote:
>>> Currently, every token-based API request reads the token.shadow file and
>>> runs the expensive password hash verification for the given token
>>> secret. This shows up as a hotspot in /status profiling (see
>>> bug #6049 [1]).
>>>
>>> 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 partly fixes bug #6049 [1].
>>>
>>> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=7017
>>>
>>> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
>>> ---
>>>   pbs-config/src/token_shadow.rs | 58 +++++++++++++++++++++++++++++++++-
>>>   1 file changed, 57 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/pbs-config/src/token_shadow.rs b/pbs-config/src/ 
>>> token_shadow.rs
>>> index 640fabbf..47aa2fc2 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::RwLock;
>>>
>>>   use anyhow::{bail, format_err, Error};
>>> +use once_cell::sync::OnceCell;
>>>   use serde::{Deserialize, Serialize};
>>>   use serde_json::{from_value, Value};
>>>
>>> @@ -13,6 +15,13 @@ 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: OnceCell<RwLock<ApiTokenSecretCache>> = 
>>> OnceCell::new();
>>
>> any reason you are using a once cell with a cutom get_or_init function
>> instead of a simple `LazyCell` [1] here? seems to me that this would be
>> the more appropriate type here? similar question for the
>> proxmox-access-control portion of this series.
>>
>> [1]: https://doc.rust-lang.org/std/cell/struct.LazyCell.html
>>
> 
> Good point, we should / can directly initialize it! Will change
> to LazyCell. Thanks!

LazyCell is however not thread safe, so could cause issues with 
concurrent inits from different threads. IMO std::sync::LazyLock [0] is 
a better fit here and follows along the line of what we do for other 
caches in PBS, e.g. in pbs-config::user.

[0] https://doc.rust-lang.org/std/sync/struct.LazyLock.html


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