Skip to content

Commit

Permalink
[darwin] Use DNSServiceReconfirmRecord for A and AAAA records to miti… (
Browse files Browse the repository at this point in the history
project-chip#23067)

* [Dnssd] Add ReconfirmRecord method to verify address that appears to be out of date

* [SetUpCodePairer] Ask Dnssd to reconfirm discovered addresses if connecting to them ends with a CHIP_ERROR_TIMEOUT
  • Loading branch information
vivien-apple authored and cliffamzn committed Feb 22, 2023
1 parent 413198b commit ebce6ae
Show file tree
Hide file tree
Showing 20 changed files with 249 additions and 15 deletions.
61 changes: 51 additions & 10 deletions src/controller/SetUpCodePairer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ bool SetUpCodePairer::ConnectToDiscoveredDevice()
// Remove it from the queue before we try to connect, in case the
// connection attempt fails and calls right back into us to try the next
// thing.
RendezvousParameters params(mDiscoveredParameters.front());
SetUpCodePairerParameters params(mDiscoveredParameters.front());
mDiscoveredParameters.pop();

params.SetSetupPINCode(mSetUpPINCode);
Expand All @@ -230,6 +230,11 @@ bool SetUpCodePairer::ConnectToDiscoveredDevice()
// Handle possibly-sync call backs from attempts to establish PASE.
ExpectPASEEstablishment();

if (params.GetPeerAddress().GetTransportType() == Transport::Type::kUdp)
{
mCurrentPASEParameters.SetValue(params);
}

CHIP_ERROR err;
if (mConnectionType == SetupCodePairerBehaviour::kCommission)
{
Expand Down Expand Up @@ -260,9 +265,7 @@ void SetUpCodePairer::OnDiscoveredDeviceOverBle(BLE_CONNECTION_OBJECT connObj)

mWaitingForDiscovery[kBLETransport] = false;

Transport::PeerAddress peerAddress = Transport::PeerAddress::BLE();
mDiscoveredParameters.emplace();
mDiscoveredParameters.back().SetPeerAddress(peerAddress).SetConnectionObject(connObj);
mDiscoveredParameters.emplace(connObj);
ConnectToDiscoveredDevice();
}

Expand Down Expand Up @@ -312,12 +315,7 @@ void SetUpCodePairer::NotifyCommissionableDeviceDiscovered(const Dnssd::Discover

ChipLogProgress(Controller, "Discovered device to be commissioned over DNS-SD");

Inet::InterfaceId interfaceId =
nodeData.resolutionData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.resolutionData.interfaceId : Inet::InterfaceId::Null();
Transport::PeerAddress peerAddress =
Transport::PeerAddress::UDP(nodeData.resolutionData.ipAddress[0], nodeData.resolutionData.port, interfaceId);
mDiscoveredParameters.emplace();
mDiscoveredParameters.back().SetPeerAddress(peerAddress);
mDiscoveredParameters.emplace(nodeData.resolutionData);
ConnectToDiscoveredDevice();
}

Expand Down Expand Up @@ -373,6 +371,7 @@ void SetUpCodePairer::ResetDiscoveryState()
mDiscoveredParameters.pop();
}

mCurrentPASEParameters.ClearValue();
mLastPASEError = CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -449,6 +448,21 @@ void SetUpCodePairer::OnPairingComplete(CHIP_ERROR error)
return;
}

// It may happen that there is a stale DNS entry. If so, ReconfirmRecord will flush
// the record from the daemon cache once it determines that it is invalid.
// It may not help for this particular resolve, but may help subsequent resolves.
if (CHIP_ERROR_TIMEOUT == error && mCurrentPASEParameters.HasValue())
{
const auto & params = mCurrentPASEParameters.Value();
auto & ip = params.GetPeerAddress().GetIPAddress();
auto err = Dnssd::Resolver::Instance().ReconfirmRecord(params.mHostName, ip, params.mInterfaceId);
if (CHIP_NO_ERROR != err && CHIP_ERROR_NOT_IMPLEMENTED != err)
{
ChipLogError(Controller, "Error when verifying the validity of an address: %" CHIP_ERROR_FORMAT, err.Format());
}
}
mCurrentPASEParameters.ClearValue();

// We failed to establish PASE. Try the next thing we have discovered, if
// any.
if (TryNextRendezvousParameters())
Expand Down Expand Up @@ -502,5 +516,32 @@ void SetUpCodePairer::OnDeviceDiscoveredTimeoutCallback(System::Layer * layer, v
}
}

SetUpCodePairerParameters::SetUpCodePairerParameters(const Dnssd::CommonResolutionData & data)
{
mInterfaceId = data.interfaceId;
Platform::CopyString(mHostName, data.hostName);

auto & ip = data.ipAddress[0];
SetPeerAddress(Transport::PeerAddress::UDP(ip, data.port, ip.IsIPv6LinkLocal() ? data.interfaceId : Inet::InterfaceId::Null()));

if (data.mrpRetryIntervalIdle.HasValue())
{
SetIdleInterval(data.mrpRetryIntervalIdle.Value());
}

if (data.mrpRetryIntervalActive.HasValue())
{
SetActiveInterval(data.mrpRetryIntervalActive.Value());
}
}

#if CONFIG_NETWORK_LAYER_BLE
SetUpCodePairerParameters::SetUpCodePairerParameters(BLE_CONNECTION_OBJECT connObj)
{
Transport::PeerAddress peerAddress = Transport::PeerAddress::BLE();
SetPeerAddress(peerAddress).SetConnectionObject(connObj);
}
#endif // CONFIG_NETWORK_LAYER_BLE

} // namespace Controller
} // namespace chip
18 changes: 17 additions & 1 deletion src/controller/SetUpCodePairer.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ namespace Controller {

class DeviceCommissioner;

class SetUpCodePairerParameters : public RendezvousParameters
{
public:
SetUpCodePairerParameters(const Dnssd::CommonResolutionData & data);
#if CONFIG_NETWORK_LAYER_BLE
SetUpCodePairerParameters(BLE_CONNECTION_OBJECT connObj);
#endif // CONFIG_NETWORK_LAYER_BLE
char mHostName[Dnssd::kHostNameMaxLength + 1] = {};
Inet::InterfaceId mInterfaceId;
};

enum class SetupCodePairerBehaviour : uint8_t
{
kCommission,
Expand Down Expand Up @@ -172,7 +183,12 @@ class DLL_EXPORT SetUpCodePairer : public DevicePairingDelegate
// Queue of things we have discovered but not tried connecting to yet. The
// general discovery/pairing process will terminate once this queue is empty
// and all the booleans in mWaitingForDiscovery are false.
std::queue<RendezvousParameters> mDiscoveredParameters;
std::queue<SetUpCodePairerParameters> mDiscoveredParameters;

// Current thing we are trying to connect to over UDP. If a PASE connection fails with
// a CHIP_ERROR_TIMEOUT, the discovered parameters will be used to ask the
// mdns daemon to invalidate the
Optional<SetUpCodePairerParameters> mCurrentPASEParameters;

// mWaitingForPASE is true if we have called either
// EstablishPASEConnection or PairDevice on mCommissioner and are now just
Expand Down
5 changes: 5 additions & 0 deletions src/controller/tests/TestCommissionableNodeController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ class MockResolver : public Resolver
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}
CHIP_ERROR StopDiscovery() override { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

CHIP_ERROR InitStatus = CHIP_NO_ERROR;
CHIP_ERROR ResolveNodeIdStatus = CHIP_NO_ERROR;
Expand Down
17 changes: 17 additions & 0 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,18 @@ CHIP_ERROR DiscoveryImplPlatform::DiscoverCommissioners(DiscoveryFilter filter)
return mResolverProxy.DiscoverCommissioners(filter);
}

CHIP_ERROR DiscoveryImplPlatform::StopDiscovery()
{
ReturnErrorOnFailure(InitImpl());
return mResolverProxy.StopDiscovery();
}

CHIP_ERROR DiscoveryImplPlatform::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId)
{
ReturnErrorOnFailure(InitImpl());
return mResolverProxy.ReconfirmRecord(hostname, address, interfaceId);
}

DiscoveryImplPlatform & DiscoveryImplPlatform::GetInstance()
{
return sManager;
Expand Down Expand Up @@ -702,5 +714,10 @@ CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter)
Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate);
}

CHIP_ERROR ResolverProxy::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId)
{
return ChipDnssdReconfirmRecord(hostname, address, interfaceId);
}

} // namespace Dnssd
} // namespace chip
2 changes: 2 additions & 0 deletions src/lib/dnssd/Discovery_ImplPlatform.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver
CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override;
CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override;
CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override;
CHIP_ERROR StopDiscovery() override;
CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override;

static DiscoveryImplPlatform & GetInstance();

Expand Down
14 changes: 14 additions & 0 deletions src/lib/dnssd/Resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,20 @@ class Resolver
*/
virtual CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) = 0;

/**
* Stop discovery (of commissionable or commissioner nodes).
*
* Some back ends may not support stopping discovery, so consumers should
* not assume they will stop getting callbacks after calling this.
*/
virtual CHIP_ERROR StopDiscovery() = 0;

/**
* Verify the validity of an address that appears to be out of date (for example
* because establishing a connection to it has failed).
*/
virtual CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) = 0;

/**
* Provides the system-wide implementation of the service resolver
*/
Expand Down
2 changes: 2 additions & 0 deletions src/lib/dnssd/ResolverProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ class ResolverProxy : public Resolver
CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override;
CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override;
CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override;
CHIP_ERROR StopDiscovery() override;
CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override;

private:
ResolverDelegateProxy * mDelegate = nullptr;
Expand Down
22 changes: 22 additions & 0 deletions src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate
CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override;
CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override;
CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override;
CHIP_ERROR StopDiscovery() override;
CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override;

private:
OperationalResolveDelegate * mOperationalDelegate = nullptr;
Expand Down Expand Up @@ -633,6 +635,16 @@ CHIP_ERROR MinMdnsResolver::DiscoverCommissioners(DiscoveryFilter filter)
return BrowseNodes(DiscoveryType::kCommissionerNode, filter);
}

CHIP_ERROR MinMdnsResolver::StopDiscovery()
{
return mActiveResolves.CompleteAllBrowses();
}

CHIP_ERROR MinMdnsResolver::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

CHIP_ERROR MinMdnsResolver::BrowseNodes(DiscoveryType type, DiscoveryFilter filter)
{
mActiveResolves.MarkPending(filter, type);
Expand Down Expand Up @@ -712,5 +724,15 @@ CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter)
return chip::Dnssd::Resolver::Instance().DiscoverCommissioners(filter);
}

CHIP_ERROR ResolverProxy::StopDiscovery()
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

CHIP_ERROR ResolverProxy::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

} // namespace Dnssd
} // namespace chip
15 changes: 15 additions & 0 deletions src/lib/dnssd/Resolver_ImplNone.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ class NoneResolver : public Resolver
return CHIP_ERROR_NOT_IMPLEMENTED;
}
CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR StopDiscovery() override { return CHIP_ERROR_NOT_IMPLEMENTED; }
CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}
};

NoneResolver gResolver;
Expand Down Expand Up @@ -73,5 +78,15 @@ CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter)
return CHIP_ERROR_NOT_IMPLEMENTED;
}

CHIP_ERROR ResolverProxy::StopDiscovery()
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

CHIP_ERROR ResolverProxy::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

} // namespace Dnssd
} // namespace chip
10 changes: 10 additions & 0 deletions src/lib/dnssd/platform/Dnssd.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,5 +228,15 @@ CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chi
CHIP_ERROR ChipDnssdResolve(DnssdService * browseResult, chip::Inet::InterfaceId interface, DnssdResolveCallback callback,
void * context);

/**
* This function asks the mdns daemon to asynchronously reconfirm an address that appears to be out of date.
*
* @param[in] hostname The hostname the address belongs to.
* @param[in] address The address to reconfirm.
* @param[in] interfaceId The interfaceId of the address.
*
*/
CHIP_ERROR ChipDnssdReconfirmRecord(const char * hostname, chip::Inet::IPAddress address, chip::Inet::InterfaceId interface);

} // namespace Dnssd
} // namespace chip
53 changes: 49 additions & 4 deletions src/platform/Darwin/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ constexpr const char * kLocalDot = "local.";
constexpr const char * kProtocolTcp = "._tcp";
constexpr const char * kProtocolUdp = "._udp";

constexpr DNSServiceFlags kRegisterFlags = kDNSServiceFlagsNoAutoRename;
constexpr DNSServiceFlags kBrowseFlags = 0;
constexpr DNSServiceFlags kGetAddrInfoFlags = kDNSServiceFlagsTimeout | kDNSServiceFlagsShareConnection;
constexpr DNSServiceFlags kResolveFlags = kDNSServiceFlagsShareConnection;
constexpr DNSServiceFlags kRegisterFlags = kDNSServiceFlagsNoAutoRename;
constexpr DNSServiceFlags kBrowseFlags = 0;
constexpr DNSServiceFlags kGetAddrInfoFlags = kDNSServiceFlagsTimeout | kDNSServiceFlagsShareConnection;
constexpr DNSServiceFlags kResolveFlags = kDNSServiceFlagsShareConnection;
constexpr DNSServiceFlags kReconfirmRecordFlags = 0;

bool IsSupportedProtocol(DnssdServiceProtocol protocol)
{
Expand Down Expand Up @@ -458,5 +459,49 @@ CHIP_ERROR ChipDnssdResolve(DnssdService * service, chip::Inet::InterfaceId inte
return Resolve(context, callback, interfaceId, service->mAddressType, regtype.c_str(), service->mName);
}

CHIP_ERROR ChipDnssdReconfirmRecord(const char * hostname, chip::Inet::IPAddress address, chip::Inet::InterfaceId interface)
{
VerifyOrReturnError(hostname != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

auto interfaceId = interface.GetPlatformInterface();
auto rrclass = kDNSServiceClass_IN;
auto fullname = GetHostNameWithDomain(hostname);

uint16_t rrtype;
uint16_t rdlen;
const void * rdata;

in6_addr ipv6;
#if INET_CONFIG_ENABLE_IPV4
in_addr ipv4;
#endif // INET_CONFIG_ENABLE_IPV4

if (address.IsIPv6())
{
ipv6 = address.ToIPv6();
rrtype = kDNSServiceType_AAAA;
rdlen = static_cast<uint16_t>(sizeof(in6_addr));
rdata = &ipv6;
}
#if INET_CONFIG_ENABLE_IPV4
else if (address.IsIPv4())
{
ipv4 = address.ToIPv4();
rrtype = kDNSServiceType_A;
rdlen = static_cast<uint16_t>(sizeof(in_addr));
rdata = &ipv4;
}
#endif // INET_CONFIG_ENABLE_IPV4
else
{
return CHIP_ERROR_INVALID_ARGUMENT;
}

auto error = DNSServiceReconfirmRecord(kReconfirmRecordFlags, interfaceId, fullname.c_str(), rrtype, rrclass, rdlen, rdata);
LogOnFailure(__func__, error);

return Error::ToChipError(error);
}

} // namespace Dnssd
} // namespace chip
5 changes: 5 additions & 0 deletions src/platform/ESP32/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -505,5 +505,10 @@ CHIP_ERROR ChipDnssdResolve(DnssdService * service, chip::Inet::InterfaceId inte
return error;
}

CHIP_ERROR ChipDnssdReconfirmRecord(const char * hostname, chip::Inet::IPAddress address, chip::Inet::InterfaceId interface)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

} // namespace Dnssd
} // namespace chip
5 changes: 5 additions & 0 deletions src/platform/Linux/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -865,5 +865,10 @@ CHIP_ERROR ChipDnssdResolve(DnssdService * browseResult, chip::Inet::InterfaceId
browseResult->mAddressType, Inet::IPAddressType::kAny, interface, callback, context);
}

CHIP_ERROR ChipDnssdReconfirmRecord(const char * hostname, chip::Inet::IPAddress address, chip::Inet::InterfaceId interface)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

} // namespace Dnssd
} // namespace chip
Loading

0 comments on commit ebce6ae

Please sign in to comment.