[tor-commits] [tor/master] Bug 17592: Clean up connection timeout logic.

nickm at torproject.org nickm at torproject.org
Mon May 8 18:00:18 UTC 2017


commit d5a151a06788c28ac1c50398c6e571d484774f47
Author: Mike Perry <mikeperry-git at torproject.org>
Date:   Tue Feb 21 21:28:00 2017 -0500

    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
---
 changes/bug17592               |  13 +++++
 doc/tor.1.txt                  |  20 +++++---
 src/or/channelpadding.c        | 105 +++++++++++++++++++++++++++++++++++++++++
 src/or/circuitlist.c           |  41 ++++++++++++++++
 src/or/circuituse.c            |  25 ++++------
 src/or/config.c                |  27 ++++++-----
 src/or/connection_or.c         |  30 ++++--------
 src/or/main.c                  |   8 +++-
 src/or/or.h                    |  11 ++++-
 src/or/rephist.c               |  87 ++++++++++++++++++++++++++++++----
 src/or/rephist.h               |   2 +
 src/test/test_channelpadding.c |  38 +++++++++++++++
 src/test/testing_common.c      |   3 ++
 13 files changed, 339 insertions(+), 71 deletions(-)

diff --git a/changes/bug17592 b/changes/bug17592
new file mode 100644
index 0000000..4b5f22c
--- /dev/null
+++ b/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.
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index eb4e02a..109efa7 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -666,8 +666,8 @@ GENERAL OPTIONS
 [[PredictedPortsRelevanceTime]] **PredictedPortsRelevanceTime** __NUM__::
     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
-    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**::
     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.
     (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__::
     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__::
     If non-zero, this option overrides our internal timeout schedule for how
diff --git a/src/or/channelpadding.c b/src/or/channelpadding.c
index 3976424..3156051 100644
--- a/src/or/channelpadding.c
+++ b/src/or/channelpadding.c
@@ -447,6 +447,111 @@ channelpadding_compute_time_until_pad_for_netflow(channel_t *chan)
 }
 
 /**
+ * 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
  * not to send padding, and disables sending padding from this side as well.
  */
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 54a7db9..1a3b7bb 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -78,6 +78,7 @@
 #include "rephist.h"
 #include "routerlist.h"
 #include "routerset.h"
+#include "channelpadding.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;
 }
 
+/** 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>
  * 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());
 
+  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;
 }
 
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index c2b4506..d0bc7d3 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -1383,11 +1383,6 @@ circuit_detach_stream(circuit_t *circ, edge_connection_t *conn)
   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
  * 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;
 
   tor_gettimeofday(&now);
-  cutoff = now;
   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) {
     if (circ->marked_for_close || !CIRCUIT_IS_ORIGIN(circ))
       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
      * 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_REND_READY_INTRO_ACKED) ||
                 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));
           circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
         } else if (!TO_ORIGIN_CIRCUIT(circ)->is_ancient) {
diff --git a/src/or/config.c b/src/or/config.c
index d7489c0..dcf6717 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -245,7 +245,8 @@ static config_var_t option_vars_[] = {
   V(PaddingStatistics,           BOOL,     "1"),
   V(LearnCircuitBuildTimeout,    BOOL,     "1"),
   V(CircuitBuildTimeout,         INTERVAL, "0"),
-  V(CircuitIdleTimeout,          INTERVAL, "1 hour"),
+  OBSOLETE("CircuitIdleTimeout"),
+  V(CircuitsAvailableTimeout,    INTERVAL, "0"),
   V(CircuitStreamTimeout,        INTERVAL, "0"),
   V(CircuitPriorityHalflife,     DOUBLE,  "-100.0"), /*negative:'Use default'*/
   V(ClientDNSRejectInternalAddresses, BOOL,"1"),
@@ -402,7 +403,7 @@ static config_var_t option_vars_[] = {
   V(NATDListenAddress,           LINELIST, NULL),
   VPORT(NATDPort),
   V(Nickname,                    STRING,   NULL),
-  V(PredictedPortsRelevanceTime,  INTERVAL, "1 hour"),
+  OBSOLETE("PredictedPortsRelevanceTime"),
   V(WarnUnsafeSocks,              BOOL,     "1"),
   VAR("NodeFamily",              LINELIST, NodeFamilies,         NULL),
   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_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. */
 #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;
   }
 
-  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 */
   if (options_validate_single_onion(options, msg) < 0)
     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
   if (options->Tor2webMode && options->UseEntryGuards) {
     /* tor2web mode clients do not (and should not) use entry guards
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 40c28e6..5d32217 100644
--- a/src/or/connection_or.c
+++ b/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
  * non-canonical otherwise. Adjust idle_timeout accordingly.
  */
@@ -840,9 +822,6 @@ void
 connection_or_set_canonical(or_connection_t *or_conn,
                             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) &&
       or_conn->idle_timeout != 0) {
     /* 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->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
diff --git a/src/or/main.c b/src/or/main.c
index ef4a1ff..e124441 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -1096,8 +1096,9 @@ run_connection_housekeeping(int i, time_t now)
   } else if (!have_any_circuits &&
              now - or_conn->idle_timeout >=
                                          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)(now - chan->timestamp_last_had_circuits),
              or_conn->idle_timeout,
@@ -3019,6 +3020,9 @@ tor_init(int argc, char *argv[])
   /* The options are now initialised */
   const or_options_t *options = get_options();
 
+  /* Initialize predicted ports list after loading options */
+  predicted_ports_init();
+
 #ifndef _WIN32
   if (geteuid()==0)
     log_warn(LD_GENERAL,"You are running Tor as root. You don't need to, "
diff --git a/src/or/or.h b/src/or/or.h
index 02c43a5..cd00fbb 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3301,6 +3301,13 @@ typedef struct origin_circuit_t {
    * adjust_exit_policy_from_exitpolicy_failure.
    */
   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;
 
 struct onion_queue_t;
@@ -3882,6 +3889,8 @@ typedef struct {
                             * adaptive algorithm learns a new value. */
   int CircuitIdleTimeout; /**< Cull open clean circuits that were born
                            * 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
                              * and try a new circuit if the stream has been
                              * 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);
 
 /** 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_MAX_TEST_FREQUENCY INT32_MAX
 
diff --git a/src/or/rephist.c b/src/or/rephist.c
index a87ae9e..aade1a0 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -84,12 +84,13 @@
 #include "router.h"
 #include "routerlist.h"
 #include "ht.h"
+#include "channelpadding.h"
 
 #include "channelpadding.h"
 #include "connection_or.h"
 
 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. */
 uint64_t rephist_total_alloc=0;
@@ -305,7 +306,7 @@ rep_hist_init(void)
 {
   history_map = digestmap_new();
   bw_arrays_init();
-  predicted_ports_init();
+  predicted_ports_alloc();
 }
 
 /** 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. */
 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
  * 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)
 {
   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->time = now;
   rephist_total_alloc += sizeof(*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
  * circuits on start-up.
  */
 static void
-predicted_ports_init(void)
+predicted_ports_alloc(void)
 {
   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
@@ -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) {
     if (pp->port == port) {
       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;
     }
   } SMARTLIST_FOREACH_END(pp);
@@ -1869,8 +1925,8 @@ rep_hist_get_predicted_ports(time_t now)
   int predicted_circs_relevance_time;
   smartlist_t *out = smartlist_new();
   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 */
   SMARTLIST_FOREACH_BEGIN(predicted_ports_list, predicted_port_t *, pp) {
@@ -1930,6 +1986,18 @@ static time_t predicted_internal_capacity_time = 0;
 void
 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;
   if (need_uptime)
     predicted_internal_uptime_time = now;
@@ -1943,7 +2011,8 @@ rep_hist_get_predicted_internal(time_t now, int *need_uptime,
                                 int *need_capacity)
 {
   int predicted_circs_relevance_time;
-  predicted_circs_relevance_time = get_options()->PredictedPortsRelevanceTime;
+
+  predicted_circs_relevance_time = prediction_timeout;
 
   if (!predicted_internal_time) { /* initialize it */
     predicted_internal_time = now;
@@ -1965,7 +2034,7 @@ int
 any_predicted_circuits(time_t now)
 {
   int predicted_circs_relevance_time;
-  predicted_circs_relevance_time = get_options()->PredictedPortsRelevanceTime;
+  predicted_circs_relevance_time = prediction_timeout;
 
   return smartlist_len(predicted_ports_list) ||
          predicted_internal_time + predicted_circs_relevance_time >= now;
diff --git a/src/or/rephist.h b/src/or/rephist.h
index 4d95bff..5a1ad82 100644
--- a/src/or/rephist.h
+++ b/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);
 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);
 smartlist_t *rep_hist_get_predicted_ports(time_t now);
 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 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 dump_pk_ops(int severity);
diff --git a/src/test/test_channelpadding.c b/src/test/test_channelpadding.c
index b6591ea..de88bb2 100644
--- a/src/test/test_channelpadding.c
+++ b/src/test/test_channelpadding.c
@@ -359,6 +359,7 @@ test_channelpadding_consensus(void *arg)
    *    consensus defaults
    * 4. Relay-to-relay padding can be enabled/disabled in consensus
    * 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;
   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_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:
   free_fake_channeltls((channel_tls_t*)chan);
   smartlist_free(connection_array);
diff --git a/src/test/testing_common.c b/src/test/testing_common.c
index caeae13..43cf0aa 100644
--- a/src/test/testing_common.c
+++ b/src/test/testing_common.c
@@ -304,10 +304,13 @@ main(int c, const char **v)
     tor_free(errmsg);
     return 1;
   }
+
   tor_set_failed_assertion_callback(an_assertion_failed);
 
   init_pregenerated_keys();
 
+  predicted_ports_init();
+
   atexit(remove_directory);
 
   int have_failed = (tinytest_main(c, v, testgroups) != 0);





More information about the tor-commits mailing list