From 0bc7eba8c479edf4dcbfb3f371f9fcac7915f47c Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 1 Aug 2023 15:26:47 +0200 Subject: [PATCH 01/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 e49dd714ea0a0093e4aa3d7ade4b2903ff1ec0f9 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 3 Aug 2023 10:55:48 +0200 Subject: [PATCH 02/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 38d5b5eb3f706084cdcdab09abbf8ca28e18660c Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 4 Aug 2023 08:18:40 +0200 Subject: [PATCH 03/16] Release 0.1.54.3 * 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 c88b4ce..3ff8803 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.2 +Version: 0.1.54.3 Release: 1 Group: Security/Secure Storage License: Apache-2.0 and BSD-3-Clause -- 2.7.4 From eacc5d6d6bf76f5fa8ed07da9734bd795b66fc03 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 8 Aug 2023 17:00:00 +0200 Subject: [PATCH 04/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 15b9f24f046d84a4505a7fc794180d166924b39c Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 9 Aug 2023 17:26:24 +0200 Subject: [PATCH 05/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 88b7b37f2c6776f5426aef6283e2170753436310 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 9 Aug 2023 17:31:21 +0200 Subject: [PATCH 06/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 f1bd8117a43539b1049dfb95886281dd79153802 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Wed, 9 Aug 2023 17:39:38 +0200 Subject: [PATCH 07/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 638b3eb55050b32f83ae4303bef3f01ba59f5183 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 10 Aug 2023 20:27:44 +0200 Subject: [PATCH 08/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 9f80c6cdb78c30f4785a4d824d3071b10195c5cb Mon Sep 17 00:00:00 2001 From: Dongsun Lee Date: Mon, 21 Aug 2023 10:16:45 +0900 Subject: [PATCH 09/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 4fc2ec1a9a168d4551d9062a5b71224a3786a7bc Mon Sep 17 00:00:00 2001 From: Dariusz Michaluk Date: Mon, 21 Aug 2023 14:22:52 +0200 Subject: [PATCH 10/16] Release 0.1.54.4 * 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: I5536a1933301c406a2f82841db05c07bcaeca6f1 --- 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 3ff8803..5feda7f 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.3 +Version: 0.1.54.4 Release: 1 Group: Security/Secure Storage License: Apache-2.0 and BSD-3-Clause -- 2.7.4 From 773456667f624f68f0226c03adf5ddd41897dc85 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 11 Aug 2023 13:29:45 +0200 Subject: [PATCH 11/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 6672eea85a72d337250134dae7f17bb4ce730624 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 11 Sep 2023 14:03:30 +0200 Subject: [PATCH 12/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 748e19e..877ea50 100644 --- a/src/include/ckmc/ckmc-manager.h +++ b/src/include/ckmc/ckmc-manager.h @@ -1168,7 +1168,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 e72896f7d5a4df447aaa1a57b4833facddfd53eb Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 12 Sep 2023 17:12:08 +0200 Subject: [PATCH 13/16] Don't use IVs shorter than 12B in tests Change-Id: I086fb913be53eeb0891a2a0a9013d768939f0b0a --- unit-tests/test_sw-backend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unit-tests/test_sw-backend.cpp b/unit-tests/test_sw-backend.cpp index 866e3eb..08bbdd4 100644 --- a/unit-tests/test_sw-backend.cpp +++ b/unit-tests/test_sw-backend.cpp @@ -1464,7 +1464,7 @@ POSITIVE_TEST_CASE(cipherAPI) RawBuffer aad[4] = {createRandom(32), createRandom(33), createRandom(34), createRandom(35)}; ca.setParam(ParamName::ED_AAD, aad[0]); BOOST_REQUIRE_NO_THROW(gcm = key->initContext(ca, true)); - iv.resize(6); + iv.resize(12); ca.setParam(ParamName::ED_IV, iv); BOOST_REQUIRE_NO_THROW(gcm = key->initContext(ca, true)); -- 2.7.4 From 412cb68f9e8bbc5a454281979546cd7c6fdb011a Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 12 Sep 2023 12:31:15 +0200 Subject: [PATCH 14/16] Revert "Workaround for GCM IV length issue" This reverts commit d6606535ac51bb3e574312106088f3f15843536e. Change-Id: Iac1edf23290c19d0866f06138a31f6181093ba92 --- src/manager/crypto/sw-backend/internals.cpp | 35 +++++++++----------------- unit-tests/test_sw-backend.cpp | 38 ----------------------------- 2 files changed, 11 insertions(+), 62 deletions(-) diff --git a/src/manager/crypto/sw-backend/internals.cpp b/src/manager/crypto/sw-backend/internals.cpp index 77d1252..75ba3e3 100644 --- a/src/manager/crypto/sw-backend/internals.cpp +++ b/src/manager/crypto/sw-backend/internals.cpp @@ -820,34 +820,21 @@ RawBuffer decryptDataAesGcm( const RawBuffer &tag, const RawBuffer &aad) { - RawBuffer result, tmp; - - auto decrypt = [&](const RawBuffer &actualIv){ - EvpCipherPtr dec; - selectCipher(AlgoType::AES_GCM, key.size(), false)(dec, key, actualIv); - void *ptr = (void *)tag.data(); + EvpCipherPtr dec; + selectCipher(AlgoType::AES_GCM, key.size(), false)(dec, key, iv); + void *ptr = (void *)tag.data(); - dec->Control(EVP_CTRL_GCM_SET_TAG, tag.size(), ptr); + dec->Control(EVP_CTRL_GCM_SET_TAG, tag.size(), ptr); - if (!aad.empty()) - dec->AppendAAD(aad); - - result = dec->Append(data); - try { - tmp = dec->Finalize(); - } catch (const Exc::Exception &e) { - ThrowErr(Exc::InputParam, - "Tag authentication failed in AES finalize function (the tag doesn't match)."); - } - }; + if (!aad.empty()) + dec->AppendAAD(aad); + RawBuffer result = dec->Append(data); + RawBuffer tmp; try { - decrypt(iv); - } catch (const Exc::InputParam &e) { - LogDebug("AES GCM decryption failed. Retry with default iv length."); - RawBuffer shortIv = iv; - shortIv.resize(Params::DEFAULT_AES_GCM_IV_LEN); - decrypt(shortIv); + tmp = dec->Finalize(); + } catch (const Exc::Exception &e) { + ThrowErr(Exc::InputParam, "Tag authentication failed in AES finalize function (the tag doesn't match)."); } std::copy(tmp.begin(), tmp.end(), std::back_inserter(result)); return result; diff --git a/unit-tests/test_sw-backend.cpp b/unit-tests/test_sw-backend.cpp index 08bbdd4..a2a9b4c 100644 --- a/unit-tests/test_sw-backend.cpp +++ b/unit-tests/test_sw-backend.cpp @@ -675,44 +675,6 @@ NEGATIVE_TEST_CASE(symmetricEncryptDecryptGcm) BOOST_REQUIRE_THROW(key->decrypt(ca2, encrypted), Exc::Crypto::InputParam); } -POSITIVE_TEST_CASE(gcmIvLengthScrewUpWorkaround) -{ - const auto key = generateAes(128); - const auto data = createRandom(128); - const auto iv = createRandom(Params::DEFAULT_AES_IV_LEN); - CryptoAlgorithm ca; - RawBuffer encrypted, decrypted; - auto shortIv = iv; - shortIv.resize(Params::DEFAULT_AES_GCM_IV_LEN); - - ca.setParam(ParamName::ALGO_TYPE, AlgoType::AES_GCM); - ca.setParam(ParamName::ED_IV, shortIv); - ca.setParam(ParamName::ED_TAG_LEN, 128); - - // encrypt with 12B IV - BOOST_REQUIRE_NO_THROW(encrypted = key->encrypt(ca, data)); - - // decrypt with 12B IV - BOOST_REQUIRE_NO_THROW(decrypted = key->decrypt(ca, encrypted)); - BOOST_REQUIRE(decrypted == data); - - // decrypt with 16B IV should also succeed (workaround) - ca.setParam(ParamName::ED_IV, iv); - BOOST_REQUIRE_NO_THROW(decrypted = key->decrypt(ca, encrypted)); - BOOST_REQUIRE(decrypted == data); - - // encrypt with 16B IV - BOOST_REQUIRE_NO_THROW(encrypted = key->encrypt(ca, data)); - - // decrypt with 16B IV - BOOST_REQUIRE_NO_THROW(decrypted = key->decrypt(ca, encrypted)); - BOOST_REQUIRE(decrypted == data); - - // decrypt with 12B IV should fail - ca.setParam(ParamName::ED_IV, shortIv); - BOOST_REQUIRE_THROW(key->decrypt(ca, encrypted), Exc::Crypto::InputParam); -} - NEGATIVE_TEST_CASE(symmetricEncryptDecryptCtr) { const auto key = generateAes(128); -- 2.7.4 From 5b1b01200d75ecb94e29900fb102e2779daf3084 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 12 Sep 2023 12:36:11 +0200 Subject: [PATCH 15/16] Revert "Fix GCM IV length setting" This reverts commit 1e6e268703fc313a973f06581e21cf62c112e903. Change-Id: Ibb87d42af732735413bdbffa9fadc15e7457eb59 --- .../crypto/generic-backend/algo-validation.h | 15 -------------- src/manager/crypto/sw-backend/crypto.h | 23 +++++----------------- src/manager/crypto/sw-backend/internals.cpp | 3 +-- unit-tests/test_crypto-logic.cpp | 5 +++++ unit-tests/test_sw-backend.cpp | 12 +++++------ 5 files changed, 17 insertions(+), 41 deletions(-) diff --git a/src/manager/crypto/generic-backend/algo-validation.h b/src/manager/crypto/generic-backend/algo-validation.h index 8946a35..dc14966 100644 --- a/src/manager/crypto/generic-backend/algo-validation.h +++ b/src/manager/crypto/generic-backend/algo-validation.h @@ -98,21 +98,6 @@ struct Type { }; }; -// Validates as true if parameter value is equal or greater than Min -template -struct GreaterOrEqual { -public: - static bool Check(const T &value) - { - return value >= Min; - } - - static void Why(std::ostringstream &os) - { - os << "is smaller than " << static_cast(Min); - } -}; - template struct Unsupported { static bool Check(const T &) diff --git a/src/manager/crypto/sw-backend/crypto.h b/src/manager/crypto/sw-backend/crypto.h index 0b49afd..a129682 100644 --- a/src/manager/crypto/sw-backend/crypto.h +++ b/src/manager/crypto/sw-backend/crypto.h @@ -99,25 +99,12 @@ public: ThrowErr(Exc::Crypto::InternalError, "Wrong key size! Expected: ", EVP_CIPHER_key_length(type), " Get: ", key.size()); - bool gcm = (EVP_CIPHER_mode(type) == EVP_CIPH_GCM_MODE); - int iv_len = EVP_CIPHER_iv_length(type); + if (static_cast(iv.size()) < EVP_CIPHER_iv_length(type)) + ThrowErr(Exc::Crypto::InternalError, "Wrong iv size! Expected: ", + EVP_CIPHER_iv_length(type), " Get: ", iv.size()); - OPENSSL_ERROR_HANDLE(EVP_CipherInit_ex(m_ctx, type, NULL, NULL, NULL, encryption ? 1 : 0)); - - if (gcm) { - if (iv.empty()) - ThrowErr(Exc::Crypto::InternalError, "Empty iv provided!"); - - OPENSSL_ERROR_HANDLE( - EVP_CIPHER_CTX_ctrl(m_ctx, EVP_CTRL_GCM_SET_IVLEN, iv.size(), NULL)); - } else { - if (static_cast(iv.size()) != iv_len) - ThrowErr(Exc::Crypto::InternalError, "Wrong iv size! Expected: ", iv_len, " Got: ", - iv.size()); - } - - OPENSSL_ERROR_HANDLE( - EVP_CipherInit_ex(m_ctx, NULL, NULL, key.data(), iv.data(), encryption ? 1 : 0)); + OPENSSL_ERROR_HANDLE(EVP_CipherInit_ex(m_ctx, type, NULL, key.data(), iv.data(), + encryption ? 1 : 0)); EVP_CIPHER_CTX_set_padding(m_ctx, 1); } diff --git a/src/manager/crypto/sw-backend/internals.cpp b/src/manager/crypto/sw-backend/internals.cpp index 75ba3e3..247b434 100644 --- a/src/manager/crypto/sw-backend/internals.cpp +++ b/src/manager/crypto/sw-backend/internals.cpp @@ -115,8 +115,7 @@ typedef ParamCheck, - BufferSizeGetter> GcmIvCheck; + DefaultValidator> GcmIvCheck; typedef ParamCheckencrypt(ca, data), Exc::Crypto::InputParam); + // short iv + ca.setParam(ParamName::ED_IV, RawBuffer(1)); + BOOST_REQUIRE_THROW(key->encrypt(ca, data), Exc::Crypto::InternalError); ca.setParam(ParamName::ED_IV, iv); // short key @@ -631,6 +628,9 @@ NEGATIVE_TEST_CASE(symmetricEncryptDecryptGcm) // no iv BOOST_REQUIRE_THROW(key->decrypt(ca2, encrypted), Exc::Crypto::InputParam); + // short iv + ca2.setParam(ParamName::ED_IV, RawBuffer(1)); + BOOST_REQUIRE_THROW(key->decrypt(ca2, encrypted), Exc::Crypto::InternalError); ca2.setParam(ParamName::ED_IV, iv); // short key -- 2.7.4 From 564eebe458acdab5dcbf573d56bf136468b2ddd6 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Tue, 12 Sep 2023 12:36:35 +0200 Subject: [PATCH 16/16] Revert "Test proper GCM IV length handling" This reverts commit 61b910797b706b3e8494eb5841e4462bf1356125. Change-Id: Ifb7e276f1dee253c606800049ef97aea5c3bec77 --- src/manager/crypto/generic-backend/crypto-params.h | 1 - unit-tests/test_sw-backend.cpp | 10 +--------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/manager/crypto/generic-backend/crypto-params.h b/src/manager/crypto/generic-backend/crypto-params.h index 41a4461..ae23fba 100644 --- a/src/manager/crypto/generic-backend/crypto-params.h +++ b/src/manager/crypto/generic-backend/crypto-params.h @@ -27,7 +27,6 @@ class Params { public: static const size_t DEFAULT_AES_IV_LEN = 16; // max acceptable size of IV - static const size_t DEFAULT_AES_GCM_IV_LEN = 12; // default size of IV in GCM mode static const int DEFAULT_AES_GCM_TAG_LEN_BYTES = 16; // length of AES GCM tag static const int DEFAULT_AES_GCM_TAG_LEN_BITS = DEFAULT_AES_GCM_TAG_LEN_BYTES * 8; static const int DERIVED_KEY_LENGTH = 16; // length of AES key derived from password in bytes diff --git a/unit-tests/test_sw-backend.cpp b/unit-tests/test_sw-backend.cpp index c4ae72c..ae0154e 100644 --- a/unit-tests/test_sw-backend.cpp +++ b/unit-tests/test_sw-backend.cpp @@ -645,17 +645,9 @@ NEGATIVE_TEST_CASE(symmetricEncryptDecryptGcm) // wrong iv auto wrongIv = iv; - wrongIv[iv.size() - 1] ^= 0x1; + wrongIv[0] ^= 0x1; ca2.setParam(ParamName::ED_IV, wrongIv); BOOST_REQUIRE_THROW(key->decrypt(ca2, encrypted), Exc::Crypto::InputParam); - - // shortened iv - auto shortenedIv = iv; - static_assert(Params::DEFAULT_AES_GCM_IV_LEN < Params::DEFAULT_AES_IV_LEN); - shortenedIv.resize(Params::DEFAULT_AES_GCM_IV_LEN); - ca2.setParam(ParamName::ED_IV, shortenedIv); - BOOST_REQUIRE_THROW(key->decrypt(ca2, encrypted), Exc::Crypto::InputParam); - ca2.setParam(ParamName::ED_IV, iv); // wrong ciphertext -- 2.7.4