diff --git a/README.md b/README.md index db91f82..6e68ba1 100644 --- a/README.md +++ b/README.md @@ -99,9 +99,10 @@ a few things you can personalize: 4. Depending on your available flash storage, choose an appropriate maximum number of supported residential keys and number of pages in `ctap/storage.rs`. -5. Change the default level for the credProtect extension. Resident credentials - become undiscoverable without user verification. This helps privacy, but - can make usage less comfortable for less important credentials. +5. Change the default level for the credProtect extension in `ctap/mod.rs`. + When changing the default, resident credentials become undiscoverable without + user verification. This helps privacy, but can make usage less comfortable + for credentials that need less protection. ### 3D printed enclosure diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 5586252..a41efa6 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -440,7 +440,7 @@ impl TryFrom<&cbor::Value> for SignatureAlgorithm { } } -#[derive(Clone, PartialEq, PartialOrd)] +#[derive(Clone, Copy, PartialEq, PartialOrd)] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] pub enum CredentialProtectionPolicy { UserVerificationOptional = 0x01, @@ -1079,6 +1079,22 @@ mod test { assert_eq!(unknown_algorithm, Ok(expected_unknown_algorithm)); } + #[test] + fn test_cred_protection_policy_order() { + assert!( + CredentialProtectionPolicy::UserVerificationOptional + < CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList + ); + assert!( + CredentialProtectionPolicy::UserVerificationOptional + < CredentialProtectionPolicy::UserVerificationRequired + ); + assert!( + CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList + < CredentialProtectionPolicy::UserVerificationRequired + ); + } + #[test] fn test_from_into_cred_protection_policy() { let cbor_policy = cbor_int!(CredentialProtectionPolicy::UserVerificationOptional as i64); diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index aaf6f5d..a8081bc 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -438,12 +438,9 @@ where let mut cred_protect = extensions .make_credential_cred_protect_policy() .transpose()?; - if cred_protect - .as_ref() - .unwrap_or(&CredentialProtectionPolicy::UserVerificationOptional) - > DEFAULT_CRED_PROTECT - .as_ref() - .unwrap_or(&CredentialProtectionPolicy::UserVerificationOptional) + if cred_protect.unwrap_or(CredentialProtectionPolicy::UserVerificationOptional) + < DEFAULT_CRED_PROTECT + .unwrap_or(CredentialProtectionPolicy::UserVerificationOptional) { cred_protect = DEFAULT_CRED_PROTECT; } @@ -704,15 +701,7 @@ where found_credentials } else { // TODO(kaczmarczyck) use GetNextAssertion - let found_credentials = self.persistent_store.filter_credential(&rp_id); - if has_uv { - found_credentials - } else { - found_credentials - .into_iter() - .filter(|cred| cred.is_discoverable()) - .collect() - } + self.persistent_store.filter_credential(&rp_id, !has_uv) }; let credential = if let Some(credential) = credentials.first() { @@ -1213,6 +1202,31 @@ mod test { } } + fn create_make_credential_parameters_with_exclude_list( + excluded_credential_id: &[u8], + ) -> AuthenticatorMakeCredentialParameters { + let excluded_credential_descriptor = PublicKeyCredentialDescriptor { + key_type: PublicKeyCredentialType::PublicKey, + key_id: excluded_credential_id.to_vec(), + transports: None, + }; + let exclude_list = Some(vec![excluded_credential_descriptor]); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.exclude_list = exclude_list; + make_credential_params + } + + fn create_make_credential_parameters_with_cred_protect_policy( + policy: CredentialProtectionPolicy, + ) -> AuthenticatorMakeCredentialParameters { + let mut extension_map = BTreeMap::new(); + extension_map.insert("credProtect".to_string(), cbor_int!(policy as i64)); + let extensions = Some(Extensions::new(extension_map)); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.extensions = extensions; + make_credential_params + } + #[test] fn test_residential_process_make_credential() { let mut rng = ThreadRng256 {}; @@ -1311,9 +1325,11 @@ mod test { let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); let excluded_credential_id = vec![0x01, 0x23, 0x45, 0x67]; + let make_credential_params = + create_make_credential_parameters_with_exclude_list(&excluded_credential_id); let excluded_credential_source = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, - credential_id: excluded_credential_id.clone(), + credential_id: excluded_credential_id, private_key: excluded_private_key, rp_id: String::from("example.com"), user_handle: vec![], @@ -1326,17 +1342,8 @@ mod test { .store_credential(excluded_credential_source) .is_ok()); - let excluded_credential_descriptor = PublicKeyCredentialDescriptor { - key_type: PublicKeyCredentialType::PublicKey, - key_id: excluded_credential_id, - transports: None, - }; - let exclude_list = Some(vec![excluded_credential_descriptor]); - let mut make_credential_params = create_minimal_make_credential_parameters(); - make_credential_params.exclude_list = exclude_list; let make_credential_response = ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); - assert_eq!( make_credential_response, Err(Ctap2StatusCode::CTAP2_ERR_CREDENTIAL_EXCLUDED) @@ -1346,53 +1353,50 @@ mod test { #[test] fn test_process_make_credential_credential_with_cred_protect() { let mut rng = ThreadRng256 {}; - let excluded_private_key = crypto::ecdsa::SecKey::gensk(&mut rng); let user_immediately_present = |_| Ok(()); let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); - let excluded_credential_id = vec![0x01, 0x23, 0x45, 0x67]; - let mut excluded_credential_source = PublicKeyCredentialSource { - key_type: PublicKeyCredentialType::PublicKey, - credential_id: excluded_credential_id.clone(), - private_key: excluded_private_key, - rp_id: String::from("example.com"), - user_handle: vec![], - other_ui: None, - cred_random: None, - cred_protect_policy: Some( - CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, - ), - }; - assert!(ctap_state - .persistent_store - .store_credential(excluded_credential_source.clone()) - .is_ok()); - - let excluded_credential_descriptor = PublicKeyCredentialDescriptor { - key_type: PublicKeyCredentialType::PublicKey, - key_id: excluded_credential_id, - transports: None, - }; - let exclude_list = Some(vec![excluded_credential_descriptor.clone()]); - let mut make_credential_params = create_minimal_make_credential_parameters(); - make_credential_params.exclude_list = exclude_list; + let test_policy = CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList; + let make_credential_params = + create_make_credential_parameters_with_cred_protect_policy(test_policy); let make_credential_response = ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + assert!(make_credential_response.is_ok()); + let stored_credential = ctap_state + .persistent_store + .filter_credential("example.com", false) + .pop() + .unwrap(); + let credential_id = stored_credential.credential_id; + assert_eq!(stored_credential.cred_protect_policy, Some(test_policy)); + + let make_credential_params = + create_make_credential_parameters_with_exclude_list(&credential_id); + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); assert_eq!( make_credential_response, Err(Ctap2StatusCode::CTAP2_ERR_CREDENTIAL_EXCLUDED) ); - excluded_credential_source.cred_protect_policy = - Some(CredentialProtectionPolicy::UserVerificationRequired); - assert!(ctap_state + let test_policy = CredentialProtectionPolicy::UserVerificationRequired; + let make_credential_params = + create_make_credential_parameters_with_cred_protect_policy(test_policy); + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + assert!(make_credential_response.is_ok()); + + let stored_credential = ctap_state .persistent_store - .store_credential(excluded_credential_source) - .is_ok()); - let exclude_list = Some(vec![excluded_credential_descriptor]); - let mut make_credential_params = create_minimal_make_credential_parameters(); - make_credential_params.exclude_list = exclude_list; + .filter_credential("example.com", false) + .pop() + .unwrap(); + let credential_id = stored_credential.credential_id; + assert_eq!(stored_credential.cred_protect_policy, Some(test_policy)); + + let make_credential_params = + create_make_credential_parameters_with_exclude_list(&credential_id); let make_credential_response = ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); assert!(make_credential_response.is_ok()); diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index ff5f98b..1dd0213 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -269,7 +269,11 @@ impl PersistentStore { Ok(()) } - pub fn filter_credential(&self, rp_id: &str) -> Vec { + pub fn filter_credential( + &self, + rp_id: &str, + check_cred_protect: bool, + ) -> Vec { self.store .find_all(&Key::Credential { rp_id: Some(rp_id.into()), @@ -282,6 +286,7 @@ impl PersistentStore { debug_assert!(credential.is_some()); credential }) + .filter(|cred| !check_cred_protect || cred.is_discoverable()) .collect() } @@ -549,7 +554,7 @@ mod test { .is_ok()); assert_eq!(persistent_store.count_credentials(), 1); assert_eq!( - &persistent_store.filter_credential("example.com"), + &persistent_store.filter_credential("example.com", false), &[expected_credential] ); @@ -597,7 +602,7 @@ mod test { .store_credential(credential_source2) .is_ok()); - let filtered_credentials = persistent_store.filter_credential("example.com"); + let filtered_credentials = persistent_store.filter_credential("example.com", false); assert_eq!(filtered_credentials.len(), 2); assert!( (filtered_credentials[0].credential_id == id0 @@ -607,6 +612,30 @@ mod test { ); } + #[test] + fn test_filter_with_cred_protect() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + assert_eq!(persistent_store.count_credentials(), 0); + let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); + let credential = PublicKeyCredentialSource { + key_type: PublicKeyCredentialType::PublicKey, + credential_id: rng.gen_uniform_u8x32().to_vec(), + private_key, + rp_id: String::from("example.com"), + user_handle: vec![0x00], + other_ui: None, + cred_random: None, + cred_protect_policy: Some( + CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, + ), + }; + assert!(persistent_store.store_credential(credential).is_ok()); + + let no_credential = persistent_store.filter_credential("example.com", true); + assert_eq!(no_credential, vec![]); + } + #[test] fn test_find() { let mut rng = ThreadRng256 {};