From 26685dd16eaf89faef17fd6608feae2a382c8d66 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 22 Jul 2022 10:57:06 -0400 Subject: [PATCH] Disallow opening commissioning windows when fail-safe is not fully disarmed. We were allowing opening commissioning windows in the "fail-safe is busy" state. We should probably not allow that. --- src/app/FailSafeContext.cpp | 2 ++ src/app/FailSafeContext.h | 5 +++++ .../administrator-commissioning-server.cpp | 4 ++-- src/app/server/CommissioningWindowManager.cpp | 2 +- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/app/FailSafeContext.cpp b/src/app/FailSafeContext.cpp index f85e538d2e5543..ceadf923c59624 100644 --- 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 index e2759d85e71f06..370b21981582e1 100644 --- 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 index 5ca4685427cec6..f33fa4c184bded 100644 --- 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 index 039bf7da5105d3..5d1e574fe97c34 100644 --- 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());