[app] use PeerId as the only key for CASESessionManager (#13074)

* [app] use PeerId as the only key for CASESessionManager

This prevents accidental misuse of a wrong node in a different fabric.

* fix build & review comments

examples/chip-tool/commands/clusters/ModelCommand.cpp

@@ -36,7 +36,7 @@ void ModelCommand::OnDeviceConnectedFn(void * context, ChipDevice * device)
     command->SendCommand(device, command->mEndPointId);
 }
 
-void ModelCommand::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR err)
+void ModelCommand::OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR err)
 {
     LogErrorOnFailure(err);
 

examples/chip-tool/commands/clusters/ModelCommand.h

@@ -54,7 +54,7 @@ class ModelCommand : public CHIPCommand
     uint8_t mEndPointId;
 
     static void OnDeviceConnectedFn(void * context, ChipDevice * device);
-    static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error);
+    static void OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR error);
 
     chip::Callback::Callback<chip::OnDeviceConnected> mOnDeviceConnectedCallback;
     chip::Callback::Callback<chip::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;

examples/chip-tool/commands/common/CHIPCommand.h

@@ -39,6 +39,7 @@ class CHIPCommand : public Command
     using ChipDeviceController   = ::chip::Controller::DeviceController;
     using IPAddress              = ::chip::Inet::IPAddress;
     using NodeId                 = ::chip::NodeId;
+    using PeerId                 = ::chip::PeerId;
     using PeerAddress            = ::chip::Transport::PeerAddress;
 
     CHIPCommand(const char * commandName) : Command(commandName)

examples/chip-tool/commands/pairing/OpenCommissioningWindowCommand.cpp

@@ -31,7 +31,7 @@ void OpenCommissioningWindowCommand::OnDeviceConnectedFn(void * context, chip::O
     VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "OnDeviceConnectedFn: context is null"));
     command->OpenCommissioningWindow();
 }
-void OpenCommissioningWindowCommand::OnDeviceConnectionFailureFn(void * context, NodeId remoteId, CHIP_ERROR err)
+void OpenCommissioningWindowCommand::OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR err)
 {
     LogErrorOnFailure(err);
 

examples/chip-tool/commands/pairing/OpenCommissioningWindowCommand.h

@@ -48,7 +48,7 @@ class OpenCommissioningWindowCommand : public CHIPCommand
 
     CHIP_ERROR OpenCommissioningWindow();
     static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device);
-    static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error);
+    static void OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR error);
     static void OnOpenCommissioningWindowResponse(void * context, NodeId deviceId, CHIP_ERROR status, chip::SetupPayload payload);
 
     chip::Callback::Callback<chip::OnDeviceConnected> mOnDeviceConnectedCallback;

examples/chip-tool/commands/tests/TestCommand.cpp

@@ -45,10 +45,10 @@ void TestCommand::OnDeviceConnectedFn(void * context, chip::OperationalDevicePro
     command->NextTest();
 }
 
-void TestCommand::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error)
+void TestCommand::OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR error)
 {
     ChipLogProgress(chipTool, " **** Test Setup: Device Connection Failure [deviceId=%" PRIu64 ". Error %" CHIP_ERROR_FORMAT "\n]",
-                    deviceId, error.Format());
+                    peerId.GetNodeId(), error.Format());
     auto * command = static_cast<TestCommand *>(context);
     VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "Test command context is null"));
     command->SetCommandExitStatus(error);

examples/chip-tool/commands/tests/TestCommand.h

@@ -66,7 +66,7 @@ class TestCommand : public CHIPCommand
     chip::NodeId mNodeId;
 
     static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device);
-    static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error);
+    static void OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR error);
     static void OnWaitForMsFn(chip::System::Layer * systemLayer, void * context);
 
     CHIP_ERROR ContinueOnChipMainThread() { return WaitForMs(0); };

src/app/CASESessionManager.cpp

@@ -21,17 +21,14 @@
 
 namespace chip {
 
-CHIP_ERROR CASESessionManager::FindOrEstablishSession(FabricInfo * fabric, NodeId nodeId,
-                                                      Callback::Callback<OnDeviceConnected> * onConnection,
+CHIP_ERROR CASESessionManager::FindOrEstablishSession(PeerId peerId, Callback::Callback<OnDeviceConnected> * onConnection,
                                                       Callback::Callback<OnDeviceConnectionFailure> * onFailure)
 {
     Dnssd::ResolvedNodeData resolutionData;
 
-    PeerId peerId = fabric->GetPeerIdForNode(nodeId);
-
     bool nodeIDWasResolved = (mConfig.dnsCache != nullptr && mConfig.dnsCache->Lookup(peerId, resolutionData) == CHIP_NO_ERROR);
 
-    OperationalDeviceProxy * session = FindExistingSession(nodeId);
+    OperationalDeviceProxy * session = FindExistingSession(peerId);
     if (session == nullptr)
     {
         // TODO - Implement LRU to evict least recently used session to handle mActiveSessions pool exhaustion
@@ -46,7 +43,7 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(FabricInfo * fabric, NodeI
 
         if (session == nullptr)
         {
-            onFailure->mCall(onFailure->mContext, nodeId, CHIP_ERROR_NO_MEMORY);
+            onFailure->mCall(onFailure->mContext, peerId, CHIP_ERROR_NO_MEMORY);
             return CHIP_ERROR_NO_MEMORY;
         }
     }
@@ -64,9 +61,9 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(FabricInfo * fabric, NodeI
     return err;
 }
 
-void CASESessionManager::ReleaseSession(NodeId nodeId)
+void CASESessionManager::ReleaseSession(PeerId peerId)
 {
-    ReleaseSession(FindExistingSession(nodeId));
+    ReleaseSession(FindExistingSession(peerId));
 }
 
 CHIP_ERROR CASESessionManager::ResolveDeviceAddress(FabricInfo * fabric, NodeId nodeId)
@@ -84,7 +81,7 @@ void CASESessionManager::OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeDa
         LogErrorOnFailure(mConfig.dnsCache->Insert(nodeData));
     }
 
-    OperationalDeviceProxy * session = FindExistingSession(nodeData.mPeerId.GetNodeId());
+    OperationalDeviceProxy * session = FindExistingSession(nodeData.mPeerId);
     VerifyOrReturn(session != nullptr,
                    ChipLogDetail(Controller, "OnNodeIdResolved was called for a device with no active sessions, ignoring it."));
 
@@ -96,17 +93,17 @@ void CASESessionManager::OnNodeIdResolutionFailed(const PeerId & peer, CHIP_ERRO
     ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error));
 }
 
-CHIP_ERROR CASESessionManager::GetPeerAddress(FabricInfo * fabric, NodeId nodeId, Transport::PeerAddress & addr)
+CHIP_ERROR CASESessionManager::GetPeerAddress(PeerId peerId, Transport::PeerAddress & addr)
 {
     if (mConfig.dnsCache != nullptr)
     {
         Dnssd::ResolvedNodeData resolutionData;
-        ReturnErrorOnFailure(mConfig.dnsCache->Lookup(fabric->GetPeerIdForNode(nodeId), resolutionData));
+        ReturnErrorOnFailure(mConfig.dnsCache->Lookup(peerId, resolutionData));
         addr = OperationalDeviceProxy::ToPeerAddress(resolutionData);
         return CHIP_NO_ERROR;
     }
 
-    OperationalDeviceProxy * session = FindExistingSession(nodeId);
+    OperationalDeviceProxy * session = FindExistingSession(peerId);
     VerifyOrReturnError(session != nullptr, CHIP_ERROR_NOT_CONNECTED);
     addr = session->GetPeerAddress();
     return CHIP_NO_ERROR;
@@ -125,9 +122,9 @@ OperationalDeviceProxy * CASESessionManager::FindSession(const SessionHandle & s
     return mConfig.devicePool->FindDevice(session);
 }
 
-OperationalDeviceProxy * CASESessionManager::FindExistingSession(NodeId id)
+OperationalDeviceProxy * CASESessionManager::FindExistingSession(PeerId peerId)
 {
-    return mConfig.devicePool->FindDevice(id);
+    return mConfig.devicePool->FindDevice(peerId);
 }
 
 void CASESessionManager::ReleaseSession(OperationalDeviceProxy * session)

src/app/CASESessionManager.h

@@ -79,12 +79,12 @@ class CASESessionManager : public SessionReleaseDelegate, public Dnssd::Resolver
      * these will be used to inform the caller about successful or failed connection establishment.
      * If the connection is already established, the `onConnection` callback will be immediately called.
      */
-    CHIP_ERROR FindOrEstablishSession(FabricInfo * fabric, NodeId nodeId, Callback::Callback<OnDeviceConnected> * onConnection,
+    CHIP_ERROR FindOrEstablishSession(PeerId peerId, Callback::Callback<OnDeviceConnected> * onConnection,
                                       Callback::Callback<OnDeviceConnectionFailure> * onFailure);
 
-    OperationalDeviceProxy * FindExistingSession(NodeId nodeId);
+    OperationalDeviceProxy * FindExistingSession(PeerId peerId);
 
-    void ReleaseSession(NodeId nodeId);
+    void ReleaseSession(PeerId peerId);
 
     /**
      * This API triggers the DNS-SD resolution for the given node ID. The node ID will be looked up
@@ -103,7 +103,7 @@ class CASESessionManager : public SessionReleaseDelegate, public Dnssd::Resolver
      * an ongoing session with the peer node. If the session doesn't exist, the API will return
      * `CHIP_ERROR_NOT_CONNECTED` error.
      */
-    CHIP_ERROR GetPeerAddress(FabricInfo * fabric, NodeId nodeId, Transport::PeerAddress & addr);
+    CHIP_ERROR GetPeerAddress(PeerId peerId, Transport::PeerAddress & addr);
 
     //////////// SessionReleaseDelegate Implementation ///////////////
     void OnSessionReleased(const SessionHandle & session) override;

src/app/OperationalDeviceProxy.cpp

@@ -84,7 +84,7 @@ CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback<OnDeviceConnected>
 
     if (err != CHIP_NO_ERROR && onFailure != nullptr)
     {
-        onFailure->mCall(onFailure->mContext, mPeerId.GetNodeId(), err);
+        onFailure->mCall(onFailure->mContext, mPeerId, err);
     }
 
     return err;
@@ -205,7 +205,7 @@ void OperationalDeviceProxy::DequeueConnectionFailureCallbacks(CHIP_ERROR error,
         cb->Cancel();
         if (executeCallback)
         {
-            cb->mCall(cb->mContext, mPeerId.GetNodeId(), error);
+            cb->mCall(cb->mContext, mPeerId, error);
         }
     }
 }

src/app/OperationalDeviceProxy.h

@@ -74,7 +74,7 @@ struct DeviceProxyInitParams
 class OperationalDeviceProxy;
 
 typedef void (*OnDeviceConnected)(void * context, OperationalDeviceProxy * device);
-typedef void (*OnDeviceConnectionFailure)(void * context, NodeId deviceId, CHIP_ERROR error);
+typedef void (*OnDeviceConnectionFailure)(void * context, PeerId peerId, CHIP_ERROR error);
 
 class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDelegate, public SessionEstablishmentDelegate
 {

src/app/OperationalDeviceProxyPool.h

@@ -35,7 +35,7 @@ class OperationalDeviceProxyPoolDelegate
 
     virtual OperationalDeviceProxy * FindDevice(const SessionHandle & session) = 0;
 
-    virtual OperationalDeviceProxy * FindDevice(NodeId id) = 0;
+    virtual OperationalDeviceProxy * FindDevice(PeerId peerId) = 0;
 
     virtual ~OperationalDeviceProxyPoolDelegate() {}
 };
@@ -74,11 +74,11 @@ class OperationalDeviceProxyPool : public OperationalDeviceProxyPoolDelegate
         return foundDevice;
     }
 
-    OperationalDeviceProxy * FindDevice(NodeId id) override
+    OperationalDeviceProxy * FindDevice(PeerId peerId) override
     {
         OperationalDeviceProxy * foundDevice = nullptr;
         mDevicePool.ForEachActiveObject([&](auto * activeDevice) {
-            if (activeDevice->GetDeviceId() == id)
+            if (activeDevice->GetPeerId() == peerId)
             {
                 foundDevice = activeDevice;
                 return Loop::Break;

src/app/clusters/ota-requestor/OTARequestor.cpp

@@ -237,8 +237,8 @@ void OTARequestor::ConnectToProvider(OnConnectedAction onConnectedAction)
 
     ChipLogDetail(SoftwareUpdate, "Establishing session to provider node ID 0x" ChipLogFormatX64 " on fabric index %d",
                   ChipLogValueX64(mProviderNodeId), mProviderFabricIndex);
-    CHIP_ERROR err = mCASESessionManager->FindOrEstablishSession(fabricInfo, mProviderNodeId, &mOnConnectedCallback,
-                                                                 &mOnConnectionFailureCallback);
+    CHIP_ERROR err = mCASESessionManager->FindOrEstablishSession(fabricInfo->GetPeerIdForNode(mProviderNodeId),
+                                                                 &mOnConnectedCallback, &mOnConnectionFailureCallback);
     VerifyOrReturn(err == CHIP_NO_ERROR,
                    ChipLogError(SoftwareUpdate, "Cannot establish connection to provider: %" CHIP_ERROR_FORMAT, err.Format()));
 }
@@ -350,9 +350,10 @@ OTARequestorInterface::OTATriggerResult OTARequestor::TriggerImmediateQuery()
 }
 
 // Called whenever FindOrEstablishSession fails
-void OTARequestor::OnConnectionFailure(void * context, NodeId deviceId, CHIP_ERROR error)
+void OTARequestor::OnConnectionFailure(void * context, PeerId peerId, CHIP_ERROR error)
 {
-    ChipLogError(SoftwareUpdate, "Failed to connect to node 0x%" PRIX64 ": %" CHIP_ERROR_FORMAT, deviceId, error.Format());
+    ChipLogError(SoftwareUpdate, "Failed to connect to node 0x%" PRIX64 ": %" CHIP_ERROR_FORMAT, peerId.GetNodeId(),
+                 error.Format());
 }
 
 void OTARequestor::ApplyUpdate()

src/app/clusters/ota-requestor/OTARequestor.h

@@ -213,7 +213,7 @@ class OTARequestor : public OTARequestorInterface
      * Session connection callbacks
      */
     static void OnConnected(void * context, OperationalDeviceProxy * deviceProxy);
-    static void OnConnectionFailure(void * context, NodeId deviceId, CHIP_ERROR error);
+    static void OnConnectionFailure(void * context, PeerId peerId, CHIP_ERROR error);
     Callback::Callback<OnDeviceConnected> mOnConnectedCallback;
     Callback::Callback<OnDeviceConnectionFailure> mOnConnectionFailureCallback;
 

src/controller/CHIPDeviceController.cpp

@@ -265,7 +265,7 @@ void DeviceController::ReleaseOperationalDevice(NodeId remoteDeviceId)
 {
     VerifyOrReturn(mState == State::Initialized,
                    ChipLogError(Controller, "ReleaseOperationalDevice was called in incorrect state"));
-    mCASESessionManager->ReleaseSession(remoteDeviceId);
+    mCASESessionManager->ReleaseSession(mFabricInfo->GetPeerIdForNode(remoteDeviceId));
 }
 
 void DeviceController::OnSessionReleased(const SessionHandle & session)
@@ -352,7 +352,7 @@ CHIP_ERROR DeviceController::GetPeerAddressAndPort(PeerId peerId, Inet::IPAddres
 {
     VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
     Transport::PeerAddress peerAddr;
-    ReturnErrorOnFailure(mCASESessionManager->GetPeerAddress(mFabricInfo, peerId.GetNodeId(), peerAddr));
+    ReturnErrorOnFailure(mCASESessionManager->GetPeerAddress(peerId, peerAddr));
     addr = peerAddr.GetIPAddress();
     port = peerAddr.GetPort();
     return CHIP_NO_ERROR;
@@ -379,7 +379,7 @@ void DeviceController::OnVIDReadResponse(void * context, uint16_t value)
     controller->mSetupPayload.vendorID = value;
 
     OperationalDeviceProxy * device =
-        controller->mCASESessionManager->FindExistingSession(controller->mDeviceWithCommissioningWindowOpen);
+        controller->mCASESessionManager->FindExistingSession(controller->GetPeerIdWithCommissioningWindowOpen());
     if (device == nullptr)
     {
         ChipLogError(Controller, "Could not find device for opening commissioning window");
@@ -476,7 +476,7 @@ CHIP_ERROR DeviceController::OpenCommissioningWindowWithCallback(NodeId deviceId
 
     if (callback != nullptr && mCommissioningWindowOption != CommissioningWindowOption::kOriginalSetupCode && readVIDPIDAttributes)
     {
-        OperationalDeviceProxy * device = mCASESessionManager->FindExistingSession(mDeviceWithCommissioningWindowOpen);
+        OperationalDeviceProxy * device = mCASESessionManager->FindExistingSession(GetPeerIdWithCommissioningWindowOpen());
         VerifyOrReturnError(device != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
         constexpr EndpointId kBasicClusterEndpoint = 0;
@@ -495,7 +495,7 @@ CHIP_ERROR DeviceController::OpenCommissioningWindowInternal()
     ChipLogProgress(Controller, "OpenCommissioningWindow for device ID %" PRIu64, mDeviceWithCommissioningWindowOpen);
     VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
 
-    OperationalDeviceProxy * device = mCASESessionManager->FindExistingSession(mDeviceWithCommissioningWindowOpen);
+    OperationalDeviceProxy * device = mCASESessionManager->FindExistingSession(GetPeerIdWithCommissioningWindowOpen());
     VerifyOrReturnError(device != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
     constexpr EndpointId kAdministratorCommissioningClusterEndpoint = 0;
@@ -1645,8 +1645,7 @@ void DeviceCommissioner::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData &
 
     mDNSCache.Insert(nodeData);
 
-    mCASESessionManager->FindOrEstablishSession(mFabricInfo, nodeData.mPeerId.GetNodeId(), &mOnDeviceConnectedCallback,
-                                                &mOnDeviceConnectionFailureCallback);
+    mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback);
     DeviceController::OnNodeIdResolved(nodeData);
 }
 
@@ -1688,15 +1687,15 @@ void DeviceCommissioner::OnDeviceConnectedFn(void * context, OperationalDevicePr
     }
 }
 
-void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error)
+void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR error)
 {
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
     ChipLogProgress(Controller, "Device connection failed. Error %s", ErrorStr(error));
     VerifyOrReturn(commissioner != nullptr,
                    ChipLogProgress(Controller, "Device connection failure callback with null context. Ignoring"));
     VerifyOrReturn(commissioner->mPairingDelegate != nullptr,
                    ChipLogProgress(Controller, "Device connection failure callback with null pairing delegate. Ignoring"));
-    commissioner->mPairingDelegate->OnCommissioningComplete(deviceId, error);
+    commissioner->mPairingDelegate->OnCommissioningComplete(peerId.GetNodeId(), error);
 }
 
 void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, CommissioningStage step, CommissioningParameters & params,

src/controller/CHIPDeviceController.h

@@ -221,7 +221,7 @@ class DLL_EXPORT DeviceController : public SessionReleaseDelegate,
                                           Callback::Callback<OnDeviceConnectionFailure> * onFailure)
     {
         VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
-        return mCASESessionManager->FindOrEstablishSession(mFabricInfo, deviceId, onConnection, onFailure);
+        return mCASESessionManager->FindOrEstablishSession(mFabricInfo->GetPeerIdForNode(deviceId), onConnection, onFailure);
     }
 
     /**
@@ -399,6 +399,8 @@ class DLL_EXPORT DeviceController : public SessionReleaseDelegate,
 
     CHIP_ERROR OpenCommissioningWindowInternal();
 
+    PeerId GetPeerIdWithCommissioningWindowOpen() { return mFabricInfo->GetPeerIdForNode(mDeviceWithCommissioningWindowOpen); }
+
     // TODO - Support opening commissioning window simultaneously on multiple devices
     Callback::Callback<OnOpenCommissioningWindow> * mCommissioningWindowCallback = nullptr;
     SetupPayload mSetupPayload;
@@ -745,7 +747,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     static void OnRootCertFailureResponse(void * context, uint8_t status);
 
     static void OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device);
-    static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error);
+    static void OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR error);
 
     static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
                                            const ByteSpan & rcac);