From 8337e3a9734fdc8c7d5f74cb75eb62d185987e7f Mon Sep 17 00:00:00 2001 From: Nitesh Kant Date: Wed, 14 Jul 2021 05:11:22 -0700 Subject: [PATCH] Improve name matching in DNS answers (#11474) __Motivation__ Upon receiving a DNS answer, we match whether the name in the question matches the name in the record. Some DNS servers we have encountered append a search domain to the record name which fails this match. eg: for question name `netty` and search domains `io` and `com`, we will do 2 queries: `netty.io.` and `netty.com.`, if the answer for `netty.io` contains `netty.com` then we ignore this record. __Modification__ If the name in the record does not match the name in the question, append configured search domains to the question name to see if it matches the record name. __Result__ Records names with appended search domains are still returned as valid answers. --- .../netty/resolver/dns/DnsResolveContext.java | 34 ++++++- .../resolver/dns/DnsNameResolverTest.java | 96 ++++++++++++++++--- 2 files changed, 118 insertions(+), 12 deletions(-) diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java index 6d55536293..9c04e916c5 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java @@ -41,6 +41,8 @@ import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.StringUtil; import io.netty.util.internal.SuppressJava6Requirement; import io.netty.util.internal.ThrowableUtil; +import io.netty.util.internal.logging.InternalLogger; +import io.netty.util.internal.logging.InternalLoggerFactory; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -61,6 +63,7 @@ import static io.netty.resolver.dns.DnsAddressDecoder.decodeAddress; import static java.lang.Math.min; abstract class DnsResolveContext { + private static final InternalLogger logger = InternalLoggerFactory.getInstance(DnsResolveContext.class); private static final RuntimeException NXDOMAIN_QUERY_FAILED_EXCEPTION = DnsResolveContextException.newStatic("No answer found and NXDOMAIN response code returned", @@ -792,12 +795,41 @@ abstract class DnsResolveContext { } while (resolved != null); if (resolved == null) { - continue; + assert questionName.isEmpty() || questionName.charAt(questionName.length() - 1) == '.'; + + for (String searchDomain : parent.searchDomains()) { + if (searchDomain.isEmpty()) { + continue; + } + + final String fqdn; + if (searchDomain.charAt(searchDomain.length() - 1) == '.') { + fqdn = questionName + searchDomain; + } else { + fqdn = questionName + searchDomain + '.'; + } + if (recordName.equals(fqdn)) { + resolved = recordName; + break; + } + } + if (resolved == null) { + if (logger.isDebugEnabled()) { + logger.debug("Ignoring record {} as it contains a different name than the " + + "question name [{}]. Cnames: {}, Search domains: {}", + r.toString(), questionName, cnames, parent.searchDomains()); + } + continue; + } } } final T converted = convertRecord(r, hostname, additionals, parent.executor()); if (converted == null) { + if (logger.isDebugEnabled()) { + logger.debug("Ignoring record {} as the converted record is null. hostname [{}], Additionals: {}", + r.toString(), hostname, additionals); + } continue; } diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java index f147a92095..28ca4d9811 100644 --- a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java +++ b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java @@ -50,6 +50,7 @@ import io.netty.util.concurrent.Promise; import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.SocketUtils; import io.netty.util.internal.StringUtil; +import io.netty.util.internal.ThreadLocalRandom; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; import org.apache.directory.server.dns.DnsException; @@ -115,6 +116,8 @@ import static io.netty.handler.codec.dns.DnsRecordType.NAPTR; import static io.netty.handler.codec.dns.DnsRecordType.SRV; import static io.netty.resolver.dns.DnsNameResolver.DEFAULT_RESOLVE_ADDRESS_TYPES; import static io.netty.resolver.dns.DnsServerAddresses.sequential; +import static io.netty.resolver.dns.TestDnsServer.newARecord; +import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assumptions.assumeThat; import static org.hamcrest.MatcherAssert.assertThat; @@ -123,6 +126,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -143,7 +147,7 @@ public class DnsNameResolverTest { // $ curl -O https://s3.amazonaws.com/alexa-static/top-1m.csv.zip // $ unzip -o top-1m.csv.zip top-1m.csv // $ head -100 top-1m.csv | cut -d, -f2 | cut -d/ -f1 | while read L; do echo '"'"$L"'",'; done > topsites.txt - private static final Set DOMAINS = Collections.unmodifiableSet(new HashSet(Arrays.asList( + private static final Set DOMAINS = Collections.unmodifiableSet(new HashSet(asList( "google.com", "youtube.com", "facebook.com", @@ -284,7 +288,7 @@ public class DnsNameResolverTest { static { EXCLUSIONS_RESOLVE_AAAA.addAll(EXCLUSIONS_RESOLVE_A); EXCLUSIONS_RESOLVE_AAAA.addAll(DOMAINS); - EXCLUSIONS_RESOLVE_AAAA.removeAll(Arrays.asList( + EXCLUSIONS_RESOLVE_AAAA.removeAll(asList( "google.com", "facebook.com", "youtube.com", @@ -378,6 +382,12 @@ public class DnsNameResolverTest { private static DnsNameResolverBuilder newResolver(boolean decodeToUnicode, DnsServerAddressStreamProvider dnsServerAddressStreamProvider) { + return newResolver(decodeToUnicode, dnsServerAddressStreamProvider, dnsServer); + } + + private static DnsNameResolverBuilder newResolver(boolean decodeToUnicode, + DnsServerAddressStreamProvider dnsServerAddressStreamProvider, + TestDnsServer dnsServer) { DnsNameResolverBuilder builder = new DnsNameResolverBuilder(group.next()) .dnsQueryLifecycleObserverFactory(new TestRecursiveCacheDnsQueryLifecycleObserverFactory()) .channelType(NioDatagramChannel.class) @@ -592,7 +602,7 @@ public class DnsNameResolverTest { DnsNameResolver resolver = newNonCachedResolver(ResolvedAddressTypes.IPV4_ONLY).build(); try { List addrs = resolver.resolveAll(inetHost).syncUninterruptibly().getNow(); - assertEquals(Arrays.asList( + assertEquals(asList( SocketUtils.allAddressesByName(inetHost)), addrs); } finally { resolver.close(); @@ -1370,7 +1380,7 @@ public class DnsNameResolverTest { List resolvedAll = resolver.resolveAll("netty.com").syncUninterruptibly().getNow(); List expected = types == ResolvedAddressTypes.IPV4_PREFERRED ? - Arrays.asList(ipv4InetAddress, ipv6InetAddress) : Arrays.asList(ipv6InetAddress, ipv4InetAddress); + asList(ipv4InetAddress, ipv6InetAddress) : asList(ipv6InetAddress, ipv4InetAddress); assertEquals(expected, resolvedAll); } finally { nonCompliantDnsServer.stop(); @@ -1383,7 +1393,7 @@ public class DnsNameResolverTest { final String hostname2 = "some2.record.netty.io"; final TestDnsServer dnsServerAuthority = new TestDnsServer(new HashSet( - Arrays.asList(hostname, hostname2))); + asList(hostname, hostname2))); dnsServerAuthority.start(); TestDnsServer dnsServer = new RedirectingTestDnsServer(hostname, @@ -1525,7 +1535,7 @@ public class DnsNameResolverTest { @Override public Set getRecords(QuestionRecord question) { if (question.getDomainName().equals(expected.getHostName())) { - return Collections.singleton(TestDnsServer.newARecord( + return Collections.singleton(newARecord( expected.getHostName(), expected.getHostAddress())); } return Collections.emptySet(); @@ -1534,7 +1544,7 @@ public class DnsNameResolverTest { dnsServerAuthority.start(); TestDnsServer redirectServer = new TestDnsServer(new HashSet( - Arrays.asList(expected.getHostName(), ns1Name, ns2Name))) { + asList(expected.getHostName(), ns1Name, ns2Name))) { @Override protected DnsMessage filterMessage(DnsMessage message) { for (QuestionRecord record: message.getQuestionRecords()) { @@ -1668,7 +1678,7 @@ public class DnsNameResolverTest { InetAddress.getByAddress(ns1Name, new byte[] { 10, 0, 0, 4 }), DefaultDnsServerAddressStreamProvider.DNS_PORT); - TestDnsServer redirectServer = new TestDnsServer(new HashSet(Arrays.asList(hostname, ns1Name))) { + TestDnsServer redirectServer = new TestDnsServer(new HashSet(asList(hostname, ns1Name))) { @Override protected DnsMessage filterMessage(DnsMessage message) { for (QuestionRecord record: message.getQuestionRecords()) { @@ -1687,7 +1697,7 @@ public class DnsNameResolverTest { } private ResourceRecord newARecord(InetSocketAddress address) { - return TestDnsServer.newARecord(address.getHostName(), address.getAddress().getHostAddress()); + return newARecord(address.getHostName(), address.getAddress().getHostAddress()); } }; redirectServer.start(); @@ -1796,7 +1806,7 @@ public class DnsNameResolverTest { final InetSocketAddress ns5Address = new InetSocketAddress( InetAddress.getByAddress(ns2Name, new byte[] { 10, 0, 0, 5 }), DefaultDnsServerAddressStreamProvider.DNS_PORT); - TestDnsServer redirectServer = new TestDnsServer(new HashSet(Arrays.asList(hostname, ns1Name))) { + TestDnsServer redirectServer = new TestDnsServer(new HashSet(asList(hostname, ns1Name))) { @Override protected DnsMessage filterMessage(DnsMessage message) { for (QuestionRecord record: message.getQuestionRecords()) { @@ -1817,7 +1827,7 @@ public class DnsNameResolverTest { } private ResourceRecord newARecord(InetSocketAddress address) { - return TestDnsServer.newARecord(address.getHostName(), address.getAddress().getHostAddress()); + return newARecord(address.getHostName(), address.getAddress().getHostAddress()); } }; redirectServer.start(); @@ -1912,6 +1922,70 @@ public class DnsNameResolverTest { testNsLoopFailsResolve(NoopAuthoritativeDnsServerCache.INSTANCE); } + @Test + public void testRRNameContainsDifferentSearchDomainNoDomains() { + assertThrows(UnknownHostException.class, new Executable() { + @Override + public void execute() throws Throwable { + testRRNameContainsDifferentSearchDomain(Collections.emptyList(), "netty"); + } + }); + } + + @Test + public void testRRNameContainsDifferentSearchDomainEmptyExtraDomain() throws Exception { + testRRNameContainsDifferentSearchDomain(asList("io", ""), "netty"); + } + + @Test + public void testRRNameContainsDifferentSearchDomainSingleExtraDomain() throws Exception { + testRRNameContainsDifferentSearchDomain(asList("io", "foo.dom"), "netty"); + } + + @Test + public void testRRNameContainsDifferentSearchDomainMultiExtraDomains() throws Exception { + testRRNameContainsDifferentSearchDomain(asList("com", "foo.dom", "bar.dom"), "google"); + } + + private static void testRRNameContainsDifferentSearchDomain(final List searchDomains, String unresolved) + throws Exception { + final String ipAddrPrefix = "1.2.3."; + TestDnsServer searchDomainServer = new TestDnsServer(new RecordStore() { + @Override + public Set getRecords(QuestionRecord questionRecord) { + Set records = new HashSet(searchDomains.size()); + final String qName = questionRecord.getDomainName(); + for (String searchDomain : searchDomains) { + if (qName.endsWith(searchDomain)) { + continue; + } + final ResourceRecord rr = newARecord(qName + '.' + searchDomain, + ipAddrPrefix + ThreadLocalRandom.current().nextInt(1, 10)); + logger.info("Adding A record: " + rr); + records.add(rr); + } + return records; + } + }); + searchDomainServer.start(); + + final DnsNameResolver resolver = newResolver(false, null, searchDomainServer) + .searchDomains(searchDomains) + .build(); + + try { + final List addresses = resolver.resolveAll(unresolved).sync().get(); + assertThat(addresses, Matchers.hasSize(greaterThan(0))); + for (InetAddress address : addresses) { + assertThat(address.getHostName(), startsWith(unresolved)); + assertThat(address.getHostAddress(), startsWith(ipAddrPrefix)); + } + } finally { + resolver.close(); + searchDomainServer.stop(); + } + } + private void testNsLoopFailsResolve(AuthoritativeDnsServerCache authoritativeDnsServerCache) throws Exception { final String domain = "netty.io"; final String ns1Name = "ns1." + domain;