diff --git a/libraries/opensk/src/ctap/ctap1.rs b/libraries/opensk/src/ctap/ctap1.rs index 87beb1c..841df67 100644 --- a/libraries/opensk/src/ctap/ctap1.rs +++ b/libraries/opensk/src/ctap/ctap1.rs @@ -13,7 +13,7 @@ // limitations under the License. use super::apdu::{Apdu, ApduStatusCode}; -use super::CtapState; +use super::{filter_listed_credential, CtapState}; use crate::api::attestation_store::{self, Attestation, AttestationStore}; use crate::api::crypto::ecdsa::{self, SecretKey as _, Signature}; use crate::api::crypto::EC_FIELD_SIZE; @@ -331,39 +331,37 @@ impl Ctap1Command { .key_store() .unwrap_credential(&key_handle, &application) .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; - if let Some(credential_source) = credential_source { - let ecdsa_key = credential_source - .private_key - .ecdsa_key(env) - .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; - if flags == Ctap1Flags::CheckOnly { - return Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED); - } - ctap_state - .increment_global_signature_counter(env) - .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; - let mut signature_data = ctap_state - .generate_auth_data( - env, - &application, - Ctap1Command::USER_PRESENCE_INDICATOR_BYTE, - ) - .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; - signature_data.extend(&challenge); - let signature = ecdsa_key.sign(&signature_data); - - let mut response = signature_data[application.len()..application.len() + 5].to_vec(); - response.extend(signature.to_der()); - Ok(response) - } else { - Err(Ctap1StatusCode::SW_WRONG_DATA) + let credential_source = filter_listed_credential(credential_source, false) + .ok_or(Ctap1StatusCode::SW_WRONG_DATA)?; + let ecdsa_key = credential_source + .private_key + .ecdsa_key(env) + .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; + if flags == Ctap1Flags::CheckOnly { + return Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED); } + ctap_state + .increment_global_signature_counter(env) + .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; + let mut signature_data = ctap_state + .generate_auth_data( + env, + &application, + Ctap1Command::USER_PRESENCE_INDICATOR_BYTE, + ) + .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; + signature_data.extend(&challenge); + let signature = ecdsa_key.sign(&signature_data); + + let mut response = signature_data[application.len()..application.len() + 5].to_vec(); + response.extend(signature.to_der()); + Ok(response) } } #[cfg(test)] mod test { - use super::super::data_formats::SignatureAlgorithm; + use super::super::data_formats::{CredentialProtectionPolicy, SignatureAlgorithm}; use super::super::TOUCH_TIMEOUT_MS; use super::*; use crate::api::crypto::sha256::Sha256; @@ -712,4 +710,27 @@ mod test { let response = Ctap1Command::process_command(&mut env, &message, &mut ctap_state); assert_eq!(response, Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED)); } + + #[test] + fn test_process_authenticate_cred_protect() { + let mut env = TestEnv::default(); + env.user_presence() + .set(|| panic!("Unexpected user presence check in CTAP1")); + let mut ctap_state = CtapState::new(&mut env); + + let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256); + let rp_id_hash = Sha::::digest(b"example.com"); + let credential_source = CredentialSource { + private_key, + rp_id_hash, + cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), + cred_blob: None, + }; + let key_handle = env.key_store().wrap_credential(credential_source).unwrap(); + let message = + create_authenticate_message(&rp_id_hash, Ctap1Flags::DontEnforceUpAndSign, &key_handle); + + let response = Ctap1Command::process_command(&mut env, &message, &mut ctap_state); + assert_eq!(response, Err(Ctap1StatusCode::SW_WRONG_DATA)); + } } diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index a1a5ab0..59073b1 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -186,28 +186,53 @@ pub fn cbor_write(value: cbor::Value, encoded_cbor: &mut Vec) -> Result<(), .map_err(|_e| Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR) } -fn decrypt_credential_id( - env: &mut E, +/// Filters the credential from the option if credProtect criteria are not met. +pub fn filter_listed_credential( + credential: Option, + has_uv: bool, +) -> Option { + credential.filter(|c| { + has_uv + || !matches!( + c.cred_protect_policy, + Some(CredentialProtectionPolicy::UserVerificationRequired) + ) + }) +} + +/// Filters the resident key from the option if credProtect criteria are not met. +fn filter_listed_resident_credential( + credential: Option, + has_uv: bool, +) -> Option { + credential.filter(|c| { + has_uv + || !matches!( + c.cred_protect_policy, + Some(CredentialProtectionPolicy::UserVerificationRequired) + ) + }) +} + +/// Populates all matching fields in a `PublicKeyCredentialSource`. +fn to_public_source( credential_id: Vec, - rp_id_hash: &[u8], -) -> Result, Ctap2StatusCode> { - let credential_source = env - .key_store() - .unwrap_credential(&credential_id, rp_id_hash)?; - Ok(credential_source.map(|c| PublicKeyCredentialSource { + credential_source: CredentialSource, +) -> PublicKeyCredentialSource { + PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, - private_key: c.private_key, + private_key: credential_source.private_key, rp_id: String::new(), user_handle: Vec::new(), user_display_name: None, - cred_protect_policy: c.cred_protect_policy, + cred_protect_policy: credential_source.cred_protect_policy, creation_order: 0, user_name: None, user_icon: None, - cred_blob: c.cred_blob, + cred_blob: credential_source.cred_blob, large_blob_key: None, - })) + } } // This function is adapted from https://doc.rust-lang.org/nightly/src/core/str/mod.rs.html#2110 @@ -698,22 +723,6 @@ 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 E, @@ -806,13 +815,18 @@ impl CtapState { let rp_id_hash = Sha::::digest(rp_id.as_bytes()); if let Some(exclude_list) = exclude_list { for cred_desc in exclude_list { - if self.check_cred_protect_for_listed_credential( - &storage::find_credential(env, &rp_id, &cred_desc.key_id)?, + if filter_listed_resident_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, - ) { + ) + .is_some() + || filter_listed_credential( + env.key_store() + .unwrap_credential(&cred_desc.key_id, &rp_id_hash)?, + has_uv, + ) + .is_some() + { // Perform this check, so bad actors can't brute force exclude_list // without user interaction. let _ = check_user_presence(env, channel); @@ -1087,13 +1101,20 @@ 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)?; - if self.check_cred_protect_for_listed_credential(&credential, has_uv) { + let credential = filter_listed_resident_credential( + storage::find_credential(env, rp_id, &allowed_credential.key_id)?, + has_uv, + ); + if credential.is_some() { return Ok(credential); } - 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); + let credential = filter_listed_credential( + env.key_store() + .unwrap_credential(&allowed_credential.key_id, &rp_id_hash)?, + has_uv, + ); + if credential.is_some() { + return Ok(credential.map(|c| to_public_source(allowed_credential.key_id, c))); } } Ok(None)