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.

diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index 87d976b2fa1a2d4d914cf0f04a57ee0cde2d3055..4c9c53d8cb159ae1f9512d6ee37aa453b6a6e04f 100644 (file)
--- a/doc/tor.1.txt
+++ b/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

diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index d6bef658ff9adb30fd8308255b42e0e1d3934cde..a4b935065d7038d6fd9c6c04c6a53afc701181b6 100644 (file)
--- a/src/or/entrynodes.c
+++ b/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) {

diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index d7cada94d3f811f11ce03c1f282cbd83f3349503..23e9b0e176cb68db82726f64e174c5faae317d84 100644 (file)
--- a/src/or/nodelist.c
+++ b/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);
   }

diff --git a/src/or/policies.c b/src/or/policies.c
index 734558d836b78742681859c5d87129f952003dbe..e2cece56e47da81d7e5601e05e40c0aacff56554 100644 (file)
--- a/src/or/policies.c
+++ b/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;
 }
 

diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c
index 14baa8c9bf891e0a07fa0dede2a045e96a381310..fd19db095d976e4c7c5c11b78f86ab2b78a76150 100644 (file)
--- a/src/test/test_entrynodes.c
+++ b/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);

diff --git a/src/test/test_policy.c b/src/test/test_policy.c
index b4cbfb257901c892b65580aa40157682a6195181..c044d9f21045b039a7d9d074660ea9efbb205c8c 100644 (file)
--- a/src/test/test_policy.c
+++ b/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));