From 19e33fcd28d6e241ca12fa0f817f1dd96ab8b1e8 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Thu, 16 Jun 2022 07:36:44 +0000 Subject: [PATCH 1/3] 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 cba5303408f2a96156c68a33662f4d6474c605e8 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Fri, 17 Jun 2022 04:22:37 +0000 Subject: [PATCH 2/3] 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 3/3] 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;