From b19cfdf41c0d5425cdd610bbdb1ebb557a276fba Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Sun, 5 Jun 2022 18:56:23 +0000 Subject: [PATCH 1/5] added system property validations --- .../javacard/keymaster/KMAndroidSEApplet.java | 9 +- .../javacard/keymaster/KMKeymasterApplet.java | 191 ++++++++------- .../keymaster/KMKeymintDataStore.java | 220 ++++++++++++------ HAL/JavacardSecureElement.cpp | 2 +- HAL/JavacardSecureElement.h | 2 +- HAL/JavacardSharedSecret.cpp | 14 +- 6 files changed, 269 insertions(+), 169 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index 115abe41..23a72e56 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -220,8 +220,8 @@ private boolean isCommandAllowed(short apduIns) { default: // Allow other commands only if provision is completed. - if (!isProvisioningComplete()) { - result = false; + if (!(isProvisioningComplete())) { + result = false; } } return result; @@ -518,7 +518,7 @@ private void processSetBootEndedCmd(APDU apdu) { ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); } //set the flag to mark boot ended - kmDataStore.setBootEndedStatus(true); + kmDataStore.updateSBInitStatus(KMKeymintDataStore.DEVICE_BOOT_ENDED_DONE); seProvider.clearDeviceBooted(false); sendError(apdu, KMError.OK); } @@ -528,7 +528,7 @@ private void processSetBootParamsCmd(APDU apdu) { && (!seProvider.isDeviceRebooted())) { ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); } - + kmDataStore.clearSBInitStatus(); short argsProto = KMArray.instance((short) 5); byte[] scratchPad = apdu.getBuffer(); // Array of 4 expected arguments @@ -580,6 +580,7 @@ private void processSetBootParamsCmd(APDU apdu) { kmDataStore.createComputedHmacKey(scratchPad, (short) 0, KMKeymintDataStore.COMPUTED_HMAC_KEY_SIZE); super.reboot(); + kmDataStore.updateSBInitStatus(KMKeymintDataStore.SET_BOOT_PARAMS_DONE); sendError(apdu, KMError.OK); } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 74d66f0f..3622f622 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -457,89 +457,93 @@ public void process(APDU apdu) { } byte[] apduBuffer = apdu.getBuffer(); byte apduIns = apduBuffer[ISO7816.OFFSET_INS]; - switch (apduIns) { - case INS_INIT_STRONGBOX_CMD: - processInitStrongBoxCmd(apdu); - sendError(apdu, KMError.OK); - return; - case INS_GENERATE_KEY_CMD: - processGenerateKey(apdu); - break; - case INS_IMPORT_KEY_CMD: - processImportKeyCmd(apdu); - break; - case INS_BEGIN_IMPORT_WRAPPED_KEY_CMD: - processBeginImportWrappedKeyCmd(apdu); - break; - case INS_FINISH_IMPORT_WRAPPED_KEY_CMD: - processFinishImportWrappedKeyCmd(apdu); - break; - case INS_EXPORT_KEY_CMD: - processExportKeyCmd(apdu); - break; - case INS_UPGRADE_KEY_CMD: - processUpgradeKeyCmd(apdu); - break; - case INS_DELETE_KEY_CMD: - processDeleteKeyCmd(apdu); - break; - case INS_DELETE_ALL_KEYS_CMD: - processDeleteAllKeysCmd(apdu); - break; - case INS_ADD_RNG_ENTROPY_CMD: - processAddRngEntropyCmd(apdu); - break; - case INS_COMPUTE_SHARED_HMAC_CMD: - processComputeSharedHmacCmd(apdu); - break; - case INS_DESTROY_ATT_IDS_CMD: - processDestroyAttIdsCmd(apdu); - break; - case INS_VERIFY_AUTHORIZATION_CMD: - processVerifyAuthorizationCmd(apdu); - break; - case INS_GET_HMAC_SHARING_PARAM_CMD: - processGetHmacSharingParamCmd(apdu); - break; - case INS_GET_KEY_CHARACTERISTICS_CMD: - processGetKeyCharacteristicsCmd(apdu); - break; - case INS_GET_HW_INFO_CMD: - processGetHwInfoCmd(apdu); - break; - case INS_BEGIN_OPERATION_CMD: - processBeginOperationCmd(apdu); - break; - case INS_UPDATE_OPERATION_CMD: - processUpdateOperationCmd(apdu); - break; - case INS_FINISH_OPERATION_CMD: - processFinishOperationCmd(apdu); - break; - case INS_ABORT_OPERATION_CMD: - processAbortOperationCmd(apdu); - break; - case INS_DEVICE_LOCKED_CMD: - processDeviceLockedCmd(apdu); - break; - case INS_EARLY_BOOT_ENDED_CMD: - processEarlyBootEndedCmd(apdu); - break; - case INS_UPDATE_AAD_OPERATION_CMD: - processUpdateAadOperationCmd(apdu); - break; - case INS_GENERATE_RKP_KEY_CMD: - case INS_BEGIN_SEND_DATA_CMD: - case INS_UPDATE_CHALLENGE_CMD: - case INS_UPDATE_EEK_CHAIN_CMD: - case INS_UPDATE_KEY_CMD: - case INS_FINISH_SEND_DATA_CMD: - case INS_GET_RESPONSE_CMD: - case INS_GET_RKP_HARDWARE_INFO: - rkp.process(apduIns, apdu); - break; - default: - ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); + if (isCommandAllowed(apduIns)) { + switch (apduIns) { + case INS_INIT_STRONGBOX_CMD: + processInitStrongBoxCmd(apdu); + sendError(apdu, KMError.OK); + return; + case INS_GENERATE_KEY_CMD: + processGenerateKey(apdu); + break; + case INS_IMPORT_KEY_CMD: + processImportKeyCmd(apdu); + break; + case INS_BEGIN_IMPORT_WRAPPED_KEY_CMD: + processBeginImportWrappedKeyCmd(apdu); + break; + case INS_FINISH_IMPORT_WRAPPED_KEY_CMD: + processFinishImportWrappedKeyCmd(apdu); + break; + case INS_EXPORT_KEY_CMD: + processExportKeyCmd(apdu); + break; + case INS_UPGRADE_KEY_CMD: + processUpgradeKeyCmd(apdu); + break; + case INS_DELETE_KEY_CMD: + processDeleteKeyCmd(apdu); + break; + case INS_DELETE_ALL_KEYS_CMD: + processDeleteAllKeysCmd(apdu); + break; + case INS_ADD_RNG_ENTROPY_CMD: + processAddRngEntropyCmd(apdu); + break; + case INS_COMPUTE_SHARED_HMAC_CMD: + processComputeSharedHmacCmd(apdu); + break; + case INS_DESTROY_ATT_IDS_CMD: + processDestroyAttIdsCmd(apdu); + break; + case INS_VERIFY_AUTHORIZATION_CMD: + processVerifyAuthorizationCmd(apdu); + break; + case INS_GET_HMAC_SHARING_PARAM_CMD: + processGetHmacSharingParamCmd(apdu); + break; + case INS_GET_KEY_CHARACTERISTICS_CMD: + processGetKeyCharacteristicsCmd(apdu); + break; + case INS_GET_HW_INFO_CMD: + processGetHwInfoCmd(apdu); + break; + case INS_BEGIN_OPERATION_CMD: + processBeginOperationCmd(apdu); + break; + case INS_UPDATE_OPERATION_CMD: + processUpdateOperationCmd(apdu); + break; + case INS_FINISH_OPERATION_CMD: + processFinishOperationCmd(apdu); + break; + case INS_ABORT_OPERATION_CMD: + processAbortOperationCmd(apdu); + break; + case INS_DEVICE_LOCKED_CMD: + processDeviceLockedCmd(apdu); + break; + case INS_EARLY_BOOT_ENDED_CMD: + processEarlyBootEndedCmd(apdu); + break; + case INS_UPDATE_AAD_OPERATION_CMD: + processUpdateAadOperationCmd(apdu); + break; + case INS_GENERATE_RKP_KEY_CMD: + case INS_BEGIN_SEND_DATA_CMD: + case INS_UPDATE_CHALLENGE_CMD: + case INS_UPDATE_EEK_CHAIN_CMD: + case INS_UPDATE_KEY_CMD: + case INS_FINISH_SEND_DATA_CMD: + case INS_GET_RESPONSE_CMD: + case INS_GET_RKP_HARDWARE_INFO: + rkp.process(apduIns, apdu); + break; + default: + ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); + } + } else { + ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); } } catch (KMException exception) { freeOperations(); @@ -562,6 +566,22 @@ public void process(APDU apdu) { } } + private boolean isCommandAllowed(short apduIns) { + boolean result = true; + switch(apduIns) { + case INS_INIT_STRONGBOX_CMD: + case INS_GET_HMAC_SHARING_PARAM_CMD: + case INS_COMPUTE_SHARED_HMAC_CMD: + break; + + default: + if(!(kmDataStore.isSBInitCompleted())) { + result = false; + } + } + return result; + } + private void generateUniqueOperationHandle(byte[] buf, short offset, short len) { do { seProvider.newRandomNumber(buf, offset, len); @@ -579,6 +599,7 @@ private void freeOperations() { private void processEarlyBootEndedCmd(APDU apdu) { kmDataStore.setEarlyBootEndedStatus(true); + sendError(apdu, KMError.OK); } private short deviceLockedCmd(APDU apdu){ @@ -1015,6 +1036,7 @@ private void processComputeSharedHmacCmd(APDU apdu) { (short) sharingCheck.length, scratchPad, keyLen); + kmDataStore.updateSBInitStatus(KMKeymintDataStore.NEGOTIATED_SHARED_SECRET_DONE); // verification signature blob - 32 bytes //tmpVariables[1] short signature = KMByteBlob.instance(scratchPad, keyLen, signLen); @@ -3422,12 +3444,11 @@ private void processInitStrongBoxCmd(APDU apdu) { setOsVersion(osVersion); setOsPatchLevel(osPatchLevel); setVendorPatchLevel(vendorPatchLevel); + kmDataStore.updateSBInitStatus(KMKeymintDataStore.INIT_STRONGBOX_DONE); } public void reboot() { kmDataStore.clearHmacNonce(); - //flag to maintain the boot state - kmDataStore.setBootEndedStatus(false); //flag to maintain early boot ended state kmDataStore.setEarlyBootEndedStatus(false); //Clear all the operation state. diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java index 3d6760ec..c89077d9 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java @@ -73,6 +73,12 @@ public class KMKeymintDataStore implements KMUpgradable { private static final short ADDITIONAL_CERT_CHAIN_MAX_SIZE = 512;//First 2 bytes for length. private static final short BCC_MAX_SIZE = 512; + public static final byte RESET_SB_INIT_STATUS = 0x00; + public static final byte SET_BOOT_PARAMS_DONE = 0x01; + public static final byte DEVICE_BOOT_ENDED_DONE = 0x02; + public static final byte INIT_STRONGBOX_DONE = 0x04; + public static final byte NEGOTIATED_SHARED_SECRET_DONE = 0x08; + // Data - originally was in repository private byte[] attIdBrand; private byte[] attIdDevice; @@ -206,32 +212,29 @@ public short getHmacNonce() { public short getOsVersion() { short blob = readData(BOOT_OS_VERSION); - if (blob != KMType.INVALID_VALUE) { - return KMInteger.uint_32( - KMByteBlob.cast(blob).getBuffer(), KMByteBlob.cast(blob).getStartOff()); - } else { - return KMInteger.uint_32(zero, (short) 0); + if (blob == KMType.INVALID_VALUE) { + KMException.throwIt(KMError.INVALID_DATA); } + return KMInteger.uint_32( + KMByteBlob.cast(blob).getBuffer(), KMByteBlob.cast(blob).getStartOff()); } public short getVendorPatchLevel() { short blob = readData(VENDOR_PATCH_LEVEL); - if (blob != KMType.INVALID_VALUE) { - return KMInteger.uint_32( - KMByteBlob.cast(blob).getBuffer(), KMByteBlob.cast(blob).getStartOff()); - } else { - return KMInteger.uint_32(zero, (short) 0); + if (blob == KMType.INVALID_VALUE) { + KMException.throwIt(KMError.INVALID_DATA); } + return KMInteger.uint_32( + KMByteBlob.cast(blob).getBuffer(), KMByteBlob.cast(blob).getStartOff()); } public short getOsPatch() { short blob = readData(BOOT_OS_PATCH_LEVEL); - if (blob != KMType.INVALID_VALUE) { - return KMInteger.uint_32( - KMByteBlob.cast(blob).getBuffer(), KMByteBlob.cast(blob).getStartOff()); - } else { - return KMInteger.uint_32(zero, (short) 0); + if (blob == KMType.INVALID_VALUE) { + KMException.throwIt(KMError.INVALID_DATA); } + return KMInteger.uint_32( + KMByteBlob.cast(blob).getBuffer(), KMByteBlob.cast(blob).getStartOff()); } private boolean readBoolean(short id) { @@ -255,17 +258,24 @@ public boolean getEarlyBootEndedStatus() { } public boolean getBootEndedStatus() { - return readBoolean(BOOT_ENDED_FLAG); + boolean result = false; + short offset = repository.allocReclaimableMemory((short) 1); + byte[] buf = repository.getHeap(); + getSBInitStatus(buf, offset); + if(DEVICE_BOOT_ENDED_DONE == (buf[offset] & DEVICE_BOOT_ENDED_DONE)) { + result = true; + } + repository.reclaimMemory((short)1); + return result; } public short getDeviceTimeStamp() { short blob = readData(DEVICE_LOCKED_TIME); - if (blob != KMType.INVALID_VALUE) { - return KMInteger.uint_64(KMByteBlob.cast(blob).getBuffer(), - KMByteBlob.cast(blob).getStartOff()); - } else { - return KMInteger.uint_64(zero, (short) 0); + if (blob == KMType.INVALID_VALUE) { + KMException.throwIt(KMError.INVALID_DATA); } + return KMInteger.uint_64(KMByteBlob.cast(blob).getBuffer(), + KMByteBlob.cast(blob).getStartOff()); } public void setOsVersion(byte[] buf, short start, short len) { @@ -311,8 +321,36 @@ public void setEarlyBootEndedStatus(boolean flag) { writeBoolean(EARLY_BOOT_ENDED_FLAG, flag); } - public void setBootEndedStatus(boolean flag) { - writeBoolean(BOOT_ENDED_FLAG, flag); + public void clearSBInitStatus() { + clearDataEntry(BOOT_ENDED_FLAG); + } + + public void updateSBInitStatus(byte initStatus) { + short offset = repository.allocReclaimableMemory((short) 1); + byte[] buf = repository.getHeap(); + getSBInitStatus(buf, offset); + buf[offset] |= initStatus; + writeDataEntry(BOOT_ENDED_FLAG, buf, offset, (short) 1); + repository.reclaimMemory((short)1); + } + + public boolean isSBInitCompleted() { + boolean result = false; + short offset = repository.allocReclaimableMemory((short) 1); + byte[] buf = repository.getHeap(); + getSBInitStatus(buf, offset); + if ((0 != (buf[offset] & SET_BOOT_PARAMS_DONE)) + && (0 != (buf[offset] & INIT_STRONGBOX_DONE)) + && (0 != (buf[offset] & NEGOTIATED_SHARED_SECRET_DONE))) { + result = true; + } + repository.reclaimMemory((short)1); + return result; + } + + public void getSBInitStatus(byte[] scratchpad, short offset) { + scratchpad[offset] = 0; + readDataEntry(BOOT_ENDED_FLAG, scratchpad, offset); } public void clearDeviceLockTimeStamp() { @@ -512,6 +550,9 @@ public void createPresharedKey(byte[] keyData, short offset, short length) { } public KMPreSharedKey getPresharedKey() { + if (preSharedKey == null) { + KMException.throwIt(KMError.INVALID_DATA); + } return preSharedKey; } @@ -527,6 +568,9 @@ public void createComputedHmacKey(byte[] keyData, short offset, short length) { } public KMComputedHmacKey getComputedHmacKey() { + if (computedHmacKey == null) { + KMException.throwIt(KMError.INVALID_DATA); + } return computedHmacKey; } @@ -571,106 +615,112 @@ public void createRkpMacKey(byte[] keydata, short offset, short length) { } public KMRkpMacKey getRkpMacKey() { - return rkpMacKey; + if (rkpMacKey == null) { + KMException.throwIt(KMError.INVALID_DATA); + } + return rkpMacKey; } public short getAttestationId(short tag, byte[] buffer, short start) { + byte[] attestId = null; switch (tag) { // Attestation Id Brand case KMType.ATTESTATION_ID_BRAND: - Util.arrayCopyNonAtomic(attIdBrand, (short) 0, buffer, start, (short) attIdBrand.length); - return (short) attIdBrand.length; + attestId = attIdBrand; + break; // Attestation Id Device case KMType.ATTESTATION_ID_DEVICE: - Util.arrayCopyNonAtomic(attIdDevice, (short) 0, buffer, start, (short) attIdDevice.length); - return (short) attIdDevice.length; + attestId = attIdDevice; + break; // Attestation Id Product case KMType.ATTESTATION_ID_PRODUCT: - Util.arrayCopyNonAtomic(attIdProduct, (short) 0, buffer, start, - (short) attIdProduct.length); - return (short) attIdProduct.length; + attestId = attIdProduct; + break; // Attestation Id Serial case KMType.ATTESTATION_ID_SERIAL: - Util.arrayCopyNonAtomic(attIdSerial, (short) 0, buffer, start, (short) attIdSerial.length); - return (short) attIdSerial.length; + attestId = attIdSerial; + break; // Attestation Id IMEI case KMType.ATTESTATION_ID_IMEI: - Util.arrayCopyNonAtomic(attIdImei, (short) 0, buffer, start, (short) attIdImei.length); - return (short) attIdImei.length; + attestId = attIdImei; + break; // Attestation Id MEID case KMType.ATTESTATION_ID_MEID: - Util.arrayCopyNonAtomic(attIdMeId, (short) 0, buffer, start, (short) attIdMeId.length); - return (short) attIdMeId.length; + attestId = attIdMeId; + break; // Attestation Id Manufacturer case KMType.ATTESTATION_ID_MANUFACTURER: - Util.arrayCopyNonAtomic(attIdManufacturer, (short) 0, buffer, start, - (short) attIdManufacturer.length); - return (short) attIdManufacturer.length; + attestId = attIdManufacturer; + break; // Attestation Id Model case KMType.ATTESTATION_ID_MODEL: - Util.arrayCopyNonAtomic(attIdModel, (short) 0, buffer, start, (short) attIdModel.length); - return (short) attIdModel.length; + attestId = attIdModel; + break; } - return (short) 0; + if(attestId == null) { + KMException.throwIt(KMError.CANNOT_ATTEST_IDS); + } + Util.arrayCopyNonAtomic(attestId, (short) 0, buffer, start, (short) attestId.length); + return (short) attestId.length; } public void setAttestationId(short tag, byte[] buffer, short start, short length) { switch (tag) { // Attestation Id Brand case KMType.ATTESTATION_ID_BRAND: - JCSystem.beginTransaction(); + JCSystem.beginTransaction(); attIdBrand = new byte[length]; Util.arrayCopyNonAtomic(buffer, (short) start, attIdBrand, (short) 0, length); - JCSystem.commitTransaction(); + JCSystem.commitTransaction(); break; // Attestation Id Device case KMType.ATTESTATION_ID_DEVICE: - JCSystem.beginTransaction(); - attIdDevice = new byte[length]; - Util.arrayCopyNonAtomic(buffer, (short) start, attIdDevice, (short) 0, length); + JCSystem.beginTransaction(); + attIdDevice = new byte[length]; + Util.arrayCopyNonAtomic(buffer, (short) start, attIdDevice, (short) 0, length); JCSystem.commitTransaction(); break; // Attestation Id Product case KMType.ATTESTATION_ID_PRODUCT: - JCSystem.beginTransaction(); - attIdProduct = new byte[length]; - Util.arrayCopyNonAtomic(buffer, (short) start, attIdProduct, (short) 0, length); - JCSystem.commitTransaction(); + JCSystem.beginTransaction(); + attIdProduct = new byte[length]; + Util.arrayCopyNonAtomic(buffer, (short) start, attIdProduct, (short) 0, length); + JCSystem.commitTransaction(); break; // Attestation Id Serial case KMType.ATTESTATION_ID_SERIAL: - JCSystem.beginTransaction(); - attIdSerial = new byte[length]; - Util.arrayCopyNonAtomic(buffer, (short) start, attIdSerial, (short) 0, length); - JCSystem.commitTransaction(); + JCSystem.beginTransaction(); + attIdSerial = new byte[length]; + Util.arrayCopyNonAtomic(buffer, (short) start, attIdSerial, (short) 0, length); + JCSystem.commitTransaction(); break; // Attestation Id IMEI case KMType.ATTESTATION_ID_IMEI: - JCSystem.beginTransaction(); - attIdImei = new byte[length]; - Util.arrayCopyNonAtomic(buffer, (short) start, attIdImei, (short) 0, length); - JCSystem.commitTransaction(); + JCSystem.beginTransaction(); + attIdImei = new byte[length]; + Util.arrayCopyNonAtomic(buffer, (short) start, attIdImei, (short) 0, length); + JCSystem.commitTransaction(); break; // Attestation Id MEID case KMType.ATTESTATION_ID_MEID: - JCSystem.beginTransaction(); - attIdMeId = new byte[length]; - Util.arrayCopyNonAtomic(buffer, (short) start, attIdMeId, (short) 0, length); - JCSystem.commitTransaction(); + JCSystem.beginTransaction(); + attIdMeId = new byte[length]; + Util.arrayCopyNonAtomic(buffer, (short) start, attIdMeId, (short) 0, length); + JCSystem.commitTransaction(); break; // Attestation Id Manufacturer case KMType.ATTESTATION_ID_MANUFACTURER: - JCSystem.beginTransaction(); - attIdManufacturer = new byte[length]; + JCSystem.beginTransaction(); + attIdManufacturer = new byte[length]; Util.arrayCopyNonAtomic(buffer, (short) start, attIdManufacturer, (short) 0, length); JCSystem.commitTransaction(); break; // Attestation Id Model case KMType.ATTESTATION_ID_MODEL: - JCSystem.beginTransaction(); - attIdModel = new byte[length]; - Util.arrayCopyNonAtomic(buffer, (short) start, attIdModel, (short) 0, length); - JCSystem.commitTransaction(); + JCSystem.beginTransaction(); + attIdModel = new byte[length]; + Util.arrayCopyNonAtomic(buffer, (short) start, attIdModel, (short) 0, length); + JCSystem.commitTransaction(); break; } } @@ -687,11 +737,17 @@ public void deleteAttestationIds() { } public short getVerifiedBootHash(byte[] buffer, short start) { + if (verifiedHash == null) { + KMException.throwIt(KMError.INVALID_DATA); + } Util.arrayCopyNonAtomic(verifiedHash, (short) 0, buffer, start, (short) verifiedHash.length); return (short) verifiedHash.length; } public short getBootKey(byte[] buffer, short start) { + if (verifiedHash == null) { + KMException.throwIt(KMError.INVALID_DATA); + } Util.arrayCopyNonAtomic(bootKey, (short) 0, buffer, start, (short) bootKey.length); return (short) bootKey.length; } @@ -705,6 +761,9 @@ public boolean isDeviceBootLocked() { } public short getBootPatchLevel(byte[] buffer, short start) { + if (bootPatchLevel == null) { + KMException.throwIt(KMError.INVALID_DATA); + } Util.arrayCopyNonAtomic(bootPatchLevel, (short) 0, buffer, start, (short) bootPatchLevel.length); return (short) bootPatchLevel.length; @@ -717,7 +776,7 @@ public void setVerifiedBootHash(byte[] buffer, short start, short length) { if (length != 32) { KMException.throwIt(KMError.UNKNOWN_ERROR); } - Util.arrayCopyNonAtomic(buffer, start, verifiedHash, (short) 0, (short) 32); + Util.arrayCopy(buffer, start, verifiedHash, (short) 0, (short) 32); } public void setBootKey(byte[] buffer, short start, short length) { @@ -727,7 +786,7 @@ public void setBootKey(byte[] buffer, short start, short length) { if (length != 32) { KMException.throwIt(KMError.UNKNOWN_ERROR); } - Util.arrayCopyNonAtomic(buffer, start, bootKey, (short) 0, (short) 32); + Util.arrayCopy(buffer, start, bootKey, (short) 0, (short) 32); } public void setBootState(short state) { @@ -745,7 +804,7 @@ public void setBootPatchLevel(byte[] buffer, short start, short length) { if (length > 4 || length < 0) { KMException.throwIt(KMError.UNKNOWN_ERROR); } - Util.arrayCopyNonAtomic(buffer, start, bootPatchLevel, (short) 0, (short) length); + Util.arrayCopy(buffer, start, bootPatchLevel, (short) 0, (short) length); } public void setProvisionLock(boolean lockValue) { @@ -802,7 +861,7 @@ public byte[] getOEMRootPublicKey() { } return oemRootPublicKey; } - + @Override public void onSave(Element element) { // Prmitives @@ -872,10 +931,11 @@ public void onRestore(Element element, short oldVersion, short currentVersion) { void handleDataUpgrade(short oldVersion, short currentVersion) { if(oldVersion == 0x0100 && currentVersion == 0x0200) { handleProvisionStatusUpgrade(); + handleSBInitStatusUpgrade(); } } - void handleProvisionStatusUpgrade( ){ + void handleProvisionStatusUpgrade(){ short dInex = repository.allocReclaimableMemory((short)2); byte data[] = repository.getHeap(); getProvisionStatus(data, dInex); @@ -893,6 +953,16 @@ void handleProvisionStatusUpgrade( ){ writeDataEntry(PROVISIONED_STATUS, data, dInex, (short) 2); repository.reclaimMemory((short)2); } + + void handleSBInitStatusUpgrade(){ + short dInex = repository.allocReclaimableMemory((short)1); + byte data[] = repository.getHeap(); + getSBInitStatus(data, dInex); + if(data[dInex] == 0x01) { + updateSBInitStatus(DEVICE_BOOT_ENDED_DONE); + } + repository.reclaimMemory((short)1); + } @Override public short getBackupPrimitiveByteCount() { diff --git a/HAL/JavacardSecureElement.cpp b/HAL/JavacardSecureElement.cpp index 252131da..0b5cef82 100644 --- a/HAL/JavacardSecureElement.cpp +++ b/HAL/JavacardSecureElement.cpp @@ -37,7 +37,7 @@ keymaster_error_t JavacardSecureElement::initializeJavacard() { request.add(Uint(getOsVersion())); request.add(Uint(getOsPatchlevel())); request.add(Uint(getVendorPatchlevel())); - auto [item, err] = sendRequest(Instruction::INS_SET_BOOT_PARAMS_CMD, request); + auto [item, err] = sendRequest(Instruction::INS_INIT_STRONGBOX_CMD, request); return err; } diff --git a/HAL/JavacardSecureElement.h b/HAL/JavacardSecureElement.h index 160d3d4d..d483a2d2 100644 --- a/HAL/JavacardSecureElement.h +++ b/HAL/JavacardSecureElement.h @@ -59,7 +59,7 @@ enum class Instruction { INS_UPDATE_AAD_OPERATION_CMD = KEYMINT_CMD_APDU_START + 23, INS_BEGIN_IMPORT_WRAPPED_KEY_CMD = KEYMINT_CMD_APDU_START + 24, INS_FINISH_IMPORT_WRAPPED_KEY_CMD = KEYMINT_CMD_APDU_START + 25, - INS_SET_BOOT_PARAMS_CMD = KEYMINT_CMD_APDU_START + 26, + INS_INIT_STRONGBOX_CMD = KEYMINT_CMD_APDU_START + 26, // RKP Commands INS_GET_RKP_HARDWARE_INFO = KEYMINT_CMD_APDU_START + 27, INS_GENERATE_RKP_KEY_CMD = KEYMINT_CMD_APDU_START + 28, diff --git a/HAL/JavacardSharedSecret.cpp b/HAL/JavacardSharedSecret.cpp index a779c1ad..d6fd0541 100644 --- a/HAL/JavacardSharedSecret.cpp +++ b/HAL/JavacardSharedSecret.cpp @@ -12,10 +12,14 @@ using std::shared_ptr; using std::vector; ScopedAStatus JavacardSharedSecret::getSharedSecretParameters(SharedSecretParameters* params) { - card_->initializeJavacard(); + auto error = card_->initializeJavacard(); + if(error != KM_ERROR_OK) { + LOG(ERROR) << "Error in initializing javacard."; + return km_utils::kmError2ScopedAStatus(error); + } auto [item, err] = card_->sendRequest(Instruction::INS_GET_SHARED_SECRET_PARAM_CMD); if (err != KM_ERROR_OK) { - LOG(ERROR) << "Error in sending in getSharedSecretParameters.!!!!!!!!!!!!!!!"; + LOG(ERROR) << "Error in sending in getSharedSecretParameters."; return km_utils::kmError2ScopedAStatus(err); } if (!cbor_.getSharedSecretParameters(item, 1, *params)) { @@ -29,7 +33,11 @@ ScopedAStatus JavacardSharedSecret::computeSharedSecret(const std::vector& params, std::vector* secret) { - card_->initializeJavacard(); + auto error = card_->initializeJavacard(); + if(error != KM_ERROR_OK) { + LOG(ERROR) << "Error in initializing javacard."; + return km_utils::kmError2ScopedAStatus(error); + } cppbor::Array request; cbor_.addSharedSecretParameters(request, params); auto [item, err] = card_->sendRequest(Instruction::INS_COMPUTE_SHARED_SECRET_CMD, request); From cc7cec19b76b59fdabd415797f7ee833a0327bc7 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Mon, 6 Jun 2022 07:30:27 +0000 Subject: [PATCH 2/5] updated system properties validation --- .../javacard/keymaster/KMAndroidSEApplet.java | 4 +- .../javacard/keymaster/KMKeymasterApplet.java | 202 +++++++++--------- .../keymaster/KMKeymintDataStore.java | 30 +-- 3 files changed, 120 insertions(+), 116 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index 23a72e56..300fabb7 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -518,7 +518,7 @@ private void processSetBootEndedCmd(APDU apdu) { ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); } //set the flag to mark boot ended - kmDataStore.updateSBInitStatus(KMKeymintDataStore.DEVICE_BOOT_ENDED_DONE); + kmDataStore.updateSBInitStatus(KMKeymintDataStore.SET_BOOT_ENDED_SUCCESS); seProvider.clearDeviceBooted(false); sendError(apdu, KMError.OK); } @@ -580,7 +580,7 @@ private void processSetBootParamsCmd(APDU apdu) { kmDataStore.createComputedHmacKey(scratchPad, (short) 0, KMKeymintDataStore.COMPUTED_HMAC_KEY_SIZE); super.reboot(); - kmDataStore.updateSBInitStatus(KMKeymintDataStore.SET_BOOT_PARAMS_DONE); + kmDataStore.updateSBInitStatus(KMKeymintDataStore.SET_BOOT_PARAMS_SUCCESS); sendError(apdu, KMError.OK); } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 3622f622..07de2c97 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -457,93 +457,92 @@ public void process(APDU apdu) { } byte[] apduBuffer = apdu.getBuffer(); byte apduIns = apduBuffer[ISO7816.OFFSET_INS]; - if (isCommandAllowed(apduIns)) { - switch (apduIns) { - case INS_INIT_STRONGBOX_CMD: - processInitStrongBoxCmd(apdu); - sendError(apdu, KMError.OK); - return; - case INS_GENERATE_KEY_CMD: - processGenerateKey(apdu); - break; - case INS_IMPORT_KEY_CMD: - processImportKeyCmd(apdu); - break; - case INS_BEGIN_IMPORT_WRAPPED_KEY_CMD: - processBeginImportWrappedKeyCmd(apdu); - break; - case INS_FINISH_IMPORT_WRAPPED_KEY_CMD: - processFinishImportWrappedKeyCmd(apdu); - break; - case INS_EXPORT_KEY_CMD: - processExportKeyCmd(apdu); - break; - case INS_UPGRADE_KEY_CMD: - processUpgradeKeyCmd(apdu); - break; - case INS_DELETE_KEY_CMD: - processDeleteKeyCmd(apdu); - break; - case INS_DELETE_ALL_KEYS_CMD: - processDeleteAllKeysCmd(apdu); - break; - case INS_ADD_RNG_ENTROPY_CMD: - processAddRngEntropyCmd(apdu); - break; - case INS_COMPUTE_SHARED_HMAC_CMD: - processComputeSharedHmacCmd(apdu); - break; - case INS_DESTROY_ATT_IDS_CMD: - processDestroyAttIdsCmd(apdu); - break; - case INS_VERIFY_AUTHORIZATION_CMD: - processVerifyAuthorizationCmd(apdu); - break; - case INS_GET_HMAC_SHARING_PARAM_CMD: - processGetHmacSharingParamCmd(apdu); - break; - case INS_GET_KEY_CHARACTERISTICS_CMD: - processGetKeyCharacteristicsCmd(apdu); - break; - case INS_GET_HW_INFO_CMD: - processGetHwInfoCmd(apdu); - break; - case INS_BEGIN_OPERATION_CMD: - processBeginOperationCmd(apdu); - break; - case INS_UPDATE_OPERATION_CMD: - processUpdateOperationCmd(apdu); - break; - case INS_FINISH_OPERATION_CMD: - processFinishOperationCmd(apdu); - break; - case INS_ABORT_OPERATION_CMD: - processAbortOperationCmd(apdu); - break; - case INS_DEVICE_LOCKED_CMD: - processDeviceLockedCmd(apdu); - break; - case INS_EARLY_BOOT_ENDED_CMD: - processEarlyBootEndedCmd(apdu); - break; - case INS_UPDATE_AAD_OPERATION_CMD: - processUpdateAadOperationCmd(apdu); - break; - case INS_GENERATE_RKP_KEY_CMD: - case INS_BEGIN_SEND_DATA_CMD: - case INS_UPDATE_CHALLENGE_CMD: - case INS_UPDATE_EEK_CHAIN_CMD: - case INS_UPDATE_KEY_CMD: - case INS_FINISH_SEND_DATA_CMD: - case INS_GET_RESPONSE_CMD: - case INS_GET_RKP_HARDWARE_INFO: - rkp.process(apduIns, apdu); - break; - default: - ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); - } - } else { - ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); + if (!isStrongBoxKeymasterCmdAllowed(apduIns)) { + ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); + } + switch (apduIns) { + case INS_INIT_STRONGBOX_CMD: + processInitStrongBoxCmd(apdu); + sendError(apdu, KMError.OK); + return; + case INS_GENERATE_KEY_CMD: + processGenerateKey(apdu); + break; + case INS_IMPORT_KEY_CMD: + processImportKeyCmd(apdu); + break; + case INS_BEGIN_IMPORT_WRAPPED_KEY_CMD: + processBeginImportWrappedKeyCmd(apdu); + break; + case INS_FINISH_IMPORT_WRAPPED_KEY_CMD: + processFinishImportWrappedKeyCmd(apdu); + break; + case INS_EXPORT_KEY_CMD: + processExportKeyCmd(apdu); + break; + case INS_UPGRADE_KEY_CMD: + processUpgradeKeyCmd(apdu); + break; + case INS_DELETE_KEY_CMD: + processDeleteKeyCmd(apdu); + break; + case INS_DELETE_ALL_KEYS_CMD: + processDeleteAllKeysCmd(apdu); + break; + case INS_ADD_RNG_ENTROPY_CMD: + processAddRngEntropyCmd(apdu); + break; + case INS_COMPUTE_SHARED_HMAC_CMD: + processComputeSharedHmacCmd(apdu); + break; + case INS_DESTROY_ATT_IDS_CMD: + processDestroyAttIdsCmd(apdu); + break; + case INS_VERIFY_AUTHORIZATION_CMD: + processVerifyAuthorizationCmd(apdu); + break; + case INS_GET_HMAC_SHARING_PARAM_CMD: + processGetHmacSharingParamCmd(apdu); + break; + case INS_GET_KEY_CHARACTERISTICS_CMD: + processGetKeyCharacteristicsCmd(apdu); + break; + case INS_GET_HW_INFO_CMD: + processGetHwInfoCmd(apdu); + break; + case INS_BEGIN_OPERATION_CMD: + processBeginOperationCmd(apdu); + break; + case INS_UPDATE_OPERATION_CMD: + processUpdateOperationCmd(apdu); + break; + case INS_FINISH_OPERATION_CMD: + processFinishOperationCmd(apdu); + break; + case INS_ABORT_OPERATION_CMD: + processAbortOperationCmd(apdu); + break; + case INS_DEVICE_LOCKED_CMD: + processDeviceLockedCmd(apdu); + break; + case INS_EARLY_BOOT_ENDED_CMD: + processEarlyBootEndedCmd(apdu); + break; + case INS_UPDATE_AAD_OPERATION_CMD: + processUpdateAadOperationCmd(apdu); + break; + case INS_GENERATE_RKP_KEY_CMD: + case INS_BEGIN_SEND_DATA_CMD: + case INS_UPDATE_CHALLENGE_CMD: + case INS_UPDATE_EEK_CHAIN_CMD: + case INS_UPDATE_KEY_CMD: + case INS_FINISH_SEND_DATA_CMD: + case INS_GET_RESPONSE_CMD: + case INS_GET_RKP_HARDWARE_INFO: + rkp.process(apduIns, apdu); + break; + default: + ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); } } catch (KMException exception) { freeOperations(); @@ -566,20 +565,21 @@ public void process(APDU apdu) { } } - private boolean isCommandAllowed(short apduIns) { - boolean result = true; - switch(apduIns) { - case INS_INIT_STRONGBOX_CMD: - case INS_GET_HMAC_SHARING_PARAM_CMD: - case INS_COMPUTE_SHARED_HMAC_CMD: - break; - + private boolean isStrongBoxKeymasterCmdAllowed(byte apduIns) { + if(kmDataStore.isSBInitCompleted()) { + return true; + } + // Data Dirty. + switch (apduIns) { + case INS_GENERATE_KEY_CMD: + case INS_IMPORT_KEY_CMD: + case INS_IMPORT_WRAPPED_KEY_CMD: + case INS_ATTEST_KEY_CMD: + return false; default: - if(!(kmDataStore.isSBInitCompleted())) { - result = false; - } + break; } - return result; + return true; } private void generateUniqueOperationHandle(byte[] buf, short offset, short len) { @@ -1036,7 +1036,7 @@ private void processComputeSharedHmacCmd(APDU apdu) { (short) sharingCheck.length, scratchPad, keyLen); - kmDataStore.updateSBInitStatus(KMKeymintDataStore.NEGOTIATED_SHARED_SECRET_DONE); + kmDataStore.updateSBInitStatus(KMKeymintDataStore.NEGOTIATED_SHARED_SECRET_SUCCESS); // verification signature blob - 32 bytes //tmpVariables[1] short signature = KMByteBlob.instance(scratchPad, keyLen, signLen); @@ -3444,7 +3444,7 @@ private void processInitStrongBoxCmd(APDU apdu) { setOsVersion(osVersion); setOsPatchLevel(osPatchLevel); setVendorPatchLevel(vendorPatchLevel); - kmDataStore.updateSBInitStatus(KMKeymintDataStore.INIT_STRONGBOX_DONE); + kmDataStore.updateSBInitStatus(KMKeymintDataStore.SET_SYSTEM_PROPERTIES_SUCCESS); } public void reboot() { diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java index c89077d9..84f3ae75 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java @@ -74,10 +74,10 @@ public class KMKeymintDataStore implements KMUpgradable { private static final short BCC_MAX_SIZE = 512; public static final byte RESET_SB_INIT_STATUS = 0x00; - public static final byte SET_BOOT_PARAMS_DONE = 0x01; - public static final byte DEVICE_BOOT_ENDED_DONE = 0x02; - public static final byte INIT_STRONGBOX_DONE = 0x04; - public static final byte NEGOTIATED_SHARED_SECRET_DONE = 0x08; + public static final byte SET_BOOT_PARAMS_SUCCESS = 0x01; + public static final byte SET_BOOT_ENDED_SUCCESS = 0x02; + public static final byte SET_SYSTEM_PROPERTIES_SUCCESS = 0x04; + public static final byte NEGOTIATED_SHARED_SECRET_SUCCESS = 0x08; // Data - originally was in repository private byte[] attIdBrand; @@ -262,7 +262,7 @@ public boolean getBootEndedStatus() { short offset = repository.allocReclaimableMemory((short) 1); byte[] buf = repository.getHeap(); getSBInitStatus(buf, offset); - if(DEVICE_BOOT_ENDED_DONE == (buf[offset] & DEVICE_BOOT_ENDED_DONE)) { + if(SET_BOOT_ENDED_SUCCESS == (buf[offset] & SET_BOOT_ENDED_SUCCESS)) { result = true; } repository.reclaimMemory((short)1); @@ -339,18 +339,19 @@ public boolean isSBInitCompleted() { short offset = repository.allocReclaimableMemory((short) 1); byte[] buf = repository.getHeap(); getSBInitStatus(buf, offset); - if ((0 != (buf[offset] & SET_BOOT_PARAMS_DONE)) - && (0 != (buf[offset] & INIT_STRONGBOX_DONE)) - && (0 != (buf[offset] & NEGOTIATED_SHARED_SECRET_DONE))) { + if ((0 != (buf[offset] & SET_BOOT_PARAMS_SUCCESS)) + && (0 != (buf[offset] & SET_BOOT_ENDED_SUCCESS)) + && (0 != (buf[offset] & SET_SYSTEM_PROPERTIES_SUCCESS)) + && (0 != (buf[offset] & NEGOTIATED_SHARED_SECRET_SUCCESS))) { result = true; } repository.reclaimMemory((short)1); return result; } - public void getSBInitStatus(byte[] scratchpad, short offset) { + public short getSBInitStatus(byte[] scratchpad, short offset) { scratchpad[offset] = 0; - readDataEntry(BOOT_ENDED_FLAG, scratchpad, offset); + return readDataEntry(BOOT_ENDED_FLAG, scratchpad, offset); } public void clearDeviceLockTimeStamp() { @@ -957,9 +958,12 @@ void handleProvisionStatusUpgrade(){ void handleSBInitStatusUpgrade(){ short dInex = repository.allocReclaimableMemory((short)1); byte data[] = repository.getHeap(); - getSBInitStatus(data, dInex); - if(data[dInex] == 0x01) { - updateSBInitStatus(DEVICE_BOOT_ENDED_DONE); + short len = getSBInitStatus(data, dInex); + if(len != 0) { + updateSBInitStatus((byte)(SET_BOOT_PARAMS_SUCCESS | + SET_BOOT_ENDED_SUCCESS | + SET_SYSTEM_PROPERTIES_SUCCESS | + NEGOTIATED_SHARED_SECRET_SUCCESS)); } repository.reclaimMemory((short)1); } From 8d56d611e7a10e88e1a648bc8e9f4d63a314b372 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Tue, 7 Jun 2022 05:31:56 +0000 Subject: [PATCH 3/5] Updated system properties validation --- .../javacard/keymaster/KMAndroidSEApplet.java | 26 ++----- .../javacard/keymaster/KMKeymasterApplet.java | 50 ++++++------ .../keymaster/KMKeymintDataStore.java | 77 +++++++------------ 3 files changed, 61 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 300fabb7..1d75d262 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -54,7 +54,7 @@ public class KMAndroidSEApplet extends KMKeymasterApplet implements OnUpgradeLis private static final byte INS_PROVISION_RKP_ADDITIONAL_CERT_CHAIN_CMD = INS_KEYMINT_PROVIDER_APDU_START + 7; private static final byte INS_SET_BOOT_ENDED_CMD = - INS_KEYMINT_PROVIDER_APDU_START + 8; + INS_KEYMINT_PROVIDER_APDU_START + 8; //unused private static final byte INS_SE_FACTORY_PROVISIONING_LOCK_CMD = INS_KEYMINT_PROVIDER_APDU_START + 9; private static final byte INS_PROVISION_OEM_ROOT_PUBLIC_KEY_CMD = INS_KEYMINT_PROVIDER_APDU_START + 10; private static final byte INS_OEM_UNLOCK_PROVISIONING_CMD = INS_KEYMINT_PROVIDER_APDU_START + 11; @@ -120,11 +120,7 @@ public void process(APDU apdu) { case INS_SET_BOOT_PARAMS_CMD: processSetBootParamsCmd(apdu); - break; - - case INS_SET_BOOT_ENDED_CMD: - processSetBootEndedCmd(apdu); - break; + break; case INS_PROVISION_RKP_DEVICE_UNIQUE_KEYPAIR_CMD: processProvisionRkpDeviceUniqueKeyPair(apdu); @@ -214,7 +210,6 @@ private boolean isCommandAllowed(short apduIns) { break; case INS_SET_BOOT_PARAMS_CMD: - case INS_SET_BOOT_ENDED_CMD: case INS_GET_PROVISION_STATUS_CMD: break; @@ -512,23 +507,13 @@ private void processGetProvisionStatusCmd(APDU apdu) { sendOutgoing(apdu, resp); } - private void processSetBootEndedCmd(APDU apdu) { - if (seProvider.isBootSignalEventSupported() - && (!seProvider.isDeviceRebooted())) { - ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); - } - //set the flag to mark boot ended - kmDataStore.updateSBInitStatus(KMKeymintDataStore.SET_BOOT_ENDED_SUCCESS); - seProvider.clearDeviceBooted(false); - sendError(apdu, KMError.OK); - } - private void processSetBootParamsCmd(APDU apdu) { if (seProvider.isBootSignalEventSupported() && (!seProvider.isDeviceRebooted())) { ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); } - kmDataStore.clearSBInitStatus(); + // clear the device reboot status + kmDataStore.clearDeviceBootStatus(); short argsProto = KMArray.instance((short) 5); byte[] scratchPad = apdu.getBuffer(); // Array of 4 expected arguments @@ -580,7 +565,8 @@ private void processSetBootParamsCmd(APDU apdu) { kmDataStore.createComputedHmacKey(scratchPad, (short) 0, KMKeymintDataStore.COMPUTED_HMAC_KEY_SIZE); super.reboot(); - kmDataStore.updateSBInitStatus(KMKeymintDataStore.SET_BOOT_PARAMS_SUCCESS); + kmDataStore.setDeviceBootStatus(KMKeymintDataStore.SET_BOOT_PARAMS_SUCCESS); + seProvider.clearDeviceBooted(false); sendError(apdu, KMError.OK); } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 07de2c97..caeffac7 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -457,7 +457,7 @@ public void process(APDU apdu) { } byte[] apduBuffer = apdu.getBuffer(); byte apduIns = apduBuffer[ISO7816.OFFSET_INS]; - if (!isStrongBoxKeymasterCmdAllowed(apduIns)) { + if (!isKeymintReady(apduIns)) { ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); } switch (apduIns) { @@ -565,21 +565,26 @@ public void process(APDU apdu) { } } - private boolean isStrongBoxKeymasterCmdAllowed(byte apduIns) { - if(kmDataStore.isSBInitCompleted()) { - return true; - } - // Data Dirty. - switch (apduIns) { - case INS_GENERATE_KEY_CMD: - case INS_IMPORT_KEY_CMD: - case INS_IMPORT_WRAPPED_KEY_CMD: - case INS_ATTEST_KEY_CMD: - return false; - default: - break; - } - return true; + //After every device boot, the Keymaster becomes ready to execute all the commands only after + // 1. boot parameters are set, + // 2. system properties are set and + // 3. computed the shared secret successfully. + private boolean isKeymintReady(byte apduIns) { + if(kmDataStore.isDeviceReady()) { + return true; + } + // Below commands are allowed even if the Keymaster is not ready. + switch (apduIns) { + case INS_GET_HW_INFO_CMD: + case INS_ADD_RNG_ENTROPY_CMD: + case INS_GET_HMAC_SHARING_PARAM_CMD: + case INS_COMPUTE_SHARED_HMAC_CMD: + case INS_INIT_STRONGBOX_CMD: + return true; + default: + break; + } + return false; } private void generateUniqueOperationHandle(byte[] buf, short offset, short len) { @@ -1036,7 +1041,7 @@ private void processComputeSharedHmacCmd(APDU apdu) { (short) sharingCheck.length, scratchPad, keyLen); - kmDataStore.updateSBInitStatus(KMKeymintDataStore.NEGOTIATED_SHARED_SECRET_SUCCESS); + kmDataStore.setDeviceBootStatus(KMKeymintDataStore.NEGOTIATED_SHARED_SECRET_SUCCESS); // verification signature blob - 32 bytes //tmpVariables[1] short signature = KMByteBlob.instance(scratchPad, keyLen, signLen); @@ -2592,6 +2597,9 @@ private void authorizeAndBeginOperation(KMOperationState op, byte[] scratchPad) authorizeDeviceUnlock(scratchPad); authorizeKeyUsageForCount(scratchPad); + KMTag.assertAbsence(data[HW_PARAMETERS], KMType.BOOL_TAG, KMType.BOOTLOADER_ONLY, + KMError.INVALID_KEY_BLOB); + //Validate early boot //VTS expects error code EARLY_BOOT_ONLY during begin operation if eary boot ended tag is present if (kmDataStore.getEarlyBootEndedStatus()) { @@ -2599,12 +2607,6 @@ private void authorizeAndBeginOperation(KMOperationState op, byte[] scratchPad) KMError.EARLY_BOOT_ENDED); } - //Validate bootloader only - if (kmDataStore.getBootEndedStatus()) { - KMTag.assertAbsence(data[HW_PARAMETERS], KMType.BOOL_TAG, KMType.BOOTLOADER_ONLY, - KMError.INVALID_KEY_BLOB); - } - // Authorize Caller Nonce - if caller nonce absent in key char and nonce present in // key params then fail if it is not a Decrypt operation data[IV] = KMType.INVALID_VALUE; @@ -3444,7 +3446,7 @@ private void processInitStrongBoxCmd(APDU apdu) { setOsVersion(osVersion); setOsPatchLevel(osPatchLevel); setVendorPatchLevel(vendorPatchLevel); - kmDataStore.updateSBInitStatus(KMKeymintDataStore.SET_SYSTEM_PROPERTIES_SUCCESS); + kmDataStore.setDeviceBootStatus(KMKeymintDataStore.SET_SYSTEM_PROPERTIES_SUCCESS); } public void reboot() { diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java index 84f3ae75..e8dac348 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java @@ -49,9 +49,10 @@ public class KMKeymintDataStore implements KMUpgradable { public static final byte DEVICE_LOCKED_TIME = 5; public static final byte DEVICE_LOCKED = 6; public static final byte DEVICE_LOCKED_PASSWORD_ONLY = 7; + // Total 8 auth tags, so the next offset is AUTH_TAG_1 + 8 public static final byte AUTH_TAG_1 = 8; - public static final byte BOOT_ENDED_FLAG = 15; + public static final byte DEVICE_STATUS_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; @@ -69,15 +70,22 @@ public class KMKeymintDataStore implements KMUpgradable { public static final short AUTH_TAG_ENTRY_SIZE = (AUTH_TAG_LENGTH + AUTH_TAG_COUNTER_SIZE + 1); private static final short MASTER_KEY_SIZE = 16; private static final short SHARED_SECRET_KEY_SIZE = 32; + private static final byte DEVICE_STATUS_FLAG_SIZE = 1; private static final short ADDITIONAL_CERT_CHAIN_MAX_SIZE = 512;//First 2 bytes for length. private static final short BCC_MAX_SIZE = 512; - public static final byte RESET_SB_INIT_STATUS = 0x00; - public static final byte SET_BOOT_PARAMS_SUCCESS = 0x01; - public static final byte SET_BOOT_ENDED_SUCCESS = 0x02; - public static final byte SET_SYSTEM_PROPERTIES_SUCCESS = 0x04; - public static final byte NEGOTIATED_SHARED_SECRET_SUCCESS = 0x08; + //Device boot states. Applet starts executing the + // core commands once all the states are set. The commands + // that are allowed irrespective of these states are: + // All the provision commands + // INS_GET_HW_INFO_CMD + // INS_ADD_RNG_ENTROPY_CMD + // INS_COMPUTE_SHARED_HMAC_CMD + // INS_GET_HMAC_SHARING_PARAM_CMD + public static final byte SET_BOOT_PARAMS_SUCCESS = 0x01; + public static final byte SET_SYSTEM_PROPERTIES_SUCCESS = 0x02; + public static final byte NEGOTIATED_SHARED_SECRET_SUCCESS = 0x04; // Data - originally was in repository private byte[] attIdBrand; @@ -257,18 +265,6 @@ public boolean getEarlyBootEndedStatus() { return readBoolean(EARLY_BOOT_ENDED_FLAG); } - public boolean getBootEndedStatus() { - boolean result = false; - short offset = repository.allocReclaimableMemory((short) 1); - byte[] buf = repository.getHeap(); - getSBInitStatus(buf, offset); - if(SET_BOOT_ENDED_SUCCESS == (buf[offset] & SET_BOOT_ENDED_SUCCESS)) { - result = true; - } - repository.reclaimMemory((short)1); - return result; - } - public short getDeviceTimeStamp() { short blob = readData(DEVICE_LOCKED_TIME); if (blob == KMType.INVALID_VALUE) { @@ -320,38 +316,37 @@ public void setDeviceLockTimestamp(byte[] buf, short start, short len) { public void setEarlyBootEndedStatus(boolean flag) { writeBoolean(EARLY_BOOT_ENDED_FLAG, flag); } - - public void clearSBInitStatus() { - clearDataEntry(BOOT_ENDED_FLAG); - } - public void updateSBInitStatus(byte initStatus) { - short offset = repository.allocReclaimableMemory((short) 1); + public void clearDeviceBootStatus() { + clearDataEntry(DEVICE_STATUS_FLAG); + } + + public void setDeviceBootStatus(byte initStatus) { + short offset = repository.allocReclaimableMemory(DEVICE_STATUS_FLAG_SIZE); byte[] buf = repository.getHeap(); - getSBInitStatus(buf, offset); + getDeviceBootStatus(buf, offset); buf[offset] |= initStatus; - writeDataEntry(BOOT_ENDED_FLAG, buf, offset, (short) 1); - repository.reclaimMemory((short)1); + writeDataEntry(DEVICE_STATUS_FLAG, buf, offset, DEVICE_STATUS_FLAG_SIZE); + repository.reclaimMemory(DEVICE_STATUS_FLAG_SIZE); } - public boolean isSBInitCompleted() { + public boolean isDeviceReady() { boolean result = false; - short offset = repository.allocReclaimableMemory((short) 1); + short offset = repository.allocReclaimableMemory(DEVICE_STATUS_FLAG_SIZE); byte[] buf = repository.getHeap(); - getSBInitStatus(buf, offset); + getDeviceBootStatus(buf, offset); if ((0 != (buf[offset] & SET_BOOT_PARAMS_SUCCESS)) - && (0 != (buf[offset] & SET_BOOT_ENDED_SUCCESS)) && (0 != (buf[offset] & SET_SYSTEM_PROPERTIES_SUCCESS)) && (0 != (buf[offset] & NEGOTIATED_SHARED_SECRET_SUCCESS))) { result = true; } - repository.reclaimMemory((short)1); + repository.reclaimMemory(DEVICE_STATUS_FLAG_SIZE); return result; } - public short getSBInitStatus(byte[] scratchpad, short offset) { + public short getDeviceBootStatus(byte[] scratchpad, short offset) { scratchpad[offset] = 0; - return readDataEntry(BOOT_ENDED_FLAG, scratchpad, offset); + return readDataEntry(DEVICE_STATUS_FLAG, scratchpad, offset); } public void clearDeviceLockTimeStamp() { @@ -932,7 +927,6 @@ public void onRestore(Element element, short oldVersion, short currentVersion) { void handleDataUpgrade(short oldVersion, short currentVersion) { if(oldVersion == 0x0100 && currentVersion == 0x0200) { handleProvisionStatusUpgrade(); - handleSBInitStatusUpgrade(); } } @@ -954,19 +948,6 @@ void handleProvisionStatusUpgrade(){ writeDataEntry(PROVISIONED_STATUS, data, dInex, (short) 2); repository.reclaimMemory((short)2); } - - void handleSBInitStatusUpgrade(){ - short dInex = repository.allocReclaimableMemory((short)1); - byte data[] = repository.getHeap(); - short len = getSBInitStatus(data, dInex); - if(len != 0) { - updateSBInitStatus((byte)(SET_BOOT_PARAMS_SUCCESS | - SET_BOOT_ENDED_SUCCESS | - SET_SYSTEM_PROPERTIES_SUCCESS | - NEGOTIATED_SHARED_SECRET_SUCCESS)); - } - repository.reclaimMemory((short)1); - } @Override public short getBackupPrimitiveByteCount() { From 5f279ab0ce1cd23e1e7a8e9ce9c63f644da921f7 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Tue, 7 Jun 2022 05:41:49 +0000 Subject: [PATCH 4/5] Updated system properties validation --- .../com/android/javacard/keymaster/KMAndroidSEApplet.java | 4 ++-- .../com/android/javacard/keymaster/KMKeymasterApplet.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index 1d75d262..f320dac4 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -120,7 +120,7 @@ public void process(APDU apdu) { case INS_SET_BOOT_PARAMS_CMD: processSetBootParamsCmd(apdu); - break; + break; case INS_PROVISION_RKP_DEVICE_UNIQUE_KEYPAIR_CMD: processProvisionRkpDeviceUniqueKeyPair(apdu); @@ -215,7 +215,7 @@ private boolean isCommandAllowed(short apduIns) { default: // Allow other commands only if provision is completed. - if (!(isProvisioningComplete())) { + if (!isProvisioningComplete()) { result = false; } } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index caeffac7..0cf17077 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -570,9 +570,9 @@ public void process(APDU apdu) { // 2. system properties are set and // 3. computed the shared secret successfully. private boolean isKeymintReady(byte apduIns) { - if(kmDataStore.isDeviceReady()) { - return true; - } + if(kmDataStore.isDeviceReady()) { + return true; + } // Below commands are allowed even if the Keymaster is not ready. switch (apduIns) { case INS_GET_HW_INFO_CMD: @@ -2598,7 +2598,7 @@ private void authorizeAndBeginOperation(KMOperationState op, byte[] scratchPad) authorizeKeyUsageForCount(scratchPad); KMTag.assertAbsence(data[HW_PARAMETERS], KMType.BOOL_TAG, KMType.BOOTLOADER_ONLY, - KMError.INVALID_KEY_BLOB); + KMError.INVALID_KEY_BLOB); //Validate early boot //VTS expects error code EARLY_BOOT_ONLY during begin operation if eary boot ended tag is present From a85fb8ec92e2ec3905dc60ed7cf829c27dbd531c Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Tue, 7 Jun 2022 07:45:18 +0000 Subject: [PATCH 5/5] added CLA validation --- .../javacard/keymaster/KMAndroidSEApplet.java | 6 ++- .../javacard/keymaster/KMJCardSimApplet.java | 52 ++++++------------- .../javacard/keymaster/KMKeymasterApplet.java | 1 - 3 files changed, 20 insertions(+), 39 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index f320dac4..848eeefd 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -647,9 +647,13 @@ public Element onSave() { private short validateApdu(APDU apdu) { // Read the apdu header and buffer. byte[] apduBuffer = apdu.getBuffer(); - byte apduClass = apduBuffer[ISO7816.OFFSET_CLA]; short P1P2 = Util.getShort(apduBuffer, ISO7816.OFFSET_P1); + // Validate CLA + if (!apdu.isValidCLA()) { + ISOException.throwIt(ISO7816.SW_CLA_NOT_SUPPORTED); + } + // Validate P1P2. if (P1P2 != KMKeymasterApplet.KM_HAL_VERSION) { sendError(apdu, KMError.INVALID_P1P2); diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java index 66f80148..2b8e55fe 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java @@ -43,29 +43,14 @@ public class KMJCardSimApplet extends KMKeymasterApplet { private static final byte INS_PROVISION_RKP_ADDITIONAL_CERT_CHAIN_CMD = INS_KEYMINT_PROVIDER_APDU_START + 7; private static final byte INS_SET_BOOT_ENDED_CMD = - INS_KEYMINT_PROVIDER_APDU_START + 8; + INS_KEYMINT_PROVIDER_APDU_START + 8; //unused private static final byte INS_SE_FACTORY_PROVISIONING_LOCK_CMD = INS_KEYMINT_PROVIDER_APDU_START + 9; private static final byte INS_PROVISION_OEM_ROOT_PUBLIC_KEY_CMD = INS_KEYMINT_PROVIDER_APDU_START + 10; private static final byte INS_OEM_UNLOCK_PROVISIONING_CMD = INS_KEYMINT_PROVIDER_APDU_START + 11; - private static final byte INS_KEYMINT_PROVIDER_APDU_END = 0x1F; public static final byte BOOT_KEY_MAX_SIZE = 32; public static final byte BOOT_HASH_MAX_SIZE = 32; - - // Provision reporting status - private static final short NOT_PROVISIONED = 0x0000; - private static final short PROVISION_STATUS_ATTESTATION_KEY = 0x0001; - private static final short PROVISION_STATUS_ATTESTATION_CERT_CHAIN = 0x0002; - private static final short PROVISION_STATUS_ATTESTATION_CERT_PARAMS = 0x0004; - private static final short PROVISION_STATUS_ATTEST_IDS = 0x0008; - private static final short PROVISION_STATUS_PRESHARED_SECRET = 0x0010; - private static final short PROVISION_STATUS_PROVISIONING_LOCKED = 0x0020; - private static final short PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR = 0x0040; - private static final short PROVISION_STATUS_ADDITIONAL_CERT_CHAIN = 0x0080; - private static final short PROVISION_STATUS_SE_LOCKED = 0x0100; - private static final short PROVISION_STATUS_OEM_PUBLIC_KEY = 0x0200; - public static final short SHARED_SECRET_KEY_SIZE = 32; // Package version. @@ -125,10 +110,6 @@ public void process(APDU apdu) { processSetBootParamsCmd(apdu); break; - case INS_SET_BOOT_ENDED_CMD: - processSetBootEndedCmd(apdu); - break; - case INS_PROVISION_RKP_DEVICE_UNIQUE_KEYPAIR_CMD: processProvisionRkpDeviceUniqueKeyPair(apdu); break; @@ -217,14 +198,13 @@ private boolean isCommandAllowed(short apduIns) { break; case INS_SET_BOOT_PARAMS_CMD: - case INS_SET_BOOT_ENDED_CMD: case INS_GET_PROVISION_STATUS_CMD: break; default: // Allow other commands only if provision is completed. if (!isProvisioningComplete()) { - result = false; + result = false; } } return result; @@ -515,23 +495,13 @@ private void processGetProvisionStatusCmd(APDU apdu) { sendOutgoing(apdu, resp); } - private void processSetBootEndedCmd(APDU apdu) { - if (seProvider.isBootSignalEventSupported() - && (!seProvider.isDeviceRebooted())) { - ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); - } - //set the flag to mark boot ended - kmDataStore.setBootEndedStatus(true); - seProvider.clearDeviceBooted(false); - sendError(apdu, KMError.OK); - } - private void processSetBootParamsCmd(APDU apdu) { if (seProvider.isBootSignalEventSupported() && (!seProvider.isDeviceRebooted())) { ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); } - + // clear the device reboot status + kmDataStore.clearDeviceBootStatus(); short argsProto = KMArray.instance((short) 5); byte[] scratchPad = apdu.getBuffer(); // Array of 4 expected arguments @@ -583,12 +553,14 @@ private void processSetBootParamsCmd(APDU apdu) { kmDataStore.createComputedHmacKey(scratchPad, (short) 0, KMKeymintDataStore.COMPUTED_HMAC_KEY_SIZE); super.reboot(); + kmDataStore.setDeviceBootStatus(KMKeymintDataStore.SET_BOOT_PARAMS_SUCCESS); + seProvider.clearDeviceBooted(false); sendError(apdu, KMError.OK); } private boolean isProvisioningComplete() { - short dInex = repository.allocReclaimableMemory((short)2); - byte data[] = repository.getHeap(); + short dInex = repository.allocReclaimableMemory((short)2); + byte data[] = repository.getHeap(); kmDataStore.getProvisionStatus(data, dInex); short pStatus = Util.getShort(data, dInex); boolean result = false; @@ -605,9 +577,15 @@ private boolean isProvisioningComplete() { private short validateApdu(APDU apdu) { // Read the apdu header and buffer. byte[] apduBuffer = apdu.getBuffer(); - byte apduClass = apduBuffer[ISO7816.OFFSET_CLA]; + short apduClass = (short) (apduBuffer[ISO7816.OFFSET_CLA] & 0x00FF); short P1P2 = Util.getShort(apduBuffer, ISO7816.OFFSET_P1); + // Validate CLA. + if (((apduClass & 0x00E0) == 0x0020) || + (apduClass == 0x00FF)) { + ISOException.throwIt(ISO7816.SW_CLA_NOT_SUPPORTED); + } + // Validate P1P2. if (P1P2 != KMKeymasterApplet.KM_HAL_VERSION) { sendError(apdu, KMError.INVALID_P1P2); diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 0cf17077..ed51caa0 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -48,7 +48,6 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe public static final short VERIFIED_BOOT_HASH_SIZE = 32; public static final short BOOT_PATCH_LVL_SIZE = 4; - protected static final byte CLA_ISO7816_NO_SM_NO_CHAN = (byte) 0x80; protected static final short KM_HAL_VERSION = (short) 0x5000; private static final short MAX_AUTH_DATA_SIZE = (short) 512; private static final short DERIVE_KEY_INPUT_SIZE = (short) 256;