From 30c356c26c7bdebadd82f7d9c130bfd45438be37 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 3 Jul 2023 13:59:21 +0200 Subject: [PATCH 01/16] Modify decider logic Allow importing of all types of asymmetric keys to TZ backend. Add unit-test Change-Id: Iebbd0d5f37b4568b8c2473cdfe178d1ddad85a86 --- src/manager/crypto/platform/decider.cpp | 38 +++++--- src/manager/crypto/platform/decider.h | 6 +- unit-tests/CMakeLists.txt | 1 + unit-tests/test_decider.cpp | 156 ++++++++++++++++++++++++++++++++ 4 files changed, 185 insertions(+), 16 deletions(-) create mode 100644 unit-tests/test_decider.cpp diff --git a/src/manager/crypto/platform/decider.cpp b/src/manager/crypto/platform/decider.cpp index bdb976e..a1409d8 100644 --- a/src/manager/crypto/platform/decider.cpp +++ b/src/manager/crypto/platform/decider.cpp @@ -91,22 +91,26 @@ GStore* Decider::tryBackend(CryptoBackend backend) /* * operation encrypted type extractable backend * ---------------------------------------------- - * import FALSE binary - TZ/SW + * import FALSE binary * TZ/SW * skey FALSE TZ/SW * skey TRUE SW - * akey - SW - * cert - SW - * TRUE binary - TZ + * akey FALSE TZ/SW + * akey TRUE SW + * cert * SW + * ---------------------------------------------- + * import TRUE binary * TZ * skey FALSE TZ * skey TRUE NONE - * akey - NONE - * cert - NONE - * generate - binary - TZ/SW - * - cert - NONE - * - skey FALSE TZ/SW - * - skey TRUE SW - * - akey FALSE TZ/SW - * - akey TRUE SW + * akey FALSE TZ + * akey TRUE NONE + * cert * NONE + * ---------------------------------------------- + * generate N/A binary * TZ/SW + * skey FALSE TZ/SW + * skey TRUE SW + * akey FALSE TZ/SW + * akey TRUE SW + * cert * NONE */ std::deque Decider::getCompatibleBackends(DataType data, const Policy &policy, @@ -131,7 +135,7 @@ std::deque Decider::getCompatibleBackends(DataType data, if (!encrypted) addSW(); - if (data.isBinaryData() || (data.isSymmetricKey() && !policy.extractable)) + if (data.isBinaryData() || (data.isKey() && !policy.extractable)) addTZ(); } else { // generate/derive assert(!encrypted); @@ -160,9 +164,13 @@ GStore &Decider::getStore(DataType data, const Policy &policy, bool import, bool ThrowErr(Exc::Crypto::InternalError, "Failed to connect to a compatible backend."); } -bool Decider::checkStore(CryptoBackend requestedBackend, DataType data, const Policy &policy, bool import) +bool Decider::checkStore(CryptoBackend requestedBackend, + DataType data, + const Policy &policy, + bool import, + bool encrypted) { - auto backends = getCompatibleBackends(data, policy, import); + auto backends = getCompatibleBackends(data, policy, import, encrypted); for (auto id : backends) { if (id == requestedBackend) return true; diff --git a/src/manager/crypto/platform/decider.h b/src/manager/crypto/platform/decider.h index 47fb08d..e4c24fe 100644 --- a/src/manager/crypto/platform/decider.h +++ b/src/manager/crypto/platform/decider.h @@ -47,7 +47,11 @@ public: const Policy &policy, bool import = true, bool encrypted = false); - bool checkStore(CryptoBackend id, DataType data, const Policy &policy, bool import); + bool checkStore(CryptoBackend id, + DataType data, + const Policy &policy, + bool import, + bool encrypted = false); private: GStore* tryBackend(CryptoBackend backend); diff --git a/unit-tests/CMakeLists.txt b/unit-tests/CMakeLists.txt index 139836d..3f7ad2e 100644 --- a/unit-tests/CMakeLists.txt +++ b/unit-tests/CMakeLists.txt @@ -74,6 +74,7 @@ SET(UNIT_TESTS_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/test_crypto-logic.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_data-type.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_db_crypto.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test_decider.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_descriptor-set.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_dpl-db.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_dpl-exception.cpp diff --git a/unit-tests/test_decider.cpp b/unit-tests/test_decider.cpp new file mode 100644 index 0000000..6328977 --- /dev/null +++ b/unit-tests/test_decider.cpp @@ -0,0 +1,156 @@ +/* + * Copyright (c) 2023 Samsung Electronics Co., Ltd All Rights Reserved + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +#include + +#include + +#include + +using namespace CKM; +using namespace CKM::Crypto; + +namespace { + +struct Mapping { + bool import; // true - import, false - generate + bool encrypted; + DataType type; + bool extractable; + bool swBackend; + bool tzBackend; +}; + +std::vector MAPPING { +// imp., enc., type, ext., SW, TZ + {true, false, DataType::BINARY_DATA, false, true, true }, + {true, false, DataType::BINARY_DATA, true, true, true }, + + {true, false, DataType::KEY_AES, false, true, true }, + {true, false, DataType::KEY_AES, true, true, false }, + + {true, false, DataType::KEY_RSA_PRIVATE, false, true, true }, + {true, false, DataType::KEY_RSA_PRIVATE, true, true, false }, + {true, false, DataType::KEY_RSA_PUBLIC, false, true, true }, + {true, false, DataType::KEY_RSA_PUBLIC, true, true, false }, + + {true, false, DataType::KEY_DSA_PRIVATE, false, true, true }, + {true, false, DataType::KEY_DSA_PRIVATE, true, true, false }, + {true, false, DataType::KEY_DSA_PUBLIC, false, true, true }, + {true, false, DataType::KEY_DSA_PUBLIC, true, true, false }, + + {true, false, DataType::KEY_ECDSA_PRIVATE, false, true, true }, + {true, false, DataType::KEY_ECDSA_PRIVATE, true, true, false }, + {true, false, DataType::KEY_ECDSA_PUBLIC, false, true, true }, + {true, false, DataType::KEY_ECDSA_PUBLIC, true, true, false }, + + {true, false, DataType::CERTIFICATE, false, true, false }, + {true, false, DataType::CERTIFICATE, true, true, false }, + + {true, false, DataType::CHAIN_CERT_0, false, true, false }, + {true, false, DataType::CHAIN_CERT_0, true, true, false }, + + + {true, true, DataType::BINARY_DATA, false, false, true }, + {true, true, DataType::BINARY_DATA, true, false, true }, + + {true, true, DataType::KEY_AES, false, false, true }, + {true, true, DataType::KEY_AES, true, false, false }, + + {true, true, DataType::KEY_RSA_PRIVATE, false, false, true }, + {true, true, DataType::KEY_RSA_PRIVATE, true, false, false }, + {true, true, DataType::KEY_RSA_PUBLIC, false, false, true }, + {true, true, DataType::KEY_RSA_PUBLIC, true, false, false }, + + {true, true, DataType::KEY_DSA_PRIVATE, false, false, true }, + {true, true, DataType::KEY_DSA_PRIVATE, true, false, false }, + {true, true, DataType::KEY_DSA_PUBLIC, false, false, true }, + {true, true, DataType::KEY_DSA_PUBLIC, true, false, false }, + + {true, true, DataType::KEY_ECDSA_PRIVATE, false, false, true }, + {true, true, DataType::KEY_ECDSA_PRIVATE, true, false, false }, + {true, true, DataType::KEY_ECDSA_PUBLIC, false, false, true }, + {true, true, DataType::KEY_ECDSA_PUBLIC, true, false, false }, + + {true, true, DataType::CERTIFICATE, false, false, false }, + {true, true, DataType::CERTIFICATE, true, false, false }, + + {true, true, DataType::CHAIN_CERT_0, false, false, false }, + {true, true, DataType::CHAIN_CERT_0, true, false, false }, + + + {false, false, DataType::BINARY_DATA, false, true, true }, + {false, false, DataType::BINARY_DATA, true, true, true }, + + {false, false, DataType::KEY_AES, false, true, true }, + {false, false, DataType::KEY_AES, true, true, false }, + + {false, false, DataType::KEY_RSA_PRIVATE, false, true, true }, + {false, false, DataType::KEY_RSA_PRIVATE, true, true, false }, + {false, false, DataType::KEY_RSA_PUBLIC, false, true, true }, + {false, false, DataType::KEY_RSA_PUBLIC, true, true, false }, + + {false, false, DataType::KEY_DSA_PRIVATE, false, true, true }, + {false, false, DataType::KEY_DSA_PRIVATE, true, true, false }, + {false, false, DataType::KEY_DSA_PUBLIC, false, true, true }, + {false, false, DataType::KEY_DSA_PUBLIC, true, true, false }, + + {false, false, DataType::KEY_ECDSA_PRIVATE, false, true, true }, + {false, false, DataType::KEY_ECDSA_PRIVATE, true, true, false }, + {false, false, DataType::KEY_ECDSA_PUBLIC, false, true, true }, + {false, false, DataType::KEY_ECDSA_PUBLIC, true, true, false }, + + {false, false, DataType::CERTIFICATE, false, false, false }, + {false, false, DataType::CERTIFICATE, true, false, false }, + + {false, false, DataType::CHAIN_CERT_0, false, false, false }, + {false, false, DataType::CHAIN_CERT_0, true, false, false }, +}; + +} // namespace + +BOOST_AUTO_TEST_SUITE(DECIDER_TEST) + +POSITIVE_TEST_CASE(MappingTest) +{ + Decider d; + bool ret; + for (const auto& row : MAPPING) { + Policy policy("", row.extractable); + + ret = d.checkStore(CryptoBackend::OpenSSL, row.type, policy, row.import, row.encrypted); + BOOST_REQUIRE(ret == row.swBackend); + + ret = d.checkStore(CryptoBackend::TrustZone, row.type, policy, row.import, row.encrypted); +#ifdef TZ_BACKEND_ENABLED + BOOST_REQUIRE(ret == row.tzBackend); +#else + BOOST_REQUIRE(ret == false); +#endif + + ret = d.checkStore(CryptoBackend::None, row.type, policy, row.import, row.encrypted); + BOOST_REQUIRE(ret == false); + + ret = d.checkStore(CryptoBackend::SecureElement, + row.type, + policy, + row.import, + row.encrypted); + BOOST_REQUIRE(ret == false); + } +} + +BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From 1a5aff3586e14aabe3326c06a74e212701fbe3c9 Mon Sep 17 00:00:00 2001 From: wchang kim Date: Tue, 4 Jul 2023 08:06:34 +0900 Subject: [PATCH 02/16] Fixed the build error using gcc 13 Change-Id: I716b3be00e9e2015591af34b33031726fa1b5969 --- common/DBFixture.cpp | 2 +- misc/ckm_db_tool/CMakeLists.txt | 2 ++ misc/encryption_perf/main.cpp | 2 +- src/manager/common/stringify.h | 6 +++++- src/manager/initial-values/NoCharactersHandler.cpp | 1 + src/manager/service/ckm-logic.cpp | 2 +- src/manager/service/for-each-file.h | 1 + src/manager/sqlcipher/sqlcipher.c | 3 ++- unit-tests/CMakeLists.txt | 2 ++ 9 files changed, 16 insertions(+), 5 deletions(-) diff --git a/common/DBFixture.cpp b/common/DBFixture.cpp index ee85abc..b310521 100644 --- a/common/DBFixture.cpp +++ b/common/DBFixture.cpp @@ -233,7 +233,7 @@ void DBFixture::insert_row(const Name &name, const ClientId &owner) void DBFixture::delete_row(const Name &name, const ClientId &owner) { - bool exit_flag; + bool exit_flag=0; BOOST_REQUIRE_NO_THROW(exit_flag = m_db.deleteRow(name, owner)); BOOST_REQUIRE_MESSAGE(true == exit_flag, "remove name failed: no rows removed"); } diff --git a/misc/ckm_db_tool/CMakeLists.txt b/misc/ckm_db_tool/CMakeLists.txt index 00b800e..a061da9 100644 --- a/misc/ckm_db_tool/CMakeLists.txt +++ b/misc/ckm_db_tool/CMakeLists.txt @@ -2,6 +2,8 @@ SET(CKM_DB_TOOL "ckm_db_tool") SET(CKM_DB_MERGE "ckm_db_merge") SET(KEY_MANAGER_PATH ${PROJECT_SOURCE_DIR}/src/manager) +ADD_DEFINITIONS("-Wno-alloc-size-larger-than") + FIND_PACKAGE(Threads REQUIRED) INCLUDE_DIRECTORIES( diff --git a/misc/encryption_perf/main.cpp b/misc/encryption_perf/main.cpp index 1a2409f..0dd6ab9 100644 --- a/misc/encryption_perf/main.cpp +++ b/misc/encryption_perf/main.cpp @@ -9,6 +9,7 @@ const char* const ALIAS = "enc_perf_test"; int main(int argc, char* argv[]) { + std::chrono::duration duration{}; int ret; ckmc_policy_s policy {nullptr, false}; ckmc_raw_buffer_s* input = nullptr; @@ -57,7 +58,6 @@ int main(int argc, char* argv[]) goto finish; } - std::chrono::duration duration; for(;;) { size_t chunkSize = std::min(size, maxSize); diff --git a/src/manager/common/stringify.h b/src/manager/common/stringify.h index 300b1ea..fc919fd 100644 --- a/src/manager/common/stringify.h +++ b/src/manager/common/stringify.h @@ -66,6 +66,10 @@ #define STRINGIFY_15(msg, ...) << msg STRINGIFY_14(__VA_ARGS__) #define STRINGIFY_(N, ...) CONCAT(STRINGIFY_, N)(__VA_ARGS__) +/* + * flush() is needed to work around a bug described here: + * https://bugs.llvm.org/show_bug.cgi?id=20596 + */ #define Stringify(...) \ - (static_cast(std::ostringstream() \ + (static_cast(std::ostringstream().flush() \ STRINGIFY_(PP_NARG(__VA_ARGS__), __VA_ARGS__))).str() diff --git a/src/manager/initial-values/NoCharactersHandler.cpp b/src/manager/initial-values/NoCharactersHandler.cpp index e43c130..a23c9d5 100644 --- a/src/manager/initial-values/NoCharactersHandler.cpp +++ b/src/manager/initial-values/NoCharactersHandler.cpp @@ -23,6 +23,7 @@ #include #include +#include #include namespace CKM { diff --git a/src/manager/service/ckm-logic.cpp b/src/manager/service/ckm-logic.cpp index f57f920..ea59099 100644 --- a/src/manager/service/ckm-logic.cpp +++ b/src/manager/service/ckm-logic.cpp @@ -1601,7 +1601,7 @@ RawBuffer CKMLogic::importWrappedKey( } Token token = wrappingKey->unwrap(params, - Crypto::Data(keyType, std::move(wrappedKey)), + Crypto::Data(keyType, wrappedKey), policy.password, digest); diff --git a/src/manager/service/for-each-file.h b/src/manager/service/for-each-file.h index 0c03234..7b35ade 100644 --- a/src/manager/service/for-each-file.h +++ b/src/manager/service/for-each-file.h @@ -23,6 +23,7 @@ #include #include +#include namespace CKM { diff --git a/src/manager/sqlcipher/sqlcipher.c b/src/manager/sqlcipher/sqlcipher.c index 97059a2..eadacc5 100644 --- a/src/manager/sqlcipher/sqlcipher.c +++ b/src/manager/sqlcipher/sqlcipher.c @@ -119238,7 +119238,8 @@ SQLITE_PRIVATE void sqlite3DefaultRowEst(Index *pIdx){ if( x<99 ){ pIdx->pTable->nRowLogEst = x = 99; } - if( pIdx->pPartIdxWhere!=0 ) x -= 10; assert( 10==sqlite3LogEst(2) ); + if( pIdx->pPartIdxWhere!=0 ) x -= 10; + assert( 10==sqlite3LogEst(2) ); a[0] = x; /* Estimate that a[1] is 10, a[2] is 9, a[3] is 8, a[4] is 7, a[5] is diff --git a/unit-tests/CMakeLists.txt b/unit-tests/CMakeLists.txt index 3f7ad2e..1dfb7be 100644 --- a/unit-tests/CMakeLists.txt +++ b/unit-tests/CMakeLists.txt @@ -34,6 +34,8 @@ ADD_DEFINITIONS("-DPKCS12_TEST_DIR=\"${PKCS12_TEST_DIR}\"") ADD_DEFINITIONS("-DBOOST_TEST_DYN_LINK") ADD_DEFINITIONS("-DOVERRIDE_SOCKET_TIMEOUT=10") +ADD_DEFINITIONS("-Wno-self-move") +ADD_DEFINITIONS("-Wno-stringop-truncation") SET(MANAGER_PATH ${PROJECT_SOURCE_DIR}/src/manager) -- 2.7.4 From 3d3465f96895b5a6eab2b211cc8742a5902bdd76 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 10 Jul 2023 17:31:33 +0200 Subject: [PATCH 03/16] Fix TZ backend issues * Add missing namespaces * Include ctx.cpp in TZ source list * Add missing operation id to internal TZ calls Change-Id: I59e71b7af5a1c418f797e7d915b8a9d1fc456edf --- CMakeLists.txt | 1 + src/manager/crypto/sw-backend/ctx.cpp | 2 ++ src/manager/crypto/sw-backend/ctx.h | 2 ++ src/manager/crypto/tz-backend/ctx.cpp | 6 ++++-- src/manager/crypto/tz-backend/ctx.h | 2 ++ 5 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6af5d8c..2ad368e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -84,6 +84,7 @@ IF (TZ_BACKEND_ENABLED) SET(TZ_BACKEND_SOURCES ${KEY_MANAGER_PATH}/crypto/tz-backend/internals.cpp ${KEY_MANAGER_PATH}/crypto/tz-backend/obj.cpp + ${KEY_MANAGER_PATH}/crypto/tz-backend/ctx.cpp ${KEY_MANAGER_PATH}/crypto/tz-backend/store.cpp ${KEY_MANAGER_PATH}/crypto/tz-backend/tz-context.cpp ${KEY_MANAGER_PATH}/crypto/tz-backend/tz-memory.cpp diff --git a/src/manager/crypto/sw-backend/ctx.cpp b/src/manager/crypto/sw-backend/ctx.cpp index cffd4f4..1dcf0f7 100644 --- a/src/manager/crypto/sw-backend/ctx.cpp +++ b/src/manager/crypto/sw-backend/ctx.cpp @@ -20,6 +20,7 @@ namespace CKM { namespace Crypto { +namespace SW { namespace { constexpr uint8_t STATE_INITIALIZED = 1; @@ -109,5 +110,6 @@ RawBuffer CipherCtx::finalize(const RawBuffer& input) return tag; } +} // namespace SW } // namespace Crypto } // namespace CKM diff --git a/src/manager/crypto/sw-backend/ctx.h b/src/manager/crypto/sw-backend/ctx.h index 04f65a3..428997f 100644 --- a/src/manager/crypto/sw-backend/ctx.h +++ b/src/manager/crypto/sw-backend/ctx.h @@ -23,6 +23,7 @@ namespace CKM { namespace Crypto { +namespace SW { class CipherCtx : public GCtx { public: @@ -38,5 +39,6 @@ private: uint8_t m_state; }; +} // namespace SW } // namespace Crypto } // namespace CKM diff --git a/src/manager/crypto/tz-backend/ctx.cpp b/src/manager/crypto/tz-backend/ctx.cpp index 1fcc80b..db877d8 100644 --- a/src/manager/crypto/tz-backend/ctx.cpp +++ b/src/manager/crypto/tz-backend/ctx.cpp @@ -20,6 +20,7 @@ namespace CKM { namespace Crypto { +namespace TZ { void CipherCtx::customize(const CryptoAlgorithm& algo) { @@ -32,7 +33,7 @@ void CipherCtx::customize(const CryptoAlgorithm& algo) RawBuffer CipherCtx::update(const RawBuffer& input) { - return Internals::updateCipher(input); + return Internals::updateCipher(m_opId, input); } RawBuffer CipherCtx::finalize(const RawBuffer& input) @@ -41,8 +42,9 @@ RawBuffer CipherCtx::finalize(const RawBuffer& input) * It is assumed that finalize for GCM encryption will return the GCM tag only. * In case of GCM decryption the tag will be passed as finalizeCipher argument. */ - return Internals::finalizeCipher(input); + return Internals::finalizeCipher(m_opId, input); } +} // namespace TZ } // namespace Crypto } // namespace CKM diff --git a/src/manager/crypto/tz-backend/ctx.h b/src/manager/crypto/tz-backend/ctx.h index d12b5ce..32eba26 100644 --- a/src/manager/crypto/tz-backend/ctx.h +++ b/src/manager/crypto/tz-backend/ctx.h @@ -20,6 +20,7 @@ namespace CKM { namespace Crypto { +namespace TZ { class CipherCtx : public GCtx { public: @@ -33,5 +34,6 @@ private: uint32_t m_opId; }; +} // namespace TZ } // namespace Crypto } // namespace CKM -- 2.7.4 From dec40f83bd7cfde6f5483caad663f3820ac1bc96 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 11 Jul 2023 12:07:18 +0200 Subject: [PATCH 04/16] Release 0.1.55 * Fix TZ backend issues * Fixed the build error using gcc 13 * Modify decider logic * Allow EC keys to be imported to TZ backend Change-Id: I409287a6d1b9f14deb34041dcce904bcbb43f7ba --- packaging/key-manager.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/key-manager.spec b/packaging/key-manager.spec index 2231921..ea86c3d 100644 --- a/packaging/key-manager.spec +++ b/packaging/key-manager.spec @@ -12,7 +12,7 @@ Name: key-manager Summary: Central Key Manager and utilities -Version: 0.1.54 +Version: 0.1.55 Release: 1 Group: Security/Secure Storage License: Apache-2.0 and BSD-3-Clause -- 2.7.4 From b43f739a1f13d98004ffa73f5f151f772ca9dce2 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 11 Jul 2023 13:05:27 +0200 Subject: [PATCH 05/16] Use proper memory type in TZ backend's addGcmAAD() We need TEEC_VALUE_INOUT to properly read the return code from op.params[0].value.a. Change-Id: I95eb5fd757f9e3235bb855269dd0a804ac7bb135 --- src/manager/crypto/tz-backend/tz-context.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index f33897b..1359258 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -510,7 +510,7 @@ void TrustZoneContext::addGcmAAD(uint32_t opId, TrustZoneMemory inMemory(m_Context, sIn.GetSize(), TEEC_MEM_INPUT); sIn.Serialize(inMemory); - TEEC_Operation op = makeOp(TEEC_VALUE_INPUT, inMemory); + TEEC_Operation op = makeOp(TEEC_VALUE_INOUT, inMemory); op.params[0].value.a = opId; Execute(CMD_CIPHER_INIT_AAD, &op); -- 2.7.4 From f746ffb6c1aba2d28630466ed014278cf4c946e2 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 12 Jul 2023 08:58:38 +0200 Subject: [PATCH 06/16] Reserve enough space for GCM tag in TZ backend Change-Id: I36f9718cfdc37f7fdac1e47fc056aeaabdeee242 --- src/manager/crypto/tz-backend/tz-context.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index 1359258..3cf5702 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -50,6 +50,9 @@ namespace { // whatever TA will return us. const uint32_t CIPHER_EXTRA_PADDING_SIZE = 16; +// Maximum size of GCM tag in bytes. +const size_t MAX_GCM_TAG_SIZE = 16; + // Identifier of our TA const TEEC_UUID KEY_MANAGER_TA_UUID = KM_TA_UUID; @@ -547,7 +550,7 @@ RawBuffer TrustZoneContext::finalizeGcmCipher(uint32_t opId, sIn.Serialize(inMemory); TZSerializer sOut; - sOut.Push(new TZSerializableBinary(data.size())); + sOut.Push(new TZSerializableBinary(MAX_GCM_TAG_SIZE, false)); TrustZoneMemory outMemory(m_Context, sOut.GetSize(), TEEC_MEM_OUTPUT); TEEC_Operation op = makeOp(TEEC_VALUE_INOUT, inMemory, outMemory); -- 2.7.4 From 9b9b398a22db98ebc16b1d942d48c3e8fc27d775 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 12 Jul 2023 10:59:31 +0200 Subject: [PATCH 07/16] Retrieve TZ raw key data only when needed Change-Id: Ia1ef537b9696e39c53c1f4972f96ead4cb0fb81a --- src/manager/crypto/tz-backend/obj.cpp | 8 ++++++++ src/manager/crypto/tz-backend/obj.h | 11 ++++------- src/manager/crypto/tz-backend/store.cpp | 5 +---- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/manager/crypto/tz-backend/obj.cpp b/src/manager/crypto/tz-backend/obj.cpp index 55a4164..8d85d1a 100644 --- a/src/manager/crypto/tz-backend/obj.cpp +++ b/src/manager/crypto/tz-backend/obj.cpp @@ -177,6 +177,14 @@ GCtxShPtr AKey::initContext(const CryptoAlgorithm &, bool) ThrowErr(Exc::Crypto::OperationNotSupported); } +RawBuffer AKey::getBinary() const +{ + if (m_type.isKeyPublic() && m_raw.empty()) + m_raw = Internals::getData(getId(), getPassword()); + + return m_raw; +} + RawBuffer AKey::sign( const CryptoAlgorithm &alg, const RawBuffer &message) diff --git a/src/manager/crypto/tz-backend/obj.h b/src/manager/crypto/tz-backend/obj.h index 2b0fff3..eec1e9f 100644 --- a/src/manager/crypto/tz-backend/obj.h +++ b/src/manager/crypto/tz-backend/obj.h @@ -93,7 +93,7 @@ protected: int m_scheme; Pwd m_password; RawBuffer m_id; - RawBuffer m_raw; + mutable RawBuffer m_raw; }; class Key : public BData { @@ -127,13 +127,10 @@ public: int scheme, RawBuffer id, Pwd pwd, - DataType dataType, - RawBuffer raw = RawBuffer()) : - Key(backendId, scheme, std::move(id), std::move(pwd)), m_type(dataType) - { - m_raw = std::move(raw); - } + DataType dataType) : + Key(backendId, scheme, std::move(id), std::move(pwd)), m_type(dataType) {} + RawBuffer getBinary() const override; RawBuffer sign(const CryptoAlgorithm &alg, const RawBuffer &message) override; int verify(const CryptoAlgorithm &alg, const RawBuffer &message, const RawBuffer &sign) override; diff --git a/src/manager/crypto/tz-backend/store.cpp b/src/manager/crypto/tz-backend/store.cpp index ff06b77..958da3a 100644 --- a/src/manager/crypto/tz-backend/store.cpp +++ b/src/manager/crypto/tz-backend/store.cpp @@ -68,7 +68,7 @@ GObjUPtr Store::getObject(const Token &token, const Password &pass) RawBuffer tag; unpack(token.data, pass, scheme, id, iv, tag); - if (token.dataType.isKeyPrivate()) + if (token.dataType.isKeyPrivate() || token.dataType.isKeyPublic()) return make(scheme, std::move(id), Pwd(pass, iv, tag), token.dataType); if (token.dataType.isSymmetricKey()) @@ -80,9 +80,6 @@ GObjUPtr Store::getObject(const Token &token, const Password &pass) auto pwd = Pwd(pass, iv, tag); RawBuffer raw = Internals::getData(id, pwd); - if (token.dataType.isKeyPublic()) - return make(scheme, std::move(id), std::move(pwd), token.dataType, std::move(raw)); - if (token.dataType.isBinaryData()) return make(scheme, std::move(id), std::move(pwd), std::move(raw)); -- 2.7.4 From c5eaefa47dbf184c791e3b6f01275b9993b191d9 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 13 Jul 2023 09:55:15 +0200 Subject: [PATCH 08/16] Documentation fixes * Do not use @see inside other tags * Add missing parenthesis Change-Id: I4b7492eb410c6f510b6848689faf622dd0b8dc5b --- src/include/ckmc/ckmc-manager.h | 14 ++++++-------- src/include/ckmc/ckmc-type.h | 7 +++---- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/include/ckmc/ckmc-manager.h b/src/include/ckmc/ckmc-manager.h index f4db37f..35d021c 100644 --- a/src/include/ckmc/ckmc-manager.h +++ b/src/include/ckmc/ckmc-manager.h @@ -1003,9 +1003,8 @@ int ckmc_remove_alias(const char *alias); * @param[in] password The password used in decrypting a key value. If password of the policy is * provided in ckmc_save_key(), the same password should be provided * @param[in] decrypted Data to be encrypted. In case of AES algorithm the backend may impose limit - * on the maximum size of processed data - * (@see ckmc_backend_get_max_chunk_size()). For RSA the size must be smaller - * or equal to key size in bytes - 42. + * on the maximum size of processed data (ckmc_backend_get_max_chunk_size()). + * For RSA the size must be smaller or equal to key size in bytes - 42. * Example: for 1024 RSA key the maximum data size is 1024/8 - 42 = 86. * @param[out] ppencrypted Encrypted data. In #CKMC_ALGO_AES_GCM mode it includes the GCM tag * appended at the end. @@ -1063,9 +1062,8 @@ int ckmc_encrypt_data(ckmc_param_list_h params, * provided in ckmc_save_key(), the same password should be provided * @param[in] encrypted Data to be decrypted. #CKMC_ALGO_AES_GCM mode requires GCM tag to be * appended at the end. In case of AES algorithm the backend may impose limit - * on the maximum size of processed data - * (@see ckmc_backend_get_max_chunk_size()). For RSA the size must be smaller - * or equal to key size in bytes - 42. + * on the maximum size of processed data (ckmc_backend_get_max_chunk_size()). + * For RSA the size must be smaller or equal to key size in bytes - 42. * @param[out] ppdecrypted Decrypted data * * @return @c 0 on success, otherwise a negative error value @@ -1115,7 +1113,7 @@ int ckmc_decrypt_data(ckmc_param_list_h params, * @remarks If extractable in @a policy is set to false, the stored key may still be exported in a * wrapped form. * @remarks Note that the backend may impose limit on the maximum size of @a wrapped_key - * (@see ckmc_backend_get_max_chunk_size()). + * (ckmc_backend_get_max_chunk_size()). * * @param[in] params Algorithm parameter list handle. See #ckmc_param_list_h and #ckmc_algo_type_e * for details. Supported algorithms: @@ -1323,7 +1321,7 @@ int ckmc_cipher_initialize(ckmc_param_list_h params, * @remarks The newly created @a ppout must be destroyed using ckmc_buffer_free() when it's no * longer needed. * @remarks Note that the backend may impose limit on the maximum size of processed data - * (@see ckmc_backend_get_max_chunk_size()). + * (ckmc_backend_get_max_chunk_size()). * * @param[in] context Encryption/decryption context created with ckmc_cipher_initialize() * @param[in] in Encryption/decryption input diff --git a/src/include/ckmc/ckmc-type.h b/src/include/ckmc/ckmc-type.h index 7275776..c3240e8 100644 --- a/src/include/ckmc/ckmc-type.h +++ b/src/include/ckmc/ckmc-type.h @@ -121,7 +121,7 @@ typedef enum __ckmc_ec_type { CKMC_EC_PRIME192V1 = 0, /**< Elliptic curve domain "secp192r1" listed in "SEC 2" recommended elliptic curve domain */ CKMC_EC_PRIME256V1, /**< "SEC 2" recommended elliptic curve domain - secp256r1 */ - CKMC_EC_SECP384R1 /**< NIST curve P-384(covers "secp384r1", the elliptic curve domain + CKMC_EC_SECP384R1 /**< NIST curve P-384 (covers "secp384r1"), the elliptic curve domain listed in See SEC 2 */ } ckmc_ec_type_e; @@ -420,14 +420,13 @@ typedef enum __ckmc_algo_type { - #CKMC_PARAM_ALGO_TYPE = #CKMC_ALGO_AES_GCM (mandatory), - #CKMC_PARAM_ED_IV = 1 to (2^64-1) bytes long initialization vector. Note that the backend may impose additional limit on the maximum size - (@see ckmc_backend_get_max_chunk_size()). Recommended length is 12B - (mandatory) + (ckmc_backend_get_max_chunk_size()). Recommended length is 12B (mandatory) - #CKMC_PARAM_ED_TAG_LEN = GCM tag length in bits. One of {32, 64, 96, 104, 112, 120, 128} (optional, if not present, the length 128 is used; since Tizen 5.0, if TrustZone backend is used, 32 and 64 lengths are not supported) - #CKMC_PARAM_ED_AAD = additional authentication data. Note that the backend - may impose limit on the maximum size (@see ckmc_backend_get_max_chunk_size()) + may impose limit on the maximum size (ckmc_backend_get_max_chunk_size()) (optional) */ CKMC_ALGO_AES_CFB, /**< AES-CFB algorithm -- 2.7.4 From 07dc5255ca7574b7b0aa445d15cab55714b870fe Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 14 Jul 2023 14:39:59 +0200 Subject: [PATCH 09/16] Release 0.1.56 * Documentation fixes * Retrieve TZ raw key data only when needed * Reserve enough space for GCM tag in TZ backend * Use proper memory type in TZ backend's addGcmAAD() Change-Id: I38b89ee3a76a62420f148a2dd836d7ffe1d1072d --- packaging/key-manager.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/key-manager.spec b/packaging/key-manager.spec index ea86c3d..cf467fb 100644 --- a/packaging/key-manager.spec +++ b/packaging/key-manager.spec @@ -12,7 +12,7 @@ Name: key-manager Summary: Central Key Manager and utilities -Version: 0.1.55 +Version: 0.1.56 Release: 1 Group: Security/Secure Storage License: Apache-2.0 and BSD-3-Clause -- 2.7.4 From 4504dd76691bf51e9920083ec45091469382ce5b Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 20 Jul 2023 14:05:22 +0200 Subject: [PATCH 10/16] Fix secret pwd passing in TZ backend KBKDF Change-Id: I6f1a4d588a6a0679b88f967fdbc71b436329153f --- src/manager/crypto/tz-backend/internals.cpp | 2 ++ src/manager/crypto/tz-backend/internals.h | 1 + src/manager/crypto/tz-backend/obj.cpp | 2 +- src/manager/crypto/tz-backend/tz-context.cpp | 2 ++ src/manager/crypto/tz-backend/tz-context.h | 1 + 5 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index ae8f9f8..d0cc232 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -802,6 +802,7 @@ void deriveECDH(const RawBuffer &prvKeyId, } void deriveKBKDF(const RawBuffer &secretId, + const Pwd &secretPwd, const CryptoAlgorithm &alg, const Password &keyPwd, const RawBuffer &keyPwdIV, @@ -824,6 +825,7 @@ void deriveKBKDF(const RawBuffer &secretId, RawBuffer keyPwdBuf(keyPwd.begin(), keyPwd.end()); TrustZoneContext::Instance().executeKbkdf(secretId, + secretPwd, label, context, fixed, diff --git a/src/manager/crypto/tz-backend/internals.h b/src/manager/crypto/tz-backend/internals.h index bb8e444..8797065 100644 --- a/src/manager/crypto/tz-backend/internals.h +++ b/src/manager/crypto/tz-backend/internals.h @@ -156,6 +156,7 @@ void deriveECDH(const RawBuffer &prvKeyId, const RawBuffer &secretHash); void deriveKBKDF(const RawBuffer &secretId, + const Pwd &secretPwd, const CryptoAlgorithm &alg, const Password &keyPwd, const RawBuffer &keyPwdIV, diff --git a/src/manager/crypto/tz-backend/obj.cpp b/src/manager/crypto/tz-backend/obj.cpp index 8d85d1a..5a8153e 100644 --- a/src/manager/crypto/tz-backend/obj.cpp +++ b/src/manager/crypto/tz-backend/obj.cpp @@ -66,7 +66,7 @@ Token BData::derive(const CryptoAlgorithm &alg, const Password &pass, const RawB iv = Internals::generateIV(); } - Internals::deriveKBKDF(getId(), alg, pass, iv, tag, hash); + Internals::deriveKBKDF(getId(), getPassword(), alg, pass, iv, tag, hash); return Token(backendId(), DataType(KeyType::KEY_AES), Store::pack(hash, pass, iv, tag)); } diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index 3cf5702..ad09c65 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -890,6 +890,7 @@ void TrustZoneContext::executeEcdh(const RawBuffer &prvKeyId, } void TrustZoneContext::executeKbkdf(const RawBuffer& secretId, + const Pwd& secretPwd, const RawBuffer& label, const RawBuffer& context, const RawBuffer& fixed, @@ -908,6 +909,7 @@ void TrustZoneContext::executeKbkdf(const RawBuffer& secretId, LogDebug("TrustZoneContext::executeKbkdf"); auto sIn = makeSerializer(secretId, + secretPwd, label, context, fixed, diff --git a/src/manager/crypto/tz-backend/tz-context.h b/src/manager/crypto/tz-backend/tz-context.h index 7233ef7..015b44b 100644 --- a/src/manager/crypto/tz-backend/tz-context.h +++ b/src/manager/crypto/tz-backend/tz-context.h @@ -183,6 +183,7 @@ public: const RawBuffer &secretHash); void executeKbkdf(const RawBuffer& secretId, + const Pwd& secretPwd, const RawBuffer& label, const RawBuffer& context, const RawBuffer& fixed, -- 2.7.4 From b5a5d2e98249f501342a6892a4f0dc7dc1b68531 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 20 Jul 2023 14:20:20 +0200 Subject: [PATCH 11/16] Pass key length to KBKDF in TZ backend Change-Id: I5cd90b8754b7aa89371e515ffac79cd35c9b5004 --- src/manager/crypto/tz-backend/internals.cpp | 3 +++ src/manager/crypto/tz-backend/tz-context.cpp | 2 ++ src/manager/crypto/tz-backend/tz-context.h | 1 + 3 files changed, 6 insertions(+) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index d0cc232..5a19cba 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -810,6 +810,8 @@ void deriveKBKDF(const RawBuffer &secretId, const RawBuffer &keyHash) { RawBuffer label, context, fixed; + size_t length; + alg.getParam(ParamName::KDF_LEN, length); alg.getParam(ParamName::KBKDF_LABEL, label); alg.getParam(ParamName::KBKDF_CONTEXT, context); alg.getParam(ParamName::KBKDF_FIXED_INPUT, fixed); @@ -826,6 +828,7 @@ void deriveKBKDF(const RawBuffer &secretId, TrustZoneContext::Instance().executeKbkdf(secretId, secretPwd, + length, label, context, fixed, diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index ad09c65..a457462 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -891,6 +891,7 @@ void TrustZoneContext::executeEcdh(const RawBuffer &prvKeyId, void TrustZoneContext::executeKbkdf(const RawBuffer& secretId, const Pwd& secretPwd, + size_t length, const RawBuffer& label, const RawBuffer& context, const RawBuffer& fixed, @@ -910,6 +911,7 @@ void TrustZoneContext::executeKbkdf(const RawBuffer& secretId, auto sIn = makeSerializer(secretId, secretPwd, + length, label, context, fixed, diff --git a/src/manager/crypto/tz-backend/tz-context.h b/src/manager/crypto/tz-backend/tz-context.h index 015b44b..51f9be1 100644 --- a/src/manager/crypto/tz-backend/tz-context.h +++ b/src/manager/crypto/tz-backend/tz-context.h @@ -184,6 +184,7 @@ public: void executeKbkdf(const RawBuffer& secretId, const Pwd& secretPwd, + size_t length, const RawBuffer& label, const RawBuffer& context, const RawBuffer& fixed, -- 2.7.4 From 2b2909782620d7531a2f0a607fba6bd628e7208c Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 24 Jul 2023 11:42:18 +0200 Subject: [PATCH 12/16] Call TA to get the max chunk size Change-Id: Iec297646564b0a49d2966fcec1ec922bac1b7615 --- src/manager/crypto/tz-backend/internals.cpp | 4 ++++ src/manager/crypto/tz-backend/internals.h | 2 ++ src/manager/crypto/tz-backend/store.cpp | 5 +++++ src/manager/crypto/tz-backend/store.h | 2 +- src/manager/crypto/tz-backend/tz-context.cpp | 13 +++++++++++++ src/manager/crypto/tz-backend/tz-context.h | 2 ++ 6 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index 5a19cba..efad483 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -844,6 +844,10 @@ void deriveKBKDF(const RawBuffer &secretId, keyHash); } +size_t maxChunkSize() +{ + return TrustZoneContext::Instance().getMaxChunkSize(); +} } // namespace Internals } // namespace TZ diff --git a/src/manager/crypto/tz-backend/internals.h b/src/manager/crypto/tz-backend/internals.h index 8797065..98171a5 100644 --- a/src/manager/crypto/tz-backend/internals.h +++ b/src/manager/crypto/tz-backend/internals.h @@ -162,6 +162,8 @@ void deriveKBKDF(const RawBuffer &secretId, const RawBuffer &keyPwdIV, RawBuffer &keyTag, const RawBuffer &keyHash); + +size_t maxChunkSize(); } // namespace Internals } // namespace TZ } // namespace Crypto diff --git a/src/manager/crypto/tz-backend/store.cpp b/src/manager/crypto/tz-backend/store.cpp index 958da3a..23ba831 100644 --- a/src/manager/crypto/tz-backend/store.cpp +++ b/src/manager/crypto/tz-backend/store.cpp @@ -208,6 +208,11 @@ void Store::unpack(const RawBuffer &packed, } } +size_t Store::maxChunkSize() const +{ + return Internals::maxChunkSize(); +} + } // namespace TZ } // namespace Crypto } // namespace CKM diff --git a/src/manager/crypto/tz-backend/store.h b/src/manager/crypto/tz-backend/store.h index 439cec4..67b480c 100644 --- a/src/manager/crypto/tz-backend/store.h +++ b/src/manager/crypto/tz-backend/store.h @@ -55,7 +55,7 @@ public: RawBuffer &data, RawBuffer &iv, RawBuffer &tag); - size_t maxChunkSize() const override { return 4; } // TODO get it from somewhere + size_t maxChunkSize() const override; // TODO device key ID is needed here to support importEncrypted }; diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index a457462..2c013ae 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -948,6 +948,19 @@ void TrustZoneContext::executeKbkdf(const RawBuffer& secretId, LogDebug("Derived object ID is (hex): " << rawToHexString(keyHash)); } +uint32_t TrustZoneContext::getMaxChunkSize() +{ + // command ID = CMD_GET_MAX_CHUNK_SIZE + LogDebug("TrustZoneContext::getMaxChunkSize"); + + TEEC_Operation op; + op.paramTypes = TEEC_PARAM_TYPES(TEEC_VALUE_OUTPUT, TEEC_NONE, TEEC_NONE, TEEC_NONE); + + Execute(CMD_GET_MAX_CHUNK_SIZE, &op); + + return op.params[0].value.b; +} + void TrustZoneContext::Initialize() { TEEC_Operation op; diff --git a/src/manager/crypto/tz-backend/tz-context.h b/src/manager/crypto/tz-backend/tz-context.h index 51f9be1..9fd3a1e 100644 --- a/src/manager/crypto/tz-backend/tz-context.h +++ b/src/manager/crypto/tz-backend/tz-context.h @@ -199,6 +199,8 @@ public: RawBuffer &keyTag, const RawBuffer &keyHash); + uint32_t getMaxChunkSize(); + private: TrustZoneContext(); ~TrustZoneContext(); -- 2.7.4 From 8e3703c97cf7df11c45f33c089267fe687bd68c2 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 24 Jul 2023 12:44:00 +0200 Subject: [PATCH 13/16] Add type parameter to "get" commands CMD_GET_DATA CMD_GET_DATA_SIZE CMD_EXPORT_WRAPPED_KEY This may be necessary if data is encrypted. Change-Id: Ie34d33c11e9c55958cb44adcb0bf5371c36e8a68 --- src/manager/crypto/tz-backend/internals.cpp | 10 +++++++--- src/manager/crypto/tz-backend/internals.h | 6 ++++-- src/manager/crypto/tz-backend/obj.cpp | 5 +++-- src/manager/crypto/tz-backend/obj.h | 14 +++++++------- src/manager/crypto/tz-backend/store.cpp | 2 +- src/manager/crypto/tz-backend/tz-context.cpp | 25 +++++++++++++++---------- src/manager/crypto/tz-backend/tz-context.h | 6 ++++-- 7 files changed, 41 insertions(+), 27 deletions(-) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index efad483..dd4aed4 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -402,7 +402,8 @@ RawBuffer exportWrappedKey(const RawBuffer &wrappingKeyId, const Pwd &wrappingKeyPwd, const CryptoAlgorithm &alg, const RawBuffer &keyToWrapId, - const Pwd &keyToWrapPwd) + const Pwd &keyToWrapPwd, + const DataType &keyToWrapType) { AlgoType algo; uint32_t ctrLenOrTagSizeBits = 0; @@ -418,15 +419,18 @@ RawBuffer exportWrappedKey(const RawBuffer &wrappingKeyId, ctrLenOrTagSizeBits, aad, keyToWrapId, - keyToWrapPwd); + keyToWrapPwd, + toTzDataType(keyToWrapType)); } RawBuffer getData(const RawBuffer &dataId, - const Pwd &pwd) + const Pwd &pwd, + const DataType &type) { RawBuffer result; TrustZoneContext::Instance().getData(dataId, pwd, + toTzDataType(type), result); return result; } diff --git a/src/manager/crypto/tz-backend/internals.h b/src/manager/crypto/tz-backend/internals.h index 98171a5..00fb25f 100644 --- a/src/manager/crypto/tz-backend/internals.h +++ b/src/manager/crypto/tz-backend/internals.h @@ -75,10 +75,12 @@ RawBuffer exportWrappedKey(const RawBuffer &wrappingKeyId, const Pwd &wrappingKeyPwd, const CryptoAlgorithm &alg, const RawBuffer &keyToWrapId, - const Pwd &keyToWrapPwd); + const Pwd &keyToWrapPwd, + const DataType &keyToWrapType); RawBuffer getData(const RawBuffer &dataId, - const Pwd &pwd); + const Pwd &pwd, + const DataType &type); void destroyData(const RawBuffer &dataId); diff --git a/src/manager/crypto/tz-backend/obj.cpp b/src/manager/crypto/tz-backend/obj.cpp index 5a8153e..a811557 100644 --- a/src/manager/crypto/tz-backend/obj.cpp +++ b/src/manager/crypto/tz-backend/obj.cpp @@ -119,7 +119,8 @@ RawBuffer Key::wrap(const CryptoAlgorithm &alg, getPassword(), alg, keyToWrapId, - Pwd(keyToWrapPass, keyToWrapIV, keyToWrapTag)); + Pwd(keyToWrapPass, keyToWrapIV, keyToWrapTag), + m_type); } RawBuffer SKey::encrypt(const CryptoAlgorithm &alg, const RawBuffer &data) @@ -180,7 +181,7 @@ GCtxShPtr AKey::initContext(const CryptoAlgorithm &, bool) RawBuffer AKey::getBinary() const { if (m_type.isKeyPublic() && m_raw.empty()) - m_raw = Internals::getData(getId(), getPassword()); + m_raw = Internals::getData(getId(), getPassword(), m_type); return m_raw; } diff --git a/src/manager/crypto/tz-backend/obj.h b/src/manager/crypto/tz-backend/obj.h index eec1e9f..3d406be 100644 --- a/src/manager/crypto/tz-backend/obj.h +++ b/src/manager/crypto/tz-backend/obj.h @@ -98,8 +98,8 @@ protected: class Key : public BData { public: - Key(CryptoBackend backendId, int scheme, RawBuffer id, Pwd pwd) : - BData(backendId, scheme, std::move(id), std::move(pwd)) {} + Key(CryptoBackend backendId, int scheme, RawBuffer id, Pwd pwd, DataType dataType) : + BData(backendId, scheme, std::move(id), std::move(pwd)), m_type(dataType) {} Token unwrap(const CryptoAlgorithm ¶ms, const Data &encryptedKey, @@ -109,12 +109,15 @@ public: RawBuffer wrap(const CryptoAlgorithm ¶ms, const Token &keyToWrap, const Password &keyToWrapPass) override; + +protected: + DataType m_type; }; class SKey : public Key { public: SKey(CryptoBackend backendId, int scheme, RawBuffer id, Pwd pwd) : - Key(backendId, scheme, std::move(id), std::move(pwd)) {} + Key(backendId, scheme, std::move(id), std::move(pwd), DataType::KEY_AES) {} RawBuffer encrypt(const CryptoAlgorithm &, const RawBuffer &) override; RawBuffer decrypt(const CryptoAlgorithm &, const RawBuffer &) override; @@ -128,7 +131,7 @@ public: RawBuffer id, Pwd pwd, DataType dataType) : - Key(backendId, scheme, std::move(id), std::move(pwd)), m_type(dataType) {} + Key(backendId, scheme, std::move(id), std::move(pwd), dataType) {} RawBuffer getBinary() const override; RawBuffer sign(const CryptoAlgorithm &alg, const RawBuffer &message) override; @@ -138,9 +141,6 @@ public: RawBuffer decrypt(const CryptoAlgorithm &, const RawBuffer &) override; Token derive(const CryptoAlgorithm &, const Password &, const RawBuffer &) override; GCtxShPtr initContext(const CryptoAlgorithm &, bool) override; - -protected: - DataType m_type; }; class Cert : public AKey { diff --git a/src/manager/crypto/tz-backend/store.cpp b/src/manager/crypto/tz-backend/store.cpp index 23ba831..ca91c54 100644 --- a/src/manager/crypto/tz-backend/store.cpp +++ b/src/manager/crypto/tz-backend/store.cpp @@ -78,7 +78,7 @@ GObjUPtr Store::getObject(const Token &token, const Password &pass) return make(scheme, std::move(id), Pwd(pass, iv, tag), token.dataType); auto pwd = Pwd(pass, iv, tag); - RawBuffer raw = Internals::getData(id, pwd); + RawBuffer raw = Internals::getData(id, pwd, token.dataType); if (token.dataType.isBinaryData()) return make(scheme, std::move(id), std::move(pwd), std::move(raw)); diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index 2c013ae..8710982 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -746,7 +746,8 @@ RawBuffer TrustZoneContext::exportWrappedKey(const RawBuffer &wrappingKeyId, const uint32_t ctrLenOrTagSizeBits, const RawBuffer &aad, const RawBuffer &keyToWrapId, - const Pwd &keyToWrapPwd) + const Pwd &keyToWrapPwd, + tz_data_type keyToWrapType) { // command ID = CMD_EXPORT_WRAPPED_KEY LogDebug("TrustZoneContext::exportWrappedKey"); @@ -758,19 +759,20 @@ RawBuffer TrustZoneContext::exportWrappedKey(const RawBuffer &wrappingKeyId, ctrLenOrTagSizeBits, aad, keyToWrapId, - keyToWrapPwd); + keyToWrapPwd, + keyToWrapType); TrustZoneMemory inMemory(m_Context, sIn.GetSize(), TEEC_MEM_INPUT); sIn.Serialize(inMemory); - uint32_t data_size = 0; - GetDataSize(keyToWrapId, data_size); + uint32_t dataSize = 0; + GetDataSize(keyToWrapId, keyToWrapType, dataSize); - LogDebug("GetData data_size = [" << data_size << "]"); + LogDebug("GetData data_size = [" << dataSize << "]"); // encrypted data may be longer TZSerializer sOut; - sOut.Push(new TZSerializableBinary(data_size + KM_ENCRYPTION_OVERHEAD)); + sOut.Push(new TZSerializableBinary(dataSize + KM_ENCRYPTION_OVERHEAD)); TrustZoneMemory outMemory(m_Context, sOut.GetSize(), TEEC_MEM_OUTPUT); sOut.Serialize(outMemory); @@ -786,12 +788,14 @@ RawBuffer TrustZoneContext::exportWrappedKey(const RawBuffer &wrappingKeyId, return wrappedKey; } -void TrustZoneContext::GetDataSize(const RawBuffer &dataId, uint32_t &dataSize) +void TrustZoneContext::GetDataSize(const RawBuffer &dataId, + const tz_data_type type, + uint32_t &dataSize) { // command ID = CMD_GET_DATA_SIZE LogDebug("Object ID (passed to CMD_GET_DATA_SIZE) is (hex): " << rawToHexString(dataId)); - auto sIn = makeSerializer(dataId); + auto sIn = makeSerializer(dataId, type); TrustZoneMemory inMemory(m_Context, sIn.GetSize(), TEEC_MEM_INPUT); sIn.Serialize(inMemory); @@ -803,17 +807,18 @@ void TrustZoneContext::GetDataSize(const RawBuffer &dataId, uint32_t &dataSize) void TrustZoneContext::getData(const RawBuffer &dataId, const Pwd &pwd, + const tz_data_type type, RawBuffer &data) { // command ID = CMD_GET_DATA LogDebug("Object ID (passed to CMD_GET_DATA) is (hex): " << rawToHexString(dataId)); - auto sIn = makeSerializer(dataId, pwd); + auto sIn = makeSerializer(dataId, pwd, type); TrustZoneMemory inMemory(m_Context, sIn.GetSize(), TEEC_MEM_INPUT); sIn.Serialize(inMemory); uint32_t data_size = 0; - GetDataSize(dataId, data_size); + GetDataSize(dataId, type, data_size); LogDebug("GetData data_size = [" << data_size << "]"); diff --git a/src/manager/crypto/tz-backend/tz-context.h b/src/manager/crypto/tz-backend/tz-context.h index 9fd3a1e..342fdfe 100644 --- a/src/manager/crypto/tz-backend/tz-context.h +++ b/src/manager/crypto/tz-backend/tz-context.h @@ -109,7 +109,8 @@ public: const uint32_t ctrLenOrTagSizeBits, const RawBuffer &aad, const RawBuffer &keyToWrapId, - const Pwd &keyToWrapPwd); + const Pwd &keyToWrapPwd, + tz_data_type keyToWrapType); void executeCrypt(tz_command cmd, tz_algo_type algo, @@ -169,6 +170,7 @@ public: void getData(const RawBuffer &dataId, const Pwd &pwd, + const tz_data_type type, RawBuffer &data); void destroyData(const RawBuffer &dataId); @@ -211,7 +213,7 @@ private: void Destroy(); void Reload(); - void GetDataSize(const RawBuffer &dataId, uint32_t &dataSize); + void GetDataSize(const RawBuffer &dataId, const tz_data_type type, uint32_t &dataSize); void Execute(tz_command commandID, TEEC_Operation* op); -- 2.7.4 From b80aa5c4a61b8648a97ba722adf138be7d9090e8 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 24 Jul 2023 12:59:40 +0200 Subject: [PATCH 14/16] Allow only symmetric key wraping/unwrapping Change-Id: I36549b09d891d0d3e34667c71aa0294389441f76 --- src/include/ckmc/ckmc-manager.h | 3 ++- src/manager/service/ckm-logic.cpp | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/include/ckmc/ckmc-manager.h b/src/include/ckmc/ckmc-manager.h index 35d021c..b911cb2 100644 --- a/src/include/ckmc/ckmc-manager.h +++ b/src/include/ckmc/ckmc-manager.h @@ -1106,7 +1106,7 @@ int ckmc_decrypt_data(ckmc_param_list_h params, * * @remarks The wrapping key must be either symmetric (#CKMC_KEY_AES) or private RSA * (#CKMC_KEY_RSA_PRIVATE). - * @remarks key_type in @a wrapped_key must not be set to #CKMC_KEY_NONE. + * @remarks key_type in @a wrapped_key can only be #CKMC_KEY_AES. * @remarks password in @a wrapped_key must be set to NULL. There's no need to additionally encrypt * a wrapped key. * @remarks If password in @a policy is provided, the stored key is additionally encrypted with it. @@ -1169,6 +1169,7 @@ int ckmc_import_wrapped_key(const ckmc_param_list_h params, * @remarks The wrapping key must be either symmetric (#CKMC_KEY_AES) or public RSA * (#CKMC_KEY_RSA_PUBLIC). * @remarks The @a ppwrapped_key should be released using ckmc_key_free(). + * @remarks The key denoted by @a wrapping_key_alias can only be #CKMC_KEY_AES. * * @param[in] params Algorithm parameter list handle. See #ckmc_param_list_h and #ckmc_algo_type_e * for details. Supported algorithms: diff --git a/src/manager/service/ckm-logic.cpp b/src/manager/service/ckm-logic.cpp index ea59099..ae9a7ed 100644 --- a/src/manager/service/ckm-logic.cpp +++ b/src/manager/service/ckm-logic.cpp @@ -1585,6 +1585,11 @@ RawBuffer CKMLogic::importWrappedKey( return SerializeMessage(msgId, tryRet([&] { Crypto::GObjUPtr wrappingKey; + if (!keyType.isSymmetricKey()) { + LogError("Only symmetric key can be imported"); + return CKM_API_ERROR_INPUT_PARAM; + } + auto [dbOp, digest, retCode] = beginSaveAndGetHash(cred, keyName, keyOwner); if (retCode != CKM_API_SUCCESS) return retCode; @@ -1638,6 +1643,11 @@ RawBuffer CKMLogic::exportWrappedKey( if (retCode2 != CKM_API_SUCCESS) return retCode2; + if (!wrappedKeyType.isSymmetricKey()) { + LogError("Only symmetric key can be exported"); + return CKM_API_ERROR_INPUT_PARAM; + } + wrappedKey = wrappingKey->wrap(params, wrappedKeyRow, keyPassword); return retCode2; -- 2.7.4 From c5445678726f97a53d86dc2ec22b4fc0c5f0d7ef Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 28 Jul 2023 13:13:19 +0200 Subject: [PATCH 15/16] Use default tag length for wrapping if not given Change-Id: I2ad6e13383621522af2de8500814f6d01868c828 --- src/manager/crypto/tz-backend/internals.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index dd4aed4..0b3e398 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -218,6 +218,7 @@ void decompose(const CryptoAlgorithm &alg, break; case AlgoType::AES_GCM: iv = unpack(alg, ParamName::ED_IV); + ctrLenOrTagSizeBits = Params::DEFAULT_AES_GCM_TAG_LEN_BITS; alg.getParam(ParamName::ED_TAG_LEN, ctrLenOrTagSizeBits); alg.getParam(ParamName::ED_AAD, aad); break; -- 2.7.4 From f3f7d305fa4ccdd1d302f157497ebec76129bb41 Mon Sep 17 00:00:00 2001 From: Dongsun Lee Date: Sat, 29 Jul 2023 16:26:10 +0900 Subject: [PATCH 16/16] Fix bugs during exporting a wrapped key - generous output size check - enlarged encryption overhead for RSA - use type of wrap to key Change-Id: I64367edf00d58e67df62a682a05c58dae5e2327b --- src/manager/crypto/tz-backend/obj.cpp | 2 +- src/manager/crypto/tz-backend/tz-context.cpp | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/manager/crypto/tz-backend/obj.cpp b/src/manager/crypto/tz-backend/obj.cpp index a811557..a1fbbb7 100644 --- a/src/manager/crypto/tz-backend/obj.cpp +++ b/src/manager/crypto/tz-backend/obj.cpp @@ -120,7 +120,7 @@ RawBuffer Key::wrap(const CryptoAlgorithm &alg, alg, keyToWrapId, Pwd(keyToWrapPass, keyToWrapIV, keyToWrapTag), - m_type); + keyToWrap.dataType); } RawBuffer SKey::encrypt(const CryptoAlgorithm &alg, const RawBuffer &data) diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index 8710982..fb05e3c 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -770,9 +770,13 @@ RawBuffer TrustZoneContext::exportWrappedKey(const RawBuffer &wrappingKeyId, LogDebug("GetData data_size = [" << dataSize << "]"); + uint32_t enc_overhead = KM_ENCRYPTION_OVERHEAD; + if (algo == ALGO_RSA) + enc_overhead = KM_RSA_BLOCK_SIZE; + // encrypted data may be longer TZSerializer sOut; - sOut.Push(new TZSerializableBinary(dataSize + KM_ENCRYPTION_OVERHEAD)); + sOut.Push(new TZSerializableBinary(dataSize + enc_overhead, false)); TrustZoneMemory outMemory(m_Context, sOut.GetSize(), TEEC_MEM_OUTPUT); sOut.Serialize(outMemory); -- 2.7.4