From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id EC5EA1FF183 for ; Wed, 22 Oct 2025 15:11:08 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C5AC3188F9; Wed, 22 Oct 2025 15:11:34 +0200 (CEST) From: Shannon Sterz To: pdm-devel@lists.proxmox.com Date: Wed, 22 Oct 2025 15:11:18 +0200 Message-ID: <20251022131126.358790-3-s.sterz@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251022131126.358790-1-s.sterz@proxmox.com> References: <20251022131126.358790-1-s.sterz@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761138681645 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.056 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pdm-devel] [PATCH proxmox v2 2/4] access-control: move functions querying privileges to the AclTree X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" instead of keeping them in the CachedUserInfo. there is nothing that's specific to the CachedUserInfo in these functions and having them in the AclTree makes it possible to use them with the `acl` feature only too. to keep backward compatability, we keep the original functions in CachedUserInfo but make them call the functions on the AclTree. Signed-off-by: Shannon Sterz --- proxmox-access-control/src/acl.rs | 109 +++++++++++++++++- .../src/cached_user_info.rs | 91 +-------------- 2 files changed, 114 insertions(+), 86 deletions(-) diff --git a/proxmox-access-control/src/acl.rs b/proxmox-access-control/src/acl.rs index 877b003c..9fb97f55 100644 --- a/proxmox-access-control/src/acl.rs +++ b/proxmox-access-control/src/acl.rs @@ -16,7 +16,6 @@ use proxmox_config_digest::ConfigDigest; #[cfg(feature = "impl")] use proxmox_product_config::{open_api_lockfile, replace_privileged_config, ApiLockGuard}; -use crate::init::{access_conf, acl_config, acl_config_lock}; use crate::init::access_conf; #[cfg(feature = "impl")] use crate::init::{acl_config, acl_config_lock}; @@ -563,6 +562,102 @@ impl AclTree { Ok(res) } + + pub fn check_privs( + &self, + auth_id: &Authid, + path: &[&str], + required_privs: u64, + partial: bool, + ) -> Result<(), Error> { + let privs = self.lookup_privs(auth_id, path); + let allowed = if partial { + (privs & required_privs) != 0 + } else { + (privs & required_privs) == required_privs + }; + if !allowed { + // printing the path doesn't leak any information as long as we + // always check privilege before resource existence + let priv_names = privs_to_priv_names(required_privs); + let priv_names = if partial { + priv_names.join("|") + } else { + priv_names.join("&") + }; + bail!( + "missing permissions '{priv_names}' on '/{}'", + path.join("/") + ); + } + Ok(()) + } + + pub fn lookup_privs(&self, auth_id: &Authid, path: &[&str]) -> u64 { + let (privs, _) = self.lookup_privs_details(auth_id, path); + privs + } + + pub fn lookup_privs_details(&self, auth_id: &Authid, path: &[&str]) -> (u64, u64) { + if access_conf().is_superuser(auth_id) { + let acm_config = access_conf(); + if let Some(admin) = acm_config.role_admin() { + if let Some((admin, _)) = acm_config.roles().get(admin) { + return (*admin, *admin); + } + } + } + + let roles = self.roles(auth_id, path); + let mut privs: u64 = 0; + let mut propagated_privs: u64 = 0; + for (role, propagate) in roles { + if let Some((role_privs, _)) = access_conf().roles().get(role.as_str()) { + if propagate { + propagated_privs |= role_privs; + } + privs |= role_privs; + } + } + + if auth_id.is_token() { + // limit privs to that of owning user + let user_auth_id = Authid::from(auth_id.user().clone()); + let (owner_privs, owner_propagated_privs) = + self.lookup_privs_details(&user_auth_id, path); + privs &= owner_privs; + propagated_privs &= owner_propagated_privs; + } + + (privs, propagated_privs) + } + + /// Checks whether the `auth_id` has any of the privileges `privs` on any object below `path`. + pub fn any_privs_below( + &self, + auth_id: &Authid, + path: &[&str], + privs: u64, + ) -> Result { + // if the anchor path itself has matching propagated privs, we skip checking children + let (_privs, propagated_privs) = self.lookup_privs_details(auth_id, path); + if propagated_privs & privs != 0 { + return Ok(true); + } + + // get all sub-paths with roles defined for `auth_id` + let paths = self.get_child_paths(auth_id, path)?; + + for path in paths.iter() { + // early return if any sub-path has any of the privs we are looking for + if privs & self.lookup_privs(auth_id, &[path.as_str()]) != 0 { + return Ok(true); + } + } + + // no paths or no matching paths + Ok(false) + } } /// Get exclusive lock @@ -651,6 +746,18 @@ pub fn save_config(acl: &AclTree) -> Result<(), Error> { Ok(()) } +fn privs_to_priv_names(privs: u64) -> Vec<&'static str> { + access_conf() + .privileges() + .iter() + .fold(Vec::new(), |mut priv_names, (name, value)| { + if value & privs != 0 { + priv_names.push(name); + } + priv_names + }) +} + #[cfg(test)] mod test { use std::{collections::HashMap, sync::OnceLock}; diff --git a/proxmox-access-control/src/cached_user_info.rs b/proxmox-access-control/src/cached_user_info.rs index f5ed2858..8db37727 100644 --- a/proxmox-access-control/src/cached_user_info.rs +++ b/proxmox-access-control/src/cached_user_info.rs @@ -2,7 +2,7 @@ use std::sync::{Arc, OnceLock, RwLock}; -use anyhow::{bail, Error}; +use anyhow::Error; use proxmox_auth_api::types::{Authid, Userid}; use proxmox_router::UserInformation; @@ -118,66 +118,16 @@ impl CachedUserInfo { required_privs: u64, partial: bool, ) -> Result<(), Error> { - let privs = self.lookup_privs(auth_id, path); - let allowed = if partial { - (privs & required_privs) != 0 - } else { - (privs & required_privs) == required_privs - }; - if !allowed { - // printing the path doesn't leak any information as long as we - // always check privilege before resource existence - let priv_names = privs_to_priv_names(required_privs); - let priv_names = if partial { - priv_names.join("|") - } else { - priv_names.join("&") - }; - bail!( - "missing permissions '{priv_names}' on '/{}'", - path.join("/") - ); - } - Ok(()) + self.acl_tree + .check_privs(auth_id, path, required_privs, partial) } pub fn lookup_privs(&self, auth_id: &Authid, path: &[&str]) -> u64 { - let (privs, _) = self.lookup_privs_details(auth_id, path); - privs + self.acl_tree.lookup_privs(auth_id, path) } pub fn lookup_privs_details(&self, auth_id: &Authid, path: &[&str]) -> (u64, u64) { - if self.is_superuser(auth_id) { - let acm_config = access_conf(); - if let Some(admin) = acm_config.role_admin() { - if let Some((admin, _)) = acm_config.roles().get(admin) { - return (*admin, *admin); - } - } - } - - let roles = self.acl_tree.roles(auth_id, path); - let mut privs: u64 = 0; - let mut propagated_privs: u64 = 0; - for (role, propagate) in roles { - if let Some((role_privs, _)) = access_conf().roles().get(role.as_str()) { - if propagate { - propagated_privs |= role_privs; - } - privs |= role_privs; - } - } - - if auth_id.is_token() { - // limit privs to that of owning user - let user_auth_id = Authid::from(auth_id.user().clone()); - let (owner_privs, owner_propagated_privs) = - self.lookup_privs_details(&user_auth_id, path); - privs &= owner_privs; - propagated_privs &= owner_propagated_privs; - } - - (privs, propagated_privs) + self.acl_tree.lookup_privs_details(auth_id, path) } /// Checks whether the `auth_id` has any of the privileges `privs` on any object below `path`. @@ -187,24 +137,7 @@ impl CachedUserInfo { path: &[&str], privs: u64, ) -> Result { - // if the anchor path itself has matching propagated privs, we skip checking children - let (_privs, propagated_privs) = self.lookup_privs_details(auth_id, path); - if propagated_privs & privs != 0 { - return Ok(true); - } - - // get all sub-paths with roles defined for `auth_id` - let paths = self.acl_tree.get_child_paths(auth_id, path)?; - - for path in paths.iter() { - // early return if any sub-path has any of the privs we are looking for - if privs & self.lookup_privs(auth_id, &[path.as_str()]) != 0 { - return Ok(true); - } - } - - // no paths or no matching paths - Ok(false) + self.acl_tree.any_privs_below(auth_id, path, privs) } } @@ -232,15 +165,3 @@ impl UserInformation for CachedUserInfo { } } } - -pub fn privs_to_priv_names(privs: u64) -> Vec<&'static str> { - access_conf() - .privileges() - .iter() - .fold(Vec::new(), |mut priv_names, (name, value)| { - if value & privs != 0 { - priv_names.push(name); - } - priv_names - }) -} -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel