adds clarifications, improvements and tests

src/ctap/data_formats.rs

@@ -1166,6 +1166,13 @@ mod test {
         let policy_error = CredentialProtectionPolicy::try_from(cbor_policy_error);
         let expected_error = Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
         assert_eq!(policy_error, expected_error);
+
+        for policy_literal in 1..=3 {
+            let cbor_policy: cbor::Value = cbor_int!(policy_literal);
+            let policy = CredentialProtectionPolicy::try_from(cbor_policy.clone());
+            let created_cbor: cbor::Value = policy.unwrap().into();
+            assert_eq!(created_cbor, cbor_policy);
+        }
     }
 
     #[test]
@@ -1180,6 +1187,15 @@ mod test {
         );
         let created_cbor: cbor::Value = authenticator_transport.unwrap().into();
         assert_eq!(created_cbor, cbor_authenticator_transport);
+
+        let transports = ["usb", "nfc", "ble", "internal"];
+        for transport in transports.iter() {
+            let cbor_authenticator_transport: cbor::Value = cbor_text!(*transport);
+            let authenticator_transport =
+                AuthenticatorTransport::try_from(cbor_authenticator_transport.clone());
+            let created_cbor: cbor::Value = authenticator_transport.unwrap().into();
+            assert_eq!(created_cbor, cbor_authenticator_transport);
+        }
     }
 
     #[test]
@@ -1322,6 +1338,17 @@ mod test {
         assert_eq!(sub_command, Ok(expected_sub_command));
         let created_cbor: cbor::Value = sub_command.unwrap().into();
         assert_eq!(created_cbor, cbor_sub_command);
+
+        #[cfg(not(feature = "with_ctap2_1"))]
+        let last_literal = 0x05;
+        #[cfg(feature = "with_ctap2_1")]
+        let last_literal = 0x09;
+        for command_literal in 1..=last_literal {
+            let cbor_sub_command: cbor::Value = cbor_int!(command_literal);
+            let sub_command = ClientPinSubCommand::try_from(cbor_sub_command.clone());
+            let created_cbor: cbor::Value = sub_command.unwrap().into();
+            assert_eq!(created_cbor, cbor_sub_command);
+        }
     }
 
     #[test]

src/ctap/pin_protocol_v1.rs

@@ -536,6 +536,11 @@ impl PinProtocolV1 {
         self.key_agreement_key = crypto::ecdh::SecKey::gensk(rng);
         self.pin_uv_auth_token = rng.gen_uniform_u8x32();
         self.consecutive_pin_mismatches = 0;
+        #[cfg(feature = "with_ctap2_1")]
+        {
+            self.permissions = 0;
+            self.permissions_rp_id = None;
+        }
     }
 
     pub fn process_hmac_secret(

src/ctap/storage.rs

@@ -62,13 +62,11 @@ const PIN_RETRIES: usize = 4;
 const ATTESTATION_PRIVATE_KEY: usize = 5;
 const ATTESTATION_CERTIFICATE: usize = 6;
 const AAGUID: usize = 7;
-#[cfg(not(feature = "with_ctap2_1"))]
-const NUM_TAGS: usize = 8;
 #[cfg(feature = "with_ctap2_1")]
 const MIN_PIN_LENGTH: usize = 8;
 #[cfg(feature = "with_ctap2_1")]
 const MIN_PIN_LENGTH_RP_IDS: usize = 9;
-#[cfg(feature = "with_ctap2_1")]
+// Different NUM_TAGS make the storage incompatible, so we use max(8,10).
 const NUM_TAGS: usize = 10;
 
 const MAX_PIN_RETRIES: u8 = 6;
@@ -397,9 +395,13 @@ impl PersistentStore {
     }
 
     pub fn pin_retries(&self) -> u8 {
-        self.store
-            .find_one(&Key::PinRetries)
-            .map_or(MAX_PIN_RETRIES, |(_, entry)| entry.data[0])
+        match self.store.find_one(&Key::PinRetries) {
+            None => MAX_PIN_RETRIES,
+            Some((_, entry)) => {
+                debug_assert_eq!(entry.data.len(), 1);
+                entry.data[0]
+            }
+        }
     }
 
     pub fn decr_pin_retries(&mut self) {
@@ -415,17 +417,19 @@ impl PersistentStore {
             }
             Some((index, entry)) => {
                 debug_assert_eq!(entry.data.len(), 1);
-                let new_value = entry.data[0].saturating_sub(1);
-                self.store
-                    .replace(
-                        index,
-                        StoreEntry {
-                            tag: PIN_RETRIES,
-                            data: &[new_value],
-                            sensitive: false,
-                        },
-                    )
-                    .unwrap();
+                if entry.data[0] > 0 {
+                    let new_value = entry.data[0].saturating_sub(1);
+                    self.store
+                        .replace(
+                            index,
+                            StoreEntry {
+                                tag: PIN_RETRIES,
+                                data: &[new_value],
+                                sensitive: false,
+                            },
+                        )
+                        .unwrap();
+                }
             }
         }
     }
@@ -1022,12 +1026,17 @@ mod test {
             _DEFAULT_MIN_PIN_LENGTH_RP_IDS
         );
 
-        // Changes by the setter are reflected by the getter..
-        let rp_ids = vec![String::from("example.com")];
+        // Changes by the setter are reflected by the getter.
+        let mut rp_ids = vec![String::from("example.com")];
         assert_eq!(
             persistent_store._set_min_pin_length_rp_ids(rp_ids.clone()),
             Ok(())
         );
+        for rp_id in _DEFAULT_MIN_PIN_LENGTH_RP_IDS {
+            if !rp_ids.contains(&rp_id) {
+                rp_ids.push(rp_id);
+            }
+        }
         assert_eq!(persistent_store._min_pin_length_rp_ids(), rp_ids);
     }