From c30268a099c53c3f3219070525ba777bbe1c7bf9 Mon Sep 17 00:00:00 2001
From: Fabian Kaczmarczyck <kaczmarczyck@google.com>
Date: Tue, 12 Jan 2021 17:57:58 +0100
Subject: [PATCH] code cleanups and clarifications

---
 src/ctap/config_command.rs  |  9 +++++----
 src/ctap/mod.rs             | 11 +----------
 src/ctap/pin_protocol_v1.rs |  3 ---
 src/ctap/storage.rs         |  5 +++++
 4 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/src/ctap/config_command.rs b/src/ctap/config_command.rs
index 57e2a97..e09bab3 100644
--- a/src/ctap/config_command.rs
+++ b/src/ctap/config_command.rs
@@ -24,7 +24,6 @@ use alloc::vec;
 /// Processes the subcommand setMinPINLength for AuthenticatorConfig.
 fn process_set_min_pin_length(
     persistent_store: &mut PersistentStore,
-    pin_protocol_v1: &mut PinProtocolV1,
     params: SetMinPinLengthParams,
 ) -> Result<ResponseData, Ctap2StatusCode> {
     let SetMinPinLengthParams {
@@ -44,8 +43,10 @@ fn process_set_min_pin_length(
     if let Some(old_length) = persistent_store.pin_code_point_length()? {
         force_change_pin |= new_min_pin_length > old_length;
     }
-    pin_protocol_v1.force_pin_change |= force_change_pin;
-    // TODO(kaczmarczyck) actually force a PIN change
+    if force_change_pin {
+        // TODO(kaczmarczyck) actually force a PIN change in PinProtocolV1
+        persistent_store.force_pin_change()?;
+    }
     persistent_store.set_min_pin_length(new_min_pin_length)?;
     if let Some(min_pin_length_rp_ids) = min_pin_length_rp_ids {
         persistent_store.set_min_pin_length_rp_ids(min_pin_length_rp_ids)?;
@@ -86,7 +87,7 @@ pub fn process_config(
     match sub_command {
         ConfigSubCommand::SetMinPinLength => {
             if let Some(ConfigSubCommandParams::SetMinPinLength(params)) = sub_command_params {
-                process_set_min_pin_length(persistent_store, pin_protocol_v1, params)
+                process_set_min_pin_length(persistent_store, params)
             } else {
                 Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)
             }
diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs
index 98ed569..1d7e286 100644
--- a/src/ctap/mod.rs
+++ b/src/ctap/mod.rs
@@ -128,8 +128,7 @@ pub fn check_pin_uv_auth_protocol(
 ) -> Result<(), Ctap2StatusCode> {
     match pin_uv_auth_protocol {
         Some(PIN_PROTOCOL_VERSION) => Ok(()),
-        Some(_) => Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID),
-        None => Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID),
+        _ => Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID),
     }
 }
 
@@ -1087,11 +1086,6 @@ mod test {
                     auth_data[0..expected_auth_data.len()],
                     expected_auth_data[..]
                 );
-                /*assert_eq!(
-                    &auth_data[expected_auth_data.len()
-                        ..expected_auth_data.len() + expected_attested_cred_data.len()],
-                    expected_attested_cred_data
-                );*/
                 assert_eq!(
                     &auth_data[auth_data.len() - expected_extension_cbor.len()..auth_data.len()],
                     expected_extension_cbor
@@ -1424,9 +1418,6 @@ mod test {
         make_credential_params.extensions = extensions;
         let make_credential_response =
             ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID);
-        let mut expected_attested_cred_data =
-            ctap_state.persistent_store.aaguid().unwrap().to_vec();
-        expected_attested_cred_data.extend(&[0x00, 0x20]);
         check_make_response(
             make_credential_response,
             0x41,
diff --git a/src/ctap/pin_protocol_v1.rs b/src/ctap/pin_protocol_v1.rs
index e2a84eb..eef0440 100644
--- a/src/ctap/pin_protocol_v1.rs
+++ b/src/ctap/pin_protocol_v1.rs
@@ -172,7 +172,6 @@ pub struct PinProtocolV1 {
     consecutive_pin_mismatches: u8,
     permissions: u8,
     permissions_rp_id: Option<String>,
-    pub force_pin_change: bool,
 }
 
 impl PinProtocolV1 {
@@ -185,7 +184,6 @@ impl PinProtocolV1 {
             consecutive_pin_mismatches: 0,
             permissions: 0,
             permissions_rp_id: None,
-            force_pin_change: false,
         }
     }
 
@@ -530,7 +528,6 @@ impl PinProtocolV1 {
             consecutive_pin_mismatches: 0,
             permissions: 0xFF,
             permissions_rp_id: None,
-            force_pin_change: false,
         }
     }
 }
diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs
index 0bc9c5f..cf9afbc 100644
--- a/src/ctap/storage.rs
+++ b/src/ctap/storage.rs
@@ -544,6 +544,11 @@ impl PersistentStore {
         self.init(rng)?;
         Ok(())
     }
+
+    pub fn force_pin_change(&mut self) -> Result<(), Ctap2StatusCode> {
+        // TODO(kaczmarczyck) implement storage logic
+        Ok(())
+    }
 }
 
 impl From<persistent_store::StoreError> for Ctap2StatusCode {