ctap2: Implement PIN protocol version 2

Nitrokey · Feb 29, 2024 · 0367cbb · 0367cbb
1 parent 2bbe43e
commit 0367cbb
Show file tree

Hide file tree

Showing 5 changed files with 217 additions and 79 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,7 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Implement the `largeBlobKey` extension and the `largeBlobs` command ([#38][])
 - Fix error type for third invalid PIN entry ([#60][])
 - Fix error type for cancelled user presence ([#61][])
-- Extract PIN protocol implementation into separate module ([#62][])
+- PIN protocol changes:
+  - Extract PIN protocol implementation into separate module ([#62][])
+  - Implement PIN protocol 2 ([#63][])
 
 [#26]: https://github.com/solokeys/fido-authenticator/issues/26
 [#28]: https://github.com/solokeys/fido-authenticator/issues/28
@@ -31,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#60]: https://github.com/Nitrokey/fido-authenticator/pull/60
 [#61]: https://github.com/Nitrokey/fido-authenticator/pull/61
 [#62]: https://github.com/Nitrokey/fido-authenticator/pull/62
+[#63]: https://github.com/Nitrokey/fido-authenticator/pull/63
 
 ## [0.1.1] - 2022-08-22
 - Fix bug that treated U2F payloads as APDU over APDU in NFC transport @conorpp

diff --git a/Cargo.toml b/Cargo.toml
@@ -23,6 +23,7 @@ serde_cbor = { version = "0.11.0", default-features = false }
 serde-indexed = "0.1.0"
 sha2 = { version = "0.10", default-features = false }
 trussed = "0.1"
+trussed-hkdf = { version = "0.1.0" }
 trussed-staging = { version = "0.1.0", default-features = false, optional = true }
 
 apdu-dispatch = { version = "0.1", optional = true }
@@ -56,11 +57,13 @@ usbd-ctaphid = "0.1.0"
 features = ["dispatch"]
 
 [patch.crates-io]
-ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "7d4ad69e64ad308944c012aef5b9cfd7654d9be8" }
+# ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "f0db7acb87cced69edde1e33f41e1bdf1eee54c0" }
+ctap-types = { git = "https://github.com/Nitrokey/ctap-types.git", branch = "pin-protocol-2" }
 ctaphid-dispatch = { git = "https://github.com/trussed-dev/ctaphid-dispatch.git", rev = "57cb3317878a8593847595319aa03ef17c29ec5b" }
 apdu-dispatch = { git = "https://github.com/trussed-dev/apdu-dispatch.git", rev = "915fc237103fcecc29d0f0b73391f19abf6576de" }
+serde-indexed = { git = "https://github.com/sosthene-nitrokey/serde-indexed.git", rev = "5005d23cb4ee8622e62188ea0f9466146f851f0d" }
 trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "b1781805a2e33615d2d00b8bec80c0b1f5870ca1" }
+trussed-hkdf = { git = "https://github.com/Nitrokey/trussed-hkdf-backend.git", tag = "v0.1.0" }
 trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging", rev = "3b9594d93f89a5e760fe78fa5a96f125dfdcd470" }
-serde-indexed = { git = "https://github.com/sosthene-nitrokey/serde-indexed.git", rev = "5005d23cb4ee8622e62188ea0f9466146f851f0d" }
 trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.1" }
 usbd-ctaphid = { git = "https://github.com/Nitrokey/usbd-ctaphid.git", tag = "v0.1.0-nitrokey.2" }
diff --git a/src/ctap2.rs b/src/ctap2.rs
@@ -68,8 +68,10 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
                 .unwrap();
         }
 
-        let mut pin_protocols = Vec::<u8, 1>::new();
-        pin_protocols.push(1).unwrap();
+        let mut pin_protocols = Vec::<u8, 2>::new();
+        for pin_protocol in self.pin_protocols() {
+            pin_protocols.push(u8::from(*pin_protocol)).unwrap();
+        }
 
         let options = ctap2::get_info::CtapOptions {
             ep: None,
@@ -692,16 +694,15 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
                 }
 
                 // 3. generate shared secret
-                let shared_secret = self
-                    .pin_protocol(pin_protocol)
-                    .shared_secret(platform_kek)?;
+                let mut pin_protocol = self.pin_protocol(pin_protocol);
+                let shared_secret = pin_protocol.shared_secret(platform_kek)?;
 
                 // TODO: there are moar early returns!!
                 // - implement Drop?
                 // - do garbage collection outside of this?
 
                 // 4. verify pinAuth
-                shared_secret.verify_pin_auth(&mut self.trussed, new_pin_enc, pin_auth)?;
+                pin_protocol.verify_pin_auth(&shared_secret, new_pin_enc, pin_auth)?;
 
                 // 5. decrypt and verify new PIN
                 let new_pin = self.decrypt_pin_check_length(&shared_secret, new_pin_enc)?;
@@ -754,17 +755,16 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
                 self.state.pin_blocked()?;
 
                 // 3. generate shared secret
-                let shared_secret = self
-                    .pin_protocol(pin_protocol)
-                    .shared_secret(platform_kek)?;
+                let mut pin_protocol_impl = self.pin_protocol(pin_protocol);
+                let shared_secret = pin_protocol_impl.shared_secret(platform_kek)?;
 
                 // 4. verify pinAuth
                 let mut data = MediumData::new();
                 data.extend_from_slice(new_pin_enc)
                     .map_err(|_| Error::InvalidParameter)?;
                 data.extend_from_slice(pin_hash_enc)
                     .map_err(|_| Error::InvalidParameter)?;
-                shared_secret.verify_pin_auth(&mut self.trussed, &data, pin_auth)?;
+                pin_protocol_impl.verify_pin_auth(&shared_secret, &data, pin_auth)?;
 
                 // 5. decrement retries
                 self.state.decrement_retries(&mut self.trussed)?;
@@ -1050,15 +1050,24 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
 
 // impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenticator<UP, T>
 impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
-    fn parse_pin_protocol(&self, version: impl Into<u32>) -> Result<PinProtocolVersion> {
-        match version.into() {
-            1 => Ok(PinProtocolVersion::V1),
-            _ => Err(Error::InvalidParameter),
+    fn parse_pin_protocol(&self, version: impl TryInto<u8>) -> Result<PinProtocolVersion> {
+        if let Ok(version) = version.try_into() {
+            for pin_protocol in self.pin_protocols() {
+                if u8::from(*pin_protocol) == version {
+                    return Ok(*pin_protocol);
+                }
+            }
         }
+        Err(Error::InvalidParameter)
     }
 
+    // This is the single source of truth for the supported PIN protocols.
     fn pin_protocols(&self) -> &'static [PinProtocolVersion] {
-        &[PinProtocolVersion::V1]
+        if self.config.pin_protocol_v2 {
+            &[PinProtocolVersion::V1, PinProtocolVersion::V2]
+        } else {
+            &[PinProtocolVersion::V1]
+        }
     }
 
     fn pin_protocol(&mut self, pin_protocol: PinProtocolVersion) -> PinProtocol<'_, T> {
@@ -1181,7 +1190,7 @@ impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
         &mut self,
         pin_protocol: PinProtocolVersion,
         shared_secret: &SharedSecret,
-        pin_hash_enc: &Bytes<64>,
+        pin_hash_enc: &Bytes<80>,
     ) -> Result<()> {
         let pin_hash = shared_secret
             .decrypt(&mut self.trussed, pin_hash_enc)
@@ -1372,17 +1381,14 @@ impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
         if self.state.persistent.pin_is_set() {
             // let mut uv_performed = false;
             if let Some(pin_auth) = pin_auth {
-                if pin_auth.len() != 16 {
-                    return Err(Error::InvalidParameter);
-                }
                 // seems a bit redundant to check here in light of 2.
                 // I guess the CTAP spec writers aren't implementers :D
                 if let Some(pin_protocol) = pin_protocol {
                     // 5. if pinAuth is present and pinProtocol = 1, verify
                     // success --> set uv = 1
                     // error --> PinAuthInvalid
                     self.pin_protocol(pin_protocol)
-                        .verify_pin_token(pin_auth, data)?;
+                        .verify_pin_token(data, pin_auth)?;
 
                     return Ok(true);
                 } else {
@@ -1449,16 +1455,17 @@ impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
             .key;
 
             // Verify the auth tag, which uses the same process as the pinAuth
-            let shared_secret = self
-                .pin_protocol(pin_protocol)
-                .shared_secret(&hmac_secret.key_agreement)?;
-            shared_secret.verify_pin_auth(
-                &mut self.trussed,
+            let mut pin_protocol = self.pin_protocol(pin_protocol);
+            let shared_secret = pin_protocol.shared_secret(&hmac_secret.key_agreement)?;
+            pin_protocol.verify_pin_auth(
+                &shared_secret,
                 &hmac_secret.salt_enc,
                 &hmac_secret.salt_auth,
             )?;
 
-            if hmac_secret.salt_enc.len() != 32 && hmac_secret.salt_enc.len() != 64 {
+            if hmac_secret.salt_enc.len() != 32
+                && (hmac_secret.salt_enc.len() != 64 || hmac_secret.salt_enc.len() == 80)
+            {
                 debug_now!("invalid hmac-secret length");
                 return Err(Error::InvalidLength);
             }