andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

DNS Resolver Search Domain Bugs

Browse Source 
Motivation:
The DNS resolver supports search domains. However the ndots are not correctly enforced. The search domain should only be appended under the following scenario [1]:

> Resolver queries having fewer than ndots dots (default is 1) in them will be attempted using each component of the search path in turn until a match is found.

The DNS resolver current appends the search domains if ndots is 0 which should never happen (because no domain can have less than 0 dots).

[1] https://linux.die.net/man/5/resolv.conf

Modifications:
- Parse /etc/resolv.conf to get the default value for ndots on Unix platforms
- The search domain shouldn't be used if ndots is 0
- Avoid failing a promise to trigger the search domain queries in DnsNameResolverContext#resolve

Result:
More correct usage of search domains in the DNS resolver.
Fixes https://github.com/netty/netty/issues/6844.
This commit is contained in:
Scott Mitchell 2017-06-21 13:48:45 -07:00
parent 5934ae8fd2
commit 6cd086050f
6 changed files with 127 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java
View File

@ -64,9 +64,9 @@ import java.util.Iterator;
import java.util.List;
import static io.netty.resolver.dns.DefaultDnsServerAddressStreamProvider.DNS_PORT;
import static io.netty.resolver.dns.UnixResolverDnsServerAddressStreamProvider.parseEtcResolverFirstNdots;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import static io.netty.util.internal.ObjectUtil.checkPositive;
import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
/**
* A DNS-based {@link InetNameResolver}.
@ -97,6 +97,7 @@ public class DnsNameResolver extends InetNameResolver {
static final ResolvedAddressTypes DEFAULT_RESOLVE_ADDRESS_TYPES;
static final String[] DEFAULT_SEARCH_DOMAINS;
private static final int DEFAULT_NDOTS;
static {
if (NetUtil.isIpV4StackPreferred()) {
@ -129,6 +130,14 @@ public class DnsNameResolver extends InetNameResolver {
searchDomains = EmptyArrays.EMPTY_STRINGS;
}
DEFAULT_SEARCH_DOMAINS = searchDomains;
int ndots;
try {
ndots = parseEtcResolverFirstNdots();
} catch (Exception ignore) {
ndots = UnixResolverDnsServerAddressStreamProvider.DEFAULT_NDOTS;
}
DEFAULT_NDOTS = ndots;
}
private static final DatagramDnsResponseDecoder DECODER = new DatagramDnsResponseDecoder();
@ -234,7 +243,7 @@ public class DnsNameResolver extends InetNameResolver {
this.dnsQueryLifecycleObserverFactory =
checkNotNull(dnsQueryLifecycleObserverFactory, "dnsQueryLifecycleObserverFactory");
this.searchDomains = searchDomains != null ? searchDomains.clone() : DEFAULT_SEARCH_DOMAINS;
this.ndots = checkPositiveOrZero(ndots, "ndots");
this.ndots = ndots >= 0 ? ndots : DEFAULT_NDOTS;
this.decodeIdn = decodeIdn;
switch (this.resolvedAddressTypes) {

2
resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java
View File

@ -55,7 +55,7 @@ public final class DnsNameResolverBuilder {
private DnsQueryLifecycleObserverFactory dnsQueryLifecycleObserverFactory =
NoopDnsQueryLifecycleObserverFactory.INSTANCE;
private String[] searchDomains;
private int ndots = 1;
private int ndots = -1;
private boolean decodeIdn = true;
/**

58
resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java
View File

@ -91,7 +91,6 @@ abstract class DnsNameResolverContext<T> {
private final DnsNameResolver parent;
private final DnsServerAddressStream nameServerAddrs;
private final String hostname;
protected String pristineHostname;
private final DnsCache resolveCache;
private final boolean traceEnabled;
private final int maxAllowedQueries;
@ -102,6 +101,7 @@ abstract class DnsNameResolverContext<T> {
Collections.newSetFromMap(
new IdentityHashMap<Future<AddressedEnvelope<DnsResponse, InetSocketAddress>>, Boolean>());
private String pristineHostname;
private List<DnsCacheEntry> resolvedEntries;
private StringBuilder trace;
private int allowedQueries;
@ -124,48 +124,46 @@ abstract class DnsNameResolverContext<T> {
allowedQueries = maxAllowedQueries;
}
void resolve(Promise<T> promise) {
boolean directSearch = parent.searchDomains().length == 0 || StringUtil.endsWith(hostname, '.');
if (directSearch) {
void resolve(final Promise<T> promise) {
if (parent.searchDomains().length == 0 || parent.ndots() == 0 || StringUtil.endsWith(hostname, '.')) {
internalResolve(promise);
} else {
final Promise<T> original = promise;
promise = parent.executor().newPromise();
promise.addListener(new FutureListener<T>() {
int count;
int dots = 0;
for (int idx = hostname.length() - 1; idx >= 0; idx--) {
if (hostname.charAt(idx) == '.' && ++dots >= parent.ndots()) {
internalResolve(promise);
return;
}
}
doSearchDomainQuery(0, new FutureListener<T>() {
private int count = 1;
@Override
public void operationComplete(Future<T> future) throws Exception {
if (future.isSuccess()) {
original.trySuccess(future.getNow());
promise.trySuccess(future.getNow());
} else if (count < parent.searchDomains().length) {
String searchDomain = parent.searchDomains()[count++];
Promise<T> nextPromise = parent.executor().newPromise();
String nextHostname = hostname + '.' + searchDomain;
DnsNameResolverContext<T> nextContext = newResolverContext(parent,
nextHostname, additionals, resolveCache, nameServerAddrs);
nextContext.pristineHostname = hostname;
nextContext.internalResolve(nextPromise);
nextPromise.addListener(this);
doSearchDomainQuery(count++, this);
} else {
original.tryFailure(future.cause());
promise.tryFailure(future.cause());
}
}
});
if (parent.ndots() == 0) {
internalResolve(promise);
} else {
int dots = 0;
for (int idx = hostname.length() - 1; idx >= 0; idx--) {
if (hostname.charAt(idx) == '.' && ++dots >= parent.ndots()) {
internalResolve(promise);
return;
}
}
promise.tryFailure(new UnknownHostException(hostname));
}
}
}
private void doSearchDomainQuery(int count, FutureListener<T> listener) {
DnsNameResolverContext<T> nextContext = newResolverContext(parent,
hostname + '.' + parent.searchDomains()[count],
additionals,
resolveCache,
nameServerAddrs);
nextContext.pristineHostname = hostname;
Promise<T> nextPromise = parent.executor().newPromise();
nextContext.internalResolve(nextPromise);
nextPromise.addListener(listener);
}
private void internalResolve(Promise<T> promise) {
DnsServerAddressStream nameServerAddressStream = getNameServers(hostname);

55
resolver-dns/src/main/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProvider.java
View File

@ -45,10 +45,15 @@ import static io.netty.util.internal.StringUtil.indexOfNonWhiteSpace;
public final class UnixResolverDnsServerAddressStreamProvider implements DnsServerAddressStreamProvider {
private static final InternalLogger logger =
InternalLoggerFactory.getInstance(UnixResolverDnsServerAddressStreamProvider.class);
private static final String ETC_RESOLV_CONF_FILE = "/etc/resolv.conf";
private static final String ETC_RESOLVER_DIR = "/etc/resolver";
private static final String NAMESERVER_ROW_LABEL = "nameserver";
private static final String SORTLIST_ROW_LABEL = "sortlist";
private static final String OPTIONS_ROW_LABEL = "options";
private static final String DOMAIN_ROW_LABEL = "domain";
private static final String PORT_ROW_LABEL = "port";
private static final String NDOTS_LABEL = "ndots:";
static final int DEFAULT_NDOTS = 1;
private final DnsServerAddresses defaultNameServerAddresses;
private final Map<String, DnsServerAddresses> domainToNameServerStreamMap;
@ -59,11 +64,11 @@ public final class UnixResolverDnsServerAddressStreamProvider implements DnsServ
static DnsServerAddressStreamProvider parseSilently() {
try {
UnixResolverDnsServerAddressStreamProvider nameServerCache =
new UnixResolverDnsServerAddressStreamProvider("/etc/resolv.conf", "/etc/resolver");
new UnixResolverDnsServerAddressStreamProvider(ETC_RESOLV_CONF_FILE, ETC_RESOLVER_DIR);
return nameServerCache.mayOverrideNameServers() ? nameServerCache
: DefaultDnsServerAddressStreamProvider.INSTANCE;
} catch (Exception e) {
logger.debug("failed to parse /etc/resolv.conf and/or /etc/resolver", e);
logger.debug("failed to parse {} and/or {}", ETC_RESOLV_CONF_FILE, ETC_RESOLVER_DIR, e);
return DefaultDnsServerAddressStreamProvider.INSTANCE;
}
}
@ -235,4 +240,50 @@ public final class UnixResolverDnsServerAddressStreamProvider implements DnsServ
domainName, existingAddresses, addresses);
}
}
/**
* Parse a file of the format <a href="https://linux.die.net/man/5/resolver">/etc/resolv.conf</a> and return the
* value corresponding to the first ndots in an options configuration.
* @return the value corresponding to the first ndots in an options configuration, or {@link #DEFAULT_NDOTS} if not
* found.
* @throws IOException If a failure occurs parsing the file.
*/
static int parseEtcResolverFirstNdots() throws IOException {
return parseEtcResolverFirstNdots(new File(ETC_RESOLV_CONF_FILE));
}
/**
* Parse a file of the format <a href="https://linux.die.net/man/5/resolver">/etc/resolv.conf</a> and return the
* value corresponding to the first ndots in an options configuration.
* @param etcResolvConf a file of the format <a href="https://linux.die.net/man/5/resolver">/etc/resolv.conf</a>.
* @return the value corresponding to the first ndots in an options configuration, or {@link #DEFAULT_NDOTS} if not
* found.
* @throws IOException If a failure occurs parsing the file.
*/
static int parseEtcResolverFirstNdots(File etcResolvConf) throws IOException {
FileReader fr = new FileReader(etcResolvConf);
BufferedReader br = null;
try {
br = new BufferedReader(fr);
String line;
while ((line = br.readLine()) != null) {
if (line.startsWith(OPTIONS_ROW_LABEL)) {
int i = line.indexOf(NDOTS_LABEL);
if (i >= 0) {
i += NDOTS_LABEL.length();
final int j = line.indexOf(' ', i);
return Integer.parseInt(line.substring(i, j < 0 ? line.length() : j));
}
break;
}
}
} finally {
if (br == null) {
fr.close();
} else {
br.close();
}
}
return DEFAULT_NDOTS;
}
}

22
resolver-dns/src/test/java/io/netty/resolver/dns/SearchDomainTest.java
View File

@ -85,7 +85,7 @@ public class SearchDomainTest {
dnsServer = new TestDnsServer(store);
dnsServer.start();
resolver = newResolver().searchDomains(Collections.singletonList("foo.com")).build();
resolver = newResolver().searchDomains(Collections.singletonList("foo.com")).ndots(2).build();
String a = "host1.foo.com";
String resolved = assertResolve(resolver, a);
@ -113,9 +113,10 @@ public class SearchDomainTest {
resolved = assertResolve(resolver, "host4.sub");
assertEquals(store.getAddress("host4.sub.foo.com"), resolved);
// "host5.sub" contains a dot and is resolved
// "host5.sub" would have been directly resolved but since it has less than ndots the "foo.com" search domain
// is used.
resolved = assertResolve(resolver, "host5.sub");
assertEquals(store.getAddress("host5.sub"), resolved);
assertEquals(store.getAddress("host5.sub.foo.com"), resolved);
}
@Test
@ -132,7 +133,7 @@ public class SearchDomainTest {
dnsServer = new TestDnsServer(store);
dnsServer.start();
resolver = newResolver().searchDomains(Collections.singletonList("foo.com")).build();
resolver = newResolver().searchDomains(Collections.singletonList("foo.com")).ndots(2).build();
String a = "host1.foo.com";
List<String> resolved = assertResolveAll(resolver, a);
@ -160,9 +161,10 @@ public class SearchDomainTest {
resolved = assertResolveAll(resolver, "host4.sub");
assertEquals(store.getAddresses("host4.sub.foo.com"), resolved);
// "host5.sub" contains a dot and is resolved
// "host5.sub" would have been directly resolved but since it has less than ndots the "foo.com" search domain
// is used.
resolved = assertResolveAll(resolver, "host5.sub");
assertEquals(store.getAddresses("host5.sub"), resolved);
assertEquals(store.getAddresses("host5.sub.foo.com"), resolved);
}
@Test
@ -237,9 +239,9 @@ public class SearchDomainTest {
resolved = assertResolve(resolver, "host1.foo.com");
assertEquals(store.getAddress("host1.foo.com"), resolved);
// "host2" resolves to host2.foo.com with the foo.com search domain
resolved = assertResolve(resolver, "host2");
assertEquals(store.getAddress("host2.foo.com"), resolved);
// "host2" shouldn't resolve because it is not in the known domain names, and "host2" has 0 dots which is not
// less ndots (which is also 0).
assertNotResolve(resolver, "host2");
}
private void assertNotResolve(DnsNameResolver resolver, String inetHost) throws InterruptedException {
@ -278,7 +280,7 @@ public class SearchDomainTest {
dnsServer = new TestDnsServer(store);
dnsServer.start();
resolver = newResolver().searchDomains(Collections.singletonList("foo.com")).build();
resolver = newResolver().searchDomains(Collections.singletonList("foo.com")).ndots(2).build();
Future<InetAddress> fut = resolver.resolve("unknown.hostname");
assertTrue(fut.await(10, TimeUnit.SECONDS));

22
resolver-dns/src/test/java/io/netty/resolver/dns/UnixResolverDnsServerAddressStreamProviderTest.java
View File

@ -26,6 +26,8 @@ import java.io.IOException;
import java.io.OutputStream;
import java.net.InetSocketAddress;
import static io.netty.resolver.dns.UnixResolverDnsServerAddressStreamProvider.DEFAULT_NDOTS;
import static io.netty.resolver.dns.UnixResolverDnsServerAddressStreamProvider.parseEtcResolverFirstNdots;
import static org.junit.Assert.assertEquals;
public class UnixResolverDnsServerAddressStreamProviderTest {
@ -77,6 +79,26 @@ public class UnixResolverDnsServerAddressStreamProviderTest {
assertHostNameEquals("127.0.0.5", stream.next());
}
@Test
public void ndotsIsParsedIfPresent() throws IOException {
File f = buildFile("search localdomain\n" +
"nameserver 127.0.0.11\n" +
"options ndots:0\n");
assertEquals(0, parseEtcResolverFirstNdots(f));
f = buildFile("search localdomain\n" +
"nameserver 127.0.0.11\n" +
"options ndots:123 foo:goo\n");
assertEquals(123, parseEtcResolverFirstNdots(f));
}
@Test
public void defaultValueReturnedIfNdotsNotPresent() throws IOException {
File f = buildFile("search localdomain\n" +
"nameserver 127.0.0.11\n");
assertEquals(DEFAULT_NDOTS, parseEtcResolverFirstNdots(f));
}
private File buildFile(String contents) throws IOException {
File f = folder.newFile();
OutputStream out = new FileOutputStream(f);