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 adbridge committed Nov 17, 2022
1 parent 4a4891f commit 5cdd85d
Show file tree
Hide file tree
Showing 20 changed files with 215 additions and 28 deletions.
74 changes: 51 additions & 23 deletions src/controller/SetUpCodePairer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,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 @@ -237,6 +237,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 @@ -267,9 +272,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 @@ -338,25 +341,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);

if (nodeData.resolutionData.mrpRetryIntervalIdle.HasValue())
{
auto interval = nodeData.resolutionData.mrpRetryIntervalIdle.Value();
mDiscoveredParameters.back().SetIdleInterval(interval);
}

if (nodeData.resolutionData.mrpRetryIntervalActive.HasValue())
{
auto interval = nodeData.resolutionData.mrpRetryIntervalActive.Value();
mDiscoveredParameters.back().SetActiveInterval(interval);
}

mDiscoveredParameters.emplace(nodeData.resolutionData);
ConnectToDiscoveredDevice();
}

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

mCurrentPASEParameters.ClearValue();
mLastPASEError = CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -488,6 +474,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 @@ -541,5 +542,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 @@ -180,7 +191,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
4 changes: 4 additions & 0 deletions src/controller/tests/TestCommissionableNodeController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ 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
11 changes: 11 additions & 0 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,12 @@ CHIP_ERROR DiscoveryImplPlatform::StopDiscovery()
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 @@ -750,5 +756,10 @@ CHIP_ERROR ResolverProxy::StopDiscovery()
return err;
}

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

} // namespace Dnssd
} // namespace chip
1 change: 1 addition & 0 deletions src/lib/dnssd/Discovery_ImplPlatform.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver
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
6 changes: 6 additions & 0 deletions src/lib/dnssd/Resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,12 @@ class Resolver
*/
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
1 change: 1 addition & 0 deletions src/lib/dnssd/ResolverProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ class ResolverProxy : public Resolver
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
11 changes: 11 additions & 0 deletions src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate
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 @@ -639,6 +640,11 @@ 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 @@ -723,5 +729,10 @@ 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
9 changes: 9 additions & 0 deletions src/lib/dnssd/Resolver_ImplNone.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ class NoneResolver : public Resolver
}
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 @@ -79,5 +83,10 @@ 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 @@ -245,5 +245,15 @@ CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier);
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 @@ -36,10 +36,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 @@ -446,5 +447,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 @@ -511,5 +511,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 @@ -872,5 +872,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
5 changes: 5 additions & 0 deletions src/platform/OpenThread/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,10 @@ CHIP_ERROR ChipDnssdResolve(DnssdService * browseResult, Inet::InterfaceId inter
#endif // CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT && CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
}

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/Tizen/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -823,5 +823,10 @@ CHIP_ERROR ChipDnssdResolve(DnssdService * browseResult, chip::Inet::InterfaceId
return DnssdTizen::GetInstance().Resolve(*browseResult, 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 5cdd85d

Please sign in to comment.