Disallow opening commissioning windows when fail-safe is not fully di…

…sarmed. (project-chip#21111) We were allowing opening commissioning windows in the "fail-safe is busy" state. We should probably not allow that.
isiu-apple · Sep 16, 2022 · 00117ea · 00117ea
1 parent 0ce8f96
commit 00117ea
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 3 deletions.
diff --git a/src/app/FailSafeContext.cpp b/src/app/FailSafeContext.cpp
@@ -86,6 +86,8 @@ void FailSafeContext::ScheduleFailSafeCleanup(FabricIndex fabricIndex, bool addN
 
 CHIP_ERROR FailSafeContext::ArmFailSafe(FabricIndex accessingFabricIndex, System::Clock::Seconds16 expiryLengthSeconds)
 {
+    VerifyOrReturnError(!IsFailSafeBusy(), CHIP_ERROR_INCORRECT_STATE);
+
     CHIP_ERROR err           = CHIP_NO_ERROR;
     bool cancelTimersIfError = false;
     if (!mFailSafeArmed)

diff --git a/src/app/FailSafeContext.h b/src/app/FailSafeContext.h
@@ -75,6 +75,11 @@ class FailSafeContext
 
     bool IsFailSafeArmed() const { return mFailSafeArmed; }
 
+    // True if it is possible to do an initial arming of the failsafe if needed.
+    // To be used in places where some action should take place only if the
+    // fail-safe could be armed after that action.
+    bool IsFailSafeFullyDisarmed() const { return !IsFailSafeArmed() && !IsFailSafeBusy(); }
+
     bool MatchesFabricIndex(FabricIndex accessingFabricIndex) const
     {
         VerifyOrDie(mFailSafeArmed);

diff --git a/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp b/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp
@@ -111,7 +111,7 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
     auto & commissionMgr          = Server::GetInstance().GetCommissioningWindowManager();
 
     VerifyOrExit(fabricInfo != nullptr, status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
-    VerifyOrExit(!failSafeContext.IsFailSafeArmed(), status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY));
+    VerifyOrExit(failSafeContext.IsFailSafeFullyDisarmed(), status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY));
 
     VerifyOrExit(commissionMgr.CommissioningWindowStatus() == CommissioningWindowStatus::kWindowNotOpen,
                  status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY));
@@ -176,7 +176,7 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac
 
     VerifyOrExit(commissionMgr.CommissioningWindowStatus() == CommissioningWindowStatus::kWindowNotOpen,
                  status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY));
-    VerifyOrExit(!failSafeContext.IsFailSafeArmed(), status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY));
+    VerifyOrExit(failSafeContext.IsFailSafeFullyDisarmed(), status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY));
     VerifyOrExit(commissioningTimeout <= commissionMgr.MaxCommissioningTimeout(),
                  globalStatus = InteractionModel::Status::InvalidCommand);
     VerifyOrExit(commissioningTimeout >= commissionMgr.MinCommissioningTimeout(),

diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp
@@ -195,7 +195,7 @@ CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow(Seconds16 commiss
     VerifyOrReturnError(commissioningTimeout <= MaxCommissioningTimeout() && commissioningTimeout >= MinCommissioningTimeout(),
                         CHIP_ERROR_INVALID_ARGUMENT);
     auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
-    VerifyOrReturnError(!failSafeContext.IsFailSafeArmed(), CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(failSafeContext.IsFailSafeFullyDisarmed(), CHIP_ERROR_INCORRECT_STATE);
 
     ReturnErrorOnFailure(Dnssd::ServiceAdvertiser::Instance().UpdateCommissionableInstanceName());