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.

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

[darwin] Use DNSServiceReconfirmRecord for A and AAAA records to miti… #23067

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whad does 'reconfirm' mean?

When reading the comment, this reads to me like reading the method name. It does say 'async' so maybe the name should be AsyncReconfirmRecord (wondering why we have ChipDnssd prefix in a namespace chip::Dnssd ... that is stuttering).

I think this should explain what reconfirmation is and when this method is to be used and how.

*
* @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