Fix totp registration workflow with broken authenticators (#516)

This commit is contained in:
Firstyear 2021-07-03 14:39:22 +10:00 committed by GitHub
parent 040e9fd352
commit e134fa5b40
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 339 additions and 45 deletions

View file

@ -982,6 +982,27 @@ impl KanidmAsyncClient {
match res {
Ok(SetCredentialResponse::Success) => Ok(()),
Ok(SetCredentialResponse::TotpCheck(u, s)) => Err(ClientError::TotpVerifyFailed(u, s)),
Ok(SetCredentialResponse::TotpInvalidSha1(u)) => Err(ClientError::TotpInvalidSha1(u)),
Ok(_) => Err(ClientError::EmptyResponse),
Err(e) => Err(e),
}
}
// Accept a sha1 totp
pub async fn idm_account_primary_credential_accept_sha1_totp(
&self,
id: &str,
session: Uuid,
) -> Result<(), ClientError> {
let r = SetCredentialRequest::TotpAcceptSha1(session);
let res: Result<SetCredentialResponse, ClientError> = self
.perform_put_request(
format!("/v1/account/{}/_credential/primary", id).as_str(),
r,
)
.await;
match res {
Ok(SetCredentialResponse::Success) => Ok(()),
Ok(_) => Err(ClientError::EmptyResponse),
Err(e) => Err(e),
}

View file

@ -53,6 +53,7 @@ pub enum ClientError {
AuthenticationFailed,
EmptyResponse,
TotpVerifyFailed(Uuid, TotpSecret),
TotpInvalidSha1(Uuid),
JsonDecode(reqwest::Error, String),
JsonEncode(SerdeJsonError),
SystemError,
@ -651,6 +652,17 @@ impl KanidmClient {
)
}
pub fn idm_account_primary_credential_accept_sha1_totp(
&self,
id: &str,
session: Uuid,
) -> Result<(), ClientError> {
tokio_block_on(
self.asclient
.idm_account_primary_credential_accept_sha1_totp(id, session),
)
}
pub fn idm_account_primary_credential_remove_totp(&self, id: &str) -> Result<(), ClientError> {
tokio_block_on(self.asclient.idm_account_primary_credential_remove_totp(id))
}

View file

@ -673,6 +673,7 @@ pub enum SetCredentialRequest {
GeneratePassword,
TotpGenerate,
TotpVerify(Uuid, u32),
TotpAcceptSha1(Uuid),
TotpRemove,
// Start the rego.
WebauthnBegin(String),
@ -747,6 +748,7 @@ pub enum SetCredentialResponse {
Success,
Token(String),
TotpCheck(Uuid, TotpSecret),
TotpInvalidSha1(Uuid),
WebauthnCreateChallenge(Uuid, CreationChallengeResponse),
BackupCodes(Vec<String>),
}

View file

@ -9,6 +9,7 @@ use time::OffsetDateTime;
use webauthn_authenticator_rs::{u2fhid::U2FHid, WebauthnAuthenticator};
use kanidm_client::ClientError;
use kanidm_client::ClientError::Http as ClientErrorHttp;
use kanidm_proto::v1::OperationError::{PasswordBadListed, PasswordTooShort, PasswordTooWeak};
@ -189,37 +190,83 @@ impl AccountOpt {
// prompt for the totp.
eprintln!("--------------------------------------------------------------");
eprint!(
"Enter a TOTP from your authenticator to complete registration: \nTOTP: "
);
eprintln!("Enter a TOTP from your authenticator to complete registration:");
let mut totp_input = String::new();
if let Err(e) = io::stdin().read_line(&mut totp_input) {
eprintln!("Failed to read from stdin -> {:?}", e);
return;
};
let mut attempts = 3;
while attempts > 0 {
eprint!("TOTP: ");
let mut totp_input = String::new();
let input_result = io::stdin().read_line(&mut totp_input);
// Finish the line?
eprintln!("");
if let Err(e) = input_result {
eprintln!("Failed to read from stdin -> {:?}", e);
break;
};
// Convert to a u32.
let totp = match u32::from_str_radix(totp_input.trim(), 10) {
Ok(v) => v,
Err(e) => {
eprintln!("Invalid TOTP -> {:?}", e);
return;
}
};
// Convert to a u32.
let totp = match u32::from_str_radix(totp_input.trim(), 10) {
Ok(v) => v,
Err(e) => {
eprintln!("Invalid TOTP -> {:?}", e);
// Try again.
continue;
}
};
match client.idm_account_primary_credential_verify_totp(
acsopt.aopts.account_id.as_str(),
totp,
session,
) {
Ok(_) => {
println!("TOTP registration success.");
match client.idm_account_primary_credential_verify_totp(
acsopt.aopts.account_id.as_str(),
totp,
session,
) {
Ok(_) => {
println!("TOTP registration success.");
break;
}
Err(ClientError::TotpInvalidSha1(session)) => {
eprintln!("⚠️ WARNING - It appears your authenticator app may be broken ⚠️ ");
eprintln!(" The TOTP authenticator you are using is forcing the use of SHA1");
eprintln!("");
eprintln!(" -- If you accept this risk, and wish to proceed, type 'I am sure' ");
eprintln!(" -- Otherwise press ENTER to cancel this operation");
eprintln!("");
eprint!("Are you sure: ");
let mut confirm_input = String::new();
if let Err(e) = io::stdin().read_line(&mut confirm_input) {
eprintln!("Failed to read from stdin -> {:?}", e);
break;
};
if confirm_input.to_lowercase().trim() == "i am sure" {
match client.idm_account_primary_credential_accept_sha1_totp(
acsopt.aopts.account_id.as_str(),
session,
) {
Ok(_) => {
println!("TOTP registration success.");
}
Err(e) => {
eprintln!("Error Completing -> {:?}", e);
}
};
break;
} else {
eprintln!("Cancelling TOTP registration");
break;
}
}
Err(ClientError::TotpVerifyFailed(_, _)) => {
eprintln!("Incorrect TOTP code - try again");
attempts -= 1;
continue;
}
Err(e) => {
eprintln!("Error Completing -> {:?}", e);
break;
}
}
Err(e) => {
eprintln!("Error Completing -> {:?}", e);
}
}
} // end loop
}
AccountCredential::RemoveTotp(acsopt) => {
let client = acsopt.copt.to_client();

View file

@ -11,9 +11,9 @@ use crate::event::{
ReviveRecycledEvent,
};
use crate::idm::event::{
GeneratePasswordEvent, GenerateTotpEvent, PasswordChangeEvent, RegenerateRadiusSecretEvent,
RemoveTotpEvent, RemoveWebauthnEvent, UnixPasswordChangeEvent, VerifyTotpEvent,
WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
AcceptSha1TotpEvent, GeneratePasswordEvent, GenerateTotpEvent, PasswordChangeEvent,
RegenerateRadiusSecretEvent, RemoveTotpEvent, RemoveWebauthnEvent, UnixPasswordChangeEvent,
VerifyTotpEvent, WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
};
use crate::modify::{Modify, ModifyInvalid, ModifyList};
use crate::value::{PartialValue, Value};
@ -596,6 +596,21 @@ impl QueryServerWriteV1 {
.verify_account_totp(&mut audit, &vte, ct)
.and_then(|r| idms_prox_write.commit(&mut audit).map(|_| r))
}
SetCredentialRequest::TotpAcceptSha1(uuid) => {
let aste =
AcceptSha1TotpEvent::from_parts(&mut audit, ident, target_uuid, uuid)
.map_err(|e| {
ladmin_error!(
audit,
"Failed to begin internal_credential_set_message: {:?}",
e
);
e
})?;
idms_prox_write
.accept_account_sha1_totp(&mut audit, &aste)
.and_then(|r| idms_prox_write.commit(&mut audit).map(|_| r))
}
SetCredentialRequest::TotpRemove => {
let rte = RemoveTotpEvent::from_parts(
&mut audit,

View file

@ -182,6 +182,18 @@ impl Totp {
},
}
}
pub fn is_legacy_algo(&self) -> bool {
matches!(&self.algo, TotpAlgo::Sha1)
}
pub fn downgrade_to_legacy(self) -> Self {
Totp {
secret: self.secret,
step: self.step,
algo: TotpAlgo::Sha1,
}
}
}
#[cfg(test)]

View file

@ -342,6 +342,40 @@ impl VerifyTotpEvent {
}
}
#[derive(Debug)]
pub struct AcceptSha1TotpEvent {
pub ident: Identity,
pub target: Uuid,
pub session: Uuid,
}
impl AcceptSha1TotpEvent {
pub fn from_parts(
_audit: &mut AuditScope,
// qs: &QueryServerWriteTransaction,
ident: Identity,
target: Uuid,
session: Uuid,
) -> Result<Self, OperationError> {
Ok(AcceptSha1TotpEvent {
ident,
target,
session,
})
}
#[cfg(test)]
pub fn new_internal(target: Uuid, session: Uuid) -> Self {
let ident = Identity::from_internal();
AcceptSha1TotpEvent {
ident,
target,
session,
}
}
}
#[derive(Debug)]
pub struct RemoveTotpEvent {
pub ident: Identity,

View file

@ -22,6 +22,7 @@ pub(crate) enum MfaRegCred {
pub(crate) enum MfaRegNext {
Success,
TotpCheck(TotpSecret),
TotpInvalidSha1,
WebauthnChallenge(CreationChallengeResponse),
}
@ -31,6 +32,7 @@ impl MfaRegNext {
match self {
MfaRegNext::Success => SetCredentialResponse::Success,
MfaRegNext::TotpCheck(secret) => SetCredentialResponse::TotpCheck(u, secret),
MfaRegNext::TotpInvalidSha1 => SetCredentialResponse::TotpInvalidSha1(u),
MfaRegNext::WebauthnChallenge(ccr) => {
SetCredentialResponse::WebauthnCreateChallenge(u, ccr)
}
@ -41,6 +43,7 @@ impl MfaRegNext {
#[derive(Clone)]
enum MfaRegState {
TotpInit(Totp),
TotpInvalidSha1(Totp),
TotpDone,
WebauthnInit(String, WebauthnRegistrationState),
WebauthnDone,
@ -103,13 +106,54 @@ impl MfaRegSession {
_ => Err(OperationError::InvalidState),
}
} else {
// Let them try again?
let accountname = self.account.name.as_str();
let issuer = self.account.spn.as_str();
Ok((
MfaRegNext::TotpCheck(token.to_proto(accountname, issuer)),
None,
))
// What if it's a broken authenticator app? Google authenticator
// and authy both force sha1 and ignore the algo we send. So lets
// check that just in case.
let token_sha1 = token.clone().downgrade_to_legacy();
if token_sha1.verify(chal, ct) {
// Greeeaaaaaatttt it's a broken app. Let's check the user
// knows this is broken, before we proceed.
let mut nstate = MfaRegState::TotpInvalidSha1(token_sha1);
mem::swap(&mut self.state, &mut nstate);
Ok((MfaRegNext::TotpInvalidSha1, None))
} else {
// Prooobbably a bad code or typo then. Lte them try again.
let accountname = self.account.name.as_str();
let issuer = self.account.spn.as_str();
Ok((
MfaRegNext::TotpCheck(token.to_proto(accountname, issuer)),
None,
))
}
}
}
_ => Err(OperationError::InvalidRequestState),
}
}
pub fn totp_accept_sha1(
&mut self,
origin: &IdentityId,
target: &Uuid,
) -> Result<(MfaRegNext, Option<MfaRegCred>), OperationError> {
if &self.origin != origin || target != &self.account.uuid {
// Verify that the same event source is the one continuing this attempt
return Err(OperationError::InvalidRequestState);
};
match &self.state {
MfaRegState::TotpInvalidSha1(_token) => {
// They have accepted the token to be sha1, so lets yield that.
// The token was mutated to sha1 in the previous step.
let mut nstate = MfaRegState::TotpDone;
mem::swap(&mut self.state, &mut nstate);
match nstate {
MfaRegState::TotpInvalidSha1(token) => {
Ok((MfaRegNext::Success, Some(MfaRegCred::Totp(token))))
}
_ => Err(OperationError::InvalidState),
}
}
_ => Err(OperationError::InvalidRequestState),

View file

@ -7,10 +7,11 @@ use crate::identity::{IdentType, IdentUser, Limits};
use crate::idm::account::Account;
use crate::idm::authsession::AuthSession;
use crate::idm::event::{
CredentialStatusEvent, GeneratePasswordEvent, GenerateTotpEvent, LdapAuthEvent,
PasswordChangeEvent, RadiusAuthTokenEvent, RegenerateRadiusSecretEvent, RemoveTotpEvent,
RemoveWebauthnEvent, UnixGroupTokenEvent, UnixPasswordChangeEvent, UnixUserAuthEvent,
UnixUserTokenEvent, VerifyTotpEvent, WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
AcceptSha1TotpEvent, CredentialStatusEvent, GeneratePasswordEvent, GenerateTotpEvent,
LdapAuthEvent, PasswordChangeEvent, RadiusAuthTokenEvent, RegenerateRadiusSecretEvent,
RemoveTotpEvent, RemoveWebauthnEvent, UnixGroupTokenEvent, UnixPasswordChangeEvent,
UnixUserAuthEvent, UnixUserTokenEvent, VerifyTotpEvent, WebauthnDoRegisterEvent,
WebauthnInitRegisterEvent,
};
use crate::idm::mfareg::{MfaRegCred, MfaRegNext, MfaRegSession};
use crate::idm::oauth2::{
@ -1741,6 +1742,62 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
Ok(next)
}
pub fn accept_account_sha1_totp(
&mut self,
au: &mut AuditScope,
aste: &AcceptSha1TotpEvent,
) -> Result<SetCredentialResponse, OperationError> {
let sessionid = aste.session;
let origin = (&aste.ident.origin).into();
ltrace!(au, "Attempting to find mfareg_session -> {:?}", sessionid);
let (next, opt_cred) = self
.mfareg_sessions
.get_mut(&sessionid)
.ok_or(OperationError::InvalidRequestState)
.and_then(|session| session.totp_accept_sha1(&origin, &aste.target))
.map_err(|e| {
ladmin_error!(au, "Failed to accept sha1 totp {:?}", e);
e
})?;
if let (MfaRegNext::Success, Some(MfaRegCred::Totp(token))) = (&next, opt_cred) {
// Purge the session.
let session = self
.mfareg_sessions
.remove(&sessionid)
.ok_or(OperationError::InvalidState)
.map_err(|e| {
ladmin_error!(au, "Session within transaction vanished!");
e
})?;
// reg the token
let modlist = session.account.gen_totp_mod(token).map_err(|e| {
ladmin_error!(au, "Failed to gen totp mod {:?}", e);
e
})?;
// Perform the mod
self.qs_write
.impersonate_modify(
au,
// Filter as executed
&filter!(f_eq("uuid", PartialValue::new_uuidr(&session.account.uuid))),
// Filter as intended (acp)
&filter_all!(f_eq("uuid", PartialValue::new_uuidr(&session.account.uuid))),
&modlist,
&aste.ident,
)
.map_err(|e| {
ladmin_error!(au, "accept_account_sha1_totp {:?}", e);
e
})?;
};
let next = next.to_proto(sessionid);
Ok(next)
}
pub fn remove_account_totp(
&mut self,
au: &mut AuditScope,
@ -1950,10 +2007,10 @@ mod tests {
use crate::event::{AuthEvent, AuthResult, CreateEvent, ModifyEvent};
use crate::idm::delayed::{BackupCodeRemoval, DelayedAction, WebauthnCounterIncrement};
use crate::idm::event::{
GenerateBackupCodeEvent, GenerateTotpEvent, PasswordChangeEvent, RadiusAuthTokenEvent,
RegenerateRadiusSecretEvent, RemoveTotpEvent, RemoveWebauthnEvent, UnixGroupTokenEvent,
UnixPasswordChangeEvent, UnixUserAuthEvent, UnixUserTokenEvent, VerifyTotpEvent,
WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
AcceptSha1TotpEvent, GenerateBackupCodeEvent, GenerateTotpEvent, PasswordChangeEvent,
RadiusAuthTokenEvent, RegenerateRadiusSecretEvent, RemoveTotpEvent, RemoveWebauthnEvent,
UnixGroupTokenEvent, UnixPasswordChangeEvent, UnixUserAuthEvent, UnixUserTokenEvent,
VerifyTotpEvent, WebauthnDoRegisterEvent, WebauthnInitRegisterEvent,
};
use crate::idm::AuthState;
use crate::modify::{Modify, ModifyList};
@ -2879,6 +2936,56 @@ mod tests {
})
}
#[test]
fn test_idm_totp_sha1_registration() {
run_idm_test!(|_qs: &QueryServer,
idms: &IdmServer,
_idms_delayed: &IdmServerDelayed,
au: &mut AuditScope| {
let ct = duration_from_epoch_now();
let mut idms_prox_write = idms.proxy_write(ct.clone());
let pce = PasswordChangeEvent::new_internal(&UUID_ADMIN, TEST_PASSWORD);
assert!(idms_prox_write.set_account_password(au, &pce).is_ok());
// Start registering the TOTP
let gte1 = GenerateTotpEvent::new_internal(UUID_ADMIN.clone());
let res = idms_prox_write
.generate_account_totp(au, &gte1, ct.clone())
.unwrap();
let (sesid, tok) = match res {
SetCredentialResponse::TotpCheck(id, tok) => (id, tok),
_ => panic!("invalid state!"),
};
let r_tok: Totp = tok.into();
// Now, assert that the Totp is NOT sha1 (correct default behaviour).
assert!(!r_tok.is_legacy_algo());
// Mutate the tok to a legacy token.
let legacy_tok = r_tok.downgrade_to_legacy();
let chal = legacy_tok
.do_totp_duration_from_epoch(&ct)
.expect("Failed to do totp?");
// attempt the verify
let vte3 = VerifyTotpEvent::new_internal(UUID_ADMIN.clone(), sesid, chal);
match idms_prox_write.verify_account_totp(au, &vte3, ct.clone()) {
Ok(SetCredentialResponse::TotpInvalidSha1(_)) => {}
_ => panic!(),
};
let aste = AcceptSha1TotpEvent::new_internal(UUID_ADMIN.clone(), sesid);
match idms_prox_write.accept_account_sha1_totp(au, &aste) {
Ok(SetCredentialResponse::Success) => {}
_ => panic!(),
};
// Done!
})
}
#[test]
fn test_idm_simple_password_upgrade() {
run_idm_test!(|qs: &QueryServer,