From cd208d6dba198903efa345ec9663a57140ca0edb Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Tue, 9 Jan 2024 11:46:56 +0100 Subject: [PATCH] [crypto] Add HKDF key handle and use it during PASE Current SPAKE2+ interface assumes that raw shared secret is extracted and used by the application to derive session keys. This prevents using secure crypto APIs, such as PSA, to perform SPAKE2+ and do the key derivation in a secure environment, and isolate the application from key material. 1. Add Hkdf128KeyHandle type and add methods for deriving session keys from an HKDF key. 2. Change SPAKE2+ interface to return HKDF key handle instead of raw key secret. A similar approach can be taken to improve CASE security in the future though we would need 256-bit HKDF key support in such a case. --- src/crypto/CHIPCryptoPAL.cpp | 18 +++--- src/crypto/CHIPCryptoPAL.h | 34 +++++------ src/crypto/CHIPCryptoPALPSA.cpp | 19 ++++-- src/crypto/CHIPCryptoPALPSA.h | 9 ++- src/crypto/PSASessionKeystore.cpp | 54 +++++++++++++++-- src/crypto/PSASessionKeystore.h | 9 +++ src/crypto/RawKeySessionKeystore.cpp | 14 +++++ src/crypto/RawKeySessionKeystore.h | 4 ++ src/crypto/SessionKeystore.h | 39 ++++++++++-- src/crypto/tests/CHIPCryptoPALTest.cpp | 63 ++++++++++++++++---- src/protocols/secure_channel/CASESession.h | 6 +- src/protocols/secure_channel/PASESession.cpp | 15 +++-- src/transport/CryptoContext.cpp | 63 ++++++++++++++------ src/transport/CryptoContext.h | 19 +++++- 14 files changed, 281 insertions(+), 85 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index 1154d25a6b5d2a..015bfba53f9cdc 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -21,6 +21,9 @@ */ #include "CHIPCryptoPAL.h" + +#include "SessionKeystore.h" + #include #include #include @@ -498,18 +501,15 @@ CHIP_ERROR Spake2p::KeyConfirm(const uint8_t * in, size_t in_len) return CHIP_NO_ERROR; } -CHIP_ERROR Spake2p::GetKeys(uint8_t * out, size_t * out_len) +CHIP_ERROR Spake2p::GetKeys(SessionKeystore & keystore, Hkdf128KeyHandle & key) const { - CHIP_ERROR error = CHIP_ERROR_INTERNAL; + VerifyOrReturnError(state == CHIP_SPAKE2P_STATE::KC, CHIP_ERROR_INTERNAL); + VerifyOrReturnError(hash_size / 2 == sizeof(Symmetric128BitsKeyByteArray), CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(state == CHIP_SPAKE2P_STATE::KC, error = CHIP_ERROR_INTERNAL); - VerifyOrExit(*out_len >= hash_size / 2, error = CHIP_ERROR_INVALID_ARGUMENT); + Symmetric128BitsKeyByteArray keyMaterial; + memcpy(keyMaterial, Ke, hash_size / 2); - memcpy(out, Ke, hash_size / 2); - error = CHIP_NO_ERROR; -exit: - *out_len = hash_size / 2; - return error; + return keystore.CreateKey(keyMaterial, key); } CHIP_ERROR Spake2p_P256_SHA256_HKDF_HMAC::InitImpl() diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 578111d35e7a16..cca09ab480d6ae 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -575,7 +575,6 @@ using Symmetric128BitsKeyByteArray = uint8_t[CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BY * * @note Symmetric128BitsKeyHandle is an abstract class to force child classes for each key handle type. * Symmetric128BitsKeyHandle class implements all the necessary components for handles. - * Child classes only need to implement a constructor and delete all the copy operators. */ class Symmetric128BitsKeyHandle { @@ -621,13 +620,6 @@ class Symmetric128BitsKeyHandle */ class Aes128KeyHandle final : public Symmetric128BitsKeyHandle { -public: - Aes128KeyHandle() = default; - - Aes128KeyHandle(const Aes128KeyHandle &) = delete; - Aes128KeyHandle(Aes128KeyHandle &&) = delete; - void operator=(const Aes128KeyHandle &) = delete; - void operator=(Aes128KeyHandle &&) = delete; }; /** @@ -635,13 +627,13 @@ class Aes128KeyHandle final : public Symmetric128BitsKeyHandle */ class Hmac128KeyHandle final : public Symmetric128BitsKeyHandle { -public: - Hmac128KeyHandle() = default; +}; - Hmac128KeyHandle(const Hmac128KeyHandle &) = delete; - Hmac128KeyHandle(Hmac128KeyHandle &&) = delete; - void operator=(const Hmac128KeyHandle &) = delete; - void operator=(Hmac128KeyHandle &&) = delete; +/** + * @brief Platform-specific HKDF key handle + */ +class Hkdf128KeyHandle final : public Symmetric128BitsKeyHandle +{ }; /** @@ -1060,6 +1052,9 @@ class PBKDF2_sha256 unsigned int iteration_count, uint32_t key_length, uint8_t * output); }; +// TODO: Extract Spake2p to a separate header and replace the forward declaration with #include SessionKeystore.h +class SessionKeystore; + /** * The below class implements the draft 01 version of the Spake2+ protocol as * defined in https://www.ietf.org/id/draft-bar-cfrg-spake2plus-01.html. @@ -1175,14 +1170,17 @@ class Spake2p virtual CHIP_ERROR KeyConfirm(const uint8_t * in, size_t in_len); /** - * @brief Return the shared secret. + * @brief Return the shared HKDF key. + * + * Returns the shared key established during the Spake2+ process, which can be used + * to derive application-specific keys using HKDF. * - * @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, Hkdf128KeyHandle & key) const; CHIP_ERROR InternalHash(const uint8_t * in, size_t in_len); CHIP_ERROR WriteMN(); diff --git a/src/crypto/CHIPCryptoPALPSA.cpp b/src/crypto/CHIPCryptoPALPSA.cpp index f6e89285f2795e..e08ab8f24bf0d8 100644 --- a/src/crypto/CHIPCryptoPALPSA.cpp +++ b/src/crypto/CHIPCryptoPALPSA.cpp @@ -271,7 +271,7 @@ void Hash_SHA256_stream::Clear() psa_hash_abort(toHashOperation(&mContext)); } -CHIP_ERROR PsaKdf::Init(psa_algorithm_t algorithm, const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info) +CHIP_ERROR PsaKdf::Init(const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info) { psa_status_t status = PSA_SUCCESS; psa_key_attributes_t attrs = PSA_KEY_ATTRIBUTES_INIT; @@ -284,7 +284,17 @@ CHIP_ERROR PsaKdf::Init(psa_algorithm_t algorithm, const ByteSpan & secret, cons psa_reset_key_attributes(&attrs); VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); - status = psa_key_derivation_setup(&mOperation, algorithm); + return InitOperation(mSecretKeyId, salt, info); +} + +CHIP_ERROR PsaKdf::Init(const Hkdf128KeyHandle & hkdfKey, const ByteSpan & salt, const ByteSpan & info) +{ + return InitOperation(hkdfKey.As(), salt, info); +} + +CHIP_ERROR PsaKdf::InitOperation(psa_key_id_t 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) @@ -293,7 +303,7 @@ CHIP_ERROR PsaKdf::Init(psa_algorithm_t algorithm, const ByteSpan & secret, cons VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); } - status = psa_key_derivation_input_key(&mOperation, PSA_KEY_DERIVATION_INPUT_SECRET, mSecretKeyId); + status = psa_key_derivation_input_key(&mOperation, PSA_KEY_DERIVATION_INPUT_SECRET, hkdfKey); VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); status = psa_key_derivation_input_bytes(&mOperation, PSA_KEY_DERIVATION_INPUT_INFO, info.data(), info.size()); @@ -328,8 +338,7 @@ CHIP_ERROR HKDF_sha::HKDF_SHA256(const uint8_t * secret, const size_t secret_len PsaKdf kdf; - ReturnErrorOnFailure(kdf.Init(PSA_ALG_HKDF(PSA_ALG_SHA_256), ByteSpan(secret, secret_length), ByteSpan(salt, salt_length), - ByteSpan(info, info_length))); + ReturnErrorOnFailure(kdf.Init(ByteSpan(secret, secret_length), ByteSpan(salt, salt_length), ByteSpan(info, info_length))); return kdf.DeriveBytes(MutableByteSpan(out_buffer, out_length)); } diff --git a/src/crypto/CHIPCryptoPALPSA.h b/src/crypto/CHIPCryptoPALPSA.h index 1a64c1f8794dfa..cce91142204a4e 100644 --- a/src/crypto/CHIPCryptoPALPSA.h +++ b/src/crypto/CHIPCryptoPALPSA.h @@ -109,7 +109,12 @@ class PsaKdf /** * @brief Initializes the key derivation operation. */ - CHIP_ERROR Init(psa_algorithm_t algorithm, const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info); + CHIP_ERROR Init(const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info); + + /** + * @brief Initializes the key derivation operation. + */ + CHIP_ERROR Init(const Hkdf128KeyHandle & hkdfKey, const ByteSpan & salt, const ByteSpan & info); /** * @brief Derives raw key material from the operation. @@ -139,6 +144,8 @@ 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); + psa_key_id_t mSecretKeyId = 0; psa_key_derivation_operation_t mOperation = PSA_KEY_DERIVATION_OPERATION_INIT; }; diff --git a/src/crypto/PSASessionKeystore.cpp b/src/crypto/PSASessionKeystore.cpp index 28f8f002e16a1c..bc04369c74ac8d 100644 --- a/src/crypto/PSASessionKeystore.cpp +++ b/src/crypto/PSASessionKeystore.cpp @@ -17,8 +17,6 @@ #include "PSASessionKeystore.h" -#include - #include namespace chip { @@ -66,6 +64,24 @@ class HmacKeyAttributes psa_key_attributes_t mAttrs = PSA_KEY_ATTRIBUTES_INIT; }; +class HkdfKeyAttributes +{ +public: + HkdfKeyAttributes() + { + psa_set_key_type(&mAttrs, PSA_KEY_TYPE_DERIVE); + psa_set_key_algorithm(&mAttrs, PSA_ALG_HKDF(PSA_ALG_SHA_256)); + psa_set_key_usage_flags(&mAttrs, PSA_KEY_USAGE_DERIVE); + } + + ~HkdfKeyAttributes() { psa_reset_key_attributes(&mAttrs); } + + const psa_key_attributes_t & Get() { return mAttrs; } + +private: + psa_key_attributes_t mAttrs = PSA_KEY_ATTRIBUTES_INIT; +}; + } // namespace CHIP_ERROR PSASessionKeystore::CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Aes128KeyHandle & key) @@ -95,11 +111,25 @@ CHIP_ERROR PSASessionKeystore::CreateKey(const Symmetric128BitsKeyByteArray & ke return CHIP_NO_ERROR; } +CHIP_ERROR PSASessionKeystore::CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hkdf128KeyHandle & key) +{ + // Destroy the old key if already allocated + psa_destroy_key(key.As()); + + HkdfKeyAttributes attrs; + psa_status_t status = + psa_import_key(&attrs.Get(), keyMaterial, sizeof(Symmetric128BitsKeyByteArray), &key.AsMutable()); + + VerifyOrReturnError(status == PSA_SUCCESS, CHIP_ERROR_INTERNAL); + + return CHIP_NO_ERROR; +} + CHIP_ERROR PSASessionKeystore::DeriveKey(const P256ECDHDerivedSecret & secret, const ByteSpan & salt, const ByteSpan & info, Aes128KeyHandle & key) { PsaKdf kdf; - ReturnErrorOnFailure(kdf.Init(PSA_ALG_HKDF(PSA_ALG_SHA_256), secret.Span(), salt, info)); + ReturnErrorOnFailure(kdf.Init(secret.Span(), salt, info)); AesKeyAttributes attrs; @@ -111,8 +141,24 @@ CHIP_ERROR PSASessionKeystore::DeriveSessionKeys(const ByteSpan & secret, const AttestationChallenge & attestationChallenge) { PsaKdf kdf; - ReturnErrorOnFailure(kdf.Init(PSA_ALG_HKDF(PSA_ALG_SHA_256), secret, salt, info)); + ReturnErrorOnFailure(kdf.Init(secret, salt, info)); + + return DeriveSessionKeys(kdf, i2rKey, r2iKey, attestationChallenge); +} + +CHIP_ERROR PSASessionKeystore::DeriveSessionKeys(const Hkdf128KeyHandle & hkdfKey, const ByteSpan & salt, const ByteSpan & info, + Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, + AttestationChallenge & attestationChallenge) +{ + PsaKdf kdf; + ReturnErrorOnFailure(kdf.Init(hkdfKey, salt, info)); + return DeriveSessionKeys(kdf, i2rKey, r2iKey, attestationChallenge); +} + +CHIP_ERROR PSASessionKeystore::DeriveSessionKeys(PsaKdf & kdf, Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, + AttestationChallenge & attestationChallenge) +{ CHIP_ERROR error; AesKeyAttributes attrs; diff --git a/src/crypto/PSASessionKeystore.h b/src/crypto/PSASessionKeystore.h index 690194fcd4314d..f81b64bf390529 100644 --- a/src/crypto/PSASessionKeystore.h +++ b/src/crypto/PSASessionKeystore.h @@ -17,6 +17,7 @@ #pragma once +#include #include namespace chip { @@ -27,11 +28,19 @@ class PSASessionKeystore : public SessionKeystore public: CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Aes128KeyHandle & key) override; CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hmac128KeyHandle & key) override; + CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hkdf128KeyHandle & key) override; CHIP_ERROR DeriveKey(const P256ECDHDerivedSecret & secret, const ByteSpan & salt, const ByteSpan & info, Aes128KeyHandle & key) override; CHIP_ERROR DeriveSessionKeys(const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info, Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, AttestationChallenge & attestationChallenge) override; + CHIP_ERROR DeriveSessionKeys(const Hkdf128KeyHandle & hkdfKey, const ByteSpan & salt, const ByteSpan & info, + Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, + AttestationChallenge & attestationChallenge) override; void DestroyKey(Symmetric128BitsKeyHandle & key) override; + +private: + CHIP_ERROR DeriveSessionKeys(PsaKdf & kdf, Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, + AttestationChallenge & attestationChallenge); }; } // namespace Crypto diff --git a/src/crypto/RawKeySessionKeystore.cpp b/src/crypto/RawKeySessionKeystore.cpp index 949d30a857ee53..eb8a8cd9c98048 100644 --- a/src/crypto/RawKeySessionKeystore.cpp +++ b/src/crypto/RawKeySessionKeystore.cpp @@ -36,6 +36,12 @@ CHIP_ERROR RawKeySessionKeystore::CreateKey(const Symmetric128BitsKeyByteArray & return CHIP_NO_ERROR; } +CHIP_ERROR RawKeySessionKeystore::CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hkdf128KeyHandle & key) +{ + memcpy(key.AsMutable(), keyMaterial, sizeof(Symmetric128BitsKeyByteArray)); + return CHIP_NO_ERROR; +} + CHIP_ERROR RawKeySessionKeystore::DeriveKey(const P256ECDHDerivedSecret & secret, const ByteSpan & salt, const ByteSpan & info, Aes128KeyHandle & key) { @@ -63,6 +69,14 @@ CHIP_ERROR RawKeySessionKeystore::DeriveSessionKeys(const ByteSpan & secret, con .StatusCode(); } +CHIP_ERROR RawKeySessionKeystore::DeriveSessionKeys(const Hkdf128KeyHandle & hkdfKey, const ByteSpan & salt, const ByteSpan & info, + Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, + AttestationChallenge & attestationChallenge) +{ + return DeriveSessionKeys(ByteSpan(hkdfKey.As()), salt, info, i2rKey, r2iKey, + attestationChallenge); +} + void RawKeySessionKeystore::DestroyKey(Symmetric128BitsKeyHandle & key) { ClearSecretData(key.AsMutable()); diff --git a/src/crypto/RawKeySessionKeystore.h b/src/crypto/RawKeySessionKeystore.h index 51f6c927500b7c..a4d571297b5d45 100644 --- a/src/crypto/RawKeySessionKeystore.h +++ b/src/crypto/RawKeySessionKeystore.h @@ -27,10 +27,14 @@ class RawKeySessionKeystore : public SessionKeystore public: CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Aes128KeyHandle & key) override; CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hmac128KeyHandle & key) override; + CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hkdf128KeyHandle & key) override; CHIP_ERROR DeriveKey(const P256ECDHDerivedSecret & secret, const ByteSpan & salt, const ByteSpan & info, Aes128KeyHandle & key) override; CHIP_ERROR DeriveSessionKeys(const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info, Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, AttestationChallenge & attestationChallenge) override; + CHIP_ERROR DeriveSessionKeys(const Hkdf128KeyHandle & hkdfKey, const ByteSpan & salt, const ByteSpan & info, + Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, + AttestationChallenge & attestationChallenge) override; void DestroyKey(Symmetric128BitsKeyHandle & key) override; }; diff --git a/src/crypto/SessionKeystore.h b/src/crypto/SessionKeystore.h index b5d1a26e30e6a9..39df472f43d9a0 100644 --- a/src/crypto/SessionKeystore.h +++ b/src/crypto/SessionKeystore.h @@ -67,6 +67,18 @@ class SessionKeystore */ virtual CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hmac128KeyHandle & key) = 0; + /** + * @brief Import raw key material and return a key handle for a 128-bit key that can be used to do HKDF. + * + * @note This method should only be used when using the raw key material in the Matter stack + * cannot be avoided. Ideally, crypto interfaces should allow platforms to perform all the + * cryptographic operations in a secure environment. + * + * If the method returns no error, the application is responsible for destroying the handle + * using the DestroyKey() method when the key is no longer needed. + */ + virtual CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hkdf128KeyHandle & key) = 0; + /** * @brief Destroy key. * @@ -96,29 +108,44 @@ class SessionKeystore * Use HKDF as defined in the Matter specification to derive AES keys for both directions, and * the attestation challenge from the shared secret. * - * If the method returns no error, the application is responsible for destroying the handles + * If the method returns no error, the application is responsible for destroying the AES keys * using DestroyKey() method when the keys are no longer needed. On failure, the method must * release all handles that it allocated so far. */ virtual CHIP_ERROR DeriveSessionKeys(const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info, Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, AttestationChallenge & attestationChallenge) = 0; + + /** + * @brief Derive session keys from the HKDF key + * + * Use HKDF as defined in the Matter specification to derive AES keys for both directions, and + * the attestation challenge from the HKDF key. + * + * If the method returns no error, the application is responsible for destroying the AES keys + * using DestroyKey() method when the keys are no longer needed. On failure, the method must + * release all handles that it allocated so far. + */ + virtual CHIP_ERROR DeriveSessionKeys(const Hkdf128KeyHandle & hkdfKey, const ByteSpan & salt, const ByteSpan & info, + Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, + AttestationChallenge & attestationChallenge) = 0; }; /** * @brief RAII class to hold a temporary key handle that is destroyed on scope exit. */ -class AutoReleaseSessionKey +template +class AutoReleaseSymmetricKey { public: - explicit AutoReleaseSessionKey(SessionKeystore & keystore) : mKeystore(keystore) {} - ~AutoReleaseSessionKey() { mKeystore.DestroyKey(mKeyHandle); } + explicit AutoReleaseSymmetricKey(SessionKeystore & keystore) : mKeystore(keystore) {} + ~AutoReleaseSymmetricKey() { mKeystore.DestroyKey(mKeyHandle); } - Aes128KeyHandle & KeyHandle() { return mKeyHandle; } + KeyHandleType & KeyHandle() { return mKeyHandle; } private: SessionKeystore & mKeystore; - Aes128KeyHandle mKeyHandle; + KeyHandleType mKeyHandle; }; } // namespace Crypto diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index 74b339abad9877..77d482aa4e6fd7 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -126,6 +126,34 @@ class HeapChecker #include "DacValidationExplicitVectors.h" +// Verify that two HKDF keys are equal by checking if they generate the same attestation challenge. +// Note that the keys cannot be compared directly because they are given as key handles. +void AssertKeysEqual(nlTestSuite * inSuite, SessionKeystore & keystore, Hkdf128KeyHandle & left, const Hkdf128KeyHandle & right) +{ + auto generateChallenge = [&](const Hkdf128KeyHandle & key, AttestationChallenge & challenge) -> void { + constexpr uint8_t kTestSalt[] = { 'T', 'E', 'S', 'T', 'S', 'A', 'L', 'T' }; + constexpr uint8_t kTestInfo[] = { 'T', 'E', 'S', 'T', 'I', 'N', 'F', 'O' }; + + Aes128KeyHandle i2rKey; + Aes128KeyHandle r2iKey; + + CHIP_ERROR error = keystore.DeriveSessionKeys(key, ByteSpan(kTestSalt), ByteSpan(kTestInfo), i2rKey, r2iKey, challenge); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + // Ignore the keys, just return the attestation challenge + keystore.DestroyKey(i2rKey); + keystore.DestroyKey(r2iKey); + }; + + AttestationChallenge leftChallenge; + AttestationChallenge rightChallenge; + + generateChallenge(left, leftChallenge); + generateChallenge(right, rightChallenge); + + NL_TEST_ASSERT(inSuite, memcmp(leftChallenge.ConstBytes(), rightChallenge.ConstBytes(), AttestationChallenge::Capacity()) == 0); +} + } // namespace static uint32_t gs_test_entropy_source_called = 0; @@ -1839,10 +1867,7 @@ static void TestSPAKE2P_RFC(nlTestSuite * inSuite, void * inContext) size_t Pverifier_len = sizeof(Pverifier); uint8_t Vverifier[kMAX_Hash_Length]; size_t Vverifier_len = sizeof(Vverifier); - uint8_t VKe[kMAX_Hash_Length]; - size_t VKe_len = sizeof(VKe); - uint8_t PKe[kMAX_Hash_Length]; - size_t PKe_len = sizeof(PKe); + DefaultSessionKeystore keystore; int numOfTestVectors = ArraySize(rfc_tvs); int numOfTestsRan = 0; @@ -1939,17 +1964,31 @@ static void TestSPAKE2P_RFC(nlTestSuite * inSuite, void * inContext) error = Verifier.KeyConfirm(Pverifier, Pverifier_len); NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); - PKe_len = sizeof(PKe); - error = Prover.GetKeys(PKe, &PKe_len); + // Import HKDF key from the test vector to the keystore + Symmetric128BitsKeyByteArray vectorKeMaterial; + Hkdf128KeyHandle vectorKe; + + NL_TEST_ASSERT(inSuite, sizeof(vectorKeMaterial) == vector->Ke_len); + memcpy(vectorKeMaterial, vector->Ke, vector->Ke_len); + error = keystore.CreateKey(vectorKeMaterial, vectorKe); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + // Verify that both sides generated the same HKDF key as in the test vector + // Since the HKDF keys may not be availabe in the raw form, do not compare them directly, + // but rather check if the same attestation challenge is derived from + Hkdf128KeyHandle PKe; + error = Prover.GetKeys(keystore, PKe); NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, PKe_len == vector->Ke_len); - NL_TEST_ASSERT(inSuite, memcmp(PKe, vector->Ke, vector->Ke_len) == 0); + AssertKeysEqual(inSuite, keystore, PKe, vectorKe); - VKe_len = sizeof(VKe); - error = Verifier.GetKeys(VKe, &VKe_len); + Hkdf128KeyHandle VKe; + error = Verifier.GetKeys(keystore, VKe); NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, VKe_len == vector->Ke_len); - NL_TEST_ASSERT(inSuite, memcmp(VKe, vector->Ke, vector->Ke_len) == 0); + AssertKeysEqual(inSuite, keystore, VKe, vectorKe); + + keystore.DestroyKey(vectorKe); + keystore.DestroyKey(PKe); + keystore.DestroyKey(VKe); numOfTestsRan += 1; } diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h index 6fc58dfb90dc83..cb7f3c7f13380d 100644 --- a/src/protocols/secure_channel/CASESession.h +++ b/src/protocols/secure_channel/CASESession.h @@ -214,6 +214,8 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, private: friend class TestCASESession; + using AutoReleaseSessionKey = Crypto::AutoReleaseSymmetricKey; + /* * Initialize the object given a reference to the SessionManager, certificate validity policy and a delegate which will be * notified of any further progress on this session. @@ -254,7 +256,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, CHIP_ERROR SendSigma2Resume(); - CHIP_ERROR DeriveSigmaKey(const ByteSpan & salt, const ByteSpan & info, Crypto::AutoReleaseSessionKey & key) const; + CHIP_ERROR DeriveSigmaKey(const ByteSpan & salt, const ByteSpan & info, AutoReleaseSessionKey & key) const; CHIP_ERROR ConstructSaltSigma2(const ByteSpan & rand, const Crypto::P256PublicKey & pubkey, const ByteSpan & ipk, MutableByteSpan & salt); CHIP_ERROR ConstructTBSData(const ByteSpan & senderNOC, const ByteSpan & senderICAC, const ByteSpan & senderPubKey, @@ -262,7 +264,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, CHIP_ERROR ConstructSaltSigma3(const ByteSpan & ipk, MutableByteSpan & salt); CHIP_ERROR ConstructSigmaResumeKey(const ByteSpan & initiatorRandom, const ByteSpan & resumptionID, const ByteSpan & skInfo, - const ByteSpan & nonce, Crypto::AutoReleaseSessionKey & resumeKey); + const ByteSpan & nonce, AutoReleaseSessionKey & resumeKey); CHIP_ERROR GenerateSigmaResumeMIC(const ByteSpan & initiatorRandom, const ByteSpan & resumptionID, const ByteSpan & skInfo, const ByteSpan & nonce, MutableByteSpan & resumeMIC); diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp index da168fa7c79c95..acd03f40af1ed0 100644 --- a/src/protocols/secure_channel/PASESession.cpp +++ b/src/protocols/secure_channel/PASESession.cpp @@ -92,7 +92,6 @@ void PASESession::Clear() // This function zeroes out and resets the memory used by the object. // It's done so that no security related information will be leaked. memset(&mPASEVerifier, 0, sizeof(mPASEVerifier)); - memset(&mKe[0], 0, sizeof(mKe)); mNextExpectedMsg.ClearValue(); mSpake2p.Clear(); @@ -105,7 +104,6 @@ void PASESession::Clear() chip::Platform::MemoryFree(mSalt); mSalt = nullptr; } - mKeLen = sizeof(mKe); mPairingComplete = false; PairingSession::Clear(); } @@ -259,8 +257,15 @@ void PASESession::OnResponseTimeout(ExchangeContext * ec) CHIP_ERROR PASESession::DeriveSecureSession(CryptoContext & session) const { VerifyOrReturnError(mPairingComplete, CHIP_ERROR_INCORRECT_STATE); - return session.InitFromSecret(*mSessionManager->GetSessionKeystore(), ByteSpan(mKe, mKeLen), ByteSpan(), - CryptoContext::SessionInfoType::kSessionEstablishment, mRole); + + SessionKeystore & keystore = *mSessionManager->GetSessionKeystore(); + AutoReleaseSymmetricKey hkdfKey(keystore); + + ReturnErrorOnFailure(mSpake2p.GetKeys(keystore, hkdfKey.KeyHandle())); + ReturnErrorOnFailure(session.InitFromSecret(keystore, hkdfKey.KeyHandle(), ByteSpan{} /* salt */, + CryptoContext::SessionInfoType::kSessionEstablishment, mRole)); + + return CHIP_NO_ERROR; } CHIP_ERROR PASESession::SendPBKDFParamRequest() @@ -667,7 +672,6 @@ CHIP_ERROR PASESession::HandleMsg2_and_SendMsg3(System::PacketBufferHandle && ms SuccessOrExit(err = mSpake2p.ComputeRoundTwo(Y, Y_len, verifier, &verifier_len)); SuccessOrExit(err = mSpake2p.KeyConfirm(peer_verifier, peer_verifier_len)); - SuccessOrExit(err = mSpake2p.GetKeys(mKe, &mKeLen)); msg2 = nullptr; { @@ -730,7 +734,6 @@ CHIP_ERROR PASESession::HandleMsg3(System::PacketBufferHandle && msg) VerifyOrExit(peer_verifier_len == kMAX_Hash_Length, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH); SuccessOrExit(err = mSpake2p.KeyConfirm(peer_verifier, peer_verifier_len)); - SuccessOrExit(err = mSpake2p.GetKeys(mKe, &mKeLen)); // Send confirmation to peer that we succeeded so they can start using the session. SendStatusReport(mExchangeCtxt, kProtocolCodeSuccess); diff --git a/src/transport/CryptoContext.cpp b/src/transport/CryptoContext.cpp index 97420cfd2d0e53..663849a7fa0206 100644 --- a/src/transport/CryptoContext.cpp +++ b/src/transport/CryptoContext.cpp @@ -69,7 +69,6 @@ CHIP_ERROR CryptoContext::InitFromSecret(SessionKeystore & keystore, const ByteS SessionInfoType infoType, SessionRole role) { VerifyOrReturnError(mKeyAvailable == false, CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(secret.size() > 0, CHIP_ERROR_INVALID_ARGUMENT); ByteSpan info = (infoType == SessionInfoType::kSessionResumption) ? ByteSpan(RSEKeysInfo) : ByteSpan(SEKeysInfo); @@ -79,30 +78,34 @@ CHIP_ERROR CryptoContext::InitFromSecret(SessionKeystore & keystore, const ByteS auto & r2iKey = (role == SessionRole::kInitiator) ? mDecryptionKey : mEncryptionKey; #if CHIP_CONFIG_SECURITY_TEST_MODE + ReturnErrorOnFailure(InitTestMode(keystore, i2rKey, r2iKey)); +#else + ReturnErrorOnFailure(keystore.DeriveSessionKeys(secret, salt, info, i2rKey, r2iKey, mAttestationChallenge)); +#endif - // If enabled, override the generated session key with a known key pair - // to allow man-in-the-middle session key recovery for testing purposes. + mKeyAvailable = true; + mSessionRole = role; + mKeystore = &keystore; - constexpr uint8_t kTestSharedSecret[CHIP_CONFIG_TEST_SHARED_SECRET_LENGTH] = CHIP_CONFIG_TEST_SHARED_SECRET_VALUE; - static_assert(sizeof(CHIP_CONFIG_TEST_SHARED_SECRET_VALUE) == CHIP_CONFIG_TEST_SHARED_SECRET_LENGTH, - "CHIP_CONFIG_TEST_SHARED_SECRET_VALUE must be 32 bytes"); - const ByteSpan & testSalt = ByteSpan(); - (void) info; + return CHIP_NO_ERROR; +} -#warning \ - "Warning: CHIP_CONFIG_SECURITY_TEST_MODE=1 bypassing key negotiation... All sessions will use known, fixed test key, and NodeID=0 in NONCE. Node can only communicate with other nodes built with this flag set. Requires build flag 'treat_warnings_as_errors=false'." - ChipLogError( - SecureChannel, - "Warning: CHIP_CONFIG_SECURITY_TEST_MODE=1 bypassing key negotiation... All sessions will use known, fixed test key, " - "and NodeID=0 in NONCE. " - "Node can only communicate with other nodes built with this flag set."); +CHIP_ERROR CryptoContext::InitFromSecret(Crypto::SessionKeystore & keystore, const Crypto::Hkdf128KeyHandle & hkdfKey, + const ByteSpan & salt, SessionInfoType infoType, SessionRole role) +{ + VerifyOrReturnError(mKeyAvailable == false, CHIP_ERROR_INCORRECT_STATE); - ReturnErrorOnFailure(keystore.DeriveSessionKeys(ByteSpan(kTestSharedSecret), testSalt, ByteSpan(SEKeysInfo), i2rKey, r2iKey, - mAttestationChallenge)); -#else + ByteSpan info = (infoType == SessionInfoType::kSessionResumption) ? ByteSpan(RSEKeysInfo) : ByteSpan(SEKeysInfo); - ReturnErrorOnFailure(keystore.DeriveSessionKeys(secret, salt, info, i2rKey, r2iKey, mAttestationChallenge)); + // If the secure session is created by session initiator, use the I2R key to encrypt + // messages being transmitted. Otherwise, use the R2I key. + auto & i2rKey = (role == SessionRole::kInitiator) ? mEncryptionKey : mDecryptionKey; + auto & r2iKey = (role == SessionRole::kInitiator) ? mDecryptionKey : mEncryptionKey; +#if CHIP_CONFIG_SECURITY_TEST_MODE + ReturnErrorOnFailure(InitTestMode(keystore, i2rKey, r2iKey)); +#else + ReturnErrorOnFailure(keystore.DeriveSessionKeys(hkdfKey, salt, info, i2rKey, r2iKey, mAttestationChallenge)); #endif mKeyAvailable = true; @@ -125,6 +128,28 @@ CHIP_ERROR CryptoContext::InitFromKeyPair(SessionKeystore & keystore, const Cryp return InitFromSecret(keystore, secret.Span(), salt, infoType, role); } +#if CHIP_CONFIG_SECURITY_TEST_MODE +CHIP_ERROR CryptoContext::InitTestMode(Crypto::SessionKeystore & keystore, Crypto::Aes128KeyHandle & i2rKey, + Crypto::Aes128KeyHandle & r2iKey) +{ + // If enabled, override the generated session key with a known key pair + // to allow man-in-the-middle session key recovery for testing purposes. + + constexpr uint8_t kTestSharedSecret[CHIP_CONFIG_TEST_SHARED_SECRET_LENGTH] = CHIP_CONFIG_TEST_SHARED_SECRET_VALUE; + +#warning \ + "Warning: CHIP_CONFIG_SECURITY_TEST_MODE=1 bypassing key negotiation... All sessions will use known, fixed test key, and NodeID=0 in NONCE. Node can only communicate with other nodes built with this flag set. Requires build flag 'treat_warnings_as_errors=false'." + ChipLogError( + SecureChannel, + "Warning: CHIP_CONFIG_SECURITY_TEST_MODE=1 bypassing key negotiation... All sessions will use known, fixed test key, " + "and NodeID=0 in NONCE. " + "Node can only communicate with other nodes built with this flag set."); + + return keystore.DeriveSessionKeys(ByteSpan(kTestSharedSecret), ByteSpan{} /* salt */, ByteSpan(SEKeysInfo), i2rKey, r2iKey, + mAttestationChallenge); +} +#endif // CHIP_CONFIG_SECURITY_TEST_MODE + CHIP_ERROR CryptoContext::BuildNonce(NonceView nonce, uint8_t securityFlags, uint32_t messageCounter, NodeId nodeId) { Encoding::LittleEndian::BufferWriter bbuf(nonce.data(), nonce.size()); diff --git a/src/transport/CryptoContext.h b/src/transport/CryptoContext.h index b08efdabbc9dcd..325ef768898d8c 100644 --- a/src/transport/CryptoContext.h +++ b/src/transport/CryptoContext.h @@ -84,9 +84,7 @@ class DLL_EXPORT CryptoContext SessionRole role); /** - * @brief - * Derive a shared key. The derived key will be used for encrypting/decrypting - * data exchanged on the secure channel. + * @brief Derive session keys and the attestation challenge from the shared secret. * * @param keystore Session keystore for management of symmetric encryption keys * @param secret A reference to the shared secret @@ -98,6 +96,19 @@ class DLL_EXPORT CryptoContext CHIP_ERROR InitFromSecret(Crypto::SessionKeystore & keystore, const ByteSpan & secret, const ByteSpan & salt, SessionInfoType infoType, SessionRole role); + /** + * @brief Derive session keys and the attestation challenge from the HKDF key. + * + * @param keystore Session keystore for management of symmetric encryption keys + * @param hkdfKey HKDF key handle + * @param salt A reference to the initial salt used for deriving the keys + * @param infoType The info buffer to use for deriving session keys + * @param role Role of the new session (initiator or responder) + * @return CHIP_ERROR The result of key derivation + */ + CHIP_ERROR InitFromSecret(Crypto::SessionKeystore & keystore, const Crypto::Hkdf128KeyHandle & hkdfKey, const ByteSpan & salt, + SessionInfoType infoType, SessionRole role); + /** @brief Build a Nonce buffer using given parameters for encrypt or decrypt. */ static CHIP_ERROR BuildNonce(NonceView nonce, uint8_t securityFlags, uint32_t messageCounter, NodeId nodeId); @@ -157,6 +168,8 @@ class DLL_EXPORT CryptoContext bool IsResponder() const { return mKeyAvailable && mSessionRole == SessionRole::kResponder; } private: + CHIP_ERROR InitTestMode(Crypto::SessionKeystore & keystore, Crypto::Aes128KeyHandle & i2rKey, Crypto::Aes128KeyHandle & r2iKey); + SessionRole mSessionRole; bool mKeyAvailable;