From 4b451a1c665904d9b2590e639398b8fd1d1b9769 Mon Sep 17 00:00:00 2001 From: subrahmanyaman Date: Wed, 9 Mar 2022 02:57:06 +0000 Subject: [PATCH 1/2] 1. Store and restore provisionLocked and provisionStatus 2. Corrected the primitive count in upgrade flow. 3. Corrected the provisionStatus in the Provision tool. 4. Moved the storage of provisionStatus and provisionLocked to DataStorage class --- .../javacard/keymaster/KMAndroidSEApplet.java | 55 ++++++++++++------- .../seprovider/KMAndroidSEProvider.java | 51 ++++++----------- .../javacard/seprovider/KMSEProvider.java | 6 -- .../javacard/keymaster/KMKeymasterApplet.java | 5 +- .../keymaster/KMKeymintDataStore.java | 37 ++++++++++--- .../RemotelyProvisionedComponentDevice.java | 2 +- ProvisioningTool/src/provision.cpp | 31 ++++------- 7 files changed, 95 insertions(+), 92 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index ffaea2fe..0b3429c7 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -68,11 +68,11 @@ public class KMAndroidSEApplet extends KMKeymasterApplet implements OnUpgradeLis private static final byte PROVISION_STATUS_ATTEST_IDS = 0x08; private static final byte PROVISION_STATUS_PRESHARED_SECRET = 0x10; private static final byte PROVISION_STATUS_PROVISIONING_LOCKED = 0x20; + private static final byte PROVISION_STATUS_DEVICE_UNIQUE_KEY = 0x40; + private static final byte PROVISION_STATUS_ADDITIONAL_CERT_CHAIN = (byte) 0x80; public static final short SHARED_SECRET_KEY_SIZE = 32; - private static byte keymasterState = ILLEGAL_STATE; - private static byte provisionStatus = NOT_PROVISIONED; // Package version. protected short packageVersion; @@ -108,7 +108,7 @@ public void process(APDU apdu) { super.powerReset(); } - if (((KMAndroidSEProvider) seProvider).isProvisionLocked()) { + if (kmDataStore.isProvisionLocked()) { switch (apduIns) { case INS_SET_BOOT_PARAMS_CMD: processSetBootParamsCmd(apdu); @@ -118,7 +118,11 @@ public void process(APDU apdu) { //set the flag to mark boot ended kmDataStore.setBootEndedStatus(true); sendError(apdu, KMError.OK); - break; + break; + + case INS_GET_PROVISION_STATUS_CMD: + processGetProvisionStatusCmd(apdu); + break; default: super.process(apdu); @@ -133,13 +137,13 @@ public void process(APDU apdu) { switch (apduIns) { case INS_PROVISION_ATTEST_IDS_CMD: processProvisionAttestIdsCmd(apdu); - provisionStatus |= PROVISION_STATUS_ATTEST_IDS; + kmDataStore.setProvisionStatus(PROVISION_STATUS_ATTEST_IDS); sendError(apdu, KMError.OK); break; case INS_PROVISION_PRESHARED_SECRET_CMD: processProvisionPreSharedSecretCmd(apdu); - provisionStatus |= PROVISION_STATUS_PRESHARED_SECRET; + kmDataStore.setProvisionStatus(PROVISION_STATUS_PRESHARED_SECRET); sendError(apdu, KMError.OK); break; @@ -152,7 +156,6 @@ public void process(APDU apdu) { break; case INS_SET_BOOT_PARAMS_CMD: - processSetBootParamsCmd(apdu); break; @@ -165,7 +168,7 @@ public void process(APDU apdu) { break; default: - super.process(apdu); + ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); break; } } catch (KMException exception) { @@ -199,6 +202,7 @@ private static void processProvisionDeviceUniqueKey(APDU apdu) { short len = KMKeymasterApplet.encodeToApduBuffer(bcc, scratchPad, (short) 0, MAX_COSE_BUF_SIZE); kmDataStore.persistBootCertificateChain(scratchPad, (short) 0, len); + kmDataStore.setProvisionStatus(PROVISION_STATUS_DEVICE_UNIQUE_KEY); sendError(apdu, KMError.OK); } @@ -245,6 +249,7 @@ private static void processProvisionAdditionalCertChain(APDU apdu) { KMException.throwIt(KMError.STATUS_FAILED); } kmDataStore.persistAdditionalCertChain(buffer, bufferStartOffset, bufferLength); + kmDataStore.setProvisionStatus(PROVISION_STATUS_ADDITIONAL_CERT_CHAIN); //reclaim memory repository.reclaimMemory(bufferLength); sendError(apdu, KMError.OK); @@ -328,9 +333,11 @@ private static short buildErrorStatus(short err) { } private void processGetProvisionStatusCmd(APDU apdu) { + byte[] scratchpad = apdu.getBuffer(); + kmDataStore.getProvisionStatus(scratchpad, (short) 0); short resp = KMArray.instance((short) 2); KMArray.cast(resp).add((short) 0, buildErrorStatus(KMError.OK)); - KMArray.cast(resp).add((short) 1, KMInteger.uint_16(provisionStatus)); + KMArray.cast(resp).add((short) 1, KMInteger.uint_8(scratchpad[0])); sendOutgoing(apdu, resp); } @@ -390,10 +397,27 @@ private void processSetBootParamsCmd(APDU apdu) { super.reboot(); sendError(apdu, KMError.OK); } + + private boolean isProvisioningComplete(byte[] scratchPad) { + kmDataStore.getProvisionStatus(scratchPad, (short) 0); + if ((0 != (scratchPad[0] & PROVISION_STATUS_DEVICE_UNIQUE_KEY)) + && (0 != (scratchPad[0] & PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)) + && (0 != (scratchPad[0] & PROVISION_STATUS_PRESHARED_SECRET))) { + return true; + } else { + return false; + } + } private void processLockProvisioningCmd(APDU apdu) { - ((KMAndroidSEProvider) seProvider).setProvisionLocked(true); - sendError(apdu, KMError.OK); + byte[] scratchPad = apdu.getBuffer(); + if (isProvisioningComplete(scratchPad)) { + kmDataStore.setProvisionLocked(); + kmDataStore.setProvisionStatus(PROVISION_STATUS_PROVISIONING_LOCKED); + sendError(apdu, KMError.OK); + } else { + ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); + } } @Override @@ -454,15 +478,6 @@ public Element onSave() { return element; } - private short computePrimitveDataSize() { - // provisionStatus + keymasterState - return (short) 2; - } - - private short computeObjectCount() { - return (short) 0; - } - private short validateApdu(APDU apdu) { // Read the apdu header and buffer. byte[] apduBuffer = apdu.getBuffer(); diff --git a/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMAndroidSEProvider.java b/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMAndroidSEProvider.java index 25a654f1..015a1c09 100644 --- a/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMAndroidSEProvider.java +++ b/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMAndroidSEProvider.java @@ -95,8 +95,6 @@ public class KMAndroidSEProvider implements KMSEProvider { // Entropy private RandomData rng; - private boolean isProvisionLocked; - private static KMAndroidSEProvider androidSEProvider = null; public static KMAndroidSEProvider getInstance() { @@ -1134,17 +1132,7 @@ public KMRkpMacKey createRkpMacKey(KMRkpMacKey rkpMacKey, byte[] keyData, ((KMHmacKey) rkpMacKey).setKey(keyData, offset, length); return rkpMacKey; } - - public void setProvisionLocked(boolean locked) { - JCSystem.beginTransaction(); - isProvisionLocked = locked; - JCSystem.commitTransaction(); - } - public boolean isProvisionLocked() { - return isProvisionLocked; - } - @Override public short messageDigest256(byte[] inBuff, short inOffset, short inLength, byte[] outBuff, short outOffset) { @@ -1173,21 +1161,7 @@ public boolean isPowerReset() { } return flag; } - - private byte mapPurpose(short purpose) { - switch (purpose) { - case KMType.ENCRYPT: - return Cipher.MODE_ENCRYPT; - case KMType.DECRYPT: - return Cipher.MODE_DECRYPT; - case KMType.SIGN: - return Signature.MODE_SIGN; - case KMType.VERIFY: - return Signature.MODE_VERIFY; - } - return -1; - } - + @Override public void onSave(Element element, byte interfaceType, Object object) { element.write(interfaceType); @@ -1237,7 +1211,7 @@ public Object onResore(Element element) { case KMDataStoreConstants.INTERFACE_TYPE_DEVICE_UNIQUE_KEY: return KMECDeviceUniqueKey.onRestore((KeyPair) element.readObject()); case KMDataStoreConstants.INTERFACE_TYPE_RKP_MAC_KEY: - return KMHmacKey.onRestore((HMACKey) element.readObject()); + return KMHmacKey.onRestore((HMACKey) element.readObject()); default: ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); } @@ -1246,23 +1220,30 @@ public Object onResore(Element element) { @Override public short getBackupPrimitiveByteCount(byte interfaceType) { + short primitiveCount = 1; // interface type switch (interfaceType) { case KMDataStoreConstants.INTERFACE_TYPE_COMPUTED_HMAC_KEY: - return KMHmacKey.getBackupPrimitiveByteCount(); + primitiveCount += KMHmacKey.getBackupPrimitiveByteCount(); + break; case KMDataStoreConstants.INTERFACE_TYPE_MASTER_KEY: - return KMAESKey.getBackupPrimitiveByteCount(); + primitiveCount += KMAESKey.getBackupPrimitiveByteCount(); + break; case KMDataStoreConstants.INTERFACE_TYPE_PRE_SHARED_KEY: - return KMHmacKey.getBackupPrimitiveByteCount(); + primitiveCount += KMHmacKey.getBackupPrimitiveByteCount(); + break; case KMDataStoreConstants.INTERFACE_TYPE_ATTESTATION_KEY: - return KMECPrivateKey.getBackupPrimitiveByteCount(); + primitiveCount += KMECPrivateKey.getBackupPrimitiveByteCount(); + break; case KMDataStoreConstants.INTERFACE_TYPE_DEVICE_UNIQUE_KEY: - return KMECDeviceUniqueKey.getBackupPrimitiveByteCount(); + primitiveCount += KMECDeviceUniqueKey.getBackupPrimitiveByteCount(); + break; case KMDataStoreConstants.INTERFACE_TYPE_RKP_MAC_KEY: - return KMHmacKey.getBackupPrimitiveByteCount(); + primitiveCount += KMHmacKey.getBackupPrimitiveByteCount(); + break; default: ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); } - return 0; + return primitiveCount; } @Override diff --git a/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMSEProvider.java b/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMSEProvider.java index 4e7a229d..4d4570b6 100644 --- a/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMSEProvider.java +++ b/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMSEProvider.java @@ -715,10 +715,4 @@ KMPreSharedKey createPreSharedKey(KMPreSharedKey presharedKey, byte[] key, short KMRkpMacKey createRkpMacKey(KMRkpMacKey createComputedHmacKey, byte[] keyData, short offset, short length); - /** - * Returns true if provision is locked. - */ - public boolean isProvisionLocked(); - - } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 2a6aee47..c087dfbe 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -222,9 +222,6 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe protected static short[] data; protected static byte[] wrappingKey; - public static void install(byte[] bArray, short bOffset, byte bLength) { - new KMAndroidSEApplet().register(bArray, (short) (bOffset + 1), bArray[bOffset]); - } /** * Registers this applet. @@ -4308,7 +4305,7 @@ public static short validateCertChain(boolean validateEekRoot, byte expCertAlg, public static short generateBcc(boolean testMode, byte[] scratchPad) { - if (!testMode && seProvider.isProvisionLocked()) { + if (!testMode && kmDataStore.isProvisionLocked()) { KMException.throwIt(KMError.STATUS_FAILED); } KMDeviceUniqueKey deviceUniqueKey = kmDataStore.getDeviceUniqueKey(testMode); diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java index 94b7d6ec..f098537b 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java @@ -32,7 +32,7 @@ public class KMKeymintDataStore implements KMUpgradable { // Data table configuration - public static final short DATA_INDEX_SIZE = 17; + public static final short DATA_INDEX_SIZE = 19; public static final short DATA_INDEX_ENTRY_SIZE = 4; public static final short DATA_INDEX_ENTRY_LENGTH = 0; public static final short DATA_INDEX_ENTRY_OFFSET = 2; @@ -53,6 +53,8 @@ public class KMKeymintDataStore implements KMUpgradable { public static final byte AUTH_TAG_1 = 8; public static final byte BOOT_ENDED_FLAG = 15; public static final byte EARLY_BOOT_ENDED_FLAG = 16; + private static final byte PROVISIONED_LOCKED = 17; + private static final byte PROVISIONED_STATUS = 18; // Data Item sizes public static final short HMAC_SEED_NONCE_SIZE = 32; @@ -746,7 +748,33 @@ public void setBootPatchLevel(byte[] buffer, short start, short length) { Util.arrayCopyNonAtomic(buffer, start, bootPatchLevel, (short) 0, (short) length); } + public void setProvisionLocked() { + writeBoolean(PROVISIONED_LOCKED, true); + } + + public boolean isProvisionLocked() { + try { + return readBoolean(PROVISIONED_LOCKED); + } catch (KMException e) { + if (KMException.reason() != KMError.INVALID_DATA) + KMException.throwIt(KMException.reason()); + } + return false; + } + public void setProvisionStatus(byte provisionStatus) { + short offset = repository.alloc((short) 1); + byte[] buf = repository.getHeap(); + getProvisionStatus(buf, offset); + buf[offset] |= provisionStatus; + writeDataEntry(PROVISIONED_STATUS, buf, offset, (short) 1); + } + + public void getProvisionStatus(byte[] scratchpad, short offset) { + scratchpad[offset] = 0; + readDataEntry(PROVISIONED_STATUS, scratchpad, offset); + } + @Override public void onSave(Element element) { // Prmitives @@ -755,7 +783,6 @@ public void onSave(Element element) { element.write(bootState); // Objects element.write(dataTable); - // element.write(certificateData); element.write(attIdBrand); element.write(attIdDevice); element.write(attIdProduct); @@ -786,7 +813,6 @@ public void onRestore(Element element) { bootState = element.readShort(); // Read Objects dataTable = (byte[]) element.readObject(); -// certificateData = (byte[]) element.readObject(); attIdBrand = (byte[]) element.readObject(); attIdDevice = (byte[]) element.readObject(); attIdProduct = (byte[]) element.readObject(); @@ -810,13 +836,10 @@ public void onRestore(Element element) { @Override public short getBackupPrimitiveByteCount() { - // Magic Number - 1 byte - // Package Version - 2 bytes // dataIndex - 2 bytes // deviceLocked - 1 byte // deviceState = 2 bytes - // interface types - 4 bytes - return (short) (12 + + return (short) (5 + seProvider.getBackupPrimitiveByteCount(KMDataStoreConstants.INTERFACE_TYPE_MASTER_KEY) + seProvider.getBackupPrimitiveByteCount( KMDataStoreConstants.INTERFACE_TYPE_COMPUTED_HMAC_KEY) + diff --git a/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java b/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java index 548812d8..982140fb 100644 --- a/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java +++ b/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java @@ -879,7 +879,7 @@ private short createDeviceInfo(byte[] scratchpad) { updateItem(deviceIds, out, DEVICE_INFO_VERSION, KMInteger.uint_8(DI_SCHEMA_VERSION)); updateItem(deviceIds, out, SECURITY_LEVEL, KMTextString.instance(DI_SECURITY_LEVEL, (short) 0, (short) DI_SECURITY_LEVEL.length)); - byte[] attestIdState = seProvider.isProvisionLocked() ? ATTEST_ID_LOCKED : ATTEST_ID_OPEN; + byte[] attestIdState = storeDataInst.isProvisionLocked() ? ATTEST_ID_LOCKED : ATTEST_ID_OPEN; updateItem(deviceIds, out, ATTEST_ID_STATE, KMTextString.instance(attestIdState, (short) 0, (short) attestIdState.length)); // Create device info map. diff --git a/ProvisioningTool/src/provision.cpp b/ProvisioningTool/src/provision.cpp index fb50dce6..ef0c8534 100644 --- a/ProvisioningTool/src/provision.cpp +++ b/ProvisioningTool/src/provision.cpp @@ -35,8 +35,9 @@ enum ProvisionStatus { PROVISION_STATUS_ATTESTATION_CERT_PARAMS = 0x04, PROVISION_STATUS_ATTEST_IDS = 0x08, PROVISION_STATUS_PRESHARED_SECRET = 0x10, - PROVISION_STATUS_BOOT_PARAM = 0x20, - PROVISION_STATUS_PROVISIONING_LOCKED = 0x40, + PROVISION_STATUS_PROVISIONING_LOCKED = 0x20, + PROVISION_STATUS_DEVICE_UNIQUE_KEY = 0x40, + PROVISION_STATUS_ADDITIONAL_CERT_CHAIN = 0x80, }; // TODO keymint provision status and lock @@ -177,7 +178,7 @@ int openConnection(std::shared_ptr& pSocket) { printf("\n Socket already opened.\n"); } return SUCCESS; -} +} // Parses the input json file. Sends the apdus to JCServer. int processInputFile() { @@ -235,28 +236,20 @@ int getProvisionStatus() { return FAILURE; } // TODO Handle Keymint Provision status once added. - if ( (0 != (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_KEY)) && - (0 != (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_CHAIN)) && - (0 != (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_PARAMS)) && - (0 != (status & ProvisionStatus::PROVISION_STATUS_PRESHARED_SECRET)) && - (0 != (status & ProvisionStatus::PROVISION_STATUS_BOOT_PARAM))) { + if ( (0 != (status & ProvisionStatus::PROVISION_STATUS_DEVICE_UNIQUE_KEY)) && + (0 != (status & ProvisionStatus::PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)) && + (0 != (status & ProvisionStatus::PROVISION_STATUS_PRESHARED_SECRET))) { printf("\n SE is provisioned \n"); } else { - if (0 == (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_KEY)) { - printf("\n Attestation key is not provisioned \n"); - } - if (0 == (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_CHAIN)) { - printf("\n Attestation certificate chain is not provisioned \n"); + if (0 == (status & ProvisionStatus::PROVISION_STATUS_DEVICE_UNIQUE_KEY)) { + printf("\n Device Unique key is not provisioned \n"); } - if (0 == (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_PARAMS)) { - printf("\n Attestation certificate params are not provisioned \n"); + if (0 == (status & ProvisionStatus::PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)) { + printf("\n Additional certificate chain is not provisioned \n"); } if (0 == (status & ProvisionStatus::PROVISION_STATUS_PRESHARED_SECRET)) { printf("\n Shared secret is not provisioned \n"); } - if (0 == (status & ProvisionStatus::PROVISION_STATUS_BOOT_PARAM)) { - printf("\n Boot params are not provisioned \n"); - } } } else { printf("\n Fail to parse the response \n"); @@ -285,7 +278,7 @@ int main(int argc, char* argv[]) { } /* getopt_long stores the option index here. */ - while ((c = getopt_long(argc, argv, ":hls:i:", longOpts, NULL)) != -1) { + while ((c = getopt_long(argc, argv, ":hlsi:", longOpts, NULL)) != -1) { switch(c) { case 'i': // input file From b13e73fd47010c1b925fcf577e3700fdb58e831f Mon Sep 17 00:00:00 2001 From: subrahmanyaman Date: Wed, 9 Mar 2022 07:59:24 +0000 Subject: [PATCH 2/2] Allow all commands only if provision is completed --- .../com/android/javacard/keymaster/KMAndroidSEApplet.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index 0b3429c7..5bd60969 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -168,7 +168,12 @@ public void process(APDU apdu) { break; default: - ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); + // Allow other commands only if provision is completed. + if (isProvisioningComplete(apdu.getBuffer())) { + super.process(apdu); + } else { + ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); + } break; } } catch (KMException exception) {