Browse Source

Bug 17592: Clean up connection timeout logic.

This unifies CircuitIdleTimeout and PredictedCircsRelevanceTime into a single
option, and randomizes it.

It also gives us control over the default value as well as relay-to-relay
connection lifespan through the consensus.

Conflicts:
	src/or/circuituse.c
	src/or/config.c
	src/or/main.c
	src/test/testing_common.c
Mike Perry 7 years ago
parent
commit
d5a151a067

+ 13 - 0
changes/bug17592

@@ -0,0 +1,13 @@
+ o Minor bugfixes (connection lifespan)
+   - Allow more control over how long TLS connections are kept open: unify
+     CircuitIdleTimeout and PredictedPortsRelevanceTime into a single option
+     called CircuitsAvailableTimeout. Also, allow the consensus to control
+     the default values for both this preference, as well as the lifespan
+     of relay-to-relay connections. Fixes bug #17592.
+   - Increase the intial circuit build timeout testing frequency, to help
+     ensure that ReducedConnectionPadding clients finish learning a timeout
+     before their orconn would expire. The initial testing rate was set back
+     in the days of TAP and before the Tor Browser updater, when we had to be
+     much more careful about new clients making lots of circuits. With this
+     change, a circuit build time is learned in about 15-20 minutes, instead
+     of ~100-120 minutes.

+ 13 - 7
doc/tor.1.txt

@@ -666,8 +666,8 @@ GENERAL OPTIONS
 [[PredictedPortsRelevanceTime]] **PredictedPortsRelevanceTime** __NUM__::
 [[PredictedPortsRelevanceTime]] **PredictedPortsRelevanceTime** __NUM__::
     Set how long, after the client has made an anonymized connection to a
     Set how long, after the client has made an anonymized connection to a
     given port, we will try to make sure that we build circuits to
     given port, we will try to make sure that we build circuits to
-    exits that support that port. The maximum value for this option is 1
-    hour. (Default: 1 hour)
+    exits that support that port. This option is deprecated. Please use
+    CircuitsAvailableTimeout instead.
 
 
 [[RunAsDaemon]] **RunAsDaemon** **0**|**1**::
 [[RunAsDaemon]] **RunAsDaemon** **0**|**1**::
     If 1, Tor forks and daemonizes to the background. This option has no effect
     If 1, Tor forks and daemonizes to the background. This option has no effect
@@ -809,13 +809,19 @@ The following options are useful only for clients (that is, if
     LearnCircuitBuildTimeout is 0, this value is the only value used.
     LearnCircuitBuildTimeout is 0, this value is the only value used.
     (Default: 60 seconds)
     (Default: 60 seconds)
 
 
+[[CircuitsAvailableTimeout]] **CircuitsAvailableTimeout** __NUM__::
+    Tor will attempt to keep at least one open, unused circuit available for
+    this amount of time. This option governs how long idle circuits are kept
+    open, as well as the amount of time Tor will keep a circuit open to each
+    of the recently used ports. This way when the Tor client is entirely
+    idle, it can expire all of its circuits, and then expire its TLS
+    connections. Note that the actual timeout value is uniformly randomized
+    from the specified value to twice that amount. (Default: 30 minutes;
+    Max: 24 hours)
+
 [[CircuitIdleTimeout]] **CircuitIdleTimeout** __NUM__::
 [[CircuitIdleTimeout]] **CircuitIdleTimeout** __NUM__::
     If we have kept a clean (never used) circuit around for NUM seconds, then
     If we have kept a clean (never used) circuit around for NUM seconds, then
-    close it. This way when the Tor client is entirely idle, it can expire all
-    of its circuits, and then expire its TLS connections. Also, if we end up
-    making a circuit that is not useful for exiting any of the requests we're
-    receiving, it won't forever take up a slot in the circuit list. (Default: 1
-    hour)
+    close it. This option is deprecated. Use CircuitsAvailableTimeout instead.
 
 
 [[CircuitStreamTimeout]] **CircuitStreamTimeout** __NUM__::
 [[CircuitStreamTimeout]] **CircuitStreamTimeout** __NUM__::
     If non-zero, this option overrides our internal timeout schedule for how
     If non-zero, this option overrides our internal timeout schedule for how

+ 105 - 0
src/or/channelpadding.c

@@ -446,6 +446,111 @@ channelpadding_compute_time_until_pad_for_netflow(channel_t *chan)
   return CHANNELPADDING_TIME_LATER;
   return CHANNELPADDING_TIME_LATER;
 }
 }
 
 
+/**
+ * Returns a randomized value for channel idle timeout in seconds.
+ * The channel idle timeout governs how quickly we close a channel
+ * after its last circuit has disappeared.
+ *
+ * There are three classes of channels:
+ *  1. Client+non-canonical. These live for 3-4.5 minutes
+ *  2. relay to relay. These live for 45-75 min by default
+ *  3. Reduced padding clients. These live for 1.5-2.25 minutes.
+ *
+ * Also allows the default relay-to-relay value to be controlled by the
+ * consensus.
+ */
+unsigned int
+channelpadding_get_channel_idle_timeout(const channel_t *chan,
+                                        int is_canonical)
+{
+  const or_options_t *options = get_options();
+  unsigned int timeout;
+
+  /* Non-canonical and client channels only last for 3-4.5 min when idle */
+  if (!is_canonical || !public_server_mode(options) ||
+      chan->is_client ||
+      !connection_or_digest_is_known_relay(chan->identity_digest)) {
+#define CONNTIMEOUT_CLIENTS_BASE 180 // 3 to 4.5 min
+    timeout = CONNTIMEOUT_CLIENTS_BASE
+        + crypto_rand_int(CONNTIMEOUT_CLIENTS_BASE/2);
+  } else { // Canonical relay-to-relay channels
+    // 45..75min or consensus +/- 25%
+#define CONNTIMEOUT_RELAYS_DFLT (60*60) // 1 hour
+#define CONNTIMEOUT_RELAYS_MIN 60
+#define CONNTIMEOUT_RELAYS_MAX (7*24*60*60) // 1 week
+    timeout = networkstatus_get_param(NULL, "nf_conntimeout_relays",
+        CONNTIMEOUT_RELAYS_DFLT,
+        CONNTIMEOUT_RELAYS_MIN,
+        CONNTIMEOUT_RELAYS_MAX);
+
+    timeout = 3*timeout/4 + crypto_rand_int(timeout/2);
+  }
+
+  /* If ReducedConnectionPadding is set, we want to halve the duration of
+   * the channel idle timeout, since reducing the additional time that
+   * a channel stays open will reduce the total overhead for making
+   * new channels. This reduction in overhead/channel expense
+   * is important for mobile users. The option cannot be set by relays.
+   *
+   * We also don't reduce any values for timeout that the user explicitly
+   * set.
+   */
+  if (options->ReducedConnectionPadding
+          && !options->CircuitsAvailableTimeout) {
+    timeout /= 2;
+  }
+
+  return timeout;
+}
+
+/**
+ * This function controls how long we keep idle circuits open,
+ * and how long we build predicted circuits. This behavior is under
+ * the control of channelpadding because circuit availability is the
+ * dominant factor in channel lifespan, which influences total padding
+ * overhead.
+ *
+ * Returns a randomized number of seconds in a range from
+ * CircuitsAvailableTimeout to 2*CircuitsAvailableTimeout. This value is halved
+ * if ReducedConnectionPadding is set. The default value of
+ * CircuitsAvailableTimeout can be controlled by the consensus.
+ */
+int
+channelpadding_get_circuits_available_timeout(void)
+{
+  const or_options_t *options = get_options();
+  int timeout = options->CircuitsAvailableTimeout;
+
+  if (!timeout) {
+#define CIRCTIMEOUT_CLIENTS_DFLT (30*60) // 30 minutes
+#define CIRCTIMEOUT_CLIENTS_MIN 60
+#define CIRCTIMEOUT_CLIENTS_MAX (24*60*60) // 24 hours
+    timeout = networkstatus_get_param(NULL, "nf_conntimeout_clients",
+        CIRCTIMEOUT_CLIENTS_DFLT,
+        CIRCTIMEOUT_CLIENTS_MIN,
+        CIRCTIMEOUT_CLIENTS_MAX);
+
+    /* If ReducedConnectionPadding is set, we want to halve the duration of
+     * the channel idle timeout, since reducing the additional time that
+     * a channel stays open will reduce the total overhead for making
+     * new connections. This reduction in overhead/connection expense
+     * is important for mobile users. The option cannot be set by relays.
+     *
+     * We also don't reduce any values for timeout that the user explicitly
+     * set.
+     */
+    if (options->ReducedConnectionPadding) {
+      // half the value to 15..30min by default
+      timeout /= 2;
+    }
+  }
+
+  // 30..60min by default
+  timeout = timeout + crypto_rand_int(timeout);
+
+  return timeout;
+}
+
 /**
 /**
  * Calling this function on a channel causes it to tell the other side
  * Calling this function on a channel causes it to tell the other side
  * not to send padding, and disables sending padding from this side as well.
  * not to send padding, and disables sending padding from this side as well.

+ 41 - 0
src/or/circuitlist.c

@@ -78,6 +78,7 @@
 #include "rephist.h"
 #include "rephist.h"
 #include "routerlist.h"
 #include "routerlist.h"
 #include "routerset.h"
 #include "routerset.h"
+#include "channelpadding.h"
 
 
 #include "ht.h"
 #include "ht.h"
 
 
@@ -814,6 +815,11 @@ init_circuit_base(circuit_t *circ)
   circ->global_circuitlist_idx = smartlist_len(circuit_get_global_list()) - 1;
   circ->global_circuitlist_idx = smartlist_len(circuit_get_global_list()) - 1;
 }
 }
 
 
+/** If we haven't yet decided on a good timeout value for circuit
+ * building, we close idle circuits aggressively so we can get more
+ * data points. */
+#define IDLE_TIMEOUT_WHILE_LEARNING (1*60)
+
 /** Allocate space for a new circuit, initializing with <b>p_circ_id</b>
 /** Allocate space for a new circuit, initializing with <b>p_circ_id</b>
  * and <b>p_conn</b>. Add it to the global circuit list.
  * and <b>p_conn</b>. Add it to the global circuit list.
  */
  */
@@ -841,6 +847,41 @@ origin_circuit_new(void)
 
 
   circuit_build_times_update_last_circ(get_circuit_build_times_mutable());
   circuit_build_times_update_last_circ(get_circuit_build_times_mutable());
 
 
+  if (! circuit_build_times_disabled(get_options()) &&
+      circuit_build_times_needs_circuits(get_circuit_build_times())) {
+    /* Circuits should be shorter lived if we need more of them
+     * for learning a good build timeout */
+    circ->circuit_idle_timeout = IDLE_TIMEOUT_WHILE_LEARNING;
+  } else {
+    // This should always be larger than the current port prediction time
+    // remaining, or else we'll end up with the case where a circuit times out
+    // and another one is built, effectively doubling the timeout window.
+    //
+    // We also randomize it by up to 5% more (ie 5% of 0 to 3600 seconds,
+    // depending on how much circuit prediction time is remaining) so that
+    // we don't close a bunch of unused circuits all at the same time.
+    int prediction_time_remaining =
+      predicted_ports_prediction_time_remaining(time(NULL));
+    circ->circuit_idle_timeout = prediction_time_remaining+1+
+        crypto_rand_int(1+prediction_time_remaining/20);
+
+    if (circ->circuit_idle_timeout <= 0) {
+      log_warn(LD_BUG,
+               "Circuit chose a negative idle timeout of %d based on "
+               "%d seconds of predictive building remaining.",
+               circ->circuit_idle_timeout,
+               prediction_time_remaining);
+      circ->circuit_idle_timeout = IDLE_TIMEOUT_WHILE_LEARNING;
+    }
+
+    log_info(LD_CIRC,
+              "Circuit " U64_FORMAT " chose an idle timeout of %d based on "
+              "%d seconds of predictive building remaining.",
+              U64_PRINTF_ARG(circ->global_identifier),
+              circ->circuit_idle_timeout,
+              prediction_time_remaining);
+  }
+
   return circ;
   return circ;
 }
 }
 
 

+ 8 - 17
src/or/circuituse.c

@@ -1383,11 +1383,6 @@ circuit_detach_stream(circuit_t *circ, edge_connection_t *conn)
   tor_fragile_assert();
   tor_fragile_assert();
 }
 }
 
 
-/** If we haven't yet decided on a good timeout value for circuit
- * building, we close idles circuits aggressively so we can get more
- * data points. */
-#define IDLE_TIMEOUT_WHILE_LEARNING (10*60)
-
 /** Find each circuit that has been unused for too long, or dirty
 /** Find each circuit that has been unused for too long, or dirty
  * for too long and has no streams on it: mark it for close.
  * for too long and has no streams on it: mark it for close.
  */
  */
@@ -1397,21 +1392,15 @@ circuit_expire_old_circuits_clientside(void)
   struct timeval cutoff, now;
   struct timeval cutoff, now;
 
 
   tor_gettimeofday(&now);
   tor_gettimeofday(&now);
-  cutoff = now;
   last_expired_clientside_circuits = now.tv_sec;
   last_expired_clientside_circuits = now.tv_sec;
 
 
-  if (! circuit_build_times_disabled(get_options()) &&
-      circuit_build_times_needs_circuits(get_circuit_build_times())) {
-    /* Circuits should be shorter lived if we need more of them
-     * for learning a good build timeout */
-    cutoff.tv_sec -= IDLE_TIMEOUT_WHILE_LEARNING;
-  } else {
-    cutoff.tv_sec -= get_options()->CircuitIdleTimeout;
-  }
-
   SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ) {
   SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ) {
     if (circ->marked_for_close || !CIRCUIT_IS_ORIGIN(circ))
     if (circ->marked_for_close || !CIRCUIT_IS_ORIGIN(circ))
       continue;
       continue;
+
+    cutoff = now;
+    cutoff.tv_sec -= TO_ORIGIN_CIRCUIT(circ)->circuit_idle_timeout;
+
     /* If the circuit has been dirty for too long, and there are no streams
     /* If the circuit has been dirty for too long, and there are no streams
      * on it, mark it for close.
      * on it, mark it for close.
      */
      */
@@ -1437,8 +1426,10 @@ circuit_expire_old_circuits_clientside(void)
                 (circ->purpose >= CIRCUIT_PURPOSE_C_INTRODUCING &&
                 (circ->purpose >= CIRCUIT_PURPOSE_C_INTRODUCING &&
                 circ->purpose <= CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED) ||
                 circ->purpose <= CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED) ||
                 circ->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND) {
                 circ->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND) {
-          log_debug(LD_CIRC,
-                    "Closing circuit that has been unused for %ld msec.",
+          log_info(LD_CIRC,
+                    "Closing circuit "U64_FORMAT
+                    " that has been unused for %ld msec.",
+                    U64_PRINTF_ARG(TO_ORIGIN_CIRCUIT(circ)->global_identifier),
                     tv_mdiff(&circ->timestamp_began, &now));
                     tv_mdiff(&circ->timestamp_began, &now));
           circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
           circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
         } else if (!TO_ORIGIN_CIRCUIT(circ)->is_ancient) {
         } else if (!TO_ORIGIN_CIRCUIT(circ)->is_ancient) {

+ 14 - 13
src/or/config.c

@@ -245,7 +245,8 @@ static config_var_t option_vars_[] = {
   V(PaddingStatistics,           BOOL,     "1"),
   V(PaddingStatistics,           BOOL,     "1"),
   V(LearnCircuitBuildTimeout,    BOOL,     "1"),
   V(LearnCircuitBuildTimeout,    BOOL,     "1"),
   V(CircuitBuildTimeout,         INTERVAL, "0"),
   V(CircuitBuildTimeout,         INTERVAL, "0"),
-  V(CircuitIdleTimeout,          INTERVAL, "1 hour"),
+  OBSOLETE("CircuitIdleTimeout"),
+  V(CircuitsAvailableTimeout,    INTERVAL, "0"),
   V(CircuitStreamTimeout,        INTERVAL, "0"),
   V(CircuitStreamTimeout,        INTERVAL, "0"),
   V(CircuitPriorityHalflife,     DOUBLE,  "-100.0"), /*negative:'Use default'*/
   V(CircuitPriorityHalflife,     DOUBLE,  "-100.0"), /*negative:'Use default'*/
   V(ClientDNSRejectInternalAddresses, BOOL,"1"),
   V(ClientDNSRejectInternalAddresses, BOOL,"1"),
@@ -402,7 +403,7 @@ static config_var_t option_vars_[] = {
   V(NATDListenAddress,           LINELIST, NULL),
   V(NATDListenAddress,           LINELIST, NULL),
   VPORT(NATDPort),
   VPORT(NATDPort),
   V(Nickname,                    STRING,   NULL),
   V(Nickname,                    STRING,   NULL),
-  V(PredictedPortsRelevanceTime,  INTERVAL, "1 hour"),
+  OBSOLETE("PredictedPortsRelevanceTime"),
   V(WarnUnsafeSocks,              BOOL,     "1"),
   V(WarnUnsafeSocks,              BOOL,     "1"),
   VAR("NodeFamily",              LINELIST, NodeFamilies,         NULL),
   VAR("NodeFamily",              LINELIST, NodeFamilies,         NULL),
   V(NumCPUs,                     UINT,     "0"),
   V(NumCPUs,                     UINT,     "0"),
@@ -2812,10 +2813,10 @@ compute_publishserverdescriptor(or_options_t *options)
 #define MIN_REND_POST_PERIOD (10*60)
 #define MIN_REND_POST_PERIOD (10*60)
 #define MIN_REND_POST_PERIOD_TESTING (5)
 #define MIN_REND_POST_PERIOD_TESTING (5)
 
 
-/** Highest allowable value for PredictedPortsRelevanceTime; if this is
- * too high, our selection of exits will decrease for an extended
- * period of time to an uncomfortable level .*/
-#define MAX_PREDICTED_CIRCS_RELEVANCE (60*60)
+/** Higest allowable value for CircuitsAvailableTimeout.
+ * If this is too large, client connections will stay open for too long,
+ * incurring extra padding overhead. */
+#define MAX_CIRCS_AVAILABLE_TIME (24*60*60)
 
 
 /** Highest allowable value for RendPostPeriod. */
 /** Highest allowable value for RendPostPeriod. */
 #define MAX_DIR_PERIOD (MIN_ONION_KEY_LIFETIME/2)
 #define MAX_DIR_PERIOD (MIN_ONION_KEY_LIFETIME/2)
@@ -3461,17 +3462,17 @@ options_validate(or_options_t *old_options, or_options_t *options,
     options->RendPostPeriod = MAX_DIR_PERIOD;
     options->RendPostPeriod = MAX_DIR_PERIOD;
   }
   }
 
 
-  if (options->PredictedPortsRelevanceTime >
-      MAX_PREDICTED_CIRCS_RELEVANCE) {
-    log_warn(LD_CONFIG, "PredictedPortsRelevanceTime is too large; "
-             "clipping to %ds.", MAX_PREDICTED_CIRCS_RELEVANCE);
-    options->PredictedPortsRelevanceTime = MAX_PREDICTED_CIRCS_RELEVANCE;
-  }
-
   /* Check the Single Onion Service options */
   /* Check the Single Onion Service options */
   if (options_validate_single_onion(options, msg) < 0)
   if (options_validate_single_onion(options, msg) < 0)
     return -1;
     return -1;
 
 
+  if (options->CircuitsAvailableTimeout > MAX_CIRCS_AVAILABLE_TIME) {
+    // options_t is immutable for new code (the above code is older),
+    // so just make the user fix the value themselves rather than
+    // silently keep a shadow value lower than what they asked for.
+    REJECT("CircuitsAvailableTimeout is too large. Max is 24 hours.");
+  }
+
 #ifdef ENABLE_TOR2WEB_MODE
 #ifdef ENABLE_TOR2WEB_MODE
   if (options->Tor2webMode && options->UseEntryGuards) {
   if (options->Tor2webMode && options->UseEntryGuards) {
     /* tor2web mode clients do not (and should not) use entry guards
     /* tor2web mode clients do not (and should not) use entry guards

+ 8 - 22
src/or/connection_or.c

@@ -815,24 +815,6 @@ connection_or_update_token_buckets(smartlist_t *conns,
   });
   });
 }
 }
 
 
-/** How long do we wait before killing non-canonical OR connections with no
- * circuits?  In Tor versions up to 0.2.1.25 and 0.2.2.12-alpha, we waited 15
- * minutes before cancelling these connections, which caused fast relays to
- * accrue many many idle connections. Hopefully 3-4.5 minutes is low enough
- * that it kills most idle connections, without being so low that we cause
- * clients to bounce on and off.
- *
- * For canonical connections, the limit is higher, at 15-22.5 minutes.
- *
- * For each OR connection, we randomly add up to 50% extra to its idle_timeout
- * field, to avoid exposing when exactly the last circuit closed.  Since we're
- * storing idle_timeout in a uint16_t, don't let these values get higher than
- * 12 hours or so without revising connection_or_set_canonical and/or expanding
- * idle_timeout.
- */
-#define IDLE_OR_CONN_TIMEOUT_NONCANONICAL 180
-#define IDLE_OR_CONN_TIMEOUT_CANONICAL 900
-
 /* Mark <b>or_conn</b> as canonical if <b>is_canonical</b> is set, and
 /* Mark <b>or_conn</b> as canonical if <b>is_canonical</b> is set, and
  * non-canonical otherwise. Adjust idle_timeout accordingly.
  * non-canonical otherwise. Adjust idle_timeout accordingly.
  */
  */
@@ -840,9 +822,6 @@ void
 connection_or_set_canonical(or_connection_t *or_conn,
 connection_or_set_canonical(or_connection_t *or_conn,
                             int is_canonical)
                             int is_canonical)
 {
 {
-  const unsigned int timeout_base = is_canonical ?
-    IDLE_OR_CONN_TIMEOUT_CANONICAL : IDLE_OR_CONN_TIMEOUT_NONCANONICAL;
-
   if (bool_eq(is_canonical, or_conn->is_canonical) &&
   if (bool_eq(is_canonical, or_conn->is_canonical) &&
       or_conn->idle_timeout != 0) {
       or_conn->idle_timeout != 0) {
     /* Don't recalculate an existing idle_timeout unless the canonical
     /* Don't recalculate an existing idle_timeout unless the canonical
@@ -851,7 +830,14 @@ connection_or_set_canonical(or_connection_t *or_conn,
   }
   }
 
 
   or_conn->is_canonical = !! is_canonical; /* force to a 1-bit boolean */
   or_conn->is_canonical = !! is_canonical; /* force to a 1-bit boolean */
-  or_conn->idle_timeout = timeout_base + crypto_rand_int(timeout_base / 2);
+  or_conn->idle_timeout = channelpadding_get_channel_idle_timeout(
+          TLS_CHAN_TO_BASE(or_conn->chan), is_canonical);
+
+  log_info(LD_CIRC,
+          "Channel " U64_FORMAT " chose an idle timeout of %d.",
+          or_conn->chan ?
+          U64_PRINTF_ARG(TLS_CHAN_TO_BASE(or_conn->chan)->global_identifier):0,
+          or_conn->idle_timeout);
 }
 }
 
 
 /** If we don't necessarily know the router we're connecting to, but we
 /** If we don't necessarily know the router we're connecting to, but we

+ 6 - 2
src/or/main.c

@@ -1096,8 +1096,9 @@ run_connection_housekeeping(int i, time_t now)
   } else if (!have_any_circuits &&
   } else if (!have_any_circuits &&
              now - or_conn->idle_timeout >=
              now - or_conn->idle_timeout >=
                                          chan->timestamp_last_had_circuits) {
                                          chan->timestamp_last_had_circuits) {
-    log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) "
-             "[no circuits for %d; timeout %d; %scanonical].",
+    log_info(LD_OR,"Expiring non-used OR connection "U64_FORMAT" to fd %d "
+             "(%s:%d) [no circuits for %d; timeout %d; %scanonical].",
+             U64_PRINTF_ARG(chan->global_identifier),
              (int)conn->s, conn->address, conn->port,
              (int)conn->s, conn->address, conn->port,
              (int)(now - chan->timestamp_last_had_circuits),
              (int)(now - chan->timestamp_last_had_circuits),
              or_conn->idle_timeout,
              or_conn->idle_timeout,
@@ -3019,6 +3020,9 @@ tor_init(int argc, char *argv[])
   /* The options are now initialised */
   /* The options are now initialised */
   const or_options_t *options = get_options();
   const or_options_t *options = get_options();
 
 
+  /* Initialize predicted ports list after loading options */
+  predicted_ports_init();
+
 #ifndef _WIN32
 #ifndef _WIN32
   if (geteuid()==0)
   if (geteuid()==0)
     log_warn(LD_GENERAL,"You are running Tor as root. You don't need to, "
     log_warn(LD_GENERAL,"You are running Tor as root. You don't need to, "

+ 10 - 1
src/or/or.h

@@ -3301,6 +3301,13 @@ typedef struct origin_circuit_t {
    * adjust_exit_policy_from_exitpolicy_failure.
    * adjust_exit_policy_from_exitpolicy_failure.
    */
    */
   smartlist_t *prepend_policy;
   smartlist_t *prepend_policy;
+
+  /** How long do we wait before closing this circuit if it remains
+   * completely idle after it was built, in seconds? This value
+   * is randomized on a per-circuit basis from CircuitsAvailableTimoeut
+   * to 2*CircuitsAvailableTimoeut. */
+  int circuit_idle_timeout;
+
 } origin_circuit_t;
 } origin_circuit_t;
 
 
 struct onion_queue_t;
 struct onion_queue_t;
@@ -3882,6 +3889,8 @@ typedef struct {
                             * adaptive algorithm learns a new value. */
                             * adaptive algorithm learns a new value. */
   int CircuitIdleTimeout; /**< Cull open clean circuits that were born
   int CircuitIdleTimeout; /**< Cull open clean circuits that were born
                            * at least this many seconds ago. */
                            * at least this many seconds ago. */
+  int CircuitsAvailableTimeout; /**< Try to have an open circuit for at
+                                     least this long after last activity */
   int CircuitStreamTimeout; /**< If non-zero, detach streams from circuits
   int CircuitStreamTimeout; /**< If non-zero, detach streams from circuits
                              * and try a new circuit if the stream has been
                              * and try a new circuit if the stream has been
                              * waiting for this many seconds. If zero, use
                              * waiting for this many seconds. If zero, use
@@ -4826,7 +4835,7 @@ typedef uint32_t build_time_t;
 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 10
 #define CBT_MIN_TEST_FREQUENCY 1
 #define CBT_MIN_TEST_FREQUENCY 1
 #define CBT_MAX_TEST_FREQUENCY INT32_MAX
 #define CBT_MAX_TEST_FREQUENCY INT32_MAX
 
 

+ 78 - 9
src/or/rephist.c

@@ -84,12 +84,13 @@
 #include "router.h"
 #include "router.h"
 #include "routerlist.h"
 #include "routerlist.h"
 #include "ht.h"
 #include "ht.h"
+#include "channelpadding.h"
 
 
 #include "channelpadding.h"
 #include "channelpadding.h"
 #include "connection_or.h"
 #include "connection_or.h"
 
 
 static void bw_arrays_init(void);
 static void bw_arrays_init(void);
-static void predicted_ports_init(void);
+static void predicted_ports_alloc(void);
 
 
 /** Total number of bytes currently allocated in fields used by rephist.c. */
 /** Total number of bytes currently allocated in fields used by rephist.c. */
 uint64_t rephist_total_alloc=0;
 uint64_t rephist_total_alloc=0;
@@ -305,7 +306,7 @@ rep_hist_init(void)
 {
 {
   history_map = digestmap_new();
   history_map = digestmap_new();
   bw_arrays_init();
   bw_arrays_init();
-  predicted_ports_init();
+  predicted_ports_alloc();
 }
 }
 
 
 /** Helper: note that we are no longer connected to the router with history
 /** Helper: note that we are no longer connected to the router with history
@@ -1799,6 +1800,36 @@ typedef struct predicted_port_t {
 
 
 /** A list of port numbers that have been used recently. */
 /** A list of port numbers that have been used recently. */
 static smartlist_t *predicted_ports_list=NULL;
 static smartlist_t *predicted_ports_list=NULL;
+/** How long do we keep predicting circuits? */
+static int prediction_timeout=0;
+/** When was the last time we added a prediction entry (HS or port) */
+static time_t last_prediction_add_time=0;
+
+/**
+ * How much time left until we stop predicting circuits?
+ */
+int
+predicted_ports_prediction_time_remaining(time_t now)
+{
+  time_t idle_delta = now - last_prediction_add_time;
+
+  /* Protect against overflow of return value. This can happen if the clock
+   * jumps backwards in time. Update the last prediction time (aka last
+   * active time) to prevent it. This update is preferable to using monotonic
+   * time because it prevents clock jumps into the past from simply causing
+   * very long idle timeouts while the monotonic time stands still. */
+  if (last_prediction_add_time > now) {
+    last_prediction_add_time = now;
+    idle_delta = 0;
+  }
+
+  /* Protect against underflow of the return value. This can happen for very
+   * large periods of inactivity/system sleep. */
+  if (idle_delta > prediction_timeout)
+    return 0;
+
+  return prediction_timeout - idle_delta;
+}
 
 
 /** We just got an application request for a connection with
 /** We just got an application request for a connection with
  * port <b>port</b>. Remember it for the future, so we can keep
  * port <b>port</b>. Remember it for the future, so we can keep
@@ -1808,21 +1839,40 @@ static void
 add_predicted_port(time_t now, uint16_t port)
 add_predicted_port(time_t now, uint16_t port)
 {
 {
   predicted_port_t *pp = tor_malloc(sizeof(predicted_port_t));
   predicted_port_t *pp = tor_malloc(sizeof(predicted_port_t));
+
+  //  If the list is empty, re-randomize predicted ports lifetime
+  if (!any_predicted_circuits(now)) {
+    prediction_timeout = channelpadding_get_circuits_available_timeout();
+  }
+
+  last_prediction_add_time = now;
+
+  log_info(LD_CIRC,
+          "New port prediction added. Will continue predictive circ building "
+          "for %d more seconds.",
+          predicted_ports_prediction_time_remaining(now));
+
   pp->port = port;
   pp->port = port;
   pp->time = now;
   pp->time = now;
   rephist_total_alloc += sizeof(*pp);
   rephist_total_alloc += sizeof(*pp);
   smartlist_add(predicted_ports_list, pp);
   smartlist_add(predicted_ports_list, pp);
 }
 }
 
 
-/** Initialize whatever memory and structs are needed for predicting
+/**
+ * Allocate whatever memory and structs are needed for predicting
  * which ports will be used. Also seed it with port 80, so we'll build
  * which ports will be used. Also seed it with port 80, so we'll build
  * circuits on start-up.
  * circuits on start-up.
  */
  */
 static void
 static void
-predicted_ports_init(void)
+predicted_ports_alloc(void)
 {
 {
   predicted_ports_list = smartlist_new();
   predicted_ports_list = smartlist_new();
-  add_predicted_port(time(NULL), 80); /* add one to kickstart us */
+}
+
+void
+predicted_ports_init(void)
+{
+  add_predicted_port(time(NULL), 443); // Add a port to get us started
 }
 }
 
 
 /** Free whatever memory is needed for predicting which ports will
 /** Free whatever memory is needed for predicting which ports will
@@ -1853,6 +1903,12 @@ rep_hist_note_used_port(time_t now, uint16_t port)
   SMARTLIST_FOREACH_BEGIN(predicted_ports_list, predicted_port_t *, pp) {
   SMARTLIST_FOREACH_BEGIN(predicted_ports_list, predicted_port_t *, pp) {
     if (pp->port == port) {
     if (pp->port == port) {
       pp->time = now;
       pp->time = now;
+
+      last_prediction_add_time = now;
+      log_info(LD_CIRC,
+               "New port prediction added. Will continue predictive circ "
+               "building for %d more seconds.",
+               predicted_ports_prediction_time_remaining(now));
       return;
       return;
     }
     }
   } SMARTLIST_FOREACH_END(pp);
   } SMARTLIST_FOREACH_END(pp);
@@ -1869,8 +1925,8 @@ rep_hist_get_predicted_ports(time_t now)
   int predicted_circs_relevance_time;
   int predicted_circs_relevance_time;
   smartlist_t *out = smartlist_new();
   smartlist_t *out = smartlist_new();
   tor_assert(predicted_ports_list);
   tor_assert(predicted_ports_list);
-  // XXX: Change this if ReducedConnectionPadding is set.
-  predicted_circs_relevance_time = get_options()->PredictedPortsRelevanceTime;
+
+  predicted_circs_relevance_time = prediction_timeout;
 
 
   /* clean out obsolete entries */
   /* clean out obsolete entries */
   SMARTLIST_FOREACH_BEGIN(predicted_ports_list, predicted_port_t *, pp) {
   SMARTLIST_FOREACH_BEGIN(predicted_ports_list, predicted_port_t *, pp) {
@@ -1930,6 +1986,18 @@ static time_t predicted_internal_capacity_time = 0;
 void
 void
 rep_hist_note_used_internal(time_t now, int need_uptime, int need_capacity)
 rep_hist_note_used_internal(time_t now, int need_uptime, int need_capacity)
 {
 {
+  // If the list is empty, re-randomize predicted ports lifetime
+  if (!any_predicted_circuits(now)) {
+    prediction_timeout = channelpadding_get_circuits_available_timeout();
+  }
+
+  last_prediction_add_time = now;
+
+  log_info(LD_CIRC,
+          "New port prediction added. Will continue predictive circ building "
+          "for %d more seconds.",
+          predicted_ports_prediction_time_remaining(now));
+
   predicted_internal_time = now;
   predicted_internal_time = now;
   if (need_uptime)
   if (need_uptime)
     predicted_internal_uptime_time = now;
     predicted_internal_uptime_time = now;
@@ -1943,7 +2011,8 @@ rep_hist_get_predicted_internal(time_t now, int *need_uptime,
                                 int *need_capacity)
                                 int *need_capacity)
 {
 {
   int predicted_circs_relevance_time;
   int predicted_circs_relevance_time;
-  predicted_circs_relevance_time = get_options()->PredictedPortsRelevanceTime;
+
+  predicted_circs_relevance_time = prediction_timeout;
 
 
   if (!predicted_internal_time) { /* initialize it */
   if (!predicted_internal_time) { /* initialize it */
     predicted_internal_time = now;
     predicted_internal_time = now;
@@ -1965,7 +2034,7 @@ int
 any_predicted_circuits(time_t now)
 any_predicted_circuits(time_t now)
 {
 {
   int predicted_circs_relevance_time;
   int predicted_circs_relevance_time;
-  predicted_circs_relevance_time = get_options()->PredictedPortsRelevanceTime;
+  predicted_circs_relevance_time = prediction_timeout;
 
 
   return smartlist_len(predicted_ports_list) ||
   return smartlist_len(predicted_ports_list) ||
          predicted_internal_time + predicted_circs_relevance_time >= now;
          predicted_internal_time + predicted_circs_relevance_time >= now;

+ 2 - 0
src/or/rephist.h

@@ -48,6 +48,7 @@ double rep_hist_get_weighted_fractional_uptime(const char *id, time_t when);
 long rep_hist_get_weighted_time_known(const char *id, time_t when);
 long rep_hist_get_weighted_time_known(const char *id, time_t when);
 int rep_hist_have_measured_enough_stability(void);
 int rep_hist_have_measured_enough_stability(void);
 
 
+void predicted_ports_init(void);
 void rep_hist_note_used_port(time_t now, uint16_t port);
 void rep_hist_note_used_port(time_t now, uint16_t port);
 smartlist_t *rep_hist_get_predicted_ports(time_t now);
 smartlist_t *rep_hist_get_predicted_ports(time_t now);
 void rep_hist_remove_predicted_ports(const smartlist_t *rmv_ports);
 void rep_hist_remove_predicted_ports(const smartlist_t *rmv_ports);
@@ -59,6 +60,7 @@ int rep_hist_get_predicted_internal(time_t now, int *need_uptime,
 
 
 int any_predicted_circuits(time_t now);
 int any_predicted_circuits(time_t now);
 int rep_hist_circbuilding_dormant(time_t now);
 int rep_hist_circbuilding_dormant(time_t now);
+int predicted_ports_prediction_time_remaining(time_t now);
 
 
 void note_crypto_pk_op(pk_op_t operation);
 void note_crypto_pk_op(pk_op_t operation);
 void dump_pk_ops(int severity);
 void dump_pk_ops(int severity);

+ 38 - 0
src/test/test_channelpadding.c

@@ -359,6 +359,7 @@ test_channelpadding_consensus(void *arg)
    *    consensus defaults
    *    consensus defaults
    * 4. Relay-to-relay padding can be enabled/disabled in consensus
    * 4. Relay-to-relay padding can be enabled/disabled in consensus
    * 5. Can enable/disable padding before actually using a connection
    * 5. Can enable/disable padding before actually using a connection
+   * 6. Can we control circ and TLS conn lifetime from the consensus?
    */
    */
   channel_t *chan;
   channel_t *chan;
   routerstatus_t *relay = tor_malloc_zero(sizeof(routerstatus_t));
   routerstatus_t *relay = tor_malloc_zero(sizeof(routerstatus_t));
@@ -491,6 +492,43 @@ test_channelpadding_consensus(void *arg)
   tt_int_op(decision, OP_EQ, CHANNELPADDING_WONTPAD);
   tt_int_op(decision, OP_EQ, CHANNELPADDING_WONTPAD);
   tt_assert(!chan->pending_padding_callback);
   tt_assert(!chan->pending_padding_callback);
 
 
+  /* Test 6: Can we control circ and TLS conn lifetime from the consensus? */
+  val = channelpadding_get_channel_idle_timeout(NULL, 0);
+  tt_int_op(val, OP_GE, 180);
+  tt_int_op(val, OP_LE, 180+90);
+  val = channelpadding_get_channel_idle_timeout(chan, 0);
+  tt_int_op(val, OP_GE, 180);
+  tt_int_op(val, OP_LE, 180+90);
+  options->ReducedConnectionPadding = 1;
+  val = channelpadding_get_channel_idle_timeout(chan, 0);
+  tt_int_op(val, OP_GE, 180/2);
+  tt_int_op(val, OP_LE, (180+90)/2);
+
+  options->ReducedConnectionPadding = 0;
+  options->ORPort_set = 1;
+  smartlist_add(current_md_consensus->net_params,
+                (void*)"nf_conntimeout_relays=600");
+  val = channelpadding_get_channel_idle_timeout(chan, 1);
+  tt_int_op(val, OP_GE, 450);
+  tt_int_op(val, OP_LE, 750);
+
+  val = channelpadding_get_circuits_available_timeout();
+  tt_int_op(val, OP_GE, 30*60);
+  tt_int_op(val, OP_LE, 30*60*2);
+
+  options->ReducedConnectionPadding = 1;
+  smartlist_add(current_md_consensus->net_params,
+                (void*)"nf_conntimeout_clients=600");
+  val = channelpadding_get_circuits_available_timeout();
+  tt_int_op(val, OP_GE, 600/2);
+  tt_int_op(val, OP_LE, 600*2/2);
+
+  options->ReducedConnectionPadding = 0;
+  options->CircuitsAvailableTimeout = 24*60*60;
+  val = channelpadding_get_circuits_available_timeout();
+  tt_int_op(val, OP_GE, 24*60*60);
+  tt_int_op(val, OP_LE, 24*60*60*2);
+
  done:
  done:
   free_fake_channeltls((channel_tls_t*)chan);
   free_fake_channeltls((channel_tls_t*)chan);
   smartlist_free(connection_array);
   smartlist_free(connection_array);

+ 3 - 0
src/test/testing_common.c

@@ -304,10 +304,13 @@ main(int c, const char **v)
     tor_free(errmsg);
     tor_free(errmsg);
     return 1;
     return 1;
   }
   }
+
   tor_set_failed_assertion_callback(an_assertion_failed);
   tor_set_failed_assertion_callback(an_assertion_failed);
 
 
   init_pregenerated_keys();
   init_pregenerated_keys();
 
 
+  predicted_ports_init();
+
   atexit(remove_directory);
   atexit(remove_directory);
 
 
   int have_failed = (tinytest_main(c, v, testgroups) != 0);
   int have_failed = (tinytest_main(c, v, testgroups) != 0);