[ICD] Implement storing of persistent keys in PSA for ICD server (#34925

)

* Refactor Key Attributes classes

Implement `keyAttributesBase` and add
persistent lifetime functionality.

* Implement `FindFreeKeySlotInRange` for PSA key slots

* Fix ICD CIP related build dependencies

* Remake `ICDMonitoringTable::Find()` not to overwrite all entry fields

If entry is not found its key handle field must not be filled with
last checked entry when using PSA as it will cause key slot to be
cleared by accident.

* Add ICD CIP and DAC key slots for PSA

* Implement `PersistICDKey` API

* Implement setting key persitence for ICD server

* Address PR comments

Modify implementation of `PersistICDKey` not to depend on type
of ICD key handle (AES, HMAC)

src/app/icd/server/BUILD.gn

@@ -93,7 +93,6 @@ source_set("manager") {
   deps = [ ":icd-server-config" ]
 
   public_deps = [
-    ":check-in-back-off",
     ":configuration-data",
     ":notifier",
     ":observer",
@@ -106,6 +105,7 @@ source_set("manager") {
 
   if (chip_enable_icd_checkin) {
     public_deps += [
+      ":check-in-back-off",
       ":monitoring-table",
       ":sender",
       "${chip_root}/src/app:app_config",

src/app/icd/server/ICDManager.h

@@ -21,7 +21,6 @@
 #include <app/AppConfig.h>
 #include <app/SubscriptionsInfoProvider.h>
 #include <app/TestEventTriggerDelegate.h>
-#include <app/icd/server/ICDCheckInBackOffStrategy.h>
 #include <app/icd/server/ICDConfigurationData.h>
 #include <app/icd/server/ICDNotifier.h>
 #include <app/icd/server/ICDStateObserver.h>
@@ -34,9 +33,10 @@
 #include <system/SystemClock.h>
 
 #if CHIP_CONFIG_ENABLE_ICD_CIP
-#include <app/icd/server/ICDCheckInSender.h>   // nogncheck
-#include <app/icd/server/ICDMonitoringTable.h> // nogncheck
-#endif                                         // CHIP_CONFIG_ENABLE_ICD_CIP
+#include <app/icd/server/ICDCheckInBackOffStrategy.h> // nogncheck
+#include <app/icd/server/ICDCheckInSender.h>          // nogncheck
+#include <app/icd/server/ICDMonitoringTable.h>        // nogncheck
+#endif                                                // CHIP_CONFIG_ENABLE_ICD_CIP
 
 namespace chip {
 namespace Crypto {

src/app/icd/server/ICDMonitoringTable.cpp

@@ -140,7 +140,6 @@ CHIP_ERROR ICDMonitoringEntry::SetKey(ByteSpan keyData)
     Crypto::Symmetric128BitsKeyByteArray keyMaterial;
     memcpy(keyMaterial, keyData.data(), sizeof(Crypto::Symmetric128BitsKeyByteArray));
 
-    // TODO - Add function to set PSA key lifetime
     ReturnErrorOnFailure(symmetricKeystore->CreateKey(keyMaterial, aesKeyHandle));
     CHIP_ERROR error = symmetricKeystore->CreateKey(keyMaterial, hmacKeyHandle);
 
@@ -242,15 +241,22 @@ CHIP_ERROR ICDMonitoringTable::Get(uint16_t index, ICDMonitoringEntry & entry) c
 
 CHIP_ERROR ICDMonitoringTable::Find(NodeId id, ICDMonitoringEntry & entry)
 {
-    uint16_t index = 0;
-    while (index < this->Limit())
+    uint16_t index;
+    ICDMonitoringEntry tempEntry(mSymmetricKeystore);
+
+    for (index = 0; index < this->Limit(); index++)
     {
-        ReturnErrorOnFailure(this->Get(index++, entry));
-        if (id == entry.checkInNodeID)
+        if (this->Get(index, tempEntry) != CHIP_NO_ERROR)
+        {
+            break;
+        }
+        if (id == tempEntry.checkInNodeID)
         {
+            entry = tempEntry;
             return CHIP_NO_ERROR;
         }
     }
+
     entry.index = index;
     return CHIP_ERROR_NOT_FOUND;
 }
@@ -263,16 +269,26 @@ CHIP_ERROR ICDMonitoringTable::Set(uint16_t index, const ICDMonitoringEntry & en
     VerifyOrReturnError(entry.keyHandleValid, CHIP_ERROR_INVALID_ARGUMENT);
 
     ICDMonitoringEntry e(this->mFabric, index);
-    e.checkInNodeID    = entry.checkInNodeID;
-    e.monitoredSubject = entry.monitoredSubject;
-    e.clientType       = entry.clientType;
-    e.index            = index;
+    e.checkInNodeID     = entry.checkInNodeID;
+    e.monitoredSubject  = entry.monitoredSubject;
+    e.clientType        = entry.clientType;
+    e.index             = index;
+    e.symmetricKeystore = entry.symmetricKeystore;
 
     memcpy(e.aesKeyHandle.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(),
            entry.aesKeyHandle.As<Crypto::Symmetric128BitsKeyByteArray>(), sizeof(Crypto::Symmetric128BitsKeyByteArray));
     memcpy(e.hmacKeyHandle.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(),
            entry.hmacKeyHandle.As<Crypto::Symmetric128BitsKeyByteArray>(), sizeof(Crypto::Symmetric128BitsKeyByteArray));
 
+    ReturnErrorOnFailure(e.symmetricKeystore->PersistICDKey(e.aesKeyHandle));
+    CHIP_ERROR error = e.symmetricKeystore->PersistICDKey(e.hmacKeyHandle);
+    if (error != CHIP_NO_ERROR)
+    {
+        // If setting the persistence of the HmacKeyHandle failed, we need to delete the AesKeyHandle to avoid a key leak
+        e.symmetricKeystore->DestroyKey(e.aesKeyHandle);
+        return error;
+    }
+
     return e.Save(this->mStorage);
 }
 

src/app/server/BUILD.gn

@@ -54,7 +54,6 @@ static_library("server") {
   public_deps = [
     "${chip_root}/src/app",
     "${chip_root}/src/app:test-event-trigger",
-    "${chip_root}/src/app/icd/server:check-in-back-off",
     "${chip_root}/src/app/icd/server:icd-server-config",
     "${chip_root}/src/app/icd/server:observer",
     "${chip_root}/src/lib/address_resolve",

src/app/server/Server.h

@@ -72,14 +72,14 @@
 #include <app/reporting/ReportSchedulerImpl.h>
 #include <transport/raw/UDP.h>
 
-#include <app/icd/server/ICDCheckInBackOffStrategy.h>
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
 #include <app/icd/server/ICDManager.h> // nogncheck
 
 #if CHIP_CONFIG_ENABLE_ICD_CIP
 #include <app/icd/server/DefaultICDCheckInBackOffStrategy.h> // nogncheck
-#endif
-#endif
+#include <app/icd/server/ICDCheckInBackOffStrategy.h>        // nogncheck
+#endif                                                       // CHIP_CONFIG_ENABLE_ICD_CIP
+#endif                                                       // CHIP_CONFIG_ENABLE_ICD_SERVER
 
 namespace chip {
 
@@ -183,9 +183,11 @@ struct ServerInitParams
     Credentials::OperationalCertificateStore * opCertStore = nullptr;
     // Required, if not provided, the Server::Init() WILL fail.
     app::reporting::ReportScheduler * reportScheduler = nullptr;
+#if CHIP_CONFIG_ENABLE_ICD_CIP
     // Optional. Support for the ICD Check-In BackOff strategy. Must be initialized before being provided.
     // If the ICD Check-In protocol use-case is supported and no strategy is provided, server will use the default strategy.
     app::ICDCheckInBackOffStrategy * icdCheckInBackOffStrategy = nullptr;
+#endif // CHIP_CONFIG_ENABLE_ICD_CIP
 };
 
 /**

src/crypto/BUILD.gn

@@ -67,6 +67,7 @@ source_set("public_headers") {
 
   public_deps = [
     ":crypto_buildconfig",
+    "${chip_root}/src/app/icd/server:icd-server-config",
     "${chip_root}/src/lib/asn1",
     "${chip_root}/src/lib/core",
     "${chip_root}/src/lib/core:types",

src/crypto/CHIPCryptoPALPSA.cpp

@@ -263,6 +263,28 @@ void Hash_SHA256_stream::Clear()
     psa_hash_abort(toHashOperation(&mContext));
 }
 
+CHIP_ERROR FindFreeKeySlotInRange(psa_key_id_t & keyId, psa_key_id_t start, uint32_t range)
+{
+    psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
+    psa_key_id_t end                = start + range;
+
+    VerifyOrReturnError(start >= PSA_KEY_ID_USER_MIN && end - 1 <= PSA_KEY_ID_USER_MAX, CHIP_ERROR_INVALID_ARGUMENT);
+
+    for (keyId = start; keyId < end; keyId++)
+    {
+        psa_status_t status = psa_get_key_attributes(keyId, &attributes);
+        if (status == PSA_ERROR_INVALID_HANDLE)
+        {
+            return CHIP_NO_ERROR;
+        }
+        else if (status != PSA_SUCCESS)
+        {
+            return CHIP_ERROR_INTERNAL;
+        }
+    }
+    return CHIP_ERROR_NOT_FOUND;
+}
+
 CHIP_ERROR PsaKdf::Init(const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info)
 {
     psa_status_t status        = PSA_SUCCESS;

src/crypto/CHIPCryptoPALPSA.h

@@ -55,19 +55,58 @@ namespace Crypto {
 #define CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE 0x30000
 #endif // CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE
 
+/**
+ * @def CHIP_CONFIG_CRYPTO_PSA_KEY_ID_END
+ *
+ * @brief
+ *   End of the PSA key identifier range used by Matter.
+ *
+ * This setting establishes the maximum limit for the key range specific to Matter, in order to
+ * prevent any overlap with other firmware components that also employ the PSA crypto API.
+ */
+#ifndef CHIP_CONFIG_CRYPTO_PSA_KEY_ID_END
+#define CHIP_CONFIG_CRYPTO_PSA_KEY_ID_END 0x3FFFF
+#endif // CHIP_CONFIG_CRYPTO_PSA_KEY_ID_END
+
+static_assert(PSA_KEY_ID_USER_MIN <= CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE && CHIP_CONFIG_CRYPTO_PSA_KEY_ID_END <= PSA_KEY_ID_USER_MAX,
+              "Matter specific PSA key range doesn't fit within PSA allowed range");
+
+// Each ICD client requires storing two keys- AES and HMAC
+static constexpr uint32_t kMaxICDClientKeys = 2 * CHIP_CONFIG_CRYPTO_PSA_ICD_MAX_CLIENTS;
+
+static_assert(kMaxICDClientKeys >= CHIP_CONFIG_ICD_CLIENTS_SUPPORTED_PER_FABRIC * CHIP_CONFIG_MAX_FABRICS,
+              "Number of allocated ICD key slots is lower than maximum number of supported ICD clients");
+
 /**
  * @brief Defines subranges of the PSA key identifier space used by Matter.
  */
 enum class KeyIdBase : psa_key_id_t
 {
-    Minimum     = CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE,
-    Operational = Minimum, ///< Base of the PSA key ID range for Node Operational Certificate private keys
-    Maximum     = Operational + kMaxValidFabricIndex,
+    Minimum          = CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE,
+    Operational      = Minimum, ///< Base of the PSA key ID range for Node Operational Certificate private keys
+    DACPrivKey       = Operational + kMaxValidFabricIndex + 1,
+    ICDKeyRangeStart = DACPrivKey + 1,
+    Maximum          = ICDKeyRangeStart + kMaxICDClientKeys,
 };
 
-static_assert(to_underlying(KeyIdBase::Minimum) >= PSA_KEY_ID_USER_MIN && to_underlying(KeyIdBase::Maximum) <= PSA_KEY_ID_USER_MAX,
+static_assert(to_underlying(KeyIdBase::Minimum) >= CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE &&
+                  to_underlying(KeyIdBase::Maximum) <= CHIP_CONFIG_CRYPTO_PSA_KEY_ID_END,
               "PSA key ID base out of allowed range");
 
+/**
+ * @brief Finds first free persistent Key slot ID within range.
+ *
+ * @param[out] keyId Key ID handler to which free ID will be set.
+ * @param[in]  start Starting ID in search range.
+ * @param[in]  range Search range.
+ *
+ * @retval CHIP_NO_ERROR               On success.
+ * @retval CHIP_ERROR_INTERNAL         On PSA crypto API error.
+ * @retval CHIP_ERROR_NOT_FOUND        On no free Key ID within range.
+ * @retval CHIP_ERROR_INVALID_ARGUMENT On search arguments out of PSA allowed range.
+ */
+CHIP_ERROR FindFreeKeySlotInRange(psa_key_id_t & keyId, psa_key_id_t start, uint32_t range);
+
 /**
  * @brief Calculates PSA key ID for Node Operational Certificate private key for the given fabric.
  */

src/crypto/PSASessionKeystore.cpp

@@ -24,70 +24,62 @@ namespace Crypto {
 
 namespace {
 
-class AesKeyAttributes
+class KeyAttributesBase
 {
 public:
-    AesKeyAttributes()
+    KeyAttributesBase(psa_key_type_t type, psa_algorithm_t algorithm, psa_key_usage_t usageFlags, size_t bits)
     {
-        constexpr psa_algorithm_t kAlgorithm = PSA_ALG_AEAD_WITH_AT_LEAST_THIS_LENGTH_TAG(PSA_ALG_CCM, 8);
-
-        psa_set_key_type(&mAttrs, PSA_KEY_TYPE_AES);
-        psa_set_key_algorithm(&mAttrs, kAlgorithm);
-        psa_set_key_usage_flags(&mAttrs, PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT);
-        psa_set_key_bits(&mAttrs, CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES * 8);
+        psa_set_key_type(&mAttrs, type);
+        psa_set_key_algorithm(&mAttrs, algorithm);
+        psa_set_key_usage_flags(&mAttrs, usageFlags);
+        psa_set_key_bits(&mAttrs, bits);
     }
 
-    ~AesKeyAttributes() { psa_reset_key_attributes(&mAttrs); }
+    ~KeyAttributesBase() { psa_reset_key_attributes(&mAttrs); }
 
     const psa_key_attributes_t & Get() { return mAttrs; }
 
 private:
     psa_key_attributes_t mAttrs = PSA_KEY_ATTRIBUTES_INIT;
 };
 
-class HmacKeyAttributes
+class AesKeyAttributes : public KeyAttributesBase
 {
 public:
-    HmacKeyAttributes()
-    {
-        psa_set_key_type(&mAttrs, PSA_KEY_TYPE_HMAC);
-        psa_set_key_algorithm(&mAttrs, PSA_ALG_HMAC(PSA_ALG_SHA_256));
-        psa_set_key_usage_flags(&mAttrs, PSA_KEY_USAGE_SIGN_MESSAGE);
-        psa_set_key_bits(&mAttrs, CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES * 8);
-    }
-
-    ~HmacKeyAttributes() { psa_reset_key_attributes(&mAttrs); }
-
-    const psa_key_attributes_t & Get() { return mAttrs; }
-
-private:
-    psa_key_attributes_t mAttrs = PSA_KEY_ATTRIBUTES_INIT;
+    AesKeyAttributes() :
+        KeyAttributesBase(PSA_KEY_TYPE_AES, PSA_ALG_AEAD_WITH_AT_LEAST_THIS_LENGTH_TAG(PSA_ALG_CCM, 8),
+                          PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT | PSA_KEY_USAGE_COPY,
+                          CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES * 8)
+    {}
 };
 
-class HkdfKeyAttributes
+class HmacKeyAttributes : public KeyAttributesBase
 {
 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; }
+    HmacKeyAttributes() :
+        KeyAttributesBase(PSA_KEY_TYPE_HMAC, PSA_ALG_HMAC(PSA_ALG_SHA_256), PSA_KEY_USAGE_SIGN_MESSAGE | PSA_KEY_USAGE_COPY,
+                          CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES * 8)
+    {}
+};
 
-private:
-    psa_key_attributes_t mAttrs = PSA_KEY_ATTRIBUTES_INIT;
+class HkdfKeyAttributes : public KeyAttributesBase
+{
+public:
+    HkdfKeyAttributes() : KeyAttributesBase(PSA_KEY_TYPE_DERIVE, PSA_ALG_HKDF(PSA_ALG_SHA_256), PSA_KEY_USAGE_DERIVE, 0) {}
 };
 
+void SetKeyId(Symmetric128BitsKeyHandle & key, psa_key_id_t newKeyId)
+{
+    auto & KeyId = key.AsMutable<psa_key_id_t>();
+
+    KeyId = newKeyId;
+}
 } // namespace
 
 CHIP_ERROR PSASessionKeystore::CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Aes128KeyHandle & key)
 {
     // Destroy the old key if already allocated
-    psa_destroy_key(key.As<psa_key_id_t>());
+    DestroyKey(key);
 
     AesKeyAttributes attrs;
     psa_status_t status =
@@ -101,7 +93,7 @@ CHIP_ERROR PSASessionKeystore::CreateKey(const Symmetric128BitsKeyByteArray & ke
 CHIP_ERROR PSASessionKeystore::CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hmac128KeyHandle & key)
 {
     // Destroy the old key if already allocated
-    psa_destroy_key(key.As<psa_key_id_t>());
+    DestroyKey(key);
 
     HmacKeyAttributes attrs;
     psa_status_t status =
@@ -192,5 +184,39 @@ void PSASessionKeystore::DestroyKey(HkdfKeyHandle & key)
     keyId = PSA_KEY_ID_NULL;
 }
 
+#if CHIP_CONFIG_ENABLE_ICD_CIP
+CHIP_ERROR PSASessionKeystore::PersistICDKey(Symmetric128BitsKeyHandle & key)
+{
+    CHIP_ERROR err;
+    psa_key_id_t newKeyId = PSA_KEY_ID_NULL;
+    psa_key_attributes_t attrs;
+
+    psa_get_key_attributes(key.As<psa_key_id_t>(), &attrs);
+
+    // Exit early if key is already persistent
+    if (psa_get_key_lifetime(&attrs) == PSA_KEY_LIFETIME_PERSISTENT)
+    {
+        psa_reset_key_attributes(&attrs);
+        return CHIP_NO_ERROR;
+    }
+
+    SuccessOrExit(err = Crypto::FindFreeKeySlotInRange(newKeyId, to_underlying(KeyIdBase::ICDKeyRangeStart), kMaxICDClientKeys));
+    psa_set_key_lifetime(&attrs, PSA_KEY_LIFETIME_PERSISTENT);
+    psa_set_key_id(&attrs, newKeyId);
+    VerifyOrExit(psa_copy_key(key.As<psa_key_id_t>(), &attrs, &newKeyId) == PSA_SUCCESS, err = CHIP_ERROR_INTERNAL);
+
+exit:
+    DestroyKey(key);
+    psa_reset_key_attributes(&attrs);
+
+    if (err == CHIP_NO_ERROR)
+    {
+        SetKeyId(key, newKeyId);
+    }
+
+    return err;
+}
+#endif
+
 } // namespace Crypto
 } // namespace chip