Using SessionHolder to store a session (#12871)

examples/shell/shell_common/cmd_ping.cpp

@@ -249,8 +249,8 @@ CHIP_ERROR EstablishSecureSession(streamer_t * stream, const Transport::PeerAddr
     peerAddr = Optional<Transport::PeerAddress>::Value(peerAddress);
 
     // Attempt to connect to the peer.
-    err = gSessionManager.NewPairing(peerAddr, kTestDeviceNodeId, testSecurePairingSecret, CryptoContext::SessionRole::kInitiator,
-                                     gFabricIndex);
+    err = gSessionManager.NewPairing(gSession, peerAddr, kTestDeviceNodeId, testSecurePairingSecret,
+                                     CryptoContext::SessionRole::kInitiator, gFabricIndex);
 
 exit:
     if (err != CHIP_NO_ERROR)
@@ -329,7 +329,7 @@ void StartPinging(streamer_t * stream, char * destination)
     err = EstablishSecureSession(stream, GetEchoPeerAddress());
     SuccessOrExit(err);
 
-    err = gEchoClient.Init(&gExchangeManager, SessionHandle(kTestDeviceNodeId, 1, 1, gFabricIndex));
+    err = gEchoClient.Init(&gExchangeManager, gSession.Get());
     SuccessOrExit(err);
 
     // Arrange to get a callback whenever an Echo Response is received.

examples/shell/shell_common/cmd_send.cpp

@@ -128,7 +128,7 @@ CHIP_ERROR SendMessage(streamer_t * stream)
     uint32_t payloadSize = gSendArguments.GetPayloadSize();
 
     // Create a new exchange context.
-    auto * ec = gExchangeManager.NewContext(SessionHandle(kTestDeviceNodeId, 1, 1, gFabricIndex), &gMockAppDelegate);
+    auto * ec = gExchangeManager.NewContext(gSession.Get(), &gMockAppDelegate);
     VerifyOrExit(ec != nullptr, err = CHIP_ERROR_NO_MEMORY);
 
     payloadBuf = MessagePacketBuffer::New(payloadSize);
@@ -181,8 +181,8 @@ CHIP_ERROR EstablishSecureSession(streamer_t * stream, Transport::PeerAddress &
     peerAddr = Optional<Transport::PeerAddress>::Value(peerAddress);
 
     // Attempt to connect to the peer.
-    err = gSessionManager.NewPairing(peerAddr, kTestDeviceNodeId, testSecurePairingSecret, CryptoContext::SessionRole::kInitiator,
-                                     gFabricIndex);
+    err = gSessionManager.NewPairing(gSession, peerAddr, kTestDeviceNodeId, testSecurePairingSecret,
+                                     CryptoContext::SessionRole::kInitiator, gFabricIndex);
 
 exit:
     if (err != CHIP_NO_ERROR)

examples/shell/shell_common/globals.cpp

@@ -21,6 +21,7 @@ chip::secure_channel::MessageCounterManager gMessageCounterManager;
 chip::Messaging::ExchangeManager gExchangeManager;
 chip::SessionManager gSessionManager;
 chip::Inet::IPAddress gDestAddr;
+chip::SessionHolder gSession;
 
 chip::FabricIndex gFabricIndex = 0;
 

examples/shell/shell_common/include/Globals.h

@@ -20,6 +20,7 @@
 #include <lib/core/CHIPCore.h>
 #include <messaging/ExchangeMgr.h>
 #include <protocols/secure_channel/MessageCounterManager.h>
+#include <transport/SessionHolder.h>
 #include <transport/SessionManager.h>
 #include <transport/raw/TCP.h>
 #include <transport/raw/UDP.h>
@@ -34,6 +35,7 @@ extern chip::secure_channel::MessageCounterManager gMessageCounterManager;
 extern chip::Messaging::ExchangeManager gExchangeManager;
 extern chip::SessionManager gSessionManager;
 extern chip::Inet::IPAddress gDestAddr;
+extern chip::SessionHolder gSession;
 
 extern chip::FabricIndex gFabricIndex;
 

src/app/CASEClient.cpp

@@ -73,18 +73,16 @@ void CASEClient::OnSessionEstablished()
     }
 }
 
-CHIP_ERROR CASEClient::DeriveSecureSessionHandle(Optional<SessionHandle> & handle)
+CHIP_ERROR CASEClient::DeriveSecureSessionHandle(SessionHolder & handle)
 {
     CHIP_ERROR err = mInitParams.sessionManager->NewPairing(
-        Optional<Transport::PeerAddress>::Value(mPeerAddress), mPeerId.GetNodeId(), &mCASESession,
+        handle, Optional<Transport::PeerAddress>::Value(mPeerAddress), mPeerId.GetNodeId(), &mCASESession,
         CryptoContext::SessionRole::kInitiator, mInitParams.fabricInfo->GetFabricIndex());
     if (err != CHIP_NO_ERROR)
     {
         ChipLogError(Controller, "Failed in setting up CASE secure channel: err %s", ErrorStr(err));
         return err;
     }
-    handle.SetValue(SessionHandle(mPeerId.GetNodeId(), mCASESession.GetLocalSessionId(), mCASESession.GetPeerSessionId(),
-                                  mInitParams.fabricInfo->GetFabricIndex()));
 
     return CHIP_NO_ERROR;
 }

src/app/CASEClient.h

@@ -55,7 +55,7 @@ class DLL_EXPORT CASEClient : public SessionEstablishmentDelegate
 
     void OnSessionEstablished() override;
 
-    CHIP_ERROR DeriveSecureSessionHandle(Optional<SessionHandle> & handle);
+    CHIP_ERROR DeriveSecureSessionHandle(SessionHolder & handle);
 
 private:
     CASEClientInitParams mInitParams;

src/app/OperationalDeviceProxy.cpp

@@ -118,7 +118,7 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress
     }
     else
     {
-        if (!mSecureSession.HasValue())
+        if (!mSecureSession)
         {
             // Nothing needs to be done here.  It's not an error to not have a
             // secureSession.  For one thing, we could have gotten an different
@@ -127,7 +127,7 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress
             return CHIP_NO_ERROR;
         }
 
-        Transport::SecureSession * secureSession = mInitParams.sessionManager->GetSecureSession(mSecureSession.Value());
+        Transport::SecureSession * secureSession = mInitParams.sessionManager->GetSecureSession(mSecureSession.Get());
         if (secureSession != nullptr)
         {
             secureSession->SetPeerAddress(addr);
@@ -249,9 +249,9 @@ void OperationalDeviceProxy::HandleCASEConnected(void * context, CASEClient * cl
 CHIP_ERROR OperationalDeviceProxy::Disconnect()
 {
     ReturnErrorCodeIf(mState != State::SecureConnected, CHIP_ERROR_INCORRECT_STATE);
-    if (mSecureSession.HasValue())
+    if (mSecureSession)
     {
-        mInitParams.sessionManager->ExpirePairing(mSecureSession.Value());
+        mInitParams.sessionManager->ExpirePairing(mSecureSession.Get());
     }
     mState = State::Initialized;
     if (mCASEClient)
@@ -292,10 +292,10 @@ void OperationalDeviceProxy::DeferCloseCASESession()
 
 void OperationalDeviceProxy::OnSessionReleased(SessionHandle session)
 {
-    VerifyOrReturn(mSecureSession.HasValue() && mSecureSession.Value() == session,
+    VerifyOrReturn(mSecureSession.Contains(session),
                    ChipLogDetail(Controller, "Connection expired, but it doesn't match the current session"));
     mState = State::Initialized;
-    mSecureSession.ClearValue();
+    mSecureSession.Release();
 }
 
 CHIP_ERROR OperationalDeviceProxy::ShutdownSubscriptions()

src/app/OperationalDeviceProxy.h

@@ -155,7 +155,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
 
     PeerId GetPeerId() const { return mPeerId; }
 
-    bool MatchesSession(SessionHandle session) const { return mSecureSession.HasValue() && mSecureSession.Value() == session; }
+    bool MatchesSession(SessionHandle session) const { return mSecureSession.Contains(session); }
 
     uint8_t GetNextSequenceNumber() override { return mSequenceNumber++; };
 
@@ -165,7 +165,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
 
     Messaging::ExchangeManager * GetExchangeManager() const override { return mInitParams.exchangeMgr; }
 
-    chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession; }
+    chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession.ToOptional(); }
 
     bool GetAddress(Inet::IPAddress & addr, uint16_t & port) const override;
 
@@ -210,7 +210,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
 
     State mState = State::Uninitialized;
 
-    Optional<SessionHandle> mSecureSession = Optional<SessionHandle>::Missing();
+    SessionHolder mSecureSession;
 
     uint8_t mSequenceNumber = 0;
 

src/app/server/CommissioningWindowManager.cpp

@@ -132,8 +132,9 @@ void CommissioningWindowManager::OnSessionEstablishmentStarted()
 void CommissioningWindowManager::OnSessionEstablished()
 {
     DeviceLayer::SystemLayer().CancelTimer(HandleSessionEstablishmentTimeout, this);
+    SessionHolder sessionHolder;
     CHIP_ERROR err = mServer->GetSecureSessionManager().NewPairing(
-        Optional<Transport::PeerAddress>::Value(mPairingSession.GetPeerAddress()), mPairingSession.GetPeerNodeId(),
+        sessionHolder, Optional<Transport::PeerAddress>::Value(mPairingSession.GetPeerAddress()), mPairingSession.GetPeerNodeId(),
         &mPairingSession, CryptoContext::SessionRole::kResponder, 0);
     if (err != CHIP_NO_ERROR)
     {

src/app/server/Server.cpp

@@ -281,13 +281,15 @@ CHIP_ERROR Server::AddTestCommissioning()
     CHIP_ERROR err            = CHIP_NO_ERROR;
     PASESession * testSession = nullptr;
     PASESessionSerializable serializedTestSession;
+    SessionHolder session;
 
     mTestPairing.ToSerializable(serializedTestSession);
 
     testSession = chip::Platform::New<PASESession>();
     testSession->FromSerializable(serializedTestSession);
-    SuccessOrExit(err = mSessions.NewPairing(Optional<PeerAddress>{ PeerAddress::Uninitialized() }, chip::kTestControllerNodeId,
-                                             testSession, CryptoContext::SessionRole::kResponder, kMinValidFabricIndex));
+    SuccessOrExit(err = mSessions.NewPairing(session, Optional<PeerAddress>{ PeerAddress::Uninitialized() },
+                                             chip::kTestControllerNodeId, testSession, CryptoContext::SessionRole::kResponder,
+                                             kMinValidFabricIndex));
 
 exit:
     if (testSession)

src/app/tests/integration/chip_im_initiator.cpp

@@ -247,7 +247,7 @@ CHIP_ERROR SendCommandRequest(std::unique_ptr<chip::app::CommandSender> && comma
     err = commandSender->FinishCommand();
     SuccessOrExit(err);
 
-    err = commandSender->SendCommandRequest(gSessionManager.FindSecureSessionForNode(chip::kTestDeviceNodeId), gMessageTimeout);
+    err = commandSender->SendCommandRequest(gSession.Get(), gMessageTimeout);
     SuccessOrExit(err);
 
     gCommandCount++;
@@ -283,7 +283,7 @@ CHIP_ERROR SendBadCommandRequest(std::unique_ptr<chip::app::CommandSender> && co
     err = commandSender->FinishCommand();
     SuccessOrExit(err);
 
-    err = commandSender->SendCommandRequest(gSessionManager.FindSecureSessionForNode(chip::kTestDeviceNodeId), gMessageTimeout);
+    err = commandSender->SendCommandRequest(gSession.Get(), gMessageTimeout);
     SuccessOrExit(err);
     gCommandCount++;
     commandSender.release();
@@ -311,7 +311,7 @@ CHIP_ERROR SendReadRequest()
 
     printf("\nSend read request message to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId);
 
-    chip::app::ReadPrepareParams readPrepareParams(chip::SessionHandle(chip::kTestDeviceNodeId, 1, 1, gFabricIndex));
+    chip::app::ReadPrepareParams readPrepareParams(gSession.Get());
     readPrepareParams.mTimeout                     = gMessageTimeout;
     readPrepareParams.mpAttributePathParamsList    = &attributePathParams;
     readPrepareParams.mAttributePathParamsListSize = 1;
@@ -352,8 +352,7 @@ CHIP_ERROR SendWriteRequest(chip::app::WriteClientHandle & apWriteClient)
     SuccessOrExit(err =
                       writer->PutBoolean(chip::TLV::ContextTag(chip::to_underlying(chip::app::AttributeDataIB::Tag::kData)), true));
     SuccessOrExit(err = apWriteClient->FinishAttribute());
-    SuccessOrExit(
-        err = apWriteClient.SendWriteRequest(gSessionManager.FindSecureSessionForNode(chip::kTestDeviceNodeId), gMessageTimeout));
+    SuccessOrExit(err = apWriteClient.SendWriteRequest(gSession.Get(), gMessageTimeout));
 
     gWriteCount++;
 
@@ -370,7 +369,7 @@ CHIP_ERROR SendSubscribeRequest()
     CHIP_ERROR err   = CHIP_NO_ERROR;
     gLastMessageTime = chip::System::SystemClock().GetMonotonicTimestamp();
 
-    chip::app::ReadPrepareParams readPrepareParams(chip::SessionHandle(chip::kTestDeviceNodeId, 1, 1, gFabricIndex));
+    chip::app::ReadPrepareParams readPrepareParams(gSession.Get());
     chip::app::EventPathParams eventPathParams[2];
     chip::app::AttributePathParams attributePathParams[1];
     readPrepareParams.mpEventPathParamsList                = eventPathParams;
@@ -416,7 +415,8 @@ CHIP_ERROR EstablishSecureSession()
     VerifyOrExit(testSecurePairingSecret != nullptr, err = CHIP_ERROR_NO_MEMORY);
 
     // Attempt to connect to the peer.
-    err = gSessionManager.NewPairing(chip::Optional<chip::Transport::PeerAddress>::Value(
+    err = gSessionManager.NewPairing(gSession,
+                                     chip::Optional<chip::Transport::PeerAddress>::Value(
                                          chip::Transport::PeerAddress::UDP(gDestAddr, CHIP_PORT, chip::Inet::InterfaceId::Null())),
                                      chip::kTestDeviceNodeId, testSecurePairingSecret, chip::CryptoContext::SessionRole::kInitiator,
                                      gFabricIndex);

src/app/tests/integration/chip_im_responder.cpp

@@ -213,8 +213,8 @@ int main(int argc, char * argv[])
 
     InitializeEventLogging(&gExchangeManager);
 
-    err = gSessionManager.NewPairing(peer, chip::kTestControllerNodeId, &gTestPairing, chip::CryptoContext::SessionRole::kResponder,
-                                     gFabricIndex);
+    err = gSessionManager.NewPairing(gSession, peer, chip::kTestControllerNodeId, &gTestPairing,
+                                     chip::CryptoContext::SessionRole::kResponder, gFabricIndex);
     SuccessOrExit(err);
 
     printf("Listening for IM requests...\n");

src/app/tests/integration/common.cpp

@@ -33,6 +33,7 @@
 chip::Messaging::ExchangeManager gExchangeManager;
 chip::SessionManager gSessionManager;
 chip::secure_channel::MessageCounterManager gMessageCounterManager;
+chip::SessionHolder gSession;
 
 void InitializeChip(void)
 {

src/app/tests/integration/common.h

@@ -27,13 +27,15 @@
 #include <app/util/basic-types.h>
 #include <messaging/ExchangeMgr.h>
 #include <protocols/secure_channel/MessageCounterManager.h>
+#include <transport/SessionHolder.h>
 
 #define MAX_MESSAGE_SOURCE_STR_LENGTH (100)
 #define NETWORK_SLEEP_TIME_MSECS (100 * 1000)
 
 extern chip::Messaging::ExchangeManager gExchangeManager;
 extern chip::SessionManager gSessionManager;
 extern chip::secure_channel::MessageCounterManager gMessageCounterManager;
+extern chip::SessionHolder gSession;
 
 constexpr chip::NodeId kTestNodeId         = 0x1ULL;
 constexpr chip::NodeId kTestNodeId1        = 0x2ULL;

src/controller/CHIPDeviceController.cpp

@@ -619,7 +619,6 @@ CHIP_ERROR DeviceCommissioner::Init(CommissionerInitParams params)
 {
     ReturnErrorOnFailure(DeviceController::Init(params));
 
-    params.systemState->SessionMgr()->RegisterCreationDelegate(*this);
     params.systemState->SessionMgr()->RegisterReleaseDelegate(*this);
     params.systemState->SessionMgr()->RegisterRecoveryDelegate(*this);
 
@@ -684,17 +683,6 @@ CHIP_ERROR DeviceCommissioner::Shutdown()
     return CHIP_NO_ERROR;
 }
 
-void DeviceCommissioner::OnNewSession(SessionHandle session)
-{
-    VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnNewConnection was called in incorrect state"));
-
-    CommissioneeDeviceProxy * device =
-        FindCommissioneeDevice(mSystemState->SessionMgr()->GetSecureSession(session)->GetPeerNodeId());
-    VerifyOrReturn(device != nullptr, ChipLogDetail(Controller, "OnNewConnection was called for unknown device, ignoring it."));
-
-    device->OnNewConnection(session);
-}
-
 void DeviceCommissioner::OnSessionReleased(SessionHandle session)
 {
     VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnConnectionExpired was called in incorrect state"));
@@ -1013,9 +1001,9 @@ void DeviceCommissioner::OnSessionEstablished()
     // TODO: the session should know which peer we are trying to connect to when started
     pairing->SetPeerNodeId(mDeviceBeingCommissioned->GetDeviceId());
 
-    CHIP_ERROR err = mSystemState->SessionMgr()->NewPairing(Optional<Transport::PeerAddress>::Value(pairing->GetPeerAddress()),
-                                                            pairing->GetPeerNodeId(), pairing,
-                                                            CryptoContext::SessionRole::kInitiator, mFabricIndex);
+    CHIP_ERROR err = mSystemState->SessionMgr()->NewPairing(
+        mDeviceBeingCommissioned->GetSecureSessionHolder(), Optional<Transport::PeerAddress>::Value(pairing->GetPeerAddress()),
+        pairing->GetPeerNodeId(), pairing, CryptoContext::SessionRole::kInitiator, mFabricIndex);
     if (err != CHIP_NO_ERROR)
     {
         ChipLogError(Controller, "Failed in setting up secure channel: err %s", ErrorStr(err));
@@ -1186,12 +1174,9 @@ CHIP_ERROR DeviceCommissioner::ValidateAttestationInfo(const ByteSpan & attestat
 
     DeviceAttestationVerifier * dac_verifier = GetDeviceAttestationVerifier();
 
-    PASESession * pairing = &mDeviceBeingCommissioned->GetPairing();
-
     // Retrieve attestation challenge
     ByteSpan attestationChallenge = mSystemState->SessionMgr()
-                                        ->GetSecureSession({ pairing->GetPeerNodeId(), pairing->GetLocalSessionId(),
-                                                             pairing->GetPeerSessionId(), mFabricIndex })
+                                        ->GetSecureSession(mDeviceBeingCommissioned->GetSecureSession().Value())
                                         ->GetCryptoContext()
                                         .GetAttestationChallenge();
 

src/controller/CHIPDeviceController.h

@@ -441,7 +441,6 @@ class DLL_EXPORT DeviceController : public SessionReleaseDelegate,
  *   will be stored.
  */
 class DLL_EXPORT DeviceCommissioner : public DeviceController,
-                                      public SessionCreationDelegate,
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY // make this commissioner discoverable
                                       public Protocols::UserDirectedCommissioning::InstanceNameResolver,
                                       public Protocols::UserDirectedCommissioning::UserConfirmationProvider,
@@ -698,8 +697,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
 
     void OnSessionEstablishmentTimeout();
 
-    //////////// SessionCreationDelegate Implementation ///////////////
-    void OnNewSession(SessionHandle session) override;
     //////////// SessionReleaseDelegate Implementation ///////////////
     void OnSessionReleased(SessionHandle session) override;