Browse Source

Merge branch 'maint-0.2.2'

Roger Dingledine 14 years ago
parent
commit
4ff97e3775

+ 9 - 0
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.
+

+ 6 - 2
doc/spec/dir-spec.txt

@@ -1173,12 +1173,14 @@
         research indicates that a lower value would mean fewer cells in
         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
         transit in the network at any given time. Obeyed by Tor 0.2.1.20
         and later.
         and later.
+        Min: 100, Max: 1000
 
 
         "CircuitPriorityHalflifeMsec" -- the halflife parameter used when
         "CircuitPriorityHalflifeMsec" -- the halflife parameter used when
         weighting which circuit will send the next cell. Obeyed by Tor
         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
         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,
         and 0.2.2.10-alpha recognized a "CircPriorityHalflifeMsec" parameter,
         but mishandled it badly.)
         but mishandled it badly.)
+        Min: -1, Max: 2147483647 (INT32_MAX)
 
 
         "perconnbwrate" and "perconnbwburst" -- if set, each relay sets
         "perconnbwrate" and "perconnbwburst" -- if set, each relay sets
         up a separate token bucket for every client OR connection,
         up a separate token bucket for every client OR connection,
@@ -1188,12 +1190,14 @@
         and later. (Note that relays running 0.2.2.7-alpha through
         and later. (Note that relays running 0.2.2.7-alpha through
         0.2.2.14-alpha looked for bwconnrate and bwconnburst, but then
         0.2.2.14-alpha looked for bwconnrate and bwconnburst, but then
         did the wrong thing with them; see bug 1830 for details.)
         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,
         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
         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
         goal is to make it harder for people to use them as one-hop
         proxies. See trac entry 1751 for details.
         proxies. See trac entry 1751 for details.
+        Min: 0, Max: 1
 
 
         See also "2.4.5. Consensus parameters governing behavior"
         See also "2.4.5. Consensus parameters governing behavior"
         in path-spec.txt for a series of circuit build time related
         in path-spec.txt for a series of circuit build time related
@@ -1832,7 +1836,7 @@
 
 
   To ensure consensus, all calculations are performed using integer math
   To ensure consensus, all calculations are performed using integer math
   with a fixed precision determined by the bwweightscale consensus
   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 future balancing improvements, Tor clients support 11 additional weights
   for directory requests and middle weighting. These weights are currently
   for directory requests and middle weighting. These weights are currently

+ 25 - 3
doc/spec/path-spec.txt

@@ -421,57 +421,79 @@ of their choices.
 
 
       cbtdisabled
       cbtdisabled
         Default: 0
         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
                 disabled and history should be discarded. For use in
                 emergency situations only.
                 emergency situations only.
 
 
       cbtnummodes
       cbtnummodes
         Default: 3
         Default: 3
+        Min: 1
+        Max: 20
         Effect: This value governs how many modes to use in the weighted
         Effect: This value governs how many modes to use in the weighted
-        average calculation of Pareto paramter Xm. A value of 3 introduces
+        average calculation of Pareto parameter Xm. A value of 3 introduces
         some bias (2-5% of CDF) under ideal conditions, but allows for better
         some bias (2-5% of CDF) under ideal conditions, but allows for better
         performance in the event that a client chooses guard nodes of radically
         performance in the event that a client chooses guard nodes of radically
         different performance characteristics.
         different performance characteristics.
 
 
       cbtrecentcount
       cbtrecentcount
         Default: 20
         Default: 20
+        Min: 3
+        Max: 1000
         Effect: This is the number of circuit build times to keep track of
         Effect: This is the number of circuit build times to keep track of
                 for the following option.
                 for the following option.
 
 
       cbtmaxtimeouts
       cbtmaxtimeouts
         Default: 18
         Default: 18
+        Min: 3
+        Max: 10000
         Effect: When this many timeouts happen in the last 'cbtrecentcount'
         Effect: When this many timeouts happen in the last 'cbtrecentcount'
                 circuit attempts, the client should discard all of its
                 circuit attempts, the client should discard all of its
                 history and begin learning a fresh timeout value.
                 history and begin learning a fresh timeout value.
 
 
       cbtmincircs
       cbtmincircs
         Default: 100
         Default: 100
+        Min: 1
+        Max: 10000
         Effect: This is the minimum number of circuits to build before
         Effect: This is the minimum number of circuits to build before
                 computing a timeout.
                 computing a timeout.
 
 
       cbtquantile
       cbtquantile
         Default: 80
         Default: 80
+        Min: 10
+        Max: 99
         Effect: This is the position on the quantile curve to use to set the
         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
       cbtclosequantile
         Default: 95
         Default: 95
+        Min: Value of cbtquantile parameter
+        Max: 99
         Effect: This is the position on the quantile curve to use to set the
         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
                 timeout value to use to actually close circuits. It is a percent
                 (0-99).
                 (0-99).
 
 
       cbttestfreq
       cbttestfreq
         Default: 60
         Default: 60
+        Min: 1
+        Max: 2147483647 (INT32_MAX)
         Effect: Describes how often in seconds to build a test circuit to
         Effect: Describes how often in seconds to build a test circuit to
                 gather timeout values. Only applies if less than 'cbtmincircs'
                 gather timeout values. Only applies if less than 'cbtmincircs'
                 have been recorded.
                 have been recorded.
 
 
       cbtmintimeout
       cbtmintimeout
         Default: 2000
         Default: 2000
+        Min: 500
+        Max: 2147483647 (INT32_MAX)
         Effect: This is the minimum allowed timeout value in milliseconds.
         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
       cbtinitialtimeout
         Default: 60000
         Default: 60000
+        Min: Value of cbtmintimeout
+        Max: 2147483647 (INT32_MAX)
         Effect: This is the timeout value to use before computing a timeout,
         Effect: This is the timeout value to use before computing a timeout,
                 in milliseconds.
                 in milliseconds.
 
 

+ 61 - 23
src/or/circuitbuild.c

@@ -107,7 +107,7 @@ circuit_build_times_disabled(void)
     return 0;
     return 0;
   } else {
   } else {
     int consensus_disabled = networkstatus_get_param(NULL, "cbtdisabled",
     int consensus_disabled = networkstatus_get_param(NULL, "cbtdisabled",
-                                                     0);
+                                                     0, 0, 1);
     int config_disabled = !get_options()->LearnCircuitBuildTimeout;
     int config_disabled = !get_options()->LearnCircuitBuildTimeout;
     int dirauth_disabled = get_options()->AuthoritativeDir;
     int dirauth_disabled = get_options()->AuthoritativeDir;
     int state_disabled = (get_or_state()->LastWritten == -1);
     int state_disabled = (get_or_state()->LastWritten == -1);
@@ -129,16 +129,19 @@ circuit_build_times_disabled(void)
 static int32_t
 static int32_t
 circuit_build_times_max_timeouts(void)
 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
 static int32_t
 circuit_build_times_default_num_xm_modes(void)
 circuit_build_times_default_num_xm_modes(void)
 {
 {
   int32_t num = networkstatus_get_param(NULL, "cbtnummodes",
   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;
   return num;
 }
 }
 
 
@@ -146,7 +149,9 @@ static int32_t
 circuit_build_times_min_circs_to_observe(void)
 circuit_build_times_min_circs_to_observe(void)
 {
 {
   int32_t num = networkstatus_get_param(NULL, "cbtmincircs",
   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;
   return num;
 }
 }
 
 
@@ -162,24 +167,46 @@ double
 circuit_build_times_quantile_cutoff(void)
 circuit_build_times_quantile_cutoff(void)
 {
 {
   int32_t num = networkstatus_get_param(NULL, "cbtquantile",
   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;
   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
 static double
 circuit_build_times_close_quantile(void)
 circuit_build_times_close_quantile(void)
 {
 {
-  int32_t num = networkstatus_get_param(NULL, "cbtclosequantile",
-          CBT_DEFAULT_CLOSE_QUANTILE);
-
-  return num/100.0;
+  int32_t param;
+  /* Cast is safe - circuit_build_times_quantile_cutoff() is capped */
+  int32_t min = (int)tor_lround(100*circuit_build_times_quantile_cutoff());
+  param = networkstatus_get_param(NULL, "cbtclosequantile",
+             CBT_DEFAULT_CLOSE_QUANTILE,
+             CBT_MIN_CLOSE_QUANTILE,
+             CBT_MAX_CLOSE_QUANTILE);
+  if (param < min) {
+    log_warn(LD_DIR, "Consensus parameter cbtclosequantile is "
+             "too small, raising to %d", min);
+    param = min;
+  }
+  return param / 100.0;
 }
 }
 
 
 static int32_t
 static int32_t
 circuit_build_times_test_frequency(void)
 circuit_build_times_test_frequency(void)
 {
 {
   int32_t num = networkstatus_get_param(NULL, "cbttestfreq",
   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;
   return num;
 }
 }
 
 
@@ -187,24 +214,35 @@ static int32_t
 circuit_build_times_min_timeout(void)
 circuit_build_times_min_timeout(void)
 {
 {
   int32_t num = networkstatus_get_param(NULL, "cbtmintimeout",
   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;
   return num;
 }
 }
 
 
 int32_t
 int32_t
 circuit_build_times_initial_timeout(void)
 circuit_build_times_initial_timeout(void)
 {
 {
-  int32_t num = networkstatus_get_param(NULL, "cbtinitialtimeout",
-                CBT_DEFAULT_TIMEOUT_INITIAL_VALUE);
-  return num;
+  int32_t min = circuit_build_times_min_timeout();
+  int32_t param = networkstatus_get_param(NULL, "cbtinitialtimeout",
+                                          CBT_DEFAULT_TIMEOUT_INITIAL_VALUE,
+                                          CBT_MIN_TIMEOUT_INITIAL_VALUE,
+                                          CBT_MAX_TIMEOUT_INITIAL_VALUE);
+  if (param < min) {
+    log_warn(LD_DIR, "Consensus parameter cbtinitialtimeout is too small, "
+             "raising to %d", min);
+    param = min;
+  }
+  return param;
 }
 }
 
 
 static int32_t
 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);
 }
 }
 
 
 /**
 /**
@@ -217,8 +255,7 @@ void
 circuit_build_times_new_consensus_params(circuit_build_times_t *cbt,
 circuit_build_times_new_consensus_params(circuit_build_times_t *cbt,
                                          networkstatus_t *ns)
                                          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) {
   if (num > 0 && num != cbt->liveness.num_recent_circs) {
     int8_t *recent_circs;
     int8_t *recent_circs;
@@ -308,7 +345,8 @@ void
 circuit_build_times_init(circuit_build_times_t *cbt)
 circuit_build_times_init(circuit_build_times_t *cbt)
 {
 {
   memset(cbt, 0, sizeof(*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.timeouts_after_firsthop = tor_malloc_zero(sizeof(int8_t)*
                                       cbt->liveness.num_recent_circs);
                                       cbt->liveness.num_recent_circs);
   cbt->close_ms = cbt->timeout_ms = circuit_build_times_get_initial_timeout();
   cbt->close_ms = cbt->timeout_ms = circuit_build_times_get_initial_timeout();

+ 2 - 0
src/or/circuitbuild.h

@@ -122,5 +122,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);
 int circuit_build_times_network_check_live(circuit_build_times_t *cbt);
 void circuit_build_times_network_circ_success(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
 #endif
 
 

+ 3 - 1
src/or/circuitlist.c

@@ -382,7 +382,9 @@ circuit_purpose_to_controller_string(uint8_t purpose)
 int32_t
 int32_t
 circuit_initial_package_window(void)
 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 the consensus tells us a negative number, we'd assert. */
   if (num < 0)
   if (num < 0)
     num = CIRCWINDOW_START;
     num = CIRCWINDOW_START;

+ 4 - 4
src/or/connection_or.c

@@ -391,11 +391,11 @@ connection_or_update_token_buckets_helper(or_connection_t *conn, int reset,
      * bandwidth parameters in the consensus, but allow local config
      * bandwidth parameters in the consensus, but allow local config
      * options to override. */
      * options to override. */
     rate = options->PerConnBWRate ? (int)options->PerConnBWRate :
     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 :
     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;
   conn->bandwidthrate = rate;

+ 0 - 3
src/or/dirvote.h

@@ -19,9 +19,6 @@
 /** Smallest allowable voting interval. */
 /** Smallest allowable voting interval. */
 #define MIN_VOTE_INTERVAL 300
 #define MIN_VOTE_INTERVAL 300
 
 
-/** Precision multiplier for the Bw weights */
-#define BW_WEIGHT_SCALE   10000
-
 void dirvote_free_all(void);
 void dirvote_free_all(void);
 
 
 /* vote manipulation */
 /* vote manipulation */

+ 43 - 11
src/or/networkstatus.c

@@ -2154,32 +2154,52 @@ networkstatus_dump_bridge_status_to_file(time_t now)
   tor_free(status);
   tor_free(status);
 }
 }
 
 
-int32_t
+static int32_t
 get_net_param_from_list(smartlist_t *net_params, const char *param_name,
 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);
   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) {
   SMARTLIST_FOREACH_BEGIN(net_params, const char *, p) {
     if (!strcmpstart(p, param_name) && p[name_len] == '=') {
     if (!strcmpstart(p, param_name) && p[name_len] == '=') {
       int ok=0;
       int ok=0;
       long v = tor_parse_long(p+name_len+1, 10, INT32_MIN,
       long v = tor_parse_long(p+name_len+1, 10, INT32_MIN,
                               INT32_MAX, &ok, NULL);
                               INT32_MAX, &ok, NULL);
-      if (ok)
-        return (int32_t) v;
+      if (ok) {
+        res = (int32_t) v;
+        break;
+      }
     }
     }
   } SMARTLIST_FOREACH_END(p);
   } 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>
 /** 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
  * 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
  * 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
 int32_t
 networkstatus_get_param(networkstatus_t *ns, const char *param_name,
 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 */
   if (!ns) /* if they pass in null, go find it ourselves */
     ns = networkstatus_get_latest_consensus();
     ns = networkstatus_get_latest_consensus();
@@ -2187,24 +2207,36 @@ networkstatus_get_param(networkstatus_t *ns, const char *param_name,
   if (!ns || !ns->net_params)
   if (!ns || !ns->net_params)
     return default_val;
     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
 /** 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
  * <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
  * 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
 int32_t
 networkstatus_get_bw_weight(networkstatus_t *ns, const char *weight_name,
 networkstatus_get_bw_weight(networkstatus_t *ns, const char *weight_name,
-                        int32_t default_val)
+                            int32_t default_val)
 {
 {
+  int32_t param;
+  int max;
   if (!ns) /* if they pass in null, go find it ourselves */
   if (!ns) /* if they pass in null, go find it ourselves */
     ns = networkstatus_get_latest_consensus();
     ns = networkstatus_get_latest_consensus();
 
 
   if (!ns || !ns->weight_params)
   if (!ns || !ns->weight_params)
     return default_val;
     return default_val;
 
 
-  return get_net_param_from_list(ns->weight_params, weight_name, default_val);
+  max = circuit_build_times_get_bw_scale(ns);
+  param = get_net_param_from_list(ns->weight_params, weight_name,
+                                  default_val, -1,
+                                  BW_MAX_WEIGHT_SCALE);
+  if (param > max) {
+    log_warn(LD_DIR, "Value of consensus weight %s was too large, capping "
+             "to %d", weight_name, max);
+    param = max;
+  }
+  return param;
 }
 }
 
 
 /** Return the name of the consensus flavor <b>flav</b> as used to identify
 /** Return the name of the consensus flavor <b>flav</b> as used to identify

+ 2 - 3
src/or/networkstatus.h

@@ -96,10 +96,9 @@ void signed_descs_update_status_from_consensus_networkstatus(
 char *networkstatus_getinfo_helper_single(const routerstatus_t *rs);
 char *networkstatus_getinfo_helper_single(const routerstatus_t *rs);
 char *networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now);
 char *networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now);
 void networkstatus_dump_bridge_status_to_file(time_t now);
 void networkstatus_dump_bridge_status_to_file(time_t now);
-int32_t get_net_param_from_list(smartlist_t *net_params, const char *name,
-                                int default_val);
 int32_t networkstatus_get_param(networkstatus_t *ns, const char *param_name,
 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,
 int getinfo_helper_networkstatus(control_connection_t *conn,
                                  const char *question, char **answer,
                                  const char *question, char **answer,
                                  const char **errmsg);
                                  const char **errmsg);

+ 26 - 1
src/or/or.h

@@ -774,6 +774,8 @@ typedef enum {
 /** Initial value for both sides of a circuit transmission window when the
 /** Initial value for both sides of a circuit transmission window when the
  * circuit is initialized.  Measured in cells. */
  * circuit is initialized.  Measured in cells. */
 #define CIRCWINDOW_START 1000
 #define CIRCWINDOW_START 1000
+#define CIRCWINDOW_START_MIN 100
+#define CIRCWINDOW_START_MAX 1000
 /** Amount to increment a circuit window when we get a circuit SENDME. */
 /** Amount to increment a circuit window when we get a circuit SENDME. */
 #define CIRCWINDOW_INCREMENT 100
 #define CIRCWINDOW_INCREMENT 100
 /** Initial value on both sides of a stream transmission window when the
 /** Initial value on both sides of a stream transmission window when the
@@ -3110,6 +3112,11 @@ struct socks_request_t {
 
 
 /* Circuit Build Timeout "public" structures. */
 /* 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.
 /** Total size of the circuit timeout history to accumulate.
  * 1000 is approx 2.5 days worth of continual-use circuits. */
  * 1000 is approx 2.5 days worth of continual-use circuits. */
 #define CBT_NCIRCUITS_TO_OBSERVE 1000
 #define CBT_NCIRCUITS_TO_OBSERVE 1000
@@ -3119,6 +3126,8 @@ struct socks_request_t {
 
 
 /** Number of modes to use in the weighted-avg computation of Xm */
 /** Number of modes to use in the weighted-avg computation of Xm */
 #define CBT_DEFAULT_NUM_XM_MODES 3
 #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 */
 /** A build_time_t is milliseconds */
 typedef uint32_t build_time_t;
 typedef uint32_t build_time_t;
@@ -3140,12 +3149,16 @@ typedef uint32_t build_time_t;
  * build in terms of CDF quantile.
  * build in terms of CDF quantile.
  */
  */
 #define CBT_DEFAULT_CLOSE_QUANTILE 95
 #define CBT_DEFAULT_CLOSE_QUANTILE 95
+#define CBT_MIN_CLOSE_QUANTILE CBT_MIN_QUANTILE_CUTOFF
+#define CBT_MAX_CLOSE_QUANTILE CBT_MAX_QUANTILE_CUTOFF
 
 
 /**
 /**
  * How many circuits count as recent when considering if the
  * How many circuits count as recent when considering if the
  * connection has gone gimpy or changed.
  * connection has gone gimpy or changed.
  */
  */
 #define CBT_DEFAULT_RECENT_CIRCUITS 20
 #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
  * Maximum count of timeouts that finish the first hop in the past
@@ -3156,25 +3169,37 @@ typedef uint32_t build_time_t;
  * gives us.
  * gives us.
  */
  */
 #define CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT (CBT_DEFAULT_RECENT_CIRCUITS*9/10)
 #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 */
 /** Minimum circuits before estimating a timeout */
 #define CBT_DEFAULT_MIN_CIRCUITS_TO_OBSERVE 100
 #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. */
 /** Cutoff percentile on the CDF for our timeout estimation. */
 #define CBT_DEFAULT_QUANTILE_CUTOFF 80
 #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);
 double circuit_build_times_quantile_cutoff(void);
 
 
 /** How often in seconds should we build a test circuit */
 /** How often in seconds should we build a test circuit */
 #define CBT_DEFAULT_TEST_FREQUENCY 60
 #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 */
 /** Lowest allowable value for CircuitBuildTimeout in milliseconds */
 #define CBT_DEFAULT_TIMEOUT_MIN_VALUE (1500)
 #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 */
 /** Initial circuit build timeout in milliseconds */
 #define CBT_DEFAULT_TIMEOUT_INITIAL_VALUE (60*1000)
 #define CBT_DEFAULT_TIMEOUT_INITIAL_VALUE (60*1000)
+#define CBT_MIN_TIMEOUT_INITIAL_VALUE CBT_MIN_TIMEOUT_MIN_VALUE
+#define CBT_MAX_TIMEOUT_INITIAL_VALUE INT32_MAX
 int32_t circuit_build_times_initial_timeout(void);
 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."
 #error "RECENT_CIRCUITS is set too low."
 #endif
 #endif
 
 

+ 3 - 3
src/or/relay.c

@@ -1993,9 +1993,9 @@ cell_ewma_set_scale_factor(or_options_t *options, networkstatus_t *consensus)
   if (options && options->CircuitPriorityHalflife >= -EPSILON) {
   if (options && options->CircuitPriorityHalflife >= -EPSILON) {
     halflife = options->CircuitPriorityHalflife;
     halflife = options->CircuitPriorityHalflife;
     source = "CircuitPriorityHalflife in configuration";
     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;
     halflife = ((double)halflife_ms)/1000.0;
     source = "CircuitPriorityHalflifeMsec in consensus";
     source = "CircuitPriorityHalflifeMsec in consensus";
   } else {
   } else {

+ 1 - 1
src/or/router.c

@@ -1073,7 +1073,7 @@ should_refuse_unknown_exits(or_options_t *options)
   if (options->RefuseUnknownExits_ != -1) {
   if (options->RefuseUnknownExits_ != -1) {
     return options->RefuseUnknownExits_;
     return options->RefuseUnknownExits_;
   } else {
   } else {
-    return networkstatus_get_param(NULL, "refuseunknownexits", 1);
+    return networkstatus_get_param(NULL, "refuseunknownexits", 1, 0, 1);
   }
   }
 }
 }
 
 

+ 1 - 2
src/or/routerlist.c

@@ -1656,8 +1656,7 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl,
     return NULL;
     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) {
   if (rule == WEIGHT_FOR_GUARD) {
     Wg = networkstatus_get_bw_weight(NULL, "Wgg", -1);
     Wg = networkstatus_get_bw_weight(NULL, "Wgg", -1);

+ 2 - 1
src/or/routerparse.c

@@ -11,6 +11,7 @@
 
 
 #include "or.h"
 #include "or.h"
 #include "config.h"
 #include "config.h"
+#include "circuitbuild.h"
 #include "dirserv.h"
 #include "dirserv.h"
 #include "dirvote.h"
 #include "dirvote.h"
 #include "policies.h"
 #include "policies.h"
@@ -2373,7 +2374,7 @@ networkstatus_verify_bw_weights(networkstatus_t *ns)
   const char *casename = NULL;
   const char *casename = NULL;
   int valid = 1;
   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);
   Wgg = networkstatus_get_bw_weight(ns, "Wgg", -1);
   Wgm = networkstatus_get_bw_weight(ns, "Wgm", -1);
   Wgm = networkstatus_get_bw_weight(ns, "Wgm", -1);
   Wgd = networkstatus_get_bw_weight(ns, "Wgd", -1);
   Wgd = networkstatus_get_bw_weight(ns, "Wgd", -1);

+ 5 - 2
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);
                          "abcd=20 c=60 cw=500 x-yz=-9 zzzzz=101", NULL, 0, 0);
   smartlist_split_string(vote4.net_params,
   smartlist_split_string(vote4.net_params,
                          "ab=900 abcd=200 c=1 cw=51 x-yz=100", NULL, 0, 0);
                          "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, &vote1);
   smartlist_add(votes, &vote2);
   smartlist_add(votes, &vote2);