Merge pull request #116 from gendx/read-cbor-map

Add a read_cbor_map macro to avoid removing values one-by-one in BTreeMaps

libraries/cbor/src/macros.rs

@@ -12,6 +12,142 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use crate::values::{KeyType, Value};
+use alloc::collections::btree_map;
+use core::cmp::Ordering;
+use core::iter::Peekable;
+
+/// This macro generates code to extract multiple values from a `BTreeMap<KeyType, Value>` at once
+/// in an optimized manner, consuming the input map.
+///
+/// It takes as input a `BTreeMap` 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<Value>`, to account for the case where keys aren't found.
+///
+/// **Important:** Keys passed to the `destructure_cbor_map!` macro **must be sorted** in increasing
+/// order. If not, the algorithm can yield incorrect results, such a assigning `None` to a variable
+/// even if the corresponding key existed in the map. **No runtime checks** are made for this in the
+/// `destructure_cbor_map!` macro, in order to avoid overhead at runtime. However, assertions that
+/// keys are sorted are added in `cfg(test)` mode, so that unit tests can verify ahead of time that
+/// 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.
+///
+/// ```rust
+/// # extern crate alloc;
+/// # #[macro_use]
+/// # extern crate cbor;
+/// #
+/// # fn main() {
+/// #     let map = alloc::collections::BTreeMap::new();
+/// destructure_cbor_map! {
+///     let {
+///         1 => x,
+///         "key" => y,
+///     } = map;
+/// }
+/// # }
+/// ```
+///
+/// ```rust
+/// # extern crate alloc;
+/// # #[macro_use]
+/// # extern crate cbor;
+/// #
+/// # fn main() {
+/// #     let mut map = alloc::collections::BTreeMap::<cbor::KeyType, _>::new();
+/// use cbor::values::IntoCborKey;
+/// let x: Option<cbor::Value> = map.remove(&1.into_cbor_key());
+/// let y: Option<cbor::Value> = map.remove(&"key".into_cbor_key());
+/// # }
+/// ```
+#[macro_export]
+macro_rules! destructure_cbor_map {
+    ( let { $( $key:expr => $variable:ident, )+ } = $map:expr; ) => {
+        // A pre-requisite for this algorithm to work is that the keys to extract from the map are
+        // sorted - the behavior is unspecified if the keys are not sorted.
+        // Therefore, in test mode we add assertions that the keys are indeed sorted.
+        #[cfg(test)]
+        assert_sorted_keys!($( $key, )+);
+
+        use $crate::values::{IntoCborKey, Value};
+        use $crate::macros::destructure_cbor_map_peek_value;
+
+        // This algorithm first converts the map into a peekable iterator - whose items are sorted
+        // in strictly increasing order of keys. Then, the repeated calls to the "peek value"
+        // helper function will consume this iterator and yield values (or `None`) when reaching
+        // the keys to extract.
+        //
+        // This is where the pre-requisite that keys to extract are sorted is important: the
+        // algorithm does a single linear scan over the iterator and therefore keys to extract have
+        // to come in the same order (i.e. sorted).
+        let mut it = $map.into_iter().peekable();
+        $(
+        let $variable: Option<Value> = destructure_cbor_map_peek_value(&mut it, $key.into_cbor_key());
+        )+
+    };
+}
+
+/// This function is an internal detail of the `destructure_cbor_map!` macro, but has public
+/// visibility so that users of the macro can use it.
+///
+/// Given a peekable iterator of key-value pairs sorted in strictly increasing key order and a
+/// needle key, this function consumes all items whose key compares less than or equal to the
+/// needle, and returns `Some(value)` if the needle was present as the key in the iterator and
+/// `None` otherwise.
+///
+/// The logic is separated into its own function to reduce binary size, as otherwise the logic
+/// 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<btree_map::IntoIter<KeyType, Value>>,
+    needle: KeyType,
+) -> Option<Value> {
+    loop {
+        match it.peek() {
+            None => return None,
+            Some(item) => {
+                let key: &KeyType = &item.0;
+                match key.cmp(&needle) {
+                    Ordering::Less => {
+                        it.next();
+                    }
+                    Ordering::Equal => {
+                        let value: Value = it.next().unwrap().1;
+                        return Some(value);
+                    }
+                    Ordering::Greater => return None,
+                }
+            }
+        }
+    }
+}
+
+#[macro_export]
+macro_rules! assert_sorted_keys {
+    // Last key
+    ( $key:expr, ) => {
+    };
+
+    ( $key1:expr, $key2:expr, $( $keys:expr, )* ) => {
+        {
+            use $crate::values::{IntoCborKey, KeyType};
+            let k1: KeyType = $key1.into_cbor_key();
+            let k2: KeyType = $key2.into_cbor_key();
+            assert!(
+                k1 < k2,
+                "{:?} < {:?} failed. The destructure_cbor_map! macro requires keys in sorted order.",
+                k1,
+                k2,
+            );
+        }
+        assert_sorted_keys!($key2, $( $keys, )*);
+    };
+}
+
 #[macro_export]
 macro_rules! cbor_map {
     // trailing comma case
@@ -497,4 +633,99 @@ mod test {
         );
         assert_eq!(a, b);
     }
+
+    fn extract_map(cbor_value: Value) -> BTreeMap<KeyType, Value> {
+        match cbor_value {
+            Value::Map(map) => map,
+            _ => panic!("Expected CBOR map."),
+        }
+    }
+
+    #[test]
+    fn test_destructure_cbor_map_simple() {
+        let map = cbor_map! {
+            1 => 10,
+            2 => 20,
+        };
+
+        destructure_cbor_map! {
+            let {
+                1 => x1,
+                2 => x2,
+            } = extract_map(map);
+        }
+
+        assert_eq!(x1, Some(cbor_unsigned!(10)));
+        assert_eq!(x2, Some(cbor_unsigned!(20)));
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_destructure_cbor_map_unsorted() {
+        let map = cbor_map! {
+            1 => 10,
+            2 => 20,
+        };
+
+        destructure_cbor_map! {
+            // The keys are not sorted here, which violates the precondition of
+            // destructure_cbor_map. An assertion should catch that and make the test panic.
+            let {
+                2 => _x2,
+                1 => _x1,
+            } = extract_map(map);
+        }
+    }
+
+    #[test]
+    fn test_destructure_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,
+        };
+
+        destructure_cbor_map! {
+            let {
+                3 => x3,
+                7 => x7,
+            } = extract_map(map);
+        }
+
+        assert_eq!(x3, Some(cbor_unsigned!(30)));
+        assert_eq!(x7, Some(cbor_unsigned!(70)));
+    }
+
+    #[test]
+    fn test_destructure_cbor_map_missing() {
+        let map = cbor_map! {
+            1 => 10,
+            3 => 30,
+            4 => 40,
+        };
+
+        destructure_cbor_map! {
+            let {
+                0 => x0,
+                1 => x1,
+                2 => x2,
+                3 => x3,
+                4 => x4,
+                5 => x5,
+            } = extract_map(map);
+        }
+
+        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);
+    }
 }

reproducible/reference_binaries_macos-10.15.sha256sum

@@ -1,9 +1,9 @@
 0b54df6d548849e24d67b9b022ca09cb33c51f078ce85d0c9c4635ffc69902e1	third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin
-f49e2205136159671f8291b284fc02300cf659f088a2ca301d74111e0e96849a	target/nrf52840dk_merged.hex
+e93f56b4b6bb602ab37cf967f1c3fd3d253e05ccc85d4718762f68216c35d68c	target/nrf52840dk_merged.hex
 052eec0ae526038352b9f7573468d0cf7fb5ec331d4dc1a2df75fdbd514ea5ca	third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin
-b35ac62a490c62d4b23dddf1d8e6946badb32b5b35b40bbd75587815530094c9	target/nrf52840_dongle_merged.hex
+8cff9a4d513be338ba6a3fd91d3d4cfdd63bc066e8bf9dc22f64176114da08b8	target/nrf52840_dongle_merged.hex
 908d7f4f40936d968b91ab6e19b2406612fe8c2c273d9c0b71ef1f55116780e0	third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin
-1adb9f71697947109020b25ad2b3fb3b03e6a07945dee14351ad67341241205e	target/nrf52840_dongle_dfu_merged.hex
+94452673fb0022a07ac886d4ab74576f067c2d727aed30ed368f4e5af382238f	target/nrf52840_dongle_dfu_merged.hex
 34ecbecaebf1188277f2310fe769c8c60310d8576493242712854deb4ba1036e	third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin
-1661fb4da7cbaf01529e593600f47c4613446a37f400cb0b238249d100a3d9f1	target/nrf52840_mdk_dfu_merged.hex
+414aaf7fcc3a0121ab02b1222d508ae503e268cb7da0df5795a6d6a01aeed345	target/nrf52840_mdk_dfu_merged.hex
-529ac9aef3941b45e7e480810ae4e821da433985b149028aa6a33f33e0dc1685	target/tab/ctap2.tab
+212698e7c7919fa4542e1263d56f601632902f86bdf3d48cf6300b96ad452cb1	target/tab/ctap2.tab

reproducible/reference_binaries_ubuntu-18.04.sha256sum

@@ -1,9 +1,9 @@
 29382e72d0f3c6a72ce9517211952ff29ea270193d7f0ddc48ca69009ee29925	third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin
-e446a94d67f77d5346be6e476641f4ff50561f5a77bfa8bc49262f89e7399893	target/nrf52840dk_merged.hex
+4d5165d8ff46148a585ade23d3030c8a95928a158d283ccd7c93e14902452b6f	target/nrf52840dk_merged.hex
 30f239390ae9bef0825731e4c82d40470fc5e9bded2bf0d942e92dbb5d4faba1	third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin
-1bf5219f7b096b4ade330e9b02544b09d10972ddf253c7fdfbd6241b03e98f31	target/nrf52840_dongle_merged.hex
+8204a87c9e93909ed79907f2d7b655d07397161ecd64bd213bc483630a38e8c9	target/nrf52840_dongle_merged.hex
 e3acf15d5ae3a22aecff6cc58db5fc311f538f47328d348b7ad7db7f9ab5e72c	third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin
-b83edda1b2588e3eff019fc8b2e16097e159f8a43fa5fc62a6e23497882c8dca	target/nrf52840_dongle_dfu_merged.hex
+b937eaeea7ae9ca3c26bee082cb5af596942947c84171cb4d03cc66bc31d35da	target/nrf52840_dongle_dfu_merged.hex
 cae312a26a513ada6c198fdc59b2bba3860c51726b817a9fd17a4331ee12c882	third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin
-d376cb19e672ab80b9dd25e9df40af7ac833d03ede32f4a2ae21fdfd4e31d365	target/nrf52840_mdk_dfu_merged.hex
+6d125877a207744a73b0b315152188a85329d31e8f85e8205ac6033e46056931	target/nrf52840_mdk_dfu_merged.hex
-ba0e11a0036f167a56864de43db3602a8a855b38be8a53afc3a97fcaa40f2201	target/tab/ctap2.tab
+e6dbbc68daa1b5269dce5ddbc91ea00169f9c8ed8d94a574dac1524e63c21b18	target/tab/ctap2.tab

reproducible/reference_elf2tab_macos-10.15.txt

@@ -5,8 +5,8 @@ Min RAM size from sections in ELF: 16 bytes
 Number of writeable flash regions: 0
   Adding .crt0_header section. Offset: 64 (0x40). Length: 64 (0x40) bytes.
 Entry point is in .text section
-  Adding .text section. Offset: 128 (0x80). Length: 171876 (0x29f64) bytes.
+  Adding .text section. Offset: 128 (0x80). Length: 179204 (0x2bc04) bytes.
-  Adding .stack section. Offset: 172004 (0x29fe4). Length: 16384 (0x4000) bytes.
+  Adding .stack section. Offset: 179332 (0x2bc84). Length: 16384 (0x4000) bytes.
 Searching for .rel.X sections to add.
 TBF Header:
                version:        2        0x2
@@ -24,8 +24,8 @@ Min RAM size from sections in ELF: 16 bytes
 Number of writeable flash regions: 0
   Adding .crt0_header section. Offset: 64 (0x40). Length: 64 (0x40) bytes.
 Entry point is in .text section
-  Adding .text section. Offset: 128 (0x80). Length: 171876 (0x29f64) bytes.
+  Adding .text section. Offset: 128 (0x80). Length: 179204 (0x2bc04) bytes.
-  Adding .stack section. Offset: 172004 (0x29fe4). Length: 16384 (0x4000) bytes.
+  Adding .stack section. Offset: 179332 (0x2bc84). Length: 16384 (0x4000) bytes.
 Searching for .rel.X sections to add.
 TBF Header:
                version:        2        0x2
@@ -43,8 +43,8 @@ Min RAM size from sections in ELF: 16 bytes
 Number of writeable flash regions: 0
   Adding .crt0_header section. Offset: 64 (0x40). Length: 64 (0x40) bytes.
 Entry point is in .text section
-  Adding .text section. Offset: 128 (0x80). Length: 171876 (0x29f64) bytes.
+  Adding .text section. Offset: 128 (0x80). Length: 179204 (0x2bc04) bytes.
-  Adding .stack section. Offset: 172004 (0x29fe4). Length: 16384 (0x4000) bytes.
+  Adding .stack section. Offset: 179332 (0x2bc84). Length: 16384 (0x4000) bytes.
 Searching for .rel.X sections to add.
 TBF Header:
                version:        2        0x2
@@ -62,8 +62,8 @@ Min RAM size from sections in ELF: 16 bytes
 Number of writeable flash regions: 0
   Adding .crt0_header section. Offset: 64 (0x40). Length: 64 (0x40) bytes.
 Entry point is in .text section
-  Adding .text section. Offset: 128 (0x80). Length: 171876 (0x29f64) bytes.
+  Adding .text section. Offset: 128 (0x80). Length: 179204 (0x2bc04) bytes.
-  Adding .stack section. Offset: 172004 (0x29fe4). Length: 16384 (0x4000) bytes.
+  Adding .stack section. Offset: 179332 (0x2bc84). Length: 16384 (0x4000) bytes.
 Searching for .rel.X sections to add.
 TBF Header:
                version:        2        0x2

reproducible/reference_elf2tab_ubuntu-18.04.txt

@@ -5,8 +5,8 @@ Min RAM size from sections in ELF: 16 bytes
 Number of writeable flash regions: 0
   Adding .crt0_header section. Offset: 64 (0x40). Length: 64 (0x40) bytes.
 Entry point is in .text section
-  Adding .text section. Offset: 128 (0x80). Length: 171316 (0x29d34) bytes.
+  Adding .text section. Offset: 128 (0x80). Length: 179236 (0x2bc24) bytes.
-  Adding .stack section. Offset: 171444 (0x29db4). Length: 16384 (0x4000) bytes.
+  Adding .stack section. Offset: 179364 (0x2bca4). Length: 16384 (0x4000) bytes.
 Searching for .rel.X sections to add.
 TBF Header:
                version:        2        0x2
@@ -24,8 +24,8 @@ Min RAM size from sections in ELF: 16 bytes
 Number of writeable flash regions: 0
   Adding .crt0_header section. Offset: 64 (0x40). Length: 64 (0x40) bytes.
 Entry point is in .text section
-  Adding .text section. Offset: 128 (0x80). Length: 171316 (0x29d34) bytes.
+  Adding .text section. Offset: 128 (0x80). Length: 179236 (0x2bc24) bytes.
-  Adding .stack section. Offset: 171444 (0x29db4). Length: 16384 (0x4000) bytes.
+  Adding .stack section. Offset: 179364 (0x2bca4). Length: 16384 (0x4000) bytes.
 Searching for .rel.X sections to add.
 TBF Header:
                version:        2        0x2
@@ -43,8 +43,8 @@ Min RAM size from sections in ELF: 16 bytes
 Number of writeable flash regions: 0
   Adding .crt0_header section. Offset: 64 (0x40). Length: 64 (0x40) bytes.
 Entry point is in .text section
-  Adding .text section. Offset: 128 (0x80). Length: 171316 (0x29d34) bytes.
+  Adding .text section. Offset: 128 (0x80). Length: 179236 (0x2bc24) bytes.
-  Adding .stack section. Offset: 171444 (0x29db4). Length: 16384 (0x4000) bytes.
+  Adding .stack section. Offset: 179364 (0x2bca4). Length: 16384 (0x4000) bytes.
 Searching for .rel.X sections to add.
 TBF Header:
                version:        2        0x2
@@ -62,8 +62,8 @@ Min RAM size from sections in ELF: 16 bytes
 Number of writeable flash regions: 0
   Adding .crt0_header section. Offset: 64 (0x40). Length: 64 (0x40) bytes.
 Entry point is in .text section
-  Adding .text section. Offset: 128 (0x80). Length: 171316 (0x29d34) bytes.
+  Adding .text section. Offset: 128 (0x80). Length: 179236 (0x2bc24) bytes.
-  Adding .stack section. Offset: 171444 (0x29db4). Length: 16384 (0x4000) bytes.
+  Adding .stack section. Offset: 179364 (0x2bca4). Length: 16384 (0x4000) bytes.
 Searching for .rel.X sections to add.
 TBF Header:
                version:        2        0x2

src/ctap/command.rs

@@ -124,26 +124,31 @@ impl TryFrom<cbor::Value> for AuthenticatorMakeCredentialParameters {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut param_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
+            let {
+                1 => client_data_hash,
+                2 => rp,
+                3 => user,
+                4 => cred_param_vec,
+                5 => exclude_list,
+                6 => extensions,
+                7 => options,
+                8 => pin_uv_auth_param,
+                9 => pin_uv_auth_protocol,
+            } = extract_map(cbor_value)?;
+        }
 
-        let client_data_hash =
+        let client_data_hash = extract_byte_string(ok_or_missing(client_data_hash)?)?;
-            extract_byte_string(ok_or_missing(param_map.remove(&cbor_unsigned!(1)))?)?;
+        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(
+        let cred_param_vec = extract_array(ok_or_missing(cred_param_vec)?)?;
-            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 pub_key_cred_params = cred_param_vec
             .into_iter()
             .map(PublicKeyCredentialParameter::try_from)
             .collect::<Result<Vec<PublicKeyCredentialParameter>, 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 +162,11 @@ impl TryFrom<cbor::Value> for AuthenticatorMakeCredentialParameters {
             None => None,
         };
 
-        let extensions = param_map
+        let extensions = extensions
-            .remove(&cbor_unsigned!(6))
             .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 +174,8 @@ impl TryFrom<cbor::Value> for AuthenticatorMakeCredentialParameters {
             },
         };
 
-        let pin_uv_auth_param = param_map
+        let pin_uv_auth_param = pin_uv_auth_param.map(extract_byte_string).transpose()?;
-            .remove(&cbor_unsigned!(8))
+        let pin_uv_auth_protocol = pin_uv_auth_protocol.map(extract_unsigned).transpose()?;
-            .map(extract_byte_string)
-            .transpose()?;
-
-        let pin_uv_auth_protocol = param_map
-            .remove(&cbor_unsigned!(9))
-            .map(extract_unsigned)
-            .transpose()?;
 
         Ok(AuthenticatorMakeCredentialParameters {
             client_data_hash,
@@ -210,14 +207,22 @@ impl TryFrom<cbor::Value> for AuthenticatorGetAssertionParameters {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut param_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
+            let {
+                1 => rp_id,
+                2 => client_data_hash,
+                3 => allow_list,
+                4 => extensions,
+                5 => options,
+                6 => pin_uv_auth_param,
+                7 => pin_uv_auth_protocol,
+            } = extract_map(cbor_value)?;
+        }
 
-        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 =
+        let allow_list = match allow_list {
-            extract_byte_string(ok_or_missing(param_map.remove(&cbor_unsigned!(2)))?)?;
-
-        let allow_list = match param_map.remove(&cbor_unsigned!(3)) {
             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 +236,11 @@ impl TryFrom<cbor::Value> for AuthenticatorGetAssertionParameters {
             None => None,
         };
 
-        let extensions = param_map
+        let extensions = extensions
-            .remove(&cbor_unsigned!(4))
             .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 +248,8 @@ impl TryFrom<cbor::Value> for AuthenticatorGetAssertionParameters {
             },
         };
 
-        let pin_uv_auth_param = param_map
+        let pin_uv_auth_param = pin_uv_auth_param.map(extract_byte_string).transpose()?;
-            .remove(&cbor_unsigned!(6))
+        let pin_uv_auth_protocol = pin_uv_auth_protocol.map(extract_unsigned).transpose()?;
-            .map(extract_byte_string)
-            .transpose()?;
-
-        let pin_uv_auth_protocol = param_map
-            .remove(&cbor_unsigned!(7))
-            .map(extract_unsigned)
-            .transpose()?;
 
         Ok(AuthenticatorGetAssertionParameters {
             rp_id,
@@ -280,33 +277,26 @@ impl TryFrom<cbor::Value> for AuthenticatorClientPinParameters {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut param_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
+            let {
+                1 => pin_protocol,
+                2 => sub_command,
+                3 => key_agreement,
+                4 => pin_auth,
+                5 => new_pin_enc,
+                6 => pin_hash_enc,
+            } = extract_map(cbor_value)?;
+        }
 
-        let pin_protocol = extract_unsigned(ok_or_missing(param_map.remove(&cbor_unsigned!(1)))?)?;
+        let pin_protocol = extract_unsigned(ok_or_missing(pin_protocol)?)?;
-
+        let sub_command = ClientPinSubCommand::try_from(ok_or_missing(sub_command)?)?;
-        let sub_command =
+        let key_agreement = key_agreement
-            ClientPinSubCommand::try_from(ok_or_missing(param_map.remove(&cbor_unsigned!(2)))?)?;
-
-        let key_agreement = param_map
-            .remove(&cbor_unsigned!(3))
             .map(extract_map)
             .transpose()?
             .map(|x| CoseKey(x));
-
+        let pin_auth = pin_auth.map(extract_byte_string).transpose()?;
-        let pin_auth = param_map
+        let new_pin_enc = new_pin_enc.map(extract_byte_string).transpose()?;
-            .remove(&cbor_unsigned!(4))
+        let pin_hash_enc = pin_hash_enc.map(extract_byte_string).transpose()?;
-            .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()?;
 
         Ok(AuthenticatorClientPinParameters {
             pin_protocol,

src/ctap/data_formats.rs

@@ -31,16 +31,18 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialRpEntity {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut rp_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        let rp_id = extract_text_string(ok_or_missing(rp_map.remove(&cbor_text!("id")))?)?;
+            let {
-        let rp_name = rp_map
+                "id" => rp_id,
-            .remove(&cbor_text!("name"))
+                "icon" => rp_icon,
-            .map(extract_text_string)
+                "name" => rp_name,
-            .transpose()?;
+            } = extract_map(cbor_value)?;
-        let rp_icon = rp_map
+        }
-            .remove(&cbor_text!("icon"))
+
-            .map(extract_text_string)
+        let rp_id = extract_text_string(ok_or_missing(rp_id)?)?;
-            .transpose()?;
+        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 +64,20 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialUserEntity {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut user_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        let user_id = extract_byte_string(ok_or_missing(user_map.remove(&cbor_text!("id")))?)?;
+            let {
-        let user_name = user_map
+                "id" => user_id,
-            .remove(&cbor_text!("name"))
+                "icon" => user_icon,
-            .map(extract_text_string)
+                "name" => user_name,
-            .transpose()?;
+                "displayName" => user_display_name,
-        let user_display_name = user_map
+            } = extract_map(cbor_value)?;
-            .remove(&cbor_text!("displayName"))
+        }
-            .map(extract_text_string)
+
-            .transpose()?;
+        let user_id = extract_byte_string(ok_or_missing(user_id)?)?;
-        let user_icon = user_map
+        let user_name = user_name.map(extract_text_string).transpose()?;
-            .remove(&cbor_text!("icon"))
+        let user_display_name = user_display_name.map(extract_text_string).transpose()?;
-            .map(extract_text_string)
+        let user_icon = user_icon.map(extract_text_string).transpose()?;
-            .transpose()?;
+
         Ok(Self {
             user_id,
             user_name,
@@ -141,13 +143,15 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialParameter {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut cred_param_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        let cred_type = PublicKeyCredentialType::try_from(ok_or_missing(
+            let {
-            cred_param_map.remove(&cbor_text!("type")),
+                "alg" => alg,
-        )?)?;
+                "type" => cred_type,
-        let alg = SignatureAlgorithm::try_from(ok_or_missing(
+            } = extract_map(cbor_value)?;
-            cred_param_map.remove(&cbor_text!("alg")),
+        }
-        )?)?;
+
+        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 +213,17 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialDescriptor {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut cred_desc_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        let key_type = PublicKeyCredentialType::try_from(ok_or_missing(
+            let {
-            cred_desc_map.remove(&cbor_text!("type")),
+                "id" => key_id,
-        )?)?;
+                "type" => key_type,
-        let key_id = extract_byte_string(ok_or_missing(cred_desc_map.remove(&cbor_text!("id")))?)?;
+                "transports" => transports,
-        let transports = match cred_desc_map.remove(&cbor_text!("transports")) {
+            } = extract_map(cbor_value)?;
+        }
+
+        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 +234,7 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialDescriptor {
             }
             None => None,
         };
+
         Ok(Self {
             key_type,
             key_id,
@@ -253,12 +263,15 @@ impl TryFrom<cbor::Value> for MakeCredentialExtensions {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut extensions_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        let hmac_secret = extensions_map
+            let {
-            .remove(&cbor_text!("hmac-secret"))
+                "credProtect" => cred_protect,
-            .map_or(Ok(false), extract_bool)?;
+                "hmac-secret" => hmac_secret,
-        let cred_protect = extensions_map
+            } = extract_map(cbor_value)?;
-            .remove(&cbor_text!("credProtect"))
+        }
+
+        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 +290,13 @@ impl TryFrom<cbor::Value> for GetAssertionExtensions {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut extensions_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        let hmac_secret = extensions_map
+            let {
-            .remove(&cbor_text!("hmac-secret"))
+                "hmac-secret" => hmac_secret,
+            } = extract_map(cbor_value)?;
+        }
+
+        let hmac_secret = hmac_secret
             .map(GetAssertionHmacSecretInput::try_from)
             .transpose()?;
         Ok(Self { hmac_secret })
@@ -297,10 +314,17 @@ impl TryFrom<cbor::Value> for GetAssertionHmacSecretInput {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut input_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        let cose_key = extract_map(ok_or_missing(input_map.remove(&cbor_unsigned!(1)))?)?;
+            let {
-        let salt_enc = extract_byte_string(ok_or_missing(input_map.remove(&cbor_unsigned!(2)))?)?;
+                1 => cose_key,
-        let salt_auth = extract_byte_string(ok_or_missing(input_map.remove(&cbor_unsigned!(3)))?)?;
+                2 => salt_enc,
+                3 => salt_auth,
+            } = extract_map(cbor_value)?;
+        }
+
+        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 +344,24 @@ impl TryFrom<cbor::Value> for MakeCredentialOptions {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut options_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        let rk = match options_map.remove(&cbor_text!("rk")) {
+            let {
+                "rk" => rk,
+                "up" => up,
+                "uv" => uv,
+            } = extract_map(cbor_value)?;
+        }
+
+        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 +379,24 @@ impl TryFrom<cbor::Value> for GetAssertionOptions {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        let mut options_map = extract_map(cbor_value)?;
+        destructure_cbor_map! {
-        if let Some(options_entry) = options_map.remove(&cbor_text!("rk")) {
+            let {
+                "rk" => rk,
+                "up" => up,
+                "uv" => uv,
+            } = extract_map(cbor_value)?;
+        }
+
+        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,
         };
@@ -501,27 +539,33 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        use PublicKeyCredentialSourceField::*;
+        use PublicKeyCredentialSourceField::{
-        let mut map = extract_map(cbor_value)?;
+            CredProtectPolicy, CredRandom, CredentialId, OtherUi, PrivateKey, RpId, UserHandle,
-        let credential_id = extract_byte_string(ok_or_missing(map.remove(&CredentialId.into()))?)?;
+        };
-        let private_key = extract_byte_string(ok_or_missing(map.remove(&PrivateKey.into()))?)?;
+        destructure_cbor_map! {
+            let {
+                CredentialId => credential_id,
+                PrivateKey => private_key,
+                RpId => rp_id,
+                UserHandle => user_handle,
+                OtherUi => other_ui,
+                CredRandom => cred_random,
+                CredProtectPolicy => cred_protect_policy,
+            } = extract_map(cbor_value)?;
+        }
+
+        let credential_id = extract_byte_string(ok_or_missing(credential_id)?)?;
+        let private_key = extract_byte_string(ok_or_missing(private_key)?)?;
         if private_key.len() != 32 {
             return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR);
         }
         let private_key = ecdsa::SecKey::from_bytes(array_ref!(private_key, 0, 32))
             .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)?;
-        let rp_id = extract_text_string(ok_or_missing(map.remove(&RpId.into()))?)?;
+        let rp_id = extract_text_string(ok_or_missing(rp_id)?)?;
-        let user_handle = extract_byte_string(ok_or_missing(map.remove(&UserHandle.into()))?)?;
+        let user_handle = extract_byte_string(ok_or_missing(user_handle)?)?;
-        let other_ui = map
+        let other_ui = other_ui.map(extract_text_string).transpose()?;
-            .remove(&OtherUi.into())
+        let cred_random = cred_random.map(extract_byte_string).transpose()?;
-            .map(extract_text_string)
+        let cred_protect_policy = cred_protect_policy
-            .transpose()?;
-        let cred_random = map
-            .remove(&CredRandom.into())
-            .map(extract_byte_string)
-            .transpose()?;
-        let cred_protect_policy = map
-            .remove(&CredProtectPolicy.into())
             .map(CredentialProtectionPolicy::try_from)
             .transpose()?;
         // We don't return whether there were unknown fields in the CBOR value. This means that
@@ -599,27 +643,37 @@ impl TryFrom<CoseKey> for ecdh::PubKey {
     type Error = Ctap2StatusCode;
 
     fn try_from(cose_key: CoseKey) -> Result<Self, Ctap2StatusCode> {
-        let mut cose_map = cose_key.0;
+        destructure_cbor_map! {
-        let key_type = extract_integer(ok_or_missing(cose_map.remove(&cbor_int!(1)))?)?;
+            let {
+                1 => key_type,
+                3 => algorithm,
+                -1 => curve,
+                -2 => x_bytes,
+                -3 => y_bytes,
+            } = cose_key.0;
+        }
+
+        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)