diff --git a/src/ctap/credential_management.rs b/src/ctap/credential_management.rs index dba7d36..2dbe961 100644 --- a/src/ctap/credential_management.rs +++ b/src/ctap/credential_management.rs @@ -22,11 +22,7 @@ use super::pin_protocol_v1::{PinPermission, PinProtocolV1}; use super::response::{AuthenticatorCredentialManagementResponse, ResponseData}; use super::status_code::Ctap2StatusCode; use super::storage::PersistentStore; -use super::timed_permission::TimedPermission; -use super::{ - check_command_permission, check_pin_uv_auth_protocol, StatefulCommand, - STATEFUL_COMMAND_TIMEOUT_DURATION, -}; +use super::{check_pin_uv_auth_protocol, StatefulCommand, StatefulPermission}; use alloc::collections::BTreeSet; use alloc::string::String; use alloc::vec; @@ -125,8 +121,7 @@ fn process_get_creds_metadata( /// Processes the subcommand enumerateRPsBegin for CredentialManagement. fn process_enumerate_rps_begin( persistent_store: &PersistentStore, - stateful_command_permission: &mut TimedPermission, - stateful_command_type: &mut Option, + stateful_command_permission: &mut StatefulPermission, now: ClockValue, ) -> Result { let rp_set = get_stored_rp_ids(persistent_store)?; @@ -134,9 +129,7 @@ fn process_enumerate_rps_begin( // TODO(kaczmarczyck) should we return CTAP2_ERR_NO_CREDENTIALS if empty? if total_rps > 1 { - *stateful_command_permission = - TimedPermission::granted(now, STATEFUL_COMMAND_TIMEOUT_DURATION); - *stateful_command_type = Some(StatefulCommand::EnumerateRps(1)); + stateful_command_permission.set_command(now, StatefulCommand::EnumerateRps(1)); } // TODO https://github.com/rust-lang/rust/issues/62924 replace with pop_first() enumerate_rps_response(rp_set.into_iter().next(), Some(total_rps as u64)) @@ -145,19 +138,16 @@ fn process_enumerate_rps_begin( /// Processes the subcommand enumerateRPsGetNextRP for CredentialManagement. fn process_enumerate_rps_get_next_rp( persistent_store: &PersistentStore, - stateful_command_permission: &mut TimedPermission, - stateful_command_type: &mut Option, - now: ClockValue, + stateful_command_permission: &mut StatefulPermission, ) -> Result { - check_command_permission(stateful_command_permission, now)?; - if let Some(StatefulCommand::EnumerateRps(rp_id_index)) = stateful_command_type { + if let StatefulCommand::EnumerateRps(rp_id_index) = stateful_command_permission.get_command()? { let rp_set = get_stored_rp_ids(persistent_store)?; // A BTreeSet is already sorted. let rp_id = rp_set .into_iter() .nth(*rp_id_index) .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; - *stateful_command_type = Some(StatefulCommand::EnumerateRps(*rp_id_index + 1)); + *rp_id_index += 1; enumerate_rps_response(Some(rp_id), None) } else { Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) @@ -167,8 +157,7 @@ fn process_enumerate_rps_get_next_rp( /// Processes the subcommand enumerateCredentialsBegin for CredentialManagement. fn process_enumerate_credentials_begin( persistent_store: &PersistentStore, - stateful_command_permission: &mut TimedPermission, - stateful_command_type: &mut Option, + stateful_command_permission: &mut StatefulPermission, sub_command_params: CredentialManagementSubCommandParameters, now: ClockValue, ) -> Result { @@ -194,9 +183,8 @@ fn process_enumerate_credentials_begin( .ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)?; let credential = persistent_store.get_credential(current_key)?; if total_credentials > 1 { - *stateful_command_permission = - TimedPermission::granted(now, STATEFUL_COMMAND_TIMEOUT_DURATION); - *stateful_command_type = Some(StatefulCommand::EnumerateCredentials(rp_credentials)); + stateful_command_permission + .set_command(now, StatefulCommand::EnumerateCredentials(rp_credentials)); } enumerate_credentials_response(credential, Some(total_credentials as u64)) } @@ -204,12 +192,10 @@ fn process_enumerate_credentials_begin( /// Processes the subcommand enumerateCredentialsGetNextCredential for CredentialManagement. fn process_enumerate_credentials_get_next_credential( persistent_store: &PersistentStore, - stateful_command_permission: &mut TimedPermission, - mut stateful_command_type: &mut Option, - now: ClockValue, + stateful_command_permission: &mut StatefulPermission, ) -> Result { - check_command_permission(stateful_command_permission, now)?; - if let Some(StatefulCommand::EnumerateCredentials(rp_credentials)) = &mut stateful_command_type + if let StatefulCommand::EnumerateCredentials(rp_credentials) = + stateful_command_permission.get_command()? { let current_key = rp_credentials .pop() @@ -251,8 +237,7 @@ fn process_update_user_information( /// Processes the CredentialManagement command and all its subcommands. pub fn process_credential_management( persistent_store: &mut PersistentStore, - stateful_command_permission: &mut TimedPermission, - mut stateful_command_type: &mut Option, + stateful_command_permission: &mut StatefulPermission, pin_protocol_v1: &mut PinProtocolV1, cred_management_params: AuthenticatorCredentialManagementParameters, now: ClockValue, @@ -264,17 +249,17 @@ pub fn process_credential_management( pin_auth, } = cred_management_params; - match (sub_command, &mut stateful_command_type) { + match (sub_command, stateful_command_permission.get_command()) { ( CredentialManagementSubCommand::EnumerateRpsGetNextRp, - Some(StatefulCommand::EnumerateRps(_)), - ) => (), - ( + Ok(StatefulCommand::EnumerateRps(_)), + ) + | ( CredentialManagementSubCommand::EnumerateCredentialsGetNextCredential, - Some(StatefulCommand::EnumerateCredentials(_)), - ) => (), + Ok(StatefulCommand::EnumerateCredentials(_)), + ) => stateful_command_permission.check_command_permission(now)?, (_, _) => { - *stateful_command_type = None; + stateful_command_permission.clear(); } } @@ -313,22 +298,15 @@ pub fn process_credential_management( CredentialManagementSubCommand::EnumerateRpsBegin => Some(process_enumerate_rps_begin( persistent_store, stateful_command_permission, - stateful_command_type, now, )?), - CredentialManagementSubCommand::EnumerateRpsGetNextRp => { - Some(process_enumerate_rps_get_next_rp( - persistent_store, - stateful_command_permission, - stateful_command_type, - now, - )?) - } + CredentialManagementSubCommand::EnumerateRpsGetNextRp => Some( + process_enumerate_rps_get_next_rp(persistent_store, stateful_command_permission)?, + ), CredentialManagementSubCommand::EnumerateCredentialsBegin => { Some(process_enumerate_credentials_begin( persistent_store, stateful_command_permission, - stateful_command_type, sub_command_params.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, now, )?) @@ -337,8 +315,6 @@ pub fn process_credential_management( Some(process_enumerate_credentials_get_next_credential( persistent_store, stateful_command_permission, - stateful_command_type, - now, )?) } CredentialManagementSubCommand::DeleteCredential => { @@ -412,7 +388,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -441,7 +416,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -496,7 +470,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -521,7 +494,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -547,7 +519,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -600,7 +571,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -631,7 +601,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -690,7 +659,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -714,7 +682,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -739,7 +706,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -793,7 +759,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -812,7 +777,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -872,7 +836,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -922,7 +885,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, @@ -950,7 +912,6 @@ mod test { let cred_management_response = process_credential_management( &mut ctap_state.persistent_store, &mut ctap_state.stateful_command_permission, - &mut ctap_state.stateful_command_type, &mut ctap_state.pin_protocol_v1, cred_management_params, DUMMY_CLOCK_VALUE, diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 49d14f4..46281fa 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -101,8 +101,9 @@ const ED_FLAG: u8 = 0x80; pub const TOUCH_TIMEOUT_MS: isize = 30000; #[cfg(feature = "with_ctap1")] const U2F_UP_PROMPT_TIMEOUT: Duration = Duration::from_ms(10000); +// TODO(kaczmarczyck) 2.1 allows Reset after Reset and 15 seconds? const RESET_TIMEOUT_DURATION: Duration = Duration::from_ms(10000); -pub const STATEFUL_COMMAND_TIMEOUT_DURATION: Duration = Duration::from_ms(30000); +const STATEFUL_COMMAND_TIMEOUT_DURATION: Duration = Duration::from_ms(30000); pub const FIDO2_VERSION_STRING: &str = "FIDO_2_0"; #[cfg(feature = "with_ctap1")] @@ -169,15 +170,50 @@ pub enum StatefulCommand { EnumerateCredentials(Vec), } -pub fn check_command_permission( - stateful_command_permission: &mut TimedPermission, - now: ClockValue, -) -> Result<(), Ctap2StatusCode> { - *stateful_command_permission = stateful_command_permission.check_expiration(now); - if stateful_command_permission.is_granted(now) { - Ok(()) - } else { - Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) +pub struct StatefulPermission { + permission: TimedPermission, + command_type: Option, +} + +impl StatefulPermission { + // Resets are only possible in the first 10 seconds after booting. + // Therefore, initialization includes allowing Reset. + pub fn new_reset(now: ClockValue) -> StatefulPermission { + StatefulPermission { + permission: TimedPermission::granted(now, RESET_TIMEOUT_DURATION), + command_type: Some(StatefulCommand::Reset), + } + } + + pub fn clear(&mut self) { + self.permission = TimedPermission::waiting(); + self.command_type = None; + } + + pub fn check_command_permission(&mut self, now: ClockValue) -> Result<(), Ctap2StatusCode> { + if self.permission.is_granted(now) { + Ok(()) + } else { + self.clear(); + Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) + } + } + + pub fn get_command(&mut self) -> Result<&mut StatefulCommand, Ctap2StatusCode> { + self.command_type + .as_mut() + .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) + } + + pub fn set_command(&mut self, now: ClockValue, new_command_type: StatefulCommand) { + match &new_command_type { + // Reset is only allowed after a power cycle. + StatefulCommand::Reset => unreachable!(), + _ => { + self.permission = TimedPermission::granted(now, STATEFUL_COMMAND_TIMEOUT_DURATION); + self.command_type = Some(new_command_type); + } + } } } @@ -194,8 +230,7 @@ pub struct CtapState<'a, R: Rng256, CheckUserPresence: Fn(ChannelID) -> Result<( #[cfg(feature = "with_ctap1")] pub u2f_up_state: U2fUserPresenceState, // The state initializes to Reset and its timeout, and never goes back to Reset. - stateful_command_permission: TimedPermission, - stateful_command_type: Option, + stateful_command_permission: StatefulPermission, } impl<'a, R, CheckUserPresence> CtapState<'a, R, CheckUserPresence> @@ -220,13 +255,15 @@ where U2F_UP_PROMPT_TIMEOUT, Duration::from_ms(TOUCH_TIMEOUT_MS), ), - stateful_command_permission: TimedPermission::granted(now, RESET_TIMEOUT_DURATION), - stateful_command_type: Some(StatefulCommand::Reset), + stateful_command_permission: StatefulPermission::new_reset(now), } } pub fn update_command_permission(&mut self, now: ClockValue) { - self.stateful_command_permission = self.stateful_command_permission.check_expiration(now); + // Ignore the result, just update. + let _ = self + .stateful_command_permission + .check_command_permission(now); } pub fn increment_global_signature_counter(&mut self) -> Result<(), Ctap2StatusCode> { @@ -345,27 +382,23 @@ where Duration::from_ms(TOUCH_TIMEOUT_MS), ); } - match (&command, &self.stateful_command_type) { - ( - Command::AuthenticatorGetNextAssertion, - Some(StatefulCommand::GetAssertion(_)), - ) => (), - ( + match (&command, self.stateful_command_permission.get_command()) { + (Command::AuthenticatorGetNextAssertion, Ok(StatefulCommand::GetAssertion(_))) + | (Command::AuthenticatorReset, Ok(StatefulCommand::Reset)) + // AuthenticatorGetInfo still allows Reset. + | (Command::AuthenticatorGetInfo, Ok(StatefulCommand::Reset)) + // AuthenticatorSelection still allows Reset. + | (Command::AuthenticatorSelection, Ok(StatefulCommand::Reset)) + // AuthenticatorCredentialManagement handles its subcommands later. + | ( Command::AuthenticatorCredentialManagement(_), - Some(StatefulCommand::EnumerateRps(_)), - ) => (), - ( + Ok(StatefulCommand::EnumerateRps(_)), + ) + | ( Command::AuthenticatorCredentialManagement(_), - Some(StatefulCommand::EnumerateCredentials(_)), + Ok(StatefulCommand::EnumerateCredentials(_)), ) => (), - (Command::AuthenticatorReset, Some(StatefulCommand::Reset)) => (), - // GetInfo does not reset stateful commands. - (Command::AuthenticatorGetInfo, _) => (), - // AuthenticatorSelection does not reset stateful commands. - (Command::AuthenticatorSelection, _) => (), - (_, _) => { - self.stateful_command_type = None; - } + (_, _) => self.stateful_command_permission.clear(), } let response = match command { Command::AuthenticatorMakeCredential(params) => { @@ -382,7 +415,6 @@ where process_credential_management( &mut self.persistent_store, &mut self.stateful_command_permission, - &mut self.stateful_command_type, &mut self.pin_protocol_v1, params, now, @@ -855,12 +887,12 @@ where None } else { let number_of_credentials = Some(next_credential_keys.len() + 1); - self.stateful_command_permission = - TimedPermission::granted(now, STATEFUL_COMMAND_TIMEOUT_DURATION); - self.stateful_command_type = Some(StatefulCommand::GetAssertion(AssertionState { + let assertion_state = StatefulCommand::GetAssertion(AssertionState { assertion_input: assertion_input.clone(), next_credential_keys, - })); + }); + self.stateful_command_permission + .set_command(now, assertion_state); number_of_credentials }; self.assertion_response(credential, assertion_input, number_of_credentials) @@ -870,20 +902,20 @@ where &mut self, now: ClockValue, ) -> Result { - check_command_permission(&mut self.stateful_command_permission, now)?; - let (assertion_input, credential) = - if let Some(StatefulCommand::GetAssertion(assertion_state)) = - &mut self.stateful_command_type - { - let credential_key = assertion_state - .next_credential_keys - .pop() - .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; - let credential = self.persistent_store.get_credential(credential_key)?; - (assertion_state.assertion_input.clone(), credential) - } else { - return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED); - }; + self.stateful_command_permission + .check_command_permission(now)?; + let (assertion_input, credential) = if let StatefulCommand::GetAssertion(assertion_state) = + self.stateful_command_permission.get_command()? + { + let credential_key = assertion_state + .next_credential_keys + .pop() + .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; + let credential = self.persistent_store.get_credential(credential_key)?; + (assertion_state.assertion_input.clone(), credential) + } else { + return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED); + }; self.assertion_response(credential, assertion_input, None) } @@ -949,11 +981,10 @@ where cid: ChannelID, now: ClockValue, ) -> Result { - // Resets are only possible in the first 10 seconds after booting. - // TODO(kaczmarczyck) 2.1 allows Reset after Reset and 15 seconds? - check_command_permission(&mut self.stateful_command_permission, now)?; - match &self.stateful_command_type { - Some(StatefulCommand::Reset) => (), + self.stateful_command_permission + .check_command_permission(now)?; + match self.stateful_command_permission.get_command()? { + StatefulCommand::Reset => (), _ => return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED), } (self.check_user_presence)(cid)?;