From 97fb22245598537e1d0bb294becb88086e05cc54 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Tue, 9 Jun 2020 14:36:16 +0200 Subject: [PATCH] Add a read_cbor_map macro to avoid the overhead of removing values on-by-one in BTreeMap. --- libraries/cbor/src/macros.rs | 151 +++++++++++++++++++++++++++++++ src/ctap/command.rs | 119 +++++++++++-------------- src/ctap/data_formats.rs | 166 +++++++++++++++++++++-------------- 3 files changed, 306 insertions(+), 130 deletions(-) diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs index 5a3d8f5..708fea6 100644 --- a/libraries/cbor/src/macros.rs +++ b/libraries/cbor/src/macros.rs @@ -12,6 +12,68 @@ // See the License for the specific language governing permissions and // limitations under the License. +#[macro_export] +macro_rules! read_cbor_map { + ( $map:expr, $( $variable:ident @ $key:expr, )+ ) => { + // A pre-requisite for this algorithm to work is that the keys to extract from the map are + // sorted. + #[cfg(test)] + test_ordered_keys!($( $key, )+); + + use $crate::values::{KeyType, Value}; + use ::core::cmp::Ordering; + use ::core::iter::Peekable; + use ::alloc::collections::btree_map::IntoIter; + + let mut it: Peekable> = $map.into_iter().peekable(); + $( + let $variable: Option = { + let needle: KeyType = $key; + loop { + match it.peek() { + None => break None, + Some(item) => { + let key: &KeyType = &item.0; + match key.cmp(&needle) { + Ordering::Less => { + it.next(); + continue + }, + Ordering::Equal => { + let value: Value = it.next().unwrap().1; + break Some(value) + }, + Ordering::Greater => break None, + } + } + } + } + }; + )+ + }; +} + +#[macro_export] +macro_rules! test_ordered_keys { + // Last key + ( $key:expr, ) => { + }; + + ( $key1:expr, $key2:expr, $( $keys:expr, )* ) => { + { + let k1: $crate::values::KeyType = $key1; + let k2: $crate::values::KeyType = $key2; + assert!( + k1 < k2, + "{:?} < {:?} failed. The read_cbor_map! macro requires keys in sorted order.", + k1, + k2, + ); + } + test_ordered_keys!($key2, $( $keys, )*); + }; +} + #[macro_export] macro_rules! cbor_map { // trailing comma case @@ -497,4 +559,93 @@ mod test { ); assert_eq!(a, b); } + + fn extract_map(cbor_value: Value) -> BTreeMap { + match cbor_value { + Value::Map(map) => map, + _ => panic!("Expected CBOR map."), + } + } + + #[test] + fn test_read_cbor_map_simple() { + let map = cbor_map! { + 1 => 10, + 2 => 20, + }; + + read_cbor_map! { + extract_map(map), + x1 @ cbor_unsigned!(1), + x2 @ cbor_unsigned!(2), + }; + + assert_eq!(x1, Some(cbor_unsigned!(10))); + assert_eq!(x2, Some(cbor_unsigned!(20))); + } + + #[test] + #[should_panic] + fn test_read_cbor_map_unordered() { + let map = cbor_map! { + 1 => 10, + 2 => 20, + }; + + read_cbor_map! { + extract_map(map), + _x2 @ cbor_unsigned!(2), + _x1 @ cbor_unsigned!(1), + }; + } + + #[test] + fn test_read_cbor_map_partial() { + let map = cbor_map! { + 1 => 10, + 2 => 20, + 3 => 30, + 4 => 40, + 5 => 50, + 6 => 60, + 7 => 70, + 8 => 80, + 9 => 90, + }; + + read_cbor_map! { + extract_map(map), + x3 @ cbor_unsigned!(3), + x7 @ cbor_unsigned!(7), + }; + + assert_eq!(x3, Some(cbor_unsigned!(30))); + assert_eq!(x7, Some(cbor_unsigned!(70))); + } + + #[test] + fn test_read_cbor_map_missing() { + let map = cbor_map! { + 1 => 10, + 3 => 30, + 4 => 40, + }; + + read_cbor_map! { + extract_map(map), + x0 @ cbor_unsigned!(0), + x1 @ cbor_unsigned!(1), + x2 @ cbor_unsigned!(2), + x3 @ cbor_unsigned!(3), + x4 @ cbor_unsigned!(4), + x5 @ cbor_unsigned!(5), + }; + + assert_eq!(x0, None); + assert_eq!(x1, Some(cbor_unsigned!(10))); + assert_eq!(x2, None); + assert_eq!(x3, Some(cbor_unsigned!(30))); + assert_eq!(x4, Some(cbor_unsigned!(40))); + assert_eq!(x5, None); + } } diff --git a/src/ctap/command.rs b/src/ctap/command.rs index d6dd0fa..03f38eb 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -124,26 +124,30 @@ impl TryFrom for AuthenticatorMakeCredentialParameters { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut param_map = extract_map(cbor_value)?; + read_cbor_map! { + extract_map(cbor_value)?, + client_data_hash @ cbor_unsigned!(1), + rp @ cbor_unsigned!(2), + user @ cbor_unsigned!(3), + cred_param_vec @ cbor_unsigned!(4), + exclude_list @ cbor_unsigned!(5), + extensions @ cbor_unsigned!(6), + options @ cbor_unsigned!(7), + pin_uv_auth_param @ cbor_unsigned!(8), + pin_uv_auth_protocol @ cbor_unsigned!(9), + }; - let client_data_hash = - extract_byte_string(ok_or_missing(param_map.remove(&cbor_unsigned!(1)))?)?; + let client_data_hash = extract_byte_string(ok_or_missing(client_data_hash)?)?; + let rp = PublicKeyCredentialRpEntity::try_from(ok_or_missing(rp)?)?; + let user = PublicKeyCredentialUserEntity::try_from(ok_or_missing(user)?)?; - let rp = PublicKeyCredentialRpEntity::try_from(ok_or_missing( - param_map.remove(&cbor_unsigned!(2)), - )?)?; - - let user = PublicKeyCredentialUserEntity::try_from(ok_or_missing( - param_map.remove(&cbor_unsigned!(3)), - )?)?; - - let cred_param_vec = extract_array(ok_or_missing(param_map.remove(&cbor_unsigned!(4)))?)?; + let cred_param_vec = extract_array(ok_or_missing(cred_param_vec)?)?; let pub_key_cred_params = cred_param_vec .into_iter() .map(PublicKeyCredentialParameter::try_from) .collect::, Ctap2StatusCode>>()?; - let exclude_list = match param_map.remove(&cbor_unsigned!(5)) { + let exclude_list = match exclude_list { Some(entry) => { let exclude_list_vec = extract_array(entry)?; let list_len = MAX_CREDENTIAL_COUNT_IN_LIST.unwrap_or(exclude_list_vec.len()); @@ -157,12 +161,11 @@ impl TryFrom for AuthenticatorMakeCredentialParameters { None => None, }; - let extensions = param_map - .remove(&cbor_unsigned!(6)) + let extensions = extensions .map(MakeCredentialExtensions::try_from) .transpose()?; - let options = match param_map.remove(&cbor_unsigned!(7)) { + let options = match options { Some(entry) => MakeCredentialOptions::try_from(entry)?, None => MakeCredentialOptions { rk: false, @@ -170,15 +173,8 @@ impl TryFrom for AuthenticatorMakeCredentialParameters { }, }; - let pin_uv_auth_param = param_map - .remove(&cbor_unsigned!(8)) - .map(extract_byte_string) - .transpose()?; - - let pin_uv_auth_protocol = param_map - .remove(&cbor_unsigned!(9)) - .map(extract_unsigned) - .transpose()?; + let pin_uv_auth_param = pin_uv_auth_param.map(extract_byte_string).transpose()?; + let pin_uv_auth_protocol = pin_uv_auth_protocol.map(extract_unsigned).transpose()?; Ok(AuthenticatorMakeCredentialParameters { client_data_hash, @@ -210,14 +206,21 @@ impl TryFrom for AuthenticatorGetAssertionParameters { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut param_map = extract_map(cbor_value)?; + read_cbor_map! { + extract_map(cbor_value)?, + rp_id @ cbor_unsigned!(1), + client_data_hash @ cbor_unsigned!(2), + allow_list @ cbor_unsigned!(3), + extensions @ cbor_unsigned!(4), + options @ cbor_unsigned!(5), + pin_uv_auth_param @ cbor_unsigned!(6), + pin_uv_auth_protocol @ cbor_unsigned!(7), + }; - let rp_id = extract_text_string(ok_or_missing(param_map.remove(&cbor_unsigned!(1)))?)?; + let rp_id = extract_text_string(ok_or_missing(rp_id)?)?; + let client_data_hash = extract_byte_string(ok_or_missing(client_data_hash)?)?; - let client_data_hash = - extract_byte_string(ok_or_missing(param_map.remove(&cbor_unsigned!(2)))?)?; - - let allow_list = match param_map.remove(&cbor_unsigned!(3)) { + let allow_list = match allow_list { Some(entry) => { let allow_list_vec = extract_array(entry)?; let list_len = MAX_CREDENTIAL_COUNT_IN_LIST.unwrap_or(allow_list_vec.len()); @@ -231,12 +234,11 @@ impl TryFrom for AuthenticatorGetAssertionParameters { None => None, }; - let extensions = param_map - .remove(&cbor_unsigned!(4)) + let extensions = extensions .map(GetAssertionExtensions::try_from) .transpose()?; - let options = match param_map.remove(&cbor_unsigned!(5)) { + let options = match options { Some(entry) => GetAssertionOptions::try_from(entry)?, None => GetAssertionOptions { up: true, @@ -244,15 +246,8 @@ impl TryFrom for AuthenticatorGetAssertionParameters { }, }; - let pin_uv_auth_param = param_map - .remove(&cbor_unsigned!(6)) - .map(extract_byte_string) - .transpose()?; - - let pin_uv_auth_protocol = param_map - .remove(&cbor_unsigned!(7)) - .map(extract_unsigned) - .transpose()?; + let pin_uv_auth_param = pin_uv_auth_param.map(extract_byte_string).transpose()?; + let pin_uv_auth_protocol = pin_uv_auth_protocol.map(extract_unsigned).transpose()?; Ok(AuthenticatorGetAssertionParameters { rp_id, @@ -280,33 +275,25 @@ impl TryFrom for AuthenticatorClientPinParameters { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut param_map = extract_map(cbor_value)?; + read_cbor_map! { + extract_map(cbor_value)?, + pin_protocol @ cbor_unsigned!(1), + sub_command @ cbor_unsigned!(2), + key_agreement @ cbor_unsigned!(3), + pin_auth @ cbor_unsigned!(4), + new_pin_enc @ cbor_unsigned!(5), + pin_hash_enc @ cbor_unsigned!(6), + }; - let pin_protocol = extract_unsigned(ok_or_missing(param_map.remove(&cbor_unsigned!(1)))?)?; - - let sub_command = - ClientPinSubCommand::try_from(ok_or_missing(param_map.remove(&cbor_unsigned!(2)))?)?; - - let key_agreement = param_map - .remove(&cbor_unsigned!(3)) + let pin_protocol = extract_unsigned(ok_or_missing(pin_protocol)?)?; + let sub_command = ClientPinSubCommand::try_from(ok_or_missing(sub_command)?)?; + let key_agreement = key_agreement .map(extract_map) .transpose()? .map(|x| CoseKey(x)); - - let pin_auth = param_map - .remove(&cbor_unsigned!(4)) - .map(extract_byte_string) - .transpose()?; - - let new_pin_enc = param_map - .remove(&cbor_unsigned!(5)) - .map(extract_byte_string) - .transpose()?; - - let pin_hash_enc = param_map - .remove(&cbor_unsigned!(6)) - .map(extract_byte_string) - .transpose()?; + 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()?; Ok(AuthenticatorClientPinParameters { pin_protocol, diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 33dec12..0a5cab5 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -31,16 +31,17 @@ impl TryFrom for PublicKeyCredentialRpEntity { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut rp_map = extract_map(cbor_value)?; - let rp_id = extract_text_string(ok_or_missing(rp_map.remove(&cbor_text!("id")))?)?; - let rp_name = rp_map - .remove(&cbor_text!("name")) - .map(extract_text_string) - .transpose()?; - let rp_icon = rp_map - .remove(&cbor_text!("icon")) - .map(extract_text_string) - .transpose()?; + read_cbor_map! { + extract_map(cbor_value)?, + rp_id @ cbor_text!("id"), + rp_icon @ cbor_text!("icon"), + rp_name @ cbor_text!("name"), + }; + + let rp_id = extract_text_string(ok_or_missing(rp_id)?)?; + let rp_name = rp_name.map(extract_text_string).transpose()?; + let rp_icon = rp_icon.map(extract_text_string).transpose()?; + Ok(Self { rp_id, rp_name, @@ -62,20 +63,19 @@ impl TryFrom for PublicKeyCredentialUserEntity { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut user_map = extract_map(cbor_value)?; - let user_id = extract_byte_string(ok_or_missing(user_map.remove(&cbor_text!("id")))?)?; - let user_name = user_map - .remove(&cbor_text!("name")) - .map(extract_text_string) - .transpose()?; - let user_display_name = user_map - .remove(&cbor_text!("displayName")) - .map(extract_text_string) - .transpose()?; - let user_icon = user_map - .remove(&cbor_text!("icon")) - .map(extract_text_string) - .transpose()?; + read_cbor_map! { + extract_map(cbor_value)?, + user_id @ cbor_text!("id"), + user_icon @ cbor_text!("icon"), + user_name @ cbor_text!("name"), + user_display_name @ cbor_text!("displayName"), + }; + + let user_id = extract_byte_string(ok_or_missing(user_id)?)?; + let user_name = user_name.map(extract_text_string).transpose()?; + let user_display_name = user_display_name.map(extract_text_string).transpose()?; + let user_icon = user_icon.map(extract_text_string).transpose()?; + Ok(Self { user_id, user_name, @@ -141,13 +141,14 @@ impl TryFrom for PublicKeyCredentialParameter { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut cred_param_map = extract_map(cbor_value)?; - let cred_type = PublicKeyCredentialType::try_from(ok_or_missing( - cred_param_map.remove(&cbor_text!("type")), - )?)?; - let alg = SignatureAlgorithm::try_from(ok_or_missing( - cred_param_map.remove(&cbor_text!("alg")), - )?)?; + read_cbor_map! { + extract_map(cbor_value)?, + alg @ cbor_text!("alg"), + cred_type @ cbor_text!("type"), + }; + + let cred_type = PublicKeyCredentialType::try_from(ok_or_missing(cred_type)?)?; + let alg = SignatureAlgorithm::try_from(ok_or_missing(alg)?)?; Ok(Self { cred_type, alg }) } } @@ -209,12 +210,16 @@ impl TryFrom for PublicKeyCredentialDescriptor { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut cred_desc_map = extract_map(cbor_value)?; - let key_type = PublicKeyCredentialType::try_from(ok_or_missing( - cred_desc_map.remove(&cbor_text!("type")), - )?)?; - let key_id = extract_byte_string(ok_or_missing(cred_desc_map.remove(&cbor_text!("id")))?)?; - let transports = match cred_desc_map.remove(&cbor_text!("transports")) { + read_cbor_map! { + extract_map(cbor_value)?, + key_id @ cbor_text!("id"), + key_type @ cbor_text!("type"), + transports @ cbor_text!("transports"), + }; + + let key_type = PublicKeyCredentialType::try_from(ok_or_missing(key_type)?)?; + let key_id = extract_byte_string(ok_or_missing(key_id)?)?; + let transports = match transports { Some(exclude_entry) => { let transport_vec = extract_array(exclude_entry)?; let transports = transport_vec @@ -225,6 +230,7 @@ impl TryFrom for PublicKeyCredentialDescriptor { } None => None, }; + Ok(Self { key_type, key_id, @@ -253,12 +259,14 @@ impl TryFrom for MakeCredentialExtensions { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut extensions_map = extract_map(cbor_value)?; - let hmac_secret = extensions_map - .remove(&cbor_text!("hmac-secret")) - .map_or(Ok(false), extract_bool)?; - let cred_protect = extensions_map - .remove(&cbor_text!("credProtect")) + read_cbor_map! { + extract_map(cbor_value)?, + cred_protect @ cbor_text!("credProtect"), + hmac_secret @ cbor_text!("hmac-secret"), + }; + + let hmac_secret = hmac_secret.map_or(Ok(false), extract_bool)?; + let cred_protect = cred_protect .map(CredentialProtectionPolicy::try_from) .transpose()?; Ok(Self { @@ -277,9 +285,12 @@ impl TryFrom for GetAssertionExtensions { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut extensions_map = extract_map(cbor_value)?; - let hmac_secret = extensions_map - .remove(&cbor_text!("hmac-secret")) + read_cbor_map! { + extract_map(cbor_value)?, + hmac_secret @ cbor_text!("hmac-secret"), + }; + + let hmac_secret = hmac_secret .map(GetAssertionHmacSecretInput::try_from) .transpose()?; Ok(Self { hmac_secret }) @@ -297,10 +308,16 @@ impl TryFrom for GetAssertionHmacSecretInput { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut input_map = extract_map(cbor_value)?; - let cose_key = extract_map(ok_or_missing(input_map.remove(&cbor_unsigned!(1)))?)?; - let salt_enc = extract_byte_string(ok_or_missing(input_map.remove(&cbor_unsigned!(2)))?)?; - let salt_auth = extract_byte_string(ok_or_missing(input_map.remove(&cbor_unsigned!(3)))?)?; + read_cbor_map! { + extract_map(cbor_value)?, + cose_key @ cbor_unsigned!(1), + salt_enc @ cbor_unsigned!(2), + salt_auth @ cbor_unsigned!(3), + }; + + let cose_key = extract_map(ok_or_missing(cose_key)?)?; + let salt_enc = extract_byte_string(ok_or_missing(salt_enc)?)?; + let salt_auth = extract_byte_string(ok_or_missing(salt_auth)?)?; Ok(Self { key_agreement: CoseKey(cose_key), salt_enc, @@ -320,17 +337,23 @@ impl TryFrom for MakeCredentialOptions { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut options_map = extract_map(cbor_value)?; - let rk = match options_map.remove(&cbor_text!("rk")) { + read_cbor_map! { + extract_map(cbor_value)?, + rk @ cbor_text!("rk"), + up @ cbor_text!("up"), + uv @ cbor_text!("uv"), + }; + + let rk = match rk { Some(options_entry) => extract_bool(options_entry)?, None => false, }; - if let Some(options_entry) = options_map.remove(&cbor_text!("up")) { + if let Some(options_entry) = up { if !extract_bool(options_entry)? { return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); } } - let uv = match options_map.remove(&cbor_text!("uv")) { + let uv = match uv { Some(options_entry) => extract_bool(options_entry)?, None => false, }; @@ -348,17 +371,23 @@ impl TryFrom for GetAssertionOptions { type Error = Ctap2StatusCode; fn try_from(cbor_value: cbor::Value) -> Result { - let mut options_map = extract_map(cbor_value)?; - if let Some(options_entry) = options_map.remove(&cbor_text!("rk")) { + read_cbor_map! { + extract_map(cbor_value)?, + rk @ cbor_text!("rk"), + up @ cbor_text!("up"), + uv @ cbor_text!("uv"), + }; + + if let Some(options_entry) = rk { // This is only for returning the correct status code. extract_bool(options_entry)?; return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); } - let up = match options_map.remove(&cbor_text!("up")) { + let up = match up { Some(options_entry) => extract_bool(options_entry)?, None => true, }; - let uv = match options_map.remove(&cbor_text!("uv")) { + let uv = match uv { Some(options_entry) => extract_bool(options_entry)?, None => false, }; @@ -599,27 +628,36 @@ impl TryFrom for ecdh::PubKey { type Error = Ctap2StatusCode; fn try_from(cose_key: CoseKey) -> Result { - let mut cose_map = cose_key.0; - let key_type = extract_integer(ok_or_missing(cose_map.remove(&cbor_int!(1)))?)?; + read_cbor_map! { + cose_key.0, + key_type @ cbor_int!(1), + algorithm @ cbor_int!(3), + curve @ cbor_int!(-1), + x_bytes @ cbor_int!(-2), + y_bytes @ cbor_int!(-3), + }; + + let key_type = extract_integer(ok_or_missing(key_type)?)?; if key_type != EC2_KEY_TYPE { return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); } - let algorithm = extract_integer(ok_or_missing(cose_map.remove(&cbor_int!(3)))?)?; + let algorithm = extract_integer(ok_or_missing(algorithm)?)?; if algorithm != ECDH_ALGORITHM && algorithm != ES256_ALGORITHM { return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); } - let curve = extract_integer(ok_or_missing(cose_map.remove(&cbor_int!(-1)))?)?; + let curve = extract_integer(ok_or_missing(curve)?)?; if curve != P_256_CURVE { return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); } - let x_bytes = extract_byte_string(ok_or_missing(cose_map.remove(&cbor_int!(-2)))?)?; + let x_bytes = extract_byte_string(ok_or_missing(x_bytes)?)?; if x_bytes.len() != ecdh::NBYTES { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } - let y_bytes = extract_byte_string(ok_or_missing(cose_map.remove(&cbor_int!(-3)))?)?; + let y_bytes = extract_byte_string(ok_or_missing(y_bytes)?)?; if y_bytes.len() != ecdh::NBYTES { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } + let x_array_ref = array_ref![x_bytes.as_slice(), 0, ecdh::NBYTES]; let y_array_ref = array_ref![y_bytes.as_slice(), 0, ecdh::NBYTES]; ecdh::PubKey::from_coordinates(x_array_ref, y_array_ref)