[Tor Trac #27490]: When ClientPreferIPv6ORPort is set to auto, and a relay is being chosen for a directory or orport connection, prefer IPv4 or IPv6 at random #317

neelchauhan · 2018-09-11T23:00:39Z

For the ticket here.

teor2345

There is one design problem with this patch:

ClientPreferIPv6ORPort already has an "auto" setting, which means "bridge clients use the configured bridge IP address, other clients use IPv4". The auto setting is the default. (I didn't realise when I wrote down the steps.)

This patch tells all clients to use IPv4 50% of the time, and IPv6 50% of the time. We can't make 50% of clients use IPv6 by default, until most relays have IPv6. Otherwise, we would overload the IPv6 Guards. Right now, about 25% of Guard consensus weight has IPv6:
https://metrics.torproject.org/advbw-ipv6.html

Please add another option for this feature. You can call it ClientAutoIPv6ORPort if you want.

I also added a ticket to set the IPv6 probability based on the number of Guards or Bridges with IPv6:
https://trac.torproject.org/projects/tor/ticket/27647

Otherwise, this is a good patch, but some documentation and tests need to be updated.

In the future, we also want to remove ClientPreferIPv6DirPort, and decide between IPv4 and IPv6 just before connecting. But this patch helps us get better IPv6 support.

Please fix the policy/fascist_firewall_choose_address unit test:
https://travis-ci.org/torproject/tor/jobs/427442291#L6076

Please update the ClientPreferIPv6ORPort documentation in the struct:

tor/src/app/config/or_options_st.h

Line 658 in 9ca1af9

/** If true, prefer an IPv6 OR port over an IPv4 one for entry node

And man page:

tor/doc/tor.1.txt

Line 1731 in 8815960

[[ClientPreferIPv6ORPort]] **ClientPreferIPv6ORPort** **0**|**1**|**auto**::

Tor logs an info-level message when a non-preferred address family is used:

tor/src/core/mainloop/connection.c

Line 1956 in 22e2403

/* Check if we couldn't satisfy an address family preference */

When ClientPreferIPv6ORPort is auto and fascist_firewall_use_ipv6() is true, we should log the address family Tor actually used. When ClientPreferIPv6ORPort is not auto, we should keep the existing logs.

fascist_firewall_prefer_ipv6_orport() and node_ipv6_or_preferred() used to return the same value every time. Please update the comments in every function that calls fascist_firewall_prefer_ipv6_orport() and node_ipv6_or_preferred() to say that they can return a random value:

If ClientPreferIPv6ORPort is auto, and the bridge has IPv4 and IPv6 addresses, assign it a random preference:

tor/src/feature/client/bridges.c

Line 760 in 8b0920b

node->ipv6_preferred = (fascist_firewall_prefer_ipv6_orport(options) &&

If ClientPreferIPv6ORPort is auto, and the node has IPv4 and IPv6 addresses, assign it a random preference:

tor/src/feature/nodelist/nodelist.c

Line 621 in 5a2374b

if (fascist_firewall_prefer_ipv6_orport(options) &&

If ClientPreferIPv6ORPort is auto, and the node has IPv4 and IPv6 addresses, returns a random preference:

tor/src/core/or/policies.c

Line 493 in 6da8c6e

fascist_firewall_prefer_ipv6_orport(const or_options_t *options)

If ClientPreferIPv6ORPort is auto, and the routerstatus has IPv4 and IPv6 addresses, choose one of those addresses at random:

tor/src/core/or/policies.c

Line 679 in 6da8c6e

int pref_ipv6 = (fw_connection == FIREWALL_OR_CONNECTION

If ClientPreferIPv6ORPort is auto, and the node has IPv4 and IPv6 addresses, prefer one of those addresses at random:

tor/src/core/or/policies.c

Line 712 in 6da8c6e

/** Like fascist_firewall_allows_base(), but takes node, and looks up pref_ipv6

If ClientPreferIPv6ORPort is auto, and the node has IPv4 and IPv6 addresses, choose one of those addresses at random:

tor/src/feature/nodelist/nodelist.c

Line 1582 in 5a2374b

/** Copy the preferred OR port (IP address and TCP port) for

teor2345 · 2018-09-12T01:32:03Z

changes/ticket27490

@@ -0,0 +1,6 @@
+  o Minor features (ipv6):
+    - If ClientPreferIPv6ORPort is set to auto and a relay is being chosen
+      for a Directory or ORPort connection, we choose IPv4 or IPv6 at random.


This line of the changes file is confusing: clients only make ORPort connections. (They tunnel directory connections over the ORPort.)

I suggest just saying: "for an ORPort connection"

Also, we still choose whatever address is in the bridge line, see:

tor/src/feature/client/bridges.c

Lines 812 to 820 in 8b0920b

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->rs->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->rs->ipv6_addr));

}

I suggest saying: "we choose IPv4 or IPv6 at random, except for bridges, which use the address in the Bridge line."

teor2345 · 2018-09-12T01:32:03Z

src/core/or/policies.c

+/* Choose whether we prefer IPv4 or IPv6 by randomly choosing an address
+ * family. Return 0 for IPv4, and 1 for IPv6. */
+static int
+fascist_firewall_choose_preferred_addr(void)


This function name is confusing: the other fascist_firewall_choose_* functions choose between two addresses. This function chooses between two address families at random.

I suggest: "fascist_firewall_rand_preferred_addr"

teor2345 · 2018-09-12T01:48:15Z

I think that this patch will break existing configs when:

there are a small number of Bridges (or EntryNodes),
they are all marked as IPv4-preferred when they are loaded, and
the device running tor only has IPv6.

We can fix this bug in a few different ways:

make sure we always fall back to non-preferred addresses if all preferred addresses fail:
- we will find these bugs when we test
reset the preferred flags on all nodes if all preferred addresses failL
- I think we should remove the flag instead
stop setting the IPv6 preferred flag on nodes:
- see https://trac.torproject.org/projects/tor/ticket/27648

…andomly.

coveralls · 2018-09-12T23:54:57Z

Coverage increased (+0.04%) to 61.865% when pulling a956184 on neelchauhan:b27490 into aebc98d on torproject:master.

…ection_connect_log_client_use_ip_version()

… can be random

… are dual-stack

…ection_connect_log_client_use_ip_version()

… can be random

… are dual-stack

… it a random preference

…t a random preference

…s auto, have a random preference

…nd we are dual stack, infer a random preference

…ddress

…PreferIPv6ORPort is auto, prefer IPv4 or IPv6 at random

…v6ORPort is auto, prefer IPv4 or IPv6 at random

…s() as we randomly prefer addresses now if ClientPreferIPv6ORPort is auto

teor2345 · 2018-09-21T03:20:18Z

src/app/config/or_options_st.h

@@ -666,6 +666,9 @@ struct or_options_t {
   * accessing this value directly.  */
  int ClientPreferIPv6DirPort;

+  /** If true, automatically choose between IPv4 and IPv6. */


We choose between IPv4 and IPv6 at random, so let's document that.

teor2345 · 2018-09-21T03:20:18Z

src/app/config/config.c

@@ -329,6 +329,7 @@ static config_var_t option_vars_[] = {
  V(ClientOnly,                  BOOL,     "0"),
  V(ClientPreferIPv6ORPort,      AUTOBOOL, "auto"),
  V(ClientPreferIPv6DirPort,     AUTOBOOL, "auto"),
+  V(ClientAutoIPv6ORPort,        BOOL,     "0"),


Each new option needs an entry in the tor man page. Edit doc/tor.1.txt

teor2345

I am confused by these commits. I will suggest a way forward in the trac ticket.

teor2345 · 2018-09-21T04:04:02Z

src/core/or/policies.c

@@ -458,8 +458,9 @@ fascist_firewall_use_ipv6(const or_options_t *options)
   * IPv4, or they prefer it.
   * ClientPreferIPv6DirPort is deprecated, but check it anyway. */
  return (options->ClientUseIPv6 == 1 || options->ClientUseIPv4 == 0 ||
-          options->ClientPreferIPv6ORPort ||
-          options->ClientPreferIPv6DirPort == 1 || options->UseBridges == 1);
+          options->ClientPreferIPv6ORPort == -1 ||


IPv6 is also used when ClientPreferIPv6ORPort is 1, so this line should not be changed.

teor2345 · 2018-09-21T04:04:02Z

src/core/or/policies.c

-  } else if (options->ClientPreferIPv6ORPort == -1) {
-    return fascist_firewall_choose_preferred_addr();
+  } else if (options->ClientAutoIPv6ORPort) {
+    return fascist_firewall_rand_preferred_addr();


If ClientAutoIPv6ORPort is 1 and ClientPreferIPv6ORPort is 1, what should happen?

I think we should prefer IPv4 and IPv6 at random, but this code will just prefer IPv6.
To make ClientAutoIPv6ORPort override ClientPreferIPv6ORPort, check it first.

teor2345 · 2018-09-21T04:04:02Z

changes/ticket27490

-      Previously, we preferred to IPv4 if ClientPreferIPv6ORPort is auto
-      except when an IPv6 bridge was specified. Closes ticket 27490. Patch
-      by Neel Chauhan.
+      for an ORPort connection, we prefer IPv4 or IPv6 at random if we have


ClientPreferIPv6ORPort does not choose at random any more. Only ClientAutoIPv6ORPort chooses at random.

teor2345 · 2018-09-21T04:04:02Z

doc/tor.1.txt

@@ -1733,8 +1733,9 @@ The following options are useful only for clients (that is, if
    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 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)
+    other clients choose what they have, or choose randomly if they have


ClientPreferIPv6ORPort does not choose randomly. ClientAutoIPv6ORPort chooses randomly.

teor2345 · 2018-09-21T04:04:02Z

src/app/config/or_options_st.h

@@ -657,8 +657,8 @@ struct or_options_t {
  int ClientUseIPv6;
  /** If true, prefer an IPv6 OR port over an IPv4 one for entry node
   * connections. If auto, bridge clients prefer IPv6, and other clients
-   * prefer IPv4. Use node_ipv6_or_preferred() instead of accessing this value
-   * directly. */
+   * prefer what they have or choose randomly if they are dual-stack. Use


ClientPreferIPv6ORPort does not choose randomly. ClientAutoIPv6ORPort chooses randomly.

teor2345 · 2018-09-21T04:04:02Z

src/feature/client/bridges.c

-      /* Mark which address to use based on user preference */
+      /* Mark which address to use based on user preference. Keep in mind
+       * that fascist_firewall_prefer_ipv6_orport() can infer a random
+       * preference if both IPv4 and IPv6 are available. */


fascist_firewall_prefer_ipv6_orport() only returns a random preference if ClientAutoIPv6ORPort is 1

teor2345 · 2018-09-21T04:04:02Z

src/feature/nodelist/nodelist.c

@@ -1529,6 +1529,8 @@ node_has_ipv6_dirport(const node_t *node)
 *
 * If you don't have a node, consider looking it up.
 * If there is no node, use fascist_firewall_prefer_ipv6_orport().
+ * fascist_firewall_prefer_ipv6_orport() can infer a random
+ * preference if we have both IPv4 and IPv6.


fascist_firewall_prefer_ipv6_orport() only returns a random preference if ClientAutoIPv6ORPort is 1

teor2345 · 2018-09-21T04:04:02Z

src/feature/nodelist/nodelist.c

@@ -1539,6 +1541,7 @@ node_ipv6_or_preferred(const node_t *node)

  /* XX/teor - node->ipv6_preferred is set from
   * fascist_firewall_prefer_ipv6_orport() each time the consensus is loaded.
+   * and can be random.


fascist_firewall_prefer_ipv6_orport() only returns a random preference if ClientAutoIPv6ORPort is 1

teor2345 · 2018-09-21T04:04:02Z

src/feature/client/bridges.c

@@ -752,9 +752,14 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node)
    }

    if (options->ClientPreferIPv6ORPort == -1) {


Do not change how ClientPreferIPv6ORPort works. Instead, add new behaviour when ClientAutoIPv6ORPort is set.

teor2345 · 2018-09-21T04:04:02Z

src/feature/nodelist/nodelist.c

@@ -618,7 +618,10 @@ nodelist_set_consensus(networkstatus_t *ns)
      node->is_bad_exit = rs->is_bad_exit;
      node->is_hs_dir = rs->is_hs_dir;
      node->ipv6_preferred = 0;
-      if (fascist_firewall_prefer_ipv6_orport(options) &&
+      if (options->ClientPreferIPv6ORPort == -1 &&


Do not change how ClientPreferIPv6ORPort works. Instead, add new behaviour when ClientAutoIPv6ORPort is set.

If ClientPreferIPv6ORPort is auto, randomly prefer IPv4 or IPv6

1a98526

teor2345 suggested changes Sep 12, 2018

View changes

neelchauhan added 2 commits Sep 12, 2018

Introduce ClientAutoIPv6ORPort to just choose between IPv4 and IPv6 r…

06eacd5

…andomly.

Update documentation on ClientPreferIPv6ORPort changes

25a5780

neelchauhan added 3 commits Sep 18, 2018

Log which IP version we use if ClientPreferIPv6ORPort is auto in conn…

f61d717

…ection_connect_log_client_use_ip_version()

Update comments to specify that fascist_firewall_prefer_ipv6_orport()…

a033d1a

… can be random

Add comments saying that node_ipv6_or_preferred() can be random if we…

4cab0f5

… are dual-stack

neelchauhan force-pushed the b27490 branch from 04a7896 to 4cab0f5 Sep 18, 2018

neelchauhan added 3 commits Sep 18, 2018

Log which IP version we use if ClientPreferIPv6ORPort is auto in conn…

2311a42

…ection_connect_log_client_use_ip_version()

Update comments to specify that fascist_firewall_prefer_ipv6_orport()…

88673e4

… can be random

Add comments saying that node_ipv6_or_preferred() can be random if we…

a956184

… are dual-stack

neelchauhan force-pushed the b27490 branch from 4cab0f5 to a956184 Sep 18, 2018

neelchauhan added 9 commits Sep 18, 2018

Merge branch 'b27490' of github.com:neelchauhan/tor into b27490

146a90f

If ClientPreferIPv6ORPort is auto, and a bridge is dual-stack, assign…

13bb10a

… it a random preference

If ClientPreferIPv6ORPort is auto, and a node is dual-stack, assign i…

42c81b6

…t a random preference

In fascist_firewall_prefer_ipv6_orport(), if ClientPreferIPv6ORPort i…

a210bda

…s auto, have a random preference

In fascist_firewall_allows_rs(), if ClientPreferIPv6ORPort is auto, a…

6559d31

…nd we are dual stack, infer a random preference

Introduce function node_has_ipv4_addr() to check if we have an IPv4 a…

be46bdb

…ddress

In fascist_firewall_allows_node(), if a node is dual-stack and Client…

b9191b8

…PreferIPv6ORPort is auto, prefer IPv4 or IPv6 at random

In node_get_pref_orport(), if a node is dual-stack and ClientPreferIP…

d261519

…v6ORPort is auto, prefer IPv4 or IPv6 at random

Remove obsolete tests in test_policies_fascist_firewall_choose_addres…

cbc91ef

…s() as we randomly prefer addresses now if ClientPreferIPv6ORPort is auto

teor2345 reviewed Sep 21, 2018

View changes

teor2345 suggested changes Sep 21, 2018

View changes

neelchauhan closed this Sep 25, 2018

torproject / tor Public

[Tor Trac #27490]: When ClientPreferIPv6ORPort is set to auto, and a relay is being chosen for a directory or orport connection, prefer IPv4 or IPv6 at random #317

[Tor Trac #27490]: When ClientPreferIPv6ORPort is set to auto, and a relay is being chosen for a directory or orport connection, prefer IPv4 or IPv6 at random #317

neelchauhan commented Sep 11, 2018

teor2345 left a comment

teor2345 Sep 12, 2018

teor2345 Sep 12, 2018

teor2345 Sep 12, 2018

teor2345 commented Sep 12, 2018

coveralls commented Sep 12, 2018 •

edited

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 left a comment

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

teor2345 Sep 21, 2018

	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->rs->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->rs->ipv6_addr));
	}

		@@ -752,9 +752,14 @@ rewrite_node_address_for_bridge(const bridge_info_t bridge, node_t node)
		}

		if (options->ClientPreferIPv6ORPort == -1) {

torproject / tor Public

[Tor Trac #27490]: When ClientPreferIPv6ORPort is set to auto, and a relay is being chosen for a directory or orport connection, prefer IPv4 or IPv6 at random #317

[Tor Trac #27490]: When ClientPreferIPv6ORPort is set to auto, and a relay is being chosen for a directory or orport connection, prefer IPv4 or IPv6 at random #317

Conversation

neelchauhan commented Sep 11, 2018

teor2345 left a comment

teor2345 commented Sep 12, 2018

coveralls commented Sep 12, 2018 • edited

teor2345 left a comment

coveralls commented Sep 12, 2018 •

edited