From d9f4adcb0e4d9ee52645b393328841dbba9fb1ef Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Sat, 29 Jul 2023 12:02:08 +0200 Subject: [PATCH] ldap: Add support for modifying the password with a modify operation --- app/src/components/change_password.rs | 8 +- app/src/components/create_user.rs | 5 +- app/src/components/reset_password_step2.rs | 2 +- auth/src/opaque.rs | 4 +- server/src/domain/sql_backend_handler.rs | 2 +- server/src/domain/sql_opaque_handler.rs | 2 +- server/src/infra/ldap_handler.rs | 175 ++++++++++++++++++++- set-password/src/main.rs | 2 +- 8 files changed, 183 insertions(+), 17 deletions(-) diff --git a/app/src/components/change_password.rs b/app/src/components/change_password.rs index d6d59d3..dd29623 100644 --- a/app/src/components/change_password.rs +++ b/app/src/components/change_password.rs @@ -128,9 +128,11 @@ impl CommonComponent for ChangePasswordForm { Msg::SubmitNewPassword => { let mut rng = rand::rngs::OsRng; let new_password = self.form.model().password; - let registration_start_request = - opaque::client::registration::start_registration(&new_password, &mut rng) - .context("Could not initiate password change")?; + let registration_start_request = opaque::client::registration::start_registration( + new_password.as_bytes(), + &mut rng, + ) + .context("Could not initiate password change")?; let req = registration::ClientRegistrationStartRequest { username: ctx.props().username.clone(), registration_start_request: registration_start_request.message, diff --git a/app/src/components/create_user.rs b/app/src/components/create_user.rs index 8bb749e..5c2bd57 100644 --- a/app/src/components/create_user.rs +++ b/app/src/components/create_user.rs @@ -117,7 +117,10 @@ impl CommonComponent for CreateUserForm { let opaque::client::registration::ClientRegistrationStartResult { state, message, - } = opaque::client::registration::start_registration(&password, &mut rng)?; + } = opaque::client::registration::start_registration( + password.as_bytes(), + &mut rng, + )?; let req = registration::ClientRegistrationStartRequest { username: user_id, registration_start_request: message, diff --git a/app/src/components/reset_password_step2.rs b/app/src/components/reset_password_step2.rs index 5a75cc2..c46d1f6 100644 --- a/app/src/components/reset_password_step2.rs +++ b/app/src/components/reset_password_step2.rs @@ -65,7 +65,7 @@ impl CommonComponent for ResetPasswordStep2Form { let mut rng = rand::rngs::OsRng; let new_password = self.form.model().password; let registration_start_request = - opaque_registration::start_registration(&new_password, &mut rng) + opaque_registration::start_registration(new_password.as_bytes(), &mut rng) .context("Could not initiate password change")?; let req = registration::ClientRegistrationStartRequest { username: self.username.clone().unwrap(), diff --git a/auth/src/opaque.rs b/auth/src/opaque.rs index 571c3b8..cfdd396 100644 --- a/auth/src/opaque.rs +++ b/auth/src/opaque.rs @@ -77,10 +77,10 @@ pub mod client { pub use opaque_ke::ClientRegistrationFinishParameters; /// Initiate the registration negotiation. pub fn start_registration( - password: &str, + password: &[u8], rng: &mut R, ) -> AuthenticationResult { - Ok(ClientRegistration::start(rng, password.as_bytes())?) + Ok(ClientRegistration::start(rng, password)?) } /// Finalize the registration negotiation. diff --git a/server/src/domain/sql_backend_handler.rs b/server/src/domain/sql_backend_handler.rs index c888a15..afb5c78 100644 --- a/server/src/domain/sql_backend_handler.rs +++ b/server/src/domain/sql_backend_handler.rs @@ -59,7 +59,7 @@ pub mod tests { insert_user_no_password(handler, name).await; let mut rng = rand::rngs::OsRng; let client_registration_start = - opaque::client::registration::start_registration(pass, &mut rng).unwrap(); + opaque::client::registration::start_registration(pass.as_bytes(), &mut rng).unwrap(); let response = handler .registration_start(registration::ClientRegistrationStartRequest { username: name.to_string(), diff --git a/server/src/domain/sql_opaque_handler.rs b/server/src/domain/sql_opaque_handler.rs index 9f92b7a..a287c12 100644 --- a/server/src/domain/sql_opaque_handler.rs +++ b/server/src/domain/sql_opaque_handler.rs @@ -210,7 +210,7 @@ pub(crate) async fn register_password( let mut rng = rand::rngs::OsRng; use registration::*; let registration_start = - opaque::client::registration::start_registration(password.unsecure(), &mut rng)?; + opaque::client::registration::start_registration(password.unsecure().as_bytes(), &mut rng)?; let start_response = opaque_handler .registration_start(ClientRegistrationStartRequest { username: username.to_string(), diff --git a/server/src/infra/ldap_handler.rs b/server/src/infra/ldap_handler.rs index 4317d7b..9956f57 100644 --- a/server/src/infra/ldap_handler.rs +++ b/server/src/infra/ldap_handler.rs @@ -22,9 +22,10 @@ use crate::{ use anyhow::Result; use ldap3_proto::proto::{ LdapAddRequest, LdapBindCred, LdapBindRequest, LdapBindResponse, LdapCompareRequest, - LdapDerefAliases, LdapExtendedRequest, LdapExtendedResponse, LdapFilter, LdapOp, - LdapPartialAttribute, LdapPasswordModifyRequest, LdapResult as LdapResultOp, LdapResultCode, - LdapSearchRequest, LdapSearchResultEntry, LdapSearchScope, + LdapDerefAliases, LdapExtendedRequest, LdapExtendedResponse, LdapFilter, LdapModify, + LdapModifyRequest, LdapModifyType, LdapOp, LdapPartialAttribute, LdapPasswordModifyRequest, + LdapResult as LdapResultOp, LdapResultCode, LdapSearchRequest, LdapSearchResultEntry, + LdapSearchScope, }; use std::collections::HashMap; use tracing::{debug, instrument, warn}; @@ -128,6 +129,15 @@ fn make_extended_response(code: LdapResultCode, message: String) -> LdapOp { }) } +fn make_modify_response(code: LdapResultCode, message: String) -> LdapOp { + LdapOp::ModifyResponse(LdapResultOp { + code, + matcheddn: "".to_string(), + message, + referral: vec![], + }) +} + fn root_dse_response(base_dn: &str) -> LdapOp { LdapOp::SearchResultEntry(LdapSearchResultEntry { dn: "".to_string(), @@ -270,7 +280,7 @@ impl LdapHandler Result<()> { use lldap_auth::*; let mut rng = rand::rngs::OsRng; @@ -334,7 +344,7 @@ impl LdapHandler LdapHandler LdapResult<()> { + if change.modification.atype.to_ascii_lowercase() != "userpassword" + || change.operation != LdapModifyType::Replace + { + return Err(LdapError { + code: LdapResultCode::UnwillingToPerform, + message: format!( + r#"Unsupported operation: `{:?}` for `{}`"#, + change.operation, change.modification.atype + ), + }); + } + if !credentials.can_change_password(user_id, user_is_admin) { + return Err(LdapError { + code: LdapResultCode::InsufficentAccessRights, + message: format!( + r#"User `{}` cannot modify the password of user `{}`"#, + &credentials.user, &user_id + ), + }); + } + if let [value] = &change.modification.vals.as_slice() { + self.change_password(self.get_opaque_handler(), user_id, value) + .await + .map_err(|e| LdapError { + code: LdapResultCode::Other, + message: format!("Error while changing the password: {:#?}", e), + })?; + } else { + return Err(LdapError { + code: LdapResultCode::InvalidAttributeSyntax, + message: format!( + r#"Wrong number of values for password attribute: {}"#, + change.modification.vals.len() + ), + }); + } + Ok(()) + } + + async fn handle_modify_request( + &mut self, + request: &LdapModifyRequest, + ) -> LdapResult> { + let credentials = self + .user_info + .as_ref() + .ok_or_else(|| LdapError { + code: LdapResultCode::InsufficentAccessRights, + message: "No user currently bound".to_string(), + })? + .clone(); + match get_user_id_from_distinguished_name( + &request.dn, + &self.ldap_info.base_dn, + &self.ldap_info.base_dn_str, + ) { + Ok(uid) => { + let user_is_admin = self + .backend_handler + .get_readable_handler(&credentials, &uid) + .expect("Unexpected permission error") + .get_user_groups(&uid) + .await + .map_err(|e| LdapError { + code: LdapResultCode::OperationsError, + message: format!("Internal error while requesting user's groups: {:#?}", e), + })? + .iter() + .any(|g| g.display_name == "lldap_admin"); + for change in &request.changes { + self.handle_modify_change(&uid, &credentials, user_is_admin, change) + .await? + } + Ok(vec![make_modify_response( + LdapResultCode::Success, + String::new(), + )]) + } + Err(e) => Err(LdapError { + code: LdapResultCode::InvalidDNSyntax, + message: format!("Invalid username: {}", e), + }), + } + } + + async fn do_modify_request(&mut self, request: &LdapModifyRequest) -> Vec { + self.handle_modify_request(request) + .await + .unwrap_or_else(|e: LdapError| vec![make_modify_response(e.code, e.message)]) + } + pub async fn do_search_or_dse( &mut self, request: &LdapSearchRequest, @@ -650,6 +758,7 @@ impl LdapHandler self.do_modify_request(&request).await, LdapOp::ExtendedRequest(request) => self.do_extended_request(&request).await, LdapOp::AddRequest(request) => self .do_create_user(request) @@ -2007,7 +2116,8 @@ mod tests { use lldap_auth::*; let mut rng = rand::rngs::OsRng; let registration_start_request = - opaque::client::registration::start_registration("password", &mut rng).unwrap(); + opaque::client::registration::start_registration("password".as_bytes(), &mut rng) + .unwrap(); let request = registration::ClientRegistrationStartRequest { username: "bob".to_string(), registration_start_request: registration_start_request.message, @@ -2045,6 +2155,56 @@ mod tests { ); } + #[tokio::test] + async fn test_password_change_modify_request() { + let mut mock = MockTestBackendHandler::new(); + mock.expect_get_user_groups() + .with(eq(UserId::new("bob"))) + .returning(|_| Ok(HashSet::new())); + use lldap_auth::*; + let mut rng = rand::rngs::OsRng; + let registration_start_request = + opaque::client::registration::start_registration("password".as_bytes(), &mut rng) + .unwrap(); + let request = registration::ClientRegistrationStartRequest { + username: "bob".to_string(), + registration_start_request: registration_start_request.message, + }; + let start_response = opaque::server::registration::start_registration( + &opaque::server::ServerSetup::new(&mut rng), + request.registration_start_request, + &request.username, + ) + .unwrap(); + mock.expect_registration_start().times(1).return_once(|_| { + Ok(registration::ServerRegistrationStartResponse { + server_data: "".to_string(), + registration_response: start_response.message, + }) + }); + mock.expect_registration_finish() + .times(1) + .return_once(|_| Ok(())); + let mut ldap_handler = setup_bound_admin_handler(mock).await; + let request = LdapOp::ModifyRequest(LdapModifyRequest { + dn: "uid=bob,ou=people,dc=example,dc=com".to_string(), + changes: vec![LdapModify { + operation: LdapModifyType::Replace, + modification: LdapPartialAttribute { + atype: "userPassword".to_owned(), + vals: vec!["password".as_bytes().to_vec()], + }, + }], + }); + assert_eq!( + ldap_handler.handle_ldap_message(request).await, + Some(vec![make_modify_response( + LdapResultCode::Success, + "".to_string(), + )]) + ); + } + #[tokio::test] async fn test_password_change_password_manager() { let mut mock = MockTestBackendHandler::new(); @@ -2054,7 +2214,8 @@ mod tests { use lldap_auth::*; let mut rng = rand::rngs::OsRng; let registration_start_request = - opaque::client::registration::start_registration("password", &mut rng).unwrap(); + opaque::client::registration::start_registration("password".as_bytes(), &mut rng) + .unwrap(); let request = registration::ClientRegistrationStartRequest { username: "bob".to_string(), registration_start_request: registration_start_request.message, diff --git a/set-password/src/main.rs b/set-password/src/main.rs index 5cbf184..bfa73a6 100644 --- a/set-password/src/main.rs +++ b/set-password/src/main.rs @@ -107,7 +107,7 @@ fn main() -> Result<()> { let mut rng = rand::rngs::OsRng; let registration_start_request = - opaque::client::registration::start_registration(&opts.password, &mut rng) + opaque::client::registration::start_registration(opts.password.as_bytes(), &mut rng) .context("Could not initiate password change")?; let start_request = registration::ClientRegistrationStartRequest { username: opts.username.to_string(),