Merge remote-tracking branch 'tor-github/pr/1888/head'

src/core/or/circuitbuild.c

@@ -439,7 +439,8 @@ onion_populate_cpath(origin_circuit_t *circ)
 
 /** Create and return a new origin circuit. Initialize its purpose and
  * build-state based on our arguments.  The <b>flags</b> argument is a
- * bitfield of CIRCLAUNCH_* flags. */
+ * bitfield of CIRCLAUNCH_* flags, see circuit_launch_by_extend_info() for
+ * more details. */
 origin_circuit_t *
 origin_circuit_init(uint8_t purpose, int flags)
 {
@@ -455,13 +456,16 @@ origin_circuit_init(uint8_t purpose, int flags)
     ((flags & CIRCLAUNCH_NEED_CAPACITY) ? 1 : 0);
   circ->build_state->is_internal =
     ((flags & CIRCLAUNCH_IS_INTERNAL) ? 1 : 0);
+  circ->build_state->is_ipv6_selftest =
+    ((flags & CIRCLAUNCH_IS_IPV6_SELFTEST) ? 1 : 0);
   circ->base_.purpose = purpose;
   return circ;
 }
 
-/** Build a new circuit for <b>purpose</b>. If <b>exit</b>
- * is defined, then use that as your exit router, else choose a suitable
- * exit node.
+/** Build a new circuit for <b>purpose</b>. If <b>exit</b> is defined, then use
+ * that as your exit router, else choose a suitable exit node. The <b>flags</b>
+ * argument is a bitfield of CIRCLAUNCH_* flags, see
+ * circuit_launch_by_extend_info() for more details.
  *
  * Also launch a connection to the first OR in the chosen path, if
  * it's not open already.
@@ -1050,7 +1054,8 @@ circuit_build_no_more_hops(origin_circuit_t *circ)
     control_event_bootstrap(BOOTSTRAP_STATUS_DONE, 0);
     control_event_client_status(LOG_NOTICE, "CIRCUIT_ESTABLISHED");
     clear_broken_connection_map(1);
-    if (server_mode(options) && !check_whether_orport_reachable(options)) {
+    if (server_mode(options) &&
+        !router_should_skip_orport_reachability_check(options)) {
       inform_testing_reachability();
       router_do_reachability_checks(1, 1);
     }
@@ -1074,14 +1079,25 @@ circuit_send_intermediate_onion_skin(origin_circuit_t *circ,
                                      crypt_path_t *hop)
 {
   int len;
+  int family = tor_addr_family(&hop->extend_info->addr);
   extend_cell_t ec;
   memset(&ec, 0, sizeof(ec));
 
   log_debug(LD_CIRC,"starting to send subsequent skin.");
 
-  if (tor_addr_family(&hop->extend_info->addr) != AF_INET) {
-    log_warn(LD_BUG, "Trying to extend to a non-IPv4 address.");
-    return - END_CIRC_REASON_INTERNAL;
+  /* Relays and bridges can send IPv6 extends. But for clients, it's an
+   * obvious version distinguisher. */
+  if (server_mode(get_options())) {
+    if (family != AF_INET && family != AF_INET6) {
+      log_warn(LD_BUG, "Server trying to extend to an invalid address "
+               "family.");
+      return - END_CIRC_REASON_INTERNAL;
+    }
+  } else {
+    if (family != AF_INET) {
+      log_warn(LD_BUG, "Client trying to extend to a non-IPv4 address.");
+      return - END_CIRC_REASON_INTERNAL;
+    }
   }
 
   circuit_pick_extend_handshake(&ec.cell_type,
@@ -1089,9 +1105,17 @@ circuit_send_intermediate_onion_skin(origin_circuit_t *circ,
                                 &ec.create_cell.handshake_type,
                                 hop->extend_info);
 
-  tor_addr_copy(&ec.orport_ipv4.addr, &hop->extend_info->addr);
-  ec.orport_ipv4.port = hop->extend_info->port;
-  tor_addr_make_unspec(&ec.orport_ipv6.addr);
+  /* At the moment, extend_info only has one ORPort address. We'll add a
+   * second address in #34069, to support dual-stack extend cells. */
+  if (family == AF_INET) {
+    tor_addr_copy(&ec.orport_ipv4.addr, &hop->extend_info->addr);
+    ec.orport_ipv4.port = hop->extend_info->port;
+    tor_addr_make_unspec(&ec.orport_ipv6.addr);
+  } else {
+    tor_addr_copy(&ec.orport_ipv6.addr, &hop->extend_info->addr);
+    ec.orport_ipv6.port = hop->extend_info->port;
+    tor_addr_make_unspec(&ec.orport_ipv4.addr);
+  }
   memcpy(ec.node_id, hop->extend_info->identity_digest, DIGEST_LEN);
   /* Set the ED25519 identity too -- it will only get included
    * in the extend2 cell if we're configured to use it, though. */
@@ -1539,7 +1563,23 @@ choose_good_exit_server_general(router_crn_flags_t flags)
   const node_t *selected_node=NULL;
   const int need_uptime = (flags & CRN_NEED_UPTIME) != 0;
   const int need_capacity = (flags & CRN_NEED_CAPACITY) != 0;
-  const int direct_conn = (flags & CRN_DIRECT_CONN) != 0;
+
+  /* We should not require guard flags on exits. */
+  IF_BUG_ONCE(flags & CRN_NEED_GUARD)
+    return NULL;
+
+  /* We reject single-hop exits for all node positions. */
+  IF_BUG_ONCE(flags & CRN_DIRECT_CONN)
+    return NULL;
+
+  /* This isn't the function for picking rendezvous nodes. */
+  IF_BUG_ONCE(flags & CRN_RENDEZVOUS_V3)
+    return NULL;
+
+  /* We only want exits to extend if we cannibalize the circuit.
+   * But we don't require IPv6 extends yet. */
+  IF_BUG_ONCE(flags & CRN_INITIATE_IPV6_EXTEND)
+    return NULL;
 
   connections = get_connection_array();
 
@@ -1572,19 +1612,14 @@ choose_good_exit_server_general(router_crn_flags_t flags)
        */
       continue;
     }
-    if (!node_has_preferred_descriptor(node, direct_conn)) {
+    if (!router_can_choose_node(node, flags)) {
       n_supported[i] = -1;
       continue;
     }
-    if (!node->is_running || node->is_bad_exit) {
+    if (node->is_bad_exit) {
       n_supported[i] = -1;
       continue; /* skip routers that are known to be down or bad exits */
     }
-    if (node_get_purpose(node) != ROUTER_PURPOSE_GENERAL) {
-      /* never pick a non-general node as a random exit. */
-      n_supported[i] = -1;
-      continue;
-    }
     if (routerset_contains_node(options->ExcludeExitNodesUnion_, node)) {
       n_supported[i] = -1;
       continue; /* user asked us not to use it, no matter what */
@@ -1594,27 +1629,6 @@ choose_good_exit_server_general(router_crn_flags_t flags)
       n_supported[i] = -1;
       continue; /* not one of our chosen exit nodes */
     }
-
-    if (node_is_unreliable(node, need_uptime, need_capacity, 0)) {
-      n_supported[i] = -1;
-      continue; /* skip routers that are not suitable.  Don't worry if
-                 * this makes us reject all the possible routers: if so,
-                 * we'll retry later in this function with need_update and
-                 * need_capacity set to 0. */
-    }
-    if (!(node->is_valid)) {
-      /* if it's invalid and we don't want it */
-      n_supported[i] = -1;
-//      log_fn(LOG_DEBUG,"Skipping node %s (index %d) -- invalid router.",
-//             router->nickname, i);
-      continue; /* skip invalid routers */
-    }
-    /* We do not allow relays that allow single hop exits by default. Option
-     * was deprecated in 0.2.9.2-alpha and removed in 0.3.1.0-alpha. */
-    if (node_allows_single_hop_exits(node)) {
-      n_supported[i] = -1;
-      continue;
-    }
     if (node_exit_policy_rejects_all(node)) {
       n_supported[i] = -1;
 //      log_fn(LOG_DEBUG,"Skipping node %s (index %d) -- it rejects all.",
@@ -1771,13 +1785,7 @@ pick_restricted_middle_node(router_crn_flags_t flags,
   tor_assert(pick_from);
 
   /* Add all running nodes to all_live_nodes */
-  router_add_running_nodes_to_smartlist(all_live_nodes,
-                                        (flags & CRN_NEED_UPTIME) != 0,
-                                        (flags & CRN_NEED_CAPACITY) != 0,
-                                        (flags & CRN_NEED_GUARD) != 0,
-                                        (flags & CRN_NEED_DESC) != 0,
-                                        (flags & CRN_PREF_ADDR) != 0,
-                                        (flags & CRN_DIRECT_CONN) != 0);
+  router_add_running_nodes_to_smartlist(all_live_nodes, flags);
 
   /* Filter all_live_nodes to only add live *and* whitelisted middles
    * to the list whitelisted_live_middles. */
@@ -1957,6 +1965,43 @@ warn_if_last_router_excluded(origin_circuit_t *circ,
   return;
 }
 
+/* Return a set of generic CRN_* flags based on <b>state</b>.
+ *
+ * Called for every position in the circuit. */
+STATIC int
+cpath_build_state_to_crn_flags(const cpath_build_state_t *state)
+{
+  router_crn_flags_t flags = 0;
+  /* These flags apply to entry, middle, and exit nodes.
+   * If a flag only applies to a specific position, it should be checked in
+   * that function. */
+  if (state->need_uptime)
+    flags |= CRN_NEED_UPTIME;
+  if (state->need_capacity)
+    flags |= CRN_NEED_CAPACITY;
+  return flags;
+}
+
+/* Return the CRN_INITIATE_IPV6_EXTEND flag, based on <b>state</b> and
+ * <b>cur_len</b>.
+ *
+ * Only called for middle nodes (for now). Must not be called on single-hop
+ * circuits. */
+STATIC int
+cpath_build_state_to_crn_ipv6_extend_flag(const cpath_build_state_t *state,
+                                          int cur_len)
+{
+  IF_BUG_ONCE(state->desired_path_len < 2)
+    return 0;
+
+  /* The last node is the relay doing the self-test. So we want to extend over
+   * IPv6 from the second-last node. */
+  if (state->is_ipv6_selftest && cur_len == state->desired_path_len - 2)
+    return CRN_INITIATE_IPV6_EXTEND;
+  else
+    return 0;
+}
+
 /** Decide a suitable length for circ's cpath, and pick an exit
  * router (or use <b>exit</b> if provided). Store these in the
  * cpath.
@@ -1990,14 +2035,13 @@ 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 */
     router_crn_flags_t flags = CRN_NEED_DESC;
-    if (state->need_uptime)
-      flags |= CRN_NEED_UPTIME;
-    if (state->need_capacity)
-      flags |= CRN_NEED_CAPACITY;
-    if (is_hs_v3_rp_circuit)
-      flags |= CRN_RENDEZVOUS_V3;
+    flags |= cpath_build_state_to_crn_flags(state);
+    /* Some internal exits are one hop, for example directory connections.
+     * (Guards are always direct, middles are never direct.) */
     if (state->onehop_tunnel)
       flags |= CRN_DIRECT_CONN;
+    if (is_hs_v3_rp_circuit)
+      flags |= CRN_RENDEZVOUS_V3;
     const node_t *node =
       choose_good_exit_server(circ, flags, state->is_internal);
     if (!node) {
@@ -2059,32 +2103,27 @@ circuit_extend_to_new_exit(origin_circuit_t *circ, extend_info_t *exit_ei)
   return 0;
 }
 
-/** Return the number of routers in <b>routers</b> that are currently up
- * and available for building circuits through.
+/** Return the number of routers in <b>nodes</b> that are currently up and
+ * available for building circuits through.
  *
- * (Note that this function may overcount or undercount, if we have
- * descriptors that are not the type we would prefer to use for some
- * particular router. See bug #25885.)
+ * If <b>direct</b> is true, only count nodes that are suitable for direct
+ * connections. Counts nodes regardless of whether their addresses are
+ * preferred.
  */
 MOCK_IMPL(STATIC int,
 count_acceptable_nodes, (const smartlist_t *nodes, int direct))
 {
   int num=0;
+  int flags = CRN_NEED_DESC;
+
+  if (direct)
+    flags |= CRN_DIRECT_CONN;
 
   SMARTLIST_FOREACH_BEGIN(nodes, const node_t *, node) {
     //    log_debug(LD_CIRC,
-//              "Contemplating whether router %d (%s) is a new option.",
-//              i, r->nickname);
-    if (! node->is_running)
-//      log_debug(LD_CIRC,"Nope, the directory says %d is not running.",i);
-      continue;
-    if (! node->is_valid)
-//      log_debug(LD_CIRC,"Nope, the directory says %d is not valid.",i);
-      continue;
-    if (! node_has_preferred_descriptor(node, direct))
-      continue;
-    /* The node has a descriptor, so we can just check the ntor key directly */
-    if (!node_has_curve25519_onion_key(node))
+    //              "Contemplating whether router %d (%s) is a new option.",
+    //              i, r->nickname);
+    if (!router_can_choose_node(node, flags))
       continue;
     ++num;
   } SMARTLIST_FOREACH_END(node);
@@ -2278,10 +2317,8 @@ choose_good_middle_server(uint8_t purpose,
 
   excluded = build_middle_exclude_list(purpose, state, head, cur_len);
 
-  if (state->need_uptime)
-    flags |= CRN_NEED_UPTIME;
-  if (state->need_capacity)
-    flags |= CRN_NEED_CAPACITY;
+  flags |= cpath_build_state_to_crn_flags(state);
+  flags |= cpath_build_state_to_crn_ipv6_extend_flag(state, cur_len);
 
   /** If a hidden service circuit wants a specific middle node, pin it. */
   if (middle_node_must_be_vanguard(options, purpose, cur_len)) {
@@ -2357,10 +2394,7 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state,
   }
 
   if (state) {
-    if (state->need_uptime)
-      flags |= CRN_NEED_UPTIME;
-    if (state->need_capacity)
-      flags |= CRN_NEED_CAPACITY;
+    flags |= cpath_build_state_to_crn_flags(state);
   }
 
   choice = router_choose_random_node(excluded, options->ExcludeNodes, flags);

src/core/or/circuitbuild.h

@@ -97,6 +97,10 @@ STATIC int onion_extend_cpath(origin_circuit_t *circ);
 STATIC int
 onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit_ei,
                       int is_hs_v3_rp_circuit);
+STATIC int cpath_build_state_to_crn_flags(const cpath_build_state_t *state);
+STATIC int cpath_build_state_to_crn_ipv6_extend_flag(
+                                             const cpath_build_state_t *state,
+                                             int cur_len);
 
 #endif /* defined(CIRCUITBUILD_PRIVATE) */
 

src/core/or/circuitlist.c

@@ -1944,7 +1944,7 @@ circuit_find_to_cannibalize(uint8_t purpose_to_produce, extend_info_t *info,
 
       /* Ignore any circuits for which we can't use the Guard. It is possible
        * that the Guard was removed from the sampled set after the circuit
-       * was created so avoid using it. */
+       * was created, so avoid using it. */
       if (!entry_guard_could_succeed(circ->guard_state)) {
         goto next;
       }

src/core/or/circuituse.c

@@ -1642,7 +1642,7 @@ static void
 circuit_testing_opened(origin_circuit_t *circ)
 {
   if (have_performed_bandwidth_test ||
-      !check_whether_orport_reachable(get_options())) {
+      !router_should_skip_orport_reachability_check(get_options())) {
     /* either we've already done everything we want with testing circuits,
      * or this testing circuit became open due to a fluke, e.g. we picked
      * a last hop where we already had the connection open due to an
@@ -1660,7 +1660,8 @@ static void
 circuit_testing_failed(origin_circuit_t *circ, int at_last_hop)
 {
   const or_options_t *options = get_options();
-  if (server_mode(options) && check_whether_orport_reachable(options))
+  if (server_mode(options) &&
+      router_should_skip_orport_reachability_check(options))
     return;
 
   log_info(LD_GENERAL,
@@ -2092,11 +2093,18 @@ circuit_should_cannibalize_to_build(uint8_t purpose_to_build,
 }
 
 /** 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
- * CIRCLAUNCH_NEED_CAPACITY is set, choose among routers with high bandwidth.
- * If CIRCLAUNCH_IS_INTERNAL is true, the last hop need not be an exit node.
- * If CIRCLAUNCH_ONEHOP_TUNNEL is set, the circuit will have only one hop.
+ * <b>extend_info</b> (or NULL to select a random exit node).
+ *
+ * If flags contains:
+ *  - CIRCLAUNCH_ONEHOP_TUNNEL: the circuit will have only one hop;
+ *  - CIRCLAUNCH_NEED_UPTIME: choose routers with high uptime;
+ *  - CIRCLAUNCH_NEED_CAPACITY: choose routers with high bandwidth;
+ *  - CIRCLAUNCH_IS_IPV6_SELFTEST: the second-last hop must support IPv6
+ *                                 extends;
+ *  - CIRCLAUNCH_IS_INTERNAL: the last hop need not be an exit node;
+ *  - CIRCLAUNCH_IS_V3_RP: the last hop must support v3 onion service
+ *                         rendezvous.
+ *
  * Return the newly allocated circuit on success, or NULL on failure. */
 origin_circuit_t *
 circuit_launch_by_extend_info(uint8_t purpose,

src/core/or/circuituse.h

@@ -36,17 +36,23 @@ void circuit_try_attaching_streams(origin_circuit_t *circ);
 void circuit_build_failed(origin_circuit_t *circ);
 
 /** Flag to set when a circuit should have only a single hop. */
-#define CIRCLAUNCH_ONEHOP_TUNNEL  (1<<0)
+#define CIRCLAUNCH_ONEHOP_TUNNEL    (1<<0)
 /** Flag to set when a circuit needs to be built of high-uptime nodes */
-#define CIRCLAUNCH_NEED_UPTIME    (1<<1)
+#define CIRCLAUNCH_NEED_UPTIME      (1<<1)
 /** Flag to set when a circuit needs to be built of high-capacity nodes */
-#define CIRCLAUNCH_NEED_CAPACITY  (1<<2)
+#define CIRCLAUNCH_NEED_CAPACITY    (1<<2)
 /** Flag to set when the last hop of a circuit doesn't need to be an
  * exit node. */
-#define CIRCLAUNCH_IS_INTERNAL    (1<<3)
+#define CIRCLAUNCH_IS_INTERNAL      (1<<3)
 /** Flag to set when we are trying to launch a v3 rendezvous circuit. We need
  *  to apply some additional filters on the node picked. */
-#define CIRCLAUNCH_IS_V3_RP (1<<4)
+#define CIRCLAUNCH_IS_V3_RP         (1<<4)
+/** Flag to set when we are trying to launch a self-testing circuit to our
+ *  IPv6 ORPort. We need to apply some additional filters on the second-last
+ *  node in the circuit. (We are both the client and the last node in the
+ *  circuit.) */
+#define CIRCLAUNCH_IS_IPV6_SELFTEST (1<<5)
+
 origin_circuit_t *circuit_launch_by_extend_info(uint8_t purpose,
                                                 extend_info_t *info,
                                                 int flags);

src/core/or/cpath_build_state_st.h

@@ -24,6 +24,8 @@ struct cpath_build_state_t {
   unsigned int need_capacity : 1;
   /** Whether the last hop was picked with exiting in mind. */
   unsigned int is_internal : 1;
+  /** Is this an IPv6 ORPort self-testing circuit? */
+  unsigned int is_ipv6_selftest : 1;
   /** Did we pick this as a one-hop tunnel (not safe for other streams)?
    * These are for encrypted dir conns that exit to this router, not
    * for arbitrary exits from the circuit. */

src/core/or/or.h

@@ -815,6 +815,18 @@ typedef struct protover_summary_flags_t {
    * accept EXTEND2 cells. This requires Relay=2. */
   unsigned int supports_extend2_cells:1;
 
+  /** True iff this router has a version or protocol list that allows it to
+   * accept IPv6 connections. This requires Relay=2 or Relay=3. */
+  unsigned int supports_accepting_ipv6_extends:1;
+
+  /** True iff this router has a version or protocol list that allows it to
+   * initiate IPv6 connections. This requires Relay=3. */
+  unsigned int supports_initiating_ipv6_extends:1;
+
+  /** True iff this router has a version or protocol list that allows it to
+   * consider IPv6 connections canonical. This requires Relay=3. */
+  unsigned int supports_canonical_ipv6_conns:1;
+
   /** True iff this router has a protocol list that allows it to negotiate
    * ed25519 identity keys on a link handshake with us. This
    * requires LinkAuth=3. */
@@ -830,6 +842,10 @@ typedef struct protover_summary_flags_t {
    * the v3 protocol detailed in proposal 224. This requires HSIntro=4. */
   unsigned int supports_ed25519_hs_intro : 1;
 
+  /** True iff this router has a protocol list that allows it to support the
+   * ESTABLISH_INTRO DoS cell extension. Requires HSIntro=5. */
+  unsigned int supports_establish_intro_dos_extension : 1;
+
   /** True iff this router has a protocol list that allows it to be an hidden
    * service directory supporting version 3 as seen in proposal 224. This
    * requires HSDir=2. */
@@ -841,12 +857,9 @@ typedef struct protover_summary_flags_t {
   unsigned int supports_v3_rendezvous_point: 1;
 
   /** True iff this router has a protocol list that allows clients to
-   * negotiate hs circuit setup padding. Requires Padding>=2. */
+   * negotiate hs circuit setup padding. Requires Padding=2. */
   unsigned int supports_hs_setup_padding : 1;
 
-  /** True iff this router has a protocol list that allows it to support the
-   * ESTABLISH_INTRO DoS cell extension. Requires HSIntro>=5. */
-  unsigned int supports_establish_intro_dos_extension : 1;
 } protover_summary_flags_t;
 
 typedef struct routerinfo_t routerinfo_t;

src/core/or/protover.c

@@ -326,6 +326,9 @@ protover_is_supported_here(protocol_type_t pr, uint32_t ver)
 /**
  * Return true iff "list" encodes a protocol list that includes support for
  * the indicated protocol and version.
+ *
+ * If the protocol list is unparseable, treat it as if it defines no
+ * protocols, and return 0.
  */
 int
 protocol_list_supports_protocol(const char *list, protocol_type_t tp,
@@ -348,6 +351,9 @@ protocol_list_supports_protocol(const char *list, protocol_type_t tp,
 /**
  * Return true iff "list" encodes a protocol list that includes support for
  * the indicated protocol and version, or some later version.
+ *
+ * If the protocol list is unparseable, treat it as if it defines no
+ * protocols, and return 0.
  */
 int
 protocol_list_supports_protocol_or_later(const char *list,
@@ -403,7 +409,7 @@ protover_get_supported_protocols(void)
 #endif
     "Microdesc=1-2 "
     "Padding=2 "
-    "Relay=1-2";
+    "Relay=1-3";
 }
 
 /** The protocols from protover_get_supported_protocols(), as parsed into a
@@ -740,6 +746,9 @@ protover_compute_vote(const smartlist_t *list_of_proto_strings,
  * one that we support, and false otherwise.  If <b>missing_out</b> is
  * provided, set it to the list of protocols we do not support.
  *
+ * If the protocol version string is unparseable, treat it as if it defines no
+ * protocols, and return 1.
+ *
  * NOTE: This is quadratic, but we don't do it much: only a few times per
  * consensus. Checking signatures should be way more expensive than this
  * ever would be.

src/core/or/protover.h

@@ -22,12 +22,32 @@ struct smartlist_t;
 ///                 `FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS`
 #define FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS "0.2.9.3-alpha"
 
-/** The protover version number that signifies HSDir support for HSv3 */
-#define PROTOVER_HSDIR_V3 2
+/** The protover version number that signifies ed25519 link handshake support
+ */
+#define PROTOVER_LINKAUTH_ED25519_HANDSHAKE 3
+
+/** The protover version number that signifies extend2 cell support */
+#define PROTOVER_RELAY_EXTEND2 2
+/** The protover version number where relays can accept IPv6 connections */
+#define PROTOVER_RELAY_ACCEPT_IPV6 2
+/** The protover version number where relays can initiate IPv6 extends */
+#define PROTOVER_RELAY_EXTEND_IPV6 3
+/** The protover version number where relays can consider IPv6 connections
+ *  canonical */
+#define PROTOVER_RELAY_CANONICAL_IPV6 3
+
 /** The protover version number that signifies HSv3 intro point support */
 #define PROTOVER_HS_INTRO_V3 4
+/** The protover version number where intro points support denial of service
+ * resistance */
+#define PROTOVER_HS_INTRO_DOS 5
+
 /** The protover version number that signifies HSv3 rendezvous point support */
 #define PROTOVER_HS_RENDEZVOUS_POINT_V3 2
+
+/** The protover version number that signifies HSDir support for HSv3 */
+#define PROTOVER_HSDIR_V3 2
+
 /** The protover that signals support for HS circuit setup padding machines */
 #define PROTOVER_HS_SETUP_PADDING 2
 

src/core/or/versions.c

@@ -408,6 +408,10 @@ static strmap_t *protover_summary_map = NULL;
 /**
  * Helper.  Given a non-NULL protover string <b>protocols</b>, set <b>out</b>
  * to its summary, and memoize the result in <b>protover_summary_map</b>.
+ *
+ * If the protover string does not contain any recognised protocols, sets
+ * protocols_known, but does not set any other flags. (Empty strings are also
+ * treated this way.)
  */
 static void
 memoize_protover_summary(protover_summary_flags_t *out,
@@ -434,25 +438,49 @@ memoize_protover_summary(protover_summary_flags_t *out,
 
   memset(out, 0, sizeof(*out));
   out->protocols_known = 1;
-  out->supports_extend2_cells =
-    protocol_list_supports_protocol(protocols, PRT_RELAY, 2);
+
   out->supports_ed25519_link_handshake_compat =
-    protocol_list_supports_protocol(protocols, PRT_LINKAUTH, 3);
+    protocol_list_supports_protocol(protocols, PRT_LINKAUTH,
+                                    PROTOVER_LINKAUTH_ED25519_HANDSHAKE);
   out->supports_ed25519_link_handshake_any =
-    protocol_list_supports_protocol_or_later(protocols, PRT_LINKAUTH, 3);
+    protocol_list_supports_protocol_or_later(
+                                     protocols,
+                                     PRT_LINKAUTH,
+                                     PROTOVER_LINKAUTH_ED25519_HANDSHAKE);
+
+  out->supports_extend2_cells =
+    protocol_list_supports_protocol(protocols, PRT_RELAY,
+                                    PROTOVER_RELAY_EXTEND2);
+  out->supports_accepting_ipv6_extends = (
+    protocol_list_supports_protocol(protocols, PRT_RELAY,
+                                    PROTOVER_RELAY_ACCEPT_IPV6) ||
+    protocol_list_supports_protocol(protocols, PRT_RELAY,
+                                    PROTOVER_RELAY_EXTEND_IPV6));
+  out->supports_initiating_ipv6_extends =
+    protocol_list_supports_protocol(protocols, PRT_RELAY,
+                                    PROTOVER_RELAY_EXTEND_IPV6);
+  out->supports_canonical_ipv6_conns =
+    protocol_list_supports_protocol(protocols, PRT_RELAY,
+                                    PROTOVER_RELAY_CANONICAL_IPV6);
+
   out->supports_ed25519_hs_intro =
-    protocol_list_supports_protocol(protocols, PRT_HSINTRO, 4);
-  out->supports_v3_hsdir =
-    protocol_list_supports_protocol(protocols, PRT_HSDIR,
-                                    PROTOVER_HSDIR_V3);
+    protocol_list_supports_protocol(protocols, PRT_HSINTRO,
+                                    PROTOVER_HS_INTRO_V3);
+  out->supports_establish_intro_dos_extension =
+    protocol_list_supports_protocol(protocols, PRT_HSINTRO,
+                                    PROTOVER_HS_INTRO_DOS);
+
   out->supports_v3_rendezvous_point =
     protocol_list_supports_protocol(protocols, PRT_HSREND,
                                     PROTOVER_HS_RENDEZVOUS_POINT_V3);
+
+  out->supports_v3_hsdir =
+    protocol_list_supports_protocol(protocols, PRT_HSDIR,
+                                    PROTOVER_HSDIR_V3);
+
   out->supports_hs_setup_padding =
     protocol_list_supports_protocol(protocols, PRT_PADDING,
                                     PROTOVER_HS_SETUP_PADDING);
-  out->supports_establish_intro_dos_extension =
-    protocol_list_supports_protocol(protocols, PRT_HSINTRO, 5);
 
   protover_summary_flags_t *new_cached = tor_memdup(out, sizeof(*out));
   cached = strmap_set(protover_summary_map, protocols, new_cached);
@@ -461,6 +489,13 @@ memoize_protover_summary(protover_summary_flags_t *out,
 
 /** Summarize the protocols listed in <b>protocols</b> into <b>out</b>,
  * falling back or correcting them based on <b>version</b> as appropriate.
+ *
+ * If protocols and version are both NULL, returns a summary with no flags
+ * set.
+ *
+ * If the protover string does not contain any recognised protocols, and the
+ * version is not recognised, sets protocols_known, but does not set any other
+ * flags. (Empty strings are also treated this way.)
  */
 void
 summarize_protover_flags(protover_summary_flags_t *out,

src/feature/control/control_getinfo.c

@@ -1278,15 +1278,18 @@ getinfo_helper_events(control_connection_t *control_conn,
       *answer = tor_strdup(directories_have_accepted_server_descriptor()
                            ? "1" : "0");
     } else if (!strcmp(question, "status/reachability-succeeded/or")) {
-      *answer = tor_strdup(check_whether_orport_reachable(options) ?
-                            "1" : "0");
+      *answer = tor_strdup(
+                    router_should_skip_orport_reachability_check(options) ?
+                    "1" : "0");
     } else if (!strcmp(question, "status/reachability-succeeded/dir")) {
-      *answer = tor_strdup(check_whether_dirport_reachable(options) ?
-                            "1" : "0");
+      *answer = tor_strdup(
+                    router_should_skip_dirport_reachability_check(options) ?
+                    "1" : "0");
     } else if (!strcmp(question, "status/reachability-succeeded")) {
-      tor_asprintf(answer, "OR=%d DIR=%d",
-                   check_whether_orport_reachable(options) ? 1 : 0,
-                   check_whether_dirport_reachable(options) ? 1 : 0);
+      tor_asprintf(
+          answer, "OR=%d DIR=%d",
+          router_should_skip_orport_reachability_check(options) ? 1 : 0,
+          router_should_skip_dirport_reachability_check(options) ? 1 : 0);
     } else if (!strcmp(question, "status/bootstrap-phase")) {
       *answer = control_event_boot_last_msg();
     } else if (!strcmpstart(question, "status/version/")) {

src/feature/nodelist/node_select.c

@@ -321,8 +321,12 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags,
   overloaded_direct = smartlist_new();
   overloaded_tunnel = smartlist_new();
 
-  const int skip_or_fw = router_skip_or_reachability(options, try_ip_pref);
-  const int skip_dir_fw = router_skip_dir_reachability(options, try_ip_pref);
+  const int skip_or_fw = router_or_conn_should_skip_reachable_address_check(
+                                                            options,
+                                                            try_ip_pref);
+  const int skip_dir_fw = router_dir_conn_should_skip_reachable_address_check(
+                                                            options,
+                                                            try_ip_pref);
   const int must_have_or = dirclient_must_use_begindir(options);
 
   /* Find all the running dirservers we know about. */
@@ -926,81 +930,25 @@ nodelist_subtract(smartlist_t *sl, const smartlist_t *excluded)
   bitarray_free(excluded_idx);
 }
 
-/** Return a random running node from the nodelist. Never
- * pick a node that is in
- * <b>excludedsmartlist</b>, or which matches <b>excludedset</b>,
- * even if they are the only nodes available.
- * If <b>CRN_NEED_UPTIME</b> is set in flags and any router has more than
- * a minimum uptime, return one of those.
- * If <b>CRN_NEED_CAPACITY</b> is set in flags, weight your choice by the
- * advertised capacity of each router.
- * If <b>CRN_NEED_GUARD</b> is set in flags, consider only Guard routers.
- * If <b>CRN_WEIGHT_AS_EXIT</b> is set in flags, we weight bandwidths as if
- * picking an exit node, otherwise we weight bandwidths for picking a relay
- * node (that is, possibly discounting exit nodes).
- * If <b>CRN_NEED_DESC</b> is set in flags, we only consider nodes that
- * have a routerinfo or microdescriptor -- that is, enough info to be
- * used to build a circuit.
- * If <b>CRN_PREF_ADDR</b> is set in flags, we only consider nodes that
- * have an address that is preferred by the ClientPreferIPv6ORPort setting
- * (regardless of this flag, we exclude nodes that aren't allowed by the
- * firewall, including ClientUseIPv4 0 and fascist_firewall_use_ipv6() == 0).
- */
-const node_t *
-router_choose_random_node(smartlist_t *excludedsmartlist,
-                          routerset_t *excludedset,
-                          router_crn_flags_t flags)
-{ /* XXXX MOVE */
-  const int need_uptime = (flags & CRN_NEED_UPTIME) != 0;
-  const int need_capacity = (flags & CRN_NEED_CAPACITY) != 0;
-  const int need_guard = (flags & CRN_NEED_GUARD) != 0;
-  const int weight_for_exit = (flags & CRN_WEIGHT_AS_EXIT) != 0;
-  const int need_desc = (flags & CRN_NEED_DESC) != 0;
-  const int pref_addr = (flags & CRN_PREF_ADDR) != 0;
-  const int direct_conn = (flags & CRN_DIRECT_CONN) != 0;
-  const int rendezvous_v3 = (flags & CRN_RENDEZVOUS_V3) != 0;
-
-  const smartlist_t *node_list = nodelist_get_list();
-  smartlist_t *sl=smartlist_new(),
-    *excludednodes=smartlist_new();
+/* Node selection helper for router_choose_random_node().
+ *
+ * Populates a node list based on <b>flags</b>, ignoring nodes in
+ * <b>excludednodes</b> and <b>excludedset</b>. Chooses the node based on
+ * <b>rule</b>. */
+static const node_t *
+router_choose_random_node_helper(smartlist_t *excludednodes,
+                                 routerset_t *excludedset,
+                                 router_crn_flags_t flags,
+                                 bandwidth_weight_rule_t rule)
+{
+  smartlist_t *sl=smartlist_new();
   const node_t *choice = NULL;
-  const routerinfo_t *r;
-  bandwidth_weight_rule_t rule;
 
-  tor_assert(!(weight_for_exit && need_guard));
-  rule = weight_for_exit ? WEIGHT_FOR_EXIT :
-    (need_guard ? WEIGHT_FOR_GUARD : WEIGHT_FOR_MID);
-
-  SMARTLIST_FOREACH_BEGIN(node_list, const node_t *, node) {
-    if (node_allows_single_hop_exits(node)) {
-      /* Exclude relays that allow single hop exit circuits. This is an
-       * obsolete option since 0.2.9.2-alpha and done by default in
-       * 0.3.1.0-alpha. */
-      smartlist_add(excludednodes, (node_t*)node);
-    } else if (rendezvous_v3 &&
-               !node_supports_v3_rendezvous_point(node)) {
-      /* Exclude relays that do not support to rendezvous for a hidden service
-       * version 3. */
-      smartlist_add(excludednodes, (node_t*)node);
-    }
-  } SMARTLIST_FOREACH_END(node);
-
-  /* If the node_t is not found we won't be to exclude ourself but we
-   * won't be able to pick ourself in router_choose_random_node() so
-   * this is fine to at least try with our routerinfo_t object. */
-  if ((r = router_get_my_routerinfo()))
-    routerlist_add_node_and_family(excludednodes, r);
-
-  router_add_running_nodes_to_smartlist(sl, need_uptime, need_capacity,
-                                        need_guard, need_desc, pref_addr,
-                                        direct_conn);
+  router_add_running_nodes_to_smartlist(sl, flags);
   log_debug(LD_CIRC,
            "We found %d running nodes.",
             smartlist_len(sl));
 
-  if (excludedsmartlist) {
-    smartlist_add_all(excludednodes, excludedsmartlist);
-  }
   nodelist_subtract(sl, excludednodes);
 
   if (excludedset) {
@@ -1014,18 +962,66 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
   choice = node_sl_choose_by_bandwidth(sl, rule);
 
   smartlist_free(sl);
+
+  return choice;
+}
+
+/** Return a random running node from the nodelist. Never pick a node that is
+ * in <b>excludedsmartlist</b>, or which matches <b>excludedset</b>, even if
+ * they are the only nodes available.
+ *
+ * <b>flags</b> is a set of CRN_* flags, see
+ * router_add_running_nodes_to_smartlist() for details.
+ */
+const node_t *
+router_choose_random_node(smartlist_t *excludedsmartlist,
+                          routerset_t *excludedset,
+                          router_crn_flags_t flags)
+{
+  /* A limited set of flags, used for fallback node selection.
+   */
+  const bool need_uptime = (flags & CRN_NEED_UPTIME) != 0;
+  const bool need_capacity = (flags & CRN_NEED_CAPACITY) != 0;
+  const bool need_guard = (flags & CRN_NEED_GUARD) != 0;
+  const bool pref_addr = (flags & CRN_PREF_ADDR) != 0;
+
+  smartlist_t *excludednodes=smartlist_new();
+  const node_t *choice = NULL;
+  const routerinfo_t *r;
+  bandwidth_weight_rule_t rule;
+
+  rule = (need_guard ? WEIGHT_FOR_GUARD : WEIGHT_FOR_MID);
+
+  /* If the node_t is not found we won't be to exclude ourself but we
+   * won't be able to pick ourself in router_choose_random_node() so
+   * this is fine to at least try with our routerinfo_t object. */
+  if ((r = router_get_my_routerinfo()))
+    routerlist_add_node_and_family(excludednodes, r);
+
+  if (excludedsmartlist) {
+    smartlist_add_all(excludednodes, excludedsmartlist);
+  }
+
+  choice = router_choose_random_node_helper(excludednodes,
+                                            excludedset,
+                                            flags,
+                                            rule);
+
   if (!choice && (need_uptime || need_capacity || need_guard || pref_addr)) {
-    /* try once more -- recurse but with fewer restrictions. */
+    /* try once more, with fewer restrictions. */
     log_info(LD_CIRC,
-             "We couldn't find any live%s%s%s routers; falling back "
+             "We couldn't find any live%s%s%s%s routers; falling back "
              "to list of all routers.",
              need_capacity?", fast":"",
              need_uptime?", stable":"",
-             need_guard?", guard":"");
+             need_guard?", guard":"",
+             pref_addr?", preferred address":"");
     flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD|
                 CRN_PREF_ADDR);
-    choice = router_choose_random_node(
-                     excludedsmartlist, excludedset, flags);
+    choice = router_choose_random_node_helper(excludednodes,
+                                              excludedset,
+                                              flags,
+                                              rule);
   }
   smartlist_free(excludednodes);
   if (!choice) {
@@ -1120,8 +1116,12 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist,
   overloaded_direct = smartlist_new();
   overloaded_tunnel = smartlist_new();
 
-  const int skip_or_fw = router_skip_or_reachability(options, try_ip_pref);
-  const int skip_dir_fw = router_skip_dir_reachability(options, try_ip_pref);
+  const int skip_or_fw = router_or_conn_should_skip_reachable_address_check(
+                                                            options,
+                                                            try_ip_pref);
+  const int skip_dir_fw = router_dir_conn_should_skip_reachable_address_check(
+                                                            options,
+                                                            try_ip_pref);
   const int must_have_or = dirclient_must_use_begindir(options);
 
   SMARTLIST_FOREACH_BEGIN(sourcelist, const dir_server_t *, d)

src/feature/nodelist/node_select.h

@@ -14,20 +14,26 @@
 /** Flags to be passed to control router_choose_random_node() to indicate what
  * kind of nodes to pick according to what algorithm. */
 typedef enum router_crn_flags_t {
+  /* Try to choose stable nodes. */
   CRN_NEED_UPTIME = 1<<0,
+  /* Try to choose nodes with a reasonable amount of bandwidth. */
   CRN_NEED_CAPACITY = 1<<1,
-  CRN_NEED_GUARD = 1<<2,
-  /* XXXX not used, apparently. */
-  CRN_WEIGHT_AS_EXIT = 1<<5,
-  CRN_NEED_DESC = 1<<6,
-  /* On clients, only provide nodes that satisfy ClientPreferIPv6OR */
-  CRN_PREF_ADDR = 1<<7,
+  /* Only choose nodes if we have downloaded their descriptor or
+   * microdescriptor. */
+  CRN_NEED_DESC = 1<<2,
+  /* Choose nodes that can be used as Guard relays. */
+  CRN_NEED_GUARD = 1<<3,
   /* On clients, only provide nodes that we can connect to directly, based on
-   * our firewall rules */
-  CRN_DIRECT_CONN = 1<<8,
-  /* On clients, only provide nodes with HSRend >= 2 protocol version which
-   * is required for hidden service version >= 3. */
-  CRN_RENDEZVOUS_V3 = 1<<9,
+   * our firewall rules. */
+  CRN_DIRECT_CONN = 1<<4,
+  /* On clients, if choosing a node for a direct connection, only provide
+   * nodes that satisfy ClientPreferIPv6OR. */
+  CRN_PREF_ADDR = 1<<5,
+  /* On clients, only provide nodes with HSRend=2 protocol version which
+   * is required for hidden service version 3. */
+  CRN_RENDEZVOUS_V3 = 1<<6,
+  /* On clients, only provide nodes that can initiate IPv6 extends. */
+  CRN_INITIATE_IPV6_EXTEND = 1<<7,
 } router_crn_flags_t;
 
 /** Possible ways to weight routers when choosing one randomly.  See

src/feature/nodelist/node_st.h

@@ -84,12 +84,11 @@ struct node_t {
 
   /* Local info: derived. */
 
-  /** True if the IPv6 OR port is preferred over the IPv4 OR port.
-   * XX/teor - can this become out of date if the torrc changes? */
+  /** True if the IPv6 OR port is preferred over the IPv4 OR port. */
   unsigned int ipv6_preferred:1;
 
   /** According to the geoip db what country is this router in? */
-  /* XXXprop186 what is this suppose to mean with multiple OR ports? */
+  /* IPv6: what is this supposed to mean with multiple OR ports? */
   country_t country;
 
   /* The below items are used only by authdirservers for

src/feature/nodelist/nodelist.c

@@ -1133,7 +1133,7 @@ node_ed25519_id_matches(const node_t *node, const ed25519_public_key_t *id)
 /** Dummy object that should be unreturnable.  Used to ensure that
  * node_get_protover_summary_flags() always returns non-NULL. */
 static const protover_summary_flags_t zero_protover_flags = {
-  0,0,0,0,0,0,0,0,0
+  0,0,0,0,0,0,0,0,0,0,0,0
 };
 
 /** Return the protover_summary_flags for a given node. */
@@ -1158,9 +1158,9 @@ node_get_protover_summary_flags(const node_t *node)
  * by ed25519 ID during the link handshake.  If <b>compatible_with_us</b>,
  * it needs to be using a link authentication method that we understand.
  * If not, any plausible link authentication method will do. */
-MOCK_IMPL(int,
+MOCK_IMPL(bool,
 node_supports_ed25519_link_authentication,(const node_t *node,
-                                           int compatible_with_us))
+                                           bool compatible_with_us))
 {
   if (! node_get_ed25519_id(node))
     return 0;
@@ -1175,7 +1175,7 @@ node_supports_ed25519_link_authentication,(const node_t *node,
 
 /** Return true iff <b>node</b> supports the hidden service directory version
  * 3 protocol (proposal 224). */
-int
+bool
 node_supports_v3_hsdir(const node_t *node)
 {
   tor_assert(node);
@@ -1185,7 +1185,7 @@ node_supports_v3_hsdir(const node_t *node)
 
 /** Return true iff <b>node</b> supports ed25519 authentication as an hidden
  * service introduction point.*/
-int
+bool
 node_supports_ed25519_hs_intro(const node_t *node)
 {
   tor_assert(node);
@@ -1193,20 +1193,9 @@ node_supports_ed25519_hs_intro(const node_t *node)
   return node_get_protover_summary_flags(node)->supports_ed25519_hs_intro;
 }
 
-/** Return true iff <b>node</b> supports the DoS ESTABLISH_INTRO cell
- * extenstion. */
-int
-node_supports_establish_intro_dos_extension(const node_t *node)
-{
-  tor_assert(node);
-
-  return node_get_protover_summary_flags(node)->
-                           supports_establish_intro_dos_extension;
-}
-
-/** Return true iff <b>node</b> supports to be a rendezvous point for hidden
+/** Return true iff <b>node</b> can be a rendezvous point for hidden
  * service version 3 (HSRend=2). */
-int
+bool
 node_supports_v3_rendezvous_point(const node_t *node)
 {
   tor_assert(node);
@@ -1219,6 +1208,67 @@ node_supports_v3_rendezvous_point(const node_t *node)
   return node_get_protover_summary_flags(node)->supports_v3_rendezvous_point;
 }
 
+/** Return true iff <b>node</b> supports the DoS ESTABLISH_INTRO cell
+ * extenstion. */
+bool
+node_supports_establish_intro_dos_extension(const node_t *node)
+{
+  tor_assert(node);
+
+  return node_get_protover_summary_flags(node)->
+                           supports_establish_intro_dos_extension;
+}
+
+/** Return true iff <b>node</b> can initiate IPv6 extends (Relay=3).
+ *
+ * This check should only be performed by client path selection code.
+ *
+ * Extending relays should check their own IPv6 support using
+ * router_can_extend_over_ipv6(). Like other extends, they should not verify
+ * the link specifiers in the extend cell against the consensus, because it
+ * may be out of date. */
+bool
+node_supports_initiating_ipv6_extends(const node_t *node)
+{
+  tor_assert(node);
+
+  /* Relays can't initiate an IPv6 extend, unless they have an IPv6 ORPort. */
+  if (!node_has_ipv6_orport(node)) {
+    return 0;
+  }
+
+  /* Initiating relays also need to support the relevant protocol version. */
+  return
+    node_get_protover_summary_flags(node)->supports_initiating_ipv6_extends;
+}
+
+/** Return true iff <b>node</b> can accept IPv6 extends (Relay=2 or Relay=3)
+ * from other relays. If <b>need_canonical_ipv6_conn</b> is true, also check
+ * if the relay supports canonical IPv6 connections (Relay=3 only).
+ *
+ * This check should only be performed by client path selection code.
+ */
+bool
+node_supports_accepting_ipv6_extends(const node_t *node,
+                                            bool need_canonical_ipv6_conn)
+{
+  tor_assert(node);
+
+  /* Relays can't accept an IPv6 extend, unless they have an IPv6 ORPort. */
+  if (!node_has_ipv6_orport(node)) {
+    return 0;
+  }
+
+  /* Accepting relays also need to support the relevant protocol version. */
+  if (need_canonical_ipv6_conn) {
+    return
+      node_get_protover_summary_flags(node)->supports_canonical_ipv6_conns;
+  } else {
+    return
+      node_get_protover_summary_flags(node)->supports_accepting_ipv6_extends;
+  }
+}
+
 /** Return the RSA ID key's SHA1 digest for the provided node. */
 const uint8_t *
 node_get_rsa_id_digest(const node_t *node)

src/feature/nodelist/nodelist.h

@@ -74,13 +74,17 @@ MOCK_DECL(const struct ed25519_public_key_t *,node_get_ed25519_id,
           (const node_t *node));
 int node_ed25519_id_matches(const node_t *node,
                             const struct ed25519_public_key_t *id);
-MOCK_DECL(int,node_supports_ed25519_link_authentication,
+MOCK_DECL(bool,node_supports_ed25519_link_authentication,
           (const node_t *node,
-           int compatible_with_us));
-int node_supports_v3_hsdir(const node_t *node);
-int node_supports_ed25519_hs_intro(const node_t *node);
-int node_supports_v3_rendezvous_point(const node_t *node);
-int node_supports_establish_intro_dos_extension(const node_t *node);
+           bool compatible_with_us));
+bool node_supports_v3_hsdir(const node_t *node);
+bool node_supports_ed25519_hs_intro(const node_t *node);
+bool node_supports_v3_rendezvous_point(const node_t *node);
+bool node_supports_establish_intro_dos_extension(const node_t *node);
+bool node_supports_initiating_ipv6_extends(const node_t *node);
+bool node_supports_accepting_ipv6_extends(const node_t *node,
+                                          bool need_canonical_ipv6_conn);
+
 const uint8_t *node_get_rsa_id_digest(const node_t *node);
 MOCK_DECL(smartlist_t *,node_get_link_specifier_smartlist,(const node_t *node,
                                                            bool direct_conn));

src/feature/nodelist/routerinfo.c

@@ -17,14 +17,37 @@
 #include "feature/nodelist/node_st.h"
 #include "feature/nodelist/routerinfo_st.h"
 
-/** Copy the primary (IPv4) OR port (IP address and TCP port) for
- * <b>router</b> into *<b>ap_out</b>. */
-void
-router_get_prim_orport(const routerinfo_t *router, tor_addr_port_t *ap_out)
+/** Copy the OR port (IP address and TCP port) for <b>router</b> and
+ * <b>family</b> into *<b>ap_out</b>.
+ *
+ * If the requested ORPort does not exist, sets *<b>ap_out</b> to the null
+ * address and port, and returns -1. Otherwise, returns 0. */
+int
+router_get_orport(const routerinfo_t *router,
+                  tor_addr_port_t *ap_out,
+                  int family)
 {
   tor_assert(ap_out != NULL);
-  tor_addr_from_ipv4h(&ap_out->addr, router->addr);
-  ap_out->port = router->or_port;
+  if (family == AF_INET) {
+    tor_addr_from_ipv4h(&ap_out->addr, router->addr);
+    ap_out->port = router->or_port;
+    return 0;
+  } else if (family == AF_INET6) {
+    /* IPv6 addresses are optional, so check if it is valid. */
+    if (tor_addr_port_is_valid(&router->ipv6_addr, router->ipv6_orport, 0)) {
+      tor_addr_copy(&ap_out->addr, &router->ipv6_addr);
+      ap_out->port = router->ipv6_orport;
+      return 0;
+    } else {
+      tor_addr_port_make_null_ap(ap_out, AF_INET6);
+      return -1;
+    }
+  } else {
+    /* Unsupported address family */
+    tor_assert_nonfatal_unreached();
+    tor_addr_port_make_null_ap(ap_out, AF_UNSPEC);
+    return -1;
+  }
 }
 
 int

src/feature/nodelist/routerinfo.h

@@ -12,8 +12,9 @@
 #ifndef TOR_ROUTERINFO_H
 #define TOR_ROUTERINFO_H
 
-void router_get_prim_orport(const routerinfo_t *router,
-                            tor_addr_port_t *addr_port_out);
+int router_get_orport(const routerinfo_t *router,
+                       tor_addr_port_t *addr_port_out,
+                       int family);
 int router_has_orport(const routerinfo_t *router,
                       const tor_addr_port_t *orport);
 

src/feature/nodelist/routerlist.c

@@ -465,11 +465,20 @@ router_reload_router_list(void)
   return 0;
 }
 
-/* When iterating through the routerlist, can OR address/port preference
- * and reachability checks be skipped?
+/* When selecting a router for a direct connection, can OR address/port
+ * preference and reachability checks be skipped?
+ *
+ * Servers never check ReachableAddresses or ClientPreferIPv6. Returns
+ * true for servers.
+ *
+ * Otherwise, if <b>try_ip_pref</b> is true, returns false. Used to make
+ * clients check ClientPreferIPv6, even if ReachableAddresses is not set.
+ * Finally, return true if ReachableAddresses is set.
  */
 int
-router_skip_or_reachability(const or_options_t *options, int try_ip_pref)
+router_or_conn_should_skip_reachable_address_check(
+                                   const or_options_t *options,
+                                   int try_ip_pref)
 {
   /* Servers always have and prefer IPv4.
    * And if clients are checking against the firewall for reachability only,
@@ -477,11 +486,15 @@ router_skip_or_reachability(const or_options_t *options, int try_ip_pref)
   return server_mode(options) || (!try_ip_pref && !firewall_is_fascist_or());
 }
 
-/* When iterating through the routerlist, can Dir address/port preference
+/* When selecting a router for a direct connection, can Dir address/port
  * and reachability checks be skipped?
+ *
+ * This function is obsolete, because clients only use ORPorts.
  */
 int
-router_skip_dir_reachability(const or_options_t *options, int try_ip_pref)
+router_dir_conn_should_skip_reachable_address_check(
+                                    const or_options_t *options,
+                                    int try_ip_pref)
 {
   /* Servers always have and prefer IPv4.
    * And if clients are checking against the firewall for reachability only,
@@ -498,40 +511,109 @@ routers_have_same_or_addrs(const routerinfo_t *r1, const routerinfo_t *r2)
     r1->ipv6_orport == r2->ipv6_orport;
 }
 
+/* Returns true if <b>node</b> can be chosen based on <b>flags</b>.
+ *
+ * The following conditions are applied to all nodes:
+ *  - is running;
+ *  - is valid;
+ *  - supports EXTEND2 cells;
+ *  - has an ntor circuit crypto key; and
+ *  - does not allow single-hop exits.
+ *
+ * If the node has a routerinfo, we're checking for a direct connection, and
+ * we're using bridges, the following condition is applied:
+ *  - has a bridge-purpose routerinfo;
+ * and for all other nodes:
+ *  - has a general-purpose routerinfo (or no routerinfo).
+ *
+ * Nodes that don't have a routerinfo must be general-purpose nodes, because
+ * routerstatuses and microdescriptors only come via consensuses.
+ *
+ * The <b>flags</b> chech that <b>node</b>:
+ *  - <b>CRN_NEED_UPTIME</b>: has more than a minimum uptime;
+ *  - <b>CRN_NEED_CAPACITY</b>: has more than a minimum capacity;
+ *  - <b>CRN_NEED_GUARD</b>: is a Guard;
+ *  - <b>CRN_NEED_DESC</b>: has a routerinfo or microdescriptor -- that is,
+ *                          enough info to be used to build a circuit;
+ *  - <b>CRN_DIRECT_CONN</b>: is suitable for direct connections. Checks
+ *                            for the relevant descriptors. Checks the address
+ *                            against ReachableAddresses, ClientUseIPv4 0, and
+ *                            fascist_firewall_use_ipv6() == 0);
+ *  - <b>CRN_PREF_ADDR</b>: if we are connecting directly to the node, it has
+ *                          an address that is preferred by the
+ *                          ClientPreferIPv6ORPort setting;
+ *  - <b>CRN_RENDEZVOUS_V3</b>: can become a v3 onion service rendezvous point;
+ *  - <b>CRN_INITIATE_IPV6_EXTEND</b>: can initiate IPv6 extends.
+ */
+bool
+router_can_choose_node(const node_t *node, int flags)
+{
+  /* The full set of flags used for node selection. */
+  const bool need_uptime = (flags & CRN_NEED_UPTIME) != 0;
+  const bool need_capacity = (flags & CRN_NEED_CAPACITY) != 0;
+  const bool need_guard = (flags & CRN_NEED_GUARD) != 0;
+  const bool need_desc = (flags & CRN_NEED_DESC) != 0;
+  const bool pref_addr = (flags & CRN_PREF_ADDR) != 0;
+  const bool direct_conn = (flags & CRN_DIRECT_CONN) != 0;
+  const bool rendezvous_v3 = (flags & CRN_RENDEZVOUS_V3) != 0;
+  const bool initiate_ipv6_extend = (flags & CRN_INITIATE_IPV6_EXTEND) != 0;
+
+  const or_options_t *options = get_options();
+  const bool check_reach =
+    !router_or_conn_should_skip_reachable_address_check(options, pref_addr);
+  const bool direct_bridge = direct_conn && options->UseBridges;
+
+  if (!node->is_running || !node->is_valid)
+    return false;
+  if (need_desc && !node_has_preferred_descriptor(node, direct_conn))
+    return false;
+  if (node->ri) {
+    if (direct_bridge && node->ri->purpose != ROUTER_PURPOSE_BRIDGE)
+      return false;
+    else if (node->ri->purpose != ROUTER_PURPOSE_GENERAL)
+      return false;
+  }
+  if (node_is_unreliable(node, need_uptime, need_capacity, need_guard))
+    return false;
+  /* Don't choose nodes if we are certain they can't do EXTEND2 cells */
+  if (node->rs && !routerstatus_version_supports_extend2_cells(node->rs, 1))
+    return false;
+  /* Don't choose nodes if we are certain they can't do ntor. */
+  if ((node->ri || node->md) && !node_has_curve25519_onion_key(node))
+    return false;
+  /* Exclude relays that allow single hop exit circuits. This is an
+   * obsolete option since 0.2.9.2-alpha and done by default in
+   * 0.3.1.0-alpha. */
+  if (node_allows_single_hop_exits(node))
+    return false;
+  /* Exclude relays that can not become a rendezvous for a hidden service
+   * version 3. */
+  if (rendezvous_v3 &&
+      !node_supports_v3_rendezvous_point(node))
+    return false;
+  /* Choose a node with an OR address that matches the firewall rules */
+  if (direct_conn && check_reach &&
+      !fascist_firewall_allows_node(node,
+                                    FIREWALL_OR_CONNECTION,
+                                    pref_addr))
+    return false;
+  if (initiate_ipv6_extend && !node_supports_initiating_ipv6_extends(node))
+    return false;
+
+  return true;
+}
+
 /** Add every suitable node from our nodelist to <b>sl</b>, so that
- * we can pick a node for a circuit.
+ * we can pick a node for a circuit based on <b>flags</b>.
+ *
+ * See router_can_choose_node() for details of <b>flags</b>.
  */
 void
-router_add_running_nodes_to_smartlist(smartlist_t *sl, int need_uptime,
-                                      int need_capacity, int need_guard,
-                                      int need_desc, int pref_addr,
-                                      int direct_conn)
+router_add_running_nodes_to_smartlist(smartlist_t *sl, int flags)
 {
-  const int check_reach = !router_skip_or_reachability(get_options(),
-                                                       pref_addr);
-  /* XXXX MOVE */
   SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) {
-    if (!node->is_running || !node->is_valid)
+    if (!router_can_choose_node(node, flags))
       continue;
-    if (need_desc && !node_has_preferred_descriptor(node, direct_conn))
-      continue;
-    if (node->ri && node->ri->purpose != ROUTER_PURPOSE_GENERAL)
-      continue;
-    if (node_is_unreliable(node, need_uptime, need_capacity, need_guard))
-      continue;
-    /* Don't choose nodes if we are certain they can't do EXTEND2 cells */
-    if (node->rs && !routerstatus_version_supports_extend2_cells(node->rs, 1))
-      continue;
-    /* Don't choose nodes if we are certain they can't do ntor. */
-    if ((node->ri || node->md) && !node_has_curve25519_onion_key(node))
-      continue;
-    /* Choose a node with an OR address that matches the firewall rules */
-    if (direct_conn && check_reach &&
-        !fascist_firewall_allows_node(node,
-                                      FIREWALL_OR_CONNECTION,
-                                      pref_addr))
-      continue;
-
     smartlist_add(sl, (void *)node);
   } SMARTLIST_FOREACH_END(node);
 }

src/feature/nodelist/routerlist.h

@@ -50,14 +50,16 @@ typedef enum was_router_added_t {
 
 int router_reload_router_list(void);
 
-int router_skip_or_reachability(const or_options_t *options, int try_ip_pref);
-int router_skip_dir_reachability(const or_options_t *options, int try_ip_pref);
+int router_or_conn_should_skip_reachable_address_check(
+                                       const or_options_t *options,
+                                       int try_ip_pref);
+int router_dir_conn_should_skip_reachable_address_check(
+                                       const or_options_t *options,
+                                       int try_ip_pref);
 void router_reset_status_download_failures(void);
 int routers_have_same_or_addrs(const routerinfo_t *r1, const routerinfo_t *r2);
-void router_add_running_nodes_to_smartlist(smartlist_t *sl, int need_uptime,
-                                           int need_capacity, int need_guard,
-                                           int need_desc, int pref_addr,
-                                           int direct_conn);
+bool router_can_choose_node(const node_t *node, int flags);
+void router_add_running_nodes_to_smartlist(smartlist_t *sl, int flags);
 
 const routerinfo_t *routerlist_find_my_routerinfo(void);
 uint32_t router_get_advertised_bandwidth(const routerinfo_t *router);

src/feature/relay/relay_periodic.c

@@ -201,7 +201,7 @@ reachability_warnings_callback(time_t now, const or_options_t *options)
       have_completed_a_circuit()) {
     /* every 20 minutes, check and complain if necessary */
     const routerinfo_t *me = router_get_my_routerinfo();
-    if (me && !check_whether_orport_reachable(options)) {
+    if (me && !router_should_skip_orport_reachability_check(options)) {
       char *address = tor_dup_ip(me->addr);
       if (address) {
         log_warn(LD_CONFIG,
@@ -217,7 +217,7 @@ reachability_warnings_callback(time_t now, const or_options_t *options)
       }
     }
 
-    if (me && !check_whether_dirport_reachable(options)) {
+    if (me && !router_should_skip_dirport_reachability_check(options)) {
       char *address = tor_dup_ip(me->addr);
       if (address) {
         log_warn(LD_CONFIG,

src/feature/relay/router.c

@@ -1363,14 +1363,14 @@ decide_if_publishable_server(void)
     return 1;
   if (!router_get_advertised_or_port(options))
     return 0;
-  if (!check_whether_orport_reachable(options))
+  if (!router_should_skip_orport_reachability_check(options))
     return 0;
   if (router_have_consensus_path() == CONSENSUS_PATH_INTERNAL) {
     /* All set: there are no exits in the consensus (maybe this is a tiny
      * test network), so we can't check our DirPort reachability. */
     return 1;
   } else {
-    return check_whether_dirport_reachable(options);
+    return router_should_skip_dirport_reachability_check(options);
   }
 }
 
@@ -1501,7 +1501,22 @@ router_has_advertised_ipv6_orport(const or_options_t *options)
   return tor_addr_port_is_valid_ap(&ipv6_ap, 0);
 }
 
-/** Returns true if this router has an advertised IPv6 ORPort. */
+/** Returns true if this router can extend over IPv6.
+ *
+ * This check should only be performed by relay extend code.
+ *
+ * Clients should check if relays can initiate and accept IPv6 extends using
+ * node_supports_initiating_ipv6_extends() and
+ * node_supports_accepting_ipv6_extends().
+ *
+ * As with other extends, relays should assume the client has already
+ * performed the relevant checks for the next hop. (Otherwise, relays that
+ * have just added IPv6 ORPorts won't be able to self-test those ORPorts.)
+ *
+ * Accepting relays don't need to perform any IPv6-specific checks before
+ * accepting a connection, because having an IPv6 ORPort implies support for
+ * the relevant protocol version.
+ */
 MOCK_IMPL(bool,
 router_can_extend_over_ipv6,(const or_options_t *options))
 {

src/feature/relay/selftest.c

@@ -15,24 +15,31 @@
 #include "core/or/or.h"
 
 #include "app/config/config.h"
+
 #include "core/mainloop/connection.h"
 #include "core/mainloop/mainloop.h"
 #include "core/mainloop/netstatus.h"
+
 #include "core/or/circuitbuild.h"
 #include "core/or/circuitlist.h"
 #include "core/or/circuituse.h"
 #include "core/or/crypt_path_st.h"
+#include "core/or/extend_info_st.h"
 #include "core/or/origin_circuit_st.h"
 #include "core/or/relay.h"
+
 #include "feature/control/control_events.h"
+
 #include "feature/dirclient/dirclient.h"
 #include "feature/dircommon/directory.h"
+
 #include "feature/nodelist/authority_cert_st.h"
 #include "feature/nodelist/routerinfo.h"
 #include "feature/nodelist/routerinfo_st.h"
 #include "feature/nodelist/routerlist.h" // but...
 #include "feature/nodelist/routerset.h"
 #include "feature/nodelist/torcert.h"
+
 #include "feature/relay/relay_periodic.h"
 #include "feature/relay/router.h"
 #include "feature/relay/selftest.h"
@@ -70,7 +77,7 @@ router_reachability_checks_disabled(const or_options_t *options)
  *   - the network is disabled.
  */
 int
-check_whether_orport_reachable(const or_options_t *options)
+router_should_skip_orport_reachability_check(const or_options_t *options)
 {
   int reach_checks_disabled = router_reachability_checks_disabled(options);
   return reach_checks_disabled ||
@@ -87,7 +94,7 @@ check_whether_orport_reachable(const or_options_t *options)
  *   - the network is disabled.
  */
 int
-check_whether_dirport_reachable(const or_options_t *options)
+router_should_skip_dirport_reachability_check(const or_options_t *options)
 {
   int reach_checks_disabled = router_reachability_checks_disabled(options) ||
                               !options->DirPort_set;
@@ -107,6 +114,7 @@ router_should_check_reachability(int test_or, int test_dir)
   if (!me)
     return 0;
 
+  /* Doesn't check our IPv6 address, see #34065. */
   if (routerset_contains_router(options->ExcludeNodes, me, -1) &&
       options->StrictNodes) {
     /* If we've excluded ourself, and StrictNodes is set, we can't test
@@ -126,18 +134,28 @@ router_should_check_reachability(int test_or, int test_dir)
 }
 
 /** Allocate and return a new extend_info_t that can be used to build
- * a circuit to or through the router <b>r</b>. Uses the primary
- * address of the router, so should only be called on a server. */
+ * a circuit to or through the router <b>r</b>, using an address from
+ * <b>family</b> (if available).
+ *
+ * Clients don't have routerinfos, so this function should only be called on a
+ * server.
+ *
+ * If the requested address is not available, returns NULL. */
 static extend_info_t *
-extend_info_from_router(const routerinfo_t *r)
+extend_info_from_router(const routerinfo_t *r, int family)
 {
   crypto_pk_t *rsa_pubkey;
   extend_info_t *info;
   tor_addr_port_t ap;
-  tor_assert(r);
 
-  /* Make sure we don't need to check address reachability */
-  tor_assert_nonfatal(router_skip_or_reachability(get_options(), 0));
+  if (BUG(!r)) {
+    return NULL;
+  }
+
+  /* Relays always assume that the first hop is reachable. They ignore
+   * ReachableAddresses. */
+  tor_assert_nonfatal(router_or_conn_should_skip_reachable_address_check(
+                                                           get_options(), 0));
 
   const ed25519_public_key_t *ed_id_key;
   if (r->cache_info.signing_key_cert)
@@ -145,7 +163,10 @@ extend_info_from_router(const routerinfo_t *r)
   else
     ed_id_key = NULL;
 
-  router_get_prim_orport(r, &ap);
+  if (router_get_orport(r, &ap, family) < 0) {
+    /* We don't have an ORPort for the requested family. */
+    return NULL;
+  }
   rsa_pubkey = router_get_rsa_onion_pkey(r->onion_pkey, r->onion_pkey_len);
   info = extend_info_new(r->nickname, r->cache_info.identity_digest,
                          ed_id_key,
@@ -155,6 +176,69 @@ extend_info_from_router(const routerinfo_t *r)
   return info;
 }
 
+/** Launch a self-testing circuit to one of our ORPorts, using an address from
+ * <b>family</b> (if available). The circuit can be used to test reachability
+ * or bandwidth. <b>me</b> is our own routerinfo.
+ *
+ * Logs an info-level status message. If <b>orport_reachable</b> is false,
+ * call it a reachability circuit. Otherwise, call it a bandwidth circuit.
+ *
+ * See router_do_reachability_checks() for details. */
+static void
+router_do_orport_reachability_checks(const routerinfo_t *me,
+                                     int family,
+                                     int orport_reachable)
+{
+  extend_info_t *ei = extend_info_from_router(me, family);
+  int ipv6_flags = (family == AF_INET6 ? CIRCLAUNCH_IS_IPV6_SELFTEST : 0);
+
+  /* If we're trying to test IPv6, but we don't have an IPv6 ORPort, ei will
+   * be NULL. */
+  if (ei) {
+    const char *family_name = fmt_af_family(family);
+    log_info(LD_CIRC, "Testing %s of my %s ORPort: %s.",
+             !orport_reachable ? "reachability" : "bandwidth",
+             family_name, fmt_addrport(&ei->addr, ei->port));
+    circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei,
+                                  CIRCLAUNCH_NEED_CAPACITY|
+                                  CIRCLAUNCH_IS_INTERNAL|
+                                  ipv6_flags);
+    extend_info_free(ei);
+  }
+}
+
+/** Launch a self-testing circuit, and ask an exit to connect to our DirPort.
+ * <b>me</b> is our own routerinfo.
+ *
+ * Relays don't advertise IPv6 DirPorts, so this function only supports IPv4.
+ *
+ * See router_do_reachability_checks() for details. */
+static void
+router_do_dirport_reachability_checks(const routerinfo_t *me)
+{
+  tor_addr_port_t my_dirport;
+  tor_addr_from_ipv4h(&my_dirport.addr, me->addr);
+  my_dirport.port = me->dir_port;
+
+  /* If there is already a pending connection, don't open another one. */
+  if (!connection_get_by_type_addr_port_purpose(
+                  CONN_TYPE_DIR,
+                  &my_dirport.addr, my_dirport.port,
+                  DIR_PURPOSE_FETCH_SERVERDESC)) {
+    /* ask myself, via tor, for my server descriptor. */
+    directory_request_t *req =
+      directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC);
+    directory_request_set_dir_addr_port(req, &my_dirport);
+    directory_request_set_directory_id_digest(req,
+                                              me->cache_info.identity_digest);
+    /* ask via an anon circuit, connecting to our dirport. */
+    directory_request_set_indirection(req, DIRIND_ANON_DIRPORT);
+    directory_request_set_resource(req, "authority.z");
+    directory_initiate_request(req);
+    directory_request_free(req);
+  }
+}
+
 /** Some time has passed, or we just got new directory information.
  * See if we currently believe our ORPort or DirPort to be
  * unreachable. If so, launch a new test for it.
@@ -171,83 +255,89 @@ router_do_reachability_checks(int test_or, int test_dir)
 {
   const routerinfo_t *me = router_get_my_routerinfo();
   const or_options_t *options = get_options();
-  int orport_reachable = check_whether_orport_reachable(options);
-  tor_addr_t addr;
+  int orport_reachable = router_should_skip_orport_reachability_check(options);
 
   if (router_should_check_reachability(test_or, test_dir)) {
     if (test_or && (!orport_reachable || !circuit_enough_testing_circs())) {
-      extend_info_t *ei = extend_info_from_router(me);
-      /* XXX IPv6 self testing */
-      log_info(LD_CIRC, "Testing %s of my ORPort: %s:%d.",
-               !orport_reachable ? "reachability" : "bandwidth",
-               fmt_addr32(me->addr), me->or_port);
-      circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei,
-                              CIRCLAUNCH_NEED_CAPACITY|CIRCLAUNCH_IS_INTERNAL);
-      extend_info_free(ei);
+      /* At the moment, tor relays believe that they are reachable when they
+       * receive any create cell on an inbound connection. We'll do separate
+       * IPv4 and IPv6 reachability checks in #34067, and make them more
+       * precise. */
+      router_do_orport_reachability_checks(me, AF_INET, orport_reachable);
+      router_do_orport_reachability_checks(me, AF_INET6, orport_reachable);
     }
 
-    /* XXX IPv6 self testing */
-    tor_addr_from_ipv4h(&addr, me->addr);
-    if (test_dir && !check_whether_dirport_reachable(options) &&
-        !connection_get_by_type_addr_port_purpose(
-                  CONN_TYPE_DIR, &addr, me->dir_port,
-                  DIR_PURPOSE_FETCH_SERVERDESC)) {
-      tor_addr_port_t my_orport, my_dirport;
-      memcpy(&my_orport.addr, &addr, sizeof(addr));
-      memcpy(&my_dirport.addr, &addr, sizeof(addr));
-      my_orport.port = me->or_port;
-      my_dirport.port = me->dir_port;
-      /* ask myself, via tor, for my server descriptor. */
-      directory_request_t *req =
-        directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC);
-      directory_request_set_or_addr_port(req, &my_orport);
-      directory_request_set_dir_addr_port(req, &my_dirport);
-      directory_request_set_directory_id_digest(req,
-                                              me->cache_info.identity_digest);
-      // ask via an anon circuit, connecting to our dirport.
-      directory_request_set_indirection(req, DIRIND_ANON_DIRPORT);
-      directory_request_set_resource(req, "authority.z");
-      directory_initiate_request(req);
-      directory_request_free(req);
+    if (test_dir && !router_should_skip_dirport_reachability_check(options)) {
+      router_do_dirport_reachability_checks(me);
     }
   }
 }
 
-/** We've decided to start our reachability testing. If all
- * is set, log this to the user. Return 1 if we did, or 0 if
- * we chose not to log anything. */
+/** If reachability testing is in progress, let the user know that it's
+ * happening.
+ *
+ * If all is set, log a notice-level message. Return 1 if we did, or 0 if
+ * we chose not to log anything, because we were unable to test reachability.
+ */
 int
 inform_testing_reachability(void)
 {
-  char dirbuf[128];
-  char *address;
+  char ipv4_or_buf[TOR_ADDRPORT_BUF_LEN];
+  char ipv6_or_buf[TOR_ADDRPORT_BUF_LEN];
+  char ipv4_dir_buf[TOR_ADDRPORT_BUF_LEN];
+
+  /* There's a race condition here, between:
+   *  - tor launching reachability tests,
+   *  - any circuits actually completing,
+   *  - routerinfo updates, and
+   *  - these log messages.
+   * In rare cases, we might log the wrong ports, log when we didn't actually
+   * start reachability tests, or fail to log after we actually started
+   * reachability tests.
+   *
+   * After we separate the IPv4 and IPv6 reachability flags in #34067, tor
+   * will test any IPv6 address that it discovers after launching reachability
+   * checks. We'll deal with late disabled IPv6 ORPorts and IPv4 DirPorts, and
+   * extra or skipped log messages in #34137.
+   */
   const routerinfo_t *me = router_get_my_routerinfo();
   if (!me)
     return 0;
 
-  address = tor_dup_ip(me->addr);
-  if (!address)
-    return 0;
-
+  /* IPv4 ORPort */
+  strlcpy(ipv4_or_buf, fmt_addr32_port(me->addr, me->or_port),
+          sizeof(ipv4_or_buf));
   control_event_server_status(LOG_NOTICE,
-                              "CHECKING_REACHABILITY ORADDRESS=%s:%d",
-                              address, me->or_port);
-  if (me->dir_port) {
-    tor_snprintf(dirbuf, sizeof(dirbuf), " and DirPort %s:%d",
-                 address, me->dir_port);
-    control_event_server_status(LOG_NOTICE,
-                                "CHECKING_REACHABILITY DIRADDRESS=%s:%d",
-                                address, me->dir_port);
+                              "CHECKING_REACHABILITY ORADDRESS=%s",
+                              ipv4_or_buf);
+  /* IPv6 ORPort */
+  const bool has_ipv6 = tor_addr_port_is_valid(&me->ipv6_addr,
+                                               me->ipv6_orport, 0);
+  if (has_ipv6) {
+    strlcpy(ipv6_or_buf, fmt_addrport(&me->ipv6_addr, me->ipv6_orport),
+            sizeof(ipv6_or_buf));
+    /* We'll add an IPv6 control event in #34068. */
   }
-  log_notice(LD_OR, "Now checking whether ORPort %s:%d%s %s reachable... "
-                         "(this may take up to %d minutes -- look for log "
-                         "messages indicating success)",
-      address, me->or_port,
-      me->dir_port ? dirbuf : "",
-      me->dir_port ? "are" : "is",
-      TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT/60);
+  /* IPv4 DirPort (there are no advertised IPv6 DirPorts) */
+  if (me->dir_port) {
+    strlcpy(ipv4_dir_buf, fmt_addr32_port(me->addr, me->dir_port),
+            sizeof(ipv4_dir_buf));
+    control_event_server_status(LOG_NOTICE,
+                                "CHECKING_REACHABILITY DIRADDRESS=%s",
+                                ipv4_dir_buf);
+  }
+  log_notice(LD_OR, "Now checking whether ORPort%s %s%s%s%s%s %s reachable... "
+             "(this may take up to %d minutes -- look for log "
+             "messages indicating success)",
+             has_ipv6 ? "s" : "",
+             ipv4_or_buf,
+             has_ipv6 ? " and " : "",
+             has_ipv6 ? ipv6_or_buf : "",
+             me->dir_port ? " and DirPort " : "",
+             me->dir_port ? ipv4_dir_buf : "",
+             has_ipv6 || me->dir_port ? "are" : "is",
+             TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT/60);
 
-  tor_free(address);
   return 1;
 }
 
@@ -266,7 +356,7 @@ router_orport_found_reachable(void)
     log_notice(LD_OR,"Self-testing indicates your ORPort is reachable from "
                "the outside. Excellent.%s",
                options->PublishServerDescriptor_ != NO_DIRINFO
-               && check_whether_dirport_reachable(options) ?
+               && router_should_skip_dirport_reachability_check(options) ?
                  " Publishing server descriptor." : "");
     can_reach_or_port = 1;
     mark_my_descriptor_dirty("ORPort found reachable");
@@ -275,6 +365,7 @@ router_orport_found_reachable(void)
     if (options->TestingTorNetwork == 1) {
       reschedule_descriptor_update_check();
     }
+    /* We'll add an IPv6 event in #34068. */
     control_event_server_status(LOG_NOTICE,
                                 "REACHABILITY_SUCCEEDED ORADDRESS=%s:%d",
                                 address, me->or_port);
@@ -297,7 +388,7 @@ router_dirport_found_reachable(void)
     log_notice(LD_DIRSERV,"Self-testing indicates your DirPort is reachable "
                "from the outside. Excellent.%s",
                options->PublishServerDescriptor_ != NO_DIRINFO
-               && check_whether_orport_reachable(options) ?
+               && router_should_skip_orport_reachability_check(options) ?
                " Publishing server descriptor." : "");
     can_reach_dir_port = 1;
     if (router_should_advertise_dirport(options, me->dir_port)) {
@@ -316,7 +407,9 @@ router_dirport_found_reachable(void)
 }
 
 /** We have enough testing circuits open. Send a bunch of "drop"
- * cells down each of them, to exercise our bandwidth. */
+ * cells down each of them, to exercise our bandwidth.
+ *
+ * May use IPv4 and IPv6 testing circuits (if available). */
 void
 router_perform_bandwidth_test(int num_circs, time_t now)
 {

src/feature/relay/selftest.h

@@ -15,8 +15,10 @@
 #ifdef HAVE_MODULE_RELAY
 
 struct or_options_t;
-int check_whether_orport_reachable(const struct or_options_t *options);
-int check_whether_dirport_reachable(const struct or_options_t *options);
+int router_should_skip_orport_reachability_check(
+                                         const struct or_options_t *options);
+int router_should_skip_dirport_reachability_check(
+                                         const struct or_options_t *options);
 
 void router_do_reachability_checks(int test_or, int test_dir);
 void router_perform_bandwidth_test(int num_circs, time_t now);
@@ -29,9 +31,9 @@ void router_reset_reachability(void);
 
 #else /* !defined(HAVE_MODULE_RELAY) */
 
-#define check_whether_orport_reachable(opts) \
+#define router_should_skip_orport_reachability_check(opts) \
   ((void)(opts), 0)
-#define check_whether_dirport_reachable(opts) \
+#define router_should_skip_dirport_reachability_check(opts) \
   ((void)(opts), 0)
 
 static inline void

src/feature/stats/predict_ports.c

@@ -270,10 +270,10 @@ rep_hist_circbuilding_dormant(time_t now)
 
   /* see if we'll still need to build testing circuits */
   if (server_mode(options) &&
-      (!check_whether_orport_reachable(options) ||
+      (!router_should_skip_orport_reachability_check(options) ||
        !circuit_enough_testing_circs()))
     return 0;
-  if (!check_whether_dirport_reachable(options))
+  if (!router_should_skip_dirport_reachability_check(options))
     return 0;
 
   return 1;

src/lib/net/address.c

@@ -1217,20 +1217,28 @@ fmt_addr32(uint32_t addr)
   return buf;
 }
 
-/** Return a string representing the family of <b>addr</b>.
+/** Like fmt_addrport(), but takes <b>addr</b> as a host-order IPv4
+ * addresses. Also not thread-safe, also clobbers its return buffer on
+ * repeated calls. */
+const char *
+fmt_addr32_port(uint32_t addr, uint16_t port)
+{
+  static char buf[INET_NTOA_BUF_LEN + 6];
+  snprintf(buf, sizeof(buf), "%s:%u", fmt_addr32(addr), port);
+  return buf;
+}
+
+/** Return a string representing <b>family</b>.
  *
  * This string is a string constant, and must not be freed.
  * This function is thread-safe.
  */
 const char *
-fmt_addr_family(const tor_addr_t *addr)
+fmt_af_family(sa_family_t family)
 {
   static int default_bug_once = 0;
 
-  IF_BUG_ONCE(!addr)
-    return "NULL pointer";
-
-  switch (tor_addr_family(addr)) {
+  switch (family) {
     case AF_INET6:
       return "IPv6";
     case AF_INET:
@@ -1242,7 +1250,7 @@ fmt_addr_family(const tor_addr_t *addr)
     default:
       if (!default_bug_once) {
         log_warn(LD_BUG, "Called with unknown address family %d",
-                 (int)tor_addr_family(addr));
+                 (int)family);
         default_bug_once = 1;
       }
       return "unknown";
@@ -1250,6 +1258,20 @@ fmt_addr_family(const tor_addr_t *addr)
   //return "(unreachable code)";
 }
 
+/** Return a string representing the family of <b>addr</b>.
+ *
+ * This string is a string constant, and must not be freed.
+ * This function is thread-safe.
+ */
+const char *
+fmt_addr_family(const tor_addr_t *addr)
+{
+  IF_BUG_ONCE(!addr)
+    return "NULL pointer";
+
+  return fmt_af_family(tor_addr_family(addr));
+}
+
 /** Convert the string in <b>src</b> to a tor_addr_t <b>addr</b>.  The string
  * may be an IPv4 address, or an IPv6 address surrounded by square brackets.
  *

src/lib/net/address.h

@@ -236,6 +236,8 @@ const char *fmt_addr_impl(const tor_addr_t *addr, int decorate);
 const char *fmt_addrport(const tor_addr_t *addr, uint16_t port);
 #define fmt_addrport_ap(ap) fmt_addrport(&(ap)->addr, (ap)->port)
 const char *fmt_addr32(uint32_t addr);
+const char *fmt_addr32_port(uint32_t addr, uint16_t port);
+const char *fmt_af_family(sa_family_t family);
 const char *fmt_addr_family(const tor_addr_t *addr);
 
 MOCK_DECL(int,get_interface_address6,(int severity, sa_family_t family,

src/rust/protover/ffi.rs

@@ -84,7 +84,7 @@ pub extern "C" fn protocol_list_supports_protocol(
     version: uint32_t,
 ) -> c_int {
     if c_protocol_list.is_null() {
-        return 1;
+        return 0;
     }
 
     // Require an unsafe block to read the version from a C string. The pointer
@@ -93,7 +93,7 @@ pub extern "C" fn protocol_list_supports_protocol(
 
     let protocol_list = match c_str.to_str() {
         Ok(n) => n,
-        Err(_) => return 1,
+        Err(_) => return 0,
     };
     let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() {
         Ok(n) => n,
@@ -140,7 +140,7 @@ pub extern "C" fn protocol_list_supports_protocol_or_later(
     version: uint32_t,
 ) -> c_int {
     if c_protocol_list.is_null() {
-        return 1;
+        return 0;
     }
 
     // Require an unsafe block to read the version from a C string. The pointer
@@ -149,7 +149,7 @@ pub extern "C" fn protocol_list_supports_protocol_or_later(
 
     let protocol_list = match c_str.to_str() {
         Ok(n) => n,
-        Err(_) => return 1,
+        Err(_) => return 0,
     };
 
     let protocol = match translate_to_rust(c_protocol) {
@@ -159,7 +159,7 @@ pub extern "C" fn protocol_list_supports_protocol_or_later(
 
     let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() {
         Ok(n) => n,
-        Err(_) => return 1,
+        Err(_) => return 0,
     };
 
     if proto_entry.supports_protocol_or_later(&protocol.into(), &version) {

src/rust/protover/protover.rs

@@ -163,13 +163,13 @@ pub(crate) fn get_supported_protocols_cstr() -> &'static CStr {
              DirCache=1-2 \
              FlowCtrl=1 \
              HSDir=1-2 \
-             HSIntro=3-4 \
+             HSIntro=3-5 \
              HSRend=1-2 \
              Link=1-5 \
              LinkAuth=3 \
              Microdesc=1-2 \
              Padding=2 \
-             Relay=1-2"
+             Relay=1-3"
         )
     } else {
         cstr!(
@@ -178,13 +178,13 @@ pub(crate) fn get_supported_protocols_cstr() -> &'static CStr {
              DirCache=1-2 \
              FlowCtrl=1 \
              HSDir=1-2 \
-             HSIntro=3-4 \
+             HSIntro=3-5 \
              HSRend=1-2 \
              Link=1-5 \
              LinkAuth=1,3 \
              Microdesc=1-2 \
              Padding=2 \
-             Relay=1-2"
+             Relay=1-3"
         )
     }
 }

src/test/test_circuitbuild.c

@@ -19,6 +19,7 @@
 #include "core/or/channel.h"
 #include "core/or/circuitbuild.h"
 #include "core/or/circuitlist.h"
+#include "core/or/circuituse.h"
 #include "core/or/onion.h"
 
 #include "core/or/cell_st.h"
@@ -29,6 +30,7 @@
 
 #include "feature/client/entrynodes.h"
 #include "feature/nodelist/nodelist.h"
+#include "feature/nodelist/node_select.h"
 #include "feature/relay/circuitbuild_relay.h"
 #include "feature/relay/router.h"
 #include "feature/relay/routermode.h"
@@ -279,10 +281,10 @@ mock_node_get_by_id(const char *identity_digest)
   return mocked_node;
 }
 
-static int mocked_supports_ed25519_link_authentication = 0;
-static int
+static bool mocked_supports_ed25519_link_authentication = 0;
+static bool
 mock_node_supports_ed25519_link_authentication(const node_t *node,
-                                                int compatible_with_us)
+                                               bool compatible_with_us)
 {
   (void)node;
   (void)compatible_with_us;
@@ -1176,6 +1178,8 @@ mock_channel_get_canonical_remote_descr(channel_t *chan)
   return "mock_channel_get_canonical_remote_descr()";
 }
 
+/* Should mock_circuit_deliver_create_cell() expect a direct connection? */
+static bool mock_circuit_deliver_create_cell_expect_direct = false;
 static int mock_circuit_deliver_create_cell_calls = 0;
 static int mock_circuit_deliver_create_cell_result = 0;
 static int
@@ -1188,10 +1192,13 @@ mock_circuit_deliver_create_cell(circuit_t *circ,
   /* circuit_deliver_create_cell() requires non-NULL arguments,
    * but we only check circ and circ->n_chan here. */
   tt_ptr_op(circ, OP_NE, NULL);
-  tt_ptr_op(circ->n_chan, OP_NE, NULL);
+  /* We expect n_chan for relayed cells. But should we also expect it for
+   * direct connections? */
+  if (!mock_circuit_deliver_create_cell_expect_direct)
+    tt_ptr_op(circ->n_chan, OP_NE, NULL);
 
   /* We should only ever get relayed cells from extends */
-  tt_int_op(relayed, OP_EQ, 1);
+  tt_int_op(relayed, OP_EQ, !mock_circuit_deliver_create_cell_expect_direct);
 
   mock_circuit_deliver_create_cell_calls++;
   return mock_circuit_deliver_create_cell_result;
@@ -1352,6 +1359,7 @@ test_circuit_extend(void *arg)
 
   /* Mock circuit_deliver_create_cell(), so it doesn't crash */
   mock_circuit_deliver_create_cell_calls = 0;
+  mock_circuit_deliver_create_cell_expect_direct = false;
   MOCK(circuit_deliver_create_cell, mock_circuit_deliver_create_cell);
 
   /* Test circuit established, re-using channel, successful delivery */
@@ -1516,6 +1524,355 @@ test_onionskin_answer(void *arg)
   tor_free(or_circ);
 }
 
+/* Test the different cases in origin_circuit_init(). */
+static void
+test_origin_circuit_init(void *arg)
+{
+  (void)arg;
+  origin_circuit_t *origin_circ = NULL;
+
+  /* Init with 0 purpose and 0 flags */
+  origin_circ = origin_circuit_init(0, 0);
+  tt_int_op(origin_circ->base_.purpose, OP_EQ, 0);
+  tt_int_op(origin_circ->base_.state, OP_EQ, CIRCUIT_STATE_CHAN_WAIT);
+  tt_ptr_op(origin_circ->build_state, OP_NE, NULL);
+  tt_int_op(origin_circ->build_state->is_internal, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->is_ipv6_selftest, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_capacity, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_uptime, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->onehop_tunnel, OP_EQ, 0);
+  /* The circuits are automatically freed by the circuitlist. */
+
+  /* Init with a purpose */
+  origin_circ = origin_circuit_init(CIRCUIT_PURPOSE_C_GENERAL, 0);
+  tt_int_op(origin_circ->base_.purpose, OP_EQ, CIRCUIT_PURPOSE_C_GENERAL);
+
+  /* Init with each flag */
+  origin_circ = origin_circuit_init(0, CIRCLAUNCH_IS_INTERNAL);
+  tt_ptr_op(origin_circ->build_state, OP_NE, NULL);
+  tt_int_op(origin_circ->build_state->is_internal, OP_EQ, 1);
+  tt_int_op(origin_circ->build_state->is_ipv6_selftest, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_capacity, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_uptime, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->onehop_tunnel, OP_EQ, 0);
+
+  origin_circ = origin_circuit_init(0, CIRCLAUNCH_IS_IPV6_SELFTEST);
+  tt_ptr_op(origin_circ->build_state, OP_NE, NULL);
+  tt_int_op(origin_circ->build_state->is_internal, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->is_ipv6_selftest, OP_EQ, 1);
+  tt_int_op(origin_circ->build_state->need_capacity, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_uptime, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->onehop_tunnel, OP_EQ, 0);
+
+  origin_circ = origin_circuit_init(0, CIRCLAUNCH_NEED_CAPACITY);
+  tt_ptr_op(origin_circ->build_state, OP_NE, NULL);
+  tt_int_op(origin_circ->build_state->is_internal, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->is_ipv6_selftest, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_capacity, OP_EQ, 1);
+  tt_int_op(origin_circ->build_state->need_uptime, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->onehop_tunnel, OP_EQ, 0);
+
+  origin_circ = origin_circuit_init(0, CIRCLAUNCH_NEED_UPTIME);
+  tt_ptr_op(origin_circ->build_state, OP_NE, NULL);
+  tt_int_op(origin_circ->build_state->is_internal, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->is_ipv6_selftest, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_capacity, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_uptime, OP_EQ, 1);
+  tt_int_op(origin_circ->build_state->onehop_tunnel, OP_EQ, 0);
+
+  origin_circ = origin_circuit_init(0, CIRCLAUNCH_ONEHOP_TUNNEL);
+  tt_ptr_op(origin_circ->build_state, OP_NE, NULL);
+  tt_int_op(origin_circ->build_state->is_internal, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->is_ipv6_selftest, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_capacity, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->need_uptime, OP_EQ, 0);
+  tt_int_op(origin_circ->build_state->onehop_tunnel, OP_EQ, 1);
+
+ done:
+  /* The circuits are automatically freed by the circuitlist. */
+  ;
+}
+
+/* Test the different cases in circuit_send_next_onion_skin(). */
+static void
+test_circuit_send_next_onion_skin(void *arg)
+{
+  (void)arg;
+  origin_circuit_t *origin_circ = NULL;
+  struct timeval circ_start_time;
+  memset(&circ_start_time, 0, sizeof(circ_start_time));
+
+  extend_info_t fakehop;
+  memset(&fakehop, 0, sizeof(fakehop));
+  extend_info_t *single_fakehop = &fakehop;
+  extend_info_t *multi_fakehop[DEFAULT_ROUTE_LEN] = {&fakehop,
+                                                     &fakehop,
+                                                     &fakehop};
+
+  extend_info_t ipv6_hop;
+  memset(&ipv6_hop, 0, sizeof(ipv6_hop));
+  tor_addr_make_null(&ipv6_hop.addr, AF_INET6);
+  extend_info_t *multi_ipv6_hop[DEFAULT_ROUTE_LEN] = {&ipv6_hop,
+                                                      &ipv6_hop,
+                                                      &ipv6_hop};
+
+  extend_info_t ipv4_hop;
+  memset(&ipv4_hop, 0, sizeof(ipv4_hop));
+  tor_addr_make_null(&ipv4_hop.addr, AF_INET);
+  extend_info_t *multi_ipv4_hop[DEFAULT_ROUTE_LEN] = {&ipv4_hop,
+                                                      &ipv4_hop,
+                                                      &ipv4_hop};
+
+  mock_circuit_deliver_create_cell_expect_direct = false;
+  MOCK(circuit_deliver_create_cell, mock_circuit_deliver_create_cell);
+  server = 0;
+  MOCK(server_mode, mock_server_mode);
+
+  /* Try a direct connection, and succeed on a client */
+  server = 0;
+  origin_circ = new_test_origin_circuit(false,
+                                        circ_start_time,
+                                        1,
+                                        &single_fakehop);
+  tt_ptr_op(origin_circ, OP_NE, NULL);
+  /* Skip some of the multi-hop checks */
+  origin_circ->build_state->onehop_tunnel = 1;
+  /* This is a direct connection */
+  mock_circuit_deliver_create_cell_expect_direct = true;
+  tt_int_op(circuit_send_next_onion_skin(origin_circ), OP_EQ, 0);
+  /* The circuits are automatically freed by the circuitlist. */
+
+  /* Try a direct connection, and succeed on a server */
+  server = 1;
+  origin_circ = new_test_origin_circuit(false,
+                                        circ_start_time,
+                                        1,
+                                        &single_fakehop);
+  tt_ptr_op(origin_circ, OP_NE, NULL);
+  origin_circ->build_state->onehop_tunnel = 1;
+  mock_circuit_deliver_create_cell_expect_direct = true;
+  tt_int_op(circuit_send_next_onion_skin(origin_circ), OP_EQ, 0);
+
+  /* Start capturing bugs */
+  setup_full_capture_of_logs(LOG_WARN);
+  tor_capture_bugs_(1);
+
+  /* Try an extend, but fail the client valid address family check */
+  server = 0;
+  origin_circ = new_test_origin_circuit(true,
+                                        circ_start_time,
+                                        ARRAY_LENGTH(multi_fakehop),
+                                        multi_fakehop);
+  tt_ptr_op(origin_circ, OP_NE, NULL);
+  /* Fix the state */
+  origin_circ->base_.state = 0;
+  /* This is an indirect connection */
+  mock_circuit_deliver_create_cell_expect_direct = false;
+  /* Fail because the address family is invalid */
+  tt_int_op(circuit_send_next_onion_skin(origin_circ), OP_EQ,
+            -END_CIRC_REASON_INTERNAL);
+  expect_log_msg("Client trying to extend to a non-IPv4 address.\n");
+  mock_clean_saved_logs();
+
+  /* Try an extend, but fail the server valid address check */
+  server = 1;
+  origin_circ = new_test_origin_circuit(true,
+                                        circ_start_time,
+                                        ARRAY_LENGTH(multi_fakehop),
+                                        multi_fakehop);
+  tt_ptr_op(origin_circ, OP_NE, NULL);
+  origin_circ->base_.state = 0;
+  mock_circuit_deliver_create_cell_expect_direct = false;
+  tt_int_op(circuit_send_next_onion_skin(origin_circ), OP_EQ,
+            -END_CIRC_REASON_INTERNAL);
+  expect_log_msg("Server trying to extend to an invalid address family.\n");
+  mock_clean_saved_logs();
+
+  /* Try an extend, but fail in the client code, with an IPv6 address */
+  server = 0;
+  origin_circ = new_test_origin_circuit(true,
+                                        circ_start_time,
+                                        ARRAY_LENGTH(multi_ipv6_hop),
+                                        multi_ipv6_hop);
+  tt_ptr_op(origin_circ, OP_NE, NULL);
+  origin_circ->base_.state = 0;
+  mock_circuit_deliver_create_cell_expect_direct = false;
+  tt_int_op(circuit_send_next_onion_skin(origin_circ), OP_EQ,
+            -END_CIRC_REASON_INTERNAL);
+  expect_log_msg("Client trying to extend to a non-IPv4 address.\n");
+  mock_clean_saved_logs();
+
+  /* Stop capturing bugs, but keep capturing logs */
+  tor_end_capture_bugs_();
+
+  /* Try an extend, pass the client IPv4 check, but fail later */
+  server = 0;
+  origin_circ = new_test_origin_circuit(true,
+                                        circ_start_time,
+                                        ARRAY_LENGTH(multi_ipv4_hop),
+                                        multi_ipv4_hop);
+  tt_ptr_op(origin_circ, OP_NE, NULL);
+  origin_circ->base_.state = 0;
+  mock_circuit_deliver_create_cell_expect_direct = false;
+  /* Fail because the circuit data is invalid */
+  tt_int_op(circuit_send_next_onion_skin(origin_circ), OP_EQ,
+            -END_CIRC_REASON_INTERNAL);
+  expect_log_msg("onion_skin_create failed.\n");
+  mock_clean_saved_logs();
+
+  /* Try an extend, pass the server IPv4 check, but fail later */
+  server = 1;
+  origin_circ = new_test_origin_circuit(true,
+                                        circ_start_time,
+                                        ARRAY_LENGTH(multi_ipv4_hop),
+                                        multi_ipv4_hop);
+  tt_ptr_op(origin_circ, OP_NE, NULL);
+  origin_circ->base_.state = 0;
+  mock_circuit_deliver_create_cell_expect_direct = false;
+  tt_int_op(circuit_send_next_onion_skin(origin_circ), OP_EQ,
+            -END_CIRC_REASON_INTERNAL);
+  expect_log_msg("onion_skin_create failed.\n");
+  mock_clean_saved_logs();
+
+  /* Try an extend, pass the server IPv6 check, but fail later */
+  server = 1;
+  origin_circ = new_test_origin_circuit(true,
+                                        circ_start_time,
+                                        ARRAY_LENGTH(multi_ipv6_hop),
+                                        multi_ipv6_hop);
+  tt_ptr_op(origin_circ, OP_NE, NULL);
+  origin_circ->base_.state = 0;
+  mock_circuit_deliver_create_cell_expect_direct = false;
+  tt_int_op(circuit_send_next_onion_skin(origin_circ), OP_EQ,
+            -END_CIRC_REASON_INTERNAL);
+  expect_log_msg("onion_skin_create failed.\n");
+  mock_clean_saved_logs();
+
+  /* Things we're not testing right now:
+   * - the addresses in the extend cell inside
+   *   circuit_send_intermediate_onion_skin() matches the address in the
+   *   supplied extend_info.
+   * - valid circuit data.
+   * - actually extending the circuit to each hop. */
+
+ done:
+  tor_end_capture_bugs_();
+  mock_clean_saved_logs();
+  teardown_capture_of_logs();
+
+  UNMOCK(circuit_deliver_create_cell);
+  UNMOCK(server_mode);
+  server = 0;
+
+  /* The circuits are automatically freed by the circuitlist. */
+}
+
+/* Test the different cases in cpath_build_state_to_crn_flags(). */
+static void
+test_cpath_build_state_to_crn_flags(void *arg)
+{
+  (void)arg;
+
+  cpath_build_state_t state;
+  memset(&state, 0, sizeof(state));
+
+  tt_int_op(cpath_build_state_to_crn_flags(&state), OP_EQ,
+            0);
+
+  memset(&state, 0, sizeof(state));
+  state.need_uptime = 1;
+  tt_int_op(cpath_build_state_to_crn_flags(&state), OP_EQ,
+            CRN_NEED_UPTIME);
+
+  memset(&state, 0, sizeof(state));
+  state.need_capacity = 1;
+  tt_int_op(cpath_build_state_to_crn_flags(&state), OP_EQ,
+            CRN_NEED_CAPACITY);
+
+  memset(&state, 0, sizeof(state));
+  state.need_capacity = 1;
+  state.need_uptime = 1;
+  tt_int_op(cpath_build_state_to_crn_flags(&state), OP_EQ,
+            CRN_NEED_CAPACITY | CRN_NEED_UPTIME);
+
+  /* Check that no other flags are handled */
+  memset(&state, 0xff, sizeof(state));
+  tt_int_op(cpath_build_state_to_crn_flags(&state), OP_EQ,
+            CRN_NEED_CAPACITY | CRN_NEED_UPTIME);
+
+ done:
+  ;
+}
+
+/* Test the different cases in cpath_build_state_to_crn_ipv6_extend_flag(). */
+static void
+test_cpath_build_state_to_crn_ipv6_extend_flag(void *arg)
+{
+  (void)arg;
+
+  cpath_build_state_t state;
+
+  memset(&state, 0, sizeof(state));
+  state.desired_path_len = DEFAULT_ROUTE_LEN;
+  tt_int_op(cpath_build_state_to_crn_ipv6_extend_flag(&state, 0), OP_EQ,
+            0);
+
+  /* Pass the state flag check, but not the length check */
+  memset(&state, 0, sizeof(state));
+  state.desired_path_len = DEFAULT_ROUTE_LEN;
+  state.is_ipv6_selftest = 1;
+  tt_int_op(cpath_build_state_to_crn_ipv6_extend_flag(&state, 0), OP_EQ,
+            0);
+
+  /* Pass the length check, but not the state flag check */
+  memset(&state, 0, sizeof(state));
+  state.desired_path_len = DEFAULT_ROUTE_LEN;
+  tt_int_op(
+      cpath_build_state_to_crn_ipv6_extend_flag(&state,
+                                                DEFAULT_ROUTE_LEN - 2),
+      OP_EQ, 0);
+
+  /* Pass both checks */
+  memset(&state, 0, sizeof(state));
+  state.desired_path_len = DEFAULT_ROUTE_LEN;
+  state.is_ipv6_selftest = 1;
+  tt_int_op(
+      cpath_build_state_to_crn_ipv6_extend_flag(&state,
+                                                DEFAULT_ROUTE_LEN - 2),
+      OP_EQ, CRN_INITIATE_IPV6_EXTEND);
+
+  /* Check that no other flags are handled */
+  memset(&state, 0xff, sizeof(state));
+  state.desired_path_len = INT_MAX;
+  tt_int_op(cpath_build_state_to_crn_ipv6_extend_flag(&state, INT_MAX), OP_EQ,
+            0);
+
+#ifndef ALL_BUGS_ARE_FATAL
+  /* Start capturing bugs */
+  setup_full_capture_of_logs(LOG_INFO);
+  tor_capture_bugs_(1);
+
+  /* Now test the single hop circuit case */
+#define SINGLE_HOP_ROUTE_LEN 1
+  memset(&state, 0, sizeof(state));
+  state.desired_path_len = SINGLE_HOP_ROUTE_LEN;
+  state.is_ipv6_selftest = 1;
+  tt_int_op(
+      cpath_build_state_to_crn_ipv6_extend_flag(&state,
+                                                SINGLE_HOP_ROUTE_LEN - 2),
+      OP_EQ, 0);
+  tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1);
+  tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ,
+            "!(ASSERT_PREDICT_UNLIKELY_(state->desired_path_len < 2))");
+  mock_clean_saved_logs();
+#endif /* !defined(ALL_BUGS_ARE_FATAL) */
+
+ done:
+  tor_end_capture_bugs_();
+  mock_clean_saved_logs();
+  teardown_capture_of_logs();
+}
+
 #define TEST(name, flags, setup, cleanup) \
   { #name, test_ ## name, flags, setup, cleanup }
 
@@ -1525,6 +1882,9 @@ test_onionskin_answer(void *arg)
 #define TEST_CIRCUIT(name, flags) \
   { #name, test_circuit_ ## name, flags, NULL, NULL }
 
+#define TEST_CPATH(name, flags) \
+  { #name, test_cpath_ ## name, flags, NULL, NULL }
+
 #ifndef COCCI
 #define TEST_CIRCUIT_PASSTHROUGH(name, flags, arg) \
   { #name "/" arg, test_circuit_ ## name, flags, \
@@ -1543,12 +1903,19 @@ struct testcase_t circuitbuild_tests[] = {
   TEST_CIRCUIT(extend_add_ed25519, TT_FORK),
   TEST_CIRCUIT(extend_lspec_valid, TT_FORK),
   TEST_CIRCUIT(choose_ip_ap_for_extend, 0),
+
   TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "4"),
   TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "6"),
   TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "dual-stack"),
+
   TEST_CIRCUIT(extend, TT_FORK),
 
   TEST(onionskin_answer, TT_FORK, NULL, NULL),
 
+  TEST(origin_circuit_init, TT_FORK, NULL, NULL),
+  TEST_CIRCUIT(send_next_onion_skin, TT_FORK),
+  TEST_CPATH(build_state_to_crn_flags, 0),
+  TEST_CPATH(build_state_to_crn_ipv6_extend_flag, TT_FORK),
+
   END_OF_TESTCASES
 };

src/test/test_circuitstats.c

@@ -17,18 +17,13 @@
 #include "core/or/circuituse.h"
 #include "core/or/channel.h"
 
-#include "core/or/cpath_build_state_st.h"
 #include "core/or/crypt_path_st.h"
 #include "core/or/extend_info_st.h"
 #include "core/or/origin_circuit_st.h"
 
-void test_circuitstats_timeout(void *arg);
-void test_circuitstats_hoplen(void *arg);
-origin_circuit_t *subtest_fourhop_circuit(struct timeval, int);
-origin_circuit_t *add_opened_threehop(void);
-origin_circuit_t *build_unopened_fourhop(struct timeval);
-
-int cpath_append_hop(crypt_path_t **head_ptr, extend_info_t *choice);
+static origin_circuit_t *add_opened_threehop(void);
+static origin_circuit_t *build_unopened_fourhop(struct timeval);
+static origin_circuit_t *subtest_fourhop_circuit(struct timeval, int);
 
 static int marked_for_close;
 /* Mock function because we are not trying to test the close circuit that does
@@ -45,85 +40,71 @@ mock_circuit_mark_for_close(circuit_t *circ, int reason, int line,
   return;
 }
 
-origin_circuit_t *
+static origin_circuit_t *
 add_opened_threehop(void)
 {
-  origin_circuit_t *or_circ = origin_circuit_new();
+  struct timeval circ_start_time;
+  memset(&circ_start_time, 0, sizeof(circ_start_time));
   extend_info_t fakehop;
   memset(&fakehop, 0, sizeof(fakehop));
+  extend_info_t *fakehop_list[DEFAULT_ROUTE_LEN] = {&fakehop,
+                                                    &fakehop,
+                                                    &fakehop};
 
-  TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL;
-
-  or_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t));
-  or_circ->build_state->desired_path_len = DEFAULT_ROUTE_LEN;
-
-  cpath_append_hop(&or_circ->cpath, &fakehop);
-  cpath_append_hop(&or_circ->cpath, &fakehop);
-  cpath_append_hop(&or_circ->cpath, &fakehop);
-
-  or_circ->has_opened = 1;
-  TO_CIRCUIT(or_circ)->state = CIRCUIT_STATE_OPEN;
-  TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL;
-
-  return or_circ;
+  return new_test_origin_circuit(true,
+                                 circ_start_time,
+                                 DEFAULT_ROUTE_LEN,
+                                 fakehop_list);
 }
 
-origin_circuit_t *
+static origin_circuit_t *
 build_unopened_fourhop(struct timeval circ_start_time)
 {
-  origin_circuit_t *or_circ = origin_circuit_new();
-  extend_info_t *fakehop = tor_malloc_zero(sizeof(extend_info_t));
-  memset(fakehop, 0, sizeof(extend_info_t));
+  extend_info_t fakehop;
+  memset(&fakehop, 0, sizeof(fakehop));
+  extend_info_t *fakehop_list[4] = {&fakehop,
+                                    &fakehop,
+                                    &fakehop,
+                                    &fakehop};
 
-  TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL;
-  TO_CIRCUIT(or_circ)->timestamp_began = circ_start_time;
-  TO_CIRCUIT(or_circ)->timestamp_created = circ_start_time;
-
-  or_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t));
-  or_circ->build_state->desired_path_len = 4;
-
-  cpath_append_hop(&or_circ->cpath, fakehop);
-  cpath_append_hop(&or_circ->cpath, fakehop);
-  cpath_append_hop(&or_circ->cpath, fakehop);
-  cpath_append_hop(&or_circ->cpath, fakehop);
-
-  tor_free(fakehop);
-
-  return or_circ;
+  return new_test_origin_circuit(false,
+                                 circ_start_time,
+                                 4,
+                                 fakehop_list);
 }
 
-origin_circuit_t *
+static origin_circuit_t *
 subtest_fourhop_circuit(struct timeval circ_start_time, int should_timeout)
 {
-  origin_circuit_t *or_circ = build_unopened_fourhop(circ_start_time);
+  origin_circuit_t *origin_circ = build_unopened_fourhop(circ_start_time);
 
   // Now make them open one at a time and call
   // circuit_build_times_handle_completed_hop();
-  or_circ->cpath->state = CPATH_STATE_OPEN;
-  circuit_build_times_handle_completed_hop(or_circ);
+  origin_circ->cpath->state = CPATH_STATE_OPEN;
+  circuit_build_times_handle_completed_hop(origin_circ);
   tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 0);
 
-  or_circ->cpath->next->state = CPATH_STATE_OPEN;
-  circuit_build_times_handle_completed_hop(or_circ);
+  origin_circ->cpath->next->state = CPATH_STATE_OPEN;
+  circuit_build_times_handle_completed_hop(origin_circ);
   tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 0);
 
   // Third hop: We should count it now.
-  or_circ->cpath->next->next->state = CPATH_STATE_OPEN;
-  circuit_build_times_handle_completed_hop(or_circ);
+  origin_circ->cpath->next->next->state = CPATH_STATE_OPEN;
+  circuit_build_times_handle_completed_hop(origin_circ);
   tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ,
             !should_timeout); // 1 if counted, 0 otherwise
 
   // Fourth hop: Don't double count
-  or_circ->cpath->next->next->next->state = CPATH_STATE_OPEN;
-  circuit_build_times_handle_completed_hop(or_circ);
+  origin_circ->cpath->next->next->next->state = CPATH_STATE_OPEN;
+  circuit_build_times_handle_completed_hop(origin_circ);
   tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ,
             !should_timeout);
 
  done:
-  return or_circ;
+  return origin_circ;
 }
 
-void
+static void
 test_circuitstats_hoplen(void *arg)
 {
   /* Plan:

src/test/test_helpers.c

@@ -16,28 +16,35 @@
 #include "core/or/or.h"
 
 #include "lib/buf/buffers.h"
-#include "app/config/config.h"
 #include "lib/confmgt/confmgt.h"
-#include "app/main/subsysmgr.h"
-#include "core/mainloop/connection.h"
-#include "core/or/connection_or.h"
 #include "lib/crypt_ops/crypto_rand.h"
-#include "core/mainloop/mainloop.h"
-#include "feature/nodelist/nodelist.h"
-#include "core/or/relay.h"
-#include "feature/nodelist/routerlist.h"
 #include "lib/dispatch/dispatch.h"
 #include "lib/dispatch/dispatch_naming.h"
-#include "lib/pubsub/pubsub_build.h"
-#include "lib/pubsub/pubsub_connect.h"
 #include "lib/encoding/confline.h"
 #include "lib/net/resolve.h"
+#include "lib/pubsub/pubsub_build.h"
+#include "lib/pubsub/pubsub_connect.h"
+
+#include "core/mainloop/connection.h"
+#include "core/mainloop/mainloop.h"
+#include "core/or/connection_or.h"
+#include "core/or/crypt_path.h"
+#include "core/or/relay.h"
+
+#include "feature/nodelist/nodelist.h"
+#include "feature/nodelist/routerlist.h"
+
+#include "app/config/config.h"
+#include "app/main/subsysmgr.h"
 
 #include "core/or/cell_st.h"
 #include "core/or/connection_st.h"
-#include "core/or/or_connection_st.h"
-#include "feature/nodelist/node_st.h"
+#include "core/or/cpath_build_state_st.h"
+#include "core/or/crypt_path_st.h"
 #include "core/or/origin_circuit_st.h"
+#include "core/or/or_connection_st.h"
+
+#include "feature/nodelist/node_st.h"
 #include "feature/nodelist/routerlist_st.h"
 
 #include "test/test.h"
@@ -441,3 +448,36 @@ helper_cleanup_pubsub(const struct testcase_t *testcase, void *dispatcher_)
 const struct testcase_setup_t helper_pubsub_setup = {
   helper_setup_pubsub, helper_cleanup_pubsub
 };
+
+origin_circuit_t *
+new_test_origin_circuit(bool has_opened,
+                        struct timeval circ_start_time,
+                        int path_len,
+                        extend_info_t **ei_list)
+{
+  origin_circuit_t *origin_circ = origin_circuit_new();
+
+  TO_CIRCUIT(origin_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL;
+
+  origin_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t));
+  origin_circ->build_state->desired_path_len = path_len;
+
+  if (ei_list) {
+    for (int i = 0; i < path_len; i++) {
+      extend_info_t *ei = ei_list[i];
+      cpath_append_hop(&origin_circ->cpath, ei);
+    }
+  }
+
+  if (has_opened) {
+    origin_circ->has_opened = 1;
+    TO_CIRCUIT(origin_circ)->state = CIRCUIT_STATE_OPEN;
+    origin_circ->cpath->state = CPATH_STATE_OPEN;
+  } else {
+    TO_CIRCUIT(origin_circ)->timestamp_began = circ_start_time;
+    TO_CIRCUIT(origin_circ)->timestamp_created = circ_start_time;
+    origin_circ->cpath->state = CPATH_STATE_CLOSED;
+  }
+
+  return origin_circ;
+}

src/test/test_helpers.h

@@ -40,5 +40,10 @@ int helper_cleanup_pubsub(const struct testcase_t *, void *);
 
 extern const struct testcase_setup_t helper_pubsub_setup;
 
+origin_circuit_t *new_test_origin_circuit(bool has_opened,
+                                          struct timeval circ_start_time,
+                                          int path_len,
+                                          extend_info_t **ei_list);
+
 #endif /* !defined(TOR_TEST_HELPERS_H) */
 

src/test/test_protover.c

@@ -7,14 +7,18 @@
 #include "orconfig.h"
 #include "test/test.h"
 
-#include "core/or/protover.h"
-
-#include "core/or/or.h"
-#include "core/or/connection_or.h"
 #include "lib/tls/tortls.h"
 
+#include "core/or/or.h"
+
+#include "core/or/connection_or.h"
+#include "core/or/protover.h"
+#include "core/or/versions.h"
+
 #include "feature/dirauth/dirvote.h"
 
+#include "feature/relay/relay_handshake.h"
+
 static void
 test_protover_parse(void *arg)
 {
@@ -409,23 +413,21 @@ test_protover_supports_version(void *arg)
  * Hard-coded here, because they are not in the code, or not exposed in the
  * headers. */
 #define PROTOVER_LINKAUTH_V1 1
-#define PROTOVER_LINKAUTH_V3 3
-
+#define PROTOVER_LINKAUTH_V2 2
 #define PROTOVER_RELAY_V1 1
-#define PROTOVER_RELAY_V2 2
 
+/* Deprecated HSIntro versions */
+#define PROTOVER_HS_INTRO_DEPRECATED_1 1
+#define PROTOVER_HS_INTRO_DEPRECATED_2 2
 /* Highest supported HSv2 introduce protocol version.
- * Hard-coded here, because it does not appear anywhere in the code.
  * It's not clear if we actually support version 2, see #25068. */
-#define PROTOVER_HSINTRO_V2 3
+#define PROTOVER_HS_INTRO_V2 3
 
-/* HSv2 Rend and HSDir protocol versions.
- * Hard-coded here, because they do not appear anywhere in the code. */
+/* HSv2 Rend and HSDir protocol versions. */
 #define PROTOVER_HS_RENDEZVOUS_POINT_V2 1
 #define PROTOVER_HSDIR_V2 1
 
-/* DirCache, Desc, Microdesc, and Cons protocol versions.
- * Hard-coded here, because they do not appear anywhere in the code. */
+/* DirCache, Desc, Microdesc, and Cons protocol versions. */
 #define PROTOVER_DIRCACHE_V1 1
 #define PROTOVER_DIRCACHE_V2 2
 
@@ -438,6 +440,10 @@ test_protover_supports_version(void *arg)
 #define PROTOVER_CONS_V1 1
 #define PROTOVER_CONS_V2 2
 
+#define PROTOVER_PADDING_V1 1
+
+#define PROTOVER_FLOWCTRL_V1 1
+
 /* Make sure we haven't forgotten any supported protocols */
 static void
 test_protover_supported_protocols(void *arg)
@@ -452,24 +458,27 @@ test_protover_supported_protocols(void *arg)
                                             PRT_LINK,
                                             MAX_LINK_PROTO));
   for (uint16_t i = 0; i < MAX_PROTOCOLS_TO_TEST; i++) {
-    if (is_or_protocol_version_known(i)) {
-      tt_assert(protocol_list_supports_protocol(supported_protocols,
+      tt_int_op(protocol_list_supports_protocol(supported_protocols,
                                                 PRT_LINK,
-                                                i));
-    }
+                                                i),
+                OP_EQ,
+                is_or_protocol_version_known(i));
   }
 
-#ifdef HAVE_WORKING_TOR_TLS_GET_TLSSECRETS
-  /* Legacy LinkAuth does not appear anywhere in the code. */
-  tt_assert(protocol_list_supports_protocol(supported_protocols,
+  /* Legacy LinkAuth is only supported on OpenSSL and similar. */
+  tt_int_op(protocol_list_supports_protocol(supported_protocols,
                                             PRT_LINKAUTH,
-                                            PROTOVER_LINKAUTH_V1));
-#endif /* defined(HAVE_WORKING_TOR_TLS_GET_TLSSECRETS) */
-  /* Latest LinkAuth is not exposed in the headers. */
-  tt_assert(protocol_list_supports_protocol(supported_protocols,
-                                            PRT_LINKAUTH,
-                                            PROTOVER_LINKAUTH_V3));
-  /* Is there any way to test for new LinkAuth? */
+                                            PROTOVER_LINKAUTH_V1),
+            OP_EQ,
+            authchallenge_type_is_supported(AUTHTYPE_RSA_SHA256_TLSSECRET));
+  /* LinkAuth=2 is unused */
+  tt_assert(!protocol_list_supports_protocol(supported_protocols,
+                                             PRT_LINKAUTH,
+                                             PROTOVER_LINKAUTH_V2));
+  tt_assert(
+      protocol_list_supports_protocol(supported_protocols,
+                                     PRT_LINKAUTH,
+                                     PROTOVER_LINKAUTH_ED25519_HANDSHAKE));
 
   /* Relay protovers do not appear anywhere in the code. */
   tt_assert(protocol_list_supports_protocol(supported_protocols,
@@ -477,20 +486,38 @@ test_protover_supported_protocols(void *arg)
                                             PROTOVER_RELAY_V1));
   tt_assert(protocol_list_supports_protocol(supported_protocols,
                                             PRT_RELAY,
-                                            PROTOVER_RELAY_V2));
-  /* Is there any way to test for new Relay? */
+                                            PROTOVER_RELAY_EXTEND2));
+  tt_assert(protocol_list_supports_protocol(supported_protocols,
+                                            PRT_RELAY,
+                                            PROTOVER_RELAY_ACCEPT_IPV6));
+  tt_assert(protocol_list_supports_protocol(supported_protocols,
+                                            PRT_RELAY,
+                                            PROTOVER_RELAY_EXTEND_IPV6));
+  tt_assert(protocol_list_supports_protocol(supported_protocols,
+                                            PRT_RELAY,
+                                            PROTOVER_RELAY_CANONICAL_IPV6));
 
+  /* These HSIntro versions are deprecated */
+  tt_assert(!protocol_list_supports_protocol(supported_protocols,
+                                            PRT_HSINTRO,
+                                            PROTOVER_HS_INTRO_DEPRECATED_1));
+  tt_assert(!protocol_list_supports_protocol(supported_protocols,
+                                            PRT_HSINTRO,
+                                            PROTOVER_HS_INTRO_DEPRECATED_2));
   /* We could test legacy HSIntro by calling rend_service_update_descriptor(),
    * and checking the protocols field. But that's unlikely to change, so
    * we just use a hard-coded value. */
   tt_assert(protocol_list_supports_protocol(supported_protocols,
                                             PRT_HSINTRO,
-                                            PROTOVER_HSINTRO_V2));
+                                            PROTOVER_HS_INTRO_V2));
   /* Test for HSv3 HSIntro */
   tt_assert(protocol_list_supports_protocol(supported_protocols,
                                             PRT_HSINTRO,
                                             PROTOVER_HS_INTRO_V3));
-  /* Is there any way to test for new HSIntro? */
+  /* Test for HSIntro DoS */
+  tt_assert(protocol_list_supports_protocol(supported_protocols,
+                                            PRT_HSINTRO,
+                                            PROTOVER_HS_INTRO_DOS));
 
   /* Legacy HSRend does not appear anywhere in the code. */
   tt_assert(protocol_list_supports_protocol(supported_protocols,
@@ -500,7 +527,6 @@ test_protover_supported_protocols(void *arg)
   tt_assert(protocol_list_supports_protocol(supported_protocols,
                                             PRT_HSREND,
                                             PROTOVER_HS_RENDEZVOUS_POINT_V3));
-  /* Is there any way to test for new HSRend? */
 
   /* Legacy HSDir does not appear anywhere in the code. */
   tt_assert(protocol_list_supports_protocol(supported_protocols,
@@ -510,7 +536,6 @@ test_protover_supported_protocols(void *arg)
   tt_assert(protocol_list_supports_protocol(supported_protocols,
                                             PRT_HSDIR,
                                             PROTOVER_HSDIR_V3));
-  /* Is there any way to test for new HSDir? */
 
   /* No DirCache versions appear anywhere in the code. */
   tt_assert(protocol_list_supports_protocol(supported_protocols,
@@ -519,7 +544,6 @@ test_protover_supported_protocols(void *arg)
   tt_assert(protocol_list_supports_protocol(supported_protocols,
                                             PRT_DIRCACHE,
                                             PROTOVER_DIRCACHE_V2));
-  /* Is there any way to test for new DirCache? */
 
   /* No Desc versions appear anywhere in the code. */
   tt_assert(protocol_list_supports_protocol(supported_protocols,
@@ -537,7 +561,6 @@ test_protover_supported_protocols(void *arg)
   tt_assert(protocol_list_supports_protocol(supported_protocols,
                                             PRT_MICRODESC,
                                             PROTOVER_MICRODESC_V2));
-  /* Is there any way to test for new Microdesc? */
 
   /* No Cons versions appear anywhere in the code. */
   tt_assert(protocol_list_supports_protocol(supported_protocols,
@@ -546,7 +569,19 @@ test_protover_supported_protocols(void *arg)
   tt_assert(protocol_list_supports_protocol(supported_protocols,
                                             PRT_CONS,
                                             PROTOVER_CONS_V2));
-  /* Is there any way to test for new Cons? */
+
+  /* Padding=1 is deprecated. */
+  tt_assert(!protocol_list_supports_protocol(supported_protocols,
+                                             PRT_PADDING,
+                                             PROTOVER_PADDING_V1));
+  tt_assert(protocol_list_supports_protocol(supported_protocols,
+                                            PRT_PADDING,
+                                            PROTOVER_HS_SETUP_PADDING));
+
+  /* FlowCtrl */
+  tt_assert(protocol_list_supports_protocol(supported_protocols,
+                                            PRT_FLOWCTRL,
+                                            PROTOVER_FLOWCTRL_V1));
 
  done:
  ;
@@ -676,6 +711,232 @@ test_protover_vote_roundtrip_ours(void *args)
   tor_free(result);
 }
 
+/* Stringifies its argument.
+ * 4 -> "4" */
+#define STR(x) #x
+
+#ifdef COCCI
+#define PROTOVER(proto_string, version_macro)
+#else
+/* Generate a protocol version string using proto_string and version_macro.
+ * PROTOVER("HSIntro", PROTOVER_HS_INTRO_DOS) -> "HSIntro" "=" "5"
+ * Uses two levels of macros to turn PROTOVER_HS_INTRO_DOS into "5".
+ */
+#define PROTOVER(proto_string, version_macro) \
+  (proto_string "=" STR(version_macro))
+#endif
+
+#define DEBUG_PROTOVER(flags) \
+  STMT_BEGIN \
+  log_debug(LD_GENERAL, \
+            "protovers:\n" \
+            "protocols_known: %d,\n" \
+            "supports_extend2_cells: %d,\n" \
+            "supports_accepting_ipv6_extends: %d,\n" \
+            "supports_initiating_ipv6_extends: %d,\n" \
+            "supports_canonical_ipv6_conns: %d,\n" \
+            "supports_ed25519_link_handshake_compat: %d,\n" \
+            "supports_ed25519_link_handshake_any: %d,\n" \
+            "supports_ed25519_hs_intro: %d,\n" \
+            "supports_establish_intro_dos_extension: %d,\n" \
+            "supports_v3_hsdir: %d,\n" \
+            "supports_v3_rendezvous_point: %d,\n" \
+            "supports_hs_setup_padding: %d.", \
+            (flags).protocols_known, \
+            (flags).supports_extend2_cells, \
+            (flags).supports_accepting_ipv6_extends, \
+            (flags).supports_initiating_ipv6_extends, \
+            (flags).supports_canonical_ipv6_conns, \
+            (flags).supports_ed25519_link_handshake_compat, \
+            (flags).supports_ed25519_link_handshake_any, \
+            (flags).supports_ed25519_hs_intro, \
+            (flags).supports_establish_intro_dos_extension, \
+            (flags).supports_v3_hsdir, \
+            (flags).supports_v3_rendezvous_point, \
+            (flags).supports_hs_setup_padding); \
+    STMT_END
+
+/* Test that the proto_string version version_macro sets summary_flag. */
+#define TEST_PROTOVER(proto_string, version_macro, summary_flag) \
+  STMT_BEGIN \
+  memset(&flags, 0, sizeof(flags)); \
+  summarize_protover_flags(&flags, \
+                           PROTOVER(proto_string, version_macro), \
+                           NULL); \
+  DEBUG_PROTOVER(flags); \
+  tt_int_op(flags.protocols_known, OP_EQ, 1); \
+  tt_int_op(flags.summary_flag, OP_EQ, 1); \
+  flags.protocols_known = 0; \
+  flags.summary_flag = 0; \
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags)); \
+  STMT_END
+
+static void
+test_protover_summarize_flags(void *args)
+{
+  (void) args;
+  char pv[30];
+  memset(&pv, 0, sizeof(pv));
+
+  protover_summary_cache_free_all();
+
+  protover_summary_flags_t zero_flags;
+  memset(&zero_flags, 0, sizeof(zero_flags));
+  protover_summary_flags_t flags;
+
+  memset(&flags, 0, sizeof(flags));
+  summarize_protover_flags(&flags, NULL, NULL);
+  DEBUG_PROTOVER(flags);
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  /* "" sets the protocols_known flag */
+  memset(&flags, 0, sizeof(flags));
+  summarize_protover_flags(&flags, "", "");
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  /* Now clear that flag, and check the rest are zero */
+  flags.protocols_known = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  /* Now check version exceptions */
+
+  /* EXTEND2 cell support */
+  memset(&flags, 0, sizeof(flags));
+  summarize_protover_flags(&flags, NULL, "Tor 0.2.4.8-alpha");
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  tt_int_op(flags.supports_extend2_cells, OP_EQ, 1);
+  /* Now clear those flags, and check the rest are zero */
+  flags.protocols_known = 0;
+  flags.supports_extend2_cells = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  /* disabling HSDir v3 support for buggy versions */
+  memset(&flags, 0, sizeof(flags));
+  summarize_protover_flags(&flags,
+                           PROTOVER("HSDir", PROTOVER_HSDIR_V3),
+                           NULL);
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  tt_int_op(flags.supports_v3_hsdir, OP_EQ, 1);
+  /* Now clear those flags, and check the rest are zero */
+  flags.protocols_known = 0;
+  flags.supports_v3_hsdir = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  memset(&flags, 0, sizeof(flags));
+  summarize_protover_flags(&flags,
+                           PROTOVER("HSDir", PROTOVER_HSDIR_V3),
+                           "Tor 0.3.0.7");
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  /* Now clear that flag, and check the rest are zero */
+  flags.protocols_known = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  /* Now check standard summaries */
+
+  /* LinkAuth */
+  memset(&flags, 0, sizeof(flags));
+  summarize_protover_flags(&flags,
+                           PROTOVER("LinkAuth",
+                                    PROTOVER_LINKAUTH_ED25519_HANDSHAKE),
+                           NULL);
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  tt_int_op(flags.supports_ed25519_link_handshake_compat, OP_EQ, 1);
+  tt_int_op(flags.supports_ed25519_link_handshake_any, OP_EQ, 1);
+  /* Now clear those flags, and check the rest are zero */
+  flags.protocols_known = 0;
+  flags.supports_ed25519_link_handshake_compat = 0;
+  flags.supports_ed25519_link_handshake_any = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  /* Test one greater */
+  memset(&flags, 0, sizeof(flags));
+  snprintf(pv, sizeof(pv),
+           "%s=%d", "LinkAuth", PROTOVER_LINKAUTH_ED25519_HANDSHAKE + 1);
+  summarize_protover_flags(&flags, pv, NULL);
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  tt_int_op(flags.supports_ed25519_link_handshake_compat, OP_EQ, 0);
+  tt_int_op(flags.supports_ed25519_link_handshake_any, OP_EQ, 1);
+  /* Now clear those flags, and check the rest are zero */
+  flags.protocols_known = 0;
+  flags.supports_ed25519_link_handshake_compat = 0;
+  flags.supports_ed25519_link_handshake_any = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  /* Test one less */
+  memset(&flags, 0, sizeof(flags));
+  snprintf(pv, sizeof(pv),
+           "%s=%d", "LinkAuth", PROTOVER_LINKAUTH_ED25519_HANDSHAKE - 1);
+  summarize_protover_flags(&flags, pv, NULL);
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  tt_int_op(flags.supports_ed25519_link_handshake_compat, OP_EQ, 0);
+  tt_int_op(flags.supports_ed25519_link_handshake_any, OP_EQ, 0);
+  /* Now clear those flags, and check the rest are zero */
+  flags.protocols_known = 0;
+  flags.supports_ed25519_link_handshake_compat = 0;
+  flags.supports_ed25519_link_handshake_any = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  /* We don't test "one more" and "one less" for each protocol version.
+   * But that could be a useful thing to add. */
+
+  /* Relay */
+  memset(&flags, 0, sizeof(flags));
+  /* This test relies on these versions being equal */
+  tt_int_op(PROTOVER_RELAY_EXTEND2, OP_EQ, PROTOVER_RELAY_ACCEPT_IPV6);
+  summarize_protover_flags(&flags,
+                           PROTOVER("Relay", PROTOVER_RELAY_EXTEND2), NULL);
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  tt_int_op(flags.supports_extend2_cells, OP_EQ, 1);
+  tt_int_op(flags.supports_accepting_ipv6_extends, OP_EQ, 1);
+  /* Now clear those flags, and check the rest are zero */
+  flags.protocols_known = 0;
+  flags.supports_extend2_cells = 0;
+  flags.supports_accepting_ipv6_extends = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  memset(&flags, 0, sizeof(flags));
+  /* This test relies on these versions being equal */
+  tt_int_op(PROTOVER_RELAY_EXTEND_IPV6, OP_EQ, PROTOVER_RELAY_CANONICAL_IPV6);
+  summarize_protover_flags(&flags,
+                           PROTOVER("Relay", PROTOVER_RELAY_EXTEND_IPV6),
+                           NULL);
+  DEBUG_PROTOVER(flags);
+  tt_int_op(flags.protocols_known, OP_EQ, 1);
+  tt_int_op(flags.supports_accepting_ipv6_extends, OP_EQ, 1);
+  tt_int_op(flags.supports_initiating_ipv6_extends, OP_EQ, 1);
+  tt_int_op(flags.supports_canonical_ipv6_conns, OP_EQ, 1);
+  /* Now clear those flags, and check the rest are zero */
+  flags.protocols_known = 0;
+  flags.supports_accepting_ipv6_extends = 0;
+  flags.supports_initiating_ipv6_extends = 0;
+  flags.supports_canonical_ipv6_conns = 0;
+  tt_mem_op(&flags, OP_EQ, &zero_flags, sizeof(flags));
+
+  TEST_PROTOVER("HSIntro", PROTOVER_HS_INTRO_V3,
+                supports_ed25519_hs_intro);
+  TEST_PROTOVER("HSIntro", PROTOVER_HS_INTRO_DOS,
+                supports_establish_intro_dos_extension);
+
+  TEST_PROTOVER("HSRend", PROTOVER_HS_RENDEZVOUS_POINT_V3,
+                supports_v3_rendezvous_point);
+
+  TEST_PROTOVER("HSDir", PROTOVER_HSDIR_V3,
+                supports_v3_hsdir);
+
+  TEST_PROTOVER("Padding", PROTOVER_HS_SETUP_PADDING,
+                supports_hs_setup_padding);
+
+ done:
+  ;
+}
+
 #define PV_TEST(name, flags)                       \
   { #name, test_protover_ ##name, (flags), NULL, NULL }
 
@@ -690,5 +951,7 @@ struct testcase_t protover_tests[] = {
   PV_TEST(supported_protocols, 0),
   PV_TEST(vote_roundtrip, 0),
   PV_TEST(vote_roundtrip_ours, 0),
+  /* fork, because we memoize flags internally */
+  PV_TEST(summarize_flags, TT_FORK),
   END_OF_TESTCASES
 };