Support up to 32767 discoverable credentials

Author: BryanJacobs

python_tests/ctap/ctap_test.py

@@ -647,13 +647,14 @@ def setUp(self, install_params: Optional[bytes] = None) -> None:
         self.pin = secrets.token_hex(10)
         ClientPin(self.ctap2).set_pin(self.pin)
 
-    def get_credential_management(self, permissions: Optional[int] = None) -> CredentialManagement:
+    def get_credential_management(self, permissions: Optional[int] = None, client = None) -> CredentialManagement:
         if permissions is None:
             permissions = self.PERMISSION_CRED_MGMT
 
-        client = self.get_high_level_client(
-            user_interaction=FixedPinUserInteraction(self.pin),
-        )
+        if client is None:
+            client = self.get_high_level_client(
+                user_interaction=FixedPinUserInteraction(self.pin),
+            )
         # noinspection PyTypeChecker
         be: _Ctap2ClientBackend = client._backend
         token = be._get_token(ClientPin(self.ctap2), permissions=permissions,

python_tests/ctap/test_cred_mgmt.py

@@ -249,3 +249,59 @@ def test_rk_overwrite_multiple_creds(self):
         self.assertEqual(1, len(rps))
         creds = cm.enumerate_creds(rp_id_hash=self.rp_id_hash(self.rp_id))
         self.assertEqual(2, len(creds))
+
+    def test_many_rks_same_rp(self):
+        pin_client = self.get_high_level_client(user_interaction=FixedPinUserInteraction(self.pin))
+        self.basic_makecred_params['options'] = {
+            'rk': True
+        }
+
+        NUM_CREDS_TO_TEST = 200
+
+        results = []
+
+        for i in range(NUM_CREDS_TO_TEST):
+            user_id = secrets.token_bytes(30)
+            res = pin_client.make_credential(
+                self.get_high_level_make_cred_options(
+                    ResidentKeyRequirement.REQUIRED, user_id=user_id
+                )
+            )
+            results.append(res)
+
+            cm = self.get_credential_management()
+            rps = cm.enumerate_rps()
+            self.assertEqual(1, len(rps))
+            creds = cm.enumerate_creds(rp_id_hash=self.rp_id_hash(self.rp_id))
+            self.assertEqual(len(results), len(creds))
+
+        pin_client.get_assertion(self.get_high_level_assertion_opts_from_cred(cred=results[len(results)//2], rp_id=self.rp_id))
+
+    def test_many_rps(self):
+        self.basic_makecred_params['options'] = {
+            'rk': True
+        }
+
+        NUM_CREDS_TO_TEST = 200
+
+        results = []
+
+        for i in range(NUM_CREDS_TO_TEST):
+            user_id = secrets.token_bytes(30)
+            rp_id = secrets.token_hex(20)
+            pin_client = self.get_high_level_client(user_interaction=FixedPinUserInteraction(self.pin), origin='https://' + rp_id)
+            res = pin_client.make_credential(
+                self.get_high_level_make_cred_options(
+                    ResidentKeyRequirement.REQUIRED, user_id=user_id,
+                    rp_id=rp_id
+                )
+            )
+            results.append(res)
+
+            pin_client = self.get_high_level_client(
+                user_interaction=FixedPinUserInteraction(self.pin),
+                origin=''
+            )
+            cm = self.get_credential_management(client=pin_client)
+            rps = cm.enumerate_rps()
+            self.assertEqual(len(results), len(rps))

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

@@ -377,11 +377,11 @@ public final class FIDO2Applet extends Applet implements ExtendedLength {
     /**
      * How many resident key slots are filled
      */
-    private byte numResidentCredentials;
+    private short numResidentCredentials;
     /**
      * How many distinct RPs are present across all resident keys
      */
-    private byte numResidentRPs;
+    private short numResidentRPs;
     /**
      * Storage for the largeBlobs extension
      */
@@ -1050,7 +1050,7 @@ private void makeCredential(APDU apdu, short lc, byte[] buffer) {
                 boolean uniqueRP = false;
                 if (!foundMatchingRK) {
                     // We're filling an empty slot
-                    numResidentCredentials++;
+                    numResidentCredentials = (short)(numResidentCredentials + 1);
                     if (!foundRPMatchInRKs) {
                         uniqueRP = true;
                     }
@@ -1556,7 +1556,7 @@ private short checkIfPubKeyBlockSupported(APDU apdu, byte[] buffer, short readId
             sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
         }
         short valLen = (short)(typeB - 0x60);
-        readIdx += valLen + 1;
+        readIdx += (short)(valLen + 1);
         if (readIdx >= lc) {
             sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_INVALID_CBOR);
         }
@@ -1574,7 +1574,7 @@ private short checkIfPubKeyBlockSupported(APDU apdu, byte[] buffer, short readId
             // Not of type "public-key", although same length
             transientStorage.readyStoredVars();
         }
-        readIdx += typeValLen + 1;
+        readIdx += (short)(typeValLen + 1);
         if (readIdx >= lc) {
             sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_INVALID_CBOR);
         }
@@ -1941,7 +1941,7 @@ private void getAssertion(final APDU apdu, final short lc, final byte[] buffer,
         boolean acceptedMatch = false;
 
         short hmacSecretBytes = 0;
-        byte numMatchesThisRP = 0;
+        short numMatchesThisRP = 0;
         short rkMatch = -1;
         short allowListLength = 0;
 
@@ -2964,11 +2964,7 @@ private boolean checkCredential(APDU apdu, byte[] credentialBuffer, short creden
             }
         }
 
-        if (!matches) {
-            return false;
-        }
-
-        return true;
+        return matches;
     }
 
     /**
@@ -4988,10 +4984,11 @@ private void handleDeleteCred(APDU apdu, byte[] buffer, short readOffset, short
                         } else {
                             residentKeys[rpHavingSameRP].setUniqueRP(true);
                         }
-                        for (short j = i; j < (short)(numResidentCredentials - 1); j++) {
+                        numResidentCredentials = (short)(numResidentCredentials - 1);
+                        for (short j = i; j < numResidentCredentials; j++) {
                             residentKeys[j] = residentKeys[(short)(j + 1)];
                         }
-                        residentKeys[--numResidentCredentials] = null;
+                        residentKeys[numResidentCredentials] = null;
                         ok = true;
                     } finally {
                         if (ok) {
@@ -5004,10 +5001,11 @@ private void handleDeleteCred(APDU apdu, byte[] buffer, short readOffset, short
                     JCSystem.beginTransaction();
                     boolean ok = false;
                     try {
-                        for (short j = i; j < (short)(numResidentCredentials - 1); j++) {
+                        numResidentCredentials = (short)(numResidentCredentials - 1);
+                        for (short j = i; j < numResidentCredentials; j++) {
                             residentKeys[j] = residentKeys[(short)(j + 1)];
                         }
-                        residentKeys[--numResidentCredentials] = null;
+                        residentKeys[numResidentCredentials] = null;
                         ok = true;
                     } finally {
                         if (ok) {
@@ -5039,7 +5037,7 @@ private void handleDeleteCred(APDU apdu, byte[] buffer, short readOffset, short
      *                     If zero, we're starting a new iteration
      * @param lc Length of the incoming request, as sent by the platform
      */
-    private void handleEnumerateCreds(APDU apdu, byte[] buffer, short bufferIdx, short startCredIdx, short lc) {
+    private void handleEnumerateCreds(APDU apdu, byte[] buffer, short bufferIdx, final short startCredIdx, short lc) {
         transientStorage.clearIterationPointers();
 
         if (startCredIdx > (short) residentKeys.length) { // intentional > instead of >=
@@ -5099,7 +5097,7 @@ private void handleEnumerateCreds(APDU apdu, byte[] buffer, short bufferIdx, sho
                     rpIdHashBuf, credIdIdx, (byte) 3)) {
                 // Cred is for this RP ID, yay.
 
-                byte matchingCount = 1; // remember to count THIS cred as a match
+                short matchingCount = 1; // remember to count THIS cred as a match
                 if (startCredIdx == 0) {
                     // Unfortunately, we need to scan forward through all remaining credentials
                     // we're not storing a list of which creds share an RP, so this is the only way to get
@@ -5119,7 +5117,7 @@ private void handleEnumerateCreds(APDU apdu, byte[] buffer, short bufferIdx, sho
                         }
                     }
                 }
-                transientStorage.setCredIterationPointer((byte)(rkIndex + 1)); // resume iteration from beyond this one
+                transientStorage.setCredIterationPointer((short)(rkIndex + 1)); // resume iteration from beyond this one
 
                 byte[] outBuf = bufferMem;
 
@@ -5231,15 +5229,15 @@ private short packCredentialId(byte[] credBuffer, short credOffset, byte[] write
      * This is, unfortunately, O(N^2) in the number of unique RPs.
      */
     private void updateRKStatekeeping(APDU apdu) {
-        short rp2Handle = bufferManager.allocate(apdu, CREDENTIAL_ID_LEN, BufferManager.ANYWHERE);
-        byte[] rp2Buffer = bufferManager.getBufferForHandle(apdu, rp2Handle);
-        short rp2Offset = bufferManager.getOffsetForHandle(rp2Handle);
+        final short rp2Handle = bufferManager.allocate(apdu, CREDENTIAL_ID_LEN, BufferManager.ANYWHERE);
+        final byte[] rp2Buffer = bufferManager.getBufferForHandle(apdu, rp2Handle);
+        final short rp2Offset = bufferManager.getOffsetForHandle(rp2Handle);
 
-        short rp1Handle = bufferManager.allocate(apdu, RP_HASH_LEN, BufferManager.ANYWHERE);
-        byte[] rp1Buffer = bufferManager.getBufferForHandle(apdu, rp1Handle);
-        short rp1Offset = bufferManager.getOffsetForHandle(rp1Handle);
+        final short rp1Handle = bufferManager.allocate(apdu, RP_HASH_LEN, BufferManager.ANYWHERE);
+        final byte[] rp1Buffer = bufferManager.getBufferForHandle(apdu, rp1Handle);
+        final short rp1Offset = bufferManager.getOffsetForHandle(rp1Handle);
 
-        byte numUniqueRPsFound = 0;
+        short numUniqueRPsFound = 0;
 
         for (short rkIndex1 = 0; rkIndex1 < (short) residentKeys.length; rkIndex1++) {
             if (residentKeys[rkIndex1] == null) {
@@ -5328,7 +5326,7 @@ private void handleEnumerateRPs(APDU apdu, short startOffset) {
 
         outBuf[writeOffset++] = FIDOConstants.CTAP2_OK;
 
-        transientStorage.setRPIterationPointer((byte)(rkIndex + 1));
+        transientStorage.setRPIterationPointer((short)(rkIndex + 1));
 
         outBuf[writeOffset++] = isContinuation ? (byte) 0xA2 : (byte) 0xA3; // map with two or three keys
         outBuf[writeOffset++] = 0x03; // map key: rp
@@ -5407,12 +5405,15 @@ private void handleCredentialManagementGetCredsMetadata(APDU apdu) {
      *
      * @return New write offset into given buffer
      */
-    private short encodeIntTo(byte[] outBuf, short writeOffset, byte v) {
+    private short encodeIntTo(byte[] outBuf, short writeOffset, short v) {
         if (v < 24) {
-            outBuf[writeOffset++] = v;
-        } else {
+            outBuf[writeOffset++] = (byte) v;
+        } else if (v < 256) {
             outBuf[writeOffset++] = 0x18; // Integer stored in one byte
-            outBuf[writeOffset++] = v;
+            outBuf[writeOffset++] = (byte) v;
+        } else {
+            outBuf[writeOffset++] = 0x19; // Integer stored in two bytes
+            writeOffset = Util.setShort(outBuf, writeOffset, v);
         }
         return writeOffset;
     }
@@ -6421,7 +6422,7 @@ private short consumeKeyAgreement(APDU apdu, byte[] buffer, short readIdx, byte
                 CannedCBOR.PUBLIC_KEY_DH_ALG_PREAMBLE, (short) 0, (short) CannedCBOR.PUBLIC_KEY_DH_ALG_PREAMBLE.length) != 0) {
             sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_UNSUPPORTED_ALGORITHM);
         }
-        readIdx += CannedCBOR.PUBLIC_KEY_DH_ALG_PREAMBLE.length;
+        readIdx += (short) CannedCBOR.PUBLIC_KEY_DH_ALG_PREAMBLE.length;
 
         short xIdx = readIdx;
         readIdx += KEY_POINT_LENGTH;

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

@@ -35,31 +35,31 @@ public final class TransientStorage {
      * Cred or RP iteration index, used when iterating through creds or RPs with credManagement commands
      * Disambiguated by the most significant bit: 1 for RPs, 0 for creds
      */
-    private static final byte IDX_CRED_RP_ITERATION_POINTER = 5; // 1 byte
+    private static final byte IDX_CRED_RP_ITERATION_POINTER = 5; // 2 bytes
     /**
      * Index of next credential to consider when iterating through assertions with getNextAssertion commands
      */
-    private static final byte IDX_ASSERT_ITERATION_POINTER = 6; // 1 byte
+    private static final byte IDX_ASSERT_ITERATION_POINTER = 7; // 1 byte
     /**
      * When writing an overlong response using chained APDUs, stores the position we're up to in the outgoing buffer
      */
-    private static final byte IDX_CONTINUATION_OUTGOING_WRITE_OFFSET = 7; // 2 bytes
+    private static final byte IDX_CONTINUATION_OUTGOING_WRITE_OFFSET = 8; // 2 bytes
     /**
      * When writing an overlong response using chained APDUs, stores the remaining bytes in the outgoing buffer
      */
-    private static final byte IDX_CONTINUATION_OUTGOING_REMAINING = 9; // 2 bytes
+    private static final byte IDX_CONTINUATION_OUTGOING_REMAINING = 10; // 2 bytes
     /**
      * When reading an overlong incoming request using chained APDUs, stores the fill level of the incoming buffer
      */
-    private static final byte IDX_CHAINING_INCOMING_READ_OFFSET = 11; // 2 bytes
+    private static final byte IDX_CHAINING_INCOMING_READ_OFFSET = 12; // 2 bytes
     /**
      * Giant boolean bitfield that holds all the BOOL_IDX variables below
      */
-    private static final byte IDX_BOOLEAN_OMNIBUS = 13; // 1 byte
+    private static final byte IDX_BOOLEAN_OMNIBUS = 14; // 1 byte
     /**
      * How many bytes long the temp storage should be
      */
-    private static final byte NUM_RESET_BYTES = 14;
+    private static final byte NUM_RESET_BYTES = 15;
 
     // boolean bit indices held in BOOLEAN_OMNIBUS byte above
     /**
@@ -172,7 +172,7 @@ public short getLargeBlobWriteTotalLength() {
     }
 
     public void clearIterationPointers() {
-        tempBytes[IDX_CRED_RP_ITERATION_POINTER] = 0;
+        Util.setShort(tempBytes, IDX_CRED_RP_ITERATION_POINTER, (short) 0);
     }
 
     public void clearAssertIterationPointer() {
@@ -304,26 +304,28 @@ public void setResetCommandSentSincePowerOn() {
         setBoolByIdx(BOOL_IDX_RESET_RECEIVED_SINCE_POWERON, true);
     }
 
-    public byte getRPIterationPointer() {
-        if ((tempBytes[IDX_CRED_RP_ITERATION_POINTER] & 0x80) == 0) {
-            return 0x00;
+    public short getRPIterationPointer() {
+        final short ret = Util.getShort(tempBytes, IDX_CRED_RP_ITERATION_POINTER);
+        if (ret < 0) {
+            return (short)(ret * -1);
         }
-        return (byte)(tempBytes[IDX_CRED_RP_ITERATION_POINTER] & 0x7F);
+        return ret;
     }
 
-    public void setRPIterationPointer(byte val) {
-        tempBytes[IDX_CRED_RP_ITERATION_POINTER] = (byte)(val | 0x80);
+    public void setRPIterationPointer(short val) {
+        Util.setShort(tempBytes, IDX_CRED_RP_ITERATION_POINTER, (short)(val * -1));
     }
 
-    public byte getCredIterationPointer() {
-        if ((tempBytes[IDX_CRED_RP_ITERATION_POINTER] & 0x80) != 0) {
-            return 0x00;
+    public short getCredIterationPointer() {
+        final short ret = Util.getShort(tempBytes, IDX_CRED_RP_ITERATION_POINTER);
+        if (ret > 0) {
+            return ret;
         }
-        return tempBytes[IDX_CRED_RP_ITERATION_POINTER];
+        return 0;
     }
 
-    public void setCredIterationPointer(byte val) {
-        tempBytes[IDX_CRED_RP_ITERATION_POINTER] = val;
+    public void setCredIterationPointer(short val) {
+        Util.setShort(tempBytes, IDX_CRED_RP_ITERATION_POINTER, val);
     }
 
     public void setPinProtocolInUse(byte pinProtocol, byte pinPermissions) {