From 8f20a75b17dde0096163209a9532c33038a834dd Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 17 Apr 2020 17:13:21 +0200 Subject: [PATCH 01/13] add 2.1 features to GetInfo --- README.md | 4 ++- src/ctap/command.rs | 45 ++++++++++++++++++------------ src/ctap/data_formats.rs | 43 +++++++++++++++++++++++++++- src/ctap/mod.rs | 60 +++++++++++++++++++++------------------- src/ctap/response.rs | 36 ++++++++++++++++++------ 5 files changed, 132 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 3f71d71..fb7d1c9 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,9 @@ few limitations: Although we tested and implemented our firmware based on 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. +be FIDO Certified. With the upcoming next version of the +[CTAP2.1 specifications](https://fidoalliance.org/specs/fido2/fido-client-to-authenticator-protocol-v2.1-rd-20191217.html), +we started adding features, so master is currently between version 2.0 and 2.1. ### Cryptography diff --git a/src/ctap/command.rs b/src/ctap/command.rs index 129c671..9ccf104 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -13,16 +13,20 @@ // limitations under the License. use super::data_formats::{ - ok_or_missing, read_array, read_byte_string, read_integer, read_map, read_text_string, - read_unsigned, ClientPinSubCommand, CoseKey, Extensions, GetAssertionOptions, - MakeCredentialOptions, PublicKeyCredentialDescriptor, PublicKeyCredentialRpEntity, - PublicKeyCredentialType, PublicKeyCredentialUserEntity, + ok_or_missing, read_array, read_byte_string, read_map, read_text_string, read_unsigned, + ClientPinSubCommand, CoseKey, Extensions, GetAssertionOptions, MakeCredentialOptions, + PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialRpEntity, + PublicKeyCredentialUserEntity, }; use super::status_code::Ctap2StatusCode; use alloc::string::String; use alloc::vec::Vec; use core::convert::TryFrom; +// Depending on your memory, you can use Some(n) to limit request sizes. +// You might also want to set the max credential size in process_get_info then. +pub const MAX_CREDENTIAL_COUNT_IN_LIST: Option = None; + // CTAP specification (version 20190130) section 6.1 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub enum Command { @@ -106,7 +110,7 @@ pub struct AuthenticatorMakeCredentialParameters { pub client_data_hash: Vec, pub rp: PublicKeyCredentialRpEntity, pub user: PublicKeyCredentialUserEntity, - pub pub_key_cred_params: Vec<(PublicKeyCredentialType, i64)>, + pub pub_key_cred_params: Vec, pub exclude_list: Option>, pub extensions: Option, // Even though options are optional, we can use the default if not present. @@ -134,12 +138,9 @@ impl TryFrom for AuthenticatorMakeCredentialParameters { let cred_param_vec = read_array(ok_or_missing(param_map.get(&cbor_unsigned!(4)))?)?; let mut pub_key_cred_params = vec![]; for cred_param_map_value in cred_param_vec { - let cred_param_map = read_map(cred_param_map_value)?; - let cred_type = PublicKeyCredentialType::try_from(ok_or_missing( - cred_param_map.get(&cbor_text!("type")), - )?)?; - let alg = read_integer(ok_or_missing(cred_param_map.get(&cbor_text!("alg")))?)?; - pub_key_cred_params.push((cred_type, alg)); + if let Ok(cred_param) = PublicKeyCredentialParameter::try_from(cred_param_map_value) { + pub_key_cred_params.push(cred_param); + } } let exclude_list = match param_map.get(&cbor_unsigned!(5)) { @@ -147,6 +148,11 @@ impl TryFrom for AuthenticatorMakeCredentialParameters { let exclude_list_vec = read_array(entry)?; let mut exclude_list = vec![]; for exclude_list_value in exclude_list_vec { + if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST { + if exclude_list.len() as u64 >= count { + break; + } + } exclude_list.push(PublicKeyCredentialDescriptor::try_from(exclude_list_value)?); } Some(exclude_list) @@ -218,6 +224,11 @@ impl TryFrom for AuthenticatorGetAssertionParameters { let allow_list_vec = read_array(entry)?; let mut allow_list = vec![]; for allow_list_value in allow_list_vec { + if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST { + if allow_list.len() as u64 >= count { + break; + } + } allow_list.push(PublicKeyCredentialDescriptor::try_from(allow_list_value)?); } Some(allow_list) @@ -316,8 +327,10 @@ impl TryFrom for AuthenticatorClientPinParameters { #[cfg(test)] mod test { use super::super::data_formats::{ - AuthenticatorTransport, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, + AuthenticatorTransport, PublicKeyCredentialRpEntity, PublicKeyCredentialType, + PublicKeyCredentialUserEntity, }; + use super::super::CREDENTIAL_PARAMETER; use super::*; use alloc::collections::BTreeMap; @@ -336,10 +349,7 @@ mod test { "displayName" => "bar", "icon" => "example.com/foo/icon.png", }, - 4 => cbor_array![ cbor_map! { - "type" => "public-key", - "alg" => -7 - } ], + 4 => cbor_array![CREDENTIAL_PARAMETER], 5 => cbor_array![], 8 => vec![0x12, 0x34], 9 => 1, @@ -362,7 +372,6 @@ mod test { user_display_name: Some("bar".to_string()), user_icon: Some("example.com/foo/icon.png".to_string()), }; - let pub_key_cred_param = (PublicKeyCredentialType::PublicKey, -7); let options = MakeCredentialOptions { rk: false, uv: false, @@ -371,7 +380,7 @@ mod test { client_data_hash, rp, user, - pub_key_cred_params: vec![pub_key_cred_param], + pub_key_cred_params: vec![CREDENTIAL_PARAMETER], exclude_list: Some(vec![]), extensions: None, options, diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 3a2bed9..a4bbe39 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -121,6 +121,36 @@ impl TryFrom<&cbor::Value> for PublicKeyCredentialType { } } +#[derive(PartialEq)] +#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] +pub struct PublicKeyCredentialParameter { + pub cred_type: PublicKeyCredentialType, + pub alg: SignatureAlgorithm, +} + +impl TryFrom<&cbor::Value> for PublicKeyCredentialParameter { + type Error = Ctap2StatusCode; + + fn try_from(cbor_value: &cbor::Value) -> Result { + let cred_param_map = read_map(cbor_value)?; + let cred_type = PublicKeyCredentialType::try_from(ok_or_missing( + cred_param_map.get(&cbor_text!("type")), + )?)?; + let alg = + SignatureAlgorithm::try_from(ok_or_missing(cred_param_map.get(&cbor_text!("alg")))?)?; + Ok(Self { cred_type, alg }) + } +} + +impl From for cbor::Value { + fn from(cred_param: PublicKeyCredentialParameter) -> Self { + cbor_map_options! { + "type" => cred_param.cred_type, + "alg" => cred_param.alg as i64, + } + } +} + #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub enum AuthenticatorTransport { Usb, @@ -369,12 +399,23 @@ impl From for cbor::Value { } } -#[cfg_attr(test, derive(PartialEq))] +#[derive(PartialEq)] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] pub enum SignatureAlgorithm { ES256 = ecdsa::PubKey::ES256_ALGORITHM as isize, } +impl TryFrom<&cbor::Value> for SignatureAlgorithm { + type Error = Ctap2StatusCode; + + fn try_from(cbor_value: &cbor::Value) -> Result { + match read_integer(cbor_value)? { + ecdsa::PubKey::ES256_ALGORITHM => Ok(SignatureAlgorithm::ES256), + _ => Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM), + } + } +} + #[derive(Clone)] #[cfg_attr(test, derive(PartialEq))] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index e4a1151..7624435 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -25,12 +25,13 @@ mod timed_permission; use self::command::{ AuthenticatorClientPinParameters, AuthenticatorGetAssertionParameters, - AuthenticatorMakeCredentialParameters, Command, + AuthenticatorMakeCredentialParameters, Command, MAX_CREDENTIAL_COUNT_IN_LIST, }; use self::data_formats::{ - ClientPinSubCommand, CoseKey, GetAssertionHmacSecretInput, PackedAttestationStatement, - PublicKeyCredentialDescriptor, PublicKeyCredentialSource, PublicKeyCredentialType, - PublicKeyCredentialUserEntity, SignatureAlgorithm, + AuthenticatorTransport, ClientPinSubCommand, CoseKey, GetAssertionHmacSecretInput, + PackedAttestationStatement, PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, + PublicKeyCredentialSource, PublicKeyCredentialType, PublicKeyCredentialUserEntity, + SignatureAlgorithm, }; use self::hid::ChannelID; use self::key_material::{AAGUID, ATTESTATION_CERTIFICATE, ATTESTATION_PRIVATE_KEY}; @@ -99,6 +100,11 @@ pub const FIDO2_VERSION_STRING: &str = "FIDO_2_0"; #[cfg(feature = "with_ctap1")] pub const U2F_VERSION_STRING: &str = "U2F_V2"; +pub const CREDENTIAL_PARAMETER: PublicKeyCredentialParameter = PublicKeyCredentialParameter { + cred_type: PublicKeyCredentialType::PublicKey, + alg: SignatureAlgorithm::ES256, +}; + fn check_pin_auth(hmac_key: &[u8], hmac_contents: &[u8], pin_auth: &[u8]) -> bool { if pin_auth.len() != PIN_AUTH_LENGTH { return false; @@ -413,15 +419,7 @@ where } } - let has_es_256 = pub_key_cred_params - .iter() - .any(|(credential_type, algorithm)| { - // Even though there is only one type now, checking seems safer in - // case of extension so you can't forget to update here. - *credential_type == PublicKeyCredentialType::PublicKey - && *algorithm == SignatureAlgorithm::ES256 as i64 - }); - if !has_es_256 { + if !pub_key_cred_params.contains(&CREDENTIAL_PARAMETER) { return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); } @@ -751,7 +749,7 @@ where fn process_get_info(&self) -> Result { let mut options_map = BTreeMap::new(); - // TODO(kaczmarczyck) add FIDO 2.1 options + // TODO(kaczmarczyck) add authenticatorConfig and credProtect options options_map.insert(String::from("rk"), true); options_map.insert(String::from("up"), true); options_map.insert( @@ -772,6 +770,13 @@ where pin_protocols: Some(vec![ CtapState::::PIN_PROTOCOL_VERSION, ]), + max_credential_count_in_list: MAX_CREDENTIAL_COUNT_IN_LIST, + // You can use ENCRYPTED_CREDENTIAL_ID_SIZE here, but if your + // browser passes that value, it might be used to fingerprint. + max_credential_id_length: None, + transports: Some(vec![AuthenticatorTransport::Usb]), + algorithms: Some(vec![CREDENTIAL_PARAMETER]), + firmware_version: None, }, )) } @@ -1093,7 +1098,7 @@ mod test { let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID); - let mut expected_response = vec![0x00, 0xA6, 0x01]; + let mut expected_response = vec![0x00, 0xA8, 0x01]; // The difference here is a longer array of supported versions. #[cfg(not(feature = "with_ctap1"))] expected_response.extend(&[0x81, 0x68, 0x46, 0x49, 0x44, 0x4F, 0x5F, 0x32, 0x5F, 0x30]); @@ -1107,10 +1112,16 @@ mod test { 0x03, 0x50, ]); expected_response.extend(AAGUID); - 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, - ]); + 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, + 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, + ] + .iter(), + ); assert_eq!(info_reponse, expected_response); } @@ -1128,10 +1139,7 @@ mod test { user_display_name: None, user_icon: None, }; - let pub_key_cred_params = vec![( - PublicKeyCredentialType::PublicKey, - SignatureAlgorithm::ES256 as i64, - )]; + let pub_key_cred_params = vec![CREDENTIAL_PARAMETER]; let options = MakeCredentialOptions { rk: true, uv: false, @@ -1228,12 +1236,8 @@ mod test { let user_immediately_present = |_| Ok(()); let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); - let pub_key_cred_params = vec![( - PublicKeyCredentialType::PublicKey, - SignatureAlgorithm::ES256 as i64 + 1, // any different number works - )]; let mut make_credential_params = create_minimal_make_credential_parameters(); - make_credential_params.pub_key_cred_params = pub_key_cred_params; + make_credential_params.pub_key_cred_params = vec![]; let make_credential_response = ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); diff --git a/src/ctap/response.rs b/src/ctap/response.rs index 4b8a089..9960f6c 100644 --- a/src/ctap/response.rs +++ b/src/ctap/response.rs @@ -13,8 +13,8 @@ // limitations under the License. use super::data_formats::{ - CoseKey, PackedAttestationStatement, PublicKeyCredentialDescriptor, - PublicKeyCredentialUserEntity, + AuthenticatorTransport, CoseKey, PackedAttestationStatement, PublicKeyCredentialDescriptor, + PublicKeyCredentialParameter, PublicKeyCredentialUserEntity, }; use alloc::collections::BTreeMap; use alloc::string::String; @@ -109,6 +109,11 @@ pub struct AuthenticatorGetInfoResponse { pub options: Option>, pub max_msg_size: Option, pub pin_protocols: Option>, + pub max_credential_count_in_list: Option, + pub max_credential_id_length: Option, + pub transports: Option>, + pub algorithms: Option>, + pub firmware_version: Option, } impl From for cbor::Value { @@ -120,6 +125,11 @@ impl From for cbor::Value { options, max_msg_size, pin_protocols, + max_credential_count_in_list, + max_credential_id_length, + transports, + algorithms, + firmware_version, } = get_info_response; let options_cbor: Option = options.map(|options| { @@ -131,12 +141,17 @@ impl From for cbor::Value { }); cbor_map_options! { - 1 => cbor_array_vec!(versions), - 2 => extensions.map(|vec| cbor_array_vec!(vec)), - 3 => &aaguid, - 4 => options_cbor, - 5 => max_msg_size, - 6 => pin_protocols.map(|vec| cbor_array_vec!(vec)), + 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)), + 0x07 => max_credential_count_in_list, + 0x08 => max_credential_id_length, + 0x09 => transports.map(|vec| cbor_array_vec!(vec)), + 0x0A => algorithms.map(|vec| cbor_array_vec!(vec)), + 0x0E => firmware_version, } } } @@ -228,6 +243,11 @@ mod test { options: None, max_msg_size: None, pin_protocols: None, + max_credential_count_in_list: None, + max_credential_id_length: None, + transports: None, + algorithms: None, + firmware_version: None, }; let response_cbor: Option = ResponseData::AuthenticatorGetInfo(get_info_response).into(); From d9c4c729e8fd871d787589353166dbd35c6ba0e2 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 27 Apr 2020 20:00:39 +0200 Subject: [PATCH 02/13] adds a feature flag for CTAP2.1, addresses comments --- Cargo.toml | 1 + README.md | 8 ++- deploy.py | 8 +++ run_desktop_tests.sh | 7 +++ src/ctap/command.rs | 49 +++++++--------- src/ctap/data_formats.rs | 120 +++++++++++++++++++++++++++++++++++---- src/ctap/mod.rs | 51 +++++++++++++---- src/ctap/response.rs | 90 +++++++++++++++++++++++++++-- 8 files changed, 274 insertions(+), 60 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1d1ad6e..4c2d16c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ std = ["cbor/std", "crypto/std", "crypto/derive_debug"] ram_storage = [] verbose = ["debug_ctap"] with_ctap1 = ["crypto/with_ctap1"] +with_ctap2_1 = [] [dev-dependencies] elf2tab = "0.4.0" diff --git a/README.md b/README.md index fb7d1c9..8dcc761 100644 --- a/README.md +++ b/README.md @@ -28,9 +28,11 @@ few limitations: Although we tested and implemented our firmware based on 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. With the upcoming next version of the -[CTAP2.1 specifications](https://fidoalliance.org/specs/fido2/fido-client-to-authenticator-protocol-v2.1-rd-20191217.html), -we started adding features, so master is currently between version 2.0 and 2.1. +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 `shell --ctap2-1` to the deploy command to include them. ### Cryptography diff --git a/deploy.py b/deploy.py index f519885..e9e599d 100755 --- a/deploy.py +++ b/deploy.py @@ -741,6 +741,14 @@ if __name__ == "__main__": help=("Compiles the OpenSK application without backward compatible " "support for U2F/CTAP1 protocol."), ) + main_parser.add_argument( + "--ctap2-1", + action=RemoveConstAction, + const="with_ctap2_1", + dest="features", + help=("Compiles the OpenSK application with backward compatible " + "support for CTAP2.1 protocol."), + ) main_parser.add_argument( "--regen-keys", action="store_true", diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index 36bde36..0613e34 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -30,6 +30,7 @@ cargo build --manifest-path third_party/tock/tools/sha256sum/Cargo.toml echo "Checking that CTAP2 builds properly..." cargo check --release --target=thumbv7em-none-eabi cargo check --release --target=thumbv7em-none-eabi --features with_ctap1 +cargo check --release --target=thumbv7em-none-eabi --features with_ctap2_1 cargo check --release --target=thumbv7em-none-eabi --features debug_ctap cargo check --release --target=thumbv7em-none-eabi --features panic_console cargo check --release --target=thumbv7em-none-eabi --features debug_allocations @@ -86,4 +87,10 @@ then echo "Running unit tests on the desktop (debug mode + CTAP1)..." cargo test --features std,with_ctap1 + + echo "Running unit tests on the desktop (release mode + CTAP2.1)..." + cargo test --release --features std,with_ctap2_1 + + echo "Running unit tests on the desktop (debug mode + CTAP2.1)..." + cargo test --features std,with_ctap2_1 fi diff --git a/src/ctap/command.rs b/src/ctap/command.rs index 9ccf104..19132bc 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -23,9 +23,10 @@ use alloc::string::String; use alloc::vec::Vec; use core::convert::TryFrom; -// Depending on your memory, you can use Some(n) to limit request sizes. +// Depending on your memory, you can use Some(n) to limit request sizes in +// MakeCredential and GetAssertion. This affects allowList and excludeList. // You might also want to set the max credential size in process_get_info then. -pub const MAX_CREDENTIAL_COUNT_IN_LIST: Option = None; +pub const MAX_CREDENTIAL_COUNT_IN_LIST: Option = None; // CTAP specification (version 20190130) section 6.1 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] @@ -136,25 +137,19 @@ impl TryFrom for AuthenticatorMakeCredentialParameters { )?)?; let cred_param_vec = read_array(ok_or_missing(param_map.get(&cbor_unsigned!(4)))?)?; - let mut pub_key_cred_params = vec![]; - for cred_param_map_value in cred_param_vec { - if let Ok(cred_param) = PublicKeyCredentialParameter::try_from(cred_param_map_value) { - pub_key_cred_params.push(cred_param); - } - } + let pub_key_cred_params = cred_param_vec + .iter() + .map(PublicKeyCredentialParameter::try_from) + .collect::, Ctap2StatusCode>>()?; let exclude_list = match param_map.get(&cbor_unsigned!(5)) { Some(entry) => { let exclude_list_vec = read_array(entry)?; - let mut exclude_list = vec![]; - for exclude_list_value in exclude_list_vec { - if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST { - if exclude_list.len() as u64 >= count { - break; - } - } - exclude_list.push(PublicKeyCredentialDescriptor::try_from(exclude_list_value)?); - } + let exclude_list = exclude_list_vec + .iter() + .take(MAX_CREDENTIAL_COUNT_IN_LIST.unwrap_or(exclude_list_vec.len())) + .map(PublicKeyCredentialDescriptor::try_from) + .collect::, Ctap2StatusCode>>()?; Some(exclude_list) } None => None, @@ -222,15 +217,11 @@ impl TryFrom for AuthenticatorGetAssertionParameters { let allow_list = match param_map.get(&cbor_unsigned!(3)) { Some(entry) => { let allow_list_vec = read_array(entry)?; - let mut allow_list = vec![]; - for allow_list_value in allow_list_vec { - if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST { - if allow_list.len() as u64 >= count { - break; - } - } - allow_list.push(PublicKeyCredentialDescriptor::try_from(allow_list_value)?); - } + let allow_list = allow_list_vec + .iter() + .take(MAX_CREDENTIAL_COUNT_IN_LIST.unwrap_or(allow_list_vec.len())) + .map(PublicKeyCredentialDescriptor::try_from) + .collect::, Ctap2StatusCode>>()?; Some(allow_list) } None => None, @@ -330,7 +321,7 @@ mod test { AuthenticatorTransport, PublicKeyCredentialRpEntity, PublicKeyCredentialType, PublicKeyCredentialUserEntity, }; - use super::super::CREDENTIAL_PARAMETER; + use super::super::ES256_CRED_PARAM; use super::*; use alloc::collections::BTreeMap; @@ -349,7 +340,7 @@ mod test { "displayName" => "bar", "icon" => "example.com/foo/icon.png", }, - 4 => cbor_array![CREDENTIAL_PARAMETER], + 4 => cbor_array![ES256_CRED_PARAM], 5 => cbor_array![], 8 => vec![0x12, 0x34], 9 => 1, @@ -380,7 +371,7 @@ mod test { client_data_hash, rp, user, - pub_key_cred_params: vec![CREDENTIAL_PARAMETER], + pub_key_cred_params: vec![ES256_CRED_PARAM], exclude_list: Some(vec![]), extensions: None, options, diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index a4bbe39..4f7e31c 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -19,6 +19,7 @@ use alloc::vec::Vec; use core::convert::TryFrom; use crypto::{ecdh, ecdsa}; +// https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialrpentity #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub struct PublicKeyCredentialRpEntity { pub rp_id: String, @@ -48,6 +49,7 @@ impl TryFrom<&cbor::Value> for PublicKeyCredentialRpEntity { } } +// https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialuserentity #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub struct PublicKeyCredentialUserEntity { pub user_id: Vec, @@ -94,16 +96,22 @@ impl From for cbor::Value { } } +// https://www.w3.org/TR/webauthn/#enumdef-publickeycredentialtype #[derive(Clone, PartialEq)] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] pub enum PublicKeyCredentialType { PublicKey, + // This is the default for all strings not covered above. + // Unknown types should be ignored, instead of returning errors. + Unknown, } impl From for cbor::Value { fn from(cred_type: PublicKeyCredentialType) -> Self { match cred_type { PublicKeyCredentialType::PublicKey => "public-key", + // We should never create this credential type. + PublicKeyCredentialType::Unknown => unreachable!(), } .into() } @@ -116,11 +124,12 @@ impl TryFrom<&cbor::Value> for PublicKeyCredentialType { let cred_type_string = read_text_string(cbor_value)?; match &cred_type_string[..] { "public-key" => Ok(PublicKeyCredentialType::PublicKey), - _ => Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM), + _ => Ok(PublicKeyCredentialType::Unknown), } } } +// https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialparameters #[derive(PartialEq)] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] pub struct PublicKeyCredentialParameter { @@ -151,6 +160,7 @@ impl From for cbor::Value { } } +// https://www.w3.org/TR/webauthn/#enumdef-authenticatortransport #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub enum AuthenticatorTransport { Usb, @@ -186,6 +196,7 @@ impl TryFrom<&cbor::Value> for AuthenticatorTransport { } } +// https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialdescriptor #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub struct PublicKeyCredentialDescriptor { pub key_type: PublicKeyCredentialType, @@ -205,10 +216,11 @@ impl TryFrom<&cbor::Value> for PublicKeyCredentialDescriptor { let transports = match cred_desc_map.get(&cbor_text!("transports")) { Some(exclude_entry) => { let transport_vec = read_array(exclude_entry)?; - let mut transports = vec![]; - for transport_value in transport_vec { - transports.push(AuthenticatorTransport::try_from(transport_value)?); - } + let transports = transport_vec + .iter() + .map(AuthenticatorTransport::try_from) + .collect::, Ctap2StatusCode>>( + )?; Some(transports) } None => None, @@ -379,6 +391,7 @@ impl TryFrom<&cbor::Value> for GetAssertionOptions { } } +// https://www.w3.org/TR/webauthn/#packed-attestation #[cfg_attr(test, derive(PartialEq))] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] pub struct PackedAttestationStatement { @@ -403,6 +416,9 @@ impl From for cbor::Value { #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] pub enum SignatureAlgorithm { ES256 = ecdsa::PubKey::ES256_ALGORITHM as isize, + // This is the default for all numbers not covered above. + // Unknown types should be ignored, instead of returning errors. + Unknown = 0, } impl TryFrom<&cbor::Value> for SignatureAlgorithm { @@ -411,11 +427,12 @@ impl TryFrom<&cbor::Value> for SignatureAlgorithm { fn try_from(cbor_value: &cbor::Value) -> Result { match read_integer(cbor_value)? { ecdsa::PubKey::ES256_ALGORITHM => Ok(SignatureAlgorithm::ES256), - _ => Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM), + _ => Ok(SignatureAlgorithm::Unknown), } } } +// https://www.w3.org/TR/webauthn/#public-key-credential-source #[derive(Clone)] #[cfg_attr(test, derive(PartialEq))] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] @@ -679,6 +696,7 @@ mod test { use self::Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE; use super::*; use alloc::collections::BTreeMap; + use crypto::rng256::{Rng256, ThreadRng256}; #[test] fn test_read_unsigned() { @@ -990,6 +1008,26 @@ mod test { assert_eq!(credential_type, Ok(expected_credential_type)); let created_cbor: cbor::Value = credential_type.unwrap().into(); assert_eq!(created_cbor, cbor_credential_type); + + let cbor_unknown_type = cbor_text!("unknown-type"); + let unknown_type = PublicKeyCredentialType::try_from(&cbor_unknown_type); + let expected_unknown_type = PublicKeyCredentialType::Unknown; + assert_eq!(unknown_type, Ok(expected_unknown_type)); + } + + #[test] + fn test_from_into_signature_algorithm() { + let cbor_signature_algorithm = cbor_int!(ecdsa::PubKey::ES256_ALGORITHM); + let signature_algorithm = SignatureAlgorithm::try_from(&cbor_signature_algorithm); + let expected_signature_algorithm = SignatureAlgorithm::ES256; + assert_eq!(signature_algorithm, Ok(expected_signature_algorithm)); + let created_cbor: cbor::Value = cbor_int!(signature_algorithm.unwrap() as i64); + assert_eq!(created_cbor, cbor_signature_algorithm); + + let cbor_unknown_algorithm = cbor_int!(-1); + let unknown_algorithm = SignatureAlgorithm::try_from(&cbor_unknown_algorithm); + let expected_unknown_algorithm = SignatureAlgorithm::Unknown; + assert_eq!(unknown_algorithm, Ok(expected_unknown_algorithm)); } #[test] @@ -1006,6 +1044,23 @@ mod test { assert_eq!(created_cbor, cbor_authenticator_transport); } + #[test] + fn test_from_into_public_key_credential_parameter() { + let cbor_credential_parameter = cbor_map! { + "type" => "public-key", + "alg" => ecdsa::PubKey::ES256_ALGORITHM, + }; + let credential_parameter = + PublicKeyCredentialParameter::try_from(&cbor_credential_parameter); + let expected_credential_parameter = PublicKeyCredentialParameter { + cred_type: PublicKeyCredentialType::PublicKey, + alg: SignatureAlgorithm::ES256, + }; + assert_eq!(credential_parameter, Ok(expected_credential_parameter)); + let created_cbor: cbor::Value = credential_parameter.unwrap().into(); + assert_eq!(created_cbor, cbor_credential_parameter); + } + #[test] fn test_from_into_public_key_credential_descriptor() { let cbor_credential_descriptor = cbor_map! { @@ -1026,7 +1081,7 @@ mod test { } #[test] - fn test_from_extensions() { + fn test_from_into_extensions() { let cbor_extensions = cbor_map! { "the_answer" => 42, }; @@ -1036,6 +1091,53 @@ mod test { .0 .insert("the_answer".to_string(), cbor_int!(42)); assert_eq!(extensions, Ok(expected_extensions)); + let created_cbor: cbor::Value = extensions.unwrap().into(); + assert_eq!(created_cbor, cbor_extensions); + } + + #[test] + fn test_from_into_get_assertion_hmac_secret_output() { + let cbor_output = cbor_bytes![vec![0xC0; 32]]; + let output = GetAssertionHmacSecretOutput::try_from(&cbor_output); + let expected_output = GetAssertionHmacSecretOutput(vec![0xC0; 32]); + assert_eq!(output, Ok(expected_output)); + let created_cbor: cbor::Value = output.unwrap().into(); + assert_eq!(created_cbor, cbor_output); + } + + #[test] + fn test_hmac_secret_extension() { + let cbor_extensions = cbor_map! { + "hmac-secret" => true, + }; + let extensions = Extensions::try_from(&cbor_extensions).unwrap(); + assert!(extensions.has_make_credential_hmac_secret().unwrap()); + + let cbor_extensions = cbor_map! { + "hmac-secret" => false, + }; + let extensions = Extensions::try_from(&cbor_extensions).unwrap(); + assert!(!extensions.has_make_credential_hmac_secret().unwrap()); + + let mut rng = ThreadRng256 {}; + let sk = crypto::ecdh::SecKey::gensk(&mut rng); + let pk = sk.genpk(); + let cose_key = CoseKey::from(pk.clone()); + let cbor_extensions = cbor_map! { + "hmac-secret" => cbor_map! { + 1 => cbor::Value::Map(cose_key.0.clone()), + 2 => vec![0x02; 32], + 3 => vec![0x03; 32], + }, + }; + let extensions = Extensions::try_from(&cbor_extensions).unwrap(); + let get_assertion_input = extensions.get_assertion_hmac_secret(); + let expected_input = GetAssertionHmacSecretInput { + key_agreement: cose_key, + salt_enc: vec![0x02; 32], + salt_auth: vec![0x03; 32], + }; + assert_eq!(get_assertion_input, Some(Ok(expected_input))); } #[test] @@ -1087,8 +1189,6 @@ mod test { #[test] fn test_from_into_cose_key() { - use crypto::rng256::ThreadRng256; - let mut rng = ThreadRng256 {}; let sk = crypto::ecdh::SecKey::gensk(&mut rng); let pk = sk.genpk(); @@ -1109,8 +1209,6 @@ mod test { #[test] fn test_credential_source_cbor_round_trip() { - use crypto::rng256::{Rng256, ThreadRng256}; - let mut rng = ThreadRng256 {}; let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 7624435..24039ad 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -23,15 +23,18 @@ 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, Command, MAX_CREDENTIAL_COUNT_IN_LIST, + AuthenticatorMakeCredentialParameters, Command, }; +#[cfg(feature = "with_ctap2_1")] +use self::data_formats::AuthenticatorTransport; use self::data_formats::{ - AuthenticatorTransport, ClientPinSubCommand, CoseKey, GetAssertionHmacSecretInput, - PackedAttestationStatement, PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, - PublicKeyCredentialSource, PublicKeyCredentialType, PublicKeyCredentialUserEntity, - SignatureAlgorithm, + ClientPinSubCommand, CoseKey, GetAssertionHmacSecretInput, PackedAttestationStatement, + PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialSource, + PublicKeyCredentialType, PublicKeyCredentialUserEntity, SignatureAlgorithm, }; use self::hid::ChannelID; use self::key_material::{AAGUID, ATTESTATION_CERTIFICATE, ATTESTATION_PRIVATE_KEY}; @@ -100,7 +103,9 @@ pub const FIDO2_VERSION_STRING: &str = "FIDO_2_0"; #[cfg(feature = "with_ctap1")] pub const U2F_VERSION_STRING: &str = "U2F_V2"; -pub const CREDENTIAL_PARAMETER: PublicKeyCredentialParameter = PublicKeyCredentialParameter { +// We currently only support one algorithm for signatures: ES256. +// This algorithm is requested in MakeCredential and advertized in GetInfo. +pub const ES256_CRED_PARAM: PublicKeyCredentialParameter = PublicKeyCredentialParameter { cred_type: PublicKeyCredentialType::PublicKey, alg: SignatureAlgorithm::ES256, }; @@ -419,7 +424,7 @@ where } } - if !pub_key_cred_params.contains(&CREDENTIAL_PARAMETER) { + if !pub_key_cred_params.contains(&ES256_CRED_PARAM) { return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); } @@ -757,6 +762,7 @@ where self.persistent_store.pin_hash().is_some(), ); Ok(ResponseData::AuthenticatorGetInfo( + #[cfg(feature = "with_ctap2_1")] AuthenticatorGetInfoResponse { versions: vec![ #[cfg(feature = "with_ctap1")] @@ -770,14 +776,29 @@ where pin_protocols: Some(vec![ CtapState::::PIN_PROTOCOL_VERSION, ]), - max_credential_count_in_list: MAX_CREDENTIAL_COUNT_IN_LIST, + max_credential_count_in_list: MAX_CREDENTIAL_COUNT_IN_LIST.map(|c| c as u64), // You can use ENCRYPTED_CREDENTIAL_ID_SIZE here, but if your // browser passes that value, it might be used to fingerprint. max_credential_id_length: None, transports: Some(vec![AuthenticatorTransport::Usb]), - algorithms: Some(vec![CREDENTIAL_PARAMETER]), + algorithms: Some(vec![ES256_CRED_PARAM]), firmware_version: None, }, + #[cfg(not(feature = "with_ctap2_1"))] + AuthenticatorGetInfoResponse { + versions: vec![ + #[cfg(feature = "with_ctap1")] + String::from(U2F_VERSION_STRING), + String::from(FIDO2_VERSION_STRING), + ], + extensions: Some(vec![String::from("hmac-secret")]), + aaguid: *AAGUID, + options: Some(options_map), + max_msg_size: Some(1024), + pin_protocols: Some(vec![ + CtapState::::PIN_PROTOCOL_VERSION, + ]), + }, )) } @@ -1098,7 +1119,10 @@ mod test { let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID); + #[cfg(feature = "with_ctap2_1")] let mut expected_response = vec![0x00, 0xA8, 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. #[cfg(not(feature = "with_ctap1"))] expected_response.extend(&[0x81, 0x68, 0x46, 0x49, 0x44, 0x4F, 0x5F, 0x32, 0x5F, 0x30]); @@ -1112,10 +1136,13 @@ mod test { 0x03, 0x50, ]); expected_response.extend(AAGUID); + 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, 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, @@ -1139,7 +1166,7 @@ mod test { user_display_name: None, user_icon: None, }; - let pub_key_cred_params = vec![CREDENTIAL_PARAMETER]; + let pub_key_cred_params = vec![ES256_CRED_PARAM]; let options = MakeCredentialOptions { rk: true, uv: false, diff --git a/src/ctap/response.rs b/src/ctap/response.rs index 9960f6c..389b82d 100644 --- a/src/ctap/response.rs +++ b/src/ctap/response.rs @@ -12,9 +12,11 @@ // 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::{ - AuthenticatorTransport, CoseKey, PackedAttestationStatement, PublicKeyCredentialDescriptor, - PublicKeyCredentialParameter, PublicKeyCredentialUserEntity, + CoseKey, PackedAttestationStatement, PublicKeyCredentialDescriptor, + PublicKeyCredentialUserEntity, }; use alloc::collections::BTreeMap; use alloc::string::String; @@ -102,21 +104,27 @@ impl From for cbor::Value { #[cfg_attr(test, derive(PartialEq))] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] pub struct AuthenticatorGetInfoResponse { - // TODO(kaczmarczyck) add fields from 2.1 + // TODO(kaczmarczyck) add maxAuthenticatorConfigLength and defaultCredProtect pub versions: Vec, pub extensions: Option>, pub aaguid: [u8; 16], 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>, + #[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, @@ -154,6 +162,35 @@ 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, + } = 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)), + } + } } #[cfg_attr(test, derive(PartialEq))] @@ -183,6 +220,8 @@ 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::*; #[test] @@ -243,17 +282,58 @@ 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, + #[cfg(feature = "with_ctap2_1")] firmware_version: None, }; let response_cbor: Option = ResponseData::AuthenticatorGetInfo(get_info_response).into(); let expected_cbor = cbor_map_options! { - 1 => cbor_array_vec![vec!["FIDO_2_0"]], - 3 => vec![0x00; 16], + 0x01 => cbor_array_vec![vec!["FIDO_2_0"]], + 0x03 => vec![0x00; 16], + }; + assert_eq!(response_cbor, Some(expected_cbor)); + } + + #[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); + let get_info_response = AuthenticatorGetInfoResponse { + versions: vec!["FIDO_2_0".to_string()], + extensions: Some(vec!["extension".to_string()]), + aaguid: [0x00; 16], + options: Some(options_map), + max_msg_size: Some(1024), + pin_protocols: Some(vec![1]), + max_credential_count_in_list: Some(20), + max_credential_id_length: Some(256), + transports: Some(vec![AuthenticatorTransport::Usb]), + algorithms: Some(vec![ES256_CRED_PARAM]), + firmware_version: Some(0), + }; + let response_cbor: Option = + ResponseData::AuthenticatorGetInfo(get_info_response).into(); + let expected_cbor = cbor_map_options! { + 0x01 => cbor_array_vec![vec!["FIDO_2_0"]], + 0x02 => cbor_array_vec![vec!["extension"]], + 0x03 => vec![0x00; 16], + 0x04 => cbor_map! {"rk" => true}, + 0x05 => 1024, + 0x06 => cbor_array_vec![vec![1]], + 0x07 => 20, + 0x08 => 256, + 0x09 => cbor_array_vec![vec!["usb"]], + 0x0A => cbor_array_vec![vec![ES256_CRED_PARAM]], + 0x0E => 0, }; assert_eq!(response_cbor, Some(expected_cbor)); } From 32f00908884351c48d2dc1c14c7b84d2e08f70bf Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Tue, 28 Apr 2020 18:24:46 +0200 Subject: [PATCH 03/13] Try to fix yapf matcher --- .github/python_matcher.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/python_matcher.json b/.github/python_matcher.json index ac38ab7..6532ac3 100644 --- a/.github/python_matcher.json +++ b/.github/python_matcher.json @@ -4,7 +4,7 @@ "owner": "yapf-diff", "pattern": [ { - "regexp": "^[+-]{3}\\s*([^\\s]*)\\s*\\((original|reformatted)\\)$", + "regexp": "^(?:---|\\+\\+\\+)\\s*([^\\s]*)\\s*\\((?:original|reformatted)\\)$", "file": 1 }, { @@ -13,7 +13,7 @@ "column": 2 }, { - "regexp": "^(\\s|\\+[^+]|\\-[^-]).*$", + "regexp": "^((?:\\s|\\+[^+]|\\-[^-]).*)$", "loop": true, "message": 1 } From a6114964971666aca91161769149d0b2873eef98 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Tue, 28 Apr 2020 18:27:05 +0200 Subject: [PATCH 04/13] Force format error to test yapf matcher --- deploy.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/deploy.py b/deploy.py index bb1c056..6b8377d 100755 --- a/deploy.py +++ b/deploy.py @@ -302,8 +302,7 @@ class OpenSKInstaller: # Need to update self.checked_command_output( ["rustup", "install", target_toolchain_fullstring]) - self.checked_command_output( - ["rustup", "target", "add", SUPPORTED_BOARDS[self.args.board].arch]) + self.checked_command_output(["rustup", "target", "add", SUPPORTED_BOARDS[self.args.board].arch]) info("Rust toolchain up-to-date") def build_tockos(self): From 332d7bc2ea775dccdd481a150e403e674d8efa23 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Tue, 28 Apr 2020 18:36:04 +0200 Subject: [PATCH 05/13] Update yapf matcher. Extracting the diff is not very useful because messages are not concatenated. The line/column info is also not useful because the diff includes some context lines. --- .github/python_matcher.json | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/.github/python_matcher.json b/.github/python_matcher.json index 6532ac3..d66fea8 100644 --- a/.github/python_matcher.json +++ b/.github/python_matcher.json @@ -5,17 +5,8 @@ "pattern": [ { "regexp": "^(?:---|\\+\\+\\+)\\s*([^\\s]*)\\s*\\((?:original|reformatted)\\)$", - "file": 1 - }, - { - "regexp": "^@@\\s*-(\\d+),(\\d+)\\s*\\+(\\d+),(\\d+)\\s*@@$", - "line": 1, - "column": 2 - }, - { - "regexp": "^((?:\\s|\\+[^+]|\\-[^-]).*)$", - "loop": true, - "message": 1 + "file": 1, + "message": "This file needs formating." } ] }, From 8bbf42623f57f09c2c17ed5acd26f77eb49d6545 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Wed, 29 Apr 2020 09:47:50 +0200 Subject: [PATCH 06/13] adds cargo checks and tests to workflows and locally --- .github/workflows/cargo_check.yml | 16 ++++++++++++++-- .github/workflows/opensk_test.yml | 24 ++++++++++++++++++++++++ README.md | 2 +- deploy.py | 2 +- run_desktop_tests.sh | 6 ++++++ src/ctap/data_formats.rs | 2 +- src/ctap/mod.rs | 21 +++++---------------- 7 files changed, 52 insertions(+), 21 deletions(-) diff --git a/.github/workflows/cargo_check.yml b/.github/workflows/cargo_check.yml index c54fc9e..07c1c0a 100644 --- a/.github/workflows/cargo_check.yml +++ b/.github/workflows/cargo_check.yml @@ -40,6 +40,12 @@ jobs: command: check args: --target thumbv7em-none-eabi --release --features with_ctap1 + - name: Check OpenSK with_ctap2_1 + uses: actions-rs/cargo@v1 + with: + command: check + args: --target thumbv7em-none-eabi --release --features with_ctap2_1 + - name: Check OpenSK debug_ctap uses: actions-rs/cargo@v1 with: @@ -76,11 +82,17 @@ jobs: command: check args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap1 - - name: Check OpenSK debug_ctap,with_ctap1,panic_console,debug_allocations,verbose + - name: Check OpenSK debug_ctap,with_ctap2_1 uses: actions-rs/cargo@v1 with: command: check - args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap1,panic_console,debug_allocations,verbose + args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap2_1 + + - name: Check OpenSK debug_ctap,with_ctap1,with_ctap2_1,panic_console,debug_allocations,verbose + uses: actions-rs/cargo@v1 + with: + command: check + args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap1,with_ctap2_1,panic_console,debug_allocations,verbose - name: Check examples uses: actions-rs/cargo@v1 diff --git a/.github/workflows/opensk_test.yml b/.github/workflows/opensk_test.yml index 6b17763..ece41e7 100644 --- a/.github/workflows/opensk_test.yml +++ b/.github/workflows/opensk_test.yml @@ -49,3 +49,27 @@ jobs: command: test args: --features std,with_ctap1 + - name: Unit testing of CTAP2 (release mode + CTAP2.1) + uses: actions-rs/cargo@v1 + with: + command: test + args: --release --features std,with_ctap2_1 + + - name: Unit testing of CTAP2 (debug mode + CTAP2.1) + uses: actions-rs/cargo@v1 + with: + command: test + args: --features std,with_ctap2_1 + + - name: Unit testing of CTAP2 (release mode + CTAP1 + CTAP2.1) + uses: actions-rs/cargo@v1 + with: + command: test + args: --release --features std,with_ctap1,with_ctap2_1 + + - name: Unit testing of CTAP2 (debug mode + CTAP1 + CTAP2.1) + uses: actions-rs/cargo@v1 + with: + command: test + args: --features std,with_ctap1,with_ctap2_1 + diff --git a/README.md b/README.md index 8dcc761..28093fc 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ 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 `shell --ctap2-1` to the deploy command to include them. +Please add the flag `--ctap2.1` to the deploy command to include them. ### Cryptography diff --git a/deploy.py b/deploy.py index 89921ca..2b1ce0c 100755 --- a/deploy.py +++ b/deploy.py @@ -755,7 +755,7 @@ if __name__ == "__main__": "support for U2F/CTAP1 protocol."), ) main_parser.add_argument( - "--ctap2-1", + "--ctap2.1", action=RemoveConstAction, const="with_ctap2_1", dest="features", diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index 0613e34..07b652e 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -93,4 +93,10 @@ then echo "Running unit tests on the desktop (debug mode + CTAP2.1)..." cargo test --features std,with_ctap2_1 + + echo "Running unit tests on the desktop (release mode + CTAP1 + CTAP2.1)..." + cargo test --release --features std,with_ctap1,with_ctap2_1 + + echo "Running unit tests on the desktop (debug mode + CTAP1 + CTAP2.1)..." + cargo test --features std,with_ctap1,with_ctap2_1 fi diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 4f7e31c..8e6c7d9 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -111,7 +111,7 @@ impl From for cbor::Value { match cred_type { PublicKeyCredentialType::PublicKey => "public-key", // We should never create this credential type. - PublicKeyCredentialType::Unknown => unreachable!(), + PublicKeyCredentialType::Unknown => "unknown", } .into() } diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 24039ad..a39a983 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -762,7 +762,6 @@ where self.persistent_store.pin_hash().is_some(), ); Ok(ResponseData::AuthenticatorGetInfo( - #[cfg(feature = "with_ctap2_1")] AuthenticatorGetInfoResponse { versions: vec![ #[cfg(feature = "with_ctap1")] @@ -776,29 +775,19 @@ 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), // You can use ENCRYPTED_CREDENTIAL_ID_SIZE here, but if your // browser passes that value, it might be used to fingerprint. + #[cfg(feature = "with_ctap2_1")] max_credential_id_length: None, + #[cfg(feature = "with_ctap2_1")] transports: Some(vec![AuthenticatorTransport::Usb]), + #[cfg(feature = "with_ctap2_1")] algorithms: Some(vec![ES256_CRED_PARAM]), + #[cfg(feature = "with_ctap2_1")] firmware_version: None, }, - #[cfg(not(feature = "with_ctap2_1"))] - AuthenticatorGetInfoResponse { - versions: vec![ - #[cfg(feature = "with_ctap1")] - String::from(U2F_VERSION_STRING), - String::from(FIDO2_VERSION_STRING), - ], - extensions: Some(vec![String::from("hmac-secret")]), - aaguid: *AAGUID, - options: Some(options_map), - max_msg_size: Some(1024), - pin_protocols: Some(vec![ - CtapState::::PIN_PROTOCOL_VERSION, - ]), - }, )) } From 2e35d0074a67ec56112e5362bd829a542bc1334a Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 29 Apr 2020 11:05:28 +0200 Subject: [PATCH 07/13] Introduce formatting error to test yapf --- deploy.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/deploy.py b/deploy.py index 6b8377d..c480656 100755 --- a/deploy.py +++ b/deploy.py @@ -302,7 +302,8 @@ class OpenSKInstaller: # Need to update self.checked_command_output( ["rustup", "install", target_toolchain_fullstring]) - self.checked_command_output(["rustup", "target", "add", SUPPORTED_BOARDS[self.args.board].arch]) + self.checked_command_output( + ["rustup", "target", "add", SUPPORTED_BOARDS[self.args.board].arch]) info("Rust toolchain up-to-date") def build_tockos(self): @@ -314,7 +315,8 @@ class OpenSKInstaller: self.checked_command_output(["make"], cwd=props.path) def build_example(self): - info("Building example {}".format(self.args.application)) + info( + "Building example {}".format(self.args.application)) self._build_app_or_example(is_example=True) def build_opensk(self): From 674c4c1b9a3d19e43e13c63c627fa763ee77705e Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 29 Apr 2020 11:38:33 +0200 Subject: [PATCH 08/13] Fixing yapf matcher --- .github/python_matcher.json | 13 ++++++++++--- deploy.py | 6 +++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/python_matcher.json b/.github/python_matcher.json index d66fea8..2778f15 100644 --- a/.github/python_matcher.json +++ b/.github/python_matcher.json @@ -4,9 +4,16 @@ "owner": "yapf-diff", "pattern": [ { - "regexp": "^(?:---|\\+\\+\\+)\\s*([^\\s]*)\\s*\\((?:original|reformatted)\\)$", - "file": 1, - "message": "This file needs formating." + "regexp": "^---\\s*([^\\s]*)\\s*\\(original\\)$", + "file": 1 + }, + { + "regexp": "^\\+\\+\\+\\s*([^\\s]*)\\s*\\(reformatted\\)$", + "message": 2 + }, + { + "regexp": "^@@\\s*-(\\d+),(\\d+)\\s*\\+(\\d+),(\\d+)\\s*@@$", + "line": 1 } ] }, diff --git a/deploy.py b/deploy.py index c480656..16276ca 100755 --- a/deploy.py +++ b/deploy.py @@ -315,8 +315,7 @@ class OpenSKInstaller: self.checked_command_output(["make"], cwd=props.path) def build_example(self): - info( - "Building example {}".format(self.args.application)) + info("Building example {}".format(self.args.application)) self._build_app_or_example(is_example=True) def build_opensk(self): @@ -343,7 +342,8 @@ class OpenSKInstaller: env["RUSTFLAGS"] = " ".join(rust_flags) command = [ - "cargo", "build", "--release", "--target={}".format(props.arch), + "cargo", + "build", "--release", "--target={}".format(props.arch), "--features={}".format(",".join(self.args.features)) ] if is_example: From 3e19c7512f12b24217ff5de8730eb74beb2dc93a Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 29 Apr 2020 11:40:14 +0200 Subject: [PATCH 09/13] Fix missing parentheses in regexp --- .github/python_matcher.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/python_matcher.json b/.github/python_matcher.json index 2778f15..4c9a945 100644 --- a/.github/python_matcher.json +++ b/.github/python_matcher.json @@ -8,7 +8,7 @@ "file": 1 }, { - "regexp": "^\\+\\+\\+\\s*([^\\s]*)\\s*\\(reformatted\\)$", + "regexp": "^\\+\\+\\+\\s*([^\\s]*)\\s*\\((reformatted)\\)$", "message": 2 }, { From 8e182b9de91ff94898c7555aa126f271c0c473a3 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 29 Apr 2020 11:40:48 +0200 Subject: [PATCH 10/13] Introduce formatting mistake --- deploy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy.py b/deploy.py index 16276ca..93dbe7b 100755 --- a/deploy.py +++ b/deploy.py @@ -342,8 +342,8 @@ class OpenSKInstaller: env["RUSTFLAGS"] = " ".join(rust_flags) command = [ - "cargo", - "build", "--release", "--target={}".format(props.arch), + "cargo", "build", + "--release", "--target={}".format(props.arch), "--features={}".format(",".join(self.args.features)) ] if is_example: From 8211df81d5cbf5b4c0902622be2675626a151132 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 29 Apr 2020 11:46:52 +0200 Subject: [PATCH 11/13] Adjust regexp --- .github/python_matcher.json | 2 +- deploy.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/python_matcher.json b/.github/python_matcher.json index 4c9a945..6c99675 100644 --- a/.github/python_matcher.json +++ b/.github/python_matcher.json @@ -8,7 +8,7 @@ "file": 1 }, { - "regexp": "^\\+\\+\\+\\s*([^\\s]*)\\s*\\((reformatted)\\)$", + "regexp": "^\\+\\+\\+\\s*([^\\s]*)\\s*\\((.*)\\)$", "message": 2 }, { diff --git a/deploy.py b/deploy.py index 93dbe7b..16276ca 100755 --- a/deploy.py +++ b/deploy.py @@ -342,8 +342,8 @@ class OpenSKInstaller: env["RUSTFLAGS"] = " ".join(rust_flags) command = [ - "cargo", "build", - "--release", "--target={}".format(props.arch), + "cargo", + "build", "--release", "--target={}".format(props.arch), "--features={}".format(",".join(self.args.features)) ] if is_example: From ca15ea43235f06bc58a70833c28e8bb461938425 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 29 Apr 2020 11:48:12 +0200 Subject: [PATCH 12/13] Fix formatting --- deploy.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/deploy.py b/deploy.py index 16276ca..bb1c056 100755 --- a/deploy.py +++ b/deploy.py @@ -342,8 +342,7 @@ class OpenSKInstaller: env["RUSTFLAGS"] = " ".join(rust_flags) command = [ - "cargo", - "build", "--release", "--target={}".format(props.arch), + "cargo", "build", "--release", "--target={}".format(props.arch), "--features={}".format(",".join(self.args.features)) ] if is_example: From 999e3313063cf8a298a7bc9831310c7d0f097314 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 29 Apr 2020 13:33:35 +0200 Subject: [PATCH 13/13] Add missing paramter that got introduced in tockloader 1.4 --- deploy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/deploy.py b/deploy.py index 2848924..5fb65f7 100755 --- a/deploy.py +++ b/deploy.py @@ -251,6 +251,7 @@ class OpenSKInstaller: self.tockloader_default_args = argparse.Namespace( arch=board.arch, board=self.args.board, + bundle_apps=False, debug=False, force=False, jlink_cmd="JLinkExe",