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 2F8489C20 for ; Mon, 26 Jun 2023 15:18:37 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1178628CF2 for ; Mon, 26 Jun 2023 15:18:07 +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 ; Mon, 26 Jun 2023 15:18:06 +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 2AFFB447EE for ; Mon, 26 Jun 2023 15:18:06 +0200 (CEST) From: Stefan Sterz To: pbs-devel@lists.proxmox.com Date: Mon, 26 Jun 2023 15:17:46 +0200 Message-Id: <20230626131745.953583-1-s.sterz@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.094 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH proxmox-backup v2] access: ldap check connection on creation and change 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: Mon, 26 Jun 2023 13:18:37 -0000 this commit makes the ldap realm endpoints check whether a new or updated configuration works correctly. it uses the new `check_connection` function to make sure that a configuration can be successfully used to connect to and query an ldap directory. doing so allows us to remove the ldap domain regex. instead of relying on a regex to make sure that a given distinguished name (dn) could be correct, we simply let the ldap directory tell us whether it accepts it. this should also aid with usability as a dn that looks correct could still be invalid. this also implicitly removes unauthenticated binds, since the new `check_connection` function does not support those. it will simply bail out of the check if a `bind_dn` but no password is configured. therefore, this is a breaking change. Signed-off-by: Stefan Sterz --- changes since v2: * dropped the `LDAP_DOMAIN_REGEX` completely pbs-api-types/src/ldap.rs | 26 ++------------------------ src/api2/config/access/ldap.rs | 26 +++++++++++++++++++++----- src/auth.rs | 12 +++++++++++- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/pbs-api-types/src/ldap.rs b/pbs-api-types/src/ldap.rs index 762f560a..f3df90a0 100644 --- a/pbs-api-types/src/ldap.rs +++ b/pbs-api-types/src/ldap.rs @@ -1,8 +1,6 @@ use serde::{Deserialize, Serialize}; -use proxmox_schema::{ - api, const_regex, ApiStringFormat, ApiType, ArraySchema, Schema, StringSchema, Updater, -}; +use proxmox_schema::{api, ApiStringFormat, ApiType, ArraySchema, Schema, StringSchema, Updater}; use super::{REALM_ID_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA}; @@ -142,27 +140,7 @@ pub enum RemoveVanished { Properties, } -macro_rules! DOMAIN_PART_REGEX { - () => { - r#"("[^"]+"|[^ ,+"/<>;=#][^,+"/<>;=]*[^ ,+"/<>;=]|[^ ,+"/<>;=#])"# - }; -} - -const_regex! { - pub LDAP_DOMAIN_REGEX = concat!( - r#"^\w+="#, - DOMAIN_PART_REGEX!(), - r#"(,\s*\w+="#, - DOMAIN_PART_REGEX!(), - ")*$" - ); -} - -pub const LDAP_DOMAIN_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&LDAP_DOMAIN_REGEX); - -pub const LDAP_DOMAIN_SCHEMA: Schema = StringSchema::new("LDAP Domain") - .format(&LDAP_DOMAIN_FORMAT) - .schema(); +pub const LDAP_DOMAIN_SCHEMA: Schema = StringSchema::new("LDAP Domain").schema(); pub const SYNC_DEFAULTS_STRING_SCHEMA: Schema = StringSchema::new("sync defaults options") .format(&ApiStringFormat::PropertyString( diff --git a/src/api2/config/access/ldap.rs b/src/api2/config/access/ldap.rs index 90cd43c9..911142a0 100644 --- a/src/api2/config/access/ldap.rs +++ b/src/api2/config/access/ldap.rs @@ -1,8 +1,10 @@ +use crate::auth::LdapAuthenticator; use ::serde::{Deserialize, Serialize}; -use anyhow::Error; +use anyhow::{format_err, Error}; use hex::FromHex; use serde_json::Value; +use proxmox_ldap::Connection; use proxmox_router::{http_bail, Permission, Router, RpcEnvironment}; use proxmox_schema::{api, param_bail}; @@ -70,6 +72,11 @@ pub fn create_ldap_realm(config: LdapRealmConfig, password: Option) -> R param_bail!("realm", "realm '{}' already exists.", config.realm); } + let ldap_config = + LdapAuthenticator::api_type_to_config_with_password(&config, password.clone())?; + let conn = Connection::new(ldap_config); + proxmox_async::runtime::block_on(conn.check_connection()).map_err(|e| format_err!("{e:#}"))?; + if let Some(password) = password { auth_helpers::store_ldap_bind_password(&config.realm, &password, &domain_config_lock)?; } @@ -317,10 +324,6 @@ pub fn update_ldap_realm( config.bind_dn = Some(bind_dn); } - if let Some(password) = password { - auth_helpers::store_ldap_bind_password(&realm, &password, &domain_config_lock)?; - } - if let Some(filter) = update.filter { config.filter = Some(filter); } @@ -334,6 +337,19 @@ pub fn update_ldap_realm( config.user_classes = Some(user_classes); } + let ldap_config = if let Some(_) = password { + LdapAuthenticator::api_type_to_config_with_password(&config, password.clone())? + } else { + LdapAuthenticator::api_type_to_config(&config)? + }; + + let conn = Connection::new(ldap_config); + proxmox_async::runtime::block_on(conn.check_connection()).map_err(|e| format_err!("{e:#}"))?; + + if let Some(password) = password { + auth_helpers::store_ldap_bind_password(&realm, &password, &domain_config_lock)?; + } + domains.set_data(&realm, "ldap", &config)?; domains::save_config(&domains)?; diff --git a/src/auth.rs b/src/auth.rs index e8d8d2da..318d1ff2 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -170,6 +170,16 @@ impl Authenticator for LdapAuthenticator { impl LdapAuthenticator { pub fn api_type_to_config(config: &LdapRealmConfig) -> Result { + Self::api_type_to_config_with_password( + config, + auth_helpers::get_ldap_bind_password(&config.realm)?, + ) + } + + pub fn api_type_to_config_with_password( + config: &LdapRealmConfig, + password: Option, + ) -> Result { let mut servers = vec![config.server1.clone()]; if let Some(server) = &config.server2 { servers.push(server.clone()); @@ -198,7 +208,7 @@ impl LdapAuthenticator { user_attr: config.user_attr.clone(), base_dn: config.base_dn.clone(), bind_dn: config.bind_dn.clone(), - bind_password: auth_helpers::get_ldap_bind_password(&config.realm)?, + bind_password: password, tls_mode, verify_certificate: config.verify.unwrap_or_default(), additional_trusted_certificates: trusted_cert, -- 2.39.2