Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
9 changes: 9 additions & 0 deletions deploy/helm/opa-operator/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
----
Expand All @@ -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

Expand Down
7 changes: 7 additions & 0 deletions rust/operator-binary/src/crd/user_info_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ pub mod versioned {
/// Custom attributes, and their LDAP attribute names.
#[serde(default)]
pub custom_attribute_mappings: BTreeMap<String, String>,

/// 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<String, String>,
}

#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
Expand Down
49 changes: 43 additions & 6 deletions rust/user-info-fetcher/src/backend/active_directory.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{
collections::{BTreeMap, HashMap},
fmt::Display,
fmt::{Display, Write},
io::{Cursor, Read},
num::ParseIntError,
str::FromStr,
Expand Down Expand Up @@ -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<String, String>,
additional_group_attribute_filters: &BTreeMap<String, String>,
) -> Result<UserInfo, Error> {
let ldap_tls = utils::tls::configure_native_tls(tls)
.await
Expand Down Expand Up @@ -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<String, String>,
additional_group_attribute_filters: &BTreeMap<String, String>,
) -> Result<UserInfo, Error> {
let user_sid = user
.bin_attrs
Expand Down Expand Up @@ -242,7 +259,14 @@ async fn user_attributes(
})
.collect::<HashMap<_, _>>();
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()
Expand All @@ -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<String, String>,
) -> Result<Vec<String>, 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.
Expand Down Expand Up @@ -309,10 +334,22 @@ async fn user_group_distinguished_names(
"({LDAP_FIELD_GROUP_MEMBER}{LDAP_MATCHING_RULE_IN_CHAIN}=<SID={primary_group_sid}>)"
);

// 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,
Expand Down
1 change: 1 addition & 0 deletions rust/user-info-fetcher/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading