diff --git a/src/ctap2.rs b/src/ctap2.rs index ea1ad40..73129ce 100644 --- a/src/ctap2.rs +++ b/src/ctap2.rs @@ -372,27 +372,44 @@ impl Authenticator for crate::Authenti self.delete_resident_key_by_user_id(&rp_id_hash, &credential.user.id) .ok(); + let mut key_store_full = false; + // then check the maximum number of RK credentials if let Some(max_count) = self.config.max_resident_credential_count { let mut cm = credential_management::CredentialManagement::new(self); - let metadata = cm.get_creds_metadata()?; - let count = metadata.existing_resident_credentials_count.unwrap_or(max_count); + let metadata = cm.get_creds_metadata(); + let count = metadata + .existing_resident_credentials_count + .unwrap_or(max_count); + debug!("resident cred count: {} (max: {})", count, max_count); if count >= max_count { - return Err(Error::KeyStoreFull); + error!("maximum resident credential count reached"); + key_store_full = true; } } - // then store key, making it resident - let credential_id_hash = self.hash(credential_id.0.as_ref()); - try_syscall!(self.trussed.write_file( - Location::Internal, - rk_path(&rp_id_hash, &credential_id_hash), - serialized_credential, - // user attribute for later easy lookup - // Some(rp_id_hash.clone()), - None, - )) - .map_err(|_| Error::KeyStoreFull)?; + if !key_store_full { + // then store key, making it resident + let credential_id_hash = self.hash(credential_id.0.as_ref()); + let result = try_syscall!(self.trussed.write_file( + Location::Internal, + rk_path(&rp_id_hash, &credential_id_hash), + serialized_credential, + // user attribute for later easy lookup + // Some(rp_id_hash.clone()), + None, + )); + key_store_full = result.is_err(); + } + + if key_store_full { + // If we previously deleted an existing cred with the same RP + UserId but then + // failed to store the new cred, the RP directory could now be empty. This is not + // a valid state so we have to delete it. + let rp_dir = rp_rk_dir(&rp_id_hash); + self.delete_rp_dir_if_empty(rp_dir); + return Err(Error::KeyStoreFull); + } } // 13. generate and return attestation statement using clientDataHash @@ -844,7 +861,7 @@ impl Authenticator for crate::Authenti let sub_parameters = ¶meters.sub_command_params; match parameters.sub_command { // 0x1 - Subcommand::GetCredsMetadata => cred_mgmt.get_creds_metadata(), + Subcommand::GetCredsMetadata => Ok(cred_mgmt.get_creds_metadata()), // 0x2 Subcommand::EnumerateRpsBegin => cred_mgmt.first_relying_party(), @@ -1669,6 +1686,25 @@ impl crate::Authenticator { Ok(()) } + + pub(crate) fn delete_rp_dir_if_empty(&mut self, rp_path: PathBuf) { + let maybe_first_remaining_rk = + syscall!(self + .trussed + .read_dir_first(Location::Internal, rp_path.clone(), None,)) + .entry; + + if maybe_first_remaining_rk.is_none() { + info!("deleting parent {:?} as this was its last RK", &rp_path); + syscall!(self.trussed.remove_dir(Location::Internal, rp_path,)); + } else { + info!( + "not deleting deleting parent {:?} as there is {:?}", + &rp_path, + &maybe_first_remaining_rk.unwrap().path(), + ); + } + } } fn rp_rk_dir(rp_id_hash: &Bytes<32>) -> PathBuf { diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index 39eaaf8..4ae63c9 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -62,7 +62,7 @@ where UP: UserPresence, T: TrussedRequirements, { - pub fn get_creds_metadata(&mut self) -> Result { + pub fn get_creds_metadata(&mut self) -> Response { info!("get metadata"); let mut response: Response = Default::default(); @@ -71,7 +71,8 @@ where .max_resident_credential_count .unwrap_or(MAX_RESIDENT_CREDENTIALS_GUESSTIMATE); response.existing_resident_credentials_count = Some(0); - response.max_possible_remaining_residential_credentials_count = Some(max_resident_credentials); + response.max_possible_remaining_residential_credentials_count = + Some(max_resident_credentials); let dir = PathBuf::from(b"rk"); let maybe_first_rp = @@ -81,11 +82,11 @@ where .entry; let first_rp = match maybe_first_rp { - None => return Ok(response), + None => return response, Some(rp) => rp, }; - let (mut num_rks, _) = self.count_rp_rks(PathBuf::from(first_rp.path()))?; + let (mut num_rks, _) = self.count_rp_rks(PathBuf::from(first_rp.path())); let mut last_rp = PathBuf::from(first_rp.file_name()); loop { @@ -101,12 +102,12 @@ where response.existing_resident_credentials_count = Some(num_rks); response.max_possible_remaining_residential_credentials_count = Some(max_resident_credentials.saturating_sub(num_rks)); - return Ok(response); + return response; } Some(rp) => { last_rp = PathBuf::from(rp.file_name()); info!("counting.."); - let (this_rp_rk_count, _) = self.count_rp_rks(PathBuf::from(rp.path()))?; + let (this_rp_rk_count, _) = self.count_rp_rks(PathBuf::from(rp.path())); info!("{:?}", this_rp_rk_count); num_rks += this_rp_rk_count; } @@ -265,21 +266,24 @@ where Ok(response) } - fn count_rp_rks(&mut self, rp_dir: PathBuf) -> Result<(u32, DirEntry)> { + fn count_rp_rks(&mut self, rp_dir: PathBuf) -> (u32, Option) { let maybe_first_rk = syscall!(self .trussed .read_dir_first(Location::Internal, rp_dir, None)) .entry; - let first_rk = maybe_first_rk.ok_or(Error::NoCredentials)?; + let Some(first_rk) = maybe_first_rk else { + warn!("empty RP directory"); + return (0, None); + }; // count the rest of them let mut num_rks = 1; while syscall!(self.trussed.read_dir_next()).entry.is_some() { num_rks += 1; } - Ok((num_rks, first_rk)) + (num_rks, Some(first_rk)) } pub fn first_credential(&mut self, rp_id_hash: &Bytes<32>) -> Result { @@ -291,7 +295,8 @@ where super::format_hex(&rp_id_hash[..8], &mut hex); let rp_dir = PathBuf::from(b"rk").join(&PathBuf::from(&hex)); - let (num_rks, first_rk) = self.count_rp_rks(rp_dir)?; + let (num_rks, first_rk) = self.count_rp_rks(rp_dir); + let first_rk = first_rk.ok_or(Error::NoCredentials)?; // extract data required into response let mut response = self.extract_response_from_credential_file(first_rk.path())?; @@ -479,23 +484,8 @@ where .parent() // by construction, RK has a parent, its RP .unwrap(); + self.delete_rp_dir_if_empty(rp_path); - let maybe_first_remaining_rk = - syscall!(self - .trussed - .read_dir_first(Location::Internal, rp_path.clone(), None,)) - .entry; - - if maybe_first_remaining_rk.is_none() { - info!("deleting parent {:?} as this was its last RK", &rp_path); - syscall!(self.trussed.remove_dir(Location::Internal, rp_path,)); - } else { - info!( - "not deleting deleting parent {:?} as there is {:?}", - &rp_path, - &maybe_first_remaining_rk.unwrap().path(), - ); - } // just return OK let response = Default::default(); Ok(response)