Sanity-check consensus param values

We need to make sure that the worst thing that a weird consensus param
can do to us is to break our Tor (and only if the other Tors are
reliably broken in the same way) so that the majority of directory
authorities can't pull any attacks that are worse than the DoS that
they can trigger by simply shutting down.

One of these worse things was the cbtnummodes parameter, which could
lead to heap corruption on some systems if the value was sufficiently
large.

This commit fixes this particular issue and also introduces sanity
checking for all consensus parameters.

2 files changed, 30 insertions, 4 deletions

diff --git a/doc/spec/dir-spec.txt b/doc/spec/dir-spec.txt
index 6e35deb00..4ba2ee38b 100644
--- a/doc/spec/dir-spec.txt
+++ b/doc/spec/dir-spec.txt
@@ -1161,12 +1161,14 @@
         research indicates that a lower value would mean fewer cells in
         transit in the network at any given time. Obeyed by Tor 0.2.1.20
         and later.
+        Min: 1, Max: 100000 XXX are these sane
 
         "CircuitPriorityHalflifeMsec" -- the halflife parameter used when
         weighting which circuit will send the next cell. Obeyed by Tor
         0.2.2.10-alpha and later.  (Versions of Tor between 0.2.2.7-alpha
         and 0.2.2.10-alpha recognized a "CircPriorityHalflifeMsec" parameter,
         but mishandled it badly.)
+        Min: -1, Max: 2147483647 (INT32_MAX) XXX are these sane?
 
         "perconnbwrate" and "perconnbwburst" -- if set, each relay sets
         up a separate token bucket for every client OR connection,
@@ -1176,12 +1178,14 @@
         and later. (Note that relays running 0.2.2.7-alpha through
         0.2.2.14-alpha looked for bwconnrate and bwconnburst, but then
         did the wrong thing with them; see bug 1830 for details.)
+        Min: 1, Max: 2147483647 (INT32_MAX)
 
-        "refuseunknownexits" -- if set and non-zero, exit relays look at
+        "refuseunknownexits" -- if set to one, exit relays look at
         the previous hop of circuits that ask to open an exit stream,
         and refuse to exit if they don't recognize it as a relay. The
         goal is to make it harder for people to use them as one-hop
         proxies. See trac entry 1751 for details.
+        Min: 0, Max: 1
 
         See also "2.4.5. Consensus parameters governing behavior"
         in path-spec.txt for a series of circuit build time related
@@ -1820,7 +1824,7 @@
 
   To ensure consensus, all calculations are performed using integer math
   with a fixed precision determined by the bwweightscale consensus
-  parameter (defaults at 10000).
+  parameter (defaults at 10000, Min: 1, Max: INT32_MAX).
 
   For future balancing improvements, Tor clients support 11 additional weights
   for directory requests and middle weighting. These weights are currently
diff --git a/doc/spec/path-spec.txt b/doc/spec/path-spec.txt
index 2e4207bd5..4e1bdd08c 100644
--- a/doc/spec/path-spec.txt
+++ b/doc/spec/path-spec.txt
@@ -421,12 +421,16 @@ of their choices.
 
       cbtdisabled
         Default: 0
-        Effect: If non-zero, all CircuitBuildTime learning code should be
+        Min: 0
+        Max: 1
+        Effect: If 1, all CircuitBuildTime learning code should be
                 disabled and history should be discarded. For use in
                 emergency situations only.
 
       cbtnummodes
         Default: 3
+        Min: 1
+        Max: 20
         Effect: This value governs how many modes to use in the weighted
         average calculation of Pareto paramter Xm. A value of 3 introduces
         some bias (2-5% of CDF) under ideal conditions, but allows for better
@@ -435,43 +439,61 @@ of their choices.
 
       cbtrecentcount
         Default: 20
+        Min: 3
+        Max: 1000
         Effect: This is the number of circuit build times to keep track of
                 for the following option.
 
       cbtmaxtimeouts
         Default: 18
+        Min: 3
+        Max: 10000
         Effect: When this many timeouts happen in the last 'cbtrecentcount'
                 circuit attempts, the client should discard all of its
                 history and begin learning a fresh timeout value.
 
       cbtmincircs
         Default: 100
+        Min: 1
+        Max: 10000
         Effect: This is the minimum number of circuits to build before
                 computing a timeout.
 
       cbtquantile
         Default: 80
+        Min: 10
+        Max: 99
         Effect: This is the position on the quantile curve to use to set the
-                timeout value. It is a percent (0-99).
+                timeout value. It is a percent (10-99).
 
       cbtclosequantile
         Default: 95
+        Min: Value of cbtquantile parameter
+        Max: 99
         Effect: This is the position on the quantile curve to use to set the
                 timeout value to use to actually close circuits. It is a percent
                 (0-99).
 
       cbttestfreq
         Default: 60
+        Min: 1
+        Max: 2147483647 (INT32_MAX)
         Effect: Describes how often in seconds to build a test circuit to
                 gather timeout values. Only applies if less than 'cbtmincircs'
                 have been recorded.
 
       cbtmintimeout
         Default: 2000
+        Min: 500
+        Max: 2147483647 (INT32_MAX)
         Effect: This is the minimum allowed timeout value in milliseconds.
+                The minimum is to prevent rounding to 0 (we only check once
+                per second).
 
       cbtinitialtimeout
         Default: 60000
+        Min: Value of cbtmintimeout
+        Max: 2147483647 (INT32_MAX)
         Effect: This is the timeout value to use before computing a timeout,
                 in milliseconds.