diff --git a/src/ctap/credential_id.rs b/src/ctap/credential_id.rs index f9845cc..52c028d 100644 --- a/src/ctap/credential_id.rs +++ b/src/ctap/credential_id.rs @@ -206,7 +206,6 @@ pub fn decrypt_credential_id( env: &mut impl Env, credential_id: Vec, rp_id_hash: &[u8], - check_cred_protect: bool, ) -> Result, Ctap2StatusCode> { if credential_id.len() < MIN_CREDENTIAL_ID_SIZE { return Ok(None); @@ -240,9 +239,7 @@ pub fn decrypt_credential_id( return Ok(None); }; - let is_protected = credential_source.cred_protect_policy - == Some(CredentialProtectionPolicy::UserVerificationRequired); - if rp_id_hash != credential_source.rp_id_hash || (check_cred_protect && is_protected) { + if rp_id_hash != credential_source.rp_id_hash { return Ok(None); } @@ -279,7 +276,7 @@ mod test { let rp_id_hash = [0x55; 32]; let encrypted_id = encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None).unwrap(); - let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, false) + let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash) .unwrap() .unwrap(); @@ -313,7 +310,7 @@ mod test { encrypted_id.extend(&id_hmac); assert_eq!( - decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, false), + decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash), Ok(None) ); } @@ -329,7 +326,7 @@ mod test { let mut modified_id = encrypted_id.clone(); modified_id[i] ^= 0x01; assert_eq!( - decrypt_credential_id(&mut env, modified_id, &rp_id_hash, false), + decrypt_credential_id(&mut env, modified_id, &rp_id_hash), Ok(None) ); } @@ -356,12 +353,7 @@ mod test { for length in (1..CBOR_CREDENTIAL_ID_SIZE).step_by(16) { assert_eq!( - decrypt_credential_id( - &mut env, - encrypted_id[..length].to_vec(), - &rp_id_hash, - false - ), + decrypt_credential_id(&mut env, encrypted_id[..length].to_vec(), &rp_id_hash), Ok(None) ); } @@ -408,13 +400,13 @@ mod test { let rp_id_hash = [0x55; 32]; let encrypted_id = legacy_encrypt_to_credential_id(&mut env, ecdsa_key, &rp_id_hash).unwrap(); - // When checking credProtect for legacy credentials the check will always pass because we didn't persist credProtect - // policy info in it. - let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true) + let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash) .unwrap() .unwrap(); assert_eq!(private_key, decrypted_source.private_key); + // Legacy credentials didn't persist credProtectPolicy info, so it should be treated as None. + assert!(decrypted_source.cred_protect_policy.is_none()); } #[test] @@ -429,7 +421,7 @@ mod test { } #[test] - fn test_check_cred_protect_fail() { + fn test_cred_protect_persisted() { let mut env = TestEnv::new(); let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256); @@ -442,34 +434,13 @@ mod test { ) .unwrap(); - assert_eq!( - decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true), - Ok(None) - ); - } - - #[test] - fn test_check_cred_protect_success() { - let mut env = TestEnv::new(); - let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256); - - let rp_id_hash = [0x55; 32]; - let encrypted_id = encrypt_to_credential_id( - &mut env, - &private_key, - &rp_id_hash, - Some(CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList), - ) - .unwrap(); - - let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true) + let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash) .unwrap() .unwrap(); - assert_eq!(decrypted_source.private_key, private_key); assert_eq!( decrypted_source.cred_protect_policy, - Some(CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList) + Some(CredentialProtectionPolicy::UserVerificationRequired) ); } } diff --git a/src/ctap/credential_management.rs b/src/ctap/credential_management.rs index caf4aa8..aee9e3a 100644 --- a/src/ctap/credential_management.rs +++ b/src/ctap/credential_management.rs @@ -872,10 +872,9 @@ mod test { Ok(ResponseData::AuthenticatorCredentialManagement(None)) ); - let updated_credential = - storage::find_credential(&mut env, "example.com", &[0x1D; 32], false) - .unwrap() - .unwrap(); + let updated_credential = storage::find_credential(&mut env, "example.com", &[0x1D; 32]) + .unwrap() + .unwrap(); assert_eq!(updated_credential.user_handle, vec![0x01]); assert_eq!(&updated_credential.user_name.unwrap(), "new_name"); assert_eq!( diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 5bed1cf..0661c42 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -309,7 +309,7 @@ impl Ctap1Command { flags: Ctap1Flags, ctap_state: &mut CtapState, ) -> Result, Ctap1StatusCode> { - let credential_source = decrypt_credential_id(env, key_handle, &application, false) + let credential_source = decrypt_credential_id(env, key_handle, &application) .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; if let Some(credential_source) = credential_source { let ecdsa_key = credential_source @@ -440,7 +440,6 @@ mod test { &mut env, response[67..67 + CBOR_CREDENTIAL_ID_SIZE].to_vec(), &application, - false ) .unwrap() .is_some()); diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 1cb99c2..0f8117d 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -717,6 +717,22 @@ impl CtapState { Ok(()) } + fn check_cred_protect_for_listed_credential( + &mut self, + credential: &Option, + has_uv: bool, + ) -> bool { + if let Some(credential) = credential { + has_uv + || !matches!( + credential.cred_protect_policy, + Some(CredentialProtectionPolicy::UserVerificationRequired), + ) + } else { + false + } + } + fn process_make_credential( &mut self, env: &mut impl Env, @@ -809,9 +825,13 @@ impl CtapState { let rp_id_hash = Sha256::hash(rp_id.as_bytes()); if let Some(exclude_list) = exclude_list { for cred_desc in exclude_list { - if storage::find_credential(env, &rp_id, &cred_desc.key_id, !has_uv)?.is_some() - || decrypt_credential_id(env, cred_desc.key_id, &rp_id_hash, !has_uv)?.is_some() - { + if self.check_cred_protect_for_listed_credential( + &storage::find_credential(env, &rp_id, &cred_desc.key_id)?, + has_uv, + ) || self.check_cred_protect_for_listed_credential( + &decrypt_credential_id(env, cred_desc.key_id, &rp_id_hash)?, + has_uv, + ) { // Perform this check, so bad actors can't brute force exclude_list // without user interaction. let _ = check_user_presence(env, channel); @@ -1078,14 +1098,12 @@ impl CtapState { has_uv: bool, ) -> Result, Ctap2StatusCode> { for allowed_credential in allow_list { - let credential = - storage::find_credential(env, rp_id, &allowed_credential.key_id, !has_uv)?; - if credential.is_some() { + let credential = storage::find_credential(env, rp_id, &allowed_credential.key_id)?; + if self.check_cred_protect_for_listed_credential(&credential, has_uv) { return Ok(credential); } - let credential = - decrypt_credential_id(env, allowed_credential.key_id, rp_id_hash, !has_uv)?; - if credential.is_some() { + let credential = decrypt_credential_id(env, allowed_credential.key_id, rp_id_hash)?; + if self.check_cred_protect_for_listed_credential(&credential, has_uv) { return Ok(credential); } } @@ -1802,6 +1820,61 @@ mod test { assert!(make_credential_response.is_ok()); } + #[test] + fn test_non_resident_process_make_credential_credential_with_cred_protect() { + let mut env = TestEnv::new(); + let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); + + let test_policy = CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList; + let mut make_credential_params = + create_make_credential_parameters_with_cred_protect_policy(test_policy); + make_credential_params.options.rk = false; + let make_credential_response = + ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL); + assert!(make_credential_response.is_ok()); + let credential_id = match make_credential_response.unwrap() { + ResponseData::AuthenticatorMakeCredential(make_credential_response) => { + let auth_data = make_credential_response.auth_data; + let offset = 37 + storage::aaguid(&mut env).unwrap().len(); + assert_eq!(auth_data[offset], 0x00); + assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE); + auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec() + } + _ => panic!("Invalid response type"), + }; + let make_credential_params = + create_make_credential_parameters_with_exclude_list(&credential_id); + let make_credential_response = + ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL); + assert_eq!( + make_credential_response, + Err(Ctap2StatusCode::CTAP2_ERR_CREDENTIAL_EXCLUDED) + ); + + let test_policy = CredentialProtectionPolicy::UserVerificationRequired; + let mut make_credential_params = + create_make_credential_parameters_with_cred_protect_policy(test_policy); + make_credential_params.options.rk = false; + let make_credential_response = + ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL); + assert!(make_credential_response.is_ok()); + let credential_id = match make_credential_response.unwrap() { + ResponseData::AuthenticatorMakeCredential(make_credential_response) => { + let auth_data = make_credential_response.auth_data; + let offset = 37 + storage::aaguid(&mut env).unwrap().len(); + assert_eq!(auth_data[offset], 0x00); + assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE); + auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec() + } + _ => panic!("Invalid response type"), + }; + let make_credential_params = + create_make_credential_parameters_with_exclude_list(&credential_id); + let make_credential_response = + ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL); + assert!(make_credential_response.is_ok()); + } + #[test] fn test_process_make_credential_hmac_secret() { let mut env = TestEnv::new(); @@ -2650,6 +2723,99 @@ mod test { ); } + #[test] + fn test_non_resident_process_get_assertion_with_cred_protect() { + let mut env = TestEnv::new(); + let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); + + let test_policy = CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList; + let mut make_credential_params = + create_make_credential_parameters_with_cred_protect_policy(test_policy); + make_credential_params.options.rk = false; + let make_credential_response = + ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL); + assert!(make_credential_response.is_ok()); + let credential_id = match make_credential_response.unwrap() { + ResponseData::AuthenticatorMakeCredential(make_credential_response) => { + let auth_data = make_credential_response.auth_data; + let offset = 37 + storage::aaguid(&mut env).unwrap().len(); + assert_eq!(auth_data[offset], 0x00); + assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE); + auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec() + } + _ => panic!("Invalid response type"), + }; + let cred_desc = PublicKeyCredentialDescriptor { + key_type: PublicKeyCredentialType::PublicKey, + key_id: credential_id, + transports: None, + }; + let get_assertion_params = AuthenticatorGetAssertionParameters { + rp_id: String::from("example.com"), + client_data_hash: vec![0xCD], + allow_list: Some(vec![cred_desc]), + extensions: GetAssertionExtensions::default(), + options: GetAssertionOptions { + up: false, + uv: false, + }, + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + let get_assertion_response = ctap_state.process_get_assertion( + &mut env, + get_assertion_params, + DUMMY_CHANNEL, + CtapInstant::new(0), + ); + assert!(get_assertion_response.is_ok()); + + let test_policy = CredentialProtectionPolicy::UserVerificationRequired; + let mut make_credential_params = + create_make_credential_parameters_with_cred_protect_policy(test_policy); + make_credential_params.options.rk = false; + let make_credential_response = + ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL); + assert!(make_credential_response.is_ok()); + let credential_id = match make_credential_response.unwrap() { + ResponseData::AuthenticatorMakeCredential(make_credential_response) => { + let auth_data = make_credential_response.auth_data; + let offset = 37 + storage::aaguid(&mut env).unwrap().len(); + assert_eq!(auth_data[offset], 0x00); + assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE); + auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec() + } + _ => panic!("Invalid response type"), + }; + let cred_desc = PublicKeyCredentialDescriptor { + key_type: PublicKeyCredentialType::PublicKey, + key_id: credential_id, + transports: None, + }; + let get_assertion_params = AuthenticatorGetAssertionParameters { + rp_id: String::from("example.com"), + client_data_hash: vec![0xCD], + allow_list: Some(vec![cred_desc]), + extensions: GetAssertionExtensions::default(), + options: GetAssertionOptions { + up: false, + uv: false, + }, + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + let get_assertion_response = ctap_state.process_get_assertion( + &mut env, + get_assertion_params, + DUMMY_CHANNEL, + CtapInstant::new(0), + ); + assert_eq!( + get_assertion_response, + Err(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS), + ); + } + #[test] fn test_process_get_assertion_with_cred_blob() { let mut env = TestEnv::new(); diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index ff932d0..2fb0939 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -19,8 +19,7 @@ use crate::api::customization::Customization; use crate::api::key_store::KeyStore; use crate::ctap::client_pin::PIN_AUTH_LENGTH; use crate::ctap::data_formats::{ - extract_array, extract_text_string, CredentialProtectionPolicy, PublicKeyCredentialSource, - PublicKeyCredentialUserEntity, + extract_array, extract_text_string, PublicKeyCredentialSource, PublicKeyCredentialUserEntity, }; use crate::ctap::status_code::Ctap2StatusCode; use crate::ctap::{key_material, INITIAL_SIGNATURE_COUNTER}; @@ -114,16 +113,13 @@ pub fn find_credential( env: &mut impl Env, rp_id: &str, credential_id: &[u8], - check_cred_protect: bool, ) -> Result, Ctap2StatusCode> { let credential = match find_credential_item(env, credential_id) { Err(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS) => return Ok(None), Err(e) => return Err(e), Ok((_key, credential)) => credential, }; - let is_protected = credential.cred_protect_policy - == Some(CredentialProtectionPolicy::UserVerificationRequired); - if credential.rp_id != rp_id || (check_cred_protect && is_protected) { + if credential.rp_id != rp_id { return Ok(None); } Ok(Some(credential)) @@ -651,7 +647,9 @@ mod test { use super::*; use crate::api::attestation_store::{self, Attestation, AttestationStore}; use crate::ctap::crypto_wrapper::PrivateKey; - use crate::ctap::data_formats::{PublicKeyCredentialSource, PublicKeyCredentialType}; + use crate::ctap::data_formats::{ + CredentialProtectionPolicy, PublicKeyCredentialSource, PublicKeyCredentialType, + }; use crate::env::test::TestEnv; use rng256::Rng256; @@ -725,14 +723,14 @@ mod test { let credential_source = create_credential_source(&mut env, "example.com", vec![0x1D]); let credential_id = credential_source.credential_id.clone(); assert!(store_credential(&mut env, credential_source).is_ok()); - let stored_credential = find_credential(&mut env, "example.com", &credential_id, false) + let stored_credential = find_credential(&mut env, "example.com", &credential_id) .unwrap() .unwrap(); assert_eq!(stored_credential.user_name, None); assert_eq!(stored_credential.user_display_name, None); assert_eq!(stored_credential.user_icon, None); assert!(update_credential(&mut env, &credential_id, user.clone()).is_ok()); - let stored_credential = find_credential(&mut env, "example.com", &credential_id, false) + let stored_credential = find_credential(&mut env, "example.com", &credential_id) .unwrap() .unwrap(); assert_eq!(stored_credential.user_name, user.user_name); @@ -796,16 +794,12 @@ mod test { assert!(store_credential(&mut env, credential_source0).is_ok()); assert!(store_credential(&mut env, credential_source1).is_ok()); assert_eq!(count_credentials(&mut env).unwrap(), 1); - assert!( - find_credential(&mut env, "example.com", &credential_id0, false) - .unwrap() - .is_none() - ); - assert!( - find_credential(&mut env, "example.com", &credential_id1, false) - .unwrap() - .is_some() - ); + assert!(find_credential(&mut env, "example.com", &credential_id0) + .unwrap() + .is_none()); + assert!(find_credential(&mut env, "example.com", &credential_id1) + .unwrap() + .is_some()); reset(&mut env).unwrap(); let max_supported_resident_keys = env.customization().max_supported_resident_keys(); @@ -858,9 +852,9 @@ mod test { assert!(store_credential(&mut env, credential_source0).is_ok()); assert!(store_credential(&mut env, credential_source1).is_ok()); - let no_credential = find_credential(&mut env, "another.example.com", &id0, false).unwrap(); + let no_credential = find_credential(&mut env, "another.example.com", &id0).unwrap(); assert_eq!(no_credential, None); - let found_credential = find_credential(&mut env, "example.com", &id0, false).unwrap(); + let found_credential = find_credential(&mut env, "example.com", &id0).unwrap(); let expected_credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: id0, @@ -878,31 +872,6 @@ mod test { assert_eq!(found_credential, Some(expected_credential)); } - #[test] - fn test_find_with_cred_protect() { - let mut env = TestEnv::new(); - assert_eq!(count_credentials(&mut env).unwrap(), 0); - let private_key = PrivateKey::new_ecdsa(&mut env); - let credential = PublicKeyCredentialSource { - key_type: PublicKeyCredentialType::PublicKey, - credential_id: env.rng().gen_uniform_u8x32().to_vec(), - private_key, - rp_id: String::from("example.com"), - user_handle: vec![0x00], - user_display_name: None, - cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), - creation_order: 0, - user_name: None, - user_icon: None, - cred_blob: None, - large_blob_key: None, - }; - assert!(store_credential(&mut env, credential).is_ok()); - - let no_credential = find_credential(&mut env, "example.com", &[0x00], true).unwrap(); - assert_eq!(no_credential, None); - } - #[test] fn test_cred_random_secret() { let mut env = TestEnv::new();