Allow forcing alwaysUv on easily

Author: BryanJacobs

README.md

@@ -13,7 +13,7 @@ system locally with [pam-u2f](https://github.com/Yubico/pam-u2f).
 
 100% of the FIDO2 CTAP2.1 spec is covered, with the exception of features
 that aren't physically on an ordinary smartcard, such as biometrics or
-other on-board user verification. The implementation is not 100% standards
+other on-board user verification. The implementation is not 100.0% standards
 compliant, but you can expect very good results generally.
 
 In order to run this outside a simulator, you will need

mds.json

@@ -34,7 +34,7 @@
   "attestationRootCertificates": [],
   "authenticatorGetInfo": {
       "versions": [ "FIDO_2_0", "FIDO_2_1", "FIDO_2_1_PRE" ],
-      "extensions": [ "credBlob", "credProtect", "hmac-secret", "largeBlobKey" ],
+      "extensions": [ "credBlob", "credProtect", "hmac-secret", "largeBlobKey", "minPinLength" ],
       "aaguid": "00000000000000000000000000000000",
       "options": {
         "ep": true,

python_tests/ctap/ctap_test.py

@@ -294,27 +294,42 @@ def get_high_level_client(self, extensions: Optional[list[Type[Ctap2Extension]]]
 
     def get_high_level_make_cred_options(self,
                                          resident_key: ResidentKeyRequirement = ResidentKeyRequirement.DISCOURAGED,
-                                         extensions=None, rp_id: Optional[str] = None,
+                                         extensions=None, rp_id: Optional[str] = None, rp_stuff: Optional[dict] = None,
+                                         user_stuff: Optional[dict] = None,
+                                         client_data: Optional[bytes] = None,
+                                         user_verification: Optional[UserVerificationRequirement] = None,
                                          user_id: Optional[bytes] = None) -> PublicKeyCredentialCreationOptions:
         if extensions is None:
             extensions = {}
 
+        if client_data is None:
+            client_data = self.client_data
+
         if rp_id is None:
             rp_id = self.rp_id
 
-        if user_id is None:
-            user_id = self.basic_makecred_params['user']['id']
+        if user_verification is None:
+            user_verification = UserVerificationRequirement.DISCOURAGED
+
+        if rp_stuff is None:
+            rp_stuff = {
+                "name": "An RP Name",
+                "id": rp_id
+            }
+
+        if user_stuff is None:
+            if user_id is None:
+                user_id = self.basic_makecred_params['user']['id']
+
+            user_stuff = {
+                "name": "Bob",
+                "id": user_id
+            }
 
         return PublicKeyCredentialCreationOptions(
-            rp=PublicKeyCredentialRpEntity(
-                name="An RP Name",
-                id=rp_id
-            ),
-            user=PublicKeyCredentialUserEntity(
-                name="Bob",
-                id=user_id
-            ),
-            challenge=self.client_data,
+            rp=PublicKeyCredentialRpEntity(**rp_stuff),
+            user=PublicKeyCredentialUserEntity(**user_stuff),
+            challenge=client_data,
             pub_key_cred_params=[
                 PublicKeyCredentialParameters(
                     type=PublicKeyCredentialType.PUBLIC_KEY,
@@ -324,7 +339,7 @@ def get_high_level_make_cred_options(self,
             extensions=extensions,
             authenticator_selection=AuthenticatorSelectionCriteria(
                 resident_key=resident_key,
-                user_verification=UserVerificationRequirement.DISCOURAGED
+                user_verification=user_verification
             )
         )
 
@@ -375,10 +390,10 @@ class BasicAttestationTestCase(CTAPTestCase):
     aaguid: bytes
     cert: bytes
 
-    def install_attestation_cert(self):
+    def install_attestation_cert(self, **kwargs):
         self.ctap2.send_cbor(
             self.VENDOR_COMMAND_SWITCH_ATT,
-            args(self.gen_attestation_cert())
+            args(self.gen_attestation_cert(**kwargs))
         )
 
     def _short_to_bytes(self, b: int) -> list[int]:

src/main/java/us/q3q/fido2/FIDO2Applet.java

@@ -17,11 +17,9 @@ public final class FIDO2Applet extends Applet implements ExtendedLength {
      */
     private static final byte FIRMWARE_VERSION = 0x01;
     /**
-     * If true, permit the creation of resident keys without a PIN being set or provided.
-     * This is "normal" for a FIDO2 authenticator, but means keys on the device could be
-     * accessed in the event of a software bug or hardware fault.
+     * If true, default the `alwaysUv` option to on, and prevent disabling it.
      */
-    private static final boolean ALLOW_RESIDENT_KEY_CREATION_WITHOUT_PIN = true;
+    private static final boolean FORCE_ALWAYS_UV = false;
     /**
      * If true, the authenticator will refuse to reset itself until the following three steps happen in order:
      * <p>
@@ -305,7 +303,7 @@ public final class FIDO2Applet extends Applet implements ExtendedLength {
     /**
      * Set to true when the use of a PIN is forced for all operations
      */
-    private boolean alwaysUv;
+    private boolean alwaysUv = FORCE_ALWAYS_UV;
     /**
      * Everything that needs to be hot in RAM instead of stored to the flash. All goes away on deselect or reset!
      */
@@ -787,10 +785,6 @@ private void makeCredential(APDU apdu, short lc, byte[] buffer) {
                 // OR: PIN not set, but we've been asked not to do this without one
                 sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_REQUIRED);
             }
-            if (!ALLOW_RESIDENT_KEY_CREATION_WITHOUT_PIN && transientStorage.hasRKOption()) {
-                // Don't allow storing resident keys without a PIN set
-                sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_OPERATION_DENIED);
-            }
         }
         loadWrappingKeyIfNoPIN();
 
@@ -1376,10 +1370,16 @@ private void verifyPinAuth(APDU apdu, byte[] buffer, short readIdx,
                 if (len >= (byte) 0x40 && len <= (byte) 0x57) {
                     sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
                 }
+                if (len == 0x58) {
+                    sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
+                }
                 sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
             }
         } else {
             if (len != 0x58) { // byte array, one-byte length
+                if (len >= 0x40 && len <= 0x57) {
+                    sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
+                }
                 sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
             }
             if (buffer[readIdx++] != desiredLength) {
@@ -3601,7 +3601,7 @@ private void handleLargeBlobs(APDU apdu, byte[] reqBuffer, short lc) {
                 }
             }
 
-            if (pinSet) {
+            if (pinSet || alwaysUv) {
                 if (pinUvAuthIdx == -1) {
                     sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_REQUIRED);
                 }
@@ -4277,6 +4277,9 @@ private void setMinPin(APDU apdu, byte[] buffer, short readIdx, short cmdParamsL
      * @param apdu Request/response context object
      */
     private void toggleAlwaysUv(APDU apdu) {
+        if (FORCE_ALWAYS_UV) {
+            sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_OPERATION_DENIED);
+        }
         alwaysUv = !alwaysUv;
 
         apdu.getBuffer()[0] = FIDOConstants.CTAP2_OK;
@@ -5083,7 +5086,7 @@ private void authenticatorReset(APDU apdu) {
             pinSet = false;
             minPinLength = 4;
             forcePinChange = false;
-            alwaysUv = false;
+            alwaysUv = FORCE_ALWAYS_UV;
             pinRetryCounter.reset(pinIdx);
             Util.arrayFillNonAtomic(largeBlobStore, (short) 0, LARGE_BLOB_STORE_MAX_SIZE, (byte) 0x00);
             Util.arrayCopyNonAtomic(CannedCBOR.INITIAL_LARGE_BLOB_ARRAY, (short) 0,