Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dynamic estimation of remaining credential space #92

Merged
merged 7 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ serde_bytes = { version = "0.11.14", default-features = false }
serde-indexed = "0.1.0"
sha2 = { version = "0.10", default-features = false }
trussed = "0.1"
trussed-fs-info = "0.1.0"
trussed-hkdf = { version = "0.2.0" }
trussed-chunked = { version = "0.1.0", optional = true }

Expand Down Expand Up @@ -65,7 +66,7 @@ p256 = { version = "0.13.2", features = ["ecdh"] }
rand = "0.8.4"
sha2 = "0.10"
trussed = { version = "0.1", features = ["virt"] }
trussed-staging = { version = "0.3.0", features = ["chunked", "hkdf", "virt"] }
trussed-staging = { version = "0.3.0", features = ["chunked", "hkdf", "virt", "fs-info"] }
trussed-usbip = { version = "0.0.1", default-features = false, features = ["ctaphid"] }
usbd-ctaphid = "0.1.0"
x509-parser = "0.16.0"
Expand All @@ -78,10 +79,11 @@ cbor-smol = { git = "https://github.com/sosthene-nitrokey/cbor-smol.git", rev =
ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "72eb68b61e3f14957c5ab89bd22f776ac860eb62" }
ctaphid-dispatch = { git = "https://github.com/trussed-dev/ctaphid-dispatch.git", rev = "57cb3317878a8593847595319aa03ef17c29ec5b" }
apdu-dispatch = { git = "https://github.com/trussed-dev/apdu-dispatch.git", rev = "915fc237103fcecc29d0f0b73391f19abf6576de" }
littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "2b45a7559ff44260c6dd693e4cb61f54ae5efc53" }
trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "b548d379dcbd67d29453d94847b7bc33ae92e673" }
trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "a055e4f79a10122c8c0c882161442e6e02f0c5c6" }
littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "960e57d9fc0d209308c8e15dc26252bbe1ff6ba8" }
trussed-chunked = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "chunked-v0.1.0" }
trussed-fs-info = { git = "https://github.com/trussed-dev/trussed-staging.git", rev = "170ab14f3bb6760399749d78e1b94e3b70106739" }
trussed-hkdf = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "hkdf-v0.2.0" }
trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "v0.3.0" }
trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", rev = "170ab14f3bb6760399749d78e1b94e3b70106739" }
trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.1" }
usbd-ctaphid = { git = "https://github.com/trussed-dev/usbd-ctaphid.git", rev = "dcff9009c3cd1ef9e5b09f8f307aca998fc9a8c8" }
9 changes: 5 additions & 4 deletions fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ cargo-fuzz = true
[dependencies]
ctap-types = { version = "0.2.0", features = ["arbitrary"] }
libfuzzer-sys = "0.4"
trussed-staging = { version = "0.3.0", features = ["chunked", "hkdf", "virt"] }
trussed-staging = { version = "0.3.0", features = ["chunked", "hkdf", "virt", "fs-info"] }

[dependencies.fido-authenticator]
path = ".."
Expand All @@ -24,9 +24,10 @@ bench = false

[patch.crates-io]
ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "72eb68b61e3f14957c5ab89bd22f776ac860eb62" }
littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "2b45a7559ff44260c6dd693e4cb61f54ae5efc53" }
trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "b548d379dcbd67d29453d94847b7bc33ae92e673" }
trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "a055e4f79a10122c8c0c882161442e6e02f0c5c6" }
littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "960e57d9fc0d209308c8e15dc26252bbe1ff6ba8" }
trussed-chunked = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "chunked-v0.1.0" }
trussed-hkdf = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "hkdf-v0.2.0" }
trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "v0.3.0" }
cbor-smol = { git = "https://github.com/sosthene-nitrokey/cbor-smol.git", rev = "9a77dc9b528b08f531d76b44af2f5336c4ef17e0"}
trussed-fs-info = { git = "https://github.com/trussed-dev/trussed-staging.git", rev = "170ab14f3bb6760399749d78e1b94e3b70106739" }
trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", rev = "170ab14f3bb6760399749d78e1b94e3b70106739" }
34 changes: 10 additions & 24 deletions src/ctap2.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! The `ctap_types::ctap2::Authenticator` implementation.

use credential_management::CredentialManagement;
use ctap_types::{
ctap2::{
self, client_pin::Permissions, AttestationFormatsPreference, AttestationStatement,
Expand All @@ -21,15 +22,9 @@ use trussed::{
};

use crate::{
constants,
constants::{self, MAX_RESIDENT_CREDENTIALS_GUESSTIMATE},
credential::{self, Credential, FullCredential, Key, StrippedCredential},
format_hex,
state::{
self,
// // (2022-02-27): 9288 bytes
// MinCredentialHeap,
},
Result, SigningAlgorithm, TrussedRequirements, UserPresence,
format_hex, state, Result, SigningAlgorithm, TrussedRequirements, UserPresence,
};

#[allow(unused_imports)]
Expand Down Expand Up @@ -393,21 +388,12 @@ impl<UP: UserPresence, T: TrussedRequirements> 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);
debug!("resident cred count: {} (max: {})", count, max_count);
if count >= max_count {
error!("maximum resident credential count reached");
key_store_full = true;
}
}
let mut key_store_full = !self.can_fit(serialized_credential.len())
|| CredentialManagement::new(self).count_credentials()
>= self
.config
.max_resident_credential_count
.unwrap_or(MAX_RESIDENT_CREDENTIALS_GUESSTIMATE);
Comment on lines +393 to +396
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we now always apply a hard limit, it would make sense to make the max_resident_credential_count configuration required and remove the guesstimate constant. This would also make the code easier to read.


if !key_store_full {
// then store key, making it resident
Expand Down Expand Up @@ -1870,7 +1856,7 @@ impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
);
} else {
info!("deleting parent {:?} as this was its last RK", &rp_path);
syscall!(self.trussed.remove_dir(Location::Internal, rp_path,));
try_syscall!(self.trussed.remove_dir(Location::Internal, rp_path,)).ok();
}
}

Expand Down
19 changes: 8 additions & 11 deletions src/ctap2/credential_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use ctap_types::{
};

use crate::{
constants::MAX_RESIDENT_CREDENTIALS_GUESSTIMATE,
credential::FullCredential,
state::{CredentialManagementEnumerateCredentials, CredentialManagementEnumerateRps},
Authenticator, Result, TrussedRequirements, UserPresence,
Expand Down Expand Up @@ -65,14 +64,15 @@ where
info!("get metadata");
let mut response: Response = Default::default();

let max_resident_credentials = self
.config
.max_resident_credential_count
.unwrap_or(MAX_RESIDENT_CREDENTIALS_GUESSTIMATE);
response.existing_resident_credentials_count = Some(0);
let max_resident_credentials = self.estimate_remaining();
response.existing_resident_credentials_count = Some(self.count_credentials());
response.max_possible_remaining_residential_credentials_count =
Some(max_resident_credentials);

response
}

pub fn count_credentials(&mut self) -> u32 {
let dir = PathBuf::from(b"rk");
let maybe_first_rp =
syscall!(self
Expand All @@ -81,7 +81,7 @@ where
.entry;

let first_rp = match maybe_first_rp {
None => return response,
None => return 0,
Some(rp) => rp,
};

Expand All @@ -98,10 +98,7 @@ where

match maybe_next_rp {
None => {
response.existing_resident_credentials_count = Some(num_rks);
response.max_possible_remaining_residential_credentials_count =
Some(max_resident_credentials.saturating_sub(num_rks));
return response;
return num_rks;
}
Some(rp) => {
last_rp = PathBuf::from(rp.file_name());
Expand Down
48 changes: 47 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ generate_macros!();

use core::time::Duration;

use trussed::{client, syscall, types::Message, Client as TrussedClient};
use trussed::{
client, syscall,
types::{Location, Message},
Client as TrussedClient,
};
use trussed_fs_info::{FsInfoClient, FsInfoReply};
use trussed_hkdf::HkdfClient;

/// Re-export of `ctap-types` authenticator errors.
Expand All @@ -38,6 +43,8 @@ pub mod state;

pub use ctap2::large_blobs::Config as LargeBlobsConfig;

use crate::constants::MAX_RESIDENT_CREDENTIALS_GUESSTIMATE;

/// Results with our [`Error`].
pub type Result<T> = core::result::Result<T, Error>;

Expand All @@ -57,6 +64,7 @@ pub trait TrussedRequirements:
+ client::Sha256
+ client::HmacSha256
+ client::Ed255 // + client::Totp
+ FsInfoClient
+ HkdfClient
+ ExtensionRequirements
{
Expand All @@ -70,6 +78,7 @@ impl<T> TrussedRequirements for T where
+ client::Sha256
+ client::HmacSha256
+ client::Ed255 // + client::Totp
+ FsInfoClient
+ HkdfClient
+ ExtensionRequirements
{
Expand Down Expand Up @@ -266,6 +275,43 @@ where
}
}

fn estimate_remaining_inner(info: &FsInfoReply) -> u32 {
let block_size = info.block_info.as_ref().map(|i| i.size).unwrap_or(255);
robin-nitrokey marked this conversation as resolved.
Show resolved Hide resolved
// 1 block for the directory, 1 for the private key, 400 bytes for a reasonnable key and metadata
let size_taken = 2 * block_size + 400;
// Remove 5 block kept as buffer
((info.available_space - 5 * block_size) / size_taken) as u32
sosthene-nitrokey marked this conversation as resolved.
Show resolved Hide resolved
}

fn estimate_remaining(&mut self) -> u32 {
let info = syscall!(self.trussed.fs_info(Location::Internal));
debug!("Got filesystem info: {info:?}");
Self::estimate_remaining_inner(&info).min(
self.config
.max_resident_credential_count
.unwrap_or(MAX_RESIDENT_CREDENTIALS_GUESSTIMATE),
)
sosthene-nitrokey marked this conversation as resolved.
Show resolved Hide resolved
}

fn can_fit_inner(info: &FsInfoReply, size: usize) -> bool {
let block_size = info.block_info.as_ref().map(|i| i.size).unwrap_or(255);
sosthene-nitrokey marked this conversation as resolved.
Show resolved Hide resolved
// 1 block for the rp directory, 5 block of margin, 50 bytes for a reasonnable metadata
let size_taken = 6 * block_size + size + 50;
size_taken < info.available_space
}

/// Can a credential of size `size` be stored with safe margins
robin-nitrokey marked this conversation as resolved.
Show resolved Hide resolved
fn can_fit(&mut self, size: usize) -> bool {
debug!("Can fit for {size} bytes");
let info = syscall!(self.trussed.fs_info(Location::Internal));
debug!("Got filesystem info: {info:?}");
debug!(
"Available storage: {}",
Self::estimate_remaining_inner(&info)
);
Self::can_fit_inner(&info, size)
}

fn hash(&mut self, data: &[u8]) -> [u8; 32] {
let hash = syscall!(self.trussed.hash_sha256(data)).hash;
hash.as_slice().try_into().expect("hash should fit")
Expand Down
Loading