From 19e33fcd28d6e241ca12fa0f817f1dd96ab8b1e8 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Thu, 16 Jun 2022 07:36:44 +0000 Subject: [PATCH 01/10] HAL review comments fixes --- HAL/CborConverter.cpp | 360 ++++++++---------- HAL/CborConverter.h | 126 +++--- HAL/ITransport.h | 2 + HAL/JavacardKeyMintDevice.cpp | 107 ++++-- HAL/JavacardKeyMintDevice.h | 5 +- HAL/JavacardKeyMintOperation.cpp | 17 +- HAL/JavacardKeyMintOperation.h | 7 +- HAL/JavacardKeyMintUtils.cpp | 4 +- HAL/JavacardKeyMintUtils.h | 10 +- ...cardRemotelyProvisionedComponentDevice.cpp | 69 ++-- ...vacardRemotelyProvisionedComponentDevice.h | 6 +- HAL/JavacardSecureElement.cpp | 12 +- HAL/JavacardSecureElement.h | 3 +- HAL/JavacardSharedSecret.cpp | 13 +- HAL/JavacardSharedSecret.h | 9 +- HAL/OmapiTransport.cpp | 4 +- HAL/OmapiTransport.h | 12 +- HAL/SocketTransport.cpp | 10 +- HAL/SocketTransport.h | 4 +- HAL/keymint_utils.cpp | 5 +- HAL/keymint_utils.h | 2 + HAL/service.cpp | 11 +- 22 files changed, 418 insertions(+), 380 deletions(-) diff --git a/HAL/CborConverter.cpp b/HAL/CborConverter.cpp index e34c651c..bdd5120e 100644 --- a/HAL/CborConverter.cpp +++ b/HAL/CborConverter.cpp @@ -16,13 +16,11 @@ */ #include "CborConverter.h" -#include -#include -#include + #include -#include #include -#include + +#include "JavacardKeyMintUtils.h" namespace keymint::javacard { using namespace cppbor; @@ -33,6 +31,10 @@ using std::string; using std::unique_ptr; using std::vector; +constexpr int SB_ENFORCED = 0; +constexpr int TEE_ENFORCED = 1; +constexpr int SW_ENFORCED = 2; + bool CborConverter::addAttestationKey(Array& array, const std::optional& attestationKey) { if (attestationKey.has_value()) { @@ -86,234 +88,222 @@ bool CborConverter::addKeyparameters(Array& array, const vector& k map.add(static_cast(param.tag & 0x00000000ffffffff), km_utils::kmBlob2vector(param.blob)); break; - default: - /* Invalid skip */ + case KM_INVALID: break; } } - if (0 < enum_repetition.size()) { - for (auto const& [key, val] : enum_repetition) { - Bstr bstr(val); - map.add(key, std::move(bstr)); - } + + for (auto const& [key, val] : enum_repetition) { + Bstr bstr(val); + map.add(key, std::move(bstr)); } - if (0 < uint_repetition.size()) { - for (auto& [key, val] : uint_repetition) { - map.add(key, std::move(val)); - } + + for (auto& [key, val] : uint_repetition) { + map.add(key, std::move(val)); } array.add(std::move(map)); return true; } // Array of three maps -bool CborConverter::getKeyCharacteristics(const unique_ptr& item, const uint32_t pos, - vector& keyCharacteristics) { - unique_ptr arrayItem(nullptr); - getItemAtPos(item, pos, arrayItem); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem))) return false; - +std::optional> CborConverter::getKeyCharacteristics(const unique_ptr& item, const uint32_t pos) { + vector keyCharacteristics; + auto arrayItem = getItemAtPos(item, pos); + if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem.value()))) { + return std::nullopt; + } KeyCharacteristics swEnf{SecurityLevel::KEYSTORE, {}}; KeyCharacteristics teeEnf{SecurityLevel::TRUSTED_ENVIRONMENT, {}}; KeyCharacteristics sbEnf{SecurityLevel::STRONGBOX, {}}; - if (!getKeyParameters(arrayItem, 0, sbEnf.authorizations) || - !getKeyParameters(arrayItem, 1, teeEnf.authorizations) || - !getKeyParameters(arrayItem, 2, swEnf.authorizations)) { - return false; + auto optSbEnf = getKeyParameters(arrayItem.value(), SB_ENFORCED); + if (!optSbEnf) { + return std::nullopt; } + sbEnf.authorizations = std::move(optSbEnf.value()); + auto optTeeEnf = getKeyParameters(arrayItem.value(), TEE_ENFORCED); + if (!optTeeEnf) { + return std::nullopt; + } + teeEnf.authorizations = std::move(optTeeEnf.value()); + auto optSwEnf = getKeyParameters(arrayItem.value(), SW_ENFORCED); + if (!optSwEnf) { + return std::nullopt; + } + swEnf.authorizations = std::move(optSwEnf.value()); // VTS will fail if the authorizations list is empty. if (!sbEnf.authorizations.empty()) keyCharacteristics.push_back(std::move(sbEnf)); if (!teeEnf.authorizations.empty()) keyCharacteristics.push_back(std::move(teeEnf)); if (!swEnf.authorizations.empty()) keyCharacteristics.push_back(std::move(swEnf)); - return true; + return keyCharacteristics; } -bool CborConverter::getKeyParameter( - const std::pair&, const unique_ptr&> pair, - vector& keyParams) { - uint64_t key; - uint64_t value; - if (!getUint64(pair.first, key)) { - return false; +std::optional> +CborConverter::getKeyParameter(const std::pair&, + const std::unique_ptr&> pair) { + std::vector keyParams; + keymaster_tag_t key; + auto optValue = getUint64(pair.first); + if (!optValue) { + return std::nullopt; } - switch (keymaster_tag_get_type(static_cast(key))) { + key = static_cast(optValue.value()); + switch (keymaster_tag_get_type(key)) { case KM_ENUM_REP: { - /* ENUM_REP contains values encoded in a Binary string */ + /* ENUM_REP contains values encoded in a Bit string */ const Bstr* bstr = pair.second.get()->asBstr(); - if (bstr == nullptr) return false; + if (bstr == nullptr) { + return std::nullopt; + } for (auto bchar : bstr->value()) { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); + keyParam.tag = key; keyParam.enumerated = bchar; keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } } break; case KM_ENUM: { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); - if (!getUint64(pair.second, value)) { - return false; + keyParam.tag = key; + if (!(optValue = getUint64(pair.second))) { + return std::nullopt; } - keyParam.enumerated = static_cast(value); + keyParam.enumerated = static_cast(optValue.value()); keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } break; case KM_UINT: { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); - if (!getUint64(pair.second, value)) { - return false; + keyParam.tag = key; + if (!(optValue = getUint64(pair.second))) { + return std::nullopt; } - keyParam.integer = static_cast(value); + keyParam.integer = static_cast(optValue.value()); keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } break; case KM_ULONG: { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); - if (!getUint64(pair.second, value)) { - return false; + keyParam.tag = key; + if (!(optValue = getUint64(pair.second))) { + return std::nullopt; } - keyParam.long_integer = value; + keyParam.long_integer = optValue.value(); keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } break; case KM_UINT_REP: { /* UINT_REP contains values encoded in a Array */ Array* array = const_cast(pair.second.get()->asArray()); - if (array == nullptr) return false; + if (array == nullptr) return std::nullopt; for (int i = 0; i < array->size(); i++) { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); - std::unique_ptr item = std::move((*array)[i]); - if (!getUint64(item, value)) { - return false; + keyParam.tag = key; + std::unique_ptr item = std::move(array->get(i)); + if (!(optValue = getUint64(item))) { + return std::nullopt; } - keyParam.integer = static_cast(value); + keyParam.integer = static_cast(optValue.value()); keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } } break; case KM_ULONG_REP: { /* ULONG_REP contains values encoded in a Array */ Array* array = const_cast(pair.second.get()->asArray()); - if (array == nullptr) return false; + if (array == nullptr) return std::nullopt; for (int i = 0; i < array->size(); i++) { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); - std::unique_ptr item = std::move((*array)[i]); - if (!getUint64(item, keyParam.long_integer)) { - return false; + keyParam.tag = key; + std::unique_ptr item = std::move(array->get(i)); + if (!(optValue = getUint64(item))) { + return std::nullopt; } + keyParam.long_integer = optValue.value(); keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } } break; case KM_DATE: { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); - if (!getUint64(pair.second, value)) { - return false; + keyParam.tag = key; + if (!(optValue = getUint64(pair.second))) { + return std::nullopt; } - keyParam.date_time = value; + keyParam.date_time = optValue.value(); keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } break; case KM_BOOL: { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); - if (!getUint64(pair.second, value)) { - return false; + keyParam.tag = key; + if (!(optValue = getUint64(pair.second))) { + return std::nullopt; } // TODO re-check the logic below - keyParam.boolean = static_cast(value); + keyParam.boolean = static_cast(optValue.value()); keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } break; case KM_BYTES: { keymaster_key_param_t keyParam; - keyParam.tag = static_cast(key); + keyParam.tag = key; const Bstr* bstr = pair.second.get()->asBstr(); - if (bstr == nullptr) return false; + if (bstr == nullptr) return std::nullopt; keyParam.blob.data = bstr->value().data(); keyParam.blob.data_length = bstr->value().size(); keyParams.push_back(km_utils::kmParam2Aidl(keyParam)); } break; default: /* Invalid - return error */ - return false; - break; + return std::nullopt; } - return true; + return keyParams; } // array of a blobs -bool CborConverter::getCertificateChain(const std::unique_ptr& item, const uint32_t pos, - vector& certChain) { - std::unique_ptr arrayItem(nullptr); - getItemAtPos(item, pos, arrayItem); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem))) return false; +std::optional> CborConverter::getCertificateChain(const std::unique_ptr& item, const uint32_t pos) { + vector certChain; + auto arrayItem = getItemAtPos(item, pos); + if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem.value()))) return std::nullopt; - const Array* arr = arrayItem.get()->asArray(); + const Array* arr = arrayItem.value().get()->asArray(); for (int i = 0; i < arr->size(); i++) { Certificate cert; - if (!getBinaryArray(arrayItem, i, cert.encodedCertificate)) return false; + auto optTemp = getByteArrayVec(arrayItem.value(), i); + if (!optTemp) return std::nullopt; + cert.encodedCertificate = std::move(optTemp.value()); certChain.push_back(std::move(cert)); } - return true; -} - -bool CborConverter::getMultiBinaryArray(const unique_ptr& item, const uint32_t pos, - vector>& data) { - bool ret = false; - std::unique_ptr arrayItem(nullptr); - - getItemAtPos(item, pos, arrayItem); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem))) return ret; - const Array* arr = arrayItem.get()->asArray(); - size_t arrSize = arr->size(); - for (int i = 0; i < arrSize; i++) { - std::vector temp; - if (!getBinaryArray(arrayItem, i, temp)) return ret; - data.push_back(std::move(temp)); - } - ret = true; // success - return ret; + return certChain; } -bool CborConverter::getBinaryArray(const unique_ptr& item, const uint32_t pos, - string& value) { - vector vec; - string str; - if (!getBinaryArray(item, pos, vec)) { - return false; - } - for (auto ch : vec) { - str += ch; +std::optional CborConverter::getByteArrayStr(const unique_ptr& item, const uint32_t pos) { + auto optTemp = getByteArrayVec(item, pos); + if (!optTemp) { + return std::nullopt; } - value = str; - return true; + std::string str(optTemp->begin(), optTemp->end()); + return str; } -bool CborConverter::getBinaryArray(const unique_ptr& item, const uint32_t pos, - vector& value) { - bool ret = false; - unique_ptr strItem(nullptr); - getItemAtPos(item, pos, strItem); - if ((strItem == nullptr) || (MajorType::BSTR != getType(strItem))) return ret; - - const Bstr* bstr = strItem.get()->asBstr(); - for (auto bchar : bstr->value()) { - value.push_back(bchar); +std::optional> CborConverter::getByteArrayVec(const unique_ptr& item, const uint32_t pos) { + auto strItem = getItemAtPos(item, pos); + if ((strItem == nullptr) || (MajorType::BSTR != getType(strItem.value()))) { + return std::nullopt; } - ret = true; - return ret; + const Bstr* bstr = strItem.value().get()->asBstr(); + return bstr->value(); } -bool CborConverter::getSharedSecretParameters(const unique_ptr& item, const uint32_t pos, - SharedSecretParameters& params) { - std::unique_ptr arrayItem(nullptr); +std::optional CborConverter::getSharedSecretParameters(const unique_ptr& item, const uint32_t pos) { + SharedSecretParameters params; // Array [seed, nonce] - getItemAtPos(item, pos, arrayItem); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem)) || - !getBinaryArray(arrayItem, 0, params.seed) || !getBinaryArray(arrayItem, 1, params.nonce)) { - return false; + auto arrayItem = getItemAtPos(item, pos); + if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem.value()))) { + return std::nullopt; } - return true; + auto optSeed = getByteArrayVec(arrayItem.value(), 0); + auto optNonce = getByteArrayVec(arrayItem.value(), 1); + if (!optSeed || !optNonce) { + return std::nullopt; + } + params.seed = std::move(optSeed.value()); + params.nonce = std::move(optNonce.value()); + return params; } bool CborConverter::addSharedSecretParameters(Array& array, @@ -351,91 +341,69 @@ bool CborConverter::addHardwareAuthToken(Array& array, const HardwareAuthToken& return true; } -bool CborConverter::getHardwareAuthToken(const unique_ptr& item, const uint32_t pos, - HardwareAuthToken& token) { - uint64_t authType; - uint64_t challenge; - uint64_t userId; - uint64_t authenticatorId; - uint64_t timestampMillis; - // challenge, userId, AuthenticatorId, AuthType, Timestamp, MAC - if (!getUint64(item, pos, challenge) || - !getUint64(item, pos + 1, userId) || - !getUint64(item, pos + 2, authenticatorId) || - !getUint64(item, pos + 3, authType) || - !getUint64(item, pos + 4, timestampMillis) || - !getBinaryArray(item, pos + 5, token.mac)) { - return false; - } - token.challenge = static_cast(challenge); - token.userId = static_cast(userId); - token.authenticatorId = static_cast(authenticatorId); - token.authenticatorType = static_cast(authType); - token.timestamp.milliSeconds = static_cast(timestampMillis); - return true; -} - -bool CborConverter::getTimeStampToken(const unique_ptr& item, const uint32_t pos, - TimeStampToken& token) { +std::optional CborConverter::getTimeStampToken(const unique_ptr& item, const uint32_t pos) { + TimeStampToken token; // {challenge, timestamp, Mac} - uint64_t challenge; - uint64_t timestampMillis; - if (!getUint64(item, pos, challenge) || - !getUint64(item, pos + 1, timestampMillis) || - !getBinaryArray(item, pos + 2, token.mac)) { - return false; + auto optChallenge = getUint64(item, pos); + auto optTimestampMillis = getUint64(item, pos + 1); + auto optTemp = getByteArrayVec(item, pos + 2); + if (!optChallenge || !optTimestampMillis || !optTemp) { + return std::nullopt; } - token.challenge = static_cast(challenge); - token.timestamp.milliSeconds = static_cast(timestampMillis); - return true; + token.mac = std::move(optTemp.value()); + token.challenge = static_cast(std::move(optChallenge.value())); + token.timestamp.milliSeconds = static_cast(std::move(optTimestampMillis.value())); + return token; } -bool CborConverter::getArrayItem(const std::unique_ptr& item, const uint32_t pos, - Array& array) { - unique_ptr arrayItem(nullptr); - getItemAtPos(item, pos, arrayItem); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem))) return false; - array = std::move(*arrayItem.get()->asArray()); - return true; +std::optional CborConverter::getArrayItem(const std::unique_ptr& item, const uint32_t pos) { + Array array; + auto arrayItem = getItemAtPos(item, pos); + if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem.value()))) { + return std::nullopt; + } + array = std::move(*(arrayItem.value().get()->asArray())); + return array; } -bool CborConverter::getMapItem(const std::unique_ptr& item, const uint32_t pos, - Map& map) { - unique_ptr mapItem(nullptr); - getItemAtPos(item, pos, mapItem); - if ((mapItem == nullptr) || (MajorType::MAP != getType(mapItem))) return false; - map = std::move(*mapItem.get()->asMap()); - return true; +std::optional CborConverter::getMapItem(const std::unique_ptr& item, const uint32_t pos) { + Map map; + auto mapItem = getItemAtPos(item, pos); + if ((mapItem == nullptr) || (MajorType::MAP != getType(mapItem.value()))) { + return std::nullopt; + } + map = std::move(*(mapItem.value().get()->asMap())); + return map; } -bool CborConverter::getKeyParameters(const unique_ptr& item, const uint32_t pos, - vector& keyParams) { - bool ret = false; - unique_ptr mapItem(nullptr); +std::optional> CborConverter::getKeyParameters(const unique_ptr& item, const uint32_t pos) { vector params; - getItemAtPos(item, pos, mapItem); - if ((mapItem == nullptr) || (MajorType::MAP != getType(mapItem))) return ret; - const Map* map = mapItem.get()->asMap(); + auto mapItem = getItemAtPos(item, pos); + if ((mapItem == nullptr) || (MajorType::MAP != getType(mapItem.value()))) return std::nullopt; + const Map* map = mapItem.value().get()->asMap(); size_t mapSize = map->size(); for (int i = 0; i < mapSize; i++) { - if (!getKeyParameter((*map)[i], params)) { - return ret; + auto optKeyParams = getKeyParameter((*map)[i]); + if (optKeyParams) { + params.insert(params.end(), optKeyParams->begin(), optKeyParams->end()); + } else { + return std::nullopt; } } - keyParams.resize(params.size()); - keyParams = params; - ret = true; - return ret; + return params; } std::tuple, keymaster_error_t> CborConverter::decodeData(const std::vector& response) { - keymaster_error_t errorCode = KM_ERROR_OK; auto [item, pos, message] = parse(response); - if (!item || MajorType::ARRAY != getType(item) || !getErrorCode(item, 0, errorCode)) { + if (!item || MajorType::ARRAY != getType(item)) { + return {nullptr, KM_ERROR_UNKNOWN_ERROR}; + } + auto optErrorCode = getErrorCode(item, 0); + if (!optErrorCode) { return {nullptr, KM_ERROR_UNKNOWN_ERROR}; } - return {std::move(item), errorCode}; + return {std::move(item), optErrorCode.value()}; } } // namespace keymint::javacard diff --git a/HAL/CborConverter.h b/HAL/CborConverter.h index ca44533f..c00b852c 100644 --- a/HAL/CborConverter.h +++ b/HAL/CborConverter.h @@ -15,18 +15,22 @@ ** limitations under the License. */ #pragma once -#include -#include -#include -#include -#include -#include + #include -#include #include #include #include +#include +#include + +#include +#include +#include +#include + +#include + namespace keymint::javacard { using namespace cppbor; using namespace aidl::android::hardware::security::keymint; @@ -44,22 +48,35 @@ class CborConverter { decodeData(const std::vector& response); template - bool getUint64(const std::unique_ptr& item, const uint32_t pos, T& value); - - template bool getUint64(const std::unique_ptr& item, T& value); + std::optional getUint64(const unique_ptr &item) { + T value; + if ((item == nullptr) || (std::is_unsigned::value && (MajorType::UINT != getType(item))) || + ((std::is_signed::value && (MajorType::NINT != getType(item))))) { + return std::nullopt; + } + if (std::is_unsigned::value) { + const Uint *uintVal = item.get()->asUint(); + value = static_cast(uintVal->value()); + } else { + const Nint *nintVal = item.get()->asNint(); + value = static_cast(nintVal->value()); + } + return value; // success + } - bool getSharedSecretParameters(const std::unique_ptr& item, const uint32_t pos, - SharedSecretParameters& params); - bool getBinaryArray(const std::unique_ptr& item, const uint32_t pos, string& value); + template + std::optional getUint64(const unique_ptr &item, const uint32_t pos) { + auto intItem = getItemAtPos(item, pos); + return getUint64(intItem.value()); + } - bool getBinaryArray(const std::unique_ptr& item, const uint32_t pos, - vector& value); + std::optional getSharedSecretParameters(const std::unique_ptr& item, const uint32_t pos); + + std::optional getByteArrayStr(const unique_ptr& item, const uint32_t pos); - bool getHardwareAuthToken(const std::unique_ptr& item, const uint32_t pos, - HardwareAuthToken& authType); + std::optional> getByteArrayVec(const unique_ptr& item, const uint32_t pos); - bool getKeyParameters(const std::unique_ptr& item, const uint32_t pos, - vector& keyParams); + std::optional> getKeyParameters(const unique_ptr& item, const uint32_t pos); bool addKeyparameters(Array& array, const vector& keyParams); @@ -69,34 +86,27 @@ class CborConverter { bool addSharedSecretParameters(Array& array, const vector& params); - bool getTimeStampToken(const std::unique_ptr& item, const uint32_t pos, - TimeStampToken& token); + std::optional getTimeStampToken(const std::unique_ptr& item, const uint32_t pos); - bool getKeyCharacteristics(const std::unique_ptr& item, const uint32_t pos, - vector& keyCharacteristics); + std::optional> getKeyCharacteristics(const std::unique_ptr& item, const uint32_t pos); - bool getCertificateChain(const std::unique_ptr& item, const uint32_t pos, - vector& keyCharacteristics); + std::optional> getCertificateChain(const std::unique_ptr& item, const uint32_t pos); - bool getMultiBinaryArray(const std::unique_ptr& item, const uint32_t pos, - vector>& data); + std::optional>> getMultiByteArray(const unique_ptr& item, const uint32_t pos); bool addTimeStampToken(Array& array, const TimeStampToken& token); - bool getMapItem(const std::unique_ptr& item, const uint32_t pos, - Map& map); + std::optional getMapItem(const std::unique_ptr& item, const uint32_t pos); - bool getArrayItem(const std::unique_ptr& item, const uint32_t pos, - Array& array); - - inline bool getErrorCode(const std::unique_ptr& item, const uint32_t pos, - keymaster_error_t& errorCode) { - uint64_t errorVal; - if (!getUint64(item, pos, errorVal)) { - return false; + std::optional getArrayItem(const std::unique_ptr& item, const uint32_t pos); + + inline std::optional getErrorCode(const std::unique_ptr& item, const uint32_t pos) { + + auto optErrorVal = getUint64(item, pos); + if (!optErrorVal) { + return std::nullopt; } - errorCode = static_cast(0 - errorVal); - return true; + return static_cast(0 - optErrorVal.value()); } private: @@ -115,49 +125,25 @@ class CborConverter { * value contains binary string. If TagType is UINT_REP or ULONG_REP the value contains Array of * unsigned integers. */ - bool getKeyParameter(const std::pair&, const unique_ptr&> pair, - vector& keyParam); + std::optional> + getKeyParameter(const std::pair&, + const std::unique_ptr&> pair); /** * Get the sub item pointer from the root item pointer at the given position. */ - inline void getItemAtPos(const unique_ptr& item, const uint32_t pos, - unique_ptr& subItem) { + inline std::optional> getItemAtPos(const unique_ptr& item, const uint32_t pos) { Array* arr = nullptr; if (MajorType::ARRAY != getType(item)) { - return; + return std::nullopt; } arr = const_cast(item.get()->asArray()); if (arr->size() < (pos + 1)) { - return; + return std::nullopt; } - subItem = std::move((*arr)[pos]); + return std::move((*arr)[pos]); } }; -template bool CborConverter::getUint64(const unique_ptr& item, T& value) { - bool ret = false; - if ((item == nullptr) || (std::is_unsigned::value && (MajorType::UINT != getType(item))) || - ((std::is_signed::value && (MajorType::NINT != getType(item))))) { - return ret; - } - - if (std::is_unsigned::value) { - const Uint* uintVal = item.get()->asUint(); - value = static_cast(uintVal->value()); - } else { - const Nint* nintVal = item.get()->asNint(); - value = static_cast(nintVal->value()); - } - ret = true; - return ret; // success -} - -template -bool CborConverter::getUint64(const unique_ptr& item, const uint32_t pos, T& value) { - unique_ptr intItem(nullptr); - getItemAtPos(item, pos, intItem); - return getUint64(intItem, value); -} } // namespace keymint::javacard diff --git a/HAL/ITransport.h b/HAL/ITransport.h index 0e74dc46..45ba5980 100644 --- a/HAL/ITransport.h +++ b/HAL/ITransport.h @@ -15,8 +15,10 @@ ** limitations under the License. */ #pragma once + #include #include + #include namespace keymint::javacard { diff --git a/HAL/JavacardKeyMintDevice.cpp b/HAL/JavacardKeyMintDevice.cpp index d8cb427c..31551ebf 100644 --- a/HAL/JavacardKeyMintDevice.cpp +++ b/HAL/JavacardKeyMintDevice.cpp @@ -15,23 +15,28 @@ */ #define LOG_TAG "javacard.keymint.device.strongbox-impl" + #include "JavacardKeyMintDevice.h" -#include "JavacardKeyMintOperation.h" -#include "JavacardSharedSecret.h" -#include + +#include + #include -#include -#include -#include #include #include -#include -#include #include -#include #include #include +#include +#include +#include +#include +#include + +#include "JavacardKeyMintOperation.h" +#include "JavacardKeyMintUtils.h" +#include "JavacardSharedSecret.h" + namespace aidl::android::hardware::security::keymint { using km_utils::KmParamSet; using namespace ::keymaster; @@ -48,24 +53,28 @@ ScopedAStatus JavacardKeyMintDevice::defaultHwInfo(KeyMintHardwareInfo* info) { ScopedAStatus JavacardKeyMintDevice::getHardwareInfo(KeyMintHardwareInfo* info) { - uint64_t tsRequired = 1; auto [item, err] = card_->sendRequest(Instruction::INS_GET_HW_INFO_CMD); - uint32_t secLevel; - uint32_t version; - if (err != KM_ERROR_OK || !cbor_.getUint64(item, 1, version) || - !cbor_.getUint64(item, 2, secLevel) || - !cbor_.getBinaryArray(item, 3, info->keyMintName) || - !cbor_.getBinaryArray(item, 4, info->keyMintAuthorName) || - !cbor_.getUint64(item, 5, tsRequired)) { + std::optional optKeyMintName; + std::optional optKeyMintAuthorName; + std::optional optSecLevel; + std::optional optVersion; + std::optional optTsRequired; + if (err != KM_ERROR_OK || !(optVersion = cbor_.getUint64(item, 1)) || + !(optSecLevel = cbor_.getUint64(item, 2)) || + !(optKeyMintName = cbor_.getByteArrayStr(item, 3)) || + !(optKeyMintAuthorName = cbor_.getByteArrayStr(item, 4)) || + !(optTsRequired = cbor_.getUint64(item, 5))) { // TODO should we return HARDWARE_NOT_YET_AVAILABLE instead of default Hardware Info. LOG(ERROR) << "Error in response of getHardwareInfo."; LOG(INFO) << "Returning defaultHwInfo in getHardwareInfo."; return defaultHwInfo(info); } card_->initializeJavacard(); - info->timestampTokenRequired = (tsRequired == 1); - info->securityLevel = static_cast(secLevel); - info->versionNumber = static_cast(version); + info->keyMintName = std::move(optKeyMintName.value()); + info->keyMintAuthorName = std::move(optKeyMintAuthorName.value()); + info->timestampTokenRequired = (optTsRequired.value() == 1); + info->securityLevel = static_cast(std::move(optSecLevel.value())); + info->versionNumber = static_cast(std::move(optVersion.value())); return ScopedAStatus::ok(); } @@ -82,12 +91,16 @@ ScopedAStatus JavacardKeyMintDevice::generateKey(const vector& key LOG(ERROR) << "Error in sending generateKey."; return km_utils::kmError2ScopedAStatus(err); } - if (!cbor_.getBinaryArray(item, 1, creationResult->keyBlob) || - !cbor_.getKeyCharacteristics(item, 2, creationResult->keyCharacteristics) || - !cbor_.getCertificateChain(item, 3, creationResult->certificateChain)) { + auto optKeyBlob = cbor_.getByteArrayVec(item, 1); + auto optKeyChars = cbor_.getKeyCharacteristics(item, 2); + auto optCertChain = cbor_.getCertificateChain(item, 3); + if (!optKeyBlob || !optKeyChars || !optCertChain) { LOG(ERROR) << "Error in decoding og response in generateKey."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + creationResult->keyCharacteristics = std::move(optKeyChars.value()); + creationResult->certificateChain = std::move(optCertChain.value()); + creationResult->keyBlob = std::move(optKeyBlob.value()); return ScopedAStatus::ok(); } @@ -123,12 +136,16 @@ ScopedAStatus JavacardKeyMintDevice::importKey(const vector& keyPa LOG(ERROR) << "Error in sending data in importKey."; return km_utils::kmError2ScopedAStatus(err); } - if (!cbor_.getBinaryArray(item, 1, creationResult->keyBlob) || - !cbor_.getKeyCharacteristics(item, 2, creationResult->keyCharacteristics) || - !cbor_.getCertificateChain(item, 3, creationResult->certificateChain)) { + auto optKeyBlob = cbor_.getByteArrayVec(item, 1); + auto optKeyChars = cbor_.getKeyCharacteristics(item, 2); + auto optCertChain = cbor_.getCertificateChain(item, 3); + if (!optKeyBlob || !optKeyChars || !optCertChain) { LOG(ERROR) << "Error in decoding response in importKey."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + creationResult->keyCharacteristics = std::move(optKeyChars.value()); + creationResult->certificateChain = std::move(optCertChain.value()); + creationResult->keyBlob = std::move(optKeyBlob.value()); return ScopedAStatus::ok(); } @@ -172,12 +189,16 @@ ScopedAStatus JavacardKeyMintDevice::importWrappedKey(const vector& wra LOG(ERROR) << "Error in send finish import wrapped key in importWrappedKey."; return km_utils::kmError2ScopedAStatus(errorCode); } - if (!cbor_.getBinaryArray(item, 1, creationResult->keyBlob) || - !cbor_.getKeyCharacteristics(item, 2, creationResult->keyCharacteristics) || - !cbor_.getCertificateChain(item, 3, creationResult->certificateChain)) { + auto optKeyBlob = cbor_.getByteArrayVec(item, 1); + auto optKeyChars = cbor_.getKeyCharacteristics(item, 2); + auto optCertChain = cbor_.getCertificateChain(item, 3); + if (!optKeyBlob || !optKeyChars || !optCertChain) { LOG(ERROR) << "Error in decoding the response in importWrappedKey."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + creationResult->keyCharacteristics = std::move(optKeyChars.value()); + creationResult->certificateChain = std::move(optCertChain.value()); + creationResult->keyBlob = std::move(optKeyBlob.value()); return ScopedAStatus::ok(); } @@ -225,10 +246,12 @@ ScopedAStatus JavacardKeyMintDevice::upgradeKey(const vector& keyBlobTo LOG(ERROR) << "Error in sending in upgradeKey."; return km_utils::kmError2ScopedAStatus(err); } - if (!cbor_.getBinaryArray(item, 1, *keyBlob)) { + auto optKeyBlob = cbor_.getByteArrayVec(item, 1); + if (!optKeyBlob) { LOG(ERROR) << "Error in decoding the response in upgradeKey."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + *keyBlob = std::move(optKeyBlob.value()); return ScopedAStatus::ok(); } @@ -284,20 +307,20 @@ ScopedAStatus JavacardKeyMintDevice::begin(KeyPurpose purpose, const std::vector return km_utils::kmError2ScopedAStatus(err); } // return the result - uint64_t opHandle; - uint8_t bufMode; - uint16_t macLength; - if (!cbor_.getKeyParameters(item, 1, result->params) || - !cbor_.getUint64(item, 2, opHandle) || - !cbor_.getUint64(item, 3, bufMode) || - !cbor_.getUint64(item, 4, macLength)) { + auto keyParams = cbor_.getKeyParameters(item, 1); + auto optOpHandle = cbor_.getUint64(item, 2); + auto optBufMode = cbor_.getUint64(item, 3); + auto optMacLength = cbor_.getUint64(item, 4); + + if (!keyParams || !optOpHandle || !optBufMode || !optMacLength) { LOG(ERROR) << "Error in decoding the response in begin."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } - result->challenge = opHandle; + result->params = std::move(keyParams.value()); + result->challenge = optOpHandle.value(); result->operation = ndk::SharedRefBase::make( - static_cast(opHandle), static_cast(bufMode), - macLength, card_); + static_cast(optOpHandle.value()), static_cast(optBufMode.value()), + optMacLength.value(), card_); return ScopedAStatus::ok(); } @@ -350,10 +373,12 @@ ScopedAStatus JavacardKeyMintDevice::getKeyCharacteristics( LOG(ERROR) << "Error in sending in getKeyCharacteristics."; return km_utils::kmError2ScopedAStatus(err); } - if (!cbor_.getKeyCharacteristics(item, 1, *result)) { + auto optKeyChars = cbor_.getKeyCharacteristics(item, 1); + if (!optKeyChars) { LOG(ERROR) << "Error in sending in upgradeKey."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + *result = std::move(optKeyChars.value()); return ScopedAStatus::ok(); } diff --git a/HAL/JavacardKeyMintDevice.h b/HAL/JavacardKeyMintDevice.h index 4bbbaa87..3d2ac681 100644 --- a/HAL/JavacardKeyMintDevice.h +++ b/HAL/JavacardKeyMintDevice.h @@ -16,13 +16,14 @@ #pragma once -#include "CborConverter.h" -#include "JavacardSecureElement.h" #include #include #include #include +#include "CborConverter.h" +#include "JavacardSecureElement.h" + namespace aidl::android::hardware::security::keymint { using namespace ::keymint::javacard; using namespace aidl::android::hardware::security::sharedsecret; diff --git a/HAL/JavacardKeyMintOperation.cpp b/HAL/JavacardKeyMintOperation.cpp index 030f8295..4d97c6d4 100644 --- a/HAL/JavacardKeyMintOperation.cpp +++ b/HAL/JavacardKeyMintOperation.cpp @@ -17,11 +17,14 @@ #define LOG_TAG "javacard.strongbox.keymint.operation-impl" #include "JavacardKeyMintOperation.h" -#include + #include #include #include +#include "CborConverter.h" +#include "JavacardKeyMintUtils.h" + namespace aidl::android::hardware::security::keymint { using namespace ::keymint::javacard; using secureclock::TimeStampToken; @@ -257,11 +260,11 @@ keymaster_error_t JavacardKeyMintOperation::sendUpdate(const vector& in if (error != KM_ERROR_OK) { return error; } - vector respData; - if (!cbor_.getBinaryArray(item, 1, respData)) { + auto optTemp = cbor_.getByteArrayVec(item, 1); + if (!optTemp) { return KM_ERROR_UNKNOWN_ERROR; } - output.insert(output.end(), respData.begin(), respData.end()); + output.insert(output.end(), optTemp.value().begin(), optTemp.value().end()); return KM_ERROR_OK; } @@ -283,12 +286,12 @@ keymaster_error_t JavacardKeyMintOperation::sendFinish(const vector& da if (err != KM_ERROR_OK) { return err; } - vector respData; - if (!cbor_.getBinaryArray(item, 1, respData)) { + auto optTemp = cbor_.getByteArrayVec(item, 1); + if (!optTemp) { return KM_ERROR_UNKNOWN_ERROR; } opHandle_ = 0; - output.insert(output.end(), respData.begin(), respData.end()); + output.insert(output.end(), optTemp.value().begin(), optTemp.value().end()); return KM_ERROR_OK; } diff --git a/HAL/JavacardKeyMintOperation.h b/HAL/JavacardKeyMintOperation.h index 2ac6930b..2f9815e2 100644 --- a/HAL/JavacardKeyMintOperation.h +++ b/HAL/JavacardKeyMintOperation.h @@ -16,13 +16,14 @@ #pragma once -#include "CborConverter.h" -#include "JavacardSecureElement.h" +#include #include #include #include -#include + +#include "CborConverter.h" +#include "JavacardSecureElement.h" #define AES_BLOCK_SIZE 16 #define DES_BLOCK_SIZE 8 diff --git a/HAL/JavacardKeyMintUtils.cpp b/HAL/JavacardKeyMintUtils.cpp index 9860d407..b6ec44f6 100644 --- a/HAL/JavacardKeyMintUtils.cpp +++ b/HAL/JavacardKeyMintUtils.cpp @@ -15,9 +15,11 @@ */ #include "JavacardKeyMintUtils.h" -#include + #include +#include + namespace aidl::android::hardware::security::keymint::km_utils { keymaster_key_param_t kInvalidTag{.tag = KM_TAG_INVALID, .integer = 0}; diff --git a/HAL/JavacardKeyMintUtils.h b/HAL/JavacardKeyMintUtils.h index 9b103ff1..9545df63 100644 --- a/HAL/JavacardKeyMintUtils.h +++ b/HAL/JavacardKeyMintUtils.h @@ -15,13 +15,17 @@ */ #pragma once -#include -#include + +#include + #include #include +#include +#include + #include #include -#include + namespace aidl::android::hardware::security::keymint::km_utils { using namespace ::keymaster; diff --git a/HAL/JavacardRemotelyProvisionedComponentDevice.cpp b/HAL/JavacardRemotelyProvisionedComponentDevice.cpp index 9055de94..b4091af9 100644 --- a/HAL/JavacardRemotelyProvisionedComponentDevice.cpp +++ b/HAL/JavacardRemotelyProvisionedComponentDevice.cpp @@ -15,13 +15,17 @@ */ #define LOG_TAG "javacard.keymint.device.rkp.strongbox-impl" -#include -#include -#include + +#include "JavacardRemotelyProvisionedComponentDevice.h" + #include + +#include #include #include +#include "JavacardKeyMintUtils.h" + namespace aidl::android::hardware::security::keymint { using namespace cppcose; using namespace keymaster; @@ -82,18 +86,20 @@ uint32_t coseKeyEncodedSize(const std::vector& keysToSign) { ScopedAStatus JavacardRemotelyProvisionedComponentDevice::getHardwareInfo(RpcHardwareInfo* info) { auto [item, err] = card_->sendRequest(Instruction::INS_GET_RKP_HARDWARE_INFO); - uint32_t versionNumber; - uint32_t supportedEekCurve; + std::optional optVersionNumber; + std::optional optSupportedEekCurve; + std::optional optRpcAuthorName; if (err != KM_ERROR_OK || - !cbor_.getUint64(item, 1, versionNumber) || - !cbor_.getBinaryArray(item, 2, info->rpcAuthorName ) || - !cbor_.getUint64(item, 3, supportedEekCurve)) { + !(optVersionNumber = cbor_.getUint64(item, 1)) || + !(optRpcAuthorName = cbor_.getByteArrayStr(item, 2)) || + !(optSupportedEekCurve = cbor_.getUint64(item, 3))) { LOG(ERROR) << "Error in response of getHardwareInfo."; LOG(INFO) << "Returning defaultHwInfo in getHardwareInfo."; return defaultHwInfo(info); } - info->versionNumber = static_cast(versionNumber); - info->supportedEekCurve = static_cast(supportedEekCurve); + info->rpcAuthorName = std::move(optRpcAuthorName.value()); + info->versionNumber = static_cast(std::move(optVersionNumber.value())); + info->supportedEekCurve = static_cast(std::move(optSupportedEekCurve.value())); return ScopedAStatus::ok(); } @@ -108,11 +114,15 @@ JavacardRemotelyProvisionedComponentDevice::generateEcdsaP256KeyPair(bool testMo LOG(ERROR) << "Error in sending generateEcdsaP256KeyPair."; return km_utils::kmError2ScopedAStatus(translateRkpErrorCode(err)); } - if (!cbor_.getBinaryArray(item, 1, macedPublicKey->macedKey) || - !cbor_.getBinaryArray(item, 2, *privateKeyHandle)) { + std::optional> optMacedKey; + std::optional> optPKeyHandle; + if (!(optMacedKey = cbor_.getByteArrayVec(item, 1)) || + !(optPKeyHandle = cbor_.getByteArrayVec(item, 2))) { LOG(ERROR) << "Error in decoding og response in generateEcdsaP256KeyPair."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + *privateKeyHandle = std::move(optPKeyHandle.value()); + macedPublicKey->macedKey = std::move(optMacedKey.value()); return ScopedAStatus::ok(); } @@ -178,24 +188,29 @@ JavacardRemotelyProvisionedComponentDevice::finishSendData( std::vector& coseEncryptProtectedHeader, cppbor::Map& coseEncryptUnProtectedHeader, std::vector& partialCipheredData, uint32_t& respFlag) { - std::vector decodedKeysToSignMac; - std::vector decodedDeviceInfo; auto [item, err] = card_->sendRequest(Instruction::INS_FINISH_SEND_DATA_CMD); if (err != KM_ERROR_OK) { LOG(ERROR) << "Error in finishSendData."; return km_utils::kmError2ScopedAStatus(translateRkpErrorCode(err)); } - if (!cbor_.getBinaryArray(item, 1, decodedKeysToSignMac) || - !cbor_.getBinaryArray(item, 2, decodedDeviceInfo) || - !cbor_.getBinaryArray(item, 3, coseEncryptProtectedHeader) || - !cbor_.getMapItem(item, 4, coseEncryptUnProtectedHeader) || - !cbor_.getBinaryArray(item, 5, partialCipheredData) || - !cbor_.getUint64(item, 6, respFlag)) { + auto optDecodedKeysToSignMac = cbor_.getByteArrayVec(item, 1); + auto optDecodedDeviceInfo = cbor_.getByteArrayVec(item, 2); + auto optCEncryptProtectedHeader = cbor_.getByteArrayVec(item, 3); + auto optCEncryptUnProtectedHeader = cbor_.getMapItem(item, 4); + auto optPCipheredData = cbor_.getByteArrayVec(item, 5); + auto optRespFlag = cbor_.getUint64(item, 6); + if (!optDecodedKeysToSignMac || !optDecodedDeviceInfo || + !optCEncryptProtectedHeader || !optCEncryptUnProtectedHeader || + !optPCipheredData || !optRespFlag) { LOG(ERROR) << "Error in decoding og response in finishSendData."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } - *keysToSignMac = decodedKeysToSignMac; - deviceInfo->deviceInfo = decodedDeviceInfo; + *keysToSignMac = std::move(optDecodedKeysToSignMac.value()); + deviceInfo->deviceInfo = std::move(optDecodedDeviceInfo.value()); + coseEncryptProtectedHeader = std::move(optCEncryptProtectedHeader.value()); + coseEncryptUnProtectedHeader = std::move(optCEncryptUnProtectedHeader.value()); + partialCipheredData.insert(partialCipheredData.end(), optPCipheredData->begin(), optPCipheredData->end()); + respFlag = std::move(optRespFlag.value()); return ScopedAStatus::ok(); } @@ -208,12 +223,16 @@ JavacardRemotelyProvisionedComponentDevice::getResponse( LOG(ERROR) << "Error in getResponse."; return km_utils::kmError2ScopedAStatus(translateRkpErrorCode(err)); } - if (!cbor_.getBinaryArray(item, 1, partialCipheredData) || - !cbor_.getArrayItem(item, 2, recepientStructure) || - !cbor_.getUint64(item, 3, respFlag)) { + auto optPCipheredData = cbor_.getByteArrayVec(item, 1); + auto optArray = cbor_.getArrayItem(item, 2); + auto optRespFlag = cbor_.getUint64(item, 3); + if (!optPCipheredData || !optArray || !optRespFlag) { LOG(ERROR) << "Error in decoding og response in getResponse."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + recepientStructure = std::move(optArray.value()); + partialCipheredData.insert(partialCipheredData.end(), optPCipheredData->begin(), optPCipheredData->end()); + respFlag = std::move(optRespFlag.value()); return ScopedAStatus::ok(); } diff --git a/HAL/JavacardRemotelyProvisionedComponentDevice.h b/HAL/JavacardRemotelyProvisionedComponentDevice.h index c9dd85c5..99b939a2 100644 --- a/HAL/JavacardRemotelyProvisionedComponentDevice.h +++ b/HAL/JavacardRemotelyProvisionedComponentDevice.h @@ -16,12 +16,14 @@ #pragma once +#include + #include #include #include -#include -#include + #include +#include #include "CborConverter.h" #include "JavacardSecureElement.h" diff --git a/HAL/JavacardSecureElement.cpp b/HAL/JavacardSecureElement.cpp index 0b5cef82..7aff3466 100644 --- a/HAL/JavacardSecureElement.cpp +++ b/HAL/JavacardSecureElement.cpp @@ -16,19 +16,23 @@ #define LOG_TAG "javacard.keymint.device.strongbox-impl" #include "JavacardSecureElement.h" -#include "keymint_utils.h" #include -#include -#include #include #include -#include #include #include #include #include +#include +#include +#include + +#include "keymint_utils.h" + + + namespace keymint::javacard { using namespace ::keymaster; diff --git a/HAL/JavacardSecureElement.h b/HAL/JavacardSecureElement.h index d483a2d2..20a3b964 100644 --- a/HAL/JavacardSecureElement.h +++ b/HAL/JavacardSecureElement.h @@ -16,9 +16,10 @@ #pragma once -#include "CborConverter.h" #include +#include "CborConverter.h" + #define APDU_CLS 0x80 #define APDU_P1 0x50 #define APDU_P2 0x00 diff --git a/HAL/JavacardSharedSecret.cpp b/HAL/JavacardSharedSecret.cpp index d6fd0541..97afeaf0 100644 --- a/HAL/JavacardSharedSecret.cpp +++ b/HAL/JavacardSharedSecret.cpp @@ -1,8 +1,9 @@ #define LOG_TAG "javacard.strongbox.keymint.operation-impl" +#include "JavacardSharedSecret.h" + #include -#include "JavacardSharedSecret.h" -#include +#include "JavacardKeyMintUtils.h" namespace aidl::android::hardware::security::sharedsecret { using namespace ::keymint::javacard; @@ -22,10 +23,12 @@ ScopedAStatus JavacardSharedSecret::getSharedSecretParameters(SharedSecretParame LOG(ERROR) << "Error in sending in getSharedSecretParameters."; return km_utils::kmError2ScopedAStatus(err); } - if (!cbor_.getSharedSecretParameters(item, 1, *params)) { + auto optSSParams = cbor_.getSharedSecretParameters(item, 1); + if (!optSSParams) { LOG(ERROR) << "Error in sending in getSharedSecretParameters."; return km_utils::kmError2ScopedAStatus(KM_ERROR_UNKNOWN_ERROR); } + *params = std::move(optSSParams.value()); return ScopedAStatus::ok(); } @@ -45,10 +48,12 @@ JavacardSharedSecret::computeSharedSecret(const std::vector +#include #include #include -#include -#include + +#include "CborConverter.h" +#include "JavacardSecureElement.h" namespace aidl::android::hardware::security::sharedsecret { using namespace ::keymint::javacard; diff --git a/HAL/OmapiTransport.cpp b/HAL/OmapiTransport.cpp index fc13744a..54d96c1b 100644 --- a/HAL/OmapiTransport.cpp +++ b/HAL/OmapiTransport.cpp @@ -14,6 +14,8 @@ ** See the License for the specific language governing permissions and ** limitations under the License. */ +#include "OmapiTransport.h" + #include #include #include @@ -23,8 +25,6 @@ #include -#include "OmapiTransport.h" - namespace keymint::javacard { constexpr uint8_t SELECTABLE_AID[] = {0xA0, 0x00, 0x00, 0x04, 0x76, 0x41, 0x6E, 0x64, diff --git a/HAL/OmapiTransport.h b/HAL/OmapiTransport.h index 6c71a081..304aaf34 100644 --- a/HAL/OmapiTransport.h +++ b/HAL/OmapiTransport.h @@ -1,16 +1,20 @@ #pragma once -#include "ITransport.h" +#include +#include +#include + + #include #include #include #include #include #include + #include -#include -#include -#include + +#include "ITransport.h" namespace keymint::javacard { using std::vector; diff --git a/HAL/SocketTransport.cpp b/HAL/SocketTransport.cpp index 698024e5..76b50c34 100644 --- a/HAL/SocketTransport.cpp +++ b/HAL/SocketTransport.cpp @@ -15,14 +15,18 @@ ** limitations under the License. */ #include "SocketTransport.h" -#include "ITransport.h" -#include + #include #include + #include -#include #include +#include +#include + +#include "ITransport.h" + #define PORT 8080 #define IPADDR "192.168.7.239" #define MAX_RECV_BUFFER_SIZE 2500 diff --git a/HAL/SocketTransport.h b/HAL/SocketTransport.h index ac8103f6..3baddc86 100644 --- a/HAL/SocketTransport.h +++ b/HAL/SocketTransport.h @@ -15,10 +15,12 @@ ** limitations under the License. */ #pragma once -#include "ITransport.h" + #include #include +#include "ITransport.h" + namespace keymint::javacard { using std::shared_ptr; using std::vector; diff --git a/HAL/keymint_utils.cpp b/HAL/keymint_utils.cpp index a98de129..eca164d5 100644 --- a/HAL/keymint_utils.cpp +++ b/HAL/keymint_utils.cpp @@ -13,11 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#include "keymint_utils.h" -#include -#include #include +#include + namespace keymint::javacard { namespace { diff --git a/HAL/keymint_utils.h b/HAL/keymint_utils.h index 6ceb7f7d..364a959e 100644 --- a/HAL/keymint_utils.h +++ b/HAL/keymint_utils.h @@ -15,6 +15,8 @@ */ #pragma once + +#include #include //#include diff --git a/HAL/service.cpp b/HAL/service.cpp index 14580f8d..4a273b92 100644 --- a/HAL/service.cpp +++ b/HAL/service.cpp @@ -16,19 +16,20 @@ #define LOG_TAG "javacard.strongbox-service" +#include + #include #include #include +#include #include "JavacardKeyMintDevice.h" -#include -#include #include "JavacardSecureElement.h" #include "JavacardSharedSecret.h" -#include "keymint_utils.h" #include "JavacardRemotelyProvisionedComponentDevice.h" -#include -#include +#include "keymint_utils.h" +#include "OmapiTransport.h" +#include "SocketTransport.h" using aidl::android::hardware::security::keymint::JavacardKeyMintDevice; using aidl::android::hardware::security::keymint::JavacardSharedSecret; From d4c5cd099b5cd2813770fb202c7c92f9e85af67d Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Thu, 16 Jun 2022 07:48:10 +0000 Subject: [PATCH 02/10] applet review comments fixes --- .../javacard/keymaster/KMAndroidSEApplet.java | 28 ++--- .../javacard/seprovider/KMPoolManager.java | 42 +++----- .../javacard/keymaster/KMJCardSimApplet.java | 28 ++--- .../javacard/keymaster/KMKeymasterApplet.java | 100 +++++++++--------- .../RemotelyProvisionedComponentDevice.java | 8 +- 5 files changed, 97 insertions(+), 109 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index 848eeefd..01fbcf3a 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -105,13 +105,13 @@ public void process(APDU apdu) { case INS_PROVISION_ATTEST_IDS_CMD: processProvisionAttestIdsCmd(apdu); kmDataStore.setProvisionStatus(PROVISION_STATUS_ATTEST_IDS); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); break; case INS_PROVISION_PRESHARED_SECRET_CMD: processProvisionPreSharedSecretCmd(apdu); kmDataStore.setProvisionStatus(PROVISION_STATUS_PRESHARED_SECRET); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); break; case INS_GET_PROVISION_STATUS_CMD: @@ -132,13 +132,13 @@ public void process(APDU apdu) { case INS_SE_FACTORY_PROVISIONING_LOCK_CMD: kmDataStore.setProvisionStatus(PROVISION_STATUS_SE_LOCKED); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); break; case INS_PROVISION_OEM_ROOT_PUBLIC_KEY_CMD: processProvisionOEMRootPublicKeyCmd(apdu); kmDataStore.setProvisionStatus(PROVISION_STATUS_OEM_PUBLIC_KEY); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); break; case INS_OEM_LOCK_PROVISIONING_CMD: @@ -157,13 +157,13 @@ public void process(APDU apdu) { ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); } } catch (KMException exception) { - sendError(apdu, KMException.reason()); + sendResponse(apdu, KMException.reason()); } catch (ISOException exp) { - sendError(apdu, mapISOErrorToKMError(exp.getReason())); + sendResponse(apdu, mapISOErrorToKMError(exp.getReason())); } catch (CryptoException e) { - sendError(apdu, mapCryptoErrorToKMError(e.getReason())); + sendResponse(apdu, mapCryptoErrorToKMError(e.getReason())); } catch (Exception e) { - sendError(apdu, KMError.GENERIC_UNKNOWN_ERROR); + sendResponse(apdu, KMError.GENERIC_UNKNOWN_ERROR); } finally { repository.clean(); } @@ -253,7 +253,7 @@ private void processOEMUnlockProvisionCmd(APDU apdu) { authenticateOEM(OEM_UNLOCK_PROVISION_VERIFICATION_LABEL, apdu); kmDataStore.setProvisionLock(false); kmDataStore.unlockProvision(PROVISION_STATUS_PROVISIONING_LOCKED); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private void processOEMLockProvisionCmd(APDU apdu) { @@ -261,7 +261,7 @@ private void processOEMLockProvisionCmd(APDU apdu) { // Enable the lock bit in provision status. kmDataStore.setProvisionLock(true); kmDataStore.setProvisionStatus(PROVISION_STATUS_PROVISIONING_LOCKED); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private void authenticateOEM(byte[] plainMsg, APDU apdu) { @@ -366,7 +366,7 @@ private static void processProvisionRkpDeviceUniqueKeyPair(APDU apdu) { MAX_COSE_BUF_SIZE); kmDataStore.persistBootCertificateChain(scratchPad, (short) 0, len); kmDataStore.setProvisionStatus(PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private static void processProvisionRkpAdditionalCertChain(APDU apdu) { @@ -415,7 +415,7 @@ private static void processProvisionRkpAdditionalCertChain(APDU apdu) { kmDataStore.setProvisionStatus(PROVISION_STATUS_ADDITIONAL_CERT_CHAIN); //reclaim memory repository.reclaimMemory(bufferLength); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private void processProvisionAttestIdsCmd(APDU apdu) { @@ -567,7 +567,7 @@ private void processSetBootParamsCmd(APDU apdu) { super.reboot(); kmDataStore.setDeviceBootStatus(KMKeymintDataStore.SET_BOOT_PARAMS_SUCCESS); seProvider.clearDeviceBooted(false); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private boolean isProvisioningComplete() { @@ -656,7 +656,7 @@ private short validateApdu(APDU apdu) { // Validate P1P2. if (P1P2 != KMKeymasterApplet.KM_HAL_VERSION) { - sendError(apdu, KMError.INVALID_P1P2); + sendResponse(apdu, KMError.INVALID_P1P2); return KMType.INVALID_VALUE; } return apduBuffer[ISO7816.OFFSET_INS]; diff --git a/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMPoolManager.java b/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMPoolManager.java index de9d9f37..7643e061 100644 --- a/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMPoolManager.java +++ b/Applet/AndroidSEProviderLib/src/com/android/javacard/seprovider/KMPoolManager.java @@ -185,12 +185,12 @@ public static void initStatics() { private KMPoolManager() { initStatics(); - cipherPool = new Object[(short) (CIPHER_ALGS.length * 4)]; - // Extra 4 algorithms are used to support TRUSTED_CONFIRMATION_REQUIRED feature. - signerPool = new Object[(short) ((SIG_ALGS.length * 4) + 4)]; - keyAgreementPool = new Object[(short) (KEY_AGREE_ALGS.length * 4)]; + cipherPool = new Object[(short) (CIPHER_ALGS.length * MAX_OPERATION_INSTANCES)]; + // Extra 4 algorithms are used to support TRUSTED_CONFIRMATION_REQUIRED feature. + signerPool = new Object[(short) ((SIG_ALGS.length * MAX_OPERATION_INSTANCES) + MAX_OPERATION_INSTANCES)]; + keyAgreementPool = new Object[(short) (KEY_AGREE_ALGS.length * MAX_OPERATION_INSTANCES)]; - keysPool = new Object[(short) ((KEY_ALGS.length * 4) + 4)]; + keysPool = new Object[(short) ((KEY_ALGS.length * MAX_OPERATION_INSTANCES) + MAX_OPERATION_INSTANCES)]; operationPool = new Object[MAX_OPERATION_INSTANCES]; hmacSignOperationPool = new Object[MAX_OPERATION_INSTANCES]; /* Initialize pools */ @@ -213,58 +213,46 @@ private void initializeRKpObjects() { } private void initializeKeysPool() { - short index = 0; - while (index < KEY_ALGS.length) { + for(short index = 0; index < KEY_ALGS.length; index++) { keysPool[index] = createKeyObjectInstance(KEY_ALGS[index]); - index++; } } private void initializeOperationPool() { - short index = 0; - while (index < MAX_OPERATION_INSTANCES) { + for(short index = 0; index < MAX_OPERATION_INSTANCES; index++) { operationPool[index] = new KMOperationImpl(); - index++; } } private void initializeHmacSignOperationPool() { - short index = 0; - while (index < MAX_OPERATION_INSTANCES) { + for(short index = 0; index < MAX_OPERATION_INSTANCES; index++) { hmacSignOperationPool[index] = new KMOperationImpl(); - index++; } } // Create a signature instance of each algorithm once. - private void initializeSignerPool() { - short index = 0; - while (index < SIG_ALGS.length) { + private void initializeSignerPool() { + short index; + for(index = 0; index < SIG_ALGS.length; index++) { signerPool[index] = getSignatureInstance(SIG_ALGS[index]); - index++; } + // Allocate extra 4 HMAC signer instances required for trusted confirmation - short len = (short) (index + 4); - while (index < len) { + for(short len = (short) (index + 4); index < len; index++) { signerPool[index] = getSignatureInstance(Signature.ALG_HMAC_SHA_256); - index++; } } //Create a cipher instance of each algorithm once. private void initializeCipherPool() { - short index = 0; - while (index < CIPHER_ALGS.length) { + for(short index = 0; index < CIPHER_ALGS.length; index++) { cipherPool[index] = getCipherInstance(CIPHER_ALGS[index]); - index++; } } private void initializeKeyAgreementPool() { - short index = 0; - while (index < KEY_AGREE_ALGS.length) { + for(short index = 0; index < KEY_AGREE_ALGS.length; index++) { keyAgreementPool[index] = getKeyAgreementInstance(KEY_AGREE_ALGS[index]); - index++; } } diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java index 2b8e55fe..bbfb6a04 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java @@ -93,13 +93,13 @@ public void process(APDU apdu) { case INS_PROVISION_ATTEST_IDS_CMD: processProvisionAttestIdsCmd(apdu); kmDataStore.setProvisionStatus(PROVISION_STATUS_ATTEST_IDS); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); break; case INS_PROVISION_PRESHARED_SECRET_CMD: processProvisionPreSharedSecretCmd(apdu); kmDataStore.setProvisionStatus(PROVISION_STATUS_PRESHARED_SECRET); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); break; case INS_GET_PROVISION_STATUS_CMD: @@ -120,13 +120,13 @@ public void process(APDU apdu) { case INS_SE_FACTORY_PROVISIONING_LOCK_CMD: kmDataStore.setProvisionStatus(PROVISION_STATUS_SE_LOCKED); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); break; case INS_PROVISION_OEM_ROOT_PUBLIC_KEY_CMD: processProvisionOEMRootPublicKeyCmd(apdu); kmDataStore.setProvisionStatus(PROVISION_STATUS_OEM_PUBLIC_KEY); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); break; case INS_OEM_LOCK_PROVISIONING_CMD: @@ -145,13 +145,13 @@ public void process(APDU apdu) { ISOException.throwIt(ISO7816.SW_COMMAND_NOT_ALLOWED); } } catch (KMException exception) { - sendError(apdu, KMException.reason()); + sendResponse(apdu, KMException.reason()); } catch (ISOException exp) { - sendError(apdu, mapISOErrorToKMError(exp.getReason())); + sendResponse(apdu, mapISOErrorToKMError(exp.getReason())); } catch (CryptoException e) { - sendError(apdu, mapCryptoErrorToKMError(e.getReason())); + sendResponse(apdu, mapCryptoErrorToKMError(e.getReason())); } catch (Exception e) { - sendError(apdu, KMError.GENERIC_UNKNOWN_ERROR); + sendResponse(apdu, KMError.GENERIC_UNKNOWN_ERROR); } finally { repository.clean(); } @@ -241,7 +241,7 @@ private void processOEMUnlockProvisionCmd(APDU apdu) { authenticateOEM(OEM_UNLOCK_PROVISION_VERIFICATION_LABEL, apdu); kmDataStore.setProvisionLock(false); kmDataStore.unlockProvision(PROVISION_STATUS_PROVISIONING_LOCKED); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private void processOEMLockProvisionCmd(APDU apdu) { @@ -249,7 +249,7 @@ private void processOEMLockProvisionCmd(APDU apdu) { // Enable the lock bit in provision status. kmDataStore.setProvisionLock(true); kmDataStore.setProvisionStatus(PROVISION_STATUS_PROVISIONING_LOCKED); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private void authenticateOEM(byte[] plainMsg, APDU apdu) { @@ -354,7 +354,7 @@ private static void processProvisionRkpDeviceUniqueKeyPair(APDU apdu) { MAX_COSE_BUF_SIZE); kmDataStore.persistBootCertificateChain(scratchPad, (short) 0, len); kmDataStore.setProvisionStatus(PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private static void processProvisionRkpAdditionalCertChain(APDU apdu) { @@ -403,7 +403,7 @@ private static void processProvisionRkpAdditionalCertChain(APDU apdu) { kmDataStore.setProvisionStatus(PROVISION_STATUS_ADDITIONAL_CERT_CHAIN); //reclaim memory repository.reclaimMemory(bufferLength); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private void processProvisionAttestIdsCmd(APDU apdu) { @@ -555,7 +555,7 @@ private void processSetBootParamsCmd(APDU apdu) { super.reboot(); kmDataStore.setDeviceBootStatus(KMKeymintDataStore.SET_BOOT_PARAMS_SUCCESS); seProvider.clearDeviceBooted(false); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private boolean isProvisioningComplete() { @@ -588,7 +588,7 @@ private short validateApdu(APDU apdu) { // Validate P1P2. if (P1P2 != KMKeymasterApplet.KM_HAL_VERSION) { - sendError(apdu, KMError.INVALID_P1P2); + sendResponse(apdu, KMError.INVALID_P1P2); return KMType.INVALID_VALUE; } return apduBuffer[ISO7816.OFFSET_INS]; diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index ed51caa0..e10e8220 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -446,7 +446,7 @@ protected short mapCryptoErrorToKMError(short reason) { @Override public void process(APDU apdu) { try { - resetData(); + resetTransientBuffers(); repository.onProcess(); // If this is select applet apdu which is selecting this applet then return if (apdu.isISOInterindustryCLA()) { @@ -462,7 +462,7 @@ public void process(APDU apdu) { switch (apduIns) { case INS_INIT_STRONGBOX_CMD: processInitStrongBoxCmd(apdu); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); return; case INS_GENERATE_KEY_CMD: processGenerateKey(apdu); @@ -546,19 +546,19 @@ public void process(APDU apdu) { } catch (KMException exception) { freeOperations(); resetWrappingKey(); - sendError(apdu, KMException.reason()); + sendResponse(apdu, KMException.reason()); } catch (ISOException exp) { freeOperations(); resetWrappingKey(); - sendError(apdu, mapISOErrorToKMError(exp.getReason())); + sendResponse(apdu, mapISOErrorToKMError(exp.getReason())); } catch (CryptoException e) { freeOperations(); resetWrappingKey(); - sendError(apdu, mapCryptoErrorToKMError(e.getReason())); + sendResponse(apdu, mapCryptoErrorToKMError(e.getReason())); } catch (Exception e) { freeOperations(); resetWrappingKey(); - sendError(apdu, KMError.GENERIC_UNKNOWN_ERROR); + sendResponse(apdu, KMError.GENERIC_UNKNOWN_ERROR); } finally { repository.clean(); } @@ -603,7 +603,7 @@ private void freeOperations() { private void processEarlyBootEndedCmd(APDU apdu) { kmDataStore.setEarlyBootEndedStatus(true); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private short deviceLockedCmd(APDU apdu){ @@ -626,13 +626,13 @@ private void processDeviceLockedCmd(APDU apdu) { short verTime = KMVerificationToken.cast(verToken).getTimestamp(); short lastDeviceLockedTime = kmDataStore.getDeviceTimeStamp(); if (KMInteger.compare(verTime, lastDeviceLockedTime) > 0) { - Util.arrayFillNonAtomic(scratchPad, (short) 0, (short) 8, (byte) 0); - KMInteger.cast(verTime).getValue(scratchPad, (short) 0, (short) 8); + Util.arrayFillNonAtomic(scratchPad, (short) 0, KMInteger.UINT_64, (byte) 0); + KMInteger.cast(verTime).getValue(scratchPad, (short) 0, KMInteger.UINT_64); kmDataStore.setDeviceLock(true); kmDataStore.setDeviceLockPasswordOnly(passwordOnly == 0x01); - kmDataStore.setDeviceLockTimestamp(scratchPad, (short) 0, (short) 8); + kmDataStore.setDeviceLockTimestamp(scratchPad, (short) 0, KMInteger.UINT_64); } - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private void resetWrappingKey(){ @@ -658,7 +658,7 @@ private short getWrappingKey(){ return KMByteBlob.instance(wrappingKey,(short)1,WRAPPING_KEY_SIZE); } - protected void resetData() { + protected void resetTransientBuffers() { short index = 0; while (index < data.length) { data[index] = KMType.INVALID_VALUE; @@ -781,7 +781,7 @@ private void processGetHwInfoCmd(APDU apdu) { JavacardKeymintDevice, (short) 0, (short) JavacardKeymintDevice.length)); resp.add((short) 4, KMByteBlob.instance(Google, (short) 0, (short) Google.length)); resp.add((short)5, KMInteger.uint_8((byte)1)); - // send buffer to master + // send buffer to host sendOutgoing(apdu, respPtr); } @@ -793,7 +793,7 @@ private short addRngEntropyCmd(APDU apdu){ } private void processAddRngEntropyCmd(APDU apdu) { - // Receive the incoming request fully from the master. + // Receive the incoming request fully from the host. short cmd = addRngEntropyCmd(apdu); // Process KMByteBlob blob = KMByteBlob.cast(KMArray.cast(cmd).get((short) 0)); @@ -802,7 +802,7 @@ private void processAddRngEntropyCmd(APDU apdu) { KMException.throwIt(KMError.INVALID_INPUT_LENGTH); } seProvider.addRngEntropy(blob.getBuffer(), blob.getStartOff(), blob.length()); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private short getKeyCharacteristicsCmd(APDU apdu){ @@ -814,7 +814,7 @@ private short getKeyCharacteristicsCmd(APDU apdu){ } private void processGetKeyCharacteristicsCmd(APDU apdu) { - // Receive the incoming request fully from the master. + // Receive the incoming request fully from the host. short cmd = getKeyCharacteristicsCmd(apdu); // Re-purpose the apdu buffer as scratch pad. byte[] scratchPad = apdu.getBuffer(); @@ -861,7 +861,7 @@ private void processGetHmacSharingParamCmd(APDU apdu) { private void processDeleteAllKeysCmd(APDU apdu) { // No arguments // Send ok - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private short createKeyBlobExp(short version) { @@ -921,7 +921,7 @@ private static short createKeyBlobInstance(byte keyType) { private void processDeleteKeyCmd(APDU apdu) { // Send ok - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private short computeSharedHmacCmd(APDU apdu){ @@ -933,7 +933,7 @@ private short computeSharedHmacCmd(APDU apdu){ } private void processComputeSharedHmacCmd(APDU apdu) { - // Receive the incoming request fully from the master into buffer. + // Receive the incoming request fully from the host into buffer. short cmd = computeSharedHmacCmd(apdu); byte[] scratchPad = apdu.getBuffer(); data[HMAC_SHARING_PARAMS] = KMArray.cast(cmd).get((short) 0); @@ -1114,7 +1114,7 @@ private boolean isKeyUpgradeRequired(short keyBlob, short appId, short appData, } private void processUpgradeKeyCmd(APDU apdu) { - // Receive the incoming request fully from the master into buffer. + // Receive the incoming request fully from the host into buffer. short cmd = upgradeKeyCmd(apdu); byte[] scratchPad = apdu.getBuffer(); @@ -1166,7 +1166,7 @@ private void processUpgradeKeyCmd(APDU apdu) { } private void processExportKeyCmd(APDU apdu) { - sendError(apdu, KMError.UNIMPLEMENTED); + sendResponse(apdu, KMError.UNIMPLEMENTED); } private void processWrappingKeyBlob(short keyBlob, short wrapParams, byte[] scratchPad) { @@ -1249,7 +1249,7 @@ private short beginImportWrappedKeyCmd(APDU apdu){ } private void processBeginImportWrappedKeyCmd(APDU apdu) { - // Receive the incoming request fully from the master into buffer. + // Receive the incoming request fully from the host into buffer. short cmd = beginImportWrappedKeyCmd(apdu); byte[] scratchPad = apdu.getBuffer(); // Step -1 parse the wrapping key blob @@ -1267,7 +1267,7 @@ private void processBeginImportWrappedKeyCmd(APDU apdu) { KMException.throwIt(KMError.UNKNOWN_ERROR); } setWrappingKey(transportKey); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private short aesGCMEncrypt(short aesSecret, short input, short nonce, short authData, short authTag,byte[] scratchPad){ Util.arrayFillNonAtomic(scratchPad, (short) 0, KMByteBlob.cast(input).length(), (byte) 0); @@ -1627,11 +1627,11 @@ private static void setUniqueId(KMAttestationCert cert, short attAppId, byte[] s private void processDestroyAttIdsCmd(APDU apdu) { kmDataStore.deleteAttestationIds(); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } private void processVerifyAuthorizationCmd(APDU apdu) { - sendError(apdu, KMError.UNIMPLEMENTED); + sendResponse(apdu, KMError.UNIMPLEMENTED); } private short abortOperationCmd(APDU apdu){ @@ -1645,10 +1645,10 @@ private void processAbortOperationCmd(APDU apdu) { data[OP_HANDLE] = KMArray.cast(cmd).get((short) 0); KMOperationState op = findOperation(data[OP_HANDLE]); if (op == null) { - sendError(apdu,KMError.INVALID_OPERATION_HANDLE); + sendResponse(apdu,KMError.INVALID_OPERATION_HANDLE); }else { releaseOperation(op); - sendError(apdu, KMError.OK); + sendResponse(apdu, KMError.OK); } } @@ -2019,16 +2019,16 @@ private boolean verifyVerificationTokenMacInBigEndian(short verToken, byte[] scr // concatenate challenge - 8 bytes short ptr = KMVerificationToken.cast(verToken).getChallenge(); KMInteger.cast(ptr) - .value(scratchPad, (short) (len + (short) (8 - KMInteger.cast(ptr).length()))); - len += 8; + .value(scratchPad, (short) (len + (short) (KMInteger.UINT_64 - KMInteger.cast(ptr).length()))); + len += KMInteger.UINT_64; // concatenate timestamp -8 bytes ptr = KMVerificationToken.cast(verToken).getTimestamp(); KMInteger.cast(ptr) - .value(scratchPad, (short) (len + (short) (8 - KMInteger.cast(ptr).length()))); - len += 8; + .value(scratchPad, (short) (len + (short) (KMInteger.UINT_64 - KMInteger.cast(ptr).length()))); + len += KMInteger.UINT_64; // concatenate security level - 4 bytes scratchPad[(short) (len + 3)] = TRUSTED_ENVIRONMENT; - len += 4; + len += KMInteger.UINT_32; // hmac the data ptr = KMVerificationToken.cast(verToken).getMac(); @@ -2225,7 +2225,7 @@ private short beginOperationCmd(APDU apdu){ } private void processBeginOperationCmd(APDU apdu) { - // Receive the incoming request fully from the master into buffer. + // Receive the incoming request fully from the host into buffer. short cmd = beginOperationCmd(apdu); byte[] scratchPad = apdu.getBuffer(); short purpose = KMArray.cast(cmd).get((short) 0); @@ -2942,27 +2942,27 @@ private boolean verifyHwTokenMacInBigEndian(short hwToken, byte[] scratchPad) { // concatenate challenge - 8 bytes short ptr = KMHardwareAuthToken.cast(hwToken).getChallenge(); KMInteger.cast(ptr) - .value(scratchPad, (short) (len + (short) (8 - KMInteger.cast(ptr).length()))); - len += 8; + .value(scratchPad, (short) (len + (short) (KMInteger.UINT_64 - KMInteger.cast(ptr).length()))); + len += KMInteger.UINT_64; // concatenate user id - 8 bytes ptr = KMHardwareAuthToken.cast(hwToken).getUserId(); KMInteger.cast(ptr) - .value(scratchPad, (short) (len + (short) (8 - KMInteger.cast(ptr).length()))); - len += 8; + .value(scratchPad, (short) (len + (short) (KMInteger.UINT_64 - KMInteger.cast(ptr).length()))); + len += KMInteger.UINT_64; // concatenate authenticator id - 8 bytes ptr = KMHardwareAuthToken.cast(hwToken).getAuthenticatorId(); KMInteger.cast(ptr) - .value(scratchPad, (short) (len + (short) (8 - KMInteger.cast(ptr).length()))); - len += 8; + .value(scratchPad, (short) (len + (short) (KMInteger.UINT_64 - KMInteger.cast(ptr).length()))); + len += KMInteger.UINT_64; // concatenate authenticator type - 4 bytes ptr = KMHardwareAuthToken.cast(hwToken).getHwAuthenticatorType(); scratchPad[(short) (len + 3)] = KMEnum.cast(ptr).getVal(); - len += 4; + len += KMInteger.UINT_32; // concatenate timestamp -8 bytes ptr = KMHardwareAuthToken.cast(hwToken).getTimestamp(); KMInteger.cast(ptr) - .value(scratchPad, (short) (len + (short) (8 - KMInteger.cast(ptr).length()))); - len += 8; + .value(scratchPad, (short) (len + (short) (KMInteger.UINT_64 - KMInteger.cast(ptr).length()))); + len += KMInteger.UINT_64; ptr = KMHardwareAuthToken.cast(hwToken).getMac(); @@ -2986,24 +2986,24 @@ private boolean verifyHwTokenMacInLittleEndian(short hwToken, byte[] scratchPad) // concatenate challenge - 8 bytes short ptr = KMHardwareAuthToken.cast(hwToken).getChallenge(); KMInteger.cast(ptr).toLittleEndian(scratchPad, len); - len += 8; + len += KMInteger.UINT_64; // concatenate user id - 8 bytes ptr = KMHardwareAuthToken.cast(hwToken).getUserId(); KMInteger.cast(ptr).toLittleEndian(scratchPad, len); - len += 8; + len += KMInteger.UINT_64; // concatenate authenticator id - 8 bytes ptr = KMHardwareAuthToken.cast(hwToken).getAuthenticatorId(); KMInteger.cast(ptr).toLittleEndian(scratchPad, len); - len += 8; + len += KMInteger.UINT_64; // concatenate authenticator type - 4 bytes ptr = KMHardwareAuthToken.cast(hwToken).getHwAuthenticatorType(); scratchPad[(short) (len + 3)] = KMEnum.cast(ptr).getVal(); - len += 4; + len += KMInteger.UINT_32; // concatenate timestamp - 8 bytes ptr = KMHardwareAuthToken.cast(hwToken).getTimestamp(); KMInteger.cast(ptr) .value(scratchPad, (short) (len + (short) (8 - KMInteger.cast(ptr).length()))); - len += 8; + len += KMInteger.UINT_64; ptr = KMHardwareAuthToken.cast(hwToken).getMac(); @@ -3045,7 +3045,7 @@ private short importKeyCmd(APDU apdu){ } private void processImportKeyCmd(APDU apdu) { - // Receive the incoming request fully from the master into buffer. + // Receive the incoming request fully from the host into buffer. short cmd = importKeyCmd(apdu); byte[] scratchPad = apdu.getBuffer(); data[KEY_PARAMETERS] = KMArray.cast(cmd).get((short) 0); @@ -3504,7 +3504,7 @@ private short generateKeyCmd(APDU apdu){ } private void processGenerateKey(APDU apdu) { - // Receive the incoming request fully from the master into buffer. + // Receive the incoming request fully from the host into buffer. short cmd = generateKeyCmd(apdu); // Re-purpose the apdu buffer as scratch pad. byte[] scratchPad = apdu.getBuffer(); @@ -4276,7 +4276,7 @@ private static short deriveKey(byte[] scratchPad) { return len; } - public static void sendError(APDU apdu, short err) { + public static void sendResponse(APDU apdu, short err) { short resp = KMArray.instance((short)1); err = KMError.translate(err); short error = KMInteger.uint_16(err); diff --git a/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java b/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java index 8cd8aab1..aaca3d72 100644 --- a/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java +++ b/Applet/src/com/android/javacard/keymaster/RemotelyProvisionedComponentDevice.java @@ -307,7 +307,7 @@ public void processBeginSendData(APDU apdu) throws Exception { createEntry(GENERATE_CSR_PHASE, BYTE_SIZE); updateState(BEGIN); // Send response. - KMKeymasterApplet.sendError(apdu, KMError.OK); + KMKeymasterApplet.sendResponse(apdu, KMError.OK); } catch (Exception e) { clearDataTable(); releaseOperation(); @@ -350,7 +350,7 @@ public void processUpdateKey(APDU apdu) throws Exception { // Update the csr state updateState(UPDATE); // Send response. - KMKeymasterApplet.sendError(apdu, KMError.OK); + KMKeymasterApplet.sendResponse(apdu, KMError.OK); } catch (Exception e) { clearDataTable(); releaseOperation(); @@ -393,7 +393,7 @@ public void processUpdateEekChain(APDU apdu) throws Exception { Util.arrayCopyNonAtomic(scratchPad, (short) 0, data, dataEntryIndex, len); // Update the state updateState(UPDATE); - KMKeymasterApplet.sendError(apdu, KMError.OK); + KMKeymasterApplet.sendResponse(apdu, KMError.OK); } catch (Exception e) { clearDataTable(); releaseOperation(); @@ -420,7 +420,7 @@ public void processUpdateChallenge(APDU apdu) throws Exception { ); // Update the state updateState(UPDATE); - KMKeymasterApplet.sendError(apdu, KMError.OK); + KMKeymasterApplet.sendResponse(apdu, KMError.OK); } catch (Exception e) { clearDataTable(); releaseOperation(); From cba5303408f2a96156c68a33662f4d6474c605e8 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Fri, 17 Jun 2022 04:22:37 +0000 Subject: [PATCH 03/10] Updated HAL review comment fixes --- HAL/CborConverter.cpp | 14 +++++++------- HAL/service.cpp | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/HAL/CborConverter.cpp b/HAL/CborConverter.cpp index bdd5120e..3df5c6f5 100644 --- a/HAL/CborConverter.cpp +++ b/HAL/CborConverter.cpp @@ -109,7 +109,7 @@ bool CborConverter::addKeyparameters(Array& array, const vector& k std::optional> CborConverter::getKeyCharacteristics(const unique_ptr& item, const uint32_t pos) { vector keyCharacteristics; auto arrayItem = getItemAtPos(item, pos); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem.value()))) { + if (!arrayItem || (MajorType::ARRAY != getType(arrayItem.value()))) { return std::nullopt; } KeyCharacteristics swEnf{SecurityLevel::KEYSTORE, {}}; @@ -258,7 +258,7 @@ CborConverter::getKeyParameter(const std::pair&, std::optional> CborConverter::getCertificateChain(const std::unique_ptr& item, const uint32_t pos) { vector certChain; auto arrayItem = getItemAtPos(item, pos); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem.value()))) return std::nullopt; + if (!arrayItem || (MajorType::ARRAY != getType(arrayItem.value()))) return std::nullopt; const Array* arr = arrayItem.value().get()->asArray(); for (int i = 0; i < arr->size(); i++) { @@ -282,7 +282,7 @@ std::optional CborConverter::getByteArrayStr(const unique_ptr& ite std::optional> CborConverter::getByteArrayVec(const unique_ptr& item, const uint32_t pos) { auto strItem = getItemAtPos(item, pos); - if ((strItem == nullptr) || (MajorType::BSTR != getType(strItem.value()))) { + if (!strItem || (MajorType::BSTR != getType(strItem.value()))) { return std::nullopt; } const Bstr* bstr = strItem.value().get()->asBstr(); @@ -293,7 +293,7 @@ std::optional CborConverter::getSharedSecretParameters(c SharedSecretParameters params; // Array [seed, nonce] auto arrayItem = getItemAtPos(item, pos); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem.value()))) { + if (!arrayItem || (MajorType::ARRAY != getType(arrayItem.value()))) { return std::nullopt; } auto optSeed = getByteArrayVec(arrayItem.value(), 0); @@ -359,7 +359,7 @@ std::optional CborConverter::getTimeStampToken(const unique_ptr< std::optional CborConverter::getArrayItem(const std::unique_ptr& item, const uint32_t pos) { Array array; auto arrayItem = getItemAtPos(item, pos); - if ((arrayItem == nullptr) || (MajorType::ARRAY != getType(arrayItem.value()))) { + if (!arrayItem || (MajorType::ARRAY != getType(arrayItem.value()))) { return std::nullopt; } array = std::move(*(arrayItem.value().get()->asArray())); @@ -369,7 +369,7 @@ std::optional CborConverter::getArrayItem(const std::unique_ptr& it std::optional CborConverter::getMapItem(const std::unique_ptr& item, const uint32_t pos) { Map map; auto mapItem = getItemAtPos(item, pos); - if ((mapItem == nullptr) || (MajorType::MAP != getType(mapItem.value()))) { + if (!mapItem || (MajorType::MAP != getType(mapItem.value()))) { return std::nullopt; } map = std::move(*(mapItem.value().get()->asMap())); @@ -379,7 +379,7 @@ std::optional CborConverter::getMapItem(const std::unique_ptr& item, std::optional> CborConverter::getKeyParameters(const unique_ptr& item, const uint32_t pos) { vector params; auto mapItem = getItemAtPos(item, pos); - if ((mapItem == nullptr) || (MajorType::MAP != getType(mapItem.value()))) return std::nullopt; + if (!mapItem || (MajorType::MAP != getType(mapItem.value()))) return std::nullopt; const Map* map = mapItem.value().get()->asMap(); size_t mapSize = map->size(); for (int i = 0; i < mapSize; i++) { diff --git a/HAL/service.cpp b/HAL/service.cpp index 4a273b92..508486c6 100644 --- a/HAL/service.cpp +++ b/HAL/service.cpp @@ -18,9 +18,9 @@ #include -#include #include #include +#include #include #include "JavacardKeyMintDevice.h" From eb2ed50008a80aef77036b4edb61e98d4e0bb5af Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Fri, 17 Jun 2022 05:42:26 +0000 Subject: [PATCH 04/10] updated review comment fixes --- HAL/CborConverter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HAL/CborConverter.cpp b/HAL/CborConverter.cpp index 3df5c6f5..1d5f1824 100644 --- a/HAL/CborConverter.cpp +++ b/HAL/CborConverter.cpp @@ -150,7 +150,7 @@ CborConverter::getKeyParameter(const std::pair&, key = static_cast(optValue.value()); switch (keymaster_tag_get_type(key)) { case KM_ENUM_REP: { - /* ENUM_REP contains values encoded in a Bit string */ + /* ENUM_REP contains values encoded in a Byte string */ const Bstr* bstr = pair.second.get()->asBstr(); if (bstr == nullptr) { return std::nullopt; From a2f438726de602c07f187d7cf2587f5ae4c6342d Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Mon, 20 Jun 2022 06:19:22 +0000 Subject: [PATCH 05/10] updated applet upgrade --- .../javacard/keymaster/KMAndroidSEApplet.java | 40 +--- .../keymaster/KMKeymintDataStore.java | 205 +++++++++--------- 2 files changed, 113 insertions(+), 132 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index 01fbcf3a..70d65026 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -35,7 +35,7 @@ public class KMAndroidSEApplet extends KMKeymasterApplet implements OnUpgradeLis // Magic number version private static final byte KM_MAGIC_NUMBER = (byte) 0x82; // MSB byte is for Major version and LSB byte is for Minor version. - private static final short KM_APPLET_PACKAGE_VERSION = 0x0200; + private static final short KM_APPLET_PACKAGE_VERSION = 0x0102; private static final byte KM_BEGIN_STATE = 0x00; private static final byte ILLEGAL_STATE = KM_BEGIN_STATE + 1; @@ -223,35 +223,26 @@ private boolean isCommandAllowed(short apduIns) { } private boolean isSeFactoryProvisioningLocked() { - short dInex = repository.allocReclaimableMemory((short)2); - byte data[] = repository.getHeap(); - kmDataStore.getProvisionStatus(data, dInex); - short pStatus = Util.getShort(data, dInex); + short pStatus = kmDataStore.getProvisionStatus(); boolean result = false; if ((0 != (pStatus & PROVISION_STATUS_SE_LOCKED))) { result = true; } - repository.reclaimMemory((short)2); return result; } private boolean isSeFactoryProvisioningComplete() { - short dIndex = repository.allocReclaimableMemory((short)2); - byte data[] = repository.getHeap(); - kmDataStore.getProvisionStatus(data, dIndex); - short pStatus = Util.getShort(data, dIndex); + short pStatus = kmDataStore.getProvisionStatus(); boolean result = false; if ((0 != (pStatus & PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR)) && (0 != ((pStatus & PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)))) { result = true; } - repository.reclaimMemory((short)2); return result; } private void processOEMUnlockProvisionCmd(APDU apdu) { authenticateOEM(OEM_UNLOCK_PROVISION_VERIFICATION_LABEL, apdu); - kmDataStore.setProvisionLock(false); kmDataStore.unlockProvision(PROVISION_STATUS_PROVISIONING_LOCKED); sendResponse(apdu, KMError.OK); } @@ -259,7 +250,6 @@ private void processOEMUnlockProvisionCmd(APDU apdu) { private void processOEMLockProvisionCmd(APDU apdu) { authenticateOEM(OEM_LOCK_PROVISION_VERIFICATION_LABEL, apdu); // Enable the lock bit in provision status. - kmDataStore.setProvisionLock(true); kmDataStore.setProvisionStatus(PROVISION_STATUS_PROVISIONING_LOCKED); sendResponse(apdu, KMError.OK); } @@ -500,7 +490,8 @@ private static short buildErrorStatus(short err) { private void processGetProvisionStatusCmd(APDU apdu) { byte[] scratchpad = apdu.getBuffer(); - kmDataStore.getProvisionStatus(scratchpad, (short) 0); + short pStatus = kmDataStore.getProvisionStatus(); + Util.setShort(scratchpad, (short)0, pStatus); short resp = KMArray.instance((short) 2); KMArray.cast(resp).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(resp).add((short) 1, KMInteger.instance(scratchpad, (short)0, (short)2)); @@ -571,10 +562,7 @@ private void processSetBootParamsCmd(APDU apdu) { } private boolean isProvisioningComplete() { - short dInex = repository.allocReclaimableMemory((short)2); - byte data[] = repository.getHeap(); - kmDataStore.getProvisionStatus(data, dInex); - short pStatus = Util.getShort(data, dInex); + short pStatus = kmDataStore.getProvisionStatus(); boolean result = false; if (kmDataStore.isProvisionLocked() || ((0 != (pStatus & PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR)) && (0 != (pStatus & PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)) @@ -582,7 +570,6 @@ private boolean isProvisioningComplete() { && (0 != (pStatus & PROVISION_STATUS_ATTEST_IDS)))) { result = true; } - repository.reclaimMemory((short)2); return result; } @@ -596,20 +583,9 @@ public void onConsolidate() { private boolean isUpgradeAllowed(short oldVersion) { boolean upgradeAllowed = false; - short oldMajorVersion = (short) ((oldVersion >> 8) & 0x00FF); - short oldMinorVersion = (short) (oldVersion & 0x00FF); - short currentMajorVersion = (short) (KM_APPLET_PACKAGE_VERSION >> 8 & 0x00FF); - short currentMinorVersion = (short) (KM_APPLET_PACKAGE_VERSION & 0x00FF); // Downgrade of the Applet is not allowed. - // Upgrade is not allowed to a next version which is not immediate. - if ((short) (currentMajorVersion - oldMajorVersion) == 1) { - if (currentMinorVersion == 0) { - upgradeAllowed = true; - } - } else if ((short) (currentMajorVersion - oldMajorVersion) == 0) { - if (currentMinorVersion >= oldMinorVersion) { - upgradeAllowed = true; - } + if (KM_APPLET_PACKAGE_VERSION >= oldVersion) { + upgradeAllowed = true; } return upgradeAllowed; } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java index a68cbbbd..5077ff3c 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java @@ -32,7 +32,8 @@ public class KMKeymintDataStore implements KMUpgradable { // Data table configuration - public static final short DATA_INDEX_SIZE = 19; + public static final short OLD_DATA_INDEX_SIZE = 19; + public static final short DATA_INDEX_SIZE = 17; public static final short DATA_INDEX_ENTRY_SIZE = 4; public static final short DATA_INDEX_ENTRY_LENGTH = 0; public static final short DATA_INDEX_ENTRY_OFFSET = 2; @@ -40,22 +41,23 @@ public class KMKeymintDataStore implements KMUpgradable { //TODO reduced data table size from 2048 to 300. public static final short DATA_MEM_SIZE = 300; + // Old Data table offsets + private static final byte OLD_PROVISIONED_LOCKED = 17; + private static final byte OLD_PROVISIONED_STATUS = 18; + // Data table offsets - public static final byte COMPUTED_HMAC_KEY = 0; - public static final byte HMAC_NONCE = 1; - public static final byte BOOT_OS_VERSION = 2; - public static final byte BOOT_OS_PATCH_LEVEL = 3; - public static final byte VENDOR_PATCH_LEVEL = 4; - public static final byte DEVICE_LOCKED_TIME = 5; - public static final byte DEVICE_LOCKED = 6; - public static final byte DEVICE_LOCKED_PASSWORD_ONLY = 7; + public static final byte HMAC_NONCE = 0; + public static final byte BOOT_OS_VERSION = 1; + public static final byte BOOT_OS_PATCH_LEVEL = 2; + public static final byte VENDOR_PATCH_LEVEL = 3; + public static final byte DEVICE_LOCKED_TIME = 4; + public static final byte DEVICE_LOCKED = 5; + public static final byte DEVICE_LOCKED_PASSWORD_ONLY = 6; // Total 8 auth tags, so the next offset is AUTH_TAG_1 + 8 - public static final byte AUTH_TAG_1 = 8; + public static final byte AUTH_TAG_1 = 7; public static final byte DEVICE_STATUS_FLAG = 15; - public static final byte EARLY_BOOT_ENDED_FLAG = 16; - private static final byte PROVISIONED_LOCKED = 17; - private static final byte PROVISIONED_STATUS = 18; + public static final byte EARLY_BOOT_ENDED_FLAG = 16; // Data Item sizes public static final short HMAC_SEED_NONCE_SIZE = 32; @@ -104,8 +106,8 @@ public class KMKeymintDataStore implements KMUpgradable { private boolean deviceBootLocked; private short bootState; - private byte[] dataTable; private short dataIndex; + private byte[] dataTable; private KMSEProvider seProvider; private KMRepository repository; private byte[] additionalCertChain; @@ -117,29 +119,28 @@ public class KMKeymintDataStore implements KMUpgradable { private KMComputedHmacKey computedHmacKey; private KMRkpMacKey rkpMacKey; private byte[] oemRootPublicKey; + private short provisionStatus; public KMKeymintDataStore(KMSEProvider provider, KMRepository repo) { seProvider = provider; repository = repo; boolean isUpgrading = provider.isUpgrading(); - initDataTable(isUpgrading); + initDataTable(); //Initialize the device locked status if (!isUpgrading) { additionalCertChain = new byte[ADDITIONAL_CERT_CHAIN_MAX_SIZE]; bcc = new byte[BCC_MAX_SIZE]; oemRootPublicKey = new byte[65]; - setDeviceLock(false); - setDeviceLockPasswordOnly(false); } + setDeviceLockPasswordOnly(false); + setDeviceLock(false); } - private void initDataTable(boolean isUpgrading) { - if (!isUpgrading) { - if (dataTable == null) { - dataTable = new byte[DATA_MEM_SIZE]; - dataIndex = (short) (DATA_INDEX_SIZE * DATA_INDEX_ENTRY_SIZE); - } - } + private void initDataTable() { + if (dataTable == null) { + dataTable = new byte[DATA_MEM_SIZE]; + dataIndex = (short) (DATA_INDEX_SIZE * DATA_INDEX_ENTRY_SIZE); + } } private short dataAlloc(short length) { @@ -196,6 +197,20 @@ private short readDataEntry(short id, byte[] buf, short offset) { } return len; } + + private short readDataEntry(byte[] dataTable, short id, byte[] buf, short offset) { + id = (short) (id * DATA_INDEX_ENTRY_SIZE); + short len = Util.getShort(dataTable, (short) (id + DATA_INDEX_ENTRY_LENGTH)); + if (len != 0) { + Util.arrayCopyNonAtomic( + dataTable, + Util.getShort(dataTable, (short) (id + DATA_INDEX_ENTRY_OFFSET)), + buf, + offset, + len); + } + return len; + } private short dataLength(short id) { id = (short) (id * DATA_INDEX_ENTRY_SIZE); @@ -741,7 +756,7 @@ public short getVerifiedBootHash(byte[] buffer, short start) { } public short getBootKey(byte[] buffer, short start) { - if (verifiedHash == null) { + if (bootKey == null) { KMException.throwIt(KMError.INVALID_DATA); } Util.arrayCopyNonAtomic(bootKey, (short) 0, buffer, start, (short) bootKey.length); @@ -802,43 +817,24 @@ public void setBootPatchLevel(byte[] buffer, short start, short length) { } Util.arrayCopy(buffer, start, bootPatchLevel, (short) 0, (short) length); } - - public void setProvisionLock(boolean lockValue) { - writeBoolean(PROVISIONED_LOCKED, lockValue); - } public boolean isProvisionLocked() { - try { - return readBoolean(PROVISIONED_LOCKED); - } catch (KMException e) { - if (KMException.reason() != KMError.INVALID_DATA) - KMException.throwIt(KMException.reason()); + if (0 != (provisionStatus & KMKeymasterApplet.PROVISION_STATUS_PROVISIONING_LOCKED)) { + return true; } return false; } - public void setProvisionStatus(short provisionStatus) { - short offset = repository.alloc((short) 2); - byte[] buf = repository.getHeap(); - getProvisionStatus(buf, offset); - provisionStatus |= Util.getShort(buf, offset); - Util.setShort(buf, offset, provisionStatus); - writeDataEntry(PROVISIONED_STATUS, buf, offset, (short) 2); + public void setProvisionStatus(short pStatus) { + provisionStatus |= pStatus; } - public void getProvisionStatus(byte[] scratchpad, short offset) { - Util.setShort(scratchpad, offset, (short)0); - readDataEntry(PROVISIONED_STATUS, scratchpad, offset); + public short getProvisionStatus() { + return provisionStatus; } - + public void unlockProvision(short unlockOffset) { - short offset = repository.alloc((short) 2); - byte[] buf = repository.getHeap(); - getProvisionStatus(buf, offset); - short temp = Util.getShort(buf, offset); - temp &= ~unlockOffset; - Util.setShort(buf, offset, temp); - writeDataEntry(PROVISIONED_STATUS, buf, offset, (short) 2); + provisionStatus &= ~unlockOffset; } public void persistOEMRootPublicKey(byte[] inBuff, short inOffset, short inLength) { @@ -861,11 +857,8 @@ public byte[] getOEMRootPublicKey() { @Override public void onSave(Element element) { // Prmitives - element.write(dataIndex); - element.write(deviceBootLocked); - element.write(bootState); + element.write(provisionStatus); // Objects - element.write(dataTable); element.write(attIdBrand); element.write(attIdDevice); element.write(attIdProduct); @@ -874,17 +867,12 @@ public void onSave(Element element) { element.write(attIdMeId); element.write(attIdManufacturer); element.write(attIdModel); - element.write(verifiedHash); - element.write(bootKey); - element.write(bootPatchLevel); element.write(additionalCertChain); element.write(bcc); element.write(oemRootPublicKey); // Key Objects seProvider.onSave(element, KMDataStoreConstants.INTERFACE_TYPE_MASTER_KEY, masterKey); - seProvider.onSave(element, KMDataStoreConstants.INTERFACE_TYPE_COMPUTED_HMAC_KEY, - computedHmacKey); seProvider.onSave(element, KMDataStoreConstants.INTERFACE_TYPE_PRE_SHARED_KEY, preSharedKey); seProvider.onSave(element, KMDataStoreConstants.INTERFACE_TYPE_DEVICE_UNIQUE_KEY_PAIR, deviceUniqueKeyPair); seProvider.onSave(element, KMDataStoreConstants.INTERFACE_TYPE_RKP_MAC_KEY, rkpMacKey); @@ -892,12 +880,51 @@ public void onSave(Element element) { @Override public void onRestore(Element element, short oldVersion, short currentVersion) { - // Read Primitives - dataIndex = element.readShort(); - deviceBootLocked = element.readBoolean(); - bootState = element.readShort(); + if (oldVersion != currentVersion) { + handlePrevisionVersionUpgrade(element); + } else { + handleCurrentVersionUpgrade(element); + } + } + + private void handlePrevisionVersionUpgrade(Element element) { + // Read Primitives + //restore old data table index + short oldDataIndex = element.readShort(); + element.readBoolean(); // pop deviceBootLocked + element.readShort(); // pop bootState + + // Read Objects + //restore old data table + byte[] oldDataTable = (byte[]) element.readObject(); + + attIdBrand = (byte[]) element.readObject(); + attIdDevice = (byte[]) element.readObject(); + attIdProduct = (byte[]) element.readObject(); + attIdSerial = (byte[]) element.readObject(); + attIdImei = (byte[]) element.readObject(); + attIdMeId = (byte[]) element.readObject(); + attIdManufacturer = (byte[]) element.readObject(); + attIdModel = (byte[]) element.readObject(); + element.readObject(); // pop verifiedHash + element.readObject(); //pop bootKey + element.readObject(); // pop bootPatchLevel + additionalCertChain = (byte[]) element.readObject(); // + bcc = (byte[]) element.readObject(); + + // Read Key Objects + masterKey = (KMMasterKey) seProvider.onRestore(element); + seProvider.onRestore(element); // pop computedHmacKey + preSharedKey = (KMPreSharedKey) seProvider.onRestore(element); + deviceUniqueKeyPair = (KMDeviceUniqueKeyPair) seProvider.onRestore(element); + rkpMacKey = (KMRkpMacKey) seProvider.onRestore(element); + handleProvisionStatusUpgrade(oldDataTable, oldDataIndex); + } + + private void handleCurrentVersionUpgrade(Element element) { + // Read Primitives + provisionStatus = element.readShort(); // Read Objects - dataTable = (byte[]) element.readObject(); attIdBrand = (byte[]) element.readObject(); attIdDevice = (byte[]) element.readObject(); attIdProduct = (byte[]) element.readObject(); @@ -906,58 +933,38 @@ public void onRestore(Element element, short oldVersion, short currentVersion) { attIdMeId = (byte[]) element.readObject(); attIdManufacturer = (byte[]) element.readObject(); attIdModel = (byte[]) element.readObject(); - verifiedHash = (byte[]) element.readObject(); - bootKey = (byte[]) element.readObject(); - bootPatchLevel = (byte[]) element.readObject(); additionalCertChain = (byte[]) element.readObject(); bcc = (byte[]) element.readObject(); - //oemRootPublicKey has to be provisioned - if (oldVersion >= 0x0200) { - oemRootPublicKey = (byte[]) element.readObject(); - } + oemRootPublicKey = (byte[]) element.readObject(); // Read Key Objects masterKey = (KMMasterKey) seProvider.onRestore(element); - computedHmacKey = (KMComputedHmacKey) seProvider.onRestore(element); preSharedKey = (KMPreSharedKey) seProvider.onRestore(element); deviceUniqueKeyPair = (KMDeviceUniqueKeyPair) seProvider.onRestore(element); rkpMacKey = (KMRkpMacKey) seProvider.onRestore(element); - handleDataUpgrade(oldVersion, currentVersion); } - void handleDataUpgrade(short oldVersion, short currentVersion) { - if(oldVersion != currentVersion) { - handleProvisionStatusUpgrade(); - } + public void getProvisionStatus(byte[] dataTable, byte[] scratchpad, short offset) { + Util.setShort(scratchpad, offset, (short)0); + readDataEntry(dataTable, OLD_PROVISIONED_STATUS, scratchpad, offset); } - void handleProvisionStatusUpgrade(){ + void handleProvisionStatusUpgrade(byte[] dataTable, short dataTableIndex){ short dInex = repository.allocReclaimableMemory((short)2); byte data[] = repository.getHeap(); - getProvisionStatus(data, dInex); - short newStatus = (short)( data[dInex] & 0x00ff); + getProvisionStatus(dataTable, data, dInex); + provisionStatus = (short)( data[dInex] & 0x00ff); if( KMKeymasterApplet.PROVISION_STATUS_PROVISIONING_LOCKED - == (newStatus & KMKeymasterApplet.PROVISION_STATUS_PROVISIONING_LOCKED)) { - newStatus |= KMKeymasterApplet.PROVISION_STATUS_SE_LOCKED; + == (provisionStatus & KMKeymasterApplet.PROVISION_STATUS_PROVISIONING_LOCKED)) { + provisionStatus |= KMKeymasterApplet.PROVISION_STATUS_SE_LOCKED; } - Util.setShort(data, dInex, newStatus); - short pStatusOff = (short) (PROVISIONED_STATUS * DATA_INDEX_ENTRY_SIZE); - JCSystem.beginTransaction(); - Util.setShort(dataTable, (short) (pStatusOff + DATA_INDEX_ENTRY_OFFSET), (short)0); - Util.setShort(dataTable, (short) (pStatusOff + DATA_INDEX_ENTRY_LENGTH), (short)0); - JCSystem.commitTransaction(); - writeDataEntry(PROVISIONED_STATUS, data, dInex, (short) 2); repository.reclaimMemory((short)2); } @Override public short getBackupPrimitiveByteCount() { - // dataIndex - 2 bytes - // deviceLocked - 1 byte - // deviceState = 2 bytes - return (short) (5 + + // provisionStatus - 2 bytes + return (short) (2 + seProvider.getBackupPrimitiveByteCount(KMDataStoreConstants.INTERFACE_TYPE_MASTER_KEY) + - seProvider.getBackupPrimitiveByteCount( - KMDataStoreConstants.INTERFACE_TYPE_COMPUTED_HMAC_KEY) + seProvider.getBackupPrimitiveByteCount(KMDataStoreConstants.INTERFACE_TYPE_PRE_SHARED_KEY) + seProvider.getBackupPrimitiveByteCount( KMDataStoreConstants.INTERFACE_TYPE_DEVICE_UNIQUE_KEY_PAIR) + seProvider.getBackupPrimitiveByteCount(KMDataStoreConstants.INTERFACE_TYPE_RKP_MAC_KEY)); @@ -965,13 +972,11 @@ public short getBackupPrimitiveByteCount() { @Override public short getBackupObjectCount() { - // dataTable - 1 // AttestationIds - 8 - // bootParameters - 3 // AdditionalCertificateChain - 1 // BCC - 1 // oemRootPublicKey - 1 - return (short) (15 + + return (short) (11 + seProvider.getBackupObjectCount(KMDataStoreConstants.INTERFACE_TYPE_COMPUTED_HMAC_KEY) + seProvider.getBackupObjectCount(KMDataStoreConstants.INTERFACE_TYPE_MASTER_KEY) + seProvider.getBackupObjectCount(KMDataStoreConstants.INTERFACE_TYPE_PRE_SHARED_KEY) + From 8daae0c7c0230c1957a012ab9fa686bc1d0fc0d9 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Mon, 20 Jun 2022 07:16:11 +0000 Subject: [PATCH 06/10] Allow earlyboot before KM is ready --- Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java | 1 + 1 file changed, 1 insertion(+) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index e10e8220..4c79abf8 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -579,6 +579,7 @@ private boolean isKeymintReady(byte apduIns) { case INS_GET_HMAC_SHARING_PARAM_CMD: case INS_COMPUTE_SHARED_HMAC_CMD: case INS_INIT_STRONGBOX_CMD: + case INS_EARLY_BOOT_ENDED_CMD: return true; default: break; From 2232e42ce097fa1cefa5b3905a5f23c659f71b74 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Mon, 20 Jun 2022 07:44:07 +0000 Subject: [PATCH 07/10] setting the device locked timestamp during first access --- .../com/android/javacard/keymaster/KMKeymasterApplet.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index e10e8220..d2836879 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -624,7 +624,12 @@ private void processDeviceLockedCmd(APDU apdu) { passwordOnly = KMInteger.cast(passwordOnly).getByte(); validateVerificationToken(verToken, scratchPad); short verTime = KMVerificationToken.cast(verToken).getTimestamp(); - short lastDeviceLockedTime = kmDataStore.getDeviceTimeStamp(); + short lastDeviceLockedTime; + try { + lastDeviceLockedTime = kmDataStore.getDeviceTimeStamp(); + } catch (KMException e) { + lastDeviceLockedTime = KMInteger.uint_8((byte) 0); + } if (KMInteger.compare(verTime, lastDeviceLockedTime) > 0) { Util.arrayFillNonAtomic(scratchPad, (short) 0, KMInteger.UINT_64, (byte) 0); KMInteger.cast(verTime).getValue(scratchPad, (short) 0, KMInteger.UINT_64); From 2792d5f959a6f5b5db059b64b5b9e7a33e84aa42 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Mon, 20 Jun 2022 10:42:13 +0000 Subject: [PATCH 08/10] validate token at first --- .../android/javacard/keymaster/KMKeymasterApplet.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index e10e8220..459c854f 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -1922,6 +1922,9 @@ private void authorizeUpdateFinishOperation(KMOperationState op, byte[] scratchP } op.setAuthTimeoutValidated(true); } else if (op.isAuthPerOperationReqd()) { // If Auth per operation is required + if (!validateHwToken(data[HW_TOKEN], scratchPad)) { + KMException.throwIt(KMError.KEY_USER_NOT_AUTHENTICATED); + } tmpVariables[0] = KMHardwareAuthToken.cast(data[HW_TOKEN]).getChallenge(); if (KMInteger.compare(data[OP_HANDLE], tmpVariables[0]) != 0) { KMException.throwIt(KMError.KEY_USER_NOT_AUTHENTICATED); @@ -1983,7 +1986,7 @@ private void authorizeDeviceUnlock(byte[] scratchPad) { KMType.BOOL_TAG, KMType.UNLOCKED_DEVICE_REQUIRED, data[HW_PARAMETERS]); if (ptr != KMType.INVALID_VALUE && kmDataStore.getDeviceLock()) { - if (!validateHwToken(data[HW_TOKEN], scratchPad)) { + if (data[HW_TOKEN] == KMType.INVALID_VALUE) { KMException.throwIt(KMError.DEVICE_LOCKED); } ptr = KMHardwareAuthToken.cast(data[HW_TOKEN]).getTimestamp(); @@ -2592,6 +2595,9 @@ private void authorizeAndBeginOperation(KMOperationState op, byte[] scratchPad) authorizeDigest(op); authorizePadding(op); authorizeBlockModeAndMacLength(op); + if (!validateHwToken(data[HW_TOKEN], scratchPad)) { + data[HW_TOKEN] = KMType.INVALID_VALUE; + } authorizeUserSecureIdAuthTimeout(op, scratchPad); authorizeDeviceUnlock(scratchPad); authorizeKeyUsageForCount(scratchPad); @@ -2861,7 +2867,7 @@ private boolean isHwAuthTokenContainsMatchingSecureId(short hwAuthToken, private boolean authTokenMatches(short userSecureIdsPtr, short authType, byte[] scratchPad) { - if (!validateHwToken(data[HW_TOKEN], scratchPad)) { + if (data[HW_TOKEN] == KMType.INVALID_VALUE) { return false; } if (!isHwAuthTokenContainsMatchingSecureId(data[HW_TOKEN], userSecureIdsPtr)) { From d76bc17dc6ec8f179a58722730215f85730105e8 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Tue, 21 Jun 2022 04:28:53 +0000 Subject: [PATCH 09/10] Updated applet upgrade --- .../javacard/keymaster/KMAndroidSEApplet.java | 27 +++++----- .../javacard/keymaster/KMJCardSimApplet.java | 50 +++++++------------ .../keymaster/KMKeymintDataStore.java | 25 ++++++---- 3 files changed, 44 insertions(+), 58 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java index 70d65026..99ca1ffd 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEApplet.java @@ -35,7 +35,7 @@ public class KMAndroidSEApplet extends KMKeymasterApplet implements OnUpgradeLis // Magic number version private static final byte KM_MAGIC_NUMBER = (byte) 0x82; // MSB byte is for Major version and LSB byte is for Minor version. - private static final short KM_APPLET_PACKAGE_VERSION = 0x0102; + private static final short KM_APPLET_PACKAGE_VERSION = 0x0200; private static final byte KM_BEGIN_STATE = 0x00; private static final byte ILLEGAL_STATE = KM_BEGIN_STATE + 1; @@ -233,22 +233,21 @@ private boolean isSeFactoryProvisioningLocked() { private boolean isSeFactoryProvisioningComplete() { short pStatus = kmDataStore.getProvisionStatus(); - boolean result = false; - if ((0 != (pStatus & PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR)) - && (0 != ((pStatus & PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)))) { - result = true; + short seCompleteStatus = PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_ADDITIONAL_CERT_CHAIN; + if (seCompleteStatus == (pStatus & seCompleteStatus)) { + return true; } - return result; + return false; } private void processOEMUnlockProvisionCmd(APDU apdu) { authenticateOEM(OEM_UNLOCK_PROVISION_VERIFICATION_LABEL, apdu); - kmDataStore.unlockProvision(PROVISION_STATUS_PROVISIONING_LOCKED); + kmDataStore.unlockProvision(); sendResponse(apdu, KMError.OK); } private void processOEMLockProvisionCmd(APDU apdu) { - authenticateOEM(OEM_LOCK_PROVISION_VERIFICATION_LABEL, apdu); + authenticateOEM(OEM_LOCK_PROVISION_VERIFICATION_LABEL, apdu); // Enable the lock bit in provision status. kmDataStore.setProvisionStatus(PROVISION_STATUS_PROVISIONING_LOCKED); sendResponse(apdu, KMError.OK); @@ -563,14 +562,12 @@ private void processSetBootParamsCmd(APDU apdu) { private boolean isProvisioningComplete() { short pStatus = kmDataStore.getProvisionStatus(); - boolean result = false; - if (kmDataStore.isProvisionLocked() || ((0 != (pStatus & PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR)) - && (0 != (pStatus & PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)) - && (0 != (pStatus & PROVISION_STATUS_PRESHARED_SECRET)) - && (0 != (pStatus & PROVISION_STATUS_ATTEST_IDS)))) { - result = true; + short pCompleteStatus = PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_ADDITIONAL_CERT_CHAIN | + PROVISION_STATUS_PRESHARED_SECRET | PROVISION_STATUS_ATTEST_IDS; + if (kmDataStore.isProvisionLocked() || (pCompleteStatus == (pStatus & pCompleteStatus))) { + return true; } - return result; + return false; } @Override diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java index bbfb6a04..cd665ef3 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimApplet.java @@ -204,50 +204,39 @@ private boolean isCommandAllowed(short apduIns) { default: // Allow other commands only if provision is completed. if (!isProvisioningComplete()) { - result = false; + result = false; } } return result; } private boolean isSeFactoryProvisioningLocked() { - short dInex = repository.allocReclaimableMemory((short)2); - byte data[] = repository.getHeap(); - kmDataStore.getProvisionStatus(data, dInex); - short pStatus = Util.getShort(data, dInex); + short pStatus = kmDataStore.getProvisionStatus(); boolean result = false; if ((0 != (pStatus & PROVISION_STATUS_SE_LOCKED))) { result = true; } - repository.reclaimMemory((short)2); return result; } private boolean isSeFactoryProvisioningComplete() { - short dIndex = repository.allocReclaimableMemory((short)2); - byte data[] = repository.getHeap(); - kmDataStore.getProvisionStatus(data, dIndex); - short pStatus = Util.getShort(data, dIndex); - boolean result = false; - if ((0 != (pStatus & PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR)) - && (0 != ((pStatus & PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)))) { - result = true; + short pStatus = kmDataStore.getProvisionStatus(); + short seCompleteStatus = PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_ADDITIONAL_CERT_CHAIN; + if (seCompleteStatus == (pStatus & seCompleteStatus)) { + return true; } - repository.reclaimMemory((short)2); - return result; + return false; } private void processOEMUnlockProvisionCmd(APDU apdu) { authenticateOEM(OEM_UNLOCK_PROVISION_VERIFICATION_LABEL, apdu); - kmDataStore.setProvisionLock(false); - kmDataStore.unlockProvision(PROVISION_STATUS_PROVISIONING_LOCKED); + kmDataStore.unlockProvision(); sendResponse(apdu, KMError.OK); } private void processOEMLockProvisionCmd(APDU apdu) { - authenticateOEM(OEM_LOCK_PROVISION_VERIFICATION_LABEL, apdu); + authenticateOEM(OEM_LOCK_PROVISION_VERIFICATION_LABEL, apdu); // Enable the lock bit in provision status. - kmDataStore.setProvisionLock(true); kmDataStore.setProvisionStatus(PROVISION_STATUS_PROVISIONING_LOCKED); sendResponse(apdu, KMError.OK); } @@ -488,7 +477,8 @@ private static short buildErrorStatus(short err) { private void processGetProvisionStatusCmd(APDU apdu) { byte[] scratchpad = apdu.getBuffer(); - kmDataStore.getProvisionStatus(scratchpad, (short) 0); + short pStatus = kmDataStore.getProvisionStatus(); + Util.setShort(scratchpad, (short)0, pStatus); short resp = KMArray.instance((short) 2); KMArray.cast(resp).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(resp).add((short) 1, KMInteger.instance(scratchpad, (short)0, (short)2)); @@ -559,19 +549,13 @@ private void processSetBootParamsCmd(APDU apdu) { } private boolean isProvisioningComplete() { - short dInex = repository.allocReclaimableMemory((short)2); - byte data[] = repository.getHeap(); - kmDataStore.getProvisionStatus(data, dInex); - short pStatus = Util.getShort(data, dInex); - boolean result = false; - if (kmDataStore.isProvisionLocked() || ((0 != (pStatus & PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR)) - && (0 != (pStatus & PROVISION_STATUS_ADDITIONAL_CERT_CHAIN)) - && (0 != (pStatus & PROVISION_STATUS_PRESHARED_SECRET)) - && (0 != (pStatus & PROVISION_STATUS_ATTEST_IDS)))) { - result = true; + short pStatus = kmDataStore.getProvisionStatus(); + short pCompleteStatus = PROVISION_STATUS_DEVICE_UNIQUE_KEYPAIR | PROVISION_STATUS_ADDITIONAL_CERT_CHAIN | + PROVISION_STATUS_PRESHARED_SECRET | PROVISION_STATUS_ATTEST_IDS; + if (kmDataStore.isProvisionLocked() || (pCompleteStatus == (pStatus & pCompleteStatus))) { + return true; } - repository.reclaimMemory((short)2); - return result; + return false; } private short validateApdu(APDU apdu) { diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java index 5077ff3c..4380cb67 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymintDataStore.java @@ -42,8 +42,7 @@ public class KMKeymintDataStore implements KMUpgradable { public static final short DATA_MEM_SIZE = 300; // Old Data table offsets - private static final byte OLD_PROVISIONED_LOCKED = 17; - private static final byte OLD_PROVISIONED_STATUS = 18; + private static final byte OLD_PROVISIONED_STATUS_OFFSET = 18; // Data table offsets public static final byte HMAC_NONCE = 0; @@ -350,9 +349,9 @@ public boolean isDeviceReady() { short offset = repository.allocReclaimableMemory(DEVICE_STATUS_FLAG_SIZE); byte[] buf = repository.getHeap(); getDeviceBootStatus(buf, offset); - if ((0 != (buf[offset] & SET_BOOT_PARAMS_SUCCESS)) - && (0 != (buf[offset] & SET_SYSTEM_PROPERTIES_SUCCESS)) - && (0 != (buf[offset] & NEGOTIATED_SHARED_SECRET_SUCCESS))) { + byte bootCompleteStatus = SET_BOOT_PARAMS_SUCCESS | SET_SYSTEM_PROPERTIES_SUCCESS | + SET_SYSTEM_PROPERTIES_SUCCESS; + if (bootCompleteStatus == (buf[offset] & bootCompleteStatus)) { result = true; } repository.reclaimMemory(DEVICE_STATUS_FLAG_SIZE); @@ -826,15 +825,19 @@ public boolean isProvisionLocked() { } public void setProvisionStatus(short pStatus) { + JCSystem.beginTransaction(); provisionStatus |= pStatus; + JCSystem.commitTransaction(); } public short getProvisionStatus() { return provisionStatus; } - public void unlockProvision(short unlockOffset) { - provisionStatus &= ~unlockOffset; + public void unlockProvision() { + JCSystem.beginTransaction(); + provisionStatus &= ~KMKeymasterApplet.PROVISION_STATUS_PROVISIONING_LOCKED; + JCSystem.commitTransaction(); } public void persistOEMRootPublicKey(byte[] inBuff, short inOffset, short inLength) { @@ -881,13 +884,13 @@ public void onSave(Element element) { @Override public void onRestore(Element element, short oldVersion, short currentVersion) { if (oldVersion != currentVersion) { - handlePrevisionVersionUpgrade(element); + handlePreviousVersionUpgrade(element); } else { handleCurrentVersionUpgrade(element); } } - private void handlePrevisionVersionUpgrade(Element element) { + private void handlePreviousVersionUpgrade(Element element) { // Read Primitives //restore old data table index short oldDataIndex = element.readShort(); @@ -945,18 +948,20 @@ private void handleCurrentVersionUpgrade(Element element) { public void getProvisionStatus(byte[] dataTable, byte[] scratchpad, short offset) { Util.setShort(scratchpad, offset, (short)0); - readDataEntry(dataTable, OLD_PROVISIONED_STATUS, scratchpad, offset); + readDataEntry(dataTable, OLD_PROVISIONED_STATUS_OFFSET, scratchpad, offset); } void handleProvisionStatusUpgrade(byte[] dataTable, short dataTableIndex){ short dInex = repository.allocReclaimableMemory((short)2); byte data[] = repository.getHeap(); getProvisionStatus(dataTable, data, dInex); + JCSystem.beginTransaction(); provisionStatus = (short)( data[dInex] & 0x00ff); if( KMKeymasterApplet.PROVISION_STATUS_PROVISIONING_LOCKED == (provisionStatus & KMKeymasterApplet.PROVISION_STATUS_PROVISIONING_LOCKED)) { provisionStatus |= KMKeymasterApplet.PROVISION_STATUS_SE_LOCKED; } + JCSystem.commitTransaction(); repository.reclaimMemory((short)2); } From 5cb7bed7f82b9907901529ec3f5ede37ad18cfe1 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Tue, 21 Jun 2022 06:34:17 +0000 Subject: [PATCH 10/10] handled aes des no pad zero input --- .../javacard/keymaster/KMKeymasterApplet.java | 22 +++++++++++++++++-- .../javacard/keymaster/KMOperationState.java | 13 +++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index e10e8220..c3339420 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -1776,7 +1776,19 @@ private void finishAesDesOperation(KMOperationState op){ KMByteBlob.cast(data[OUTPUT_DATA]).getStartOff()); } catch (CryptoException e) { if (e.getReason() == CryptoException.ILLEGAL_USE) { - KMException.throwIt(KMError.INVALID_INPUT_LENGTH); + // As per VTS, zero length input on AES/DES with PADDING_NONE Should return a zero length + // output. But JavaCard fails with CryptoException.ILLEGAL_USE if no input data is + // provided via update() method. So ignore this exception in case if all below conditions + // are satisfied and simply return empty output. + // 1. padding mode is PADDING_NONE. + // 2. No input message is processed in update(). + // 3. Zero length input data is passed in finish operation. + if ((op.getPadding() == KMType.PADDING_NONE) && + !op.isInputMsgProcessed() && (KMByteBlob.cast(data[INPUT_DATA]).length() == 0)) { + len = 0; + } else { + KMException.throwIt(KMError.INVALID_INPUT_LENGTH); + } } } KMByteBlob.cast(data[OUTPUT_DATA]).setLength(len); @@ -2136,7 +2148,13 @@ private void processUpdateOperationCmd(APDU apdu) { } catch (CryptoException e) { KMException.throwIt(KMError.INVALID_TAG); } - + if (KMByteBlob.cast(data[INPUT_DATA]).length() > 0) { + // This flag is used to denote that an input data of length > 0 is received and processed + // successfully in update command. This flag is later used in the finish operation + // to handle a particular use case, where a zero length input data on AES/DES algorithm + // with PADDING_NONE should return a zero length output with OK response. + op.setProcessedInputMsg(true); + } // Adjust the Output data if it is not equal to input data. // This happens in case of JCardSim provider. KMByteBlob.cast(data[OUTPUT_DATA]).setLength(len); diff --git a/Applet/src/com/android/javacard/keymaster/KMOperationState.java b/Applet/src/com/android/javacard/keymaster/KMOperationState.java index a6ed2280..8c9bb545 100644 --- a/Applet/src/com/android/javacard/keymaster/KMOperationState.java +++ b/Applet/src/com/android/javacard/keymaster/KMOperationState.java @@ -56,6 +56,7 @@ public class KMOperationState { private static final short SECURE_USER_ID_REQD = 2; private static final short AUTH_TIMEOUT_VALIDATED = 4; private static final short AES_GCM_UPDATE_ALLOWED = 8; + private static final byte PROCESSED_INPUT_MSG = 16; // Max user secure ids. private static final byte MAX_SECURE_USER_IDS = 5; @@ -122,6 +123,10 @@ public void setPurpose(short purpose) { data[PURPOSE] = purpose; } + public boolean isInputMsgProcessed() { + return (data[FLAGS] & PROCESSED_INPUT_MSG) != 0; + } + public void setOperation(KMOperation op) { operations[OPERATION] = op; } @@ -150,6 +155,14 @@ public void setAuthTime(byte[] timeBuf, short start) { Util.arrayCopyNonAtomic(timeBuf, start, authTime, (short) 0, AUTH_TIME_SIZE); } + public void setProcessedInputMsg(boolean flag) { + if (flag) { + data[FLAGS] = (byte) (data[FLAGS] | PROCESSED_INPUT_MSG); + } else { + data[FLAGS] = (byte) (data[FLAGS] & (~PROCESSED_INPUT_MSG)); + } + } + public void setOneTimeAuthReqd(boolean flag) { if (flag) { data[FLAGS] = (short) (data[FLAGS] | SECURE_USER_ID_REQD);