Skip to content

Commit

Permalink
Make maximum resident credential count configurable
Browse files Browse the repository at this point in the history
Previously, we estimated that we can handle 100 resident keys when
returning the number of remaining resident keys in the credential
management command.

This patch introduces a config option to set a maximum count of resident
keys that is used to report the number of remaining resident keys and
that is enforced when trying to create a new resident key.
  • Loading branch information
robin-nitrokey committed Apr 20, 2023
1 parent d3e1753 commit 04ae7a9
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 13 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased
- Add config option for setting a maximum number of resident credentials.

## [0.1.1] - 2022-08-22
- Fix bug that treated U2F payloads as APDU over APDU in NFC transport @conorpp
- Add config option to skip UP when device was just booted,
as insertion is a kind of UP check @robin-nitrokey

## [Unreleased]
## [0.1.0] - 2022-03-17

- use 2021 edition
- use @szszszsz's credential ID shortening
Expand Down
2 changes: 2 additions & 0 deletions src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ pub const U2F_UP_TIMEOUT: u32 = 250;

pub const ATTESTATION_CERT_ID: CertId = CertId::from_special(0);
pub const ATTESTATION_KEY_ID: KeyId = KeyId::from_special(0);

pub const MAX_RESIDENT_CREDENTIALS_GUESSTIMATE: u32 = 100;
10 changes: 10 additions & 0 deletions src/ctap2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,16 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
self.delete_resident_key_by_user_id(&rp_id_hash, &credential.user.id)
.ok();

// 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);
if count >= max_count {
return Err(Error::KeyStoreFull);
}
}

// then store key, making it resident
let credential_id_hash = self.hash(credential_id.0.as_ref());
try_syscall!(self.trussed.write_file(
Expand Down
14 changes: 7 additions & 7 deletions src/ctap2/credential_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use ctap_types::{
use littlefs2::path::{Path, PathBuf};

use crate::{
constants::MAX_RESIDENT_CREDENTIALS_GUESSTIMATE,
credential::Credential,
state::{CredentialManagementEnumerateCredentials, CredentialManagementEnumerateRps},
Authenticator, Result, TrussedRequirements, UserPresence,
Expand Down Expand Up @@ -67,9 +68,12 @@ where
info!("get metadata");
let mut response: Response = Default::default();

let guesstimate = self.state.persistent.max_resident_credentials_guesstimate();
let max_resident_credentials = self
.config
.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(guesstimate);
response.max_possible_remaining_residential_credentials_count = Some(max_resident_credentials);

let dir = PathBuf::from(b"rk");
let maybe_first_rp =
Expand Down Expand Up @@ -98,11 +102,7 @@ where
None => {
response.existing_resident_credentials_count = Some(num_rks);
response.max_possible_remaining_residential_credentials_count =
Some(if num_rks >= guesstimate {
0
} else {
guesstimate - num_rks
});
Some(max_resident_credentials.saturating_sub(num_rks));
return Ok(response);
}
Some(rp) => {
Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ pub struct Config {
/// If set, the first Get Assertion or Authenticate request within the specified time after
/// boot is accepted without additional user presence verification.
pub skip_up_timeout: Option<Duration>,
/// The maximum number of resident credentials.
pub max_resident_credential_count: Option<u32>,
}

// impl Default for Config {
Expand Down
5 changes: 0 additions & 5 deletions src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,6 @@ pub struct PersistentState {
impl PersistentState {
const RESET_RETRIES: u8 = 8;
const FILENAME: &'static [u8] = b"persistent-state.cbor";
const MAX_RESIDENT_CREDENTIALS_GUESSTIMATE: u32 = 100;

pub fn max_resident_credentials_guesstimate(&self) -> u32 {
Self::MAX_RESIDENT_CREDENTIALS_GUESSTIMATE
}

pub fn load<T: client::Client + client::Chacha8Poly1305>(trussed: &mut T) -> Result<Self> {
// TODO: add "exists_file" method instead?
Expand Down

0 comments on commit 04ae7a9

Please sign in to comment.