From f90d43a6a1e8c27e7bef162f0d67d84e732d12e2 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 5 Feb 2021 18:43:27 +0100 Subject: [PATCH] implements alwaysUv and makeCredUvNotRqd --- src/ctap/apdu.rs | 1 + src/ctap/config_command.rs | 105 ++++++++++++++++++++++++++++++++++- src/ctap/ctap1.rs | 21 +++++++ src/ctap/hid/mod.rs | 2 +- src/ctap/large_blobs.rs | 2 +- src/ctap/mod.rs | 110 ++++++++++++++++++++++++++++++++++--- src/ctap/storage.rs | 30 ++++++++++ src/ctap/storage/key.rs | 3 + 8 files changed, 262 insertions(+), 12 deletions(-) diff --git a/src/ctap/apdu.rs b/src/ctap/apdu.rs index f12ded2..455e574 100644 --- a/src/ctap/apdu.rs +++ b/src/ctap/apdu.rs @@ -29,6 +29,7 @@ pub enum ApduStatusCode { SW_WRONG_DATA = 0x6a_80, SW_WRONG_LENGTH = 0x67_00, SW_COND_USE_NOT_SATISFIED = 0x69_85, + SW_COMMAND_NOT_ALLOWED = 0x69_86, SW_FILE_NOT_FOUND = 0x6a_82, SW_INCORRECT_P1P2 = 0x6a_86, /// Instruction code not supported or invalid diff --git a/src/ctap/config_command.rs b/src/ctap/config_command.rs index 5e4daf3..edff8f0 100644 --- a/src/ctap/config_command.rs +++ b/src/ctap/config_command.rs @@ -21,6 +21,21 @@ use super::status_code::Ctap2StatusCode; use super::storage::PersistentStore; use alloc::vec; +/// The specification mandates that authenticators support users disabling +/// alwaysUv unless required not to by specific external certifications. +const CAN_DISABLE_ALWAYS_UV: bool = true; + +/// Processes the subcommand toggleAlwaysUv for AuthenticatorConfig. +fn process_toggle_always_uv( + persistent_store: &mut PersistentStore, +) -> Result { + if !CAN_DISABLE_ALWAYS_UV && persistent_store.has_always_uv()? { + return Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED); + } + persistent_store.toggle_always_uv()?; + Ok(ResponseData::AuthenticatorConfig) +} + /// Processes the subcommand setMinPINLength for AuthenticatorConfig. fn process_set_min_pin_length( persistent_store: &mut PersistentStore, @@ -66,7 +81,11 @@ pub fn process_config( pin_uv_auth_protocol, } = params; - if persistent_store.pin_hash()?.is_some() { + let enforce_uv = match sub_command { + ConfigSubCommand::ToggleAlwaysUv => false, + _ => true, + } && persistent_store.has_always_uv()?; + if persistent_store.pin_hash()?.is_some() || enforce_uv { // TODO(kaczmarczyck) The error code is specified inconsistently with other commands. check_pin_uv_auth_protocol(pin_uv_auth_protocol) .map_err(|_| Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED)?; @@ -85,6 +104,7 @@ pub fn process_config( } match sub_command { + ConfigSubCommand::ToggleAlwaysUv => process_toggle_always_uv(persistent_store), ConfigSubCommand::SetMinPinLength => { if let Some(ConfigSubCommandParams::SetMinPinLength(params)) = sub_command_params { process_set_min_pin_length(persistent_store, params) @@ -101,6 +121,89 @@ mod test { use super::*; use crypto::rng256::ThreadRng256; + #[test] + fn test_process_toggle_always_uv() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + let key_agreement_key = crypto::ecdh::SecKey::gensk(&mut rng); + let pin_uv_auth_token = [0x55; 32]; + let mut pin_protocol_v1 = PinProtocolV1::new_test(key_agreement_key, pin_uv_auth_token); + + let config_params = AuthenticatorConfigParameters { + sub_command: ConfigSubCommand::ToggleAlwaysUv, + sub_command_params: None, + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + let config_response = + process_config(&mut persistent_store, &mut pin_protocol_v1, config_params); + assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); + assert!(persistent_store.has_always_uv().unwrap()); + + let config_params = AuthenticatorConfigParameters { + sub_command: ConfigSubCommand::ToggleAlwaysUv, + sub_command_params: None, + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + let config_response = + process_config(&mut persistent_store, &mut pin_protocol_v1, config_params); + if CAN_DISABLE_ALWAYS_UV { + assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); + assert!(!persistent_store.has_always_uv().unwrap()); + } else { + assert_eq!( + config_response, + Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED) + ); + assert!(persistent_store.has_always_uv().unwrap()); + } + } + + #[test] + fn test_process_toggle_always_uv_with_pin() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + let key_agreement_key = crypto::ecdh::SecKey::gensk(&mut rng); + let pin_uv_auth_token = [0x55; 32]; + let mut pin_protocol_v1 = PinProtocolV1::new_test(key_agreement_key, pin_uv_auth_token); + persistent_store.set_pin(&[0x88; 16], 4).unwrap(); + + let pin_uv_auth_param = Some(vec![ + 0x99, 0xBA, 0x0A, 0x57, 0x9D, 0x95, 0x5A, 0x44, 0xE3, 0x77, 0xCF, 0x95, 0x51, 0x3F, + 0xFD, 0xBE, + ]); + let config_params = AuthenticatorConfigParameters { + sub_command: ConfigSubCommand::ToggleAlwaysUv, + sub_command_params: None, + pin_uv_auth_param: pin_uv_auth_param.clone(), + pin_uv_auth_protocol: Some(1), + }; + let config_response = + process_config(&mut persistent_store, &mut pin_protocol_v1, config_params); + assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); + assert!(persistent_store.has_always_uv().unwrap()); + + let config_params = AuthenticatorConfigParameters { + sub_command: ConfigSubCommand::ToggleAlwaysUv, + sub_command_params: None, + pin_uv_auth_param, + pin_uv_auth_protocol: Some(1), + }; + let config_response = + process_config(&mut persistent_store, &mut pin_protocol_v1, config_params); + if CAN_DISABLE_ALWAYS_UV { + assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); + assert!(!persistent_store.has_always_uv().unwrap()); + } else { + assert_eq!( + config_response, + Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED) + ); + assert!(persistent_store.has_always_uv().unwrap()); + } + } + fn create_min_pin_config_params( min_pin_length: u8, min_pin_length_rp_ids: Option>, diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 09a3c6c..d5e60ee 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -189,6 +189,12 @@ impl Ctap1Command { R: Rng256, CheckUserPresence: Fn(ChannelID) -> Result<(), Ctap2StatusCode>, { + if !ctap_state + .allows_ctap1() + .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)? + { + return Err(Ctap1StatusCode::SW_COMMAND_NOT_ALLOWED); + } let command = U2fCommand::try_from(message)?; match command { U2fCommand::Register { @@ -398,6 +404,21 @@ mod test { message } + #[test] + fn test_process_allowed() { + let mut rng = ThreadRng256 {}; + let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); + ctap_state.persistent_store.toggle_always_uv().unwrap(); + + let application = [0x0A; 32]; + let message = create_register_message(&application); + ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); + ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); + let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); + assert_eq!(response, Err(Ctap1StatusCode::SW_COMMAND_NOT_ALLOWED)); + } + #[test] fn test_process_register() { let mut rng = ThreadRng256 {}; diff --git a/src/ctap/hid/mod.rs b/src/ctap/hid/mod.rs index 71bd7c8..03cb637 100644 --- a/src/ctap/hid/mod.rs +++ b/src/ctap/hid/mod.rs @@ -177,7 +177,7 @@ impl CtapHid { match message.cmd { // CTAP specification (version 20190130) section 8.1.9.1.1 CtapHid::COMMAND_MSG => { - // If we don't have CTAP1 backward compatibilty, this command in invalid. + // If we don't have CTAP1 backward compatibilty, this command is invalid. #[cfg(not(feature = "with_ctap1"))] return CtapHid::error_message(cid, CtapHid::ERR_INVALID_CMD); diff --git a/src/ctap/large_blobs.rs b/src/ctap/large_blobs.rs index ab38df0..f84c1a9 100644 --- a/src/ctap/large_blobs.rs +++ b/src/ctap/large_blobs.rs @@ -88,7 +88,7 @@ impl LargeBlobs { if offset != self.expected_next_offset { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_SEQ); } - if persistent_store.pin_hash()?.is_some() { + if persistent_store.pin_hash()?.is_some() || persistent_store.has_always_uv()? { let pin_uv_auth_param = pin_uv_auth_param.ok_or(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED)?; // TODO(kaczmarczyck) Error codes for PIN protocol differ across commands. diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index ab66177..8b90b68 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -341,6 +341,14 @@ where Ok(()) } + // Returns whether CTAP1 commands are currently supported. + // If alwaysUv is enabled and the authenticator does not support internal UV, + // CTAP1 needs to be disabled. + #[cfg(feature = "with_ctap1")] + pub fn allows_ctap1(&self) -> Result { + Ok(!self.persistent_store.has_always_uv()?) + } + // Encrypts the private key and relying party ID hash into a credential ID. Other // information, such as a user name, are not stored, because encrypted credential IDs // are used for credentials stored server-side. Also, we want the key handle to be @@ -642,7 +650,11 @@ where UP_FLAG | UV_FLAG | AT_FLAG | ed_flag } None => { - if self.persistent_store.pin_hash()?.is_some() { + 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 { @@ -927,8 +939,10 @@ where UV_FLAG } None => { + if self.persistent_store.has_always_uv()? { + return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); + } if options.uv { - // The specification (inconsistently) wants CTAP2_ERR_UNSUPPORTED_OPTION. return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); } 0x00 @@ -1017,6 +1031,23 @@ where } fn process_get_info(&self) -> Result { + let has_always_uv = self.persistent_store.has_always_uv()?; + #[cfg(feature = "with_ctap1")] + let mut versions = vec![ + String::from(FIDO2_VERSION_STRING), + String::from(FIDO2_1_VERSION_STRING), + ]; + #[cfg(feature = "with_ctap1")] + { + if !has_always_uv { + versions.insert(0, String::from(U2F_VERSION_STRING)) + } + } + #[cfg(not(feature = "with_ctap1"))] + let versions = vec![ + String::from(FIDO2_VERSION_STRING), + String::from(FIDO2_1_VERSION_STRING), + ]; let mut options_map = BTreeMap::new(); options_map.insert(String::from("rk"), true); options_map.insert( @@ -1029,15 +1060,11 @@ where options_map.insert(String::from("authnrCfg"), true); options_map.insert(String::from("credMgmt"), true); options_map.insert(String::from("setMinPINLength"), true); - options_map.insert(String::from("makeCredUvNotRqd"), true); + options_map.insert(String::from("makeCredUvNotRqd"), !has_always_uv); + options_map.insert(String::from("alwaysUv"), has_always_uv); Ok(ResponseData::AuthenticatorGetInfo( AuthenticatorGetInfoResponse { - versions: vec![ - #[cfg(feature = "with_ctap1")] - String::from(U2F_VERSION_STRING), - String::from(FIDO2_VERSION_STRING), - String::from(FIDO2_1_VERSION_STRING), - ], + versions, extensions: Some(vec![ String::from("hmac-secret"), String::from("credProtect"), @@ -1286,6 +1313,7 @@ mod test { "credMgmt" => true, "setMinPINLength" => true, "makeCredUvNotRqd" => true, + "alwaysUv" => false, }, 0x05 => MAX_MSG_SIZE as u64, 0x06 => cbor_array_vec![vec![1]], @@ -1727,6 +1755,70 @@ mod test { assert_eq!(stored_credential.large_blob_key.unwrap(), large_blob_key); } + #[test] + fn test_non_resident_process_make_credential_with_pin() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + ctap_state.persistent_store.set_pin(&[0x88; 16], 4).unwrap(); + + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.options.rk = false; + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + + check_make_response( + make_credential_response, + 0x41, + &ctap_state.persistent_store.aaguid().unwrap(), + 0x70, + &[], + ); + } + + #[test] + fn test_resident_process_make_credential_with_pin() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + ctap_state.persistent_store.set_pin(&[0x88; 16], 4).unwrap(); + + let make_credential_params = create_minimal_make_credential_parameters(); + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + assert_eq!( + make_credential_response, + Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED) + ); + } + + #[test] + fn test_process_make_credential_with_pin_always_uv() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + + ctap_state.persistent_store.toggle_always_uv().unwrap(); + let make_credential_params = create_minimal_make_credential_parameters(); + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + assert_eq!( + make_credential_response, + Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED) + ); + + ctap_state.persistent_store.set_pin(&[0x88; 16], 4).unwrap(); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.pin_uv_auth_param = Some(vec![0xA4; 16]); + make_credential_params.pin_uv_auth_protocol = Some(1); + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + assert_eq!( + make_credential_response, + Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID) + ); + } + #[test] fn test_process_make_credential_cancelled() { let mut rng = ThreadRng256 {}; diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index b982922..b586d6d 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -610,6 +610,24 @@ impl PersistentStore { pub fn force_pin_change(&mut self) -> Result<(), Ctap2StatusCode> { Ok(self.store.insert(key::FORCE_PIN_CHANGE, &[])?) } + + /// Returns whether alwaysUv is enabled. + pub fn has_always_uv(&self) -> Result { + match self.store.find(key::ALWAYS_UV)? { + None => Ok(false), + Some(value) if value.is_empty() => Ok(true), + _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), + } + } + + /// Enables alwaysUv, when disabled, and vice versa. + pub fn toggle_always_uv(&mut self) -> Result<(), Ctap2StatusCode> { + if self.has_always_uv()? { + Ok(self.store.remove(key::ALWAYS_UV)?) + } else { + Ok(self.store.insert(key::ALWAYS_UV, &[])?) + } + } } impl From for Ctap2StatusCode { @@ -1308,6 +1326,18 @@ mod test { assert!(!persistent_store.has_force_pin_change().unwrap()); } + #[test] + fn test_always_uv() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + + assert!(!persistent_store.has_always_uv().unwrap()); + assert_eq!(persistent_store.toggle_always_uv(), Ok(())); + assert!(persistent_store.has_always_uv().unwrap()); + assert_eq!(persistent_store.toggle_always_uv(), Ok(())); + assert!(!persistent_store.has_always_uv().unwrap()); + } + #[test] fn test_serialize_deserialize_credential() { let mut rng = ThreadRng256 {}; diff --git a/src/ctap/storage/key.rs b/src/ctap/storage/key.rs index 2093685..9387931 100644 --- a/src/ctap/storage/key.rs +++ b/src/ctap/storage/key.rs @@ -93,6 +93,9 @@ make_partition! { /// The stored large blob can be too big for one key, so it has to be sharded. LARGE_BLOB_SHARDS = 2000..2004; + /// If this entry exists and is empty, alwaysUv is enabled. + ALWAYS_UV = 2038; + /// If this entry exists and equals 1, the PIN needs to be changed. FORCE_PIN_CHANGE = 2040;