From 5547a6e1084f3ff4be7b8855538cdba70e3242df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Tue, 17 May 2022 17:40:56 +0200 Subject: [PATCH] [nrf fromtree] [opcreds] Fix LEAVE event on RemoveFabric (#18434) * [opcreds] Fix LEAVE event on RemoveFabric LEAVE event is not emitted on RemoveFabric or factory reset. There are two reasons for that: - Removing a fabric causes closing all active ReadHandlers for a given fabric, so we must flush the event log before that happens. - There are two listeners for removing a fabric; the one that removes ACLs for a given fabric index is executed prior to the other one that generates LEAVE event. That causes "Accces control: denied" error. Fix both issues. * Add comment (cherry picked from commit d016401e551c35c9eb1d14bed6b5cb129c656a86) --- .../operational-credentials-server.cpp | 23 +++++++++++++++++-- src/app/server/Server.h | 15 ------------ 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp index 7c02d23d3730af..2f08e8ddd58a3b 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -364,8 +364,7 @@ class OpCredsFabricTableDelegate : public FabricTableDelegate static_cast(fabricIndex)); fabricListChanged(); - // The Leave event SHOULD be emitted by a Node prior to permanently - // leaving the Fabric. + // The Leave event SHOULD be emitted by a Node prior to permanently leaving the Fabric. for (auto endpoint : EnabledEndpointsWithServerCluster(Basic::Id)) { // If Basic cluster is implemented on this endpoint @@ -377,6 +376,26 @@ class OpCredsFabricTableDelegate : public FabricTableDelegate ChipLogError(Zcl, "OpCredsFabricTableDelegate: Failed to record Leave event"); } } + + // Try to send the queued events as soon as possible. If the just emitted leave event won't + // be sent this time, it will likely not be delivered at all for the following reasons: + // - removing the fabric expires all associated ReadHandlers, so all subscriptions to + // the leave event will be cancelled. + // - removing the fabric removes all associated access control entries, so generating + // subsequent reports containing the leave event will fail the access control check. + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleUrgentEventDeliverySync(); + EventManagement::GetInstance().FabricRemoved(fabricIndex); + + // Remove access control entries in reverse order (it could be any order, but reverse order + // will cause less churn in persistent storage). + size_t aclCount = 0; + if (Access::GetAccessControl().GetEntryCount(fabricIndex, aclCount) == CHIP_NO_ERROR) + { + while (aclCount) + { + (void) Access::GetAccessControl().DeleteEntry(nullptr, fabricIndex, --aclCount); + } + } } // Gets called when a fabric is loaded into the FabricTable from storage diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 86d5114e1815e3..aca60a783341c4 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -314,21 +314,6 @@ class Server { groupDataProvider->RemoveFabric(fabricIndex); } - - { - // Remove access control entries in reverse order. (It could be - // any order, but reverse order will cause less churn in - // persistent storage.) - size_t count = 0; - if (Access::GetAccessControl().GetEntryCount(fabricIndex, count) == CHIP_NO_ERROR) - { - while (count) - { - Access::GetAccessControl().DeleteEntry(nullptr, fabricIndex, --count); - } - } - } - app::EventManagement::GetInstance().FabricRemoved(fabricIndex); }; void OnFabricRetrievedFromStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; }