From 81c67eeea2abb5250befa43aefb152cc639038af Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Fri, 21 Jul 2023 08:48:26 -0400 Subject: [PATCH] [ICD]Add SAT Common TXT key (#28123) * Add SAT key to the commissionable node adv * Add tests fix some issues * Update after rebase * Fix lint complaint * Fix Darwin Tidy issue, attempt to try Linux mdnsPlatform.Advertise tests * address naming comments * Restyled by clang-format * Fix unit test on linux * address comment --------- Co-authored-by: Restyled.io --- src/app/ReadClient.cpp | 2 +- src/app/icd/ICDManager.cpp | 5 +- src/controller/tests/data_model/TestRead.cpp | 2 +- src/lib/address_resolve/tool.cpp | 1 + src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 29 ++-- src/lib/dnssd/Discovery_ImplPlatform.cpp | 45 ++++-- src/lib/dnssd/Resolver.h | 5 +- src/lib/dnssd/TxtFields.cpp | 19 ++- src/lib/dnssd/TxtFields.h | 15 +- .../minimal_mdns/tests/CheckOnlyServer.h | 2 +- .../minimal_mdns/tests/TestAdvertiser.cpp | 9 +- src/lib/dnssd/platform/tests/TestPlatform.cpp | 6 +- src/lib/dnssd/tests/TestTxtFields.cpp | 128 ++++++++++++++---- .../ReliableMessageProtocolConfig.cpp | 18 ++- src/messaging/ReliableMessageProtocolConfig.h | 19 ++- src/platform/fake/DnssdImpl.h | 4 +- src/protocols/secure_channel/CASESession.cpp | 17 ++- .../secure_channel/PairingSession.cpp | 26 +++- .../secure_channel/tests/TestCASESession.cpp | 10 +- .../secure_channel/tests/TestPASESession.cpp | 8 +- .../tests/TestPairingSession.cpp | 2 +- src/system/SystemClock.h | 5 + src/tracing/json/json_tracing.cpp | 1 + src/transport/SecureSession.h | 5 +- src/transport/Session.h | 2 - src/transport/UnauthenticatedSessionTable.h | 5 +- .../tests/TestSecureSessionTable.cpp | 3 +- 27 files changed, 284 insertions(+), 109 deletions(-) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 11cabe42dabc44..93b6448f56f774 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -832,7 +832,7 @@ CHIP_ERROR ReadClient::ComputeLivenessCheckTimerTimeout(System::Clock::Timeout * const auto & ourMrpConfig = GetDefaultMRPConfig(); auto publisherTransmissionTimeout = GetRetransmissionTimeout(ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout, - System::SystemClock().GetMonotonicTimestamp(), Transport::kMinActiveTime); + System::SystemClock().GetMonotonicTimestamp(), ourMrpConfig.mActiveThresholdTime); *aTimeout = System::Clock::Seconds16(mMaxInterval) + publisherTransmissionTimeout; return CHIP_NO_ERROR; } diff --git a/src/app/icd/ICDManager.cpp b/src/app/icd/ICDManager.cpp index 6ab66b2e252975..f400a7e5801087 100644 --- a/src/app/icd/ICDManager.cpp +++ b/src/app/icd/ICDManager.cpp @@ -35,7 +35,7 @@ using namespace chip::app; using namespace chip::app::Clusters; using namespace chip::app::Clusters::IcdManagement; -void ICDManager::ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable) +void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable) { VerifyOrDie(storage != nullptr); VerifyOrDie(fabricTable != nullptr); @@ -44,11 +44,12 @@ void ICDManager::ICDManager::Init(PersistentStorageDelegate * storage, FabricTab uint32_t activeModeInterval = IcdManagementServer::GetInstance().GetActiveModeInterval(); VerifyOrDie(kFastPollingInterval.count() < activeModeInterval); + UpdateIcdMode(); UpdateOperationState(OperationalState::ActiveMode); } -void ICDManager::ICDManager::Shutdown() +void ICDManager::Shutdown() { // cancel any running timer of the icd DeviceLayer::SystemLayer().CancelTimer(OnIdleModeDone, this); diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 5c59c6c1501488..30120eca8ad794 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -4651,7 +4651,7 @@ System::Clock::Timeout TestReadInteraction::ComputeSubscriptionTimeout(System::C const auto & ourMrpConfig = GetDefaultMRPConfig(); auto publisherTransmissionTimeout = GetRetransmissionTimeout(ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout, - System::SystemClock().GetMonotonicTimestamp(), Transport::kMinActiveTime); + System::SystemClock().GetMonotonicTimestamp(), ourMrpConfig.mActiveThresholdTime); return publisherTransmissionTimeout + aMaxInterval + System::Clock::Milliseconds32(1000); } diff --git a/src/lib/address_resolve/tool.cpp b/src/lib/address_resolve/tool.cpp index bac2c906fa4b75..094b4e8c55c50b 100644 --- a/src/lib/address_resolve/tool.cpp +++ b/src/lib/address_resolve/tool.cpp @@ -59,6 +59,7 @@ class PrintOutNodeListener : public chip::AddressResolve::NodeListener ChipLogProgress(Discovery, " Supports TCP: %s", result.supportsTcp ? "YES" : "NO"); ChipLogProgress(Discovery, " MRP IDLE retransmit timeout: %u ms", result.mrpRemoteConfig.mIdleRetransTimeout.count()); ChipLogProgress(Discovery, " MRP ACTIVE retransmit timeout: %u ms", result.mrpRemoteConfig.mActiveRetransTimeout.count()); + ChipLogProgress(Discovery, " MRP ACTIVE Threshold timet: %u ms", result.mrpRemoteConfig.mActiveThresholdTime.count()); NotifyDone(); } diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index c3817ce4c3811a..26c0a37d0dd879 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -213,8 +213,11 @@ class AdvertiserMinMdns : public ServiceAdvertiser, struct CommonTxtEntryStorage { // +2 for all to account for '=' and terminating nullchar - char sleepyIdleIntervalBuf[KeySize(TxtFieldKey::kSleepyIdleInterval) + ValSize(TxtFieldKey::kSleepyIdleInterval) + 2]; - char sleepyActiveIntervalBuf[KeySize(TxtFieldKey::kSleepyActiveInterval) + ValSize(TxtFieldKey::kSleepyActiveInterval) + 2]; + char sessionIdleIntervalBuf[KeySize(TxtFieldKey::kSessionIdleInterval) + ValSize(TxtFieldKey::kSessionIdleInterval) + 2]; + char sessionActiveIntervalBuf[KeySize(TxtFieldKey::kSessionActiveInterval) + ValSize(TxtFieldKey::kSessionActiveInterval) + + 2]; + char sessionActiveThresholdBuf[KeySize(TxtFieldKey::kSessionActiveThreshold) + + ValSize(TxtFieldKey::kSessionActiveThreshold) + 2]; char tcpSupportedBuf[KeySize(TxtFieldKey::kTcpSupported) + ValSize(TxtFieldKey::kTcpSupported) + 2]; }; template @@ -234,13 +237,13 @@ class AdvertiserMinMdns : public ServiceAdvertiser, mrp.mIdleRetransTimeout = kMaxRetryInterval; } size_t writtenCharactersNumber = - static_cast(snprintf(storage.sleepyIdleIntervalBuf, sizeof(storage.sleepyIdleIntervalBuf), + static_cast(snprintf(storage.sessionIdleIntervalBuf, sizeof(storage.sessionIdleIntervalBuf), "SII=%" PRIu32, mrp.mIdleRetransTimeout.count())); VerifyOrReturnError((writtenCharactersNumber > 0) && - (writtenCharactersNumber < sizeof(storage.sleepyIdleIntervalBuf)), + (writtenCharactersNumber < sizeof(storage.sessionIdleIntervalBuf)), CHIP_ERROR_INVALID_STRING_LENGTH); - txtFields[numTxtFields++] = storage.sleepyIdleIntervalBuf; + txtFields[numTxtFields++] = storage.sessionIdleIntervalBuf; } { @@ -251,12 +254,22 @@ class AdvertiserMinMdns : public ServiceAdvertiser, mrp.mActiveRetransTimeout = kMaxRetryInterval; } size_t writtenCharactersNumber = - static_cast(snprintf(storage.sleepyActiveIntervalBuf, sizeof(storage.sleepyActiveIntervalBuf), + static_cast(snprintf(storage.sessionActiveIntervalBuf, sizeof(storage.sessionActiveIntervalBuf), "SAI=%" PRIu32, mrp.mActiveRetransTimeout.count())); VerifyOrReturnError((writtenCharactersNumber > 0) && - (writtenCharactersNumber < sizeof(storage.sleepyActiveIntervalBuf)), + (writtenCharactersNumber < sizeof(storage.sessionActiveIntervalBuf)), CHIP_ERROR_INVALID_STRING_LENGTH); - txtFields[numTxtFields++] = storage.sleepyActiveIntervalBuf; + txtFields[numTxtFields++] = storage.sessionActiveIntervalBuf; + } + + { + size_t writtenCharactersNumber = + static_cast(snprintf(storage.sessionActiveThresholdBuf, sizeof(storage.sessionActiveThresholdBuf), + "SAT=%u", mrp.mActiveThresholdTime.count())); + VerifyOrReturnError((writtenCharactersNumber > 0) && + (writtenCharactersNumber < sizeof(storage.sessionActiveThresholdBuf)), + CHIP_ERROR_INVALID_STRING_LENGTH); + txtFields[numTxtFields++] = storage.sessionActiveThresholdBuf; } } if (params.GetTcpSupported().HasValue()) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 5ce64f9b21ed4e..cf6c127599bf25 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -252,20 +252,32 @@ CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, chip::Optional optional, - bool isIdle) + TxtFieldKey key) { + VerifyOrReturnError((key == TxtFieldKey::kSessionIdleInterval || key == TxtFieldKey::kSessionActiveInterval || + key == TxtFieldKey::kSessionActiveThreshold), + CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(optional.HasValue(), CHIP_ERROR_WELL_UNINITIALIZED); - auto retryInterval = isIdle ? optional.Value().mIdleRetransTimeout : optional.Value().mActiveRetransTimeout; - - if (retryInterval > kMaxRetryInterval) + CHIP_ERROR err; + if (key == TxtFieldKey::kSessionActiveThreshold) { - ChipLogProgress(Discovery, "MRP retry interval %s value exceeds allowed range of 1 hour, using maximum available", - isIdle ? "idle" : "active"); - retryInterval = kMaxRetryInterval; + err = CopyTextRecordValue(buffer, bufferLen, optional.Value().mActiveThresholdTime.count()); + } + else + { + bool isIdle = (key == TxtFieldKey::kSessionIdleInterval); + auto retryInterval = isIdle ? optional.Value().mIdleRetransTimeout : optional.Value().mActiveRetransTimeout; + if (retryInterval > kMaxRetryInterval) + { + ChipLogProgress(Discovery, "MRP retry interval %s value exceeds allowed range of 1 hour, using maximum available", + isIdle ? "idle" : "active"); + retryInterval = kMaxRetryInterval; + } + err = CopyTextRecordValue(buffer, bufferLen, retryInterval.count()); } - return CopyTextRecordValue(buffer, bufferLen, retryInterval.count()); + return err; } template @@ -275,9 +287,10 @@ CHIP_ERROR CopyTxtRecord(TxtFieldKey key, char * buffer, size_t bufferLen, const { case TxtFieldKey::kTcpSupported: return CopyTextRecordValue(buffer, bufferLen, params.GetTcpSupported()); - case TxtFieldKey::kSleepyIdleInterval: - case TxtFieldKey::kSleepyActiveInterval: - return CopyTextRecordValue(buffer, bufferLen, params.GetLocalMRPConfig(), key == TxtFieldKey::kSleepyIdleInterval); + case TxtFieldKey::kSessionIdleInterval: + case TxtFieldKey::kSessionActiveInterval: + case TxtFieldKey::kSessionActiveThreshold: + return CopyTextRecordValue(buffer, bufferLen, params.GetLocalMRPConfig(), key); default: return CHIP_ERROR_INVALID_ARGUMENT; } @@ -552,8 +565,9 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const OperationalAdvertisingParamete { PREPARE_RECORDS(Operational); - ADD_TXT_RECORD(SleepyIdleInterval); - ADD_TXT_RECORD(SleepyActiveInterval); + ADD_TXT_RECORD(SessionIdleInterval); + ADD_TXT_RECORD(SessionActiveInterval); + ADD_TXT_RECORD(SessionActiveThreshold); ADD_TXT_RECORD(TcpSupported); ADD_PTR_RECORD(CompressedFabricId); @@ -570,8 +584,9 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter ADD_TXT_RECORD(VendorProduct); ADD_TXT_RECORD(DeviceType); ADD_TXT_RECORD(DeviceName); - ADD_TXT_RECORD(SleepyIdleInterval); - ADD_TXT_RECORD(SleepyActiveInterval); + ADD_TXT_RECORD(SessionIdleInterval); + ADD_TXT_RECORD(SessionActiveInterval); + ADD_TXT_RECORD(SessionActiveThreshold); ADD_TXT_RECORD(TcpSupported); ADD_PTR_RECORD(VendorId); diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index eecf21db306fa2..a9039352c64b36 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -50,6 +50,7 @@ struct CommonResolutionData bool supportsTcp = false; Optional mrpRetryIntervalIdle; Optional mrpRetryIntervalActive; + Optional mrpRetryActiveThreshold; CommonResolutionData() { Reset(); } @@ -59,10 +60,12 @@ struct CommonResolutionData { const ReliableMessageProtocolConfig defaultConfig = GetDefaultMRPConfig(); return ReliableMessageProtocolConfig(GetMrpRetryIntervalIdle().ValueOr(defaultConfig.mIdleRetransTimeout), - GetMrpRetryIntervalActive().ValueOr(defaultConfig.mActiveRetransTimeout)); + GetMrpRetryIntervalActive().ValueOr(defaultConfig.mActiveRetransTimeout), + GetMrpRetryActiveThreshold().ValueOr(defaultConfig.mActiveThresholdTime)); } Optional GetMrpRetryIntervalIdle() const { return mrpRetryIntervalIdle; } Optional GetMrpRetryIntervalActive() const { return mrpRetryIntervalActive; } + Optional GetMrpRetryActiveThreshold() const { return mrpRetryActiveThreshold; } bool IsDeviceTreatedAsSleepy(const ReliableMessageProtocolConfig * defaultMRPConfig) const { diff --git a/src/lib/dnssd/TxtFields.cpp b/src/lib/dnssd/TxtFields.cpp index a09efaaf5aa4b9..8befc309a085b5 100644 --- a/src/lib/dnssd/TxtFields.cpp +++ b/src/lib/dnssd/TxtFields.cpp @@ -184,6 +184,18 @@ Optional GetRetryInterval(const ByteSpan & value) return NullOptional; } +Optional GetRetryActiveThreshold(const ByteSpan & value) +{ + const auto retryInterval = MakeU16FromAsciiDecimal(value); + + if (retryInterval == 0) + { + return NullOptional; + } + + return MakeOptional(System::Clock::Milliseconds16(retryInterval)); +} + TxtFieldKey GetTxtFieldKey(const ByteSpan & key) { for (auto & info : txtFieldInfo) @@ -237,12 +249,15 @@ void FillNodeDataFromTxt(const ByteSpan & key, const ByteSpan & value, CommonRes { switch (Internal::GetTxtFieldKey(key)) { - case TxtFieldKey::kSleepyIdleInterval: + case TxtFieldKey::kSessionIdleInterval: nodeData.mrpRetryIntervalIdle = Internal::GetRetryInterval(value); break; - case TxtFieldKey::kSleepyActiveInterval: + case TxtFieldKey::kSessionActiveInterval: nodeData.mrpRetryIntervalActive = Internal::GetRetryInterval(value); break; + case TxtFieldKey::kSessionActiveThreshold: + nodeData.mrpRetryActiveThreshold = Internal::GetRetryActiveThreshold(value); + break; case TxtFieldKey::kTcpSupported: nodeData.supportsTcp = Internal::MakeBoolFromAsciiDecimal(value); break; diff --git a/src/lib/dnssd/TxtFields.h b/src/lib/dnssd/TxtFields.h index 1bd5d84805f73f..b39f8617d8ddd1 100644 --- a/src/lib/dnssd/TxtFields.h +++ b/src/lib/dnssd/TxtFields.h @@ -31,8 +31,9 @@ namespace Dnssd { using namespace System::Clock::Literals; // Operational node TXT entries -static constexpr size_t kKeySleepyIdleIntervalMaxLength = 7; // [SII] 0-3600000 -static constexpr size_t kKeySleepyActiveIntervalMaxLength = 7; // [SAI] 0-3600000 +static constexpr size_t kKeySessionIdleIntervalMaxLength = 7; // [SII] 0-3600000 +static constexpr size_t kKeySessionActiveIntervalMaxLength = 7; // [SAI] 0-3600000 +static constexpr size_t kKeySessionActiveThresholdMaxLength = 5; // [SAT] 0-65535 static constexpr System::Clock::Milliseconds32 kMaxRetryInterval = 3600000_ms32; static constexpr size_t kKeyTcpSupportedMaxLength = 1; @@ -66,8 +67,9 @@ enum class TxtFieldKey : uint8_t kRotatingDeviceId, kPairingInstruction, kPairingHint, - kSleepyIdleInterval, - kSleepyActiveInterval, + kSessionIdleInterval, + kSessionActiveInterval, + kSessionActiveThreshold, kTcpSupported, kCount, }; @@ -92,8 +94,9 @@ constexpr const TxtFieldInfo txtFieldInfo[static_cast(TxtFieldKey::kCoun { TxtFieldKey::kRotatingDeviceId, kKeyRotatingDeviceIdMaxLength, "RI", TxtKeyUse::kCommission }, { TxtFieldKey::kPairingInstruction, kKeyPairingInstructionMaxLength, "PI", TxtKeyUse::kCommission }, { TxtFieldKey::kPairingHint, kKeyPairingHintMaxLength, "PH", TxtKeyUse::kCommission }, - { TxtFieldKey::kSleepyIdleInterval, kKeySleepyIdleIntervalMaxLength, "SII", TxtKeyUse::kCommon }, - { TxtFieldKey::kSleepyActiveInterval, kKeySleepyActiveIntervalMaxLength, "SAI", TxtKeyUse::kCommon }, + { TxtFieldKey::kSessionIdleInterval, kKeySessionIdleIntervalMaxLength, "SII", TxtKeyUse::kCommon }, + { TxtFieldKey::kSessionActiveInterval, kKeySessionActiveIntervalMaxLength, "SAI", TxtKeyUse::kCommon }, + { TxtFieldKey::kSessionActiveThreshold, kKeySessionActiveThresholdMaxLength, "SAT", TxtKeyUse::kCommon }, { TxtFieldKey::kTcpSupported, kKeyTcpSupportedMaxLength, "T", TxtKeyUse::kCommon }, }; #ifdef CHIP_CONFIG_TEST diff --git a/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h b/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h index 3e468f3e6238a3..e82dc5ee6c3492 100644 --- a/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h +++ b/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h @@ -311,7 +311,7 @@ class CheckOnlyServer : private chip::PoolImpl(false)) - .SetLocalMRPConfig(Optional::Value(32_ms32, 30_ms32)); // Match SII, SAI below + .SetLocalMRPConfig(chip::Optional::Value( + 32_ms32, 30_ms32)); // Match SII, SAI. SAT not provided so it uses default 4000ms OperationalAdvertisingParameters operationalParams2 = OperationalAdvertisingParameters().SetPeerId(kPeerId2).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); OperationalAdvertisingParameters operationalParams3 = @@ -91,7 +92,7 @@ OperationalAdvertisingParameters operationalParams5 = OperationalAdvertisingParameters().SetPeerId(kPeerId5).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); OperationalAdvertisingParameters operationalParams6 = OperationalAdvertisingParameters().SetPeerId(kPeerId6).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); -const QNamePart txtOperational1Parts[] = { "SII=32", "SAI=30", "T=0" }; +const QNamePart txtOperational1Parts[] = { "SII=32", "SAI=30", "SAT=4000", "T=0" }; PtrResourceRecord ptrOperationalService = PtrResourceRecord(kDnsSdQueryName, kMatterOperationalQueryName); PtrResourceRecord ptrOperational1 = PtrResourceRecord(kMatterOperationalQueryName, kInstanceName1); SrvResourceRecord srvOperational1 = SrvResourceRecord(kInstanceName1, kHostnameName, CHIP_PORT); @@ -181,10 +182,10 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced = .SetRotatingDeviceId(chip::Optional("id_that_spins")) .SetTcpSupported(chip::Optional(true)) // 3600005 is more than the max so should be adjusted down - .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600005_ms32)); + .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600005_ms32, 65535_ms16)); QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22", "VP=555+897", "CM=2", "DT=70000", "DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3", - "SAI=3600000", "SII=3600000", "T=1" }; + "SAI=3600000", "SII=3600000", "SAT=65535", "T=1" }; FullQName txtCommissionableNodeParamsLargeEnhancedName = FullQName(txtCommissionableNodeParamsLargeEnhancedParts); TxtResourceRecord txtCommissionableNodeParamsLargeEnhanced = TxtResourceRecord(instanceName, txtCommissionableNodeParamsLargeEnhancedName); diff --git a/src/lib/dnssd/platform/tests/TestPlatform.cpp b/src/lib/dnssd/platform/tests/TestPlatform.cpp index b0285963ddd70c..31940aeacee332 100644 --- a/src/lib/dnssd/platform/tests/TestPlatform.cpp +++ b/src/lib/dnssd/platform/tests/TestPlatform.cpp @@ -52,7 +52,7 @@ OperationalAdvertisingParameters operationalParams2 = .SetMac(ByteSpan(kMac)) .SetPort(CHIP_PORT) .EnableIpV4(true) - .SetLocalMRPConfig(Optional::Value(32_ms32, 30_ms32)) // SII and SAI to match below + .SetLocalMRPConfig(Optional::Value(32_ms32, 30_ms32, 10_ms16)) // SII and SAI to match below .SetTcpSupported(Optional(true)); test::ExpectedCall operationalCall2 = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolTcp) @@ -62,6 +62,7 @@ test::ExpectedCall operationalCall2 = test::ExpectedCall() .AddSubtype("_I5555666677778888") .AddTxt("SII", "32") .AddTxt("SAI", "30") + .AddTxt("SAT", "10") .AddTxt("T", "1"); CommissionAdvertisingParameters commissionableNodeParamsSmall = @@ -96,7 +97,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic = .SetRotatingDeviceId(chip::Optional("id_that_spins")) .SetTcpSupported(chip::Optional(true)) // 3600005 is over the max, so this should be adjusted by the platform - .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600005_ms32)); + .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600005_ms32, 65535_ms16)); test::ExpectedCall commissionableLargeBasic = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolUdp) @@ -113,6 +114,7 @@ test::ExpectedCall commissionableLargeBasic = test::ExpectedCall() .AddTxt("T", "1") .AddTxt("SII", "3600000") .AddTxt("SAI", "3600000") + .AddTxt("SAT", "65535") .AddSubtype("_S2") .AddSubtype("_L22") .AddSubtype("_V555") diff --git a/src/lib/dnssd/tests/TestTxtFields.cpp b/src/lib/dnssd/tests/TestTxtFields.cpp index d83954692c0a8c..d4502b7f756b1a 100644 --- a/src/lib/dnssd/tests/TestTxtFields.cpp +++ b/src/lib/dnssd/tests/TestTxtFields.cpp @@ -69,10 +69,13 @@ void TestGetTxtFieldKey(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, GetTxtFieldKey(GetSpan(key)) == TxtFieldKey::kPairingHint); strcpy(key, "SII"); - NL_TEST_ASSERT(inSuite, GetTxtFieldKey(GetSpan(key)) == TxtFieldKey::kSleepyIdleInterval); + NL_TEST_ASSERT(inSuite, GetTxtFieldKey(GetSpan(key)) == TxtFieldKey::kSessionIdleInterval); strcpy(key, "SAI"); - NL_TEST_ASSERT(inSuite, GetTxtFieldKey(GetSpan(key)) == TxtFieldKey::kSleepyActiveInterval); + NL_TEST_ASSERT(inSuite, GetTxtFieldKey(GetSpan(key)) == TxtFieldKey::kSessionActiveInterval); + + strcpy(key, "SAT"); + NL_TEST_ASSERT(inSuite, GetTxtFieldKey(GetSpan(key)) == TxtFieldKey::kSessionActiveThreshold); strcpy(key, "T"); NL_TEST_ASSERT(inSuite, GetTxtFieldKey(GetSpan(key)) == TxtFieldKey::kTcpSupported); @@ -409,7 +412,7 @@ void ResetRetryIntervalActive(ResolvedNodeData & nodeData) // Test SAI (formally CRI) template -void TxtFieldSleepyIdleInterval(nlTestSuite * inSuite, void * inContext) +void TxtFieldSessionIdleInterval(nlTestSuite * inSuite, void * inContext) { char key[4]; char val[16]; @@ -472,7 +475,7 @@ void TxtFieldSleepyIdleInterval(nlTestSuite * inSuite, void * inContext) // Test SAI (formerly CRA) template -void TxtFieldSleepyActiveInterval(nlTestSuite * inSuite, void * inContext) +void TxtFieldSessionActiveInterval(nlTestSuite * inSuite, void * inContext) { char key[4]; char val[16]; @@ -533,6 +536,69 @@ void TxtFieldSleepyActiveInterval(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !nodeData.resolutionData.GetMrpRetryIntervalActive().HasValue()); } +// Test SAT (Session Active Threshold) +template +void TxtFieldSessionActiveThreshold(nlTestSuite * inSuite, void * inContext) +{ + char key[4]; + char val[16]; + NodeData nodeData; + + // Minimum + strcpy(key, "SAT"); + strcpy(val, "1"); + FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData.resolutionData); + NL_TEST_ASSERT(inSuite, nodeData.resolutionData.GetMrpRetryActiveThreshold().HasValue()); + NL_TEST_ASSERT(inSuite, nodeData.resolutionData.GetMrpRetryActiveThreshold().Value() == 1_ms16); + + // Maximum + strcpy(key, "SAT"); + strcpy(val, "65535"); + FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData.resolutionData); + NL_TEST_ASSERT(inSuite, nodeData.resolutionData.GetMrpRetryActiveThreshold().HasValue()); + NL_TEST_ASSERT(inSuite, nodeData.resolutionData.GetMrpRetryActiveThreshold().Value() == 65535_ms16); + + // Test no other fields were populated + ResetRetryIntervalActive(nodeData); + NL_TEST_ASSERT(inSuite, NodeDataIsEmpty(nodeData)); + + // Invalid SAI - negative value + strcpy(key, "SAT"); + strcpy(val, "-1"); + FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData.resolutionData); + NL_TEST_ASSERT(inSuite, !nodeData.resolutionData.GetMrpRetryActiveThreshold().HasValue()); + + // Invalid SAI - greater than maximum + strcpy(key, "SAT"); + strcpy(val, "65536"); + FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData.resolutionData); + NL_TEST_ASSERT(inSuite, !nodeData.resolutionData.GetMrpRetryActiveThreshold().HasValue()); + + // Invalid SAT - much greater than maximum + strcpy(key, "SAT"); + strcpy(val, "1095216660481"); // 0xFF00000001 == 1 (mod 2^32) + FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData.resolutionData); + NL_TEST_ASSERT(inSuite, !nodeData.resolutionData.GetMrpRetryActiveThreshold().HasValue()); + + // Invalid SAT - hexadecimal value + strcpy(key, "SAT"); + strcpy(val, "0x20"); + FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData.resolutionData); + NL_TEST_ASSERT(inSuite, !nodeData.resolutionData.GetMrpRetryActiveThreshold().HasValue()); + + // Invalid SAT - leading zeros + strcpy(key, "SAT"); + strcpy(val, "0700"); + FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData.resolutionData); + NL_TEST_ASSERT(inSuite, !nodeData.resolutionData.GetMrpRetryActiveThreshold().HasValue()); + + // Invalid SAT - text at the end + strcpy(key, "SAT"); + strcpy(val, "123abc"); + FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData.resolutionData); + NL_TEST_ASSERT(inSuite, !nodeData.resolutionData.GetMrpRetryActiveThreshold().HasValue()); +} + // Test T (TCP support) template void TxtFieldTcpSupport(nlTestSuite * inSuite, void * inContext) @@ -566,7 +632,7 @@ void TxtFieldTcpSupport(nlTestSuite * inSuite, void * inContext) // Test IsDeviceTreatedAsSleepy() with CRI template -void TestIsDeviceSleepyIdle(nlTestSuite * inSuite, void * inContext) +void TestIsDeviceSessionIdle(nlTestSuite * inSuite, void * inContext) { char key[4]; char val[32]; @@ -592,7 +658,7 @@ void TestIsDeviceSleepyIdle(nlTestSuite * inSuite, void * inContext) // Test IsDeviceTreatedAsSleepy() with CRA template -void TestIsDeviceSleepyActive(nlTestSuite * inSuite, void * inContext) +void TestIsDeviceSessionActive(nlTestSuite * inSuite, void * inContext) { char key[4]; char val[32]; @@ -616,29 +682,33 @@ void TestIsDeviceSleepyActive(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, nodeData.resolutionData.IsDeviceTreatedAsSleepy(&defaultMRPConfig)); } -const nlTest sTests[] = { NL_TEST_DEF("TxtFieldKey", TestGetTxtFieldKey), // - NL_TEST_DEF("TxtFieldKeyCaseInsensitive", TestGetTxtFieldKeyCaseInsensitive), // - NL_TEST_DEF("TxtFieldProduct", TestGetProduct), // - NL_TEST_DEF("TxtFieldVendor", TestGetVendor), // - NL_TEST_DEF("TxtFieldLongDiscriminator", TestGetLongDiscriminator), // - NL_TEST_DEF("TxtFieldCommissioningMode", TestGetCommissioningMode), // - NL_TEST_DEF("TxtFieldDeviceType", TestGetDeviceType), // - NL_TEST_DEF("TxtFieldDeviceName", TestGetDeviceName), // - NL_TEST_DEF("TxtFieldRotatingDeviceId", TestGetRotatingDeviceId), // - NL_TEST_DEF("TxtFieldPairingHint", TestGetPairingHint), // - NL_TEST_DEF("TxtFieldPairingInstruction", TestGetPairingInstruction), // - NL_TEST_DEF("TxtFieldFillDiscoveredNodeDataFromTxt", TestFillDiscoveredNodeDataFromTxt), // - NL_TEST_DEF("TxtDiscoveredFieldMrpRetryIntervalIdle", TxtFieldSleepyIdleInterval), - NL_TEST_DEF("TxtDiscoveredFieldMrpRetryIntervalActive", TxtFieldSleepyActiveInterval), - NL_TEST_DEF("TxtDiscoveredFieldTcpSupport", (TxtFieldTcpSupport) ), - NL_TEST_DEF("TxtDiscoveredIsDeviceSleepyIdle", TestIsDeviceSleepyIdle), - NL_TEST_DEF("TxtDiscoveredIsDeviceSleepyActive", TestIsDeviceSleepyActive), - NL_TEST_DEF("TxtResolvedFieldMrpRetryIntervalIdle", TxtFieldSleepyIdleInterval), - NL_TEST_DEF("TxtResolvedFieldMrpRetryIntervalActive", TxtFieldSleepyActiveInterval), - NL_TEST_DEF("TxtResolvedFieldTcpSupport", (TxtFieldTcpSupport) ), - NL_TEST_DEF("TxtResolvedIsDeviceSleepyIdle", TestIsDeviceSleepyIdle), - NL_TEST_DEF("TxtResolvedIsDeviceSleepyActive", TestIsDeviceSleepyActive), - NL_TEST_SENTINEL() }; +const nlTest sTests[] = { + NL_TEST_DEF("TxtFieldKey", TestGetTxtFieldKey), // + NL_TEST_DEF("TxtFieldKeyCaseInsensitive", TestGetTxtFieldKeyCaseInsensitive), // + NL_TEST_DEF("TxtFieldProduct", TestGetProduct), // + NL_TEST_DEF("TxtFieldVendor", TestGetVendor), // + NL_TEST_DEF("TxtFieldLongDiscriminator", TestGetLongDiscriminator), // + NL_TEST_DEF("TxtFieldCommissioningMode", TestGetCommissioningMode), // + NL_TEST_DEF("TxtFieldDeviceType", TestGetDeviceType), // + NL_TEST_DEF("TxtFieldDeviceName", TestGetDeviceName), // + NL_TEST_DEF("TxtFieldRotatingDeviceId", TestGetRotatingDeviceId), // + NL_TEST_DEF("TxtFieldPairingHint", TestGetPairingHint), // + NL_TEST_DEF("TxtFieldPairingInstruction", TestGetPairingInstruction), // + NL_TEST_DEF("TxtFieldFillDiscoveredNodeDataFromTxt", TestFillDiscoveredNodeDataFromTxt), // + NL_TEST_DEF("TxtDiscoveredFieldMrpRetryIntervalIdle", TxtFieldSessionIdleInterval), + NL_TEST_DEF("TxtDiscoveredFieldMrpRetryIntervalActive", TxtFieldSessionActiveInterval), + NL_TEST_DEF("TxtDiscoveredFieldMrpRetryActiveThreshold", TxtFieldSessionActiveThreshold), + NL_TEST_DEF("TxtDiscoveredFieldTcpSupport", (TxtFieldTcpSupport) ), + NL_TEST_DEF("TxtDiscoveredIsDeviceSessiondle", TestIsDeviceSessionIdle), + NL_TEST_DEF("TxtDiscoveredIsDeviceSessionActive", TestIsDeviceSessionActive), + NL_TEST_DEF("TxtResolvedFieldMrpRetryIntervalIdle", TxtFieldSessionIdleInterval), + NL_TEST_DEF("TxtResolvedFieldMrpRetryIntervalActive", TxtFieldSessionActiveInterval), + NL_TEST_DEF("TxtResolvedFieldMrpRetryActiveThreshold", TxtFieldSessionActiveThreshold), + NL_TEST_DEF("TxtResolvedFieldTcpSupport", (TxtFieldTcpSupport) ), + NL_TEST_DEF("TxtResolvedIsDeviceSessionIdle", TestIsDeviceSessionIdle), + NL_TEST_DEF("TxtResolvedIsDeviceSessionActive", TestIsDeviceSessionActive), + NL_TEST_SENTINEL() +}; } // namespace diff --git a/src/messaging/ReliableMessageProtocolConfig.cpp b/src/messaging/ReliableMessageProtocolConfig.cpp index 442d1ee81d4596..795e02c3ce3679 100644 --- a/src/messaging/ReliableMessageProtocolConfig.cpp +++ b/src/messaging/ReliableMessageProtocolConfig.cpp @@ -28,7 +28,8 @@ #include #if CHIP_CONFIG_ENABLE_ICD_SERVER -#include // nogncheck +#include // nogncheck +#include // nogncheck #endif namespace chip { @@ -38,17 +39,21 @@ using namespace System::Clock::Literals; #if CONFIG_BUILD_FOR_HOST_UNIT_TEST static Optional idleRetransTimeoutOverride = NullOptional; static Optional activeRetransTimeoutOverride = NullOptional; +static Optional activeThresholdTimeOverride = NullOptional; -void OverrideLocalMRPConfig(System::Clock::Timeout idleRetransTimeout, System::Clock::Timeout activeRetransTimeout) +void OverrideLocalMRPConfig(System::Clock::Timeout idleRetransTimeout, System::Clock::Timeout activeRetransTimeout, + System::Clock::Timeout activeThresholdTime) { idleRetransTimeoutOverride.SetValue(idleRetransTimeout); activeRetransTimeoutOverride.SetValue(activeRetransTimeout); + activeThresholdTimeOverride.SetValue(activeThresholdTime); } void ClearLocalMRPConfigOverride() { activeRetransTimeoutOverride.ClearValue(); idleRetransTimeoutOverride.ClearValue(); + activeThresholdTimeOverride.ClearValue(); } #endif @@ -57,7 +62,8 @@ ReliableMessageProtocolConfig GetDefaultMRPConfig() // Default MRP intervals are defined in spec <2.11.3. Parameters and Constants> static constexpr const System::Clock::Milliseconds32 idleRetransTimeout = 300_ms32; static constexpr const System::Clock::Milliseconds32 activeRetransTimeout = 300_ms32; - return ReliableMessageProtocolConfig(idleRetransTimeout, activeRetransTimeout); + static constexpr const System::Clock::Milliseconds16 activeThresholdTime = 4000_ms16; + return ReliableMessageProtocolConfig(idleRetransTimeout, activeRetransTimeout, activeThresholdTime); } Optional GetLocalMRPConfig() @@ -69,6 +75,7 @@ Optional GetLocalMRPConfig() // which the device can be at sleep and not be able to receive any messages). config.mIdleRetransTimeout += app::ICDManager::GetSlowPollingInterval(); config.mActiveRetransTimeout += app::ICDManager::GetFastPollingInterval(); + config.mActiveThresholdTime = System::Clock::Milliseconds16(IcdManagementServer::GetInstance().GetActiveModeThreshold()); #elif CHIP_DEVICE_CONFIG_ENABLE_SED DeviceLayer::ConnectivityManager::SEDIntervalsConfig sedIntervalsConfig; @@ -91,6 +98,11 @@ Optional GetLocalMRPConfig() { config.mActiveRetransTimeout = activeRetransTimeoutOverride.Value(); } + + if (activeThresholdTimeOverride.HasValue()) + { + config.mActiveThresholdTime = activeRetransTimeoutOverride.Value(); + } #endif return (config == GetDefaultMRPConfig()) ? Optional::Missing() diff --git a/src/messaging/ReliableMessageProtocolConfig.h b/src/messaging/ReliableMessageProtocolConfig.h index a77483f3262e27..6a570ba25f93e7 100644 --- a/src/messaging/ReliableMessageProtocolConfig.h +++ b/src/messaging/ReliableMessageProtocolConfig.h @@ -66,7 +66,7 @@ namespace chip { * timeout when it sends a message to the present node and the present node is * perceived by the peer as idle. * - * This value is announced to the peer using SII (Sleepy Idle Interval) key + * This value is announced to the peer using SII (Session Idle Interval) key * in the advertised DNS Service Discovery TXT records. Additionally, it is * exchanged in the initial phase of the PASE/CASE session establishment. * @@ -167,14 +167,18 @@ namespace chip { #endif #endif // CHIP_CONFIG_MRP_RETRY_INTERVAL_SENDER_BOOST +constexpr System::Clock::Milliseconds32 kDefaultActiveTime = System::Clock::Milliseconds16(4000); + /** * @brief * The ReliableMessageProtocol configuration. */ struct ReliableMessageProtocolConfig { - ReliableMessageProtocolConfig(System::Clock::Milliseconds32 idleInterval, System::Clock::Milliseconds32 activeInterval) : - mIdleRetransTimeout(idleInterval), mActiveRetransTimeout(activeInterval) + ReliableMessageProtocolConfig(System::Clock::Milliseconds32 idleInterval, System::Clock::Milliseconds32 activeInterval, + System::Clock::Milliseconds16 activeThreshold = kDefaultActiveTime) : + mIdleRetransTimeout(idleInterval), + mActiveRetransTimeout(activeInterval), mActiveThresholdTime(activeThreshold) {} // Configurable timeout in msec for retransmission of the first sent message. @@ -183,9 +187,13 @@ struct ReliableMessageProtocolConfig // Configurable timeout in msec for retransmission of all subsequent messages. System::Clock::Milliseconds32 mActiveRetransTimeout; + // Configurable amount of time the node SHOULD stay active after network activity. + System::Clock::Milliseconds16 mActiveThresholdTime; + bool operator==(const ReliableMessageProtocolConfig & that) const { - return mIdleRetransTimeout == that.mIdleRetransTimeout && mActiveRetransTimeout == that.mActiveRetransTimeout; + return mIdleRetransTimeout == that.mIdleRetransTimeout && mActiveRetransTimeout == that.mActiveRetransTimeout && + mActiveThresholdTime == that.mActiveThresholdTime; } }; @@ -226,7 +234,8 @@ System::Clock::Timestamp GetRetransmissionTimeout(System::Clock::Timestamp activ * time defines). This is reserved for tests that need the ability to set these at runtime to make certain test scenarios possible. * */ -void OverrideLocalMRPConfig(System::Clock::Timeout idleRetransTimeout, System::Clock::Timeout activeRetransTimeout); +void OverrideLocalMRPConfig(System::Clock::Timeout idleRetransTimeout, System::Clock::Timeout activeRetransTimeout, + System::Clock::Timeout activeThresholdTime = kDefaultActiveTime); /** * @brief diff --git a/src/platform/fake/DnssdImpl.h b/src/platform/fake/DnssdImpl.h index 720bd25726e799..1608a30247c7ef 100644 --- a/src/platform/fake/DnssdImpl.h +++ b/src/platform/fake/DnssdImpl.h @@ -163,13 +163,13 @@ struct ExpectedCall ChipLogProgress(Discovery, "\t%s", subtype[i].name); } ChipLogProgress(Discovery, "num txt = %lu", static_cast(numTxt)); - for (size_t i = 0; i < numSubtypes; ++i) + for (size_t i = 0; i < numTxt; ++i) { ChipLogProgress(Discovery, "\t%s = %s", txt[i].key, txt[i].value); } } - static constexpr size_t kMaxTxtRecords = 11; + static constexpr size_t kMaxTxtRecords = 12; static constexpr size_t kMaxSubtypes = 10; CallType callType = CallType::kUnknown; DnssdServiceProtocol protocol = DnssdServiceProtocol::kDnssdProtocolUnknown; diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index c617e46ef7584b..07a75cf5fcbb2a 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -640,8 +640,9 @@ CHIP_ERROR CASESession::RecoverInitiatorIpk() CHIP_ERROR CASESession::SendSigma1() { MATTER_TRACE_SCOPE("SendSigma1", "CASESession"); - const size_t mrpParamsSize = mLocalMRPConfig.HasValue() ? TLV::EstimateStructOverhead(sizeof(uint16_t), sizeof(uint16_t)) : 0; - size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, // initiatorRandom + const size_t mrpParamsSize = + mLocalMRPConfig.HasValue() ? TLV::EstimateStructOverhead(sizeof(uint16_t), sizeof(uint16_t), sizeof(uint16_t)) : 0; + size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, // initiatorRandom sizeof(uint16_t), // initiatorSessionId, kSHA256_Hash_Length, // destinationId kP256_PublicKey_Length, // InitiatorEphPubKey, @@ -915,7 +916,8 @@ CHIP_ERROR CASESession::HandleSigma1(System::PacketBufferHandle && msg) CHIP_ERROR CASESession::SendSigma2Resume() { MATTER_TRACE_SCOPE("SendSigma2Resume", "CASESession"); - const size_t mrpParamsSize = mLocalMRPConfig.HasValue() ? TLV::EstimateStructOverhead(sizeof(uint16_t), sizeof(uint16_t)) : 0; + const size_t mrpParamsSize = + mLocalMRPConfig.HasValue() ? TLV::EstimateStructOverhead(sizeof(uint16_t), sizeof(uint16_t), sizeof(uint16_t)) : 0; size_t max_sigma2_resume_data_len = TLV::EstimateStructOverhead( SessionResumptionStorage::kResumptionIdSize, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, sizeof(uint16_t), mrpParamsSize); @@ -1065,8 +1067,9 @@ CHIP_ERROR CASESession::SendSigma2() msg_R2_Encrypted.Get() + msg_r2_signed_enc_len, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES)); // Construct Sigma2 Msg - const size_t mrpParamsSize = mLocalMRPConfig.HasValue() ? TLV::EstimateStructOverhead(sizeof(uint16_t), sizeof(uint16_t)) : 0; - size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, sizeof(uint16_t), kP256_PublicKey_Length, + const size_t mrpParamsSize = + mLocalMRPConfig.HasValue() ? TLV::EstimateStructOverhead(sizeof(uint16_t), sizeof(uint16_t), sizeof(uint16_t)) : 0; + size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, sizeof(uint16_t), kP256_PublicKey_Length, msg_r2_signed_enc_len, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, mrpParamsSize); System::PacketBufferHandle msg_R2 = System::PacketBufferHandle::New(data_len); @@ -2227,7 +2230,7 @@ System::Clock::Timeout CASESession::ComputeSigma1ResponseTimeout(const ReliableM return GetRetransmissionTimeout(remoteMrpConfig.mActiveRetransTimeout, remoteMrpConfig.mIdleRetransTimeout, // Assume peer is idle, since that's what we // will assume for our initial message. - System::Clock::kZero, Transport::kMinActiveTime) + + System::Clock::kZero, remoteMrpConfig.mActiveThresholdTime) + kExpectedSigma1ProcessingTime; } @@ -2235,7 +2238,7 @@ System::Clock::Timeout CASESession::ComputeSigma2ResponseTimeout(const ReliableM { return GetRetransmissionTimeout(remoteMrpConfig.mActiveRetransTimeout, remoteMrpConfig.mIdleRetransTimeout, // Assume peer is idle, as a worst-case assumption. - System::Clock::kZero, Transport::kMinActiveTime) + + System::Clock::kZero, remoteMrpConfig.mActiveThresholdTime) + kExpectedHighProcessingTime; } diff --git a/src/protocols/secure_channel/PairingSession.cpp b/src/protocols/secure_channel/PairingSession.cpp index ef38eb37ba5f80..af555bfeee5046 100644 --- a/src/protocols/secure_channel/PairingSession.cpp +++ b/src/protocols/secure_channel/PairingSession.cpp @@ -95,6 +95,7 @@ CHIP_ERROR PairingSession::EncodeMRPParameters(TLV::Tag tag, const ReliableMessa ReturnErrorOnFailure(tlvWriter.StartContainer(tag, TLV::kTLVType_Structure, mrpParamsContainer)); ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(1), mrpLocalConfig.mIdleRetransTimeout.count())); ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(2), mrpLocalConfig.mActiveRetransTimeout.count())); + ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(3), mrpLocalConfig.mActiveThresholdTime.count())); return tlvWriter.EndContainer(mrpParamsContainer); } @@ -115,7 +116,7 @@ CHIP_ERROR PairingSession::DecodeMRPParametersIfPresent(TLV::Tag expectedTag, TL ChipLogDetail(SecureChannel, "Found MRP parameters in the message"); - // Both TLV elements in the structure are optional. If the first element is present, process it and move + // All TLV elements in the structure are optional. If the first element is present, process it and move // the TLV reader to the next element. if (TLV::TagNumFromTag(tlvReader.GetTag()) == 1) { @@ -131,9 +132,26 @@ CHIP_ERROR PairingSession::DecodeMRPParametersIfPresent(TLV::Tag expectedTag, TL ReturnErrorOnFailure(err); } - VerifyOrReturnError(TLV::TagNumFromTag(tlvReader.GetTag()) == 2, CHIP_ERROR_INVALID_TLV_TAG); - ReturnErrorOnFailure(tlvReader.Get(tlvElementValue)); - mRemoteMRPConfig.mActiveRetransTimeout = System::Clock::Milliseconds32(tlvElementValue); + if (TLV::TagNumFromTag(tlvReader.GetTag()) == 2) + { + ReturnErrorOnFailure(tlvReader.Get(tlvElementValue)); + mRemoteMRPConfig.mActiveRetransTimeout = System::Clock::Milliseconds32(tlvElementValue); + + // The next element is optional. If it's not present, return CHIP_NO_ERROR. + CHIP_ERROR err = tlvReader.Next(); + if (err == CHIP_END_OF_TLV) + { + return tlvReader.ExitContainer(containerType); + } + ReturnErrorOnFailure(err); + } + + if (TLV::TagNumFromTag(tlvReader.GetTag()) == 3) + { + ReturnErrorOnFailure(tlvReader.Get(tlvElementValue)); + mRemoteMRPConfig.mActiveThresholdTime = System::Clock::Milliseconds16(tlvElementValue); + } + // Future proofing - Don't error out if there are other tags return tlvReader.ExitContainer(containerType); } diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp index 11d5000f71df03..26cb1e0f71ed58 100644 --- a/src/protocols/secure_channel/tests/TestCASESession.cpp +++ b/src/protocols/secure_channel/tests/TestCASESession.cpp @@ -417,10 +417,10 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, S // Test all combinations of invalid parameters TestCASESecurePairingDelegate delegateAccessory; CASESession pairingAccessory; - ReliableMessageProtocolConfig verySleepyAccessoryRmpConfig(System::Clock::Milliseconds32(360000), - System::Clock::Milliseconds32(100000)); - ReliableMessageProtocolConfig nonSleepyCommissionerRmpConfig(System::Clock::Milliseconds32(5000), - System::Clock::Milliseconds32(300)); + ReliableMessageProtocolConfig verySleepyAccessoryRmpConfig( + System::Clock::Milliseconds32(360000), System::Clock::Milliseconds32(100000), System::Clock::Milliseconds16(300)); + ReliableMessageProtocolConfig nonSleepyCommissionerRmpConfig( + System::Clock::Milliseconds32(5000), System::Clock::Milliseconds32(300), System::Clock::Milliseconds16(4000)); auto & loopback = ctx.GetLoopback(); loopback.mSentMessageCount = 0; @@ -450,9 +450,11 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, S NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 0); NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mIdleRetransTimeout == System::Clock::Milliseconds32(5000)); NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mActiveRetransTimeout == System::Clock::Milliseconds32(300)); + NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mActiveThresholdTime == System::Clock::Milliseconds16(4000)); NL_TEST_ASSERT(inSuite, pairingCommissioner.GetRemoteMRPConfig().mIdleRetransTimeout == System::Clock::Milliseconds32(360000)); NL_TEST_ASSERT(inSuite, pairingCommissioner.GetRemoteMRPConfig().mActiveRetransTimeout == System::Clock::Milliseconds32(100000)); + NL_TEST_ASSERT(inSuite, pairingCommissioner.GetRemoteMRPConfig().mActiveThresholdTime == System::Clock::Milliseconds16(300)); #if CONFIG_BUILD_FOR_HOST_UNIT_TEST // Confirming that FabricTable sending a notification that fabric was updated doesn't affect // already established connections. diff --git a/src/protocols/secure_channel/tests/TestPASESession.cpp b/src/protocols/secure_channel/tests/TestPASESession.cpp index c9a3d6a2d45354..af5f64b4048461 100644 --- a/src/protocols/secure_channel/tests/TestPASESession.cpp +++ b/src/protocols/secure_channel/tests/TestPASESession.cpp @@ -363,7 +363,7 @@ void SecurePairingHandshakeWithCommissionerMRPTest(nlTestSuite * inSuite, void * PASESession pairingCommissioner; auto & loopback = ctx.GetLoopback(); loopback.Reset(); - ReliableMessageProtocolConfig config(1000_ms32, 10000_ms32); + ReliableMessageProtocolConfig config(1000_ms32, 10000_ms32, 4000_ms16); SecurePairingHandshakeTestCommon(inSuite, inContext, sessionManager, pairingCommissioner, Optional::Value(config), Optional::Missing(), delegateCommissioner); @@ -378,7 +378,7 @@ void SecurePairingHandshakeWithDeviceMRPTest(nlTestSuite * inSuite, void * inCon PASESession pairingCommissioner; auto & loopback = ctx.GetLoopback(); loopback.Reset(); - ReliableMessageProtocolConfig config(1000_ms32, 10000_ms32); + ReliableMessageProtocolConfig config(1000_ms32, 10000_ms32, 4000_ms16); SecurePairingHandshakeTestCommon(inSuite, inContext, sessionManager, pairingCommissioner, Optional::Missing(), Optional::Value(config), delegateCommissioner); @@ -393,8 +393,8 @@ void SecurePairingHandshakeWithAllMRPTest(nlTestSuite * inSuite, void * inContex PASESession pairingCommissioner; auto & loopback = ctx.GetLoopback(); loopback.Reset(); - ReliableMessageProtocolConfig commissionerConfig(1000_ms32, 10000_ms32); - ReliableMessageProtocolConfig deviceConfig(2000_ms32, 7000_ms32); + ReliableMessageProtocolConfig commissionerConfig(1000_ms32, 10000_ms32, 4000_ms16); + ReliableMessageProtocolConfig deviceConfig(2000_ms32, 7000_ms32, 4000_ms16); SecurePairingHandshakeTestCommon(inSuite, inContext, sessionManager, pairingCommissioner, Optional::Value(commissionerConfig), Optional::Value(deviceConfig), delegateCommissioner); diff --git a/src/protocols/secure_channel/tests/TestPairingSession.cpp b/src/protocols/secure_channel/tests/TestPairingSession.cpp index 833fb7afd6c92d..714a18e03fc1a4 100644 --- a/src/protocols/secure_channel/tests/TestPairingSession.cpp +++ b/src/protocols/secure_channel/tests/TestPairingSession.cpp @@ -60,7 +60,7 @@ void PairingSessionEncodeDecodeMRPParams(nlTestSuite * inSuite, void * inContext { TestPairingSession session; - ReliableMessageProtocolConfig config(Milliseconds32(100), Milliseconds32(200)); + ReliableMessageProtocolConfig config(Milliseconds32(100), Milliseconds32(200), Milliseconds16(4000)); System::PacketBufferHandle buf = System::PacketBufferHandle::New(64, 0); System::PacketBufferTLVWriter writer; diff --git a/src/system/SystemClock.h b/src/system/SystemClock.h index d669200ae37000..566e23d9738bc7 100644 --- a/src/system/SystemClock.h +++ b/src/system/SystemClock.h @@ -62,6 +62,7 @@ using Microseconds32 = std::chrono::duration; using Milliseconds64 = std::chrono::duration; using Milliseconds32 = std::chrono::duration; +using Milliseconds16 = std::chrono::duration; using Seconds64 = std::chrono::duration; using Seconds32 = std::chrono::duration; @@ -96,6 +97,10 @@ constexpr Milliseconds32 operator""_ms32(unsigned long long int ms) { return Milliseconds32(ms); } +constexpr Milliseconds16 operator""_ms16(unsigned long long int ms) +{ + return Milliseconds16(ms); +} constexpr Seconds64 operator""_s(unsigned long long int s) { diff --git a/src/tracing/json/json_tracing.cpp b/src/tracing/json/json_tracing.cpp index e375c2c1113b31..8dd45887cf9f37 100644 --- a/src/tracing/json/json_tracing.cpp +++ b/src/tracing/json/json_tracing.cpp @@ -371,6 +371,7 @@ void JsonBackend::LogNodeDiscovered(NodeDiscoveredInfo & info) result["mrp"]["idle_retransmit_timeout_ms"] = info.result->mrpRemoteConfig.mIdleRetransTimeout.count(); result["mrp"]["active_retransmit_timeout_ms"] = info.result->mrpRemoteConfig.mActiveRetransTimeout.count(); + result["mrp"]["active_threshold_time_ms"] = info.result->mrpRemoteConfig.mActiveThresholdTime.count(); value["result"] = result; } diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index f1482ca77ae422..dc122039b7cd29 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -164,7 +164,7 @@ class SecureSession : public Session, public ReferenceCounted & session.Value()->AsSecureSession()->Activate( ScopedNodeId(1, sessionParams[i].mPeer.GetFabricIndex()), sessionParams[i].mPeer, CATValues(), static_cast(i), - ReliableMessageProtocolConfig(System::Clock::Milliseconds32(0), System::Clock::Milliseconds32(0))); + ReliableMessageProtocolConfig(System::Clock::Milliseconds32(0), System::Clock::Milliseconds32(0), + System::Clock::Milliseconds16(0))); session.Value()->AsSecureSession()->mLastActivityTime = sessionParams[i].mLastActivityTime; session.Value()->AsSecureSession()->mState = sessionParams[i].mState;