From 3137e3d682e5825f96ff11368b89344ca0e47bd1 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Sat, 20 Feb 2021 12:41:22 +1000 Subject: [PATCH] Complete MFA and Webauthn handlers (#360) Fixes #357 - this allows the password MFA handler to correct handle a mixed totp or webauthn credential with passwords. This is likely the "majority" of accounts we will see on the service. --- kanidm_client/src/lib.rs | 17 + kanidm_client/tests/proto_v1_test.rs | 115 ++++- kanidm_proto/src/v1.rs | 2 + kanidm_tools/src/cli/account.rs | 15 + kanidm_tools/src/opt/kanidm.rs | 2 + kanidmd/src/lib/actors/v1_write.rs | 24 +- kanidmd/src/lib/credential/mod.rs | 57 ++- kanidmd/src/lib/idm/account.rs | 18 + kanidmd/src/lib/idm/authsession.rs | 645 +++++++++++++++++++++++---- kanidmd/src/lib/idm/event.rs | 36 ++ kanidmd/src/lib/idm/server.rs | 72 ++- 11 files changed, 911 insertions(+), 92 deletions(-) diff --git a/kanidm_client/src/lib.rs b/kanidm_client/src/lib.rs index ecc1ff27e..4685f4353 100644 --- a/kanidm_client/src/lib.rs +++ b/kanidm_client/src/lib.rs @@ -1068,6 +1068,23 @@ impl KanidmClient { } } + pub fn idm_account_primary_credential_remove_webauthn( + &self, + id: &str, + label: &str, + ) -> Result { + let r = SetCredentialRequest::WebauthnRemove(label.to_string()); + let res: Result = self.perform_put_request( + format!("/v1/account/{}/_credential/primary", id).as_str(), + r, + ); + match res { + Ok(SetCredentialResponse::Success) => Ok(true), + Ok(_) => Err(ClientError::EmptyResponse), + Err(e) => Err(e), + } + } + pub fn idm_account_radius_credential_get( &self, id: &str, diff --git a/kanidm_client/tests/proto_v1_test.rs b/kanidm_client/tests/proto_v1_test.rs index a1d9db6e7..1e93fc2b9 100644 --- a/kanidm_client/tests/proto_v1_test.rs +++ b/kanidm_client/tests/proto_v1_test.rs @@ -788,6 +788,103 @@ fn test_server_rest_totp_auth_lifecycle() { #[test] fn test_server_rest_webauthn_auth_lifecycle() { + run_test(|mut rsclient: KanidmClient| { + let res = rsclient.auth_simple_password("admin", ADMIN_TEST_PASSWORD); + assert!(res.is_ok()); + + // Not recommended in production! + rsclient + .idm_group_add_members("idm_admins", &["admin"]) + .unwrap(); + + // Create a new account + rsclient + .idm_account_create("demo_account", "Deeeeemo") + .unwrap(); + + // Enroll a soft token to the account webauthn. + let mut wa_softtok = WebauthnAuthenticator::new(U2FSoft::new()); + + // Do the challenge + let (sessionid, regchal) = rsclient + .idm_account_primary_credential_register_webauthn("demo_account", "softtok") + .unwrap(); + + let rego = wa_softtok + .do_registration("https://idm.example.com", regchal) + .expect("Failed to register to softtoken"); + + // Enroll the cred after signing. + rsclient + .idm_account_primary_credential_complete_webuthn_registration( + "demo_account", + rego, + sessionid, + ) + .unwrap(); + + // ====== Reg a second token. + let mut wa_softtok_2 = WebauthnAuthenticator::new(U2FSoft::new()); + + // Do the challenge + let (sessionid, regchal) = rsclient + .idm_account_primary_credential_register_webauthn("demo_account", "softtok_2") + .unwrap(); + + let rego = wa_softtok_2 + .do_registration("https://idm.example.com", regchal) + .expect("Failed to register to softtoken"); + + // Enroll the cred after signing. + rsclient + .idm_account_primary_credential_complete_webuthn_registration( + "demo_account", + rego, + sessionid, + ) + .unwrap(); + + // Now do an auth + let mut rsclient_good = rsclient.new_session().unwrap(); + + let pkr = rsclient_good.auth_webauthn_begin("demo_account").unwrap(); + + // Get the auth chal. + let auth = wa_softtok_2 + .do_authentication("https://idm.example.com", pkr) + .expect("Failed to auth to softtoken"); + + // Submit the webauthn auth. + rsclient_good + .auth_webauthn_complete(auth) + .expect("Failed to authenticate"); + + // ======== remove the second softtok. + + rsclient + .idm_account_primary_credential_remove_webauthn("demo_account", "softtok_2") + .expect("failed to remove softtoken"); + + // All good, check first tok auth. + + let mut rsclient_good = rsclient.new_session().unwrap(); + + let pkr = rsclient_good.auth_webauthn_begin("demo_account").unwrap(); + + // Get the auth chal. + let auth = wa_softtok + .do_authentication("https://idm.example.com", pkr) + .expect("Failed to auth to softtoken"); + + // Submit the webauthn auth. + rsclient_good + .auth_webauthn_complete(auth) + .expect("Failed to authenticate"); + }); +} + +#[test] +fn test_server_rest_webauthn_mfa_auth_lifecycle() { run_test(|mut rsclient: KanidmClient| { let res = rsclient.auth_simple_password("admin", ADMIN_TEST_PASSWORD); assert!(res.is_ok()); @@ -836,7 +933,23 @@ fn test_server_rest_webauthn_auth_lifecycle() { // Submit the webauthn auth. rsclient_good .auth_webauthn_complete(auth) - .expect("Failed to authenticate") + .expect("Failed to authenticate"); + + // Set a password to cause the state to change to PasswordMFA + assert!(rsclient + .idm_account_primary_credential_set_password("demo_account", "sohdi3iuHo6mai7noh0a") + .is_ok()); + + // Now remove Webauthn ... + rsclient + .idm_account_primary_credential_remove_webauthn("demo_account", "softtok") + .expect("failed to remove softtoken"); + + // Check pw only + let mut rsclient_good = rsclient.new_session().unwrap(); + assert!(rsclient_good + .auth_simple_password("demo_account", "sohdi3iuHo6mai7noh0a") + .is_ok()); }); } diff --git a/kanidm_proto/src/v1.rs b/kanidm_proto/src/v1.rs index d59cd4d89..6e3a4c802 100644 --- a/kanidm_proto/src/v1.rs +++ b/kanidm_proto/src/v1.rs @@ -583,6 +583,8 @@ pub enum SetCredentialRequest { WebauthnBegin(String), // Finish it. WebauthnRegister(Uuid, RegisterPublicKeyCredential), + // Remove + WebauthnRemove(String), } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/kanidm_tools/src/cli/account.rs b/kanidm_tools/src/cli/account.rs index 1c7e548fe..b6b777609 100644 --- a/kanidm_tools/src/cli/account.rs +++ b/kanidm_tools/src/cli/account.rs @@ -16,6 +16,7 @@ impl AccountOpt { AccountCredential::SetPassword(acs) => acs.copt.debug, AccountCredential::GeneratePassword(acs) => acs.copt.debug, AccountCredential::RegisterWebauthn(acs) => acs.copt.debug, + AccountCredential::RemoveWebauthn(acs) => acs.copt.debug, AccountCredential::RegisterTOTP(acs) => acs.copt.debug, AccountCredential::RemoveTOTP(acs) => acs.copt.debug, }, @@ -126,6 +127,20 @@ impl AccountOpt { } } } + AccountCredential::RemoveWebauthn(acsopt) => { + let client = acsopt.copt.to_client(); + match client.idm_account_primary_credential_remove_webauthn( + acsopt.aopts.account_id.as_str(), + acsopt.tag.as_str(), + ) { + Ok(_) => { + println!("Webauthn removal success."); + } + Err(e) => { + eprintln!("Error Removing Webauthn from account -> {:?}", e); + } + } + } AccountCredential::RegisterTOTP(acsopt) => { let client = acsopt.copt.to_client(); let (session, tok) = match client.idm_account_primary_credential_generate_totp( diff --git a/kanidm_tools/src/opt/kanidm.rs b/kanidm_tools/src/opt/kanidm.rs index ab9ff9f97..6ec99bbde 100644 --- a/kanidm_tools/src/opt/kanidm.rs +++ b/kanidm_tools/src/opt/kanidm.rs @@ -156,6 +156,8 @@ pub enum AccountCredential { GeneratePassword(AccountCredentialSet), #[structopt(name = "register_webauthn")] RegisterWebauthn(AccountNamedTagOpt), + #[structopt(name = "remove_webauthn")] + RemoveWebauthn(AccountNamedTagOpt), /// Set the TOTP credential of the account. If a TOTP already exists, on a successful /// registration, this will replace it. #[structopt(name = "set_totp")] diff --git a/kanidmd/src/lib/actors/v1_write.rs b/kanidmd/src/lib/actors/v1_write.rs index 951bdb46f..4738060f2 100644 --- a/kanidmd/src/lib/actors/v1_write.rs +++ b/kanidmd/src/lib/actors/v1_write.rs @@ -9,8 +9,8 @@ use crate::event::{ }; use crate::idm::event::{ GeneratePasswordEvent, GenerateTOTPEvent, PasswordChangeEvent, RegenerateRadiusSecretEvent, - RemoveTOTPEvent, UnixPasswordChangeEvent, VerifyTOTPEvent, WebauthnDoRegisterEvent, - WebauthnInitRegisterEvent, + RemoveTOTPEvent, RemoveWebauthnEvent, UnixPasswordChangeEvent, VerifyTOTPEvent, + WebauthnDoRegisterEvent, WebauthnInitRegisterEvent, }; use crate::modify::{Modify, ModifyInvalid, ModifyList}; use crate::value::{PartialValue, Value}; @@ -703,6 +703,26 @@ impl QueryServerWriteV1 { .reg_account_webauthn_complete(&mut audit, &wre) .and_then(|r| idms_prox_write.commit(&mut audit).map(|_| r)) } + SetCredentialRequest::WebauthnRemove(label) => { + let rwe = RemoveWebauthnEvent::from_parts( + &mut audit, + &idms_prox_write.qs_write, + msg.uat.as_ref(), + target_uuid, + label, + ) + .map_err(|e| { + ladmin_error!( + audit, + "Failed to begin internal_credential_set_message: {:?}", + e + ); + e + })?; + idms_prox_write + .remove_account_webauthn(&mut audit, &rwe) + .and_then(|r| idms_prox_write.commit(&mut audit).map(|_| r)) + } } } ); diff --git a/kanidmd/src/lib/credential/mod.rs b/kanidmd/src/lib/credential/mod.rs index 520132226..df886b4d2 100644 --- a/kanidmd/src/lib/credential/mod.rs +++ b/kanidmd/src/lib/credential/mod.rs @@ -380,6 +380,57 @@ impl Credential { }) } + pub fn remove_webauthn(&self, label: &str) -> Result { + let type_ = match &self.type_ { + CredentialType::Password(_) | CredentialType::GeneratedPassword(_) => { + return Err(OperationError::InvalidAttribute( + "Webauthn is not present on this credential".to_string(), + )); + } + CredentialType::PasswordMFA(pw, totp, map) => { + let mut nmap = map.clone(); + if nmap.remove(label).is_none() { + return Err(OperationError::InvalidAttribute(format!( + "Removing Webauthn token with label '{:?}': does not exist", + label + ))); + } + if nmap.is_empty() { + if totp.is_some() { + CredentialType::PasswordMFA(pw.clone(), totp.clone(), nmap) + } else { + CredentialType::Password(pw.clone()) + } + } else { + CredentialType::PasswordMFA(pw.clone(), totp.clone(), nmap) + } + } + CredentialType::Webauthn(map) => { + let mut nmap = map.clone(); + if nmap.remove(label).is_none() { + return Err(OperationError::InvalidAttribute(format!( + "Removing Webauthn token with label '{:?}': does not exist", + label + ))); + } + if nmap.is_empty() { + return Err(OperationError::InvalidAttribute(format!( + "Removing Webauthn token with label '{:?}': unable to remove, this is the last webauthn token", + label + ))); + } + CredentialType::Webauthn(nmap) + } + }; + + // Check stuff + Ok(Credential { + type_, + claims: self.claims.clone(), + uuid: self.uuid, + }) + } + #[allow(clippy::ptr_arg)] pub fn update_webauthn_counter( &self, @@ -527,11 +578,7 @@ impl Credential { CredentialType::PasswordMFA(_, totp, wan) => { CredentialType::PasswordMFA(pw, totp.clone(), wan.clone()) } - CredentialType::Webauthn(wan) => { - // Or should this become PasswordWebauthn? - debug_assert!(false); - CredentialType::Webauthn(wan.clone()) - } + CredentialType::Webauthn(wan) => CredentialType::PasswordMFA(pw, None, wan.clone()), }; Credential { type_, diff --git a/kanidmd/src/lib/idm/account.rs b/kanidmd/src/lib/idm/account.rs index c9cf475ad..52f4a4559 100644 --- a/kanidmd/src/lib/idm/account.rs +++ b/kanidmd/src/lib/idm/account.rs @@ -274,6 +274,24 @@ impl Account { Ok(ModifyList::new_purge_and_set("primary_credential", vcred)) } + pub(crate) fn gen_webauthn_remove_mod( + &self, + label: &str, + ) -> Result, OperationError> { + match &self.primary { + // Change the cred + Some(primary) => { + let ncred = primary.remove_webauthn(label)?; + let vcred = Value::new_credential("primary", ncred); + Ok(ModifyList::new_purge_and_set("primary_credential", vcred)) + } + None => { + // No credential exists, we can't remove what is not real. + Err(OperationError::InvalidState) + } + } + } + #[allow(clippy::ptr_arg)] pub(crate) fn gen_webauthn_counter_mod( &self, diff --git a/kanidmd/src/lib/idm/authsession.rs b/kanidmd/src/lib/idm/authsession.rs index 34e4d94af..0b0d6f023 100644 --- a/kanidmd/src/lib/idm/authsession.rs +++ b/kanidmd/src/lib/idm/authsession.rs @@ -44,11 +44,12 @@ enum CredVerifyState { } #[derive(Clone, Debug)] -struct CredTotpPw { +struct CredMfa { pw: Password, pw_state: CredVerifyState, - totp: TOTP, - totp_state: CredVerifyState, + totp: Option, + wan: Option<(RequestChallengeResponse, AuthenticationState)>, + mfa_state: CredVerifyState, } #[derive(Clone, Debug)] @@ -63,7 +64,7 @@ enum CredHandler { Anonymous, // AppPassword (?) Password(Password), - PasswordMFA(CredTotpPw), + PasswordMFA(Box), Webauthn(CredWebauthn), // Webauthn + Password } @@ -79,15 +80,41 @@ impl CredHandler { CredentialType::Password(pw) | CredentialType::GeneratedPassword(pw) => { Ok(CredHandler::Password(pw.clone())) } - CredentialType::PasswordMFA(pw, Some(totp), _) => { - Ok(CredHandler::PasswordMFA(CredTotpPw { + CredentialType::PasswordMFA(pw, maybe_totp, maybe_wan) => { + let wan = if !maybe_wan.is_empty() { + webauthn + .generate_challenge_authenticate(maybe_wan.values().cloned().collect()) + .map(Some) + .map_err(|e| { + lsecurity!( + au, + "Unable to create webauthn authentication challenge -> {:?}", + e + ); + })? + } else { + None + }; + + let cmfa = Box::new(CredMfa { pw: pw.clone(), pw_state: CredVerifyState::Init, - totp: totp.clone(), - totp_state: CredVerifyState::Init, - })) + totp: maybe_totp.clone(), + wan, + mfa_state: CredVerifyState::Init, + }); + + // Paranoia. Should NEVER occur. + if cmfa.totp.is_none() && cmfa.wan.is_none() { + lsecurity_critical!( + au, + "Unable to create CredHandler::PasswordMFA - totp and webauthn are both not present. Credentials MAY be corrupt!" + ); + return Err(()); + } + + Ok(CredHandler::PasswordMFA(cmfa)) } - CredentialType::PasswordMFA(_, None, _) => Err(()), CredentialType::Webauthn(wan) => webauthn .generate_challenge_authenticate(wan.values().cloned().collect()) .map(|(chal, wan_state)| { @@ -177,60 +204,110 @@ impl CredHandler { } } - fn validate_totp_password( + fn validate_password_mfa( au: &mut AuditScope, cred: &AuthCredential, ts: &Duration, - pw_totp: &mut CredTotpPw, + pw_mfa: &mut CredMfa, + webauthn: &Webauthn, who: Uuid, async_tx: &Sender, ) -> CredState { - match (cred, &pw_totp.totp_state, &pw_totp.pw_state) { - // Must be done first. - (AuthCredential::TOTP(totp_chal), CredVerifyState::Init, CredVerifyState::Init) => { - if pw_totp.totp.verify(*totp_chal, ts) { - pw_totp.totp_state = CredVerifyState::Success; - lsecurity!( - au, - "Handler::PasswordMFA -> Result::Continue - TOTP OK, password -" - ); - CredState::Continue(vec![AuthAllowed::Password]) - } else { - pw_totp.totp_state = CredVerifyState::Fail; - lsecurity!( - au, - "Handler::PasswordMFA -> Result::Denied - TOTP Fail, password -" - ); - CredState::Denied(BAD_TOTP_MSG) + match (&pw_mfa.mfa_state, &pw_mfa.pw_state) { + (CredVerifyState::Init, CredVerifyState::Init) => { + // MFA first + match (cred, pw_mfa.totp.as_ref(), pw_mfa.wan.as_ref()) { + (AuthCredential::Webauthn(resp), _, Some((_, wan_state))) => { + webauthn.authenticate_credential(&resp, wan_state.clone()) + .map(|r| { + pw_mfa.mfa_state = CredVerifyState::Success; + // Success. Determine if we need to update the counter + // async from r. + if let Some((cid, counter)) = r { + // Do async + if let Err(_e) = async_tx.send(DelayedAction::WebauthnCounterIncrement(WebauthnCounterIncrement { + target_uuid: who, + cid, + counter, + })) { + ladmin_warning!(au, "unable to queue delayed webauthn counter increment, continuing ... "); + }; + }; + CredState::Continue(vec![AuthAllowed::Password]) + }) + .unwrap_or_else(|e| { + pw_mfa.mfa_state = CredVerifyState::Fail; + // Denied. + lsecurity!(au, "Handler::Webauthn -> Result::Denied - webauthn error {:?}", e); + CredState::Denied(BAD_WEBAUTHN_MSG) + }) + } + (AuthCredential::TOTP(totp_chal), Some(totp), _) => { + if totp.verify(*totp_chal, ts) { + pw_mfa.mfa_state = CredVerifyState::Success; + lsecurity!( + au, + "Handler::PasswordMFA -> Result::Continue - TOTP OK, password -" + ); + CredState::Continue(vec![AuthAllowed::Password]) + } else { + pw_mfa.mfa_state = CredVerifyState::Fail; + lsecurity!( + au, + "Handler::PasswordMFA -> Result::Denied - TOTP Fail, password -" + ); + CredState::Denied(BAD_TOTP_MSG) + } + } + _ => { + lsecurity!( + au, + "Handler::PasswordMFA -> Result::Denied - invalid cred type for handler" + ); + CredState::Denied(BAD_AUTH_TYPE_MSG) + } } } - // Must only proceed if totp was success. - ( - AuthCredential::Password(cleartext), - CredVerifyState::Success, - CredVerifyState::Init, - ) => { - if pw_totp.pw.verify(cleartext.as_str()).unwrap_or(false) { - pw_totp.pw_state = CredVerifyState::Success; - lsecurity!( - au, - "Handler::PasswordMFA -> Result::Success - TOTP OK, password OK" - ); - Self::maybe_pw_upgrade(au, &pw_totp.pw, who, cleartext.as_str(), async_tx); - CredState::Success(Vec::new()) - } else { - pw_totp.pw_state = CredVerifyState::Fail; - lsecurity!( - au, - "Handler::PasswordMFA -> Result::Denied - TOTP OK, password Fail" - ); - CredState::Denied(BAD_PASSWORD_MSG) + (CredVerifyState::Success, CredVerifyState::Init) => { + // PW second. + match cred { + AuthCredential::Password(cleartext) => { + if pw_mfa.pw.verify(cleartext.as_str()).unwrap_or(false) { + pw_mfa.pw_state = CredVerifyState::Success; + lsecurity!( + au, + "Handler::PasswordMFA -> Result::Success - TOTP OK, password OK" + ); + Self::maybe_pw_upgrade( + au, + &pw_mfa.pw, + who, + cleartext.as_str(), + async_tx, + ); + CredState::Success(Vec::new()) + } else { + pw_mfa.pw_state = CredVerifyState::Fail; + lsecurity!( + au, + "Handler::PasswordMFA -> Result::Denied - TOTP OK, password Fail" + ); + CredState::Denied(BAD_PASSWORD_MSG) + } + } + _ => { + lsecurity!( + au, + "Handler::PasswordMFA -> Result::Denied - invalid cred type for handler" + ); + CredState::Denied(BAD_AUTH_TYPE_MSG) + } } } _ => { lsecurity!( au, - "Handler::PasswordMFA -> Result::Denied - invalid cred type for handler" + "Handler::PasswordMFA -> Result::Denied - invalid credential mfa and pw state" ); CredState::Denied(BAD_AUTH_TYPE_MSG) } @@ -304,8 +381,8 @@ impl CredHandler { CredHandler::Password(ref mut pw) => { Self::validate_password(au, cred, pw, who, async_tx) } - CredHandler::PasswordMFA(ref mut pw_totp) => { - Self::validate_totp_password(au, cred, ts, pw_totp, who, async_tx) + CredHandler::PasswordMFA(ref mut pw_mfa) => { + Self::validate_password_mfa(au, cred, ts, pw_mfa, webauthn, who, async_tx) } CredHandler::Webauthn(ref mut wan_cred) => { Self::validate_webauthn(au, cred, wan_cred, webauthn, who, async_tx) @@ -317,9 +394,17 @@ impl CredHandler { match &self { CredHandler::Anonymous => vec![AuthAllowed::Anonymous], CredHandler::Password(_) => vec![AuthAllowed::Password], - // webauth - // mfa - CredHandler::PasswordMFA(_) => vec![AuthAllowed::TOTP], + CredHandler::PasswordMFA(ref pw_mfa) => pw_mfa + .totp + .iter() + .map(|_| AuthAllowed::TOTP) + .chain( + pw_mfa + .wan + .iter() + .map(|(chal, _)| AuthAllowed::Webauthn(chal.clone())), + ) + .collect(), CredHandler::Webauthn(webauthn) => vec![AuthAllowed::Webauthn(webauthn.chal.clone())], } } @@ -806,7 +891,7 @@ mod tests { $webauthn, duration_from_epoch_now(), ); - let mut session = session.unwrap(); + let mut session = session.expect("Session was unable to be created."); if let AuthState::Choose(auth_mechs) = state { assert!( @@ -823,10 +908,16 @@ mod tests { .start_session($audit, &AuthMech::PasswordMFA) .expect("Failed to select anonymous mech."); + let mut rchal = None; + if let AuthState::Continue(auth_mechs) = state { assert!( true == auth_mechs.iter().fold(false, |acc, x| match x { // TODO: How to return webauthn chal? + AuthAllowed::Webauthn(chal) => { + rchal = Some(chal.clone()); + true + } AuthAllowed::TOTP => true, _ => acc, }) @@ -835,7 +926,7 @@ mod tests { panic!("Invalid auth state") } - session + (session, rchal) }}; } @@ -880,7 +971,7 @@ mod tests { // check send anon (fail) { - let mut session = start_password_mfa_session!(&mut audit, account, &webauthn); + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); match session.validate_creds( &mut audit, @@ -898,7 +989,7 @@ mod tests { // Sending a PW first is an immediate fail. { - let mut session = start_password_mfa_session!(&mut audit, account, &webauthn); + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); match session.validate_creds( &mut audit, @@ -913,7 +1004,7 @@ mod tests { } // check send bad totp, should fail immediate { - let mut session = start_password_mfa_session!(&mut audit, account, &webauthn); + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); match session.validate_creds( &mut audit, @@ -930,7 +1021,7 @@ mod tests { // check send good totp, should continue // then bad pw, fail pw { - let mut session = start_password_mfa_session!(&mut audit, account, &webauthn); + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); match session.validate_creds( &mut audit, @@ -957,7 +1048,7 @@ mod tests { // check send good totp, should continue // then good pw, success { - let mut session = start_password_mfa_session!(&mut audit, account, &webauthn); + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); match session.validate_creds( &mut audit, @@ -1033,24 +1124,19 @@ mod tests { }}; } - #[test] - fn test_idm_authsession_webauthn_only_mech() { - let mut audit = AuditScope::new( - "test_idm_authsession_webauthn_mech", - uuid::Uuid::new_v4(), - None, - ); + fn setup_webauthn( + name: &str, + ) -> ( + webauthn_rs::Webauthn, + webauthn_authenticator_rs::WebauthnAuthenticator, + webauthn_rs::proto::Credential, + ) { let webauthn = create_webauthn(); - let (async_tx, mut async_rx) = unbounded(); - let ts = duration_from_epoch_now(); - // create the ent - let mut account = entry_str_to_account!(JSON_ADMIN_V1); - // Setup a soft token let mut wa = WebauthnAuthenticator::new(U2FSoft::new()); let (chal, reg_state) = webauthn - .generate_challenge_register(&account.name, Some(UserVerificationPolicy::Discouraged)) + .generate_challenge_register(name, Some(UserVerificationPolicy::Discouraged)) .expect("Failed to setup webauthn rego challenge"); let r = wa @@ -1061,13 +1147,28 @@ mod tests { .register_credential(&r, reg_state, |_| Ok(false)) .expect("Failed to register soft token"); + (webauthn, wa, wan_cred) + } + + #[test] + fn test_idm_authsession_webauthn_only_mech() { + let mut audit = AuditScope::new( + "test_idm_authsession_webauthn_only_mech", + uuid::Uuid::new_v4(), + None, + ); + let (async_tx, mut async_rx) = unbounded(); + let ts = duration_from_epoch_now(); + // create the ent + let mut account = entry_str_to_account!(JSON_ADMIN_V1); + + let (webauthn, mut wa, wan_cred) = setup_webauthn(account.name.as_str()); + // Now create the credential for the account. let cred = Credential::new_webauthn_only("soft".to_string(), wan_cred); account.primary = Some(cred); - // now check correct mech was offered. we stash this challenge for later - // to help generate a failure. - let (_session, inv_chal) = start_webauthn_only_session!(&mut audit, account, &webauthn); + // now check correct mech was offered. // check send anon (fail) { @@ -1114,6 +1215,7 @@ mod tests { // Check bad challenge. { + let (_session, inv_chal) = start_webauthn_only_session!(&mut audit, account, &webauthn); let (mut session, _chal) = start_webauthn_only_session!(&mut audit, account, &webauthn); let resp = wa @@ -1181,4 +1283,389 @@ mod tests { assert!(async_rx.blocking_recv().is_none()); audit.write_log(); } + + #[test] + fn test_idm_authsession_webauthn_password_mech() { + let mut audit = AuditScope::new( + "test_idm_authsession_webauthn_password_mech", + uuid::Uuid::new_v4(), + None, + ); + let (async_tx, mut async_rx) = unbounded(); + let ts = duration_from_epoch_now(); + // create the ent + let mut account = entry_str_to_account!(JSON_ADMIN_V1); + + let (webauthn, mut wa, wan_cred) = setup_webauthn(account.name.as_str()); + let pw_good = "test_password"; + let pw_bad = "bad_password"; + + // Now create the credential for the account. + let p = CryptoPolicy::minimum(); + let cred = Credential::new_password_only(&p, pw_good) + .unwrap() + .append_webauthn("soft".to_string(), wan_cred) + .unwrap(); + + account.primary = Some(cred); + + // check pw first (fail) + { + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); + + match session.validate_creds( + &mut audit, + &AuthCredential::Password(pw_bad.to_string()), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_AUTH_TYPE_MSG), + _ => panic!(), + }; + } + + // Check totp first attempt fails. + { + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); + + match session.validate_creds( + &mut audit, + &AuthCredential::TOTP(0), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_AUTH_TYPE_MSG), + _ => panic!(), + }; + } + + // check bad webauthn (fail) + // NOTE: We only check bad challenge here as bad softtoken is already + // extensively tested. + { + let (_session, inv_chal) = start_password_mfa_session!(&mut audit, account, &webauthn); + let (mut session, _chal) = start_password_mfa_session!(&mut audit, account, &webauthn); + + let inv_chal = inv_chal.unwrap(); + + let resp = wa + // HERE -> we use inv_chal instead. + .do_authentication("https://idm.example.com", inv_chal) + .expect("failed to use softtoken to authenticate"); + + match session.validate_creds( + &mut audit, + &AuthCredential::Webauthn(resp), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_WEBAUTHN_MSG), + _ => panic!(), + }; + } + + // check good webauthn/bad pw (fail) + { + let (mut session, chal) = start_password_mfa_session!(&mut audit, account, &webauthn); + let chal = chal.unwrap(); + + let resp = wa + .do_authentication("https://idm.example.com", chal) + .expect("failed to use softtoken to authenticate"); + + match session.validate_creds( + &mut audit, + &AuthCredential::Webauthn(resp), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]), + _ => panic!(), + }; + match session.validate_creds( + &mut audit, + &AuthCredential::Password(pw_bad.to_string()), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_PASSWORD_MSG), + _ => panic!(), + }; + + // Check the async counter update was sent. + match async_rx.blocking_recv() { + Some(DelayedAction::WebauthnCounterIncrement(_)) => {} + _ => assert!(false), + } + } + + // Check good webauthn/good pw (pass) + { + let (mut session, chal) = start_password_mfa_session!(&mut audit, account, &webauthn); + let chal = chal.unwrap(); + + let resp = wa + .do_authentication("https://idm.example.com", chal) + .expect("failed to use softtoken to authenticate"); + + match session.validate_creds( + &mut audit, + &AuthCredential::Webauthn(resp), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]), + _ => panic!(), + }; + match session.validate_creds( + &mut audit, + &AuthCredential::Password(pw_good.to_string()), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Success(_)) => {} + _ => panic!(), + }; + + // Check the async counter update was sent. + match async_rx.blocking_recv() { + Some(DelayedAction::WebauthnCounterIncrement(_)) => {} + _ => assert!(false), + } + } + + drop(async_tx); + assert!(async_rx.blocking_recv().is_none()); + audit.write_log(); + } + + #[test] + fn test_idm_authsession_webauthn_password_totp_mech() { + let mut audit = AuditScope::new( + "test_idm_authsession_webauthn_password_totp_mech", + uuid::Uuid::new_v4(), + None, + ); + let (async_tx, mut async_rx) = unbounded(); + let ts = duration_from_epoch_now(); + // create the ent + let mut account = entry_str_to_account!(JSON_ADMIN_V1); + + let (webauthn, mut wa, wan_cred) = setup_webauthn(account.name.as_str()); + + let totp = TOTP::generate_secure("test_totp".to_string(), TOTP_DEFAULT_STEP); + let totp_good = totp + .do_totp_duration_from_epoch(&ts) + .expect("failed to perform totp."); + let totp_bad = totp + .do_totp_duration_from_epoch(&Duration::from_secs(1234567)) + .expect("failed to perform totp."); + assert!(totp_bad != totp_good); + + let pw_good = "test_password"; + let pw_bad = "bad_password"; + + // Now create the credential for the account. + let p = CryptoPolicy::minimum(); + let cred = Credential::new_password_only(&p, pw_good) + .unwrap() + .append_webauthn("soft".to_string(), wan_cred) + .unwrap() + .update_totp(totp); + + account.primary = Some(cred); + + // check pw first (fail) + { + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); + + match session.validate_creds( + &mut audit, + &AuthCredential::Password(pw_bad.to_string()), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_AUTH_TYPE_MSG), + _ => panic!(), + }; + } + + // Check bad totp (fail) + { + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); + + match session.validate_creds( + &mut audit, + &AuthCredential::TOTP(totp_bad), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_TOTP_MSG), + _ => panic!(), + }; + } + + // check bad webauthn (fail) + { + let (_session, inv_chal) = start_password_mfa_session!(&mut audit, account, &webauthn); + let (mut session, _chal) = start_password_mfa_session!(&mut audit, account, &webauthn); + + let inv_chal = inv_chal.unwrap(); + + let resp = wa + // HERE -> we use inv_chal instead. + .do_authentication("https://idm.example.com", inv_chal) + .expect("failed to use softtoken to authenticate"); + + match session.validate_creds( + &mut audit, + &AuthCredential::Webauthn(resp), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_WEBAUTHN_MSG), + _ => panic!(), + }; + } + + // check good webauthn/bad pw (fail) + { + let (mut session, chal) = start_password_mfa_session!(&mut audit, account, &webauthn); + let chal = chal.unwrap(); + + let resp = wa + .do_authentication("https://idm.example.com", chal) + .expect("failed to use softtoken to authenticate"); + + match session.validate_creds( + &mut audit, + &AuthCredential::Webauthn(resp), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]), + _ => panic!(), + }; + match session.validate_creds( + &mut audit, + &AuthCredential::Password(pw_bad.to_string()), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_PASSWORD_MSG), + _ => panic!(), + }; + + // Check the async counter update was sent. + match async_rx.blocking_recv() { + Some(DelayedAction::WebauthnCounterIncrement(_)) => {} + _ => assert!(false), + } + } + + // check good totp/bad pw (fail) + { + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); + + match session.validate_creds( + &mut audit, + &AuthCredential::TOTP(totp_good), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]), + _ => panic!(), + }; + match session.validate_creds( + &mut audit, + &AuthCredential::Password(pw_bad.to_string()), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Denied(msg)) => assert!(msg == BAD_PASSWORD_MSG), + _ => panic!(), + }; + } + + // check good totp/good pw (pass) + { + let (mut session, _) = start_password_mfa_session!(&mut audit, account, &webauthn); + + match session.validate_creds( + &mut audit, + &AuthCredential::TOTP(totp_good), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]), + _ => panic!(), + }; + match session.validate_creds( + &mut audit, + &AuthCredential::Password(pw_good.to_string()), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Success(_)) => {} + _ => panic!(), + }; + } + + // Check good webauthn/good pw (pass) + { + let (mut session, chal) = start_password_mfa_session!(&mut audit, account, &webauthn); + let chal = chal.unwrap(); + + let resp = wa + .do_authentication("https://idm.example.com", chal) + .expect("failed to use softtoken to authenticate"); + + match session.validate_creds( + &mut audit, + &AuthCredential::Webauthn(resp), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Continue(cont)) => assert!(cont == vec![AuthAllowed::Password]), + _ => panic!(), + }; + match session.validate_creds( + &mut audit, + &AuthCredential::Password(pw_good.to_string()), + &ts, + &async_tx, + &webauthn, + ) { + Ok(AuthState::Success(_)) => {} + _ => panic!(), + }; + + // Check the async counter update was sent. + match async_rx.blocking_recv() { + Some(DelayedAction::WebauthnCounterIncrement(_)) => {} + _ => assert!(false), + } + } + + drop(async_tx); + assert!(async_rx.blocking_recv().is_none()); + audit.write_log(); + } } diff --git a/kanidmd/src/lib/idm/event.rs b/kanidmd/src/lib/idm/event.rs index 6a9cb225e..f2e21d495 100644 --- a/kanidmd/src/lib/idm/event.rs +++ b/kanidmd/src/lib/idm/event.rs @@ -440,6 +440,42 @@ impl WebauthnDoRegisterEvent { } } +#[derive(Debug)] +pub struct RemoveWebauthnEvent { + pub event: Event, + pub target: Uuid, + pub label: String, +} + +impl RemoveWebauthnEvent { + pub fn from_parts( + audit: &mut AuditScope, + qs: &QueryServerWriteTransaction, + uat: Option<&UserAuthToken>, + target: Uuid, + label: String, + ) -> Result { + let e = Event::from_rw_uat(audit, qs, uat)?; + + Ok(RemoveWebauthnEvent { + event: e, + target, + label, + }) + } + + #[cfg(test)] + pub fn new_internal(target: Uuid, label: String) -> Self { + let e = Event::from_internal(); + + RemoveWebauthnEvent { + event: e, + target, + label, + } + } +} + pub struct LdapAuthEvent { // pub event: Event, pub target: Uuid, diff --git a/kanidmd/src/lib/idm/server.rs b/kanidmd/src/lib/idm/server.rs index fcdcb4240..4d86ec186 100644 --- a/kanidmd/src/lib/idm/server.rs +++ b/kanidmd/src/lib/idm/server.rs @@ -9,9 +9,9 @@ use crate::idm::account::Account; use crate::idm::authsession::AuthSession; use crate::idm::event::{ GeneratePasswordEvent, GenerateTOTPEvent, LdapAuthEvent, PasswordChangeEvent, - RadiusAuthTokenEvent, RegenerateRadiusSecretEvent, RemoveTOTPEvent, UnixGroupTokenEvent, - UnixPasswordChangeEvent, UnixUserAuthEvent, UnixUserTokenEvent, VerifyTOTPEvent, - WebauthnDoRegisterEvent, WebauthnInitRegisterEvent, + RadiusAuthTokenEvent, RegenerateRadiusSecretEvent, RemoveTOTPEvent, RemoveWebauthnEvent, + UnixGroupTokenEvent, UnixPasswordChangeEvent, UnixUserAuthEvent, UnixUserTokenEvent, + VerifyTOTPEvent, WebauthnDoRegisterEvent, WebauthnInitRegisterEvent, }; use crate::idm::mfareg::{MfaRegCred, MfaRegNext, MfaRegSession}; use crate::idm::radius::RadiusAccount; @@ -1211,6 +1211,43 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { Ok(next) } + pub fn remove_account_webauthn( + &mut self, + au: &mut AuditScope, + rwe: &RemoveWebauthnEvent, + ) -> Result { + ltrace!( + au, + "Attempting to remove webauthn {:?} -> {:?}", + rwe.label, + rwe.target + ); + + let account = self.target_to_account(au, &rwe.target)?; + let modlist = account + .gen_webauthn_remove_mod(rwe.label.as_str()) + .map_err(|e| { + ladmin_error!(au, "Failed to gen webauthn remove mod {:?}", e); + e + })?; + // Perform the mod + self.qs_write + .impersonate_modify( + au, + // Filter as executed + &filter!(f_eq("uuid", PartialValue::new_uuidr(&account.uuid))), + // Filter as intended (acp) + &filter_all!(f_eq("uuid", PartialValue::new_uuidr(&account.uuid))), + &modlist, + &rwe.event, + ) + .map_err(|e| { + ladmin_error!(au, "remove_account_webauthn {:?}", e); + e + }) + .map(|_| SetCredentialResponse::Success) + } + pub fn generate_account_totp( &mut self, au: &mut AuditScope, @@ -1460,8 +1497,9 @@ mod tests { use crate::idm::delayed::{DelayedAction, WebauthnCounterIncrement}; use crate::idm::event::{ GenerateTOTPEvent, PasswordChangeEvent, RadiusAuthTokenEvent, RegenerateRadiusSecretEvent, - RemoveTOTPEvent, UnixGroupTokenEvent, UnixPasswordChangeEvent, UnixUserAuthEvent, - UnixUserTokenEvent, VerifyTOTPEvent, WebauthnDoRegisterEvent, WebauthnInitRegisterEvent, + RemoveTOTPEvent, RemoveWebauthnEvent, UnixGroupTokenEvent, UnixPasswordChangeEvent, + UnixUserAuthEvent, UnixUserTokenEvent, VerifyTOTPEvent, WebauthnDoRegisterEvent, + WebauthnInitRegisterEvent, }; use crate::idm::AuthState; use crate::modify::{Modify, ModifyList}; @@ -3027,6 +3065,30 @@ mod tests { }); let r = task::block_on(idms.delayed_action(au, duration_from_epoch_now(), da)); assert!(Ok(true) == r); + + // Check we can remove the webauthn device - provided we set a pw. + let mut idms_prox_write = idms.proxy_write(ct.clone()); + let rwe = + RemoveWebauthnEvent::new_internal(UUID_ADMIN.clone(), "softtoken".to_string()); + // This fails because the acc is webauthn only. + match idms_prox_write.remove_account_webauthn(au, &rwe) { + Err(OperationError::InvalidAttribute(_)) => { + //ok + } + _ => assert!(false), + }; + // Reg a pw. + let pce = PasswordChangeEvent::new_internal(&UUID_ADMIN, TEST_PASSWORD, None); + assert!(idms_prox_write.set_account_password(au, &pce).is_ok()); + // Now remove, it will work. + idms_prox_write + .remove_account_webauthn(au, &rwe) + .expect("Failed to remove webauthn"); + + assert!(idms_prox_write.commit(au).is_ok()); + + check_admin_password(idms, au, TEST_PASSWORD); + // All done! }) } }