diff --git a/Applet/src/com/android/javacard/keymaster/KMEncoder.java b/Applet/src/com/android/javacard/keymaster/KMEncoder.java index 0ba1986c..e47d799d 100644 --- a/Applet/src/com/android/javacard/keymaster/KMEncoder.java +++ b/Applet/src/com/android/javacard/keymaster/KMEncoder.java @@ -82,8 +82,8 @@ public short encode(short object, byte[] buffer, short startOff) { bufferRef[0] = buffer; scratchBuf[START_OFFSET] = startOff; short len = (short) (buffer.length - startOff); - if ((len < 0) || len > KMKeymasterApplet.MAX_LENGTH) { - scratchBuf[LEN_OFFSET] = KMKeymasterApplet.MAX_LENGTH; + if ((len < 0) || len > KMRepository.HEAP_SIZE) { + scratchBuf[LEN_OFFSET] = KMRepository.HEAP_SIZE; } else { scratchBuf[LEN_OFFSET] = (short) buffer.length; } diff --git a/Applet/src/com/android/javacard/keymaster/KMError.java b/Applet/src/com/android/javacard/keymaster/KMError.java index 52398824..5e93b49b 100644 --- a/Applet/src/com/android/javacard/keymaster/KMError.java +++ b/Applet/src/com/android/javacard/keymaster/KMError.java @@ -45,6 +45,7 @@ public class KMError { public static final short KEY_USER_NOT_AUTHENTICATED = 26; public static final short INVALID_OPERATION_HANDLE = 28; + public static final short INSUFFICIENT_BUFFER_SPACE = 29; public static final short VERIFICATION_FAILED = 30; public static final short TOO_MANY_OPERATIONS = 31; public static final short INVALID_KEY_BLOB = 33; diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index d413cc05..e78e7bc6 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -41,7 +41,6 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe public static final byte[] F4 = {0x01, 0x00, 0x01}; public static final byte AES_BLOCK_SIZE = 16; public static final byte DES_BLOCK_SIZE = 8; - public static final short MAX_LENGTH = 10000; public static final short MASTER_KEY_SIZE = 128; public static final short WRAPPING_KEY_SIZE = 32; public static final short MAX_OPERATIONS_COUNT = 4; @@ -106,8 +105,9 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe }; public static final short MAX_COSE_BUF_SIZE = (short) 1024; - // Maximum possible encoded keyparams size - public static final short MAX_KEY_PARAMS_BUF_SIZE = (short) 2048; + // Maximum allowed buffer size for to encode the key parameters + // which is used while creating mac for key paramters. + public static final short MAX_KEY_PARAMS_BUF_SIZE = (short) 3072; // 3K // Top 32 commands are reserved for provisioning. private static final byte KEYMINT_CMD_APDU_START = 0x20; @@ -3506,8 +3506,13 @@ private void processAttestKeyCmd(APDU apdu) { data[APP_DATA] = getApplicationData(data[KEY_PARAMETERS]); // Check if key requires upgrade. The KeyBlob is parsed inside isKeyUpgradeRequired // function itself. - parseEncryptedKeyBlob(data[KEY_BLOB], data[APP_ID], data[APP_DATA], scratchPad, - KEYBLOB_CURRENT_VERSION); + if (isKeyUpgradeRequired(data[KEY_BLOB], data[APP_ID], data[APP_DATA], scratchPad)) { + // This condition occurs if either any of the system properties (OsVersion, OsPatchLevel, + // VendorPatchLevel or BootPatchLevel) changes or KeyBlob format changed. So return + // KEY_REQUIRES_UPGRADE error as this scenario is application to ATTEST_KEY_BLOB as well + // as ATTEST_KEY_BLOB got generated before the KEY_BLOB. + KMException.throwIt(KMError.KEY_REQUIRES_UPGRADE); + } // Validate KeyParams Mac if (!validateKeyParamsMac(data[KEY_PARAMETERS], keyParamsMac, scratchPad)) { KMException.throwIt(KMError.INVALID_KEY_BLOB); @@ -3801,18 +3806,26 @@ private short macKeyParams(short keyParams, byte[] scratchPad) { if (SYM_KEY_TYPE == getKeyType(keyParams)) { return KMByteBlob.instance((short) 0); } - short len = encodeToApduBuffer(keyParams, scratchPad, (short) 0, MAX_KEY_PARAMS_BUF_SIZE); + short offset = repository.allocReclaimableMemory(MAX_KEY_PARAMS_BUF_SIZE); + short len = encoder.encode(keyParams, repository.getHeap(), offset); + if (len > MAX_KEY_PARAMS_BUF_SIZE) { + // KeyParamters exceeded the maximum allowed size. + KMException.throwIt(KMError.INSUFFICIENT_BUFFER_SPACE); + } short signLen = seProvider.hmacSign( KMByteBlob.cast(data[AUTH_TAG]).getBuffer(), KMByteBlob.cast(data[AUTH_TAG]).getStartOff(), KMByteBlob.cast(data[AUTH_TAG]).length(), - scratchPad, - (short) 0, + repository.getHeap(), + offset, len, scratchPad, - len); - return KMByteBlob.instance(scratchPad, len, signLen); + (short) 0); + //release memory + repository.reclaimMemory(MAX_KEY_PARAMS_BUF_SIZE); + + return KMByteBlob.instance(scratchPad, (short) 0, signLen); } private boolean validateKeyParamsMac(short keyParams, short keyParamsMac, byte[] scratchPad) { diff --git a/HAL/JavacardKeyMintDevice.cpp b/HAL/JavacardKeyMintDevice.cpp index 0cb1bec6..03a1b004 100644 --- a/HAL/JavacardKeyMintDevice.cpp +++ b/HAL/JavacardKeyMintDevice.cpp @@ -93,7 +93,7 @@ ScopedAStatus JavacardKeyMintDevice::generateKey(const vector& key // Call attestKey only Asymmetric algorithms. keymaster_algorithm_t algorithm; paramSet.GetTagValue(TAG_ALGORITHM, &algorithm); - if (algorithm == KM_ALGORITHM_RSA || algorithm == KM_ALGORITHM_EC) { + if (algorithm == KM_ALGORITHM_RSA || algorithm == KM_ALGORITHM_EC) { cppbor::Array attestKeyArray; attestKeyArray.add(creationResult->keyBlob); cbor_.addKeyparameters(attestKeyArray, keyParams); @@ -101,12 +101,12 @@ ScopedAStatus JavacardKeyMintDevice::generateKey(const vector& key attestKeyArray.add(keyParamsMac); auto [certItem, error] = card_->sendRequest(Instruction::INS_ATTEST_KEY_CMD, attestKeyArray); if (error != KM_ERROR_OK) { - LOG(ERROR) << "Failed in attestKey err: "; - return km_utils::kmError2ScopedAStatus(error); + LOG(ERROR) << "Failed in attestKey err: " << error; + return km_utils::kmError2ScopedAStatus(error); } if (!cbor_.getCertificateChain(certItem, 1, creationResult->certificateChain)) { - LOG(ERROR) << "Error in decoding og response in generateKey."; - return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); + LOG(ERROR) << "Error in decoding og response in generateKey."; + return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } } return ScopedAStatus::ok(); @@ -163,7 +163,7 @@ ScopedAStatus JavacardKeyMintDevice::importKey(const vector& keyPa attestKeyArray.add(keyParamsMac); auto [certItem, error] = card_->sendRequest(Instruction::INS_ATTEST_KEY_CMD, attestKeyArray); if (error != KM_ERROR_OK) { - LOG(ERROR) << "Failed in attestKey err: "; + LOG(ERROR) << "Failed in attestKey err: " << error; return km_utils::kmError2ScopedAStatus(error); } if (!cbor_.getCertificateChain(certItem, 1, creationResult->certificateChain)) { @@ -235,7 +235,7 @@ ScopedAStatus JavacardKeyMintDevice::importWrappedKey(const vector& wra cbor_.addAttestationKey(attestKeyArray, std::nullopt); auto [certItem, error] = card_->sendRequest(Instruction::INS_ATTEST_KEY_CMD, attestKeyArray); if (error != KM_ERROR_OK) { - LOG(ERROR) << "Failed in attestKey err: "; + LOG(ERROR) << "Failed in attestKey err: " << error; return km_utils::kmError2ScopedAStatus(error); } if (!cbor_.getCertificateChain(certItem, 1, creationResult->certificateChain)) {