Skip to content

Commit

Permalink
Allow empty RP dirs in get_creds_metadata
Browse files Browse the repository at this point in the history
If we overwrite an existing resident credential, the corresponding RP
directory can be empty when we call
CredentialManagement::get_creds_metadata to count the existing
credentials.  This causes an error in the current implementation.

This patch changes CredentialManagement::get_creds_metadata to
gracefully handle empty RP directories.

Fixes Nitrokey/nitrokey-3-firmware#254
  • Loading branch information
robin-nitrokey committed Apr 28, 2023
1 parent 7f6a093 commit db14bcf
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 13 deletions.
10 changes: 7 additions & 3 deletions src/ctap2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,13 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
// 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 {
error!("maximum resident credential count reached");
return Err(Error::KeyStoreFull);
}
}
Expand Down Expand Up @@ -844,7 +848,7 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
let sub_parameters = &parameters.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(),
Expand Down
25 changes: 15 additions & 10 deletions src/ctap2/credential_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ where
UP: UserPresence,
T: TrussedRequirements,
{
pub fn get_creds_metadata(&mut self) -> Result<Response> {
pub fn get_creds_metadata(&mut self) -> Response {
info!("get metadata");
let mut response: Response = Default::default();

Expand All @@ -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 =
Expand All @@ -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 {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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<DirEntry>) {
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<Response> {
Expand All @@ -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())?;
Expand Down

0 comments on commit db14bcf

Please sign in to comment.