Don't factory reset device if reboot with a pending arm failsafe (#16497

)

* Don't factory reset device after reboot with a pending arm failsafe

* Update src/platform/Ameba/ConfigurationManagerImpl.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/platform/Ameba/ConfigurationManagerImpl.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Address review comments

* Update src/include/platform/internal/GenericConfigurationManagerImpl.ipp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/include/platform/internal/GenericConfigurationManagerImpl.ipp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/lib/support/DefaultStorageKeyAllocator.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* debug

* Schedule work to do all disarming failsafe works async

* SetAddNocCommandInvoked right after add new fabric

* Update src/include/platform/FailSafeContext.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/include/platform/internal/GenericConfigurationManagerImpl.ipp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Add FailSafeBusy state

* Update src/app/clusters/operational-credentials-server/operational-credentials-server.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/app/clusters/general-commissioning-server/general-commissioning-server.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/include/platform/FailSafeContext.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/platform/FailSafeContext.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Set mFailSafeArmed to false in FailSafeTimerExpired

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

src/app/clusters/general-commissioning-server/general-commissioning-server.cpp

@@ -139,14 +139,16 @@ bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler *
     Commands::ArmFailSafeResponse::Type response;
 
     /*
+     * If the fail-safe timer is not fully disarmed, don't allow arming a new fail-safe.
      * If the fail-safe timer was not currently armed, then the fail-safe timer SHALL be armed.
      * If the fail-safe timer was currently armed, and current accessing fabric matches the fail-safe
      * context’s Fabric Index, then the fail-safe timer SHALL be re-armed.
      */
 
     FabricIndex accessingFabricIndex = commandObj->GetAccessingFabricIndex();
 
-    if (!failSafeContext.IsFailSafeArmed() || failSafeContext.MatchesFabricIndex(accessingFabricIndex))
+    if (!failSafeContext.IsFailSafeBusy() &&
+        (!failSafeContext.IsFailSafeArmed() || failSafeContext.MatchesFabricIndex(accessingFabricIndex)))
     {
         CheckSuccess(failSafeContext.ArmFailSafe(accessingFabricIndex, System::Clock::Seconds16(commandData.expiryLengthSeconds)),
                      Failure);

src/app/clusters/operational-credentials-server/operational-credentials-server.cpp

@@ -629,6 +629,16 @@ bool emberAfOperationalCredentialsClusterAddNOCCallback(app::CommandHandler * co
     err = Server::GetInstance().GetFabricTable().Store(fabricIndex);
     VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));
 
+    // The Fabric Index associated with the armed fail-safe context SHALL be updated to match the Fabric
+    // Index just allocated.
+    err = failSafeContext.SetAddNocCommandInvoked(fabricIndex);
+    if (err != CHIP_NO_ERROR)
+    {
+        Server::GetInstance().GetFabricTable().Delete(fabricIndex);
+        nocResponse = ConvertToNOCResponseStatus(err);
+        SuccessOrExit(err);
+    }
+
     // Keep this after other possible failures, so it doesn't need to be rolled back in case of
     // subsequent failures. This should only typically fail if there is no space for the new entry.
     err = CreateAccessControlEntryForNewFabricAdministrator(fabricIndex, commandData.caseAdminNode);
@@ -651,10 +661,6 @@ bool emberAfOperationalCredentialsClusterAddNOCCallback(app::CommandHandler * co
     // We might have a new operational identity, so we should start advertising it right away.
     app::DnssdServer::Instance().AdvertiseOperational();
 
-    // The Fabric Index associated with the armed fail-safe context SHALL be updated to match the Fabric
-    // Index just allocated.
-    failSafeContext.SetAddNocCommandInvoked(fabricIndex);
-
 exit:
 
     gFabricBeingCommissioned.Reset();
@@ -705,6 +711,10 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *
     FabricInfo * fabric = RetrieveCurrentFabric(commandObj);
     VerifyOrExit(fabric != nullptr, nocResponse = ConvertToNOCResponseStatus(CHIP_ERROR_INVALID_FABRIC_ID));
 
+    // Flag on the fail-safe context that the UpdateNOC command was invoked.
+    err = failSafeContext.SetUpdateNocCommandInvoked(fabricIndex);
+    VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));
+
     err = fabric->SetNOCCert(NOCValue);
     VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));
 
@@ -718,10 +728,6 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *
     // So we need to StartServer() here.
     app::DnssdServer::Instance().StartServer();
 
-    // 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.SetUpdateNocCommandInvoked(fabricIndex);
-
 exit:
 
     SendNOCResponse(commandObj, commandPath, nocResponse, fabricIndex, CharSpan());

src/include/platform/ConfigurationManager.h

@@ -29,6 +29,7 @@
 #include <app-common/zap-generated/cluster-objects.h>
 #include <lib/support/Span.h>
 #include <platform/CHIPDeviceBuildConfig.h>
+#include <platform/FailSafeContext.h>
 #include <platform/PersistedStorage.h>
 #include <setup_payload/CHIPAdditionalDataPayloadBuildConfig.h>
 
@@ -134,6 +135,8 @@ class ConfigurationManager
     virtual CHIP_ERROR GetUniqueId(char * buf, size_t bufSize)                         = 0;
     virtual CHIP_ERROR StoreUniqueId(const char * uniqueId, size_t uniqueIdLen)        = 0;
     virtual CHIP_ERROR GenerateUniqueId(char * buf, size_t bufSize)                    = 0;
+    virtual CHIP_ERROR GetFailSafeArmed(bool & val)                                    = 0;
+    virtual CHIP_ERROR SetFailSafeArmed(bool val)                                      = 0;
 
     virtual CHIP_ERROR GetBLEDeviceIdentificationInfo(Ble::ChipBLEDeviceIdentificationInfo & deviceIdInfo) = 0;
 
@@ -176,8 +179,6 @@ class ConfigurationManager
 
     virtual CHIP_ERROR Init()                                                                                   = 0;
     virtual bool CanFactoryReset()                                                                              = 0;
-    virtual CHIP_ERROR GetFailSafeArmed(bool & val)                                                             = 0;
-    virtual CHIP_ERROR SetFailSafeArmed(bool val)                                                               = 0;
     virtual CHIP_ERROR ReadPersistedStorageValue(::chip::Platform::PersistedStorage::Key key, uint32_t & value) = 0;
     virtual CHIP_ERROR WritePersistedStorageValue(::chip::Platform::PersistedStorage::Key key, uint32_t value)  = 0;
 

src/include/platform/FailSafeContext.h

@@ -41,14 +41,22 @@ class FailSafeContext
      */
     CHIP_ERROR ArmFailSafe(FabricIndex accessingFabricIndex, System::Clock::Timeout expiryLength);
     CHIP_ERROR DisarmFailSafe();
+    CHIP_ERROR SetAddNocCommandInvoked(FabricIndex nocFabricIndex);
+    CHIP_ERROR SetUpdateNocCommandInvoked(FabricIndex nocFabricIndex);
 
     inline bool IsFailSafeArmed(FabricIndex accessingFabricIndex) const
     {
         return mFailSafeArmed && MatchesFabricIndex(accessingFabricIndex);
     }
 
+    // Returns true if the fail-safe is in a state where commands that require an armed
+    // fail-safe can no longer execute, but a new fail-safe can't be armed yet.
+    inline bool IsFailSafeBusy() const { return mFailSafeBusy; }
+
     inline bool IsFailSafeArmed() const { return mFailSafeArmed; }
 
+    inline void SetFailSafeBusy(bool val) { mFailSafeBusy = val; }
+
     inline bool MatchesFabricIndex(FabricIndex accessingFabricIndex) const
     {
         VerifyOrDie(mFailSafeArmed);
@@ -59,36 +67,46 @@ class FailSafeContext
     inline bool AddNocCommandHasBeenInvoked() { return mAddNocCommandHasBeenInvoked; }
     inline bool UpdateNocCommandHasBeenInvoked() { return mUpdateNocCommandHasBeenInvoked; }
 
-    inline void SetAddNocCommandInvoked(FabricIndex nocFabricIndex)
-    {
-        mAddNocCommandHasBeenInvoked = true;
-        mFabricIndex                 = nocFabricIndex;
-    }
-
-    inline void SetUpdateNocCommandInvoked(FabricIndex nocFabricIndex)
-    {
-        mUpdateNocCommandHasBeenInvoked = true;
-        mFabricIndex                    = nocFabricIndex;
-    }
-
     inline FabricIndex GetFabricIndex() const
     {
         VerifyOrDie(mFailSafeArmed);
         return mFabricIndex;
     }
 
+    static CHIP_ERROR LoadFromStorage(FabricIndex & fabricIndex, bool & addNocCommandInvoked, bool & updateNocCommandInvoked);
+    static CHIP_ERROR DeleteFromStorage();
+
 private:
     // ===== Private members reserved for use by this class only.
 
     bool mFailSafeArmed                  = false;
+    bool mFailSafeBusy                   = false;
     bool mAddNocCommandHasBeenInvoked    = false;
     bool mUpdateNocCommandHasBeenInvoked = false;
     FabricIndex mFabricIndex             = kUndefinedFabricIndex;
 
     // TODO:: Track the state of what was mutated during fail-safe.
 
-    static void HandleArmFailSafe(System::Layer * layer, void * aAppState);
+    static constexpr size_t FailSafeContextTLVMaxSize()
+    {
+        return TLV::EstimateStructOverhead(sizeof(FabricIndex), sizeof(bool), sizeof(bool));
+    }
+
+    /**
+     * @brief
+     *  The callback function to be called when "fail-safe timer" expires.
+     */
+    static void HandleArmFailSafeTimer(System::Layer * layer, void * aAppState);
+
+    /**
+     * @brief
+     *  The callback function to be called asynchronously after various cleanup work has completed
+     *  to actually disarm the fail-safe.
+     */
+    static void HandleDisarmFailSafe(intptr_t arg);
+
     void FailSafeTimerExpired();
+    CHIP_ERROR CommitToStorage();
 };
 
 } // namespace DeviceLayer

src/include/platform/internal/GenericConfigurationManagerImpl.h

@@ -153,6 +153,9 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     virtual CHIP_ERROR WriteConfigValueStr(Key key, const char * str, size_t strLen)               = 0;
     virtual CHIP_ERROR WriteConfigValueBin(Key key, const uint8_t * data, size_t dataLen)          = 0;
     virtual void RunConfigUnitTest(void)                                                           = 0;
+
+private:
+    static void HandleFailSafeContextCleanup(intptr_t arg);
 };
 
 } // namespace Internal

src/include/platform/internal/GenericConfigurationManagerImpl.ipp

@@ -37,6 +37,7 @@
 #include <lib/support/CodeUtils.h>
 #include <lib/support/ScopedBuffer.h>
 #include <platform/CommissionableDataProvider.h>
+#include <platform/DeviceControlServer.h>
 #include <platform/internal/CHIPDeviceLayerInternal.h>
 #include <platform/internal/GenericConfigurationManagerImpl.h>
 
@@ -224,7 +225,7 @@ CHIP_ERROR LegacyTemporaryCommissionableDataProvider<ConfigClass>::GetSpake2pVer
 template <class ConfigClass>
 CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::Init()
 {
-    CHIP_ERROR err;
+    CHIP_ERROR err = CHIP_NO_ERROR;
 
 #if CHIP_ENABLE_ROTATING_DEVICE_ID && defined(CHIP_DEVICE_CONFIG_ROTATING_DEVICE_ID_UNIQUE_ID)
     mLifetimePersistedCounter.Init(CHIP_CONFIG_LIFETIIME_PERSISTED_COUNTER_KEY);
@@ -249,7 +250,39 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::Init()
     ReturnErrorOnFailure(err);
 
     err = StoreUniqueId(uniqueId, strlen(uniqueId));
+    ReturnErrorOnFailure(err);
+
+    bool failSafeArmed;
+
+    // If the fail-safe was armed when the device last shutdown, initiate cleanup based on the pending Fail Safe Context with
+    // which the fail-safe timer was armed.
+    if (GetFailSafeArmed(failSafeArmed) == CHIP_NO_ERROR && failSafeArmed)
+    {
+        FabricIndex fabricIndex;
+        bool addNocCommandInvoked;
+        bool updateNocCommandInvoked;
+
+        ChipLogProgress(DeviceLayer, "Detected fail-safe armed on reboot");
+
+        err = FailSafeContext::LoadFromStorage(fabricIndex, addNocCommandInvoked, updateNocCommandInvoked);
+        SuccessOrExit(err);
+
+        ChipDeviceEvent event;
+        event.Type                                                = DeviceEventType::kFailSafeTimerExpired;
+        event.FailSafeTimerExpired.PeerFabricIndex                = fabricIndex;
+        event.FailSafeTimerExpired.AddNocCommandHasBeenInvoked    = addNocCommandInvoked;
+        event.FailSafeTimerExpired.UpdateNocCommandHasBeenInvoked = updateNocCommandInvoked;
+
+        err = PlatformMgr().PostEvent(&event);
+        SuccessOrExit(err);
+
+        DeviceControlServer::DeviceControlSvr().GetFailSafeContext().SetFailSafeBusy(true);
+
+        // Ensure HandleFailSafeContextCleanup runs after the timer-expired event has been processed.
+        PlatformMgr().ScheduleWork(HandleFailSafeContextCleanup);
+    }
 
+exit:
     return err;
 }
 
@@ -868,6 +901,22 @@ void GenericConfigurationManagerImpl<ConfigClass>::LogDeviceConfig()
     }
 }
 
+template <class ConfigClass>
+void GenericConfigurationManagerImpl<ConfigClass>::HandleFailSafeContextCleanup(intptr_t arg)
+{
+    if (ConfigurationMgr().SetFailSafeArmed(false) != CHIP_NO_ERROR)
+    {
+        ChipLogError(DeviceLayer, "Failed to set FailSafeArmed config to false");
+    }
+
+    if (FailSafeContext::DeleteFromStorage() != CHIP_NO_ERROR)
+    {
+        ChipLogError(DeviceLayer, "Failed to delete FailSafeContext from configuration");
+    }
+
+    DeviceControlServer::DeviceControlSvr().GetFailSafeContext().SetFailSafeBusy(false);
+}
+
 } // namespace Internal
 } // namespace DeviceLayer
 } // namespace chip

src/lib/support/DefaultStorageKeyAllocator.h

@@ -42,6 +42,9 @@ class DefaultStorageKeyAllocator
     const char * FabricMetadata(FabricIndex fabric) { return Format("f/%x/m", fabric); }
     const char * FabricOpKey(FabricIndex fabric) { return Format("f/%x/o", fabric); }
 
+    // FailSafeContext
+    const char * FailSafeContextKey() { return Format("g/fsc"); }
+
     // Access Control List
 
     const char * AccessControlList() { return Format("acl"); }

src/platform/Ameba/ConfigurationManagerImpl.cpp

@@ -47,7 +47,6 @@ CHIP_ERROR ConfigurationManagerImpl::Init()
 {
     CHIP_ERROR err;
     uint32_t rebootCount;
-    bool failSafeArmed;
 
     // Force initialization of NVS namespaces if they doesn't already exist.
     err = AmebaConfig::EnsureNamespace(AmebaConfig::kConfigNamespace_ChipFactory);
@@ -88,12 +87,6 @@ CHIP_ERROR ConfigurationManagerImpl::Init()
     err = Internal::GenericConfigurationManagerImpl<AmebaConfig>::Init();
     SuccessOrExit(err);
 
-    // If the fail-safe was armed when the device last shutdown, initiate a factory reset.
-    if (GetFailSafeArmed(failSafeArmed) == CHIP_NO_ERROR && failSafeArmed)
-    {
-        ChipLogProgress(DeviceLayer, "Detected fail-safe armed on reboot; initiating factory reset");
-        InitiateFactoryReset();
-    }
     err = CHIP_NO_ERROR;
 
 exit:

src/platform/EFR32/ConfigurationManagerImpl.cpp

@@ -50,7 +50,6 @@ ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 CHIP_ERROR ConfigurationManagerImpl::Init()
 {
     CHIP_ERROR err;
-    bool failSafeArmed;
 
     // Initialize the generic implementation base class.
     err = Internal::GenericConfigurationManagerImpl<EFR32Config>::Init();
@@ -64,12 +63,6 @@ CHIP_ERROR ConfigurationManagerImpl::Init()
     rebootCause = RMU_ResetCauseGet();
     RMU_ResetCauseClear();
 
-    // If the fail-safe was armed when the device last shutdown, initiate a factory reset.
-    if (GetFailSafeArmed(failSafeArmed) == CHIP_NO_ERROR && failSafeArmed)
-    {
-        ChipLogProgress(DeviceLayer, "Detected fail-safe armed on reboot; initiating factory reset");
-        InitiateFactoryReset();
-    }
     err = CHIP_NO_ERROR;
 
 exit:

src/platform/ESP32/ConfigurationManagerImpl.cpp

@@ -60,7 +60,6 @@ CHIP_ERROR ConfigurationManagerImpl::Init()
 {
     CHIP_ERROR err;
     uint32_t rebootCount;
-    bool failSafeArmed;
 
     // Force initialization of NVS namespaces if they doesn't already exist.
     err = ESP32Config::EnsureNamespace(ESP32Config::kConfigNamespace_ChipFactory);
@@ -112,12 +111,6 @@ CHIP_ERROR ConfigurationManagerImpl::Init()
 
 #endif // CHIP_DEVICE_CONFIG_ENABLE_FACTORY_PROVISIONING
 
-    // If the fail-safe was armed when the device last shutdown, initiate a factory reset.
-    if (GetFailSafeArmed(failSafeArmed) == CHIP_NO_ERROR && failSafeArmed)
-    {
-        ChipLogProgress(DeviceLayer, "Detected fail-safe armed on reboot; initiating factory reset");
-        InitiateFactoryReset();
-    }
     err = CHIP_NO_ERROR;
 
 exit: