Standards compliance work

Author: BryanJacobs

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

@@ -103,4 +103,9 @@ public abstract class CannedCBOR {
                 0x62, // string - two bytes long
                     0x69, 0x64, // id
     };
+
+    static final byte[] PUBLIC_KEY_TYPE = {
+            0x70, 0x75, 0x62, 0x6C, 0x69, 0x63, 0x2D, 0x6B, 0x65, 0x79
+          //   p     u     b     l     i     c     -     k     e     y
+    };
 }

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

@@ -1152,6 +1152,14 @@ private short checkIfPubKeyBlockSupported(APDU apdu, short readIdx, short lc) {
             sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
         }
         short typeValLen = (short) (bufferMem[readIdx] & 0x0F);
+        if (typeValLen != CannedCBOR.PUBLIC_KEY_TYPE.length) {
+            // not the same length as type "public-key": can't be a match
+            transientStorage.resetFoundKeyMatch();
+        } else if (Util.arrayCompare(bufferMem, (short)(readIdx + 1),
+                CannedCBOR.PUBLIC_KEY_TYPE, (short) 0, typeValLen) != 0) {
+            // Not of type "public-key", although same length
+            transientStorage.resetFoundKeyMatch();
+        }
         readIdx += typeValLen + 1;
         if (readIdx >= lc) {
             sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_INVALID_CBOR);
@@ -2507,7 +2515,22 @@ public void process(APDU apdu) throws ISOException {
 
         byte[] apduBytes = apdu.getBuffer();
 
-        if (apduBytes[ISO7816.OFFSET_CLA] == 0x00 && apduBytes[ISO7816.OFFSET_INS] == (byte) 0xC0) {
+        byte cla = apduBytes[ISO7816.OFFSET_CLA];
+        byte ins = apduBytes[ISO7816.OFFSET_INS];
+
+        if (cla == (byte) 0x80 && ins == 0x12 && apduBytes[ISO7816.OFFSET_P1] == 0x01
+                && apduBytes[ISO7816.OFFSET_P2] == 0x00) {
+            // Explicit disable command (NFCCTAP_CONTROL end CTAP_MSG). Turn off, and stay off.
+            transientStorage.disableAuthenticator();
+        }
+
+        if (transientStorage.authenticatorDisabled()) {
+            return;
+        }
+
+        if ((cla == 0x00 || cla == (byte) 0x80)
+                && ins == (byte) 0xC0) {
+            // continue outgoing response from buffer
             if (transientStorage.getOutgoingContinuationRemaining() == 0) {
                 throwException(ISO7816.SW_CONDITIONS_NOT_SATISFIED);
             }
@@ -2539,6 +2562,7 @@ public void process(APDU apdu) throws ISOException {
         }
 
         if (apdu.isCommandChainingCLA()) {
+            // Incoming chained request
             short amtRead = apdu.setIncomingAndReceive();
 
             short lc = apdu.getIncomingLength();
@@ -2552,7 +2576,7 @@ public void process(APDU apdu) throws ISOException {
             return;
         }
 
-        if (apduBytes[ISO7816.OFFSET_CLA] != (byte)0x80) {
+        if (apduBytes[ISO7816.OFFSET_CLA] != (byte) 0x80) {
             throwException(ISO7816.SW_CLA_NOT_SUPPORTED);
         }
 
@@ -2568,7 +2592,7 @@ public void process(APDU apdu) throws ISOException {
         short lc = apdu.getIncomingLength();
 
         if (amtRead == 0) {
-            throwException(ISO7816.SW_UNKNOWN);
+            throwException(ISO7816.SW_DATA_INVALID);
         }
 
         short incomingOffset = 1;
@@ -2726,7 +2750,7 @@ private void credManagementSubcommand(APDU apdu, short lc, short readIdx) {
             short scratchAmt = (short)(1 + subCommandParamsLen);
             short scratchOff = scratchAlloc(scratchAmt);
             scratch[scratchOff] = bufferMem[subcommandIdx];
-            if (subCommandParamsLen > CREDENTIAL_ID_LEN + MAX_USER_ID_LENGTH + 30) {
+            if (subCommandParamsLen > CREDENTIAL_ID_LEN + MAX_USER_ID_LENGTH + 35) {
                 sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_REQUEST_TOO_LARGE);
             }
             if (subCommandParamsLen > 0) {
@@ -2827,29 +2851,33 @@ private void handleUserUpdate(APDU apdu, short readOffset, short lc) {
             // Don't need to decrypt creds, just byte-compare them
             if (Util.arrayCompare(residentKeyData, (short)(i * CREDENTIAL_ID_LEN),
                     bufferMem, credIdIdx, CREDENTIAL_ID_LEN) == 0) {
-                // Credential matches - check user ID
+                // Matching cred.
+                // We need to extract the credential to check that our PIN token WOULD have permission
+                short scratchExtractedCred = scratchAlloc(CREDENTIAL_ID_LEN);
+                symmetricUnwrapper.doFinal(residentKeyData, (short)(i * CREDENTIAL_ID_LEN), CREDENTIAL_ID_LEN,
+                        scratch, scratchExtractedCred
+                );
+                if (permissionsRpId[0] != 0x00) {
+                    if (Util.arrayCompare(scratch, (short)(scratchExtractedCred + 32),
+                            permissionsRpId, (short) 1, RP_HASH_LEN) != 0) {
+                        // permissions RP ID in use, but doesn't match RP of this credential
+                        sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
+                    }
+                }
+
+                // Now that we have permission, check the user ID
                 symmetricUnwrapper.doFinal(residentKeyUserIds, (short)(i * MAX_USER_ID_LENGTH), MAX_USER_ID_LENGTH,
                         scratch, uidOffset);
                 if (Util.arrayCompare(scratch, uidOffset,
-                        bufferMem,userIdIdx,userIdLen) == 0) {
+                        bufferMem, userIdIdx, userIdLen) == 0) {
                     // Matches both credential and user ID - it's a hit.
-                    // No actual updating work to do here because we don't store anything other than the ID...
-
-                    // ... but we need to extract the credential to check that our PIN token WOULD have permission
-                    short scratchExtractedCred = scratchAlloc(CREDENTIAL_ID_LEN);
-                    symmetricUnwrapper.doFinal(residentKeyData, (short)(i * CREDENTIAL_ID_LEN), CREDENTIAL_ID_LEN,
-                            scratch, scratchExtractedCred
-                    );
-                    if (permissionsRpId[0] != 0x00) {
-                        if (Util.arrayCompare(scratch, (short)(scratchExtractedCred + 32),
-                                permissionsRpId, (short) 1, RP_HASH_LEN) != 0) {
-                            // permissions RP ID in use, but doesn't match RP of this credential
-                            sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
-                        }
-                    }
+                    // No actual updating work to do here because we don't store anything other than the ID
 
                     foundHit = true;
                     break;
+                } else {
+                    // matches credential ID, but doesn't match user ID
+                    sendErrorByte(apdu, FIDOConstants.CTAP1_ERR_INVALID_PARAMETER);
                 }
             }
         }
@@ -3505,7 +3533,7 @@ private void handleClientPinChange(APDU apdu, short readIdx, short lc, byte pinP
                 break;
             }
         }
-        if (realPinLength < 4) {
+        if (realPinLength < 4 || realPinLength > 63) {
             sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_POLICY_VIOLATION);
         }
 
@@ -3789,7 +3817,7 @@ private void testAndReadyPIN(APDU apdu, byte[] buf, short off, byte pinProtocol,
 
         // BAD PIN
         forceInitKeyAgreementKey();
-        sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
+        sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_INVALID);
     }
 
     /**

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

@@ -17,50 +17,50 @@ public class TransientStorage {
     /**
      * Used for storing found lengths in searches
      */
-    private static final short IDX_TEMP_BUF_IDX_LEN = 1;
+    private static final byte IDX_TEMP_BUF_IDX_LEN = 1;
     /**
      * When writing an overlong response using chained APDUs, stores the position we're up to in the outgoing buffer
      */
-    private static final short IDX_CONTINUATION_OUTGOING_WRITE_OFFSET = 2;
+    private static final byte IDX_CONTINUATION_OUTGOING_WRITE_OFFSET = 2;
     /**
      * When writing an overlong response using chained APDUs, stores the remaining bytes in the outgoing buffer
      */
-    private static final short IDX_CONTINUATION_OUTGOING_REMAINING = 3;
+    private static final byte IDX_CONTINUATION_OUTGOING_REMAINING = 3;
     /**
      * When reading an overlong incoming request using chained APDUs, stores the fill level of the incoming buffer
      */
-    private static final short IDX_CHAINING_INCOMING_READ_OFFSET = 4;
+    private static final byte IDX_CHAINING_INCOMING_READ_OFFSET = 4;
     /**
      * When reading incoming request chains, which FIDO2 command the request represents (pulled from the first packet)
      */
-    private static final short IDX_STORED_COMMAND_BYTE = 5;
+    private static final byte IDX_STORED_COMMAND_BYTE = 5;
     /**
      * How full the scratch buffer is
      */
-    private static final short IDX_SCRATCH_ALLOC_SIZE = 6;
+    private static final byte IDX_SCRATCH_ALLOC_SIZE = 6;
     /**
      * Index of next credential to consider when iterating through RPs with credManagement commands
      */
-    private static final short IDX_RP_ITERATION_POINTER = 7;
+    private static final byte IDX_RP_ITERATION_POINTER = 7;
     /**
      * Index of next credential to consider when iterating through creds with credManagement commands
      */
-    private static final short IDX_CRED_ITERATION_POINTER = 8;
+    private static final byte IDX_CRED_ITERATION_POINTER = 8;
     /**
      * Index of next credential (in either allowList or resident keys) to consider when iterating through
      * assertions with getNextAssertion commands
      */
-    private static final short IDX_ASSERT_ITERATION_POINTER = 9;
+    private static final byte IDX_ASSERT_ITERATION_POINTER = 9;
     /**
      * Two vars for the price of one!
      * In the upper 8 bits, a permissions bitfield for the currently set PIN token
      * In the lower 8 bits, which PIN protocol is currently in use, as at the time the pinToken was sent to the platform
      */
-    private static final short IDX_PIN_PROTOCOL_IN_USE = 10;
+    private static final byte IDX_PIN_PROTOCOL_IN_USE = 10;
     /**
      * Total number of in-memory short variables
      */
-    private static final short NUM_TEMP_SHORTS = 11;
+    private static final byte NUM_TEMP_SHORTS = 11;
 
     /**
      * array of per-reset booleans used internally
@@ -69,35 +69,39 @@ public class TransientStorage {
     /**
      * set when authenticator key initialized
      */
-    private static final short BOOL_IDX_RESET_PLATFORM_KEY_SET = 0;
+    private static final byte BOOL_IDX_RESET_PLATFORM_KEY_SET = 0;
     /**
      * set if platform supports authenticator-compatible key
      */
-    private static final short BOOL_IDX_RESET_FOUND_KEY_MATCH = 1;
+    private static final byte BOOL_IDX_RESET_FOUND_KEY_MATCH = 1;
     /**
      * set if the "up" (User Presence) option is enabled
      */
-    private static final short BOOL_IDX_OPTION_UP = 2;
+    private static final byte BOOL_IDX_OPTION_UP = 2;
     /**
      * set if the "uv" (User Validation) option is enabled
      */
-    private static final short BOOL_IDX_OPTION_UV = 3;
+    private static final byte BOOL_IDX_OPTION_UV = 3;
     /**
      * set if the "rk" (Resident Key) option is enabled
      */
-    private static final short BOOL_IDX_OPTION_RK = 4;
+    private static final byte BOOL_IDX_OPTION_RK = 4;
     /**
      * set if chaining responses should come from the getNextAssertion buffer
      */
-    private static final short BOOL_IDX_RESPONSE_FROM_SCRATCH = 5;
+    private static final byte BOOL_IDX_RESPONSE_FROM_SCRATCH = 5;
     /**
      * For reset "protection" feature, checks if a reset request has been received since the last authenticator powerup
      */
-    private static final short BOOL_IDX_RESET_RECEIVED_SINCE_POWERON = 6;
+    private static final byte BOOL_IDX_RESET_RECEIVED_SINCE_POWERON = 6;
+    /**
+     * Set to true when the authenticator app is fully disabled until next reselect
+     */
+    private static final byte BOOL_IDX_AUTHENTICATOR_DISABLED = 7;
     /**
      * number of booleans total in array
      */
-    private static final short NUM_RESET_BOOLS = 7;
+    private static final byte NUM_RESET_BOOLS = 8;
 
     /**
      * Pin-retries-since-reset counter, which must be cleared on RESET, not on deselect
@@ -117,7 +121,14 @@ public void fullyReset() {
         for (short s = 0; s < tempBools.length; s++) {
             tempBools[s] = false;
         }
+    }
+
+    public boolean authenticatorDisabled() {
+        return tempBools[BOOL_IDX_AUTHENTICATOR_DISABLED];
+    }
 
+    public void disableAuthenticator() {
+        tempBools[BOOL_IDX_AUTHENTICATOR_DISABLED] = true;
     }
 
     public void clearIterationPointers() {