From af4eef808519b10055a92cce3aa8a97960df3d2a Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Tue, 17 Nov 2020 16:57:03 +0100 Subject: [PATCH] adds credential ordering --- src/ctap/data_formats.rs | 7 +++++++ src/ctap/mod.rs | 11 +++++++++-- src/ctap/storage.rs | 25 +++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 35d5bcf..fa747bc 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -500,6 +500,7 @@ pub struct PublicKeyCredentialSource { pub other_ui: Option, pub cred_random: Option>, pub cred_protect_policy: Option, + pub creation_order: u64, } // We serialize credentials for the persistent storage using CBOR maps. Each field of a credential @@ -512,6 +513,7 @@ enum PublicKeyCredentialSourceField { OtherUi = 4, CredRandom = 5, CredProtectPolicy = 6, + CreationOrder = 7, // When a field is removed, its tag should be reserved and not used for new fields. We document // those reserved tags below. // Reserved tags: none. @@ -535,6 +537,7 @@ impl From for cbor::Value { PublicKeyCredentialSourceField::OtherUi => credential.other_ui, PublicKeyCredentialSourceField::CredRandom => credential.cred_random, PublicKeyCredentialSourceField::CredProtectPolicy => credential.cred_protect_policy, + PublicKeyCredentialSourceField::CreationOrder => credential.creation_order, } } } @@ -552,6 +555,7 @@ impl TryFrom for PublicKeyCredentialSource { PublicKeyCredentialSourceField::OtherUi => other_ui, PublicKeyCredentialSourceField::CredRandom => cred_random, PublicKeyCredentialSourceField::CredProtectPolicy => cred_protect_policy, + PublicKeyCredentialSourceField::CreationOrder => creation_order, } = extract_map(cbor_value)?; } @@ -569,6 +573,7 @@ impl TryFrom for PublicKeyCredentialSource { let cred_protect_policy = cred_protect_policy .map(CredentialProtectionPolicy::try_from) .transpose()?; + let creation_order = creation_order.map(extract_unsigned).unwrap_or(Ok(0))?; // We don't return whether there were unknown fields in the CBOR value. This means that // deserialization is not injective. In particular deserialization is only an inverse of // serialization at a given version of OpenSK. This is not a problem because: @@ -588,6 +593,7 @@ impl TryFrom for PublicKeyCredentialSource { other_ui, cred_random, cred_protect_policy, + creation_order, }) } } @@ -1357,6 +1363,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; assert_eq!( diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index f037655..331ccc1 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -155,6 +155,7 @@ pub struct CtapState<'a, R: Rng256, CheckUserPresence: Fn(ChannelID) -> Result<( accepts_reset: bool, #[cfg(feature = "with_ctap1")] pub u2f_up_state: U2fUserPresenceState, + // Sorted by ascending order of creation, so the last element is the most recent one. next_credentials: Vec, next_assertion_input: Option, } @@ -302,6 +303,7 @@ where other_ui: None, cred_random, cred_protect_policy: None, + creation_order: 0, })) } @@ -424,7 +426,6 @@ where } else { None }; - // TODO(kaczmarczyck) unsolicited output for default credProtect level let has_extension_output = use_hmac_extension || cred_protect_policy.is_some(); let rp_id = rp.rp_id; @@ -501,6 +502,7 @@ where .map(|s| truncate_to_char_boundary(&s, 64).to_string()), cred_random: cred_random.map(|c| c.to_vec()), cred_protect_policy, + creation_order: self.persistent_store.new_creation_order()?, }; self.persistent_store.store_credential(credential_source)?; random_id @@ -717,8 +719,9 @@ where let credential = if let Some((credential, remaining_credentials)) = credentials.split_first() { - // TODO(kaczmarczyck) correct credential order self.next_credentials = remaining_credentials.to_vec(); + self.next_credentials + .sort_unstable_by_key(|c| c.creation_order); credential } else { decrypted_credential @@ -1091,6 +1094,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; assert!(ctap_state .persistent_store @@ -1457,6 +1461,7 @@ mod test { cred_protect_policy: Some( CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, ), + creation_order: 0, }; assert!(ctap_state .persistent_store @@ -1507,6 +1512,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), + creation_order: 0, }; assert!(ctap_state .persistent_store @@ -1550,6 +1556,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; assert!(ctap_state .persistent_store diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 127dc23..6e4506a 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -337,6 +337,26 @@ impl PersistentStore { .count()) } + pub fn new_creation_order(&self) -> Result { + Ok(self + .store + .find_all(&Key::Credential { + rp_id: None, + credential_id: None, + user_handle: None, + }) + .filter_map(|(_, entry)| { + debug_assert_eq!(entry.tag, TAG_CREDENTIAL); + let credential = deserialize_credential(entry.data); + debug_assert!(credential.is_some()); + credential + }) + .map(|c| c.creation_order) + .max() + .unwrap_or(0) + + 1) + } + pub fn global_signature_counter(&self) -> Result { Ok(self .store @@ -690,6 +710,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, } } @@ -853,6 +874,7 @@ mod test { cred_protect_policy: Some( CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, ), + creation_order: 0, }; assert!(persistent_store.store_credential(credential).is_ok()); @@ -894,6 +916,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; assert_eq!(found_credential, Some(expected_credential)); } @@ -913,6 +936,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), + creation_order: 0, }; assert!(persistent_store.store_credential(credential).is_ok()); @@ -1090,6 +1114,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; let serialized = serialize_credential(credential.clone()).unwrap(); let reconstructed = deserialize_credential(&serialized).unwrap();