Add spec-required checks to CommissioningComplete. (#16642)

We were allowing CommissioningComplete over PASE (or group message, for that matter), not checking for a matching fail-safe, etc.
project-chip · Mar 31, 2022 · 963936a · 963936a
1 parent 07c62e9
commit 963936a
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 11 deletions.
diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp
@@ -40,6 +40,8 @@ using namespace chip::app::Clusters;
 using namespace chip::app::Clusters::GeneralCommissioning;
 using namespace chip::app::Clusters::GeneralCommissioning::Attributes;
 using namespace chip::DeviceLayer;
+using Transport::SecureSession;
+using Transport::Session;
 
 #define CheckSuccess(expr, code)                                                                                                   \
     do                                                                                                                             \
@@ -165,19 +167,40 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
     const Commands::CommissioningComplete::DecodableType & commandData)
 {
     MATTER_TRACE_EVENT_SCOPE("CommissioningComplete", "GeneralCommissioning");
-    DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
-
-    /*
-     * Pass fabric of commissioner to DeviceControlSvr.
-     * This allows device to send messages back to commissioner.
-     * Once bindings are implemented, this may no longer be needed.
-     */
-    SessionHandle handle = commandObj->GetExchangeContext()->GetSessionHandle();
 
-    CheckSuccess(server->CommissioningComplete(handle->AsSecureSession()->GetPeerNodeId(), handle->GetFabricIndex()), Failure);
+    DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
+    const auto & failSafe        = server->GetFailSafeContext();
 
     Commands::CommissioningCompleteResponse::Type response;
-    response.errorCode = CommissioningError::kOk;
+    if (!failSafe.IsFailSafeArmed())
+    {
+        response.errorCode = CommissioningError::kNoFailSafe;
+    }
+    else
+    {
+        SessionHandle handle = commandObj->GetExchangeContext()->GetSessionHandle();
+        // If not a CASE session, or the fabric does not match the fail-safe,
+        // error out.
+        if (handle->GetSessionType() != Session::SessionType::kSecure ||
+            handle->AsSecureSession()->GetSecureSessionType() != SecureSession::Type::kCASE ||
+            !failSafe.MatchesFabricIndex(commandObj->GetAccessingFabricIndex()))
+        {
+            response.errorCode = CommissioningError::kInvalidAuthentication;
+        }
+        else
+        {
+            /*
+             * Pass fabric of commissioner to DeviceControlSvr.
+             * This allows device to send messages back to commissioner.
+             * Once bindings are implemented, this may no longer be needed.
+             */
+            CheckSuccess(server->CommissioningComplete(handle->AsSecureSession()->GetPeerNodeId(), handle->GetFabricIndex()),
+                         Failure);
+
+            response.errorCode = CommissioningError::kOk;
+        }
+    }
+
     CheckSuccess(commandObj->AddResponseData(commandPath, response), Failure);
 
     return true;

diff --git a/src/include/platform/FailSafeContext.h b/src/include/platform/FailSafeContext.h
@@ -42,7 +42,7 @@ class FailSafeContext
     CHIP_ERROR ArmFailSafe(FabricIndex accessingFabricIndex, System::Clock::Timeout expiryLength);
     CHIP_ERROR DisarmFailSafe();
 
-    inline bool IsFailSafeArmed(FabricIndex accessingFabricIndex)
+    inline bool IsFailSafeArmed(FabricIndex accessingFabricIndex) const
     {
         return mFailSafeArmed && MatchesFabricIndex(accessingFabricIndex);
     }