[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 <kamil.kasperczyk@nordicsemi.no>
nrfconnect · Feb 16, 2024 · 1fa4e7c · 1fa4e7c
1 parent 157480a
commit 1fa4e7c
Show file tree

Hide file tree

Showing 14 changed files with 99 additions and 45 deletions.
diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp
@@ -501,7 +501,7 @@ CHIP_ERROR Spake2p::KeyConfirm(const uint8_t * in, size_t in_len)
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR Spake2p::GetKeys(SessionKeystore & keystore, HkdfKeyHandle & key) const
+CHIP_ERROR Spake2p::GetKeys(SessionKeystore & keystore, HkdfKeyHandle & key)
 {
     VerifyOrReturnError(state == CHIP_SPAKE2P_STATE::KC, CHIP_ERROR_INTERNAL);
 

diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h
@@ -1184,7 +1184,7 @@ class Spake2p
      *
      * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
      **/
-    CHIP_ERROR GetKeys(SessionKeystore & keystore, HkdfKeyHandle & key) const;
+    CHIP_ERROR GetKeys(SessionKeystore & keystore, HkdfKeyHandle & key);
 
     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
@@ -284,45 +284,68 @@ 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<psa_key_id_t>(), salt, info);
+    return InitOperation(hkdfKey.As<PsaHkdfKeyHandle>(), 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;
 }
 
 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
@@ -95,6 +95,17 @@ inline const PsaP256KeypairContext & ToConstPsaContext(const P256KeypairContext
     return *SafePointerCast<const PsaP256KeypairContext *>(&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 +156,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                             = PSA_KEY_ID_NULL;
+    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
@@ -17,6 +17,7 @@
 
 #include "PSASessionKeystore.h"
 
+#include <lib/support/CHIPMem.h>
 #include <psa/crypto.h>
 
 namespace chip {
@@ -185,10 +186,18 @@ void PSASessionKeystore::DestroyKey(Symmetric128BitsKeyHandle & key)
 
 void PSASessionKeystore::DestroyKey(HkdfKeyHandle & key)
 {
-    auto & keyId = key.AsMutable<psa_key_id_t>();
+    auto & keyHandle = key.AsMutable<PsaHkdfKeyHandle>();
 
-    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
@@ -19,6 +19,7 @@
 
 #include "CHIPCryptoPALPSA.h"
 
+#include <lib/support/CHIPMem.h>
 #include <lib/support/CodeUtils.h>
 
 #include <psa/crypto.h>
@@ -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)
 {
-    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<psa_key_derivation_operation_t>();
+    Platform::UniquePtr<psa_key_derivation_operation_t> 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<PsaHkdfKeyHandle>();
+    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
@@ -18,6 +18,7 @@
 #pragma once
 
 #include "CHIPCryptoPAL.h"
+#include <crypto/SessionKeystore.h>
 
 #include <psa/crypto.h>
 
@@ -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 HKDF Key handle containing a key reference to the shared secret.
      *
-     * @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);
 
 private:
     psa_pake_operation_t mOperation = PSA_PAKE_OPERATION_INIT;

diff --git 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 (sizeof(psa_key_id_t) + sizeof(bool))
 #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/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
@@ -560,7 +560,7 @@ void CASESession::AbortPendingEstablish(CHIP_ERROR err)
     NotifySessionEstablishmentError(err);
 }
 
-CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session) const
+CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session)
 {
     switch (mState)
     {

diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
@@ -146,7 +146,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
      * @param session     Reference to the secure session that will be initialized once session establishment is complete
      * @return CHIP_ERROR The result of session derivation
      */
-    CHIP_ERROR DeriveSecureSession(CryptoContext & session) const override;
+    CHIP_ERROR DeriveSecureSession(CryptoContext & session) override;
 
     //// UnsolicitedMessageHandler Implementation ////
     CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override

diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp
@@ -253,7 +253,7 @@ void PASESession::OnResponseTimeout(ExchangeContext * ec)
     NotifySessionEstablishmentError(CHIP_ERROR_TIMEOUT);
 }
 
-CHIP_ERROR PASESession::DeriveSecureSession(CryptoContext & session) const
+CHIP_ERROR PASESession::DeriveSecureSession(CryptoContext & session)
 {
     VerifyOrReturnError(mPairingComplete, CHIP_ERROR_INCORRECT_STATE);
 

diff --git a/src/protocols/secure_channel/PASESession.h b/src/protocols/secure_channel/PASESession.h
@@ -141,7 +141,7 @@ class DLL_EXPORT PASESession : public Messaging::UnsolicitedMessageHandler,
      * @param session     Reference to the secure session that will be initialized once pairing is complete
      * @return CHIP_ERROR The result of session derivation
      */
-    CHIP_ERROR DeriveSecureSession(CryptoContext & session) const override;
+    CHIP_ERROR DeriveSecureSession(CryptoContext & session) override;
 
     // TODO: remove Clear, we should create a new instance instead reset the old instance.
     /** @brief This function zeroes out and resets the memory used by the object.

diff --git a/src/protocols/secure_channel/PairingSession.h b/src/protocols/secure_channel/PairingSession.h
@@ -94,7 +94,7 @@ class DLL_EXPORT PairingSession : public SessionDelegate
      * @param session     Reference to the secure session that will be initialized once pairing is complete
      * @return CHIP_ERROR The result of session derivation
      */
-    virtual CHIP_ERROR DeriveSecureSession(CryptoContext & session) const = 0;
+    virtual CHIP_ERROR DeriveSecureSession(CryptoContext & session) = 0;
 
     const ReliableMessageProtocolConfig & GetRemoteMRPConfig() const { return mRemoteMRPConfig; }
     void SetRemoteMRPConfig(const ReliableMessageProtocolConfig & config) { mRemoteMRPConfig = config; }

diff --git 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)
     {