Make bridge clients prefer the configured bridge address

When ClientPreferIPv6ORPort is auto, bridges prefer the configured
bridge ORPort address. Otherwise, they use the value of the option.
Other clients prefer IPv4 ORPorts if ClientPreferIPv6ORPort is auto.

When ClientPreferIPv6DirPort is auto, all clients prefer IPv4 DirPorts.

doc/tor.1.txt

@@ -1504,17 +1504,17 @@ The following options are useful only for clients (that is, if
     If this option is set to 1, Tor prefers a directory port with an IPv6
     address over one with IPv4, for direct connections, if a given directory
     server has both. (Tor also prefers an IPv6 DirPort if IPv4Client is set to
-    0.) If this option is set to auto, Tor bridge clients prefer IPv6, and
-    other clients prefer IPv4. Other things may influence the choice. This
-    option breaks a tie to the favor of IPv6. (Default: auto)
+    0.) If this option is set to auto, clients prefer IPv4. Other things may
+    influence the choice. This option breaks a tie to the favor of IPv6.
+    (Default: auto)
 
 [[ClientPreferIPv6ORPort]] **ClientPreferIPv6ORPort** **0**|**1**|**auto**::
     If this option is set to 1, Tor prefers an OR port with an IPv6
     address over one with IPv4 if a given entry node has both. (Tor also
     prefers an IPv6 ORPort if IPv4Client is set to 0.) If this option is set
-    to auto, Tor bridge clients prefer IPv6, and other clients prefer IPv4.
-    Other things may influence the choice. This option breaks a tie to the
-    favor of IPv6. (Default: auto)
+    to auto, Tor bridge clients prefer the configured bridge address, and
+    other clients prefer IPv4. Other things may influence the choice. This
+    option breaks a tie to the favor of IPv6. (Default: auto)
 
 [[PathsNeededToBuildCircuits]] **PathsNeededToBuildCircuits** __NUM__::
     Tor clients don't build circuits for user traffic until they know

src/or/entrynodes.c

@@ -2240,6 +2240,7 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node)
    *   does so through an address from any source other than node_get_addr().
    */
   tor_addr_t addr;
+  const or_options_t *options = get_options();
 
   if (node->ri) {
     routerinfo_t *ri = node->ri;
@@ -2272,9 +2273,15 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node)
       }
     }
 
-    /* Mark which address to use based on which bridge_t we got. */
-    node->ipv6_preferred = (tor_addr_family(&bridge->addr) == AF_INET6 &&
-                            !tor_addr_is_null(&node->ri->ipv6_addr));
+    if (options->ClientPreferIPv6ORPort == -1) {
+      /* Mark which address to use based on which bridge_t we got. */
+      node->ipv6_preferred = (tor_addr_family(&bridge->addr) == AF_INET6 &&
+                              !tor_addr_is_null(&node->ri->ipv6_addr));
+    } else {
+      /* Mark which address to use based on user preference */
+      node->ipv6_preferred = (fascist_firewall_prefer_ipv6_orport(options) &&
+                              !tor_addr_is_null(&node->ri->ipv6_addr));
+    }
 
     /* XXXipv6 we lack support for falling back to another address for
        the same relay, warn the user */
@@ -2283,10 +2290,13 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node)
       node_get_pref_orport(node, &ap);
       log_notice(LD_CONFIG,
                  "Bridge '%s' has both an IPv4 and an IPv6 address.  "
-                 "Will prefer using its %s address (%s).",
+                 "Will prefer using its %s address (%s) based on %s.",
                  ri->nickname,
-                 tor_addr_family(&ap.addr) == AF_INET6 ? "IPv6" : "IPv4",
-                 fmt_addrport(&ap.addr, ap.port));
+                 node->ipv6_preferred ? "IPv6" : "IPv4",
+                 fmt_addrport(&ap.addr, ap.port),
+                 options->ClientPreferIPv6ORPort == -1 ?
+                 "the configured Bridge address" :
+                 "ClientPreferIPv6ORPort");
     }
   }
   if (node->rs) {

src/or/nodelist.c

@@ -981,10 +981,6 @@ node_has_ipv6_dirport(const node_t *node)
  *  i) the node_t says that it prefers IPv6
  *  or
  *  ii) the router has no IPv4 OR address.
- *  or
- *  iii) our preference is for IPv6 addresses.
- *  (This extra step is needed in case our preferences have changed since
- *  node->ipv6_preferred was set at the time the consensus was loaded.)
  */
 int
 node_ipv6_or_preferred(const node_t *node)
@@ -993,10 +989,12 @@ node_ipv6_or_preferred(const node_t *node)
   tor_addr_port_t ipv4_addr;
   node_assert_ok(node);
 
+  /* XX/teor - node->ipv6_preferred is set from
+   * fascist_firewall_prefer_ipv6_orport() each time the consensus is loaded.
+   */
   if (!fascist_firewall_use_ipv6(options)) {
     return 0;
-  } else if (node->ipv6_preferred || node_get_prim_orport(node, &ipv4_addr)
-      || fascist_firewall_prefer_ipv6_orport(get_options())) {
+  } else if (node->ipv6_preferred || node_get_prim_orport(node, &ipv4_addr)) {
     return node_has_ipv6_orport(node);
   }
   return 0;
@@ -1077,13 +1075,9 @@ node_get_pref_ipv6_orport(const node_t *node, tor_addr_port_t *ap_out)
  *
  *  We prefer the IPv6 address if the router has an IPv6 address,
  *  and we can use IPv6 addresses, and:
- *  i) the node_t says that it prefers IPv6
+ *  i) the router has no IPv4 Dir address.
  *  or
- *  ii) the router has no IPv4 Dir address.
- *  or
- *  iii) our preference is for IPv6 addresses.
- *  (This extra step is needed in case our preferences have changed since
- *  node->ipv6_preferred was set at the time the consensus was loaded.)
+ *  ii) our preference is for IPv6 Dir addresses.
  */
 int
 node_ipv6_dir_preferred(const node_t *node)
@@ -1092,9 +1086,13 @@ node_ipv6_dir_preferred(const node_t *node)
   tor_addr_port_t ipv4_addr;
   node_assert_ok(node);
 
+  /* node->ipv6_preferred is set from fascist_firewall_prefer_ipv6_orport(),
+   * so we can't use it to determine DirPort IPv6 preference.
+   * This means that bridge clients will use IPv4 DirPorts by default.
+   */
   if (!fascist_firewall_use_ipv6(options)) {
     return 0;
-  } else if (node->ipv6_preferred || node_get_prim_dirport(node, &ipv4_addr)
+  } else if (node_get_prim_dirport(node, &ipv4_addr)
       || fascist_firewall_prefer_ipv6_dirport(get_options())) {
     return node_has_ipv6_dirport(node);
   }

src/or/policies.c

@@ -458,6 +458,13 @@ fascist_firewall_prefer_ipv6_impl(const or_options_t *options)
 int
 fascist_firewall_prefer_ipv6_orport(const or_options_t *options)
 {
+  /* node->ipv6_preferred is set from fascist_firewall_prefer_ipv6_orport()
+   * each time the consensus is loaded.
+   * If our preferences change, we will only reset ipv6_preferred on the node
+   * when the next consensus is loaded. But the consensus is realoded when the
+   * configuration changes after a HUP. So as long as the result of this
+   * function only depends on Tor's options, everything should work ok.
+   */
   int pref_ipv6 = fascist_firewall_prefer_ipv6_impl(options);
 
   if (pref_ipv6 >= 0) {
@@ -469,11 +476,6 @@ fascist_firewall_prefer_ipv6_orport(const or_options_t *options)
     return 1;
   }
 
-  /* For bridge clients, ClientPreferIPv6ORPort auto means "prefer IPv6". */
-  if (options->UseBridges && options->ClientPreferIPv6ORPort != 0) {
-    return 1;
-  }
-
   return 0;
 }
 
@@ -493,12 +495,6 @@ fascist_firewall_prefer_ipv6_dirport(const or_options_t *options)
     return 1;
   }
 
-  /* For bridge clients, ClientPreferIPv6ORPort auto means "prefer IPv6".
-   * XX/teor - do bridge clients ever use a DirPort? */
-  if (options->UseBridges && options->ClientPreferIPv6DirPort != 0) {
-    return 1;
-  }
-
   return 0;
 }
 

src/test/test_entrynodes.c

@@ -9,14 +9,16 @@
 
 #include "or.h"
 #include "test.h"
+
+#include "config.h"
 #include "entrynodes.h"
-#include "routerparse.h"
 #include "nodelist.h"
-#include "util.h"
+#include "policies.h"
 #include "routerlist.h"
+#include "routerparse.h"
 #include "routerset.h"
 #include "statefile.h"
-#include "config.h"
+#include "util.h"
 
 #include "test_helpers.h"
 
@@ -826,7 +828,7 @@ test_node_preferred_orport(void *arg)
    * ClientUseIPv4 is 0 */
   mocked_options.ClientUseIPv4 = 0;
   mocked_options.ClientUseIPv6 = 1;
-  node.ipv6_preferred = 0;
+  node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport(&mocked_options);
   node_get_pref_orport(&node, &ap);
   tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
   tt_assert(ap.port == ipv6_port);

src/test/test_policy.c

@@ -1510,21 +1510,25 @@ test_policies_fascist_firewall_choose_address(void *arg)
                                             FIREWALL_DIR_CONNECTION, 1)
             == &ipv4_dir_ap);
 
-  /* Auto (Preferring IPv6 for bridge clients) */
+  /* Auto:
+   * - bridge clients prefer the configured bridge OR address,
+   * - other clients prefer IPv4 OR by default,
+   * - all clients prefer IPv4 Dir by default.
+   */
   mock_options.ClientPreferIPv6ORPort = -1;
   mock_options.ClientPreferIPv6DirPort = -1;
   tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
                                             FIREWALL_OR_CONNECTION, 0)
-            == &ipv6_or_ap);
+            == &ipv4_or_ap);
   tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
                                             FIREWALL_OR_CONNECTION, 1)
-            == &ipv6_or_ap);
+            == &ipv4_or_ap);
   tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
                                             FIREWALL_DIR_CONNECTION, 0)
-            == &ipv6_dir_ap);
+            == &ipv4_dir_ap);
   tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
                                             FIREWALL_DIR_CONNECTION, 1)
-            == &ipv6_dir_ap);
+            == &ipv4_dir_ap);
 
   /* Preferring IPv6 */
   mock_options.ClientPreferIPv6ORPort = 1;
@@ -1544,22 +1548,23 @@ test_policies_fascist_firewall_choose_address(void *arg)
 
 
   /* In the default configuration (Auto / IPv6 off), bridge clients should
-   * still use and prefer IPv6 regardless of ClientUseIPv6. */
+   * still use IPv6, and only prefer it for bridges configured with an IPv6
+   * address, regardless of ClientUseIPv6. */
   mock_options.ClientUseIPv6 = 0;
   mock_options.ClientPreferIPv6ORPort = -1;
   mock_options.ClientPreferIPv6DirPort = -1;
   tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
                                             FIREWALL_OR_CONNECTION, 0)
-            == &ipv6_or_ap);
+            == &ipv4_or_ap);
   tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
                                             FIREWALL_OR_CONNECTION, 1)
-            == &ipv6_or_ap);
+            == &ipv4_or_ap);
   tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
                                             FIREWALL_DIR_CONNECTION, 0)
-            == &ipv6_dir_ap);
+            == &ipv4_dir_ap);
   tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
                                             FIREWALL_DIR_CONNECTION, 1)
-            == &ipv6_dir_ap);
+            == &ipv4_dir_ap);
 
   /* Choose an address with IPv4 on */
   memset(&mock_options, 0, sizeof(or_options_t));