From d5029698a529b4cf165794fa82ff6514d4cf3303 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Wed, 23 Mar 2022 09:58:55 +0000 Subject: [PATCH 1/5] changes to split generate key function --- .../javacard/keymaster/KMKeymasterApplet.java | 164 ++++++++++-------- .../javacard/keymaster/KMRepository.java | 2 +- HAL/JavacardKeyMintDevice.cpp | 27 ++- 3 files changed, 113 insertions(+), 80 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 56105fdf..d0c29143 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -396,6 +396,9 @@ public void process(APDU apdu) { case INS_GENERATE_KEY_CMD: processGenerateKey(apdu); break; + case INS_ATTEST_KEY_CMD: + processAttestKeyCmd(apdu); + break; case INS_IMPORT_KEY_CMD: processImportKeyCmd(apdu); break; @@ -1220,7 +1223,6 @@ private KMAttestationCert makeCommonCert(byte[] scratchPad) { return cert; } - private KMAttestationCert makeAttestationCert(short attKeyBlob, short attKeyParam, short attChallenge, short issuer, short hwParameters, short swParameters, byte[] scratchPad) { KMAttestationCert cert = makeCommonCert(scratchPad); @@ -1273,32 +1275,29 @@ private KMAttestationCert makeAttestationCert(short attKeyBlob, short attKeyPara cert.publicKey(data[PUB_KEY]); // Save attestation application id - must be present. - short attAppId = + short attAppId = KMKeyParameters.findTag( KMType.BYTES_TAG, KMType.ATTESTATION_APPLICATION_ID, data[KEY_PARAMETERS]); - if (attAppId == KMType.INVALID_VALUE) { - KMException.throwIt(KMError.ATTESTATION_APPLICATION_ID_MISSING); - } - cert.extensionTag(attAppId, false); - // unique id byte blob - uses application id and temporal month count of creation time. - setUniqueId(cert, scratchPad); - // Add Attestation Ids if present - addAttestationIds(cert, scratchPad); - - // Add Tags - addTags(hwParameters, true, cert); - addTags(swParameters, false, cert); - // Add Device Boot locked status - cert.deviceLocked(kmDataStore.isDeviceBootLocked()); - // VB data - cert.verifiedBootHash(getVerifiedBootHash(scratchPad)); - cert.verifiedBootKey(getBootKey(scratchPad)); - cert.verifiedBootState((byte)kmDataStore.getBootState()); - - //TODO remove the following line - makeKeyCharacteristics(scratchPad); + if (attAppId == KMType.INVALID_VALUE) { + KMException.throwIt(KMError.ATTESTATION_APPLICATION_ID_MISSING); + } + cert.extensionTag(attAppId, false); + // unique id byte blob - uses application id and temporal month count of creation time. + setUniqueId(cert, scratchPad); + // Add Attestation Ids if present + addAttestationIds(cert, scratchPad); + + // Add Tags + addTags(hwParameters, true, cert); + addTags(swParameters, false, cert); + // Add Device Boot locked status + cert.deviceLocked(kmDataStore.isDeviceBootLocked()); + // VB data + cert.verifiedBootHash(getVerifiedBootHash(scratchPad)); + cert.verifiedBootKey(getBootKey(scratchPad)); + cert.verifiedBootState((byte)kmDataStore.getBootState()); + data[SECRET] = privKey; data[KEY_BLOB] = origBlob; - return cert; } @@ -3300,13 +3299,9 @@ protected void setVendorPatchLevel(short patch){ private short generateKeyCmd(APDU apdu){ short params = KMKeyParameters.expAny(); - short blob = KMByteBlob.exp(); // Array of expected arguments - short cmd = KMArray.instance((short) 4); + short cmd = KMArray.instance((short) 1); KMArray.cast(cmd).add((short) 0, params); //key params - KMArray.cast(cmd).add((short) 1, blob); //attest key - KMArray.cast(cmd).add((short) 2, params); //attest key params - KMArray.cast(cmd).add((short) 3, blob); //issuer return receiveIncoming(apdu, cmd); } @@ -3316,10 +3311,6 @@ private void processGenerateKey(APDU apdu) { // Re-purpose the apdu buffer as scratch pad. byte[] scratchPad = apdu.getBuffer(); data[KEY_PARAMETERS] = KMArray.cast(cmd).get((short) 0); - data[ATTEST_KEY_BLOB] = KMArray.cast(cmd).get((short) 1); - data[ATTEST_KEY_PARAMS] = KMArray.cast(cmd).get((short) 2); - data[ATTEST_KEY_ISSUER] = KMArray.cast(cmd).get((short) 3); - data[CERTIFICATE] = KMArray.instance((short)0); //by default the cert is empty. // ROLLBACK_RESISTANCE not supported. KMTag.assertAbsence(data[KEY_PARAMETERS], KMType.BOOL_TAG,KMType.ROLLBACK_RESISTANCE, KMError.ROLLBACK_RESISTANCE_UNAVAILABLE); @@ -3365,7 +3356,6 @@ private void processGenerateKey(APDU apdu) { // create key blob and associated attestation. data[ORIGIN] = KMType.GENERATED; makeKeyCharacteristics(scratchPad); - generateAttestation(data[ATTEST_KEY_BLOB], data[ATTEST_KEY_PARAMS],scratchPad); createEncryptedKeyBlob(scratchPad); // Remove custom tags from key characteristics short teeParams = KMKeyCharacteristics.cast(data[KEY_CHARACTERISTICS]).getTeeEnforced(); @@ -3373,14 +3363,77 @@ private void processGenerateKey(APDU apdu) { KMKeyParameters.cast(teeParams).deleteCustomTags(); } // prepare the response - short resp = KMArray.instance((short) 4); + short resp = KMArray.instance((short) 3); KMArray.cast(resp).add((short) 0, KMInteger.uint_16(KMError.OK)); KMArray.cast(resp).add((short) 1, data[KEY_BLOB]); KMArray.cast(resp).add((short) 2, data[KEY_CHARACTERISTICS]); - KMArray.cast(resp).add((short) 3, data[CERTIFICATE]); sendOutgoing(apdu, resp); } + private short generateAttestKeyCmd(APDU apdu) { + short params = KMKeyParameters.expAny(); + short blob = KMByteBlob.exp(); + // Array of expected arguments + short cmd = KMArray.instance((short) 5); + KMArray.cast(cmd).add((short) 0, blob); // key blob + KMArray.cast(cmd).add((short) 1, params); // keyparamters to be attested. + KMArray.cast(cmd).add((short) 2, blob); // attest key blob + KMArray.cast(cmd).add((short) 3, params); // attest key params + KMArray.cast(cmd).add((short) 4, blob); // attest issuer + + return receiveIncoming(apdu, cmd); + } + + public void getAttestKeyInputParameters(short arrPtr, short[] data, byte keyBlobOff, + byte keyParametersOff, + byte attestKeyBlobOff, byte attestKeyParamsOff, byte attestKeyIssuerOff) { + data[keyBlobOff] = KMArray.cast(arrPtr).get((short) 0); + data[keyParametersOff] = KMArray.cast(arrPtr).get((short) 1); + data[attestKeyBlobOff] = KMType.INVALID_VALUE; + data[attestKeyParamsOff] = KMType.INVALID_VALUE; + data[attestKeyIssuerOff] = KMType.INVALID_VALUE; + } + + private void processAttestKeyCmd(APDU apdu) { + // Receive the incoming request fully from the master into buffer. + short cmd = generateAttestKeyCmd(apdu); + // Re-purpose the apdu buffer as scratch pad. + byte[] scratchPad = apdu.getBuffer(); + data[KEY_BLOB] = KMArray.cast(cmd).get((short) 0); + data[KEY_PARAMETERS] = KMArray.cast(cmd).get((short) 1); + data[ATTEST_KEY_BLOB] = KMArray.cast(cmd).get((short) 2); + data[ATTEST_KEY_PARAMS] = KMArray.cast(cmd).get((short) 3); + data[ATTEST_KEY_ISSUER] = KMArray.cast(cmd).get((short) 4); + + data[CERTIFICATE] = KMArray.instance((short) 0); //by default the cert is empty. + + // Check for app id and app data. + data[APP_ID] = + KMKeyParameters.findTag(KMType.BYTES_TAG, KMType.APPLICATION_ID, data[KEY_PARAMETERS]); + data[APP_DATA] = + KMKeyParameters.findTag(KMType.BYTES_TAG, KMType.APPLICATION_DATA, data[KEY_PARAMETERS]); + if (data[APP_ID] != KMTag.INVALID_VALUE) { + data[APP_ID] = KMByteTag.getValue(data[APP_ID]); + } + if (data[APP_DATA] != KMTag.INVALID_VALUE) { + data[APP_DATA] = KMByteTag.getValue(data[APP_DATA]); + } + // parse key blob + parseEncryptedKeyBlob(data[KEY_BLOB], data[APP_ID], data[APP_DATA], scratchPad); + // The key which is being attested should be asymmetric i.e. RSA or EC + short alg = KMEnumTag.getValue(KMType.ALGORITHM, data[HW_PARAMETERS]); + if (alg != KMType.RSA && alg != KMType.EC) { + KMException.throwIt(KMError.INCOMPATIBLE_ALGORITHM); + } + // Build certificate + generateAttestation(data[ATTEST_KEY_BLOB], data[ATTEST_KEY_PARAMS], scratchPad); + + short resp = KMArray.instance((short) 2); + KMArray.cast(resp).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(resp).add((short) 1, data[CERTIFICATE]); + sendOutgoing(apdu, resp); + } + private short getAttestationMode(short attKeyBlob, short attChallenge){ short alg = KMKeyParameters.findTag(KMType.ENUM_TAG, KMType.ALGORITHM, data[KEY_PARAMETERS]); short mode = KMType.NO_CERT; @@ -3455,45 +3508,6 @@ private void generateAttestation(short attKeyBlob, short attKeyParam, byte[] sc // Initialize the certificate as array of blob data[CERTIFICATE] = KMArray.instance((short)1); KMArray.cast(data[CERTIFICATE]).add((short)0, certData); - -/* - boolean rsaCert = KMEnumTag.cast(alg).getValue() == KMType.EC; - short appId = KMType.INVALID_VALUE; - short appData = KMType.INVALID_VALUE; - if(attKeyParam != KMType.INVALID_VALUE) { - appId = - KMKeyParameters.findTag(KMType.BYTES_TAG, KMType.APPLICATION_ID, attKeyParam); - if (appId != KMTag.INVALID_VALUE) { - appId = KMByteTag.cast(appId).getValue(); - } - appData = - KMKeyParameters.findTag(KMType.BYTES_TAG, KMType.APPLICATION_DATA, attKeyParam); - if (appData != KMTag.INVALID_VALUE) { - appData = KMByteTag.cast(appData).getValue(); - } - } - - KMAttestationCert cert = makeCert(attKeyBlob, appId, appData, scratchPad); - if(cert == null) {// No certificate - data[CERTIFICATE] = KMArray.instance((short)0); - return; - } - - // Allocate memory - short certData = KMByteBlob.instance(MAX_CERT_SIZE); - cert.buffer(KMByteBlob.cast(certData).getBuffer(), - KMByteBlob.cast(certData).getStartOff(), - KMByteBlob.cast(certData).length()); - // Build the certificate - this will sign the cert - cert.build(); - // Adjust the start and length of the certificate in the blob - KMByteBlob.cast(certData).setStartOff(cert.getCertStart()); - KMByteBlob.cast(certData).setLength(cert.getCertLength()); - - // Initialize the certificate as array of blob - data[CERTIFICATE] = KMArray.instance((short)1); - KMArray.cast(data[CERTIFICATE]).add((short)0, certData); - */ } /** diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index 0a158878..ecff6b68 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -32,7 +32,7 @@ */ public class KMRepository { - public static final short HEAP_SIZE = 15000; + public static final short HEAP_SIZE = 13000; // Class Attributes private byte[] heap; diff --git a/HAL/JavacardKeyMintDevice.cpp b/HAL/JavacardKeyMintDevice.cpp index 584c75ed..faa1cd43 100644 --- a/HAL/JavacardKeyMintDevice.cpp +++ b/HAL/JavacardKeyMintDevice.cpp @@ -33,6 +33,7 @@ #include namespace aidl::android::hardware::security::keymint { +using km_utils::KmParamSet; using namespace ::keymaster; using namespace ::keymint::javacard; @@ -81,19 +82,37 @@ ScopedAStatus JavacardKeyMintDevice::generateKey(const vector& key cppbor::Array array; // add key params cbor_.addKeyparameters(array, keyParams); - // add attestation key if any - cbor_.addAttestationKey(array, attestationKey); auto [item, err] = card_->sendRequest(Instruction::INS_GENERATE_KEY_CMD, array); if (err != KM_ERROR_OK) { LOG(ERROR) << "Error in sending generateKey."; return km_utils::kmError2ScopedAStatus(err); } if (!cbor_.getBinaryArray(item, 1, creationResult->keyBlob) || - !cbor_.getKeyCharacteristics(item, 2, creationResult->keyCharacteristics) || - !cbor_.getCertificateChain(item, 3, creationResult->certificateChain)) { + !cbor_.getKeyCharacteristics(item, 2, creationResult->keyCharacteristics)) { LOG(ERROR) << "Error in decoding og response in generateKey."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + + AuthorizationSet paramSet; + paramSet.Reinitialize(KmParamSet(keyParams)); + // Call attestKey only Asymmetric algorithms. + keymaster_algorithm_t algorithm; + paramSet.GetTagValue(TAG_ALGORITHM, &algorithm); + if (algorithm == KM_ALGORITHM_RSA || algorithm == KM_ALGORITHM_EC) { + cppbor::Array attestKeyArray; + attestKeyArray.add(creationResult->keyBlob); + cbor_.addKeyparameters(attestKeyArray, keyParams); + cbor_.addAttestationKey(attestKeyArray, attestationKey); + auto [item, err] = card_->sendRequest(Instruction::INS_ATTEST_KEY_CMD, attestKeyArray); + if (err != KM_ERROR_OK) { + LOG(ERROR) << "Failed in attestKey err: "; + return km_utils::kmError2ScopedAStatus(err); + } + if (!cbor_.getCertificateChain(item, 1, creationResult->certificateChain)) { + LOG(ERROR) << "Error in decoding og response in generateKey."; + return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); + } + } return ScopedAStatus::ok(); } From 32962b3e81d33b6c3be7484bce9f15b02adc30e7 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Thu, 24 Mar 2022 14:43:24 +0000 Subject: [PATCH 2/5] Fix for attest key failure and heap memory optimisation --- .../javacard/keymaster/KMKeymasterApplet.java | 16 ++++++++++------ .../android/javacard/keymaster/KMRepository.java | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index d0c29143..fcf7bbcf 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -1288,7 +1288,8 @@ private KMAttestationCert makeAttestationCert(short attKeyBlob, short attKeyPara // Add Tags addTags(hwParameters, true, cert); - addTags(swParameters, false, cert); + short swParams = KMKeyParameters.makeKeystoreEnforced(data[KEY_PARAMETERS], scratchPad); + addTags(swParams, false, cert); // Add Device Boot locked status cert.deviceLocked(kmDataStore.isDeviceBootLocked()); // VB data @@ -3413,10 +3414,10 @@ private void processAttestKeyCmd(APDU apdu) { data[APP_DATA] = KMKeyParameters.findTag(KMType.BYTES_TAG, KMType.APPLICATION_DATA, data[KEY_PARAMETERS]); if (data[APP_ID] != KMTag.INVALID_VALUE) { - data[APP_ID] = KMByteTag.getValue(data[APP_ID]); + data[APP_ID] = KMByteTag.cast(data[APP_ID]).getValue(); } if (data[APP_DATA] != KMTag.INVALID_VALUE) { - data[APP_DATA] = KMByteTag.getValue(data[APP_DATA]); + data[APP_DATA] = KMByteTag.cast(data[APP_DATA]).getValue(); } // parse key blob parseEncryptedKeyBlob(data[KEY_BLOB], data[APP_ID], data[APP_DATA], scratchPad); @@ -3478,7 +3479,7 @@ private void generateAttestation(short attKeyBlob, short attKeyParam, byte[] sc switch (mode){ case KMType.ATTESTATION_CERT: - cert = makeAttestationCert(attKeyBlob,attKeyParam, attChallenge, data[ATTEST_KEY_ISSUER],data[HW_PARAMETERS], + cert = makeAttestationCert(attKeyBlob, attKeyParam, attChallenge, data[ATTEST_KEY_ISSUER],data[HW_PARAMETERS], data[SW_PARAMETERS], scratchPad); break; case KMType.SELF_SIGNED_CERT: @@ -3897,7 +3898,7 @@ private static void makeAuthData(byte[] scratchPad) { KMArray.cast(params).add((short) 2, data[PUB_KEY]); } - short authIndex = repository.alloc(MAX_AUTH_DATA_SIZE); + short authIndex = repository.allocReclaimableMemory(MAX_AUTH_DATA_SIZE); short index = 0; short len = 0; short paramsLen = KMArray.cast(params).length(); @@ -3914,7 +3915,10 @@ private static void makeAuthData(byte[] scratchPad) { } index++; } - data[AUTH_DATA] = authIndex; + short authDataIndex = repository.alloc(len); + Util.arrayCopyNonAtomic(repository.getHeap(), authIndex, repository.getHeap(), authDataIndex, len); + repository.reclaimMemory(MAX_AUTH_DATA_SIZE); + data[AUTH_DATA] = authDataIndex; data[AUTH_DATA_LENGTH] = len; } diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index ecff6b68..1e6c8371 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -32,7 +32,7 @@ */ public class KMRepository { - public static final short HEAP_SIZE = 13000; + public static final short HEAP_SIZE = 10000; // Class Attributes private byte[] heap; From 5ad4900562f08bb97adba4a0fd642bc1de586c14 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Thu, 31 Mar 2022 17:08:45 +0000 Subject: [PATCH 3/5] added heap memory profiling code and APDU command to get the used heap in repository --- .../javacard/keymaster/KMKeymasterApplet.java | 17 ++++++++++++++++- .../javacard/keymaster/KMRepository.java | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 9949695a..2054ef2e 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -147,7 +147,8 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe public static final byte INS_UPDATE_CHALLENGE_CMD = KEYMINT_CMD_APDU_START + 32; //0x40 public static final byte INS_FINISH_SEND_DATA_CMD = KEYMINT_CMD_APDU_START + 33; //0x41 public static final byte INS_GET_RESPONSE_CMD = KEYMINT_CMD_APDU_START + 34; //0x42 - private static final byte KEYMINT_CMD_APDU_END = KEYMINT_CMD_APDU_START + 35; //0x43 + private static final byte INS_GET_HEAP_PROFILE_DATA = KEYMINT_CMD_APDU_START + 35; //0x43 + private static final byte KEYMINT_CMD_APDU_END = KEYMINT_CMD_APDU_START + 36; //0x44 private static final byte INS_END_KM_CMD = 0x7F; @@ -472,6 +473,9 @@ public void process(APDU apdu) { case INS_GET_RKP_HARDWARE_INFO: rkp.process(apduIns, apdu); break; + case INS_GET_HEAP_PROFILE_DATA: + processGetHeapProfileData(apdu); + break; default: ISOException.throwIt(ISO7816.SW_INS_NOT_SUPPORTED); } @@ -585,6 +589,7 @@ protected void resetData() { */ public static void sendOutgoing(APDU apdu, short resp) { //TODO handle the extended buffer stuff. We can reuse this. + short usedHeap = repository.getHeapIndex(); short bufferStartOffset = repository.allocAvailableMemory(); byte[] buffer = repository.getHeap(); // TODO we can change the following to incremental send. @@ -593,6 +598,7 @@ public static void sendOutgoing(APDU apdu, short resp) { .getHeap().length)) { ISOException.throwIt(ISO7816.SW_WRONG_LENGTH); } + repository.updateHeapProfileData((short)(usedHeap + bufferLength)); // Send data apdu.setOutgoing(); apdu.setOutgoingLength(bufferLength); @@ -4220,4 +4226,13 @@ private void finishTrustedConfirmationOperation(KMOperationState op) { } } } + + private void processGetHeapProfileData(APDU apdu) { + // No Arguments + // prepare the response + short resp = KMArray.instance((short) 2); + KMArray.cast(resp).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(resp).add((short) 1, KMInteger.uint_16(repository.getMaxHeapUsed())); + sendOutgoing(apdu, resp); + } } diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index 1e6c8371..bd637578 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -38,6 +38,8 @@ public class KMRepository { private byte[] heap; private short[] heapIndex; private short reclaimIndex; + //used for heap profiling + public static short[] maxHeapUsage; // Singleton instance private static KMRepository repository; @@ -49,6 +51,7 @@ public static KMRepository instance() { public KMRepository(boolean isUpgrading) { heap = JCSystem.makeTransientByteArray(HEAP_SIZE, JCSystem.CLEAR_ON_RESET); heapIndex = JCSystem.makeTransientShortArray((short)1, JCSystem.CLEAR_ON_RESET); + maxHeapUsage = JCSystem.makeTransientShortArray((short)1, JCSystem.CLEAR_ON_RESET); reclaimIndex = HEAP_SIZE; repository = this; } @@ -114,5 +117,19 @@ public short alloc(short length) { public byte[] getHeap() { return heap; } + + public short getHeapIndex() { + return heapIndex[0]; + } + public void updateHeapProfileData(short size) { + if(size > maxHeapUsage[0]) { + maxHeapUsage[0] = size; + } + } + + public short getMaxHeapUsed() { + return maxHeapUsage[0]; + } + } From 1ff434b3401dcbaa5969b708ac3b995c2281f125 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Thu, 31 Mar 2022 17:15:01 +0000 Subject: [PATCH 4/5] bug fixed- In HAL, handled large size input data in ECDSA NO digest finish operation. --- .../javacard/keymaster/KMKeymasterApplet.java | 15 --------------- HAL/JavacardKeyMintOperation.cpp | 8 +++++++- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 2054ef2e..1457bbf6 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -750,21 +750,6 @@ private short keyBlob(){ } private void processDeleteKeyCmd(APDU apdu) { - short cmd = deleteKeyCmd(apdu); - data[KEY_BLOB] = KMArray.cast(cmd).get((short) 0); - try { - data[KEY_BLOB] = decoder.decodeArray(keyBlob(), - KMByteBlob.cast(data[KEY_BLOB]).getBuffer(), - KMByteBlob.cast(data[KEY_BLOB]).getStartOff(), - KMByteBlob.cast(data[KEY_BLOB]).length()); - } catch (ISOException e) { - // As per VTS, deleteKey should return KMError.OK but in case if - // input is empty then VTS accepts UNIMPLEMENTED errorCode as well. - KMException.throwIt(KMError.UNIMPLEMENTED); - } - if (KMArray.cast(data[KEY_BLOB]).length() < 4) { - KMException.throwIt(KMError.INVALID_KEY_BLOB); - } // Send ok sendError(apdu, KMError.OK); } diff --git a/HAL/JavacardKeyMintOperation.cpp b/HAL/JavacardKeyMintOperation.cpp index 38cbb0b4..030f8295 100644 --- a/HAL/JavacardKeyMintOperation.cpp +++ b/HAL/JavacardKeyMintOperation.cpp @@ -82,15 +82,21 @@ ScopedAStatus JavacardKeyMintOperation::finish( const vector inData = input.value_or(vector()); DataView view = {.buffer = {}, .data = inData, .start = 0, .length = inData.size()}; const vector sign = signature.value_or(vector()); - appendBufferedData(view); if (!(bufferingMode_ == BufferingMode::EC_NO_DIGEST || bufferingMode_ == BufferingMode::RSA_NO_DIGEST)) { + appendBufferedData(view); if (view.length > MAX_CHUNK_SIZE) { auto err = updateInChunks(view, aToken, tToken, output); if (err != KM_ERROR_OK) { return km_utils::kmError2ScopedAStatus(err); } } + } else { + keymaster_error_t err = bufferData(view); + if (err != KM_ERROR_OK) { + return km_utils::kmError2ScopedAStatus(err); + } + appendBufferedData(view); } vector remaining = popNextChunk(view, view.length); return km_utils::kmError2ScopedAStatus(sendFinish(remaining, sign, aToken, tToken, confToken, *output)); From 70c1485240cdf8fd19e19d189931257e3facb01f Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Fri, 1 Apr 2022 17:24:13 +0000 Subject: [PATCH 5/5] added validation for application id and application data length --- .../android/javacard/keymaster/KMByteTag.java | 25 +++++++++++-------- .../javacard/keymaster/KMKeymasterApplet.java | 4 +++ 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMByteTag.java b/Applet/src/com/android/javacard/keymaster/KMByteTag.java index 1677a744..f896722d 100644 --- a/Applet/src/com/android/javacard/keymaster/KMByteTag.java +++ b/Applet/src/com/android/javacard/keymaster/KMByteTag.java @@ -30,6 +30,9 @@ public class KMByteTag extends KMTag { private static KMByteTag prototype; + // MAX ApplicationID or Application Data size + public static final short MAX_APP_ID_APP_DATA_SIZE = 64; + // The allowed tag keys of type bool tag private static final short[] tags = { APPLICATION_ID, @@ -76,15 +79,8 @@ public static short exp() { return ptr; } - public static short instance(short key) { - if (!validateKey(key)) { - ISOException.throwIt(ISO7816.SW_DATA_INVALID); - } - return instance(key, KMByteBlob.exp()); - } - public static short instance(short key, short byteBlob) { - if (!validateKey(key)) { + if (!validateKey(key, byteBlob)) { ISOException.throwIt(ISO7816.SW_DATA_INVALID); } if (heap[byteBlob] != BYTE_BLOB_TYPE) { @@ -124,13 +120,20 @@ public short length() { return KMByteBlob.cast(blobPtr).length(); } - private static boolean validateKey(short key) { + private static boolean validateKey(short key, short keyBlob) { + boolean result = false; short index = (short) tags.length; while (--index >= 0) { if (tags[index] == key) { - return true; + result = true; + if(key == APPLICATION_ID || key == APPLICATION_DATA) { + if (KMByteBlob.cast(keyBlob).length() > MAX_APP_ID_APP_DATA_SIZE) { + result = false; + } + } + break; } } - return false; + return result; } } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 1457bbf6..e9600777 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -689,6 +689,10 @@ private void processGetKeyCharacteristicsCmd(APDU apdu) { data[KEY_BLOB] = KMArray.cast(cmd).get((short) 0); data[APP_ID] = KMArray.cast(cmd).get((short) 1); data[APP_DATA] = KMArray.cast(cmd).get((short) 2); + if (KMByteBlob.cast(data[APP_ID]).length() > KMByteTag.MAX_APP_ID_APP_DATA_SIZE + || KMByteBlob.cast(data[APP_DATA]).length() > KMByteTag.MAX_APP_ID_APP_DATA_SIZE) { + ISOException.throwIt(ISO7816.SW_DATA_INVALID); + } if (!KMByteBlob.cast(data[APP_ID]).isValid()) { data[APP_ID] = KMType.INVALID_VALUE; }