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 129041FF165 for ; Thu, 6 Nov 2025 15:38:11 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0C5301A91D; Thu, 6 Nov 2025 15:38:47 +0100 (CET) From: Shannon Sterz To: pdm-devel@lists.proxmox.com Date: Thu, 6 Nov 2025 15:38:27 +0100 Message-ID: <20251106143836.288888-2-s.sterz@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251106143836.288888-1-s.sterz@proxmox.com> References: <20251106143836.288888-1-s.sterz@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762439899179 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.062 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 v3 1/5] access-control: add acl feature to only expose types and 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" this is useful, when an application wants to only handle an acl tree without depending on more complex features provided by the rest of the `impl` feature or its bigger dependencies (e.g. openssl). access-control: clean up `cfg`s for impl feature by moving code into separate impl blocks and module. this avoids adding a lot of cfg macros separatelly. Signed-off-by: Shannon Sterz --- proxmox-access-control/Cargo.toml | 5 +- proxmox-access-control/src/acl.rs | 386 +++++++++++---------- proxmox-access-control/src/init.rs | 91 ++--- proxmox-access-control/src/lib.rs | 4 +- proxmox-access-control/src/token_shadow.rs | 2 +- proxmox-access-control/src/user.rs | 3 +- 6 files changed, 266 insertions(+), 225 deletions(-) diff --git a/proxmox-access-control/Cargo.toml b/proxmox-access-control/Cargo.toml index d23d272d..6df3b3ba 100644 --- a/proxmox-access-control/Cargo.toml +++ b/proxmox-access-control/Cargo.toml @@ -35,17 +35,20 @@ proxmox-uuid = { workspace = true } [features] default = [] +acl = [ + "dep:proxmox-section-config", +] api = [ "impl", "dep:hex", ] impl = [ + "acl", "dep:nix", "dep:openssl", "dep:proxmox-config-digest", "dep:proxmox-product-config", "dep:proxmox-router", - "dep:proxmox-section-config", "dep:proxmox-shared-memory", "dep:proxmox-sys", "dep:serde_json", diff --git a/proxmox-access-control/src/acl.rs b/proxmox-access-control/src/acl.rs index ec0a77b5..7813ea5d 100644 --- a/proxmox-access-control/src/acl.rs +++ b/proxmox-access-control/src/acl.rs @@ -1,15 +1,18 @@ -use std::collections::{BTreeMap, BTreeSet, HashMap}; +#[cfg(feature = "impl")] +use std::collections::BTreeSet; +use std::collections::{BTreeMap, HashMap}; +#[cfg(feature = "impl")] use std::io::Write; +#[cfg(feature = "impl")] use std::path::Path; -use std::sync::{Arc, OnceLock, RwLock}; use anyhow::{bail, Error}; use proxmox_auth_api::types::{Authid, Userid}; +#[cfg(feature = "impl")] use proxmox_config_digest::ConfigDigest; -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; pub fn split_acl_path(path: &str) -> Vec<&str> { let items = path.split('/'); @@ -302,6 +305,120 @@ impl AclTree { node.insert_user_role(auth_id.to_owned(), role.to_string(), propagate); } + fn parse_acl_line(&mut self, line: &str) -> Result<(), Error> { + let items: Vec<&str> = line.split(':').collect(); + + if items.len() != 5 { + bail!("wrong number of items."); + } + + if items[0] != "acl" { + bail!("line does not start with 'acl'."); + } + + let propagate = if items[1] == "0" { + false + } else if items[1] == "1" { + true + } else { + bail!("expected '0' or '1' for propagate flag."); + }; + + let path_str = items[2]; + let path = split_acl_path(path_str); + let node = self.get_or_insert_node(&path); + + let uglist: Vec<&str> = items[3].split(',').map(|v| v.trim()).collect(); + + let rolelist: Vec<&str> = items[4].split(',').map(|v| v.trim()).collect(); + + for user_or_group in &uglist { + for role in &rolelist { + if !access_conf().roles().contains_key(role) { + bail!("unknown role '{}'", role); + } + if let Some(group) = user_or_group.strip_prefix('@') { + node.insert_group_role(group.to_string(), role.to_string(), propagate); + } else { + node.insert_user_role(user_or_group.parse()?, role.to_string(), propagate); + } + } + } + + Ok(()) + } + + /// This is used for testing + pub fn from_raw(raw: &str) -> Result { + let mut tree = Self::new(); + for (linenr, line) in raw.lines().enumerate() { + let line = line.trim(); + if line.is_empty() { + continue; + } + if let Err(err) = tree.parse_acl_line(line) { + bail!( + "unable to parse acl config data, line {} - {}", + linenr + 1, + err + ); + } + } + Ok(tree) + } + + /// Returns a map of role name and propagation status for a given `auth_id` and `path`. + /// + /// This will collect role mappings according to the following algorithm: + /// - iterate over all intermediate nodes along `path` and collect roles with `propagate` set + /// - get all (propagating and non-propagating) roles for last component of path + /// - more specific role maps replace less specific role maps + /// -- user/token is more specific than group at each level + /// -- roles lower in the tree are more specific than those higher up along the path + pub fn roles(&self, auth_id: &Authid, path: &[&str]) -> HashMap { + let mut node = &self.root; + let mut role_map = node.extract_roles(auth_id, path.is_empty()); + + let mut comp_iter = path.iter().peekable(); + + while let Some(comp) = comp_iter.next() { + let last_comp = comp_iter.peek().is_none(); + + let mut sub_comp_iter = comp.split('/').peekable(); + + while let Some(sub_comp) = sub_comp_iter.next() { + let last_sub_comp = last_comp && sub_comp_iter.peek().is_none(); + + node = match node.children.get(sub_comp) { + Some(n) => n, + None => return role_map, // path not found + }; + + let new_map = node.extract_roles(auth_id, last_sub_comp); + if !new_map.is_empty() { + // overwrite previous mappings + role_map = new_map; + } + } + } + + role_map + } + + pub fn get_child_paths(&self, auth_id: &Authid, path: &[&str]) -> Result, Error> { + let mut res = Vec::new(); + + if let Some(node) = self.get_node(path) { + let path = path.join("/"); + node.get_child_paths(path, auth_id, &mut res)?; + } + + Ok(res) + } +} + +#[cfg(feature = "impl")] +impl AclTree { fn write_node_config(node: &AclTreeNode, path: &str, w: &mut dyn Write) -> Result<(), Error> { let mut role_ug_map0: HashMap<_, BTreeSet<_>> = HashMap::new(); let mut role_ug_map1: HashMap<_, BTreeSet<_>> = HashMap::new(); @@ -406,49 +523,6 @@ impl AclTree { Self::write_node_config(&self.root, "", w) } - fn parse_acl_line(&mut self, line: &str) -> Result<(), Error> { - let items: Vec<&str> = line.split(':').collect(); - - if items.len() != 5 { - bail!("wrong number of items."); - } - - if items[0] != "acl" { - bail!("line does not start with 'acl'."); - } - - let propagate = if items[1] == "0" { - false - } else if items[1] == "1" { - true - } else { - bail!("expected '0' or '1' for propagate flag."); - }; - - let path_str = items[2]; - let path = split_acl_path(path_str); - let node = self.get_or_insert_node(&path); - - let uglist: Vec<&str> = items[3].split(',').map(|v| v.trim()).collect(); - - let rolelist: Vec<&str> = items[4].split(',').map(|v| v.trim()).collect(); - - for user_or_group in &uglist { - for role in &rolelist { - if !access_conf().roles().contains_key(role) { - bail!("unknown role '{}'", role); - } - if let Some(group) = user_or_group.strip_prefix('@') { - node.insert_group_role(group.to_string(), role.to_string(), propagate); - } else { - node.insert_user_role(user_or_group.parse()?, role.to_string(), propagate); - } - } - } - - Ok(()) - } - fn load(filename: &Path) -> Result<(Self, ConfigDigest), Error> { let mut tree = Self::new(); @@ -482,156 +556,106 @@ impl AclTree { Ok((tree, digest)) } +} - /// This is used for testing - pub fn from_raw(raw: &str) -> Result { - let mut tree = Self::new(); - for (linenr, line) in raw.lines().enumerate() { - let line = line.trim(); - if line.is_empty() { - continue; - } - if let Err(err) = tree.parse_acl_line(line) { - bail!( - "unable to parse acl config data, line {} - {}", - linenr + 1, - err - ); - } - } - Ok(tree) +#[cfg(feature = "impl")] +pub use impl_feature::{cached_config, config, lock_config, save_config}; + +#[cfg(feature = "impl")] +mod impl_feature { + use std::sync::{Arc, OnceLock, RwLock}; + + use anyhow::{bail, Error}; + + use proxmox_config_digest::ConfigDigest; + use proxmox_product_config::{open_api_lockfile, replace_privileged_config, ApiLockGuard}; + + use crate::acl::AclTree; + use crate::init::access_conf; + use crate::init::impl_feature::{acl_config, acl_config_lock}; + + /// Get exclusive lock + pub fn lock_config() -> Result { + open_api_lockfile(acl_config_lock(), None, true) } - /// Returns a map of role name and propagation status for a given `auth_id` and `path`. + /// Reads the [`AclTree`] from `acl.cfg` in the configuration directory. + pub fn config() -> Result<(AclTree, ConfigDigest), Error> { + let path = acl_config(); + AclTree::load(&path) + } + + /// Returns a cached [`AclTree`] or a fresh copy read directly from `acl.cfg` in the configuration + /// directory. /// - /// This will collect role mappings according to the following algorithm: - /// - iterate over all intermediate nodes along `path` and collect roles with `propagate` set - /// - get all (propagating and non-propagating) roles for last component of path - /// - more specific role maps replace less specific role maps - /// -- user/token is more specific than group at each level - /// -- roles lower in the tree are more specific than those higher up along the path - pub fn roles(&self, auth_id: &Authid, path: &[&str]) -> HashMap { - let mut node = &self.root; - let mut role_map = node.extract_roles(auth_id, path.is_empty()); - - let mut comp_iter = path.iter().peekable(); - - while let Some(comp) = comp_iter.next() { - let last_comp = comp_iter.peek().is_none(); - - let mut sub_comp_iter = comp.split('/').peekable(); - - while let Some(sub_comp) = sub_comp_iter.next() { - let last_sub_comp = last_comp && sub_comp_iter.peek().is_none(); - - node = match node.children.get(sub_comp) { - Some(n) => n, - None => return role_map, // path not found - }; - - let new_map = node.extract_roles(auth_id, last_sub_comp); - if !new_map.is_empty() { - // overwrite previous mappings - role_map = new_map; - } - } + /// Since the AclTree is used for every API request's permission check, this caching mechanism + /// allows to skip reading and parsing the file again if it is unchanged. + pub fn cached_config() -> Result, Error> { + struct ConfigCache { + data: Option>, + last_mtime: i64, + last_mtime_nsec: i64, } - role_map - } + static CACHED_CONFIG: OnceLock> = OnceLock::new(); + let cached_conf = CACHED_CONFIG.get_or_init(|| { + RwLock::new(ConfigCache { + data: None, + last_mtime: 0, + last_mtime_nsec: 0, + }) + }); - pub fn get_child_paths(&self, auth_id: &Authid, path: &[&str]) -> Result, Error> { - let mut res = Vec::new(); + let conf = acl_config(); + let stat = match nix::sys::stat::stat(&conf) { + Ok(stat) => Some(stat), + Err(nix::errno::Errno::ENOENT) => None, + Err(err) => bail!("unable to stat '{}' - {err}", conf.display()), + }; - if let Some(node) = self.get_node(path) { - let path = path.join("/"); - node.get_child_paths(path, auth_id, &mut res)?; - } - - Ok(res) - } -} - -/// Get exclusive lock -pub fn lock_config() -> Result { - open_api_lockfile(acl_config_lock(), None, true) -} - -/// Reads the [`AclTree`] from `acl.cfg` in the configuration directory. -pub fn config() -> Result<(AclTree, ConfigDigest), Error> { - let path = acl_config(); - AclTree::load(&path) -} - -/// Returns a cached [`AclTree`] or a fresh copy read directly from `acl.cfg` in the configuration -/// directory. -/// -/// Since the AclTree is used for every API request's permission check, this caching mechanism -/// allows to skip reading and parsing the file again if it is unchanged. -pub fn cached_config() -> Result, Error> { - struct ConfigCache { - data: Option>, - last_mtime: i64, - last_mtime_nsec: i64, - } - - static CACHED_CONFIG: OnceLock> = OnceLock::new(); - let cached_conf = CACHED_CONFIG.get_or_init(|| { - RwLock::new(ConfigCache { - data: None, - last_mtime: 0, - last_mtime_nsec: 0, - }) - }); - - let conf = acl_config(); - let stat = match nix::sys::stat::stat(&conf) { - Ok(stat) => Some(stat), - Err(nix::errno::Errno::ENOENT) => None, - Err(err) => bail!("unable to stat '{}' - {err}", conf.display()), - }; - - { - // limit scope - let cache = cached_conf.read().unwrap(); - if let Some(ref config) = cache.data { - if let Some(stat) = stat { - if stat.st_mtime == cache.last_mtime && stat.st_mtime_nsec == cache.last_mtime_nsec - { + { + // limit scope + let cache = cached_conf.read().unwrap(); + if let Some(ref config) = cache.data { + if let Some(stat) = stat { + if stat.st_mtime == cache.last_mtime + && stat.st_mtime_nsec == cache.last_mtime_nsec + { + return Ok(config.clone()); + } + } else if cache.last_mtime == 0 && cache.last_mtime_nsec == 0 { return Ok(config.clone()); } - } else if cache.last_mtime == 0 && cache.last_mtime_nsec == 0 { - return Ok(config.clone()); } } + + let (config, _digest) = config()?; + let config = Arc::new(config); + + let mut cache = cached_conf.write().unwrap(); + if let Some(stat) = stat { + cache.last_mtime = stat.st_mtime; + cache.last_mtime_nsec = stat.st_mtime_nsec; + } + cache.data = Some(config.clone()); + + Ok(config) } - let (config, _digest) = config()?; - let config = Arc::new(config); + /// Saves an [`AclTree`] to `acl.cfg` in the configuration directory, ensuring proper ownership and + /// file permissions. + pub fn save_config(acl: &AclTree) -> Result<(), Error> { + let mut raw: Vec = Vec::new(); + acl.write_config(&mut raw)?; - let mut cache = cached_conf.write().unwrap(); - if let Some(stat) = stat { - cache.last_mtime = stat.st_mtime; - cache.last_mtime_nsec = stat.st_mtime_nsec; + let conf = acl_config(); + replace_privileged_config(conf, &raw)?; + + // increase cache generation so we reload it next time we access it + access_conf().increment_cache_generation()?; + + Ok(()) } - cache.data = Some(config.clone()); - - Ok(config) -} - -/// Saves an [`AclTree`] to `acl.cfg` in the configuration directory, ensuring proper ownership and -/// file permissions. -pub fn save_config(acl: &AclTree) -> Result<(), Error> { - let mut raw: Vec = Vec::new(); - acl.write_config(&mut raw)?; - - let conf = acl_config(); - replace_privileged_config(conf, &raw)?; - - // increase cache generation so we reload it next time we access it - access_conf().increment_cache_generation()?; - - Ok(()) } #[cfg(test)] diff --git a/proxmox-access-control/src/init.rs b/proxmox-access-control/src/init.rs index 39a12352..e64398e8 100644 --- a/proxmox-access-control/src/init.rs +++ b/proxmox-access-control/src/init.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::path::{Path, PathBuf}; use std::sync::OnceLock; use anyhow::{format_err, Error}; @@ -8,7 +7,6 @@ use proxmox_auth_api::types::{Authid, Userid}; use proxmox_section_config::SectionConfigData; static ACCESS_CONF: OnceLock<&'static dyn AccessControlConfig> = OnceLock::new(); -static ACCESS_CONF_DIR: OnceLock = OnceLock::new(); /// This trait specifies the functions a product needs to implement to get ACL tree based access /// control management from this plugin. @@ -105,21 +103,7 @@ pub trait AccessControlConfig: Send + Sync { } } -pub fn init>( - acm_config: &'static dyn AccessControlConfig, - config_dir: P, -) -> Result<(), Error> { - init_access_config(acm_config)?; - init_access_config_dir(config_dir) -} - -pub(crate) fn init_access_config_dir>(config_dir: P) -> Result<(), Error> { - ACCESS_CONF_DIR - .set(config_dir.as_ref().to_owned()) - .map_err(|_e| format_err!("cannot initialize acl tree config twice!")) -} - -pub(crate) fn init_access_config(config: &'static dyn AccessControlConfig) -> Result<(), Error> { +pub fn init_access_config(config: &'static dyn AccessControlConfig) -> Result<(), Error> { ACCESS_CONF .set(config) .map_err(|_e| format_err!("cannot initialize acl tree config twice!")) @@ -131,32 +115,61 @@ pub(crate) fn access_conf() -> &'static dyn AccessControlConfig { .expect("please initialize the acm config before using it!") } -fn conf_dir() -> &'static PathBuf { - ACCESS_CONF_DIR - .get() - .expect("please initialize acm config dir before using it!") -} +#[cfg(feature = "impl")] +pub use impl_feature::init; -pub(crate) fn acl_config() -> PathBuf { - conf_dir().join("acl.cfg") -} +#[cfg(feature = "impl")] +pub(crate) mod impl_feature { + use std::path::{Path, PathBuf}; + use std::sync::OnceLock; -pub(crate) fn acl_config_lock() -> PathBuf { - conf_dir().join(".acl.lck") -} + use anyhow::{format_err, Error}; -pub(crate) fn user_config() -> PathBuf { - conf_dir().join("user.cfg") -} + use crate::init::{init_access_config, AccessControlConfig}; -pub(crate) fn user_config_lock() -> PathBuf { - conf_dir().join(".user.lck") -} + static ACCESS_CONF_DIR: OnceLock = OnceLock::new(); -pub(crate) fn token_shadow() -> PathBuf { - conf_dir().join("token.shadow") -} + pub fn init>( + acm_config: &'static dyn AccessControlConfig, + config_dir: P, + ) -> Result<(), Error> { + init_access_config(acm_config)?; + init_access_config_dir(config_dir) + } -pub(crate) fn token_shadow_lock() -> PathBuf { - conf_dir().join("token.shadow.lock") + pub(crate) fn init_access_config_dir>(config_dir: P) -> Result<(), Error> { + ACCESS_CONF_DIR + .set(config_dir.as_ref().to_owned()) + .map_err(|_e| format_err!("cannot initialize acl tree config twice!")) + } + + fn conf_dir() -> &'static PathBuf { + ACCESS_CONF_DIR + .get() + .expect("please initialize acm config dir before using it!") + } + + pub(crate) fn acl_config() -> PathBuf { + conf_dir().join("acl.cfg") + } + + pub(crate) fn acl_config_lock() -> PathBuf { + conf_dir().join(".acl.lck") + } + + pub(crate) fn user_config() -> PathBuf { + conf_dir().join("user.cfg") + } + + pub(crate) fn user_config_lock() -> PathBuf { + conf_dir().join(".user.lck") + } + + pub(crate) fn token_shadow() -> PathBuf { + conf_dir().join("token.shadow") + } + + pub(crate) fn token_shadow_lock() -> PathBuf { + conf_dir().join("token.shadow.lock") + } } diff --git a/proxmox-access-control/src/lib.rs b/proxmox-access-control/src/lib.rs index 62683924..9195c999 100644 --- a/proxmox-access-control/src/lib.rs +++ b/proxmox-access-control/src/lib.rs @@ -2,13 +2,13 @@ pub mod types; -#[cfg(feature = "impl")] +#[cfg(feature = "acl")] pub mod acl; #[cfg(feature = "api")] pub mod api; -#[cfg(feature = "impl")] +#[cfg(feature = "acl")] pub mod init; #[cfg(feature = "impl")] diff --git a/proxmox-access-control/src/token_shadow.rs b/proxmox-access-control/src/token_shadow.rs index 373910f3..c586d834 100644 --- a/proxmox-access-control/src/token_shadow.rs +++ b/proxmox-access-control/src/token_shadow.rs @@ -6,7 +6,7 @@ use serde_json::{from_value, Value}; use proxmox_auth_api::types::Authid; use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockGuard}; -use crate::init::{token_shadow, token_shadow_lock}; +use crate::init::impl_feature::{token_shadow, token_shadow_lock}; // Get exclusive lock fn lock_config() -> Result { diff --git a/proxmox-access-control/src/user.rs b/proxmox-access-control/src/user.rs index 95b70f25..a4b59edc 100644 --- a/proxmox-access-control/src/user.rs +++ b/proxmox-access-control/src/user.rs @@ -9,7 +9,8 @@ use proxmox_product_config::{open_api_lockfile, replace_privileged_config, ApiLo use proxmox_schema::*; use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin}; -use crate::init::{access_conf, user_config, user_config_lock}; +use crate::init::access_conf; +use crate::init::impl_feature::{user_config, user_config_lock}; use crate::types::{ApiToken, User}; fn get_or_init_config() -> &'static SectionConfig { -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel