Choose OR Entry Guards using IPv4/IPv6 preferences

Update unit tests.

5 files changed, 108 insertions, 25 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index d44fcd74ef..dcb9de3a80 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1778,7 +1778,7 @@ pick_tor2web_rendezvous_node(router_crn_flags_t flags,
   router_add_running_nodes_to_smartlist(all_live_nodes,
                                         allow_invalid,
                                         0, 0, 0,
-                                        need_desc);
+                                        need_desc, 0);
 
   /* Filter all_live_nodes to only add live *and* whitelisted RPs to
    * the list whitelisted_live_rps. */
@@ -2144,7 +2144,9 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
   const node_t *choice;
   smartlist_t *excluded;
   const or_options_t *options = get_options();
-  router_crn_flags_t flags = CRN_NEED_GUARD|CRN_NEED_DESC;
+  /* If possible, choose an entry server with a preferred address,
+   * otherwise, choose one with an allowed address */
+  router_crn_flags_t flags = CRN_NEED_GUARD|CRN_NEED_DESC|CRN_PREF_ADDR;
   const node_t *node;
 
   if (state && options->UseEntryGuards &&
@@ -2161,12 +2163,6 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
      * family. */
     nodelist_add_node_and_family(excluded, node);
   }
-  /* Exclude all ORs that we can't reach through our firewall */
-  smartlist_t *nodes = nodelist_get_list();
-  SMARTLIST_FOREACH(nodes, const node_t *, node, {
-    if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, 0))
-      smartlist_add(excluded, (void*)node);
-  });
   /* and exclude current entry guards and their families,
    * unless we're in a test network, and excluding guards
    * would exclude all nodes (i.e. we're in an incredibly small tor network,
@@ -2332,8 +2328,9 @@ extend_info_from_node(const node_t *node, int for_direct_connect)
   if (node->ri == NULL && (node->rs == NULL || node->md == NULL))
     return NULL;
 
+  /* Choose a preferred address first, but fall back to an allowed address*/
   if (for_direct_connect)
-    node_get_pref_orport(node, &ap);
+    fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0, &ap);
   else
     node_get_prim_orport(node, &ap);
 
diff --git a/src/or/or.h b/src/or/or.h
index b1765d1d57..412789cafe 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -5221,7 +5221,9 @@ typedef enum {
   CRN_ALLOW_INVALID = 1<<3,
   /* XXXX not used, apparently. */
   CRN_WEIGHT_AS_EXIT = 1<<5,
-  CRN_NEED_DESC = 1<<6
+  CRN_NEED_DESC = 1<<6,
+  /* On clients, only provide nodes that satisfy ClientPreferIPv6OR */
+  CRN_PREF_ADDR = 1<<7
 } router_crn_flags_t;
 
 /** Return value for router_add_to_routerlist() and dirserv_add_descriptor() */
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index c45854c52f..804ff29edb 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1826,7 +1826,8 @@ routerlist_add_node_and_family(smartlist_t *sl, const routerinfo_t *router)
 void
 router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid,
                                       int need_uptime, int need_capacity,
-                                      int need_guard, int need_desc)
+                                      int need_guard, int need_desc,
+                                      int pref_addr)
 { /* XXXX MOVE */
   SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) {
     if (!node->is_running ||
@@ -1838,6 +1839,9 @@ router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid,
       continue;
     if (node_is_unreliable(node, need_uptime, need_capacity, need_guard))
       continue;
+    /* Choose a node with a preferred OR address */
+    if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, pref_addr))
+      continue;
 
     smartlist_add(sl, (void *)node);
   } SMARTLIST_FOREACH_END(node);
@@ -2299,6 +2303,10 @@ node_sl_choose_by_bandwidth(const smartlist_t *sl,
  * 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 ClientUseIPv6 0).
  */
 const node_t *
 router_choose_random_node(smartlist_t *excludedsmartlist,
@@ -2311,6 +2319,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
   const int allow_invalid = (flags & CRN_ALLOW_INVALID) != 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;
 
   smartlist_t *sl=smartlist_new(),
     *excludednodes=smartlist_new();
@@ -2336,7 +2345,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
 
   router_add_running_nodes_to_smartlist(sl, allow_invalid,
                                         need_uptime, need_capacity,
-                                        need_guard, need_desc);
+                                        need_guard, need_desc, pref_addr);
   log_debug(LD_CIRC,
            "We found %d running nodes.",
             smartlist_len(sl));
@@ -2365,7 +2374,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
   choice = node_sl_choose_by_bandwidth(sl, rule);
 
   smartlist_free(sl);
-  if (!choice && (need_uptime || need_capacity || need_guard)) {
+  if (!choice && (need_uptime || need_capacity || need_guard || pref_addr)) {
     /* try once more -- recurse but with fewer restrictions. */
     log_info(LD_CIRC,
              "We couldn't find any live%s%s%s routers; falling back "
@@ -2373,7 +2382,8 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
              need_capacity?", fast":"",
              need_uptime?", stable":"",
              need_guard?", guard":"");
-    flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD);
+    flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD|
+                CRN_PREF_ADDR);
     choice = router_choose_random_node(
                      excludedsmartlist, excludedset, flags);
   }
diff --git a/src/or/routerlist.h b/src/or/routerlist.h
index 339e34ae03..4c828d6572 100644
--- a/src/or/routerlist.h
+++ b/src/or/routerlist.h
@@ -61,7 +61,8 @@ 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 allow_invalid,
                                            int need_uptime, int need_capacity,
-                                           int need_guard, int need_desc);
+                                           int need_guard, int need_desc,
+                                           int pref_addr);
 
 const routerinfo_t *routerlist_find_my_routerinfo(void);
 uint32_t router_get_advertised_bandwidth(const routerinfo_t *router);
diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c
index 87276dbbf8..e4947e0959 100644
--- a/src/test/test_entrynodes.c
+++ b/src/test/test_entrynodes.c
@@ -70,6 +70,14 @@ fake_network_setup(const struct testcase_t *testcase)
   return dummy_state;
 }
 
+static or_options_t mocked_options;
+
+static const or_options_t *
+mock_get_options(void)
+{
+  return &mocked_options;
+}
+
 /** Test choose_random_entry() with none of our routers being guard nodes. */
 static void
 test_choose_random_entry_no_guards(void *arg)
@@ -78,6 +86,14 @@ test_choose_random_entry_no_guards(void *arg)
 
   (void) arg;
 
+  MOCK(get_options, mock_get_options);
+
+  /* Check that we get a guard if it passes preferred
+   * address settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientPreferIPv6ORPort = 0;
+
   /* Try to pick an entry even though none of our routers are guards. */
   chosen_entry = choose_random_entry(NULL);
 
@@ -86,8 +102,35 @@ test_choose_random_entry_no_guards(void *arg)
      can't find a proper entry guard. */
   tt_assert(chosen_entry);
 
+  /* And with the other IP version active */
+  mocked_options.ClientUseIPv6 = 1;
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
+  /* Check that we don't get a guard if it doesn't pass mandatory address
+   * settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 0;
+  mocked_options.ClientPreferIPv6ORPort = 0;
+
+  chosen_entry = choose_random_entry(NULL);
+
+  /* If we don't allow IPv4 at all, we don't get a guard*/
+  tt_assert(!chosen_entry);
+
+  /* Check that we get a guard if it passes allowed but not preferred address
+   * settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientUseIPv6 = 1;
+  mocked_options.ClientPreferIPv6ORPort = 1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
  done:
-  ;
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  UNMOCK(get_options);
 }
 
 /** Test choose_random_entry() with only one of our routers being a
@@ -101,17 +144,55 @@ test_choose_random_entry_one_possible_guard(void *arg)
 
   (void) arg;
 
+  MOCK(get_options, mock_get_options);
+
   /* Set one of the nodes to be a guard. */
   our_nodelist = nodelist_get_list();
   the_guard = smartlist_get(our_nodelist, 4); /* chosen by fair dice roll */
   the_guard->is_possible_guard = 1;
 
+  /* Check that we get the guard if it passes preferred
+   * address settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientPreferIPv6ORPort = 0;
+
   /* Pick an entry. Make sure we pick the node we marked as guard. */
   chosen_entry = choose_random_entry(NULL);
   tt_ptr_op(chosen_entry, OP_EQ, the_guard);
 
+  /* And with the other IP version active */
+  mocked_options.ClientUseIPv6 = 1;
+  chosen_entry = choose_random_entry(NULL);
+  tt_ptr_op(chosen_entry, OP_EQ, the_guard);
+
+  /* Check that we don't get a guard if it doesn't pass mandatory address
+   * settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 0;
+  mocked_options.ClientPreferIPv6ORPort = 0;
+
+  chosen_entry = choose_random_entry(NULL);
+
+  /* If we don't allow IPv4 at all, we don't get a guard*/
+  tt_assert(!chosen_entry);
+
+  /* Check that we get a node if it passes allowed but not preferred
+   * address settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientUseIPv6 = 1;
+  mocked_options.ClientPreferIPv6ORPort = 1;
+
+  chosen_entry = choose_random_entry(NULL);
+
+  /* We disable the guard check and the preferred address check at the same
+   * time, so we can't be sure we get the guard */
+  tt_assert(chosen_entry);
+
  done:
-  ;
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  UNMOCK(get_options);
 }
 
 /** Helper to conduct tests for populate_live_entry_guards().
@@ -624,14 +705,6 @@ test_entry_is_live(void *arg)
   ; /* XXX */
 }
 
-static or_options_t mocked_options;
-
-static const or_options_t *
-mock_get_options(void)
-{
-  return &mocked_options;
-}
-
 #define TEST_IPV4_ADDR "123.45.67.89"
 #define TEST_IPV6_ADDR "[1234:5678:90ab:cdef::]"