Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop advertising IPv4 addresses when IPv4 and IPv6 ports do not match. #38057

Merged
merged 1 commit into from
Mar 19, 2025
Merged

Stop advertising IPv4 addresses when IPv4 and IPv6 ports do not match. #38057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/app/server/Dnssd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()
.SetPort(GetSecuredPort())
.SetInterfaceId(GetInterfaceId())
.SetLocalMRPConfig(GetLocalMRPConfig().std_optional())
.EnableIpV4(true);
.EnableIpV4(SecuredIPv4PortMatchesIPv6Port());

#if CHIP_CONFIG_ENABLE_ICD_SERVER
AddICDKeyToAdvertisement(advertiseParameters);
Expand All @@ -234,7 +234,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
auto advertiseParameters = chip::Dnssd::CommissionAdvertisingParameters()
.SetPort(commissionableNode ? GetSecuredPort() : GetUnsecuredPort())
.SetInterfaceId(GetInterfaceId())
.EnableIpV4(true);
.EnableIpV4(!commissionableNode || SecuredIPv4PortMatchesIPv6Port());
advertiseParameters.SetCommissionAdvertiseMode(commissionableNode ? chip::Dnssd::CommssionAdvertiseMode::kCommissionableNode
: chip::Dnssd::CommssionAdvertiseMode::kCommissioner);

Expand Down
29 changes: 25 additions & 4 deletions src/app/server/Dnssd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <app/icd/server/ICDStateObserver.h>
#include <app/server/CommissioningModeProvider.h>
#include <credentials/FabricTable.h>
#include <inet/InetConfig.h>
#include <lib/core/CHIPError.h>
#include <lib/core/Optional.h>
#include <lib/dnssd/Advertiser.h>
Expand All @@ -46,11 +47,16 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver
return instance;
}

/// Sets the secure Matter port
void SetSecuredPort(uint16_t port) { mSecuredPort = port; }
/// Sets the secure Matter IPv6 port
void SetSecuredIPv6Port(uint16_t port) { mSecuredIPv6Port = port; }

#if INET_CONFIG_ENABLE_IPV4
/// Sets the secure Matter IPv4 port.
void SetSecuredIPv4Port(uint16_t port) { mSecuredIPv4Port = port; }
#endif // INET_CONFIG_ENABLE_IPV4

/// Gets the secure Matter port
uint16_t GetSecuredPort() const { return mSecuredPort; }
uint16_t GetSecuredPort() const { return mSecuredIPv6Port; }

/// Sets the unsecure Matter port
void SetUnsecuredPort(uint16_t port) { mUnsecuredPort = port; }
Expand Down Expand Up @@ -176,6 +182,18 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver
//
bool HaveOperationalCredentials();

// Check whether the secured IPv4 port matches the secured IPv6 port. If it
// does not, we should not advertise our IPv4 bits, because we can only
// advertise one port number.
bool SecuredIPv4PortMatchesIPv6Port() const
{
#if INET_CONFIG_ENABLE_IPV4
return mSecuredIPv4Port == mSecuredIPv6Port;
#else
return false;
#endif // INET_CONFIG_ENABLE_IPV4
}

FabricTable * mFabricTable = nullptr;
CommissioningModeProvider * mCommissioningModeProvider = nullptr;

Expand All @@ -187,7 +205,10 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver
bool mTCPServerEnabled = true;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

uint16_t mSecuredPort = CHIP_PORT;
uint16_t mSecuredIPv6Port = CHIP_PORT;
#if INET_CONFIG_ENABLE_IPV4
uint16_t mSecuredIPv4Port = CHIP_PORT;
#endif // INET_CONFIG_ENABLE_IPV4
uint16_t mUnsecuredPort = CHIP_UDC_PORT;
Inet::InterfaceId mInterfaceId = Inet::InterfaceId::Null();

Expand Down
16 changes: 9 additions & 7 deletions src/app/server/Server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,17 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
}

// Init transport before operations with secure session mgr.
//
// The logic below expects that the IPv6 transport is at index 0. Keep that logic in sync with
// this code.
err = mTransports.Init(UdpListenParameters(DeviceLayer::UDPEndPointManager())
.SetAddressType(IPAddressType::kIPv6)
.SetListenPort(mOperationalServicePort)
.SetNativeParams(initParams.endpointNativeParams)
#if INET_CONFIG_ENABLE_IPV4
,
// The logic below expects that the IPv4 transport, if enabled, is at
// index 1. Keep that logic in sync with this code.
UdpListenParameters(DeviceLayer::UDPEndPointManager())
.SetAddressType(IPAddressType::kIPv4)
.SetListenPort(mOperationalServicePort)
Expand Down Expand Up @@ -318,13 +323,10 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
SuccessOrExit(err);
#endif

//
// We need to advertise the port that we're listening to for unsolicited messages over UDP. However, we have both a IPv4
// and IPv6 endpoint to pick from. Given that the listen port passed in may be set to 0 (which then has the kernel select
// a valid port at bind time), that will result in two possible ports being provided back from the resultant endpoint
// initializations. Since IPv6 is POR for Matter, let's go ahead and pick that port.
//
app::DnssdServer::Instance().SetSecuredPort(mTransports.GetTransport().GetImplAtIndex<0>().GetBoundPort());
app::DnssdServer::Instance().SetSecuredIPv6Port(mTransports.GetTransport().GetImplAtIndex<0>().GetBoundPort());
#if INET_CONFIG_ENABLE_IPV4
app::DnssdServer::Instance().SetSecuredIPv4Port(mTransports.GetTransport().GetImplAtIndex<1>().GetBoundPort());
#endif // INET_CONFIG_ENABLE_IPV4

app::DnssdServer::Instance().SetUnsecuredPort(mUserDirectedCommissioningPort);
app::DnssdServer::Instance().SetInterfaceId(mInterfaceId);
Expand Down
23 changes: 15 additions & 8 deletions src/controller/CHIPDeviceControllerFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,18 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
stateParams.transportMgr = chip::Platform::New<DeviceTransportMgr>();

//
// The logic below expects IPv6 to be at index 0 of this tuple. Please do not alter that.
// The logic below expects IPv6 to be at index 0 of this tuple. Keep that logic in sync with
// this code.
//
ReturnErrorOnFailure(stateParams.transportMgr->Init(Transport::UdpListenParameters(stateParams.udpEndPointManager)
.SetAddressType(Inet::IPAddressType::kIPv6)
.SetListenPort(params.listenPort)
#if INET_CONFIG_ENABLE_IPV4
,
//
// The logic below expects IPv4 to be at index 1 of this tuple,
// if it's enabled. Keep that logic in sync with this code.
//
Transport::UdpListenParameters(stateParams.udpEndPointManager)
.SetAddressType(Inet::IPAddressType::kIPv4)
.SetListenPort(params.listenPort)
Expand Down Expand Up @@ -272,13 +277,15 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
stateParams.exchangeMgr, stateParams.sessionMgr, stateParams.fabricTable, sessionResumptionStorage,
stateParams.certificateValidityPolicy, stateParams.groupDataProvider));

//
// We need to advertise the port that we're listening to for unsolicited messages over UDP. However, we have both a IPv4
// and IPv6 endpoint to pick from. Given that the listen port passed in may be set to 0 (which then has the kernel select
// a valid port at bind time), that will result in two possible ports being provided back from the resultant endpoint
// initializations. Since IPv6 is POR for Matter, let's go ahead and pick that port.
//
app::DnssdServer::Instance().SetSecuredPort(stateParams.transportMgr->GetTransport().GetImplAtIndex<0>().GetBoundPort());
// Our IPv6 transport is at index 0.
app::DnssdServer::Instance().SetSecuredIPv6Port(
stateParams.transportMgr->GetTransport().GetImplAtIndex<0>().GetBoundPort());

#if INET_CONFIG_ENABLE_IPV4
// If enabled, our IPv4 transport is at index 1.
app::DnssdServer::Instance().SetSecuredIPv4Port(
stateParams.transportMgr->GetTransport().GetImplAtIndex<1>().GetBoundPort());
#endif // INET_CONFIG_ENABLE_IPV4

//
// TODO: This is a hack to workaround the fact that we have a bi-polar stack that has controller and server modalities that
Expand Down
23 changes: 15 additions & 8 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <app/icd/server/ICDServerConfig.h>
#include <crypto/RandUtils.h>
#include <inet/InetConfig.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPSafeCasts.h>
#include <lib/dnssd/IPAddressSorter.h>
Expand Down Expand Up @@ -527,33 +528,39 @@ CHIP_ERROR DiscoveryImplPlatform::PublishService(const char * serviceType, TextE
const OperationalAdvertisingParameters & params)
{
return PublishService(serviceType, textEntries, textEntrySize, subTypes, subTypeSize, params.GetPort(), params.GetInterfaceId(),
params.GetMac(), DnssdServiceProtocol::kDnssdProtocolTcp, params.GetPeerId());
params.GetMac(), DnssdServiceProtocol::kDnssdProtocolTcp, params.GetPeerId(), params.IsIPv4Enabled());
}

CHIP_ERROR DiscoveryImplPlatform::PublishService(const char * serviceType, TextEntry * textEntries, size_t textEntrySize,
const char ** subTypes, size_t subTypeSize,
const CommissionAdvertisingParameters & params)
{
return PublishService(serviceType, textEntries, textEntrySize, subTypes, subTypeSize, params.GetPort(), params.GetInterfaceId(),
params.GetMac(), DnssdServiceProtocol::kDnssdProtocolUdp, PeerId());
params.GetMac(), DnssdServiceProtocol::kDnssdProtocolUdp, PeerId(), params.IsIPv4Enabled());
}

CHIP_ERROR DiscoveryImplPlatform::PublishService(const char * serviceType, TextEntry * textEntries, size_t textEntrySize,
const char ** subTypes, size_t subTypeSize, uint16_t port,
Inet::InterfaceId interfaceId, const chip::ByteSpan & mac,
DnssdServiceProtocol protocol, PeerId peerId)
DnssdServiceProtocol protocol, PeerId peerId, bool ipv4Enabled)
{
DnssdService service;
ReturnErrorOnFailure(MakeHostName(service.mHostName, sizeof(service.mHostName), mac));
ReturnErrorOnFailure(protocol == DnssdServiceProtocol::kDnssdProtocolTcp
? MakeInstanceName(service.mName, sizeof(service.mName), peerId)
: GetCommissionableInstanceName(service.mName, sizeof(service.mName)));
Platform::CopyString(service.mType, serviceType);
#if INET_CONFIG_ENABLE_IPV4
service.mAddressType = Inet::IPAddressType::kAny;
#else
service.mAddressType = Inet::IPAddressType::kIPv6;
#endif
#if !INET_CONFIG_ENABLE_IPV4
ipv4Enabled = false;
#endif // INET_CONFIG_ENABLE_IPV4
if (ipv4Enabled)
{
service.mAddressType = Inet::IPAddressType::kAny;
}
else
{
service.mAddressType = Inet::IPAddressType::kIPv6;
}
service.mInterface = interfaceId;
service.mProtocol = protocol;
service.mPort = port;
Expand Down
3 changes: 2 additions & 1 deletion src/lib/dnssd/Discovery_ImplPlatform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,10 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver
size_t subTypeSize, const OperationalAdvertisingParameters & params);
CHIP_ERROR PublishService(const char * serviceType, TextEntry * textEntries, size_t textEntrySize, const char ** subTypes,
size_t subTypeSize, const CommissionAdvertisingParameters & params);
// ipv4Enabled will be ignored if we don't actually support IPv4.
CHIP_ERROR PublishService(const char * serviceType, TextEntry * textEntries, size_t textEntrySize, const char ** subTypes,
size_t subTypeSize, uint16_t port, Inet::InterfaceId interfaceId, const chip::ByteSpan & mac,
DnssdServiceProtocol procotol, PeerId peerId);
DnssdServiceProtocol procotol, PeerId peerId, bool ipv4Enabled);

static void HandleNodeIdResolve(void * context, DnssdService * result, const Span<Inet::IPAddress> & addresses,
CHIP_ERROR error);
Expand Down
Loading