diff --git a/CHANGELOG.md b/CHANGELOG.md index 72f50b55..fd9963a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file. - Run a `containerdebug` process in the background of each OPA container to collect debugging information ([#666]). - Added support for OPA `1.0.x` ([#677]) and ([#687]). - Aggregate emitted Kubernetes events on the CustomResources ([#675]). +- Added support for filtering groups searched by Active Directory ([#693]). ### Removed @@ -23,6 +24,7 @@ All notable changes to this project will be documented in this file. [#675]: https://github.com/stackabletech/opa-operator/pull/675 [#677]: https://github.com/stackabletech/opa-operator/pull/677 [#687]: https://github.com/stackabletech/opa-operator/pull/687 +[#693]: https://github.com/stackabletech/opa-operator/pull/693 ## [24.11.1] - 2025-01-10 diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index 59bf7630..95b0f804 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -68,6 +68,15 @@ spec: experimentalActiveDirectory: description: Backend that fetches user information from Active Directory properties: + additionalGroupAttributeFilters: + additionalProperties: + type: string + default: {} + description: |- + Attributes that groups must have to be returned. + + These fields will be spliced into an LDAP Search Query, so wildcards can be used, but characters with a special meaning in LDAP will need to be escaped. + type: object baseDistinguishedName: description: The root Distinguished Name (DN) where users and groups are located. type: string diff --git a/docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc b/docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc index 6c9f90ba..101b722f 100644 --- a/docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc +++ b/docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc @@ -89,12 +89,14 @@ spec: baseDistinguishedName: DC=sble,DC=test # <3> customAttributeMappings: # <4> country: c # <5> - kerberosSecretClassName: kerberos-ad # <6> + additionalGroupAttributeFilters: # <6> + foo: bar + kerberosSecretClassName: kerberos-ad # <7> tls: verification: server: caCert: - secretClass: tls-ad # <7> + secretClass: tls-ad # <8> cache: # optional, enabled by default entryTimeToLive: 60s # optional, defaults to 60s ---- @@ -103,8 +105,9 @@ spec: <3> The distinguished name to search, users and groups outside of this will not be seen <4> Arbitrary LDAP attributes can be requested to be fetched <5> https://learn.microsoft.com/en-us/windows/win32/ad/address-book-properties[`c`] stores the ISO-3166 country code of the user -<6> The name of the SecretClass that knows how to create Kerberos keytabs trusted by Active Directory -<7> The name of the SecretClass that contains the Active Directory's root CA certificate(s) +<6> Groups can be filtered by LDAP attributes to reduce the load in searching for membership. `*` can be used as a wildcard in these filters. +<7> The name of the SecretClass that knows how to create Kerberos keytabs trusted by Active Directory +<8> The name of the SecretClass that contains the Active Directory's root CA certificate(s) == User info fetcher API diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 830fbac8..d3a7d688 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -101,6 +101,13 @@ pub mod versioned { /// Custom attributes, and their LDAP attribute names. #[serde(default)] pub custom_attribute_mappings: BTreeMap, + + /// Attributes that groups must have to be returned. + /// + /// These fields will be spliced into an LDAP Search Query, so wildcards can be used, + /// but characters with a special meaning in LDAP will need to be escaped. + #[serde(default)] + pub additional_group_attribute_filters: BTreeMap, } #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] diff --git a/rust/user-info-fetcher/src/backend/active_directory.rs b/rust/user-info-fetcher/src/backend/active_directory.rs index 8f3f44f1..da5fd280 100644 --- a/rust/user-info-fetcher/src/backend/active_directory.rs +++ b/rust/user-info-fetcher/src/backend/active_directory.rs @@ -1,6 +1,6 @@ use std::{ collections::{BTreeMap, HashMap}, - fmt::Display, + fmt::{Display, Write}, io::{Cursor, Read}, num::ParseIntError, str::FromStr, @@ -90,13 +90,19 @@ const LDAP_FIELD_USER_NAME: &str = "userPrincipalName"; const LDAP_FIELD_USER_PRIMARY_GROUP_RID: &str = "primaryGroupID"; const LDAP_FIELD_GROUP_MEMBER: &str = "member"; -#[tracing::instrument(skip(tls, base_distinguished_name, custom_attribute_mappings))] +#[tracing::instrument(skip( + tls, + base_distinguished_name, + custom_attribute_mappings, + additional_group_attribute_filters, +))] pub(crate) async fn get_user_info( request: &UserInfoRequest, ldap_server: &str, tls: &TlsClientDetails, base_distinguished_name: &str, custom_attribute_mappings: &BTreeMap, + additional_group_attribute_filters: &BTreeMap, ) -> Result { let ldap_tls = utils::tls::configure_native_tls(tls) .await @@ -168,16 +174,27 @@ pub(crate) async fn get_user_info( base_distinguished_name, &user, custom_attribute_mappings, + additional_group_attribute_filters, ) .await } -#[tracing::instrument(skip(ldap, base_dn, user, custom_attribute_mappings), fields(user.dn))] +#[tracing::instrument( + skip( + ldap, + base_dn, + user, + custom_attribute_mappings, + additional_group_attribute_filters, + ), + fields(user.dn), +)] async fn user_attributes( ldap: &mut Ldap, base_dn: &str, user: &SearchEntry, custom_attribute_mappings: &BTreeMap, + additional_group_attribute_filters: &BTreeMap, ) -> Result { let user_sid = user .bin_attrs @@ -242,7 +259,14 @@ async fn user_attributes( }) .collect::>(); let groups = if let Some(user_sid) = &user_sid { - user_group_distinguished_names(ldap, base_dn, user, user_sid).await? + user_group_distinguished_names( + ldap, + base_dn, + user, + user_sid, + additional_group_attribute_filters, + ) + .await? } else { tracing::debug!(user.dn, "user has no SID, cannot fetch groups..."); Vec::new() @@ -257,12 +281,13 @@ async fn user_attributes( } /// Gets the distinguished names of all of `user`'s groups, both primary and secondary. -#[tracing::instrument(skip(ldap, base_dn, user, user_sid))] +#[tracing::instrument(skip(ldap, base_dn, user, user_sid, additional_group_attribute_filters))] async fn user_group_distinguished_names( ldap: &mut Ldap, base_dn: &str, user: &SearchEntry, user_sid: &SecurityId, + additional_group_attribute_filters: &BTreeMap, ) -> Result, Error> { // User group memberships are tricky, because users have exactly one *primary* and any number of *secondary* groups. // Additionally groups can be members of other groups. @@ -309,10 +334,22 @@ async fn user_group_distinguished_names( "({LDAP_FIELD_GROUP_MEMBER}{LDAP_MATCHING_RULE_IN_CHAIN}=)" ); + // Users can also specify custom filters via `group_attribute_filters` + let custom_group_filter = + additional_group_attribute_filters + .iter() + .fold(String::new(), |mut out, (k, v)| { + // NOTE: This is technically an LDAP injection vuln, but these are provided statically by the OPA administrator, + // who would be able to do plenty of other harm... (like providing their own OPA images that do whatever they want). + // We could base64 the value to "defuse" it entirely, but that would also prevent using wildcards. + write!(out, "({k}={v})").expect("string concatenation is infallible"); + out + }); + // Let's put it all together, and make it go... let groups_filter = format!("(|{primary_group_filter}{primary_group_parents_filter}{secondary_groups_filter})"); - let groups_query_filter = format!("(&(objectClass=group){groups_filter})"); + let groups_query_filter = format!("(&(objectClass=group){custom_group_filter}{groups_filter})"); let requested_group_attrs = [LDAP_FIELD_OBJECT_DISTINGUISHED_NAME]; tracing::debug!( groups_query_filter, diff --git a/rust/user-info-fetcher/src/main.rs b/rust/user-info-fetcher/src/main.rs index 8e7cac3e..e5a0a079 100644 --- a/rust/user-info-fetcher/src/main.rs +++ b/rust/user-info-fetcher/src/main.rs @@ -300,6 +300,7 @@ async fn get_user_info( &ad.tls, &ad.base_distinguished_name, &ad.custom_attribute_mappings, + &ad.additional_group_attribute_filters, ) .await .context(get_user_info_error::ActiveDirectorySnafu)