[ICD] Update KeyStore for Check-in keys

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