diff --git a/examples/chip-tool/commands/tests/TestCommand.cpp b/examples/chip-tool/commands/tests/TestCommand.cpp index 36cd8b785ac4f1..57bcf3d415cc22 100644 --- a/examples/chip-tool/commands/tests/TestCommand.cpp +++ b/examples/chip-tool/commands/tests/TestCommand.cpp @@ -32,7 +32,17 @@ CHIP_ERROR TestCommand::RunCommand() CHIP_ERROR TestCommand::WaitForCommissionee(chip::NodeId nodeId) { - CurrentCommissioner().ReleaseOperationalDevice(nodeId); + chip::FabricIndex fabricIndex; + + ReturnErrorOnFailure(CurrentCommissioner().GetFabricIndex(&fabricIndex)); + + // + // There's a chance the commissionee may have rebooted before this call here as part of a test flow + // or is just starting out fresh outright. Let's make sure we're not re-using any cached CASE sessions + // that will now be stale and mismatched with the peer, causing subsequent interactions to fail. + // + CurrentCommissioner().SessionMgr()->ExpireAllPairings(nodeId, fabricIndex); + return CurrentCommissioner().GetConnectedDevice(nodeId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); } diff --git a/src/app/OperationalDeviceProxy.cpp b/src/app/OperationalDeviceProxy.cpp index 7682323c7e8f8f..404c6181ca46ee 100644 --- a/src/app/OperationalDeviceProxy.cpp +++ b/src/app/OperationalDeviceProxy.cpp @@ -58,18 +58,17 @@ void OperationalDeviceProxy::MoveToState(State aTargetState) } } -bool OperationalDeviceProxy::CheckAndLoadExistingSession() +bool OperationalDeviceProxy::AttachToExistingSecureSession() { VerifyOrReturnError(mState == State::NeedsAddress || mState == State::Initialized, false); - SessionHolder existingSession; ScopedNodeId peerNodeId(mPeerId.GetNodeId(), mFabricInfo->GetFabricIndex()); - - mInitParams.sessionManager->FindSecureSessionForNode(mSecureSession, peerNodeId, Transport::SecureSession::Type::kCASE); - if (mSecureSession) + auto sessionHandle = mInitParams.sessionManager->FindSecureSessionForNode(peerNodeId, Transport::SecureSession::Type::kCASE); + if (sessionHandle.HasValue()) { - ChipLogProgress(Controller, "Found an existing secure session to [" ChipLogFormatX64 ":" ChipLogFormatX64 "]!", + ChipLogProgress(Controller, "Found an existing secure session to [" ChipLogFormatX64 "-" ChipLogFormatX64 "]!", ChipLogValueX64(mPeerId.GetCompressedFabricId()), ChipLogValueX64(mPeerId.GetNodeId())); + mSecureSession.Grab(sessionHandle.Value()); return true; } @@ -96,7 +95,7 @@ CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback break; case State::NeedsAddress: - isConnected = CheckAndLoadExistingSession(); + isConnected = AttachToExistingSecureSession(); if (!isConnected) { err = LookupPeerAddress(); @@ -105,7 +104,7 @@ CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback break; case State::Initialized: - isConnected = CheckAndLoadExistingSession(); + isConnected = AttachToExistingSecureSession(); if (!isConnected) { err = EstablishConnection(); @@ -235,17 +234,24 @@ void OperationalDeviceProxy::EnqueueConnectionCallbacks(Callback::Callback * cb = - Callback::Callback::FromCancelable(ready.mNext); + Callback::Callback::FromCancelable(failureReady.mNext); cb->Cancel(); @@ -255,10 +261,9 @@ void OperationalDeviceProxy::DequeueConnectionCallbacks(CHIP_ERROR error) } } - mConnectionSuccess.DequeueAll(ready); - while (ready.mNext != &ready) + while (successReady.mNext != &successReady) { - Callback::Callback * cb = Callback::Callback::FromCancelable(ready.mNext); + Callback::Callback * cb = Callback::Callback::FromCancelable(successReady.mNext); cb->Cancel(); if (error == CHIP_NO_ERROR) @@ -286,7 +291,7 @@ void OperationalDeviceProxy::HandleCASEConnectionFailure(void * context, CASECli device->DequeueConnectionCallbacks(error); // - // Do not touch this instance anymore; it might have been destroyed by a failure + // Do not touch device instance anymore; it might have been destroyed by a failure // callback. // } @@ -311,7 +316,7 @@ void OperationalDeviceProxy::HandleCASEConnected(void * context, CASEClient * cl } // - // Do not touch this instance anymore; it might have been destroyed by a failure + // Do not touch this instance anymore; it might have been destroyed by a // callback. // } diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index fb65eaaa2cf5db..ea5e60892369b2 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -270,7 +270,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, * Returns true if a valid session was found, false otherwise. * */ - bool CheckAndLoadExistingSession(); + bool AttachToExistingSecureSession(); bool IsSecureConnected() const override { return mState == State::SecureConnected; } diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 306dc050998ae2..561dedd2682069 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -146,6 +146,16 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, public Abstr */ virtual CHIP_ERROR Shutdown(); + SessionManager * SessionMgr() + { + if (mSystemState) + { + return mSystemState->SessionMgr(); + } + + return nullptr; + } + CHIP_ERROR GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port); /** diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 8d1705543af798..c2f6c2af961466 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -814,8 +814,7 @@ void SessionManager::ExpiryTimerCallback(System::Layer * layer, void * param) mgr->ScheduleExpiryTimer(); // re-schedule the oneshot timer } -void SessionManager::FindSecureSessionForNode(SessionHolder & sessionHolder, ScopedNodeId peerNodeId, - Transport::SecureSession::Type type) +Optional SessionManager::FindSecureSessionForNode(ScopedNodeId peerNodeId, Transport::SecureSession::Type type) { SecureSession * found = nullptr; mSecureSessions.ForEachSession([&peerNodeId, type, &found](auto session) { @@ -828,12 +827,7 @@ void SessionManager::FindSecureSessionForNode(SessionHolder & sessionHolder, Sco return Loop::Continue; }); - sessionHolder.Release(); - - if (found) - { - sessionHolder.Grab(SessionHandle(*found)); - } + return found != nullptr ? MakeOptional(*found) : Optional::Missing(); } /** diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 988c6b30e3188e..6d771eaa31c6a8 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -251,10 +251,15 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate // // Find an existing secure session given a peer's scoped NodeId and a type of session to match against. - // If matching against all types of sessions is desired, kUnDefined should be passed into type. + // If matching against all types of sessions is desired, kUndefined should be passed into type. // - void FindSecureSessionForNode(SessionHolder & sessionHolder, ScopedNodeId peerNodeId, - Transport::SecureSession::Type type = Transport::SecureSession::Type::kUndefined); + // If a valid session is found, an Optional with the value set to the SessionHandle of the session + // is returned. Otherwise, an Optional with no value set is returned. + // + // + Optional + FindSecureSessionForNode(ScopedNodeId peerNodeId, + Transport::SecureSession::Type type = Transport::SecureSession::Type::kUndefined); using SessionHandleCallback = bool (*)(void * context, SessionHandle & sessionHandle); CHIP_ERROR ForEachSessionHandle(void * context, SessionHandleCallback callback);