From ebffc1c0864c7e5972a3c9149f9184ea5b3599ed Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Sun, 8 May 2022 20:12:06 +0200 Subject: [PATCH] server, ldap: Use group membership for admin status --- server/src/infra/ldap_handler.rs | 100 ++++++++++++++++++++++++------- server/src/infra/ldap_server.rs | 19 +++--- 2 files changed, 84 insertions(+), 35 deletions(-) diff --git a/server/src/infra/ldap_handler.rs b/server/src/infra/ldap_handler.rs index 68651a9..31c65a7 100644 --- a/server/src/infra/ldap_handler.rs +++ b/server/src/infra/ldap_handler.rs @@ -286,20 +286,23 @@ fn root_dse_response(base_dn: &str) -> LdapOp { }) } +#[derive(Clone, Copy, PartialEq, Debug)] +enum Permission { + Admin, + Regular, +} + pub struct LdapHandler { - dn: LdapDn, - user_id: UserId, + user_info: Option<(UserId, Permission)>, backend_handler: Backend, pub base_dn: Vec<(String, String)>, base_dn_str: String, - ldap_user_dn: LdapDn, } impl LdapHandler { - pub fn new(backend_handler: Backend, ldap_base_dn: String, ldap_user_dn: UserId) -> Self { + pub fn new(backend_handler: Backend, ldap_base_dn: String) -> Self { Self { - dn: LdapDn("unauthenticated".to_string()), - user_id: UserId::new("unauthenticated"), + user_info: None, backend_handler, base_dn: parse_distinguished_name(&ldap_base_dn).unwrap_or_else(|_| { panic!( @@ -307,7 +310,6 @@ impl LdapHandler LdapHandler { - self.dn = LdapDn(request.dn.clone()); - self.user_id = user_id; + let is_admin = self + .backend_handler + .get_user_groups(&user_id) + .await + .map(|groups| groups.iter().any(|g| g.1 == "lldap_admin")) + .unwrap_or(false); + self.user_info = Some(( + user_id, + if is_admin { + Permission::Admin + } else { + Permission::Regular + }, + )); (LdapResultCode::Success, "".to_string()) } Err(_) => (LdapResultCode::InvalidCredentials, "".to_string()), @@ -367,10 +381,28 @@ impl LdapHandler Vec { + let (user_id, permission) = match &self.user_info { + Some(info) => info, + _ => { + return vec![make_search_error( + LdapResultCode::InsufficentAccessRights, + "No user currently bound".to_string(), + )]; + } + }; match (&request.user_identity, &request.new_password) { (Some(user), Some(password)) => { match get_user_id_from_distinguished_name(user, &self.base_dn, &self.base_dn_str) { Ok(uid) => { + if *permission != Permission::Admin && user_id != &uid { + return vec![make_search_error( + LdapResultCode::InsufficentAccessRights, + format!( + r#"User {} cannot modify the password of user {}"#, + &user_id, &uid + ), + )]; + } if let Err(e) = self.change_password(&uid, password).await { vec![make_extended_response( LdapResultCode::Other, @@ -407,7 +439,16 @@ impl LdapHandler Vec { - let admin = self.dn == self.ldap_user_dn; + let user_filter = match &self.user_info { + Some((_, Permission::Admin)) => None, + Some((user_id, Permission::Regular)) => Some(user_id), + None => { + return vec![make_search_error( + LdapResultCode::InsufficentAccessRights, + "No user currently bound".to_string(), + )]; + } + }; if request.base.is_empty() && request.scope == LdapSearchScope::Base && request.filter == LdapFilter::Present("objectClass".to_string()) @@ -435,7 +476,6 @@ impl LdapHandler LdapHandler self.do_search(&request).await, LdapOp::UnbindRequest => { - self.dn = LdapDn("unauthenticated".to_string()); - self.user_id = UserId::new("unauthenticated"); + self.user_info = None; // No need to notify on unbind (per rfc4511) return None; } @@ -779,8 +818,14 @@ mod tests { password: "pass".to_string(), })) .return_once(|_| Ok(())); - let mut ldap_handler = - LdapHandler::new(mock, "dc=example,dc=com".to_string(), UserId::new("test")); + mock.expect_get_user_groups() + .with(eq(UserId::new("test"))) + .return_once(|_| { + let mut set = HashSet::new(); + set.insert(GroupIdAndName(GroupId(42), "lldap_admin".to_string())); + Ok(set) + }); + let mut ldap_handler = LdapHandler::new(mock, "dc=example,dc=com".to_string()); let request = LdapBindRequest { dn: "uid=test,ou=people,dc=example,dc=com".to_string(), cred: LdapBindCred::Simple("pass".to_string()), @@ -802,8 +847,10 @@ mod tests { })) .times(1) .return_once(|_| Ok(())); - let mut ldap_handler = - LdapHandler::new(mock, "dc=example,dc=com".to_string(), UserId::new("test")); + mock.expect_get_user_groups() + .with(eq(UserId::new("bob"))) + .return_once(|_| Ok(HashSet::new())); + let mut ldap_handler = LdapHandler::new(mock, "dc=example,dc=com".to_string()); let request = LdapOp::BindRequest(LdapBindRequest { dn: "uid=bob,ou=people,dc=example,dc=com".to_string(), @@ -833,8 +880,14 @@ mod tests { })) .times(1) .return_once(|_| Ok(())); - let mut ldap_handler = - LdapHandler::new(mock, "dc=example,dc=com".to_string(), UserId::new("test")); + mock.expect_get_user_groups() + .with(eq(UserId::new("test"))) + .return_once(|_| { + let mut set = HashSet::new(); + set.insert(GroupIdAndName(GroupId(42), "lldap_admin".to_string())); + Ok(set) + }); + let mut ldap_handler = LdapHandler::new(mock, "dc=example,dc=com".to_string()); let request = LdapBindRequest { dn: "uid=test,ou=people,dc=example,dc=com".to_string(), @@ -868,8 +921,10 @@ mod tests { ..Default::default() }]) }); - let mut ldap_handler = - LdapHandler::new(mock, "dc=example,dc=com".to_string(), UserId::new("admin")); + mock.expect_get_user_groups() + .with(eq(UserId::new("test"))) + .return_once(|_| Ok(HashSet::new())); + let mut ldap_handler = LdapHandler::new(mock, "dc=example,dc=com".to_string()); let request = LdapBindRequest { dn: "uid=test,ou=people,dc=example,dc=com".to_string(), @@ -897,8 +952,7 @@ mod tests { #[tokio::test] async fn test_bind_invalid_dn() { let mock = MockTestBackendHandler::new(); - let mut ldap_handler = - LdapHandler::new(mock, "dc=example,dc=com".to_string(), UserId::new("admin")); + let mut ldap_handler = LdapHandler::new(mock, "dc=example,dc=com".to_string()); let request = LdapBindRequest { dn: "cn=bob,dc=example,dc=com".to_string(), diff --git a/server/src/infra/ldap_server.rs b/server/src/infra/ldap_server.rs index fc4cd35..49cf842 100644 --- a/server/src/infra/ldap_server.rs +++ b/server/src/infra/ldap_server.rs @@ -1,6 +1,6 @@ use crate::{ domain::{ - handler::{BackendHandler, LoginHandler, UserId}, + handler::{BackendHandler, LoginHandler}, opaque_handler::OpaqueHandler, }, infra::{configuration::Configuration, ldap_handler::LdapHandler}, @@ -70,7 +70,6 @@ async fn handle_ldap_stream( stream: Stream, backend_handler: Backend, ldap_base_dn: String, - ldap_user_dn: UserId, ) -> Result where Backend: BackendHandler + LoginHandler + OpaqueHandler + 'static, @@ -82,7 +81,7 @@ where let mut requests = FramedRead::new(r, LdapCodec); let mut resp = FramedWrite::new(w, LdapCodec); - let mut session = LdapHandler::new(backend_handler, ldap_base_dn, ldap_user_dn); + let mut session = LdapHandler::new(backend_handler, ldap_base_dn); while let Some(msg) = requests.next().await { if !handle_incoming_message(msg, &mut resp, &mut session) @@ -111,11 +110,7 @@ pub fn build_ldap_server( where Backend: BackendHandler + LoginHandler + OpaqueHandler + 'static, { - let context = ( - backend_handler, - config.ldap_base_dn.clone(), - config.ldap_user_dn.clone(), - ); + let context = (backend_handler, config.ldap_base_dn.clone()); let context_for_tls = context.clone(); @@ -124,8 +119,8 @@ where fn_service(move |stream: TcpStream| { let context = context.clone(); async move { - let (handler, base_dn, user_dn) = context; - handle_ldap_stream(stream, handler, base_dn, user_dn).await + let (handler, base_dn) = context; + handle_ldap_stream(stream, handler, base_dn).await } }) .map_err(|err: anyhow::Error| error!("[LDAP] Service Error: {:#}", err)) @@ -144,9 +139,9 @@ where fn_service(move |stream: TcpStream| { let tls_context = tls_context.clone(); async move { - let ((handler, base_dn, user_dn), tls_acceptor) = tls_context; + let ((handler, base_dn), tls_acceptor) = tls_context; let tls_stream = tls_acceptor.accept(stream).await?; - handle_ldap_stream(tls_stream, handler, base_dn, user_dn).await + handle_ldap_stream(tls_stream, handler, base_dn).await } }) .map_err(|err: anyhow::Error| error!("[LDAPS] Service Error: {:#}", err))