fix comments

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

@@ -218,9 +218,9 @@ void ApplicationInit()
     Clusters::TemperatureControl::SetInstance(&sAppSupportedTemperatureLevelsDelegate);
 
     // Issue 29397
-    // Somehow All-cluster-app test the ICDManagementServer cluster without having the
+    // Somehow All-cluster-app test the ICDManagementServer cluster without having
     // CHIP_CONFIG_ENABLE_ICD_SERVER set to 1.
-    ICDManagementServer::GetInstance().SetSessionKeyStore(Server::GetInstance().GetSessionKeystore());
+    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));

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(), mSessionKeyStore);
+    ICDMonitoringTable table(storage, fabric_index, GetClientsSupportedPerFabric(), mSymmetricKeystore);
 
     // Get current entry, if exists
-    ICDMonitoringEntry entry(mSessionKeyStore);
+    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(entry.EnsureKeyEquivalent(verification_key.Value()), InteractionModel::Status::Failure);
+            VerifyOrReturnError(entry.IsKeyEquivalent(verification_key.Value()), InteractionModel::Status::Failure);
         }
     }
     else if (CHIP_ERROR_NOT_FOUND == err)
@@ -60,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(), mSessionKeyStore);
+    ICDMonitoringTable table(storage, fabric_index, GetClientsSupportedPerFabric(), mSymmetricKeystore);
 
     // Get current entry, if exists
-    ICDMonitoringEntry entry(mSessionKeyStore);
+    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);
@@ -72,7 +71,7 @@ Status ICDManagementServer::UnregisterClient(PersistentStorageDelegate & storage
     if (!is_admin)
     {
         VerifyOrReturnError(verificationKey.HasValue(), InteractionModel::Status::Failure);
-        VerifyOrReturnError(entry.EnsureKeyEquivalent(verificationKey.Value()), InteractionModel::Status::Failure);
+        VerifyOrReturnError(entry.IsKeyEquivalent(verificationKey.Value()), InteractionModel::Status::Failure);
     }
 
     err = table.Remove(entry.index);

src/app/icd/ICDManagementServer.h

@@ -24,6 +24,8 @@
 #include <lib/support/Span.h>
 #include <protocols/interaction_model/StatusCode.h>
 
+#include <app/icd/ICDMonitoringTable.h>
+
 namespace chip {
 
 using chip::Protocols::InteractionModel::Status;
@@ -35,7 +37,7 @@ class ICDManagementServer
 
     uint32_t GetActiveModeIntervalMs() { return mActiveInterval_ms; }
 
-    void SetSessionKeyStore(Crypto::SessionKeystore * keyStore) { mSessionKeyStore = keyStore; }
+    void SetSymmetricKeystore(Crypto::SymmetricKeystore * keyStore) { mSymmetricKeystore = keyStore; }
 
     uint16_t GetActiveModeThresholdMs() { return mActiveThreshold_ms; }
 
@@ -59,7 +61,7 @@ class ICDManagementServer
     ICDManagementServer() = default;
 
     static ICDManagementServer mInstance;
-    Crypto::SessionKeystore * mSessionKeyStore = nullptr;
+    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

@@ -45,22 +45,22 @@ 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,
-                      Crypto::SessionKeystore * sessionKeyStore)
+                      Crypto::SymmetricKeystore * symmetricKeystore)
 {
     VerifyOrDie(storage != nullptr);
     VerifyOrDie(fabricTable != nullptr);
     VerifyOrDie(stateObserver != nullptr);
-    VerifyOrDie(sessionKeyStore != nullptr);
+    VerifyOrDie(symmetricKeystore != nullptr);
 
     mStorage       = storage;
     mFabricTable   = fabricTable;
     mStateObserver = stateObserver;
     VerifyOrDie(ICDNotifier::GetInstance().Subscribe(this) == CHIP_NO_ERROR);
-    mSessionKeyStore = sessionKeyStore;
+    mSymmetricKeystore = symmetricKeystore;
 
     uint32_t activeModeInterval = ICDManagementServer::GetInstance().GetActiveModeIntervalMs();
 
-    ICDManagementServer::GetInstance().SetSessionKeyStore(mSessionKeyStore);
+    ICDManagementServer::GetInstance().SetSymmetricKeystore(mSymmetricKeystore);
 
     VerifyOrDie(kFastPollingInterval.count() < activeModeInterval);
 
@@ -108,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*/, mSessionKeyStore);
+            ICDMonitoringTable table(*mStorage, fabricInfo.GetFabricIndex(), 1 /*Table entry limit*/, mSymmetricKeystore);
             if (!table.IsEmpty())
             {
                 tempMode = ICDMode::LIT;

src/app/icd/ICDManager.h

@@ -16,10 +16,10 @@
  */
 #pragma once
 
+#include <app/icd/ICDMonitoringTable.h>
 #include <app/icd/ICDNotifier.h>
 #include <app/icd/ICDStateObserver.h>
 #include <credentials/FabricTable.h>
-#include <crypto/SessionKeystore.h>
 #include <lib/support/BitFlags.h>
 #include <platform/CHIPDeviceConfig.h>
 #include <platform/internal/CHIPDeviceLayerInternal.h>
@@ -52,7 +52,7 @@ class ICDManager : public ICDListener
 
     ICDManager() {}
     void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, ICDStateObserver * stateObserver,
-              Crypto::SessionKeystore * sessionKeyStore);
+              Crypto::SymmetricKeystore * symmetricKeyStore);
     void Shutdown();
     void UpdateICDMode();
     void UpdateOperationState(OperationalState state);
@@ -101,13 +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;
-    Crypto::SessionKeystore * mSessionKeyStore = nullptr;
+    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

src/app/icd/ICDMonitoringTable.cpp

@@ -119,7 +119,7 @@ CHIP_ERROR ICDMonitoringEntry::DeleteKey()
     return CHIP_NO_ERROR;
 }
 
-bool ICDMonitoringEntry::EnsureKeyEquivalent(ByteSpan keyData)
+bool ICDMonitoringEntry::IsKeyEquivalent(ByteSpan keyData)
 {
     VerifyOrReturnValue(keyData.size() == Crypto::CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES, false);
     VerifyOrReturnValue(symmetricKeystore != nullptr, false);
@@ -215,6 +215,8 @@ CHIP_ERROR ICDMonitoringTable::Remove(uint16_t index)
     // Remove last entry
     entry.fabricIndex = this->mFabric;
     entry.index       = index;
+
+    // entry.Delete() doesn't delete the key from the AES128KeyHandle
     return entry.Delete(this->mStorage);
 }
 

src/app/icd/ICDMonitoringTable.h

@@ -64,20 +64,20 @@ struct ICDMonitoringEntry : public PersistentData<kICDMonitoringBufferSize>
 
     /**
      * @brief Implement the key verification needed by the ICDManagement Server.
-     *        Since we cannot retrieve the key from the AES128KeyHandle IF using
-     *        PSA CRYPTO, we must implement a way to deduct if the verification key
+     *        Since for some key implementations we cannot retrieve the key from the AES128KeyHandle
+     *        we must implement a way to deduce whether the verification key
      *        received is the same or at least works as the same way as the one stored.
      *
      *        This method will produce a random number and then encrypt it with the keyData.
      *        It will then decrypt it with the key stored in the entry. If the resulting decrypted
-     *        challenges matches the randomly generated number, then we can safely assume that both key are interchangeable.
+     *        challenge matches the randomly generated number, then we can safely assume that both key are interchangeable.
      *        This method cannot guarantee a perfect match since the probability of two keys generating the same output in AES128 is
-     * not 0 but 1/2^128 which is small enough for our usage.
+     *        not 0 but 1/2^128 which is small enough for our purposes.
      *
      * @param keyData
      * @return bool True if the key is equivalent to the one stored, otherwise false
      */
-    bool EnsureKeyEquivalent(ByteSpan keyData);
+    bool IsKeyEquivalent(ByteSpan keyData);
 
     chip::FabricIndex fabricIndex                 = kUndefinedFabricIndex;
     chip::NodeId checkInNodeID                    = kUndefinedNodeId;

src/app/tests/TestICDMonitoringTable.cpp

@@ -75,9 +75,9 @@ void TestEntryKeyFunctions(nlTestSuite * aSuite, void * aContext)
     NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == entry.SetKey(ByteSpan(kKeyBuffer1b)));
 
     // Test Comparing Key
-    NL_TEST_ASSERT(aSuite, !entry.EnsureKeyEquivalent(ByteSpan(kKeyBuffer1a)));
+    NL_TEST_ASSERT(aSuite, !entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1a)));
 
-    NL_TEST_ASSERT(aSuite, entry.EnsureKeyEquivalent(ByteSpan(kKeyBuffer1b)));
+    NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1b)));
 
     // Test Deleting Key
     NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == entry.DeleteKey());