[tor-commits] [doctor/master] Distinguish between conflicting and unknown consensus params.

[karsten at torproject.org]

commit 1ea13ab733bbd97e16ff76d56c26bcc7eb888090
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date:   Mon Sep 24 13:13:51 2012 -0400

    Distinguish between conflicting and unknown consensus params.
    
    Fixes #6904.
---
 src/org/torproject/doctor/Checker.java          |   62 ++++++++++++++++-------
 src/org/torproject/doctor/StatusFileReport.java |   11 +++-
 src/org/torproject/doctor/Warning.java          |   10 +++-
 3 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/src/org/torproject/doctor/Checker.java b/src/org/torproject/doctor/Checker.java
index 9de06cb..86ef7f7 100644
--- a/src/org/torproject/doctor/Checker.java
+++ b/src/org/torproject/doctor/Checker.java
@@ -38,7 +38,8 @@ public class Checker {
       if (this.isConsensusFresh(this.downloadedConsensus)) {
         this.checkConsensusMethods();
         this.checkRecommendedVersions();
-        this.checkConsensusParameters();
+        this.checkUnknownConsensusParameters();
+        this.checkConflictingConsensusParameters();
         this.checkAuthorityKeys();
         this.checkMissingVotes();
         this.checkBandwidthScanners();
@@ -253,10 +254,9 @@ public class Checker {
     }
   }
 
-  /* Check if a vote contains conflicting or invalid consensus
-   * parameters. */
-  private void checkConsensusParameters() {
-    Set<String> validParameters = new HashSet<String>(Arrays.asList(
+  /* Check if a vote contains unknown consensus parameters. */
+  private void checkUnknownConsensusParameters() {
+    Set<String> knownParameters = new HashSet<String>(Arrays.asList(
         ("circwindow,CircuitPriorityHalflifeMsec,refuseunknownexits,"
         + "cbtdisabled,cbtnummodes,cbtrecentcount,cbtmaxtimeouts,"
         + "cbtmincircs,cbtquantile,cbtclosequantile,cbttestfreq,"
@@ -266,31 +266,55 @@ public class Checker {
     for (RelayNetworkStatusVote vote : this.downloadedVotes) {
       Map<String, Integer> voteConsensusParams =
           vote.getConsensusParams();
-      boolean conflictOrInvalid = false;
       if (voteConsensusParams != null) {
+        StringBuilder message = new StringBuilder();
+        message.append(vote.getNickname());
+        int unknownParameters = 0;
+        for (Map.Entry<String, Integer> e :
+            voteConsensusParams.entrySet()) {
+          if (!knownParameters.contains(e.getKey()) &&
+              !e.getKey().startsWith("bwauth")) {
+            message.append(" " + e.getKey() + "=" + e.getValue());
+            unknownParameters++;
+          }
+        }
+        if (unknownParameters > 0) {
+          conflicts.add(message.toString());
+        }
+      }
+    }
+    if (!conflicts.isEmpty()) {
+      this.warnings.put(Warning.UnknownConsensusParams, conflicts);
+    }
+  }
+
+  /* Check if a vote contains conflicting consensus parameters. */
+  private void checkConflictingConsensusParameters() {
+    SortedSet<String> conflicts = new TreeSet<String>();
+    for (RelayNetworkStatusVote vote : this.downloadedVotes) {
+      Map<String, Integer> voteConsensusParams =
+          vote.getConsensusParams();
+      if (voteConsensusParams != null) {
+        StringBuilder message = new StringBuilder();
+        message.append(vote.getNickname());
+        int conflictingParameters = 0;
         for (Map.Entry<String, Integer> e :
             voteConsensusParams.entrySet()) {
           if (!downloadedConsensus.getConsensusParams().containsKey(
               e.getKey()) ||
               !downloadedConsensus.getConsensusParams().get(e.getKey()).
-              equals(e.getValue()) ||
-              (!validParameters.contains(e.getKey()) &&
-              !e.getKey().startsWith("bwauth"))) {
-            StringBuilder message = new StringBuilder();
-            message.append(vote.getNickname());
-            for (Map.Entry<String, Integer> p :
-                voteConsensusParams.entrySet()) {
-              message.append(" " + p.getKey() + "=" + p.getValue());
-            }
-            conflicts.add(message.toString());
-            break;
+              equals(e.getValue())) {
+            message.append(" " + e.getKey() + "=" + e.getValue());
+            conflictingParameters++;
           }
         }
+        if (conflictingParameters > 0) {
+          conflicts.add(message.toString());
+        }
       }
     }
     if (!conflicts.isEmpty()) {
-      this.warnings.put(Warning.ConflictingOrInvalidConsensusParams,
-          conflicts);
+      this.warnings.put(Warning.ConflictingConsensusParams, conflicts);
     }
   }
 
diff --git a/src/org/torproject/doctor/StatusFileReport.java b/src/org/torproject/doctor/StatusFileReport.java
index a0aef09..107b435 100644
--- a/src/org/torproject/doctor/StatusFileReport.java
+++ b/src/org/torproject/doctor/StatusFileReport.java
@@ -114,10 +114,15 @@ public class StatusFileReport {
               + "authorities recommend other server versions than the "
               + "consensus: " + detailsString, 150L * 60L * 1000L);
           break;
-        case ConflictingOrInvalidConsensusParams:
+        case UnknownConsensusParams:
           warningStrings.put("NOTICE: The following directory "
-              + "authorities set conflicting or invalid consensus "
-              + "parameters: " + detailsString, 330L * 60L * 1000L);
+              + "authorities set unknown consensus parameters: "
+              + detailsString, 330L * 60L * 1000L);
+          break;
+        case ConflictingConsensusParams:
+          warningStrings.put("NOTICE: The following directory "
+              + "authorities set conflicting consensus parameters: "
+              + detailsString, 330L * 60L * 1000L);
           break;
         case CertificateExpiresInThreeMonths:
           warningStrings.put("NOTICE: The certificates of the following "
diff --git a/src/org/torproject/doctor/Warning.java b/src/org/torproject/doctor/Warning.java
index ca1181a..8194425 100644
--- a/src/org/torproject/doctor/Warning.java
+++ b/src/org/torproject/doctor/Warning.java
@@ -28,9 +28,13 @@ public enum Warning {
    * versions than the ones in the consensus. */
   DifferentRecommendedServerVersions,
 
-  /* One or more directory authorities set conflicting or invalid
-   * consensus parameters. */
-  ConflictingOrInvalidConsensusParams,
+  /* One or more directory authorities set unknown consensus
+   * parameters. */
+  UnknownConsensusParams,
+
+  /* One or more directory authorities set conflicting consensus
+   * parameters. */
+  ConflictingConsensusParams,
 
   /* The certificate(s) of one or more directory authorities expire within
    * the next three months, which we warn about just once. */