Commit 4b05f840 authored by Iain R. Learmonth's avatar Iain R. Learmonth Committed by Karsten Loesing
Browse files

Fixes for reverse DNS resolver

 * SortedSets are used in place of Lists to ensure deterministic
   ordering of looked up names
 * The NodeStatus serialization is extended to include verified
   and unverified host names
 * The existing host name field in NodeStatus serializations is
   removed and a placeholder inserted
 * The last reverse DNS lookup time is now only updated on successful
   lookups
 * The host name field is removed from summary and details documents
 * Tests are updated to use SortedSets in place of Lists

Fixes: #27050
parent 6c94482a
......@@ -9,6 +9,7 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.SortedSet;
import java.util.TreeSet;
public class DetailsDocument extends Document {
......@@ -260,19 +261,19 @@ public class DetailsDocument extends Document {
return unescapeJson(this.hostName);
}
private List<String> verifiedHostNames;
private SortedSet<String> verifiedHostNames;
/**
* Creates a copy of the list with each string escaped for JSON compatibility
* and sets this as the verified host names, unless the argument was null in
* which case the verified host names are just set to null.
*/
public void setVerifiedHostNames(List<String> verifiedHostNames) {
public void setVerifiedHostNames(SortedSet<String> verifiedHostNames) {
if (null == verifiedHostNames) {
this.verifiedHostNames = null;
return;
}
this.verifiedHostNames = new ArrayList<>();
this.verifiedHostNames = new TreeSet<>();
for (String hostName : verifiedHostNames) {
this.verifiedHostNames.add(escapeJson(hostName));
}
......@@ -283,30 +284,30 @@ public class DetailsDocument extends Document {
* compatibility reversed and returns the copy, unless the held reference was
* null in which case null is returned.
*/
public List<String> getVerifiedHostNames() {
public SortedSet<String> getVerifiedHostNames() {
if (null == this.verifiedHostNames) {
return null;
}
List<String> verifiedHostNames = new ArrayList<>();
SortedSet<String> verifiedHostNames = new TreeSet<>();
for (String escapedHostName : this.verifiedHostNames) {
verifiedHostNames.add(unescapeJson(escapedHostName));
}
return verifiedHostNames;
}
private List<String> unverifiedHostNames;
private SortedSet<String> unverifiedHostNames;
/**
* Creates a copy of the list with each string escaped for JSON compatibility
* and sets this as the unverified host names, unless the argument was null in
* which case the unverified host names are just set to null.
*/
public void setUnverifiedHostNames(List<String> unverifiedHostNames) {
public void setUnverifiedHostNames(SortedSet<String> unverifiedHostNames) {
if (null == unverifiedHostNames) {
this.unverifiedHostNames = null;
return;
}
this.unverifiedHostNames = new ArrayList<>();
this.unverifiedHostNames = new TreeSet<>();
for (String hostName : unverifiedHostNames) {
this.unverifiedHostNames.add(escapeJson(hostName));
}
......@@ -317,11 +318,11 @@ public class DetailsDocument extends Document {
* compatibility reversed and returns the copy, unless the held reference was
* null in which case null is returned.
*/
public List<String> getUnverifiedHostNames() {
public SortedSet<String> getUnverifiedHostNames() {
if (null == this.unverifiedHostNames) {
return null;
}
List<String> unverifiedHostNames = new ArrayList<>();
SortedSet<String> unverifiedHostNames = new TreeSet<>();
for (String escapedHostName : this.unverifiedHostNames) {
unverifiedHostNames.add(unescapeJson(escapedHostName));
}
......
......@@ -538,7 +538,7 @@ public class DetailsStatus extends Document {
* and sets this as the verified host names, unless the argument was null in
* which case the verified host names are just set to null.
*/
public void setVerifiedHostNames(List<String> verifiedHostNames) {
public void setVerifiedHostNames(SortedSet<String> verifiedHostNames) {
if (null == verifiedHostNames) {
this.verifiedHostNames = null;
return;
......@@ -554,30 +554,30 @@ public class DetailsStatus extends Document {
* compatibility reversed and returns the copy, unless the held reference was
* null in which case null is returned.
*/
public List<String> getVerifiedHostNames() {
public SortedSet<String> getVerifiedHostNames() {
if (null == this.verifiedHostNames) {
return null;
}
List<String> verifiedHostNames = new ArrayList<>();
SortedSet<String> verifiedHostNames = new TreeSet<>();
for (String escapedHostName : this.verifiedHostNames) {
verifiedHostNames.add(unescapeJson(escapedHostName));
}
return verifiedHostNames;
}
private List<String> unverifiedHostNames;
private SortedSet<String> unverifiedHostNames;
/**
* Creates a copy of the list with each string escaped for JSON compatibility
* and sets this as the unverified host names, unless the argument was null in
* which case the unverified host names are just set to null.
*/
public void setUnverifiedHostNames(List<String> unverifiedHostNames) {
public void setUnverifiedHostNames(SortedSet<String> unverifiedHostNames) {
if (null == unverifiedHostNames) {
this.unverifiedHostNames = null;
return;
}
this.unverifiedHostNames = new ArrayList<>();
this.unverifiedHostNames = new TreeSet<>();
for (String hostName : unverifiedHostNames) {
this.unverifiedHostNames.add(escapeJson(hostName));
}
......@@ -588,11 +588,11 @@ public class DetailsStatus extends Document {
* compatibility reversed and returns the copy, unless the held reference was
* null in which case null is returned.
*/
public List<String> getUnverifiedHostNames() {
public SortedSet<String> getUnverifiedHostNames() {
if (null == this.unverifiedHostNames) {
return null;
}
List<String> unverifiedHostNames = new ArrayList<>();
SortedSet<String> unverifiedHostNames = new TreeSet<>();
for (String escapedHostName : this.unverifiedHostNames) {
unverifiedHostNames.add(unescapeJson(escapedHostName));
}
......
......@@ -445,15 +445,14 @@ public class DocumentStore {
long lastSeenMillis = -1L;
long consensusWeight = -1L;
long firstSeenMillis = -1L;
String hostName = null;
List<String> verifiedHostNames = null;
List<String> unverifiedHostNames = null;
SortedSet<String> verifiedHostNames = null;
SortedSet<String> unverifiedHostNames = null;
Boolean recommendedVersion = null;
SummaryDocument summaryDocument = new SummaryDocument(isRelay,
nickname, fingerprint, addresses, lastSeenMillis, running,
relayFlags, consensusWeight, countryCode, firstSeenMillis,
asNumber, asName, contact, family, family, version, operatingSystem,
hostName, verifiedHostNames, unverifiedHostNames,
verifiedHostNames, unverifiedHostNames,
recommendedVersion);
return summaryDocument;
}
......
......@@ -15,7 +15,6 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.SortedMap;
......@@ -409,23 +408,23 @@ public class NodeStatus extends Document {
return this.hostName;
}
private List<String> verifiedHostNames;
private SortedSet<String> verifiedHostNames;
public void setVerifiedHostNames(List<String> verifiedHostNames) {
public void setVerifiedHostNames(SortedSet<String> verifiedHostNames) {
this.verifiedHostNames = verifiedHostNames;
}
public List<String> getVerifiedHostNames() {
public SortedSet<String> getVerifiedHostNames() {
return this.verifiedHostNames;
}
private List<String> unverifiedHostNames;
private SortedSet<String> unverifiedHostNames;
public void setUnverifiedHostNames(List<String> unverifiedHostNames) {
public void setUnverifiedHostNames(SortedSet<String> unverifiedHostNames) {
this.unverifiedHostNames = unverifiedHostNames;
}
public List<String> getUnverifiedHostNames() {
public SortedSet<String> getUnverifiedHostNames() {
return this.unverifiedHostNames;
}
......@@ -549,15 +548,7 @@ public class NodeStatus extends Document {
Arrays.asList(parts[8].split(","))));
nodeStatus.setConsensusWeight(Long.parseLong(parts[9]));
nodeStatus.setCountryCode(parts[10]);
if (!parts[11].equals("")) {
/* This is a (possibly surprising) hack that is part of moving the
* host name field from node status to details status. As part of
* that move we ignore all previously looked up host names trigger
* a new lookup by setting the last lookup time to 1969-12-31
* 23:59:59.999. This hack may be removed after it has been run
* at least once. */
parts[12] = "-1";
}
/* parts[11] previously contained a hostname */
nodeStatus.setLastRdnsLookup(Long.parseLong(parts[12]));
if (!parts[13].equals("null")) {
nodeStatus.setDefaultPolicy(parts[13]);
......@@ -621,15 +612,26 @@ public class NodeStatus extends Document {
if (parts.length >= 24 && !parts[23].isEmpty()) {
nodeStatus.setVersion(parts[23]);
}
if (parts.length >= 25 && !parts[24].isEmpty()) {
nodeStatus.setHostName(parts[24]);
}
/* parts[24] previously contained a hostname */
if (parts.length >= 26) {
nodeStatus.setVersionStatus(TorVersionStatus.ofAbbreviation(parts[25]));
}
if (parts.length >= 27) {
nodeStatus.setAsName(parts[26]);
}
if (parts.length >= 28) {
SortedSet<String> verifiedHostNames = new TreeSet<>();
SortedSet<String> unverifiedHostNames = new TreeSet<>();
String[] groups = parts[27].split(":", -1);
if (groups[0].length() > 0) {
verifiedHostNames.addAll(Arrays.asList(groups[0].split(";")));
}
if (groups.length > 1 && groups[1].length() > 0) {
unverifiedHostNames.addAll(Arrays.asList(groups[1].split(";")));
}
nodeStatus.setVerifiedHostNames(verifiedHostNames);
nodeStatus.setUnverifiedHostNames(unverifiedHostNames);
}
return nodeStatus;
} catch (NumberFormatException e) {
log.error("Number format exception while parsing node "
......@@ -696,11 +698,14 @@ public class NodeStatus extends Document {
.append(StringUtils.join(this.getIndirectFamily(), ";"));
sb.append("\t")
.append((this.getVersion() != null ? this.getVersion() : ""));
sb.append("\t")
.append((this.getHostName() != null ? this.getHostName() : ""));
sb.append("\t"); /* formerly used for storing host names */
sb.append("\t").append(null != this.getVersionStatus()
? this.getVersionStatus().getAbbreviation() : "");
sb.append("\t").append((this.asName != null ? this.asName : ""));
sb.append("\t").append(this.verifiedHostNames != null
? StringUtils.join(this.getVerifiedHostNames(), ";") : "")
.append(":").append(this.unverifiedHostNames != null
? StringUtils.join(this.getUnverifiedHostNames(), ";") : "");
return sb.toString();
}
}
......
......@@ -332,24 +332,24 @@ public class SummaryDocument extends Document {
}
@JsonProperty("vh")
private List<String> verifiedHostNames;
private SortedSet<String> verifiedHostNames;
public void setVerifiedHostNames(List<String> verifiedHostNames) {
public void setVerifiedHostNames(SortedSet<String> verifiedHostNames) {
this.verifiedHostNames = verifiedHostNames;
}
public List<String> getVerifiedHostNames() {
public SortedSet<String> getVerifiedHostNames() {
return this.verifiedHostNames;
}
@JsonProperty("uh")
private List<String> unverifiedHostNames;
private SortedSet<String> unverifiedHostNames;
public void setUnverifiedHostNames(List<String> unverifiedHostNames) {
public void setUnverifiedHostNames(SortedSet<String> unverifiedHostNames) {
this.unverifiedHostNames = unverifiedHostNames;
}
public List<String> getUnverifiedHostNames() {
public SortedSet<String> getUnverifiedHostNames() {
return this.unverifiedHostNames;
}
......@@ -378,8 +378,8 @@ public class SummaryDocument extends Document {
String countryCode, long firstSeenMillis, String asNumber, String asName,
String contact, SortedSet<String> familyFingerprints,
SortedSet<String> effectiveFamily, String version, String operatingSystem,
String hostName, List<String> verifiedHostNames,
List<String> unverifiedHostNames, Boolean recommendedVersion) {
SortedSet<String> verifiedHostNames,
SortedSet<String> unverifiedHostNames, Boolean recommendedVersion) {
this.setRelay(isRelay);
this.setNickname(nickname);
this.setFingerprint(fingerprint);
......@@ -397,7 +397,6 @@ public class SummaryDocument extends Document {
this.setEffectiveFamily(effectiveFamily);
this.setVersion(version);
this.setOperatingSystem(operatingSystem);
this.setHostName(hostName);
this.setVerifiedHostNames(verifiedHostNames);
this.setUnverifiedHostNames(unverifiedHostNames);
this.setRecommendedVersion(recommendedVersion);
......
......@@ -15,15 +15,14 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.File;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.SortedMap;
import java.util.SortedSet;
import java.util.TreeMap;
import java.util.TreeSet;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;
......@@ -298,12 +297,12 @@ public class NodeIndexer implements ServletContextListener, Runnable {
newRelaysByOperatingSystem.get(operatingSystem).add(fingerprint);
newRelaysByOperatingSystem.get(operatingSystem).add(hashedFingerprint);
}
List<String> allHostNames = new ArrayList<>();
List<String> verifiedHostNames = entry.getVerifiedHostNames();
SortedSet<String> allHostNames = new TreeSet<>();
SortedSet<String> verifiedHostNames = entry.getVerifiedHostNames();
if (null != verifiedHostNames) {
allHostNames.addAll(verifiedHostNames);
}
List<String> unverifiedHostNames = entry.getUnverifiedHostNames();
SortedSet<String> unverifiedHostNames = entry.getUnverifiedHostNames();
if (null != unverifiedHostNames) {
allHostNames.addAll(unverifiedHostNames);
}
......
......@@ -287,8 +287,6 @@ public class ResponseBuilder {
dd.setAsName(detailsDocument.getAsName());
} else if (field.equals(DetailsDocumentFields.CONSENSUS_WEIGHT)) {
dd.setConsensusWeight(detailsDocument.getConsensusWeight());
} else if (field.equals("host_name")) {
dd.setHostName(detailsDocument.getHostName());
} else if (field.equals("verified_host_names")) {
dd.setVerifiedHostNames(detailsDocument.getVerifiedHostNames());
} else if (field.equals("unverified_host_names")) {
......
......@@ -757,35 +757,25 @@ public class NodeDetailsStatusUpdater implements DescriptorListener,
private long startedRdnsLookups = -1L;
private SortedMap<String, String> rdnsLookupResults = new TreeMap<>();
private SortedMap<String, List<String>> rdnsVerifiedLookupResults =
private SortedMap<String, SortedSet<String>> rdnsVerifiedLookupResults =
new TreeMap<>();
private SortedMap<String, List<String>> rdnsUnverifiedLookupResults =
private SortedMap<String, SortedSet<String>> rdnsUnverifiedLookupResults =
new TreeMap<>();
private void finishReverseDomainNameLookups() {
this.reverseDomainNameResolver.finishReverseDomainNameLookups();
this.startedRdnsLookups =
this.reverseDomainNameResolver.getLookupStartMillis();
Map<String, String> lookupResults =
this.reverseDomainNameResolver.getLookupResults();
Map<String, List<String>> verifiedLookupResults =
Map<String, SortedSet<String>> verifiedLookupResults =
this.reverseDomainNameResolver.getVerifiedLookupResults();
Map<String, List<String>> unverifiedLookupResults =
Map<String, SortedSet<String>> unverifiedLookupResults =
this.reverseDomainNameResolver.getUnverifiedLookupResults();
for (String fingerprint : this.currentRelays) {
NodeStatus nodeStatus = this.knownNodes.get(fingerprint);
String hostName = lookupResults.get(nodeStatus.getAddress());
List<String> verifiedHostNames =
SortedSet<String> verifiedHostNames =
verifiedLookupResults.get(nodeStatus.getAddress());
List<String> unverifiedHostNames =
SortedSet<String> unverifiedHostNames =
unverifiedLookupResults.get(nodeStatus.getAddress());
if (null != hostName) {
this.rdnsLookupResults.put(fingerprint, hostName);
} else {
/* Maintains bug compatibility with previous implementation */
this.rdnsLookupResults.put(fingerprint, nodeStatus.getAddress());
}
if (null != verifiedHostNames && !verifiedHostNames.isEmpty()) {
this.rdnsVerifiedLookupResults.put(fingerprint, verifiedHostNames);
}
......@@ -902,15 +892,8 @@ public class NodeDetailsStatusUpdater implements DescriptorListener,
this.exitProbabilities.get(fingerprint));
}
if (this.rdnsLookupResults.containsKey(fingerprint)) {
String hostName = this.rdnsLookupResults.get(fingerprint);
detailsStatus.setHostName(hostName);
nodeStatus.setHostName(hostName);
nodeStatus.setLastRdnsLookup(this.startedRdnsLookups);
}
if (this.rdnsVerifiedLookupResults.containsKey(fingerprint)) {
List<String> verifiedHostNames =
SortedSet<String> verifiedHostNames =
this.rdnsVerifiedLookupResults.get(fingerprint);
detailsStatus.setVerifiedHostNames(verifiedHostNames);
nodeStatus.setVerifiedHostNames(verifiedHostNames);
......@@ -918,7 +901,7 @@ public class NodeDetailsStatusUpdater implements DescriptorListener,
}
if (this.rdnsUnverifiedLookupResults.containsKey(fingerprint)) {
List<String> unverifiedHostNames =
SortedSet<String> unverifiedHostNames =
this.rdnsUnverifiedLookupResults.get(fingerprint);
detailsStatus.setUnverifiedHostNames(unverifiedHostNames);
nodeStatus.setUnverifiedHostNames(unverifiedHostNames);
......
......@@ -3,10 +3,9 @@
package org.torproject.onionoo.updater;
import java.util.ArrayList;
import java.util.Hashtable;
import java.util.List;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import javax.naming.Context;
......@@ -23,9 +22,9 @@ class RdnsLookupRequest extends Thread {
private String address;
private List<String> verifiedHostNames;
private SortedSet<String> verifiedHostNames;
private List<String> unverifiedHostNames;
private SortedSet<String> unverifiedHostNames;
private long lookupStartedMillis = -1L;
......@@ -41,8 +40,8 @@ class RdnsLookupRequest extends Thread {
public void run() {
this.lookupStartedMillis = System.currentTimeMillis();
try {
final List<String> verifiedResults = new ArrayList<>();
final List<String> unverifiedResults = new ArrayList<>();
final SortedSet<String> verifiedResults = new TreeSet<>();
final SortedSet<String> unverifiedResults = new TreeSet<>();
final String[] bytes = this.address.split("\\.");
if (bytes.length == 4) {
final String reverseDnsDomain =
......@@ -107,20 +106,11 @@ class RdnsLookupRequest extends Thread {
return results.toArray(new String[results.size()]);
}
public synchronized String getHostName() {
List<String> verifiedHostNames = this.verifiedHostNames;
if (null != verifiedHostNames && !verifiedHostNames.isEmpty() ) {
return verifiedHostNames.get(0);
} else {
return null;
}
}
public synchronized List<String> getVerifiedHostNames() {
public synchronized SortedSet<String> getVerifiedHostNames() {
return this.verifiedHostNames;
}
public synchronized List<String> getUnverifiedHostNames() {
public synchronized SortedSet<String> getUnverifiedHostNames() {
return this.unverifiedHostNames;
}
......
......@@ -3,7 +3,7 @@
package org.torproject.onionoo.updater;
import java.util.List;
import java.util.SortedSet;
class RdnsLookupWorker extends Thread {
......@@ -39,14 +39,7 @@ class RdnsLookupWorker extends Thread {
} catch (InterruptedException e) {
/* Getting interrupted should be the default case. */
}
String hostName = request.getHostName();
if (null != hostName) {
synchronized (this.reverseDomainNameResolver.rdnsLookupResults) {
this.reverseDomainNameResolver.rdnsLookupResults.put(
rdnsLookupJob, hostName);
}
}
List<String> verifiedHostNames = request.getVerifiedHostNames();
SortedSet<String> verifiedHostNames = request.getVerifiedHostNames();
if (null != verifiedHostNames && !verifiedHostNames.isEmpty()) {
synchronized (this.reverseDomainNameResolver
.rdnsVerifiedLookupResults) {
......@@ -54,7 +47,7 @@ class RdnsLookupWorker extends Thread {
rdnsLookupJob, verifiedHostNames);
}
}
List<String> unverifiedHostNames = request.getUnverifiedHostNames();
SortedSet<String> unverifiedHostNames = request.getUnverifiedHostNames();
if (null != unverifiedHostNames && !unverifiedHostNames.isEmpty()) {
synchronized (this.reverseDomainNameResolver
.rdnsUnverifiedLookupResults) {
......
......@@ -12,6 +12,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.SortedSet;
public class ReverseDomainNameResolver {
......@@ -28,11 +29,9 @@ public class ReverseDomainNameResolver {
Set<String> rdnsLookupJobs;
Map<String, String> rdnsLookupResults;
Map<String, SortedSet<String>> rdnsVerifiedLookupResults;
Map<String, List<String>> rdnsVerifiedLookupResults;
Map<String, List<String>> rdnsUnverifiedLookupResults;
Map<String, SortedSet<String>> rdnsUnverifiedLookupResults;
List<Long> rdnsLookupMillis;
......@@ -56,7 +55,6 @@ public class ReverseDomainNameResolver {
this.rdnsLookupJobs.add(e.getKey());
}
}
this.rdnsLookupResults = new HashMap<>();
this.rdnsVerifiedLookupResults = new HashMap<>();
this.rdnsUnverifiedLookupResults = new HashMap<>();
this.rdnsLookupMillis = new ArrayList<>();
......@@ -82,22 +80,15 @@ public class ReverseDomainNameResolver {
}
}
/** Returns reverse domain name lookup results. */
public Map<String, String> getLookupResults() {
synchronized (this.rdnsLookupResults) {
return new HashMap<>(this.rdnsLookupResults);
}
}
/** Returns reverse domain name verified lookup results. */
public Map<String, List<String>> getVerifiedLookupResults() {
public Map<String, SortedSet<String>> getVerifiedLookupResults() {
synchronized (this.rdnsVerifiedLookupResults) {
return new HashMap<>(this.rdnsVerifiedLookupResults);
}
}
/** Returns reverse domain name unverified lookup results. */
public Map<String, List<String>> getUnverifiedLookupResults() {
public Map<String, SortedSet<String>> getUnverifiedLookupResults() {
synchronized (this.rdnsUnverifiedLookupResults) {
return new HashMap<>(this.rdnsUnverifiedLookupResults);
}
......
......@@ -75,7 +75,6 @@ public class DetailsDocumentWriter implements DocumentWriter {
detailsDocument.setFlags(detailsStatus.getRelayFlags());
detailsDocument.setConsensusWeight(
detailsStatus.getConsensusWeight());
detailsDocument.setHostName(detailsStatus.getHostName());
detailsDocument.setVerifiedHostNames(
detailsStatus.getVerifiedHostNames());
detailsDocument.setUnverifiedHostNames(
......