From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 556351FF136 for ; Mon, 23 Mar 2026 13:16:50 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8A689151BB; Mon, 23 Mar 2026 13:17:08 +0100 (CET) Message-ID: Date: Mon, 23 Mar 2026 13:16:32 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: partially-applied: [PATCH proxmox{-backup,,-datacenter-manager} v7 00/11] token-shadow: reduce api token verification overhead To: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= , pbs-devel@lists.proxmox.com References: <20260312103708.125282-1-s.rufinatscha@proxmox.com> <1773921586.9wosknv4cp.astroid@yuna.none> Content-Language: en-US From: Samuel Rufinatscha In-Reply-To: <1773921586.9wosknv4cp.astroid@yuna.none> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.235 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: BI5HEKKAKNNDJ3XSDDLOHVGRT44XBL7B X-Message-ID-Hash: BI5HEKKAKNNDJ3XSDDLOHVGRT44XBL7B X-MailFrom: s.rufinatscha@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 3/19/26 1:25 PM, Fabian Grünbichler wrote: > On March 12, 2026 11:36 am, Samuel Rufinatscha wrote: >> Hi, >> >> [..] >> >> 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 > > applied these, with some follow-ups as discussed off-list > Thanks again for applying these and for the provided follow-ups. >> 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 > > skipped these for now - I think the split between the traits there makes > things a bit too intertwined while sort of pretending they are separate. > we probably would have noticed earlier that things aren't cleanly > separated if PDM had wired up user.cfg/acl.cfg caching ;) > > we should probably split the two traits completely, instead of nesting: > - one for the ACL-related parts needed by the UI, guarded by the `acl` > feature > - one for the user.cfg/token.shadow and caching related parts, guarded > by the `impl` feature > > with an `init` call each consuming them (or one consuming the acl one, > and second impl-one consuming both?). > > neither of these traits is used by the product code itself, except for > implementing them to tell proxmox-access-control about product-specific > bits, so having two traits allows properly separating the concerns on > the product side. > > it would probably also make sense to include the follow-ups (which are > mostly renaming things) to make it easier to at some point switch the > PBS code over to proxmox-access-control.. > Fully agree on this, we should really split the traits completely. Thanks for the summary. This will be adjusted as part of the next version. >> 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 >> >> This version and the version before only incorporate the reviewers' >> feedback [4][5][6], also please consider Christian's R-b tag [4]. >> >> [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 >> [4] https://lore.proxmox.com/pbs-devel/20260121151408.731516-1-s.rufinatscha@proxmox.com/T/#t >> [5] https://lore.proxmox.com/pbs-devel/20260217111229.78661-1-s.rufinatscha@proxmox.com/T/#t >> [6] https://lore.proxmox.com/pbs-devel/725687dd-5a35-41ed-af62-6dc9f062cbd4@proxmox.com/T/#t >> >> 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 | 314 ++++++++++++++++++++++++- >> 5 files changed, 335 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 | 315 ++++++++++++++++++++- >> 5 files changed, 413 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, 825 insertions(+), 44 deletions(-) >> >> -- >> Generated by git-murpp 0.8.1 >> >> >> >> >>