From de69cd1ffc2785dc75ef0c13601f4f33115bebc6 Mon Sep 17 00:00:00 2001
From: Emanuele Cesena <emanuele.cesena@gmail.com>
Date: Tue, 26 May 2026 17:21:05 -0700
Subject: [PATCH] ctap2: accept up=true on MakeCredential; LargeBlobs Set
 accepts both PIN protocols

---
 CHANGELOG.md   |  2 ++
 src/ctap2.rs   | 18 +++++++-----------
 tests/basic.rs |  3 +--
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 20b3416..68548ef 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Indicate support for `FIDO_2_3` in `get_info`.
 - Load full credential from filesstem for getAssertion if an allow list is used with a discoverable credential.
 - Use UTF-8 code points instead of bytes when checking the minimum length for PINs.
+- Accept `up = true` in makeCredential.
+- Fix PIN verification in `large_blobs_set`.
 
 ## [v0.3.0](https://github.com/trussed-dev/fido-authenticator/releases/tag/v0.3.0) (2026-03-25)
 
diff --git a/src/ctap2.rs b/src/ctap2.rs
index f9e6cdf..f2c89b4 100644
--- a/src/ctap2.rs
+++ b/src/ctap2.rs
@@ -214,8 +214,9 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
 
         // 1-4.
         if let Some(options) = parameters.options.as_ref() {
-            // up option is not valid for make_credential
-            if options.up.is_some() {
+            // CTAP 2.1 §6.1.2: MakeCredential allows `up` only with value
+            // true (UP is implicit and required); `up=false` is invalid.
+            if options.up == Some(false) {
                 return Err(Error::InvalidOption);
             }
         }
@@ -2427,15 +2428,10 @@ impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
             let Some(pin_uv_auth_protocol) = request.pin_uv_auth_protocol else {
                 return Err(Error::PinRequired);
             };
-            if pin_uv_auth_protocol != 1 {
-                return Err(Error::PinAuthInvalid);
-            }
             let pin_protocol = self.parse_pin_protocol(pin_uv_auth_protocol)?;
-            // TODO: check pinUvAuthToken
-            let pin_auth: [u8; 16] = pin_uv_auth_param
-                .as_ref()
-                .try_into()
-                .map_err(|_| Error::PinAuthInvalid)?;
+            // verify_pin_token truncates per protocol (16 B for v1, 32 B
+            // for v2), so pass the full param.
+            let pin_auth: &[u8] = pin_uv_auth_param.as_ref();
 
             let mut auth_data: Bytes<70> = Bytes::new();
             // 32x 0xff
@@ -2451,7 +2447,7 @@ impl<UP: UserPresence, T: TrussedRequirements> crate::Authenticator<UP, T> {
             auth_data.extend_from_slice(&Sha256::digest(data)).unwrap();
 
             let mut pin_protocol = self.pin_protocol(pin_protocol);
-            let pin_token = pin_protocol.verify_pin_token(&pin_auth, &auth_data)?;
+            let pin_token = pin_protocol.verify_pin_token(&auth_data, pin_auth)?;
             pin_token.require_permissions(Permissions::LARGE_BLOB_WRITE)?;
         }
 
diff --git a/tests/basic.rs b/tests/basic.rs
index 10ce4be..31ba50d 100644
--- a/tests/basic.rs
+++ b/tests/basic.rs
@@ -571,8 +571,7 @@ struct TestMakeCredential {
 impl TestMakeCredential {
     fn expected_error(&self) -> Option<u8> {
         if let Some(options) = self.options {
-            // TODO: this is the current implementation, but the spec allows Some(true)
-            if options.up.is_some() {
+            if options.up == Some(false) {
                 return Some(0x2c);
             }
             if !matches!(self.pin_auth, PinAuth::PinToken(_)) && options.uv == Some(true) {