From a398c404dcf1c44f4e783edf30ead68cb9df793b Mon Sep 17 00:00:00 2001
From: Fabian Kaczmarczyck <kaczmarczyck@google.com>
Date: Mon, 27 Jul 2020 22:18:51 +0200
Subject: [PATCH] improves documentation to address comments

---
 README.md                   | 2 +-
 src/ctap/mod.rs             | 7 +++++--
 src/ctap/pin_protocol_v1.rs | 9 ++++++---
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/README.md b/README.md
index c5e4bfc..20a10ca 100644
--- a/README.md
+++ b/README.md
@@ -107,7 +107,7 @@ a few things you can personalize:
     The current minimum is 4. Values from 4 to 63 are allowed. Requiring longer
     PINs can help establish trust between users and relying parties. It makes
     user verification harder to break, but less convenient.
-    NIST recommends 6 at least digit PINs in section 5.1.9.1:
+    NIST recommends at least 6-digit PINs in section 5.1.9.1:
     https://pages.nist.gov/800-63-3/sp800-63b.html
     You can add relying parties to the list of readers of the minimum PIN length.
 
diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs
index 6df8ad2..692a15a 100644
--- a/src/ctap/mod.rs
+++ b/src/ctap/mod.rs
@@ -755,8 +755,11 @@ where
         &mut self,
         client_pin_params: AuthenticatorClientPinParameters,
     ) -> Result<ResponseData, Ctap2StatusCode> {
-        self.pin_protocol_v1
-            .process(self.rng, &mut self.persistent_store, client_pin_params)
+        self.pin_protocol_v1.process_subcommand(
+            self.rng,
+            &mut self.persistent_store,
+            client_pin_params,
+        )
     }
 
     fn process_reset(&mut self, cid: ChannelID) -> Result<ResponseData, Ctap2StatusCode> {
diff --git a/src/ctap/pin_protocol_v1.rs b/src/ctap/pin_protocol_v1.rs
index db06265..876f108 100644
--- a/src/ctap/pin_protocol_v1.rs
+++ b/src/ctap/pin_protocol_v1.rs
@@ -72,6 +72,7 @@ fn encrypt_hmac_secret_output(
 
     // Initialization of 4 blocks in any case makes this function more readable.
     let mut blocks = [[0u8; 16]; 4];
+    // With the if clause restriction above, block_len can only be 2 or 4.
     let block_len = salt_enc.len() / 16;
     for i in 0..block_len {
         blocks[i].copy_from_slice(&salt_enc[16 * i..16 * (i + 1)]);
@@ -395,6 +396,8 @@ impl PinProtocolV1 {
                     if self.consecutive_pin_mismatches >= 3 {
                         return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_BLOCKED);
                     }
+                    // TODO(kaczmarczyck) Values are taken from the (not yet public) new revision
+                    // of CTAP 2.1. The code should link the specification when published.
                     let mut message = vec![0xFF; 32];
                     message.extend(&[0x06, 0x08]);
                     message.extend(&[min_pin_length as u8, 0x00, 0x00, 0x00]);
@@ -449,7 +452,7 @@ impl PinProtocolV1 {
         Ok(response)
     }
 
-    pub fn process(
+    pub fn process_subcommand(
         &mut self,
         rng: &mut impl Rng256,
         persistent_store: &mut PersistentStore,
@@ -975,7 +978,7 @@ mod test {
             permissions_rp_id: None,
         };
         assert!(pin_protocol_v1
-            .process(&mut rng, &mut persistent_store, client_pin_params)
+            .process_subcommand(&mut rng, &mut persistent_store, client_pin_params)
             .is_ok());
 
         let client_pin_params = AuthenticatorClientPinParameters {
@@ -999,7 +1002,7 @@ mod test {
         #[cfg(feature = "with_ctap2_1")]
         let error_code = Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER;
         assert_eq!(
-            pin_protocol_v1.process(&mut rng, &mut persistent_store, client_pin_params),
+            pin_protocol_v1.process_subcommand(&mut rng, &mut persistent_store, client_pin_params),
             Err(error_code)
         );
     }