From 82fc81a019053f6cf7b96d8b935919f3b66cc37f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 31 Jan 2024 14:57:47 +0100 Subject: [PATCH 01/14] pin_info_hash: use serde-byte-array --- Cargo.toml | 3 +++ src/state.rs | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 1270ab7..c5fbc67 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ trussed-staging = { version = "0.1.0", default-features = false, optional = true apdu-dispatch = { version = "0.1", optional = true } ctaphid-dispatch = { version = "0.1", optional = true } iso7816 = { version = "0.1", optional = true } +serde-byte-array = "0.1.2" [features] dispatch = ["apdu-dispatch", "ctaphid-dispatch", "iso7816"] @@ -46,6 +47,7 @@ log-error = [] [dev-dependencies] env_logger = "0.11.0" +hex-literal = "0.4.1" # quickcheck = "1" rand = "0.8.4" trussed = { version = "0.1", features = ["virt"] } @@ -56,6 +58,7 @@ usbd-ctaphid = "0.1.0" features = ["dispatch"] [patch.crates-io] +cbor-smol = { git = "https://github.com/sosthene-nitrokey/cbor-smol.git", rev = "94ee8c28edf9248b402aa4335c1dee157995197b"} ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "7d4ad69e64ad308944c012aef5b9cfd7654d9be8" } 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" } diff --git a/src/state.rs b/src/state.rs index 7b524aa..af05287 100644 --- a/src/state.rs +++ b/src/state.rs @@ -263,6 +263,7 @@ pub struct PersistentState { key_encryption_key: Option, key_wrapping_key: Option, consecutive_pin_mismatches: u8, + #[serde(with = "serde_byte_array")] pin_hash: Option<[u8; 16]>, // Ideally, we'd dogfood a "Monotonic Counter" from trussed. // TODO: Add per-key counters for resident keys. @@ -592,3 +593,23 @@ impl RuntimeState { Ok(shared_secret) } } + +#[cfg(test)] +mod tests { + use super::*; + use hex_literal::hex; + + #[test] + fn deser() { + let _state: PersistentState = trussed::cbor_deserialize(&hex!( + " + a5726b65795f656e6372797074696f6e5f6b657950b19a5a2845e5ec71e3 + 2a1b890892376c706b65795f7772617070696e675f6b6579f6781a636f6e + 73656375746976655f70696e5f6d69736d617463686573006870696e5f68 + 6173689018ef1879187c1881181818f0182d18fb186418960718dd185d18 + 3f188c18766974696d657374616d7009 + " + )) + .unwrap(); + } +} From 13e6303913466da874df7ea38933d5f348ef947f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 2 Feb 2024 16:19:31 +0100 Subject: [PATCH 02/14] Make credential: change the path of rks to `rp_id_hash.credential_id_hash` from `rp_id_hash/credential_id_hash` The goal is to make credential storage more efficient, by making use of littlefs's ability to inline file contents into the directory metadata when the file is small. --- src/ctap2.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ctap2.rs b/src/ctap2.rs index 0ef2fe2..542881b 100644 --- a/src/ctap2.rs +++ b/src/ctap2.rs @@ -1941,11 +1941,10 @@ fn rp_rk_dir(rp_id_hash: &Bytes<32>) -> PathBuf { } fn rk_path(rp_id_hash: &Bytes<32>, credential_id_hash: &Bytes<32>) -> PathBuf { - let mut path = rp_rk_dir(rp_id_hash); + let mut buf = [0; 33]; + buf[16] = b'.'; + format_hex(&rp_id_hash[..8], &mut buf[..16]); + format_hex(&credential_id_hash[..8], &mut buf[17..]); - let mut hex = [0u8; 16]; - format_hex(&credential_id_hash[..8], &mut hex); - path.push(&PathBuf::from(&hex)); - - path + PathBuf::from(buf.as_slice()) } From 3909598302064e59ee42d165f15e80928956fa87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 2 Feb 2024 16:52:53 +0100 Subject: [PATCH 03/14] Iterate over the new placements for resident keys --- Cargo.toml | 3 ++- src/ctap2.rs | 71 ++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c5fbc67..8da17a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,7 +62,8 @@ cbor-smol = { git = "https://github.com/sosthene-nitrokey/cbor-smol.git", rev = ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "7d4ad69e64ad308944c012aef5b9cfd7654d9be8" } 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" } -trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "b1781805a2e33615d2d00b8bec80c0b1f5870ca1" } +trussed = { path = "../trussed" } +littlefs2 = { path = "../littlefs2" } trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging", rev = "3b9594d93f89a5e760fe78fa5a96f125dfdcd470" } serde-indexed = { git = "https://github.com/sosthene-nitrokey/serde-indexed.git", rev = "5005d23cb4ee8622e62188ea0f9466146f851f0d" } trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.1" } diff --git a/src/ctap2.rs b/src/ctap2.rs index 542881b..a3885f0 100644 --- a/src/ctap2.rs +++ b/src/ctap2.rs @@ -1128,7 +1128,69 @@ impl crate::Authenticator { // we are only dealing with discoverable credentials. debug_now!("Allowlist not passed, fetching RKs"); + let legacy_present = self.prepare_cache(rp_id_hash, uv_performed)?; + if legacy_present { + self.prepare_cache_legacy(rp_id_hash, uv_performed)?; + } + + let num_credentials = self.state.runtime.remaining_credentials(); + let credential = self.state.runtime.pop_credential(&mut self.trussed); + credential.map(|credential| (Credential::Full(credential), num_credentials)) + } + + /// Populate the cache with the RP credentials. + /// + /// Returns true if legacy credentials are present and therefore prepare_cache_legacy should be called too + #[inline(never)] + fn prepare_cache(&mut self, rp_id_hash: &Bytes<32>, uv_performed: bool) -> Option { + use crate::state::CachedCredential; + use core::str::FromStr; + + let rp_rk_dir = rp_rk_dir(rp_id_hash); + let mut maybe_entry = syscall!(self.trussed.read_dir_first( + Location::Internal, + PathBuf::from(b"rk"), + Some(rp_rk_dir.clone()) + )) + .entry; + + let mut legacy_detected = false; + while let Some(entry) = maybe_entry.take() { + if !entry.path().as_ref().starts_with(rp_rk_dir.as_ref()) { + // We got past all credentials for the relevant RP + break; + } + + if entry.path() == &*rp_rk_dir { + debug_assert!(entry.metadata().is_dir()); + legacy_detected = true; + continue; + } + + let credential_data = syscall!(self + .trussed + .read_file(Location::Internal, entry.path().into(),)) + .data; + let credential = FullCredential::deserialize(&credential_data).ok()?; + let timestamp = credential.creation_time; + let credential = Credential::Full(credential); + + if self.check_credential_applicable(&credential, false, uv_performed) { + self.state.runtime.push_credential(CachedCredential { + timestamp, + path: String::from_str(entry.path().as_str_ref_with_trailing_nul()).ok()?, + }); + } + + maybe_entry = syscall!(self.trussed.read_dir_next()).entry; + } + Some(legacy_detected) + } + + /// Populate the cache with the legacy `rp_id/cred_id` rk + #[inline(never)] + fn prepare_cache_legacy(&mut self, rp_id_hash: &Bytes<32>, uv_performed: bool) -> Option<()> { let mut maybe_path = syscall!(self .trussed @@ -1158,10 +1220,7 @@ impl crate::Authenticator { .entry .map(|entry| PathBuf::from(entry.path())); } - - let num_credentials = self.state.runtime.remaining_credentials(); - let credential = self.state.runtime.pop_credential(&mut self.trussed); - credential.map(|credential| (Credential::Full(credential), num_credentials)) + Some(()) } fn decrypt_pin_hash_and_maybe_escalate( @@ -1946,5 +2005,7 @@ fn rk_path(rp_id_hash: &Bytes<32>, credential_id_hash: &Bytes<32>) -> PathBuf { format_hex(&rp_id_hash[..8], &mut buf[..16]); format_hex(&credential_id_hash[..8], &mut buf[17..]); - PathBuf::from(buf.as_slice()) + let mut path = PathBuf::from("rk"); + path.push(&PathBuf::from(buf.as_slice())); + path } From 594af2252d6b076b4004e0e15913bc9a071c9cb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 5 Feb 2024 14:36:46 +0100 Subject: [PATCH 04/14] Update RP listing to support both the old format --- src/ctap2/credential_management.rs | 214 +++++++++++++++-------------- src/state.rs | 4 +- 2 files changed, 113 insertions(+), 105 deletions(-) diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index 0aba519..aff7669 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -1,6 +1,6 @@ //! TODO: T -use core::convert::TryFrom; +use core::{convert::TryFrom, num::NonZeroU32}; use trussed::{ syscall, @@ -57,6 +57,16 @@ where } } +/// Get the hex hashed ID of the RP from the filename of a RP directory OR a "new" RK path +fn get_id_hex(entry: &DirEntry) -> &str { + entry + .file_name() + .as_str() + .split('.') + .next() + .expect("Split always returns at least one empty string") +} + impl CredentialManagement<'_, UP, T> where UP: UserPresence, @@ -118,78 +128,76 @@ where pub fn first_relying_party(&mut self) -> Result { info!("first rp"); - // rp (0x03): PublicKeyCredentialRpEntity - // rpIDHash (0x04) : RP ID SHA-256 hash. - // totalRPs (0x05) : Total number of RPs present on the authenticator. - - let mut response: Response = Default::default(); - + let mut response = Response::default(); let dir = PathBuf::from(b"rk"); let maybe_first_rp = - syscall!(self.trussed.read_dir_first(Location::Internal, dir, None)).entry; - - response.total_rps = Some(match maybe_first_rp { - None => 0, - _ => { - let mut num_rps = 1; - loop { - let maybe_next_rp = syscall!(self.trussed.read_dir_next()).entry; - match maybe_next_rp { - None => break, - _ => num_rps += 1, - } - } - num_rps - } - }); + syscall!(self + .trussed + .read_dir_first(Location::Internal, dir.clone(), None)) + .entry; + + let Some(first_rp) = maybe_first_rp else { + response.total_rps = Some(0); + return Ok(response); + }; - if let Some(rp) = maybe_first_rp { - // load credential and extract rp and rpIdHash - let maybe_first_credential = syscall!(self.trussed.read_dir_first( + // The first one counts + let mut total_rps = 1; + + let first_credential_data; + if first_rp.metadata().is_dir() { + first_credential_data = syscall!(self.trussed.read_dir_files_first( Location::Internal, - PathBuf::from(rp.path()), + first_rp.path().into(), None )) - .entry; + .data + .expect("RP directory is expected to never be empty"); - match maybe_first_credential { - None => panic!("chaos! disorder!"), - Some(rk_entry) => { - let serialized = syscall!(self - .trussed - .read_file(Location::Internal, rk_entry.path().into(),)) - .data; + // Restart the iteration over the directory + syscall!(self.trussed.read_dir_first(Location::Internal, dir, None)); + } else { + debug_assert!(first_rp.metadata().is_file()); + first_credential_data = syscall!(self + .trussed + .read_file(Location::Internal, first_rp.path().into())) + .data; + } - let credential = FullCredential::deserialize(&serialized) - // this may be a confusing error message - .map_err(|_| Error::InvalidCredential)?; + let credential = FullCredential::deserialize(&first_credential_data)?; + let rp_id_hash = syscall!(self.trussed.hash_sha256(credential.rp.id.as_ref())) + .hash + .to_bytes() + .map_err(|_| Error::Other)?; - let rp = credential.data.rp; + let mut current_rp = first_rp; - response.rp_id_hash = Some(self.hash(rp.id.as_ref())); - response.rp = Some(rp); - } - } + let mut current_id_hex = get_id_hex(¤t_rp); - // cache state for next call - if let Some(total_rps) = response.total_rps { - if total_rps > 1 { - let rp_id_hash = response.rp_id_hash.as_ref().unwrap().clone(); - self.state.runtime.cached_rp = Some(CredentialManagementEnumerateRps { - remaining: total_rps - 1, - rp_id_hash, - }); - } + while let Some(entry) = syscall!(self.trussed.read_dir_next()).entry { + let id_hex = get_id_hex(¤t_rp); + if id_hex != current_id_hex { + total_rps += 1; + current_rp = entry; + current_id_hex = get_id_hex(¤t_rp) } } + if let Some(remaining) = NonZeroU32::new(total_rps - 1) { + self.state.runtime.cached_rp = Some(CredentialManagementEnumerateRps { + remaining, + rp_id_hash: rp_id_hash.clone(), + }); + } + + response.total_rps = Some(total_rps); + response.rp_id_hash = Some(rp_id_hash); + response.rp = Some(credential.data.rp); Ok(response) } pub fn next_relying_party(&mut self) -> Result { - info!("next rp"); - let CredentialManagementEnumerateRps { remaining, rp_id_hash: last_rp_id_hash, @@ -197,73 +205,71 @@ where .state .runtime .cached_rp - .clone() + .take() .ok_or(Error::NotAllowed)?; - let dir = PathBuf::from(b"rk"); - let mut hex = [b'0'; 16]; super::format_hex(&last_rp_id_hash[..8], &mut hex); let filename = PathBuf::from(&hex); - let mut maybe_next_rp = + let dir = PathBuf::from(b"rk"); + + let maybe_next_rp = syscall!(self .trussed - .read_dir_first(Location::Internal, dir, Some(filename),)) + .read_dir_first(Location::Internal, dir, Some(filename))) .entry; - // Advance to the next - if maybe_next_rp.is_some() { - maybe_next_rp = syscall!(self.trussed.read_dir_next()).entry; - } else { + let mut response = Response::default(); + + let Some(current_rp) = maybe_next_rp else { return Err(Error::NotAllowed); - } + }; - let mut response: Response = Default::default(); + let current_id_hex = get_id_hex(¤t_rp); - if let Some(rp) = maybe_next_rp { - // load credential and extract rp and rpIdHash - let maybe_first_credential = syscall!(self.trussed.read_dir_first( - Location::Internal, - PathBuf::from(rp.path()), - None - )) - .entry; + debug_assert!(current_rp.file_name().as_str().as_bytes().starts_with(&hex)); - match maybe_first_credential { - None => panic!("chaos! disorder!"), - Some(rk_entry) => { - let serialized = syscall!(self - .trussed - .read_file(Location::Internal, rk_entry.path().into(),)) - .data; - - let credential = FullCredential::deserialize(&serialized) - // this may be a confusing error message - .map_err(|_| Error::InvalidCredential)?; - - let rp = credential.data.rp; - - response.rp_id_hash = Some(self.hash(rp.id.as_ref())); - response.rp = Some(rp); - - // cache state for next call - if remaining > 1 { - let rp_id_hash = response.rp_id_hash.as_ref().unwrap().clone(); - self.state.runtime.cached_rp = Some(CredentialManagementEnumerateRps { - remaining: remaining - 1, - rp_id_hash, - }); - } else { - self.state.runtime.cached_rp = None; - } - } + while let Some(entry) = syscall!(self.trussed.read_dir_next()).entry { + let id_hex = get_id_hex(&entry); + if id_hex == current_id_hex { + continue; } - } else { - self.state.runtime.cached_rp = None; + + let data = if entry.metadata().is_dir() { + syscall!(self.trussed.read_dir_files_first( + Location::Internal, + entry.path().into(), + None + )) + .data + .expect("RP dir should not be empty") + } else { + syscall!(self + .trussed + .read_file(Location::Internal, entry.path().into())) + .data + }; + + let credential = FullCredential::deserialize(&data)?; + let rp_id_hash = syscall!(self.trussed.hash_sha256(credential.rp.id.as_ref())) + .hash + .to_bytes() + .map_err(|_| Error::Other)?; + response.rp_id_hash = Some(rp_id_hash.clone()); + response.rp = Some(credential.data.rp); + + if let Some(new_remaining) = NonZeroU32::new(remaining.get() - 1) { + self.state.runtime.cached_rp = Some(CredentialManagementEnumerateRps { + remaining: new_remaining, + rp_id_hash, + }); + } + + return Ok(response); } - Ok(response) + return Err(Error::NotAllowed); } fn count_rp_rks(&mut self, rp_dir: PathBuf) -> (u32, Option) { diff --git a/src/state.rs b/src/state.rs index af05287..a7a7537 100644 --- a/src/state.rs +++ b/src/state.rs @@ -2,6 +2,8 @@ //! //! Needs cleanup. +use core::num::NonZeroU32; + use ctap_types::{ cose::EcdhEsHkdf256PublicKey as CoseEcdhEsHkdf256PublicKey, // 2022-02-27: 10 credentials @@ -195,7 +197,7 @@ impl Identity { #[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)] pub struct CredentialManagementEnumerateRps { - pub remaining: u32, + pub remaining: NonZeroU32, pub rp_id_hash: Bytes<32>, } From ad4d5a9ddf2b0f01cb84685dffa71fcd548a867d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 5 Feb 2024 16:03:23 +0100 Subject: [PATCH 05/14] Rename credential counts to credential_counts_legacy --- src/ctap2/credential_management.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index aff7669..0273b33 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -96,7 +96,8 @@ where Some(rp) => rp, }; - let (mut num_rks, _) = self.count_rp_rks(PathBuf::from(first_rp.path())); + // TODO: FIX + let (mut num_rks, _) = self.count_legacy_rp_rks(PathBuf::from(first_rp.path())); let mut last_rp = PathBuf::from(first_rp.file_name()); loop { @@ -117,7 +118,8 @@ where Some(rp) => { last_rp = PathBuf::from(rp.file_name()); info!("counting.."); - let (this_rp_rk_count, _) = self.count_rp_rks(PathBuf::from(rp.path())); + // TODO: FIX + let (this_rp_rk_count, _) = self.count_legacy_rp_rks(PathBuf::from(rp.path())); info!("{:?}", this_rp_rk_count); num_rks += this_rp_rk_count; } @@ -272,7 +274,7 @@ where return Err(Error::NotAllowed); } - fn count_rp_rks(&mut self, rp_dir: PathBuf) -> (u32, Option) { + fn count_legacy_rp_rks(&mut self, rp_dir: PathBuf) -> (u32, Option) { let maybe_first_rk = syscall!(self .trussed @@ -301,7 +303,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); + // TODO: FIX + let (num_rks, first_rk) = self.count_legacy_rp_rks(rp_dir); let first_rk = first_rk.ok_or(Error::NoCredentials)?; // extract data required into response From 58aeeb77a89fc3c4199a09538216ea78c6c40e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 5 Feb 2024 16:53:12 +0100 Subject: [PATCH 06/14] Use selected trussed iteration not_before behaviour --- src/ctap2.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctap2.rs b/src/ctap2.rs index a3885f0..7d822a4 100644 --- a/src/ctap2.rs +++ b/src/ctap2.rs @@ -1147,7 +1147,7 @@ impl crate::Authenticator { use core::str::FromStr; let rp_rk_dir = rp_rk_dir(rp_id_hash); - let mut maybe_entry = syscall!(self.trussed.read_dir_first( + let mut maybe_entry = syscall!(self.trussed.read_dir_first_alphabetical( Location::Internal, PathBuf::from(b"rk"), Some(rp_rk_dir.clone()) From fcf25eec2964580031a3c5a0cd6bac6333f990d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 5 Feb 2024 17:03:38 +0100 Subject: [PATCH 07/14] Update get_creds_metadata to support new credentials --- src/ctap2/credential_management.rs | 38 ++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index 0273b33..acb5508 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -91,19 +91,28 @@ where .read_dir_first(Location::Internal, dir.clone(), None)) .entry; - let first_rp = match maybe_first_rp { - None => return response, - Some(rp) => rp, + let Some(first_rp) = maybe_first_rp else { + return response; }; - // TODO: FIX - let (mut num_rks, _) = self.count_legacy_rp_rks(PathBuf::from(first_rp.path())); - let mut last_rp = PathBuf::from(first_rp.file_name()); + let mut num_rks = 0; + + if first_rp.metadata().is_dir() { + let (rk_count, _) = self.count_legacy_rp_rks(PathBuf::from(first_rp.path())); + num_rks += rk_count; + } else { + debug_assert!(first_rp.metadata().is_file()); + num_rks += 1; + } + + let mut previous_credential = first_rp.file_name().into(); loop { - syscall!(self - .trussed - .read_dir_first(Location::Internal, dir.clone(), Some(last_rp),)) + syscall!(self.trussed.read_dir_first( + Location::Internal, + dir.clone(), + Some(previous_credential), + )) .entry .unwrap(); let maybe_next_rp = syscall!(self.trussed.read_dir_next()).entry; @@ -116,8 +125,17 @@ where return response; } Some(rp) => { - last_rp = PathBuf::from(rp.file_name()); + previous_credential = PathBuf::from(rp.file_name()); info!("counting.."); + + if first_rp.metadata().is_dir() { + let (rk_count, _) = self.count_legacy_rp_rks(first_rp.path().into()); + num_rks += rk_count; + } else { + debug_assert!(first_rp.metadata().is_file()); + num_rks += 1; + } + // TODO: FIX let (this_rp_rk_count, _) = self.count_legacy_rp_rks(PathBuf::from(rp.path())); info!("{:?}", this_rp_rk_count); From c54e769c18a3c39e5f9837255c1bc79a12fb5785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 6 Feb 2024 11:05:51 +0100 Subject: [PATCH 08/14] Implement credential iteration with support for new credentials --- src/ctap2/credential_management.rs | 189 ++++++++++++++++++++++------- src/state.rs | 5 +- 2 files changed, 147 insertions(+), 47 deletions(-) diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index acb5508..7d94ebf 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -1,5 +1,6 @@ //! TODO: T +use core::cmp::Ordering; use core::{convert::TryFrom, num::NonZeroU32}; use trussed::{ @@ -225,7 +226,7 @@ where .state .runtime .cached_rp - .take() + .clone() .ok_or(Error::NotAllowed)?; let mut hex = [b'0'; 16]; @@ -320,9 +321,51 @@ where let mut hex = [b'0'; 16]; super::format_hex(&rp_id_hash[..8], &mut hex); - let rp_dir = PathBuf::from(b"rk").join(&PathBuf::from(&hex)); + let rk_dir = PathBuf::from(b"rk"); + let rp_dir_start = PathBuf::from(b"rk").join(&PathBuf::from(&hex)); + + let mut num_rks = 0; + + let mut maybe_entry = syscall!(self.trussed.read_dir_first_alphabetical( + Location::Internal, + rk_dir.clone(), + Some(rk_dir) + )) + .entry; + + let mut legacy_detected = false; + let mut only_legacy = true; + + let mut first_rk = None; + + while let Some(entry) = maybe_entry { + if !entry.path().as_str().as_bytes().starts_with(&hex) { + // We got past all credentials for the relevant RP + break; + } + + if entry.path() == &*rp_dir_start { + // This is the case where we + debug_assert!(entry.metadata().is_dir()); + legacy_detected = true; + // Because of the littlefs iteration order, we know that we are at the end + break; + } + + first_rk = first_rk.or(Some(entry)); + only_legacy = false; + num_rks += 1; + + maybe_entry = syscall!(self.trussed.read_dir_next()).entry; + } + + if legacy_detected { + let (legacy_rks, first_legacy_rk) = self.count_legacy_rp_rks(rp_dir_start); + num_rks += legacy_rks; + first_rk = first_rk.or(first_legacy_rk); + } + // TODO: FIX - let (num_rks, first_rk) = self.count_legacy_rp_rks(rp_dir); let first_rk = first_rk.ok_or(Error::NoCredentials)?; // extract data required into response @@ -335,8 +378,9 @@ where // let rp_id_hash = response.rp_id_hash.as_ref().unwrap().clone(); self.state.runtime.cached_rk = Some(CredentialManagementEnumerateCredentials { remaining: num_rks - 1, - rp_dir: first_rk.path().parent().unwrap(), - prev_filename: PathBuf::from(first_rk.file_name()), + rp_dir: PathBuf::from(&hex), + prev_filename: Some(first_rk.file_name().into()), + iterating_legacy: only_legacy, }); } } @@ -344,63 +388,116 @@ where Ok(response) } - pub fn next_credential(&mut self) -> Result { - info!("next credential"); - + pub fn next_legacy_credential( + &mut self, + cache: CredentialManagementEnumerateCredentials, + ) -> Result { let CredentialManagementEnumerateCredentials { remaining, rp_dir, prev_filename, - } = self + iterating_legacy, + } = cache; + + debug_assert!(iterating_legacy); + + let mut maybe_next_rk = syscall!(self.trussed.read_dir_first( + Location::Internal, + rp_dir.clone(), + prev_filename.clone() + )) + .entry; + + if maybe_next_rk.is_none() { + return Err(Error::NotAllowed); + } + + if prev_filename.is_some() { + // This is not the first iteration + maybe_next_rk = syscall!(self.trussed.read_dir_next()).entry; + } + + let Some(rk) = maybe_next_rk else { + return Err(Error::NoCredentials); + }; + + let response = self.extract_response_from_credential_file(rk.path())?; + + // cache state for next call + if remaining > 1 { + self.state.runtime.cached_rk = Some(CredentialManagementEnumerateCredentials { + remaining: remaining - 1, + rp_dir, + prev_filename: Some(PathBuf::from(rk.file_name())), + iterating_legacy: true, + }); + } + + Ok(response) + } + + pub fn next_credential(&mut self) -> Result { + info!("next credential"); + + let cache = self .state .runtime .cached_rk - .clone() + .take() .ok_or(Error::NotAllowed)?; - // let (remaining, rp_dir, prev_filename) = match self.state.runtime.cached_rk { - // Some(CredentialManagementEnumerateCredentials( - // x, ref y, ref z)) - // => (x, y.clone(), z.clone()), - // _ => return Err(Error::NotAllowed), - // }; - self.state.runtime.cached_rk = None; + if cache.iterating_legacy { + return self.next_legacy_credential(cache); + } - // let mut hex = [b'0'; 16]; - // super::format_hex(&rp_id_hash[..8], &mut hex); - // let rp_dir = PathBuf::from(b"rk").join(&PathBuf::from(&hex)); + let CredentialManagementEnumerateCredentials { + remaining, + rp_dir, + prev_filename, + iterating_legacy: _, + } = cache; - let mut maybe_next_rk = - syscall!(self - .trussed - .read_dir_first(Location::Internal, rp_dir, Some(prev_filename))) - .entry; + debug_assert!(prev_filename.is_some()); - // Advance to the next - if maybe_next_rk.is_some() { - maybe_next_rk = syscall!(self.trussed.read_dir_next()).entry; - } else { - return Err(Error::NotAllowed); + syscall!(self.trussed.read_dir_first_alphabetical( + Location::Internal, + rp_dir.clone(), + prev_filename + )) + .entry; + + // The previous entry was already read. Skip to the next + let Some(entry) = syscall!(self.trussed.read_dir_next()).entry else { + return Err(Error::NoCredentials); + }; + + if entry.file_name().cmp_lfs(&rp_dir) == Ordering::Greater { + // We reached the end of the credentials for the rp + return Err(Error::NoCredentials); } - match maybe_next_rk { - Some(rk) => { - // extract data required into response - let response = self.extract_response_from_credential_file(rk.path())?; - - // cache state for next call - if remaining > 1 { - self.state.runtime.cached_rk = Some(CredentialManagementEnumerateCredentials { - remaining: remaining - 1, - rp_dir: rk.path().parent().unwrap(), - prev_filename: PathBuf::from(rk.file_name()), - }); - } + if entry.metadata().is_dir() { + return self.next_legacy_credential(CredentialManagementEnumerateCredentials { + remaining, + rp_dir, + prev_filename: None, + iterating_legacy: true, + }); + } - Ok(response) - } - None => Err(Error::NoCredentials), + let response = self.extract_response_from_credential_file(entry.path())?; + + // cache state for next call + if remaining > 1 { + self.state.runtime.cached_rk = Some(CredentialManagementEnumerateCredentials { + remaining: remaining - 1, + rp_dir, + prev_filename: Some(entry.path().into()), + iterating_legacy: true, + }); } + + Ok(response) } fn extract_response_from_credential_file(&mut self, rk_path: &Path) -> Result { diff --git a/src/state.rs b/src/state.rs index a7a7537..e101144 100644 --- a/src/state.rs +++ b/src/state.rs @@ -205,7 +205,10 @@ pub struct CredentialManagementEnumerateRps { pub struct CredentialManagementEnumerateCredentials { pub remaining: u32, pub rp_dir: PathBuf, - pub prev_filename: PathBuf, + /// None means that we finished iterating over the legacy credentials, + /// and are starting to iterate over the legacy credentials + pub prev_filename: Option, + pub iterating_legacy: bool, } #[derive( From d2ac9f82a711b2053d62702eebb261c0bd2908ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 6 Feb 2024 18:02:39 +0100 Subject: [PATCH 09/14] Fix credential listing to support older credentials --- src/ctap2/credential_management.rs | 24 +++++++++++++++--------- src/dispatch.rs | 5 +++-- src/lib.rs | 2 +- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index 7d94ebf..8bc1e3b 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -322,14 +322,14 @@ where super::format_hex(&rp_id_hash[..8], &mut hex); let rk_dir = PathBuf::from(b"rk"); - let rp_dir_start = PathBuf::from(b"rk").join(&PathBuf::from(&hex)); + let rp_dir_start = PathBuf::from(&hex); let mut num_rks = 0; let mut maybe_entry = syscall!(self.trussed.read_dir_first_alphabetical( Location::Internal, rk_dir.clone(), - Some(rk_dir) + Some(rp_dir_start.clone()) )) .entry; @@ -338,13 +338,13 @@ where let mut first_rk = None; - while let Some(entry) = maybe_entry { - if !entry.path().as_str().as_bytes().starts_with(&hex) { + while let Some(entry) = dbg!(maybe_entry) { + if !entry.file_name().as_str().as_bytes().starts_with(&hex) { // We got past all credentials for the relevant RP break; } - if entry.path() == &*rp_dir_start { + if entry.file_name() == &*rp_dir_start { // This is the case where we debug_assert!(entry.metadata().is_dir()); legacy_detected = true; @@ -360,7 +360,8 @@ where } if legacy_detected { - let (legacy_rks, first_legacy_rk) = self.count_legacy_rp_rks(rp_dir_start); + let (legacy_rks, first_legacy_rk) = + dbg!(self.count_legacy_rp_rks(rk_dir.join(&rp_dir_start))); num_rks += legacy_rks; first_rk = first_rk.or(first_legacy_rk); } @@ -378,7 +379,11 @@ where // let rp_id_hash = response.rp_id_hash.as_ref().unwrap().clone(); self.state.runtime.cached_rk = Some(CredentialManagementEnumerateCredentials { remaining: num_rks - 1, - rp_dir: PathBuf::from(&hex), + rp_dir: if only_legacy { + rk_dir.join(&PathBuf::from(&hex)) + } else { + PathBuf::from(&hex) + }, prev_filename: Some(first_rk.file_name().into()), iterating_legacy: only_legacy, }); @@ -445,6 +450,7 @@ where .cached_rk .take() .ok_or(Error::NotAllowed)?; + dbg!(&cache); if cache.iterating_legacy { return self.next_legacy_credential(cache); @@ -479,7 +485,7 @@ where if entry.metadata().is_dir() { return self.next_legacy_credential(CredentialManagementEnumerateCredentials { remaining, - rp_dir, + rp_dir: entry.path().into(), prev_filename: None, iterating_legacy: true, }); @@ -492,7 +498,7 @@ where self.state.runtime.cached_rk = Some(CredentialManagementEnumerateCredentials { remaining: remaining - 1, rp_dir, - prev_filename: Some(entry.path().into()), + prev_filename: Some(entry.file_name().into()), iterating_legacy: true, }); } diff --git a/src/dispatch.rs b/src/dispatch.rs index 50849e6..b4dc09a 100644 --- a/src/dispatch.rs +++ b/src/dispatch.rs @@ -188,14 +188,15 @@ where debug!("2a SP: {:X}", msp()); use ctap2::Authenticator; authenticator - .call_ctap2(&ctap_request) + .call_ctap2(dbg!(&ctap_request)) .map(|response| { info!("Sending CTAP2 response {:?}", response_operation(&response)); trace!("CTAP2 response: {:?}", response); - response + dbg!(response) }) .map_err(|error| { info!("CTAP2 error: {:?}", error); + dbg!(error); error as u8 }) } diff --git a/src/lib.rs b/src/lib.rs index b252c02..d2ae603 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,7 +8,7 @@ //! With feature `dispatch` activated, it also implements the `App` traits //! of [`apdu_dispatch`] and [`ctaphid_dispatch`]. -#![cfg_attr(not(test), no_std)] +// #![cfg_attr(not(test), no_std)] // #![warn(missing_docs)] #[macro_use] From 2f25be72b3bce93c3d9398c7d02df9c7900057ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 7 Feb 2024 14:54:52 +0100 Subject: [PATCH 10/14] Fix patch --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8da17a0..7349887 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,8 +62,8 @@ cbor-smol = { git = "https://github.com/sosthene-nitrokey/cbor-smol.git", rev = ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "7d4ad69e64ad308944c012aef5b9cfd7654d9be8" } 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" } -trussed = { path = "../trussed" } -littlefs2 = { path = "../littlefs2" } +trussed = { git = "https://github.com/nitrokey/trussed.git", rev = "907805eb84c8eb34ed99c922b24433602440ff9f" } +littlefs2 = { git = "https://github.com/sosthene-nitrokey/littlefs2.git", rev = "99b1a9832c46c9097e73ca1fa43e119026e2068f" } trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging", rev = "3b9594d93f89a5e760fe78fa5a96f125dfdcd470" } serde-indexed = { git = "https://github.com/sosthene-nitrokey/serde-indexed.git", rev = "5005d23cb4ee8622e62188ea0f9466146f851f0d" } trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.1" } From 145dcd198a2fe6bd56f4d43b3b61f9782af62468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Thu, 8 Feb 2024 16:02:28 +0100 Subject: [PATCH 11/14] Fix credential counting and iteration --- src/ctap2/credential_management.rs | 20 ++++++++------------ src/lib.rs | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index 8bc1e3b..d687fd9 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -109,6 +109,8 @@ where let mut previous_credential = first_rp.file_name().into(); loop { + // We need to restart the iteration each time because count_legacy_rp_rk has + // its own iteration loop syscall!(self.trussed.read_dir_first( Location::Internal, dir.clone(), @@ -129,18 +131,13 @@ where previous_credential = PathBuf::from(rp.file_name()); info!("counting.."); - if first_rp.metadata().is_dir() { - let (rk_count, _) = self.count_legacy_rp_rks(first_rp.path().into()); + if rp.metadata().is_dir() { + let (rk_count, _) = self.count_legacy_rp_rks(rp.path().into()); num_rks += rk_count; } else { - debug_assert!(first_rp.metadata().is_file()); + debug_assert!(rp.metadata().is_file()); num_rks += 1; } - - // TODO: FIX - let (this_rp_rk_count, _) = self.count_legacy_rp_rks(PathBuf::from(rp.path())); - info!("{:?}", this_rp_rk_count); - num_rks += this_rp_rk_count; } } } @@ -338,7 +335,7 @@ where let mut first_rk = None; - while let Some(entry) = dbg!(maybe_entry) { + while let Some(entry) = maybe_entry { if !entry.file_name().as_str().as_bytes().starts_with(&hex) { // We got past all credentials for the relevant RP break; @@ -361,7 +358,7 @@ where if legacy_detected { let (legacy_rks, first_legacy_rk) = - dbg!(self.count_legacy_rp_rks(rk_dir.join(&rp_dir_start))); + self.count_legacy_rp_rks(rk_dir.join(&rp_dir_start)); num_rks += legacy_rks; first_rk = first_rk.or(first_legacy_rk); } @@ -382,7 +379,7 @@ where rp_dir: if only_legacy { rk_dir.join(&PathBuf::from(&hex)) } else { - PathBuf::from(&hex) + rk_dir }, prev_filename: Some(first_rk.file_name().into()), iterating_legacy: only_legacy, @@ -450,7 +447,6 @@ where .cached_rk .take() .ok_or(Error::NotAllowed)?; - dbg!(&cache); if cache.iterating_legacy { return self.next_legacy_credential(cache); diff --git a/src/lib.rs b/src/lib.rs index d2ae603..b252c02 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,7 +8,7 @@ //! With feature `dispatch` activated, it also implements the `App` traits //! of [`apdu_dispatch`] and [`ctaphid_dispatch`]. -// #![cfg_attr(not(test), no_std)] +#![cfg_attr(not(test), no_std)] // #![warn(missing_docs)] #[macro_use] From c709619d4256fda96e05d39ee62e56ba10e27041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 9 Feb 2024 14:05:31 +0100 Subject: [PATCH 12/14] Remove dbg! calls --- src/dispatch.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/dispatch.rs b/src/dispatch.rs index b4dc09a..50849e6 100644 --- a/src/dispatch.rs +++ b/src/dispatch.rs @@ -188,15 +188,14 @@ where debug!("2a SP: {:X}", msp()); use ctap2::Authenticator; authenticator - .call_ctap2(dbg!(&ctap_request)) + .call_ctap2(&ctap_request) .map(|response| { info!("Sending CTAP2 response {:?}", response_operation(&response)); trace!("CTAP2 response: {:?}", response); - dbg!(response) + response }) .map_err(|error| { info!("CTAP2 error: {:?}", error); - dbg!(error); error as u8 }) } From 280bcde4237153f51739a7eb32ef38a24cb31383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 9 Feb 2024 16:21:28 +0100 Subject: [PATCH 13/14] Fix iteration with multiple "new" credentials --- src/ctap2/credential_management.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index d687fd9..3a79c05 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -495,7 +495,7 @@ where remaining: remaining - 1, rp_dir, prev_filename: Some(entry.file_name().into()), - iterating_legacy: true, + iterating_legacy: false, }); } From 5579fbd91fb1b487ac0fdfe57c86d55b75d47236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 12 Feb 2024 10:43:49 +0100 Subject: [PATCH 14/14] Fix credential listing with multiple RP --- src/ctap2/credential_management.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index 3a79c05..17e9c0a 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -194,7 +194,7 @@ where let mut current_id_hex = get_id_hex(¤t_rp); while let Some(entry) = syscall!(self.trussed.read_dir_next()).entry { - let id_hex = get_id_hex(¤t_rp); + let id_hex = get_id_hex(&entry); if id_hex != current_id_hex { total_rps += 1; current_rp = entry; @@ -232,11 +232,12 @@ where let dir = PathBuf::from(b"rk"); - let maybe_next_rp = - syscall!(self - .trussed - .read_dir_first(Location::Internal, dir, Some(filename))) - .entry; + let maybe_next_rp = syscall!(self.trussed.read_dir_first_alphabetical( + Location::Internal, + dir, + Some(filename) + )) + .entry; let mut response = Response::default();