From 3a8936a55b245053293f50ee160e0bce47991648 Mon Sep 17 00:00:00 2001 From: Kamil Kasperczyk Date: Fri, 9 Feb 2024 15:24:26 +0100 Subject: [PATCH] [nrf noup] Integrated HKDF key handle with Spake2+ * Aligned Spake2+ implementation with the new CHIPCryptoPAL API using HKDF key handle instead of raw data. * Removed workaround extracting raw key data from oberon context. * Removed const from GetKeys and DeriveSecureSession methods, as PSA API requires access to non-const members to obtains shared secret. Signed-off-by: Kamil Kasperczyk --- src/crypto/CHIPCryptoPALPSA.cpp | 51 ++++++++++++++----- src/crypto/CHIPCryptoPALPSA.h | 20 ++++++-- src/crypto/PSASessionKeystore.cpp | 14 +++-- src/crypto/PSASpake2p.cpp | 31 +++++++---- src/crypto/PSASpake2p.h | 9 ++-- src/platform/nrfconnect/CHIPPlatformConfig.h | 3 +- .../tests/TestPairingSession.cpp | 2 +- 7 files changed, 92 insertions(+), 38 deletions(-) diff --git a/src/crypto/CHIPCryptoPALPSA.cpp b/src/crypto/CHIPCryptoPALPSA.cpp index 5b0de9e7a4..fe47747398 100644 --- a/src/crypto/CHIPCryptoPALPSA.cpp +++ b/src/crypto/CHIPCryptoPALPSA.cpp @@ -284,37 +284,59 @@ CHIP_ERROR PsaKdf::Init(const ByteSpan & secret, const ByteSpan & salt, const By psa_reset_key_attributes(&attrs); VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); - return InitOperation(mSecretKeyId, salt, info); + PsaHkdfKeyHandle hkdfKeyHandle = { .mKeyId = mSecretKeyId, .mIsKeyId = true }; + + return InitOperation(hkdfKeyHandle, salt, info); } CHIP_ERROR PsaKdf::Init(const HkdfKeyHandle & hkdfKey, const ByteSpan & salt, const ByteSpan & info) { - return InitOperation(hkdfKey.As(), salt, info); + return InitOperation(hkdfKey.As(), salt, info); } -CHIP_ERROR PsaKdf::InitOperation(psa_key_id_t hkdfKey, const ByteSpan & salt, const ByteSpan & info) +CHIP_ERROR PsaKdf::InitOperation(PsaHkdfKeyHandle hkdfKey, const ByteSpan & salt, const ByteSpan & info) { - psa_status_t status = psa_key_derivation_setup(&mOperation, PSA_ALG_HKDF(PSA_ALG_SHA_256)); - VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); - - if (salt.size() > 0) + psa_status_t status; + if (hkdfKey.mIsKeyId) { - status = psa_key_derivation_input_bytes(&mOperation, PSA_KEY_DERIVATION_INPUT_SALT, salt.data(), salt.size()); + status = psa_key_derivation_setup(&mOperation, PSA_ALG_HKDF(PSA_ALG_SHA_256)); VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + + if (salt.size() > 0) + { + status = psa_key_derivation_input_bytes(&mOperation, PSA_KEY_DERIVATION_INPUT_SALT, salt.data(), salt.size()); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + } + + status = psa_key_derivation_input_key(&mOperation, PSA_KEY_DERIVATION_INPUT_SECRET, hkdfKey.mKeyId); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + + status = psa_key_derivation_input_bytes(&mOperation, PSA_KEY_DERIVATION_INPUT_INFO, info.data(), info.size()); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + + mDerivationOperation = &mOperation; } + else + { + mDerivationOperation = hkdfKey.mKeyDerivationOp; - status = psa_key_derivation_input_key(&mOperation, PSA_KEY_DERIVATION_INPUT_SECRET, hkdfKey); - VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + if (salt.size() > 0) + { + status = psa_key_derivation_input_bytes(mDerivationOperation, PSA_KEY_DERIVATION_INPUT_SALT, salt.data(), salt.size()); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + } - status = psa_key_derivation_input_bytes(&mOperation, PSA_KEY_DERIVATION_INPUT_INFO, info.data(), info.size()); - VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + status = psa_key_derivation_input_bytes(mDerivationOperation, PSA_KEY_DERIVATION_INPUT_INFO, info.data(), info.size()); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + } return CHIP_NO_ERROR; } CHIP_ERROR PsaKdf::DeriveBytes(const MutableByteSpan & output) { - psa_status_t status = psa_key_derivation_output_bytes(&mOperation, output.data(), output.size()); + psa_status_t status = psa_key_derivation_output_bytes(mDerivationOperation, output.data(), output.size()); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); return CHIP_NO_ERROR; @@ -322,7 +344,8 @@ CHIP_ERROR PsaKdf::DeriveBytes(const MutableByteSpan & output) CHIP_ERROR PsaKdf::DeriveKey(const psa_key_attributes_t & attributes, psa_key_id_t & keyId) { - psa_status_t status = psa_key_derivation_output_key(&attributes, &mOperation, &keyId); + psa_status_t status = psa_key_derivation_output_key(&attributes, mDerivationOperation, &keyId); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); return CHIP_NO_ERROR; diff --git a/src/crypto/CHIPCryptoPALPSA.h b/src/crypto/CHIPCryptoPALPSA.h index 23af690b6b..8a5b4d84b3 100644 --- a/src/crypto/CHIPCryptoPALPSA.h +++ b/src/crypto/CHIPCryptoPALPSA.h @@ -28,6 +28,7 @@ #include "CHIPCryptoPAL.h" #include +#include #include #include @@ -95,6 +96,17 @@ inline const PsaP256KeypairContext & ToConstPsaContext(const P256KeypairContext return *SafePointerCast(&context); } +struct PsaHkdfKeyHandle +{ + union + { + psa_key_id_t mKeyId; + psa_key_derivation_operation_t * mKeyDerivationOp; + }; + + bool mIsKeyId = true; +}; + /** * @brief Wrapper for PSA key derivation API. */ @@ -145,11 +157,11 @@ class PsaKdf CHIP_ERROR DeriveKey(const psa_key_attributes_t & attributes, psa_key_id_t & keyId); private: - CHIP_ERROR InitOperation(psa_key_id_t hkdfKey, const ByteSpan & salt, const ByteSpan & info); + CHIP_ERROR InitOperation(PsaHkdfKeyHandle hkdfKey, const ByteSpan & salt, const ByteSpan & info); - psa_key_id_t mSecretKeyId = 0; - psa_key_derivation_operation_t mOperation = PSA_KEY_DERIVATION_OPERATION_INIT; + psa_key_id_t mSecretKeyId = 0; + psa_key_derivation_operation_t mOperation = PSA_KEY_DERIVATION_OPERATION_INIT; + psa_key_derivation_operation_t * mDerivationOperation = nullptr; }; - } // namespace Crypto } // namespace chip diff --git a/src/crypto/PSASessionKeystore.cpp b/src/crypto/PSASessionKeystore.cpp index 7a0fc4af10..32dc9566f4 100644 --- a/src/crypto/PSASessionKeystore.cpp +++ b/src/crypto/PSASessionKeystore.cpp @@ -185,10 +185,18 @@ void PSASessionKeystore::DestroyKey(Symmetric128BitsKeyHandle & key) void PSASessionKeystore::DestroyKey(HkdfKeyHandle & key) { - auto & keyId = key.AsMutable(); + auto & keyHandle = key.AsMutable(); - psa_destroy_key(keyId); - keyId = 0; + if (keyHandle.mIsKeyId) + { + psa_destroy_key(keyHandle.mKeyId); + keyHandle.mKeyId = 0; + } + else + { + Platform::Delete(keyHandle.mKeyDerivationOp); + keyHandle.mKeyDerivationOp = nullptr; + } } } // namespace Crypto diff --git a/src/crypto/PSASpake2p.cpp b/src/crypto/PSASpake2p.cpp index ecbc72426e..6731574822 100644 --- a/src/crypto/PSASpake2p.cpp +++ b/src/crypto/PSASpake2p.cpp @@ -19,6 +19,7 @@ #include "CHIPCryptoPALPSA.h" +#include #include #include @@ -179,22 +180,30 @@ CHIP_ERROR PSASpake2p_P256_SHA256_HKDF_HMAC::KeyConfirm(const uint8_t * in, size return CHIP_NO_ERROR; } -CHIP_ERROR PSASpake2p_P256_SHA256_HKDF_HMAC::GetKeys(uint8_t * out, size_t * out_len) +CHIP_ERROR PSASpake2p_P256_SHA256_HKDF_HMAC::GetKeys(SessionKeystore & keystore, HkdfKeyHandle & key) const { - VerifyOrReturnError(out != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(out_len != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - /* - * TODO: either: - * - use psa_pake_shared_secret() proposed in https://github.com/ARM-software/psa-api/issues/86 - * - refactor Matter's GetKeys API to take an abstract shared secret instead of raw secret bytes. + * TODO: use psa_pake_shared_secret() proposed in https://github.com/ARM-software/psa-api/issues/86 */ - oberon_spake2p_operation_t & oberonCtx = mOperation.MBEDTLS_PRIVATE(ctx).oberon_pake_ctx.ctx.oberon_spake2p_ctx; - VerifyOrReturnError((oberonCtx.hash_len / 2) <= *out_len, CHIP_ERROR_BUFFER_TOO_SMALL); + psa_key_derivation_operation_t * kdf = Platform::New(); + Platform::UniquePtr kdfPtr(kdf); + + VerifyOrReturnError(kdfPtr, CHIP_ERROR_NO_MEMORY); + + *kdfPtr = PSA_KEY_DERIVATION_OPERATION_INIT; + + psa_status_t status = psa_key_derivation_setup(kdfPtr.get(), PSA_ALG_HKDF(PSA_ALG_SHA_256)); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + + status = psa_pake_get_implicit_key(&mOperation, kdfPtr.get()); + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + + auto & hkdfKeyHandle = key.AsMutable(); + hkdfKeyHandle.mKeyDerivationOp = kdfPtr.get(); + hkdfKeyHandle.mIsKeyId = false; - memcpy(out, oberonCtx.shared, oberonCtx.hash_len / 2); - *out_len = oberonCtx.hash_len / 2; + kdfPtr.release(); return CHIP_NO_ERROR; } diff --git a/src/crypto/PSASpake2p.h b/src/crypto/PSASpake2p.h index b416fa002b..f8825af648 100644 --- a/src/crypto/PSASpake2p.h +++ b/src/crypto/PSASpake2p.h @@ -18,6 +18,7 @@ #pragma once #include "CHIPCryptoPAL.h" +#include #include @@ -144,14 +145,14 @@ class PSASpake2p_P256_SHA256_HKDF_HMAC CHIP_ERROR KeyConfirm(const uint8_t * in, size_t in_len); /** - * @brief Return the shared secret. + * @brief Return the shared HKDF key. * - * @param out The output secret. - * @param out_len The output secret length. + * @param keystore The session keystore for managing the HKDF key lifetime. + * @param key The output HKDF key. * * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise **/ - CHIP_ERROR GetKeys(uint8_t * out, size_t * out_len); + CHIP_ERROR GetKeys(SessionKeystore & keystore, HkdfKeyHandle & key) const; private: psa_pake_operation_t mOperation = PSA_PAKE_OPERATION_INIT; diff --git a/src/platform/nrfconnect/CHIPPlatformConfig.h b/src/platform/nrfconnect/CHIPPlatformConfig.h index 3ece933d37..e23f218bea 100644 --- a/src/platform/nrfconnect/CHIPPlatformConfig.h +++ b/src/platform/nrfconnect/CHIPPlatformConfig.h @@ -40,7 +40,8 @@ #ifdef CONFIG_CHIP_CRYPTO_PSA #define CHIP_CONFIG_SHA256_CONTEXT_SIZE sizeof(psa_hash_operation_t) -#define CHIP_CONFIG_HKDF_KEY_HANDLE_CONTEXT_SIZE sizeof(psa_key_id_t) +// Alignment to sizeof(PsaHkdfKeyHandle) from crypto/CHIPCryptoPALPSA.h. +#define CHIP_CONFIG_HKDF_KEY_HANDLE_CONTEXT_SIZE (8) #elif defined(CONFIG_CC3XX_BACKEND) // Size of the statically allocated context for SHA256 operations in CryptoPAL // determined empirically. diff --git a/src/protocols/secure_channel/tests/TestPairingSession.cpp b/src/protocols/secure_channel/tests/TestPairingSession.cpp index 714a18e03f..fe0b30bacc 100644 --- a/src/protocols/secure_channel/tests/TestPairingSession.cpp +++ b/src/protocols/secure_channel/tests/TestPairingSession.cpp @@ -48,7 +48,7 @@ class TestPairingSession : public PairingSession const ReliableMessageProtocolConfig & GetRemoteMRPConfig() const { return mRemoteMRPConfig; } - CHIP_ERROR DeriveSecureSession(CryptoContext & session) const override { return CHIP_NO_ERROR; } + CHIP_ERROR DeriveSecureSession(CryptoContext & session) override { return CHIP_NO_ERROR; } CHIP_ERROR DecodeMRPParametersIfPresent(TLV::Tag expectedTag, System::PacketBufferTLVReader & tlvReader) {