From be63d5de39916769b959a19c1baa5fcc5306d702 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 1 May 2023 10:34:08 -0400 Subject: [PATCH] Remove the kDNSServiceInterfaceIndexLocalOnly special-case on Darwin. (#26287) Interface kDNSServiceInterfaceIndexLocalOnly means the _registration_ is local-only, not that the registered host is localhost and has a loopback address. There can be local-only registrations for other hosts, and we need to do actual address resolution. To make this work right in our unit test setup, fix our registration code for the local-only case (which tests use) to register hostname -> IP mappings, which it was not doing before. --- src/platform/Darwin/DnssdContexts.cpp | 22 +++++------ .../Darwin/DnssdHostNameRegistrar.cpp | 37 +++++++++++++++---- src/platform/Darwin/DnssdHostNameRegistrar.h | 12 ++++-- src/platform/Darwin/DnssdImpl.cpp | 6 --- src/platform/Darwin/DnssdImpl.h | 1 - 5 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/platform/Darwin/DnssdContexts.cpp b/src/platform/Darwin/DnssdContexts.cpp index 832b33637d88fa..27bb5140a55651 100644 --- a/src/platform/Darwin/DnssdContexts.cpp +++ b/src/platform/Darwin/DnssdContexts.cpp @@ -420,7 +420,6 @@ CHIP_ERROR ResolveContext::OnNewAddress(uint32_t interfaceId, const struct socka chip::Inet::IPAddress ip; ReturnErrorOnFailure(chip::Inet::IPAddress::GetIPAddressFromSockAddr(*address, ip)); - interfaces[interfaceId].addresses.push_back(ip); #ifdef CHIP_PROGRESS_LOGGING char addrStr[INET6_ADDRSTRLEN]; @@ -428,19 +427,18 @@ CHIP_ERROR ResolveContext::OnNewAddress(uint32_t interfaceId, const struct socka ChipLogProgress(Discovery, "Mdns: %s interface: %" PRIu32 " ip:%s", __func__, interfaceId, addrStr); #endif // CHIP_PROGRESS_LOGGING - return CHIP_NO_ERROR; -} + if (ip.IsIPv6LinkLocal() && interfaceId == kDNSServiceInterfaceIndexLocalOnly) + { + // We need a real interface to use a link-local address. Just ignore + // this one, because trying to use it will simply lead to "No route to + // host" errors. + ChipLogProgress(Discovery, "Mdns: Ignoring link-local address with no usable interface"); + return CHIP_NO_ERROR; + } -CHIP_ERROR ResolveContext::OnNewLocalOnlyAddress() -{ - sockaddr_in6 sockaddr; - memset(&sockaddr, 0, sizeof(sockaddr)); - sockaddr.sin6_len = sizeof(sockaddr); - sockaddr.sin6_family = AF_INET6; - sockaddr.sin6_addr = in6addr_loopback; - sockaddr.sin6_port = htons((unsigned short) interfaces[kDNSServiceInterfaceIndexLocalOnly].service.mPort); + interfaces[interfaceId].addresses.push_back(ip); - return OnNewAddress(kDNSServiceInterfaceIndexLocalOnly, reinterpret_cast(&sockaddr)); + return CHIP_NO_ERROR; } bool ResolveContext::HasAddress() diff --git a/src/platform/Darwin/DnssdHostNameRegistrar.cpp b/src/platform/Darwin/DnssdHostNameRegistrar.cpp index 572aa043043e4d..287bc0bec3380f 100644 --- a/src/platform/Darwin/DnssdHostNameRegistrar.cpp +++ b/src/platform/Darwin/DnssdHostNameRegistrar.cpp @@ -284,9 +284,31 @@ DNSServiceErrorType HostNameRegistrar::Init(const char * hostname, Inet::IPAddre CHIP_ERROR HostNameRegistrar::Register() { - // If the target interface is kDNSServiceInterfaceIndexLocalOnly, there are no interfaces to register against - // the dns daemon. - VerifyOrReturnError(!IsLocalOnly(), CHIP_NO_ERROR); + // If the target interface is kDNSServiceInterfaceIndexLocalOnly, just + // register the loopback addresses. + if (IsLocalOnly()) + { + ReturnErrorOnFailure(ResetSharedConnection()); + + InetInterfacesVector inetInterfaces; + Inet6InterfacesVector inet6Interfaces; + // Instead of mInterfaceId (which will not match any actual interface), + // use kDNSServiceInterfaceIndexAny and restrict to loopback interfaces. + GetInterfaceAddresses(kDNSServiceInterfaceIndexAny, mAddressType, inetInterfaces, inet6Interfaces, + true /* searchLoopbackOnly */); + + // But we register the IPs with mInterfaceId, not the actual interface + // IDs, so that resolution code that is grouping addresses by interface + // ends up doing the right thing, since we registered our SRV record on + // mInterfaceId. + // + // And only register the IPv6 ones, for simplicity. + for (auto & interface : inet6Interfaces) + { + ReturnErrorOnFailure(RegisterInterface(mInterfaceId, interface.second, kDNSServiceType_AAAA)); + } + return CHIP_NO_ERROR; + } return StartMonitorInterfaces(^(InetInterfacesVector inetInterfaces, Inet6InterfacesVector inet6Interfaces) { ReturnOnFailure(ResetSharedConnection()); @@ -305,11 +327,10 @@ CHIP_ERROR HostNameRegistrar::RegisterInterface(uint32_t interfaceId, uint16_t r void HostNameRegistrar::Unregister() { - // If the target interface is kDNSServiceInterfaceIndexLocalOnly, there are no interfaces to register against - // the dns daemon. - VerifyOrReturn(!IsLocalOnly()); - - StopMonitorInterfaces(); + if (!IsLocalOnly()) + { + StopMonitorInterfaces(); + } StopSharedConnection(); } diff --git a/src/platform/Darwin/DnssdHostNameRegistrar.h b/src/platform/Darwin/DnssdHostNameRegistrar.h index 9bb5ce463cfdf6..68de1407298dfb 100644 --- a/src/platform/Darwin/DnssdHostNameRegistrar.h +++ b/src/platform/Darwin/DnssdHostNameRegistrar.h @@ -50,14 +50,18 @@ namespace Dnssd { { for (auto & interface : interfaces) { auto interfaceId = interface.first; - auto interfaceAddress = static_cast(&interface.second); - auto interfaceAddressLen = sizeof(interface.second); - LogErrorOnFailure( - RegisterInterface(interfaceId, type, interfaceAddress, static_cast(interfaceAddressLen))); + LogErrorOnFailure(RegisterInterface(interfaceId, interface.second, type)); } } + template CHIP_ERROR RegisterInterface(uint32_t interfaceId, const T & interfaceAddress, uint16_t type) + { + auto interfaceAddressLen = sizeof(interfaceAddress); + + return RegisterInterface(interfaceId, type, &interfaceAddress, static_cast(interfaceAddressLen)); + } + CHIP_ERROR RegisterInterface(uint32_t interfaceId, uint16_t rtype, const void * rdata, uint16_t rdlen); CHIP_ERROR StartSharedConnection(); diff --git a/src/platform/Darwin/DnssdImpl.cpp b/src/platform/Darwin/DnssdImpl.cpp index acf65e09511534..c3e839e3308249 100644 --- a/src/platform/Darwin/DnssdImpl.cpp +++ b/src/platform/Darwin/DnssdImpl.cpp @@ -328,12 +328,6 @@ static void OnResolve(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t inter if (kDNSServiceErr_NoError == err) { sdCtx->OnNewInterface(interfaceId, fullname, hostname, port, txtLen, txtRecord); - if (kDNSServiceInterfaceIndexLocalOnly == interfaceId) - { - sdCtx->OnNewLocalOnlyAddress(); - sdCtx->Finalize(); - return; - } } if (!(flags & kDNSServiceFlagsMoreComing)) diff --git a/src/platform/Darwin/DnssdImpl.h b/src/platform/Darwin/DnssdImpl.h index d599b052cdb1f6..9b624bae73e4f3 100644 --- a/src/platform/Darwin/DnssdImpl.h +++ b/src/platform/Darwin/DnssdImpl.h @@ -204,7 +204,6 @@ struct ResolveContext : public GenericContext void DispatchSuccess() override; CHIP_ERROR OnNewAddress(uint32_t interfaceId, const struct sockaddr * address); - CHIP_ERROR OnNewLocalOnlyAddress(); bool HasAddress(); void OnNewInterface(uint32_t interfaceId, const char * fullname, const char * hostname, uint16_t port, uint16_t txtLen,