diff --git a/src/ctap/client_pin.rs b/src/ctap/client_pin.rs index 96c3f02..8b6588e 100644 --- a/src/ctap/client_pin.rs +++ b/src/ctap/client_pin.rs @@ -198,7 +198,7 @@ impl ClientPin { ) -> Result { Ok(AuthenticatorClientPinResponse { key_agreement: None, - pin_token: None, + pin_uv_auth_token: None, retries: Some(persistent_store.pin_retries()? as u64), power_cycle_state: Some(self.consecutive_pin_mismatches >= 3), }) @@ -214,7 +214,7 @@ impl ClientPin { ); Ok(AuthenticatorClientPinResponse { key_agreement, - pin_token: None, + pin_uv_auth_token: None, retries: None, power_cycle_state: None, }) @@ -298,10 +298,15 @@ impl ClientPin { pin_uv_auth_protocol, key_agreement, pin_hash_enc, + permissions, + permissions_rp_id, .. } = client_pin_params; let key_agreement = ok_or_missing(key_agreement)?; let pin_hash_enc = ok_or_missing(pin_hash_enc)?; + if permissions.is_some() || permissions_rp_id.is_some() { + return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); + } if persistent_store.pin_retries()? == 0 { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_BLOCKED); @@ -317,21 +322,21 @@ impl ClientPin { if persistent_store.has_force_pin_change()? { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID); } - let pin_token = shared_secret.encrypt( - rng, - self.get_pin_protocol(pin_uv_auth_protocol) - .get_pin_uv_auth_token(), - )?; self.pin_protocol_v1.reset_pin_uv_auth_token(rng); self.pin_protocol_v2.reset_pin_uv_auth_token(rng); self.pin_uv_auth_token_state .begin_using_pin_uv_auth_token(now); self.pin_uv_auth_token_state.set_default_permissions(); + let pin_uv_auth_token = shared_secret.encrypt( + rng, + self.get_pin_protocol(pin_uv_auth_protocol) + .get_pin_uv_auth_token(), + )?; Ok(AuthenticatorClientPinResponse { key_agreement: None, - pin_token: Some(pin_token), + pin_uv_auth_token: Some(pin_uv_auth_token), retries: None, power_cycle_state: None, }) @@ -359,9 +364,10 @@ impl ClientPin { mut client_pin_params: AuthenticatorClientPinParameters, now: ClockValue, ) -> Result { - let permissions = ok_or_missing(client_pin_params.permissions)?; // Mutating client_pin_params is just an optimization to move it into // process_get_pin_token, without cloning permissions_rp_id here. + // getPinToken requires permissions* to be None. + let permissions = ok_or_missing(client_pin_params.permissions.take())?; let permissions_rp_id = client_pin_params.permissions_rp_id.take(); if permissions == 0 { @@ -657,6 +663,13 @@ mod test { .as_ref() .encrypt(&mut rng, &pin_hash[..16]) .unwrap(); + let (permissions, permissions_rp_id) = match sub_command { + ClientPinSubCommand::GetPinUvAuthTokenUsingUvWithPermissions + | ClientPinSubCommand::GetPinUvAuthTokenUsingPinWithPermissions => { + (Some(0x03), Some("example.com".to_string())) + } + _ => (None, None), + }; let params = AuthenticatorClientPinParameters { pin_uv_auth_protocol, sub_command, @@ -668,8 +681,8 @@ mod test { pin_uv_auth_param: Some(pin_uv_auth_param), new_pin_enc: Some(new_pin_enc), pin_hash_enc: Some(pin_hash_enc), - permissions: Some(0x03), - permissions_rp_id: Some("example.com".to_string()), + permissions, + permissions_rp_id, }; (client_pin, params) } @@ -813,7 +826,7 @@ mod test { let mut persistent_store = PersistentStore::new(&mut rng); let expected_response = Some(AuthenticatorClientPinResponse { key_agreement: None, - pin_token: None, + pin_uv_auth_token: None, retries: Some(persistent_store.pin_retries().unwrap() as u64), power_cycle_state: Some(false), }); @@ -830,7 +843,7 @@ mod test { client_pin.consecutive_pin_mismatches = 3; let expected_response = Some(AuthenticatorClientPinResponse { key_agreement: None, - pin_token: None, + pin_uv_auth_token: None, retries: Some(persistent_store.pin_retries().unwrap() as u64), power_cycle_state: Some(true), }); @@ -859,7 +872,7 @@ mod test { let mut persistent_store = PersistentStore::new(&mut rng); let expected_response = Some(AuthenticatorClientPinResponse { key_agreement: params.key_agreement.clone(), - pin_token: None, + pin_uv_auth_token: None, retries: None, power_cycle_state: None, }); @@ -964,18 +977,37 @@ mod test { pin_uv_auth_protocol, ClientPinSubCommand::GetPinToken, ); + let shared_secret = client_pin + .get_pin_protocol(pin_uv_auth_protocol) + .decapsulate( + params.key_agreement.clone().unwrap(), + params.pin_uv_auth_protocol, + ) + .unwrap(); let mut rng = ThreadRng256 {}; let mut persistent_store = PersistentStore::new(&mut rng); set_standard_pin(&mut persistent_store); - assert!(client_pin + let response = client_pin .process_command( &mut rng, &mut persistent_store, params.clone(), - DUMMY_CLOCK_VALUE + DUMMY_CLOCK_VALUE, ) - .is_ok()); + .unwrap(); + let encrypted_token = match response { + ResponseData::AuthenticatorClientPin(Some(response)) => { + response.pin_uv_auth_token.unwrap() + } + _ => panic!("Invalid response type"), + }; + assert_eq!( + &shared_secret.decrypt(&encrypted_token).unwrap(), + client_pin + .get_pin_protocol(pin_uv_auth_protocol) + .get_pin_uv_auth_token() + ); assert_eq!( client_pin .pin_uv_auth_token_state @@ -1051,18 +1083,37 @@ mod test { pin_uv_auth_protocol, ClientPinSubCommand::GetPinUvAuthTokenUsingPinWithPermissions, ); + let shared_secret = client_pin + .get_pin_protocol(pin_uv_auth_protocol) + .decapsulate( + params.key_agreement.clone().unwrap(), + params.pin_uv_auth_protocol, + ) + .unwrap(); let mut rng = ThreadRng256 {}; let mut persistent_store = PersistentStore::new(&mut rng); set_standard_pin(&mut persistent_store); - assert!(client_pin + let response = client_pin .process_command( &mut rng, &mut persistent_store, params.clone(), - DUMMY_CLOCK_VALUE + DUMMY_CLOCK_VALUE, ) - .is_ok()); + .unwrap(); + let encrypted_token = match response { + ResponseData::AuthenticatorClientPin(Some(response)) => { + response.pin_uv_auth_token.unwrap() + } + _ => panic!("Invalid response type"), + }; + assert_eq!( + &shared_secret.decrypt(&encrypted_token).unwrap(), + client_pin + .get_pin_protocol(pin_uv_auth_protocol) + .get_pin_uv_auth_token() + ); assert_eq!( client_pin .pin_uv_auth_token_state diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 1721e62..b135a11 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -409,8 +409,11 @@ impl TryFrom for MakeCredentialOptions { Some(options_entry) => extract_bool(options_entry)?, None => false, }; - if up.is_some() { - return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); + // In CTAP2.0, the up option is supposed to always fail when present. + if let Some(options_entry) = up { + if !extract_bool(options_entry)? { + return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); + } } let uv = match uv { Some(options_entry) => extract_bool(options_entry)?, diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 5d07789..1ecac53 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -554,6 +554,68 @@ where false }; + // MakeCredential always requires user presence. + // User verification depends on the PIN auth inputs, which are checked here. + // The ED flag is added later, if applicable. + let has_uv = pin_uv_auth_param.is_some(); + let mut flags = match pin_uv_auth_param { + Some(pin_uv_auth_param) => { + // This case is not mentioned in CTAP2.1, so we keep 2.0 logic. + if self.persistent_store.pin_hash()?.is_none() { + return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); + } + self.client_pin.verify_pin_uv_auth_token( + &client_data_hash, + &pin_uv_auth_param, + pin_uv_auth_protocol.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, + )?; + self.client_pin + .has_permission(PinPermission::MakeCredential)?; + self.client_pin.check_user_verified_flag()?; + // Checking for the correct permissions_rp_id is specified earlier. + // Error codes are identical though, so the implementation can be identical with + // GetAssertion. + self.client_pin.ensure_rp_id_permission(&rp_id)?; + UV_FLAG + } + None => { + if options.uv { + return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); + } + if self.persistent_store.has_always_uv()? { + return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); + } + // Corresponds to makeCredUvNotRqd set to true. + if options.rk && self.persistent_store.pin_hash()?.is_some() { + return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); + } + 0x00 + } + }; + flags |= UP_FLAG | AT_FLAG; + + let rp_id_hash = Sha256::hash(rp_id.as_bytes()); + 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, !has_uv)? + .is_some() + || self + .decrypt_credential_source(cred_desc.key_id, &rp_id_hash)? + .is_some() + { + // Perform this check, so bad actors can't brute force exclude_list + // without user interaction. + let _ = (self.check_user_presence)(cid); + return Err(Ctap2StatusCode::CTAP2_ERR_CREDENTIAL_EXCLUDED); + } + } + } + + (self.check_user_presence)(cid)?; + self.client_pin.clear_token_flags(); + let mut cred_protect_policy = extensions.cred_protect; if cred_protect_policy.unwrap_or(CredentialProtectionPolicy::UserVerificationOptional) < DEFAULT_CRED_PROTECT.unwrap_or(CredentialProtectionPolicy::UserVerificationOptional) @@ -576,74 +638,17 @@ where None }; let has_extension_output = extensions.hmac_secret - || cred_protect_policy.is_some() + || extensions.cred_protect.is_some() || min_pin_length || has_cred_blob_output; + if has_extension_output { + flags |= ED_FLAG + }; let large_blob_key = match (options.rk, extensions.large_blob_key) { (true, Some(true)) => Some(self.rng.gen_uniform_u8x32().to_vec()), _ => None, }; - let rp_id_hash = Sha256::hash(rp_id.as_bytes()); - 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, pin_uv_auth_param.is_none())? - .is_some() - || self - .decrypt_credential_source(cred_desc.key_id, &rp_id_hash)? - .is_some() - { - // Perform this check, so bad actors can't brute force exclude_list - // without user interaction. - (self.check_user_presence)(cid)?; - return Err(Ctap2StatusCode::CTAP2_ERR_CREDENTIAL_EXCLUDED); - } - } - } - - // 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_uv_auth_param) => { - if self.persistent_store.pin_hash()?.is_none() { - // Specification is unclear, could be CTAP2_ERR_INVALID_OPTION. - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); - } - self.client_pin.verify_pin_uv_auth_token( - &client_data_hash, - &pin_uv_auth_param, - pin_uv_auth_protocol.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, - )?; - self.client_pin - .has_permission(PinPermission::MakeCredential)?; - self.client_pin.check_user_verified_flag()?; - // Checking for the correct permissions_rp_id is specified earlier. - // Error codes are identical though, so the implementation can be identical with - // GetAssertion. - self.client_pin.ensure_rp_id_permission(&rp_id)?; - UP_FLAG | UV_FLAG | AT_FLAG | ed_flag - } - None => { - if self.persistent_store.has_always_uv()? { - return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); - } - // Corresponds to makeCredUvNotRqd set to true. - if options.rk && self.persistent_store.pin_hash()?.is_some() { - return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); - } - if options.uv { - return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); - } - UP_FLAG | AT_FLAG | ed_flag - } - }; - - (self.check_user_presence)(cid)?; - self.client_pin.clear_token_flags(); - let sk = crypto::ecdsa::SecKey::gensk(self.rng); let pk = sk.genpk(); @@ -699,9 +704,10 @@ where } else { None }; + let cred_protect_output = extensions.cred_protect.and(cred_protect_policy); let extensions_output = cbor_map_options! { "hmac-secret" => hmac_secret_output, - "credProtect" => cred_protect_policy, + "credProtect" => cred_protect_output, "minPinLength" => min_pin_length_output, "credBlob" => cred_blob_output, }; @@ -904,8 +910,8 @@ where let has_uv = pin_uv_auth_param.is_some(); let mut flags = match pin_uv_auth_param { Some(pin_uv_auth_param) => { + // This case is not mentioned in CTAP2.1, so we keep 2.0 logic. if self.persistent_store.pin_hash()?.is_none() { - // Specification is unclear, could be CTAP2_ERR_UNSUPPORTED_OPTION. return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } self.client_pin.verify_pin_uv_auth_token( @@ -923,12 +929,12 @@ where UV_FLAG } None => { - if self.persistent_store.has_always_uv()? { - return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); - } if options.uv { return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); } + if options.up && self.persistent_store.has_always_uv()? { + return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); + } 0x00 } }; @@ -970,15 +976,14 @@ where (credential, stored_credentials) }; + let credential = credential.ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)?; + // This check comes before CTAP2_ERR_NO_CREDENTIALS in CTAP 2.0. - // For CTAP 2.1, it was moved to a later protocol step. if options.up { (self.check_user_presence)(cid)?; self.client_pin.clear_token_flags(); } - let credential = credential.ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)?; - self.increment_global_signature_counter()?; let assertion_input = AssertionInput { diff --git a/src/ctap/response.rs b/src/ctap/response.rs index ddc186b..6a47172 100644 --- a/src/ctap/response.rs +++ b/src/ctap/response.rs @@ -211,7 +211,7 @@ impl From for cbor::Value { #[derive(Debug, PartialEq)] pub struct AuthenticatorClientPinResponse { pub key_agreement: Option, - pub pin_token: Option>, + pub pin_uv_auth_token: Option>, pub retries: Option, pub power_cycle_state: Option, // - 0x05: uvRetries missing as we don't support internal UV. @@ -221,14 +221,14 @@ impl From for cbor::Value { fn from(client_pin_response: AuthenticatorClientPinResponse) -> Self { let AuthenticatorClientPinResponse { key_agreement, - pin_token, + pin_uv_auth_token, retries, power_cycle_state, } = client_pin_response; cbor_map_options! { 0x01 => key_agreement.map(cbor::Value::from), - 0x02 => pin_token, + 0x02 => pin_uv_auth_token, 0x03 => retries, 0x04 => power_cycle_state, } @@ -495,7 +495,7 @@ mod test { let cose_key = CoseKey::from(pk); let client_pin_response = AuthenticatorClientPinResponse { key_agreement: Some(cose_key.clone()), - pin_token: Some(vec![70]), + pin_uv_auth_token: Some(vec![70]), retries: Some(8), power_cycle_state: Some(false), };