Skip to content

Commit

Permalink
fixup! Reduce ID length for new credentials
Browse files Browse the repository at this point in the history
  • Loading branch information
robin-nitrokey committed Jul 13, 2023
1 parent b61d477 commit a5aa9ef
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 9 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ log-error = []
[dev-dependencies]
# quickcheck = "1"
rand = "0.8.4"
trussed = { version = "0.1", features = ["virt"] }

[package.metadata.docs.rs]
features = ["dispatch"]
Expand Down
94 changes: 86 additions & 8 deletions src/credential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,13 @@ impl FullCredential {
StrippedCredential::from(self).id(trussed, key_encryption_key, &rp_id_hash)
} else {
let stripped_credential = self.strip();
CredentialId::new(trussed, &stripped_credential, key_encryption_key, &rp_id_hash, &self.nonce)
CredentialId::new(
trussed,
&stripped_credential,
key_encryption_key,
&rp_id_hash,
&self.nonce,
)
}
}

Expand All @@ -403,6 +409,7 @@ impl FullCredential {

// This method is only kept for compatibility. To strip new credentials, use
// `StrippedCredential`.
#[must_use]
fn strip(&self) -> Self {
info_now!(":: stripping ID");
let mut stripped = self.clone();
Expand Down Expand Up @@ -459,13 +466,7 @@ impl StrippedCredential {
key_encryption_key: KeyId,
rp_id_hash: &Bytes<32>,
) -> Result<CredentialId> {
CredentialId::new(
trussed,
self,
key_encryption_key,
rp_id_hash,
&self.nonce,
)
CredentialId::new(trussed, self, key_encryption_key, rp_id_hash, &self.nonce)
}
}

Expand All @@ -487,6 +488,10 @@ impl From<&FullCredential> for StrippedCredential {
#[cfg(test)]
mod test {
use super::*;
use trussed::{
client::{Chacha8Poly1305, Sha256},
types::Location,
};

fn credential_data() -> CredentialData {
use ctap_types::webauthn::{PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity};
Expand Down Expand Up @@ -610,6 +615,79 @@ mod test {
assert_eq!(credential_data, deserialized);
}

#[test]
fn credential_ids() {
trussed::virt::with_ram_client("fido", |mut client| {
let kek = syscall!(client.generate_chacha8poly1305_key(Location::Internal)).key;
let mut nonce = Bytes::new();
nonce.extend_from_slice(&[0; 12]).unwrap();
let data = credential_data();
let mut full_credential = FullCredential {
ctap: CtapVersion::Fido21Pre,
data,
nonce,
};
let rp_id_hash = syscall!(client.hash_sha256(full_credential.rp.id.as_ref()))
.hash
.to_bytes()
.unwrap();

// Case 1: credential with use_short_id = Some(true) uses new (short) format
full_credential.data.use_short_id = Some(true);
let stripped_credential = StrippedCredential::from(&full_credential);
let full_id = full_credential
.id(&mut client, kek, Some(&rp_id_hash))
.unwrap();
let short_id = stripped_credential
.id(&mut client, kek, &rp_id_hash)
.unwrap();
assert_eq!(full_id.0, short_id.0);

// Case 2: credential with use_short_id = None uses old (long) format
full_credential.data.use_short_id = None;
let stripped_credential = full_credential.strip();
let full_id = full_credential
.id(&mut client, kek, Some(&rp_id_hash))
.unwrap();
let long_id = CredentialId::new(
&mut client,
&stripped_credential,
kek,
&rp_id_hash,
&full_credential.nonce,
)
.unwrap();
assert_eq!(full_id.0, long_id.0);

assert!(short_id.0.len() < long_id.0.len());
});
}

#[test]
fn max_credential_id() {
let rp_id: String<256> = core::iter::repeat('?').take(256).collect();
let key = Bytes::from_slice(&[u8::MAX; 128]).unwrap();
let credential = StrippedCredential {
ctap: CtapVersion::Fido21Pre,
creation_time: u32::MAX,
use_counter: true,
algorithm: i32::MAX,
key: Key::WrappedKey(key),
nonce: Bytes::from_slice(&[u8::MAX; 12]).unwrap(),
hmac_secret: Some(true),
cred_protect: Some(CredentialProtectionPolicy::Required),
};
trussed::virt::with_ram_client("fido", |mut client| {
let kek = syscall!(client.generate_chacha8poly1305_key(Location::Internal)).key;
let rp_id_hash = syscall!(client.hash_sha256(rp_id.as_ref()))
.hash
.to_bytes()
.unwrap();
let id = credential.id(&mut client, kek, &rp_id_hash).unwrap();
assert_eq!(id.0.len(), 204);
});
}

// use quickcheck::TestResult;
// quickcheck::quickcheck! {
// fn prop(
Expand Down
3 changes: 2 additions & 1 deletion src/ctap2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,8 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
);

// note that this does the "stripping" of OptionalUI etc.
let credential_id = StrippedCredential::from(&credential).id(&mut self.trussed, kek, &rp_id_hash)?;
let credential_id =
StrippedCredential::from(&credential).id(&mut self.trussed, kek, &rp_id_hash)?;

if rk_requested {
// serialization with all metadata
Expand Down

0 comments on commit a5aa9ef

Please sign in to comment.