From c873d3b6147e81f1be7518718308d15deec04b85 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Wed, 6 Jan 2021 19:24:56 +0100 Subject: [PATCH 1/2] removes all occurences of CTAP 2.1 flags --- Cargo.toml | 1 - README.md | 15 ++++--- deploy.py | 10 +---- src/ctap/command.rs | 43 -------------------- src/ctap/data_formats.rs | 11 ----- src/ctap/mod.rs | 80 +++++++++++-------------------------- src/ctap/pin_protocol_v1.rs | 79 ++++-------------------------------- src/ctap/response.rs | 61 +--------------------------- src/ctap/status_code.rs | 6 --- src/ctap/storage.rs | 22 ++-------- src/ctap/storage/key.rs | 2 - 11 files changed, 46 insertions(+), 284 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 15984a0f..bca92101 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,6 @@ panic_console = ["lang_items/panic_console"] std = ["cbor/std", "crypto/std", "crypto/derive_debug", "lang_items/std", "persistent_store/std"] verbose = ["debug_ctap", "libtock_drivers/verbose_usb"] with_ctap1 = ["crypto/with_ctap1"] -with_ctap2_1 = [] with_nfc = ["libtock_drivers/with_nfc"] [dev-dependencies] diff --git a/README.md b/README.md index 68e9f717..decee760 100644 --- a/README.md +++ b/README.md @@ -24,15 +24,14 @@ few limitations: ### FIDO2 -Although we tested and implemented our firmware based on the published +The stable branch implements the published [CTAP2.0 specifications](https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html), -our implementation was not reviewed nor officially tested and doesn't claim to -be FIDO Certified. -We started adding features of the upcoming next version of the -[CTAP2.1 specifications](https://fidoalliance.org/specs/fido2/fido-client-to-authenticator-protocol-v2.1-rd-20191217.html). -The development is currently between 2.0 and 2.1, with updates hidden behind -a feature flag. -Please add the flag `--ctap2.1` to the deploy command to include them. +but our implementation was not reviewed nor officially tested and doesn't claim +to be FIDO Certified. It already contains some preview features of 2.1, that you +can try by adding the flag `--ctap2.1` to the deploy command. +The develop branch offers only the +[CTAP2.1 specifications](https://fidoalliance.org/specs/fido-v2.1-rd-20201208/fido-client-to-authenticator-protocol-v2.1-rd-20201208.html). +The new features of 2.1 are currently work in progress. ### Cryptography diff --git a/deploy.py b/deploy.py index d5dcf2d0..e8d5ffd2 100755 --- a/deploy.py +++ b/deploy.py @@ -881,14 +881,6 @@ def main(args): help=("Compiles the OpenSK application without backward compatible " "support for U2F/CTAP1 protocol."), ) - main_parser.add_argument( - "--ctap2.1", - action="append_const", - const="with_ctap2_1", - dest="features", - help=("Compiles the OpenSK application with backward compatible " - "support for CTAP2.1 protocol."), - ) main_parser.add_argument( "--nfc", action="append_const", @@ -947,7 +939,7 @@ def main(args): dest="application", action="store_const", const="store_latency", - help=("Compiles and installs the store_latency example which print " + help=("Compiles and installs the store_latency example which prints " "latency statistics of the persistent store library.")) apps_group.add_argument( "--erase_storage", diff --git a/src/ctap/command.rs b/src/ctap/command.rs index ecfae9e6..0a860933 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -41,7 +41,6 @@ pub enum Command { AuthenticatorClientPin(AuthenticatorClientPinParameters), AuthenticatorReset, AuthenticatorGetNextAssertion, - #[cfg(feature = "with_ctap2_1")] AuthenticatorSelection, // TODO(kaczmarczyck) implement FIDO 2.1 commands (see below consts) // Vendor specific commands @@ -111,7 +110,6 @@ impl Command { // Parameters are ignored. Ok(Command::AuthenticatorGetNextAssertion) } - #[cfg(feature = "with_ctap2_1")] Command::AUTHENTICATOR_SELECTION => { // Parameters are ignored. Ok(Command::AuthenticatorSelection) @@ -292,13 +290,9 @@ pub struct AuthenticatorClientPinParameters { pub pin_auth: Option>, pub new_pin_enc: Option>, pub pin_hash_enc: Option>, - #[cfg(feature = "with_ctap2_1")] pub min_pin_length: Option, - #[cfg(feature = "with_ctap2_1")] pub min_pin_length_rp_ids: Option>, - #[cfg(feature = "with_ctap2_1")] pub permissions: Option, - #[cfg(feature = "with_ctap2_1")] pub permissions_rp_id: Option, } @@ -306,18 +300,6 @@ impl TryFrom for AuthenticatorClientPinParameters { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - #[cfg(not(feature = "with_ctap2_1"))] - destructure_cbor_map! { - let { - 1 => pin_protocol, - 2 => sub_command, - 3 => key_agreement, - 4 => pin_auth, - 5 => new_pin_enc, - 6 => pin_hash_enc, - } = extract_map(cbor_value)?; - } - #[cfg(feature = "with_ctap2_1")] destructure_cbor_map! { let { 1 => pin_protocol, @@ -339,14 +321,12 @@ impl TryFrom for AuthenticatorClientPinParameters { let pin_auth = pin_auth.map(extract_byte_string).transpose()?; let new_pin_enc = new_pin_enc.map(extract_byte_string).transpose()?; let pin_hash_enc = pin_hash_enc.map(extract_byte_string).transpose()?; - #[cfg(feature = "with_ctap2_1")] let min_pin_length = min_pin_length .map(extract_unsigned) .transpose()? .map(u8::try_from) .transpose() .map_err(|_| Ctap2StatusCode::CTAP2_ERR_PIN_POLICY_VIOLATION)?; - #[cfg(feature = "with_ctap2_1")] let min_pin_length_rp_ids = match min_pin_length_rp_ids { Some(entry) => Some( extract_array(entry)? @@ -356,14 +336,12 @@ impl TryFrom for AuthenticatorClientPinParameters { ), None => None, }; - #[cfg(feature = "with_ctap2_1")] // We expect a bit field of 8 bits, and drop everything else. // This means we ignore extensions in future versions. let permissions = permissions .map(extract_unsigned) .transpose()? .map(|p| p as u8); - #[cfg(feature = "with_ctap2_1")] let permissions_rp_id = permissions_rp_id.map(extract_text_string).transpose()?; Ok(AuthenticatorClientPinParameters { @@ -373,13 +351,9 @@ impl TryFrom for AuthenticatorClientPinParameters { pin_auth, new_pin_enc, pin_hash_enc, - #[cfg(feature = "with_ctap2_1")] min_pin_length, - #[cfg(feature = "with_ctap2_1")] min_pin_length_rp_ids, - #[cfg(feature = "with_ctap2_1")] permissions, - #[cfg(feature = "with_ctap2_1")] permissions_rp_id, }) } @@ -560,18 +534,6 @@ mod test { #[test] fn test_from_cbor_client_pin_parameters() { - // TODO(kaczmarczyck) inline the #cfg when #128 is resolved: - // https://github.com/google/OpenSK/issues/128 - #[cfg(not(feature = "with_ctap2_1"))] - let cbor_value = cbor_map! { - 1 => 1, - 2 => ClientPinSubCommand::GetPinRetries, - 3 => cbor_map!{}, - 4 => vec! [0xBB], - 5 => vec! [0xCC], - 6 => vec! [0xDD], - }; - #[cfg(feature = "with_ctap2_1")] let cbor_value = cbor_map! { 1 => 1, 2 => ClientPinSubCommand::GetPinRetries, @@ -594,13 +556,9 @@ mod test { pin_auth: Some(vec![0xBB]), new_pin_enc: Some(vec![0xCC]), pin_hash_enc: Some(vec![0xDD]), - #[cfg(feature = "with_ctap2_1")] min_pin_length: Some(4), - #[cfg(feature = "with_ctap2_1")] min_pin_length_rp_ids: Some(vec!["example.com".to_string()]), - #[cfg(feature = "with_ctap2_1")] permissions: Some(0x03), - #[cfg(feature = "with_ctap2_1")] permissions_rp_id: Some("example.com".to_string()), }; @@ -632,7 +590,6 @@ mod test { assert_eq!(command, Ok(Command::AuthenticatorGetNextAssertion)); } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_deserialize_selection() { let cbor_bytes = [Command::AUTHENTICATOR_SELECTION]; diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index a2b490d4..8081567b 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -704,13 +704,9 @@ pub enum ClientPinSubCommand { SetPin = 0x03, ChangePin = 0x04, GetPinToken = 0x05, - #[cfg(feature = "with_ctap2_1")] GetPinUvAuthTokenUsingUvWithPermissions = 0x06, - #[cfg(feature = "with_ctap2_1")] GetUvRetries = 0x07, - #[cfg(feature = "with_ctap2_1")] SetMinPinLength = 0x08, - #[cfg(feature = "with_ctap2_1")] GetPinUvAuthTokenUsingPinWithPermissions = 0x09, } @@ -731,18 +727,11 @@ impl TryFrom for ClientPinSubCommand { 0x03 => Ok(ClientPinSubCommand::SetPin), 0x04 => Ok(ClientPinSubCommand::ChangePin), 0x05 => Ok(ClientPinSubCommand::GetPinToken), - #[cfg(feature = "with_ctap2_1")] 0x06 => Ok(ClientPinSubCommand::GetPinUvAuthTokenUsingUvWithPermissions), - #[cfg(feature = "with_ctap2_1")] 0x07 => Ok(ClientPinSubCommand::GetUvRetries), - #[cfg(feature = "with_ctap2_1")] 0x08 => Ok(ClientPinSubCommand::SetMinPinLength), - #[cfg(feature = "with_ctap2_1")] 0x09 => Ok(ClientPinSubCommand::GetPinUvAuthTokenUsingPinWithPermissions), - #[cfg(feature = "with_ctap2_1")] _ => Err(Ctap2StatusCode::CTAP2_ERR_INVALID_SUBCOMMAND), - #[cfg(not(feature = "with_ctap2_1"))] - _ => Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER), } } } diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 7a23a1de..4168f8f0 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -25,23 +25,19 @@ pub mod status_code; mod storage; mod timed_permission; -#[cfg(feature = "with_ctap2_1")] -use self::command::MAX_CREDENTIAL_COUNT_IN_LIST; use self::command::{ AuthenticatorClientPinParameters, AuthenticatorGetAssertionParameters, AuthenticatorMakeCredentialParameters, AuthenticatorVendorConfigureParameters, Command, + MAX_CREDENTIAL_COUNT_IN_LIST, }; -#[cfg(feature = "with_ctap2_1")] -use self::data_formats::AuthenticatorTransport; use self::data_formats::{ - CredentialProtectionPolicy, GetAssertionHmacSecretInput, PackedAttestationStatement, - PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialSource, - PublicKeyCredentialType, PublicKeyCredentialUserEntity, SignatureAlgorithm, + AuthenticatorTransport, CredentialProtectionPolicy, GetAssertionHmacSecretInput, + PackedAttestationStatement, PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, + PublicKeyCredentialSource, PublicKeyCredentialType, PublicKeyCredentialUserEntity, + SignatureAlgorithm, }; use self::hid::ChannelID; -#[cfg(feature = "with_ctap2_1")] -use self::pin_protocol_v1::PinPermission; -use self::pin_protocol_v1::PinProtocolV1; +use self::pin_protocol_v1::{PinPermission, PinProtocolV1}; use self::response::{ AuthenticatorGetAssertionResponse, AuthenticatorGetInfoResponse, AuthenticatorMakeCredentialResponse, AuthenticatorVendorResponse, ResponseData, @@ -108,7 +104,6 @@ pub const FIDO2_VERSION_STRING: &str = "FIDO_2_0"; #[cfg(feature = "with_ctap1")] pub const U2F_VERSION_STRING: &str = "U2F_V2"; // TODO(#106) change to final string when ready -#[cfg(feature = "with_ctap2_1")] pub const FIDO2_1_VERSION_STRING: &str = "FIDO_2_1_PRE"; // We currently only support one algorithm for signatures: ES256. @@ -339,7 +334,6 @@ where // GetInfo does not reset stateful commands. (Command::AuthenticatorGetInfo, _) => (), // AuthenticatorSelection does not reset stateful commands. - #[cfg(feature = "with_ctap2_1")] (Command::AuthenticatorSelection, _) => (), (_, _) => { self.stateful_command_type = None; @@ -356,7 +350,6 @@ where Command::AuthenticatorGetInfo => self.process_get_info(), Command::AuthenticatorClientPin(params) => self.process_client_pin(params), Command::AuthenticatorReset => self.process_reset(cid, now), - #[cfg(feature = "with_ctap2_1")] Command::AuthenticatorSelection => self.process_selection(cid), // TODO(kaczmarczyck) implement FIDO 2.1 commands // Vendor specific commands @@ -484,12 +477,9 @@ where { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); } - #[cfg(feature = "with_ctap2_1")] - { - self.pin_protocol_v1 - .has_permission(PinPermission::MakeCredential)?; - self.pin_protocol_v1.has_permission_for_rp_id(&rp_id)?; - } + self.pin_protocol_v1 + .has_permission(PinPermission::MakeCredential)?; + self.pin_protocol_v1.has_permission_for_rp_id(&rp_id)?; UP_FLAG | UV_FLAG | AT_FLAG | ed_flag } None => { @@ -738,12 +728,9 @@ where { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); } - #[cfg(feature = "with_ctap2_1")] - { - self.pin_protocol_v1 - .has_permission(PinPermission::GetAssertion)?; - self.pin_protocol_v1.has_permission_for_rp_id(&rp_id)?; - } + self.pin_protocol_v1 + .has_permission(PinPermission::GetAssertion)?; + self.pin_protocol_v1.has_permission_for_rp_id(&rp_id)?; UV_FLAG } None => { @@ -851,7 +838,6 @@ where #[cfg(feature = "with_ctap1")] String::from(U2F_VERSION_STRING), String::from(FIDO2_VERSION_STRING), - #[cfg(feature = "with_ctap2_1")] String::from(FIDO2_1_VERSION_STRING), ], extensions: Some(vec![String::from("hmac-secret")]), @@ -861,19 +847,13 @@ where pin_protocols: Some(vec![ CtapState::::PIN_PROTOCOL_VERSION, ]), - #[cfg(feature = "with_ctap2_1")] max_credential_count_in_list: MAX_CREDENTIAL_COUNT_IN_LIST.map(|c| c as u64), // #TODO(106) update with version 2.1 of HMAC-secret - #[cfg(feature = "with_ctap2_1")] max_credential_id_length: Some(CREDENTIAL_ID_SIZE as u64), - #[cfg(feature = "with_ctap2_1")] transports: Some(vec![AuthenticatorTransport::Usb]), - #[cfg(feature = "with_ctap2_1")] algorithms: Some(vec![ES256_CRED_PARAM]), default_cred_protect: DEFAULT_CRED_PROTECT, - #[cfg(feature = "with_ctap2_1")] min_pin_length: self.persistent_store.min_pin_length()?, - #[cfg(feature = "with_ctap2_1")] firmware_version: None, }, )) @@ -916,7 +896,6 @@ where Ok(ResponseData::AuthenticatorReset) } - #[cfg(feature = "with_ctap2_1")] fn process_selection(&self, cid: ChannelID) -> Result { (self.check_user_presence)(cid)?; Ok(ResponseData::AuthenticatorSelection) @@ -1036,42 +1015,31 @@ mod test { let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); - #[cfg(feature = "with_ctap2_1")] let mut expected_response = vec![0x00, 0xAA, 0x01]; - #[cfg(not(feature = "with_ctap2_1"))] - let mut expected_response = vec![0x00, 0xA6, 0x01]; // The difference here is a longer array of supported versions. let mut version_count = 0; - // CTAP 2 is always supported - version_count += 1; + // CTAP 2.0 and 2.1 are always supported + version_count += 2; #[cfg(feature = "with_ctap1")] { version_count += 1; } - #[cfg(feature = "with_ctap2_1")] - { - version_count += 1; - } expected_response.push(0x80 + version_count); #[cfg(feature = "with_ctap1")] expected_response.extend(&[0x66, 0x55, 0x32, 0x46, 0x5F, 0x56, 0x32]); - expected_response.extend(&[0x68, 0x46, 0x49, 0x44, 0x4F, 0x5F, 0x32, 0x5F, 0x30]); - #[cfg(feature = "with_ctap2_1")] - expected_response.extend(&[ - 0x6C, 0x46, 0x49, 0x44, 0x4F, 0x5F, 0x32, 0x5F, 0x31, 0x5F, 0x50, 0x52, 0x45, - ]); - expected_response.extend(&[ - 0x02, 0x81, 0x6B, 0x68, 0x6D, 0x61, 0x63, 0x2D, 0x73, 0x65, 0x63, 0x72, 0x65, 0x74, - 0x03, 0x50, - ]); + expected_response.extend( + [ + 0x68, 0x46, 0x49, 0x44, 0x4F, 0x5F, 0x32, 0x5F, 0x30, 0x6C, 0x46, 0x49, 0x44, 0x4F, + 0x5F, 0x32, 0x5F, 0x31, 0x5F, 0x50, 0x52, 0x45, 0x02, 0x81, 0x6B, 0x68, 0x6D, 0x61, + 0x63, 0x2D, 0x73, 0x65, 0x63, 0x72, 0x65, 0x74, 0x03, 0x50, + ] + .iter(), + ); expected_response.extend(&ctap_state.persistent_store.aaguid().unwrap()); - expected_response.extend(&[ - 0x04, 0xA3, 0x62, 0x72, 0x6B, 0xF5, 0x62, 0x75, 0x70, 0xF5, 0x69, 0x63, 0x6C, 0x69, - 0x65, 0x6E, 0x74, 0x50, 0x69, 0x6E, 0xF4, 0x05, 0x19, 0x04, 0x00, 0x06, 0x81, 0x01, - ]); - #[cfg(feature = "with_ctap2_1")] expected_response.extend( [ + 0x04, 0xA3, 0x62, 0x72, 0x6B, 0xF5, 0x62, 0x75, 0x70, 0xF5, 0x69, 0x63, 0x6C, 0x69, + 0x65, 0x6E, 0x74, 0x50, 0x69, 0x6E, 0xF4, 0x05, 0x19, 0x04, 0x00, 0x06, 0x81, 0x01, 0x08, 0x18, 0x70, 0x09, 0x81, 0x63, 0x75, 0x73, 0x62, 0x0A, 0x81, 0xA2, 0x63, 0x61, 0x6C, 0x67, 0x26, 0x64, 0x74, 0x79, 0x70, 0x65, 0x6A, 0x70, 0x75, 0x62, 0x6C, 0x69, 0x63, 0x2D, 0x6B, 0x65, 0x79, 0x0D, 0x04, diff --git a/src/ctap/pin_protocol_v1.rs b/src/ctap/pin_protocol_v1.rs index 96e92b3e..b8aeb21d 100644 --- a/src/ctap/pin_protocol_v1.rs +++ b/src/ctap/pin_protocol_v1.rs @@ -17,7 +17,6 @@ use super::data_formats::{ClientPinSubCommand, CoseKey, GetAssertionHmacSecretIn use super::response::{AuthenticatorClientPinResponse, ResponseData}; use super::status_code::Ctap2StatusCode; use super::storage::PersistentStore; -#[cfg(feature = "with_ctap2_1")] use alloc::string::String; use alloc::vec; use alloc::vec::Vec; @@ -28,7 +27,7 @@ use crypto::hmac::{hmac_256, verify_hmac_256_first_128bits}; use crypto::rng256::Rng256; use crypto::sha256::Sha256; use crypto::Hash256; -#[cfg(all(test, feature = "with_ctap2_1"))] +#[cfg(test)] use enum_iterator::IntoEnumIterator; use subtle::ConstantTimeEq; @@ -141,10 +140,7 @@ fn check_and_store_new_pin( let pin = decrypt_pin(aes_dec_key, new_pin_enc) .ok_or(Ctap2StatusCode::CTAP2_ERR_PIN_POLICY_VIOLATION)?; - #[cfg(feature = "with_ctap2_1")] let min_pin_length = persistent_store.min_pin_length()? as usize; - #[cfg(not(feature = "with_ctap2_1"))] - let min_pin_length = 4; if pin.len() < min_pin_length || pin.len() == PIN_PADDED_LENGTH { // TODO(kaczmarczyck) check 4 code point minimum instead return Err(Ctap2StatusCode::CTAP2_ERR_PIN_POLICY_VIOLATION); @@ -155,7 +151,6 @@ fn check_and_store_new_pin( Ok(()) } -#[cfg(feature = "with_ctap2_1")] #[cfg_attr(test, derive(IntoEnumIterator))] // TODO remove when all variants are used #[allow(dead_code)] @@ -173,9 +168,7 @@ pub struct PinProtocolV1 { key_agreement_key: crypto::ecdh::SecKey, pin_uv_auth_token: [u8; PIN_TOKEN_LENGTH], consecutive_pin_mismatches: u8, - #[cfg(feature = "with_ctap2_1")] permissions: u8, - #[cfg(feature = "with_ctap2_1")] permissions_rp_id: Option, } @@ -187,9 +180,7 @@ impl PinProtocolV1 { key_agreement_key, pin_uv_auth_token, consecutive_pin_mismatches: 0, - #[cfg(feature = "with_ctap2_1")] permissions: 0, - #[cfg(feature = "with_ctap2_1")] permissions_rp_id: None, } } @@ -345,11 +336,8 @@ impl PinProtocolV1 { cbc_encrypt(&token_encryption_key, iv, &mut blocks); let pin_token: Vec = blocks.iter().flatten().cloned().collect(); - #[cfg(feature = "with_ctap2_1")] - { - self.permissions = 0x03; - self.permissions_rp_id = None; - } + self.permissions = 0x03; + self.permissions_rp_id = None; Ok(AuthenticatorClientPinResponse { key_agreement: None, @@ -358,7 +346,6 @@ impl PinProtocolV1 { }) } - #[cfg(feature = "with_ctap2_1")] fn process_get_pin_uv_auth_token_using_uv_with_permissions( &self, // If you want to support local user verification, implement this function. @@ -368,30 +355,14 @@ impl PinProtocolV1 { _permissions_rp_id: Option, ) -> Result { // User verifications is only supported through PIN currently. - #[cfg(not(feature = "with_ctap2_1"))] - { - Err(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND) - } - #[cfg(feature = "with_ctap2_1")] - { - Err(Ctap2StatusCode::CTAP2_ERR_INVALID_SUBCOMMAND) - } + Err(Ctap2StatusCode::CTAP2_ERR_INVALID_SUBCOMMAND) } - #[cfg(feature = "with_ctap2_1")] fn process_get_uv_retries(&self) -> Result { // User verifications is only supported through PIN currently. - #[cfg(not(feature = "with_ctap2_1"))] - { - Err(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND) - } - #[cfg(feature = "with_ctap2_1")] - { - Err(Ctap2StatusCode::CTAP2_ERR_INVALID_SUBCOMMAND) - } + Err(Ctap2StatusCode::CTAP2_ERR_INVALID_SUBCOMMAND) } - #[cfg(feature = "with_ctap2_1")] fn process_set_min_pin_length( &mut self, persistent_store: &mut PersistentStore, @@ -440,7 +411,6 @@ impl PinProtocolV1 { Ok(()) } - #[cfg(feature = "with_ctap2_1")] fn process_get_pin_uv_auth_token_using_pin_with_permissions( &mut self, rng: &mut impl Rng256, @@ -480,20 +450,13 @@ impl PinProtocolV1 { pin_auth, new_pin_enc, pin_hash_enc, - #[cfg(feature = "with_ctap2_1")] min_pin_length, - #[cfg(feature = "with_ctap2_1")] min_pin_length_rp_ids, - #[cfg(feature = "with_ctap2_1")] permissions, - #[cfg(feature = "with_ctap2_1")] permissions_rp_id, } = client_pin_params; if pin_protocol != 1 { - #[cfg(not(feature = "with_ctap2_1"))] - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); - #[cfg(feature = "with_ctap2_1")] return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } @@ -528,7 +491,6 @@ impl PinProtocolV1 { key_agreement.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, pin_hash_enc.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, )?), - #[cfg(feature = "with_ctap2_1")] ClientPinSubCommand::GetPinUvAuthTokenUsingUvWithPermissions => Some( self.process_get_pin_uv_auth_token_using_uv_with_permissions( key_agreement.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?, @@ -536,9 +498,7 @@ impl PinProtocolV1 { permissions_rp_id, )?, ), - #[cfg(feature = "with_ctap2_1")] ClientPinSubCommand::GetUvRetries => Some(self.process_get_uv_retries()?), - #[cfg(feature = "with_ctap2_1")] ClientPinSubCommand::SetMinPinLength => { self.process_set_min_pin_length( persistent_store, @@ -548,7 +508,6 @@ impl PinProtocolV1 { )?; None } - #[cfg(feature = "with_ctap2_1")] ClientPinSubCommand::GetPinUvAuthTokenUsingPinWithPermissions => Some( self.process_get_pin_uv_auth_token_using_pin_with_permissions( rng, @@ -571,11 +530,8 @@ impl PinProtocolV1 { self.key_agreement_key = crypto::ecdh::SecKey::gensk(rng); self.pin_uv_auth_token = rng.gen_uniform_u8x32(); self.consecutive_pin_mismatches = 0; - #[cfg(feature = "with_ctap2_1")] - { - self.permissions = 0; - self.permissions_rp_id = None; - } + self.permissions = 0; + self.permissions_rp_id = None; } pub fn process_hmac_secret( @@ -598,7 +554,6 @@ impl PinProtocolV1 { encrypt_hmac_secret_output(&shared_secret, &salt_enc[..], cred_random) } - #[cfg(feature = "with_ctap2_1")] pub fn has_permission(&self, permission: PinPermission) -> Result<(), Ctap2StatusCode> { // Relies on the fact that all permissions are represented by powers of two. if permission as u8 & self.permissions != 0 { @@ -608,7 +563,6 @@ impl PinProtocolV1 { } } - #[cfg(feature = "with_ctap2_1")] pub fn has_permission_for_rp_id(&mut self, rp_id: &str) -> Result<(), Ctap2StatusCode> { if let Some(permissions_rp_id) = &self.permissions_rp_id { if rp_id != permissions_rp_id { @@ -629,9 +583,7 @@ impl PinProtocolV1 { key_agreement_key, pin_uv_auth_token, consecutive_pin_mismatches: 0, - #[cfg(feature = "with_ctap2_1")] permissions: 0xFF, - #[cfg(feature = "with_ctap2_1")] permissions_rp_id: None, } } @@ -919,7 +871,6 @@ mod test { ); } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_process_get_pin_uv_auth_token_using_pin_with_permissions() { let mut rng = ThreadRng256 {}; @@ -963,7 +914,7 @@ mod test { &mut rng, &mut persistent_store, key_agreement.clone(), - pin_hash_enc.clone(), + pin_hash_enc, 0x03, None, ), @@ -984,7 +935,6 @@ mod test { ); } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_process_set_min_pin_length() { let mut rng = ThreadRng256 {}; @@ -1031,13 +981,9 @@ mod test { pin_auth: None, new_pin_enc: None, pin_hash_enc: None, - #[cfg(feature = "with_ctap2_1")] min_pin_length: None, - #[cfg(feature = "with_ctap2_1")] min_pin_length_rp_ids: None, - #[cfg(feature = "with_ctap2_1")] permissions: None, - #[cfg(feature = "with_ctap2_1")] permissions_rp_id: None, }; assert!(pin_protocol_v1 @@ -1051,18 +997,11 @@ mod test { pin_auth: None, new_pin_enc: None, pin_hash_enc: None, - #[cfg(feature = "with_ctap2_1")] min_pin_length: None, - #[cfg(feature = "with_ctap2_1")] min_pin_length_rp_ids: None, - #[cfg(feature = "with_ctap2_1")] permissions: None, - #[cfg(feature = "with_ctap2_1")] permissions_rp_id: None, }; - #[cfg(not(feature = "with_ctap2_1"))] - let error_code = Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID; - #[cfg(feature = "with_ctap2_1")] let error_code = Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER; assert_eq!( pin_protocol_v1.process_subcommand(&mut rng, &mut persistent_store, client_pin_params), @@ -1231,7 +1170,6 @@ mod test { assert_eq!(&output_dec[..32], &expected_output1); } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_has_permission() { let mut rng = ThreadRng256 {}; @@ -1249,7 +1187,6 @@ mod test { } } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_has_permission_for_rp_id() { let mut rng = ThreadRng256 {}; diff --git a/src/ctap/response.rs b/src/ctap/response.rs index 64229596..390b0cbf 100644 --- a/src/ctap/response.rs +++ b/src/ctap/response.rs @@ -12,11 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -#[cfg(feature = "with_ctap2_1")] -use super::data_formats::{AuthenticatorTransport, PublicKeyCredentialParameter}; use super::data_formats::{ - CoseKey, CredentialProtectionPolicy, PackedAttestationStatement, PublicKeyCredentialDescriptor, - PublicKeyCredentialUserEntity, + AuthenticatorTransport, CoseKey, CredentialProtectionPolicy, PackedAttestationStatement, + PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialUserEntity, }; use alloc::collections::BTreeMap; use alloc::string::String; @@ -32,7 +30,6 @@ pub enum ResponseData { AuthenticatorGetInfo(AuthenticatorGetInfoResponse), AuthenticatorClientPin(Option), AuthenticatorReset, - #[cfg(feature = "with_ctap2_1")] AuthenticatorSelection, AuthenticatorVendor(AuthenticatorVendorResponse), } @@ -47,7 +44,6 @@ impl From for Option { ResponseData::AuthenticatorClientPin(Some(data)) => Some(data.into()), ResponseData::AuthenticatorClientPin(None) => None, ResponseData::AuthenticatorReset => None, - #[cfg(feature = "with_ctap2_1")] ResponseData::AuthenticatorSelection => None, ResponseData::AuthenticatorVendor(data) => Some(data.into()), } @@ -118,23 +114,16 @@ pub struct AuthenticatorGetInfoResponse { pub options: Option>, pub max_msg_size: Option, pub pin_protocols: Option>, - #[cfg(feature = "with_ctap2_1")] pub max_credential_count_in_list: Option, - #[cfg(feature = "with_ctap2_1")] pub max_credential_id_length: Option, - #[cfg(feature = "with_ctap2_1")] pub transports: Option>, - #[cfg(feature = "with_ctap2_1")] pub algorithms: Option>, pub default_cred_protect: Option, - #[cfg(feature = "with_ctap2_1")] pub min_pin_length: u8, - #[cfg(feature = "with_ctap2_1")] pub firmware_version: Option, } impl From for cbor::Value { - #[cfg(feature = "with_ctap2_1")] fn from(get_info_response: AuthenticatorGetInfoResponse) -> Self { let AuthenticatorGetInfoResponse { versions, @@ -176,37 +165,6 @@ impl From for cbor::Value { 0x0E => firmware_version, } } - - #[cfg(not(feature = "with_ctap2_1"))] - fn from(get_info_response: AuthenticatorGetInfoResponse) -> Self { - let AuthenticatorGetInfoResponse { - versions, - extensions, - aaguid, - options, - max_msg_size, - pin_protocols, - default_cred_protect, - } = get_info_response; - - let options_cbor: Option = options.map(|options| { - let option_map: BTreeMap<_, _> = options - .into_iter() - .map(|(key, value)| (cbor_text!(key), cbor_bool!(value))) - .collect(); - cbor_map_btree!(option_map) - }); - - cbor_map_options! { - 0x01 => cbor_array_vec!(versions), - 0x02 => extensions.map(|vec| cbor_array_vec!(vec)), - 0x03 => &aaguid, - 0x04 => options_cbor, - 0x05 => max_msg_size, - 0x06 => pin_protocols.map(|vec| cbor_array_vec!(vec)), - 0x0C => default_cred_protect.map(|p| p as u64), - } - } } #[cfg_attr(test, derive(PartialEq))] @@ -257,7 +215,6 @@ impl From for cbor::Value { #[cfg(test)] mod test { use super::super::data_formats::PackedAttestationStatement; - #[cfg(feature = "with_ctap2_1")] use super::super::ES256_CRED_PARAM; use super::*; use cbor::{cbor_bytes, cbor_map}; @@ -321,28 +278,16 @@ mod test { options: None, max_msg_size: None, pin_protocols: None, - #[cfg(feature = "with_ctap2_1")] max_credential_count_in_list: None, - #[cfg(feature = "with_ctap2_1")] max_credential_id_length: None, - #[cfg(feature = "with_ctap2_1")] transports: None, - #[cfg(feature = "with_ctap2_1")] algorithms: None, default_cred_protect: None, - #[cfg(feature = "with_ctap2_1")] min_pin_length: 4, - #[cfg(feature = "with_ctap2_1")] firmware_version: None, }; let response_cbor: Option = ResponseData::AuthenticatorGetInfo(get_info_response).into(); - #[cfg(not(feature = "with_ctap2_1"))] - let expected_cbor = cbor_map_options! { - 0x01 => cbor_array_vec![versions], - 0x03 => vec![0x00; 16], - }; - #[cfg(feature = "with_ctap2_1")] let expected_cbor = cbor_map_options! { 0x01 => cbor_array_vec![versions], 0x03 => vec![0x00; 16], @@ -352,7 +297,6 @@ mod test { } #[test] - #[cfg(feature = "with_ctap2_1")] fn test_get_info_optionals_into_cbor() { let mut options_map = BTreeMap::new(); options_map.insert(String::from("rk"), true); @@ -418,7 +362,6 @@ mod test { assert_eq!(response_cbor, None); } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_selection_into_cbor() { let response_cbor: Option = ResponseData::AuthenticatorSelection.into(); diff --git a/src/ctap/status_code.rs b/src/ctap/status_code.rs index 40f258ef..5a9ec71d 100644 --- a/src/ctap/status_code.rs +++ b/src/ctap/status_code.rs @@ -31,9 +31,7 @@ pub enum Ctap2StatusCode { CTAP2_ERR_INVALID_CBOR = 0x12, CTAP2_ERR_MISSING_PARAMETER = 0x14, CTAP2_ERR_LIMIT_EXCEEDED = 0x15, - #[cfg(feature = "with_ctap2_1")] CTAP2_ERR_FP_DATABASE_FULL = 0x17, - #[cfg(feature = "with_ctap2_1")] CTAP2_ERR_LARGE_BLOB_STORAGE_FULL = 0x18, CTAP2_ERR_CREDENTIAL_EXCLUDED = 0x19, CTAP2_ERR_PROCESSING = 0x21, @@ -63,13 +61,9 @@ pub enum Ctap2StatusCode { CTAP2_ERR_ACTION_TIMEOUT = 0x3A, CTAP2_ERR_UP_REQUIRED = 0x3B, CTAP2_ERR_UV_BLOCKED = 0x3C, - #[cfg(feature = "with_ctap2_1")] CTAP2_ERR_INTEGRITY_FAILURE = 0x3D, - #[cfg(feature = "with_ctap2_1")] CTAP2_ERR_INVALID_SUBCOMMAND = 0x3E, - #[cfg(feature = "with_ctap2_1")] CTAP2_ERR_UV_INVALID = 0x3F, - #[cfg(feature = "with_ctap2_1")] CTAP2_ERR_UNAUTHORIZED_PERMISSION = 0x40, CTAP1_ERR_OTHER = 0x7F, _CTAP2_ERR_SPEC_LAST = 0xDF, diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 73bbc162..28c1599b 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -14,20 +14,18 @@ mod key; -#[cfg(feature = "with_ctap2_1")] -use crate::ctap::data_formats::{extract_array, extract_text_string}; -use crate::ctap::data_formats::{CredentialProtectionPolicy, PublicKeyCredentialSource}; +use crate::ctap::data_formats::{ + extract_array, extract_text_string, CredentialProtectionPolicy, PublicKeyCredentialSource, +}; 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::embedded_flash::{new_storage, Storage}; -#[cfg(feature = "with_ctap2_1")] use alloc::string::String; use alloc::vec; use alloc::vec::Vec; use arrayref::array_ref; -#[cfg(feature = "with_ctap2_1")] use cbor::cbor_array_vec; use core::convert::TryInto; use crypto::rng256::Rng256; @@ -54,14 +52,11 @@ const NUM_PAGES: usize = 20; const MAX_SUPPORTED_RESIDENTIAL_KEYS: usize = 150; const MAX_PIN_RETRIES: u8 = 8; -#[cfg(feature = "with_ctap2_1")] const DEFAULT_MIN_PIN_LENGTH: u8 = 4; // TODO(kaczmarczyck) use this for the minPinLength extension // https://github.com/google/OpenSK/issues/129 -#[cfg(feature = "with_ctap2_1")] const _DEFAULT_MIN_PIN_LENGTH_RP_IDS: Vec = Vec::new(); // TODO(kaczmarczyck) Check whether this constant is necessary, or replace it accordingly. -#[cfg(feature = "with_ctap2_1")] const _MAX_RP_IDS_LENGTH: usize = 8; /// Wrapper for master keys. @@ -348,7 +343,6 @@ impl PersistentStore { } /// Returns the minimum PIN length. - #[cfg(feature = "with_ctap2_1")] pub fn min_pin_length(&self) -> Result { match self.store.find(key::MIN_PIN_LENGTH)? { None => Ok(DEFAULT_MIN_PIN_LENGTH), @@ -358,14 +352,12 @@ impl PersistentStore { } /// Sets the minimum PIN length. - #[cfg(feature = "with_ctap2_1")] pub fn set_min_pin_length(&mut self, min_pin_length: u8) -> Result<(), Ctap2StatusCode> { Ok(self.store.insert(key::MIN_PIN_LENGTH, &[min_pin_length])?) } /// Returns the list of RP IDs that are used to check if reading the minimum PIN length is /// allowed. - #[cfg(feature = "with_ctap2_1")] pub fn _min_pin_length_rp_ids(&self) -> Result, Ctap2StatusCode> { let rp_ids = self .store @@ -374,11 +366,10 @@ impl PersistentStore { _deserialize_min_pin_length_rp_ids(&value) }); debug_assert!(rp_ids.is_some()); - Ok(rp_ids.unwrap_or(vec![])) + Ok(rp_ids.unwrap_or_default()) } /// Sets the list of RP IDs that are used to check if reading the minimum PIN length is allowed. - #[cfg(feature = "with_ctap2_1")] pub fn _set_min_pin_length_rp_ids( &mut self, min_pin_length_rp_ids: Vec, @@ -582,7 +573,6 @@ fn serialize_credential(credential: PublicKeyCredentialSource) -> Result } /// Deserializes a list of RP IDs from storage representation. -#[cfg(feature = "with_ctap2_1")] fn _deserialize_min_pin_length_rp_ids(data: &[u8]) -> Option> { let cbor = cbor::read(data).ok()?; extract_array(cbor) @@ -594,7 +584,6 @@ fn _deserialize_min_pin_length_rp_ids(data: &[u8]) -> Option> { } /// Serializes a list of RP IDs to storage representation. -#[cfg(feature = "with_ctap2_1")] fn _serialize_min_pin_length_rp_ids(rp_ids: Vec) -> Result, Ctap2StatusCode> { let mut data = Vec::new(); if cbor::write(cbor_array_vec!(rp_ids), &mut data) { @@ -988,7 +977,6 @@ mod test { assert_eq!(&persistent_store.aaguid().unwrap(), key_material::AAGUID); } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_min_pin_length() { let mut rng = ThreadRng256 {}; @@ -1011,7 +999,6 @@ mod test { ); } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_min_pin_length_rp_ids() { let mut rng = ThreadRng256 {}; @@ -1080,7 +1067,6 @@ mod test { assert_eq!(credential, reconstructed); } - #[cfg(feature = "with_ctap2_1")] #[test] fn test_serialize_deserialize_min_pin_length_rp_ids() { let rp_ids = vec![String::from("example.com")]; diff --git a/src/ctap/storage/key.rs b/src/ctap/storage/key.rs index 5c5b20e6..ec39efa1 100644 --- a/src/ctap/storage/key.rs +++ b/src/ctap/storage/key.rs @@ -92,13 +92,11 @@ make_partition! { CRED_RANDOM_SECRET = 2041; /// List of RP IDs allowed to read the minimum PIN length. - #[cfg(feature = "with_ctap2_1")] _MIN_PIN_LENGTH_RP_IDS = 2042; /// The minimum PIN length. /// /// If the entry is absent, the minimum PIN length is `DEFAULT_MIN_PIN_LENGTH`. - #[cfg(feature = "with_ctap2_1")] MIN_PIN_LENGTH = 2043; /// The number of PIN retries. From da03f77a32e059491ee8e1e2b4a3a0b027e8fa45 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 8 Jan 2021 13:13:52 +0100 Subject: [PATCH 2/2] small readbility fix for variable assignment with cfg --- src/ctap/mod.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 4168f8f0..fe60e80e 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -1016,14 +1016,11 @@ mod test { let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); let mut expected_response = vec![0x00, 0xAA, 0x01]; - // The difference here is a longer array of supported versions. - let mut version_count = 0; - // CTAP 2.0 and 2.1 are always supported - version_count += 2; + // The version array differs with CTAP1, always including 2.0 and 2.1. + #[cfg(not(feature = "with_ctap1"))] + let version_count = 2; #[cfg(feature = "with_ctap1")] - { - version_count += 1; - } + let version_count = 3; expected_response.push(0x80 + version_count); #[cfg(feature = "with_ctap1")] expected_response.extend(&[0x66, 0x55, 0x32, 0x46, 0x5F, 0x56, 0x32]);