Refactor SessionHandle (#13330)

* Fix EPS32 build warning

* app/ReadPrepareParams: store SessionHolder instead of SessionHandle

* Refactor SessionHandle

* Fix GetFabricIndex/GetPeerNodeId calls in general commissioning server

* Fix usage of . instead of -> (tested with esp32 compile)

* Remove the define of global for nxp compilation: conflicts with STL usage and does not seem to be utilized at least for matter  builds

* Restyle fixes

* Fix getting peer address compilation for exchange context

Co-authored-by: Andrei Litvin <andy314@gmail.com>

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

@@ -100,33 +100,22 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
         return CHIP_NO_ERROR;
     }
 
-    CHIP_ERROR InvokeGroupCommand(DeviceProxy * aDevice, GroupId groupId, const RequestType & aRequestData)
+    CHIP_ERROR InvokeGroupCommand(Messaging::ExchangeManager * exchangeManager, GroupId groupId, const RequestType & aRequestData)
     {
         app::CommandPathParams commandPath = { 0 /* endpoint */, groupId, RequestType::GetClusterId(), RequestType::GetCommandId(),
                                                (app::CommandPathFlags::kGroupIdValid) };
 
-        auto commandSender = Platform::MakeUnique<app::CommandSender>(this, aDevice->GetExchangeManager());
+        auto commandSender = Platform::MakeUnique<app::CommandSender>(this, exchangeManager);
         VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY);
 
         ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, aRequestData));
 
-        if (aDevice->GetSecureSession().HasValue())
+        Optional<SessionHandle> session = exchangeManager->GetSessionManager()->CreateGroupSession(groupId);
+        if (!session.HasValue())
         {
-            SessionHandle session = aDevice->GetSecureSession().Value();
-            session.SetGroupId(groupId);
-
-            if (!session.IsGroupSession())
-            {
-                return CHIP_ERROR_INCORRECT_STATE;
-            }
-
-            ReturnErrorOnFailure(commandSender->SendCommandRequest(session));
-        }
-        else
-        {
-            // something fishy is going on
-            return CHIP_ERROR_INCORRECT_STATE;
+            return CHIP_ERROR_NO_MEMORY;
         }
+        ReturnErrorOnFailure(commandSender->SendCommandRequest(session));
 
         commandSender.release();
         return CHIP_NO_ERROR;

examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp

@@ -114,7 +114,7 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c
     {
         // TODO: This uses the current node as the provider to supply the OTA image. This can be configurable such that the provider
         // supplying the response is not the provider supplying the OTA image.
-        FabricIndex fabricIndex = commandObj->GetExchangeContext()->GetSessionHandle().GetFabricIndex();
+        FabricIndex fabricIndex = commandObj->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
         FabricInfo * fabricInfo = Server::GetInstance().GetFabricTable().FindFabricWithIndex(fabricIndex);
         NodeId nodeId           = fabricInfo->GetPeerId().GetNodeId();
 

src/app/CASESessionManager.cpp

@@ -109,14 +109,6 @@ CHIP_ERROR CASESessionManager::GetPeerAddress(PeerId peerId, Transport::PeerAddr
     return CHIP_NO_ERROR;
 }
 
-void CASESessionManager::OnSessionReleased(const SessionHandle & sessionHandle)
-{
-    OperationalDeviceProxy * session = FindSession(sessionHandle);
-    VerifyOrReturn(session != nullptr, ChipLogDetail(Controller, "OnSessionReleased was called for unknown device, ignoring it."));
-
-    session->OnSessionReleased(sessionHandle);
-}
-
 OperationalDeviceProxy * CASESessionManager::FindSession(const SessionHandle & session)
 {
     return mConfig.devicePool->FindDevice(session);

src/app/CASESessionManager.h

@@ -48,7 +48,7 @@ struct CASESessionManagerConfig
  * 4. During session establishment, trigger node ID resolution (if needed), and update the DNS-SD cache (if resolution is
  * successful)
  */
-class CASESessionManager : public SessionReleaseDelegate, public Dnssd::ResolverDelegate
+class CASESessionManager : public Dnssd::ResolverDelegate
 {
 public:
     CASESessionManager() = delete;
@@ -105,9 +105,6 @@ class CASESessionManager : public SessionReleaseDelegate, public Dnssd::Resolver
      */
     CHIP_ERROR GetPeerAddress(PeerId peerId, Transport::PeerAddress & addr);
 
-    //////////// SessionReleaseDelegate Implementation ///////////////
-    void OnSessionReleased(const SessionHandle & session) override;
-
     //////////// ResolverDelegate Implementation ///////////////
     void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) override;
     void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override;

src/app/CommandHandler.cpp

@@ -436,7 +436,7 @@ TLV::TLVWriter * CommandHandler::GetCommandDataIBTLVWriter()
 
 FabricIndex CommandHandler::GetAccessingFabricIndex() const
 {
-    return mpExchangeCtx->GetSessionHandle().GetFabricIndex();
+    return mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
 }
 
 CommandHandler * CommandHandler::Handle::Get()

src/app/InteractionModelEngine.cpp

@@ -269,8 +269,8 @@ CHIP_ERROR InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeConte
     for (auto & readHandler : mReadHandlers)
     {
         if (!readHandler.IsFree() && readHandler.IsSubscriptionType() &&
-            readHandler.GetInitiatorNodeId() == apExchangeContext->GetSessionHandle().GetPeerNodeId() &&
-            readHandler.GetAccessingFabricIndex() == apExchangeContext->GetSessionHandle().GetFabricIndex())
+            readHandler.GetInitiatorNodeId() == apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId() &&
+            readHandler.GetAccessingFabricIndex() == apExchangeContext->GetSessionHandle()->AsSecureSession()->GetFabricIndex())
         {
             bool keepSubscriptions = true;
             System::PacketBufferTLVReader reader;

src/app/InteractionModelEngine.h

@@ -308,7 +308,7 @@ bool ServerClusterCommandExists(const ConcreteCommandPath & aCommandPath);
  *
  *  @param[in]    aSubjectDescriptor    The subject descriptor for the read.
  *  @param[in]    aPath                 The concrete path of the data being read.
- *  @param[in]    aAttributeReport      The TLV Builder for Cluter attribute builder.
+ *  @param[in]    aAttributeReports      The TLV Builder for Cluter attribute builder.
  *
  *  @retval  CHIP_NO_ERROR on success
  */

src/app/OperationalDeviceProxy.cpp

@@ -127,11 +127,7 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress
             return CHIP_NO_ERROR;
         }
 
-        Transport::SecureSession * secureSession = mInitParams.sessionManager->GetSecureSession(mSecureSession.Get());
-        if (secureSession != nullptr)
-        {
-            secureSession->SetPeerAddress(addr);
-        }
+        mSecureSession.Get()->AsSecureSession()->SetPeerAddress(addr);
     }
 
     return err;
@@ -262,7 +258,7 @@ CHIP_ERROR OperationalDeviceProxy::Disconnect()
     return CHIP_NO_ERROR;
 }
 
-void OperationalDeviceProxy::SetConnectedSession(SessionHandle handle)
+void OperationalDeviceProxy::SetConnectedSession(const SessionHandle & handle)
 {
     mSecureSession.Grab(handle);
     mState = State::SecureConnected;
@@ -296,12 +292,9 @@ void OperationalDeviceProxy::DeferCloseCASESession()
     mSystemLayer->ScheduleWork(CloseCASESessionTask, this);
 }
 
-void OperationalDeviceProxy::OnSessionReleased(const SessionHandle & session)
+void OperationalDeviceProxy::OnSessionReleased()
 {
-    VerifyOrReturn(mSecureSession.Contains(session),
-                   ChipLogDetail(Controller, "Connection expired, but it doesn't match the current session"));
     mState = State::Initialized;
-    mSecureSession.Release();
 }
 
 CHIP_ERROR OperationalDeviceProxy::ShutdownSubscriptions()

src/app/OperationalDeviceProxy.h

@@ -80,7 +80,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
 {
 public:
     virtual ~OperationalDeviceProxy();
-    OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId)
+    OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId) : mSecureSession(*this)
     {
         VerifyOrReturn(params.Validate() == CHIP_NO_ERROR);
 
@@ -124,7 +124,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
      *   Called when a connection is closing.
      *   The object releases all resources associated with the connection.
      */
-    void OnSessionReleased(const SessionHandle & session) override;
+    void OnSessionReleased() override;
 
     void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeResolutionData)
     {
@@ -150,7 +150,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
      *
      * Note: Avoid using this function generally as it is Deprecated
      */
-    void SetConnectedSession(SessionHandle handle);
+    void SetConnectedSession(const SessionHandle & handle);
 
     NodeId GetDeviceId() const override { return mPeerId.GetNodeId(); }
 
@@ -219,7 +219,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
 
     State mState = State::Uninitialized;
 
-    SessionHolder mSecureSession;
+    SessionHolderWithDelegate mSecureSession;
 
     uint8_t mSequenceNumber = 0;
 

src/app/ReadClient.cpp

@@ -182,16 +182,16 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams)
         ReturnErrorOnFailure(writer.Finalize(&msgBuf));
     }
 
-    mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHandle, this);
+    mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get(), this);
     VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);
 
     mpExchangeCtx->SetResponseTimeout(aReadPrepareParams.mTimeout);
 
     ReturnErrorOnFailure(mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf),
                                                     Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)));
 
-    mPeerNodeId  = aReadPrepareParams.mSessionHandle.GetPeerNodeId();
-    mFabricIndex = aReadPrepareParams.mSessionHandle.GetFabricIndex();
+    mPeerNodeId  = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetPeerNodeId();
+    mFabricIndex = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetFabricIndex();
 
     MoveToState(ClientState::AwaitingInitialReport);
 
@@ -576,8 +576,10 @@ CHIP_ERROR ReadClient::RefreshLivenessCheckTimer()
     CHIP_ERROR err = CHIP_NO_ERROR;
     CancelLivenessCheckTimer();
     VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(mpExchangeCtx->HasSessionHandle(), err = CHIP_ERROR_INCORRECT_STATE);
 
-    System::Clock::Timeout timeout = System::Clock::Seconds16(mMaxIntervalCeilingSeconds) + mpExchangeCtx->GetAckTimeout();
+    System::Clock::Timeout timeout =
+        System::Clock::Seconds16(mMaxIntervalCeilingSeconds) + mpExchangeCtx->GetSessionHandle()->GetAckTimeout();
     // EFR32/MBED/INFINION/K32W's chrono count return long unsinged, but other platform returns unsigned
     ChipLogProgress(DataManagement, "Refresh LivenessCheckTime with %lu milliseconds", static_cast<long unsigned>(timeout.count()));
     err = InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer(
@@ -699,21 +701,15 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara
 
     ReturnErrorOnFailure(writer.Finalize(&msgBuf));
 
-    mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHandle, this);
+    mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get(), this);
     VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);
-
     mpExchangeCtx->SetResponseTimeout(kImMessageTimeout);
-    if (mpExchangeCtx->IsBLETransport())
-    {
-        ChipLogError(DataManagement, "IM Subscribe cannot work with BLE");
-        return CHIP_ERROR_INCORRECT_STATE;
-    }
 
     ReturnErrorOnFailure(mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeRequest, std::move(msgBuf),
                                                     Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)));
 
-    mPeerNodeId  = aReadPrepareParams.mSessionHandle.GetPeerNodeId();
-    mFabricIndex = aReadPrepareParams.mSessionHandle.GetFabricIndex();
+    mPeerNodeId  = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetPeerNodeId();
+    mFabricIndex = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetFabricIndex();
 
     MoveToState(ClientState::AwaitingInitialReport);
 

src/app/ReadHandler.cpp

@@ -58,8 +58,8 @@ CHIP_ERROR ReadHandler::Init(Messaging::ExchangeManager * apExchangeMgr, Interac
     mActiveSubscription     = false;
     mIsChunkedReport        = false;
     mInteractionType        = aInteractionType;
-    mInitiatorNodeId        = apExchangeContext->GetSessionHandle().GetPeerNodeId();
-    mSubjectDescriptor      = apExchangeContext->GetSessionHandle().GetSubjectDescriptor();
+    mInitiatorNodeId        = apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId();
+    mSubjectDescriptor      = apExchangeContext->GetSessionHandle()->GetSubjectDescriptor();
     mHoldSync               = false;
     mLastWrittenEventsBytes = 0;
     if (apExchangeContext != nullptr)
@@ -201,12 +201,13 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b
     VerifyOrReturnLogError(IsReportable(), CHIP_ERROR_INCORRECT_STATE);
     if (IsPriming() || IsChunkedReport())
     {
-        mSessionHandle.SetValue(mpExchangeCtx->GetSessionHandle());
+        mSessionHandle.Grab(mpExchangeCtx->GetSessionHandle());
     }
     else
     {
         VerifyOrReturnLogError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE);
-        mpExchangeCtx = mpExchangeMgr->NewContext(mSessionHandle.Value(), this);
+        VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE);
+        mpExchangeCtx = mpExchangeMgr->NewContext(mSessionHandle.Get(), this);
         mpExchangeCtx->SetResponseTimeout(kImMessageTimeout);
     }
     VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE);

src/app/ReadHandler.h

@@ -216,7 +216,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
     uint64_t mSubscriptionId            = 0;
     uint16_t mMinIntervalFloorSeconds   = 0;
     uint16_t mMaxIntervalCeilingSeconds = 0;
-    Optional<SessionHandle> mSessionHandle;
+    SessionHolder mSessionHandle;
     bool mHoldReport         = false;
     bool mDirty              = false;
     bool mActiveSubscription = false;

src/app/ReadPrepareParams.h

@@ -29,7 +29,7 @@ namespace chip {
 namespace app {
 struct ReadPrepareParams
 {
-    SessionHandle mSessionHandle;
+    SessionHolder mSessionHolder;
     EventPathParams * mpEventPathParamsList         = nullptr;
     size_t mEventPathParamsListSize                 = 0;
     AttributePathParams * mpAttributePathParamsList = nullptr;
@@ -40,8 +40,8 @@ struct ReadPrepareParams
     uint16_t mMaxIntervalCeilingSeconds             = 0;
     bool mKeepSubscriptions                         = true;
 
-    ReadPrepareParams(const SessionHandle & sessionHandle) : mSessionHandle(sessionHandle) {}
-    ReadPrepareParams(ReadPrepareParams && other) : mSessionHandle(other.mSessionHandle)
+    ReadPrepareParams(const SessionHandle & sessionHandle) { mSessionHolder.Grab(sessionHandle); }
+    ReadPrepareParams(ReadPrepareParams && other) : mSessionHolder(other.mSessionHolder)
     {
         mKeepSubscriptions                 = other.mKeepSubscriptions;
         mpEventPathParamsList              = other.mpEventPathParamsList;
@@ -64,7 +64,7 @@ struct ReadPrepareParams
             return *this;
 
         mKeepSubscriptions                 = other.mKeepSubscriptions;
-        mSessionHandle                     = other.mSessionHandle;
+        mSessionHolder                     = other.mSessionHolder;
         mpEventPathParamsList              = other.mpEventPathParamsList;
         mEventPathParamsListSize           = other.mEventPathParamsListSize;
         mpAttributePathParamsList          = other.mpAttributePathParamsList;

src/app/WriteClient.cpp

@@ -245,7 +245,7 @@ CHIP_ERROR WriteClient::SendWriteRequest(const SessionHandle & session, System::
 exit:
     if (err != CHIP_NO_ERROR)
     {
-        ChipLogError(DataManagement, "Write client failed to SendWriteRequest");
+        ChipLogError(DataManagement, "Write client failed to SendWriteRequest: %s", ErrorStr(err));
         ClearExistingExchangeContext();
     }
     else
@@ -254,7 +254,7 @@ CHIP_ERROR WriteClient::SendWriteRequest(const SessionHandle & session, System::
         // handle this object dying (e.g. due to IM enging shutdown) while the
         // async bits are pending we'd need to malloc some state bit that we can
         // twiddle if we die.  For now just do the OnDone callback sync.
-        if (session.IsGroupSession())
+        if (session->IsGroupSession())
         {
             // Always shutdown on Group communication
             ChipLogDetail(DataManagement, "Closing on group Communication ");

src/app/WriteClient.h

@@ -117,7 +117,7 @@ class WriteClient : public Messaging::ExchangeDelegate
 
     NodeId GetSourceNodeId() const
     {
-        return mpExchangeCtx != nullptr ? mpExchangeCtx->GetSessionHandle().GetPeerNodeId() : kUndefinedNodeId;
+        return mpExchangeCtx != nullptr ? mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetPeerNodeId() : kUndefinedNodeId;
     }
 
 private:

src/app/WriteHandler.cpp

@@ -113,7 +113,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
     CHIP_ERROR err = CHIP_NO_ERROR;
 
     ReturnErrorCodeIf(mpExchangeCtx == nullptr, CHIP_ERROR_INTERNAL);
-    const Access::SubjectDescriptor subjectDescriptor = mpExchangeCtx->GetSessionHandle().GetSubjectDescriptor();
+    const Access::SubjectDescriptor subjectDescriptor = mpExchangeCtx->GetSessionHandle()->GetSubjectDescriptor();
 
     while (CHIP_NO_ERROR == (err = aAttributeDataIBsReader.Next()))
     {
@@ -279,7 +279,7 @@ CHIP_ERROR WriteHandler::AddStatus(const AttributePathParams & aAttributePathPar
 
 FabricIndex WriteHandler::GetAccessingFabricIndex() const
 {
-    return mpExchangeCtx->GetSessionHandle().GetFabricIndex();
+    return mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
 }
 
 const char * WriteHandler::GetStateStr() const

src/app/clusters/general-commissioning-server/general-commissioning-server.cpp

@@ -134,8 +134,9 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
      * This allows device to send messages back to commissioner.
      * Once bindings are implemented, this may no longer be needed.
      */
-    server->SetFabricIndex(commandObj->GetExchangeContext()->GetSessionHandle().GetFabricIndex());
-    server->SetPeerNodeId(commandObj->GetExchangeContext()->GetSessionHandle().GetPeerNodeId());
+    SessionHandle handle = commandObj->GetExchangeContext()->GetSessionHandle();
+    server->SetFabricIndex(handle->AsSecureSession()->GetFabricIndex());
+    server->SetPeerNodeId(handle->AsSecureSession()->GetPeerNodeId());
 
     CheckSuccess(server->CommissioningComplete(), Failure);
 

src/app/clusters/operational-credentials-server/operational-credentials-server.cpp

@@ -173,12 +173,8 @@ CHIP_ERROR ComputeAttestationSignature(app::CommandHandler * commandObj,
 
     // TODO: Create an alternative way to retrieve the Attestation Challenge without this huge amount of calls.
     // Retrieve attestation challenge
-    ByteSpan attestationChallenge = commandObj->GetExchangeContext()
-                                        ->GetExchangeMgr()
-                                        ->GetSessionManager()
-                                        ->GetSecureSession(commandObj->GetExchangeContext()->GetSessionHandle())
-                                        ->GetCryptoContext()
-                                        .GetAttestationChallenge();
+    ByteSpan attestationChallenge =
+        commandObj->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->GetCryptoContext().GetAttestationChallenge();
 
     Hash_SHA256_stream hashStream;
     ReturnErrorOnFailure(hashStream.Begin());
@@ -291,7 +287,7 @@ class FabricCleanupExchangeDelegate : public chip::Messaging::ExchangeDelegate
     void OnResponseTimeout(chip::Messaging::ExchangeContext * ec) override {}
     void OnExchangeClosing(chip::Messaging::ExchangeContext * ec) override
     {
-        FabricIndex currentFabricIndex = ec->GetSessionHandle().GetFabricIndex();
+        FabricIndex currentFabricIndex = ec->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
         ec->GetExchangeMgr()->GetSessionManager()->ExpireAllPairingsForFabric(currentFabricIndex);
     }
 };