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.
This commit is contained in:
Nitesh Kant 2021-07-14 05:11:22 -07:00 committed by GitHub
parent 876624a22b
commit 8337e3a973
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 118 additions and 12 deletions

View File

@ -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<T> {
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<T> {
} 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;
}

View File

@ -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<String> DOMAINS = Collections.unmodifiableSet(new HashSet<String>(Arrays.asList(
private static final Set<String> DOMAINS = Collections.unmodifiableSet(new HashSet<String>(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<InetAddress> 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<InetAddress> resolvedAll = resolver.resolveAll("netty.com").syncUninterruptibly().getNow();
List<InetAddress> 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<String>(
Arrays.asList(hostname, hostname2)));
asList(hostname, hostname2)));
dnsServerAuthority.start();
TestDnsServer dnsServer = new RedirectingTestDnsServer(hostname,
@ -1525,7 +1535,7 @@ public class DnsNameResolverTest {
@Override
public Set<ResourceRecord> 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<String>(
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<String>(Arrays.asList(hostname, ns1Name))) {
TestDnsServer redirectServer = new TestDnsServer(new HashSet<String>(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<String>(Arrays.asList(hostname, ns1Name))) {
TestDnsServer redirectServer = new TestDnsServer(new HashSet<String>(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.<String>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<String> searchDomains, String unresolved)
throws Exception {
final String ipAddrPrefix = "1.2.3.";
TestDnsServer searchDomainServer = new TestDnsServer(new RecordStore() {
@Override
public Set<ResourceRecord> getRecords(QuestionRecord questionRecord) {
Set<ResourceRecord> records = new HashSet<ResourceRecord>(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<InetAddress> addresses = resolver.resolveAll(unresolved).sync().get();
assertThat(addresses, Matchers.<InetAddress>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;