diff --git a/proto/src/constants.rs b/proto/src/constants.rs index 9575a0a41..cbfbeb2ff 100644 --- a/proto/src/constants.rs +++ b/proto/src/constants.rs @@ -39,6 +39,8 @@ pub const DEFAULT_SERVER_ADDRESS: &str = "127.0.0.1:8443"; pub const DEFAULT_SERVER_LOCALHOST: &str = "localhost:8443"; /// The default LDAP bind address for the Kanidm client pub const DEFAULT_LDAP_LOCALHOST: &str = "localhost:636"; +/// The default amount of attributes that can be queried in LDAP +pub const DEFAULT_LDAP_MAXIMUM_QUERYABLE_ATTRIBUTES: usize = 100; /// Default replication configuration pub const DEFAULT_REPLICATION_ADDRESS: &str = "127.0.0.1:8444"; pub const DEFAULT_REPLICATION_ORIGIN: &str = "repl://localhost:8444"; diff --git a/server/core/src/config.rs b/server/core/src/config.rs index 78a9cdf72..981ef3988 100644 --- a/server/core/src/config.rs +++ b/server/core/src/config.rs @@ -14,6 +14,7 @@ use kanidm_proto::constants::DEFAULT_SERVER_ADDRESS; use kanidm_proto::internal::FsType; use kanidm_proto::messages::ConsoleOutputMode; +use kanidmd_lib::prelude::DEFAULT_LDAP_MAXIMUM_QUERYABLE_ATTRIBUTES; use serde::Deserialize; use sketching::LogLevel; use url::Url; @@ -116,7 +117,9 @@ pub struct ServerConfig { /// /// If unset, the LDAP server will be disabled. pub ldapbindaddress: Option<String>, - + /// The maximum number of LDAP attributes that can be queried in one operation. + /// Defaults to [kanidm_proto::constants::DEFAULT_LDAP_MAXIMUM_QUERYABLE_ATTRIBUTES] + pub ldap_maximum_queryable_attributes: Option<usize>, /// The role of this server, one of write_replica, write_replica_no_ui, read_only_replica, defaults to [ServerRole::WriteReplica] #[serde(default)] pub role: ServerRole, @@ -477,6 +480,7 @@ pub struct IntegrationReplConfig { pub struct Configuration { pub address: String, pub ldapaddress: Option<String>, + pub ldap_maximum_queryable_attrs: usize, pub adminbindpath: String, pub threads: usize, // db type later @@ -571,6 +575,7 @@ impl Configuration { Configuration { address: DEFAULT_SERVER_ADDRESS.to_string(), ldapaddress: None, + ldap_maximum_queryable_attrs: DEFAULT_LDAP_MAXIMUM_QUERYABLE_ATTRIBUTES, adminbindpath: env!("KANIDM_SERVER_ADMIN_BIND_PATH").to_string(), threads: std::thread::available_parallelism() .map(|t| t.get()) @@ -648,6 +653,9 @@ impl Configuration { self.update_ldapbind(&sconfig.ldapbindaddress); self.update_online_backup(&sconfig.online_backup); self.update_log_level(&sconfig.log_level); + if let Some(ldap_maximum_queryable_attrs) = sconfig.ldap_maximum_queryable_attributes { + self.update_ldap_maximum_queryable_attrs(ldap_maximum_queryable_attrs); + } } pub fn update_trust_x_forward_for(&mut self, t: Option<bool>) { @@ -734,4 +742,9 @@ impl Configuration { pub fn update_threads_count(&mut self, threads: usize) { self.threads = std::cmp::min(self.threads, threads); } + + // Updates the maximum number of LDAP attributes that can be queried in a single operation + pub fn update_ldap_maximum_queryable_attrs(&mut self, maximum_queryable_attrs: usize) { + self.ldap_maximum_queryable_attrs = maximum_queryable_attrs; + } } diff --git a/server/core/src/lib.rs b/server/core/src/lib.rs index 4826ec83f..4c466468d 100644 --- a/server/core/src/lib.rs +++ b/server/core/src/lib.rs @@ -937,7 +937,7 @@ pub async fn create_server_core( } } - let ldap = match LdapServer::new(&idms).await { + let ldap = match LdapServer::new(&idms, config.ldap_maximum_queryable_attrs).await { Ok(l) => l, Err(e) => { error!("Unable to start LdapServer -> {:?}", e); diff --git a/server/lib/src/idm/ldap.rs b/server/lib/src/idm/ldap.rs index 63956762f..cc85463f6 100644 --- a/server/lib/src/idm/ldap.rs +++ b/server/lib/src/idm/ldap.rs @@ -60,6 +60,7 @@ pub struct LdapServer { basedn: String, dnre: Regex, binddnre: Regex, + maximum_queryable_attrs: usize, } #[derive(Debug)] @@ -70,7 +71,10 @@ enum LdapBindTarget { } impl LdapServer { - pub async fn new(idms: &IdmServer) -> Result<Self, OperationError> { + pub async fn new( + idms: &IdmServer, + maximum_queryable_attrs: usize, + ) -> Result<Self, OperationError> { // let ct = duration_from_epoch_now(); let mut idms_prox_read = idms.proxy_read().await?; // This is the rootdse path. @@ -154,6 +158,7 @@ impl LdapServer { basedn, dnre, binddnre, + maximum_queryable_attrs, }) } @@ -239,11 +244,10 @@ impl LdapServer { let mut all_attrs = false; let mut all_op_attrs = false; - // TODO #3406: limit the number of attributes here! if sr.attrs.is_empty() { // If [], then "all" attrs all_attrs = true; - } else { + } else if sr.attrs.len() < self.maximum_queryable_attrs { sr.attrs.iter().for_each(|a| { if a == "*" { all_attrs = true; @@ -267,6 +271,9 @@ impl LdapServer { } } }) + } else { + return Err(OperationError::ResourceLimit); + // TODO: Should we return ResourceLimit or InvalidRequestState here? } // We need to retain this to know what the client requested. @@ -859,7 +866,9 @@ mod tests { #[idm_test] async fn test_ldap_simple_bind(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let mut idms_prox_write = idms.proxy_write(duration_from_epoch_now()).await.unwrap(); // make the admin a valid posix account @@ -1054,7 +1063,9 @@ mod tests { #[idm_test] async fn test_ldap_application_dnre(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let testdn = format!("app=app1,{0}", ldaps.basedn); let captures = ldaps.dnre.captures(testdn.as_str()).unwrap(); @@ -1077,7 +1088,9 @@ mod tests { #[idm_test] async fn test_ldap_application_search(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let usr_uuid = Uuid::new_v4(); let grp_uuid = Uuid::new_v4(); @@ -1160,7 +1173,9 @@ mod tests { #[idm_test] async fn test_ldap_spn_search(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let usr_uuid = Uuid::new_v4(); let usr_name = "panko"; @@ -1242,7 +1257,9 @@ mod tests { #[idm_test] async fn test_ldap_application_bind(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let usr_uuid = Uuid::new_v4(); let grp_uuid = Uuid::new_v4(); @@ -1398,7 +1415,9 @@ mod tests { idms: &IdmServer, _idms_delayed: &IdmServerDelayed, ) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let usr_uuid = Uuid::new_v4(); let usr_name = "testuser1"; @@ -1602,7 +1621,9 @@ mod tests { idms: &IdmServer, _idms_delayed: &IdmServerDelayed, ) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let usr_uuid = Uuid::new_v4(); let usr_name = "testuser1"; @@ -1739,7 +1760,9 @@ mod tests { idms: &IdmServer, _idms_delayed: &IdmServerDelayed, ) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let ssh_ed25519 = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAeGW1P6Pc2rPq0XqbRaDKBcXZUPRklo0L1EyR30CwoP william@amethyst"; @@ -1902,7 +1925,9 @@ mod tests { _idms_delayed: &IdmServerDelayed, ) { // Setup the ldap server - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); // Prebuild the search req we'll be using this test. let sr = SearchRequest { @@ -2142,7 +2167,9 @@ mod tests { idms: &IdmServer, _idms_delayed: &IdmServerDelayed, ) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let acct_uuid = uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"); @@ -2211,7 +2238,9 @@ mod tests { // Test behaviour of the 1.1 attribute. #[idm_test] async fn test_ldap_one_dot_one_attribute(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let acct_uuid = uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"); @@ -2300,7 +2329,9 @@ mod tests { #[idm_test] async fn test_ldap_rootdse_basedn_change(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let anon_t = ldaps.do_bind(idms, "", "").await.unwrap().unwrap(); assert_eq!( @@ -2356,7 +2387,9 @@ mod tests { assert!(idms_prox_write.commit().is_ok()); // Now re-test - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let anon_t = ldaps.do_bind(idms, "", "").await.unwrap().unwrap(); assert_eq!( @@ -2397,7 +2430,9 @@ mod tests { #[idm_test] async fn test_ldap_sssd_compat(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); let acct_uuid = uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"); @@ -2505,7 +2540,9 @@ mod tests { #[idm_test] async fn test_ldap_compare_request(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) { - let ldaps = LdapServer::new(idms).await.expect("failed to start ldap"); + let ldaps = LdapServer::new(idms, 1000) + .await + .expect("failed to start ldap"); // Setup a user we want to check. { @@ -2631,4 +2668,94 @@ mod tests { &OperationError::InvalidAttributeName("invalid".to_string()), ); } + + #[idm_test] + async fn test_ldap_maximum_queryable_attributes( + idms: &IdmServer, + _idms_delayed: &IdmServerDelayed, + ) { + let ldaps = LdapServer::new(idms, 2) + .await + .expect("failed to start ldap"); + + let usr_uuid = Uuid::new_v4(); + let grp_uuid = Uuid::new_v4(); + let app_uuid = Uuid::new_v4(); + let app_name = "testapp1"; + + // Setup person, group and application + { + let e1 = entry_init!( + (Attribute::Class, EntryClass::Object.to_value()), + (Attribute::Class, EntryClass::Account.to_value()), + (Attribute::Class, EntryClass::Person.to_value()), + (Attribute::Name, Value::new_iname("testperson1")), + (Attribute::Uuid, Value::Uuid(usr_uuid)), + (Attribute::Description, Value::new_utf8s("testperson1")), + (Attribute::DisplayName, Value::new_utf8s("testperson1")) + ); + + let e2 = entry_init!( + (Attribute::Class, EntryClass::Object.to_value()), + (Attribute::Class, EntryClass::Group.to_value()), + (Attribute::Name, Value::new_iname("testgroup1")), + (Attribute::Uuid, Value::Uuid(grp_uuid)) + ); + + let e3 = entry_init!( + (Attribute::Class, EntryClass::Object.to_value()), + (Attribute::Class, EntryClass::ServiceAccount.to_value()), + (Attribute::Class, EntryClass::Application.to_value()), + (Attribute::Name, Value::new_iname(app_name)), + (Attribute::Uuid, Value::Uuid(app_uuid)), + (Attribute::LinkedGroup, Value::Refer(grp_uuid)) + ); + + let ct = duration_from_epoch_now(); + let mut server_txn = idms.proxy_write(ct).await.unwrap(); + assert!(server_txn + .qs_write + .internal_create(vec![e1, e2, e3]) + .and_then(|_| server_txn.commit()) + .is_ok()); + } + + // Setup the anonymous login + let anon_t = ldaps.do_bind(idms, "", "").await.unwrap().unwrap(); + assert_eq!( + anon_t.effective_session, + LdapSession::UnixBind(UUID_ANONYMOUS) + ); + + let invalid_search = SearchRequest { + msgid: 1, + base: "dc=example,dc=com".to_string(), + scope: LdapSearchScope::Subtree, + filter: LdapFilter::Present(Attribute::ObjectClass.to_string()), + attrs: vec![ + "objectClass".to_string(), + "cn".to_string(), + "givenName".to_string(), + ], + }; + + let valid_search = SearchRequest { + msgid: 1, + base: "dc=example,dc=com".to_string(), + scope: LdapSearchScope::Subtree, + filter: LdapFilter::Present(Attribute::ObjectClass.to_string()), + attrs: vec!["objectClass: person".to_string()], + }; + + let invalid_res: Result<Vec<LdapMsg>, OperationError> = ldaps + .do_search(idms, &invalid_search, &anon_t, Source::Internal) + .await; + + let valid_res: Result<Vec<LdapMsg>, OperationError> = ldaps + .do_search(idms, &valid_search, &anon_t, Source::Internal) + .await; + + assert_eq!(invalid_res, Err(OperationError::ResourceLimit)); + assert_ne!(valid_res, Err(OperationError::ResourceLimit)); + } }