Commit 2b6916bd authored by Iain R. Learmonth's avatar Iain R. Learmonth Committed by Karsten Loesing
Browse files

Stop sharing the JNDI DirContext

In the implementation of more accurate DNS resolution, the DirContext
used by JNDI lookups was shared in a utility class. It turns out that
InitialDirContext, the implementation we used, is very not thread-safe
and so as a result lookups would often fail to complete.

This commit creates a new DirContext for every lookup. The overhead in
doing so is minimal and ensures that we do not have thread safety
issues.

This commit also increases RDNS_LOOKUP_WORKERS_NUM to 30 (from 5). The
majority of time in these threads is spent waiting for replies and so
there are gains to be had in increasing the number of requests in
flight.

Fixes: #26963
parent 8ea8ab0f
......@@ -3,19 +3,22 @@
package org.torproject.onionoo.updater;
import org.torproject.onionoo.util.DomainNameSystem;
import java.util.ArrayList;
import java.util.Hashtable;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import javax.naming.Context;
import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
import javax.naming.directory.Attribute;
import javax.naming.directory.Attributes;
import javax.naming.directory.DirContext;
import javax.naming.directory.InitialDirContext;
class RdnsLookupRequest extends Thread {
private ReverseDomainNameResolver reverseDomainNameResolver;
private DomainNameSystem domainNameSystem;
private RdnsLookupWorker parent;
private String address;
......@@ -29,11 +32,7 @@ class RdnsLookupRequest extends Thread {
private long lookupCompletedMillis = -1L;
public RdnsLookupRequest(
ReverseDomainNameResolver reverseDomainNameResolver,
RdnsLookupWorker parent, String address) {
this.reverseDomainNameResolver = reverseDomainNameResolver;
this.domainNameSystem =
this.reverseDomainNameResolver.getDomainNameSystemInstance();
this.parent = parent;
this.address = address;
}
......@@ -51,11 +50,11 @@ class RdnsLookupRequest extends Thread {
+ "." + bytes[2] + "." + bytes[1] + "." + bytes[0]
+ ".in-addr.arpa";
String[] reverseDnsRecords =
this.domainNameSystem.getRecords(reverseDnsDomain, "PTR");
this.getRecords(reverseDnsDomain, "PTR");
for (String reverseDnsRecord : reverseDnsRecords) {
boolean verified = false;
String[] forwardDnsRecords =
this.domainNameSystem.getRecords(reverseDnsRecord, "A");
this.getRecords(reverseDnsRecord, "A");
for (String forwardDnsRecord : forwardDnsRecords) {
if (forwardDnsRecord.equals(this.address)) {
verified = true;
......@@ -83,6 +82,31 @@ class RdnsLookupRequest extends Thread {
this.parent.interrupt();
}
/**
* Returns all the values of DNS resource records found for a given host name
* and type.
*/
public String[] getRecords(String hostName, String type)
throws NamingException {
Hashtable<String, String> envProps = new Hashtable<>();
envProps.put(Context.INITIAL_CONTEXT_FACTORY,
"com.sun.jndi.dns.DnsContextFactory");
final DirContext dnsContext = new InitialDirContext(envProps);
Set<String> results = new TreeSet<String>();
Attributes dnsEntries =
dnsContext.getAttributes(hostName, new String[] { type });
if (dnsEntries != null) {
Attribute dnsAttribute = dnsEntries.get(type);
if (dnsAttribute != null) {
NamingEnumeration<?> dnsEntryIterator = dnsEntries.get(type).getAll();
while (dnsEntryIterator.hasMoreElements()) {
results.add(dnsEntryIterator.next().toString());
}
}
}
return results.toArray(new String[results.size()]);
}
public synchronized String getHostName() {
List<String> verifiedHostNames = this.verifiedHostNames;
if (null != verifiedHostNames && !verifiedHostNames.isEmpty() ) {
......
......@@ -30,7 +30,7 @@ class RdnsLookupWorker extends Thread {
break;
}
RdnsLookupRequest request = new RdnsLookupRequest(
this.reverseDomainNameResolver, this, rdnsLookupJob);
this, rdnsLookupJob);
request.setDaemon(true);
request.start();
try {
......
......@@ -3,7 +3,6 @@
package org.torproject.onionoo.updater;
import org.torproject.onionoo.util.DomainNameSystem;
import org.torproject.onionoo.util.FormattingUtils;
import java.util.ArrayList;
......@@ -23,9 +22,7 @@ public class ReverseDomainNameResolver {
private static final long RDNS_LOOKUP_MAX_AGE_MILLIS =
12L * 60L * 60L * 1000L;
private static final int RDNS_LOOKUP_WORKERS_NUM = 5;
private DomainNameSystem domainNameSystem;
private static final int RDNS_LOOKUP_WORKERS_NUM = 30;
private Map<String, Long> addressLastLookupTimes;
......@@ -50,7 +47,6 @@ public class ReverseDomainNameResolver {
/** Starts reverse domain name lookups in one or more background
* threads and returns immediately. */
public void startReverseDomainNameLookups() {
this.domainNameSystem = new DomainNameSystem();
this.startedRdnsLookups = System.currentTimeMillis();
this.rdnsLookupJobs = new HashSet<>();
for (Map.Entry<String, Long> e :
......@@ -113,10 +109,6 @@ public class ReverseDomainNameResolver {
return this.startedRdnsLookups;
}
public DomainNameSystem getDomainNameSystemInstance() {
return this.domainNameSystem;
}
/** Returns a string with the number of performed reverse domain name
* lookups and some simple statistics on lookup time. */
public String getStatsString() {
......
/* Copyright 2018 The Tor Project
* See LICENSE for licensing information */
package org.torproject.onionoo.util;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Hashtable;
import java.util.Set;
import java.util.TreeSet;
import javax.naming.Context;
import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
import javax.naming.directory.Attribute;
import javax.naming.directory.Attributes;
import javax.naming.directory.DirContext;
import javax.naming.directory.InitialDirContext;
public class DomainNameSystem {
private DirContext dnsContext;
private Logger log;
/** Creates a new instance. */
public DomainNameSystem() {
log = LoggerFactory.getLogger(DomainNameSystem.class);
Hashtable<String, String> envProps = new Hashtable<>();
envProps.put(Context.INITIAL_CONTEXT_FACTORY,
"com.sun.jndi.dns.DnsContextFactory");
try {
dnsContext = new InitialDirContext(envProps);
} catch (NamingException e) {
log.error(
"Unable to create directory context. "
+ "Host name lookup will be disabled!");
}
}
/**
* Returns all the values of DNS resource records found for a given host name
* and type.
*/
public String[] getRecords(String hostName, String type)
throws NamingException {
if (dnsContext == null) {
/* Initial setup failed, so all lookups will fail. */
throw new NamingException();
}
Set<String> results = new TreeSet<String>();
Attributes dnsEntries =
dnsContext.getAttributes(hostName, new String[] { type });
if (dnsEntries != null) {
Attribute dnsAttribute = dnsEntries.get(type);
if (dnsAttribute != null) {
NamingEnumeration<?> dnsEntryIterator = dnsEntries.get(type).getAll();
while (dnsEntryIterator.hasMoreElements()) {
results.add(dnsEntryIterator.next().toString());
}
}
}
return results.toArray(new String[results.size()]);
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment