From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id C344D1FF0B4 for ; Tue, 10 Nov 2026 16:05:15 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id EB86821679; Tue, 10 Nov 2026 16:05:09 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 10 Nov 2026 16:05:03 +0100 Message-Id: To: "Thomas Lamprecht" , Subject: Re: [PATCH cluster/access-control/manager/docs/proxmox 0/9] fix #7805: add a datacenter-wide API token policy X-Mailer: aerc 0.20.0 References: <20260923210000.4031318-1-t.lamprecht@proxmox.com> In-Reply-To: <20260923210000.4031318-1-t.lamprecht@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1794323103466 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.644 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: K2XUTI4AK6LDI63Q6IRWOI5SL5AO65KL X-Message-ID-Hash: K2XUTI4AK6LDI63Q6IRWOI5SL5AO65KL X-MailFrom: s.sterz@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 VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed Sep 23, 2026 at 10:59 PM CEST, Thomas Lamprecht wrote: > Compliance rules like PCI DSS, SOC 2, or ISO 27001 often require > that credentials have a limited lifetime, but there is no central way > to enforce that for API tokens. > > Add an optional token-policy option to datacenter.cfg to require an > expiration date, limit the maximum token lifetime, forbid changing > the expiration date of existing tokens, and require privilege > separation. > > The policy is only checked when a token is created or updated, and > only for values that actually change, so existing tokens stay valid even > if their lifetime exceeds a newly configured maximum one. > Extending the expiration date within the limit stays possible, unless > the opt-in 'disallow-expiry-changes' switch is set (see [0] for the > rationale). > > The web UI exposes these options under Datacenter -> Options and > adjusts the token dialogs to the policy. Changing the policy requires > Sys.Modify on '/', like any other datacenter option; an extra > auth-specific privilege check was skipped for now, for the current > built-in roles it would land in the same roles as Sys.Modify, adding new > built-in roles here is IMO scope creep and this can still be done later > (with some more thoughts on how to handle all these datacenter options). > > FWIW, the first manager patch (4/9 overall) fixes a pre-existing issue > w.r.t. only submitting the expiration date when changed, which the > policy checks would expose more often, but it'd also make sense on its > own and could be applied independently already. > > The last two patches port the backend part to the common rust stack, so > PBS and PDM can opt in through their AccessControlConfig, and to prepare > moving pve-access-control over to rust later. Only the expiry parts > apply there, tokens in the rust stack always use separate ACLs, i.e. no > sharing with the underlying users there. > > Dependencies: access-control needs the cluster patch, manager needs > both, and building manager needs the pve-doc-generator with the new > online-help anchor from the docs patch (or ALLOW_MISSING=3D1), which is > why the docs patch comes before the manager ones; the proxmox patches > are independent. d/control needs the respective versioned dependency > bumps added on applying, and the generated datacenter option reference > in pve-docs needs the usual make update once the cluster side is > packaged. > > [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=3D7805 Hi, one high level question: If I didn't miss anything, adding a token policy would break compatibility with PDM to an extend, since its wizard doesn't currently set any expiry. This is mostly relevant when using PDM's wizard for adding remotes and manually adding a token as a workaround should still work, but an expiry policy makes this cumbersome. A nice follow up here might be adding an option that automatically set the current max token lifetime as the expiry. PDM could also be taught to rotate token secrets automatically somewhat analogously to my TLS certificate rotation series. For that an option to allow expiry extension on secret rotation only would be useful. IMO, the security impact of allowing an extension when the secret is regenerated is essentially 0 as creating a new token with a long expiry as is left for the old token is always possible. It would also encourage setting less broad permissions as they don't have to be re-added every time the token expires. What do you think?