diff --git a/src/ctap/credential_management.rs b/src/ctap/credential_management.rs index f560ee1..a644689 100644 --- a/src/ctap/credential_management.rs +++ b/src/ctap/credential_management.rs @@ -158,7 +158,7 @@ fn process_enumerate_rps_get_next_rp( env: &mut E, stateful_command_permission: &mut StatefulPermission, ) -> Result { - let rp_id_index = stateful_command_permission.next_enumerate_rp()?; + let rp_id_index = stateful_command_permission.next_enumerate_rp(env)?; let rp_set = get_stored_rp_ids(env)?; // A BTreeSet is already sorted. let rp_id = rp_set @@ -213,7 +213,7 @@ fn process_enumerate_credentials_get_next_credential( env: &mut E, stateful_command_permission: &mut StatefulPermission, ) -> Result { - let credential_key = stateful_command_permission.next_enumerate_credential()?; + let credential_key = stateful_command_permission.next_enumerate_credential(env)?; let credential = storage::get_credential(env, credential_key)?; enumerate_credentials_response(env, credential, None) } @@ -264,7 +264,7 @@ pub fn process_credential_management( pin_uv_auth_param, } = cred_management_params; - match (sub_command, stateful_command_permission.get_command()) { + match (sub_command, stateful_command_permission.get_command(env)) { ( CredentialManagementSubCommand::EnumerateRpsGetNextRp, Ok(StatefulCommand::EnumerateRps(_)), diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 7ec0a47..f7b9dbb 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -395,7 +395,7 @@ impl StatefulPermission { // If we wanted to switch to (B) or (C), we'd have to be very careful that the state does // not go stale. For example, we keep credential keys that we expect to still exist. // However, interleaving (stateless) commands could delete credentials or change the PIN, - // which chould make invalidate our access. Some read-only commands should be okay to run, + // which could invalidate our access. Some read-only commands should be okay to run, // but (A) is the safest and easiest solution. if let Some(c) = &self.channel { if c != channel { @@ -405,14 +405,15 @@ impl StatefulPermission { } /// Clears all state if the permission timed out. - pub fn clear_timer(&mut self, env: &mut E) { + fn clear_timer(&mut self, env: &mut E) { if env.clock().is_elapsed(&self.permission) { self.clear(); } } /// Gets a reference to the current command state, if any exists. - pub fn get_command(&self) -> Result<&StatefulCommand, Ctap2StatusCode> { + pub fn get_command(&mut self, env: &mut E) -> Result<&StatefulCommand, Ctap2StatusCode> { + self.clear_timer(env); self.command_type .as_ref() .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) @@ -442,7 +443,9 @@ impl StatefulPermission { /// to the next credential that needs to be processed. pub fn next_assertion_credential( &mut self, + env: &mut E, ) -> Result<(AssertionInput, usize), Ctap2StatusCode> { + self.clear_timer(env); if let Some(StatefulCommand::GetAssertion(assertion_state)) = &mut self.command_type { let credential_key = assertion_state .next_credential_keys @@ -455,7 +458,8 @@ impl StatefulPermission { } /// Returns the index to the next RP ID for enumeration and advances it. - pub fn next_enumerate_rp(&mut self) -> Result { + pub fn next_enumerate_rp(&mut self, env: &mut E) -> Result { + self.clear_timer(env); if let Some(StatefulCommand::EnumerateRps(rp_id_index)) = &mut self.command_type { let current_index = *rp_id_index; *rp_id_index += 1; @@ -466,7 +470,8 @@ impl StatefulPermission { } /// Returns the next storage credential key for enumeration and advances it. - pub fn next_enumerate_credential(&mut self) -> Result { + pub fn next_enumerate_credential(&mut self, env: &mut E) -> Result { + self.clear_timer(env); if let Some(StatefulCommand::EnumerateCredentials(rp_credentials)) = &mut self.command_type { rp_credentials @@ -502,11 +507,6 @@ impl CtapState { } } - pub fn update_timeouts(&mut self, env: &mut E) { - self.stateful_command_permission.clear_timer(env); - self.client_pin.update_timeouts(env); - } - pub fn increment_global_signature_counter( &mut self, env: &mut E, @@ -560,6 +560,9 @@ impl CtapState { command: Command, channel: Channel, ) -> Result { + // The auth token timeouts are checked once here, to make error codes consistent. If your + // auth token hasn't timed out now, you can fully use it for this command. + self.client_pin.update_timeouts(env); // Correct behavior between CTAP1 and CTAP2 isn't defined yet. Just a guess. #[cfg(feature = "with_ctap1")] { @@ -569,8 +572,7 @@ impl CtapState { } self.stateful_command_permission .clear_old_channels(&channel); - self.stateful_command_permission.clear_timer(env); - match (&command, self.stateful_command_permission.get_command()) { + match (&command, self.stateful_command_permission.get_command(env)) { (Command::AuthenticatorGetNextAssertion, Ok(StatefulCommand::GetAssertion(_))) | (Command::AuthenticatorReset, Ok(StatefulCommand::Reset)) // AuthenticatorGetInfo still allows Reset. @@ -1213,7 +1215,7 @@ impl CtapState { fn process_get_next_assertion(&mut self, env: &mut E) -> Result { let (assertion_input, credential_key) = self .stateful_command_permission - .next_assertion_credential()?; + .next_assertion_credential(env)?; let credential = storage::get_credential(env, credential_key)?; self.assertion_response(env, credential, assertion_input, None, true) } @@ -1295,9 +1297,11 @@ impl CtapState { env: &mut E, channel: Channel, ) -> Result { - match self.stateful_command_permission.get_command()? { - StatefulCommand::Reset => (), - _ => return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED), + if !matches!( + self.stateful_command_permission.get_command(env)?, + StatefulCommand::Reset + ) { + return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED); } check_user_presence(env, channel)?; diff --git a/src/lib.rs b/src/lib.rs index a04183e..d0a0636 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -107,10 +107,6 @@ impl Ctap { } } - pub fn update_timeouts(&mut self) { - self.state.update_timeouts(&mut self.env); - } - pub fn should_wink(&mut self) -> bool { self.hid.should_wink(&mut self.env) } diff --git a/src/main.rs b/src/main.rs index 8862ccb..9c40508 100644 --- a/src/main.rs +++ b/src/main.rs @@ -213,9 +213,8 @@ fn main() { drop(buttons_callback); } - // These calls are making sure that even for long inactivity, wrapping clock values + // This call is making sure that even for long inactivity, wrapping clock values // don't cause problems with timers. - ctap.update_timeouts(); ctap.env().clock().tickle(); if let Some(endpoint) = usb_endpoint {