Make sure wrappingKey is actually transient

Author: BryanJacobs

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

@@ -760,6 +760,7 @@ private void makeCredential(APDU apdu, short lc, short readIdx) {
                 sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_OPERATION_DENIED);
             }
         }
+        loadWrappingKeyIfNoPIN();
 
         final short scratchRPIDHashOffset = scratchAlloc(RP_HASH_LEN);
         short digested = sha256.doFinal(bufferMem, rpIdIdx, rpIdLen, scratch, scratchRPIDHashOffset);
@@ -965,6 +966,18 @@ private void makeCredential(APDU apdu, short lc, short readIdx) {
         doSendResponse(apdu, outputLen);
     }
 
+    /**
+     * If, and only if, no PIN is set, directly initialize symmetric crypto
+     * from our flash-stored wrapping key (which should be unencrypted)
+     */
+    private void loadWrappingKeyIfNoPIN() {
+        if (!pinSet) {
+            wrappingKey.setKey(wrappingKeySpace, (short) 0);
+
+            initSymmetricCrypto();
+        }
+    }
+
     /**
      * Copies a section of a raw RP ID into a target buffer. If it is longer than the max it will be cut down.
      * If it is shorter, it will be zero-padded.
@@ -1433,39 +1446,19 @@ private void getAssertion(APDU apdu, short lc, short readIdx) {
         byte[] matchingPubKeyBuffer = bufferMem;
         short matchingPubKeyCredDataLen = 0;
         short rkMatch = -1;
+        short allowListIdx = -1;
 
         short paramsRead = 2;
         if (bufferMem[readIdx] == 0x03) { // allowList
             readIdx++;
-            paramsRead++;
-            if (readIdx >= lc) {
-                sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_INVALID_CBOR);
-            }
-
-            if (((byte)(bufferMem[readIdx] & 0xF0)) != (byte)0x80) {
-                sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
-            }
-            short numPubKeysToCheck = (short) (bufferMem[readIdx++] & 0x0F);
             if (readIdx >= lc) {
                 sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_INVALID_CBOR);
             }
 
-            for (short i = 0; i < numPubKeysToCheck; i++) {
-                tempShorts[IDX_TEMP_BUF_IDX_STORAGE] = 0;
-                short beforeReadIdx = readIdx;
-                readIdx = consumeMapAndGetID(apdu, readIdx, lc);
-                short pubKeyIdx = tempShorts[IDX_TEMP_BUF_IDX_STORAGE];
-                short pubKeyLen = tempShorts[IDX_TEMP_BUF_IDX_LEN];
-
-                if (startOfMatchingPubKeyCredData == (short) -1) {
-                    boolean matches = checkCredential(bufferMem, pubKeyIdx, pubKeyLen, scratch, scratchRPIDHashIdx,
-                            privateScratch, (short) 0);
-                    if (matches) {
-                        startOfMatchingPubKeyCredData = beforeReadIdx;
-                        matchingPubKeyCredDataLen = (short) (readIdx - startOfMatchingPubKeyCredData);
-                    }
-                }
-            }
+            // We need to defer this until after we do PIN processing
+            allowListIdx = readIdx;
+            paramsRead++;
+            readIdx = consumeAnyEntity(apdu, readIdx, lc);
         }
 
         short hmacSecretBytes = 0;
@@ -1540,6 +1533,36 @@ private void getAssertion(APDU apdu, short lc, short readIdx) {
             sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_OPERATION_DENIED);
         }
 
+        loadWrappingKeyIfNoPIN();
+
+        if (allowListIdx != -1) {
+            short blockReadIdx = allowListIdx;
+            if (((byte)(bufferMem[blockReadIdx] & 0xF0)) != (byte)0x80) {
+                sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
+            }
+            short numPubKeysToCheck = (short) (bufferMem[blockReadIdx++] & 0x0F);
+            if (blockReadIdx >= lc) {
+                sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_INVALID_CBOR);
+            }
+
+            for (short i = 0; i < numPubKeysToCheck; i++) {
+                tempShorts[IDX_TEMP_BUF_IDX_STORAGE] = 0;
+                short beforeReadIdx = blockReadIdx;
+                blockReadIdx = consumeMapAndGetID(apdu, blockReadIdx, lc);
+                short pubKeyIdx = tempShorts[IDX_TEMP_BUF_IDX_STORAGE];
+                short pubKeyLen = tempShorts[IDX_TEMP_BUF_IDX_LEN];
+
+                if (startOfMatchingPubKeyCredData == (short) -1) {
+                    boolean matches = checkCredential(bufferMem, pubKeyIdx, pubKeyLen, scratch, scratchRPIDHashIdx,
+                            privateScratch, (short) 0);
+                    if (matches) {
+                        startOfMatchingPubKeyCredData = beforeReadIdx;
+                        matchingPubKeyCredDataLen = (short) (blockReadIdx - startOfMatchingPubKeyCredData);
+                    }
+                }
+            }
+        }
+
         if (startOfMatchingPubKeyCredData == -1) {
             // Scan resident keys for match
             for (short i = 0; i < NUM_RESIDENT_KEY_SLOTS; i++) {
@@ -3364,7 +3387,9 @@ private void rawSetPIN(APDU apdu, byte[] pinBuf, short offset, short pinLength)
         JCSystem.beginTransaction();
         boolean ok = false;
         try {
-            wrappingKey.getKey(wrappingKeySpace, (short) 0);
+            if (pinSet) {
+                wrappingKey.getKey(wrappingKeySpace, (short) 0);
+            }
 
             pinSet = true;
             tempBools[IDX_RESET_PIN_PROVIDED] = false;
@@ -3611,7 +3636,7 @@ protected FIDO2Applet(byte[] array, short offset, byte length) {
         wrappingKeyValidation = new byte[64];
         hmacWrapperBytes = new byte[32];
         wrappingIV = new byte[16];
-        wrappingKey = getPersistentAESKey(); // Our most important treasure, from which all other crypto is born...
+        wrappingKey = getTransientAESKey(); // Our most important treasure, from which all other crypto is born...
         // Resident key data, of course, must all be in flash. Losing that on reset would be Bad
         residentKeyData = new byte[NUM_RESIDENT_KEY_SLOTS * CREDENTIAL_ID_LEN];
         residentKeyValidity = new boolean[NUM_RESIDENT_KEY_SLOTS];

src/test/java/MyTestHarness.java …/test/java/us/q3q/fido2/UnitTesting.javasrc/test/java/MyTestHarness.java renamed to src/test/java/us/q3q/fido2/UnitTesting.java src/test/java/MyTestHarness.java renamed to src/test/java/us/q3q/fido2/UnitTesting.java

@@ -1,3 +1,5 @@
+package us.q3q.fido2;
+
 import com.licel.jcardsim.smartcardio.CardSimulator;
 import com.licel.jcardsim.utils.AIDUtil;
 
@@ -17,7 +19,10 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
-public class MyTestHarness {
+/**
+ * Example (only example) unit tests with jcardsim
+ */
+public class UnitTesting {
 
     CardSimulator simulator;
     AID appletAID = AIDUtil.create("F000000001");

src/test/java/VSim.java src/test/java/us/q3q/fido2/VSim.javasrc/test/java/VSim.java renamed to src/test/java/us/q3q/fido2/VSim.java

@@ -1,8 +1,13 @@
+package us.q3q.fido2;
+
 import com.licel.jcardsim.remote.VSmartCard;
 import com.licel.jcardsim.utils.AIDUtil;
 import javacard.framework.AID;
 import us.q3q.fido2.FIDO2Applet;
 
+/**
+ * Launches jcardsim with VSmartCard connectivity
+ */
 public class VSim {
 
     static AID appletAID = AIDUtil.create("A0000006472F0001");