From 51217153aa424b35b9ad020bca7f21b220cb1bfb Mon Sep 17 00:00:00 2001 From: Jerry Johns Date: Thu, 9 Jun 2022 21:46:47 -0700 Subject: [PATCH] [1/3] Reserve `SecureSession` for CASE establishment on the responder at init time (#19261) * Initial commit. * Review feedback * Review feedback * Minor integer promotion issue in a test * Linux build fix --- src/app/server/Server.cpp | 1 + .../CHIPDeviceControllerFactory.cpp | 1 + src/lib/core/CHIPConfig.h | 20 ++++--- src/protocols/secure_channel/CASEServer.cpp | 60 +++++++++++++++---- src/protocols/secure_channel/CASEServer.h | 37 +++++++++++- src/protocols/secure_channel/CASESession.cpp | 17 +++--- src/protocols/secure_channel/CASESession.h | 18 +++++- .../secure_channel/PairingSession.cpp | 4 +- src/protocols/secure_channel/PairingSession.h | 24 +++++++- .../secure_channel/tests/TestCASESession.cpp | 14 +++-- src/transport/SecureSessionTable.h | 2 +- src/transport/SessionManager.cpp | 8 ++- src/transport/SessionManager.h | 7 ++- src/transport/tests/TestPeerConnections.cpp | 10 ++-- src/transport/tests/TestSessionManager.cpp | 26 +++++--- 15 files changed, 194 insertions(+), 55 deletions(-) diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index abd7edb535baae..8143bceabba693 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -348,6 +348,7 @@ void Server::ScheduleFactoryReset() void Server::Shutdown() { + mCASEServer.Shutdown(); mCASESessionManager.Shutdown(); app::DnssdServer::Instance().SetCommissioningModeProvider(nullptr); chip::Dnssd::ServiceAdvertiser::Instance().Shutdown(); diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp index ee497e79611b0a..fa63a50f702b69 100644 --- a/src/controller/CHIPDeviceControllerFactory.cpp +++ b/src/controller/CHIPDeviceControllerFactory.cpp @@ -339,6 +339,7 @@ CHIP_ERROR DeviceControllerSystemState::Shutdown() if (mCASEServer != nullptr) { + mCASEServer->Shutdown(); chip::Platform::Delete(mCASEServer); mCASEServer = nullptr; } diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index c819dc92809586..8994cf7bc0f5e9 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -644,15 +644,21 @@ #endif // CHIP_CONFIG_UNAUTHENTICATED_CONNECTION_POOL_SIZE /** - * @def CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE + * @def CHIP_CONFIG_SECURE_SESSION_POOL_SIZE + * + * @brief Defines the size of the pool used for tracking the state of + * secure sessions. This controls the maximum number of concurrent + * established secure sessions across all supported transports. + * + * This is sized to cover the sum of the following: + * - At least 3 CASE sessions / fabric (Spec Ref: 4.13.2.8) + * - 1 reserved slot for CASEServer as a responder. + * - 1 reserved slot for PASE. * - * @brief Define the size of the pool used for tracking CHIP - * Peer connections. This defines maximum number of concurrent - * device connections across all supported transports. */ -#ifndef CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE -#define CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE 16 -#endif // CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE +#ifndef CHIP_CONFIG_SECURE_SESSION_POOL_SIZE +#define CHIP_CONFIG_SECURE_SESSION_POOL_SIZE (CHIP_CONFIG_MAX_FABRICS * 3 + 2) +#endif // CHIP_CONFIG_SECURE_SESSION_POOL_SIZE /** * @def CHIP_CONFIG_SECURE_SESSION_REFCOUNT_LOGGING diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index f7cab3a9c4236a..5751cd56850eac 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -45,7 +45,11 @@ CHIP_ERROR CASEServer::ListenForSessionEstablishment(Messaging::ExchangeManager mExchangeManager = exchangeManager; mGroupDataProvider = responderGroupDataProvider; - Cleanup(); + // Set up the group state provider that persists across all handshakes. + GetSession().SetGroupDataProvider(mGroupDataProvider); + + PrepareForSessionEstablishment(); + return CHIP_NO_ERROR; } @@ -53,12 +57,6 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec) { ReturnErrorCodeIf(ec == nullptr, CHIP_ERROR_INVALID_ARGUMENT); - // Setup CASE state machine using the credentials for the current fabric. - GetSession().SetGroupDataProvider(mGroupDataProvider); - ReturnErrorOnFailure(GetSession().ListenForSessionEstablishment( - *mSessionManager, mFabrics, mSessionResumptionStorage, mCertificateValidityPolicy, this, - Optional::Value(GetLocalMRPConfig()))); - // Hand over the exchange context to the CASE session. ec->SetDelegate(&GetSession()); @@ -90,12 +88,13 @@ CHIP_ERROR CASEServer::OnMessageReceived(Messaging::ExchangeContext * ec, const exit: if (err != CHIP_NO_ERROR) { - Cleanup(); + PrepareForSessionEstablishment(); } + return err; } -void CASEServer::Cleanup() +void CASEServer::PrepareForSessionEstablishment(const ScopedNodeId & previouslyEstablishedPeer) { // Let's re-register for CASE Sigma1 message, so that the next CASE session setup request can be processed. // https://github.com/project-chip/connectedhomeip/issues/8342 @@ -103,18 +102,57 @@ void CASEServer::Cleanup() mExchangeManager->RegisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::MsgType::CASE_Sigma1, this); GetSession().Clear(); + + // + // Indicate to the underlying CASE session to prepare for session establishment requests coming its way. This will + // involve allocating a SecureSession that will be held until it's needed for the next CASE session handshake. + // + // Logically speaking, we're attempting to evict a session using details of the just-established session (to ensure + // we're evicting sessions from the right fabric if needed) and then transferring the just established session into that + // slot (and thereby free'ing up the slot for the next session attempt). However, this transfer isn't necessary - just + // evicting a session will ensure it is available for the next attempt. + // + // This call can fail if we have run out memory to allocate SecureSessions. Continuing without taking any action + // however will render this node deaf to future handshake requests, so it's better to die here to raise attention to the problem + // / facilitate recovery. + // + // TODO(#17568): Once session eviction is actually in place, this call should NEVER fail and if so, is a logic bug. + // Dying here on failure is even more appropriate then. + // + VerifyOrDie(GetSession().PrepareForSessionEstablishment(*mSessionManager, mFabrics, mSessionResumptionStorage, + mCertificateValidityPolicy, this, previouslyEstablishedPeer, + Optional::Value(GetLocalMRPConfig())) == + CHIP_NO_ERROR); + + // + // PairingSession::mSecureSessionHolder is a weak-reference. If MarkForRemoval is called on this session, the session is + // going to get de-allocated from underneath us. This session that has just been allocated should *never* get evicted, and + // remain available till the next hand-shake is received. + // + // TODO: Converting SessionHolder to a true weak-ref and making PairingSession hold a strong-ref (#18397) would avoid this + // headache... + // + // Let's create a SessionHandle strong-reference to it to keep it resident. + // + mPinnedSecureSession = GetSession().CopySecureSession(); + + // + // If we've gotten this far, it means we have successfully allocated a SecureSession to back our next attempt. If we haven't, + // there is a bug somewhere and we should raise attention to it by dying. + // + VerifyOrDie(mPinnedSecureSession.HasValue()); } void CASEServer::OnSessionEstablishmentError(CHIP_ERROR err) { ChipLogError(Inet, "CASE Session establishment failed: %s", ErrorStr(err)); - Cleanup(); + PrepareForSessionEstablishment(); } void CASEServer::OnSessionEstablished(const SessionHandle & session) { ChipLogProgress(Inet, "CASE Session established to peer: " ChipLogFormatScopedNodeId, ChipLogValueScopedNodeId(session->GetPeer())); - Cleanup(); + PrepareForSessionEstablishment(session->GetPeer()); } } // namespace chip diff --git a/src/protocols/secure_channel/CASEServer.h b/src/protocols/secure_channel/CASEServer.h index f2a44ea33e1623..4f05b5dfc56511 100644 --- a/src/protocols/secure_channel/CASEServer.h +++ b/src/protocols/secure_channel/CASEServer.h @@ -31,12 +31,23 @@ class CASEServer : public SessionEstablishmentDelegate, { public: CASEServer() {} - ~CASEServer() override + ~CASEServer() override { Shutdown(); } + + /* + * This method will shutdown this object, releasing the strong reference to the pinned SecureSession object. + * It will also unregister the unsolicited handler and clear out the session object (which will release the weak + * reference through the underlying SessionHolder). + * + */ + void Shutdown() { if (mExchangeManager != nullptr) { mExchangeManager->UnregisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::MsgType::CASE_Sigma1); } + + GetSession().Clear(); + mPinnedSecureSession.ClearValue(); } CHIP_ERROR ListenForSessionEstablishment(Messaging::ExchangeManager * exchangeManager, SessionManager * sessionManager, @@ -64,6 +75,19 @@ class CASEServer : public SessionEstablishmentDelegate, SessionResumptionStorage * mSessionResumptionStorage = nullptr; Credentials::CertificateValidityPolicy * mCertificateValidityPolicy = nullptr; + // + // When we're in the process of establishing a session, this is used + // to maintain an additional, strong reference to the underlying SecureSession. + // This is because the existing reference in PairingSession is a weak one + // (i.e a SessionHolder) and can lose its reference if the session is evicted + // for any reason. + // + // This initially points to a session that is not yet active. Upon activation, it + // transfers ownership of the session to the SecureSessionManager and this reference + // is released before simultaneously acquiring ownership of a new SecureSession. + // + Optional mPinnedSecureSession; + CASESession mPairingSession; SessionManager * mSessionManager = nullptr; @@ -72,7 +96,16 @@ class CASEServer : public SessionEstablishmentDelegate, CHIP_ERROR InitCASEHandshake(Messaging::ExchangeContext * ec); - void Cleanup(); + /* + * This will clean up any state from a previous session establishment + * attempt (if any) and setup the machinery to listen for and handle + * any session handshakes there-after. + * + * If a session had previously been established successfully, previouslyEstablishedPeer + * should be set to the scoped node-id of the peer associated with that session. + * + */ + void PrepareForSessionEstablishment(const ScopedNodeId & previouslyEstablishedPeer = ScopedNodeId()); }; } // namespace chip diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index ee723e33912562..a47940b3805b46 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -151,7 +151,7 @@ void CASESession::Clear() } CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::CertificateValidityPolicy * policy, - SessionEstablishmentDelegate * delegate) + SessionEstablishmentDelegate * delegate, const ScopedNodeId & sessionEvictionHint) { VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(mGroupDataProvider != nullptr, CHIP_ERROR_INVALID_ARGUMENT); @@ -161,7 +161,7 @@ CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::Certi ReturnErrorOnFailure(mCommissioningHash.Begin()); mDelegate = delegate; - ReturnErrorOnFailure(AllocateSecureSession(sessionManager)); + ReturnErrorOnFailure(AllocateSecureSession(sessionManager, sessionEvictionHint)); mValidContext.Reset(); mValidContext.mRequiredKeyUsages.Set(KeyUsageFlags::kDigitalSignature); @@ -172,13 +172,14 @@ CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::Certi } CHIP_ERROR -CASESession::ListenForSessionEstablishment(SessionManager & sessionManager, FabricTable * fabrics, - SessionResumptionStorage * sessionResumptionStorage, - Credentials::CertificateValidityPolicy * policy, SessionEstablishmentDelegate * delegate, - Optional mrpConfig) +CASESession::PrepareForSessionEstablishment(SessionManager & sessionManager, FabricTable * fabrics, + SessionResumptionStorage * sessionResumptionStorage, + Credentials::CertificateValidityPolicy * policy, + SessionEstablishmentDelegate * delegate, ScopedNodeId previouslyEstablishedPeer, + Optional mrpConfig) { VerifyOrReturnError(fabrics != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - ReturnErrorOnFailure(Init(sessionManager, policy, delegate)); + ReturnErrorOnFailure(Init(sessionManager, policy, delegate, previouslyEstablishedPeer)); mRole = CryptoContext::SessionRole::kResponder; mFabricsTable = fabrics; @@ -207,7 +208,7 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric ReturnErrorCodeIf(fabricIndex == kUndefinedFabricIndex, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError((fabricInfo = fabricTable->FindFabricWithIndex(fabricIndex)) != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - err = Init(sessionManager, policy, delegate); + err = Init(sessionManager, policy, delegate, ScopedNodeId(peerNodeId, fabricIndex)); mRole = CryptoContext::SessionRole::kInitiator; diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h index eb2a94e28314f9..8e55b6fd8e821e 100644 --- a/src/protocols/secure_channel/CASESession.h +++ b/src/protocols/secure_channel/CASESession.h @@ -75,12 +75,17 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, * @param fabrics Table of fabrics that are currently configured on the device * @param policy Optional application-provided certificate validity policy * @param delegate Callback object + * @param previouslyEstablishedPeer If a session had previously been established successfully to a peer, this should + * be set to its scoped node-id. Else, this should be initialized to a + * default-constructed ScopedNodeId(). + * @param mrpConfig MRP configuration to encode into Sigma2. If not provided, it won't be encoded. * * @return CHIP_ERROR The result of initialization */ - CHIP_ERROR ListenForSessionEstablishment( + CHIP_ERROR PrepareForSessionEstablishment( SessionManager & sessionManager, FabricTable * fabrics, SessionResumptionStorage * sessionResumptionStorage, Credentials::CertificateValidityPolicy * policy, SessionEstablishmentDelegate * delegate, + ScopedNodeId previouslyEstablishedPeer, Optional mrpConfig = Optional::Missing()); /** @@ -181,8 +186,17 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, kFinishedViaResume = 7, }; + /* + * Initialize the object given a reference to the SessionManager, certificate validity policy and a delegate which will be + * notified of any further progress on this session. + * + * If we're either establishing or finished establishing a session to a peer in either initiator or responder + * roles, the node id of that peer should be provided in sessionEvictionHint. Else, it should be initialized + * to a default-constructed ScopedNodeId(). + * + */ CHIP_ERROR Init(SessionManager & sessionManager, Credentials::CertificateValidityPolicy * policy, - SessionEstablishmentDelegate * delegate); + SessionEstablishmentDelegate * delegate, const ScopedNodeId & sessionEvictionHint); // On success, sets mIpk to the correct value for outgoing Sigma1 based on internal state CHIP_ERROR RecoverInitiatorIpk(); diff --git a/src/protocols/secure_channel/PairingSession.cpp b/src/protocols/secure_channel/PairingSession.cpp index 259f66c53be98e..b3721ca8533a45 100644 --- a/src/protocols/secure_channel/PairingSession.cpp +++ b/src/protocols/secure_channel/PairingSession.cpp @@ -23,9 +23,9 @@ namespace chip { -CHIP_ERROR PairingSession::AllocateSecureSession(SessionManager & sessionManager) +CHIP_ERROR PairingSession::AllocateSecureSession(SessionManager & sessionManager, const ScopedNodeId & sessionEvictionHint) { - auto handle = sessionManager.AllocateSession(GetSecureSessionType()); + auto handle = sessionManager.AllocateSession(GetSecureSessionType(), sessionEvictionHint); VerifyOrReturnError(handle.HasValue(), CHIP_ERROR_NO_MEMORY); VerifyOrReturnError(mSecureSessionHolder.GrabPairingSession(handle.Value()), CHIP_ERROR_INTERNAL); mSessionManager = &sessionManager; diff --git a/src/protocols/secure_channel/PairingSession.h b/src/protocols/secure_channel/PairingSession.h index 25e0c9f95bd06b..bc9d75892b2439 100644 --- a/src/protocols/secure_channel/PairingSession.h +++ b/src/protocols/secure_channel/PairingSession.h @@ -63,11 +63,27 @@ class DLL_EXPORT PairingSession : public SessionDelegate return localSessionId; } + /** + * Copy the underlying session (if present) into a SessionHandle that a caller can use to + * obtain a reference to the session. + */ + Optional CopySecureSession() + { + if (mSecureSessionHolder) + { + VerifyOrDie(mSecureSessionHolder->GetSessionType() == Transport::Session::SessionType::kSecure); + return MakeOptional(*mSecureSessionHolder->AsSecureSession()); + } + + return Optional::Missing(); + } + uint16_t GetPeerSessionId() const { VerifyOrDie(mPeerSessionId.HasValue()); return mPeerSessionId.Value(); } + bool IsValidPeerSessionId() const { return mPeerSessionId.HasValue(); } /** @@ -93,10 +109,14 @@ class DLL_EXPORT PairingSession : public SessionDelegate * Allocate a secure session object from the passed session manager for the * pending session establishment operation. * - * @param sessionManager session manager from which to allocate a secure session object + * @param sessionManager Session manager from which to allocate a secure session object + * @param sessionEvictionHint If we're either establishing or just finished establishing a session to a peer in either + * initiator or responder roles, the node id of that peer should be provided in this argument. Else, it should be initialized to + * a default-constructed ScopedNodeId(). + * * @return CHIP_ERROR The outcome of the allocation attempt */ - CHIP_ERROR AllocateSecureSession(SessionManager & sessionManager); + CHIP_ERROR AllocateSecureSession(SessionManager & sessionManager, const ScopedNodeId & sessionEvictionHint = ScopedNodeId()); CHIP_ERROR ActivateSecureSession(const Transport::PeerAddress & peerAddress); diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp index 816682767df078..378f1fce7ae4fb 100644 --- a/src/protocols/secure_channel/tests/TestCASESession.cpp +++ b/src/protocols/secure_channel/tests/TestCASESession.cpp @@ -198,13 +198,14 @@ void CASE_SecurePairingWaitTest(nlTestSuite * inSuite, void * inContext) pairing.SetGroupDataProvider(&gDeviceGroupDataProvider); NL_TEST_ASSERT(inSuite, - pairing.ListenForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, nullptr) == + pairing.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, nullptr, ScopedNodeId()) == CHIP_ERROR_INVALID_ARGUMENT); NL_TEST_ASSERT(inSuite, - pairing.ListenForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, &delegate) == + pairing.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, &delegate, ScopedNodeId()) == CHIP_ERROR_INVALID_ARGUMENT); NL_TEST_ASSERT(inSuite, - pairing.ListenForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate) == CHIP_NO_ERROR); + pairing.PrepareForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate, ScopedNodeId()) == + CHIP_NO_ERROR); } void CASE_SecurePairingStartTest(nlTestSuite * inSuite, void * inContext) @@ -283,9 +284,9 @@ void CASE_SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inConte pairingAccessory.SetGroupDataProvider(&gDeviceGroupDataProvider); NL_TEST_ASSERT(inSuite, - pairingAccessory.ListenForSessionEstablishment(sessionManager, &gDeviceFabrics, nullptr, nullptr, - &delegateAccessory, - MakeOptional(verySleepyAccessoryRmpConfig)) == CHIP_NO_ERROR); + pairingAccessory.PrepareForSessionEstablishment(sessionManager, &gDeviceFabrics, nullptr, nullptr, + &delegateAccessory, ScopedNodeId(), + MakeOptional(verySleepyAccessoryRmpConfig)) == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, pairingCommissioner.EstablishSession(sessionManager, &gCommissionerFabrics, gCommissionerFabricIndex, Node01_01, contextCommissioner, nullptr, nullptr, &delegateCommissioner, @@ -847,6 +848,7 @@ int CASE_TestSecurePairing_Setup(void * inContext) */ int CASE_TestSecurePairing_Teardown(void * inContext) { + gPairingServer.Shutdown(); gCommissionerStorageDelegate.ClearStorage(); gDeviceStorageDelegate.ClearStorage(); gCommissionerFabrics.DeleteAllFabrics(); diff --git a/src/transport/SecureSessionTable.h b/src/transport/SecureSessionTable.h index cf1fb0c9b6cbf6..3fcbf86cd14abe 100644 --- a/src/transport/SecureSessionTable.h +++ b/src/transport/SecureSessionTable.h @@ -214,7 +214,7 @@ class SecureSessionTable return NullOptional; } - BitMapObjectPool mEntries; + BitMapObjectPool mEntries; uint16_t mNextSessionId = 0; }; diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 0298e4595f09ab..a9e95411e778aa 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -387,8 +387,14 @@ void SessionManager::ExpireAllPASEPairings() }); } -Optional SessionManager::AllocateSession(SecureSession::Type secureSessionType) +Optional SessionManager::AllocateSession(SecureSession::Type secureSessionType, + const ScopedNodeId & sessionEvictionHint) { + // + // This is currently not being utilized yet but will be once session eviction logic is added. + // + (void) sessionEvictionHint; + return mSecureSessions.CreateNewSecureSession(secureSessionType); } diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index a3ed1f5b440474..1b01f1d5bcb5e7 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -160,10 +160,15 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate * Allocate a secure session and non-colliding session ID in the secure * session table. * + * If we're either establishing or just finished establishing a session to a peer in either initiator or responder + * roles, the node id of that peer should be provided in sessionEvictionHint. Else, it should be initialized + * to a default-constructed ScopedNodeId(). + * * @return SessionHandle with a reference to a SecureSession, else NullOptional on failure */ CHECK_RETURN_VALUE - Optional AllocateSession(Transport::SecureSession::Type secureSessionType); + Optional AllocateSession(Transport::SecureSession::Type secureSessionType, + const ScopedNodeId & sessionEvictionHint); void ExpirePairing(const SessionHandle & session); void ExpireAllPairings(const ScopedNodeId & node); diff --git a/src/transport/tests/TestPeerConnections.cpp b/src/transport/tests/TestPeerConnections.cpp index b7a156fa307909..b7227c52f287c5 100644 --- a/src/transport/tests/TestPeerConnections.cpp +++ b/src/transport/tests/TestPeerConnections.cpp @@ -67,7 +67,7 @@ void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) System::Clock::Internal::SetSystemClockForTesting(&clock); clock.SetMonotonic(100_ms64); CATValues peerCATs; - Optional sessions[CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE]; + Optional sessions[CHIP_CONFIG_SECURE_SESSION_POOL_SIZE]; // First node, peer session id 1, local session id 2 auto optionalSession = connections.CreateNewSecureSessionForTest(SecureSession::Type::kCASE, 2, kLocalNodeId, kCasePeer1NodeId, @@ -97,11 +97,11 @@ void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) // This define guard will be needed when migrating SecureSessionTable to ObjectPool //#if !CHIP_SYSTEM_CONFIG_POOL_USE_HEAP // If not using a heap, we can fill the SecureSessionTable - for (uint16_t i = 2; i < CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE; ++i) + for (int i = 2; i < CHIP_CONFIG_SECURE_SESSION_POOL_SIZE; ++i) { - sessions[i] = - connections.CreateNewSecureSessionForTest(SecureSession::Type::kCASE, static_cast(i + 6u), kLocalNodeId, - kCasePeer2NodeId, kPeer2CATs, 3, kFabricIndex, GetLocalMRPConfig()); + sessions[i] = connections.CreateNewSecureSessionForTest(SecureSession::Type::kCASE, + static_cast(static_cast(i) + 6u), kLocalNodeId, + kCasePeer2NodeId, kPeer2CATs, 3, kFabricIndex, GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, sessions[i].HasValue()); } diff --git a/src/transport/tests/TestSessionManager.cpp b/src/transport/tests/TestSessionManager.cpp index ccf751bf594420..a665391af4a25a 100644 --- a/src/transport/tests/TestSessionManager.cpp +++ b/src/transport/tests/TestSessionManager.cpp @@ -656,7 +656,9 @@ static void RandomSessionIdAllocatorOffset(nlTestSuite * inSuite, SessionManager const int bound = rand() % max; for (int i = 0; i < bound; ++i) { - auto handle = sessionManager.AllocateSession(Transport::SecureSession::Type::kPASE); + auto handle = sessionManager.AllocateSession( + Transport::SecureSession::Type::kPASE, + ScopedNodeId(NodeIdFromPAKEKeyId(kDefaultCommissioningPasscodeId), kUndefinedFabricIndex)); NL_TEST_ASSERT(inSuite, handle.HasValue()); sessionManager.ExpirePairing(handle.Value()); } @@ -669,7 +671,9 @@ void SessionAllocationTest(nlTestSuite * inSuite, void * inContext) // Allocate a session. uint16_t sessionId1; { - auto handle = sessionManager.AllocateSession(Transport::SecureSession::Type::kPASE); + auto handle = sessionManager.AllocateSession( + Transport::SecureSession::Type::kPASE, + ScopedNodeId(NodeIdFromPAKEKeyId(kDefaultCommissioningPasscodeId), kUndefinedFabricIndex)); NL_TEST_ASSERT(inSuite, handle.HasValue()); SessionHolder session; session.GrabPairingSession(handle.Value()); @@ -681,7 +685,9 @@ void SessionAllocationTest(nlTestSuite * inSuite, void * inContext) auto prevSessionId = sessionId1; for (uint32_t i = 0; i < 10; ++i) { - auto handle = sessionManager.AllocateSession(Transport::SecureSession::Type::kPASE); + auto handle = sessionManager.AllocateSession( + Transport::SecureSession::Type::kPASE, + ScopedNodeId(NodeIdFromPAKEKeyId(kDefaultCommissioningPasscodeId), kUndefinedFabricIndex)); if (!handle.HasValue()) { break; @@ -702,7 +708,9 @@ void SessionAllocationTest(nlTestSuite * inSuite, void * inContext) // sessions are immediately freed. for (uint32_t i = 0; i < UINT16_MAX + 10; ++i) { - auto handle = sessionManager.AllocateSession(Transport::SecureSession::Type::kPASE); + auto handle = sessionManager.AllocateSession( + Transport::SecureSession::Type::kPASE, + ScopedNodeId(NodeIdFromPAKEKeyId(kDefaultCommissioningPasscodeId), kUndefinedFabricIndex)); NL_TEST_ASSERT(inSuite, handle.HasValue()); auto sessionId = handle.Value()->AsSecureSession()->GetLocalSessionId(); NL_TEST_ASSERT(inSuite, sessionId - prevSessionId == 1 || (sessionId == 1 && prevSessionId == 65535)); @@ -717,13 +725,15 @@ void SessionAllocationTest(nlTestSuite * inSuite, void * inContext) { // Allocate some session handles at pseudo-random offsets in the session // ID space. - constexpr size_t numHandles = CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE - 1; + constexpr size_t numHandles = CHIP_CONFIG_SECURE_SESSION_POOL_SIZE - 1; Optional handles[numHandles]; uint16_t sessionIds[numHandles]; for (size_t h = 0; h < numHandles; ++h) { constexpr int maxOffset = 5000; - handles[h] = sessionManager.AllocateSession(Transport::SecureSession::Type::kPASE); + handles[h] = sessionManager.AllocateSession( + Transport::SecureSession::Type::kPASE, + ScopedNodeId(NodeIdFromPAKEKeyId(kDefaultCommissioningPasscodeId), kUndefinedFabricIndex)); NL_TEST_ASSERT(inSuite, handles[h].HasValue()); sessionIds[h] = handles[h].Value()->AsSecureSession()->GetLocalSessionId(); RandomSessionIdAllocatorOffset(inSuite, sessionManager, maxOffset); @@ -739,7 +749,9 @@ void SessionAllocationTest(nlTestSuite * inSuite, void * inContext) // these collide either. for (int j = 0; j < UINT16_MAX; ++j) { - auto handle = sessionManager.AllocateSession(Transport::SecureSession::Type::kPASE); + auto handle = sessionManager.AllocateSession( + Transport::SecureSession::Type::kPASE, + ScopedNodeId(NodeIdFromPAKEKeyId(kDefaultCommissioningPasscodeId), kUndefinedFabricIndex)); NL_TEST_ASSERT(inSuite, handle.HasValue()); auto potentialCollision = handle.Value()->AsSecureSession()->GetLocalSessionId(); for (size_t h = 0; h < numHandles; ++h)