From f3f7d305fa4ccdd1d302f157497ebec76129bb41 Mon Sep 17 00:00:00 2001 From: Dongsun Lee Date: Sat, 29 Jul 2023 16:26:10 +0900 Subject: [PATCH 01/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 From 7badfc11c9e54efcc1a4e4b792eef98c9e08631a Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 1 Aug 2023 15:26:47 +0200 Subject: [PATCH 02/16] Add context cleanup command for TZ If encryption fails and the "finalize" is not called the context will not removed on TA side. Fix it by adding new command. Change-Id: Id6bfb6821ba2c83565eb79d825fa98c096a346fc --- src/manager/crypto/tz-backend/ctx.cpp | 11 +++++++++++ src/manager/crypto/tz-backend/ctx.h | 2 ++ src/manager/crypto/tz-backend/internals.cpp | 5 +++++ src/manager/crypto/tz-backend/internals.h | 2 ++ src/manager/crypto/tz-backend/store.cpp | 1 - src/manager/crypto/tz-backend/tz-context.cpp | 9 +++++++++ src/manager/crypto/tz-backend/tz-context.h | 2 ++ 7 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/manager/crypto/tz-backend/ctx.cpp b/src/manager/crypto/tz-backend/ctx.cpp index db877d8..0fd5c47 100644 --- a/src/manager/crypto/tz-backend/ctx.cpp +++ b/src/manager/crypto/tz-backend/ctx.cpp @@ -17,6 +17,7 @@ #include #include #include +#include namespace CKM { namespace Crypto { @@ -45,6 +46,16 @@ RawBuffer CipherCtx::finalize(const RawBuffer& input) return Internals::finalizeCipher(m_opId, input); } +CipherCtx::~CipherCtx() +{ + // Always try to cleanup the TA side. Ignore the results. + try { + Internals::cleanupCipher(m_opId); + } catch (...) { + LogError("Context cleanup failed"); + } +} + } // 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 32eba26..883feec 100644 --- a/src/manager/crypto/tz-backend/ctx.h +++ b/src/manager/crypto/tz-backend/ctx.h @@ -30,6 +30,8 @@ public: RawBuffer update(const RawBuffer& input) override; RawBuffer finalize(const RawBuffer& input) override; + ~CipherCtx(); + private: uint32_t m_opId; }; diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index 0b3e398..9817480 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -717,6 +717,11 @@ RawBuffer finalizeCipher(uint32_t opId, return TrustZoneContext::Instance().finalizeGcmCipher(opId, data); } +void cleanupCipher(uint32_t opId) +{ + return TrustZoneContext::Instance().cleanupCipher(opId); +} + RawBuffer sign(const RawBuffer &pkeyId, const Pwd &pwd, const CryptoAlgorithm &alg, diff --git a/src/manager/crypto/tz-backend/internals.h b/src/manager/crypto/tz-backend/internals.h index 00fb25f..ad267ff 100644 --- a/src/manager/crypto/tz-backend/internals.h +++ b/src/manager/crypto/tz-backend/internals.h @@ -138,6 +138,8 @@ RawBuffer updateCipher(uint32_t opId, RawBuffer finalizeCipher(uint32_t opId, const RawBuffer &data); +void cleanupCipher(uint32_t opId); + RawBuffer sign(const RawBuffer &pkeyId, const Pwd &pwd, const CryptoAlgorithm &alg, diff --git a/src/manager/crypto/tz-backend/store.cpp b/src/manager/crypto/tz-backend/store.cpp index ca91c54..e4752cf 100644 --- a/src/manager/crypto/tz-backend/store.cpp +++ b/src/manager/crypto/tz-backend/store.cpp @@ -25,7 +25,6 @@ #include #include -#include #include namespace CKM { diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index fb05e3c..7a6e921 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -565,6 +565,15 @@ RawBuffer TrustZoneContext::finalizeGcmCipher(uint32_t opId, return out; } +void TrustZoneContext::cleanupCipher(uint32_t opId) +{ + TEEC_Operation op; + op.paramTypes = TEEC_PARAM_TYPES(TEEC_VALUE_INOUT, TEEC_NONE, TEEC_NONE, TEEC_NONE); + op.params[0].value.a = opId; + + Execute(CMD_CIPHER_CLEANUP, &op); +} + void TrustZoneContext::executeSign(tz_algo_type algo, tz_hash_type hash, const RawBuffer &keyId, diff --git a/src/manager/crypto/tz-backend/tz-context.h b/src/manager/crypto/tz-backend/tz-context.h index 342fdfe..c69299d 100644 --- a/src/manager/crypto/tz-backend/tz-context.h +++ b/src/manager/crypto/tz-backend/tz-context.h @@ -153,6 +153,8 @@ public: RawBuffer finalizeGcmCipher(uint32_t opId, const RawBuffer &data); + void cleanupCipher(uint32_t opId); + void executeSign(tz_algo_type algo, tz_hash_type hash, const RawBuffer &keyId, -- 2.7.4 From c3bd3d6e3622821b5a07970be9fa705bc6bcd254 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 3 Aug 2023 10:55:48 +0200 Subject: [PATCH 03/16] Deserialize tags only if password was given Change-Id: I1c598e17740785e4bd49edc120b6844ebe65e88a --- src/manager/crypto/tz-backend/tz-context.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index 7a6e921..ffa41a0 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -249,28 +249,27 @@ void TrustZoneContext::GenerateAKey(tz_command commandId, const RawBuffer &hashPriv, const RawBuffer &hashPub) { - uint32_t pubTagSize = 0; - uint32_t privTagSize = 0; uint32_t pubPwdExists = pubPwd.empty() ? 0 : 1; - if (pubPwdExists) { - pubTagSize = Params::DEFAULT_AES_GCM_TAG_LEN_BYTES; - } + TZSerializer sOut; + if (pubPwdExists) + sOut.Push(new TZSerializableBinary(Params::DEFAULT_AES_GCM_TAG_LEN_BYTES)); + uint32_t privPwdExists = privPwd.empty() ? 0 : 1; - if (privPwdExists) { - privTagSize = Params::DEFAULT_AES_GCM_TAG_LEN_BYTES; - } + if (privPwdExists) + sOut.Push(new TZSerializableBinary(Params::DEFAULT_AES_GCM_TAG_LEN_BYTES)); push(sIn, EncPwd{pubPwd, pubPwdIv}, EncPwd{privPwd, privPwdIv}, hashPriv, hashPub); TrustZoneMemory inMemory(m_Context, sIn.GetSize(), TEEC_MEM_INPUT); sIn.Serialize(inMemory); - TZSerializer sOut; - sOut.Push(new TZSerializableBinary(pubTagSize)); - sOut.Push(new TZSerializableBinary(privTagSize)); - TrustZoneMemory outMemory(m_Context, sOut.GetSize(), TEEC_MEM_OUTPUT); - TEEC_Operation op = makeOp(TEEC_VALUE_INOUT, inMemory, outMemory); + TEEC_Operation op; + if (sOut.GetSize() == 0) { + op = makeOp(TEEC_VALUE_INOUT, inMemory); + } else { + op = makeOp(TEEC_VALUE_INOUT, inMemory, outMemory); + } op.params[0].value.b = genParam; Execute(commandId, &op); -- 2.7.4 From bb89cc7f9a99ad892f74fb7577498060ad43d762 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 4 Aug 2023 08:18:40 +0200 Subject: [PATCH 04/16] Release 0.1.57 * Deserialize tags only if password was given * Add context cleanup command for TZ * Fix bugs during exporting a wrapped key * Use default tag length for wrapping if not given * Allow only symmetric key wraping/unwrapping * Add type parameter to "get" commands * Call TA to get the max chunk size * Pass key length to KBKDF in TZ backend * Fix secret pwd passing in TZ backend KBKDF Change-Id: Ia17f64eacf32400ef0f53c0d2cf82ceb1c07e45c --- 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 cf467fb..9de3927 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.56 +Version: 0.1.57 Release: 1 Group: Security/Secure Storage License: Apache-2.0 and BSD-3-Clause -- 2.7.4 From 97ca6f65f6477e7d2fe771d7728e365f88a9c143 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 8 Aug 2023 17:00:00 +0200 Subject: [PATCH 05/16] Use default CTR length in TZ encryption According to API documentation the counter length parameter is optional. TZ implementation should not assume its existence. Change-Id: I89ef3b78e95f2a3a8c79688ee21c9d04a43a5116 --- src/manager/crypto/tz-backend/internals.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index 9817480..8470c12 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -203,7 +203,8 @@ void decompose(const CryptoAlgorithm &alg, switch (algo) { case AlgoType::AES_CTR: iv = unpack(alg, ParamName::ED_IV); - ctrLenOrTagSizeBits = unpack(alg, ParamName::ED_CTR_LEN); + ctrLenOrTagSizeBits = Params::DEFAULT_AES_IV_LEN * 8; + alg.getParam(ParamName::ED_CTR_LEN, ctrLenOrTagSizeBits); // counter length is in bits if (ctrLenOrTagSizeBits != Params::DEFAULT_AES_IV_LEN * 8) { LogError("CTR length invalid: " << std::to_string(ctrLenOrTagSizeBits)); @@ -514,11 +515,11 @@ RawBuffer symmetricEncrypt(const RawBuffer &keyId, const RawBuffer &data) { AlgoType algo = unpack(alg, ParamName::ALGO_TYPE); - uint64_t ctrLen = 0; + uint64_t ctrLen = Params::DEFAULT_AES_IV_LEN * 8; switch (algo) { case AlgoType::AES_CTR: { - ctrLen = unpack(alg, ParamName::ED_CTR_LEN); + alg.getParam(ParamName::ED_CTR_LEN, ctrLen); // counter length is in bits if (ctrLen != Params::DEFAULT_AES_IV_LEN * 8) { LogError("CTR length invalid: " << std::to_string(ctrLen)); @@ -565,11 +566,11 @@ RawBuffer symmetricDecrypt(const RawBuffer &keyId, const RawBuffer &data) { AlgoType algo = unpack(alg, ParamName::ALGO_TYPE); - uint64_t ctrLen = 0; + uint64_t ctrLen = Params::DEFAULT_AES_IV_LEN * 8; switch (algo) { case AlgoType::AES_CTR: { - ctrLen = unpack(alg, ParamName::ED_CTR_LEN); + alg.getParam(ParamName::ED_CTR_LEN, ctrLen); // counter length is in bits if (ctrLen != Params::DEFAULT_AES_IV_LEN * 8) { LogError("CTR length invalid: " << std::to_string(ctrLen)); -- 2.7.4 From 497b0dd3f68c57f6d57176ac02fab2c122431948 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 9 Aug 2023 17:26:24 +0200 Subject: [PATCH 06/16] Pass public key curve info to TZ's ECDH This is needed to compare curve with private EC curve in TZ. Change-Id: I5c42b7395683bd14c391415537e31efc3dcb6fc4 --- src/manager/crypto/tz-backend/internals.cpp | 21 +++++++++++++++++++++ src/manager/crypto/tz-backend/tz-context.cpp | 3 ++- src/manager/crypto/tz-backend/tz-context.h | 1 + 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index 8470c12..b41c5cb 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -802,8 +802,29 @@ void deriveECDH(const RawBuffer &prvKeyId, RawBuffer secretPwdBuf(secretPwd.begin(), secretPwd.end()); + int pubCurve = EC_GROUP_get_curve_name(ecGroup); + tz_ec tzCurve; + switch (pubCurve) + { + case NID_X9_62_prime192v1: + tzCurve = EC_NIST_P192; + break; + + case NID_X9_62_prime256v1: + tzCurve = EC_NIST_P256; + break; + + case NID_secp384r1: + tzCurve = EC_NIST_P384; + break; + + default: + ThrowErr(Exc::Crypto::InputParam, "Unsupported public key EC"); + } + TrustZoneContext::Instance().executeEcdh(prvKeyId, prvKeyPwd, + tzCurve, xBuf, yBuf, secretPwdBuf, diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index ffa41a0..b2bdf31 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -869,6 +869,7 @@ TZSerializablePwdData* makeSerializablePwd(const Pwd &pwd) void TrustZoneContext::executeEcdh(const RawBuffer &prvKeyId, const Pwd &prvKeyPwd, + const tz_ec curve, const RawBuffer &pubX, const RawBuffer &pubY, const RawBuffer &secretPwdBuf, @@ -880,7 +881,7 @@ void TrustZoneContext::executeEcdh(const RawBuffer &prvKeyId, LogDebug("TrustZoneContext::executeEcdh"); auto sIn = makeSerializer( - prvKeyId, prvKeyPwd, pubX, pubY, EncPwd{secretPwdBuf, secretPwdIV}, secretHash); + prvKeyId, prvKeyPwd, curve, pubX, pubY, EncPwd{secretPwdBuf, secretPwdIV}, secretHash); TrustZoneMemory inMemory(m_Context, sIn.GetSize(), TEEC_MEM_INPUT); sIn.Serialize(inMemory); diff --git a/src/manager/crypto/tz-backend/tz-context.h b/src/manager/crypto/tz-backend/tz-context.h index c69299d..9471807 100644 --- a/src/manager/crypto/tz-backend/tz-context.h +++ b/src/manager/crypto/tz-backend/tz-context.h @@ -179,6 +179,7 @@ public: void executeEcdh(const RawBuffer &prvKeyId, const Pwd &prvKeyPwd, + const tz_ec curve, const RawBuffer &pubX, const RawBuffer &pubY, const RawBuffer &secretPwdBuf, -- 2.7.4 From 3d26350439ca9e6548d09916abd33f3a3b746302 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 9 Aug 2023 17:31:21 +0200 Subject: [PATCH 07/16] Pass password to CMD_GET_DATA_SIZE In case of encrypted objects the password may be necessary do get the actual size. Change-Id: I5636325a8a120c0226ab5cc06ddef2aa05b96992 --- src/manager/crypto/tz-backend/tz-context.cpp | 7 ++++--- src/manager/crypto/tz-backend/tz-context.h | 5 ++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/manager/crypto/tz-backend/tz-context.cpp b/src/manager/crypto/tz-backend/tz-context.cpp index b2bdf31..9bfc390 100644 --- a/src/manager/crypto/tz-backend/tz-context.cpp +++ b/src/manager/crypto/tz-backend/tz-context.cpp @@ -774,7 +774,7 @@ RawBuffer TrustZoneContext::exportWrappedKey(const RawBuffer &wrappingKeyId, sIn.Serialize(inMemory); uint32_t dataSize = 0; - GetDataSize(keyToWrapId, keyToWrapType, dataSize); + GetDataSize(keyToWrapId, keyToWrapPwd, keyToWrapType, dataSize); LogDebug("GetData data_size = [" << dataSize << "]"); @@ -801,13 +801,14 @@ RawBuffer TrustZoneContext::exportWrappedKey(const RawBuffer &wrappingKeyId, } void TrustZoneContext::GetDataSize(const RawBuffer &dataId, + const Pwd &pwd, 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, type); + auto sIn = makeSerializer(dataId, pwd, type); TrustZoneMemory inMemory(m_Context, sIn.GetSize(), TEEC_MEM_INPUT); sIn.Serialize(inMemory); @@ -830,7 +831,7 @@ void TrustZoneContext::getData(const RawBuffer &dataId, sIn.Serialize(inMemory); uint32_t data_size = 0; - GetDataSize(dataId, type, data_size); + GetDataSize(dataId, pwd, 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 9471807..2033cf7 100644 --- a/src/manager/crypto/tz-backend/tz-context.h +++ b/src/manager/crypto/tz-backend/tz-context.h @@ -216,7 +216,10 @@ private: void Destroy(); void Reload(); - void GetDataSize(const RawBuffer &dataId, const tz_data_type type, uint32_t &dataSize); + void GetDataSize(const RawBuffer &dataId, + const Pwd &pwd, + const tz_data_type type, + uint32_t &dataSize); void Execute(tz_command commandID, TEEC_Operation* op); -- 2.7.4 From cb41ffff1addfb746aa755d434a2f6ce6d8f7376 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 9 Aug 2023 17:39:38 +0200 Subject: [PATCH 08/16] Check KBKDF parameters on key-manager side Change-Id: I1afb107d6fd286f5524561c1631ef65c2043f3c2 --- src/manager/crypto/tz-backend/internals.cpp | 49 ++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index b41c5cb..2a5d83c 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -842,19 +842,38 @@ void deriveKBKDF(const RawBuffer &secretId, const RawBuffer &keyHash) { RawBuffer label, context, fixed; - size_t length; + KbkdfCounterLocation counterLocation; + KdfPrf prf; + KbkdfMode mode; + size_t length, rlenBits = 32, llenBits = 32, tmp; + bool hasLabel = alg.getParam(ParamName::KBKDF_LABEL, label); + bool hasContext = alg.getParam(ParamName::KBKDF_CONTEXT, context); + bool hasFixed = alg.getParam(ParamName::KBKDF_FIXED_INPUT, fixed); + alg.getParam(ParamName::KBKDF_COUNTER_LOCATION, counterLocation); + alg.getParam(ParamName::KBKDF_MODE, mode); + alg.getParam(ParamName::KDF_PRF, prf); 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); - auto prf = unpack(alg, ParamName::KDF_PRF); - auto mode = unpack(alg, ParamName::KBKDF_MODE); - auto location = unpack(alg, ParamName::KBKDF_COUNTER_LOCATION); - - size_t rlen = 32, llen = 32, dummy; - alg.getParam(ParamName::KBKDF_RLEN, rlen); - alg.getParam(ParamName::KBKDF_LLEN, llen); - bool noSeparator = alg.getParam(ParamName::KBKDF_NO_SEPARATOR, dummy); + alg.getParam(ParamName::KBKDF_RLEN, rlenBits); + bool hasLLen = alg.getParam(ParamName::KBKDF_LLEN, llenBits); + bool noSeparator = alg.getParam(ParamName::KBKDF_NO_SEPARATOR, tmp); + + RawBuffer key; + if (hasFixed) { + if (hasLabel || hasContext || noSeparator || hasLLen || + counterLocation == KbkdfCounterLocation::MIDDLE_FIXED) + ThrowErr(Exc::Crypto::InputParam, "Unexpected parameters for fixed input mode."); + } else { + if (!hasLabel || !hasContext) + ThrowErr(Exc::Crypto::InputParam, "Missing label and/or context."); + + if (llenBits != 0 && llenBits != 8 && llenBits != 16 && llenBits != 24 && llenBits != 32) + ThrowErr(Exc::Crypto::InputParam, "Invalid llen value"); + } + if (length != 16 && length != 24 && length != 32) + ThrowErr(Exc::Crypto::InputParam, "Invalid key length"); + + if (rlenBits != 8 && rlenBits != 16 && rlenBits != 24 && rlenBits != 32) + ThrowErr(Exc::Crypto::InputParam, "Invalid rlen value"); RawBuffer keyPwdBuf(keyPwd.begin(), keyPwd.end()); @@ -866,9 +885,9 @@ void deriveKBKDF(const RawBuffer &secretId, fixed, toTzPrf(prf), toTzKbkdfMode(mode), - toTzCtrLoc(location), - rlen, - llen, + toTzCtrLoc(counterLocation), + rlenBits, + llenBits, noSeparator, keyPwdBuf, keyPwdIV, -- 2.7.4 From 722b99e3cfe89406adb0626588bab8b18dca1317 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 10 Aug 2023 20:27:44 +0200 Subject: [PATCH 09/16] Check RSA padding in TZ The only supported padding method is PKCS1 Change-Id: I3cd769d68f67b3ee2afb959bca2e74db8e6295c4 --- src/manager/crypto/tz-backend/internals.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index 2a5d83c..1d14e08 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -733,6 +733,11 @@ RawBuffer sign(const RawBuffer &pkeyId, if (algo != AlgoType::RSA_SV && hash == HashAlgorithm::NONE) ThrowErr(Exc::Crypto::InputParam, "Only RSA supports no hash option"); + RSAPaddingAlgorithm padding = RSAPaddingAlgorithm::NONE; + alg.getParam(ParamName::SV_RSA_PADDING, padding); + if (algo == AlgoType::RSA_SV && padding != RSAPaddingAlgorithm::PKCS1) + ThrowErr(Exc::Crypto::InputParam, "Only PKCS1 padding is supported"); + RawBuffer signature; TrustZoneContext::Instance().executeSign(getAlgType(algo), getHashType(hash), @@ -754,6 +759,11 @@ int verify(const RawBuffer &pkeyId, if (algo != AlgoType::RSA_SV && hash == HashAlgorithm::NONE) ThrowErr(Exc::Crypto::InputParam, "Only RSA supports no hash option"); + RSAPaddingAlgorithm padding = RSAPaddingAlgorithm::NONE; + alg.getParam(ParamName::SV_RSA_PADDING, padding); + if (algo == AlgoType::RSA_SV && padding != RSAPaddingAlgorithm::PKCS1) + ThrowErr(Exc::Crypto::InputParam, "Only PKCS1 padding is supported"); + return TrustZoneContext::Instance().executeVerify(getAlgType(algo), getHashType(hash), pkeyId, -- 2.7.4 From e502d038b159fef2facaa420b4797958e8d81f14 Mon Sep 17 00:00:00 2001 From: Dongsun Lee Date: Mon, 21 Aug 2023 10:16:45 +0900 Subject: [PATCH 10/16] Set initial values to remove build warnings Change-Id: Icf793bed432aeb72de8471bd770feb9326450936 --- src/manager/crypto/tz-backend/internals.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index 1d14e08..b97a70f 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -852,9 +852,9 @@ void deriveKBKDF(const RawBuffer &secretId, const RawBuffer &keyHash) { RawBuffer label, context, fixed; - KbkdfCounterLocation counterLocation; - KdfPrf prf; - KbkdfMode mode; + KbkdfCounterLocation counterLocation = KbkdfCounterLocation::BEFORE_FIXED; + KdfPrf prf = KdfPrf::HMAC_SHA256; + KbkdfMode mode = KbkdfMode::COUNTER; size_t length, rlenBits = 32, llenBits = 32, tmp; bool hasLabel = alg.getParam(ParamName::KBKDF_LABEL, label); bool hasContext = alg.getParam(ParamName::KBKDF_CONTEXT, context); -- 2.7.4 From ed36e01bf88fe78cb338754d250a2770a59d8c6c Mon Sep 17 00:00:00 2001 From: Dongsun Lee Date: Mon, 21 Aug 2023 15:41:03 +0900 Subject: [PATCH 11/16] Release 0.1.58 * Add compiler flags for build warnings * Check RSA padding in TZ * Check KBKDF parameters on key-manager side * Pass password to CMD_GET_DATA_SIZE * Pass public key curve info to TZ's ECDH * Use default CTR length in TZ encryption Change-Id: Ibe2928a9b18bba49764a9779eaed1712b9bf5b36 --- 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 9de3927..72c602e 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.57 +Version: 0.1.58 Release: 1 Group: Security/Secure Storage License: Apache-2.0 and BSD-3-Clause -- 2.7.4 From 44882b59af7b0a42509dd544074c391391576140 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 11 Aug 2023 13:29:45 +0200 Subject: [PATCH 12/16] Adjust scheme tests to TZ TZ does not support saving asymmetric keys with password. Change-Id: Ia6ec92b610908a52079d2f22f32a9387237faee7 --- misc/encryption_scheme/scheme-test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/misc/encryption_scheme/scheme-test.cpp b/misc/encryption_scheme/scheme-test.cpp index 89f8a7b..c5b6609 100644 --- a/misc/encryption_scheme/scheme-test.cpp +++ b/misc/encryption_scheme/scheme-test.cpp @@ -194,11 +194,13 @@ Group GROUPS[] = { Item("pkcs-alias2", DataType::CHAIN_CERT_0, policy[NO_PASS][EXP]), } }, +#ifndef TZ_BACKEND_ENABLED // TZ does not support importing asymmetric keys with password { Group::SINGLE_ITEM, { Item("pkcs-alias3", DataType::CHAIN_CERT_0, policy[PASS][NO_EXP]), } }, +#endif { Group::SINGLE_ITEM, { Item("pkcs-alias4", DataType::CHAIN_CERT_0, policy[PASS][EXP]), -- 2.7.4 From a329e6358ef616c0047c19d39949d71aeae59825 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 11 Sep 2023 14:03:30 +0200 Subject: [PATCH 13/16] Fix key-wrapping documentation Change-Id: I67d762c719e2fc1a7cae7f3537f69760fa9eef3f --- src/include/ckmc/ckmc-manager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/ckmc/ckmc-manager.h b/src/include/ckmc/ckmc-manager.h index b911cb2..ee3cc6a 100644 --- a/src/include/ckmc/ckmc-manager.h +++ b/src/include/ckmc/ckmc-manager.h @@ -1169,7 +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. + * @remarks The key denoted by @a 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: -- 2.7.4 From f33f2eeb40166e33558165c62060756d89afafd3 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 19 Sep 2023 14:56:06 +0200 Subject: [PATCH 14/16] Check mandatory KBKFD params in TZ Change-Id: I151207b55b1051ac3cc870c885a33b951331bc61 --- src/manager/crypto/tz-backend/internals.cpp | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/manager/crypto/tz-backend/internals.cpp b/src/manager/crypto/tz-backend/internals.cpp index b97a70f..91098a4 100644 --- a/src/manager/crypto/tz-backend/internals.cpp +++ b/src/manager/crypto/tz-backend/internals.cpp @@ -852,21 +852,29 @@ void deriveKBKDF(const RawBuffer &secretId, const RawBuffer &keyHash) { RawBuffer label, context, fixed; - KbkdfCounterLocation counterLocation = KbkdfCounterLocation::BEFORE_FIXED; - KdfPrf prf = KdfPrf::HMAC_SHA256; - KbkdfMode mode = KbkdfMode::COUNTER; - size_t length, rlenBits = 32, llenBits = 32, tmp; + size_t rlenBits = 32, llenBits = 32, tmp; bool hasLabel = alg.getParam(ParamName::KBKDF_LABEL, label); bool hasContext = alg.getParam(ParamName::KBKDF_CONTEXT, context); bool hasFixed = alg.getParam(ParamName::KBKDF_FIXED_INPUT, fixed); - alg.getParam(ParamName::KBKDF_COUNTER_LOCATION, counterLocation); - alg.getParam(ParamName::KBKDF_MODE, mode); - alg.getParam(ParamName::KDF_PRF, prf); - alg.getParam(ParamName::KDF_LEN, length); + auto counterLocation = unpack(alg, ParamName::KBKDF_COUNTER_LOCATION); + auto mode = unpack(alg, ParamName::KBKDF_MODE); + auto prf = unpack(alg, ParamName::KDF_PRF); + auto length = unpack(alg, ParamName::KDF_LEN); alg.getParam(ParamName::KBKDF_RLEN, rlenBits); bool hasLLen = alg.getParam(ParamName::KBKDF_LLEN, llenBits); bool noSeparator = alg.getParam(ParamName::KBKDF_NO_SEPARATOR, tmp); + if (counterLocation != KbkdfCounterLocation::BEFORE_FIXED && + counterLocation != KbkdfCounterLocation::MIDDLE_FIXED && + counterLocation != KbkdfCounterLocation::AFTER_FIXED) + ThrowErr(Exc::Crypto::InputParam, "Invalid counter location"); + + if (mode != KbkdfMode::COUNTER) + ThrowErr(Exc::Crypto::InputParam, "Invalid mode"); + + if (prf != KdfPrf::HMAC_SHA256 && prf != KdfPrf::HMAC_SHA384 && prf != KdfPrf::HMAC_SHA512) + ThrowErr(Exc::Crypto::InputParam, "Invalid pseudo random function"); + RawBuffer key; if (hasFixed) { if (hasLabel || hasContext || noSeparator || hasLLen || -- 2.7.4 From 36671df676d073023d0f2483cdc9eaacc9258a3d Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 20 Sep 2023 11:52:25 +0200 Subject: [PATCH 15/16] Specify the RSA OAEP encryption padding Change-Id: I88abe53b11230121f594728abf64c0cf1c38895f --- src/include/ckmc/ckmc-type.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/include/ckmc/ckmc-type.h b/src/include/ckmc/ckmc-type.h index c3240e8..1602f62 100644 --- a/src/include/ckmc/ckmc-type.h +++ b/src/include/ckmc/ckmc-type.h @@ -434,11 +434,11 @@ typedef enum __ckmc_algo_type { - #CKMC_PARAM_ALGO_TYPE = #CKMC_ALGO_AES_CFB (mandatory), - #CKMC_PARAM_ED_IV = 16-byte initialization vector (mandatory) */ - CKMC_ALGO_RSA_OAEP, /**< RSA-OAEP algorithm + CKMC_ALGO_RSA_OAEP, /**< RSA-OAEP algorithm (EME-OAEP as defined in PKCS #1 with SHA-1 and MGF1) Supported parameters: - #CKMC_PARAM_ALGO_TYPE = #CKMC_ALGO_RSA_OAEP (mandatory), - - #CKMC_PARAM_ED_LABEL = label to be associated with the message - (optional, not supported at the moment) */ + - #CKMC_PARAM_ED_LABEL = label (encoding parameter) to be associated with + the message (optional, not supported at the moment) */ CKMC_ALGO_KBKDF, /**< Key based key derivation algorithm Supported parameters: -- 2.7.4 From 655222fc440e9e96fc0dbe97514815bbac95b018 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 20 Sep 2023 12:57:56 +0200 Subject: [PATCH 16/16] Prevent using public key for decryption We could leave it for backends but since we have all the info, let's fail early. Change-Id: I7d3257370124ad19d423b859f380ce60f6da4d95 --- src/manager/service/ckm-logic.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/manager/service/ckm-logic.cpp b/src/manager/service/ckm-logic.cpp index ae9a7ed..4bcb3c7 100644 --- a/src/manager/service/ckm-logic.cpp +++ b/src/manager/service/ckm-logic.cpp @@ -1594,11 +1594,23 @@ RawBuffer CKMLogic::importWrappedKey( if (retCode != CKM_API_SUCCESS) return retCode; - retCode = readDataHelper(false, cred, DataType::DB_KEY_FIRST, wrappingKeyName, - wrappingKeyOwner, wrappingKeyPassword, wrappingKey); + DataType wrappingKeyType; + retCode = readDataHelper(false, + cred, + DataType::DB_KEY_FIRST, + wrappingKeyName, + wrappingKeyOwner, + wrappingKeyPassword, + wrappingKey, + wrappingKeyType); if (retCode != CKM_API_SUCCESS) return retCode; + if (wrappingKeyType.isKeyPublic()) { + LogError("Public key can not be used for decryption"); + return CKM_API_ERROR_INPUT_PARAM; + } + if (!m_decider.checkStore(wrappingKey->backendId(), keyType, policy, true)) { LogDebug("Can't import the wrapped key to backend " << static_cast(wrappingKey->backendId()) << " with given policy"); -- 2.7.4