From f90d43a6a1e8c27e7bef162f0d67d84e732d12e2 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 5 Feb 2021 18:43:27 +0100 Subject: [PATCH 1/6] 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; From 842c592c9fab0d6f7f0147c4112bb5333f4934b9 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 5 Feb 2021 18:51:56 +0100 Subject: [PATCH 2/6] adds changes to README --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index da46d7f..14c3dc6 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,12 @@ a few things you can personalize: length. 1. Increase the `MAX_CRED_BLOB_LENGTH` in `ctap/mod.rs`, if you expect blobs bigger than the default value. +1. If a certification (additional to FIDO's) requires that all requests are + protected with user verification, set `CAN_DISABLE_ALWAYS_UV` in + `ctap/config_command.rs` to `false`. In that case, consider deploying + authenticators after calling `toggleAlwaysUv` to activate the feature. + Alternatively, you could change `ctap/storage.rs` to set `alwaysUv` in its + initialization. ### 3D printed enclosure From 54e9da7a5b265349f6e27f26ce04887954673740 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 8 Feb 2021 07:49:58 +0100 Subject: [PATCH 3/6] conditional allow instead of cfg not --- src/ctap/mod.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 8b90b68..748afe6 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -1032,7 +1032,7 @@ where fn process_get_info(&self) -> Result { let has_always_uv = self.persistent_store.has_always_uv()?; - #[cfg(feature = "with_ctap1")] + #[cfg_attr(not(feature = "with_ctap1"), allow(unused_mut))] let mut versions = vec![ String::from(FIDO2_VERSION_STRING), String::from(FIDO2_1_VERSION_STRING), @@ -1043,11 +1043,6 @@ where 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( From 160c83d242bc7d7609d3297074aa33eb013d6e7a Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 8 Feb 2021 17:53:30 +0100 Subject: [PATCH 4/6] changes always uv constant to a clearer version --- src/ctap/config_command.rs | 36 +++++++++++++++--------------------- src/ctap/mod.rs | 3 +++ src/ctap/storage.rs | 22 ++++++++++++++++------ 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/ctap/config_command.rs b/src/ctap/config_command.rs index edff8f0..0d55717 100644 --- a/src/ctap/config_command.rs +++ b/src/ctap/config_command.rs @@ -12,24 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::check_pin_uv_auth_protocol; use super::command::AuthenticatorConfigParameters; use super::data_formats::{ConfigSubCommand, ConfigSubCommandParams, SetMinPinLengthParams}; use super::pin_protocol_v1::PinProtocolV1; use super::response::ResponseData; use super::status_code::Ctap2StatusCode; use super::storage::PersistentStore; +use super::{check_pin_uv_auth_protocol, ENFORCE_ALWAYS_UV}; 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()? { + if ENFORCE_ALWAYS_UV { return Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED); } persistent_store.toggle_always_uv()?; @@ -148,15 +144,14 @@ mod test { }; 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 { + if ENFORCE_ALWAYS_UV { assert_eq!( config_response, Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED) ); - assert!(persistent_store.has_always_uv().unwrap()); + } else { + assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); + assert!(!persistent_store.has_always_uv().unwrap()); } } @@ -181,6 +176,13 @@ mod test { }; let config_response = process_config(&mut persistent_store, &mut pin_protocol_v1, config_params); + if ENFORCE_ALWAYS_UV { + assert_eq!( + config_response, + Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED) + ); + return; + } assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); assert!(persistent_store.has_always_uv().unwrap()); @@ -192,16 +194,8 @@ mod test { }; 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()); - } + assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); + assert!(!persistent_store.has_always_uv().unwrap()); } fn create_min_pin_config_params( diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 748afe6..2180eb7 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -129,6 +129,9 @@ pub const ES256_CRED_PARAM: PublicKeyCredentialParameter = PublicKeyCredentialPa const DEFAULT_CRED_PROTECT: Option = None; // Maximum size stored with the credBlob extension. Must be at least 32. const MAX_CRED_BLOB_LENGTH: usize = 32; +// Enforce the alwaysUv option. With this constant set to true, commands require +// a PIN to be set up. The command toggleAlwaysUv will fail to disable alwaysUv. +pub const ENFORCE_ALWAYS_UV: bool = false; // Checks the PIN protocol parameter against all supported versions. pub fn check_pin_uv_auth_protocol( diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index b586d6d..3b0fb9e 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -18,10 +18,10 @@ use crate::ctap::data_formats::{ extract_array, extract_text_string, CredentialProtectionPolicy, PublicKeyCredentialSource, PublicKeyCredentialUserEntity, }; -use crate::ctap::key_material; use crate::ctap::pin_protocol_v1::PIN_AUTH_LENGTH; use crate::ctap::status_code::Ctap2StatusCode; use crate::ctap::INITIAL_SIGNATURE_COUNTER; +use crate::ctap::{key_material, ENFORCE_ALWAYS_UV}; use crate::embedded_flash::{new_storage, Storage}; use alloc::string::String; use alloc::vec; @@ -613,6 +613,9 @@ impl PersistentStore { /// Returns whether alwaysUv is enabled. pub fn has_always_uv(&self) -> Result { + if ENFORCE_ALWAYS_UV { + return Ok(true); + } match self.store.find(key::ALWAYS_UV)? { None => Ok(false), Some(value) if value.is_empty() => Ok(true), @@ -622,6 +625,9 @@ impl PersistentStore { /// Enables alwaysUv, when disabled, and vice versa. pub fn toggle_always_uv(&mut self) -> Result<(), Ctap2StatusCode> { + if ENFORCE_ALWAYS_UV { + return Ok(()); + } if self.has_always_uv()? { Ok(self.store.remove(key::ALWAYS_UV)?) } else { @@ -1331,11 +1337,15 @@ mod test { 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()); + if ENFORCE_ALWAYS_UV { + assert!(persistent_store.has_always_uv().unwrap()); + } else { + 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] From b9072047b3267bf166215454e11cdf442e2bd2c8 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 8 Feb 2021 17:56:27 +0100 Subject: [PATCH 5/6] update README to new constant --- README.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 14c3dc6..41f5378 100644 --- a/README.md +++ b/README.md @@ -126,11 +126,8 @@ a few things you can personalize: 1. Increase the `MAX_CRED_BLOB_LENGTH` in `ctap/mod.rs`, if you expect blobs bigger than the default value. 1. If a certification (additional to FIDO's) requires that all requests are - protected with user verification, set `CAN_DISABLE_ALWAYS_UV` in - `ctap/config_command.rs` to `false`. In that case, consider deploying - authenticators after calling `toggleAlwaysUv` to activate the feature. - Alternatively, you could change `ctap/storage.rs` to set `alwaysUv` in its - initialization. + protected with user verification, set `ENFORCE_ALWAYS_UV` in + `ctap/config_mod.rs` to `true`. ### 3D printed enclosure From 6a31e06a5557fbdf2bd701638418a5dd7bca5138 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 8 Feb 2021 21:54:22 +0100 Subject: [PATCH 6/6] move some logic into storage.rs --- src/ctap/config_command.rs | 6 ++---- src/ctap/mod.rs | 2 +- src/ctap/storage.rs | 6 +++++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/ctap/config_command.rs b/src/ctap/config_command.rs index b4c7cc0..c65fc56 100644 --- a/src/ctap/config_command.rs +++ b/src/ctap/config_command.rs @@ -18,7 +18,7 @@ use super::pin_protocol_v1::PinProtocolV1; use super::response::ResponseData; use super::status_code::Ctap2StatusCode; use super::storage::PersistentStore; -use super::{check_pin_uv_auth_protocol, ENFORCE_ALWAYS_UV, ENTERPRISE_ATTESTATION_MODE}; +use super::{check_pin_uv_auth_protocol, ENTERPRISE_ATTESTATION_MODE}; use alloc::vec; /// Processes the subcommand enableEnterpriseAttestation for AuthenticatorConfig. @@ -37,9 +37,6 @@ fn process_enable_enterprise_attestation( fn process_toggle_always_uv( persistent_store: &mut PersistentStore, ) -> Result { - if ENFORCE_ALWAYS_UV { - return Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED); - } persistent_store.toggle_always_uv()?; Ok(ResponseData::AuthenticatorConfig) } @@ -130,6 +127,7 @@ pub fn process_config( #[cfg(test)] mod test { use super::*; + use crate::ctap::ENFORCE_ALWAYS_UV; use crypto::rng256::ThreadRng256; #[test] diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 85deafe..18b0345 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -148,7 +148,7 @@ const DEFAULT_CRED_PROTECT: Option = None; // Maximum size stored with the credBlob extension. Must be at least 32. const MAX_CRED_BLOB_LENGTH: usize = 32; // Enforce the alwaysUv option. With this constant set to true, commands require -// a PIN to be set up. The command toggleAlwaysUv will fail to disable alwaysUv. +// a PIN to be set up. alwaysUv can not be disabled by commands. pub const ENFORCE_ALWAYS_UV: bool = false; // Checks the PIN protocol parameter against all supported versions. diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index a8be6f1..b1aa4ec 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -649,7 +649,7 @@ impl PersistentStore { /// Enables alwaysUv, when disabled, and vice versa. pub fn toggle_always_uv(&mut self) -> Result<(), Ctap2StatusCode> { if ENFORCE_ALWAYS_UV { - return Ok(()); + return Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED); } if self.has_always_uv()? { Ok(self.store.remove(key::ALWAYS_UV)?) @@ -1375,6 +1375,10 @@ mod test { if ENFORCE_ALWAYS_UV { assert!(persistent_store.has_always_uv().unwrap()); + assert_eq!( + persistent_store.toggle_always_uv(), + Err(Ctap2StatusCode::CTAP2_ERR_OPERATION_DENIED) + ); } else { assert!(!persistent_store.has_always_uv().unwrap()); assert_eq!(persistent_store.toggle_always_uv(), Ok(()));