Skip to content

Commit 321fcfd

Browse files
pidarpedpidarped
committed
Mitigation to unauthenticated TCP support advertisement in DNS-SD
that made it susceptible to potential DoS attacks. As part of this resolution, Add TCP support and Max receive size info within negotiated CASE Session parameters. --Nodes publish TCP support information during CASE session(over MRP) negotiations. --Add TCP param storage logic to persist received TCP support info to storage during CASE session establishment and retrieve it during future TCP session setup. --During TCP session setup, first check if the TCP support info for peer is available in storage before, going to the DNS-SD advertised values.
1 parent ba949bf commit 321fcfd

15 files changed

+803
-12
lines changed

‎src/app/OperationalSessionSetup.cpp

+32-10
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <lib/address_resolve/AddressResolve.h>
3434
#include <lib/core/CHIPCore.h>
3535
#include <lib/core/CHIPEncoding.h>
36+
#include <lib/dnssd/Advertiser.h>
3637
#include <lib/dnssd/Resolver.h>
3738
#include <lib/support/CodeUtils.h>
3839
#include <lib/support/logging/CHIPLogging.h>
@@ -300,19 +301,40 @@ CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ResolveResult & re
300301
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
301302
if (mTransportPayloadCapability == TransportPayloadCapability::kLargePayload)
302303
{
303-
if (result.supportsTcpServer)
304+
// First check if the TCPSupport information is available from prior
305+
// CASE session negotiations with peer.
306+
// Else, fall back to TCP support information from DNS-SD
307+
// advertisements.
308+
uint16_t supportedTransports;
309+
uint32_t maxTCPMessageSize;
310+
VerifyOrDie(mInitParams.sessionManager != nullptr);
311+
CHIP_ERROR err =
312+
mInitParams.sessionManager->GetPeerTCPParamsStorage().FindByScopedNodeId(mPeerId, supportedTransports, maxTCPMessageSize);
313+
if (err == CHIP_NO_ERROR)
304314
{
305-
// Set the transport type for carrying large payloads
306-
mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
315+
if ((supportedTransports & to_underlying(Dnssd::TCPModeAdvertise::kTCPServer)) != 0)
316+
{
317+
// Set the transport type for carrying large payloads
318+
mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
319+
}
307320
}
308-
else
321+
else // Failed to retrieve TCP Params from storage
309322
{
310-
// we should not set the large payload while the TCP support is not enabled
311-
ChipLogError(
312-
Discovery,
313-
"LargePayload session requested but peer does not support TCP server, PeerNodeId=" ChipLogFormatScopedNodeId,
314-
ChipLogValueScopedNodeId(mPeerId));
315-
return CHIP_ERROR_INTERNAL;
323+
// Check DNS-SD advertisement values if information not available via CASE session parameters
324+
if (result.supportsTcpServer)
325+
{
326+
// Set the transport type for carrying large payloads
327+
mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
328+
}
329+
else
330+
{
331+
// we should not set the large payload while the TCP support is not enabled from the peer's side.
332+
ChipLogError(
333+
Discovery,
334+
"LargePayload session requested but peer does not support TCP server, PeerNodeId=" ChipLogFormatScopedNodeId,
335+
ChipLogValueScopedNodeId(mPeerId));
336+
return CHIP_ERROR_INTERNAL;
337+
}
316338
}
317339
}
318340
#endif

‎src/controller/python/chip/utils/DeviceProxyUtils.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ struct __attribute__((packed)) SessionParametersStruct
4444
uint16_t interactionModelRevision = 0;
4545
uint32_t specificationVersion = 0;
4646
uint16_t maxPathsPerInvoke = 0;
47+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
48+
uint16_t supportedTransports = 0;
49+
uint32_t maxTCPMessageSize = 0;
50+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
4751
};
4852

4953
} // namespace python
@@ -99,6 +103,10 @@ PyChipError pychip_DeviceProxy_GetRemoteSessionParameters(DeviceProxy * device,
99103
sessionParam->interactionModelRevision = remoteSessionParameters.GetInteractionModelRevision().ValueOr(0);
100104
sessionParam->specificationVersion = remoteSessionParameters.GetSpecificationVersion().ValueOr(0);
101105
sessionParam->maxPathsPerInvoke = remoteSessionParameters.GetMaxPathsPerInvoke();
106+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
107+
sessionParam->supportedTransports = remoteSessionParameters.GetSupportedTransports().ValueOr(0);
108+
sessionParam->maxTCPMessageSize = remoteSessionParameters.GetMaxTCPMessageSize().ValueOr(System::PacketBuffer::kLargeBufMaxSizeWithoutReserve - 4);
109+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
102110
return ToPyChipError(CHIP_NO_ERROR);
103111
}
104112
}

‎src/lib/support/DefaultStorageKeyAllocator.h

+9
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,15 @@ class DefaultStorageKeyAllocator
256256
// when new fabric is created, this list needs to be updated,
257257
// when client init DefaultICDClientStorage, this table needs to be loaded.
258258
static StorageKeyName ICDFabricList() { return StorageKeyName::FromConst("g/icdfl"); }
259+
260+
// TCP peer parameters
261+
static StorageKeyName TCPPeerParams(FabricIndex fabric, NodeId nodeId)
262+
{
263+
return StorageKeyName::Formatted("f/%x/tcp/%08" PRIX32 "%08" PRIX32, fabric, static_cast<uint32_t>(nodeId >> 32),
264+
static_cast<uint32_t>(nodeId));
265+
}
266+
static StorageKeyName TCPPeerList() { return StorageKeyName::FromConst("g/tcpl"); }
267+
259268
};
260269

261270
} // namespace chip

‎src/messaging/SessionParameters.h

+23-1
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,18 @@ class SessionParameters
4141
static constexpr size_t kSizeOfInteractionModelRevision = sizeof(uint16_t);
4242
static constexpr size_t kSizeOfSpecificationVersion = sizeof(uint32_t);
4343
static constexpr size_t kSizeOfMaxPathsPerInvoke = sizeof(uint16_t);
44+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
45+
static constexpr size_t kSizeOfSupportedTransports = sizeof(uint16_t);
46+
static constexpr size_t kSizeOfMaxTCPMessageSize = sizeof(uint32_t);
47+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
4448

4549
static constexpr size_t kEstimatedTLVSize = TLV::EstimateStructOverhead(
4650
kSizeOfSessionIdleInterval, kSizeOfSessionActiveInterval, kSizeOfSessionActiveThreshold, kSizeOfDataModelRevision,
47-
kSizeOfInteractionModelRevision, kSizeOfSpecificationVersion, kSizeOfMaxPathsPerInvoke);
51+
kSizeOfInteractionModelRevision, kSizeOfSpecificationVersion, kSizeOfMaxPathsPerInvoke
52+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
53+
, kSizeOfSupportedTransports, kSizeOfMaxTCPMessageSize
54+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
55+
);
4856

4957
// From Section 4.12.8 "Parameters and Constants" in chapter "Secure Channel".
5058
enum Tag : uint32_t
@@ -56,6 +64,8 @@ class SessionParameters
5664
kInteractionModelRevision = 5,
5765
kSpecificationVersion = 6,
5866
kMaxPathsPerInvoke = 7,
67+
kSupportedTransports = 8,
68+
kMaxTCPMessageSize = 9,
5969
};
6070

6171
const ReliableMessageProtocolConfig & GetMRPConfig() const { return mMRPConfig; }
@@ -91,6 +101,13 @@ class SessionParameters
91101
uint16_t GetMaxPathsPerInvoke() const { return mMaxPathsPerInvoke; }
92102
void SetMaxPathsPerInvoke(const uint16_t maxPathsPerInvoke) { mMaxPathsPerInvoke = maxPathsPerInvoke; }
93103

104+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
105+
const Optional<uint16_t> & GetSupportedTransports() const { return mSupportedTransports; }
106+
void SetSupportedTransports(const uint16_t supportedTransports) { mSupportedTransports = MakeOptional(supportedTransports); }
107+
108+
const Optional<uint32_t> & GetMaxTCPMessageSize() const { return mMaxTCPMessageSize; }
109+
void SetMaxTCPMessageSize(const uint32_t maxTCPMessageSize) { mMaxTCPMessageSize = MakeOptional(maxTCPMessageSize); }
110+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
94111
private:
95112
ReliableMessageProtocolConfig mMRPConfig;
96113
// For legacy reasons if we do not get DataModelRevision it means either 16 or 17. But there isn't
@@ -104,6 +121,11 @@ class SessionParameters
104121
Optional<uint32_t> mSpecificationVersion;
105122
// When maxPathsPerInvoke is not provided legacy is always 1
106123
uint16_t mMaxPathsPerInvoke = 1;
124+
125+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
126+
Optional<uint16_t> mSupportedTransports;
127+
Optional<uint32_t> mMaxTCPMessageSize;
128+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
107129
};
108130

109131
} // namespace chip

‎src/protocols/secure_channel/CASESession.cpp

+32
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include <tracing/macros.h>
5050
#include <tracing/metric_event.h>
5151
#include <transport/SessionManager.h>
52+
#include <transport/raw/PeerTCPParamsStorage.h>
5253

5354
namespace {
5455

@@ -696,6 +697,24 @@ CHIP_ERROR CASESession::RecoverInitiatorIpk()
696697
}
697698

698699
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
700+
CHIP_ERROR CASESession::SaveTCPInfoFromRemoteSessionParams()
701+
{
702+
if (GetRemoteSessionParameters().GetSupportedTransports().HasValue() && GetRemoteSessionParameters().GetMaxTCPMessageSize().HasValue())
703+
{
704+
ReturnErrorOnFailure(mSessionManager->GetPeerTCPParamsStorage().SaveTCPParams(GetPeer(),
705+
GetRemoteSessionParameters().GetSupportedTransports().Value(),
706+
GetRemoteSessionParameters().GetMaxTCPMessageSize().Value()));
707+
}
708+
else
709+
{
710+
// Store zeroes for the TCP parameters as sentinel values from CASE
711+
// session for no TCP support.
712+
ReturnErrorOnFailure(mSessionManager->GetPeerTCPParamsStorage().SaveTCPParams(GetPeer(), 0, 0));
713+
}
714+
715+
return CHIP_NO_ERROR;
716+
}
717+
699718
void CASESession::HandleConnectionAttemptComplete(Transport::ActiveTCPConnectionState * conn, CHIP_ERROR err)
700719
{
701720
VerifyOrReturn(conn != nullptr);
@@ -1298,6 +1317,10 @@ CHIP_ERROR CASESession::HandleSigma2Resume(System::PacketBufferHandle && msg)
12981317
SuccessOrExit(err = DecodeMRPParametersIfPresent(TLV::ContextTag(4), tlvReader));
12991318
mExchangeCtxt.Value()->GetSessionHandle()->AsUnauthenticatedSession()->SetRemoteSessionParameters(
13001319
GetRemoteSessionParameters());
1320+
1321+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
1322+
ReturnErrorOnFailure(SaveTCPInfoFromRemoteSessionParams());
1323+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
13011324
}
13021325

13031326
ChipLogDetail(SecureChannel, "Peer assigned session session ID %d", responderSessionId);
@@ -1500,6 +1523,10 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
15001523
SuccessOrExit(err = DecodeMRPParametersIfPresent(TLV::ContextTag(kTag_Sigma2_ResponderMRPParams), tlvReader));
15011524
mExchangeCtxt.Value()->GetSessionHandle()->AsUnauthenticatedSession()->SetRemoteSessionParameters(
15021525
GetRemoteSessionParameters());
1526+
1527+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
1528+
ReturnErrorOnFailure(SaveTCPInfoFromRemoteSessionParams());
1529+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
15031530
}
15041531

15051532
exit:
@@ -2198,6 +2225,11 @@ CHIP_ERROR CASESession::ParseSigma1(TLV::ContiguousBufferTLVReader & tlvReader,
21982225
ReturnErrorOnFailure(DecodeMRPParametersIfPresent(TLV::ContextTag(kInitiatorMRPParamsTag), tlvReader));
21992226
mExchangeCtxt.Value()->GetSessionHandle()->AsUnauthenticatedSession()->SetRemoteSessionParameters(
22002227
GetRemoteSessionParameters());
2228+
2229+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
2230+
ReturnErrorOnFailure(SaveTCPInfoFromRemoteSessionParams());
2231+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
2232+
22012233
err = tlvReader.Next();
22022234
}
22032235

‎src/protocols/secure_channel/CASESession.h

+1
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
287287
void InvalidateIfPendingEstablishmentOnFabric(FabricIndex fabricIndex);
288288

289289
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
290+
CHIP_ERROR SaveTCPInfoFromRemoteSessionParams();
290291
static void HandleConnectionAttemptComplete(Transport::ActiveTCPConnectionState * conn, CHIP_ERROR conErr);
291292
static void HandleConnectionClosed(Transport::ActiveTCPConnectionState * conn, CHIP_ERROR conErr);
292293

‎src/protocols/secure_channel/PairingSession.cpp

+35
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,16 @@ CHIP_ERROR PairingSession::EncodeSessionParameters(TLV::Tag tag, const ReliableM
142142

143143
uint16_t maxPathsPerInvoke = CHIP_CONFIG_MAX_PATHS_PER_INVOKE;
144144
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(SessionParameters::Tag::kMaxPathsPerInvoke), maxPathsPerInvoke));
145+
146+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
147+
// Support for both TCP CLient and Server(0x06)
148+
uint16_t supportedTransports = 0x06;
149+
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(SessionParameters::Tag::kSupportedTransports), supportedTransports));
150+
151+
uint32_t maxTCPMessageSize = System::PacketBuffer::kLargeBufMaxSizeWithoutReserve - 4/* Framing length field of 4 bytes */;
152+
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(SessionParameters::Tag::kMaxTCPMessageSize), maxTCPMessageSize));
153+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
154+
145155
return tlvWriter.EndContainer(mrpParamsContainer);
146156
}
147157

@@ -234,6 +244,31 @@ CHIP_ERROR PairingSession::DecodeMRPParametersIfPresent(TLV::Tag expectedTag, TL
234244
SuccessOrExit(err = tlvReader.Next());
235245
}
236246

247+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
248+
249+
if (TLV::TagNumFromTag(tlvReader.GetTag()) == SessionParameters::Tag::kSupportedTransports)
250+
{
251+
ChipLogDetail(SecureChannel, "Found TCPSupport parameter in the message");
252+
uint16_t supportedTransports;
253+
ReturnErrorOnFailure(tlvReader.Get(supportedTransports));
254+
mRemoteSessionParams.SetSupportedTransports(supportedTransports);
255+
256+
// The next element is optional. If it's not present, return CHIP_NO_ERROR.
257+
SuccessOrExit(err = tlvReader.Next());
258+
}
259+
260+
if (TLV::TagNumFromTag(tlvReader.GetTag()) == SessionParameters::Tag::kMaxTCPMessageSize)
261+
{
262+
ChipLogDetail(SecureChannel, "Found TCP Max message size parameter in the message");
263+
uint32_t maxTCPMessageSize;
264+
ReturnErrorOnFailure(tlvReader.Get(maxTCPMessageSize));
265+
mRemoteSessionParams.SetMaxTCPMessageSize(maxTCPMessageSize);
266+
267+
// The next element is optional. If it's not present, return CHIP_NO_ERROR.
268+
SuccessOrExit(err = tlvReader.Next());
269+
}
270+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
271+
237272
// Future proofing - Don't error out if there are other tags
238273
exit:
239274
if (err == CHIP_END_OF_TLV)

‎src/transport/SessionManager.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ CHIP_ERROR SessionManager::Init(System::Layer * systemLayer, TransportMgrBase *
150150
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
151151
mConnCompleteCb = nullptr;
152152
mConnClosedCb = nullptr;
153+
154+
ReturnErrorOnFailure(mPeerTCPParamsStorage.Init(storageDelegate));
153155
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
154156

155157
return CHIP_NO_ERROR;

‎src/transport/SessionManager.h

+3
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454

5555
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
5656
#include <transport/SessionConnectionDelegate.h>
57+
#include <transport/raw/PeerTCPParamsStorage.h>
5758
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
5859

5960
namespace chip {
@@ -494,6 +495,7 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate, public FabricTabl
494495

495496
using OnTCPConnectionClosedCallback = void (*)(Transport::ActiveTCPConnectionState * conn, CHIP_ERROR conErr);
496497

498+
Transport::PeerTCPParamsStorage & GetPeerTCPParamsStorage() { return mPeerTCPParamsStorage; }
497499
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
498500

499501
Optional<SessionHandle> CreateUnauthenticatedSession(const Transport::PeerAddress & peerAddress,
@@ -566,6 +568,7 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate, public FabricTabl
566568
// Hold the TCPConnection callback context for the receiver application in the SessionManager.
567569
// On receipt of a connection from a peer, the SessionManager
568570
Transport::AppTCPConnectionCallbackCtxt * mServerTCPConnCbCtxt = nullptr;
571+
Transport::PeerTCPParamsStorage mPeerTCPParamsStorage;
569572
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
570573

571574
SessionMessageDelegate * mCB = nullptr;

‎src/transport/raw/BUILD.gn

+2
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ static_library("raw") {
3232
if (chip_inet_config_enable_tcp_endpoint) {
3333
sources += [
3434
"ActiveTCPConnectionState.h",
35+
"PeerTCPParamsStorage.h",
36+
"PeerTCPParamsStorage.cpp",
3537
"TCP.cpp",
3638
"TCP.h",
3739
"TCPConfig.h",

0 commit comments

Comments
 (0)