Skip to content

Commit

Permalink
Mitigation to unauthenticated TCP support advertisement in DNS-SD
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pidarped committed Aug 23, 2024
1 parent 96ee61e commit 88bb211
Show file tree
Hide file tree
Showing 20 changed files with 983 additions and 18 deletions.
43 changes: 33 additions & 10 deletions src/app/OperationalSessionSetup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <lib/address_resolve/AddressResolve.h>
#include <lib/core/CHIPCore.h>
#include <lib/core/CHIPEncoding.h>
#include <lib/dnssd/Advertiser.h>
#include <lib/dnssd/Resolver.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down Expand Up @@ -300,19 +301,41 @@ CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ResolveResult & re
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
if (mTransportPayloadCapability == TransportPayloadCapability::kLargePayload)
{
if (result.supportsTcpServer)
// First check if the TCPSupport information is available from prior
// CASE session negotiations with peer.
// Else, fall back to TCP support information from DNS-SD
// advertisements.
uint16_t supportedTransports;
uint32_t maxTCPMessageSize;
VerifyOrDie(mInitParams.sessionManager != nullptr);

if (mInitParams.sessionManager->GetPeerTCPParamsStorage() != nullptr &&
mInitParams.sessionManager->GetPeerTCPParamsStorage()->FindByScopedNodeId(mPeerId, supportedTransports, maxTCPMessageSize) == CHIP_NO_ERROR)
{
// Set the transport type for carrying large payloads
mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
if ((supportedTransports & to_underlying(Dnssd::TCPModeAdvertise::kTCPServer)) != 0)
{
// Set the transport type for carrying large payloads
mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
}
}
else
else // Failed to retrieve TCP Params from storage or TCPParamsStorage
// is not set up
{
// we should not set the large payload while the TCP support is not enabled
ChipLogError(
Discovery,
"LargePayload session requested but peer does not support TCP server, PeerNodeId=" ChipLogFormatScopedNodeId,
ChipLogValueScopedNodeId(mPeerId));
return CHIP_ERROR_INTERNAL;
// Check DNS-SD advertisement values if information not available via CASE session parameters
if (result.supportsTcpServer)
{
// Set the transport type for carrying large payloads
mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
}
else
{
// we should not set the large payload while the TCP support is not enabled from the peer's side.
ChipLogError(
Discovery,
"LargePayload session requested but peer does not support TCP server, PeerNodeId=" ChipLogFormatScopedNodeId,
ChipLogValueScopedNodeId(mPeerId));
return CHIP_ERROR_INTERNAL;
}
}
}
#endif
Expand Down
11 changes: 11 additions & 0 deletions src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
mOperationalKeystore = initParams.operationalKeystore;
mOpCertStore = initParams.opCertStore;
mSessionKeystore = initParams.sessionKeystore;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
mPeerTCPParamsStorage = initParams.peerTCPParamsStorage;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

if (initParams.certificateValidityPolicy)
{
Expand Down Expand Up @@ -278,6 +281,11 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
}
#endif // CHIP_CONFIG_ENABLE_SERVER_IM_EVENT

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
// Set the TCP Params storage after initializing SessionManager
mSessions.SetPeerTCPParamsStorage(mPeerTCPParamsStorage);
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

// This initializes clusters, so should come after lower level initialization.
InitDataModelHandler();

Expand Down Expand Up @@ -788,5 +796,8 @@ Crypto::DefaultSessionKeystore CommonCaseDeviceServerInitParams::sSessionKeystor
#if CHIP_CONFIG_ENABLE_ICD_CIP
app::DefaultICDCheckInBackOffStrategy CommonCaseDeviceServerInitParams::sDefaultICDCheckInBackOffStrategy;
#endif
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
Transport::PeerTCPParamsStorage CommonCaseDeviceServerInitParams::sPeerTCPParamsStorage;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

} // namespace chip
19 changes: 19 additions & 0 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
#include <app/TimerDelegates.h>
#include <app/reporting/ReportSchedulerImpl.h>
#include <transport/raw/UDP.h>
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
#include <transport/raw/PeerTCPParamsStorage.h>
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

#include <app/icd/server/ICDCheckInBackOffStrategy.h>
#if CHIP_CONFIG_ENABLE_ICD_SERVER
Expand Down Expand Up @@ -179,6 +182,11 @@ struct ServerInitParams
// Optional. Support for the ICD Check-In BackOff strategy. Must be initialized before being provided.
// If the ICD Check-In protocol use-case is supported and no strategy is provided, server will use the default strategy.
app::ICDCheckInBackOffStrategy * icdCheckInBackOffStrategy = nullptr;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
// Optional. Support Peer node's TCP Params when provided.
// Must be initialized before being provided.
chip::Transport::TCPParamsStorageInterface * peerTCPParamsStorage = nullptr;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
};

/**
Expand Down Expand Up @@ -300,6 +308,11 @@ struct CommonCaseDeviceServerInitParams : public ServerInitParams
}
#endif

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
ReturnErrorOnFailure(sPeerTCPParamsStorage.Init(this->persistentStorageDelegate));
this->peerTCPParamsStorage = &sPeerTCPParamsStorage;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

return CHIP_NO_ERROR;
}

Expand All @@ -311,6 +324,9 @@ struct CommonCaseDeviceServerInitParams : public ServerInitParams
static chip::app::DefaultTimerDelegate sTimerDelegate;
static app::reporting::ReportSchedulerImpl sReportScheduler;

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
static chip::Transport::PeerTCPParamsStorage sPeerTCPParamsStorage;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
#if CHIP_CONFIG_ENABLE_SESSION_RESUMPTION
static SimpleSessionResumptionStorage sSessionResumptionStorage;
#endif
Expand Down Expand Up @@ -675,6 +691,9 @@ class Server
GroupDataProviderListener mListener;
ServerFabricDelegate mFabricDelegate;
app::reporting::ReportScheduler * mReportScheduler;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
chip::Transport::TCPParamsStorageInterface * mPeerTCPParamsStorage;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

Access::AccessControl mAccessControl;
app::AclStorage * mAclStorage;
Expand Down
33 changes: 32 additions & 1 deletion src/controller/CHIPDeviceControllerFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
#include <app/server/Dnssd.h>
#include <protocols/secure_channel/CASEServer.h>
#include <protocols/secure_channel/SimpleSessionResumptionStorage.h>
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
#include <transport/raw/PeerTCPParamsStorage.h>
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

using namespace chip::Inet;
using namespace chip::System;
Expand Down Expand Up @@ -68,6 +71,9 @@ CHIP_ERROR DeviceControllerFactory::Init(FactoryInitParams params)
mSessionResumptionStorage = params.sessionResumptionStorage;
mEnableServerInteractions = params.enableServerInteractions;

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
mPeerTCPParamsStorage = params.peerTCPParamsStorage;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
// Initialize the system state. Note that it is left in a somewhat
// special state where it is initialized, but has a ref count of 0.
CHIP_ERROR err = InitSystemState(params);
Expand All @@ -84,7 +90,8 @@ CHIP_ERROR DeviceControllerFactory::ReinitSystemStateIfNecessary()
params.systemLayer = mSystemState->SystemLayer();
params.udpEndPointManager = mSystemState->UDPEndPointManager();
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
params.tcpEndPointManager = mSystemState->TCPEndPointManager();
params.tcpEndPointManager = mSystemState->TCPEndPointManager();
params.peerTCPParamsStorage = mPeerTCPParamsStorage;
#endif
#if CONFIG_NETWORK_LAYER_BLE
params.bleLayer = mSystemState->BleLayer();
Expand Down Expand Up @@ -238,6 +245,27 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
ReturnErrorOnFailure(stateParams.unsolicitedStatusHandler->Init(stateParams.exchangeMgr));
ReturnErrorOnFailure(stateParams.bdxTransferServer->Init(stateParams.systemLayer, stateParams.exchangeMgr));

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
chip::Transport::TCPParamsStorageInterface * peerTCPParamsStorage;
if (params.peerTCPParamsStorage == nullptr)
{
auto ownedPeerTCPParamStorage = chip::Platform::MakeUnique<chip::Transport::PeerTCPParamsStorage>();
ReturnErrorOnFailure(ownedPeerTCPParamStorage->Init(params.fabricIndependentStorage));
stateParams.ownedPeerTCPParamsStorage = std::move(ownedPeerTCPParamStorage);
stateParams.peerTCPParamsStorage = nullptr;
peerTCPParamsStorage = stateParams.ownedPeerTCPParamsStorage.get();
}
else
{
stateParams.ownedPeerTCPParamsStorage = nullptr;
stateParams.peerTCPParamsStorage = params.peerTCPParamsStorage;
peerTCPParamsStorage = stateParams.peerTCPParamsStorage;
}

// Set the TCP Params storage after initializing SessionManager
stateParams.sessionMgr->SetPeerTCPParamsStorage(peerTCPParamsStorage);
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

InitDataModelHandler();

ReturnErrorOnFailure(Dnssd::Resolver::Instance().Init(stateParams.udpEndPointManager));
Expand Down Expand Up @@ -429,6 +457,9 @@ void DeviceControllerFactory::Shutdown()
mOpCertStore = nullptr;
mCertificateValidityPolicy = nullptr;
mSessionResumptionStorage = nullptr;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
mPeerTCPParamsStorage = nullptr;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
}

void DeviceControllerSystemState::Shutdown()
Expand Down
11 changes: 10 additions & 1 deletion src/controller/CHIPDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
#include <credentials/OperationalCertificateStore.h>
#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
#include <protocols/secure_channel/SessionResumptionStorage.h>
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
#include <transport/raw/TCPParamsStorageInterface.h>
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

namespace chip {

Expand Down Expand Up @@ -148,6 +151,9 @@ struct FactoryInitParams
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
Transport::WiFiPAFLayer * wifipaf_layer = nullptr;
#endif
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
chip::Transport::TCPParamsStorageInterface * peerTCPParamsStorage = nullptr;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

//
// Controls enabling server cluster interactions on a controller. This in turn
Expand Down Expand Up @@ -292,7 +298,10 @@ class DeviceControllerFactory
Credentials::OperationalCertificateStore * mOpCertStore = nullptr;
Credentials::CertificateValidityPolicy * mCertificateValidityPolicy = nullptr;
SessionResumptionStorage * mSessionResumptionStorage = nullptr;
bool mEnableServerInteractions = false;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
chip::Transport::TCPParamsStorageInterface * mPeerTCPParamsStorage = nullptr;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
bool mEnableServerInteractions = false;
};

} // namespace Controller
Expand Down
22 changes: 22 additions & 0 deletions src/controller/CHIPDeviceControllerSystemState.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
#include <protocols/secure_channel/MessageCounterManager.h>
#include <protocols/secure_channel/SimpleSessionResumptionStorage.h>
#include <protocols/secure_channel/UnsolicitedStatusHandler.h>
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
#include <transport/raw/PeerTCPParamsStorage.h>
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

#include <transport/TransportMgr.h>
#include <transport/raw/UDP.h>
Expand Down Expand Up @@ -135,6 +138,10 @@ struct DeviceControllerSystemStateParams
FabricTable::Delegate * fabricTableDelegate = nullptr;
chip::app::reporting::ReportScheduler::TimerDelegate * timerDelegate = nullptr;
chip::app::reporting::ReportScheduler * reportScheduler = nullptr;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
chip::Transport::TCPParamsStorageInterface * peerTCPParamsStorage = nullptr;
Platform::UniquePtr<chip::Transport::PeerTCPParamsStorage> ownedPeerTCPParamsStorage;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
};

// A representation of the internal state maintained by the DeviceControllerFactory.
Expand Down Expand Up @@ -182,6 +189,17 @@ class DeviceControllerSystemState
#if CONFIG_NETWORK_LAYER_BLE
mBleLayer = params.bleLayer;
#endif
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
mOwnedPeerTCPParamsStorage = std::move(params.ownedPeerTCPParamsStorage);
if (mOwnedPeerTCPParamsStorage)
{
mPeerTCPParamsStorage = mOwnedPeerTCPParamsStorage.get();
}
else
{
mPeerTCPParamsStorage = params.peerTCPParamsStorage;
}
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
VerifyOrDie(IsInitialized());
};

Expand Down Expand Up @@ -273,6 +291,10 @@ class DeviceControllerSystemState
FabricTable::Delegate * mFabricTableDelegate = nullptr;
SessionResumptionStorage * mSessionResumptionStorage = nullptr;
Platform::UniquePtr<SimpleSessionResumptionStorage> mOwnedSessionResumptionStorage;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
chip::Transport::TCPParamsStorageInterface * mPeerTCPParamsStorage = nullptr;
Platform::UniquePtr<chip::Transport::PeerTCPParamsStorage> mOwnedPeerTCPParamsStorage;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

// If mTempFabricTable is not null, it was created during
// DeviceControllerFactory::InitSystemState and needs to be
Expand Down
9 changes: 9 additions & 0 deletions src/controller/python/chip/utils/DeviceProxyUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ struct __attribute__((packed)) SessionParametersStruct
uint16_t interactionModelRevision = 0;
uint32_t specificationVersion = 0;
uint16_t maxPathsPerInvoke = 0;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
uint16_t supportedTransports = 0;
uint32_t maxTCPMessageSize = 0;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
};

} // namespace python
Expand Down Expand Up @@ -99,6 +103,11 @@ PyChipError pychip_DeviceProxy_GetRemoteSessionParameters(DeviceProxy * device,
sessionParam->interactionModelRevision = remoteSessionParameters.GetInteractionModelRevision().ValueOr(0);
sessionParam->specificationVersion = remoteSessionParameters.GetSpecificationVersion().ValueOr(0);
sessionParam->maxPathsPerInvoke = remoteSessionParameters.GetMaxPathsPerInvoke();
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
sessionParam->supportedTransports = remoteSessionParameters.GetSupportedTransports().ValueOr(0);
sessionParam->maxTCPMessageSize =
remoteSessionParameters.GetMaxTCPMessageSize().ValueOr(System::PacketBuffer::kLargeBufMaxSizeWithoutReserve - 4);
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
return ToPyChipError(CHIP_NO_ERROR);
}
}
8 changes: 8 additions & 0 deletions src/lib/support/DefaultStorageKeyAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,14 @@ class DefaultStorageKeyAllocator
// when new fabric is created, this list needs to be updated,
// when client init DefaultICDClientStorage, this table needs to be loaded.
static StorageKeyName ICDFabricList() { return StorageKeyName::FromConst("g/icdfl"); }

// TCP peer parameters
static StorageKeyName TCPPeerParams(FabricIndex fabric, NodeId nodeId)
{
return StorageKeyName::Formatted("f/%x/tcp/%08" PRIX32 "%08" PRIX32, fabric, static_cast<uint32_t>(nodeId >> 32),
static_cast<uint32_t>(nodeId));
}
static StorageKeyName TCPPeerList() { return StorageKeyName::FromConst("g/tcpl"); }
};

} // namespace chip
25 changes: 24 additions & 1 deletion src/messaging/SessionParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,19 @@ class SessionParameters
static constexpr size_t kSizeOfInteractionModelRevision = sizeof(uint16_t);
static constexpr size_t kSizeOfSpecificationVersion = sizeof(uint32_t);
static constexpr size_t kSizeOfMaxPathsPerInvoke = sizeof(uint16_t);
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
static constexpr size_t kSizeOfSupportedTransports = sizeof(uint16_t);
static constexpr size_t kSizeOfMaxTCPMessageSize = sizeof(uint32_t);
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

static constexpr size_t kEstimatedTLVSize = TLV::EstimateStructOverhead(
kSizeOfSessionIdleInterval, kSizeOfSessionActiveInterval, kSizeOfSessionActiveThreshold, kSizeOfDataModelRevision,
kSizeOfInteractionModelRevision, kSizeOfSpecificationVersion, kSizeOfMaxPathsPerInvoke);
kSizeOfInteractionModelRevision, kSizeOfSpecificationVersion, kSizeOfMaxPathsPerInvoke
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
,
kSizeOfSupportedTransports, kSizeOfMaxTCPMessageSize
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
);

// From Section 4.12.8 "Parameters and Constants" in chapter "Secure Channel".
enum Tag : uint32_t
Expand All @@ -56,6 +65,8 @@ class SessionParameters
kInteractionModelRevision = 5,
kSpecificationVersion = 6,
kMaxPathsPerInvoke = 7,
kSupportedTransports = 8,
kMaxTCPMessageSize = 9,
};

const ReliableMessageProtocolConfig & GetMRPConfig() const { return mMRPConfig; }
Expand Down Expand Up @@ -91,6 +102,13 @@ class SessionParameters
uint16_t GetMaxPathsPerInvoke() const { return mMaxPathsPerInvoke; }
void SetMaxPathsPerInvoke(const uint16_t maxPathsPerInvoke) { mMaxPathsPerInvoke = maxPathsPerInvoke; }

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
const Optional<uint16_t> & GetSupportedTransports() const { return mSupportedTransports; }
void SetSupportedTransports(const uint16_t supportedTransports) { mSupportedTransports = MakeOptional(supportedTransports); }

const Optional<uint32_t> & GetMaxTCPMessageSize() const { return mMaxTCPMessageSize; }
void SetMaxTCPMessageSize(const uint32_t maxTCPMessageSize) { mMaxTCPMessageSize = MakeOptional(maxTCPMessageSize); }
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
private:
ReliableMessageProtocolConfig mMRPConfig;
// For legacy reasons if we do not get DataModelRevision it means either 16 or 17. But there isn't
Expand All @@ -104,6 +122,11 @@ class SessionParameters
Optional<uint32_t> mSpecificationVersion;
// When maxPathsPerInvoke is not provided legacy is always 1
uint16_t mMaxPathsPerInvoke = 1;

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
Optional<uint16_t> mSupportedTransports;
Optional<uint32_t> mMaxTCPMessageSize;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
};

} // namespace chip
Loading

0 comments on commit 88bb211

Please sign in to comment.