From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 4DAF5CDD3 for ; Wed, 16 Aug 2023 16:49:40 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 65E4A181F3 for ; Wed, 16 Aug 2023 16:49:08 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 16 Aug 2023 16:49:05 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 39A624101D for ; Wed, 16 Aug 2023 16:49:05 +0200 (CEST) From: Christoph Heiss To: pbs-devel@lists.proxmox.com Date: Wed, 16 Aug 2023 16:47:44 +0200 Message-ID: <20230816144746.1265108-15-c.heiss@proxmox.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230816144746.1265108-1-c.heiss@proxmox.com> References: <20230816144746.1265108-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.040 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: [pbs-devel] [RFC PATCH proxmox-backup v2 14/15] api: add case-insensitive support for Active Directory realms X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Aug 2023 14:49:40 -0000 To properly support case-insensitive comparison of user names, `CachedUserInfo` first needs to gain logic whether to look up the userid in a case-sensitive or -insensitive manner. The API part is pretty straight-forward, adding a new `case-sensitive` parameter to the API (which is on-by-default). Signed-off-by: Christoph Heiss --- It probably would be a good idea to warn the user/administrator in `CachedUserInfo::is_active_user_id() that an ambiguous userid match occured, but pulling the `log` crate into `pbs-config` seems unneccesary. Should the error be bubbled up the call chain? Changes v1 -> v2: * New patch pbs-api-types/src/ad.rs | 3 ++ pbs-config/src/cached_user_info.rs | 35 ++++++++++++--- pbs-config/src/domains.rs | 69 +++++++++++++++++++++++++++++- src/api2/config/access/ad.rs | 9 ++++ src/api2/config/sync.rs | 18 +++++++- 5 files changed, 126 insertions(+), 8 deletions(-) diff --git a/pbs-api-types/src/ad.rs b/pbs-api-types/src/ad.rs index 910571a0..446715c7 100644 --- a/pbs-api-types/src/ad.rs +++ b/pbs-api-types/src/ad.rs @@ -61,6 +61,9 @@ pub struct AdRealmConfig { /// overridden if the need arises. #[serde(skip_serializing_if = "Option::is_none")] pub base_dn: Option, + /// Whether usernames should be matched case-sensitive + #[serde(skip_serializing_if = "Option::is_none")] + pub case_sensitive: Option, /// Comment #[serde(skip_serializing_if = "Option::is_none")] pub comment: Option, diff --git a/pbs-config/src/cached_user_info.rs b/pbs-config/src/cached_user_info.rs index b9534b80..5731fb0a 100644 --- a/pbs-config/src/cached_user_info.rs +++ b/pbs-config/src/cached_user_info.rs @@ -9,7 +9,9 @@ use proxmox_router::UserInformation; use proxmox_section_config::SectionConfigData; use proxmox_time::epoch_i64; -use pbs_api_types::{privs_to_priv_names, ApiToken, Authid, User, Userid, ROLE_ADMIN}; +use pbs_api_types::{ + privs_to_priv_names, AdRealmConfig, ApiToken, Authid, User, Userid, ROLE_ADMIN, +}; use crate::acl::{AclTree, ROLE_NAMES}; use crate::ConfigVersionCache; @@ -17,6 +19,7 @@ use crate::ConfigVersionCache; /// Cache User/Group/Token/Acl configuration data for fast permission tests pub struct CachedUserInfo { user_cfg: Arc, + domains_cfg: Arc, acl_tree: Arc, } @@ -56,6 +59,7 @@ impl CachedUserInfo { let config = Arc::new(CachedUserInfo { user_cfg: crate::user::cached_config()?, + domains_cfg: crate::domains::cached_config()?, acl_tree: crate::acl::cached_config()?, }); @@ -69,19 +73,40 @@ impl CachedUserInfo { /// Only exposed for testing #[doc(hidden)] - pub fn test_new(user_cfg: SectionConfigData, acl_tree: AclTree) -> Self { + pub fn test_new( + user_cfg: SectionConfigData, + domains_cfg: SectionConfigData, + acl_tree: AclTree, + ) -> Self { Self { user_cfg: Arc::new(user_cfg), + domains_cfg: Arc::new(domains_cfg), acl_tree: Arc::new(acl_tree), } } /// Test if a user_id is enabled and not expired pub fn is_active_user_id(&self, userid: &Userid) -> bool { - if let Ok(info) = self.user_cfg.lookup::("user", userid.as_str()) { - info.is_active() + // Only Active Directory realms have the possibility to be case-insensitive + // Default is case-sensitive + let case_sensitive = match self + .domains_cfg + .lookup::("ad", userid.realm().as_str()) + { + Ok(ad) => ad.case_sensitive.unwrap_or(true), + _ => true, + }; + + if case_sensitive { + self.user_cfg + .lookup::("user", userid.as_str()) + .map(|info| info.is_active()) + .unwrap_or_default() } else { - false + match self.user_cfg.lookup_icase::("user", userid.as_str()) { + Ok(users) if users.len() == 1 => users[0].is_active(), + _ => false, + } } } diff --git a/pbs-config/src/domains.rs b/pbs-config/src/domains.rs index 4a9beec3..49a7b2d9 100644 --- a/pbs-config/src/domains.rs +++ b/pbs-config/src/domains.rs @@ -1,6 +1,9 @@ -use std::collections::HashMap; +use std::{ + collections::HashMap, + sync::{Arc, RwLock}, +}; -use anyhow::Error; +use anyhow::{bail, Error}; use lazy_static::lazy_static; use pbs_buildcfg::configdir; @@ -58,11 +61,73 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> { Ok((data, digest)) } +/// Returns a cached [`SectionConfigData`] or fresh copy read directly from the [default +/// path](DOMAINS_CFG_FILENAME). +/// +/// This allows fast access to the domains config without having to read and parse it every time. +pub fn cached_config() -> Result, Error> { + struct ConfigCache { + data: Option>, + last_mtime: i64, + last_mtime_nsec: i64, + } + + lazy_static! { + static ref CACHED_CONFIG: RwLock = RwLock::new(ConfigCache { + data: None, + last_mtime: 0, + last_mtime_nsec: 0 + }); + } + + let stat = match nix::sys::stat::stat(DOMAINS_CFG_FILENAME) { + Ok(stat) => Some(stat), + Err(nix::errno::Errno::ENOENT) => None, + Err(err) => bail!("unable to stat '{}' - {}", DOMAINS_CFG_FILENAME, err), + }; + + { + // limit scope + let cache = CACHED_CONFIG.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()); + } + } + } + + let (config, _digest) = config()?; + let config = Arc::new(config); + + let mut cache = CACHED_CONFIG.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) +} + pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(DOMAINS_CFG_FILENAME, config)?; replace_backup_config(DOMAINS_CFG_FILENAME, raw.as_bytes()) } +/// Only exposed for testing +#[doc(hidden)] +pub fn test_cfg_from_str(raw: &str) -> Result<(SectionConfigData, [u8; 32]), Error> { + let cfg = init(); + let parsed = cfg.parse("test_domains_cfg", raw)?; + + Ok((parsed, [0; 32])) +} + /// Check if a realm with the given name exists pub fn exists(domains: &SectionConfigData, realm: &str) -> bool { realm == "pbs" || realm == "pam" || domains.sections.get(realm).is_some() diff --git a/src/api2/config/access/ad.rs b/src/api2/config/access/ad.rs index c202291a..f59b80af 100644 --- a/src/api2/config/access/ad.rs +++ b/src/api2/config/access/ad.rs @@ -152,6 +152,8 @@ pub enum DeletableProperty { SyncAttributes, /// User classes UserClasses, + /// Whether usernames are compared case-sensitive or not + CaseSensitive, } #[api( @@ -244,6 +246,9 @@ pub async fn update_ad_realm( DeletableProperty::UserClasses => { config.user_classes = None; } + DeletableProperty::CaseSensitive => { + config.case_sensitive = None; + } } } } @@ -301,6 +306,10 @@ pub async fn update_ad_realm( config.user_classes = Some(user_classes); } + if let Some(case_sensitive) = update.case_sensitive { + config.case_sensitive = Some(case_sensitive); + } + let mut ldap_config = if password.is_some() { AdAuthenticator::api_type_to_config_with_password(&config, password.clone())? } else { diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs index 01e5f2ce..86b53b72 100644 --- a/src/api2/config/sync.rs +++ b/src/api2/config/sync.rs @@ -481,6 +481,22 @@ user: write@pbs "###, ) .expect("test user.cfg is not parsable"); + let (domains_cfg, _) = pbs_config::domains::test_cfg_from_str( + r###" +ldap: ldap_example + base-dn dc=example,dc=com + mode ldap + server1 192.0.2.1 + user-attr uid + +ad: ad_example + base-dn dc=example,dc=com + mode ldaps + server1 192.0.2.1 + +"###, + ) + .expect("test domains.cfg is not parsable"); let acl_tree = pbs_config::acl::AclTree::from_raw( r###" acl:1:/datastore/localstore1:read@pbs,write@pbs:DatastoreAudit @@ -493,7 +509,7 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator ) .expect("test acl.cfg is not parsable"); - let user_info = CachedUserInfo::test_new(user_cfg, acl_tree); + let user_info = CachedUserInfo::test_new(user_cfg, domains_cfg, acl_tree); let root_auth_id = Authid::root_auth_id(); -- 2.41.0