Fix Session ID Allocation

examples/tv-casting-app/linux/main.cpp

@@ -321,7 +321,6 @@ class TargetVideoPlayerInfo
         chip::DeviceProxyInitParams initParams = {
             .sessionManager = &(server->GetSecureSessionManager()),
             .exchangeMgr    = &(server->GetExchangeManager()),
-            .idAllocator    = &(server->GetSessionIDAllocator()),
             .fabricTable    = &(server->GetFabricTable()),
             .clientPool     = &gCASEClientPool,
         };

src/app/CASEClient.cpp

@@ -35,8 +35,8 @@ CHIP_ERROR CASEClient::EstablishSession(PeerId peer, const Transport::PeerAddres
     Optional<SessionHandle> session = mInitParams.sessionManager->CreateUnauthenticatedSession(peerAddress, mrpConfig);
     VerifyOrReturnError(session.HasValue(), CHIP_ERROR_NO_MEMORY);
 
-    uint16_t keyID = 0;
-    ReturnErrorOnFailure(mInitParams.idAllocator->Allocate(keyID));
+    SessionHolder secureSessionHolder = mInitParams.sessionManager->AllocateSession();
+    VerifyOrReturnError(secureSessionHolder, CHIP_ERROR_NO_MEMORY);
 
     // Allocate the exchange immediately before calling CASESession::EstablishSession.
     //
@@ -48,8 +48,8 @@ CHIP_ERROR CASEClient::EstablishSession(PeerId peer, const Transport::PeerAddres
     VerifyOrReturnError(exchange != nullptr, CHIP_ERROR_INTERNAL);
 
     mCASESession.SetGroupDataProvider(mInitParams.groupDataProvider);
-    ReturnErrorOnFailure(mCASESession.EstablishSession(peerAddress, mInitParams.fabricInfo, peer.GetNodeId(), keyID, exchange, this,
-                                                       mInitParams.mrpLocalConfig));
+    ReturnErrorOnFailure(mCASESession.EstablishSession(peerAddress, mInitParams.fabricInfo, peer.GetNodeId(), secureSessionHolder,
+                                                       exchange, this, mInitParams.mrpLocalConfig));
     mConnectionSuccessCallback = onConnection;
     mConnectionFailureCallback = onFailure;
     mConectionContext          = context;
@@ -61,8 +61,6 @@ CHIP_ERROR CASEClient::EstablishSession(PeerId peer, const Transport::PeerAddres
 
 void CASEClient::OnSessionEstablishmentError(CHIP_ERROR error)
 {
-    mInitParams.idAllocator->Free(mCASESession.GetLocalSessionId());
-
     if (mConnectionFailureCallback)
     {
         mConnectionFailureCallback(mConectionContext, this, error);

src/app/CASEClient.h

@@ -21,7 +21,6 @@
 #include <messaging/ExchangeMgr.h>
 #include <messaging/ReliableMessageProtocolConfig.h>
 #include <protocols/secure_channel/CASESession.h>
-#include <protocols/secure_channel/SessionIDAllocator.h>
 
 namespace chip {
 
@@ -34,7 +33,6 @@ struct CASEClientInitParams
 {
     SessionManager * sessionManager                    = nullptr;
     Messaging::ExchangeManager * exchangeMgr           = nullptr;
-    SessionIDAllocator * idAllocator                   = nullptr;
     FabricInfo * fabricInfo                            = nullptr;
     Credentials::GroupDataProvider * groupDataProvider = nullptr;
 

src/app/OperationalDeviceProxy.cpp

@@ -167,9 +167,9 @@ bool OperationalDeviceProxy::GetAddress(Inet::IPAddress & addr, uint16_t & port)
 
 CHIP_ERROR OperationalDeviceProxy::EstablishConnection()
 {
-    mCASEClient = mInitParams.clientPool->Allocate(
-        CASEClientInitParams{ mInitParams.sessionManager, mInitParams.exchangeMgr, mInitParams.idAllocator, mFabricInfo,
-                              mInitParams.groupDataProvider, mInitParams.mrpLocalConfig });
+    mCASEClient =
+        mInitParams.clientPool->Allocate(CASEClientInitParams{ mInitParams.sessionManager, mInitParams.exchangeMgr, mFabricInfo,
+                                                               mInitParams.groupDataProvider, mInitParams.mrpLocalConfig });
     ReturnErrorCodeIf(mCASEClient == nullptr, CHIP_ERROR_NO_MEMORY);
     CHIP_ERROR err =
         mCASEClient->EstablishSession(mPeerId, mDeviceAddress, mMRPConfig, HandleCASEConnected, HandleCASEConnectionFailure, this);

src/app/OperationalDeviceProxy.h

@@ -38,7 +38,6 @@
 #include <messaging/ExchangeMgr.h>
 #include <messaging/Flags.h>
 #include <protocols/secure_channel/CASESession.h>
-#include <protocols/secure_channel/SessionIDAllocator.h>
 #include <system/SystemLayer.h>
 #include <transport/SessionManager.h>
 #include <transport/TransportMgr.h>
@@ -51,7 +50,6 @@ struct DeviceProxyInitParams
 {
     SessionManager * sessionManager                    = nullptr;
     Messaging::ExchangeManager * exchangeMgr           = nullptr;
-    SessionIDAllocator * idAllocator                   = nullptr;
     FabricTable * fabricTable                          = nullptr;
     CASEClientPoolDelegate * clientPool                = nullptr;
     Credentials::GroupDataProvider * groupDataProvider = nullptr;
@@ -62,7 +60,6 @@ struct DeviceProxyInitParams
     {
         ReturnErrorCodeIf(sessionManager == nullptr, CHIP_ERROR_INCORRECT_STATE);
         ReturnErrorCodeIf(exchangeMgr == nullptr, CHIP_ERROR_INCORRECT_STATE);
-        ReturnErrorCodeIf(idAllocator == nullptr, CHIP_ERROR_INCORRECT_STATE);
         ReturnErrorCodeIf(fabricTable == nullptr, CHIP_ERROR_INCORRECT_STATE);
         ReturnErrorCodeIf(groupDataProvider == nullptr, CHIP_ERROR_INCORRECT_STATE);
         ReturnErrorCodeIf(clientPool == nullptr, CHIP_ERROR_INCORRECT_STATE);

src/app/server/CommissioningWindowManager.cpp

@@ -176,8 +176,8 @@ CHIP_ERROR CommissioningWindowManager::AdvertiseAndListenForPASE()
 {
     VerifyOrReturnError(mCommissioningTimeoutTimerArmed, CHIP_ERROR_INCORRECT_STATE);
 
-    uint16_t keyID = 0;
-    ReturnErrorOnFailure(mIDAllocator->Allocate(keyID));
+    SessionHolder secureSessionHolder = mServer->GetSecureSessionManager().AllocateSession();
+    VerifyOrReturnError(secureSessionHolder, CHIP_ERROR_NO_MEMORY);
 
     mPairingSession.Clear();
 
@@ -188,9 +188,9 @@ CHIP_ERROR CommissioningWindowManager::AdvertiseAndListenForPASE()
     if (mUseECM)
     {
         ReturnErrorOnFailure(SetTemporaryDiscriminator(mECMDiscriminator));
-        ReturnErrorOnFailure(
-            mPairingSession.WaitForPairing(mECMPASEVerifier, mECMIterations, ByteSpan(mECMSalt, mECMSaltLength), keyID,
-                                           Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
+        ReturnErrorOnFailure(mPairingSession.WaitForPairing(
+            mECMPASEVerifier, mECMIterations, ByteSpan(mECMSalt, mECMSaltLength), secureSessionHolder,
+            Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
     }
     else
     {
@@ -211,8 +211,9 @@ CHIP_ERROR CommissioningWindowManager::AdvertiseAndListenForPASE()
 
         ReturnErrorOnFailure(verifier.Deserialize(ByteSpan(serializedVerifier)));
 
-        ReturnErrorOnFailure(mPairingSession.WaitForPairing(
-            verifier, iterationCount, saltSpan, keyID, Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
+        ReturnErrorOnFailure(mPairingSession.WaitForPairing(verifier, iterationCount, saltSpan, secureSessionHolder,
+                                                            Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()),
+                                                            this));
     }
 
     ReturnErrorOnFailure(StartAdvertisement());

src/app/server/CommissioningWindowManager.h

@@ -23,7 +23,6 @@
 #include <lib/dnssd/Advertiser.h>
 #include <platform/CHIPDeviceConfig.h>
 #include <protocols/secure_channel/RendezvousParameters.h>
-#include <protocols/secure_channel/SessionIDAllocator.h>
 #include <system/SystemClock.h>
 
 namespace chip {
@@ -65,8 +64,6 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate, public a
 
     void SetAppDelegate(AppDelegate * delegate) { mAppDelegate = delegate; }
 
-    void SetSessionIDAllocator(SessionIDAllocator * idAllocator) { mIDAllocator = idAllocator; }
-
     /**
      * Open the pairing window using default configured parameters.
      */
@@ -146,7 +143,6 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate, public a
 
     bool mIsBLE = true;
 
-    SessionIDAllocator * mIDAllocator = nullptr;
     PASESession mPairingSession;
 
     uint8_t mFailedCommissioningAttempts = 0;

src/app/server/Server.cpp

@@ -111,7 +111,6 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
 
     SuccessOrExit(err = mCommissioningWindowManager.Init(this));
     mCommissioningWindowManager.SetAppDelegate(delegate);
-    mCommissioningWindowManager.SetSessionIDAllocator(&mSessionIDAllocator);
 
     // Set up attribute persistence before we try to bring up the data model
     // handler.
@@ -241,7 +240,6 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
         .sessionInitParams =  {
             .sessionManager    = &mSessions,
             .exchangeMgr       = &mExchangeMgr,
-            .idAllocator       = &mSessionIDAllocator,
             .fabricTable       = &mFabrics,
             .clientPool        = &mCASEClientPool,
             .groupDataProvider = &mGroupsProvider,

src/app/server/Server.h

@@ -80,8 +80,6 @@ class Server
 
     Messaging::ExchangeManager & GetExchangeManager() { return mExchangeMgr; }
 
-    SessionIDAllocator & GetSessionIDAllocator() { return mSessionIDAllocator; }
-
     SessionManager & GetSecureSessionManager() { return mSessions; }
 
     TransportMgrBase & GetTransportManager() { return mTransports; }
@@ -248,12 +246,10 @@ class Server
 
     Messaging::ExchangeManager mExchangeMgr;
     FabricTable mFabrics;
-    SessionIDAllocator mSessionIDAllocator;
     secure_channel::MessageCounterManager mMessageCounterManager;
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
     chip::Protocols::UserDirectedCommissioning::UserDirectedCommissioningClient gUDCClient;
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
-    SecurePairingUsingTestSecret mTestPairing;
     CommissioningWindowManager mCommissioningWindowManager;
 
     // Both PersistentStorageDelegate, and GroupDataProvider should be injected by the applications

src/app/tests/TestOperationalDeviceProxy.cpp

@@ -23,7 +23,6 @@
 #include <lib/support/UnitTestRegistration.h>
 #include <nlunit-test.h>
 #include <protocols/secure_channel/MessageCounterManager.h>
-#include <protocols/secure_channel/SessionIDAllocator.h>
 #include <system/SystemLayerImpl.h>
 #include <transport/SessionManager.h>
 #include <transport/TransportMgr.h>
@@ -56,7 +55,6 @@ void TestOperationalDeviceProxy_EstablishSessionDirectly(nlTestSuite * inSuite,
     VerifyOrDie(fabric != nullptr);
     secure_channel::MessageCounterManager messageCounterManager;
     chip::TestPersistentStorageDelegate deviceStorage;
-    SessionIDAllocator idAllocator;
     GroupDataProviderImpl groupDataProvider;
 
     systemLayer.Init();
@@ -72,7 +70,6 @@ void TestOperationalDeviceProxy_EstablishSessionDirectly(nlTestSuite * inSuite,
     DeviceProxyInitParams params = {
         .sessionManager    = &sessionManager,
         .exchangeMgr       = &exchangeMgr,
-        .idAllocator       = &idAllocator,
         .fabricInfo        = fabric,
         .groupDataProvider = &groupDataProvider,
     };

src/app/tests/integration/chip_im_initiator.cpp

@@ -433,6 +433,7 @@ CHIP_ERROR EstablishSecureSession()
 
     chip::SecurePairingUsingTestSecret * testSecurePairingSecret = chip::Platform::New<chip::SecurePairingUsingTestSecret>();
     VerifyOrExit(testSecurePairingSecret != nullptr, err = CHIP_ERROR_NO_MEMORY);
+    testSecurePairingSecret->Init(gSessionManager);
 
     // Attempt to connect to the peer.
     err = gSessionManager.NewPairing(gSession,

src/app/tests/integration/chip_im_responder.cpp

@@ -197,6 +197,7 @@ int main(int argc, char * argv[])
 
     InitializeEventLogging(&gExchangeManager);
 
+    gTestPairing.Init(gSessionManager);
     err = gSessionManager.NewPairing(gSession, peer, chip::kTestControllerNodeId, &gTestPairing,
                                      chip::CryptoContext::SessionRole::kResponder, gFabricIndex);
     SuccessOrExit(err);

src/controller/CHIPDeviceController.cpp

@@ -413,7 +413,6 @@ ControllerDeviceInitParams DeviceController::GetControllerDeviceInitParams()
         .exchangeMgr        = mSystemState->ExchangeMgr(),
         .udpEndPointManager = mSystemState->UDPEndPointManager(),
         .storageDelegate    = mStorageDelegate,
-        .idAllocator        = mSystemState->SessionIDAlloc(),
         .fabricsTable       = mSystemState->Fabrics(),
     };
 }
@@ -610,8 +609,7 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
 
     Messaging::ExchangeContext * exchangeCtxt = nullptr;
     Optional<SessionHandle> session;
-
-    uint16_t keyID = 0;
+    SessionHolder secureSessionHolder;
 
     VerifyOrExit(mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE);
     VerifyOrExit(mDeviceInPASEEstablishment == nullptr, err = CHIP_ERROR_INCORRECT_STATE);
@@ -677,8 +675,8 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
     session = mSystemState->SessionMgr()->CreateUnauthenticatedSession(params.GetPeerAddress(), device->GetMRPConfig());
     VerifyOrExit(session.HasValue(), err = CHIP_ERROR_NO_MEMORY);
 
-    err = mSystemState->SessionIDAlloc()->Allocate(keyID);
-    SuccessOrExit(err);
+    secureSessionHolder = mSystemState->SessionMgr()->AllocateSession();
+    VerifyOrExit(secureSessionHolder, err = CHIP_ERROR_NO_MEMORY);
 
     // TODO - Remove use of SetActive/IsActive from CommissioneeDeviceProxy
     device->SetActive(true);
@@ -692,7 +690,7 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
     exchangeCtxt = mSystemState->ExchangeMgr()->NewContext(session.Value(), &device->GetPairing());
     VerifyOrExit(exchangeCtxt != nullptr, err = CHIP_ERROR_INTERNAL);
 
-    err = device->GetPairing().Pair(params.GetPeerAddress(), params.GetSetupPINCode(), keyID,
+    err = device->GetPairing().Pair(params.GetPeerAddress(), params.GetSetupPINCode(), secureSessionHolder,
                                     Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), exchangeCtxt, this);
     SuccessOrExit(err);
 

src/controller/CHIPDeviceControllerFactory.cpp

@@ -214,14 +214,12 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
         chip::app::DnssdServer::Instance().StartServer();
     }
 
-    stateParams.sessionIDAllocator    = Platform::New<SessionIDAllocator>();
     stateParams.operationalDevicePool = Platform::New<DeviceControllerSystemStateParams::OperationalDevicePool>();
     stateParams.caseClientPool        = Platform::New<DeviceControllerSystemStateParams::CASEClientPool>();
 
     DeviceProxyInitParams deviceInitParams = {
         .sessionManager    = stateParams.sessionMgr,
         .exchangeMgr       = stateParams.exchangeMgr,
-        .idAllocator       = stateParams.sessionIDAllocator,
         .fabricTable       = stateParams.fabricTable,
         .clientPool        = stateParams.caseClientPool,
         .groupDataProvider = stateParams.groupDataProvider,
@@ -336,13 +334,8 @@ CHIP_ERROR DeviceControllerSystemState::Shutdown()
         mCASESessionManager = nullptr;
     }
 
-    // mSessionIDAllocator, mCASEClientPool, and mDevicePool must be deallocated
+    // mCASEClientPool and mDevicePool must be deallocated
     // after mCASESessionManager, which uses them.
-    if (mSessionIDAllocator != nullptr)
-    {
-        Platform::Delete(mSessionIDAllocator);
-        mSessionIDAllocator = nullptr;
-    }
 
     if (mOperationalDevicePool != nullptr)
     {

src/controller/CHIPDeviceControllerSystemState.h

@@ -36,7 +36,6 @@
 #include <lib/core/CHIPConfig.h>
 #include <protocols/secure_channel/CASEServer.h>
 #include <protocols/secure_channel/MessageCounterManager.h>
-#include <protocols/secure_channel/SessionIDAllocator.h>
 
 #include <transport/TransportMgr.h>
 #include <transport/raw/UDP.h>
@@ -88,7 +87,6 @@ struct DeviceControllerSystemStateParams
     FabricTable * fabricTable                                     = nullptr;
     CASEServer * caseServer                                       = nullptr;
     CASESessionManager * caseSessionManager                       = nullptr;
-    SessionIDAllocator * sessionIDAllocator                       = nullptr;
     OperationalDevicePool * operationalDevicePool                 = nullptr;
     CASEClientPool * caseClientPool                               = nullptr;
     Credentials::GroupDataProvider * groupDataProvider            = nullptr;
@@ -109,8 +107,8 @@ class DeviceControllerSystemState
         mUDPEndPointManager(params.udpEndPointManager), mTransportMgr(params.transportMgr), mSessionMgr(params.sessionMgr),
         mExchangeMgr(params.exchangeMgr), mMessageCounterManager(params.messageCounterManager), mFabrics(params.fabricTable),
         mCASEServer(params.caseServer), mCASESessionManager(params.caseSessionManager),
-        mSessionIDAllocator(params.sessionIDAllocator), mOperationalDevicePool(params.operationalDevicePool),
-        mCASEClientPool(params.caseClientPool), mGroupDataProvider(params.groupDataProvider)
+        mOperationalDevicePool(params.operationalDevicePool), mCASEClientPool(params.caseClientPool),
+        mGroupDataProvider(params.groupDataProvider)
     {
 #if CONFIG_NETWORK_LAYER_BLE
         mBleLayer = params.bleLayer;
@@ -143,8 +141,7 @@ class DeviceControllerSystemState
     {
         return mSystemLayer != nullptr && mUDPEndPointManager != nullptr && mTransportMgr != nullptr && mSessionMgr != nullptr &&
             mExchangeMgr != nullptr && mMessageCounterManager != nullptr && mFabrics != nullptr && mCASESessionManager != nullptr &&
-            mSessionIDAllocator != nullptr && mOperationalDevicePool != nullptr && mCASEClientPool != nullptr &&
-            mGroupDataProvider != nullptr;
+            mOperationalDevicePool != nullptr && mCASEClientPool != nullptr && mGroupDataProvider != nullptr;
     };
 
     System::Layer * SystemLayer() { return mSystemLayer; };
@@ -159,7 +156,6 @@ class DeviceControllerSystemState
     Ble::BleLayer * BleLayer() { return mBleLayer; };
 #endif
     CASESessionManager * CASESessionMgr() const { return mCASESessionManager; }
-    SessionIDAllocator * SessionIDAlloc() const { return mSessionIDAllocator; }
     Credentials::GroupDataProvider * GetGroupDataProvider() const { return mGroupDataProvider; }
 
 private:
@@ -178,7 +174,6 @@ class DeviceControllerSystemState
     FabricTable * mFabrics                                         = nullptr;
     CASEServer * mCASEServer                                       = nullptr;
     CASESessionManager * mCASESessionManager                       = nullptr;
-    SessionIDAllocator * mSessionIDAllocator                       = nullptr;
     OperationalDevicePool * mOperationalDevicePool                 = nullptr;
     CASEClientPool * mCASEClientPool                               = nullptr;
     Credentials::GroupDataProvider * mGroupDataProvider            = nullptr;

src/controller/CommissioneeDeviceProxy.h

@@ -39,7 +39,6 @@
 #include <messaging/ExchangeMgr.h>
 #include <messaging/Flags.h>
 #include <protocols/secure_channel/PASESession.h>
-#include <protocols/secure_channel/SessionIDAllocator.h>
 #include <transport/SessionHolder.h>
 #include <transport/SessionManager.h>
 #include <transport/TransportMgr.h>
@@ -69,7 +68,6 @@ struct ControllerDeviceInitParams
     Messaging::ExchangeManager * exchangeMgr                      = nullptr;
     Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager = nullptr;
     PersistentStorageDelegate * storageDelegate                   = nullptr;
-    SessionIDAllocator * idAllocator                              = nullptr;
 #if CONFIG_NETWORK_LAYER_BLE
     Ble::BleLayer * bleLayer = nullptr;
 #endif
@@ -120,7 +118,6 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat
         mExchangeMgr        = params.exchangeMgr;
         mUDPEndPointManager = params.udpEndPointManager;
         mFabricIndex        = fabric;
-        mIDAllocator        = params.idAllocator;
 #if CONFIG_NETWORK_LAYER_BLE
         mBleLayer = params.bleLayer;
 #endif
@@ -287,8 +284,6 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat
     CHIP_ERROR LoadSecureSessionParametersIfNeeded(bool & didLoad);
 
     FabricIndex mFabricIndex = kUndefinedFabricIndex;
-
-    SessionIDAllocator * mIDAllocator = nullptr;
 };
 
 } // namespace chip