From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id DFEFA1FF0B4 for ; Tue, 10 Nov 2026 16:05:23 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 1946021679; Tue, 10 Nov 2026 16:05:19 +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:06 +0100 Message-Id: Subject: Re: [PATCH cluster 1/9] datacenter config: add token-policy option To: "Thomas Lamprecht" , X-Mailer: aerc 0.20.0 References: <20260923210000.4031318-1-t.lamprecht@proxmox.com> <20260923210000.4031318-2-t.lamprecht@proxmox.com> In-Reply-To: <20260923210000.4031318-2-t.lamprecht@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1794323106525 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.640 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: NVYBT56KLAP4LHCL7FFTPQA4BGPIZBH5 X-Message-ID-Hash: NVYBT56KLAP4LHCL7FFTPQA4BGPIZBH5 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 commonly mandate > bounded credential lifetimes, but without a central policy every > token creator must remember to set an expiration date manually. > > Add an optional cluster-wide policy that can require an expiration > date, bound the maximum token lifetime, forbid changing the expiration > date of existing tokens, and require privilege separation for API > tokens. It is enforced by pve-access-control on token creation and > update only; existing tokens deliberately stay untouched, so opting in > does not invalidate already deployed tokens, and (by default) > prolonging a token within the bounds stays possible as a deliberate > act (see #7805). > > Note that an unparsable policy is dropped with a warning and the rest > of datacenter.cfg still loads, the same as for every other property > string here, so a typo leaves tokens unrestricted rather than locked > down. > > Signed-off-by: Thomas Lamprecht > --- > src/PVE/DataCenterConfig.pm | 51 +++++++++++++++++++++++++ > src/test/Makefile | 6 ++- > src/test/test_token_policy.pl | 72 +++++++++++++++++++++++++++++++++++ > 3 files changed, 128 insertions(+), 1 deletion(-) > create mode 100644 src/test/test_token_policy.pl > > diff --git a/src/PVE/DataCenterConfig.pm b/src/PVE/DataCenterConfig.pm > index 004122e..a01ef47 100644 > --- a/src/PVE/DataCenterConfig.pm > +++ b/src/PVE/DataCenterConfig.pm > @@ -268,6 +268,41 @@ my $webauthn_format =3D { > }, > }; > > +my $token_policy_format =3D { > + 'require-expiry' =3D> { > + type =3D> 'boolean', > + optional =3D> 1, > + default =3D> 0, > + description =3D> "Require an expiration date for new API tokens = and when changing the" > + . " expiration date of existing ones.", > + }, > + 'max-lifetime' =3D> { > + type =3D> 'integer', > + optional =3D> 1, > + minimum =3D> 1, > + format_description =3D> 'seconds', > + description =3D> "Maximum lifetime of API tokens in seconds, cou= nted from when the" > + . " expiration date is set, that is on creation or when an u= pdate changes it." > + . " Implies 'require-expiry'.", > + }, > + 'disallow-expiry-changes' =3D> { > + type =3D> 'boolean', > + optional =3D> 1, > + default =3D> 0, > + description =3D> "Disallow changing the expiration date of exist= ing API tokens, so that" > + . " 'max-lifetime' cannot be circumvented by extending token= s repeatedly. Such" > + . " tokens can still be deleted and recreated.", currently this also seems to disallow setting an expiry for old tokens or shortening the lifetime of an existing token with a pre-existing expiry. while this may be intentional, id argue this makes compliance less convenient to achieve. auditors may want an admin to set an expiry asap on a token without one. this option would allow only removing the old token and creating a new one as an option. creating a new token with specific permissions may be cumbersome. since there is no downside to shortening a token lifetime, im not sure what the value here is. especially if we do decide to allow lifetime extensions by secret regeneration. the rational here also only talks about avoid lifetime extension, so at the very least it should maybe also mention why lifetime shortening is also disallowed. > + }, > + 'require-privilege-separation' =3D> { > + type =3D> 'boolean', > + optional =3D> 1, > + default =3D> 0, > + description =3D> "Require privilege separation for new API token= s and when changing that" > + . " setting on existing ones, that is, disallow tokens with = the full privileges of" > + . " their user.", > + }, > +}; > + > PVE::JSONSchema::register_format('mac-prefix', \&pve_verify_mac_prefix); > > sub pve_verify_mac_prefix { > @@ -510,6 +545,13 @@ my $datacenter_schema =3D { > format =3D> $webauthn_format, > description =3D> 'webauthn configuration', > }, > + 'token-policy' =3D> { > + optional =3D> 1, > + type =3D> 'string', > + format =3D> $token_policy_format, > + description =3D> "Cluster-wide policy for creating and updat= ing API tokens, for" > + . " example to enforce an expiration date or privilege s= eparation.", > + }, > description =3D> { > type =3D> 'string', > description =3D> > @@ -614,6 +656,10 @@ sub parse_datacenter_config { > $res->{webauthn} =3D parse_property_string($webauthn_format, $we= bauthn); > } > > + if (my $token_policy =3D $res->{'token-policy'}) { > + $res->{'token-policy'} =3D parse_property_string($token_policy_f= ormat, $token_policy); > + } > + > if (my $tag_style =3D $res->{'tag-style'}) { > $res->{'tag-style'} =3D parse_property_string($tag_style_format,= $tag_style); > } > @@ -707,6 +753,11 @@ sub write_datacenter_config { > $cfg->{webauthn} =3D PVE::JSONSchema::print_property_string($web= authn, $webauthn_format); > } > > + if (ref(my $token_policy =3D $cfg->{'token-policy'})) { > + $cfg->{'token-policy'} =3D > + PVE::JSONSchema::print_property_string($token_policy, $token= _policy_format); > + } > + > if (ref(my $tag_style =3D $cfg->{'tag-style'})) { > $cfg->{'tag-style'} =3D PVE::JSONSchema::print_property_string($= tag_style, $tag_style_format); > } > diff --git a/src/test/Makefile b/src/test/Makefile > index cdd37d0..3a47b8c 100644 > --- a/src/test/Makefile > +++ b/src/test/Makefile > @@ -4,7 +4,7 @@ cpgtest: cpgtest.c > gcc -Wall cpgtest.c $(shell pkg-config --cflags --libs libcpg libqb) -o= cpgtest > > .PHONY: check install clean distclean > -check: corosync-parser-test test-mac-prefix > +check: corosync-parser-test test-mac-prefix test-token-policy > > .PHONY: corosync-parser-test > corosync-parser-test: > @@ -14,5 +14,9 @@ corosync-parser-test: > test-mac-prefix: > perl test_mac_prefix.pl > > +.PHONY: test-token-policy > +test-token-policy: > + perl test_token_policy.pl > + > distclean: clean > clean: > diff --git a/src/test/test_token_policy.pl b/src/test/test_token_policy.p= l > new file mode 100644 > index 0000000..269dc14 > --- /dev/null > +++ b/src/test/test_token_policy.pl > @@ -0,0 +1,72 @@ > +use strict; > +use warnings; small nit: this could just do `use v5.36` instead. the other test cases could adopt this as well, but i won't mention it each time as it's minor. > +use Test::More; > + > +use lib ('.', '..'); > + > +use PVE::DataCenterConfig; > + > +# the policy is a property string, so it has to survive a parse and writ= e round trip and it must > +# not accept values that the enforcing side would not be able to interpr= et > + > +my $parse =3D sub { > + my ($value) =3D @_; > + my $raw =3D defined($value) ? "token-policy: $value\n" : ''; > + my $cfg =3D PVE::DataCenterConfig::parse_datacenter_config('datacent= er.cfg', > + $raw . "keyboard: en-us\n"); > + return $cfg; > +}; > + > +my $policy =3D $parse->('require-expiry=3D1,max-lifetime=3D86400')->{'to= ken-policy'}; > +is($policy->{'require-expiry'}, 1, 'require-expiry is parsed'); > +is($policy->{'max-lifetime'}, 86400, 'max-lifetime is parsed'); > + > +$policy =3D $parse->('disallow-expiry-changes=3D1,require-privilege-sepa= ration=3D1')->{'token-policy'}; > +is($policy->{'disallow-expiry-changes'}, 1, 'disallow-expiry-changes is = parsed'); > +is($policy->{'require-privilege-separation'}, 1, 'require-privilege-sepa= ration is parsed'); > + > +is($parse->(undef)->{'token-policy'}, undef, 'an absent token-policy is = not invented'); > + > +# an unusable policy has to be dropped, never half applied, and must not= take the rest of the > +# configuration down with it > +for my $invalid ( > + 'max-lifetime=3D0', > + 'max-lifetime=3D-1', > + 'max-lifetime=3Dforever', > + 'require-expiry=3Dmaybe', > + 'unknown-key=3D1', > +) { > + my $cfg; > + { > + local $SIG{__WARN__} =3D sub { }; > + $cfg =3D $parse->($invalid); > + } > + is($cfg->{'token-policy'}, undef, "'$invalid' does not yield a polic= y"); > + is($cfg->{keyboard}, 'en-us', "'$invalid' does not break the rest of= the config"); > +} > + > +# writing has to turn the parsed hash back into the property string > +my $raw =3D PVE::DataCenterConfig::write_datacenter_config( > + 'datacenter.cfg', > + { > + 'token-policy' =3D> { > + 'require-expiry' =3D> 1, > + 'max-lifetime' =3D> 86400, > + }, > + }, > +); > +like( > + $raw, > + qr/^token-policy:\s*(require-expiry=3D1,max-lifetime=3D86400|max-lif= etime=3D86400,require-expiry=3D1)$/m, > + 'the policy is written back as a property string', > +); > + > +my $round_tripped =3D PVE::DataCenterConfig::parse_datacenter_config('da= tacenter.cfg', $raw); > +is_deeply( > + $round_tripped->{'token-policy'}, > + { 'require-expiry' =3D> 1, 'max-lifetime' =3D> 86400 }, > + 'the policy survives a write and parse round trip', > +); > + > +done_testing();