[Group] Fabric removal sync (#15874)

* Fabric removal sync
project-chip · May 26, 2022 · 2510290 · 2510290
1 parent 76898b1
commit 2510290
Show file tree

Hide file tree

Showing 12 changed files with 187 additions and 16 deletions.
diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
@@ -184,6 +184,10 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
     err = mSessions.Init(&DeviceLayer::SystemLayer(), &mTransports, &mMessageCounterManager, &mDeviceStorage, &GetFabricTable());
     SuccessOrExit(err);
 
+    err = mFabricDelegate.Init(&mSessions);
+    SuccessOrExit(err);
+    mFabrics.AddFabricDelegate(&mFabricDelegate);
+
     err = mExchangeMgr.Init(&mSessions);
     SuccessOrExit(err);
     err = mMessageCounterManager.Init(&mExchangeMgr);

diff --git a/src/app/server/Server.h b/src/app/server/Server.h
@@ -170,6 +170,40 @@ class Server
         ServerTransportMgr * mTransports;
     };
 
+    class ServerFabricDelegate final : public FabricTableDelegate
+    {
+    public:
+        ServerFabricDelegate() {}
+
+        CHIP_ERROR Init(SessionManager * sessionManager)
+        {
+            VerifyOrReturnError(sessionManager != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+
+            mSessionManager = sessionManager;
+            return CHIP_NO_ERROR;
+        };
+
+        void OnFabricDeletedFromStorage(CompressedFabricId compressedId, FabricIndex fabricIndex) override
+        {
+            (void) compressedId;
+            if (mSessionManager != nullptr)
+            {
+                mSessionManager->FabricRemoved(fabricIndex);
+            }
+            Credentials::GroupDataProvider * groupDataProvider = Credentials::GetGroupDataProvider();
+            if (groupDataProvider != nullptr)
+            {
+                groupDataProvider->RemoveFabric(fabricIndex);
+            }
+        };
+        void OnFabricRetrievedFromStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; }
+
+        void OnFabricPersistedToStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; }
+
+    private:
+        SessionManager * mSessionManager = nullptr;
+    };
+
 #if CONFIG_NETWORK_LAYER_BLE
     Ble::BleLayer * mBleLayer = nullptr;
 #endif
@@ -198,6 +232,7 @@ class Server
     Credentials::GroupDataProviderImpl mGroupsProvider;
     app::DefaultAttributePersistenceProvider mAttributePersister;
     GroupDataProviderListener mListener;
+    ServerFabricDelegate mFabricDelegate;
 
     Access::AccessControl mAccessControl;
 

diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -146,6 +146,10 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
 
     ReturnErrorOnFailure(stateParams.fabricTable->Init(params.fabricIndependentStorage));
 
+    auto delegate = chip::Platform::MakeUnique<ControllerFabricDelegate>(stateParams.sessionMgr);
+    ReturnErrorOnFailure(stateParams.fabricTable->AddFabricDelegate(delegate.get()));
+    delegate.release();
+
     ReturnErrorOnFailure(stateParams.sessionMgr->Init(stateParams.systemLayer, stateParams.transportMgr,
                                                       stateParams.messageCounterManager, params.fabricIndependentStorage,
                                                       stateParams.fabricTable));

diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h
@@ -31,6 +31,7 @@
 
 #include <controller/CHIPDeviceController.h>
 #include <controller/CHIPDeviceControllerSystemState.h>
+#include <credentials/GroupDataProviderImpl.h>
 #include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
 
 namespace chip {
@@ -137,6 +138,41 @@ class DeviceControllerFactory
     //
     void ReleaseSystemState() { mSystemState->Release(); }
 
+    class ControllerFabricDelegate final : public FabricTableDelegate
+    {
+    public:
+        ControllerFabricDelegate() {}
+        ControllerFabricDelegate(SessionManager * sessionManager) : FabricTableDelegate(true), mSessionManager(sessionManager) {}
+
+        CHIP_ERROR Init(SessionManager * sessionManager)
+        {
+            VerifyOrReturnError(sessionManager != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+
+            mSessionManager = sessionManager;
+            return CHIP_NO_ERROR;
+        };
+
+        void OnFabricDeletedFromStorage(CompressedFabricId compressedId, FabricIndex fabricIndex) override
+        {
+            if (mSessionManager != nullptr)
+            {
+                mSessionManager->FabricRemoved(fabricIndex);
+            }
+            Credentials::GroupDataProvider * groupDataProvider = Credentials::GetGroupDataProvider();
+            if (groupDataProvider != nullptr)
+            {
+                groupDataProvider->RemoveFabric(fabricIndex);
+            }
+        };
+
+        void OnFabricRetrievedFromStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; }
+
+        void OnFabricPersistedToStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; }
+
+    private:
+        SessionManager * mSessionManager = nullptr;
+    };
+
 private:
     DeviceControllerFactory(){};
     void PopulateInitParams(ControllerInitParams & controllerParams, const SetupParams & params);

diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
@@ -401,6 +401,20 @@ CHIP_ERROR FabricInfo::MatchDestinationID(const ByteSpan & targetDestinationId,
     return CHIP_ERROR_CERT_NOT_TRUSTED;
 }
 
+FabricTable::~FabricTable()
+{
+    FabricTableDelegate * delegate = mDelegate;
+    while (delegate)
+    {
+        FabricTableDelegate * temp = delegate->mNext;
+        if (delegate->mOwnedByFabricTable)
+        {
+            chip::Platform::Delete(delegate);
+        }
+        delegate = temp;
+    }
+}
+
 void FabricTable::ReleaseFabricIndex(FabricIndex fabricIndex)
 {
     FabricInfo * fabric = FindFabricWithIndex(fabricIndex);
@@ -629,6 +643,7 @@ CHIP_ERROR FabricTable::Delete(FabricIndex index)
             mFabricCount--;
         }
         ChipLogProgress(Discovery, "Fabric (%d) deleted. Calling OnFabricDeletedFromStorage", index);
+
         FabricTableDelegate * delegate = mDelegate;
         while (delegate)
         {
@@ -651,6 +666,7 @@ void FabricTable::DeleteAllFabrics()
 CHIP_ERROR FabricTable::Init(PersistentStorageDelegate * storage)
 {
     VerifyOrReturnError(storage != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+
     mStorage = storage;
     ChipLogDetail(Discovery, "Init fabric pairing table with server storage");
 

diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
@@ -274,6 +274,7 @@ class DLL_EXPORT FabricTableDelegate
     friend class FabricTable;
 
 public:
+    FabricTableDelegate(bool ownedByFabricTable = false) : mOwnedByFabricTable(ownedByFabricTable) {}
     virtual ~FabricTableDelegate() {}
     /**
      * Gets called when a fabric is deleted from KVS store.
@@ -292,6 +293,7 @@ class DLL_EXPORT FabricTableDelegate
 
 private:
     FabricTableDelegate * mNext = nullptr;
+    bool mOwnedByFabricTable    = false;
 };
 
 /**
@@ -365,6 +367,8 @@ class DLL_EXPORT FabricTable
 {
 public:
     FabricTable() { Reset(); }
+    ~FabricTable();
+
     CHIP_ERROR Store(FabricIndex index);
     CHIP_ERROR LoadFromStorage(FabricInfo * info);
 

diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h
@@ -166,7 +166,7 @@ class GroupDataProvider
         /**
          *  Callback invoked when an existing group is removed.
          *
-         *  @param[in] removed_state  GroupInfo structure of the removed group.
+         *  @param[in] old_group  GroupInfo structure of the removed group.
          */
         virtual void OnGroupRemoved(FabricIndex fabric_index, const GroupInfo & old_group) = 0;
     };

diff --git a/src/transport/GroupPeerMessageCounter.cpp b/src/transport/GroupPeerMessageCounter.cpp
@@ -151,27 +151,36 @@ CHIP_ERROR GroupPeerTable::RemovePeer(FabricIndex fabricIndex, NodeId nodeId, bo
     {
         if (mGroupFabrics[fabricIt].mDataPeerCount == 0 && mGroupFabrics[fabricIt].mControlPeerCount == 0)
         {
-            mGroupFabrics[fabricIt].mFabricIndex = kUndefinedFabricIndex;
-            // To maintain logic integrity Fabric array cannot have empty slot in between data
-            // Find the last non empty element
-            for (uint32_t i = CHIP_CONFIG_MAX_FABRICS - 1; i > fabricIt; i--)
-            {
-                if (mGroupFabrics[i].mFabricIndex != kUndefinedFabricIndex)
-                {
-                    // Logic works since all buffer are static
-                    // move it up front
-                    new (&mGroupFabrics[fabricIt]) GroupFabric(mGroupFabrics[i]);
-                    new (&mGroupFabrics[i]) GroupFabric();
-                    break;
-                }
-            }
+            RemoveAndCompactFabric(fabricIt);
         }
     }
 
     // Cannot find Peer to remove
     return err;
 }
 
+CHIP_ERROR GroupPeerTable::FabricRemoved(FabricIndex fabricIndex)
+{
+    CHIP_ERROR err = CHIP_ERROR_NOT_FOUND;
+
+    if (fabricIndex == kUndefinedFabricIndex)
+    {
+        return CHIP_ERROR_INVALID_ARGUMENT;
+    }
+
+    for (uint32_t it = 0; it < CHIP_CONFIG_MAX_FABRICS; it++)
+    {
+        if (fabricIndex == mGroupFabrics[it].mFabricIndex)
+        {
+            RemoveAndCompactFabric(it);
+            return CHIP_NO_ERROR;
+        }
+    }
+
+    // Cannot find Fabric to remove
+    return err;
+}
+
 bool GroupPeerTable::RemoveSpecificPeer(GroupSender * list, NodeId nodeId, uint32_t size)
 {
     bool removed = false;
@@ -222,6 +231,28 @@ void GroupPeerTable::CompactPeers(GroupSender * list, uint32_t size)
     }
 }
 
+void GroupPeerTable::RemoveAndCompactFabric(uint32_t tableIndex)
+{
+    if (tableIndex >= CHIP_CONFIG_MAX_FABRICS)
+    {
+        return;
+    }
+    mGroupFabrics[tableIndex].mFabricIndex = kUndefinedFabricIndex;
+    // To maintain logic integrity Fabric array cannot have empty slot in between data
+    // Find the last non empty element
+    for (uint32_t i = CHIP_CONFIG_MAX_FABRICS - 1; i > tableIndex; i--)
+    {
+        if (mGroupFabrics[i].mFabricIndex != kUndefinedFabricIndex)
+        {
+            // Logic works since all buffer are static
+            // move it up front
+            new (&mGroupFabrics[tableIndex]) GroupFabric(mGroupFabrics[i]);
+            new (&mGroupFabrics[i]) GroupFabric();
+            break;
+        }
+    }
+}
+
 GroupOutgoingCounters::GroupOutgoingCounters(chip::PersistentStorageDelegate * storage_delegate)
 {
     Init(storage_delegate);

diff --git a/src/transport/GroupPeerMessageCounter.h b/src/transport/GroupPeerMessageCounter.h
@@ -62,10 +62,13 @@ class GroupPeerTable
     // Used in case of MCSP failure
     CHIP_ERROR RemovePeer(FabricIndex fabricIndex, NodeId nodeId, bool isControl);
 
+    CHIP_ERROR FabricRemoved(FabricIndex fabricIndex);
+
     // Protected for Unit Tests inheritance
 protected:
     bool RemoveSpecificPeer(GroupSender * list, NodeId nodeId, uint32_t size);
     void CompactPeers(GroupSender * list, uint32_t size);
+    void RemoveAndCompactFabric(uint32_t tableIndex);
 
     GroupFabric mGroupFabrics[CHIP_CONFIG_MAX_FABRICS];
 };

diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp
@@ -117,6 +117,17 @@ void SessionManager::Shutdown()
     mCB           = nullptr;
 }
 
+/**
+ * @brief Notification that a fabric was removed.
+ *        This function doesn't call ExpireAllPairingsForFabric
+ *        since the CASE session might still be open to send a response
+ *        on the removed fabric.
+ */
+void SessionManager::FabricRemoved(FabricIndex fabricIndex)
+{
+    mGroupPeerMsgCounter.FabricRemoved(fabricIndex);
+}
+
 CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, PayloadHeader & payloadHeader,
                                           System::PacketBufferHandle && message, EncryptedPacketBufferHandle & preparedMessage)
 {

diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h
@@ -197,6 +197,11 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate
      */
     void Shutdown();
 
+    /**
+     * @brief Notification that a fabric was removed.
+     */
+    void FabricRemoved(FabricIndex fabricIndex);
+
     TransportMgrBase * GetTransportManager() const { return mTransportMgr; }
 
     /**

diff --git a/src/transport/tests/TestGroupMessageCounter.cpp b/src/transport/tests/TestGroupMessageCounter.cpp
@@ -134,7 +134,7 @@ void RemovePeerTest(nlTestSuite * inSuite, void * inContext)
     FabricIndex fabricIndex                       = 1;
     CHIP_ERROR err                                = CHIP_NO_ERROR;
     chip::Transport::PeerMessageCounter * counter = nullptr;
-    chip::Transport::GroupPeerTable mGroupPeerMsgCounter;
+    TestGroupPeerTable mGroupPeerMsgCounter;
 
     // Fill table up (max fabric and mac peer)
     for (uint32_t it = 0; it < CHIP_CONFIG_MAX_FABRICS; it++)
@@ -165,6 +165,28 @@ void RemovePeerTest(nlTestSuite * inSuite, void * inContext)
     // Try re-adding the previous peer without any error
     err = mGroupPeerMsgCounter.FindOrAddPeer(99, 99, true, counter);
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+    err = mGroupPeerMsgCounter.FindOrAddPeer(104, 99, true, counter);
+    NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+    err = mGroupPeerMsgCounter.FindOrAddPeer(105, 99, true, counter);
+    NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+    err = mGroupPeerMsgCounter.FindOrAddPeer(106, 99, true, counter);
+    NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+    // Fabric removal test
+    err = mGroupPeerMsgCounter.FabricRemoved(123);
+    NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_NOT_FOUND);
+
+    err = mGroupPeerMsgCounter.FabricRemoved(99);
+    NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+    err = mGroupPeerMsgCounter.FabricRemoved(99);
+    NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_NOT_FOUND);
+
+    // Verify that the Fabric List was compacted.
+    NL_TEST_ASSERT(inSuite, 106 == mGroupPeerMsgCounter.GetFabricIndexAt(0));
 }
 
 void PeerRetrievalTest(nlTestSuite * inSuite, void * inContext)