Skip to content

Commit f933359

Browse files
sosthene-nitrokeyrobin-nitrokey
authored andcommitted
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.
1 parent c145a45 commit f933359

File tree

10 files changed

+583
-261
lines changed

10 files changed

+583
-261
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2626
- Support CTAP 2.1
2727
- Serialize PIN hash with `serde-bytes` ([#52][])
2828
- Reduce the space taken by credential serializaiton ([#59][])
29+
- Remove the per-relying party directory to save space ([#55][])
2930

3031
[#26]: https://github.com/solokeys/fido-authenticator/issues/26
3132
[#28]: https://github.com/solokeys/fido-authenticator/issues/28
@@ -41,6 +42,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4142
[#63]: https://github.com/Nitrokey/fido-authenticator/pull/63
4243
[#52]: https://github.com/Nitrokey/fido-authenticator/issues/52
4344
[#59]: https://github.com/Nitrokey/fido-authenticator/issues/59
45+
[#55]: https://github.com/Nitrokey/fido-authenticator/issues/55
4446

4547
## [0.1.1] - 2022-08-22
4648
- Fix bug that treated U2F payloads as APDU over APDU in NFC transport @conorpp

Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ log-warn = []
5353
log-error = []
5454

5555
[dev-dependencies]
56+
admin-app = { version = "0.1.0", features = ["migration-tests"] }
5657
aes = "0.8.4"
5758
cbc = { version = "0.1.2", features = ["alloc"] }
5859
ciborium = { version = "0.2.2" }
@@ -80,11 +81,13 @@ x509-parser = "0.16.0"
8081
features = ["dispatch"]
8182

8283
[patch.crates-io]
84+
admin-app = { git = "https://github.com/Nitrokey/admin-app.git", tag = "v0.1.0-nitrokey.18" }
8385
ctaphid-dispatch = { git = "https://github.com/trussed-dev/ctaphid-dispatch.git", rev = "57cb3317878a8593847595319aa03ef17c29ec5b" }
8486
trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "046478b7a4f6e2315acf9112d98308379c2e3eee" }
8587
trussed-chunked = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "chunked-v0.1.0" }
8688
trussed-fs-info = { git = "https://github.com/trussed-dev/trussed-staging.git", rev = "fs-info-v0.1.0" }
8789
trussed-hkdf = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "hkdf-v0.2.0" }
90+
trussed-manage = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "manage-v0.1.0" }
8891
trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", rev = "53eba84d2cd0bcacc3a7096d4b7a2490dcf6f069" }
8992
trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.5" }
9093
usbd-ctaphid = { git = "https://github.com/trussed-dev/usbd-ctaphid.git", rev = "dcff9009c3cd1ef9e5b09f8f307aca998fc9a8c8" }

src/ctap2.rs

+54-29
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
392392
.ok();
393393

394394
let mut key_store_full = self.can_fit(serialized_credential.len()) == Some(false)
395-
|| CredentialManagement::new(self).count_credentials()
395+
|| CredentialManagement::new(self).count_credentials()?
396396
>= self
397397
.config
398398
.max_resident_credential_count
@@ -914,7 +914,7 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
914914
// TODO: use custom enum of known commands
915915
match parameters.sub_command {
916916
// 0x1
917-
Subcommand::GetCredsMetadata => Ok(cred_mgmt.get_creds_metadata()),
917+
Subcommand::GetCredsMetadata => cred_mgmt.get_creds_metadata(),
918918

919919
// 0x2
920920
Subcommand::EnumerateRpsBegin => cred_mgmt.first_relying_party(),
@@ -1011,7 +1011,7 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
10111011
// If no allowList is passed, credential is None and the retrieved credentials
10121012
// are stored in state.runtime.credential_heap
10131013
let (credential, num_credentials) = self
1014-
.prepare_credentials(&rp_id_hash, &parameters.allow_list, uv_performed)
1014+
.prepare_credentials(&rp_id_hash, &parameters.allow_list, uv_performed)?
10151015
.ok_or(Error::NoCredentials)?;
10161016

10171017
info_now!("found {:?} applicable credentials", num_credentials);
@@ -1152,7 +1152,7 @@ impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
11521152
rp_id_hash: &[u8; 32],
11531153
allow_list: &Option<ctap2::get_assertion::AllowList>,
11541154
uv_performed: bool,
1155-
) -> Option<(Credential, u32)> {
1155+
) -> Result<Option<(Credential, u32)>> {
11561156
debug_now!("remaining stack size: {} bytes", msp() - 0x2000_0000);
11571157

11581158
self.state.runtime.clear_credential_cache();
@@ -1186,50 +1186,74 @@ impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
11861186
continue;
11871187
}
11881188

1189-
return Some((credential, 1));
1189+
return Ok(Some((credential, 1)));
11901190
}
11911191

11921192
// we don't recognize any credentials in the allowlist
1193-
return None;
1193+
return Ok(None);
11941194
}
11951195
}
11961196

11971197
// we are only dealing with discoverable credentials.
11981198
debug_now!("Allowlist not passed, fetching RKs");
1199+
self.prepare_cache(rp_id_hash, uv_performed)?;
11991200

1200-
let mut maybe_path =
1201-
syscall!(self
1202-
.trussed
1203-
.read_dir_first(Location::Internal, rp_rk_dir(rp_id_hash), None,))
1204-
.entry
1205-
.map(|entry| PathBuf::from(entry.path()));
1201+
let num_credentials = self.state.runtime.remaining_credentials();
1202+
let credential = self.state.runtime.pop_credential(&mut self.trussed);
1203+
Ok(credential.map(|credential| (Credential::Full(credential), num_credentials)))
1204+
}
12061205

1206+
/// Populate the cache with the RP credentials.
1207+
///
1208+
/// Returns true if legacy credentials are present and therefore prepare_cache_legacy should be called too
1209+
#[inline(never)]
1210+
fn prepare_cache(&mut self, rp_id_hash: &[u8; 32], uv_performed: bool) -> Result<()> {
12071211
use crate::state::CachedCredential;
12081212
use core::str::FromStr;
12091213

1210-
while let Some(path) = maybe_path {
1211-
let credential_data =
1212-
syscall!(self.trussed.read_file(Location::Internal, path.clone(),)).data;
1214+
let rp_rk_dir = rp_rk_dir(rp_id_hash);
1215+
let mut maybe_entry = syscall!(self.trussed.read_dir_first_alphabetical(
1216+
Location::Internal,
1217+
PathBuf::from(RK_DIR),
1218+
Some(rp_rk_dir.clone())
1219+
))
1220+
.entry;
1221+
1222+
while let Some(entry) = maybe_entry.take() {
1223+
if !entry.path().as_ref().starts_with(rp_rk_dir.as_ref()) {
1224+
// We got past all credentials for the relevant RP
1225+
break;
1226+
}
12131227

1214-
let credential = FullCredential::deserialize(&credential_data).ok()?;
1228+
if entry.path() == &*rp_rk_dir {
1229+
debug_assert!(entry.metadata().is_dir());
1230+
error!("Migration missing");
1231+
return Err(Error::Other);
1232+
}
1233+
1234+
let credential_data = syscall!(self
1235+
.trussed
1236+
.read_file(Location::Internal, entry.path().into(),))
1237+
.data;
1238+
1239+
let credential = FullCredential::deserialize(&credential_data).map_err(|_err| {
1240+
error!("Failed to deserialize credential: {_err:?}");
1241+
Error::Other
1242+
})?;
12151243
let timestamp = credential.creation_time;
12161244
let credential = Credential::Full(credential);
12171245

12181246
if self.check_credential_applicable(&credential, false, uv_performed) {
12191247
self.state.runtime.push_credential(CachedCredential {
12201248
timestamp,
1221-
path: String::from_str(path.as_str_ref_with_trailing_nul()).ok()?,
1249+
path: String::from_str(entry.path().as_str_ref_with_trailing_nul())
1250+
.map_err(|_| Error::Other)?,
12221251
});
12231252
}
12241253

1225-
maybe_path = syscall!(self.trussed.read_dir_next())
1226-
.entry
1227-
.map(|entry| PathBuf::from(entry.path()));
1254+
maybe_entry = syscall!(self.trussed.read_dir_next()).entry;
12281255
}
1229-
1230-
let num_credentials = self.state.runtime.remaining_credentials();
1231-
let credential = self.state.runtime.pop_credential(&mut self.trussed);
1232-
credential.map(|credential| (Credential::Full(credential), num_credentials))
1256+
Ok(())
12331257
}
12341258

12351259
fn decrypt_pin_hash_and_maybe_escalate(
@@ -2078,11 +2102,12 @@ fn rp_rk_dir(rp_id_hash: &[u8; 32]) -> PathBuf {
20782102
}
20792103

20802104
fn rk_path(rp_id_hash: &[u8; 32], credential_id_hash: &[u8; 32]) -> PathBuf {
2081-
let mut path = rp_rk_dir(rp_id_hash);
2082-
2083-
let mut hex = [0u8; 16];
2084-
format_hex(&credential_id_hash[..8], &mut hex);
2085-
path.push(&PathBuf::try_from(&hex).unwrap());
2105+
let mut buf = [0; 33];
2106+
buf[16] = b'.';
2107+
format_hex(&rp_id_hash[..8], &mut buf[..16]);
2108+
format_hex(&credential_id_hash[..8], &mut buf[17..]);
20862109

2110+
let mut path = PathBuf::from(RK_DIR);
2111+
path.push(&PathBuf::try_from(buf.as_slice()).unwrap());
20872112
path
20882113
}

0 commit comments

Comments
 (0)