[ICD] Update KeyStore for Check-in keys (#29260)

* Update KeyStore for Check-in keys

* fix comments

* fix comments

examples/all-clusters-app/linux/BUILD.gn

@@ -48,6 +48,9 @@ source_set("chip-all-clusters-common") {
   deps = [
     "${chip_root}/examples/all-clusters-app/all-clusters-common",
     "${chip_root}/examples/platform/linux:app-main",
+
+    # Issue 29397 for the icd:cluster dep
+    "${chip_root}/src/app/icd:cluster",
     "${chip_root}/src/app/tests/suites/credentials:dac_provider",
     "${chip_root}/src/lib",
     "${chip_root}/third_party/jsoncpp",

examples/all-clusters-app/linux/main-common.cpp

@@ -46,6 +46,8 @@
 #include <transport/SessionManager.h>
 #include <transport/raw/PeerAddress.h>
 
+#include <app/icd/ICDManagementServer.h>
+
 #include <Options.h>
 
 using namespace chip;
@@ -215,6 +217,11 @@ void ApplicationInit()
 #endif
     Clusters::TemperatureControl::SetInstance(&sAppSupportedTemperatureLevelsDelegate);
 
+    // Issue 29397
+    // Somehow All-cluster-app test the ICDManagementServer cluster without having
+    // CHIP_CONFIG_ENABLE_ICD_SERVER set to 1.
+    ICDManagementServer::GetInstance().SetSymmetricKeystore(Server::GetInstance().GetSessionKeystore());
+
     SetTagList(/* endpoint= */ 0, Span<const Clusters::Descriptor::Structs::SemanticTagStruct::Type>(gEp0TagList));
     SetTagList(/* endpoint= */ 1, Span<const Clusters::Descriptor::Structs::SemanticTagStruct::Type>(gEp1TagList));
     SetTagList(/* endpoint= */ 2, Span<const Clusters::Descriptor::Structs::SemanticTagStruct::Type>(gEp2TagList));

src/app/chip_data_model.gni

@@ -288,7 +288,6 @@ template("chip_data_model") {
         ]
       } else if (cluster == "icd-management-server") {
         sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ]
-
         deps += [ "${chip_root}/src/app/icd:cluster" ]
       } else if (cluster == "resource-monitoring-server") {
         sources += [

src/app/clusters/icd-management-server/icd-management-server.cpp

@@ -107,13 +107,14 @@ CHIP_ERROR IcdManagementAttributeAccess::ReadRegisteredClients(EndpointId endpoi
     uint16_t supported_clients = ICDManagementServer::GetInstance().GetClientsSupportedPerFabric();
 
     return encoder.EncodeList([supported_clients](const auto & subEncoder) -> CHIP_ERROR {
-        ICDMonitoringEntry e;
+        Crypto::SessionKeystore * keyStore = Server::GetInstance().GetSessionKeystore();
+        ICDMonitoringEntry e(keyStore);
 
-        const auto & fabricTable = Server::GetInstance().GetFabricTable();
+        const auto & fabricTable            = Server::GetInstance().GetFabricTable();
+        PersistentStorageDelegate & storage = Server::GetInstance().GetPersistentStorage();
         for (const auto & fabricInfo : fabricTable)
         {
-            PersistentStorageDelegate & storage = chip::Server::GetInstance().GetPersistentStorage();
-            ICDMonitoringTable table(storage, fabricInfo.GetFabricIndex(), supported_clients);
+            ICDMonitoringTable table(storage, fabricInfo.GetFabricIndex(), supported_clients, keyStore);
             for (uint16_t i = 0; i < table.Limit(); ++i)
             {
                 CHIP_ERROR err = table.Get(i, e);
@@ -165,7 +166,8 @@ class IcdManagementFabricDelegate : public chip::FabricTable::Delegate
     void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override
     {
         uint16_t supported_clients = ICDManagementServer::GetInstance().GetClientsSupportedPerFabric();
-        ICDMonitoringTable table(chip::Server::GetInstance().GetPersistentStorage(), fabricIndex, supported_clients);
+        ICDMonitoringTable table(Server::GetInstance().GetPersistentStorage(), fabricIndex, supported_clients,
+                                 Server::GetInstance().GetSessionKeystore());
         table.RemoveAll();
     }
 };

src/app/icd/ICDManagementServer.cpp

@@ -1,5 +1,4 @@
 #include "ICDManagementServer.h"
-#include <app/icd/ICDMonitoringTable.h>
 
 using namespace chip;
 using namespace chip::Protocols;
@@ -12,18 +11,18 @@ Status ICDManagementServer::RegisterClient(PersistentStorageDelegate & storage,
                                            uint64_t monitored_subject, chip::ByteSpan key,
                                            Optional<chip::ByteSpan> verification_key, bool is_admin)
 {
-    ICDMonitoringTable table(storage, fabric_index, GetClientsSupportedPerFabric());
+    ICDMonitoringTable table(storage, fabric_index, GetClientsSupportedPerFabric(), mSymmetricKeystore);
 
     // Get current entry, if exists
-    ICDMonitoringEntry entry;
+    ICDMonitoringEntry entry(mSymmetricKeystore);
     CHIP_ERROR err = table.Find(node_id, entry);
     if (CHIP_NO_ERROR == err)
     {
         // Existing entry: Validate Key if, and only if, the ISD has NOT administrator permissions
         if (!is_admin)
         {
             VerifyOrReturnError(verification_key.HasValue(), InteractionModel::Status::Failure);
-            VerifyOrReturnError(verification_key.Value().data_equal(entry.key), InteractionModel::Status::Failure);
+            VerifyOrReturnError(entry.IsKeyEquivalent(verification_key.Value()), InteractionModel::Status::Failure);
         }
     }
     else if (CHIP_ERROR_NOT_FOUND == err)
@@ -40,8 +39,17 @@ Status ICDManagementServer::RegisterClient(PersistentStorageDelegate & storage,
     // Save
     entry.checkInNodeID    = node_id;
     entry.monitoredSubject = monitored_subject;
-    entry.key              = key;
-    err                    = table.Set(entry.index, entry);
+    err                    = entry.SetKey(key);
+    VerifyOrReturnError(CHIP_ERROR_INVALID_ARGUMENT != err, InteractionModel::Status::ConstraintError);
+    VerifyOrReturnError(CHIP_NO_ERROR == err, InteractionModel::Status::Failure);
+    err = table.Set(entry.index, entry);
+
+    // Delete key upon failure to prevent key storage leakage.
+    if (err != CHIP_NO_ERROR)
+    {
+        entry.DeleteKey();
+    }
+
     VerifyOrReturnError(CHIP_ERROR_INVALID_ARGUMENT != err, InteractionModel::Status::ConstraintError);
     VerifyOrReturnError(CHIP_NO_ERROR == err, InteractionModel::Status::Failure);
 
@@ -51,10 +59,10 @@ Status ICDManagementServer::RegisterClient(PersistentStorageDelegate & storage,
 Status ICDManagementServer::UnregisterClient(PersistentStorageDelegate & storage, FabricIndex fabric_index, chip::NodeId node_id,
                                              Optional<chip::ByteSpan> verificationKey, bool is_admin)
 {
-    ICDMonitoringTable table(storage, fabric_index, GetClientsSupportedPerFabric());
+    ICDMonitoringTable table(storage, fabric_index, GetClientsSupportedPerFabric(), mSymmetricKeystore);
 
     // Get current entry, if exists
-    ICDMonitoringEntry entry;
+    ICDMonitoringEntry entry(mSymmetricKeystore);
     CHIP_ERROR err = table.Find(node_id, entry);
     VerifyOrReturnError(CHIP_ERROR_NOT_FOUND != err, InteractionModel::Status::NotFound);
     VerifyOrReturnError(CHIP_NO_ERROR == err, InteractionModel::Status::Failure);
@@ -63,7 +71,7 @@ Status ICDManagementServer::UnregisterClient(PersistentStorageDelegate & storage
     if (!is_admin)
     {
         VerifyOrReturnError(verificationKey.HasValue(), InteractionModel::Status::Failure);
-        VerifyOrReturnError(verificationKey.Value().data_equal(entry.key), InteractionModel::Status::Failure);
+        VerifyOrReturnError(entry.IsKeyEquivalent(verificationKey.Value()), InteractionModel::Status::Failure);
     }
 
     err = table.Remove(entry.index);

src/app/icd/ICDManagementServer.h

@@ -18,11 +18,14 @@
 #pragma once
 
 #include <app/util/basic-types.h>
+#include <crypto/SessionKeystore.h>
 #include <lib/core/CHIPPersistentStorageDelegate.h>
 #include <lib/core/Optional.h>
 #include <lib/support/Span.h>
 #include <protocols/interaction_model/StatusCode.h>
 
+#include <app/icd/ICDMonitoringTable.h>
+
 namespace chip {
 
 using chip::Protocols::InteractionModel::Status;
@@ -34,6 +37,8 @@ class ICDManagementServer
 
     uint32_t GetActiveModeIntervalMs() { return mActiveInterval_ms; }
 
+    void SetSymmetricKeystore(Crypto::SymmetricKeystore * keyStore) { mSymmetricKeystore = keyStore; }
+
     uint16_t GetActiveModeThresholdMs() { return mActiveThreshold_ms; }
 
     uint32_t GetICDCounter() { return mICDCounter; }
@@ -56,6 +61,7 @@ class ICDManagementServer
     ICDManagementServer() = default;
 
     static ICDManagementServer mInstance;
+    Crypto::SymmetricKeystore * mSymmetricKeystore = nullptr;
 
     static_assert((CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL_SEC) <= 64800,
                   "Spec requires the IdleModeInterval to be equal or inferior to 64800s.");

src/app/icd/ICDManager.cpp

@@ -44,18 +44,24 @@ uint8_t ICDManager::OpenExchangeContextCount = 0;
 static_assert(UINT8_MAX >= CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS,
               "ICDManager::OpenExchangeContextCount cannot hold count for the max exchange count");
 
-void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, ICDStateObserver * stateObserver)
+void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, ICDStateObserver * stateObserver,
+                      Crypto::SymmetricKeystore * symmetricKeystore)
 {
     VerifyOrDie(storage != nullptr);
     VerifyOrDie(fabricTable != nullptr);
     VerifyOrDie(stateObserver != nullptr);
+    VerifyOrDie(symmetricKeystore != nullptr);
 
     mStorage       = storage;
     mFabricTable   = fabricTable;
     mStateObserver = stateObserver;
     VerifyOrDie(ICDNotifier::GetInstance().Subscribe(this) == CHIP_NO_ERROR);
+    mSymmetricKeystore = symmetricKeystore;
 
     uint32_t activeModeInterval = ICDManagementServer::GetInstance().GetActiveModeIntervalMs();
+
+    ICDManagementServer::GetInstance().SetSymmetricKeystore(mSymmetricKeystore);
+
     VerifyOrDie(kFastPollingInterval.count() < activeModeInterval);
 
     UpdateICDMode();
@@ -102,7 +108,7 @@ void ICDManager::UpdateICDMode()
         for (const auto & fabricInfo : *mFabricTable)
         {
             // We only need 1 valid entry to ensure LIT compliance
-            ICDMonitoringTable table(*mStorage, fabricInfo.GetFabricIndex(), 1 /*Table entry limit*/);
+            ICDMonitoringTable table(*mStorage, fabricInfo.GetFabricIndex(), 1 /*Table entry limit*/, mSymmetricKeystore);
             if (!table.IsEmpty())
             {
                 tempMode = ICDMode::LIT;

src/app/icd/ICDManager.h

@@ -16,6 +16,7 @@
  */
 #pragma once
 
+#include <app/icd/ICDMonitoringTable.h>
 #include <app/icd/ICDNotifier.h>
 #include <app/icd/ICDStateObserver.h>
 #include <credentials/FabricTable.h>
@@ -50,7 +51,8 @@ class ICDManager : public ICDListener
     };
 
     ICDManager() {}
-    void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, ICDStateObserver * stateObserver);
+    void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, ICDStateObserver * stateObserver,
+              Crypto::SymmetricKeystore * symmetricKeyStore);
     void Shutdown();
     void UpdateICDMode();
     void UpdateOperationState(OperationalState state);
@@ -99,12 +101,13 @@ class ICDManager : public ICDListener
 
     BitFlags<KeepActiveFlags> mKeepActiveFlags{ 0 };
 
-    OperationalState mOperationalState   = OperationalState::IdleMode;
-    ICDMode mICDMode                     = ICDMode::SIT;
-    PersistentStorageDelegate * mStorage = nullptr;
-    FabricTable * mFabricTable           = nullptr;
-    ICDStateObserver * mStateObserver    = nullptr;
-    bool mTransitionToIdleCalled         = false;
+    OperationalState mOperationalState             = OperationalState::IdleMode;
+    ICDMode mICDMode                               = ICDMode::SIT;
+    PersistentStorageDelegate * mStorage           = nullptr;
+    FabricTable * mFabricTable                     = nullptr;
+    ICDStateObserver * mStateObserver              = nullptr;
+    bool mTransitionToIdleCalled                   = false;
+    Crypto::SymmetricKeystore * mSymmetricKeystore = nullptr;
 };
 
 } // namespace app