From f84df969d07ad3a1a82e44fb934c384efc8d74a1 Mon Sep 17 00:00:00 2001 From: Subrahmanyaman Date: Tue, 4 Oct 2022 06:02:36 +0000 Subject: [PATCH] Skip AdditionalCertChain in rkp protected data --- .../javacard/keymaster/KMAndroidSEApplet.java | 16 +++++++----- .../javacard/keymaster/KMJCardSimApplet.java | 16 +++++++----- .../javacard/test/KMFunctionalTest.java | 4 +-- .../javacard/test/KMRKPFunctionalTest.java | 2 +- .../keymaster/KMKeymintDataStore.java | 2 ++ .../RemotelyProvisionedComponentDevice.java | 26 ++++++++++--------- 6 files changed, 37 insertions(+), 29 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index 647cb93d..6c2a5a77 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -211,8 +211,9 @@ private boolean isCommandAllowed(short apduIns) { // 1. All the necessary provisioning commands are succcessfully executed // 2. SE provision is locked // 3. OEM Root Public is provisioned. - if (kmDataStore.isProvisionLocked() || !(isProvisioningComplete() && isSeFactoryProvisioningLocked())) { - result = false; + if (kmDataStore.isProvisionLocked() || !(isProvisioningComplete() + && isSeFactoryProvisioningLocked())) { + result = false; } break; @@ -246,8 +247,8 @@ private boolean isSeFactoryProvisioningLocked() { private boolean isSeFactoryProvisioningComplete() { short pStatus = kmDataStore.getProvisionStatus(); - short seCompleteStatus = PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_ADDITIONAL_CERT_CHAIN; - if (seCompleteStatus == (pStatus & seCompleteStatus)) { + if (PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR == + (pStatus & PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR)) { return true; } return false; @@ -523,9 +524,10 @@ private void processGetProvisionStatusCmd(APDU apdu) { private boolean isProvisioningComplete() { short pStatus = kmDataStore.getProvisionStatus(); - short pCompleteStatus = PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_ADDITIONAL_CERT_CHAIN | - PROVISION_STATUS_PRESHARED_SECRET | PROVISION_STATUS_ATTEST_IDS | PROVISION_STATUS_OEM_PUBLIC_KEY | - PROVISION_STATUS_SECURE_BOOT_MODE; + short pCompleteStatus = + PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_PRESHARED_SECRET + | PROVISION_STATUS_ATTEST_IDS | PROVISION_STATUS_OEM_PUBLIC_KEY | + PROVISION_STATUS_SECURE_BOOT_MODE; if (kmDataStore.isProvisionLocked() || (pCompleteStatus == (pStatus & pCompleteStatus))) { return true; } diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java index 58c5dc45..29536c25 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java @@ -203,8 +203,9 @@ private boolean isCommandAllowed(short apduIns) { // 1. All the necessary provisioning commands are succcessfully executed // 2. SE provision is locked // 3. OEM Root Public is provisioned. - if (kmDataStore.isProvisionLocked() || !(isProvisioningComplete() && isSeFactoryProvisioningLocked())) { - result = false; + if (kmDataStore.isProvisionLocked() || !(isProvisioningComplete() + && isSeFactoryProvisioningLocked())) { + result = false; } break; @@ -239,8 +240,8 @@ private boolean isSeFactoryProvisioningLocked() { private boolean isSeFactoryProvisioningComplete() { short pStatus = kmDataStore.getProvisionStatus(); - short seCompleteStatus = PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_ADDITIONAL_CERT_CHAIN; - if (seCompleteStatus == (pStatus & seCompleteStatus)) { + if (PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR == (pStatus + & PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR)) { return true; } return false; @@ -579,9 +580,10 @@ private void processSetBootParamsCmd(APDU apdu) { private boolean isProvisioningComplete() { short pStatus = kmDataStore.getProvisionStatus(); - short pCompleteStatus = PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_ADDITIONAL_CERT_CHAIN | - PROVISION_STATUS_PRESHARED_SECRET | PROVISION_STATUS_ATTEST_IDS | PROVISION_STATUS_OEM_PUBLIC_KEY | - PROVISION_STATUS_SECURE_BOOT_MODE; + short pCompleteStatus = + PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_PRESHARED_SECRET + | PROVISION_STATUS_ATTEST_IDS | PROVISION_STATUS_OEM_PUBLIC_KEY | + PROVISION_STATUS_SECURE_BOOT_MODE; if (kmDataStore.isProvisionLocked() || (pCompleteStatus == (pStatus & pCompleteStatus))) { return true; } diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index bcc02018..43f8e132 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -1565,7 +1565,7 @@ public void testVerifyOemLockWithOutOemRootKeyFailure() { } @Test - public void testVerifySeLockWithOutCertDataFailure() { + public void testVerifySeLockWithOutCertDataSuccess() { AID appletAID1 = AIDUtil.create("A000000062"); simulator.installApplet(appletAID1, KMJCardSimApplet.class); // Select applet @@ -1573,7 +1573,7 @@ public void testVerifySeLockWithOutCertDataFailure() { // provision attest key Assert.assertEquals(KMError.OK, KMTestUtils.decodeError(decoder, KMProvision.provisionDeviceUniqueKeyPair(simulator, cryptoProvider, encoder, decoder))); - Assert.assertEquals(KMError.UNKNOWN_ERROR, KMTestUtils.decodeError(decoder, + Assert.assertEquals(KMError.OK, KMTestUtils.decodeError(decoder, KMProvision.provisionSeLocked(simulator, decoder))); cleanUp(); } diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMRKPFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMRKPFunctionalTest.java index 133d4ca5..aa4d1714 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMRKPFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMRKPFunctionalTest.java @@ -96,7 +96,7 @@ public class KMRKPFunctionalTest { private static final byte KEYMINT_CMD_APDU_END = KEYMINT_CMD_APDU_START + 48; //0x50 private static final byte INS_END_KM_CMD = 0x7F; - public static byte[] CSR_CHALLENGE = {0x56, 0x78, 0x65, 0x23, (byte) 0xFE, 0x32}; + public static byte[] CSR_CHALLENGE = new byte[32]; private CardSimulator simulator; private KMEncoder encoder; diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java index af8981d9..776f93fa 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java @@ -756,6 +756,8 @@ public void deleteAttestationIds() { attIdMeId = null; attIdManufacturer = null; attIdModel = null; + // Trigger garbage collection. + JCSystem.requestObjectDeletion(); } public short getVerifiedBootHash(byte[] buffer, short start) { diff --git a/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java b/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java index 66d0127b..8c385e6e 100644 --- a/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java +++ b/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java @@ -88,6 +88,7 @@ public class RemotelyProvisionedComponentDevice { public static final byte DI_SCHEMA_VERSION = 2; public static final byte[] DI_SECURITY_LEVEL = {0x73, 0x74, 0x72, 0x6F, 0x6E, 0x67, 0x62, 0x6F, 0x78}; + private static final boolean IS_ACC_SUPPORTED_IN_RKP_SERVER = false; private static final short MAX_SEND_DATA = 512; private static final byte[] google = {0x47, 0x6F, 0x6F, 0x67, 0x6C, 0x65}; @@ -445,7 +446,7 @@ public void processFinishSendData(APDU apdu) throws Exception { ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); } // PubKeysToSignMac - short empty = repository.alloc((short)0) ; + short empty = repository.alloc((short)0); short len = ((KMOperation) operation[0]).sign(repository.getHeap(), (short) empty, (short) 0, scratchPad, (short) 0); @@ -564,9 +565,9 @@ public void process(short ins, APDU apdu) throws Exception { } private boolean isAdditionalCertificateChainPresent() { - if ((0x02 == RKP_VERSION) || (TRUE == data[getEntry(TEST_MODE)])) { + if (!IS_ACC_SUPPORTED_IN_RKP_SERVER || (TRUE == data[getEntry(TEST_MODE)])) { // Don't include AdditionalCertificateChain in ProtectedData if either - // 1. KeyMint Version is 2 or + // 1. RKP server does not support processing of X.509 Additional Certificate Chain. // 2. Requested CSR for test mode. return false; } @@ -575,9 +576,10 @@ private boolean isAdditionalCertificateChainPresent() { private short processFinalData(byte[] scratchPad) { // Call finish on AES GCM Cipher - short empty = repository.alloc((short)0) ; + short empty = repository.alloc((short) 0); short len = - ((KMOperation) operation[0]).finish(repository.getHeap(), (short) empty, (short) 0, scratchPad, (short) 0); + ((KMOperation) operation[0]).finish(repository.getHeap(), (short) empty, (short) 0, + scratchPad, (short) 0); return len; } @@ -888,7 +890,7 @@ private short createDeviceInfo(byte[] scratchpad) { updateItem(rkpTmpVariables, metaOffset, DEVICE_INFO_VERSION, KMInteger.uint_8(DI_SCHEMA_VERSION)); updateItem(rkpTmpVariables, metaOffset, SECURITY_LEVEL, KMTextString.instance(DI_SECURITY_LEVEL, (short) 0, (short) DI_SECURITY_LEVEL.length)); - updateItem(rkpTmpVariables, metaOffset, FUSED, KMInteger.uint_8((byte) storeDataInst.secureBootMode)); + updateItem(rkpTmpVariables, metaOffset, FUSED, KMInteger.uint_8(storeDataInst.secureBootMode)); // Create device info map. short map = KMMap.instance(rkpTmpVariables[1]); short mapIndex = 0; @@ -925,18 +927,18 @@ private void updateItem(short[] deviceIds, short metaOffset, byte[] item, short private short getAttestationId(short attestId, byte[] scratchpad) { short attIdTagLen = storeDataInst.getAttestationId(attestId, scratchpad, (short) 0); - if (attIdTagLen != 0) { - return KMTextString.instance(scratchpad, (short) 0, attIdTagLen); + if (attIdTagLen == 0) { + KMException.throwIt(KMError.INVALID_STATE); } - return KMType.INVALID_VALUE; + return KMTextString.instance(scratchpad, (short) 0, attIdTagLen); } private short getVerifiedBootHash(byte[] scratchPad) { short len = storeDataInst.getVerifiedBootHash(scratchPad, (short) 0); - if (len != 0) { - return KMByteBlob.instance(scratchPad, (short) 0, len); + if (len == 0) { + KMException.throwIt(KMError.INVALID_STATE); } - return KMType.INVALID_VALUE; + return KMByteBlob.instance(scratchPad, (short) 0, len); } private short getBootloaderState() {