all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH proxmox 8/9] access-control: add API token policy type with expiry checks
Date: Wed, 23 Sep 2026 22:59:57 +0200	[thread overview]
Message-ID: <20260923210000.4031318-9-t.lamprecht@proxmox.com> (raw)
In-Reply-To: <20260923210000.4031318-1-t.lamprecht@proxmox.com>

Counterpart of the datacenter-wide token-policy option introduced
for PVE with #7805, so that products building on this crate can
centrally require expiration dates for API tokens, limit the maximum
token lifetime, and forbid changing the expiration date of existing
tokens. Compliance rules like PCI DSS, SOC 2, or ISO 27001 often
require such limits.

Products opt in by overriding the new AccessControlConfig method to
return the policy from their configuration, by default none is
enforced. The check is meant to be called only with an expiration
date an operation actually sets or changes, existing tokens stay
valid on purpose, so opting into a policy does not invalidate already
deployed tokens. Only the expiry-related settings are ported, tokens
in this stack always use separate ACLs, so PVE's privilege
separation setting only becomes relevant once it moves over to
this implementation.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 proxmox-access-control/src/init.rs  |  15 ++
 proxmox-access-control/src/types.rs | 227 ++++++++++++++++++++++++++++
 2 files changed, 242 insertions(+)

diff --git a/proxmox-access-control/src/init.rs b/proxmox-access-control/src/init.rs
index 52c3393a..b8bcdafd 100644
--- a/proxmox-access-control/src/init.rs
+++ b/proxmox-access-control/src/init.rs
@@ -6,6 +6,8 @@ use anyhow::{Error, format_err};
 use proxmox_auth_api::types::{Authid, Userid};
 use proxmox_section_config::SectionConfigData;
 
+use crate::types::TokenPolicy;
+
 static ACCESS_CONF: OnceLock<&'static dyn AccessControlConfig> = OnceLock::new();
 
 /// This trait specifies the functions a product needs to implement to get ACL tree based access
@@ -101,6 +103,19 @@ pub trait AccessControlConfig: Send + Sync {
     fn allow_partial_permission_match(&self) -> bool {
         true
     }
+
+    /// Returns the token policy to enforce when API tokens are created or updated, if any.
+    ///
+    /// Existing tokens are deliberately never re-validated against a policy, so enabling one
+    /// does not invalidate already deployed tokens. Override this to return the policy from the
+    /// product's configuration. Return an error if that configuration cannot be loaded, so token
+    /// creation and expiration-date updates fail instead of silently proceeding without the
+    /// policy.
+    ///
+    /// Default: Returns `Ok(None)`, no policy is enforced.
+    fn token_policy(&self) -> Result<Option<TokenPolicy>, Error> {
+        Ok(None)
+    }
 }
 
 pub fn init_access_config(config: &'static dyn AccessControlConfig) -> Result<(), Error> {
diff --git a/proxmox-access-control/src/types.rs b/proxmox-access-control/src/types.rs
index 875b3d93..7fef5adc 100644
--- a/proxmox-access-control/src/types.rs
+++ b/proxmox-access-control/src/types.rs
@@ -1,3 +1,4 @@
+use anyhow::{Error, bail};
 use serde::{Deserialize, Serialize};
 
 use const_format::concatcp;
@@ -174,6 +175,82 @@ pub struct TokenApiEntry {
     pub token: ApiToken,
 }
 
+#[api(
+    properties: {
+        "require-expiry": {
+            optional: true,
+            default: false,
+        },
+        "max-lifetime": {
+            optional: true,
+            minimum: 1,
+        },
+        "disallow-expiry-changes": {
+            optional: true,
+            default: false,
+        },
+    },
+)]
+#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, Updater)]
+#[serde(rename_all = "kebab-case")]
+/// Policy for the creation and update of API tokens.
+///
+/// The policy is enforced when API tokens are created or updated only, existing tokens are
+/// deliberately not affected, so enabling a policy does not invalidate already deployed tokens.
+pub struct TokenPolicy {
+    /// Require an expiration date for new API tokens and when changing the expiration date of
+    /// existing ones.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub require_expiry: Option<bool>,
+
+    /// Maximum lifetime of API tokens in seconds, counted from when the expiration date is set,
+    /// that is on creation or when an update changes it. Implies `require-expiry`.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub max_lifetime: Option<i64>,
+
+    /// Disallow changing the expiration date of existing API tokens, so that `max-lifetime`
+    /// cannot be circumvented by extending tokens repeatedly. Such tokens can still be deleted
+    /// and recreated.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub disallow_expiry_changes: Option<bool>,
+}
+
+impl TokenPolicy {
+    /// Check a new or changed token expiration date against the policy.
+    ///
+    /// Must only be called when an operation actually sets or changes the expiration date, with
+    /// an `expire` of 0 meaning no expiration date. This lets callers enforce the policy only for
+    /// the expiration date an operation sets, so unrelated updates do not retroactively reject
+    /// tokens that predate the policy. `is_update` distinguishes updating an existing token from
+    /// creating a new one, `now` is the reference time the maximum lifetime is bound against.
+    pub fn check_expiry_change(&self, expire: i64, is_update: bool, now: i64) -> Result<(), Error> {
+        if is_update && self.disallow_expiry_changes.unwrap_or(false) {
+            bail!(
+                "the token policy does not allow changing the expiration date of an existing token"
+            );
+        }
+
+        // a token that never expires trivially exceeds any finite maximum lifetime, so a set
+        // maximum lifetime also requires an expiration date
+        if expire == 0 && (self.require_expiry.unwrap_or(false) || self.max_lifetime.is_some()) {
+            bail!("the token policy requires an expiration date");
+        }
+
+        if let Some(max_lifetime) = self.max_lifetime {
+            // an extreme configured lifetime must not overflow, which panics in debug builds and
+            // wraps into a bound rejecting everything otherwise
+            if expire > 0 && expire > now.saturating_add(max_lifetime) {
+                bail!(
+                    "the expiration date exceeds the maximum token lifetime of {max_lifetime} \
+                     seconds set by the token policy"
+                );
+            }
+        }
+
+        Ok(())
+    }
+}
+
 #[api(
     properties: {
         userid: {
@@ -316,3 +393,153 @@ pub const REGENERATE_TOKEN_SCHEMA: Schema =
     BooleanSchema::new("Regenerate token secret while keeping permissions.")
         .default(false)
         .schema();
+
+#[cfg(test)]
+mod tests {
+    use super::TokenPolicy;
+
+    // fixed reference time and a 90 day maximum lifetime for deterministic bounds checks
+    const NOW: i64 = 1_000_000_000;
+    const MAX: i64 = 90 * 24 * 60 * 60;
+
+    const EMPTY: TokenPolicy = TokenPolicy {
+        require_expiry: None,
+        max_lifetime: None,
+        disallow_expiry_changes: None,
+    };
+
+    const REQUIRE_EXPIRY: TokenPolicy = TokenPolicy {
+        require_expiry: Some(true),
+        ..EMPTY
+    };
+
+    const MAX_LIFETIME: TokenPolicy = TokenPolicy {
+        max_lifetime: Some(MAX),
+        ..EMPTY
+    };
+
+    const FIXED_EXPIRY: TokenPolicy = TokenPolicy {
+        disallow_expiry_changes: Some(true),
+        ..EMPTY
+    };
+
+    #[test]
+    fn empty_policy_allows_everything() {
+        assert!(EMPTY.check_expiry_change(0, false, NOW).is_ok());
+        assert!(EMPTY.check_expiry_change(NOW + 100, false, NOW).is_ok());
+        assert!(EMPTY.check_expiry_change(0, true, NOW).is_ok());
+    }
+
+    #[test]
+    fn require_expiry() {
+        assert!(REQUIRE_EXPIRY.check_expiry_change(0, false, NOW).is_err());
+        assert!(
+            REQUIRE_EXPIRY
+                .check_expiry_change(NOW + 100, false, NOW)
+                .is_ok()
+        );
+        // an update removing the expiration date is a change and gets rejected as well
+        assert!(REQUIRE_EXPIRY.check_expiry_change(0, true, NOW).is_err());
+        // while a compliant expiration date change on update stays allowed
+        assert!(
+            REQUIRE_EXPIRY
+                .check_expiry_change(NOW + 100, true, NOW)
+                .is_ok()
+        );
+    }
+
+    #[test]
+    fn max_lifetime_implies_require_expiry() {
+        assert!(MAX_LIFETIME.check_expiry_change(0, false, NOW).is_err());
+        let explicitly_not_required = TokenPolicy {
+            require_expiry: Some(false),
+            ..MAX_LIFETIME
+        };
+        assert!(
+            explicitly_not_required
+                .check_expiry_change(0, false, NOW)
+                .is_err()
+        );
+    }
+
+    #[test]
+    fn max_lifetime_bounds() {
+        assert!(
+            MAX_LIFETIME
+                .check_expiry_change(NOW + 100, false, NOW)
+                .is_ok()
+        );
+        assert!(
+            MAX_LIFETIME
+                .check_expiry_change(NOW + MAX, false, NOW)
+                .is_ok()
+        );
+        assert!(
+            MAX_LIFETIME
+                .check_expiry_change(NOW + MAX + 1, false, NOW)
+                .is_err()
+        );
+        // prolonging past the bound on update is rejected just the same
+        assert!(
+            MAX_LIFETIME
+                .check_expiry_change(NOW + MAX + 1, true, NOW)
+                .is_err()
+        );
+        // an expiration date in the past is odd, but within any bound
+        assert!(
+            MAX_LIFETIME
+                .check_expiry_change(NOW - 100, false, NOW)
+                .is_ok()
+        );
+        // prolonging within the bound on update stays allowed
+        assert!(
+            MAX_LIFETIME
+                .check_expiry_change(NOW + 100, true, NOW)
+                .is_ok()
+        );
+    }
+
+    #[test]
+    fn extreme_max_lifetime_does_not_overflow() {
+        let policy = TokenPolicy {
+            max_lifetime: Some(i64::MAX),
+            ..EMPTY
+        };
+        assert!(policy.check_expiry_change(NOW + 100, false, NOW).is_ok());
+        assert!(policy.check_expiry_change(i64::MAX, false, NOW).is_ok());
+    }
+
+    #[test]
+    fn disallow_expiry_changes() {
+        // any change on update is rejected: prolonging, shortening and removal
+        assert!(
+            FIXED_EXPIRY
+                .check_expiry_change(NOW + 100, true, NOW)
+                .is_err()
+        );
+        assert!(
+            FIXED_EXPIRY
+                .check_expiry_change(NOW - 100, true, NOW)
+                .is_err()
+        );
+        assert!(FIXED_EXPIRY.check_expiry_change(0, true, NOW).is_err());
+        // creation is unaffected, with or without an expiration date
+        assert!(
+            FIXED_EXPIRY
+                .check_expiry_change(NOW + 100, false, NOW)
+                .is_ok()
+        );
+        assert!(FIXED_EXPIRY.check_expiry_change(0, false, NOW).is_ok());
+    }
+
+    #[test]
+    fn disallow_expiry_changes_wins_over_compliant_expiry() {
+        let policy = TokenPolicy {
+            require_expiry: Some(true),
+            disallow_expiry_changes: Some(true),
+            ..EMPTY
+        };
+        assert!(policy.check_expiry_change(NOW + 100, true, NOW).is_err());
+        assert!(policy.check_expiry_change(NOW + 100, false, NOW).is_ok());
+    }
+}
-- 
2.47.3





  parent reply	other threads:[~2026-09-23 21:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-23 20:59 [PATCH cluster/access-control/manager/docs/proxmox 0/9] fix #7805: add a datacenter-wide API token policy Thomas Lamprecht
2026-09-23 20:59 ` [PATCH cluster 1/9] datacenter config: add token-policy option Thomas Lamprecht
2026-09-23 20:59 ` [PATCH access-control 2/9] fix #7805: api: token: enforce datacenter token policy Thomas Lamprecht
2026-09-23 20:59 ` [PATCH docs 3/9] user management: document the API " Thomas Lamprecht
2026-09-23 20:59 ` [PATCH manager 4/9] ui: token edit: only submit the expiration date when changed Thomas Lamprecht
2026-09-23 20:59 ` [PATCH manager 5/9] api: cluster options: return token-policy without Sys.Audit Thomas Lamprecht
2026-09-23 20:59 ` [PATCH manager 6/9] ui: dc options: allow editing the API token policy Thomas Lamprecht
2026-09-23 20:59 ` [PATCH manager 7/9] ui: token edit: adapt to the datacenter " Thomas Lamprecht
2026-09-23 20:59 ` Thomas Lamprecht [this message]
2026-09-23 20:59 ` [PATCH proxmox 9/9] access-control: enforce token policy on token create and update Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260923210000.4031318-9-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal