[or-cvs] [tor/master] Sanity-check consensus param values

arma at torproject.org arma at torproject.org
Sun Jan 16 03:40:09 UTC 2011


commit 026e7987ad312a26efb926ae44adc158770de7cd
Author: Sebastian Hahn <sebastian at torproject.org>
Date:   Thu Dec 30 19:54:13 2010 +0100

    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.
---
 changes/bug2317        |    9 ++++++
 doc/spec/dir-spec.txt  |    8 ++++-
 doc/spec/path-spec.txt |   26 +++++++++++++++++-
 src/or/circuitbuild.c  |   67 +++++++++++++++++++++++++++++++++---------------
 src/or/circuitbuild.h  |    2 +
 src/or/circuitlist.c   |    4 ++-
 src/or/connection_or.c |    8 +++---
 src/or/dirvote.h       |    3 --
 src/or/networkstatus.c |   43 +++++++++++++++++++++++-------
 src/or/networkstatus.h |    3 +-
 src/or/or.h            |   28 +++++++++++++++++++-
 src/or/relay.c         |    6 ++--
 src/or/router.c        |    2 +-
 src/or/routerlist.c    |    3 +-
 src/or/routerparse.c   |    3 +-
 src/test/test_dir.c    |    7 +++-
 16 files changed, 168 insertions(+), 54 deletions(-)

diff --git a/changes/bug2317 b/changes/bug2317
new file mode 100644
index 0000000..0b9366c
--- /dev/null
+++ b/changes/bug2317
@@ -0,0 +1,9 @@
+  o Major features:
+    - Introduce minimum/maximum values that a client is going to believe
+      in a consensus. This helps to avoid crashes or worse when a param
+      has a weird value.
+
+  o Major bugfixes:
+    - Prevent crash/heap corruption when cbtnumnodes consensus parameter is
+      set to 0 or large values. Fixes bug 2317.
+
diff --git a/doc/spec/dir-spec.txt b/doc/spec/dir-spec.txt
index 6e35deb..4ba2ee3 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 2e4207b..4e1bdd0 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.
 
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index b174384..a8e9778 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -106,7 +106,7 @@ circuit_build_times_disabled(void)
     return 0;
   } else {
     int consensus_disabled = networkstatus_get_param(NULL, "cbtdisabled",
-                                                     0);
+                                                     0, 0, 1);
     int config_disabled = !get_options()->LearnCircuitBuildTimeout;
     int dirauth_disabled = get_options()->AuthoritativeDir;
     int state_disabled = (get_or_state()->LastWritten == -1);
@@ -128,16 +128,19 @@ circuit_build_times_disabled(void)
 static int32_t
 circuit_build_times_max_timeouts(void)
 {
-  int32_t num = networkstatus_get_param(NULL, "cbtmaxtimeouts",
-          CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT);
-  return num;
+  return networkstatus_get_param(NULL, "cbtmaxtimeouts",
+                                 CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT,
+                                 CBT_MIN_MAX_RECENT_TIMEOUT_COUNT,
+                                 CBT_MAX_MAX_RECENT_TIMEOUT_COUNT);
 }
 
 static int32_t
 circuit_build_times_default_num_xm_modes(void)
 {
   int32_t num = networkstatus_get_param(NULL, "cbtnummodes",
-          CBT_DEFAULT_NUM_XM_MODES);
+                                        CBT_DEFAULT_NUM_XM_MODES,
+                                        CBT_MIN_NUM_XM_MODES,
+                                        CBT_MAX_NUM_XM_MODES);
   return num;
 }
 
@@ -145,7 +148,9 @@ static int32_t
 circuit_build_times_min_circs_to_observe(void)
 {
   int32_t num = networkstatus_get_param(NULL, "cbtmincircs",
-                CBT_DEFAULT_MIN_CIRCUITS_TO_OBSERVE);
+                                        CBT_DEFAULT_MIN_CIRCUITS_TO_OBSERVE,
+                                        CBT_MIN_MIN_CIRCUITS_TO_OBSERVE,
+                                        CBT_MAX_MIN_CIRCUITS_TO_OBSERVE);
   return num;
 }
 
@@ -161,24 +166,39 @@ double
 circuit_build_times_quantile_cutoff(void)
 {
   int32_t num = networkstatus_get_param(NULL, "cbtquantile",
-                CBT_DEFAULT_QUANTILE_CUTOFF);
+                                        CBT_DEFAULT_QUANTILE_CUTOFF,
+                                        CBT_MIN_QUANTILE_CUTOFF,
+                                        CBT_MAX_QUANTILE_CUTOFF);
   return num/100.0;
 }
 
+int
+circuit_build_times_get_bw_scale(networkstatus_t *ns)
+{
+  return networkstatus_get_param(ns, "bwweightscale",
+                                 BW_WEIGHT_SCALE,
+                                 BW_MIN_WEIGHT_SCALE,
+                                 BW_MAX_WEIGHT_SCALE);
+}
+
 static double
 circuit_build_times_close_quantile(void)
 {
-  int32_t num = networkstatus_get_param(NULL, "cbtclosequantile",
-          CBT_DEFAULT_CLOSE_QUANTILE);
-
-  return num/100.0;
+  return networkstatus_get_param(NULL, "cbtclosequantile",
+             CBT_DEFAULT_CLOSE_QUANTILE,
+             /* Cast is safe, cbtquantile is capped at
+             * CBT_MAX_QUANTILE_CUTOFF. */
+             (int)tor_lround(100*circuit_build_times_quantile_cutoff()),
+             CBT_MAX_CLOSE_QUANTILE) / 100.0;
 }
 
 static int32_t
 circuit_build_times_test_frequency(void)
 {
   int32_t num = networkstatus_get_param(NULL, "cbttestfreq",
-                CBT_DEFAULT_TEST_FREQUENCY);
+                                        CBT_DEFAULT_TEST_FREQUENCY,
+                                        CBT_MIN_TEST_FREQUENCY,
+                                        CBT_MAX_TEST_FREQUENCY);
   return num;
 }
 
@@ -186,7 +206,9 @@ static int32_t
 circuit_build_times_min_timeout(void)
 {
   int32_t num = networkstatus_get_param(NULL, "cbtmintimeout",
-                CBT_DEFAULT_TIMEOUT_MIN_VALUE);
+                                        CBT_DEFAULT_TIMEOUT_MIN_VALUE,
+                                        CBT_MIN_TIMEOUT_MIN_VALUE,
+                                        CBT_MAX_TIMEOUT_MIN_VALUE);
   return num;
 }
 
@@ -194,16 +216,19 @@ int32_t
 circuit_build_times_initial_timeout(void)
 {
   int32_t num = networkstatus_get_param(NULL, "cbtinitialtimeout",
-                CBT_DEFAULT_TIMEOUT_INITIAL_VALUE);
+                                        CBT_DEFAULT_TIMEOUT_INITIAL_VALUE,
+                                        circuit_build_times_min_timeout(),
+                                        CBT_MAX_TIMEOUT_INITIAL_VALUE);
   return num;
 }
 
 static int32_t
-circuit_build_times_recent_circuit_count(void)
+circuit_build_times_recent_circuit_count(networkstatus_t *ns)
 {
-  int32_t num = networkstatus_get_param(NULL, "cbtrecentcount",
-                CBT_DEFAULT_RECENT_CIRCUITS);
-  return num;
+  return networkstatus_get_param(ns, "cbtrecentcount",
+                                 CBT_DEFAULT_RECENT_CIRCUITS,
+                                 CBT_MIN_RECENT_CIRCUITS,
+                                 CBT_MAX_RECENT_CIRCUITS);
 }
 
 /**
@@ -216,8 +241,7 @@ void
 circuit_build_times_new_consensus_params(circuit_build_times_t *cbt,
                                          networkstatus_t *ns)
 {
-  int32_t num = networkstatus_get_param(ns, "cbtrecentcount",
-                   CBT_DEFAULT_RECENT_CIRCUITS);
+  int32_t num = circuit_build_times_recent_circuit_count(ns);
 
   if (num > 0 && num != cbt->liveness.num_recent_circs) {
     int8_t *recent_circs;
@@ -307,7 +331,8 @@ void
 circuit_build_times_init(circuit_build_times_t *cbt)
 {
   memset(cbt, 0, sizeof(*cbt));
-  cbt->liveness.num_recent_circs = circuit_build_times_recent_circuit_count();
+  cbt->liveness.num_recent_circs =
+      circuit_build_times_recent_circuit_count(NULL);
   cbt->liveness.timeouts_after_firsthop = tor_malloc_zero(sizeof(int8_t)*
                                       cbt->liveness.num_recent_circs);
   cbt->close_ms = cbt->timeout_ms = circuit_build_times_get_initial_timeout();
diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h
index 74bbd4f..af24931 100644
--- a/src/or/circuitbuild.h
+++ b/src/or/circuitbuild.h
@@ -121,5 +121,7 @@ void circuit_build_times_network_is_live(circuit_build_times_t *cbt);
 int circuit_build_times_network_check_live(circuit_build_times_t *cbt);
 void circuit_build_times_network_circ_success(circuit_build_times_t *cbt);
 
+int circuit_build_times_get_bw_scale(networkstatus_t *ns);
+
 #endif
 
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 58ff27e..b4f5f45 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -384,7 +384,9 @@ circuit_purpose_to_controller_string(uint8_t purpose)
 int32_t
 circuit_initial_package_window(void)
 {
-  int32_t num = networkstatus_get_param(NULL, "circwindow", CIRCWINDOW_START);
+  int32_t num = networkstatus_get_param(NULL, "circwindow", CIRCWINDOW_START,
+                                        CIRCWINDOW_START_MIN,
+                                        CIRCWINDOW_START_MAX);
   /* If the consensus tells us a negative number, we'd assert. */
   if (num < 0)
     num = CIRCWINDOW_START;
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index cf7c09a..b93699c 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -370,11 +370,11 @@ connection_or_update_token_buckets_helper(or_connection_t *conn, int reset,
      * bandwidth parameters in the consensus, but allow local config
      * options to override. */
     rate = options->PerConnBWRate ? (int)options->PerConnBWRate :
-        (int)networkstatus_get_param(NULL, "perconnbwrate",
-                                     (int)options->BandwidthRate);
+        networkstatus_get_param(NULL, "perconnbwrate",
+                                (int)options->BandwidthRate, 1, INT32_MAX);
     burst = options->PerConnBWBurst ? (int)options->PerConnBWBurst :
-        (int)networkstatus_get_param(NULL, "perconnbwburst",
-                                     (int)options->BandwidthBurst);
+        networkstatus_get_param(NULL, "perconnbwburst",
+                                (int)options->BandwidthBurst, 1, INT32_MAX);
   }
 
   conn->bandwidthrate = rate;
diff --git a/src/or/dirvote.h b/src/or/dirvote.h
index cd5fe86..67540a3 100644
--- a/src/or/dirvote.h
+++ b/src/or/dirvote.h
@@ -19,9 +19,6 @@
 /** Smallest allowable voting interval. */
 #define MIN_VOTE_INTERVAL 300
 
-/** Precision multiplier for the Bw weights */
-#define BW_WEIGHT_SCALE   10000
-
 void dirvote_free_all(void);
 
 /* vote manipulation */
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 76e2159..50bb88b 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -2127,30 +2127,50 @@ networkstatus_dump_bridge_status_to_file(time_t now)
 
 static int32_t
 get_net_param_from_list(smartlist_t *net_params, const char *param_name,
-                        int default_val)
+                        int32_t default_val, int32_t min_val, int32_t max_val)
 {
+  int32_t res = default_val;
   size_t name_len = strlen(param_name);
 
+  tor_assert(max_val > min_val);
+  tor_assert(min_val <= default_val);
+  tor_assert(max_val >= default_val);
+
   SMARTLIST_FOREACH_BEGIN(net_params, const char *, p) {
     if (!strcmpstart(p, param_name) && p[name_len] == '=') {
       int ok=0;
       long v = tor_parse_long(p+name_len+1, 10, INT32_MIN,
                               INT32_MAX, &ok, NULL);
-      if (ok)
-        return (int32_t) v;
+      if (ok) {
+        res = (int32_t) v;
+        break;
+      }
     }
   } SMARTLIST_FOREACH_END(p);
 
-  return default_val;
+  if (res < min_val) {
+    log_warn(LD_DIR, "Consensus parameter %s is too small. Got %d, raising to "
+             "%d.", param_name, res, min_val);
+    res = min_val;
+  } else if (res > max_val) {
+    log_warn(LD_DIR, "Consensus parameter %s is too large. Got %d, capping to "
+             "%d.", param_name, res, max_val);
+    res = max_val;
+  }
+
+  return res;
 }
 
 /** Return the value of a integer parameter from the networkstatus <b>ns</b>
  * whose name is <b>param_name</b>.  If <b>ns</b> is NULL, try loading the
  * latest consensus ourselves. Return <b>default_val</b> if no latest
- * consensus, or if it has no parameter called <b>param_name</b>. */
+ * consensus, or if it has no parameter called <b>param_name</b>.
+ * Make sure the value parsed from the consensus is at least
+ * <b>min_val</b> and at most <b>max_val</b> and raise/cap the parsed value
+ * if necessary. */
 int32_t
 networkstatus_get_param(networkstatus_t *ns, const char *param_name,
-                        int32_t default_val)
+                        int32_t default_val, int32_t min_val, int32_t max_val)
 {
   if (!ns) /* if they pass in null, go find it ourselves */
     ns = networkstatus_get_latest_consensus();
@@ -2158,16 +2178,17 @@ networkstatus_get_param(networkstatus_t *ns, const char *param_name,
   if (!ns || !ns->net_params)
     return default_val;
 
-  return get_net_param_from_list(ns->net_params, param_name, default_val);
+  return get_net_param_from_list(ns->net_params, param_name,
+                                 default_val, min_val, max_val);
 }
 
 /** Return the value of a integer bw weight parameter from the networkstatus
  * <b>ns</b> whose name is <b>weight_name</b>.  If <b>ns</b> is NULL, try
  * loading the latest consensus ourselves. Return <b>default_val</b> if no
- * latest consensus, or if it has no parameter called <b>param_name</b>. */
+ * latest consensus, or if it has no parameter called <b>weight_name</b>. */
 int32_t
 networkstatus_get_bw_weight(networkstatus_t *ns, const char *weight_name,
-                        int32_t default_val)
+                            int32_t default_val)
 {
   if (!ns) /* if they pass in null, go find it ourselves */
     ns = networkstatus_get_latest_consensus();
@@ -2175,7 +2196,9 @@ networkstatus_get_bw_weight(networkstatus_t *ns, const char *weight_name,
   if (!ns || !ns->weight_params)
     return default_val;
 
-  return get_net_param_from_list(ns->weight_params, weight_name, default_val);
+  return get_net_param_from_list(ns->weight_params, weight_name,
+                                 default_val, -1,
+                                 circuit_build_times_get_bw_scale(ns));
 }
 
 /** Return the name of the consensus flavor <b>flav</b> as used to identify
diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
index f95c156..ec2e8f8 100644
--- a/src/or/networkstatus.h
+++ b/src/or/networkstatus.h
@@ -82,7 +82,8 @@ char *networkstatus_getinfo_helper_single(routerstatus_t *rs);
 char *networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now);
 void networkstatus_dump_bridge_status_to_file(time_t now);
 int32_t networkstatus_get_param(networkstatus_t *ns, const char *param_name,
-                                int32_t default_val);
+                                int32_t default_val, int32_t min_val,
+                                int32_t max_val);
 int getinfo_helper_networkstatus(control_connection_t *conn,
                                  const char *question, char **answer,
                                  const char **errmsg);
diff --git a/src/or/or.h b/src/or/or.h
index cb36126..01ff5e8 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -765,6 +765,8 @@ typedef enum {
 /** Initial value for both sides of a circuit transmission window when the
  * circuit is initialized.  Measured in cells. */
 #define CIRCWINDOW_START 1000
+#define CIRCWINDOW_START_MIN 1
+#define CIRCWINDOW_START_MAX 100000
 /** Amount to increment a circuit window when we get a circuit SENDME. */
 #define CIRCWINDOW_INCREMENT 100
 /** Initial value on both sides of a stream transmission window when the
@@ -2942,6 +2944,11 @@ struct socks_request_t {
 
 /* Circuit Build Timeout "public" structures. */
 
+/** Precision multiplier for the Bw weights */
+#define BW_WEIGHT_SCALE   10000
+#define BW_MIN_WEIGHT_SCALE 1
+#define BW_MAX_WEIGHT_SCALE INT32_MAX
+
 /** Total size of the circuit timeout history to accumulate.
  * 1000 is approx 2.5 days worth of continual-use circuits. */
 #define CBT_NCIRCUITS_TO_OBSERVE 1000
@@ -2951,6 +2958,8 @@ struct socks_request_t {
 
 /** Number of modes to use in the weighted-avg computation of Xm */
 #define CBT_DEFAULT_NUM_XM_MODES 3
+#define CBT_MIN_NUM_XM_MODES 1
+#define CBT_MAX_NUM_XM_MODES 20
 
 /** A build_time_t is milliseconds */
 typedef uint32_t build_time_t;
@@ -2972,12 +2981,16 @@ typedef uint32_t build_time_t;
  * build in terms of CDF quantile.
  */
 #define CBT_DEFAULT_CLOSE_QUANTILE 95
+/* Minimum value derived from cbtquantile parameter. */
+#define CBT_MAX_CLOSE_QUANTILE 99
 
 /**
  * How many circuits count as recent when considering if the
  * connection has gone gimpy or changed.
  */
 #define CBT_DEFAULT_RECENT_CIRCUITS 20
+#define CBT_MIN_RECENT_CIRCUITS 3
+#define CBT_MAX_RECENT_CIRCUITS 1000
 
 /**
  * Maximum count of timeouts that finish the first hop in the past
@@ -2988,25 +3001,38 @@ typedef uint32_t build_time_t;
  * gives us.
  */
 #define CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT (CBT_DEFAULT_RECENT_CIRCUITS*9/10)
+#define CBT_MIN_MAX_RECENT_TIMEOUT_COUNT 3
+#define CBT_MAX_MAX_RECENT_TIMEOUT_COUNT 10000
 
 /** Minimum circuits before estimating a timeout */
 #define CBT_DEFAULT_MIN_CIRCUITS_TO_OBSERVE 100
+#define CBT_MIN_MIN_CIRCUITS_TO_OBSERVE 1
+#define CBT_MAX_MIN_CIRCUITS_TO_OBSERVE 10000
 
 /** Cutoff percentile on the CDF for our timeout estimation. */
 #define CBT_DEFAULT_QUANTILE_CUTOFF 80
+#define CBT_MIN_QUANTILE_CUTOFF 10
+#define CBT_MAX_QUANTILE_CUTOFF 99
 double circuit_build_times_quantile_cutoff(void);
 
 /** How often in seconds should we build a test circuit */
 #define CBT_DEFAULT_TEST_FREQUENCY 60
+#define CBT_MIN_TEST_FREQUENCY 1
+#define CBT_MAX_TEST_FREQUENCY INT32_MAX
 
 /** Lowest allowable value for CircuitBuildTimeout in milliseconds */
 #define CBT_DEFAULT_TIMEOUT_MIN_VALUE (1500)
+#define CBT_MIN_TIMEOUT_MIN_VALUE 500
+#define CBT_MAX_TIMEOUT_MIN_VALUE INT32_MAX
 
 /** Initial circuit build timeout in milliseconds */
 #define CBT_DEFAULT_TIMEOUT_INITIAL_VALUE (60*1000)
+#define CBT_MAX_TIMEOUT_INITIAL_VALUE INT32_MAX
+/* CBT_MIN_TIMEOUT_INITIAL_VALUE dependent on
+ * circuit_build_times_min_timeout() */
 int32_t circuit_build_times_initial_timeout(void);
 
-#if CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT < 1
+#if CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT < CBT_MIN_MAX_RECENT_TIMEOUT_COUNT
 #error "RECENT_CIRCUITS is set too low."
 #endif
 
diff --git a/src/or/relay.c b/src/or/relay.c
index 32ac96e..a6c2506 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -1999,9 +1999,9 @@ cell_ewma_set_scale_factor(or_options_t *options, networkstatus_t *consensus)
   if (options && options->CircuitPriorityHalflife >= -EPSILON) {
     halflife = options->CircuitPriorityHalflife;
     source = "CircuitPriorityHalflife in configuration";
-  } else if (consensus &&
-             (halflife_ms = networkstatus_get_param(
-                   consensus, "CircuitPriorityHalflifeMsec", -1)) >= 0) {
+  } else if (consensus && (halflife_ms = networkstatus_get_param(
+                 consensus, "CircuitPriorityHalflifeMsec",
+                 -1, -1, INT32_MAX)) >= 0) {
     halflife = ((double)halflife_ms)/1000.0;
     source = "CircuitPriorityHalflifeMsec in consensus";
   } else {
diff --git a/src/or/router.c b/src/or/router.c
index 3bb37de..26ac351 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1068,7 +1068,7 @@ should_refuse_unknown_exits(or_options_t *options)
   if (options->RefuseUnknownExits_ != -1) {
     return options->RefuseUnknownExits_;
   } else {
-    return networkstatus_get_param(NULL, "refuseunknownexits", 1);
+    return networkstatus_get_param(NULL, "refuseunknownexits", 1, 0, 1);
   }
 }
 
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index e29b4c4..47caebf 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1650,8 +1650,7 @@ smartlist_choose_by_bandwidth_weights(smartlist_t *sl,
     return NULL;
   }
 
-  weight_scale = networkstatus_get_param(NULL, "bwweightscale",
-                                         BW_WEIGHT_SCALE);
+  weight_scale = circuit_build_times_get_bw_scale(NULL);
 
   if (rule == WEIGHT_FOR_GUARD) {
     Wg = networkstatus_get_bw_weight(NULL, "Wgg", -1);
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index a6eef2d..08f81d9 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -11,6 +11,7 @@
 
 #include "or.h"
 #include "config.h"
+#include "circuitbuild.h"
 #include "dirserv.h"
 #include "dirvote.h"
 #include "policies.h"
@@ -2373,7 +2374,7 @@ networkstatus_verify_bw_weights(networkstatus_t *ns)
   const char *casename = NULL;
   int valid = 1;
 
-  weight_scale = networkstatus_get_param(ns, "bwweightscale", BW_WEIGHT_SCALE);
+  weight_scale = circuit_build_times_get_bw_scale(ns);
   Wgg = networkstatus_get_bw_weight(ns, "Wgg", -1);
   Wgm = networkstatus_get_bw_weight(ns, "Wgm", -1);
   Wgd = networkstatus_get_bw_weight(ns, "Wgd", -1);
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index e618150..1f3beb4 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -609,8 +609,11 @@ test_dir_param_voting(void)
                          "abcd=20 c=60 cw=500 x-yz=-9 zzzzz=101", NULL, 0, 0);
   smartlist_split_string(vote4.net_params,
                          "ab=900 abcd=200 c=1 cw=51 x-yz=100", NULL, 0, 0);
-  test_eq(100, networkstatus_get_param(&vote4, "x-yz", 50));
-  test_eq(222, networkstatus_get_param(&vote4, "foobar", 222));
+  test_eq(100, networkstatus_get_param(&vote4, "x-yz", 50, 0, 300));
+  test_eq(222, networkstatus_get_param(&vote4, "foobar", 222, 0, 300));
+  test_eq(80, networkstatus_get_param(&vote4, "ab", 12, 0, 80));
+  test_eq(-8, networkstatus_get_param(&vote4, "ab", -12, -100, -8));
+  test_eq(0, networkstatus_get_param(&vote4, "foobar", 0, -100, 8));
 
   smartlist_add(votes, &vote1);
   smartlist_add(votes, &vote2);





More information about the tor-commits mailing list