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

9 files changed, 324 insertions, 157 deletions

diff --git a/src/feature/nodelist/node_select.c b/src/feature/nodelist/node_select.c
index e831248413..25904d4c63 100644
--- a/src/feature/nodelist/node_select.c
+++ b/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,64 +930,67 @@ 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).
+/* 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;
+
+  router_add_running_nodes_to_smartlist(sl, flags);
+  log_debug(LD_CIRC,
+           "We found %d running nodes.",
+            smartlist_len(sl));
+
+  nodelist_subtract(sl, excludednodes);
+
+  if (excludedset) {
+    routerset_subtract_nodes(sl,excludedset);
+    log_debug(LD_CIRC,
+              "We removed excludedset, leaving %d nodes.",
+              smartlist_len(sl));
+  }
+
+  // Always weight by bandwidth
+  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)
-{ /* 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();
+{
+  /* 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;
 
-  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);
+  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
@@ -991,41 +998,30 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
   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);
-  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) {
-    routerset_subtract_nodes(sl,excludedset);
-    log_debug(LD_CIRC,
-              "We removed excludedset, leaving %d nodes.",
-              smartlist_len(sl));
-  }
+  choice = router_choose_random_node_helper(excludednodes,
+                                            excludedset,
+                                            flags,
+                                            rule);
 
-  // Always weight by bandwidth
-  choice = node_sl_choose_by_bandwidth(sl, rule);
-
-  smartlist_free(sl);
   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)
diff --git a/src/feature/nodelist/node_select.h b/src/feature/nodelist/node_select.h
index 2e67f990f6..1776d8ea1a 100644
--- a/src/feature/nodelist/node_select.h
+++ b/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
diff --git a/src/feature/nodelist/node_st.h b/src/feature/nodelist/node_st.h
index b1ec4db202..3769f9dc84 100644
--- a/src/feature/nodelist/node_st.h
+++ b/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
diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c
index 7454f342f9..6b2c0d2016 100644
--- a/src/feature/nodelist/nodelist.c
+++ b/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,9 +1193,24 @@ 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> can be a rendezvous point for hidden
+ * service version 3 (HSRend=2). */
+bool
+node_supports_v3_rendezvous_point(const node_t *node)
+{
+  tor_assert(node);
+
+  /* We can't use a v3 rendezvous point without the curve25519 onion pk. */
+  if (!node_get_curve25519_onion_key(node)) {
+    return 0;
+  }
+
+  return node_get_protover_summary_flags(node)->supports_v3_rendezvous_point;
+}
+
 /** Return true iff <b>node</b> supports the DoS ESTABLISH_INTRO cell
  * extenstion. */
-int
+bool
 node_supports_establish_intro_dos_extension(const node_t *node)
 {
   tor_assert(node);
@@ -1204,19 +1219,54 @@ node_supports_establish_intro_dos_extension(const node_t *node)
                            supports_establish_intro_dos_extension;
 }
 
-/** Return true iff <b>node</b> supports to be a rendezvous point for hidden
- * service version 3 (HSRend=2). */
-int
-node_supports_v3_rendezvous_point(const node_t *node)
+/** 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);
 
-  /* We can't use a v3 rendezvous point without the curve25519 onion pk. */
-  if (!node_get_curve25519_onion_key(node)) {
+  /* Relays can't initiate an IPv6 extend, unless they have an IPv6 ORPort. */
+  if (!node_has_ipv6_orport(node)) {
     return 0;
   }
 
-  return node_get_protover_summary_flags(node)->supports_v3_rendezvous_point;
+  /* 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. */
diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h
index 57ab2d5913..4ba699d69d 100644
--- a/src/feature/nodelist/nodelist.h
+++ b/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));
diff --git a/src/feature/nodelist/routerinfo.c b/src/feature/nodelist/routerinfo.c
index 0bf2a977f5..55f21dfe67 100644
--- a/src/feature/nodelist/routerinfo.c
+++ b/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
diff --git a/src/feature/nodelist/routerinfo.h b/src/feature/nodelist/routerinfo.h
index 604e478999..2e12cbeba3 100644
--- a/src/feature/nodelist/routerinfo.h
+++ b/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);
 
diff --git a/src/feature/nodelist/routerlist.c b/src/feature/nodelist/routerlist.c
index 80c1aa6893..ece3379fa5 100644
--- a/src/feature/nodelist/routerlist.c
+++ b/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)
-{
-  const int check_reach = !router_skip_or_reachability(get_options(),
-                                                       pref_addr);
-  /* XXXX MOVE */
+router_add_running_nodes_to_smartlist(smartlist_t *sl, int flags)
+{
   SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) {
-    if (!node->is_running || !node->is_valid)
-      continue;
-    if (need_desc && !node_has_preferred_descriptor(node, direct_conn))
-      continue;
-    if (node->ri && node->ri->purpose != ROUTER_PURPOSE_GENERAL)
+    if (!router_can_choose_node(node, flags))
       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);
 }
diff --git a/src/feature/nodelist/routerlist.h b/src/feature/nodelist/routerlist.h
index 81a2343540..98472b2771 100644
--- a/src/feature/nodelist/routerlist.h
+++ b/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);