Bug 23101: Pre-build HS-specific circuits (instead of general).

Prebuilt circs are 4 hops, since only server side HSDIR and intro circs
are 3 hops, and it is OK if those sometimes take longer to build.

changes/bug23101

@@ -0,0 +1,3 @@
+  o Minor features (Performance):
+    - Support predictive circuit building for onion service circuits with
+      multiple layers of guards. Closes ticket 23101.

src/or/circuitbuild.c

@@ -80,6 +80,10 @@ static int circuit_send_first_onion_skin(origin_circuit_t *circ);
 static int circuit_build_no_more_hops(origin_circuit_t *circ);
 static int circuit_send_intermediate_onion_skin(origin_circuit_t *circ,
                                                 crypt_path_t *hop);
+static const node_t *choose_good_middle_server(uint8_t purpose,
+                          cpath_build_state_t *state,
+                          crypt_path_t *head,
+                          int cur_len);
 
 /** This function tries to get a channel to the specified endpoint,
  * and then calls command_setup_channel() to give it the right
@@ -1653,7 +1657,7 @@ onionskin_answer(or_circuit_t *circ,
  * new_route_len()) in the one-hop tunnel case, so we don't need to
  * handle that.
  */
-static int
+int
 route_len_for_purpose(uint8_t purpose, extend_info_t *exit_ei)
 {
   int routelen = DEFAULT_ROUTE_LEN;
@@ -1670,6 +1674,7 @@ route_len_for_purpose(uint8_t purpose, extend_info_t *exit_ei)
      */
     if (purpose == CIRCUIT_PURPOSE_C_ESTABLISH_REND ||
         purpose == CIRCUIT_PURPOSE_S_HSDIR_POST ||
+        purpose == CIRCUIT_PURPOSE_HS_VANGUARDS ||
         purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO)
       return routelen+1;
 
@@ -2264,9 +2269,8 @@ pick_restricted_middle_node(router_crn_flags_t flags,
  * toward the preferences in 'options'.
  */
 static const node_t *
-choose_good_exit_server(uint8_t purpose,
-                        int need_uptime, int need_capacity, int is_internal,
-                        int need_hs_v3)
+choose_good_exit_server(origin_circuit_t *circ, int need_uptime,
+                        int need_capacity, int is_internal, int need_hs_v3)
 {
   const or_options_t *options = get_options();
   router_crn_flags_t flags = CRN_NEED_DESC;
@@ -2277,9 +2281,13 @@ choose_good_exit_server(uint8_t purpose,
   if (need_hs_v3)
     flags |= CRN_RENDEZVOUS_V3;
 
-  switch (purpose) {
+  switch (TO_CIRCUIT(circ)->purpose) {
     case CIRCUIT_PURPOSE_C_HSDIR_GET:
     case CIRCUIT_PURPOSE_S_HSDIR_POST:
+    case CIRCUIT_PURPOSE_HS_VANGUARDS:
+      /* For these three, we want to pick the exit like a middle hop,
+       * since it should be random. */
+      tor_assert_nonfatal(is_internal);
     case CIRCUIT_PURPOSE_C_GENERAL:
       if (is_internal) /* pick it like a middle hop */
         return router_choose_random_node(NULL, options->ExcludeNodes, flags);
@@ -2294,7 +2302,7 @@ choose_good_exit_server(uint8_t purpose,
         return rendezvous_node;
       }
   }
-  log_warn(LD_BUG,"Unhandled purpose %d", purpose);
+  log_warn(LD_BUG,"Unhandled purpose %d", TO_CIRCUIT(circ)->purpose);
   tor_fragile_assert();
   return NULL;
 }
@@ -2410,7 +2418,7 @@ onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit_ei,
     exit_ei = extend_info_dup(exit_ei);
   } else { /* we have to decide one */
     const node_t *node =
-      choose_good_exit_server(circ->base_.purpose, state->need_uptime,
+      choose_good_exit_server(circ, state->need_uptime,
                               state->need_capacity, state->is_internal,
                               is_hs_v3_rp_circuit);
     if (!node) {

src/or/circuitbuild.h

@@ -12,6 +12,7 @@
 #ifndef TOR_CIRCUITBUILD_H
 #define TOR_CIRCUITBUILD_H
 
+int route_len_for_purpose(uint8_t purpose, extend_info_t *exit_ei);
 char *circuit_list_path(origin_circuit_t *circ, int verbose);
 char *circuit_list_path_for_controller(origin_circuit_t *circ);
 void circuit_log_path(int severity, unsigned int domain,

src/or/circuitlist.c

@@ -723,6 +723,8 @@ circuit_purpose_to_controller_string(uint8_t purpose)
       return "CONTROLLER";
     case CIRCUIT_PURPOSE_PATH_BIAS_TESTING:
       return "PATH_BIAS_TESTING";
+    case CIRCUIT_PURPOSE_HS_VANGUARDS:
+      return "HS_VANGUARDS";
 
     default:
       tor_snprintf(buf, sizeof(buf), "UNKNOWN_%d", (int)purpose);
@@ -751,6 +753,7 @@ circuit_purpose_to_controller_hs_state_string(uint8_t purpose)
     case CIRCUIT_PURPOSE_TESTING:
     case CIRCUIT_PURPOSE_CONTROLLER:
     case CIRCUIT_PURPOSE_PATH_BIAS_TESTING:
+    case CIRCUIT_PURPOSE_HS_VANGUARDS:
       return NULL;
 
     case CIRCUIT_PURPOSE_INTRO_POINT:
@@ -848,6 +851,9 @@ circuit_purpose_to_string(uint8_t purpose)
     case CIRCUIT_PURPOSE_PATH_BIAS_TESTING:
       return "Path-bias testing circuit";
 
+    case CIRCUIT_PURPOSE_HS_VANGUARDS:
+      return "Hidden service: Pre-built vanguard circuit";
+
     default:
       tor_snprintf(buf, sizeof(buf), "UNKNOWN_%d", (int)purpose);
       return buf;
@@ -1719,14 +1725,29 @@ circuit_can_be_cannibalized_for_v3_rp(const origin_circuit_t *circ)
   return 0;
 }
 
+/** We are trying to create a circuit of purpose <b>purpose</b> and we are
+ *  looking for cannibalizable circuits. Return the circuit purpose we would be
+ *  willing to cannibalize. */
+static uint8_t
+get_circuit_purpose_needed_to_cannibalize(uint8_t purpose)
+{
+  if (circuit_should_use_vanguards(purpose)) {
+    /* If we are using vanguards, then we should only cannibalize vanguard
+     * circuits so that we get the same path construction logic. */
+    return CIRCUIT_PURPOSE_HS_VANGUARDS;
+  } else {
+    /* If no vanguards are used just get a general circuit! */
+    return CIRCUIT_PURPOSE_C_GENERAL;
+  }
+}
+
 /** Return a circuit that is open, is CIRCUIT_PURPOSE_C_GENERAL,
  * has a timestamp_dirty value of 0, has flags matching the CIRCLAUNCH_*
  * flags in <b>flags</b>, and if info is defined, does not already use info
  * as any of its hops; or NULL if no circuit fits this description.
  *
- * The <b>purpose</b> argument (currently ignored) refers to the purpose of
- * the circuit we want to create, not the purpose of the circuit we want to
- * cannibalize.
+ * The <b>purpose</b> argument refers to the purpose of the circuit we want to
+ * create, not the purpose of the circuit we want to cannibalize.
  *
  * If !CIRCLAUNCH_NEED_UPTIME, prefer returning non-uptime circuits.
  *
@@ -1739,7 +1760,7 @@ circuit_can_be_cannibalized_for_v3_rp(const origin_circuit_t *circ)
  * a new circuit.)
  */
 origin_circuit_t *
-circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
+circuit_find_to_cannibalize(uint8_t purpose_to_produce, extend_info_t *info,
                             int flags)
 {
   origin_circuit_t *best=NULL;
@@ -1747,29 +1768,46 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
   int need_capacity = (flags & CIRCLAUNCH_NEED_CAPACITY) != 0;
   int internal = (flags & CIRCLAUNCH_IS_INTERNAL) != 0;
   const or_options_t *options = get_options();
+  /* We want the circuit we are trying to cannibalize to have this purpose */
+  int purpose_to_search_for;
 
   /* Make sure we're not trying to create a onehop circ by
    * cannibalization. */
   tor_assert(!(flags & CIRCLAUNCH_ONEHOP_TUNNEL));
 
+  purpose_to_search_for = get_circuit_purpose_needed_to_cannibalize(
+                                                  purpose_to_produce);
+
+  tor_assert_nonfatal(purpose_to_search_for == CIRCUIT_PURPOSE_C_GENERAL ||
+                      purpose_to_search_for == CIRCUIT_PURPOSE_HS_VANGUARDS);
+
   log_debug(LD_CIRC,
             "Hunting for a circ to cannibalize: purpose %d, uptime %d, "
             "capacity %d, internal %d",
-            purpose, need_uptime, need_capacity, internal);
+            purpose_to_produce, need_uptime, need_capacity, internal);
 
   SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ_) {
     if (CIRCUIT_IS_ORIGIN(circ_) &&
         circ_->state == CIRCUIT_STATE_OPEN &&
         !circ_->marked_for_close &&
-        circ_->purpose == CIRCUIT_PURPOSE_C_GENERAL &&
+        circ_->purpose == purpose_to_search_for &&
         !circ_->timestamp_dirty) {
       origin_circuit_t *circ = TO_ORIGIN_CIRCUIT(circ_);
+
+      /* Only cannibalize from reasonable length circuits. If we
+       * want C_GENERAL, then only choose 3 hop circs. If we want
+       * HS_VANGUARDS, only choose 4 hop circs.
+       */
+      if (circ->build_state->desired_path_len !=
+          route_len_for_purpose(purpose_to_search_for, NULL)) {
+        goto next;
+      }
+
       if ((!need_uptime || circ->build_state->need_uptime) &&
           (!need_capacity || circ->build_state->need_capacity) &&
           (internal == circ->build_state->is_internal) &&
           !circ->unusable_for_new_conns &&
           circ->remaining_relay_early_cells &&
-          circ->build_state->desired_path_len == DEFAULT_ROUTE_LEN &&
           !circ->build_state->onehop_tunnel &&
           !circ->isolation_values_set) {
         if (info) {

src/or/circuituse.c

@@ -54,6 +54,7 @@
 #include "rephist.h"
 #include "router.h"
 #include "routerlist.h"
+#include "config.h"
 
 static void circuit_expire_old_circuits_clientside(void);
 static void circuit_increment_failure_count(void);
@@ -133,6 +134,7 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ,
   }
 
   if (purpose == CIRCUIT_PURPOSE_C_GENERAL ||
+      purpose == CIRCUIT_PURPOSE_HS_VANGUARDS ||
       purpose == CIRCUIT_PURPOSE_C_REND_JOINED) {
     if (circ->timestamp_dirty &&
        circ->timestamp_dirty+get_options()->MaxCircuitDirtiness <= now)
@@ -327,6 +329,7 @@ circuit_get_best(const entry_connection_t *conn,
   tor_assert(conn);
 
   tor_assert(purpose == CIRCUIT_PURPOSE_C_GENERAL ||
+             purpose == CIRCUIT_PURPOSE_HS_VANGUARDS ||
              purpose == CIRCUIT_PURPOSE_C_HSDIR_GET ||
              purpose == CIRCUIT_PURPOSE_S_HSDIR_POST ||
              purpose == CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT ||
@@ -1086,7 +1089,8 @@ circuit_is_available_for_use(const circuit_t *circ)
     return 0; /* Don't mess with marked circs */
   if (circ->timestamp_dirty)
     return 0; /* Only count clean circs */
-  if (circ->purpose != CIRCUIT_PURPOSE_C_GENERAL)
+  if (circ->purpose != CIRCUIT_PURPOSE_C_GENERAL &&
+      circ->purpose != CIRCUIT_PURPOSE_HS_VANGUARDS)
     return 0; /* We only pay attention to general purpose circuits.
                  General purpose circuits are always origin circuits. */
 
@@ -1198,6 +1202,25 @@ needs_circuits_for_build(int num)
   return 0;
 }
 
+/**
+ * Launch the appropriate type of predicted circuit for hidden
+ * services, depending on our options.
+ */
+static void
+circuit_launch_predicted_hs_circ(int flags)
+{
+  /* K.I.S.S. implementation of bug #23101: If we are using
+   * vanguards or pinned middles, pre-build a specific purpose
+   * for HS circs. */
+  if (circuit_should_use_vanguards(CIRCUIT_PURPOSE_HS_VANGUARDS)) {
+    circuit_launch(CIRCUIT_PURPOSE_HS_VANGUARDS, flags);
+  } else {
+    /* If no vanguards, then no HS-specific prebuilt circuits are needed.
+     * Normal GENERAL circs are fine */
+    circuit_launch(CIRCUIT_PURPOSE_C_GENERAL, flags);
+  }
+}
+
 /** Determine how many circuits we have open that are clean,
  * Make sure it's enough for all the upcoming behaviors we predict we'll have.
  * But put an upper bound on the total number of circuits.
@@ -1251,7 +1274,7 @@ circuit_predict_and_launch_new(void)
              "Have %d clean circs (%d internal), need another internal "
              "circ for my hidden service.",
              num, num_internal);
-    circuit_launch(CIRCUIT_PURPOSE_C_GENERAL, flags);
+    circuit_launch_predicted_hs_circ(flags);
     return;
   }
 
@@ -1269,7 +1292,8 @@ circuit_predict_and_launch_new(void)
              "Have %d clean circs (%d uptime-internal, %d internal), need"
              " another hidden service circ.",
              num, num_uptime_internal, num_internal);
-    circuit_launch(CIRCUIT_PURPOSE_C_GENERAL, flags);
+
+    circuit_launch_predicted_hs_circ(flags);
     return;
   }
 
@@ -1466,6 +1490,7 @@ circuit_expire_old_circuits_clientside(void)
         if (circ->purpose == CIRCUIT_PURPOSE_C_GENERAL ||
                 circ->purpose == CIRCUIT_PURPOSE_C_HSDIR_GET ||
                 circ->purpose == CIRCUIT_PURPOSE_S_HSDIR_POST ||
+                circ->purpose == CIRCUIT_PURPOSE_HS_VANGUARDS ||
                 circ->purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT ||
                 circ->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO ||
                 circ->purpose == CIRCUIT_PURPOSE_TESTING ||
@@ -1737,6 +1762,8 @@ circuit_build_failed(origin_circuit_t *circ)
       circ->cpath->prev->prev->state == CPATH_STATE_OPEN) {
     failed_at_last_hop = 1;
   }
+
+  /* Check if we failed at first hop */
   if (circ->cpath &&
       circ->cpath->state != CPATH_STATE_OPEN &&
       ! circ->base_.received_destroy) {
@@ -1888,6 +1915,10 @@ have_enough_path_info(int need_exit)
 int
 circuit_purpose_is_hidden_service(uint8_t purpose)
 {
+   if (purpose == CIRCUIT_PURPOSE_HS_VANGUARDS) {
+     return 1;
+   }
+
    /* Client-side purpose */
    if (purpose >= CIRCUIT_PURPOSE_C_HS_MIN_ &&
        purpose <= CIRCUIT_PURPOSE_C_HS_MAX_) {
@@ -1929,6 +1960,55 @@ circuit_should_use_vanguards(uint8_t purpose)
   return 0;
 }
 
+/**
+ * Return true for the set of conditions for which it is OK to use
+ * a cannibalized circuit.
+ *
+ * Don't cannibalize for onehops, or tor2web, or certain purposes.
+ */
+static int
+circuit_should_cannibalize_to_build(uint8_t purpose_to_build,
+                                    int has_extend_info,
+                                    int onehop_tunnel,
+                                    int need_specific_rp)
+{
+
+  /* Do not try to cannibalize if this is a one hop circuit, or
+   * is a tor2web/special rp. */
+  if (onehop_tunnel || need_specific_rp) {
+    return 0;
+  }
+
+  /* Don't try to cannibalize for general purpose circuits that do not
+   * specify a custom exit. */
+  if (purpose_to_build == CIRCUIT_PURPOSE_C_GENERAL && !has_extend_info) {
+    return 0;
+  }
+
+  /* Don't cannibalize for testing circuits. We want to see if they
+   * complete normally. Also don't cannibalize for vanguard-purpose
+   * circuits, since those are specially pre-built for later
+   * cannibalization by the actual specific circuit types that need
+   * vanguards.
+   */
+  if (purpose_to_build == CIRCUIT_PURPOSE_TESTING ||
+      purpose_to_build == CIRCUIT_PURPOSE_HS_VANGUARDS) {
+    return 0;
+  }
+
+  /* For vanguards, the server-side intro circ is not cannibalized
+   * because we pre-build 4 hop HS circuits, and it only needs a 3 hop
+   * circuit. It is also long-lived, so it is more important that
+   * it have lower latency than get built fast.
+   */
+  if (circuit_should_use_vanguards(purpose_to_build) &&
+      purpose_to_build == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO) {
+    return 0;
+  }
+
+  return 1;
+}
+
 /** Launch a new circuit with purpose <b>purpose</b> and exit node
  * <b>extend_info</b> (or NULL to select a random exit node).  If flags
  * contains CIRCLAUNCH_NEED_UPTIME, choose among routers with high uptime.  If
@@ -1963,10 +2043,12 @@ circuit_launch_by_extend_info(uint8_t purpose,
     need_specific_rp = 1;
   }
 
-  if ((extend_info || purpose != CIRCUIT_PURPOSE_C_GENERAL) &&
-      purpose != CIRCUIT_PURPOSE_TESTING &&
-      !circuit_should_use_vanguards(purpose) &&
-      !onehop_tunnel && !need_specific_rp) {
+  /* If we can/should cannibalize another circuit to build this one,
+   * then do so. */
+  if (circuit_should_cannibalize_to_build(purpose,
+                                          extend_info != NULL,
+                                          onehop_tunnel,
+                                          need_specific_rp)) {
     /* see if there are appropriate circs available to cannibalize. */
     /* XXX if we're planning to add a hop, perhaps we want to look for
      * internal circs rather than exit circs? -RD */

src/or/or.h

@@ -547,13 +547,23 @@ typedef enum {
 /** This circuit is used for uploading hsdirs */
 #define CIRCUIT_PURPOSE_S_HSDIR_POST 19
 #define CIRCUIT_PURPOSE_S_HS_MAX_ 19
+
 /** A testing circuit; not meant to be used for actual traffic. */
 #define CIRCUIT_PURPOSE_TESTING 20
 /** A controller made this circuit and Tor should not use it. */
 #define CIRCUIT_PURPOSE_CONTROLLER 21
 /** This circuit is used for path bias probing only */
 #define CIRCUIT_PURPOSE_PATH_BIAS_TESTING 22
-#define CIRCUIT_PURPOSE_MAX_ 22
+
+/** This circuit is used for vanguards/restricted paths.
+ *
+ *  This type of circuit is *only* created preemptively and never
+ *  on-demand. When an HS operation needs to take place (e.g. connect to an
+ *  intro point), these circuits are then cannibalized and repurposed to the
+ *  actual needed HS purpose. */
+#define CIRCUIT_PURPOSE_HS_VANGUARDS 23
+
+#define CIRCUIT_PURPOSE_MAX_ 23
 /** A catch-all for unrecognized purposes. Currently we don't expect
  * to make or see any circuits with this purpose. */
 #define CIRCUIT_PURPOSE_UNKNOWN 255