From 1974d27dd8bc348c855326f9988138885115c154 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Thu, 27 Apr 2023 22:37:44 +1000 Subject: [PATCH] Filter rdns and dns for ldap filters (#1576) --- server/lib/src/idm/ldap.rs | 10 ++++------ server/lib/src/server/mod.rs | 27 ++++++++++++++++++++++++--- server/lib/src/value.rs | 8 ++++++++ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/server/lib/src/idm/ldap.rs b/server/lib/src/idm/ldap.rs index d3b24e94b..e7cf5bdd6 100644 --- a/server/lib/src/idm/ldap.rs +++ b/server/lib/src/idm/ldap.rs @@ -419,14 +419,12 @@ impl LdapServer { }); }; - let rdn = match self + let rdn = self .binddnre .captures(dn) - .and_then(|caps| caps.name("val").map(|v| v.as_str().to_string())) - { - Some(r) => r, - None => return Err(OperationError::NoMatchingEntries), - }; + .and_then(|caps| caps.name("val")) + .map(|v| v.as_str().to_string()) + .ok_or(OperationError::NoMatchingEntries)?; trace!(?rdn, "relative dn"); diff --git a/server/lib/src/server/mod.rs b/server/lib/src/server/mod.rs index 8440bce1d..9526a15fe 100644 --- a/server/lib/src/server/mod.rs +++ b/server/lib/src/server/mod.rs @@ -31,6 +31,7 @@ use crate::schema::{ Schema, SchemaAttribute, SchemaClass, SchemaReadTransaction, SchemaTransaction, SchemaWriteTransaction, }; +use crate::value::EXTRACT_VAL_DN; use crate::valueset::uuid_to_proto_string; pub mod access; @@ -265,11 +266,25 @@ pub trait QueryServerTransaction<'a> { } fn name_to_uuid(&mut self, name: &str) -> Result { + // There are some contexts where we will be passed an rdn or dn. We need + // to remove these elements if they exist. + // + // Why is it okay to ignore the attr and dn here? In Kani spn and name are + // always unique and absolutes, so even if the dn/rdn are not expected, there + // is only a single correct answer that *can* match these values. This also + // hugely simplifies the process of matching when we have app based searches + // in future too. + + let work = EXTRACT_VAL_DN + .captures(name) + .and_then(|caps| caps.name("val")) + .map(|v| v.as_str().to_lowercase()) + .ok_or(OperationError::InvalidValueState)?; + // Is it just a uuid? - Uuid::parse_str(name).or_else(|_| { - let lname = name.to_lowercase(); + Uuid::parse_str(&work).or_else(|_| { self.get_be_txn() - .name2uuid(lname.as_str())? + .name2uuid(&work)? .ok_or(OperationError::NoMatchingEntries) }) } @@ -1503,6 +1518,12 @@ mod tests { // Name is not syntax normalised (but exists) let r4 = server_txn.name_to_uuid("tEsTpErSoN1"); assert!(r4 == Ok(t_uuid)); + // Name is an rdn + let r5 = server_txn.name_to_uuid("name=testperson1"); + assert!(r5 == Ok(t_uuid)); + // Name is a dn + let r6 = server_txn.name_to_uuid("name=testperson1,o=example"); + assert!(r6 == Ok(t_uuid)); } #[qs_test] diff --git a/server/lib/src/value.rs b/server/lib/src/value.rs index aff883e75..a7df9acb4 100644 --- a/server/lib/src/value.rs +++ b/server/lib/src/value.rs @@ -40,6 +40,7 @@ lazy_static! { #[allow(clippy::expect_used)] Regex::new("(?P[^@]+)@(?P[^@]+)").expect("Invalid SPN regex found") }; + pub static ref DISALLOWED_NAMES: HashSet<&'static str> = { let mut m = HashSet::with_capacity(16); m.insert("root"); @@ -61,6 +62,13 @@ lazy_static! { #[allow(clippy::expect_used)] Regex::new("^[a-z][a-z0-9-_\\.]+$").expect("Invalid Iname regex found") }; + + pub static ref EXTRACT_VAL_DN: Regex = { + #[allow(clippy::expect_used)] + Regex::new("^(([^=,]+)=)?(?P[^=,]+)").expect("extract val from dn regex") + // Regex::new("^(([^=,]+)=)?(?P[^=,]+)(,.*)?$").expect("Invalid Iname regex found") + }; + pub static ref NSUNIQUEID_RE: Regex = { #[allow(clippy::expect_used)] Regex::new("^[0-9a-fA-F]{8}-[0-9a-fA-F]{8}-[0-9a-fA-F]{8}-[0-9a-fA-F]{8}$").expect("Invalid Nsunique regex found")