From 2571926eabcf0650d42f720a159e2388f98a77b4 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 11 Mar 2025 12:49:34 +0100 Subject: [PATCH 1/6] working lookup --- .../src/active_directory.rs | 92 ++++++++++++++++--- 1 file changed, 79 insertions(+), 13 deletions(-) diff --git a/rust/krb5-provision-keytab/src/active_directory.rs b/rust/krb5-provision-keytab/src/active_directory.rs index 62004693..429c4e00 100644 --- a/rust/krb5-provision-keytab/src/active_directory.rs +++ b/rust/krb5-provision-keytab/src/active_directory.rs @@ -6,7 +6,7 @@ use std::{ use byteorder::{LittleEndian, WriteBytesExt}; use krb5::{Keyblock, Keytab, KrbContext, Principal, PrincipalUnparseOptions}; -use ldap3::{Ldap, LdapConnAsync, LdapConnSettings}; +use ldap3::{Ldap, LdapConnAsync, LdapConnSettings, Scope, SearchEntry}; use rand::{seq::IndexedRandom, CryptoRng}; use snafu::{OptionExt, ResultExt, Snafu}; use stackable_krb5_provision_keytab::ActiveDirectorySamAccountNameRules; @@ -15,6 +15,7 @@ use stackable_operator::{ kube::{self, runtime::reflector::ObjectRef}, }; use stackable_secret_operator_crd_utils::SecretReference; +use tracing::info; use crate::credential_cache::{self, CredentialCache}; @@ -70,6 +71,15 @@ pub enum Error { #[snafu(display("configured samAccountName prefix is longer than the requested length"))] SamAccountNamePrefixLongerThanRequestedLength, + + #[snafu(display("failed to execute LDAP search"))] + SearchLdap { source: ldap3::LdapError }, + + #[snafu(display("failed to extract successful LDAP search"))] + SearchLdapSuccess { source: ldap3::LdapError }, + + #[snafu(display("the user did not have an associated kvno"))] + KvnoNotFound, } pub type Result = std::result::Result; @@ -162,18 +172,28 @@ impl<'a> AdAdmin<'a> { // FIXME: What about cases where ldap.add() succeeds but not the cache write? .context(PasswordCacheSnafu)??; let password_c = CString::new(password).context(DecodePasswordSnafu)?; - principal - .default_salt() - .and_then(|salt| { - Keyblock::from_password( - self.krb, - krb5::enctype::AES256_CTS_HMAC_SHA1_96, - &password_c, - &salt, - ) - }) - .and_then(|key| kt.add(principal, 0, &key.as_ref())) - .context(AddToKeytabSnafu)?; + + let kvno = get_user_kvno(&mut self.ldap, principal, &self.user_distinguished_name).await?; + if let Some(kvno) = kvno { + info!("using kvno {kvno}"); + principal + .default_salt() + .and_then(|salt| { + Keyblock::from_password( + self.krb, + krb5::enctype::AES256_CTS_HMAC_SHA1_96, + &password_c, + &salt, + ) + }) + .and_then(|key| kt.add(principal, kvno, &key.as_ref())) + .context(AddToKeytabSnafu)?; + } else { + // FIXME: if we can't detect the kvno then some applications may not + // authenticate if the keytab/kvno does not match the kvno of the + // ticket from the KDC. Always throw an exception? + //return Err(Error::KvnoNotFound); + } Ok(()) } } @@ -350,3 +370,49 @@ async fn create_ad_user( }; Ok(()) } + +#[tracing::instrument(skip(ldap), fields(%principal, %user_dn_base))] +async fn get_user_kvno( + ldap: &mut Ldap, + principal: &Principal<'_>, + user_dn_base: &str, +) -> Result> { + let princ_name = principal + .unparse(PrincipalUnparseOptions::default()) + .context(UnparsePrincipalSnafu)?; + let principal_cn = ldap3::dn_escape(&princ_name); + let principal_cn = principal_cn.get(..64).unwrap_or(&*principal_cn); + + let distinguished_name = &format!("CN={principal_cn},{user_dn_base}"); + info!("search with distinguished_name {:?}", distinguished_name); + + // Perform search with KVNO attribute + let (search_results, _) = ldap + .search( + distinguished_name, + Scope::Base, + "(objectClass=user)", + vec!["msDS-KeyVersionNumber"], + ) + .await + .context(SearchLdapSnafu)? + .success() + .context(SearchLdapSuccessSnafu)?; + + // Extract KVNO from first result + let mut kvno = None; + + if let Some(entry) = search_results.into_iter().next() { + let entry = SearchEntry::construct(entry); + info!("detected entry {:?}", entry); + kvno = entry + .attrs + .get("msDS-KeyVersionNumber") + .and_then(|v| v.first()) + .and_then(|s| s.parse::().ok()); + } + + info!("detected kvno {:?}", kvno); + + Ok(kvno) +} From 0a571967bdac57cf6a7d90b27b4168548404015b Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 11 Mar 2025 14:22:19 +0100 Subject: [PATCH 2/6] throw error when kvno not found --- rust/krb5-provision-keytab/src/active_directory.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rust/krb5-provision-keytab/src/active_directory.rs b/rust/krb5-provision-keytab/src/active_directory.rs index 429c4e00..29c4fce2 100644 --- a/rust/krb5-provision-keytab/src/active_directory.rs +++ b/rust/krb5-provision-keytab/src/active_directory.rs @@ -175,7 +175,6 @@ impl<'a> AdAdmin<'a> { let kvno = get_user_kvno(&mut self.ldap, principal, &self.user_distinguished_name).await?; if let Some(kvno) = kvno { - info!("using kvno {kvno}"); principal .default_salt() .and_then(|salt| { @@ -192,7 +191,7 @@ impl<'a> AdAdmin<'a> { // FIXME: if we can't detect the kvno then some applications may not // authenticate if the keytab/kvno does not match the kvno of the // ticket from the KDC. Always throw an exception? - //return Err(Error::KvnoNotFound); + return Err(Error::KvnoNotFound); } Ok(()) } From 02c7d89bbdee8caa5c756bd95431a6e9b486279b Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 11 Mar 2025 16:29:55 +0100 Subject: [PATCH 3/6] use a function to derive data from principal --- .../src/active_directory.rs | 55 ++++++++++++------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/rust/krb5-provision-keytab/src/active_directory.rs b/rust/krb5-provision-keytab/src/active_directory.rs index 29c4fce2..5c493c66 100644 --- a/rust/krb5-provision-keytab/src/active_directory.rs +++ b/rust/krb5-provision-keytab/src/active_directory.rs @@ -146,9 +146,7 @@ impl<'a> AdAdmin<'a> { principal: &Principal<'_>, kt: &mut Keytab<'_>, ) -> Result<()> { - let princ_name = principal - .unparse(PrincipalUnparseOptions::default()) - .context(UnparsePrincipalSnafu)?; + let princ_name = get_principal_data(principal)?.princ_name; let password_cache_key = princ_name.replace(['/', '@'], "__"); let password = self .password_cache @@ -281,18 +279,12 @@ async fn create_ad_user( const AD_ENCTYPE_AES256_HMAC_SHA1: u32 = 0x10; tracing::info!("creating principal"); - let princ_name = principal - .unparse(PrincipalUnparseOptions::default()) - .context(UnparsePrincipalSnafu)?; - let princ_name_realmless = principal - .unparse(PrincipalUnparseOptions { - realm: krb5::PrincipalRealmDisplayMode::Never, - ..Default::default() - }) - .context(UnparsePrincipalSnafu)?; - let principal_cn = ldap3::dn_escape(&princ_name); - // FIXME: AD restricts RDNs to 64 characters - let principal_cn = principal_cn.get(..64).unwrap_or(&*principal_cn); + + let principal_data = get_principal_data(principal)?; + let princ_name = principal_data.princ_name; + let principal_cn = principal_data.principal_cn; + let princ_name_realmless = principal_data.princ_name_realmless; + let sam_account_name = generate_sam_account_name .map(|sam_rules| { let mut name = sam_rules.prefix.clone(); @@ -370,18 +362,39 @@ async fn create_ad_user( Ok(()) } +pub struct PrincipalData { + princ_name: String, + principal_cn: String, + princ_name_realmless: String, +} + +fn get_principal_data(principal: &Principal<'_>) -> Result { + let princ_name = principal + .unparse(PrincipalUnparseOptions::default()) + .context(UnparsePrincipalSnafu)?; + let principal_cn = ldap3::dn_escape(&princ_name); + // FIXME: AD restricts RDNs to 64 characters + let principal_cn = principal_cn.get(..64).unwrap_or(&*principal_cn).to_string(); + let princ_name_realmless = principal + .unparse(PrincipalUnparseOptions { + realm: krb5::PrincipalRealmDisplayMode::Never, + ..Default::default() + }) + .context(UnparsePrincipalSnafu)?; + Ok(PrincipalData { + princ_name, + principal_cn, + princ_name_realmless, + }) +} + #[tracing::instrument(skip(ldap), fields(%principal, %user_dn_base))] async fn get_user_kvno( ldap: &mut Ldap, principal: &Principal<'_>, user_dn_base: &str, ) -> Result> { - let princ_name = principal - .unparse(PrincipalUnparseOptions::default()) - .context(UnparsePrincipalSnafu)?; - let principal_cn = ldap3::dn_escape(&princ_name); - let principal_cn = principal_cn.get(..64).unwrap_or(&*principal_cn); - + let principal_cn = get_principal_data(principal)?.principal_cn; let distinguished_name = &format!("CN={principal_cn},{user_dn_base}"); info!("search with distinguished_name {:?}", distinguished_name); From 52dab1157222e3696a0048231de373984f18d5fc Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 11 Mar 2025 16:41:38 +0100 Subject: [PATCH 4/6] changelog --- CHANGELOG.md | 2 ++ rust/krb5-provision-keytab/src/active_directory.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0365d95f..278a397b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ All notable changes to this project will be documented in this file. - Underscores are now allowed in Kerberos principal names ([#563]). - The issuer in generated TLS certificates is set to the subject of the issuing certificate ([#566]). +- Lookup KVNO from Active Directory rather than hard coding it ([#571]). [#528]: https://github.com/stackabletech/secret-operator/pull/528 [#544]: https://github.com/stackabletech/secret-operator/pull/544 @@ -36,6 +37,7 @@ All notable changes to this project will be documented in this file. [#564]: https://github.com/stackabletech/secret-operator/pull/564 [#566]: https://github.com/stackabletech/secret-operator/pull/566 [#569]: https://github.com/stackabletech/secret-operator/pull/569 +[#571]: https://github.com/stackabletech/secret-operator/pull/571 ## [24.11.1] - 2025-01-10 diff --git a/rust/krb5-provision-keytab/src/active_directory.rs b/rust/krb5-provision-keytab/src/active_directory.rs index 5c493c66..5462803e 100644 --- a/rust/krb5-provision-keytab/src/active_directory.rs +++ b/rust/krb5-provision-keytab/src/active_directory.rs @@ -186,9 +186,9 @@ impl<'a> AdAdmin<'a> { .and_then(|key| kt.add(principal, kvno, &key.as_ref())) .context(AddToKeytabSnafu)?; } else { - // FIXME: if we can't detect the kvno then some applications may not + // TODO: if we can't detect the kvno then some applications may not // authenticate if the keytab/kvno does not match the kvno of the - // ticket from the KDC. Always throw an exception? + // ticket from the KDC. So always throw an exception? return Err(Error::KvnoNotFound); } Ok(()) From 0e937f76e3fe04155d7e54093b22d718e004aa95 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Thu, 13 Mar 2025 15:40:28 +0100 Subject: [PATCH 5/6] log/comment cleanup --- .../src/active_directory.rs | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/rust/krb5-provision-keytab/src/active_directory.rs b/rust/krb5-provision-keytab/src/active_directory.rs index 5462803e..d632e916 100644 --- a/rust/krb5-provision-keytab/src/active_directory.rs +++ b/rust/krb5-provision-keytab/src/active_directory.rs @@ -4,6 +4,7 @@ use std::{ sync::OnceLock, }; +use crate::credential_cache::{self, CredentialCache}; use byteorder::{LittleEndian, WriteBytesExt}; use krb5::{Keyblock, Keytab, KrbContext, Principal, PrincipalUnparseOptions}; use ldap3::{Ldap, LdapConnAsync, LdapConnSettings, Scope, SearchEntry}; @@ -15,9 +16,6 @@ use stackable_operator::{ kube::{self, runtime::reflector::ObjectRef}, }; use stackable_secret_operator_crd_utils::SecretReference; -use tracing::info; - -use crate::credential_cache::{self, CredentialCache}; #[derive(Debug, Snafu)] pub enum Error { @@ -186,9 +184,9 @@ impl<'a> AdAdmin<'a> { .and_then(|key| kt.add(principal, kvno, &key.as_ref())) .context(AddToKeytabSnafu)?; } else { - // TODO: if we can't detect the kvno then some applications may not + // If we can't detect the kvno then some applications may not // authenticate if the keytab/kvno does not match the kvno of the - // ticket from the KDC. So always throw an exception? + // ticket from the KDC. So always throw an exception. return Err(Error::KvnoNotFound); } Ok(()) @@ -373,7 +371,7 @@ fn get_principal_data(principal: &Principal<'_>) -> Result { .unparse(PrincipalUnparseOptions::default()) .context(UnparsePrincipalSnafu)?; let principal_cn = ldap3::dn_escape(&princ_name); - // FIXME: AD restricts RDNs to 64 characters + // AD restricts RDNs to 64 characters let principal_cn = principal_cn.get(..64).unwrap_or(&*principal_cn).to_string(); let princ_name_realmless = principal .unparse(PrincipalUnparseOptions { @@ -396,7 +394,7 @@ async fn get_user_kvno( ) -> Result> { let principal_cn = get_principal_data(principal)?.principal_cn; let distinguished_name = &format!("CN={principal_cn},{user_dn_base}"); - info!("search with distinguished_name {:?}", distinguished_name); + tracing::info!("searching for kvno using DN {distinguished_name}"); // Perform search with KVNO attribute let (search_results, _) = ldap @@ -411,20 +409,21 @@ async fn get_user_kvno( .success() .context(SearchLdapSuccessSnafu)?; - // Extract KVNO from first result let mut kvno = None; + // Extract KVNO from first result if let Some(entry) = search_results.into_iter().next() { let entry = SearchEntry::construct(entry); - info!("detected entry {:?}", entry); + tracing::debug!("detected search result entry {:?}", entry); kvno = entry .attrs .get("msDS-KeyVersionNumber") .and_then(|v| v.first()) .and_then(|s| s.parse::().ok()); + tracing::debug!("detected kvno {:?} for DN {distinguished_name}", kvno); + } else { + tracing::info!("no kvno detected for DN {distinguished_name}"); } - info!("detected kvno {:?}", kvno); - Ok(kvno) } From 409ddc11cceb3c2f04320233311d5d33cdbb7996 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Thu, 13 Mar 2025 15:54:30 +0100 Subject: [PATCH 6/6] linting --- rust/krb5-provision-keytab/src/active_directory.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rust/krb5-provision-keytab/src/active_directory.rs b/rust/krb5-provision-keytab/src/active_directory.rs index d632e916..13007c76 100644 --- a/rust/krb5-provision-keytab/src/active_directory.rs +++ b/rust/krb5-provision-keytab/src/active_directory.rs @@ -4,7 +4,6 @@ use std::{ sync::OnceLock, }; -use crate::credential_cache::{self, CredentialCache}; use byteorder::{LittleEndian, WriteBytesExt}; use krb5::{Keyblock, Keytab, KrbContext, Principal, PrincipalUnparseOptions}; use ldap3::{Ldap, LdapConnAsync, LdapConnSettings, Scope, SearchEntry}; @@ -17,6 +16,8 @@ use stackable_operator::{ }; use stackable_secret_operator_crd_utils::SecretReference; +use crate::credential_cache::{self, CredentialCache}; + #[derive(Debug, Snafu)] pub enum Error { #[snafu(display("failed to retrieve LDAP TLS CA {ca_ref}"))]