From 78b7767682e4e3f0b36ae577882827f22db38176 Mon Sep 17 00:00:00 2001 From: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com> Date: Tue, 13 Apr 2021 14:46:28 +0200 Subject: [PATCH] CBOR maps use Vec instead of BTreeMap (#303) * CBOR uses Vec for map internally * remove BTreeMap from get_info * rename cbor_map_btree and clean up cbor_array_vec * destructure now takes Vec, not BTreeMap * adds dedup in CBOR writer * fail to write CBOR maps with duplicates * CBOR interface refinements * macro documentation for CBOR map and array --- libraries/cbor/src/macros.rs | 130 ++++++++++++++++++++++------------- libraries/cbor/src/reader.rs | 9 ++- libraries/cbor/src/values.rs | 9 ++- libraries/cbor/src/writer.rs | 97 ++++++++++++++++++++++---- src/ctap/command.rs | 6 +- src/ctap/data_formats.rs | 49 ++++++------- src/ctap/mod.rs | 70 ++++++++++--------- src/ctap/response.rs | 39 +++++------ src/ctap/storage.rs | 12 ++-- 9 files changed, 258 insertions(+), 163 deletions(-) diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs index 40669d1..758925e 100644 --- a/libraries/cbor/src/macros.rs +++ b/libraries/cbor/src/macros.rs @@ -13,14 +13,14 @@ // limitations under the License. use crate::values::{KeyType, Value}; -use alloc::collections::btree_map; +use alloc::vec; use core::cmp::Ordering; use core::iter::Peekable; -/// This macro generates code to extract multiple values from a `BTreeMap` at once -/// in an optimized manner, consuming the input map. +/// This macro generates code to extract multiple values from a `Vec<(KeyType, Value)>` at once +/// in an optimized manner, consuming the input vector. /// -/// It takes as input a `BTreeMap` as well as a list of identifiers and keys, and generates code +/// It takes as input a `Vec` as well as a list of identifiers and keys, and generates code /// that assigns the corresponding values to new variables using the given identifiers. Each of /// these variables has type `Option`, to account for the case where keys aren't found. /// @@ -32,16 +32,14 @@ use core::iter::Peekable; /// the keys are indeed sorted. This macro is therefore **not suitable for dynamic keys** that can /// change at runtime. /// -/// Semantically, provided that the keys are sorted as specified above, the following two snippets -/// of code are equivalent, but the `destructure_cbor_map!` version is more optimized, as it doesn't -/// re-balance the `BTreeMap` for each key, contrary to the `BTreeMap::remove` operations. +/// Example usage: /// /// ```rust /// # extern crate alloc; /// # use cbor::destructure_cbor_map; /// # /// # fn main() { -/// # let map = alloc::collections::BTreeMap::new(); +/// # let map = alloc::vec::Vec::new(); /// destructure_cbor_map! { /// let { /// 1 => x, @@ -50,17 +48,6 @@ use core::iter::Peekable; /// } /// # } /// ``` -/// -/// ```rust -/// # extern crate alloc; -/// # -/// # fn main() { -/// # let mut map = alloc::collections::BTreeMap::::new(); -/// use cbor::values::IntoCborKey; -/// let x: Option = map.remove(&1.into_cbor_key()); -/// let y: Option = map.remove(&"key".into_cbor_key()); -/// # } -/// ``` #[macro_export] macro_rules! destructure_cbor_map { ( let { $( $key:expr => $variable:ident, )+ } = $map:expr; ) => { @@ -100,7 +87,7 @@ macro_rules! destructure_cbor_map { /// would be inlined for every use case. As of June 2020, this saves ~40KB of binary size for the /// CTAP2 application of OpenSK. pub fn destructure_cbor_map_peek_value( - it: &mut Peekable>, + it: &mut Peekable>, needle: KeyType, ) -> Option { loop { @@ -145,6 +132,23 @@ macro_rules! assert_sorted_keys { }; } +/// Creates a CBOR Value of type Map with the specified key-value pairs. +/// +/// Keys and values are expressions and converted into CBOR Keys and Values. +/// The syntax for these pairs is `key_expression => value_expression,`. +/// Duplicate keys will lead to invalid CBOR, i.e. writing these values fails. +/// Keys do not have to be sorted. +/// +/// Example usage: +/// +/// ```rust +/// # extern crate alloc; +/// # use cbor::cbor_map; +/// let map = cbor_map! { +/// 0x01 => false, +/// "02" => -3, +/// }; +/// ``` #[macro_export] macro_rules! cbor_map { // trailing comma case @@ -157,15 +161,35 @@ macro_rules! cbor_map { // The import is unused if the list is empty. #[allow(unused_imports)] use $crate::values::{IntoCborKey, IntoCborValue}; - let mut _map = ::alloc::collections::BTreeMap::new(); + let mut _map = ::alloc::vec::Vec::new(); $( - _map.insert($key.into_cbor_key(), $value.into_cbor_value()); + _map.push(($key.into_cbor_key(), $value.into_cbor_value())); )* $crate::values::Value::Map(_map) } }; } +/// Creates a CBOR Value of type Map with key-value pairs where values can be Options. +/// +/// Keys and values are expressions and converted into CBOR Keys and Value Options. +/// The map entry is included iff the Value is not an Option or Option is Some. +/// The syntax for these pairs is `key_expression => value_expression,`. +/// Duplicate keys will lead to invalid CBOR, i.e. writing these values fails. +/// Keys do not have to be sorted. +/// +/// Example usage: +/// +/// ```rust +/// # extern crate alloc; +/// # use cbor::cbor_map_options; +/// let missing_value: Option = None; +/// let map = cbor_map_options! { +/// 0x01 => Some(false), +/// "02" => -3, +/// "not in map" => missing_value, +/// }; +/// ``` #[macro_export] macro_rules! cbor_map_options { // trailing comma case @@ -178,12 +202,12 @@ macro_rules! cbor_map_options { // The import is unused if the list is empty. #[allow(unused_imports)] use $crate::values::{IntoCborKey, IntoCborValueOption}; - let mut _map = ::alloc::collections::BTreeMap::<_, $crate::values::Value>::new(); + let mut _map = ::alloc::vec::Vec::<(_, $crate::values::Value)>::new(); $( { let opt: Option<$crate::values::Value> = $value.into_cbor_value_option(); if let Some(val) = opt { - _map.insert($key.into_cbor_key(), val); + _map.push(($key.into_cbor_key(), val)); } } )* @@ -192,13 +216,25 @@ macro_rules! cbor_map_options { }; } +/// Creates a CBOR Value of type Map from a Vec<(KeyType, Value)>. #[macro_export] -macro_rules! cbor_map_btree { - ( $tree:expr ) => { - $crate::values::Value::Map($tree) - }; +macro_rules! cbor_map_collection { + ( $tree:expr ) => {{ + $crate::values::Value::from($tree) + }}; } +/// Creates a CBOR Value of type Array with the given elements. +/// +/// Elements are expressions and converted into CBOR Values. Elements are comma-separated. +/// +/// Example usage: +/// +/// ```rust +/// # extern crate alloc; +/// # use cbor::cbor_array; +/// let array = cbor_array![1, "2"]; +/// ``` #[macro_export] macro_rules! cbor_array { // trailing comma case @@ -216,6 +252,7 @@ macro_rules! cbor_array { }; } +/// Creates a CBOR Value of type Array from a Vec. #[macro_export] macro_rules! cbor_array_vec { ( $vec:expr ) => {{ @@ -329,7 +366,6 @@ macro_rules! cbor_key_bytes { #[cfg(test)] mod test { use super::super::values::{KeyType, SimpleValue, Value}; - use alloc::collections::BTreeMap; #[test] fn test_cbor_simple_values() { @@ -421,7 +457,7 @@ mod test { Value::KeyValue(KeyType::Unsigned(0)), Value::KeyValue(KeyType::Unsigned(1)), ]), - Value::Map(BTreeMap::new()), + Value::Map(Vec::new()), Value::Map( [(KeyType::Unsigned(2), Value::KeyValue(KeyType::Unsigned(3)))] .iter() @@ -518,7 +554,7 @@ mod test { Value::KeyValue(KeyType::Unsigned(1)), ]), ), - (KeyType::Unsigned(9), Value::Map(BTreeMap::new())), + (KeyType::Unsigned(9), Value::Map(Vec::new())), ( KeyType::Unsigned(10), Value::Map( @@ -589,7 +625,7 @@ mod test { Value::KeyValue(KeyType::Unsigned(1)), ]), ), - (KeyType::Unsigned(9), Value::Map(BTreeMap::new())), + (KeyType::Unsigned(9), Value::Map(Vec::new())), ( KeyType::Unsigned(10), Value::Map( @@ -608,30 +644,26 @@ mod test { } #[test] - fn test_cbor_map_btree_empty() { - let a = cbor_map_btree!(BTreeMap::new()); - let b = Value::Map(BTreeMap::new()); + fn test_cbor_map_collection_empty() { + let a = cbor_map_collection!(Vec::<(_, _)>::new()); + let b = Value::Map(Vec::new()); assert_eq!(a, b); } #[test] - fn test_cbor_map_btree_foo() { - let a = cbor_map_btree!( - [(KeyType::Unsigned(2), Value::KeyValue(KeyType::Unsigned(3)))] - .iter() - .cloned() - .collect() - ); - let b = Value::Map( - [(KeyType::Unsigned(2), Value::KeyValue(KeyType::Unsigned(3)))] - .iter() - .cloned() - .collect(), - ); + fn test_cbor_map_collection_foo() { + let a = cbor_map_collection!(vec![( + KeyType::Unsigned(2), + Value::KeyValue(KeyType::Unsigned(3)) + )]); + let b = Value::Map(vec![( + KeyType::Unsigned(2), + Value::KeyValue(KeyType::Unsigned(3)), + )]); assert_eq!(a, b); } - fn extract_map(cbor_value: Value) -> BTreeMap { + fn extract_map(cbor_value: Value) -> Vec<(KeyType, Value)> { match cbor_value { Value::Map(map) => map, _ => panic!("Expected CBOR map."), diff --git a/libraries/cbor/src/reader.rs b/libraries/cbor/src/reader.rs index a1061a0..0634d84 100644 --- a/libraries/cbor/src/reader.rs +++ b/libraries/cbor/src/reader.rs @@ -13,8 +13,7 @@ // limitations under the License. use super::values::{Constants, KeyType, SimpleValue, Value}; -use crate::{cbor_array_vec, cbor_bytes_lit, cbor_map_btree, cbor_text, cbor_unsigned}; -use alloc::collections::BTreeMap; +use crate::{cbor_array_vec, cbor_bytes_lit, cbor_map_collection, cbor_text, cbor_unsigned}; use alloc::str; use alloc::vec::Vec; @@ -174,7 +173,7 @@ impl<'a> Reader<'a> { size_value: u64, remaining_depth: i8, ) -> Result { - let mut value_map = BTreeMap::new(); + let mut value_map = Vec::new(); let mut last_key_option = None; for _ in 0..size_value { let key_value = self.decode_complete_data_item(remaining_depth - 1)?; @@ -185,12 +184,12 @@ impl<'a> Reader<'a> { } } last_key_option = Some(key.clone()); - value_map.insert(key, self.decode_complete_data_item(remaining_depth - 1)?); + value_map.push((key, self.decode_complete_data_item(remaining_depth - 1)?)); } else { return Err(DecoderError::IncorrectMapKeyType); } } - Ok(cbor_map_btree!(value_map)) + Ok(cbor_map_collection!(value_map)) } fn decode_to_simple_value( diff --git a/libraries/cbor/src/values.rs b/libraries/cbor/src/values.rs index b20d109..c2f7b72 100644 --- a/libraries/cbor/src/values.rs +++ b/libraries/cbor/src/values.rs @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use alloc::collections::BTreeMap; use alloc::string::{String, ToString}; use alloc::vec::Vec; use core::cmp::Ordering; @@ -21,7 +20,7 @@ use core::cmp::Ordering; pub enum Value { KeyValue(KeyType), Array(Vec), - Map(BTreeMap), + Map(Vec<(KeyType, Value)>), // TAG is omitted Simple(SimpleValue), } @@ -183,6 +182,12 @@ where } } +impl From> for Value { + fn from(map: Vec<(KeyType, Value)>) -> Self { + Value::Map(map) + } +} + impl From for Value { fn from(b: bool) -> Self { Value::bool_value(b) diff --git a/libraries/cbor/src/writer.rs b/libraries/cbor/src/writer.rs index 592048d..386f0b5 100644 --- a/libraries/cbor/src/writer.rs +++ b/libraries/cbor/src/writer.rs @@ -56,8 +56,14 @@ impl<'a> Writer<'a> { } } } - Value::Map(map) => { - self.start_item(5, map.len() as u64); + Value::Map(mut map) => { + map.sort_by(|a, b| a.0.cmp(&b.0)); + let map_len = map.len(); + map.dedup_by(|a, b| a.0.eq(&b.0)); + if map_len != map.len() { + return false; + } + self.start_item(5, map_len as u64); for (k, v) in map { if !self.encode_cbor(Value::KeyValue(k), remaining_depth - 1) { return false; @@ -209,9 +215,16 @@ mod test { #[test] fn test_write_map() { let value_map = cbor_map! { - "aa" => "AA", - "e" => "E", - "" => ".", + 0 => "a", + 23 => "b", + 24 => "c", + std::u8::MAX as i64 => "d", + 256 => "e", + std::u16::MAX as i64 => "f", + 65536 => "g", + std::u32::MAX as i64 => "h", + 4294967296_i64 => "i", + std::i64::MAX => "j", -1 => "k", -24 => "l", -25 => "m", @@ -224,16 +237,9 @@ mod test { b"a" => 2, b"bar" => 3, b"foo" => 4, - 0 => "a", - 23 => "b", - 24 => "c", - std::u8::MAX as i64 => "d", - 256 => "e", - std::u16::MAX as i64 => "f", - 65536 => "g", - std::u32::MAX as i64 => "h", - 4294967296_i64 => "i", - std::i64::MAX => "j", + "" => ".", + "e" => "E", + "aa" => "AA", }; let expected_cbor = vec![ 0xb8, 0x19, // map of 25 pairs: @@ -288,6 +294,67 @@ mod test { assert_eq!(write_return(value_map), Some(expected_cbor)); } + #[test] + fn test_write_map_sorted() { + let sorted_map = cbor_map! { + 0 => "a", + 1 => "b", + -1 => "c", + -2 => "d", + b"a" => "e", + b"b" => "f", + "" => "g", + "c" => "h", + }; + let unsorted_map = cbor_map! { + 1 => "b", + -2 => "d", + b"b" => "f", + "c" => "h", + "" => "g", + b"a" => "e", + -1 => "c", + 0 => "a", + }; + assert_eq!(write_return(sorted_map), write_return(unsorted_map)); + } + + #[test] + fn test_write_map_duplicates() { + let duplicate0 = cbor_map! { + 0 => "a", + -1 => "c", + b"a" => "e", + "c" => "g", + 0 => "b", + }; + assert_eq!(write_return(duplicate0), None); + let duplicate1 = cbor_map! { + 0 => "a", + -1 => "c", + b"a" => "e", + "c" => "g", + -1 => "d", + }; + assert_eq!(write_return(duplicate1), None); + let duplicate2 = cbor_map! { + 0 => "a", + -1 => "c", + b"a" => "e", + "c" => "g", + b"a" => "f", + }; + assert_eq!(write_return(duplicate2), None); + let duplicate3 = cbor_map! { + 0 => "a", + -1 => "c", + b"a" => "e", + "c" => "g", + "c" => "h", + }; + assert_eq!(write_return(duplicate3), None); + } + #[test] fn test_write_map_with_array() { let value_map = cbor_map! { diff --git a/src/ctap/command.rs b/src/ctap/command.rs index 4616b02..387a687 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -594,14 +594,14 @@ mod test { 0x01 => vec![0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F], 0x02 => cbor_map! { "id" => "example.com", - "name" => "Example", "icon" => "example.com/icon.png", + "name" => "Example", }, 0x03 => cbor_map! { "id" => vec![0x1D, 0x1D, 0x1D, 0x1D], + "icon" => "example.com/foo/icon.png", "name" => "foo", "displayName" => "bar", - "icon" => "example.com/foo/icon.png", }, 0x04 => cbor_array![ES256_CRED_PARAM], 0x05 => cbor_array![], @@ -656,8 +656,8 @@ mod test { 0x01 => "example.com", 0x02 => vec![0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F], 0x03 => cbor_array![ cbor_map! { - "type" => "public-key", "id" => vec![0x2D, 0x2D, 0x2D, 0x2D], + "type" => "public-key", "transports" => cbor_array!["usb"], } ], 0x06 => vec![0x12, 0x34], diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index b135a11..673b850 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -13,7 +13,6 @@ // limitations under the License. use super::status_code::Ctap2StatusCode; -use alloc::collections::BTreeMap; use alloc::string::String; use alloc::vec::Vec; use arrayref::array_ref; @@ -62,8 +61,8 @@ impl From for cbor::Value { fn from(entity: PublicKeyCredentialRpEntity) -> Self { cbor_map_options! { "id" => entity.rp_id, - "name" => entity.rp_name, "icon" => entity.rp_icon, + "name" => entity.rp_name, } } } @@ -108,9 +107,9 @@ impl From for cbor::Value { fn from(entity: PublicKeyCredentialUserEntity) -> Self { cbor_map_options! { "id" => entity.user_id, + "icon" => entity.user_icon, "name" => entity.user_name, "displayName" => entity.user_display_name, - "icon" => entity.user_icon, } } } @@ -174,8 +173,8 @@ impl TryFrom for PublicKeyCredentialParameter { impl From for cbor::Value { fn from(cred_param: PublicKeyCredentialParameter) -> Self { cbor_map_options! { - "type" => cred_param.cred_type, "alg" => cred_param.alg, + "type" => cred_param.cred_type, } } } @@ -262,8 +261,8 @@ impl TryFrom for PublicKeyCredentialDescriptor { impl From for cbor::Value { fn from(desc: PublicKeyCredentialDescriptor) -> Self { cbor_map_options! { - "type" => desc.key_type, "id" => desc.key_id, + "type" => desc.key_type, "transports" => desc.transports.map(|vec| cbor_array_vec!(vec)), } } @@ -1119,7 +1118,7 @@ pub(super) fn extract_array(cbor_value: cbor::Value) -> Result, pub(super) fn extract_map( cbor_value: cbor::Value, -) -> Result, Ctap2StatusCode> { +) -> Result, Ctap2StatusCode> { match cbor_value { cbor::Value::Map(map) => Ok(map), _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE), @@ -1142,7 +1141,6 @@ pub(super) fn ok_or_missing(value_option: Option) -> Result cbor_false!(), - "foo" => b"bar", b"bin" => -42, + "foo" => b"bar", }), - Ok([ + Ok(vec![ (cbor_unsigned!(1), cbor_false!()), - (cbor_text!("foo"), cbor_bytes_lit!(b"bar")), (cbor_bytes_lit!(b"bin"), cbor_int!(-42)), - ] - .iter() - .cloned() - .collect::>()) + (cbor_text!("foo"), cbor_bytes_lit!(b"bar")), + ]) ); } @@ -1419,8 +1414,8 @@ mod test { fn test_from_public_key_credential_rp_entity() { let cbor_rp_entity = cbor_map! { "id" => "example.com", - "name" => "Example", "icon" => "example.com/icon.png", + "name" => "Example", }; let rp_entity = PublicKeyCredentialRpEntity::try_from(cbor_rp_entity); let expected_rp_entity = PublicKeyCredentialRpEntity { @@ -1435,9 +1430,9 @@ mod test { fn test_from_into_public_key_credential_user_entity() { let cbor_user_entity = cbor_map! { "id" => vec![0x1D, 0x1D, 0x1D, 0x1D], + "icon" => "example.com/foo/icon.png", "name" => "foo", "displayName" => "bar", - "icon" => "example.com/foo/icon.png", }; let user_entity = PublicKeyCredentialUserEntity::try_from(cbor_user_entity.clone()); let expected_user_entity = PublicKeyCredentialUserEntity { @@ -1541,8 +1536,8 @@ mod test { #[test] fn test_from_into_public_key_credential_parameter() { let cbor_credential_parameter = cbor_map! { - "type" => "public-key", "alg" => ES256_ALGORITHM, + "type" => "public-key", }; let credential_parameter = PublicKeyCredentialParameter::try_from(cbor_credential_parameter.clone()); @@ -1558,8 +1553,8 @@ mod test { #[test] fn test_from_into_public_key_credential_descriptor() { let cbor_credential_descriptor = cbor_map! { - "type" => "public-key", "id" => vec![0x2D, 0x2D, 0x2D, 0x2D], + "type" => "public-key", "transports" => cbor_array!["usb"], }; let credential_descriptor = @@ -1577,11 +1572,11 @@ mod test { #[test] fn test_from_make_credential_extensions() { let cbor_extensions = cbor_map! { - "hmac-secret" => true, - "credProtect" => CredentialProtectionPolicy::UserVerificationRequired, - "minPinLength" => true, "credBlob" => vec![0xCB], + "credProtect" => CredentialProtectionPolicy::UserVerificationRequired, + "hmac-secret" => true, "largeBlobKey" => true, + "minPinLength" => true, }; let extensions = MakeCredentialExtensions::try_from(cbor_extensions); let expected_extensions = MakeCredentialExtensions { @@ -1601,12 +1596,12 @@ mod test { let pk = sk.genpk(); let cose_key = CoseKey::from(pk); let cbor_extensions = cbor_map! { + "credBlob" => true, "hmac-secret" => cbor_map! { 1 => cbor::Value::from(cose_key.clone()), 2 => vec![0x02; 32], 3 => vec![0x03; 16], }, - "credBlob" => true, "largeBlobKey" => true, }; let extensions = GetAssertionExtensions::try_from(cbor_extensions); @@ -1631,13 +1626,13 @@ mod test { let pk = sk.genpk(); let cose_key = CoseKey::from(pk); let cbor_extensions = cbor_map! { + "credBlob" => true, "hmac-secret" => cbor_map! { 1 => cbor::Value::from(cose_key.clone()), 2 => vec![0x02; 32], 3 => vec![0x03; 16], 4 => 2, }, - "credBlob" => true, "largeBlobKey" => true, }; let extensions = GetAssertionExtensions::try_from(cbor_extensions); @@ -1690,7 +1685,7 @@ mod test { let cbor_packed_attestation_statement = cbor_map! { "alg" => 1, "sig" => vec![0x55, 0x55, 0x55, 0x55], - "x5c" => cbor_array_vec![vec![certificate]], + "x5c" => cbor_array![certificate], "ecdaaKeyId" => vec![0xEC, 0xDA, 0x1D], }; let packed_attestation_statement = PackedAttestationStatement { @@ -1878,7 +1873,7 @@ mod test { }; let cbor_params = cbor_map! { 0x01 => 6, - 0x02 => cbor_array_vec!(vec!["example.com".to_string()]), + 0x02 => cbor_array!("example.com".to_string()), 0x03 => true, }; assert_eq!(cbor::Value::from(params.clone()), cbor_params); @@ -1897,7 +1892,7 @@ mod test { ConfigSubCommandParams::SetMinPinLength(set_min_pin_length_params); let cbor_params = cbor_map! { 0x01 => 6, - 0x02 => cbor_array_vec!(vec!["example.com".to_string()]), + 0x02 => cbor_array!("example.com".to_string()), 0x03 => true, }; assert_eq!(cbor::Value::from(config_sub_command_params), cbor_params); diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 548523a..eaca021 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -63,7 +63,6 @@ use self::timed_permission::TimedPermission; #[cfg(feature = "with_ctap1")] use self::timed_permission::U2fUserPresenceState; use alloc::boxed::Box; -use alloc::collections::BTreeMap; use alloc::string::{String, ToString}; use alloc::vec; use alloc::vec::Vec; @@ -706,10 +705,10 @@ where }; let cred_protect_output = extensions.cred_protect.and(cred_protect_policy); let extensions_output = cbor_map_options! { - "hmac-secret" => hmac_secret_output, - "credProtect" => cred_protect_output, - "minPinLength" => min_pin_length_output, "credBlob" => cred_blob_output, + "credProtect" => cred_protect_output, + "hmac-secret" => hmac_secret_output, + "minPinLength" => min_pin_length_output, }; if !cbor::write(extensions_output, &mut auth_data) { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); @@ -805,8 +804,8 @@ where None }; let extensions_output = cbor_map_options! { - "hmac-secret" => encrypted_output, "credBlob" => cred_blob, + "hmac-secret" => encrypted_output, }; if !cbor::write(extensions_output, &mut auth_data) { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); @@ -1033,26 +1032,29 @@ where versions.insert(0, String::from(U2F_VERSION_STRING)) } } - let mut options_map = BTreeMap::new(); - options_map.insert(String::from("rk"), true); - options_map.insert( - String::from("clientPin"), - self.persistent_store.pin_hash()?.is_some(), - ); - options_map.insert(String::from("up"), true); - options_map.insert(String::from("pinUvAuthToken"), true); - options_map.insert(String::from("largeBlobs"), true); + let mut options = vec![]; if ENTERPRISE_ATTESTATION_MODE.is_some() { - options_map.insert( + options.push(( String::from("ep"), self.persistent_store.enterprise_attestation()?, - ); + )); } - options_map.insert(String::from("authnrCfg"), true); - options_map.insert(String::from("credMgmt"), true); - options_map.insert(String::from("setMinPINLength"), true); - options_map.insert(String::from("makeCredUvNotRqd"), !has_always_uv); - options_map.insert(String::from("alwaysUv"), has_always_uv); + options.append(&mut vec![ + (String::from("rk"), true), + (String::from("up"), true), + (String::from("alwaysUv"), has_always_uv), + (String::from("credMgmt"), true), + (String::from("authnrCfg"), true), + ( + String::from("clientPin"), + self.persistent_store.pin_hash()?.is_some(), + ), + (String::from("largeBlobs"), true), + (String::from("pinUvAuthToken"), true), + (String::from("setMinPINLength"), true), + (String::from("makeCredUvNotRqd"), !has_always_uv), + ]); + Ok(ResponseData::AuthenticatorGetInfo( AuthenticatorGetInfoResponse { versions, @@ -1064,7 +1066,7 @@ where String::from("largeBlobKey"), ]), aaguid: self.persistent_store.aaguid()?, - options: Some(options_map), + options: Some(options), max_msg_size: Some(MAX_MSG_SIZE as u64), // The order implies preference. We favor the new V2. pin_protocols: Some(vec![ @@ -1285,33 +1287,33 @@ mod test { String::from(FIDO2_VERSION_STRING), String::from(FIDO2_1_VERSION_STRING), ]], - 0x02 => cbor_array_vec![vec![ + 0x02 => cbor_array![ String::from("hmac-secret"), String::from("credProtect"), String::from("minPinLength"), String::from("credBlob"), String::from("largeBlobKey"), - ]], + ], 0x03 => ctap_state.persistent_store.aaguid().unwrap(), 0x04 => cbor_map_options! { - "rk" => true, - "clientPin" => false, - "up" => true, - "pinUvAuthToken" => true, - "largeBlobs" => true, "ep" => ENTERPRISE_ATTESTATION_MODE.map(|_| false), - "authnrCfg" => true, + "rk" => true, + "up" => true, + "alwaysUv" => false, "credMgmt" => true, + "authnrCfg" => true, + "clientPin" => false, + "largeBlobs" => true, + "pinUvAuthToken" => true, "setMinPINLength" => true, "makeCredUvNotRqd" => true, - "alwaysUv" => false, }, 0x05 => MAX_MSG_SIZE as u64, - 0x06 => cbor_array_vec![vec![2, 1]], + 0x06 => cbor_array![2, 1], 0x07 => MAX_CREDENTIAL_COUNT_IN_LIST.map(|c| c as u64), 0x08 => CREDENTIAL_ID_SIZE as u64, - 0x09 => cbor_array_vec![vec!["usb"]], - 0x0A => cbor_array_vec![vec![ES256_CRED_PARAM]], + 0x09 => cbor_array!["usb"], + 0x0A => cbor_array![ES256_CRED_PARAM], 0x0B => MAX_LARGE_BLOB_ARRAY_SIZE as u64, 0x0C => false, 0x0D => ctap_state.persistent_store.min_pin_length().unwrap() as u64, diff --git a/src/ctap/response.rs b/src/ctap/response.rs index 6a47172..2568bae 100644 --- a/src/ctap/response.rs +++ b/src/ctap/response.rs @@ -17,10 +17,9 @@ use super::data_formats::{ PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, }; -use alloc::collections::BTreeMap; use alloc::string::String; use alloc::vec::Vec; -use cbor::{cbor_array_vec, cbor_bool, cbor_int, cbor_map_btree, cbor_map_options, cbor_text}; +use cbor::{cbor_array_vec, cbor_bool, cbor_int, cbor_map_collection, cbor_map_options, cbor_text}; #[derive(Debug, PartialEq)] pub enum ResponseData { @@ -123,7 +122,7 @@ pub struct AuthenticatorGetInfoResponse { pub versions: Vec, pub extensions: Option>, pub aaguid: [u8; 16], - pub options: Option>, + pub options: Option>, pub max_msg_size: Option, pub pin_protocols: Option>, pub max_credential_count_in_list: Option, @@ -141,7 +140,7 @@ pub struct AuthenticatorGetInfoResponse { // - 0x12: uvModality // Add them when your hardware supports any kind of user verification within // the boundary of the device, e.g. fingerprint or built-in keyboard. - pub certifications: Option>, + pub certifications: Option>, pub remaining_discoverable_credentials: Option, // - 0x15: vendorPrototypeConfigCommands missing as we don't support it. } @@ -170,19 +169,19 @@ impl From for cbor::Value { } = get_info_response; let options_cbor: Option = options.map(|options| { - let options_map: BTreeMap<_, _> = options + let options_map: Vec<(_, _)> = options .into_iter() .map(|(key, value)| (cbor_text!(key), cbor_bool!(value))) .collect(); - cbor_map_btree!(options_map) + cbor_map_collection!(options_map) }); let certifications_cbor: Option = certifications.map(|certifications| { - let certifications_map: BTreeMap<_, _> = certifications + let certifications_map: Vec<(_, _)> = certifications .into_iter() .map(|(key, value)| (cbor_text!(key), cbor_int!(value))) .collect(); - cbor_map_btree!(certifications_map) + cbor_map_collection!(certifications_map) }); cbor_map_options! { @@ -337,7 +336,7 @@ mod test { let cbor_packed_attestation_statement = cbor_map! { "alg" => 1, "sig" => vec![0x55, 0x55, 0x55, 0x55], - "x5c" => cbor_array_vec![vec![certificate]], + "x5c" => cbor_array![certificate], "ecdaaKeyId" => vec![0xEC, 0xDA, 0x1D], }; @@ -385,17 +384,17 @@ mod test { ResponseData::AuthenticatorGetAssertion(get_assertion_response).into(); let expected_cbor = cbor_map_options! { 0x01 => cbor_map! { - "type" => "public-key", "id" => vec![0x2D, 0x2D, 0x2D, 0x2D], + "type" => "public-key", "transports" => cbor_array!["usb"], }, 0x02 => vec![0xAD], 0x03 => vec![0x51], 0x04 => cbor_map! { "id" => vec![0x1D, 0x1D, 0x1D, 0x1D], + "icon" => "example.com/foo/icon.png".to_string(), "name" => "foo".to_string(), "displayName" => "bar".to_string(), - "icon" => "example.com/foo/icon.png".to_string(), }, 0x05 => 2, 0x07 => vec![0x1B], @@ -438,15 +437,11 @@ mod test { #[test] fn test_get_info_optionals_into_cbor() { - let mut options_map = BTreeMap::new(); - options_map.insert(String::from("rk"), true); - let mut certifications_map = BTreeMap::new(); - certifications_map.insert(String::from("example-cert"), 1); 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), + options: Some(vec![(String::from("rk"), true)]), max_msg_size: Some(1024), pin_protocols: Some(vec![1]), max_credential_count_in_list: Some(20), @@ -459,22 +454,22 @@ mod test { firmware_version: Some(0), max_cred_blob_length: Some(1024), max_rp_ids_for_set_min_pin_length: Some(8), - certifications: Some(certifications_map), + certifications: Some(vec![(String::from("example-cert"), 1)]), remaining_discoverable_credentials: Some(150), }; 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"]], + 0x01 => cbor_array!["FIDO_2_0"], + 0x02 => cbor_array!["extension"], 0x03 => vec![0x00; 16], 0x04 => cbor_map! {"rk" => true}, 0x05 => 1024, - 0x06 => cbor_array_vec![vec![1]], + 0x06 => cbor_array![1], 0x07 => 20, 0x08 => 256, - 0x09 => cbor_array_vec![vec!["usb"]], - 0x0A => cbor_array_vec![vec![ES256_CRED_PARAM]], + 0x09 => cbor_array!["usb"], + 0x0A => cbor_array![ES256_CRED_PARAM], 0x0B => 1024, 0x0C => false, 0x0D => 4, diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index a650d4d..2ba8d77 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -1370,13 +1370,13 @@ mod test { private_key, rp_id: String::from("example.com"), user_handle: vec![0x00], - user_display_name: None, - cred_protect_policy: None, + user_display_name: Some(String::from("Display Name")), + cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationOptional), creation_order: 0, - user_name: None, - user_icon: None, - cred_blob: None, - large_blob_key: None, + user_name: Some(String::from("name")), + user_icon: Some(String::from("icon")), + cred_blob: Some(vec![0xCB]), + large_blob_key: Some(vec![0x1B]), }; let serialized = serialize_credential(credential.clone()).unwrap(); let reconstructed = deserialize_credential(&serialized).unwrap();