diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 9c9cbc627bdd91..84b0dbe85195fa 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -65,13 +65,11 @@ class SecureSession }; SecureSession(Type secureSessionType, uint16_t localSessionId, NodeId peerNodeId, CATValues peerCATs, uint16_t peerSessionId, - FabricIndex fabric, const ReliableMessageProtocolConfig & config, System::Clock::Timestamp currentTime) : + FabricIndex fabric, const ReliableMessageProtocolConfig & config) : mSecureSessionType(secureSessionType), mPeerNodeId(peerNodeId), mPeerCATs(peerCATs), mLocalSessionId(localSessionId), mPeerSessionId(peerSessionId), - mFabric(fabric), mMRPConfig(config) - { - SetLastActivityTime(currentTime); - } + mFabric(fabric), mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()), mMRPConfig(config) + {} SecureSession(SecureSession &&) = delete; SecureSession(const SecureSession &) = delete; @@ -95,7 +93,7 @@ class SecureSession FabricIndex GetFabricIndex() const { return mFabric; } System::Clock::Timestamp GetLastActivityTime() const { return mLastActivityTime; } - void SetLastActivityTime(System::Clock::Timestamp value) { mLastActivityTime = value; } + void MarkActive() { mLastActivityTime = System::SystemClock().GetMonotonicTimestamp(); } CryptoContext & GetCryptoContext() { return mCryptoContext; } diff --git a/src/transport/SecureSessionTable.h b/src/transport/SecureSessionTable.h index 00392e70d9a479..a680ae050a0641 100644 --- a/src/transport/SecureSessionTable.h +++ b/src/transport/SecureSessionTable.h @@ -37,7 +37,7 @@ constexpr const uint16_t kAnyKeyId = 0xffff; * - handle session active time and expiration * - allocate and free space for sessions. */ -template +template class SecureSessionTable { public: @@ -64,8 +64,7 @@ class SecureSessionTable CATValues peerCATs, uint16_t peerSessionId, FabricIndex fabric, const ReliableMessageProtocolConfig & config) { - return mEntries.CreateObject(secureSessionType, localSessionId, peerNodeId, peerCATs, peerSessionId, fabric, config, - mTimeSource.GetMonotonicTimestamp()); + return mEntries.CreateObject(secureSessionType, localSessionId, peerNodeId, peerCATs, peerSessionId, fabric, config); } void ReleaseSession(SecureSession * session) { mEntries.ReleaseObject(session); } @@ -98,9 +97,6 @@ class SecureSessionTable return result; } - /// Convenience method to mark a session as active - void MarkSessionActive(SecureSession * state) { state->SetLastActivityTime(mTimeSource.GetMonotonicTimestamp()); } - /** * Iterates through all active sessions and expires any sessions with an idle time * larger than the given amount. @@ -110,9 +106,8 @@ class SecureSessionTable template void ExpireInactiveSessions(System::Clock::Timestamp maxIdleTime, Callback callback) { - const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); mEntries.ForEachActiveObject([&](auto session) { - if (session->GetLastActivityTime() + maxIdleTime < currentTime) + if (session->GetLastActivityTime() + maxIdleTime < System::SystemClock().GetMonotonicTimestamp()) { callback(*session); ReleaseSession(session); @@ -121,11 +116,7 @@ class SecureSessionTable }); } - /// Allows access to the underlying time source used for keeping track of session active time - Time::TimeSource & GetTimeSource() { return mTimeSource; } - private: - Time::TimeSource mTimeSource; BitMapObjectPool mEntries; }; diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 8eacaadaa66d79..cee779e8c09860 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -226,7 +226,7 @@ CHIP_ERROR SessionManager::SendPreparedMessage(const SessionHandle & sessionHand } // This marks any connection where we send data to as 'active' - mSecureSessions.MarkSessionActive(session); + session->MarkActive(); destination = &session->GetPeerAddress(); @@ -241,7 +241,7 @@ CHIP_ERROR SessionManager::SendPreparedMessage(const SessionHandle & sessionHand else { auto unauthenticated = sessionHandle.GetUnauthenticatedSession(); - mUnauthenticatedSessions.MarkSessionActive(unauthenticated); + unauthenticated->MarkActive(); destination = &unauthenticated->GetPeerAddress(); ChipLogProgress(Inet, @@ -439,7 +439,7 @@ void SessionManager::MessageDispatch(const PacketHeader & packetHeader, const Tr } VerifyOrDie(err == CHIP_NO_ERROR); - mUnauthenticatedSessions.MarkSessionActive(session); + session->MarkActive(); PayloadHeader payloadHeader; ReturnOnFailure(payloadHeader.DecodeAndConsume(msg)); @@ -502,7 +502,7 @@ void SessionManager::SecureUnicastMessageDispatch(const PacketHeader & packetHea return; } - mSecureSessions.MarkSessionActive(session); + session->MarkActive(); if (isDuplicate == SessionMessageDelegate::DuplicateMessage::Yes && !payloadHeader.NeedsAck()) { @@ -628,7 +628,8 @@ void SessionManager::ExpiryTimerCallback(System::Layer * layer, void * param) // TODO(#2279): session expiration is currently disabled until rekeying is supported // the #ifdef should be removed after that. mgr->mSecureSessions.ExpireInactiveSessions( - CHIP_PEER_CONNECTION_TIMEOUT_MS, [this](const Transport::SecureSession & state1) { HandleConnectionExpired(state1); }); + System::SystemClock().GetMonotonicTimestamp(), System::Clock::Milliseconds32(CHIP_PEER_CONNECTION_TIMEOUT_MS), + [this](const Transport::SecureSession & state1) { HandleConnectionExpired(state1); }); #endif mgr->ScheduleExpiryTimer(); // re-schedule the oneshot timer } diff --git a/src/transport/UnauthenticatedSessionTable.h b/src/transport/UnauthenticatedSessionTable.h index 55c1e126ec003a..c4ca79f1396cf1 100644 --- a/src/transport/UnauthenticatedSessionTable.h +++ b/src/transport/UnauthenticatedSessionTable.h @@ -49,7 +49,7 @@ class UnauthenticatedSession : public ReferenceCounted +template class UnauthenticatedSessionTable { public: @@ -114,15 +113,6 @@ class UnauthenticatedSessionTable } } - /// Mark a session as active - void MarkSessionActive(UnauthenticatedSessionHandle session) - { - session->SetLastActivityTime(mTimeSource.GetMonotonicTimestamp()); - } - - /// Allows access to the underlying time source used for keeping track of session active time - Time::TimeSource & GetTimeSource() { return mTimeSource; } - private: /** * Allocates a new session out of the internal resource pool. @@ -224,7 +214,6 @@ class UnauthenticatedSessionTable return false; } - Time::TimeSource mTimeSource; BitMapObjectPool mEntries; }; diff --git a/src/transport/tests/TestPeerConnections.cpp b/src/transport/tests/TestPeerConnections.cpp index ce603e28a86592..66569e023c8159 100644 --- a/src/transport/tests/TestPeerConnections.cpp +++ b/src/transport/tests/TestPeerConnections.cpp @@ -60,11 +60,33 @@ const CATValues kPeer1CATs = { { 0xABCD0001, 0xABCE0100, 0xABCD0020 } }; const CATValues kPeer2CATs = { { 0xABCD0012, kUndefinedCAT, kUndefinedCAT } }; const CATValues kPeer3CATs; +class MockClock : public System::Clock::ClockBase +{ +public: + System::Clock::Microseconds64 GetMonotonicMicroseconds64() override { return timeSource.GetMonotonicTimestamp(); } + System::Clock::Milliseconds64 GetMonotonicMilliseconds64() override { return timeSource.GetMonotonicTimestamp(); } + CHIP_ERROR GetClock_RealTime(System::Clock::Microseconds64 & aCurTime) override { return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; } + CHIP_ERROR GetClock_RealTimeMS(System::Clock::Milliseconds64 & aCurTime) override + { + return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; + } + CHIP_ERROR SetClock_RealTime(System::Clock::Microseconds64 aNewCurTime) override { return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; } + + System::Clock::Timestamp GetMonotonicTimestamp() { return timeSource.GetMonotonicTimestamp(); } + void SetMonotonicTimestamp(System::Clock::Timestamp value) { timeSource.SetMonotonicTimestamp(value); } + +private: + Time::TimeSource timeSource; +}; + void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) { SecureSession * statePtr; - SecureSessionTable<2, Time::Source::kTest> connections; - connections.GetTimeSource().SetMonotonicTimestamp(100_ms64); + SecureSessionTable<2> connections; + MockClock clock; + System::Clock::ClockBase * realClock = &System::SystemClock(); + System::Clock::Internal::SetSystemClockForTesting(&clock); + clock.SetMonotonicTimestamp(100_ms64); CATValues peerCATs; // Node ID 1, peer key 1, local key 2 @@ -90,12 +112,16 @@ void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) statePtr = connections.CreateNewSecureSession(kPeer3SessionType, 6, kPeer3NodeId, kPeer3CATs, 5, 0 /* fabricIndex */, gDefaultMRPConfig); NL_TEST_ASSERT(inSuite, statePtr == nullptr); + System::Clock::Internal::SetSystemClockForTesting(realClock); } void TestFindByKeyId(nlTestSuite * inSuite, void * inContext) { SecureSession * statePtr; - SecureSessionTable<2, Time::Source::kTest> connections; + SecureSessionTable<2> connections; + MockClock clock; + System::Clock::ClockBase * realClock = &System::SystemClock(); + System::Clock::Internal::SetSystemClockForTesting(&clock); // Node ID 1, peer key 1, local key 2 statePtr = connections.CreateNewSecureSession(kPeer1SessionType, 2, kPeer1NodeId, kPeer1CATs, 1, 0 /* fabricIndex */, @@ -112,6 +138,8 @@ void TestFindByKeyId(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(3)); NL_TEST_ASSERT(inSuite, connections.FindSecureSessionByLocalKey(4)); + + System::Clock::Internal::SetSystemClockForTesting(realClock); } struct ExpiredCallInfo @@ -125,9 +153,13 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) { ExpiredCallInfo callInfo; SecureSession * statePtr; - SecureSessionTable<2, Time::Source::kTest> connections; + SecureSessionTable<2> connections; + + MockClock clock; + System::Clock::ClockBase * realClock = &System::SystemClock(); + System::Clock::Internal::SetSystemClockForTesting(&clock); - connections.GetTimeSource().SetMonotonicTimestamp(100_ms64); + clock.SetMonotonicTimestamp(100_ms64); // Node ID 1, peer key 1, local key 2 statePtr = connections.CreateNewSecureSession(kPeer1SessionType, 2, kPeer1NodeId, kPeer1CATs, 1, 0 /* fabricIndex */, @@ -135,7 +167,7 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, statePtr != nullptr); statePtr->SetPeerAddress(kPeer1Addr); - connections.GetTimeSource().SetMonotonicTimestamp(200_ms64); + clock.SetMonotonicTimestamp(200_ms64); // Node ID 2, peer key 3, local key 4 statePtr = connections.CreateNewSecureSession(kPeer2SessionType, 4, kPeer2NodeId, kPeer2CATs, 3, 0 /* fabricIndex */, gDefaultMRPConfig); @@ -143,7 +175,7 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) statePtr->SetPeerAddress(kPeer2Addr); // cannot add before expiry - connections.GetTimeSource().SetMonotonicTimestamp(300_ms64); + clock.SetMonotonicTimestamp(300_ms64); statePtr = connections.CreateNewSecureSession(kPeer3SessionType, 6, kPeer3NodeId, kPeer3CATs, 5, 0 /* fabricIndex */, gDefaultMRPConfig); NL_TEST_ASSERT(inSuite, statePtr == nullptr); @@ -160,24 +192,24 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(2)); // now that the connections were expired, we can add peer3 - connections.GetTimeSource().SetMonotonicTimestamp(300_ms64); + clock.SetMonotonicTimestamp(300_ms64); // Node ID 3, peer key 5, local key 6 statePtr = connections.CreateNewSecureSession(kPeer3SessionType, 6, kPeer3NodeId, kPeer3CATs, 5, 0 /* fabricIndex */, gDefaultMRPConfig); NL_TEST_ASSERT(inSuite, statePtr != nullptr); statePtr->SetPeerAddress(kPeer3Addr); - connections.GetTimeSource().SetMonotonicTimestamp(400_ms64); + clock.SetMonotonicTimestamp(400_ms64); NL_TEST_ASSERT(inSuite, statePtr = connections.FindSecureSessionByLocalKey(4)); - connections.MarkSessionActive(statePtr); - NL_TEST_ASSERT(inSuite, statePtr->GetLastActivityTime() == connections.GetTimeSource().GetMonotonicTimestamp()); + statePtr->MarkActive(); + NL_TEST_ASSERT(inSuite, statePtr->GetLastActivityTime() == clock.GetMonotonicTimestamp()); // At this time: // Peer 3 active at time 300 // Peer 2 active at time 400 - connections.GetTimeSource().SetMonotonicTimestamp(500_ms64); + clock.SetMonotonicTimestamp(500_ms64); callInfo.callCount = 0; connections.ExpireInactiveSessions(150_ms64, [&callInfo](const SecureSession & state) { callInfo.callCount++; @@ -202,7 +234,7 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(6)); // peer 1 and 2 are active - connections.GetTimeSource().SetMonotonicTimestamp(1000_ms64); + clock.SetMonotonicTimestamp(1000_ms64); callInfo.callCount = 0; connections.ExpireInactiveSessions(100_ms64, [&callInfo](const SecureSession & state) { callInfo.callCount++; @@ -213,6 +245,8 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(2)); NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(4)); NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(6)); + + System::Clock::Internal::SetSystemClockForTesting(realClock); } } // namespace