From 43d77fd10662d9578588f42864d4c3d024884466 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Wed, 13 May 2020 16:36:36 +0200 Subject: [PATCH] implements the credProtect extension --- src/ctap/data_formats.rs | 93 +++++++++++++++- src/ctap/mod.rs | 230 +++++++++++++++++++++++++++++++++++---- src/ctap/storage.rs | 42 ++++++- 3 files changed, 335 insertions(+), 30 deletions(-) diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 8e6c7d9..538090d 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -161,7 +161,7 @@ impl From for cbor::Value { } // https://www.w3.org/TR/webauthn/#enumdef-authenticatortransport -#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] +#[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))] pub enum AuthenticatorTransport { Usb, Nfc, @@ -197,7 +197,7 @@ impl TryFrom<&cbor::Value> for AuthenticatorTransport { } // https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialdescriptor -#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] +#[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))] pub struct PublicKeyCredentialDescriptor { pub key_type: PublicKeyCredentialType, pub key_id: Vec, @@ -292,6 +292,14 @@ impl Extensions { .get("hmac-secret") .map(GetAssertionHmacSecretInput::try_from) } + + pub fn make_credential_cred_protect_policy( + &self, + ) -> Option> { + self.0 + .get("credProtect") + .map(CredentialProtectionPolicy::try_from) + } } #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] @@ -432,6 +440,27 @@ impl TryFrom<&cbor::Value> for SignatureAlgorithm { } } +#[derive(Clone, PartialEq)] +#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] +pub enum CredentialProtectionPolicy { + UserVerificationOptional = 0x01, + UserVerificationOptionalWithCredentialIdList = 0x02, + UserVerificationRequired = 0x03, +} + +impl TryFrom<&cbor::Value> for CredentialProtectionPolicy { + type Error = Ctap2StatusCode; + + fn try_from(cbor_value: &cbor::Value) -> Result { + match read_integer(cbor_value)? { + 0x01 => Ok(CredentialProtectionPolicy::UserVerificationOptional), + 0x02 => Ok(CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList), + 0x03 => Ok(CredentialProtectionPolicy::UserVerificationRequired), + _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE), + } + } +} + // https://www.w3.org/TR/webauthn/#public-key-credential-source #[derive(Clone)] #[cfg_attr(test, derive(PartialEq))] @@ -445,6 +474,7 @@ pub struct PublicKeyCredentialSource { pub user_handle: Vec, // not optional, but nullable pub other_ui: Option, pub cred_random: Option>, + pub cred_protect_policy: Option, } impl From for cbor::Value { @@ -459,6 +489,10 @@ impl From for cbor::Value { None => cbor_null!(), Some(cred_random) => cbor_bytes!(cred_random), }; + let cred_protect_policy = match credential.cred_protect_policy { + None => cbor_null!(), + Some(cred_protect_policy) => cbor_int!(cred_protect_policy as i64), + }; cbor_array! { credential.credential_id, private_key, @@ -466,6 +500,7 @@ impl From for cbor::Value { credential.user_handle, other_ui, cred_random, + cred_protect_policy, } } } @@ -477,7 +512,7 @@ impl TryFrom for PublicKeyCredentialSource { use cbor::{SimpleValue, Value}; let fields = read_array(&cbor_value)?; - if fields.len() != 6 { + if fields.len() != 7 { return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR); } let credential_id = read_byte_string(&fields[0])?; @@ -497,6 +532,10 @@ impl TryFrom for PublicKeyCredentialSource { Value::Simple(SimpleValue::NullValue) => None, cbor_value => Some(read_byte_string(cbor_value)?), }; + let cred_protect_policy = match &fields[6] { + Value::Simple(SimpleValue::NullValue) => None, + cbor_value => Some(CredentialProtectionPolicy::try_from(cbor_value)?), + }; Ok(PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, @@ -505,10 +544,20 @@ impl TryFrom for PublicKeyCredentialSource { user_handle, other_ui, cred_random, + cred_protect_policy, }) } } +impl PublicKeyCredentialSource { + // Relying parties do not need to provide the credential ID in an allow_list if true. + pub fn is_discoverable(&self) -> bool { + self.cred_protect_policy.is_none() + || self.cred_protect_policy + == Some(CredentialProtectionPolicy::UserVerificationOptional) + } +} + // TODO(kaczmarczyck) we could decide to split this data type up // It depends on the algorithm though, I think. // So before creating a mess, this is my workaround. @@ -1030,6 +1079,21 @@ mod test { assert_eq!(unknown_algorithm, Ok(expected_unknown_algorithm)); } + #[test] + fn test_from_into_cred_protection_policy() { + let cbor_policy = cbor_int!(CredentialProtectionPolicy::UserVerificationOptional as i64); + let policy = CredentialProtectionPolicy::try_from(&cbor_policy); + let expected_policy = CredentialProtectionPolicy::UserVerificationOptional; + assert_eq!(policy, Ok(expected_policy)); + let created_cbor: cbor::Value = cbor_int!(policy.unwrap() as i64); + assert_eq!(created_cbor, cbor_policy); + + let cbor_policy_error = cbor_int!(-1); + let policy_error = CredentialProtectionPolicy::try_from(&cbor_policy_error); + let expected_error = Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE); + assert_eq!(policy_error, expected_error); + } + #[test] fn test_from_into_authenticator_transport() { let cbor_authenticator_transport = cbor_text!("usb"); @@ -1140,6 +1204,18 @@ mod test { assert_eq!(get_assertion_input, Some(Ok(expected_input))); } + #[test] + fn test_cred_protect_extension() { + let cbor_extensions = cbor_map! { + "credProtect" => CredentialProtectionPolicy::UserVerificationRequired as i64, + }; + let extensions = Extensions::try_from(&cbor_extensions).unwrap(); + assert_eq!( + extensions.make_credential_cred_protect_policy(), + Some(Ok(CredentialProtectionPolicy::UserVerificationRequired)) + ); + } + #[test] fn test_from_make_credential_options() { let cbor_make_options = cbor_map! { @@ -1218,6 +1294,7 @@ mod test { user_handle: b"foo".to_vec(), other_ui: None, cred_random: None, + cred_protect_policy: None, }; assert_eq!( @@ -1240,6 +1317,16 @@ mod test { ..credential }; + assert_eq!( + PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + Ok(credential.clone()) + ); + + let credential = PublicKeyCredentialSource { + cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationOptional), + ..credential + }; + assert_eq!( PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), Ok(credential) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index a39a983..b61c650 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -335,6 +335,7 @@ where user_handle: vec![], other_ui: None, cred_random: None, + cred_protect_policy: None, }) } @@ -428,30 +429,40 @@ where return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); } - let use_hmac_extension = - extensions.map_or(Ok(false), |e| e.has_make_credential_hmac_secret())?; - if use_hmac_extension && !options.rk { - // The extension is actually supported, but we need resident keys. - return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION); - } + let (use_hmac_extension, cred_protect_policy) = if let Some(extensions) = extensions { + ( + extensions.has_make_credential_hmac_secret()?, + extensions + .make_credential_cred_protect_policy() + .transpose()?, + ) + } else { + (false, None) + }; + let cred_random = if use_hmac_extension { + if !options.rk { + // The extension is actually supported, but we need resident keys. + return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION); + } Some(self.rng.gen_uniform_u8x32().to_vec()) } else { None }; - let ed_flag = if use_hmac_extension { ED_FLAG } else { 0 }; + // 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; if let Some(exclude_list) = exclude_list { for cred_desc in exclude_list { if self .persistent_store - .find_credential(&rp_id, &cred_desc.key_id) + .find_credential(&rp_id, &cred_desc.key_id, pin_uv_auth_param.is_none()) .is_some() { // Perform this check, so bad actors can't brute force exclude_list - // without user interaction. Discard the user presence check's outcome. - let _ = (self.check_user_presence)(cid); + // without user interaction. + (self.check_user_presence)(cid)?; return Err(Ctap2StatusCode::CTAP2_ERR_CREDENTIAL_EXCLUDED); } } @@ -459,6 +470,7 @@ where // MakeCredential always requires user presence. // User verification depends on the PIN auth inputs, which are checked here. + let ed_flag = if has_extension_output { ED_FLAG } else { 0 }; let flags = match pin_uv_auth_param { Some(pin_auth) => { if self.persistent_store.pin_hash().is_none() { @@ -501,6 +513,7 @@ where .user_display_name .map(|s| truncate_to_char_boundary(&s, 64).to_string()), cred_random, + cred_protect_policy: cred_protect_policy.clone(), }; self.persistent_store.store_credential(credential_source)?; random_id @@ -521,9 +534,11 @@ where None => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_CANNOT_WRITE_CBOR), }; auth_data.extend(cose_key); - if use_hmac_extension { - let extensions = cbor_map! { - "hmac-secret" => true, + if has_extension_output { + let hmac_secret_output = if use_hmac_extension { Some(true) } else { None }; + let extensions = cbor_map_options! { + "hmac-secret" => hmac_secret_output, + "credProtect" => cred_protect_policy.map(|policy| policy as i64), }; if !cbor::write(extensions, &mut auth_data) { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_CANNOT_WRITE_CBOR); @@ -624,8 +639,9 @@ where return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION); } - // The user verification bit depends on the existance of PIN auth, whereas - // user presence is requested as an option. + // The user verification bit depends on the existance of PIN auth, since we do + // not support internal UV. User presence is requested as an option. + let has_uv = pin_uv_auth_param.is_some(); let mut flags = match pin_uv_auth_param { Some(pin_auth) => { if self.persistent_store.pin_hash().is_none() { @@ -657,11 +673,14 @@ where let credentials = if let Some(allow_list) = allow_list { let mut found_credentials = vec![]; for allowed_credential in allow_list { - match self - .persistent_store - .find_credential(&rp_id, &allowed_credential.key_id) - { - Some(credential) => found_credentials.push(credential), + match self.persistent_store.find_credential( + &rp_id, + &allowed_credential.key_id, + !has_uv, + ) { + Some(credential) => { + found_credentials.push(credential); + } None => { if decrypted_credential.is_none() { decrypted_credential = self @@ -673,7 +692,15 @@ where found_credentials } else { // TODO(kaczmarczyck) use GetNextAssertion - self.persistent_store.filter_credential(&rp_id) + 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() + } }; let credential = if let Some(credential) = credentials.first() { @@ -1089,8 +1116,8 @@ where #[cfg(test)] mod test { use super::data_formats::{ - Extensions, GetAssertionOptions, MakeCredentialOptions, PublicKeyCredentialRpEntity, - PublicKeyCredentialUserEntity, + CredentialProtectionPolicy, Extensions, GetAssertionOptions, MakeCredentialOptions, + PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, }; use super::*; use crypto::rng256::ThreadRng256; @@ -1279,6 +1306,7 @@ mod test { user_handle: vec![], other_ui: None, cred_random: None, + cred_protect_policy: None, }; assert!(ctap_state .persistent_store @@ -1302,6 +1330,61 @@ 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 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 + .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; + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + assert!(make_credential_response.is_ok()); + } + #[test] fn test_process_make_credential_hmac_secret() { let mut rng = ThreadRng256 {}; @@ -1460,6 +1543,106 @@ mod test { ); } + #[test] + fn test_residential_process_get_assertion_with_cred_protect() { + let mut rng = ThreadRng256 {}; + let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); + let credential_id = rng.gen_uniform_u8x32().to_vec(); + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + + let cred_desc = PublicKeyCredentialDescriptor { + key_type: PublicKeyCredentialType::PublicKey, + key_id: credential_id.clone(), + transports: None, // You can set USB as a hint here. + }; + let credential = PublicKeyCredentialSource { + key_type: PublicKeyCredentialType::PublicKey, + credential_id: credential_id.clone(), + private_key: private_key.clone(), + rp_id: String::from("example.com"), + user_handle: vec![0x00], + other_ui: None, + cred_random: None, + cred_protect_policy: Some( + CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, + ), + }; + assert!(ctap_state + .persistent_store + .store_credential(credential) + .is_ok()); + + let get_assertion_params = AuthenticatorGetAssertionParameters { + rp_id: String::from("example.com"), + client_data_hash: vec![0xCD], + allow_list: None, + extensions: None, + 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(get_assertion_params, DUMMY_CHANNEL_ID); + assert_eq!( + get_assertion_response, + Err(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS), + ); + + let get_assertion_params = AuthenticatorGetAssertionParameters { + rp_id: String::from("example.com"), + client_data_hash: vec![0xCD], + allow_list: Some(vec![cred_desc.clone()]), + extensions: None, + 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(get_assertion_params, DUMMY_CHANNEL_ID); + assert!(get_assertion_response.is_ok()); + + let credential = PublicKeyCredentialSource { + key_type: PublicKeyCredentialType::PublicKey, + credential_id, + private_key, + rp_id: String::from("example.com"), + user_handle: vec![0x00], + other_ui: None, + cred_random: None, + cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), + }; + assert!(ctap_state + .persistent_store + .store_credential(credential) + .is_ok()); + + let get_assertion_params = AuthenticatorGetAssertionParameters { + rp_id: String::from("example.com"), + client_data_hash: vec![0xCD], + allow_list: Some(vec![cred_desc]), + extensions: None, + 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(get_assertion_params, DUMMY_CHANNEL_ID); + assert_eq!( + get_assertion_response, + Err(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS), + ); + } + #[test] fn test_process_reset() { let mut rng = ThreadRng256 {}; @@ -1476,6 +1659,7 @@ mod test { user_handle: vec![], other_ui: None, cred_random: None, + cred_protect_policy: None, }; assert!(ctap_state .persistent_store diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index f0a2ca4..ff5f98b 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -13,7 +13,7 @@ // limitations under the License. use crate::crypto::rng256::Rng256; -use crate::ctap::data_formats::PublicKeyCredentialSource; +use crate::ctap::data_formats::{CredentialProtectionPolicy, PublicKeyCredentialSource}; use crate::ctap::status_code::Ctap2StatusCode; use crate::ctap::PIN_AUTH_LENGTH; use alloc::string::String; @@ -217,6 +217,7 @@ impl PersistentStore { &self, rp_id: &str, credential_id: &[u8], + check_cred_protect: bool, ) -> Option { let key = Key::Credential { rp_id: Some(rp_id.into()), @@ -227,7 +228,16 @@ impl PersistentStore { debug_assert_eq!(entry.tag, TAG_CREDENTIAL); let result = deserialize_credential(entry.data); debug_assert!(result.is_some()); - result + if check_cred_protect + && result.as_ref().map_or(false, |cred| { + cred.cred_protect_policy + == Some(CredentialProtectionPolicy::UserVerificationRequired) + }) + { + None + } else { + result + } } pub fn store_credential( @@ -454,6 +464,7 @@ mod test { user_handle, other_ui: None, cred_random: None, + cred_protect_policy: None, } } @@ -612,9 +623,9 @@ mod test { .store_credential(credential_source1) .is_ok()); - let no_credential = persistent_store.find_credential("another.example.com", &id0); + let no_credential = persistent_store.find_credential("another.example.com", &id0, false); assert_eq!(no_credential, None); - let found_credential = persistent_store.find_credential("example.com", &id0); + let found_credential = persistent_store.find_credential("example.com", &id0, false); let expected_credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: id0, @@ -623,10 +634,33 @@ mod test { user_handle: vec![0x00], other_ui: None, cred_random: None, + cred_protect_policy: None, }; assert_eq!(found_credential, Some(expected_credential)); } + #[test] + fn test_find_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::UserVerificationRequired), + }; + assert!(persistent_store.store_credential(credential).is_ok()); + + let no_credential = persistent_store.find_credential("example.com", &vec![0x00], true); + assert_eq!(no_credential, None); + } + #[test] fn test_master_keys() { let mut rng = ThreadRng256 {};