From 16acf88252b63cdedb59aacc5a6a7b2305e3adf8 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Mon, 14 Mar 2022 14:12:15 -0700 Subject: [PATCH 1/4] Cleanup OperationalCredentials if the fail-safe timer expires --- .../operational-credentials-server.cpp | 56 ++++++++++++++++++- src/include/platform/FailSafeContext.h | 23 +++++--- src/platform/FailSafeContext.cpp | 10 ++-- 3 files changed, 76 insertions(+), 13 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 9ef192c645c184..e294dfe8fcc3b7 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -262,6 +262,56 @@ FabricInfo * RetrieveCurrentFabric(CommandHandler * aCommandHandler) return Server::GetInstance().GetFabricTable().FindFabricWithIndex(index); } +void FailSafeCleanup(FailSafeContext & failSafeContext) +{ + FabricInfo * fabricInfo = Server::GetInstance().GetFabricTable().FindFabricWithIndex(failSafeContext.GetFabricIndex()); + + emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: Call to FailSafeCleanup"); + + if (fabricInfo) + { + // If an AddNOC or UpdateNOC command has been successfully invoked, terminate all CASE sessions associated with the Fabric + // whose Fabric Index is recorded in the Fail-Safe context (see ArmFailSafe Command) by clearing any associated Secure + // Session Context at the Server. + if (failSafeContext.NocCommandHasBeenInvoked()) + { + CASESessionManager * caseSessionManager = Server::GetInstance().GetCASESessionManager(); + + if (caseSessionManager) + { + caseSessionManager->ReleaseSession(fabricInfo->GetPeerId()); + } + } + + // If an AddNOC command had been successfully invoked, achieve the equivalent effect of invoking the RemoveFabric command + // against the Fabric Index stored in the Fail-Safe Context for the Fabric Index that was the subject of the AddNOC + // command. + if (failSafeContext.AddNocCommandHasBeenInvoked()) + { + Server::GetInstance().GetFabricTable().Delete(fabricInfo->GetFabricIndex()); + } + + // If an UpdateNOC command had been successfully invoked, revert the state of operational key pair, NOC and ICAC for that + // Fabric to the state prior to the Fail-Safe timer being armed, for the Fabric Index that was the subject of the UpdateNOC + // command. + if (failSafeContext.UpdateNocCommandHasBeenInvoked()) + { + // TODO: Revert the state of operational key pair, NOC and ICAC + } + } +} + +void OnPlatformEventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, intptr_t arg) +{ + if (event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete) + { + if (event->CommissioningComplete.Status != CHIP_NO_ERROR) + { + FailSafeCleanup(DeviceControlServer::DeviceControlSvr().GetFailSafeContext()); + } + } +} + } // anonymous namespace // As per specifications section 11.22.5.1. Constant RESP_MAX @@ -340,6 +390,8 @@ void MatterOperationalCredentialsPluginServerInitCallback(void) registerAttributeAccessOverride(&gAttrAccess); Server::GetInstance().GetFabricTable().AddFabricDelegate(&gFabricDelegate); + + DeviceLayer::PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler); } namespace { @@ -595,7 +647,7 @@ bool emberAfOperationalCredentialsClusterAddNOCCallback(app::CommandHandler * co // The Fabric Index associated with the armed fail-safe context SHALL be updated to match the Fabric // Index just allocated. - failSafeContext.SetNocCommandInvoked(fabricIndex); + failSafeContext.SetAddNocCommandInvoked(fabricIndex); exit: @@ -661,7 +713,7 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler * // The Fabric Index associated with the armed fail-safe context SHALL be updated to match the Fabric // Index associated with the UpdateNOC command being invoked. - failSafeContext.SetNocCommandInvoked(fabricIndex); + failSafeContext.SetUpdateNocCommandInvoked(fabricIndex); exit: diff --git a/src/include/platform/FailSafeContext.h b/src/include/platform/FailSafeContext.h index 41eaac12023c07..e31af9bf43e9e2 100644 --- a/src/include/platform/FailSafeContext.h +++ b/src/include/platform/FailSafeContext.h @@ -55,12 +55,20 @@ class FailSafeContext return (accessingFabricIndex == mFabricIndex); } - inline bool NocCommandHasBeenInvoked() const { return mNocCommandHasBeenInvoked; } + inline bool NocCommandHasBeenInvoked() const { return mAddNocCommandHasBeenInvoked || mUpdateNocCommandHasBeenInvoked; } + inline bool AddNocCommandHasBeenInvoked() { return mAddNocCommandHasBeenInvoked; } + inline bool UpdateNocCommandHasBeenInvoked() { return mUpdateNocCommandHasBeenInvoked; } - inline void SetNocCommandInvoked(FabricIndex nocFabricIndex) + inline void SetAddNocCommandInvoked(FabricIndex nocFabricIndex) { - mNocCommandHasBeenInvoked = true; - mFabricIndex = nocFabricIndex; + mAddNocCommandHasBeenInvoked = true; + mFabricIndex = nocFabricIndex; + } + + inline void SetUpdateNocCommandInvoked(FabricIndex nocFabricIndex) + { + mUpdateNocCommandHasBeenInvoked = true; + mFabricIndex = nocFabricIndex; } inline FabricIndex GetFabricIndex() const @@ -72,9 +80,10 @@ class FailSafeContext private: // ===== Private members reserved for use by this class only. - bool mFailSafeArmed = false; - bool mNocCommandHasBeenInvoked = false; - FabricIndex mFabricIndex = kUndefinedFabricIndex; + bool mFailSafeArmed = false; + bool mAddNocCommandHasBeenInvoked = false; + bool mUpdateNocCommandHasBeenInvoked = false; + FabricIndex mFabricIndex = kUndefinedFabricIndex; // TODO:: Track the state of what was mutated during fail-safe. diff --git a/src/platform/FailSafeContext.cpp b/src/platform/FailSafeContext.cpp index 0a732fd8b194d4..0f771e4f80bc7c 100644 --- a/src/platform/FailSafeContext.cpp +++ b/src/platform/FailSafeContext.cpp @@ -43,8 +43,9 @@ void FailSafeContext::CommissioningFailedTimerComplete() event.CommissioningComplete.Status = CHIP_ERROR_TIMEOUT; CHIP_ERROR status = PlatformMgr().PostEvent(&event); - mFailSafeArmed = false; - mNocCommandHasBeenInvoked = false; + mFailSafeArmed = false; + mAddNocCommandHasBeenInvoked = false; + mUpdateNocCommandHasBeenInvoked = false; if (status != CHIP_NO_ERROR) { @@ -62,8 +63,9 @@ CHIP_ERROR FailSafeContext::ArmFailSafe(FabricIndex accessingFabricIndex, System CHIP_ERROR FailSafeContext::DisarmFailSafe() { - mFailSafeArmed = false; - mNocCommandHasBeenInvoked = false; + mFailSafeArmed = false; + mAddNocCommandHasBeenInvoked = false; + mUpdateNocCommandHasBeenInvoked = false; DeviceLayer::SystemLayer().CancelTimer(HandleArmFailSafe, this); return CHIP_NO_ERROR; } From 1b62a5ce1f32e1a38fe34f41cc6d387ce53e645d Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Tue, 15 Mar 2022 15:11:02 -0700 Subject: [PATCH 2/4] Address review comments --- .../operational-credentials-server.cpp | 54 ++++++++++--------- src/platform/FailSafeContext.cpp | 7 +-- 2 files changed, 32 insertions(+), 29 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 e294dfe8fcc3b7..2f7101c35bb26c 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -268,36 +268,35 @@ void FailSafeCleanup(FailSafeContext & failSafeContext) emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: Call to FailSafeCleanup"); - if (fabricInfo) - { - // If an AddNOC or UpdateNOC command has been successfully invoked, terminate all CASE sessions associated with the Fabric - // whose Fabric Index is recorded in the Fail-Safe context (see ArmFailSafe Command) by clearing any associated Secure - // Session Context at the Server. - if (failSafeContext.NocCommandHasBeenInvoked()) - { - CASESessionManager * caseSessionManager = Server::GetInstance().GetCASESessionManager(); + VerifyOrReturn(fabricInfo != nullptr); - if (caseSessionManager) - { - caseSessionManager->ReleaseSession(fabricInfo->GetPeerId()); - } - } + // If an AddNOC or UpdateNOC command has been successfully invoked, terminate all CASE sessions associated with the Fabric + // whose Fabric Index is recorded in the Fail-Safe context (see ArmFailSafe Command) by clearing any associated Secure + // Session Context at the Server. + if (failSafeContext.NocCommandHasBeenInvoked()) + { + CASESessionManager * caseSessionManager = Server::GetInstance().GetCASESessionManager(); - // If an AddNOC command had been successfully invoked, achieve the equivalent effect of invoking the RemoveFabric command - // against the Fabric Index stored in the Fail-Safe Context for the Fabric Index that was the subject of the AddNOC - // command. - if (failSafeContext.AddNocCommandHasBeenInvoked()) + if (caseSessionManager) { - Server::GetInstance().GetFabricTable().Delete(fabricInfo->GetFabricIndex()); + caseSessionManager->ReleaseSessionsForFabric(fabricInfo->GetCompressedId()); } + } - // If an UpdateNOC command had been successfully invoked, revert the state of operational key pair, NOC and ICAC for that - // Fabric to the state prior to the Fail-Safe timer being armed, for the Fabric Index that was the subject of the UpdateNOC - // command. - if (failSafeContext.UpdateNocCommandHasBeenInvoked()) - { - // TODO: Revert the state of operational key pair, NOC and ICAC - } + // If an AddNOC command had been successfully invoked, achieve the equivalent effect of invoking the RemoveFabric command + // against the Fabric Index stored in the Fail-Safe Context for the Fabric Index that was the subject of the AddNOC + // command. + if (failSafeContext.AddNocCommandHasBeenInvoked()) + { + Server::GetInstance().GetFabricTable().Delete(fabricInfo->GetFabricIndex()); + } + + // If an UpdateNOC command had been successfully invoked, revert the state of operational key pair, NOC and ICAC for that + // Fabric to the state prior to the Fail-Safe timer being armed, for the Fabric Index that was the subject of the UpdateNOC + // command. + if (failSafeContext.UpdateNocCommandHasBeenInvoked()) + { + // TODO: Revert the state of operational key pair, NOC and ICAC } } @@ -307,7 +306,10 @@ void OnPlatformEventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, in { if (event->CommissioningComplete.Status != CHIP_NO_ERROR) { - FailSafeCleanup(DeviceControlServer::DeviceControlSvr().GetFailSafeContext()); + FailSafeContext & failSafeContext = DeviceControlServer::DeviceControlSvr().GetFailSafeContext(); + + VerifyOrReturn(event->CommissioningComplete.PeerFabricIndex == failSafeContext.GetFabricIndex()); + FailSafeCleanup(failSafeContext); } } } diff --git a/src/platform/FailSafeContext.cpp b/src/platform/FailSafeContext.cpp index 0f771e4f80bc7c..c13befbb6d39c6 100644 --- a/src/platform/FailSafeContext.cpp +++ b/src/platform/FailSafeContext.cpp @@ -39,9 +39,10 @@ void FailSafeContext::CommissioningFailedTimerComplete() // successfully invoked, conduct clean-up steps. ChipDeviceEvent event; - event.Type = DeviceEventType::kCommissioningComplete; - event.CommissioningComplete.Status = CHIP_ERROR_TIMEOUT; - CHIP_ERROR status = PlatformMgr().PostEvent(&event); + event.Type = DeviceEventType::kCommissioningComplete; + event.CommissioningComplete.PeerFabricIndex = mFabricIndex; + event.CommissioningComplete.Status = CHIP_ERROR_TIMEOUT; + CHIP_ERROR status = PlatformMgr().PostEvent(&event); mFailSafeArmed = false; mAddNocCommandHasBeenInvoked = false; From 3f19e61cde451147e27a96aae050d2ebfdaa2c47 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 17 Mar 2022 09:14:51 -0700 Subject: [PATCH 3/4] Capture failSafeContext in CommissioningFailedTimerComplete event --- .../operational-credentials-server.cpp | 26 +++++-------------- src/include/platform/CHIPDeviceEvent.h | 2 ++ src/platform/FailSafeContext.cpp | 13 +++++----- 3 files changed, 15 insertions(+), 26 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 2f7101c35bb26c..989c2431cbc145 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -262,39 +262,30 @@ FabricInfo * RetrieveCurrentFabric(CommandHandler * aCommandHandler) return Server::GetInstance().GetFabricTable().FindFabricWithIndex(index); } -void FailSafeCleanup(FailSafeContext & failSafeContext) +void FailSafeCleanup(const chip::DeviceLayer::ChipDeviceEvent * event) { - FabricInfo * fabricInfo = Server::GetInstance().GetFabricTable().FindFabricWithIndex(failSafeContext.GetFabricIndex()); - emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: Call to FailSafeCleanup"); - VerifyOrReturn(fabricInfo != nullptr); - // If an AddNOC or UpdateNOC command has been successfully invoked, terminate all CASE sessions associated with the Fabric // whose Fabric Index is recorded in the Fail-Safe context (see ArmFailSafe Command) by clearing any associated Secure // Session Context at the Server. - if (failSafeContext.NocCommandHasBeenInvoked()) + if (event->CommissioningComplete.AddNocCommandHasBeenInvoked || event->CommissioningComplete.UpdateNocCommandHasBeenInvoked) { - CASESessionManager * caseSessionManager = Server::GetInstance().GetCASESessionManager(); - - if (caseSessionManager) - { - caseSessionManager->ReleaseSessionsForFabric(fabricInfo->GetCompressedId()); - } + Server::GetInstance().GetSecureSessionManager().ExpireAllPairingsForFabric(event->CommissioningComplete.PeerFabricIndex); } // If an AddNOC command had been successfully invoked, achieve the equivalent effect of invoking the RemoveFabric command // against the Fabric Index stored in the Fail-Safe Context for the Fabric Index that was the subject of the AddNOC // command. - if (failSafeContext.AddNocCommandHasBeenInvoked()) + if (event->CommissioningComplete.AddNocCommandHasBeenInvoked) { - Server::GetInstance().GetFabricTable().Delete(fabricInfo->GetFabricIndex()); + Server::GetInstance().GetFabricTable().Delete(event->CommissioningComplete.PeerFabricIndex); } // If an UpdateNOC command had been successfully invoked, revert the state of operational key pair, NOC and ICAC for that // Fabric to the state prior to the Fail-Safe timer being armed, for the Fabric Index that was the subject of the UpdateNOC // command. - if (failSafeContext.UpdateNocCommandHasBeenInvoked()) + if (event->CommissioningComplete.UpdateNocCommandHasBeenInvoked) { // TODO: Revert the state of operational key pair, NOC and ICAC } @@ -306,10 +297,7 @@ void OnPlatformEventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, in { if (event->CommissioningComplete.Status != CHIP_NO_ERROR) { - FailSafeContext & failSafeContext = DeviceControlServer::DeviceControlSvr().GetFailSafeContext(); - - VerifyOrReturn(event->CommissioningComplete.PeerFabricIndex == failSafeContext.GetFabricIndex()); - FailSafeCleanup(failSafeContext); + FailSafeCleanup(event); } } } diff --git a/src/include/platform/CHIPDeviceEvent.h b/src/include/platform/CHIPDeviceEvent.h index b6431bc8b22e33..3a043d12ba61cc 100644 --- a/src/include/platform/CHIPDeviceEvent.h +++ b/src/include/platform/CHIPDeviceEvent.h @@ -444,6 +444,8 @@ struct ChipDeviceEvent final CHIP_ERROR Status; uint64_t PeerNodeId; FabricIndex PeerFabricIndex; + bool AddNocCommandHasBeenInvoked; + bool UpdateNocCommandHasBeenInvoked; } CommissioningComplete; struct diff --git a/src/platform/FailSafeContext.cpp b/src/platform/FailSafeContext.cpp index c13befbb6d39c6..14e0bb417db72f 100644 --- a/src/platform/FailSafeContext.cpp +++ b/src/platform/FailSafeContext.cpp @@ -35,14 +35,13 @@ void FailSafeContext::HandleArmFailSafe(System::Layer * layer, void * aAppState) void FailSafeContext::CommissioningFailedTimerComplete() { - // TODO: If the fail-safe timer expires before the CommissioningComplete command is - // successfully invoked, conduct clean-up steps. - ChipDeviceEvent event; - event.Type = DeviceEventType::kCommissioningComplete; - event.CommissioningComplete.PeerFabricIndex = mFabricIndex; - event.CommissioningComplete.Status = CHIP_ERROR_TIMEOUT; - CHIP_ERROR status = PlatformMgr().PostEvent(&event); + event.Type = DeviceEventType::kCommissioningComplete; + event.CommissioningComplete.PeerFabricIndex = mFabricIndex; + event.CommissioningComplete.AddNocCommandHasBeenInvoked = mAddNocCommandHasBeenInvoked; + event.CommissioningComplete.UpdateNocCommandHasBeenInvoked = mUpdateNocCommandHasBeenInvoked; + event.CommissioningComplete.Status = CHIP_ERROR_TIMEOUT; + CHIP_ERROR status = PlatformMgr().PostEvent(&event); mFailSafeArmed = false; mAddNocCommandHasBeenInvoked = false; From fa86a57ff34bf72c9df1ae912bc314115313ce45 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 17 Mar 2022 21:56:05 -0700 Subject: [PATCH 4/4] Clear things out from the CASESessionManager --- .../operational-credentials-server.cpp | 15 +++++++++++++-- src/platform/DeviceControlServer.cpp | 13 +++++++++---- 2 files changed, 22 insertions(+), 6 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 989c2431cbc145..e0be8c72915e97 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -266,12 +266,23 @@ void FailSafeCleanup(const chip::DeviceLayer::ChipDeviceEvent * event) { emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: Call to FailSafeCleanup"); + FabricIndex fabricIndex = event->CommissioningComplete.PeerFabricIndex; + // If an AddNOC or UpdateNOC command has been successfully invoked, terminate all CASE sessions associated with the Fabric // whose Fabric Index is recorded in the Fail-Safe context (see ArmFailSafe Command) by clearing any associated Secure // Session Context at the Server. if (event->CommissioningComplete.AddNocCommandHasBeenInvoked || event->CommissioningComplete.UpdateNocCommandHasBeenInvoked) { - Server::GetInstance().GetSecureSessionManager().ExpireAllPairingsForFabric(event->CommissioningComplete.PeerFabricIndex); + CASESessionManager * caseSessionManager = Server::GetInstance().GetCASESessionManager(); + if (caseSessionManager) + { + FabricInfo * fabricInfo = Server::GetInstance().GetFabricTable().FindFabricWithIndex(fabricIndex); + VerifyOrReturn(fabricInfo != nullptr); + + caseSessionManager->ReleaseSessionsForFabric(fabricInfo->GetCompressedId()); + } + + Server::GetInstance().GetSecureSessionManager().ExpireAllPairingsForFabric(fabricIndex); } // If an AddNOC command had been successfully invoked, achieve the equivalent effect of invoking the RemoveFabric command @@ -279,7 +290,7 @@ void FailSafeCleanup(const chip::DeviceLayer::ChipDeviceEvent * event) // command. if (event->CommissioningComplete.AddNocCommandHasBeenInvoked) { - Server::GetInstance().GetFabricTable().Delete(event->CommissioningComplete.PeerFabricIndex); + Server::GetInstance().GetFabricTable().Delete(fabricIndex); } // If an UpdateNOC command had been successfully invoked, revert the state of operational key pair, NOC and ICAC for that diff --git a/src/platform/DeviceControlServer.cpp b/src/platform/DeviceControlServer.cpp index eb9bc6731c2c9d..1582f9e4f47ddf 100644 --- a/src/platform/DeviceControlServer.cpp +++ b/src/platform/DeviceControlServer.cpp @@ -37,11 +37,16 @@ DeviceControlServer & DeviceControlServer::DeviceControlSvr() CHIP_ERROR DeviceControlServer::CommissioningComplete(NodeId peerNodeId, FabricIndex accessingFabricIndex) { VerifyOrReturnError(CHIP_NO_ERROR == mFailSafeContext.DisarmFailSafe(), CHIP_ERROR_INTERNAL); + ChipDeviceEvent event; - event.Type = DeviceEventType::kCommissioningComplete; - event.CommissioningComplete.PeerNodeId = peerNodeId; - event.CommissioningComplete.PeerFabricIndex = accessingFabricIndex; - event.CommissioningComplete.Status = CHIP_NO_ERROR; + + event.Type = DeviceEventType::kCommissioningComplete; + event.CommissioningComplete.PeerNodeId = peerNodeId; + event.CommissioningComplete.PeerFabricIndex = accessingFabricIndex; + event.CommissioningComplete.AddNocCommandHasBeenInvoked = mFailSafeContext.AddNocCommandHasBeenInvoked(); + event.CommissioningComplete.UpdateNocCommandHasBeenInvoked = mFailSafeContext.UpdateNocCommandHasBeenInvoked(); + event.CommissioningComplete.Status = CHIP_NO_ERROR; + return PlatformMgr().PostEvent(&event); }