From 7e0f20611586022aab2a242211374e663cc9d2f6 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Tue, 8 Dec 2020 21:51:41 +0000 Subject: [PATCH 01/24] Fixed the VTS 4.1 EarlyBootEnded usecase with DeleteKey --- .../javacard/keymaster/KMKeymasterApplet.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 43ada045..c0a55751 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -912,12 +912,16 @@ private void processDeleteKeyCmd(APDU apdu) { tmpVariables[2] = KMKeyCharacteristics.exp(); KMArray.cast(tmpVariables[1]).add(KMKeymasterApplet.KEY_BLOB_KEYCHAR, tmpVariables[2]); KMArray.cast(tmpVariables[1]).add(KMKeymasterApplet.KEY_BLOB_PUB_KEY, KMByteBlob.exp()); - data[KEY_BLOB] = - decoder.decodeArray( - tmpVariables[1], - KMByteBlob.cast(data[KEY_BLOB]).getBuffer(), - KMByteBlob.cast(data[KEY_BLOB]).getStartOff(), - KMByteBlob.cast(data[KEY_BLOB]).length()); + try { + data[KEY_BLOB] = decoder.decodeArray(tmpVariables[1], + 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); + } tmpVariables[0] = KMArray.cast(data[KEY_BLOB]).length(); if (tmpVariables[0] < 4) { KMException.throwIt(KMError.INVALID_KEY_BLOB); From f678ad2f70807f83a280d7f7ab6b8a7c9adee2ea Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 9 Dec 2020 23:04:04 +0000 Subject: [PATCH 02/24] Clear the certificate chain persisted in memory whenever provisionCertificateChain command gets called multiple times. --- .../android/javacard/keymaster/KMAndroidSEProvider.java | 7 +++++++ .../com/android/javacard/keymaster/KMJCardSimulator.java | 7 +++++++ .../com/android/javacard/keymaster/KMKeymasterApplet.java | 5 +++++ .../src/com/android/javacard/keymaster/KMSEProvider.java | 5 +++++ 4 files changed, 24 insertions(+) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java index 9d9812c1..aba38934 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java @@ -1137,6 +1137,13 @@ public short cmacKdf(byte[] keyMaterial, short keyMaterialStart, return key.getKey(keyBuf, keyStart); } + @Override + public void clearCertificateChain() { + JCSystem.beginTransaction(); + Util.arrayFillNonAtomic(certificateChain, (short)0, CERT_CHAIN_MAX_SIZE, (byte) 0); + JCSystem.commitTransaction(); + } + //This function supports multi-part request data. @Override public void persistPartialCertificateChain(byte[] buf, short offset, short len, short totalLen) { diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java index 70311e51..9c9e71c6 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java @@ -1252,6 +1252,13 @@ public short ecSign256(byte[] secret, short secretStart, short secretLength, outputDataBuf, outputDataStart); } + @Override + public void clearCertificateChain() { + JCSystem.beginTransaction(); + Util.arrayFillNonAtomic(certificateChain, (short)0, CERT_CHAIN_MAX_SIZE, (byte) 0); + JCSystem.commitTransaction(); + } + @Override public void persistPartialCertificateChain(byte[] buf, short offset, short len, short totalLen) { diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index c0a55751..d9691a1e 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -644,6 +644,11 @@ private void processProvisionAttestationCertParams(APDU apdu) { } private void processProvisionAttestationCertChainCmd(APDU apdu) { + tmpVariables[0] = seProvider.getCertificateChainLength(); + if (tmpVariables[0] != 0) { + //Clear the previous certificate chain. + seProvider.clearCertificateChain(); + } byte[] srcBuffer = apdu.getBuffer(); short recvLen = apdu.setIncomingAndReceive(); short srcOffset = apdu.getOffsetCdata(); diff --git a/Applet/src/com/android/javacard/keymaster/KMSEProvider.java b/Applet/src/com/android/javacard/keymaster/KMSEProvider.java index b3e9c266..81b35db2 100644 --- a/Applet/src/com/android/javacard/keymaster/KMSEProvider.java +++ b/Applet/src/com/android/javacard/keymaster/KMSEProvider.java @@ -441,6 +441,11 @@ KMOperation initAsymmetricOperation( */ public void persistPartialCertificateChain(byte[] buf, short offset, short len, short totalLen); + /** + * This operation clears the certificate chain from persistent memory. + */ + public void clearCertificateChain(); + /** * The operation reads the certificate chain from persistent memory. * From a816c1915bd10ab6907fda0f95c0d4749748d120 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Tue, 12 Jan 2021 22:42:34 +0530 Subject: [PATCH 03/24] HAL fixes shared by NXP team. --- .../4.1/JavacardKeymaster4Device.cpp | 104 +++++++++--------- .../4.1/JavacardOperationContext.cpp | 17 ++- HAL/keymaster/Android.bp | 5 + 3 files changed, 73 insertions(+), 53 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 23583024..71350e9b 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -304,14 +304,10 @@ Return JavacardKeymaster4Device::getHardwareInfo(getHardwareInfo_cb _hidl_ hidl_string jcKeymasterAuthor; ErrorCode ret = sendData(Instruction::INS_GET_HW_INFO_CMD, input, resp); - if(ret != ErrorCode::OK) { - //Socket not connected. - _hidl_cb(SecurityLevel::STRONGBOX, JAVACARD_KEYMASTER_NAME, JAVACARD_KEYMASTER_AUTHOR); - return Void(); - } else { + if (ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, ret) = cborConverter_.decodeData(std::vector(resp.begin(), resp.end()-2), - true); + false); if (item != nullptr) { std::vector temp; if(!cborConverter_.getUint64(item, 0, securityLevel) || @@ -324,29 +320,23 @@ Return JavacardKeymaster4Device::getHardwareInfo(getHardwareInfo_cb _hidl_ _hidl_cb(static_cast(securityLevel), jcKeymasterName, jcKeymasterAuthor); return Void(); } +#ifdef TRANSCEIVE_VIA_SOCKET + else { + //TODO Should we throw fatal error here.? + _hidl_cb(SecurityLevel::STRONGBOX, JAVACARD_KEYMASTER_NAME, JAVACARD_KEYMASTER_AUTHOR); + return Void(); + } +#endif } Return JavacardKeymaster4Device::getHmacSharingParameters(getHmacSharingParameters_cb _hidl_cb) { - /* TODO temporary fix: vold daemon calls performHmacKeyAgreement. At that time when vold calls this API there is no - * network connectivity and socket cannot be connected. So as a hack we are calling softkeymaster to getHmacSharing - * parameters. - */ std::vector cborData; std::vector input; std::unique_ptr item; HmacSharingParameters hmacSharingParameters; ErrorCode errorCode = ErrorCode::UNKNOWN_ERROR; errorCode = sendData(Instruction::INS_GET_HMAC_SHARING_PARAM_CMD, input, cborData); - if(errorCode != ErrorCode::OK) { - auto response = softKm_->GetHmacSharingParameters(); - ::android::hardware::keymaster::V4_0::HmacSharingParameters params; - params.seed.setToExternal(const_cast(response.params.seed.data), - response.params.seed.data_length); - static_assert(sizeof(response.params.nonce) == params.nonce.size(), "Nonce sizes don't match"); - memcpy(params.nonce.data(), response.params.nonce, params.nonce.size()); - _hidl_cb(legacy_enum_conversion(response.error), params); - return Void(); - } else { + if (ErrorCode::OK == errorCode) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborData.begin(), cborData.end()-2), true); @@ -355,16 +345,26 @@ Return JavacardKeymaster4Device::getHmacSharingParameters(getHmacSharingPa errorCode = ErrorCode::UNKNOWN_ERROR; } } - _hidl_cb(errorCode, hmacSharingParameters); - return Void(); } +#ifdef TRANSCEIVE_VIA_SOCKET + /* TODO temporary fix: vold daemon calls performHmacKeyAgreement. At that time when vold calls this API there is no + * network connectivity and socket cannot be connected. So as a hack we are calling softkeymaster to getHmacSharing + * parameters. + */ + else { + auto response = softKm_->GetHmacSharingParameters(); + hmacSharingParameters.seed.setToExternal(const_cast(response.params.seed.data), + response.params.seed.data_length); + static_assert(sizeof(response.params.nonce) == hmacSharingParameters.nonce.size(), "Nonce sizes don't match"); + memcpy(hmacSharingParameters.nonce.data(), response.params.nonce, hmacSharingParameters.nonce.size()); + errorCode = legacy_enum_conversion(response.error); + } +#endif + _hidl_cb(errorCode, hmacSharingParameters); + return Void(); } Return JavacardKeymaster4Device::computeSharedHmac(const hidl_vec& params, computeSharedHmac_cb _hidl_cb) { - /* TODO temporary fix: vold daemon calls performHmacKeyAgreement. At that time when vold calls this API there is no - * network connectivity and socket cannot be connected. So as a hack we are calling softkeymaster to - * computeSharedHmac. - */ cppbor::Array array; std::unique_ptr item; std::vector cborOutData; @@ -387,7 +387,25 @@ Return JavacardKeymaster4Device::computeSharedHmac(const hidl_vec cborData = array.encode(); errorCode = sendData(Instruction::INS_COMPUTE_SHARED_HMAC_CMD, cborData, cborOutData); - if(errorCode != ErrorCode::OK) { + if (ErrorCode::OK == errorCode) { + //Skip last 2 bytes in cborData, it contains status. + std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + true); + if (item != nullptr) { + std::vector bstr; + if(!cborConverter_.getBinaryArray(item, 1, bstr)) { + errorCode = ErrorCode::UNKNOWN_ERROR; + } else { + sharingCheck = bstr; + } + } + } +#ifdef TRANSCEIVE_VIA_SOCKET + /* TODO temporary fix: vold daemon calls performHmacKeyAgreement. At that time when vold calls this API there is no + * network connectivity and socket cannot be connected. So as a hack we are calling softkeymaster to + * computeSharedHmac. + */ + else { ComputeSharedHmacRequest request; request.params_array.params_array = new keymaster::HmacSharingParameters[params.size()]; request.params_array.num_params = params.size(); @@ -401,29 +419,13 @@ Return JavacardKeymaster4Device::computeSharedHmac(const hidl_vecComputeSharedHmac(request); - hidl_vec sharing_check; - if (response.error == KM_ERROR_OK) sharing_check = kmBlob2hidlVec(response.sharing_check); - - _hidl_cb(legacy_enum_conversion(response.error), sharing_check); - return Void(); - - } else { - //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), - true); - if (item != nullptr) { - std::vector bstr; - if(!cborConverter_.getBinaryArray(item, 1, bstr)) { - errorCode = ErrorCode::UNKNOWN_ERROR; - } else { - sharingCheck = bstr; - } - } - _hidl_cb(errorCode, sharingCheck); - return Void(); + if (response.error == KM_ERROR_OK) sharingCheck = kmBlob2hidlVec(response.sharing_check); + errorCode = legacy_enum_conversion(response.error); } - -} +#endif + _hidl_cb(errorCode, sharingCheck); + return Void(); + } Return JavacardKeymaster4Device::verifyAuthorization(uint64_t , const hidl_vec& , const HardwareAuthToken& , verifyAuthorization_cb _hidl_cb) { VerificationToken verificationToken; @@ -1061,8 +1063,10 @@ Return JavacardKeymaster4Device::finish(uint64_t operationHandle, const hi sendDataCallback))) { output = tempOut; } + if (ErrorCode::OK != errorCode) { + abort(operationHandle); + } } - abort(operationHandle); _hidl_cb(errorCode, outParams, output); return Void(); } diff --git a/HAL/keymaster/4.1/JavacardOperationContext.cpp b/HAL/keymaster/4.1/JavacardOperationContext.cpp index 457a6d87..1872f75b 100644 --- a/HAL/keymaster/4.1/JavacardOperationContext.cpp +++ b/HAL/keymaster/4.1/JavacardOperationContext.cpp @@ -183,9 +183,18 @@ ErrorCode OperationContext::finish(uint64_t operHandle, const std::vector newInput(first, end); - if(ErrorCode::OK != (errorCode = handleInternalUpdate(operHandle, newInput.data(), newInput.size(), - Operation::Update, cb))) { - return errorCode; + if(extraData == 0 && (i == noOfChunks - 1)) { + //Last chunk + if(ErrorCode::OK != (errorCode = handleInternalUpdate(operHandle, newInput.data(), newInput.size(), + Operation::Finish, cb, true))) { + return errorCode; + } + + } else { + if(ErrorCode::OK != (errorCode = handleInternalUpdate(operHandle, newInput.data(), newInput.size(), + Operation::Update, cb))) { + return errorCode; + } } } if(extraData > 0) { @@ -217,6 +226,8 @@ ErrorCode OperationContext::getBlockAlignedData(uint64_t operHandle, uint8_t* in blockSize = AES_BLOCK_SIZE; } else if(Algorithm::TRIPLE_DES == operationTable[operHandle].info.alg) { blockSize = DES_BLOCK_SIZE; + } else { + return ErrorCode::INCOMPATIBLE_ALGORITHM; } if(opr == Operation::Finish) { diff --git a/HAL/keymaster/Android.bp b/HAL/keymaster/Android.bp index 8b34023d..eb4fa49c 100644 --- a/HAL/keymaster/Android.bp +++ b/HAL/keymaster/Android.bp @@ -49,6 +49,11 @@ cc_binary { "libjc_transport", "libcrypto", ], + product_variables: { + debuggable: { + cflags: ["-DTRANSCEIVE_VIA_SOCKET"] + } + } } cc_library { From 8a36d0282090084a04006ed62cfe5454d21df03a Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 13 Jan 2021 23:14:09 +0530 Subject: [PATCH 04/24] Create operation handle map in HAL to differentiate between public and strongbox operations in update, finish and abort operations. --- .../4.1/JavacardKeymaster4Device.cpp | 169 ++++++++++++++---- 1 file changed, 134 insertions(+), 35 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 71350e9b..ab8ee0ed 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -49,6 +49,9 @@ #define INS_BEGIN_KM_CMD 0x00 #define INS_END_KM_PROVISION_CMD 0x20 #define INS_END_KM_CMD 0x7F +#define MAX_COUNTER_VALUE 25 +#define SW_KM_OPR 0UL +#define SB_KM_OPR 1UL namespace keymaster { namespace V4_1 { @@ -56,6 +59,7 @@ namespace javacard { static std::unique_ptr pTransportFactory = nullptr; constexpr size_t kOperationTableSize = 4; +std::map operationTable; struct KM_AUTH_LIST_Delete { void operator()(KM_AUTH_LIST* p) { KM_AUTH_LIST_free(p); } @@ -116,6 +120,63 @@ static inline bool getTag(const hidl_vec& params, Tag tag, KeyPara return false; } +static ErrorCode generateOperationHandle(uint64_t& oprHandle) { + std::map::iterator it; + //Check if oprHandleCnt is present in operationTable + uint64_t cnt = 1; + uint64_t mask; + for(; cnt < MAX_COUNTER_VALUE; cnt++) { + mask = cnt | (SB_KM_OPR << 56); + it = operationTable.find(mask); + if (it == operationTable.end()) { + //NO HW Operation found with this operation handle. + //Check for SW. + mask = cnt | (SW_KM_OPR << 56); + it = operationTable.find(mask); + if (it != operationTable.end()) + continue; + else + break; + } else { + continue; + } + } + if (cnt == MAX_COUNTER_VALUE) { + LOG(ERROR) << "generateOperationHandle TOO_MANY_OPERATIONS"; + return ErrorCode::TOO_MANY_OPERATIONS; + } + oprHandle = cnt; + return ErrorCode::OK; +} + +static ErrorCode createOprHandleEntry(uint64_t origOprHandle, uint64_t mask/* SW or HW */, uint64_t& generatedOprHandle) { + ErrorCode errorCode = ErrorCode::OK; + if (ErrorCode::OK != (errorCode = generateOperationHandle(generatedOprHandle))) { + return errorCode; + } + //mask the operationhandle + generatedOprHandle |= (mask << 56); + operationTable[generatedOprHandle] = origOprHandle; + return errorCode; +} + +static ErrorCode getOrigOperationHandle(uint64_t generatedOprHandle, uint64_t& origOprHandle) { + std::map::iterator it = operationTable.find(generatedOprHandle); + if (it == operationTable.end()) { + return ErrorCode::INVALID_OPERATION_HANDLE; + } + origOprHandle = it->second; + return ErrorCode::OK; +} + +static bool isStrongboxOperation(uint64_t generatedOprHandle) { + return (SB_KM_OPR == (generatedOprHandle >> 56)); +} + +static void deleteOprHandleEntry(uint64_t generatedOprHandle) { + operationTable.erase(generatedOprHandle); +} + ErrorCode encodeParametersVerified(const VerificationToken& verificationToken, std::vector& asn1ParamsVerified) { if (verificationToken.parametersVerified.size() > 0) { AuthorizationSet paramSet; @@ -793,12 +854,13 @@ Return JavacardKeymaster4Device::begin(KeyPurpose purpose, const hidl_vec< hidl_vec outParams; uint64_t operationHandle = 0; hidl_vec resultParams; + uint64_t generatedOpHandle = 0; if(keyBlob.size() == 0) { _hidl_cb(ErrorCode::INVALID_ARGUMENT, resultParams, operationHandle); return Void(); } - + /* Asymmetric public key operations are handled by softkeymaster. */ if (KeyPurpose::ENCRYPT == purpose || KeyPurpose::VERIFY == purpose) { BeginOperationRequest request; request.purpose = legacy_enum_conversion(purpose); @@ -806,13 +868,17 @@ Return JavacardKeymaster4Device::begin(KeyPurpose purpose, const hidl_vec< request.additional_params.Reinitialize(KmParamSet(inParams)); BeginOperationResponse response; + /* For Symmetric key operation, the BeginOperation returns KM_ERROR_INCOMPATIBLE_ALGORITHM error. */ softKm_->BeginOperation(request, &response); if (response.error == KM_ERROR_OK) { resultParams = kmParamSet2Hidl(response.output_params); } if (response.error != KM_ERROR_INCOMPATIBLE_ALGORITHM) { /*Incompatible algorithm could be handled by JavaCard*/ - _hidl_cb(legacy_enum_conversion(response.error), resultParams, response.op_handle); + errorCode = legacy_enum_conversion(response.error); + if (errorCode == ErrorCode::OK) + errorCode = createOprHandleEntry(response.op_handle, SW_KM_OPR, generatedOpHandle); + _hidl_cb(errorCode, resultParams, generatedOpHandle); return Void(); } } @@ -871,29 +937,40 @@ Return JavacardKeymaster4Device::begin(KeyPurpose purpose, const hidl_vec< } } } - _hidl_cb(errorCode, outParams, operationHandle); + if (ErrorCode::OK == errorCode) + errorCode = createOprHandleEntry(operationHandle, SB_KM_OPR, generatedOpHandle); + _hidl_cb(errorCode, outParams, generatedOpHandle); return Void(); } -Return JavacardKeymaster4Device::update(uint64_t operationHandle, const hidl_vec& inParams, const hidl_vec& input, const HardwareAuthToken& authToken, const VerificationToken& verificationToken, update_cb _hidl_cb) { +Return JavacardKeymaster4Device::update(uint64_t halGeneratedOprHandle, const hidl_vec& inParams, const hidl_vec& input, const HardwareAuthToken& authToken, const VerificationToken& verificationToken, update_cb _hidl_cb) { ErrorCode errorCode = ErrorCode::UNKNOWN_ERROR; - UpdateOperationRequest request; - request.op_handle = operationHandle; - request.input.Reinitialize(input.data(), input.size()); - request.additional_params.Reinitialize(KmParamSet(inParams)); - - UpdateOperationResponse response; - softKm_->UpdateOperation(request, &response); - uint32_t inputConsumed = 0; hidl_vec outParams; hidl_vec output; - errorCode = legacy_enum_conversion(response.error); - if (response.error == KM_ERROR_OK) { - inputConsumed = response.input_consumed; - outParams = kmParamSet2Hidl(response.output_params); - output = kmBuffer2hidlVec(response.output); - } else if(response.error == KM_ERROR_INVALID_OPERATION_HANDLE) { + uint64_t operationHandle; + UpdateOperationResponse response; + if (ErrorCode::OK != (errorCode = getOrigOperationHandle(halGeneratedOprHandle, operationHandle))) { + _hidl_cb(errorCode, inputConsumed, outParams, output); + return Void(); + } + + if (!isStrongboxOperation(halGeneratedOprHandle)) { + /* SW keymaster (Public key operation) */ + UpdateOperationRequest request; + request.op_handle = operationHandle; + request.input.Reinitialize(input.data(), input.size()); + request.additional_params.Reinitialize(KmParamSet(inParams)); + + softKm_->UpdateOperation(request, &response); + errorCode = legacy_enum_conversion(response.error); + if (response.error == KM_ERROR_OK) { + inputConsumed = response.input_consumed; + outParams = kmParamSet2Hidl(response.output_params); + output = kmBuffer2hidlVec(response.output); + } + } else { + /* Strongbox Keymaster operation */ std::vector tempOut; /* OperationContext calls this below sendDataCallback callback function. This callback * may be called multiple times if the input data is larger than MAX_ALLOWED_INPUT_SIZE. @@ -954,32 +1031,48 @@ Return JavacardKeymaster4Device::update(uint64_t operationHandle, const hi inputConsumed = input.size(); output = tempOut; } + if(ErrorCode::OK != errorCode) { + abort(operationHandle); + } } if(ErrorCode::OK != errorCode) { - abort(operationHandle); + deleteOprHandleEntry(halGeneratedOprHandle); } + _hidl_cb(errorCode, inputConsumed, outParams, output); return Void(); } -Return JavacardKeymaster4Device::finish(uint64_t operationHandle, const hidl_vec& inParams, const hidl_vec& input, const hidl_vec& signature, const HardwareAuthToken& authToken, const VerificationToken& verificationToken, finish_cb _hidl_cb) { +Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, const hidl_vec& inParams, const hidl_vec& input, const hidl_vec& signature, const HardwareAuthToken& authToken, const VerificationToken& verificationToken, finish_cb _hidl_cb) { ErrorCode errorCode = ErrorCode::UNKNOWN_ERROR; - FinishOperationRequest request; - request.op_handle = operationHandle; - request.input.Reinitialize(input.data(), input.size()); - request.signature.Reinitialize(signature.data(), signature.size()); - request.additional_params.Reinitialize(KmParamSet(inParams)); - - FinishOperationResponse response; - softKm_->FinishOperation(request, &response); - + uint64_t operationHandle; hidl_vec outParams; hidl_vec output; - errorCode = legacy_enum_conversion(response.error); - if (response.error == KM_ERROR_OK) { - outParams = kmParamSet2Hidl(response.output_params); - output = kmBuffer2hidlVec(response.output); - } else if (response.error == KM_ERROR_INVALID_OPERATION_HANDLE) { + FinishOperationResponse response; + + if (ErrorCode::OK != (errorCode = getOrigOperationHandle(halGeneratedOprHandle, operationHandle))) { + _hidl_cb(errorCode, outParams, output); + return Void(); + } + + if (!isStrongboxOperation(halGeneratedOprHandle)) { + /* SW keymaster (Public key operation) */ + FinishOperationRequest request; + request.op_handle = operationHandle; + request.input.Reinitialize(input.data(), input.size()); + request.signature.Reinitialize(signature.data(), signature.size()); + request.additional_params.Reinitialize(KmParamSet(inParams)); + + //FinishOperationResponse response; + softKm_->FinishOperation(request, &response); + + errorCode = legacy_enum_conversion(response.error); + if (response.error == KM_ERROR_OK) { + outParams = kmParamSet2Hidl(response.output_params); + output = kmBuffer2hidlVec(response.output); + } + } else { + /* Strongbox Keymaster operation */ std::vector tempOut; bool aadTag = false; /* OperationContext calls this below sendDataCallback callback function. This callback @@ -1067,12 +1160,17 @@ Return JavacardKeymaster4Device::finish(uint64_t operationHandle, const hi abort(operationHandle); } } + deleteOprHandleEntry(halGeneratedOprHandle); _hidl_cb(errorCode, outParams, output); return Void(); } -Return JavacardKeymaster4Device::abort(uint64_t operationHandle) { +Return JavacardKeymaster4Device::abort(uint64_t halGeneratedOprHandle) { ErrorCode errorCode = ErrorCode::UNKNOWN_ERROR; + uint64_t operationHandle; + if (ErrorCode::OK != (errorCode = getOrigOperationHandle(halGeneratedOprHandle, operationHandle))) { + return errorCode; + } AbortOperationRequest request; request.op_handle = operationHandle; @@ -1099,6 +1197,7 @@ Return JavacardKeymaster4Device::abort(uint64_t operationHandle) { } /* Delete the entry on this operationHandle */ oprCtx_->clearOperationData(operationHandle); + deleteOprHandleEntry(halGeneratedOprHandle); return errorCode; } From 39357b51c3a168e970f3fd62a82d9d157feb3ca8 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Thu, 14 Jan 2021 00:44:08 +0530 Subject: [PATCH 05/24] Pass hal generated operation handle to abort() call from update and finish. --- HAL/keymaster/4.1/JavacardKeymaster4Device.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index ab8ee0ed..cbcbdfb7 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -142,7 +142,7 @@ static ErrorCode generateOperationHandle(uint64_t& oprHandle) { } } if (cnt == MAX_COUNTER_VALUE) { - LOG(ERROR) << "generateOperationHandle TOO_MANY_OPERATIONS"; + LOG(ERROR) << "generateOperationHandle TOO_MANY_OPERATIONS error."; return ErrorCode::TOO_MANY_OPERATIONS; } oprHandle = cnt; @@ -1032,7 +1032,7 @@ Return JavacardKeymaster4Device::update(uint64_t halGeneratedOprHandle, co output = tempOut; } if(ErrorCode::OK != errorCode) { - abort(operationHandle); + abort(halGeneratedOprHandle); } } if(ErrorCode::OK != errorCode) { @@ -1129,7 +1129,6 @@ Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, co cborConverter_.addHardwareAuthToken(array, authToken); cborConverter_.addVerificationToken(array, verificationToken, asn1ParamsVerified); std::vector cborData = array.encode(); - errorCode = sendData(ins, cborData, cborOutData); if(errorCode == ErrorCode::OK) { @@ -1157,7 +1156,7 @@ Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, co output = tempOut; } if (ErrorCode::OK != errorCode) { - abort(operationHandle); + abort(halGeneratedOprHandle); } } deleteOprHandleEntry(halGeneratedOprHandle); From 5c630f2a70d2349754a63bc2f66d30c7213f9f76 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Thu, 14 Jan 2021 12:11:49 +0530 Subject: [PATCH 06/24] Added comments for the newly added code. --- .../4.1/JavacardKeymaster4Device.cpp | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index cbcbdfb7..f36fcc67 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -120,17 +120,18 @@ static inline bool getTag(const hidl_vec& params, Tag tag, KeyPara return false; } +/* Generate new operation handle */ static ErrorCode generateOperationHandle(uint64_t& oprHandle) { std::map::iterator it; - //Check if oprHandleCnt is present in operationTable + //Check if operationHandle is present in operationTable uint64_t cnt = 1; uint64_t mask; for(; cnt < MAX_COUNTER_VALUE; cnt++) { mask = cnt | (SB_KM_OPR << 56); it = operationTable.find(mask); if (it == operationTable.end()) { - //NO HW Operation found with this operation handle. - //Check for SW. + //No Strongbox Operation found with this operation handle. + //Check for Software operation. mask = cnt | (SW_KM_OPR << 56); it = operationTable.find(mask); if (it != operationTable.end()) @@ -142,26 +143,28 @@ static ErrorCode generateOperationHandle(uint64_t& oprHandle) { } } if (cnt == MAX_COUNTER_VALUE) { - LOG(ERROR) << "generateOperationHandle TOO_MANY_OPERATIONS error."; + LOG(ERROR) << "operation handle count reached max operations"; return ErrorCode::TOO_MANY_OPERATIONS; } oprHandle = cnt; return ErrorCode::OK; } -static ErrorCode createOprHandleEntry(uint64_t origOprHandle, uint64_t mask/* SW or HW */, uint64_t& generatedOprHandle) { +/* Create a new operation handle entry in operation table.*/ +static ErrorCode createOprHandleEntry(uint64_t origOprHandle, uint64_t mask/* SW or HW */, uint64_t& newOperationHandle) { ErrorCode errorCode = ErrorCode::OK; - if (ErrorCode::OK != (errorCode = generateOperationHandle(generatedOprHandle))) { + if (ErrorCode::OK != (errorCode = generateOperationHandle(newOperationHandle))) { return errorCode; } //mask the operationhandle - generatedOprHandle |= (mask << 56); - operationTable[generatedOprHandle] = origOprHandle; + newOperationHandle |= (mask << 56); + operationTable[newOperationHandle] = origOprHandle; return errorCode; } -static ErrorCode getOrigOperationHandle(uint64_t generatedOprHandle, uint64_t& origOprHandle) { - std::map::iterator it = operationTable.find(generatedOprHandle); +/* Get original operation handle generated by softkeymaster/strongboxkeymaster. */ +static ErrorCode getOrigOperationHandle(uint64_t halGeneratedOperationHandle, uint64_t& origOprHandle) { + std::map::iterator it = operationTable.find(halGeneratedOperationHandle); if (it == operationTable.end()) { return ErrorCode::INVALID_OPERATION_HANDLE; } @@ -169,12 +172,14 @@ static ErrorCode getOrigOperationHandle(uint64_t generatedOprHandle, uint64_t& o return ErrorCode::OK; } -static bool isStrongboxOperation(uint64_t generatedOprHandle) { - return (SB_KM_OPR == (generatedOprHandle >> 56)); +/* Tells if the operation handle belongs to strongbox keymaster. */ +static bool isStrongboxOperation(uint64_t halGeneratedOperationHandle) { + return (SB_KM_OPR == (halGeneratedOperationHandle >> 56)); } -static void deleteOprHandleEntry(uint64_t generatedOprHandle) { - operationTable.erase(generatedOprHandle); +/* Delete the operation handle entry from operation table. */ +static void deleteOprHandleEntry(uint64_t halGeneratedOperationHandle) { + operationTable.erase(halGeneratedOperationHandle); } ErrorCode encodeParametersVerified(const VerificationToken& verificationToken, std::vector& asn1ParamsVerified) { @@ -876,6 +881,10 @@ Return JavacardKeymaster4Device::begin(KeyPurpose purpose, const hidl_vec< } if (response.error != KM_ERROR_INCOMPATIBLE_ALGORITHM) { /*Incompatible algorithm could be handled by JavaCard*/ errorCode = legacy_enum_conversion(response.error); + /* Create a new operation handle and add a entry inside the operation table map with + * key - new operation handle + * value - hal generated operation handle. + */ if (errorCode == ErrorCode::OK) errorCode = createOprHandleEntry(response.op_handle, SW_KM_OPR, generatedOpHandle); _hidl_cb(errorCode, resultParams, generatedOpHandle); @@ -937,6 +946,10 @@ Return JavacardKeymaster4Device::begin(KeyPurpose purpose, const hidl_vec< } } } + /* Create a new operation handle and add a entry inside the operation table map with + * key - new operation handle + * value - hal generated operation handle. + */ if (ErrorCode::OK == errorCode) errorCode = createOprHandleEntry(operationHandle, SB_KM_OPR, generatedOpHandle); _hidl_cb(errorCode, outParams, generatedOpHandle); @@ -1036,6 +1049,7 @@ Return JavacardKeymaster4Device::update(uint64_t halGeneratedOprHandle, co } } if(ErrorCode::OK != errorCode) { + /* Delete the entry from operation table. */ deleteOprHandleEntry(halGeneratedOprHandle); } @@ -1159,6 +1173,7 @@ Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, co abort(halGeneratedOprHandle); } } + /* Delete the entry from operation table. */ deleteOprHandleEntry(halGeneratedOprHandle); _hidl_cb(errorCode, outParams, output); return Void(); From ed9c07af66ba07c09c3612f966d06fbb4c90b571 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Fri, 15 Jan 2021 20:44:40 +0530 Subject: [PATCH 07/24] Used Rand_bytes for hal generated operation handle. Updated comments --- .../4.1/JavacardKeymaster4Device.cpp | 70 ++++++++----------- .../4.1/JavacardOperationContext.cpp | 3 +- HAL/keymaster/Android.bp | 4 +- 3 files changed, 34 insertions(+), 43 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index f36fcc67..5ac1a431 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -49,7 +50,6 @@ #define INS_BEGIN_KM_CMD 0x00 #define INS_END_KM_PROVISION_CMD 0x20 #define INS_END_KM_CMD 0x7F -#define MAX_COUNTER_VALUE 25 #define SW_KM_OPR 0UL #define SB_KM_OPR 1UL @@ -59,7 +59,10 @@ namespace javacard { static std::unique_ptr pTransportFactory = nullptr; constexpr size_t kOperationTableSize = 4; -std::map operationTable; +/* Key is the newly generated operation handle. Value is a pair with first element having + * original operation handle and second element represents SW or SB operation. + */ +std::map> operationTable; struct KM_AUTH_LIST_Delete { void operator()(KM_AUTH_LIST* p) { KM_AUTH_LIST_free(p); } @@ -122,59 +125,44 @@ static inline bool getTag(const hidl_vec& params, Tag tag, KeyPara /* Generate new operation handle */ static ErrorCode generateOperationHandle(uint64_t& oprHandle) { - std::map::iterator it; - //Check if operationHandle is present in operationTable - uint64_t cnt = 1; - uint64_t mask; - for(; cnt < MAX_COUNTER_VALUE; cnt++) { - mask = cnt | (SB_KM_OPR << 56); - it = operationTable.find(mask); - if (it == operationTable.end()) { - //No Strongbox Operation found with this operation handle. - //Check for Software operation. - mask = cnt | (SW_KM_OPR << 56); - it = operationTable.find(mask); - if (it != operationTable.end()) - continue; - else - break; - } else { - continue; + std::map>::iterator it; + do { + keymaster_error_t err = GenerateRandom(reinterpret_cast(&oprHandle), (size_t)sizeof(oprHandle)); + if (err != KM_ERROR_OK) { + return legacy_enum_conversion(err); } - } - if (cnt == MAX_COUNTER_VALUE) { - LOG(ERROR) << "operation handle count reached max operations"; - return ErrorCode::TOO_MANY_OPERATIONS; - } - oprHandle = cnt; + it = operationTable.find(oprHandle); + } while (it != operationTable.end()); return ErrorCode::OK; } /* Create a new operation handle entry in operation table.*/ -static ErrorCode createOprHandleEntry(uint64_t origOprHandle, uint64_t mask/* SW or HW */, uint64_t& newOperationHandle) { +static ErrorCode createOprHandleEntry(uint64_t origOprHandle, uint64_t keymasterSrc, uint64_t& newOperationHandle) { ErrorCode errorCode = ErrorCode::OK; if (ErrorCode::OK != (errorCode = generateOperationHandle(newOperationHandle))) { return errorCode; } - //mask the operationhandle - newOperationHandle |= (mask << 56); - operationTable[newOperationHandle] = origOprHandle; + operationTable[newOperationHandle] = std::make_pair(origOprHandle, keymasterSrc); return errorCode; } /* Get original operation handle generated by softkeymaster/strongboxkeymaster. */ static ErrorCode getOrigOperationHandle(uint64_t halGeneratedOperationHandle, uint64_t& origOprHandle) { - std::map::iterator it = operationTable.find(halGeneratedOperationHandle); + std::map>::iterator it = operationTable.find(halGeneratedOperationHandle); if (it == operationTable.end()) { return ErrorCode::INVALID_OPERATION_HANDLE; } - origOprHandle = it->second; + origOprHandle = it->second.first; return ErrorCode::OK; } /* Tells if the operation handle belongs to strongbox keymaster. */ static bool isStrongboxOperation(uint64_t halGeneratedOperationHandle) { - return (SB_KM_OPR == (halGeneratedOperationHandle >> 56)); + std::map>::iterator it = operationTable.find(halGeneratedOperationHandle); + if (it == operationTable.end()) { + return false; + } + return (SB_KM_OPR == it->second.second); } /* Delete the operation handle entry from operation table. */ @@ -385,14 +373,12 @@ Return JavacardKeymaster4Device::getHardwareInfo(getHardwareInfo_cb _hidl_ } _hidl_cb(static_cast(securityLevel), jcKeymasterName, jcKeymasterAuthor); return Void(); - } -#ifdef TRANSCEIVE_VIA_SOCKET - else { - //TODO Should we throw fatal error here.? + } else { + // It should not come here, but incase if for any reason SB keymaster fails to getHardwareInfo + // return proper values from HAL. _hidl_cb(SecurityLevel::STRONGBOX, JAVACARD_KEYMASTER_NAME, JAVACARD_KEYMASTER_AUTHOR); return Void(); } -#endif } Return JavacardKeymaster4Device::getHmacSharingParameters(getHmacSharingParameters_cb _hidl_cb) { @@ -412,7 +398,7 @@ Return JavacardKeymaster4Device::getHmacSharingParameters(getHmacSharingPa } } } -#ifdef TRANSCEIVE_VIA_SOCKET +#ifdef VTS_EMULATOR /* TODO temporary fix: vold daemon calls performHmacKeyAgreement. At that time when vold calls this API there is no * network connectivity and socket cannot be connected. So as a hack we are calling softkeymaster to getHmacSharing * parameters. @@ -466,7 +452,7 @@ Return JavacardKeymaster4Device::computeSharedHmac(const hidl_vec JavacardKeymaster4Device::begin(KeyPurpose purpose, const hidl_vec< _hidl_cb(ErrorCode::INVALID_ARGUMENT, resultParams, operationHandle); return Void(); } - /* Asymmetric public key operations are handled by softkeymaster. */ + /* Asymmetric public key operations like RSA Verify, RSA Encrypt, ECDSA verify + * are handled by softkeymaster. + */ if (KeyPurpose::ENCRYPT == purpose || KeyPurpose::VERIFY == purpose) { BeginOperationRequest request; request.purpose = legacy_enum_conversion(purpose); diff --git a/HAL/keymaster/4.1/JavacardOperationContext.cpp b/HAL/keymaster/4.1/JavacardOperationContext.cpp index 1872f75b..e3f4c800 100644 --- a/HAL/keymaster/4.1/JavacardOperationContext.cpp +++ b/HAL/keymaster/4.1/JavacardOperationContext.cpp @@ -123,6 +123,7 @@ ErrorCode OperationContext::validateInputData(uint64_t operHandle, Operation opr memset(oprData.data.buf, 0x00, sizeof(oprData.data.buf)); oprData.data.buf_len = 0; } + return ErrorCode::OK; } } input = actualInput; @@ -342,7 +343,7 @@ ErrorCode OperationContext::handleInternalUpdate(uint64_t operHandle, uint8_t* d return errorCode; } } else { - //For strongbox keymaster, in NoDigest case the length of the input message for RSA should be more than + //For strongbox keymaster, in NoDigest case the length of the input message for RSA should not be more than //256 and for EC it should not be more than 32. This validation is already happening in //validateInputData function. Just for safety sake we are checking the length to MAX_BUF_SIZE. if(operationTable[operHandle].data.buf_len <= MAX_BUF_SIZE) { diff --git a/HAL/keymaster/Android.bp b/HAL/keymaster/Android.bp index eb4fa49c..d92809e8 100644 --- a/HAL/keymaster/Android.bp +++ b/HAL/keymaster/Android.bp @@ -51,7 +51,9 @@ cc_binary { ], product_variables: { debuggable: { - cflags: ["-DTRANSCEIVE_VIA_SOCKET"] + cflags: [ + "-DVTS_EMULATOR", + ] } } } From 20ca29b22f785b7d3e0488645f3b0e6e5690f4f9 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Fri, 15 Jan 2021 21:53:46 +0530 Subject: [PATCH 08/24] Clearing the operation data inside the operation context --- HAL/keymaster/4.1/JavacardKeymaster4Device.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 5ac1a431..445f3737 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -1163,6 +1163,7 @@ Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, co } /* Delete the entry from operation table. */ deleteOprHandleEntry(halGeneratedOprHandle); + oprCtx_->clearOperationData(operationHandle); _hidl_cb(errorCode, outParams, output); return Void(); } From 8e97a450ef51e0311d439d896d48126e0e895e3d Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Sun, 17 Jan 2021 19:50:33 +0530 Subject: [PATCH 09/24] Added comments for getBlockAlignedData function --- .../4.1/JavacardOperationContext.cpp | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardOperationContext.cpp b/HAL/keymaster/4.1/JavacardOperationContext.cpp index e3f4c800..7319cb3c 100644 --- a/HAL/keymaster/4.1/JavacardOperationContext.cpp +++ b/HAL/keymaster/4.1/JavacardOperationContext.cpp @@ -23,7 +23,6 @@ #define DES_BLOCK_SIZE 8 #define RSA_INPUT_MSG_LEN 256 #define EC_INPUT_MSG_LEN 32 -#define MAX_RSA_BUFFER_SIZE 256 #define MAX_EC_BUFFER_SIZE 32 namespace keymaster { @@ -104,11 +103,10 @@ ErrorCode OperationContext::validateInputData(uint64_t operHandle, Operation opr } if(KeyPurpose::DECRYPT == oprData.info.purpose && Algorithm::RSA == oprData.info.alg) { - if((oprData.data.buf_len+actualInput.size()) > MAX_RSA_BUFFER_SIZE) { + if((oprData.data.buf_len+actualInput.size()) > RSA_INPUT_MSG_LEN) { return ErrorCode::INVALID_INPUT_LENGTH; } } - if(opr == Operation::Finish) { //If it is observed in finish operation that buffered data + input data exceeds the MAX_ALLOWED_INPUT_SIZE then //combine both the data in a single buffer. This helps in making sure that no data is left out in the buffer after @@ -171,7 +169,6 @@ ErrorCode OperationContext::update(uint64_t operHandle, const std::vector& actualInput, sendDataToSE_cb cb) { ErrorCode errorCode = ErrorCode::OK; std::vector input; - /* Validate the input data */ if(ErrorCode::OK != (errorCode = validateInputData(operHandle, Operation::Finish, actualInput, input))) { return errorCode; @@ -214,15 +211,25 @@ ErrorCode OperationContext::finish(uint64_t operHandle, const std::vector& out) { - size_t dataToSELen = 0; + size_t dataToSELen = 0;/*Length of the data to be send to the Applet.*/ size_t inputConsumed = 0;/*Length of the data consumed from input */ size_t blockSize = 0; BufferedData& data = operationTable[operHandle].data; int bufIndex = data.buf_len; - if(Algorithm::AES == operationTable[operHandle].info.alg) { blockSize = AES_BLOCK_SIZE; } else if(Algorithm::TRIPLE_DES == operationTable[operHandle].info.alg) { @@ -265,6 +272,8 @@ ErrorCode OperationContext::getBlockAlignedData(uint64_t operHandle, uint8_t* in if(dataToSELen > 0) { //If buffer length is greater than the data length to be send to SE, then input data consumed is 0. //That means all the data to be send to SE is consumed from the buffer. + //The buffer length might become greater than dataToSELen in the cases where we are saving the last block of + //data i.e. AES/TDES Decryption with PKC7Padding or AES GCM Decryption operations. inputConsumed = (data.buf_len > dataToSELen) ? 0 : (dataToSELen - data.buf_len); //Copy the buffer to be send to SE. @@ -301,7 +310,6 @@ ErrorCode OperationContext::handleInternalUpdate(uint64_t operHandle, uint8_t* d sendDataToSE_cb cb, bool finish) { ErrorCode errorCode = ErrorCode::OK; std::vector out; - if(Algorithm::AES == operationTable[operHandle].info.alg || Algorithm::TRIPLE_DES == operationTable[operHandle].info.alg) { /*Symmetric */ @@ -345,16 +353,14 @@ ErrorCode OperationContext::handleInternalUpdate(uint64_t operHandle, uint8_t* d } else { //For strongbox keymaster, in NoDigest case the length of the input message for RSA should not be more than //256 and for EC it should not be more than 32. This validation is already happening in - //validateInputData function. Just for safety sake we are checking the length to MAX_BUF_SIZE. - if(operationTable[operHandle].data.buf_len <= MAX_BUF_SIZE) { - size_t bufIndex = operationTable[operHandle].data.buf_len; - size_t pos = 0; - for(; (pos < len) && (pos < (MAX_BUF_SIZE-bufIndex)); pos++) - { - operationTable[operHandle].data.buf[bufIndex+pos] = data[pos]; - } - operationTable[operHandle].data.buf_len += pos; + //validateInputData function. + size_t bufIndex = operationTable[operHandle].data.buf_len; + size_t pos = 0; + for(; pos < len; ++pos) + { + operationTable[operHandle].data.buf[bufIndex+pos] = data[pos]; } + operationTable[operHandle].data.buf_len += pos; } } else { /* With Digest */ for(size_t j=0; j < len; ++j) From 25325b3cda43ae28dafe6724f9788842d4a04cc1 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Tue, 19 Jan 2021 21:22:57 +0530 Subject: [PATCH 10/24] Handle extended errorcodes in HAL --- .../4.1/JavacardKeymaster4Device.cpp | 103 ++++++++++++++---- 1 file changed, 81 insertions(+), 22 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 445f3737..20e5cd5a 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -94,6 +94,23 @@ enum class Instruction { INS_GET_CERT_CHAIN_CMD = INS_END_KM_PROVISION_CMD+22 }; +//Extended error codes +enum ExtendedErrors { + SW_CONDITIONS_NOT_SATISFIED = -1001, + UNSUPPORTED_CLA = -1002, + INVALID_P1P2 = -1003, + UNSUPPORTED_INSTRUCTION = -1004, + CMD_NOT_ALLOWED = -1005, + SW_WRONG_LENGTH = -1006, + INVALID_DATA = -1007, + CRYPTO_ILLEGAL_USE = -1008, + CRYPTO_ILLEGAL_VALUE = -1009, + CRYPTO_INVALID_INIT = -1010, + CRYPTO_NO_SUCH_ALGORITHM = -1011, + CRYPTO_UNINITIALIZED_KEY = -1012, + GENERIC_UNKNOWN_ERROR = -1013 +}; + static inline std::unique_ptr& getTransportFactoryInstance() { if(pTransportFactory == nullptr) { pTransportFactory = std::unique_ptr(new se_transport::TransportFactory( @@ -123,6 +140,47 @@ static inline bool getTag(const hidl_vec& params, Tag tag, KeyPara return false; } +template +static T translateExtendedErrorsToHalErrors(T& errorCode) { + T err; + switch(static_cast(errorCode)) { + case SW_CONDITIONS_NOT_SATISFIED: + case UNSUPPORTED_CLA: + case INVALID_P1P2: + case INVALID_DATA: + case CRYPTO_ILLEGAL_USE: + case CRYPTO_ILLEGAL_VALUE: + case CRYPTO_INVALID_INIT: + case CRYPTO_UNINITIALIZED_KEY: + case GENERIC_UNKNOWN_ERROR: + err = T::UNKNOWN_ERROR; + break; + case CRYPTO_NO_SUCH_ALGORITHM: + err = T::UNSUPPORTED_ALGORITHM; + break; + case UNSUPPORTED_INSTRUCTION: + case CMD_NOT_ALLOWED: + case SW_WRONG_LENGTH: + err = T::UNIMPLEMENTED; + break; + default: + err = static_cast(errorCode); + break; + } + return err; +} + +template +static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response, bool + hasErrorCode) { + std::unique_ptr item(nullptr); + T errorCode = T::OK; + std::tie(item, errorCode) = cb.decodeData(response, hasErrorCode); + if (ErrorCode::OK != errorCode) + errorCode = translateExtendedErrorsToHalErrors(errorCode); + return {std::move(item), errorCode}; +} + /* Generate new operation handle */ static ErrorCode generateOperationHandle(uint64_t& oprHandle) { std::map>::iterator it; @@ -390,7 +448,7 @@ Return JavacardKeymaster4Device::getHmacSharingParameters(getHmacSharingPa errorCode = sendData(Instruction::INS_GET_HMAC_SHARING_PARAM_CMD, input, cborData); if (ErrorCode::OK == errorCode) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborData.begin(), cborData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborData.begin(), cborData.end()-2), true); if (item != nullptr) { if(!cborConverter_.getHmacSharingParameters(item, 1, hmacSharingParameters)) { @@ -441,7 +499,7 @@ Return JavacardKeymaster4Device::computeSharedHmac(const hidl_vec(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { std::vector bstr; @@ -498,7 +556,7 @@ Return JavacardKeymaster4Device::addRngEntropy(const hidl_vec(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); } return errorCode; @@ -530,7 +588,7 @@ Return JavacardKeymaster4Device::generateKey(const hidl_vec& if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { if(!cborConverter_.getBinaryArray(item, 1, keyBlob) || @@ -576,7 +634,7 @@ Return JavacardKeymaster4Device::importKey(const hidl_vec& k if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { if(!cborConverter_.getBinaryArray(item, 1, keyBlob) || @@ -631,7 +689,7 @@ Return JavacardKeymaster4Device::importWrappedKey(const hidl_vec& if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { if(!cborConverter_.getBinaryArray(item, 1, keyBlob) || @@ -664,7 +722,7 @@ Return JavacardKeymaster4Device::getKeyCharacteristics(const hidl_vec(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { if(!cborConverter_.getKeyCharacteristics(item, 1, keyCharacteristics)) { @@ -730,7 +788,7 @@ Return JavacardKeymaster4Device::attestKey(const hidl_vec& keyToA std::vector> temp; std::vector rootCert; //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { if(!cborConverter_.getMultiBinaryArray(item, 1, temp)) { @@ -741,7 +799,8 @@ Return JavacardKeymaster4Device::attestKey(const hidl_vec& keyToA errorCode = sendData(Instruction::INS_GET_CERT_CHAIN_CMD, cborData, cborOutData, true); if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), + cborOutData.end()-2), true); if (item != nullptr) { std::vector chain; @@ -779,7 +838,7 @@ Return JavacardKeymaster4Device::upgradeKey(const hidl_vec& keyBl if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { if(!cborConverter_.getBinaryArray(item, 1, upgradedKeyBlob)) @@ -802,7 +861,7 @@ Return JavacardKeymaster4Device::deleteKey(const hidl_vec& k if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); } return errorCode; @@ -818,7 +877,7 @@ Return JavacardKeymaster4Device::deleteAllKeys() { if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); } return errorCode; @@ -834,7 +893,7 @@ Return JavacardKeymaster4Device::destroyAttestationIds() { if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); } return errorCode; @@ -918,7 +977,7 @@ Return JavacardKeymaster4Device::begin(KeyPurpose purpose, const hidl_vec< errorCode = sendData(Instruction::INS_BEGIN_OPERATION_CMD, cborData, cborOutData); if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { if(!cborConverter_.getKeyParameters(item, 1, outParams) || @@ -1007,7 +1066,7 @@ Return JavacardKeymaster4Device::update(uint64_t halGeneratedOprHandle, co if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { /*Ignore inputConsumed from javacard SE since HAL consumes all the input */ @@ -1065,10 +1124,10 @@ Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, co request.signature.Reinitialize(signature.data(), signature.size()); request.additional_params.Reinitialize(KmParamSet(inParams)); - //FinishOperationResponse response; softKm_->FinishOperation(request, &response); errorCode = legacy_enum_conversion(response.error); + if (response.error == KM_ERROR_OK) { outParams = kmParamSet2Hidl(response.output_params); output = kmBuffer2hidlVec(response.output); @@ -1135,7 +1194,7 @@ Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, co if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); if (item != nullptr) { //There is a change that this finish callback may gets called multiple times if the input data size @@ -1194,7 +1253,7 @@ Return JavacardKeymaster4Device::abort(uint64_t halGeneratedOprHandle if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData(std::vector(cborOutData.begin(), cborOutData.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); } } @@ -1226,8 +1285,8 @@ Return<::android::hardware::keymaster::V4_1::ErrorCode> JavacardKeymaster4Device if(ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData<::android::hardware::keymaster::V4_1::ErrorCode>(std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + std::tie(item, errorCode) = cborConverter_.decodeData<::android::hardware::keymaster::V4_1::ErrorCode>( + std::vector(cborOutData.begin(), cborOutData.end()-2), true); } return errorCode; } @@ -1243,8 +1302,8 @@ Return<::android::hardware::keymaster::V4_1::ErrorCode> JavacardKeymaster4Device if(ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData<::android::hardware::keymaster::V4_1::ErrorCode>(std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + std::tie(item, errorCode) = cborConverter_.decodeData<::android::hardware::keymaster::V4_1::ErrorCode>( + std::vector(cborOutData.begin(), cborOutData.end()-2), true); } return errorCode; } From 407a8b7db1807655f2459b2bed148c8218238b0b Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Tue, 19 Jan 2021 20:49:17 +0530 Subject: [PATCH 11/24] 1. Exception handling for CryptoException and Generic exceptions 2. PKCS7 padding decrypt fix for AES/DES 3. Few tags were not supported. --- .../keymaster/KMAndroidSEProvider.java | 16 +-- .../javacard/keymaster/KMOperationImpl.java | 6 + .../javacard/keymaster/KMCipherImpl.java | 6 + .../javacard/keymaster/KMJCardSimulator.java | 28 +---- .../android/javacard/keymaster/KMUtils.java | 1 + .../android/javacard/keymaster/KMError.java | 9 ++ .../javacard/keymaster/KMKeyParameters.java | 32 +++++ .../javacard/keymaster/KMKeymasterApplet.java | 118 ++++++------------ .../javacard/keymaster/KMRepository.java | 2 +- .../javacard/keymaster/KMSEProvider.java | 72 +++++------ 10 files changed, 125 insertions(+), 165 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java index aba38934..e18ae600 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java @@ -100,7 +100,7 @@ public class KMAndroidSEProvider implements KMSEProvider { (byte) 0x25, (byte) 0x51 }; static final short secp256r1_H = 1; // -------------------------------------------------------------- - public static final short AES_GCM_TAG_LENGTH = 12; + public static final short AES_GCM_TAG_LENGTH = 16; public static final short AES_GCM_NONCE_LENGTH = 12; public static final byte KEYSIZE_128_OFFSET = 0x00; public static final byte KEYSIZE_256_OFFSET = 0x01; @@ -608,7 +608,7 @@ public short aesGCMEncrypt(byte[] aesKey, short aesKeyStart, short aesKeyLen, short authTagStart, short authTagLen) { if (authTagLen != AES_GCM_TAG_LENGTH) { - KMException.throwIt(KMError.UNKNOWN_ERROR); + CryptoException.throwIt(CryptoException.ILLEGAL_VALUE); } if (nonceLen != AES_GCM_NONCE_LENGTH) { CryptoException.throwIt(CryptoException.ILLEGAL_VALUE); @@ -1115,18 +1115,6 @@ public KMAttestationCert getAttestationCert(boolean rsaCert) { return KMAttestationCertImpl.instance(rsaCert); } - @Override - public short aesCCMSign(byte[] bufIn, short bufInStart, short buffInLength, - byte[] masterKeySecret, short masterKeyStart, short masterKeyLen, - byte[] bufOut, short bufStart) { - if (masterKeyLen > 16) { - return -1; - } - aesKeys[KEYSIZE_128_OFFSET].setKey(masterKeySecret, (short) masterKeyStart); - kdf.init(aesKeys[KEYSIZE_128_OFFSET], Signature.MODE_SIGN); - return kdf.sign(bufIn, bufInStart, buffInLength, bufOut, bufStart); - } - @Override public short cmacKdf(byte[] keyMaterial, short keyMaterialStart, short keyMaterialLen, byte[] label, short labelStart, short labelLen, diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMOperationImpl.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMOperationImpl.java index b1f48827..1c7ab32d 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMOperationImpl.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMOperationImpl.java @@ -161,6 +161,12 @@ public short finish(byte[] inputDataBuf, short inputDataStart, // padding byte always should be <= block size if ((short) paddingByte > blkSize || (short) paddingByte <= 0) KMException.throwIt(KMError.INVALID_ARGUMENT); + + for(short j = 1; j <= paddingByte; ++j) { + if (outputDataBuf[(short) (outputDataStart + len - j)] != paddingByte) { + KMException.throwIt(KMError.INVALID_ARGUMENT); + } + } len = (short) (len - (short) paddingByte);// remove the padding bytes } } else if (cipherAlg == KMType.AES && blockMode == KMType.GCM) { diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMCipherImpl.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMCipherImpl.java index ea65a94c..264b74e5 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMCipherImpl.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMCipherImpl.java @@ -121,6 +121,12 @@ public short doFinal(byte[] buffer, short startOff, short length, byte[] scratch //padding byte always should be <= block size if((short)paddingByte > blkSize || (short)paddingByte <= 0) KMException.throwIt(KMError.INVALID_ARGUMENT); + + for(short j = 1; j <= paddingByte; ++j) { + if (scratchPad[i+len -j] != paddingByte) { + KMException.throwIt(KMError.INVALID_ARGUMENT); + } + } len = (short)(len - (short)paddingByte);// remove the padding bytes } } diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java index 9c9e71c6..eb50fd6d 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java @@ -66,7 +66,7 @@ * creates its own RNG using PRNG. */ public class KMJCardSimulator implements KMSEProvider { - public static final short AES_GCM_TAG_LENGTH = 12; + public static final short AES_GCM_TAG_LENGTH = 16; public static final short AES_GCM_NONCE_LENGTH = 12; public static final short MAX_RND_NUM_SIZE = 64; public static final short ENTROPY_POOL_SIZE = 16; // simulator does not support 256 bit aes keys @@ -483,31 +483,7 @@ public boolean aesGCMDecrypt( public void getTrueRandomNumber(byte[] buf, short start, short length) { Util.arrayCopy(entropyPool,(short)0,buf,start,length); } - - @Override - public short aesCCMSign( - byte[] bufIn, - short bufInStart, - short buffInLength, - byte[] masterKeySecret, - short masterKeyStart, - short masterKeyLen, - byte[] bufOut, - short bufStart) { - if (masterKeyLen > 16) { - return -1; - } - AESKey key = (AESKey) KeyBuilder.buildKey(KeyBuilder.TYPE_AES, KeyBuilder.LENGTH_AES_128, false); - key.setKey(masterKeySecret, masterKeyStart); - byte[] in = new byte[buffInLength]; - Util.arrayCopyNonAtomic(bufIn, bufInStart,in,(short)0,buffInLength); - kdf.init(key, Signature.MODE_SIGN); - short len = kdf.sign(bufIn, bufInStart, buffInLength, bufOut, bufStart); - byte[] out = new byte[len]; - Util.arrayCopyNonAtomic(bufOut, bufStart,out,(short)0,len); - return len; - } - + public HMACKey cmacKdf(byte[] keyMaterial, short keyMaterialStart, short keyMaterialLen, byte[] label, short labelStart, short labelLen, byte[] context, short contextStart, short contextLength) { // This is hardcoded to requirement - 32 byte output with two concatenated 16 bytes K1 and K2. diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java index d9dce03b..756c3150 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java @@ -102,6 +102,7 @@ public static short convertToDate(short time, byte[] scratchPad, Util.arrayCopyNonAtomic(oneMonthMsec, (short) 0, scratchPad, (short) 8, (short) 8); monthCount = divide(scratchPad, (short) 0, (short) 8, (short) 16); + monthCount =+ 1; Util.arrayCopyNonAtomic(scratchPad, (short) 16, scratchPad, (short) 0, (short) 8); } diff --git a/Applet/src/com/android/javacard/keymaster/KMError.java b/Applet/src/com/android/javacard/keymaster/KMError.java index e70dc8c2..abb0e6df 100644 --- a/Applet/src/com/android/javacard/keymaster/KMError.java +++ b/Applet/src/com/android/javacard/keymaster/KMError.java @@ -92,4 +92,13 @@ public class KMError { public static short CMD_NOT_ALLOWED = 1005; public static short SW_WRONG_LENGTH = 1006; public static short INVALID_DATA = 1007; + //Crypto errors + public static short CRYPTO_ILLEGAL_USE = 1008; + public static short CRYPTO_ILLEGAL_VALUE = 1009; + public static short CRYPTO_INVALID_INIT = 1010; + public static short CRYPTO_NO_SUCH_ALGORITHM = 1011; + public static short CRYPTO_UNINITIALIZED_KEY = 1012; + //Generic Unknown error. + public static short GENERIC_UNKNOWN_ERROR = 1013; + } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeyParameters.java b/Applet/src/com/android/javacard/keymaster/KMKeyParameters.java index ae759ffc..8c8475ac 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeyParameters.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeyParameters.java @@ -100,6 +100,38 @@ public short findTag(short tagType, short tagKey){ } return ret; } + + public static boolean hasUnsupportedTags(short keyParamsPtr) { + final short[] tagArr = { + // Unsupported tags. + KMType.BOOL_TAG, KMType.TRUSTED_CONFIRMATION_REQUIRED, + KMType.BOOL_TAG, KMType.TRUSTED_USER_PRESENCE_REQUIRED, + KMType.BOOL_TAG, KMType.ALLOW_WHILE_ON_BODY, + KMType.UINT_TAG, KMType.MIN_SEC_BETWEEN_OPS, + }; + byte index = 0; + short tagInd; + short tagPtr; + short tagKey; + short tagType; + short arrPtr = KMKeyParameters.cast(keyParamsPtr).getVals(); + short len = KMArray.cast(arrPtr).length(); + while (index < len) { + tagInd = 0; + tagPtr = KMArray.cast(arrPtr).get(index); + tagKey = KMTag.getKey(tagPtr); + tagType = KMTag.getTagType(tagPtr); + while (tagInd < (short) tagArr.length) { + if ((tagArr[tagInd] == tagType) + && (tagArr[(short) (tagInd + 1)] == tagKey)) { + return true; + } + tagInd += 2; + } + index++; + } + return false; + } // KDF, ECIES_SINGLE_HASH_MODE missing from types.hal public static short makeHwEnforced(short keyParamsPtr, byte origin, diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index d9691a1e..febc7ec7 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -160,7 +160,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe public static final byte KEY_BLOB_KEYCHAR = 3; public static final byte KEY_BLOB_PUB_KEY = 4; // AES GCM constants - private static final byte AES_GCM_AUTH_TAG_LENGTH = 12; + private static final byte AES_GCM_AUTH_TAG_LENGTH = 16; private static final byte AES_GCM_NONCE_LENGTH = 12; // ComputeHMAC constants private static final short HMAC_SHARED_PARAM_MAX_SIZE = 64; @@ -245,6 +245,23 @@ private short mapISOErrorToKMError(short reason) { return KMError.UNKNOWN_ERROR; } } + + private short mapCryptoErrorToKMError(short reason) { + switch (reason) { + case CryptoException.ILLEGAL_USE: + return KMError.CRYPTO_ILLEGAL_USE; + case CryptoException.ILLEGAL_VALUE: + return KMError.CRYPTO_ILLEGAL_VALUE; + case CryptoException.INVALID_INIT: + return KMError.CRYPTO_INVALID_INIT; + case CryptoException.NO_SUCH_ALGORITHM: + return KMError.CRYPTO_NO_SUCH_ALGORITHM; + case CryptoException.UNINITIALIZED_KEY: + return KMError.CRYPTO_UNINITIALIZED_KEY; + default: + return KMError.UNKNOWN_ERROR; + } + } protected void validateApduHeader(APDU apdu) { // Read the apdu header and buffer. @@ -443,6 +460,12 @@ && isProvisioningComplete())) { } catch (ISOException exp) { sendError(apdu, mapISOErrorToKMError(exp.getReason())); freeOperations(); + } catch (CryptoException e) { + freeOperations(); + sendError(apdu, mapCryptoErrorToKMError(e.getReason())); + } catch (Exception e) { + freeOperations(); + sendError(apdu, KMError.GENERIC_UNKNOWN_ERROR); } finally { resetData(); repository.clean(); @@ -578,7 +601,6 @@ private void processAddRngEntropyCmd(APDU apdu) { //reclaim memory repository.reclaimMemory(bufferLength); - // Process KMByteBlob blob = KMByteBlob.cast(KMArray.cast(args).get((short) 0)); // Maximum 2KiB of seed is allowed. @@ -606,7 +628,6 @@ private void processGetCertChainCmd(APDU apdu) { sendOutgoing(apdu); } - private void processProvisionAttestationCertParams(APDU apdu) { receiveIncoming(apdu); // Arguments @@ -620,7 +641,6 @@ private void processProvisionAttestationCertParams(APDU apdu) { //reclaim memory repository.reclaimMemory(bufferLength); - // save issuer - DER Encoded tmpVariables[0] = KMArray.cast(args).get((short) 0); repository.setIssuer( @@ -1607,7 +1627,6 @@ private void processFinishOperationCmd(APDU apdu) { // Finish trusted Confirmation operation switch (op.getPurpose()) { case KMType.SIGN: - finishTrustedConfirmationOperation(op); case KMType.VERIFY: finishSigningVerifyingOperation(op, scratchPad); break; @@ -1631,7 +1650,6 @@ private void processFinishOperationCmd(APDU apdu) { KMArray.cast(tmpVariables[2]).add((short) 1, tmpVariables[1]); KMArray.cast(tmpVariables[2]).add((short) 2, data[OUTPUT_DATA]); - bufferStartOffset = repository.allocAvailableMemory(); // Encode the response bufferLength = encoder.encode(tmpVariables[2], buffer, bufferStartOffset); @@ -1905,31 +1923,6 @@ private void finishSigningVerifyingOperation(KMOperationState op, byte[] scratch } } - private void finishTrustedConfirmationOperation(KMOperationState op) { - // Perform trusted confirmation if required - if (op.isTrustedConfirmationRequired()) { - tmpVariables[0] = - KMKeyParameters.findTag( - KMType.BYTES_TAG, KMType.CONFIRMATION_TOKEN, data[KEY_PARAMETERS]); - if (tmpVariables[0] == KMType.INVALID_VALUE) { - KMException.throwIt(KMError.INVALID_ARGUMENT); - } - tmpVariables[0] = KMByteTag.cast(tmpVariables[0]).getValue(); - boolean verified = - op.getTrustedConfirmationSigner() - .verify( - KMByteBlob.cast(data[INPUT_DATA]).getBuffer(), - KMByteBlob.cast(data[INPUT_DATA]).getStartOff(), - KMByteBlob.cast(data[INPUT_DATA]).length(), - KMByteBlob.cast(tmpVariables[0]).getBuffer(), - KMByteBlob.cast(tmpVariables[0]).getStartOff(), - KMByteBlob.cast(tmpVariables[0]).length()); - if (!verified) { - KMException.throwIt(KMError.VERIFICATION_FAILED); - } - } - } - private void authorizeUpdateFinishOperation(KMOperationState op, byte[] scratchPad) { // If one time user Authentication is required if (op.isSecureUserIdReqd() && !op.isAuthTimeoutValidated()) { @@ -2111,8 +2104,6 @@ private void processUpdateOperationCmd(APDU apdu) { KMByteBlob.cast(data[INPUT_DATA]).getBuffer(), KMByteBlob.cast(data[INPUT_DATA]).getStartOff(), KMByteBlob.cast(data[INPUT_DATA]).length()); - // update trusted confirmation operation - updateTrustedConfirmationOperation(op); data[OUTPUT_DATA] = KMType.INVALID_VALUE; } else if (op.getPurpose() == KMType.ENCRYPT || op.getPurpose() == KMType.DECRYPT) { // Update for encrypt/decrypt using RSA will not be supported because to do this op state @@ -2200,16 +2191,6 @@ private void processUpdateOperationCmd(APDU apdu) { sendOutgoing(apdu); } - private void updateTrustedConfirmationOperation(KMOperationState op) { - if (op.isTrustedConfirmationRequired()) { - op.getTrustedConfirmationSigner() - .update( - KMByteBlob.cast(data[INPUT_DATA]).getBuffer(), - KMByteBlob.cast(data[INPUT_DATA]).getStartOff(), - KMByteBlob.cast(data[INPUT_DATA]).length()); - } - } - private void processBeginOperationCmd(APDU apdu) { // Receive the incoming request fully from the master into buffer. receiveIncoming(apdu); @@ -2257,7 +2238,6 @@ private void processBeginOperationCmd(APDU apdu) { authorizeAndBeginOperation(op, scratchPad); switch (op.getPurpose()) { case KMType.SIGN: - beginTrustedConfirmationOperation(op); case KMType.VERIFY: beginSignVerifyOperation(op); break; @@ -2304,42 +2284,6 @@ private void processBeginOperationCmd(APDU apdu) { sendOutgoing(apdu); } - private void beginTrustedConfirmationOperation(KMOperationState op) { - // Check for trusted confirmation - if required then set the signer in op state. - if (KMKeyParameters.findTag( - KMType.BOOL_TAG, KMType.TRUSTED_CONFIRMATION_REQUIRED, data[HW_PARAMETERS]) - != KMType.INVALID_VALUE) { - // get operation - // get the hmac key - short key = repository.getComputedHmacKey(); - if (key == 0) { - KMException.throwIt(KMError.OPERATION_CANCELLED); - } - /* - op.setTrustedConfirmationSigner(seProvider.initSymmetricOperation( - KMType.VERIFY,KMType.HMAC,KMType.SHA2_256,(byte)0,(byte)0,repository.getComputedHmacKey(), - (short) 0, (short) repository.getComputedHmacKey().length,null,(short)0,(short)0,(short)0)); - */ - op.setTrustedConfirmationSigner( - seProvider.initSymmetricOperation( - KMType.VERIFY, - KMType.HMAC, - KMType.SHA2_256, - (byte) 0, - (byte) 0, - KMByteBlob.cast(key).getBuffer(), - KMByteBlob.cast(key).getStartOff(), - KMByteBlob.cast(key).length(), - null, - (short) 0, - (short) 0, - (short) 0)); - - op.getTrustedConfirmationSigner() - .update(confirmationToken, (short) 0, (short) confirmationToken.length); - } - } - private void authorizeAlgorithm(KMOperationState op) { short alg = KMEnumTag.getValue(KMType.ALGORITHM, data[HW_PARAMETERS]); if (alg == KMType.INVALID_VALUE) { @@ -3435,6 +3379,20 @@ private static void processGenerateKey(APDU apdu) { KMException.throwIt(KMError.UNSUPPORTED_KEY_SIZE); } } + // Only STANDALONE is supported for BLOB_USAGE_REQ tag. + tmpVariables[0] = + KMKeyParameters.findTag(KMType.ENUM_TAG, KMType.BLOB_USAGE_REQ, data[KEY_PARAMETERS]); + if (tmpVariables[0] != KMType.INVALID_VALUE) { + tmpVariables[0] = KMEnumTag.getValue(KMType.BLOB_USAGE_REQ, data[KEY_PARAMETERS]); + if (tmpVariables[0] != KMType.STANDALONE) { + KMException.throwIt(KMError.UNSUPPORTED_TAG); + } + } + //Check if the tags are supported. + if(KMKeyParameters.hasUnsupportedTags(data[KEY_PARAMETERS])) { + KMException.throwIt(KMError.UNSUPPORTED_TAG); + } + // Check algorithm and dispatch to appropriate handler. switch (tmpVariables[3]) { case KMType.RSA: diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index b8dc8b8d..062e31b5 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -84,7 +84,7 @@ public class KMRepository implements KMUpgradable { public static final short DEVICE_LOCK_FLAG_SIZE = 1; public static final short BOOT_STATE_SIZE = 1; public static final short MAX_BLOB_STORAGE = 8; - public static final short AUTH_TAG_LENGTH = 12; + public static final short AUTH_TAG_LENGTH = 16; public static final short AUTH_TAG_ENTRY_SIZE = 15; public static final short MAX_OPS = 4; public static final byte BOOT_KEY_MAX_SIZE = 32; diff --git a/Applet/src/com/android/javacard/keymaster/KMSEProvider.java b/Applet/src/com/android/javacard/keymaster/KMSEProvider.java index 81b35db2..3e981c9a 100644 --- a/Applet/src/com/android/javacard/keymaster/KMSEProvider.java +++ b/Applet/src/com/android/javacard/keymaster/KMSEProvider.java @@ -49,11 +49,12 @@ void createAsymmetricKey( short[] lengths); /** - * Verify that the imported key is valid. + * Verify that the imported key is valid. If the algorithm and/or keysize are not supported then it + * should throw a CryptoException. * * @param alg will be KMType.AES, KMType.DES or KMType.HMAC. * @param keysize will be 128 or 256 for AES or DES. It can be 64 to 512 (multiple of 8) for HMAC. - * @param buf is the buffer in which key has to be returned + * @param buf is the buffer that contains the symmetric key. * @param startOff is the start offset. * @param length of the data in the buf. This should match the keysize (in bytes). * @return true if the symmetric key is supported and valid. @@ -63,14 +64,15 @@ void createAsymmetricKey( /** * Validate that the imported asymmetric key pair is valid. For RSA the public key exponent must * always be 0x010001. The key size of RSA key pair must be 2048 bits and key size of EC key pair - * must be for p256 curve. + * must be for p256 curve. If the algorithms are not supported then it should throw a + * CryptoException. * * @param alg will be KMType.RSA or KMType.EC. - * @param privKeyBuf is the buffer to return the private key exponent in case of RSA or private + * @param privKeyBuf is the buffer that contains the private key exponent in case of RSA or private * key in case of EC. * @param privKeyStart is the start offset. * @param privKeyLength is the length of this private key buffer. - * @param pubModBuf is the buffer to return the modulus in case of RSA or public key in case of + * @param pubModBuf is the buffer that contains the modulus in case of RSA or public key in case of * EC. * @param pubModStart is the start of offset. * @param pubModLength is the length of this public key buffer. @@ -88,14 +90,14 @@ boolean importAsymmetricKey( /** * This is a oneshot operation that generates random number of desired length. * - * @param num is the buffer in which random number is returned to applet. + * @param num is the buffer in which random number is returned to the applet. * @param offset is start of the buffer. * @param length indicates the size of buffer and desired length of random number in bytes. */ void newRandomNumber(byte[] num, short offset, short length); /** - * This is a oneshot operation that adds the entropy to the entroy pool. This operation + * This is a oneshot operation that adds the entropy to the entropy pool. This operation * corresponds to addRndEntropy command. This method may ignore the added entropy value if the SE * provider does not support it. * @@ -108,14 +110,16 @@ boolean importAsymmetricKey( /** * This is a oneshot operation that generates and returns back a true random number. * - * @param num is the buffer in which entropy value is given. + * @param num is the buffer in which entropy value is returned. * @param offset is start of the buffer. * @param length length of the buffer. */ void getTrueRandomNumber(byte[] num, short offset, short length); /** - * This is a oneshot operation that performs encryption operation using AES GCM algorithm. + * This is a oneshot operation that performs encryption operation using AES GCM algorithm. It throws + * CryptoException if algorithm is not supported or if tag length is not equal to 16 or + * nonce length is not equal to 12. * * @param aesKey is the buffer that contains 128 bit or 256 bit aes key used to encrypt. * @param aesKeyStart is the start in aes key buffer. @@ -156,7 +160,8 @@ short aesGCMEncrypt( short authTagLen); /** - * This is a oneshot operation that performs decryption operation using AES GCM algorithm. + * This is a oneshot operation that performs decryption operation using AES GCM algorithm. It throws + * CryptoException if algorithm is not supported. * * @param aesKey is the buffer that contains 128 bit or 256 bit aes key used to encrypt. * @param aesKeyStart is the start in aes key buffer. @@ -164,7 +169,7 @@ short aesGCMEncrypt( * @param encData is the buffer of the input encrypted data. * @param encDataStart is the start of the encrypted data buffer. * @param encDataLen is the length of the data buffer. - * @param data is the buffer that contains output data to encrypt. + * @param data is the buffer that contains output decrypted data. * @param dataStart is the start of the data buffer. * @param nonce is the buffer of nonce. * @param nonceStart is the start of the nonce buffer. @@ -196,30 +201,7 @@ boolean aesGCMDecrypt( short authTagStart, short authTagLen); - /** - * This is a oneshot operation that performs signing using AES CCM algorithm. - * - * @param data is the input data buffer. - * @param dataStart is the start of the buffer. - * @param dataLen is the length of the input data - * @param aesKey is the aesKey buffer either 128 bit or 256 bit aes key. - * @param aesKeyStart is the start of the aes key. - * @param aesKeyLen is the length of the aes key buffer in bytes. - * @param signature is the output signature buffer. - * @param signatureStart is the start of the signature buffer. - * @return length of the signature buffer. - */ - short aesCCMSign( - byte[] data, - short dataStart, - short dataLen, - byte[] aesKey, - short aesKeyStart, - short aesKeyLen, - byte[] signature, - short signatureStart); - - /** + /** * This is a oneshot operation that performs key derivation function using cmac kdf (CKDF) as * defined in android keymaster hal definition. * @@ -299,7 +281,8 @@ boolean hmacVerify( /** * This is a oneshot operation that decrypts the data using RSA algorithm with oaep256 padding. - * The public exponent is always 0x010001. + * The public exponent is always 0x010001. It throws CryptoException if OAEP encoding validation + * fails. * * @param privExp is the private exponent (2048 bit) buffer. * @param privExpStart is the start of the private exponent buffer. @@ -309,8 +292,8 @@ boolean hmacVerify( * @param modLength is the length of the modulus buffer in bytes. * @param inputDataBuf is the buffer of the input data. * @param inputDataStart is the start of the input data buffer. - * @param inputDataLength is the length of the inpur data buffer in bytes. - * @param outputDataBuf is the buffer of the decrypted data buffer. + * @param inputDataLength is the length of the input data buffer in bytes. + * @param outputDataBuf is the output buffer that contains the decrypted data. * @param outputDataStart is the start of the output data buffer. * @return length of the decrypted data. */ @@ -336,11 +319,11 @@ short rsaDecipherOAEP256( * @param inputDataBuf is the buffer of the input data. * @param inputDataStart is the start of the input data buffer. * @param inputDataLength is the length of the inpur data buffer in bytes. - * @param outputDataBuf is the buffer of the decrypted data buffer. + * @param outputDataBuf is the output buffer that contains the signature. * @param outputDataStart is the start of the output data buffer. * @return length of the decrypted data. */ - public short ecSign256( + short ecSign256( byte[] secret, short secretStart, short secretLength, @@ -354,7 +337,7 @@ public short ecSign256( * This creates a persistent operation for signing, verify, encryption and decryption using HMAC, * AES and DES algorithms when keymaster hal's beginOperation function is executed. The * KMOperation instance can be reclaimed by the seProvider when KMOperation is finished or - * aborted. + * aborted. It throws CryptoException if algorithm is not supported. * * @param purpose is KMType.ENCRYPT or KMType.DECRYPT for AES and DES algorithm. It will be * KMType.SIGN and KMType.VERIFY for HMAC algorithm @@ -391,7 +374,8 @@ KMOperation initSymmetricOperation( * This creates a persistent operation for signing, verify, encryption and decryption using RSA * and EC algorithms when keymaster hal's beginOperation function is executed. For RSA the public * exponent is always 0x0100101. For EC the curve is always p256. The KMOperation instance can be - * reclaimed by the seProvider when KMOperation is finished or aborted. + * reclaimed by the seProvider when KMOperation is finished or aborted. It throws CryptoException + * if algorithm is not supported. * * @param purpose is KMType.ENCRYPT or KMType.DECRYPT for RSA. It will be * KMType.SIGN and * KMType.VERIFY for RSA and EC algorithms. @@ -439,12 +423,12 @@ KMOperation initAsymmetricOperation( * @param len is the length of the buffer. * @param totalLen is the total length of cert chain. */ - public void persistPartialCertificateChain(byte[] buf, short offset, short len, short totalLen); + void persistPartialCertificateChain(byte[] buf, short offset, short len, short totalLen); /** * This operation clears the certificate chain from persistent memory. */ - public void clearCertificateChain(); + void clearCertificateChain(); /** * The operation reads the certificate chain from persistent memory. From f297133d7d5519212f453aba7ce7895d41ddaf09 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 20 Jan 2021 20:41:07 +0530 Subject: [PATCH 12/24] Remove trusted confirmation related code --- .../android/javacard/keymaster/KMUtils.java | 13 +++++----- .../android/javacard/keymaster/KMUtils.java | 14 +++++----- .../android/javacard/keymaster/KMError.java | 26 +++++++++---------- .../javacard/keymaster/KMKeymasterApplet.java | 1 - .../javacard/keymaster/KMOperationState.java | 22 ++-------------- .../javacard/keymaster/KMRepository.java | 4 +-- 6 files changed, 30 insertions(+), 50 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java index d9dce03b..08309f86 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java @@ -13,23 +13,23 @@ public class KMUtils { public static final byte[] oneDayMsec = { 0, 0, 0, 0, 0x05, 0x26, 0x5C, 0x00 }; // 86400000 msec public static final byte[] oneMonthMsec = { - 0, 0, 0, 0, (byte) 0x9A, 0x7E, (byte) 0xC8, 0x00 }; // 2592000000 msec + 0, 0, 0, 0, (byte) 0x9C,(byte) 0xBE, (byte) 0xBD, 0x50}; // 2629746000 msec public static final byte[] oneYearMsec = { - 0, 0, 0, 0x07, 0x57, (byte) 0xB1, 0x2C, 0x00 }; // 31536000000 msec + 0, 0, 0, 0x75, (byte) 0x8F, 0x0D, (byte) 0xFC, 0x00 }; // 31556952000 msec // Leap year + 3 yrs public static final byte[] fourYrsMsec = { - 0, 0, 0, 0x1D, 0x63, (byte) 0xEB, 0x0C, 0x00 }; // 126230400000 msec + 0, 0, 0, 0x1D, 0x63, (byte) 0xC3, 0x7F, 0x00 }; // 126227808000 msec public static final byte[] firstJan2020 = { - 0, 0, 0x01, 0x6F, 0x60, 0x1E, 0x5C, 0x00 }; // 1577865600000 msec + 0, 0, 0x01, 0x76, (byte) 0xBB, 0x3E, (byte) 0x70, 0x40 }; // 1609459200000 msec public static final byte[] firstJan2051 = { - 0, 0, 0x02, 0x53, 0x27, (byte) 0xC5, (byte) 0x90, 0x00 }; // 2556172800000 + 0, 0, 0x02, 0x53, 0x26, (byte) 0x0E, (byte) 0x1C, 0x00 }; // 2556144000000 // msec // -------------------------------------- public static short convertToDate(short time, byte[] scratchPad, boolean utcFlag) { short yrsCount = 0; - short monthCount = 0; + short monthCount = 1; short dayCount = 0; short hhCount = 0; short mmCount = 0; @@ -102,6 +102,7 @@ public static short convertToDate(short time, byte[] scratchPad, Util.arrayCopyNonAtomic(oneMonthMsec, (short) 0, scratchPad, (short) 8, (short) 8); monthCount = divide(scratchPad, (short) 0, (short) 8, (short) 16); + monthCount++; Util.arrayCopyNonAtomic(scratchPad, (short) 16, scratchPad, (short) 0, (short) 8); } diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java index 756c3150..08309f86 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java @@ -13,23 +13,23 @@ public class KMUtils { public static final byte[] oneDayMsec = { 0, 0, 0, 0, 0x05, 0x26, 0x5C, 0x00 }; // 86400000 msec public static final byte[] oneMonthMsec = { - 0, 0, 0, 0, (byte) 0x9A, 0x7E, (byte) 0xC8, 0x00 }; // 2592000000 msec + 0, 0, 0, 0, (byte) 0x9C,(byte) 0xBE, (byte) 0xBD, 0x50}; // 2629746000 msec public static final byte[] oneYearMsec = { - 0, 0, 0, 0x07, 0x57, (byte) 0xB1, 0x2C, 0x00 }; // 31536000000 msec + 0, 0, 0, 0x75, (byte) 0x8F, 0x0D, (byte) 0xFC, 0x00 }; // 31556952000 msec // Leap year + 3 yrs public static final byte[] fourYrsMsec = { - 0, 0, 0, 0x1D, 0x63, (byte) 0xEB, 0x0C, 0x00 }; // 126230400000 msec + 0, 0, 0, 0x1D, 0x63, (byte) 0xC3, 0x7F, 0x00 }; // 126227808000 msec public static final byte[] firstJan2020 = { - 0, 0, 0x01, 0x6F, 0x60, 0x1E, 0x5C, 0x00 }; // 1577865600000 msec + 0, 0, 0x01, 0x76, (byte) 0xBB, 0x3E, (byte) 0x70, 0x40 }; // 1609459200000 msec public static final byte[] firstJan2051 = { - 0, 0, 0x02, 0x53, 0x27, (byte) 0xC5, (byte) 0x90, 0x00 }; // 2556172800000 + 0, 0, 0x02, 0x53, 0x26, (byte) 0x0E, (byte) 0x1C, 0x00 }; // 2556144000000 // msec // -------------------------------------- public static short convertToDate(short time, byte[] scratchPad, boolean utcFlag) { short yrsCount = 0; - short monthCount = 0; + short monthCount = 1; short dayCount = 0; short hhCount = 0; short mmCount = 0; @@ -102,7 +102,7 @@ public static short convertToDate(short time, byte[] scratchPad, Util.arrayCopyNonAtomic(oneMonthMsec, (short) 0, scratchPad, (short) 8, (short) 8); monthCount = divide(scratchPad, (short) 0, (short) 8, (short) 16); - monthCount =+ 1; + monthCount++; Util.arrayCopyNonAtomic(scratchPad, (short) 16, scratchPad, (short) 0, (short) 8); } diff --git a/Applet/src/com/android/javacard/keymaster/KMError.java b/Applet/src/com/android/javacard/keymaster/KMError.java index abb0e6df..85c71654 100644 --- a/Applet/src/com/android/javacard/keymaster/KMError.java +++ b/Applet/src/com/android/javacard/keymaster/KMError.java @@ -85,20 +85,20 @@ public class KMError { public static short UNKNOWN_ERROR = 1000; //Extended errors - public static short SW_CONDITIONS_NOT_SATISFIED = 1001; - public static short UNSUPPORTED_CLA = 1002; - public static short INVALID_P1P2 = 1003; - public static short UNSUPPORTED_INSTRUCTION = 1004; - public static short CMD_NOT_ALLOWED = 1005; - public static short SW_WRONG_LENGTH = 1006; - public static short INVALID_DATA = 1007; + public static short SW_CONDITIONS_NOT_SATISFIED = 10001; + public static short UNSUPPORTED_CLA = 10002; + public static short INVALID_P1P2 = 10003; + public static short UNSUPPORTED_INSTRUCTION = 10004; + public static short CMD_NOT_ALLOWED = 10005; + public static short SW_WRONG_LENGTH = 10006; + public static short INVALID_DATA = 10007; //Crypto errors - public static short CRYPTO_ILLEGAL_USE = 1008; - public static short CRYPTO_ILLEGAL_VALUE = 1009; - public static short CRYPTO_INVALID_INIT = 1010; - public static short CRYPTO_NO_SUCH_ALGORITHM = 1011; - public static short CRYPTO_UNINITIALIZED_KEY = 1012; + public static short CRYPTO_ILLEGAL_USE = 10008; + public static short CRYPTO_ILLEGAL_VALUE = 10009; + public static short CRYPTO_INVALID_INIT = 10010; + public static short CRYPTO_NO_SUCH_ALGORITHM = 10011; + public static short CRYPTO_UNINITIALIZED_KEY = 10012; //Generic Unknown error. - public static short GENERIC_UNKNOWN_ERROR = 1013; + public static short GENERIC_UNKNOWN_ERROR = 10013; } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index febc7ec7..a37e6aa8 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -1624,7 +1624,6 @@ private void processFinishOperationCmd(APDU apdu) { } // Authorize the finish operation authorizeUpdateFinishOperation(op, scratchPad); - // Finish trusted Confirmation operation switch (op.getPurpose()) { case KMType.SIGN: case KMType.VERIFY: diff --git a/Applet/src/com/android/javacard/keymaster/KMOperationState.java b/Applet/src/com/android/javacard/keymaster/KMOperationState.java index 6647a6a8..347c9920 100644 --- a/Applet/src/com/android/javacard/keymaster/KMOperationState.java +++ b/Applet/src/com/android/javacard/keymaster/KMOperationState.java @@ -28,7 +28,7 @@ public class KMOperationState { public static final byte MAX_DATA = 20; - public static final byte MAX_REFS = 2; + public static final byte MAX_REFS = 1; private static final byte DATA = 0; private static final byte REFS = 1; // byte type @@ -53,9 +53,6 @@ public class KMOperationState { // Object References private static final byte OPERATION = 0; - private static final byte HMAC_SIGNER = 1; - - private static KMOperation hmacSigner; // used for trusted confirmation. private static KMOperation op; private static byte[] data; private static Object[] slot; @@ -85,14 +82,13 @@ public static KMOperationState read(Object[] slot) { Util.arrayCopy((byte[]) slot[DATA], (short) 0, data, (short) 0, (short) data.length); Object[] ops = ((Object[]) slot[REFS]); op = (KMOperation) ops[OPERATION]; - hmacSigner = (KMOperation) ops[HMAC_SIGNER]; KMOperationState.slot = slot; return opState; } public void persist() { if (!dFlag) return; - KMRepository.instance().persistOperation(data, Util.getShort(data, OP_HANDLE), op, hmacSigner); + KMRepository.instance().persistOperation(data, Util.getShort(data, OP_HANDLE), op); dFlag = false; } @@ -104,21 +100,8 @@ public short getKeySize() { return Util.getShort(data, KEY_SIZE); } - public void setTrustedConfirmationSigner(KMOperation hmacSigner) { - KMOperationState.hmacSigner = hmacSigner; - } - - public KMOperation getTrustedConfirmationSigner() { - return KMOperationState.hmacSigner; - } - - public boolean isTrustedConfirmationRequired() { - return KMOperationState.hmacSigner != null; - } - public void reset() { dFlag = false; - hmacSigner = null; op = null; slot = null; Util.arrayFillNonAtomic( @@ -135,7 +118,6 @@ public void release() { Util.arrayFillNonAtomic( (byte[]) slot[0], (short) 0, (short) ((byte[]) slot[0]).length, (byte) 0); ops[OPERATION] = null; - ops[HMAC_SIGNER] = null; JCSystem.commitTransaction(); reset(); } diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index 062e31b5..c2809c08 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -148,7 +148,7 @@ public KMOperationState reserveOperation(){ } //TODO refactor following method - public void persistOperation(byte[] data, short opHandle, KMOperation op, KMOperation hmacSigner) { + public void persistOperation(byte[] data, short opHandle, KMOperation op) { short index = 0; byte[] opId; //Update an existing operation state. @@ -160,7 +160,6 @@ public void persistOperation(byte[] data, short opHandle, KMOperation op, KMOper Util.arrayCopy(data, (short) 0, (byte[]) slot[0], (short) 0, (short) ((byte[]) slot[0]).length); Object[] ops = ((Object[]) slot[1]); ops[0] = op; - ops[1] = hmacSigner; JCSystem.commitTransaction(); return; } @@ -178,7 +177,6 @@ public void persistOperation(byte[] data, short opHandle, KMOperation op, KMOper Util.arrayCopy(data, (short) 0, (byte[]) slot[0], (short) 0, (short) ((byte[]) slot[0]).length); Object[] ops = ((Object[]) slot[1]); ops[0] = op; - ops[1] = hmacSigner; JCSystem.commitTransaction(); break; } From 6c97a2578328a06d0c35378f58c8eac1e1f5ac89 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 20 Jan 2021 20:47:01 +0530 Subject: [PATCH 13/24] Corrected the extended error code values. --- .../4.1/JavacardKeymaster4Device.cpp | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 20e5cd5a..f422bc26 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -96,19 +96,19 @@ enum class Instruction { //Extended error codes enum ExtendedErrors { - SW_CONDITIONS_NOT_SATISFIED = -1001, - UNSUPPORTED_CLA = -1002, - INVALID_P1P2 = -1003, - UNSUPPORTED_INSTRUCTION = -1004, - CMD_NOT_ALLOWED = -1005, - SW_WRONG_LENGTH = -1006, - INVALID_DATA = -1007, - CRYPTO_ILLEGAL_USE = -1008, - CRYPTO_ILLEGAL_VALUE = -1009, - CRYPTO_INVALID_INIT = -1010, - CRYPTO_NO_SUCH_ALGORITHM = -1011, - CRYPTO_UNINITIALIZED_KEY = -1012, - GENERIC_UNKNOWN_ERROR = -1013 + SW_CONDITIONS_NOT_SATISFIED = -10001, + UNSUPPORTED_CLA = -10002, + INVALID_P1P2 = -10003, + UNSUPPORTED_INSTRUCTION = -10004, + CMD_NOT_ALLOWED = -10005, + SW_WRONG_LENGTH = -10006, + INVALID_DATA = -10007, + CRYPTO_ILLEGAL_USE = -10008, + CRYPTO_ILLEGAL_VALUE = -10009, + CRYPTO_INVALID_INIT = -10010, + CRYPTO_NO_SUCH_ALGORITHM = -10011, + CRYPTO_UNINITIALIZED_KEY = -10012, + GENERIC_UNKNOWN_ERROR = -10013 }; static inline std::unique_ptr& getTransportFactoryInstance() { @@ -176,7 +176,8 @@ static std::tuple, T> decodeData(CborConverter& cb, const std::unique_ptr item(nullptr); T errorCode = T::OK; std::tie(item, errorCode) = cb.decodeData(response, hasErrorCode); - if (ErrorCode::OK != errorCode) + + if (T::OK != errorCode) errorCode = translateExtendedErrorsToHalErrors(errorCode); return {std::move(item), errorCode}; } @@ -1285,8 +1286,8 @@ Return<::android::hardware::keymaster::V4_1::ErrorCode> JavacardKeymaster4Device if(ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData<::android::hardware::keymaster::V4_1::ErrorCode>( - std::vector(cborOutData.begin(), cborOutData.end()-2), true); + std::tie(item, errorCode) = decodeData<::android::hardware::keymaster::V4_1::ErrorCode>( + cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); } return errorCode; } @@ -1302,8 +1303,8 @@ Return<::android::hardware::keymaster::V4_1::ErrorCode> JavacardKeymaster4Device if(ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, errorCode) = cborConverter_.decodeData<::android::hardware::keymaster::V4_1::ErrorCode>( - std::vector(cborOutData.begin(), cborOutData.end()-2), true); + std::tie(item, errorCode) = decodeData<::android::hardware::keymaster::V4_1::ErrorCode>( + cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); } return errorCode; } From 489e175a4ef80bd03e90f2ae8753b718d5c9cb17 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 20 Jan 2021 22:17:20 +0530 Subject: [PATCH 14/24] certificate chain input validation --- .../com/android/javacard/keymaster/KMAndroidSEProvider.java | 3 +++ .../src/com/android/javacard/keymaster/KMJCardSimulator.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java index e18ae600..b2833e8b 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java @@ -1143,6 +1143,9 @@ public void persistPartialCertificateChain(byte[] buf, short offset, short len, // Next single byte holds the array header. // Next 3 bytes holds the Byte array header with the cert1 length. // Next 3 bytes holds the Byte array header with the cert2 length. + if (totalLen > CERT_CHAIN_MAX_SIZE) { + KMException.throwIt(KMError.INVALID_INPUT_LENGTH); + } short persistedLen = Util.getShort(certificateChain, (short) 0); if (persistedLen > totalLen) { KMException.throwIt(KMError.INVALID_INPUT_LENGTH); diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java index eb50fd6d..a9a6a937 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java @@ -1246,6 +1246,9 @@ public void persistPartialCertificateChain(byte[] buf, short offset, // Next single byte holds the array header. // Next 3 bytes holds the Byte array header with the cert1 length. // Next 3 bytes holds the Byte array header with the cert2 length. + if (totalLen > CERT_CHAIN_MAX_SIZE) { + KMException.throwIt(KMError.INVALID_INPUT_LENGTH); + } short persistedLen = Util.getShort(certificateChain, (short) 0); if (persistedLen > totalLen) { KMException.throwIt(KMError.INVALID_INPUT_LENGTH); From 7d34a133433ab6de1fedfb692f976b9ad678c148 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Thu, 21 Jan 2021 12:18:38 +0530 Subject: [PATCH 15/24] 1. Renamed INS_SHARED_SECRET to INS_PRESHARED_SECRET. 2. Removed AuthorityKeyIdentifier. --- .../keymaster/KMAttestationCertImpl.java | 93 +------------------ .../keymaster/KMAttestationCertImpl.java | 81 ---------------- .../javacard/keymaster/KMAttestationCert.java | 8 -- .../javacard/keymaster/KMKeymasterApplet.java | 26 ++---- .../javacard/keymaster/KMRepository.java | 48 ++++------ 5 files changed, 32 insertions(+), 224 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java index 40bd751b..5a36e3c3 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java @@ -30,8 +30,6 @@ public class KMAttestationCertImpl implements KMAttestationCert { private static final byte[] androidExtn = { 0x06, 0x0A, 0X2B, 0X06, 0X01, 0X04, 0X01, (byte) 0XD6, 0X79, 0X02, 0X01, 0X11 }; - // Authority Key Identifier Extn - 2.5.29.35 - private static final byte[] authKeyIdExtn = {0x06, 0x03, 0X55, 0X1D, 0X23}; private static final short ECDSA_MAX_SIG_LEN = 72; //Signature algorithm identifier - always ecdsaWithSha256 - 1.2.840.10045.4.3.2 @@ -95,7 +93,6 @@ public class KMAttestationCertImpl implements KMAttestationCert { private static short verifiedBootKey; private static byte verifiedState; private static short verifiedHash; - private static short authKey; private static short issuer; private static short signPriv; @@ -138,7 +135,6 @@ private static void init() { verifiedState = 0; rsaCert = true; deviceLocked = 0; - authKey = 0; signPriv = 0; } @@ -148,12 +144,6 @@ public KMAttestationCert verifiedBootHash(short obj) { return this; } - @Override - public KMAttestationCert authKey(short obj) { - authKey = obj; - return this; - } - @Override public KMAttestationCert verifiedBootKey(short obj) { verifiedBootKey = obj; @@ -259,46 +249,6 @@ private void createKeyUsage(short tag) { } } - private static void encodeCert( - short buf, - short keyChar, - short uniqueId, - short notBefore, - short notAfter, - short pubKey, - short attChallenge, - short attAppId, - boolean rsaCert) { - init(); - stack = KMByteBlob.cast(buf).getBuffer(); - start = KMByteBlob.cast(buf).getStartOff(); - length = KMByteBlob.cast(buf).length(); - stackPtr = (short) (start + length); - /* KMAttestationCertImpl.attChallenge = attChallenge; - KMAttestationCertImpl.attAppId = attAppId; - KMAttestationCertImpl.hwParams = KMKeyCharacteristics.cast(keyChar).getHardwareEnforced(); - KMAttestationCertImpl.swParams = KMKeyCharacteristics.cast(keyChar).getSoftwareEnforced(); - KMAttestationCertImpl.notBefore = notBefore; - KMAttestationCertImpl.notAfter = notAfter; - KMAttestationCertImpl.pubKey = pubKey; - KMAttestationCertImpl.uniqueId = uniqueId; - - */ - short last = stackPtr; - decrementStackPtr((short) 256); - signatureOffset = stackPtr; - pushBitStringHeader((byte) 0, (short) (last - stackPtr)); - // signatureOffset = pushSignature(null, (short) 0, (short) 256); - pushAlgorithmId(X509SignAlgIdentifier); - tbsLength = stackPtr; - pushTbsCert(rsaCert); - tbsOffset = stackPtr; - tbsLength = (short) (tbsLength - tbsOffset); - pushSequenceHeader((short) (last - stackPtr)); - // print(stack, stackPtr, (short)(last - stackPtr)); - certStart = stackPtr; - } - private static void pushTbsCert(boolean rsaCert) { short last = stackPtr; pushExtensions(); @@ -335,7 +285,6 @@ private static void pushExtensions() { short last = stackPtr; // byte keyusage = 0; // byte unusedBits = 8; - pushAuthKeyId(); /* if (KMEnumArrayTag.contains(KMType.PURPOSE, KMType.SIGN, hwParams)) { keyusage = (byte) (keyusage | keyUsageSign); @@ -480,19 +429,13 @@ private static void pushKeyDescription() { private static void pushSWParams() { short last = stackPtr; - // ATTESTATION_APPLICATION_ID 709 is softwareEnforced. + // Below are the allowed softwareEnforced Authorization tags inside the attestation certificate's extension. short[] tagIds = { KMType.ATTESTATION_APPLICATION_ID, KMType.CREATION_DATETIME, KMType.USAGE_EXPIRE_DATETIME, KMType.ORIGINATION_EXPIRE_DATETIME, KMType.ACTIVE_DATETIME, KMType.UNLOCKED_DEVICE_REQUIRED }; byte index = 0; do { - /* - if(tagIds[index] == KMType.ATTESTATION_APPLICATION_ID) { - pushAttIds(tagIds[index]); - continue; - } - */ pushParams(swParams, swParamsIndex, tagIds[index]); } while (++index < tagIds.length); pushSequenceHeader((short) (last - stackPtr)); @@ -500,7 +443,7 @@ private static void pushSWParams() { private static void pushHWParams() { short last = stackPtr; - // Attestation ids are not included. As per VTS attestation ids are not supported currenlty. + // Below are the allowed hardwareEnforced Authorization tags inside the attestation certificate's extension. short[] tagIds = { KMType.BOOT_PATCH_LEVEL, KMType.VENDOR_PATCH_LEVEL, KMType.OS_PATCH_LEVEL, KMType.OS_VERSION, KMType.ROOT_OF_TRUST, @@ -511,9 +454,9 @@ private static void pushHWParams() { KMType.ROLLBACK_RESISTANCE, KMType.RSA_PUBLIC_EXPONENT, KMType.ECCURVE, KMType.PADDING, KMType.DIGEST, KMType.KEYSIZE, KMType.ALGORITHM, KMType.PURPOSE }; + byte index = 0; do { - // if(pushAttIds(tagIds[index])) continue; if (tagIds[index] == KMType.ROOT_OF_TRUST) { pushRoT(); continue; @@ -600,7 +543,6 @@ private static void pushTag(short tag) { // } private static void pushRoT() { short last = stackPtr; - byte val = 0x00; // verified boot hash // pushOctetString(repo.verifiedBootHash, (short) 0, (short) repo.verifiedBootHash.length); pushOctetString( @@ -764,39 +706,10 @@ private static void pushKeyUsage(byte keyUsage, byte unusedBits) { pushSequenceHeader((short) (last - stackPtr)); } - // SEQUENCE {ObjId, OCTET STRING{SEQUENCE{[0]keyIdentifier}}} - private static void pushAuthKeyId() { - short last = stackPtr; - // if (repo.getAuthKeyId() == 0) return; - if (authKey == 0) return; - - pushKeyIdentifier( - KMByteBlob.cast(authKey).getBuffer(), - KMByteBlob.cast(authKey).getStartOff(), - KMByteBlob.cast(authKey).length()); - pushSequenceHeader((short) (last - stackPtr)); - pushOctetStringHeader((short) (last - stackPtr)); - pushBytes(authKeyIdExtn, (short) 0, (short) authKeyIdExtn.length); // ObjId - pushSequenceHeader((short) (last - stackPtr)); - } - - private static void pushKeyIdentifier(byte[] buf, short start, short len) { - pushBytes(buf, start, len); // keyIdentifier - pushLength(len); // len - pushByte((byte) 0x80); // Context specific tag [0] - } - private static void pushAlgorithmId(byte[] algId) { pushBytes(algId, (short) 0, (short) algId.length); } - private static short pushSignature(byte[] buf, short start, short len) { - pushBytes(buf, start, len); - short signatureOff = stackPtr; - pushBitStringHeader((byte) 0, len); - return signatureOff; - } - private static void pushIntegerHeader(short len) { pushLength(len); pushByte((byte) 0x02); diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java index 136ec4ba..cd9567d3 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java @@ -30,8 +30,6 @@ public class KMAttestationCertImpl implements KMAttestationCert { private static final byte[] androidExtn = { 0x06, 0x0A, 0X2B, 0X06, 0X01, 0X04, 0X01, (byte) 0XD6, 0X79, 0X02, 0X01, 0X11 }; - // Authority Key Identifier Extn - 2.5.29.35 - private static final byte[] authKeyIdExtn = {0x06, 0x03, 0X55, 0X1D, 0X23}; private static final short ECDSA_MAX_SIG_LEN = 72; //Signature algorithm identifier - always ecdsaWithSha256 - 1.2.840.10045.4.3.2 @@ -95,7 +93,6 @@ public class KMAttestationCertImpl implements KMAttestationCert { private static short verifiedBootKey; private static byte verifiedState; private static short verifiedHash; - private static short authKey; private static short issuer; private static short signPriv; @@ -138,7 +135,6 @@ private static void init() { verifiedState = 0; rsaCert = true; deviceLocked = 0; - authKey = 0; signPriv = 0; } @@ -148,12 +144,6 @@ public KMAttestationCert verifiedBootHash(short obj) { return this; } - @Override - public KMAttestationCert authKey(short obj) { - authKey = obj; - return this; - } - @Override public KMAttestationCert verifiedBootKey(short obj) { verifiedBootKey = obj; @@ -259,46 +249,6 @@ private void createKeyUsage(short tag) { } } - private static void encodeCert( - short buf, - short keyChar, - short uniqueId, - short notBefore, - short notAfter, - short pubKey, - short attChallenge, - short attAppId, - boolean rsaCert) { - init(); - stack = KMByteBlob.cast(buf).getBuffer(); - start = KMByteBlob.cast(buf).getStartOff(); - length = KMByteBlob.cast(buf).length(); - stackPtr = (short) (start + length); - /* KMAttestationCertImpl.attChallenge = attChallenge; - KMAttestationCertImpl.attAppId = attAppId; - KMAttestationCertImpl.hwParams = KMKeyCharacteristics.cast(keyChar).getHardwareEnforced(); - KMAttestationCertImpl.swParams = KMKeyCharacteristics.cast(keyChar).getSoftwareEnforced(); - KMAttestationCertImpl.notBefore = notBefore; - KMAttestationCertImpl.notAfter = notAfter; - KMAttestationCertImpl.pubKey = pubKey; - KMAttestationCertImpl.uniqueId = uniqueId; - - */ - short last = stackPtr; - decrementStackPtr((short) 256); - signatureOffset = stackPtr; - pushBitStringHeader((byte) 0, (short) (last - stackPtr)); - // signatureOffset = pushSignature(null, (short) 0, (short) 256); - pushAlgorithmId(X509SignAlgIdentifier); - tbsLength = stackPtr; - pushTbsCert(rsaCert); - tbsOffset = stackPtr; - tbsLength = (short) (tbsLength - tbsOffset); - pushSequenceHeader((short) (last - stackPtr)); - // print(stack, stackPtr, (short)(last - stackPtr)); - certStart = stackPtr; - } - private static void pushTbsCert(boolean rsaCert) { short last = stackPtr; pushExtensions(); @@ -335,7 +285,6 @@ private static void pushExtensions() { short last = stackPtr; // byte keyusage = 0; // byte unusedBits = 8; - pushAuthKeyId(); /* if (KMEnumArrayTag.contains(KMType.PURPOSE, KMType.SIGN, hwParams)) { keyusage = (byte) (keyusage | keyUsageSign); @@ -594,7 +543,6 @@ private static void pushTag(short tag) { // } private static void pushRoT() { short last = stackPtr; - byte val = 0x00; // verified boot hash // pushOctetString(repo.verifiedBootHash, (short) 0, (short) repo.verifiedBootHash.length); pushOctetString( @@ -758,39 +706,10 @@ private static void pushKeyUsage(byte keyUsage, byte unusedBits) { pushSequenceHeader((short) (last - stackPtr)); } - // SEQUENCE {ObjId, OCTET STRING{SEQUENCE{[0]keyIdentifier}}} - private static void pushAuthKeyId() { - short last = stackPtr; - // if (repo.getAuthKeyId() == 0) return; - if (authKey == 0) return; - - pushKeyIdentifier( - KMByteBlob.cast(authKey).getBuffer(), - KMByteBlob.cast(authKey).getStartOff(), - KMByteBlob.cast(authKey).length()); - pushSequenceHeader((short) (last - stackPtr)); - pushOctetStringHeader((short) (last - stackPtr)); - pushBytes(authKeyIdExtn, (short) 0, (short) authKeyIdExtn.length); // ObjId - pushSequenceHeader((short) (last - stackPtr)); - } - - private static void pushKeyIdentifier(byte[] buf, short start, short len) { - pushBytes(buf, start, len); // keyIdentifier - pushLength(len); // len - pushByte((byte) 0x80); // Context specific tag [0] - } - private static void pushAlgorithmId(byte[] algId) { pushBytes(algId, (short) 0, (short) algId.length); } - private static short pushSignature(byte[] buf, short start, short len) { - pushBytes(buf, start, len); - short signatureOff = stackPtr; - pushBitStringHeader((byte) 0, len); - return signatureOff; - } - private static void pushIntegerHeader(short len) { pushLength(len); pushByte((byte) 0x02); diff --git a/Applet/src/com/android/javacard/keymaster/KMAttestationCert.java b/Applet/src/com/android/javacard/keymaster/KMAttestationCert.java index f15fbfcb..d27f015a 100644 --- a/Applet/src/com/android/javacard/keymaster/KMAttestationCert.java +++ b/Applet/src/com/android/javacard/keymaster/KMAttestationCert.java @@ -32,14 +32,6 @@ public interface KMAttestationCert { */ KMAttestationCert verifiedBootState(byte val); - /** - * Set authentication key Id from CA Certificate set during provisioning. - * - * @param obj This is a KMByteBlob containing authentication Key Id. - * @return instance of KMAttestationCert - */ - KMAttestationCert authKey(short obj); - /** * Set uniqueId received from CA certificate during provisioning. * diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index a37e6aa8..a1157f34 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -75,7 +75,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe private static final byte INS_PROVISION_ATTESTATION_CERT_CHAIN_CMD = INS_BEGIN_KM_CMD + 2; //0x02 private static final byte INS_PROVISION_ATTESTATION_CERT_PARAMS_CMD = INS_BEGIN_KM_CMD + 3; //0x03 private static final byte INS_PROVISION_ATTEST_IDS_CMD = INS_BEGIN_KM_CMD + 4; //0x04 - private static final byte INS_PROVISION_SHARED_SECRET_CMD = INS_BEGIN_KM_CMD + 5; //0x05 + private static final byte INS_PROVISION_PRESHARED_SECRET_CMD = INS_BEGIN_KM_CMD + 5; //0x05 private static final byte INS_SET_BOOT_PARAMS_CMD = INS_BEGIN_KM_CMD + 6; //0x06 private static final byte INS_LOCK_PROVISIONING_CMD = INS_BEGIN_KM_CMD + 7; //0x07 private static final byte INS_GET_PROVISION_STATUS_CMD = INS_BEGIN_KM_CMD + 8; //0x08 @@ -113,7 +113,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe private static final byte PROVISION_STATUS_ATTESTATION_CERT_CHAIN = 0x02; private static final byte PROVISION_STATUS_ATTESTATION_CERT_PARAMS = 0x04; private static final byte PROVISION_STATUS_ATTEST_IDS = 0x08; - private static final byte PROVISION_STATUS_SHARED_SECRET = 0x10; + private static final byte PROVISION_STATUS_PRESHARED_SECRET = 0x10; private static final byte PROVISION_STATUS_BOOT_PARAM = 0x20; private static final byte PROVISION_STATUS_PROVISIONING_LOCKED = 0x40; @@ -339,9 +339,9 @@ public void process(APDU apdu) { sendError(apdu, KMError.OK); return; - case INS_PROVISION_SHARED_SECRET_CMD: + case INS_PROVISION_PRESHARED_SECRET_CMD: processProvisionSharedSecretCmd(apdu); - provisionStatus |= KMKeymasterApplet.PROVISION_STATUS_SHARED_SECRET; + provisionStatus |= KMKeymasterApplet.PROVISION_STATUS_PRESHARED_SECRET; sendError(apdu, KMError.OK); return; @@ -461,9 +461,11 @@ && isProvisioningComplete())) { sendError(apdu, mapISOErrorToKMError(exp.getReason())); freeOperations(); } catch (CryptoException e) { + e.printStackTrace(); freeOperations(); sendError(apdu, mapCryptoErrorToKMError(e.getReason())); } catch (Exception e) { + e.printStackTrace(); freeOperations(); sendError(apdu, KMError.GENERIC_UNKNOWN_ERROR); } finally { @@ -476,7 +478,7 @@ private boolean isProvisioningComplete() { if((0 != (provisionStatus & PROVISION_STATUS_ATTESTATION_KEY)) && (0 != (provisionStatus & PROVISION_STATUS_ATTESTATION_CERT_CHAIN)) && (0 != (provisionStatus & PROVISION_STATUS_ATTESTATION_CERT_PARAMS)) - && (0 != (provisionStatus & PROVISION_STATUS_SHARED_SECRET)) + && (0 != (provisionStatus & PROVISION_STATUS_PRESHARED_SECRET)) && (0 != (provisionStatus & PROVISION_STATUS_BOOT_PARAM))) { return true; } else { @@ -632,10 +634,9 @@ private void processProvisionAttestationCertParams(APDU apdu) { receiveIncoming(apdu); // Arguments short blob = KMByteBlob.exp(); - short argsProto = KMArray.instance((short) 3); + short argsProto = KMArray.instance((short) 2); KMArray.cast(argsProto).add((short) 0, blob); // Cert - DER encoded issuer KMArray.cast(argsProto).add((short) 1, blob); // Cert - Expiry Time - KMArray.cast(argsProto).add((short) 2, blob); // Cert - Auth Key Id // Decode the argument. short args = decoder.decode(argsProto, buffer, bufferStartOffset, bufferLength); //reclaim memory @@ -654,13 +655,6 @@ private void processProvisionAttestationCertParams(APDU apdu) { KMByteBlob.cast(tmpVariables[0]).getBuffer(), KMByteBlob.cast(tmpVariables[0]).getStartOff(), KMByteBlob.cast(tmpVariables[0]).length()); - - // Auth Key Id - from cert associated with imported attestation key. - tmpVariables[0] = KMArray.cast(args).get((short) 2); - repository.setAuthKeyId( - KMByteBlob.cast(tmpVariables[0]).getBuffer(), - KMByteBlob.cast(tmpVariables[0]).getStartOff(), - KMByteBlob.cast(tmpVariables[0]).length()); } private void processProvisionAttestationCertChainCmd(APDU apdu) { @@ -1438,9 +1432,7 @@ private void processAttestKeyCmd(APDU apdu) { addTags(KMKeyCharacteristics.cast(data[KEY_CHARACTERISTICS]).getHardwareEnforced(), true, cert); addTags( KMKeyCharacteristics.cast(data[KEY_CHARACTERISTICS]).getSoftwareEnforced(), false, cert); - if (repository.getAuthKeyId() != 0) { - cert.authKey(repository.getAuthKeyId()); - } + cert.deviceLocked(repository.getDeviceLock()); cert.issuer(repository.getIssuer()); cert.publicKey(data[PUB_KEY]); diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index c2809c08..4bcdf28b 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -29,7 +29,7 @@ */ public class KMRepository implements KMUpgradable { // Data table configuration - public static final short DATA_INDEX_SIZE = 33; + public static final short DATA_INDEX_SIZE = 32; public static final short DATA_INDEX_ENTRY_SIZE = 4; public static final short DATA_MEM_SIZE = 2048; public static final short HEAP_SIZE = 10000; @@ -50,26 +50,25 @@ public class KMRepository implements KMUpgradable { public static final byte ATT_ID_MANUFACTURER = 6; public static final byte ATT_ID_MODEL = 7; public static final byte ATT_EC_KEY = 12; - public static final byte CERT_AUTH_KEY_ID = 13; - public static final byte CERT_ISSUER = 14; - public static final byte CERT_EXPIRY_TIME = 15; - public static final byte BOOT_OS_VERSION = 16; - public static final byte BOOT_OS_PATCH = 17; - public static final byte VENDOR_PATCH_LEVEL = 18; - public static final byte BOOT_PATCH_LEVEL = 19; - public static final byte BOOT_VERIFIED_BOOT_KEY = 20; - public static final byte BOOT_VERIFIED_BOOT_HASH = 21; - public static final byte BOOT_VERIFIED_BOOT_STATE = 22; - public static final byte BOOT_DEVICE_LOCKED_STATUS = 23; - public static final byte BOOT_DEVICE_LOCKED_TIME = 24; - public static final byte AUTH_TAG_1 = 25; - public static final byte AUTH_TAG_2 = 26; - public static final byte AUTH_TAG_3 = 27; - public static final byte AUTH_TAG_4 = 28; - public static final byte AUTH_TAG_5 = 29; - public static final byte AUTH_TAG_6 = 30; - public static final byte AUTH_TAG_7 = 31; - public static final byte AUTH_TAG_8 = 32; + public static final byte CERT_ISSUER = 13; + public static final byte CERT_EXPIRY_TIME = 14; + public static final byte BOOT_OS_VERSION = 15; + public static final byte BOOT_OS_PATCH = 16; + public static final byte VENDOR_PATCH_LEVEL = 17; + public static final byte BOOT_PATCH_LEVEL = 18; + public static final byte BOOT_VERIFIED_BOOT_KEY = 19; + public static final byte BOOT_VERIFIED_BOOT_HASH = 20; + public static final byte BOOT_VERIFIED_BOOT_STATE = 21; + public static final byte BOOT_DEVICE_LOCKED_STATUS = 22; + public static final byte BOOT_DEVICE_LOCKED_TIME = 23; + public static final byte AUTH_TAG_1 = 24; + public static final byte AUTH_TAG_2 = 25; + public static final byte AUTH_TAG_3 = 26; + public static final byte AUTH_TAG_4 = 27; + public static final byte AUTH_TAG_5 = 28; + public static final byte AUTH_TAG_6 = 29; + public static final byte AUTH_TAG_7 = 30; + public static final byte AUTH_TAG_8 = 31; // Data Item sizes public static final short MASTER_KEY_SIZE = 16; @@ -567,13 +566,6 @@ public void setCertExpiryTime(byte[] buf, short start, short len) { writeDataEntry(CERT_EXPIRY_TIME, buf,start,len); } - public short getAuthKeyId() { - return readData(CERT_AUTH_KEY_ID); - } - - public void setAuthKeyId(byte[] buf, short start, short len) { - writeDataEntry(CERT_AUTH_KEY_ID,buf,start,len); - } private static final byte[] zero = {0,0,0,0,0,0,0,0}; public short getOsVersion(){ From 606138149106d4dee78adc51eb2b589e73a445d4 Mon Sep 17 00:00:00 2001 From: bvenkateswarlu Date: Thu, 21 Jan 2021 14:19:13 +0530 Subject: [PATCH 16/24] Fixed the issue with KMFunctionalTest --- .../src/com/android/javacard/keymaster/KMUtils.java | 2 +- .../src/com/android/javacard/keymaster/KMUtils.java | 2 +- .../com/android/javacard/test/KMFunctionalTest.java | 13 +++++-------- .../javacard/keymaster/KMKeymasterApplet.java | 2 -- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java index 08309f86..2e0f8d99 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java @@ -20,7 +20,7 @@ public class KMUtils { public static final byte[] fourYrsMsec = { 0, 0, 0, 0x1D, 0x63, (byte) 0xC3, 0x7F, 0x00 }; // 126227808000 msec public static final byte[] firstJan2020 = { - 0, 0, 0x01, 0x76, (byte) 0xBB, 0x3E, (byte) 0x70, 0x40 }; // 1609459200000 msec + 0, 0, 0x01, 0x6F, 0x5E, 0x66, (byte)0xE8, 0x00 }; // 1577836800000 msec public static final byte[] firstJan2051 = { 0, 0, 0x02, 0x53, 0x26, (byte) 0x0E, (byte) 0x1C, 0x00 }; // 2556144000000 // msec diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java index 08309f86..4569fb15 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java @@ -20,7 +20,7 @@ public class KMUtils { public static final byte[] fourYrsMsec = { 0, 0, 0, 0x1D, 0x63, (byte) 0xC3, 0x7F, 0x00 }; // 126227808000 msec public static final byte[] firstJan2020 = { - 0, 0, 0x01, 0x76, (byte) 0xBB, 0x3E, (byte) 0x70, 0x40 }; // 1609459200000 msec + 0, 0, 0x01, 0x6F, 0x5E, 0x66, (byte)0xE8, 0x00 }; // 1577836800000 msec public static final byte[] firstJan2051 = { 0, 0, 0x02, 0x53, 0x26, (byte) 0x0E, (byte) 0x1C, 0x00 }; // 2556144000000 // msec diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index fb248f2d..0e6408d2 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -59,7 +59,7 @@ public class KMFunctionalTest { private static final byte INS_PROVISION_ATTESTATION_CERT_CHAIN_CMD = INS_BEGIN_KM_CMD + 2; //0x02 private static final byte INS_PROVISION_ATTESTATION_CERT_PARAMS_CMD = INS_BEGIN_KM_CMD + 3; //0x03 private static final byte INS_PROVISION_ATTEST_IDS_CMD = INS_BEGIN_KM_CMD + 4; //0x04 - private static final byte INS_PROVISION_SHARED_SECRET_CMD = INS_BEGIN_KM_CMD + 5; //0x05 + private static final byte INS_PROVISION_PRESHARED_SECRET_CMD = INS_BEGIN_KM_CMD + 5; //0x05 private static final byte INS_SET_BOOT_PARAMS_CMD = INS_BEGIN_KM_CMD + 6; //0x06 private static final byte INS_LOCK_PROVISIONING_CMD = INS_BEGIN_KM_CMD + 7; //0x07 private static final byte INS_GET_PROVISION_STATUS_CMD = INS_BEGIN_KM_CMD + 8; //0x08 @@ -538,16 +538,13 @@ private void provisionSigningKey(CardSimulator simulator) { private void provisionCertificateParams(CardSimulator simulator) { - short arrPtr = KMArray.instance((short) 3); + short arrPtr = KMArray.instance((short) 2); short byteBlob1 = KMByteBlob.instance(X509Issuer, (short) 0, (short) X509Issuer.length); KMArray.cast(arrPtr).add((short) 0, byteBlob1); short byteBlob2 = KMByteBlob.instance(expiryTime, (short) 0, (short) expiryTime.length); KMArray.cast(arrPtr).add((short) 1, byteBlob2); - short byteBlob3 = KMByteBlob.instance(authKeyId, (short) 0, - (short) authKeyId.length); - KMArray.cast(arrPtr).add((short) 2, byteBlob3); CommandAPDU apdu = encodeApdu( (byte) INS_PROVISION_ATTESTATION_CERT_PARAMS_CMD, arrPtr); @@ -565,7 +562,7 @@ private void provisionSharedSecret(CardSimulator simulator) { (short) sharedKeySecret.length); KMArray.cast(arrPtr).add((short) 0, byteBlob); - CommandAPDU apdu = encodeApdu((byte) INS_PROVISION_SHARED_SECRET_CMD, + CommandAPDU apdu = encodeApdu((byte) INS_PROVISION_PRESHARED_SECRET_CMD, arrPtr); // print(commandAPDU.getBytes()); ResponseAPDU response = simulator.transmitCommand(apdu); @@ -1622,11 +1619,11 @@ public void testImportWrappedKey(){ byte[] nonce = new byte[12]; cryptoProvider.newRandomNumber(nonce,(short)0,(short)12); byte[] authData = "Auth Data".getBytes(); - byte[] authTag = new byte[12]; + byte[] authTag = new byte[16]; cryptoProvider.aesGCMEncrypt(transportKeyMaterial,(short)0,(short)32,wrappedKey, (short)0,(short)16,encWrappedKey,(short)0, nonce,(short)0, (short)12,authData,(short)0,(short)authData.length, - authTag, (short)0, (short)12); + authTag, (short)0, (short)16); byte[] maskingKey = {1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0}; byte[] maskedTransportKey = new byte[32]; for(int i=0; i< maskingKey.length;i++){ diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index a1157f34..b9ae5ce4 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -461,11 +461,9 @@ && isProvisioningComplete())) { sendError(apdu, mapISOErrorToKMError(exp.getReason())); freeOperations(); } catch (CryptoException e) { - e.printStackTrace(); freeOperations(); sendError(apdu, mapCryptoErrorToKMError(e.getReason())); } catch (Exception e) { - e.printStackTrace(); freeOperations(); sendError(apdu, KMError.GENERIC_UNKNOWN_ERROR); } finally { From 2023d6da399ffc2b377b6e3c7b605a8d85af0873 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Thu, 21 Jan 2021 14:23:56 +0530 Subject: [PATCH 17/24] Renamed SHARED_SECRET to PRESHARED_SECRET --- HAL/keymaster/4.1/Provision.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/HAL/keymaster/4.1/Provision.cpp b/HAL/keymaster/4.1/Provision.cpp index 34b6420d..e90cd7ef 100644 --- a/HAL/keymaster/4.1/Provision.cpp +++ b/HAL/keymaster/4.1/Provision.cpp @@ -49,7 +49,7 @@ enum class Instruction { INS_PROVISION_CERT_CHAIN_CMD = INS_BEGIN_KM_CMD+2, INS_PROVISION_CERT_PARAMS_CMD = INS_BEGIN_KM_CMD+3, INS_PROVISION_ATTEST_IDS_CMD = INS_BEGIN_KM_CMD+4, - INS_PROVISION_SHARED_SECRET_CMD = INS_BEGIN_KM_CMD+5, + INS_PROVISION_PRESHARED_SECRET_CMD = INS_BEGIN_KM_CMD+5, INS_SET_BOOT_PARAMS_CMD = INS_BEGIN_KM_CMD+6, INS_LOCK_PROVISIONING_CMD = INS_BEGIN_KM_CMD+7, INS_GET_PROVISION_STATUS_CMD = INS_BEGIN_KM_CMD+8, @@ -61,7 +61,7 @@ enum ProvisionStatus { PROVISION_STATUS_ATTESTATION_CERT_CHAIN = 0x02, PROVISION_STATUS_ATTESTATION_CERT_PARAMS = 0x04, PROVISION_STATUS_ATTEST_IDS = 0x08, - PROVISION_STATUS_SHARED_SECRET = 0x10, + PROVISION_STATUS_PRESHARED_SECRET = 0x10, PROVISION_STATUS_BOOT_PARAM = 0x20, PROVISION_STATUS_PROVISIONING_LOCKED = 0x40, }; @@ -406,7 +406,7 @@ static ErrorCode provisionAttestationIDs(std::unique_ptr& transport) { ErrorCode errorCode = ErrorCode::OK; cppbor::Array array; - Instruction ins = Instruction::INS_PROVISION_SHARED_SECRET_CMD; + Instruction ins = Instruction::INS_PROVISION_PRESHARED_SECRET_CMD; std::vector response; std::vector masterKey(kFakeKeyAgreementKey, kFakeKeyAgreementKey + sizeof(kFakeKeyAgreementKey)/sizeof(kFakeKeyAgreementKey[0])); @@ -484,7 +484,7 @@ static bool isSEProvisioned(uint64_t status) { if(status != (ProvisionStatus::PROVISION_STATUS_ATTESTATION_KEY | ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_CHAIN | ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_PARAMS | ProvisionStatus::PROVISION_STATUS_ATTEST_IDS | - ProvisionStatus::PROVISION_STATUS_SHARED_SECRET | ProvisionStatus::PROVISION_STATUS_BOOT_PARAM + ProvisionStatus::PROVISION_STATUS_PRESHARED_SECRET | ProvisionStatus::PROVISION_STATUS_BOOT_PARAM |ProvisionStatus::PROVISION_STATUS_PROVISIONING_LOCKED)) { ret = false; } From 5aae09d3dcc008e3d855ebf5f36f77337c4e01c4 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Thu, 21 Jan 2021 17:18:47 +0530 Subject: [PATCH 18/24] Corrected the month milliseconds hex value --- .../src/com/android/javacard/keymaster/KMUtils.java | 2 +- .../src/com/android/javacard/keymaster/KMUtils.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java index 2e0f8d99..d9d7e111 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMUtils.java @@ -15,7 +15,7 @@ public class KMUtils { public static final byte[] oneMonthMsec = { 0, 0, 0, 0, (byte) 0x9C,(byte) 0xBE, (byte) 0xBD, 0x50}; // 2629746000 msec public static final byte[] oneYearMsec = { - 0, 0, 0, 0x75, (byte) 0x8F, 0x0D, (byte) 0xFC, 0x00 }; // 31556952000 msec + 0, 0, 0, 0x07, 0x58, (byte) 0xF0, (byte) 0xDF, (byte) 0xC0 }; // 31556952000 msec // Leap year + 3 yrs public static final byte[] fourYrsMsec = { 0, 0, 0, 0x1D, 0x63, (byte) 0xC3, 0x7F, 0x00 }; // 126227808000 msec diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java index 4569fb15..41bbe1ff 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMUtils.java @@ -15,7 +15,7 @@ public class KMUtils { public static final byte[] oneMonthMsec = { 0, 0, 0, 0, (byte) 0x9C,(byte) 0xBE, (byte) 0xBD, 0x50}; // 2629746000 msec public static final byte[] oneYearMsec = { - 0, 0, 0, 0x75, (byte) 0x8F, 0x0D, (byte) 0xFC, 0x00 }; // 31556952000 msec + 0, 0, 0, 0x07, 0x58, (byte) 0xF0, (byte) 0xDF, (byte) 0xC0 }; // 31556952000 msec // Leap year + 3 yrs public static final byte[] fourYrsMsec = { 0, 0, 0, 0x1D, 0x63, (byte) 0xC3, 0x7F, 0x00 }; // 126227808000 msec From 3dadaad1d7e0d51ed674780d612d0368d5a17412 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Thu, 21 Jan 2021 17:25:31 +0530 Subject: [PATCH 19/24] Removed authenticationIdentifier from cert params --- HAL/keymaster/4.1/Provision.cpp | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/HAL/keymaster/4.1/Provision.cpp b/HAL/keymaster/4.1/Provision.cpp index e90cd7ef..6249e34d 100644 --- a/HAL/keymaster/4.1/Provision.cpp +++ b/HAL/keymaster/4.1/Provision.cpp @@ -125,28 +125,6 @@ static inline void getDerSubjectName(X509* x509, std::vector& subject) subject.insert(subject.begin(), subjectDer, subjectDer+len); } -static inline void getAuthorityKeyIdentifier(X509* x509, std::vector& authKeyId) { - long xlen; - int tag, xclass; - - int loc = X509_get_ext_by_NID(x509, NID_authority_key_identifier, -1); - X509_EXTENSION *ext = X509_get_ext(x509, loc); - if(ext == NULL) { - LOG(ERROR) << " Failed to read authority key identifier."; - return; - } - - ASN1_OCTET_STRING *asn1AuthKeyId = X509_EXTENSION_get_data(ext); - const uint8_t *strAuthKeyId = ASN1_STRING_get0_data(asn1AuthKeyId); - int strAuthKeyIdLen = ASN1_STRING_length(asn1AuthKeyId); - int ret = ASN1_get_object(&strAuthKeyId, &xlen, &tag, &xclass, strAuthKeyIdLen); - if (ret == 0x80 || strAuthKeyId == NULL) { - LOG(ERROR) << "Failed to get the auth key identifier from ASN1 sequence."; - return; - } - authKeyId.insert(authKeyId.begin(), strAuthKeyId, strAuthKeyId + xlen); -} - static inline void getNotAfter(X509* x509, std::vector& notAfterDate) { const ASN1_TIME* notAfter = X509_get0_notAfter(x509); if(notAfter == NULL) { @@ -334,7 +312,6 @@ static ErrorCode provisionAttestationCertificateParams(std::unique_ptr response; X509 *x509 = NULL; std::vector subject; - std::vector authorityKeyIdentifier; std::vector notAfter; /* Subject, AuthorityKeyIdentifier and Expirty time of the root certificate are required by javacard. */ @@ -345,8 +322,6 @@ static ErrorCode provisionAttestationCertificateParams(std::unique_ptr cborData = array.encode(); if(ErrorCode::OK != (errorCode = sendProvisionData(transport, ins, cborData, response))) { From ee1228fc070a8e927e6d19eec934a34b66905b77 Mon Sep 17 00:00:00 2001 From: bvenkateswarlu Date: Thu, 21 Jan 2021 20:37:58 +0530 Subject: [PATCH 20/24] Fixed the issue with keyblob decryption with AndroidSEProvider --- .../com/android/javacard/keymaster/KMAndroidSEProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java index b2833e8b..4b0994f4 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java @@ -643,8 +643,8 @@ public boolean aesGCMDecrypt(byte[] aesKey, short aesKeyStart, // encrypt the secret aesGcmCipher.doFinal(encSecret, encSecretStart, encSecretLen, secret, secretStart); - verification = aesGcmCipher.verifyTag(authTag, authTagStart, (short) 12, - (short) 12); + verification = aesGcmCipher.verifyTag(authTag, authTagStart, (short) authTagLen, + (short) AES_GCM_TAG_LENGTH); return verification; } From f21600112c2211c3fe5d7da485890dbe77f67208 Mon Sep 17 00:00:00 2001 From: bvenkateswarlu Date: Fri, 22 Jan 2021 16:34:29 +0530 Subject: [PATCH 21/24] Remove MAX_USES_PER_BOOT related code as it is not supported by strongbox --- .../javacard/keymaster/KMKeyParameters.java | 4 +- .../javacard/keymaster/KMKeymasterApplet.java | 59 +------- .../javacard/keymaster/KMRepository.java | 137 +----------------- 3 files changed, 7 insertions(+), 193 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeyParameters.java b/Applet/src/com/android/javacard/keymaster/KMKeyParameters.java index 8c8475ac..f08ee388 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeyParameters.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeyParameters.java @@ -108,6 +108,7 @@ public static boolean hasUnsupportedTags(short keyParamsPtr) { KMType.BOOL_TAG, KMType.TRUSTED_USER_PRESENCE_REQUIRED, KMType.BOOL_TAG, KMType.ALLOW_WHILE_ON_BODY, KMType.UINT_TAG, KMType.MIN_SEC_BETWEEN_OPS, + KMType.UINT_TAG, KMType.MAX_USES_PER_BOOT }; byte index = 0; short tagInd; @@ -148,15 +149,12 @@ public static short makeHwEnforced(short keyParamsPtr, byte origin, KMType.ENUM_ARRAY_TAG, KMType.DIGEST, KMType.ENUM_ARRAY_TAG, KMType.PADDING, KMType.ENUM_ARRAY_TAG, KMType.BLOCK_MODE, - KMType.UINT_TAG, KMType.MIN_SEC_BETWEEN_OPS, - KMType.UINT_TAG, KMType.MAX_USES_PER_BOOT, KMType.ULONG_ARRAY_TAG, KMType.USER_SECURE_ID, KMType.BOOL_TAG, KMType.NO_AUTH_REQUIRED, KMType.UINT_TAG, KMType.AUTH_TIMEOUT, KMType.BOOL_TAG, KMType.CALLER_NONCE, KMType.UINT_TAG, KMType.MIN_MAC_LENGTH, KMType.ENUM_TAG, KMType.ECCURVE, - KMType.BOOL_TAG, KMType.TRUSTED_CONFIRMATION_REQUIRED, KMType.BOOL_TAG, KMType.INCLUDE_UNIQUE_ID, KMType.BOOL_TAG, KMType.ROLLBACK_RESISTANCE, KMType.ENUM_TAG, KMType.USER_AUTH_TYPE, diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index b9ae5ce4..467fe81a 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -901,7 +901,6 @@ private void processGetHmacSharingParamCmd(APDU apdu) { private void processDeleteAllKeysCmd(APDU apdu) { // No arguments - repository.removeAllAuthTags(); // Send ok sendError(apdu, KMError.OK); } @@ -943,12 +942,6 @@ private void processDeleteKeyCmd(APDU apdu) { if (tmpVariables[0] < 4) { KMException.throwIt(KMError.INVALID_KEY_BLOB); } - // Validate Auth Tag - data[AUTH_TAG] = KMArray.cast(data[KEY_BLOB]).get(KEY_BLOB_AUTH_TAG); - if (repository.validateAuthTag(data[AUTH_TAG])) { - // delete the auth tag - repository.removeAuthTag(data[AUTH_TAG]); - } // Send ok sendError(apdu, KMError.OK); } @@ -1156,19 +1149,11 @@ private void processUpgradeKeyCmd(APDU apdu) { } } - boolean blobPersisted = false; if (tmpVariables[5] != KMError.INVALID_ARGUMENT) { - if (repository.validateAuthTag(data[AUTH_TAG])) { - repository.removeAuthTag(data[AUTH_TAG]); - blobPersisted = true; - } // copy origin data[ORIGIN] = KMEnumTag.getValue(KMType.ORIGIN, data[HW_PARAMETERS]); // create new key blob with current os version etc. createEncryptedKeyBlob(scratchPad); - if (blobPersisted) { - repository.persistAuthTag(data[AUTH_TAG]); - } } else { data[KEY_BLOB] = KMByteBlob.instance((short) 0); } @@ -2452,7 +2437,6 @@ private void authorizeAndBeginOperation(KMOperationState op, byte[] scratchPad) authorizeDigest(op); authorizePadding(op); authorizeBlockModeAndMacLength(op); - authorizeKeyUsageForCount(); if (!validateHwToken(data[HW_TOKEN], scratchPad)) { data[HW_TOKEN] = KMType.INVALID_VALUE; } @@ -2798,25 +2782,6 @@ private boolean validateHwToken(short hwToken, byte[] scratchPad) { KMByteBlob.cast(ptr).length()); } - private void authorizeKeyUsageForCount() { - // TODO currently only short usageLimit supported - max count 32K. - short usageLimit = - KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.MAX_USES_PER_BOOT, data[HW_PARAMETERS]); - if (usageLimit == KMType.INVALID_VALUE) return; - // get current counter - short usage = repository.getRateLimitedKeyCount(data[AUTH_TAG]); - if (usage != KMType.INVALID_VALUE) { - if (usage < usageLimit) { - KMException.throwIt(KMError.KEY_MAX_OPS_EXCEEDED); - } - // increment the counter and store it back. - usage++; - repository.setRateLimitedKeyCount(data[AUTH_TAG], usage); - } else { - KMException.throwIt(KMError.UNKNOWN_ERROR); - } - } - private void processImportKeyCmd(APDU apdu) { // Receive the incoming request fully from the master into buffer. receiveIncoming(apdu); @@ -2863,6 +2828,11 @@ private void importKey(APDU apdu, byte[] scratchPad) { if (tmpVariables[3] == KMType.INVALID_VALUE) { KMException.throwIt(KMError.INVALID_ARGUMENT); } + + //Check if the tags are supported. + if(KMKeyParameters.hasUnsupportedTags(data[KEY_PARAMETERS])) { + KMException.throwIt(KMError.UNSUPPORTED_TAG); + } // Check algorithm and dispatch to appropriate handler. switch (tmpVariables[3]) { case KMType.RSA: @@ -2886,15 +2856,6 @@ private void importKey(APDU apdu, byte[] scratchPad) { } // create key blob createEncryptedKeyBlob(scratchPad); - tmpVariables[0] = - KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.MAX_USES_PER_BOOT, data[KEY_PARAMETERS]); - if (tmpVariables[0] != KMType.INVALID_VALUE) { - // before generating key, check whether max count reached - if (repository.getKeyBlobCount() > KMRepository.MAX_BLOB_STORAGE) { - KMException.throwIt(KMError.UNKNOWN_ERROR); - } - repository.persistAuthTag(data[AUTH_TAG]); - } // prepare the response tmpVariables[0] = KMArray.instance((short) 3); @@ -3407,16 +3368,6 @@ private static void processGenerateKey(APDU apdu) { data[ORIGIN] = KMType.GENERATED; createEncryptedKeyBlob(scratchPad); - tmpVariables[0] = - KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.MAX_USES_PER_BOOT, data[KEY_PARAMETERS]); - if (tmpVariables[0] != KMType.INVALID_VALUE) { - // before generating key, check whether max count reached - if (repository.getKeyBlobCount() > KMRepository.MAX_BLOB_STORAGE) { - KMException.throwIt(KMError.UNKNOWN_ERROR); - } - repository.persistAuthTag(data[AUTH_TAG]); - } - // prepare the response tmpVariables[0] = KMArray.instance((short) 3); KMArray.cast(tmpVariables[0]).add((short) 0, KMInteger.uint_16(KMError.OK)); diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index 4bcdf28b..2b95697a 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -29,7 +29,7 @@ */ public class KMRepository implements KMUpgradable { // Data table configuration - public static final short DATA_INDEX_SIZE = 32; + public static final short DATA_INDEX_SIZE = 24; public static final short DATA_INDEX_ENTRY_SIZE = 4; public static final short DATA_MEM_SIZE = 2048; public static final short HEAP_SIZE = 10000; @@ -61,14 +61,6 @@ public class KMRepository implements KMUpgradable { public static final byte BOOT_VERIFIED_BOOT_STATE = 21; public static final byte BOOT_DEVICE_LOCKED_STATUS = 22; public static final byte BOOT_DEVICE_LOCKED_TIME = 23; - public static final byte AUTH_TAG_1 = 24; - public static final byte AUTH_TAG_2 = 25; - public static final byte AUTH_TAG_3 = 26; - public static final byte AUTH_TAG_4 = 27; - public static final byte AUTH_TAG_5 = 28; - public static final byte AUTH_TAG_6 = 29; - public static final byte AUTH_TAG_7 = 30; - public static final byte AUTH_TAG_8 = 31; // Data Item sizes public static final short MASTER_KEY_SIZE = 16; @@ -82,9 +74,6 @@ public class KMRepository implements KMUpgradable { public static final short DEVICE_LOCK_TS_SIZE = 8; public static final short DEVICE_LOCK_FLAG_SIZE = 1; public static final short BOOT_STATE_SIZE = 1; - public static final short MAX_BLOB_STORAGE = 8; - public static final short AUTH_TAG_LENGTH = 16; - public static final short AUTH_TAG_ENTRY_SIZE = 15; public static final short MAX_OPS = 4; public static final byte BOOT_KEY_MAX_SIZE = 32; public static final byte BOOT_HASH_MAX_SIZE = 32; @@ -398,120 +387,6 @@ public short getComputedHmacKey() { return readData(COMPUTED_HMAC_KEY); } - private byte readAuthTagState(byte[] buf, short offset) { - return buf[offset]; - } - - private void writeAuthTagState(byte[] buf, short offset, byte state) { - buf[offset] = state; - } - - public void persistAuthTag(short authTag) { - if (KMByteBlob.cast(authTag).length() != AUTH_TAG_LENGTH) - KMException.throwIt(KMError.INVALID_INPUT_LENGTH); - short authTagEntry = alloc(AUTH_TAG_ENTRY_SIZE); - short offset = alloc(AUTH_TAG_ENTRY_SIZE); - writeAuthTagState( - KMByteBlob.cast(authTagEntry).getBuffer(), - KMByteBlob.cast(authTagEntry).getStartOff(), - (byte) 1); - Util.arrayCopyNonAtomic( - KMByteBlob.cast(authTag).getBuffer(), - KMByteBlob.cast(authTag).getStartOff(), - getHeap(), authTagEntry, AUTH_TAG_LENGTH); - Util.setShort(getHeap(), (short) (authTagEntry + AUTH_TAG_LENGTH +1), - (short) 0); - short index = 0; - while (index < MAX_BLOB_STORAGE) { - if (dataLength((short) (index + AUTH_TAG_1)) != 0) { - readDataEntry((short) (index + AUTH_TAG_1), getHeap(), offset); - if (0 == readAuthTagState(getHeap(), offset)) { - writeDataEntry((short) (index + AUTH_TAG_1), - KMByteBlob.cast(authTagEntry).getBuffer(), - KMByteBlob.cast(authTagEntry).getStartOff(), - AUTH_TAG_ENTRY_SIZE); - break; - } - } else { - writeDataEntry((short) (index + AUTH_TAG_1), - KMByteBlob.cast(authTagEntry).getBuffer(), - KMByteBlob.cast(authTagEntry).getStartOff(), - AUTH_TAG_ENTRY_SIZE); - break; - } - index++; - } - } - - public boolean validateAuthTag(short authTag) { - short tag = findTag(authTag); - return tag !=KMType.INVALID_VALUE; - } - - public void removeAuthTag(short authTag) { - short tag = findTag(authTag); - if(tag ==KMType.INVALID_VALUE) KMException.throwIt(KMError.UNKNOWN_ERROR); - clearDataEntry(tag); - } - - public void removeAllAuthTags() { - short index = 0; - while (index < MAX_BLOB_STORAGE) { - clearDataEntry((short)(index+AUTH_TAG_1)); - index++; - } - } - - private short findTag(short authTag) { - if(KMByteBlob.cast(authTag).length() != AUTH_TAG_LENGTH)KMException.throwIt(KMError.INVALID_INPUT_LENGTH); - short index = 0; - short found; - short offset = alloc(AUTH_TAG_ENTRY_SIZE); - while (index < MAX_BLOB_STORAGE) { - if (dataLength((short)(index+AUTH_TAG_1)) != 0) { - readDataEntry((short)(index+AUTH_TAG_1), - getHeap(), offset); - found = - Util.arrayCompare( - getHeap(), - (short)(offset+1), - KMByteBlob.cast(authTag).getBuffer(), - KMByteBlob.cast(authTag).getStartOff(), - AUTH_TAG_LENGTH); - if(found == 0)return (short)(index+AUTH_TAG_1); - } - index++; - } - return KMType.INVALID_VALUE; - } - - public short getRateLimitedKeyCount(short authTag) { - short tag = findTag(authTag); - short blob; - if (tag != KMType.INVALID_VALUE) { - blob = readData(tag); - return Util.getShort(KMByteBlob.cast(blob).getBuffer(), - (short)(KMByteBlob.cast(blob).getStartOff()+AUTH_TAG_LENGTH+1)); - } - return KMType.INVALID_VALUE; - } - - public void setRateLimitedKeyCount(short authTag, short val) { - short tag = findTag(authTag); - if (tag != KMType.INVALID_VALUE) { - short dataPtr = readData(tag); - Util.setShort( - KMByteBlob.cast(dataPtr).getBuffer(), - (short)(KMByteBlob.cast(dataPtr).getStartOff()+AUTH_TAG_LENGTH+1), - val); - writeDataEntry(tag, - KMByteBlob.cast(dataPtr).getBuffer(), - KMByteBlob.cast(dataPtr).getStartOff(), - KMByteBlob.cast(dataPtr).length()); - } - } - - public void persistAttestationKey(short secret) { writeDataEntry(ATT_EC_KEY, KMByteBlob.cast(secret).getBuffer(), @@ -703,16 +578,6 @@ public void setBootState(byte state){ writeDataEntry(BOOT_VERIFIED_BOOT_STATE,getHeap(),start,BOOT_STATE_SIZE); } - public short getKeyBlobCount(){ - byte index = 0; - byte count = 0; - while(index < MAX_BLOB_STORAGE){ - if(dataLength((short)(index+AUTH_TAG_1)) != 0) count++; - index++; - } - return count; - } - @Override public void onSave(Element ele) { ele.write(dataIndex); From fe1f2fc5a710a58bc4bf1b188ddfc14b88d21463 Mon Sep 17 00:00:00 2001 From: bvenkateswarlu Date: Sat, 23 Jan 2021 00:34:50 +0530 Subject: [PATCH 22/24] generate random number for operation handle --- .../javacard/test/KMFunctionalTest.java | 20 ++- .../javacard/keymaster/KMKeymasterApplet.java | 30 +++-- .../javacard/keymaster/KMOperationState.java | 11 +- .../javacard/keymaster/KMRepository.java | 127 ++++++++++++------ 4 files changed, 126 insertions(+), 62 deletions(-) diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index 0e6408d2..74892031 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -2241,10 +2241,13 @@ public void testAbortOperation(){ byte[] plainData= "Hello World 123!".getBytes(); short ret = begin(KMType.ENCRYPT, KMByteBlob.instance(keyBlob,(short)0, (short)keyBlob.length), KMKeyParameters.instance(inParams), (short)0); short opHandle = KMArray.cast(ret).get((short) 2); - opHandle = KMInteger.cast(opHandle).getShort(); - abort(KMInteger.uint_16(opHandle)); + byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; + KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); + opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); + abort(opHandle); short dataPtr = KMByteBlob.instance(plainData, (short) 0, (short) plainData.length); - ret = update(KMInteger.uint_16(opHandle), dataPtr, (short) 0, (short) 0, (short) 0); + opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); + ret = update(opHandle, dataPtr, (short) 0, (short) 0, (short) 0); Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE,ret); cleanUp(); } @@ -2513,7 +2516,8 @@ public short processMessage( boolean aesGcmFlag) { short beginResp = begin(keyPurpose, keyBlob, inParams, hwToken); short opHandle = KMArray.cast(beginResp).get((short) 2); - opHandle = KMInteger.cast(opHandle).getShort(); + byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; + KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); short dataPtr = KMByteBlob.instance(data, (short) 0, (short) data.length); short ret = KMType.INVALID_VALUE; byte[] outputData = new byte[128]; @@ -2537,7 +2541,8 @@ public short processMessage( KMArray.cast(inParams).add((short)0, associatedData); inParams = KMKeyParameters.instance(inParams); } - ret = update(KMInteger.uint_16(opHandle), dataPtr, inParams, (short) 0, (short) 0); + opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); + ret = update(opHandle, dataPtr, inParams, (short) 0, (short) 0); dataPtr = KMArray.cast(ret).get((short) 3); if (KMByteBlob.cast(dataPtr).length() > 0) { Util.arrayCopyNonAtomic( @@ -2553,10 +2558,11 @@ public short processMessage( } } + opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); if (keyPurpose == KMType.VERIFY) { - ret = finish(KMInteger.uint_16(opHandle), dataPtr, signature, (short) 0, (short) 0, (short) 0); + ret = finish(opHandle, dataPtr, signature, (short) 0, (short) 0, (short) 0); } else { - ret = finish(KMInteger.uint_16(opHandle), dataPtr, null, (short) 0, (short) 0, (short) 0); + ret = finish(opHandle, dataPtr, null, (short) 0, (short) 0, (short) 0); } if(len >0){ dataPtr = KMArray.cast(ret).get((short)2); diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 467fe81a..a85a7243 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -472,6 +472,12 @@ && isProvisioningComplete())) { } } + private void generateUniqueOperationHandle(byte[] buf, short offset, short len) { + do { + seProvider.newRandomNumber(buf, offset, len); + } while (null != repository.findOperation(buf, offset, len)); + } + private boolean isProvisioningComplete() { if((0 != (provisionStatus & PROVISION_STATUS_ATTESTATION_KEY)) && (0 != (provisionStatus & PROVISION_STATUS_ATTESTATION_CERT_CHAIN)) @@ -486,7 +492,7 @@ private boolean isProvisioningComplete() { private void freeOperations() { if (data[OP_HANDLE] != KMType.INVALID_VALUE) { - KMOperationState op = repository.findOperation(KMInteger.cast(data[OP_HANDLE]).getShort()); + KMOperationState op = repository.findOperation(data[OP_HANDLE]); if (op != null) { repository.releaseOperation(op); } @@ -1557,8 +1563,7 @@ private void processAbortOperationCmd(APDU apdu) { repository.reclaimMemory(bufferLength); data[OP_HANDLE] = KMArray.cast(tmpVariables[2]).get((short) 0); - tmpVariables[1] = KMInteger.cast(data[OP_HANDLE]).getShort(); - KMOperationState op = repository.findOperation(tmpVariables[1]); + KMOperationState op = repository.findOperation(data[OP_HANDLE]); if (op == null) { KMException.throwIt(KMError.INVALID_OPERATION_HANDLE); } @@ -1592,8 +1597,7 @@ private void processFinishOperationCmd(APDU apdu) { data[HW_TOKEN] = KMArray.cast(tmpVariables[2]).get((short) 4); data[VERIFICATION_TOKEN] = KMArray.cast(tmpVariables[2]).get((short) 5); // Check Operation Handle - tmpVariables[1] = KMInteger.cast(data[OP_HANDLE]).getShort(); - KMOperationState op = repository.findOperation(tmpVariables[1]); + KMOperationState op = repository.findOperation(data[OP_HANDLE]); if (op == null) { KMException.throwIt(KMError.INVALID_OPERATION_HANDLE); } @@ -2064,8 +2068,7 @@ private void processUpdateOperationCmd(APDU apdu) { } // Check Operation Handle and get op state // Check Operation Handle - tmpVariables[1] = KMInteger.cast(data[OP_HANDLE]).getShort(); - KMOperationState op = repository.findOperation(tmpVariables[1]); + KMOperationState op = repository.findOperation(data[OP_HANDLE]); if (op == null) KMException.throwIt(KMError.INVALID_OPERATION_HANDLE); // authorize the update operation authorizeUpdateFinishOperation(op, scratchPad); @@ -2203,7 +2206,18 @@ private void processBeginOperationCmd(APDU apdu) { tmpVariables[0] = KMArray.cast(args).get((short) 0); tmpVariables[0] = KMEnum.cast(tmpVariables[0]).getVal(); data[HW_TOKEN] = KMArray.cast(args).get((short) 3); - KMOperationState op = repository.reserveOperation(); + /*Generate a random number for operation handle */ + short buf = KMByteBlob.instance(KMRepository.OPERATION_HANDLE_SIZE); + generateUniqueOperationHandle( + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + KMByteBlob.cast(buf).length()); + /* opHandle is a KMInteger and is encoded as KMInteger when it is returned back. */ + short opHandle = KMInteger.instance( + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + KMByteBlob.cast(buf).length()); + KMOperationState op = repository.reserveOperation(opHandle); if (op == null) KMException.throwIt(KMError.TOO_MANY_OPERATIONS); data[OP_HANDLE] = op.getHandle(); diff --git a/Applet/src/com/android/javacard/keymaster/KMOperationState.java b/Applet/src/com/android/javacard/keymaster/KMOperationState.java index 347c9920..6ea96941 100644 --- a/Applet/src/com/android/javacard/keymaster/KMOperationState.java +++ b/Applet/src/com/android/javacard/keymaster/KMOperationState.java @@ -68,20 +68,21 @@ private static KMOperationState proto() { return prototype; } - public static KMOperationState instance(short opId, Object[] slot) { + public static KMOperationState instance(short opHandle, Object[] slot) { KMOperationState opState = proto(); opState.reset(); - Util.setShort(data, OP_HANDLE, opId); + Util.setShort(data, OP_HANDLE, opHandle); KMOperationState.slot = slot; return opState; } - public static KMOperationState read(Object[] slot) { + public static KMOperationState read(byte[] oprHandle, short off, Object[] slot) { KMOperationState opState = proto(); opState.reset(); Util.arrayCopy((byte[]) slot[DATA], (short) 0, data, (short) 0, (short) data.length); Object[] ops = ((Object[]) slot[REFS]); op = (KMOperation) ops[OPERATION]; + Util.setShort(data, OP_HANDLE, KMInteger.uint_64(oprHandle, off)); KMOperationState.slot = slot; return opState; } @@ -123,10 +124,6 @@ public void release() { } public short getHandle() { - return KMInteger.uint_16(Util.getShort(KMOperationState.data, OP_HANDLE)); - } - - public short handle() { return Util.getShort(KMOperationState.data, OP_HANDLE); } diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index 2b95697a..4c30a613 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -35,6 +35,11 @@ public class KMRepository implements KMUpgradable { public static final short HEAP_SIZE = 10000; public static final short DATA_INDEX_ENTRY_LENGTH = 0; public static final short DATA_INDEX_ENTRY_OFFSET = 2; + public static final short OPERATION_HANDLE_SIZE = 8; /* 8 bytes */ + private static final short OPERATION_HANDLE_STATUS_OFFSET = 0; + private static final short OPERATION_HANDLE_STATUS_SIZE = 1; + private static final short OPERATION_HANDLE_OFFSET = 1; + private static final short OPERATION_HANDLE_ENTRY_SIZE = OPERATION_HANDLE_SIZE + OPERATION_HANDLE_STATUS_SIZE; // Data table offsets public static final byte MASTER_KEY = 8; @@ -80,7 +85,6 @@ public class KMRepository implements KMUpgradable { // Class Attributes private Object[] operationStateTable; - private static short opIdCounter; private byte[] heap; private short heapIndex; private byte[] dataTable; @@ -101,9 +105,11 @@ public KMRepository(boolean isUpgrading) { reclaimIndex = HEAP_SIZE; operationStateTable = new Object[MAX_OPS]; // create and initialize operation state table. + //First byte in the operation handle buffer denotes whether the operation is + //reserved or unreserved. byte index = 0; while(index < MAX_OPS){ - operationStateTable[index] = new Object[]{new byte[2], + operationStateTable[index] = new Object[]{new byte[OPERATION_HANDLE_ENTRY_SIZE], new Object[] {new byte[KMOperationState.MAX_DATA], new Object[KMOperationState.MAX_REFS]}}; index++; @@ -111,24 +117,51 @@ public KMRepository(boolean isUpgrading) { repository = this; } - public KMOperationState findOperation(short opHandle) { + public void getOperationHandle(short oprHandle, byte[] buf, short off, short len) { + if (KMInteger.cast(oprHandle).length() != OPERATION_HANDLE_SIZE) { + KMException.throwIt(KMError.INVALID_OPERATION_HANDLE); + } + KMInteger.cast(oprHandle).getValue(buf, off, len); + } + + public KMOperationState findOperation(byte[] buf, short off, short len) { short index = 0; byte[] opId; - while(index < MAX_OPS){ - opId = ((byte[])((Object[])operationStateTable[index])[0]); - if(Util.getShort(opId,(short)0) == opHandle)return KMOperationState.read((Object[])((Object[])operationStateTable[index])[1]); + while (index < MAX_OPS) { + opId = ((byte[]) ((Object[]) operationStateTable[index])[0]); + if (0 == Util.arrayCompare(buf, off, opId, OPERATION_HANDLE_OFFSET, len)) { + return KMOperationState + .read(opId, OPERATION_HANDLE_OFFSET, (Object[]) ((Object[]) operationStateTable[index])[1]); + } index++; } + return null; } - public KMOperationState reserveOperation(){ + /* operationHandle is a KMInteger */ + public KMOperationState findOperation(short operationHandle) { + short buf = KMByteBlob.instance(OPERATION_HANDLE_SIZE); + getOperationHandle( + operationHandle, + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + KMByteBlob.cast(buf).length()); + return findOperation( + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + KMByteBlob.cast(buf).length()); + } + + /* opHandle is a KMInteger */ + public KMOperationState reserveOperation(short opHandle){ short index = 0; byte[] opId; while(index < MAX_OPS){ opId = (byte[])((Object[])operationStateTable[index])[0]; - if(Util.getShort(opId,(short)0) == 0){ - return KMOperationState.instance(/*Util.getShort(opId,(short)0)*/getOpId(),(Object[])((Object[])operationStateTable[index])[1]); + /* Check for unreserved operation state */ + if (opId[OPERATION_HANDLE_STATUS_OFFSET] == 0) { + return KMOperationState.instance(opHandle, (Object[])((Object[])operationStateTable[index])[1]); } index++; } @@ -139,29 +172,48 @@ public KMOperationState reserveOperation(){ public void persistOperation(byte[] data, short opHandle, KMOperation op) { short index = 0; byte[] opId; + short buf = KMByteBlob.instance(OPERATION_HANDLE_SIZE); + getOperationHandle( + opHandle, + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + KMByteBlob.cast(buf).length()); //Update an existing operation state. - while(index < MAX_OPS){ - opId = (byte[])((Object[])operationStateTable[index])[0]; - if(Util.getShort(opId,(short)0) == opHandle){ - Object[] slot = (Object[])((Object[])operationStateTable[index])[1]; - JCSystem.beginTransaction(); - Util.arrayCopy(data, (short) 0, (byte[]) slot[0], (short) 0, (short) ((byte[]) slot[0]).length); + while (index < MAX_OPS) { + opId = (byte[]) ((Object[]) operationStateTable[index])[0]; + if ((1 == opId[OPERATION_HANDLE_STATUS_OFFSET]) + && (0 == Util.arrayCompare( + opId, + OPERATION_HANDLE_OFFSET, + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + KMByteBlob.cast(buf).length()))) { + Object[] slot = (Object[]) ((Object[]) operationStateTable[index])[1]; + JCSystem.beginTransaction(); + Util.arrayCopy(data, (short) 0, (byte[]) slot[0], (short) 0, + (short) ((byte[]) slot[0]).length); Object[] ops = ((Object[]) slot[1]); ops[0] = op; JCSystem.commitTransaction(); return; - } - index++; + } + index++; } index = 0; //Persist a new operation. while(index < MAX_OPS){ opId = (byte[])((Object[])operationStateTable[index])[0]; - if(Util.getShort(opId,(short)0) == 0){ - Util.setShort(opId, (short)0, opHandle); + if(0 == opId[OPERATION_HANDLE_STATUS_OFFSET]){ Object[] slot = (Object[])((Object[])operationStateTable[index])[1]; JCSystem.beginTransaction(); + opId[OPERATION_HANDLE_STATUS_OFFSET] = 1;/*reserved */ + Util.arrayCopy( + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + opId, + OPERATION_HANDLE_OFFSET, + OPERATION_HANDLE_SIZE); Util.arrayCopy(data, (short) 0, (byte[]) slot[0], (short) 0, (short) ((byte[]) slot[0]).length); Object[] ops = ((Object[]) slot[1]); ops[0] = op; @@ -172,28 +224,24 @@ public void persistOperation(byte[] data, short opHandle, KMOperation op) { } } - private short getOpId() { - byte index = 0; - opIdCounter++; - while (index < MAX_OPS) { - if (Util.getShort((byte[]) ((Object[]) operationStateTable[index])[0], (short) 0) - == opIdCounter) { - opIdCounter++; - index = 0; - continue; - } - index++; - } - return opIdCounter; - } - - public void releaseOperation(KMOperationState op){ + public void releaseOperation(KMOperationState op) { short index = 0; byte[] var; - while(index < MAX_OPS){ - var = ((byte[])((Object[])operationStateTable[index])[0]); - if(Util.getShort(var,(short)0) == op.handle()){ - Util.arrayFillNonAtomic(var,(short)0,(short)var.length,(byte)0); + short buf = KMByteBlob.instance(OPERATION_HANDLE_SIZE); + getOperationHandle( + op.getHandle(), + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + KMByteBlob.cast(buf).length()); + while (index < MAX_OPS) { + var = ((byte[]) ((Object[]) operationStateTable[index])[0]); + if ((var[OPERATION_HANDLE_STATUS_OFFSET] == 1) && + (0 == Util.arrayCompare(var, + OPERATION_HANDLE_OFFSET, + KMByteBlob.cast(buf).getBuffer(), + KMByteBlob.cast(buf).getStartOff(), + KMByteBlob.cast(buf).length()))) { + Util.arrayFillNonAtomic(var, (short) 0, (short) var.length, (byte) 0); op.release(); break; } @@ -211,7 +259,6 @@ public void initHmacSharedSecretKey(byte[] key, short start, short len) { writeDataEntry(SHARED_KEY,key,start,len); } - public void initComputedHmac(byte[] key, short start, short len) { if(len != COMPUTED_HMAC_KEY_SIZE) KMException.throwIt(KMError.INVALID_INPUT_LENGTH); writeDataEntry(COMPUTED_HMAC_KEY,key,start,len); From f80df71c308003cb6be3b3c48318294f2fbd5723 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Sat, 23 Jan 2021 00:47:05 +0530 Subject: [PATCH 23/24] Corrected the condition for SE provisioned status --- HAL/keymaster/4.1/Provision.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/HAL/keymaster/4.1/Provision.cpp b/HAL/keymaster/4.1/Provision.cpp index 6249e34d..ebcf7e37 100644 --- a/HAL/keymaster/4.1/Provision.cpp +++ b/HAL/keymaster/4.1/Provision.cpp @@ -458,8 +458,7 @@ static bool isSEProvisioned(uint64_t status) { if(status != (ProvisionStatus::PROVISION_STATUS_ATTESTATION_KEY | ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_CHAIN | ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_PARAMS | ProvisionStatus::PROVISION_STATUS_ATTEST_IDS | - ProvisionStatus::PROVISION_STATUS_PRESHARED_SECRET | ProvisionStatus::PROVISION_STATUS_BOOT_PARAM - |ProvisionStatus::PROVISION_STATUS_PROVISIONING_LOCKED)) { + ProvisionStatus::PROVISION_STATUS_PRESHARED_SECRET | ProvisionStatus::PROVISION_STATUS_BOOT_PARAM)) { ret = false; } return ret; From 89cbcc44454510cf3374debf56a1998b229a6ecc Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Sat, 23 Jan 2021 01:45:42 +0530 Subject: [PATCH 24/24] Corrected the condition for SE provisioned status --- HAL/keymaster/4.1/Provision.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/HAL/keymaster/4.1/Provision.cpp b/HAL/keymaster/4.1/Provision.cpp index ebcf7e37..24f65667 100644 --- a/HAL/keymaster/4.1/Provision.cpp +++ b/HAL/keymaster/4.1/Provision.cpp @@ -454,12 +454,13 @@ static ErrorCode setBootParameters(std::unique_ptr