commit 7d673e70b072663e3f9f11819b166846e578251d Author: Neel Chauhan neel@neelc.org Date: Tue Feb 18 08:20:11 2020 -0800
Remove the ClientAutoIPv6ORPort option --- changes/ticket32905 | 6 +++ doc/tor.1.txt | 7 --- src/app/config/config.c | 2 +- src/app/config/or_options_st.h | 3 -- src/core/or/policies.c | 17 +------- src/core/or/policies.h | 1 - src/feature/client/bridges.c | 3 +- src/test/conf_examples/large_1/expected | 1 - src/test/conf_examples/large_1/expected_no_dirauth | 1 - src/test/conf_examples/large_1/torrc | 1 - src/test/test_policy.c | 50 ---------------------- 11 files changed, 10 insertions(+), 82 deletions(-)
diff --git a/changes/ticket32905 b/changes/ticket32905 new file mode 100644 index 000000000..6f420ec69 --- /dev/null +++ b/changes/ticket32905 @@ -0,0 +1,6 @@ + o Removed features: + - Remove the ClientAutoIPv6ORPort option. This option attempted + to randomly choose between IPv4 and IPv6 for client connections, + and isn't a true implementation of Happy Eyeballs. Often, this + option failed on IPv4-only or IPv6-only connections. Closes + ticket 32905. Patch by Neel Chauhan. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index db4dd2755..f580cb618 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1018,13 +1018,6 @@ The following options are useful only for clients (that is, if via the UI to mobile users for use where bandwidth may be expensive. (Default: 0)
-[[ClientAutoIPv6ORPort]] **ClientAutoIPv6ORPort** **0**|**1**:: - If this option is set to 1, Tor clients randomly prefer a node's IPv4 or - IPv6 ORPort. The random preference is set every time a node is loaded - from a new consensus or bridge config. When this option is set to 1, - **ClientPreferIPv6ORPort** is ignored. (Default: 0) (DEPRECATED: This - option is unreliable if a connection isn't reliably dual-stack.) - [[ClientBootstrapConsensusAuthorityDownloadInitialDelay]] **ClientBootstrapConsensusAuthorityDownloadInitialDelay** __N__:: Initial delay in seconds for when clients should download consensuses from authorities if they are bootstrapping (that is, they don't have a usable, reasonably diff --git a/src/app/config/config.c b/src/app/config/config.c index bbf984ad0..6ead03ffd 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -366,7 +366,7 @@ static const config_var_t option_vars_[] = { #endif /* defined(HAVE_MODULE_RELAY) || defined(TOR_UNIT_TESTS) */ V(ClientPreferIPv6ORPort, AUTOBOOL, "auto"), V(ClientPreferIPv6DirPort, AUTOBOOL, "auto"), - V(ClientAutoIPv6ORPort, BOOL, "0"), + OBSOLETE("ClientAutoIPv6ORPort"), V(ClientRejectInternalAddresses, BOOL, "1"), V(ClientTransportPlugin, LINELIST, NULL), V(ClientUseIPv6, BOOL, "0"), diff --git a/src/app/config/or_options_st.h b/src/app/config/or_options_st.h index 35ba15a9e..bf58205f8 100644 --- a/src/app/config/or_options_st.h +++ b/src/app/config/or_options_st.h @@ -662,9 +662,6 @@ struct or_options_t { * accessing this value directly. */ int ClientPreferIPv6DirPort;
- /** If true, prefer an IPv4 or IPv6 OR port at random. */ - int ClientAutoIPv6ORPort; - /** The length of time that we think a consensus should be fresh. */ int V3AuthVotingInterval; /** The length of time we think it will take to distribute votes. */ diff --git a/src/core/or/policies.c b/src/core/or/policies.c index a82995fe1..a13b2e2cb 100644 --- a/src/core/or/policies.c +++ b/src/core/or/policies.c @@ -463,8 +463,7 @@ fascist_firewall_use_ipv6(const or_options_t *options) * ClientPreferIPv6DirPort is deprecated, but check it anyway. */ return (options->ClientUseIPv6 == 1 || options->ClientUseIPv4 == 0 || options->ClientPreferIPv6ORPort == 1 || - options->ClientPreferIPv6DirPort == 1 || options->UseBridges == 1 || - options->ClientAutoIPv6ORPort == 1); + options->ClientPreferIPv6DirPort == 1 || options->UseBridges == 1); }
/** Do we prefer to connect to IPv6, ignoring ClientPreferIPv6ORPort and @@ -491,15 +490,6 @@ fascist_firewall_prefer_ipv6_impl(const or_options_t *options) return -1; }
-/* Choose whether we prefer IPv4 or IPv6 by randomly choosing an address - * family. Return 0 for IPv4, and 1 for IPv6. */ -MOCK_IMPL(int, -fascist_firewall_rand_prefer_ipv6_addr, (void)) -{ - /* TODO: Check for failures, and infer our preference based on this. */ - return crypto_rand_int(2); -} - /** Do we prefer to connect to IPv6 ORPorts? * Use node_ipv6_or_preferred() whenever possible: it supports bridge client * per-node IPv6 preferences. @@ -514,10 +504,7 @@ fascist_firewall_prefer_ipv6_orport(const or_options_t *options) }
/* We can use both IPv4 and IPv6 - which do we prefer? */ - if (options->ClientAutoIPv6ORPort == 1) { - /* If ClientAutoIPv6ORPort is 1, we prefer IPv4 or IPv6 at random. */ - return fascist_firewall_rand_prefer_ipv6_addr(); - } else if (options->ClientPreferIPv6ORPort == 1) { + if (options->ClientPreferIPv6ORPort == 1) { return 1; }
diff --git a/src/core/or/policies.h b/src/core/or/policies.h index b9477b2db..72a37d62b 100644 --- a/src/core/or/policies.h +++ b/src/core/or/policies.h @@ -70,7 +70,6 @@ typedef struct short_policy_t { int firewall_is_fascist_or(void); int firewall_is_fascist_dir(void); int fascist_firewall_use_ipv6(const or_options_t *options); -MOCK_DECL(int, fascist_firewall_rand_prefer_ipv6_addr, (void)); int fascist_firewall_prefer_ipv6_orport(const or_options_t *options); int fascist_firewall_prefer_ipv6_dirport(const or_options_t *options);
diff --git a/src/feature/client/bridges.c b/src/feature/client/bridges.c index 2b52a1173..66b04f3bc 100644 --- a/src/feature/client/bridges.c +++ b/src/feature/client/bridges.c @@ -844,8 +844,7 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node) } }
- if (options->ClientPreferIPv6ORPort == -1 || - options->ClientAutoIPv6ORPort == 0) { + 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)); diff --git a/src/test/conf_examples/large_1/expected b/src/test/conf_examples/large_1/expected index 5866f5823..99a12ffc8 100644 --- a/src/test/conf_examples/large_1/expected +++ b/src/test/conf_examples/large_1/expected @@ -15,7 +15,6 @@ CellStatistics 1 CircuitBuildTimeout 200 CircuitsAvailableTimeout 10 CircuitStreamTimeout 20 -ClientAutoIPv6ORPort 1 ClientOnly 1 ClientPreferIPv6DirPort 1 ClientPreferIPv6ORPort 1 diff --git a/src/test/conf_examples/large_1/expected_no_dirauth b/src/test/conf_examples/large_1/expected_no_dirauth index 17c11f85f..26a33bdc7 100644 --- a/src/test/conf_examples/large_1/expected_no_dirauth +++ b/src/test/conf_examples/large_1/expected_no_dirauth @@ -15,7 +15,6 @@ CellStatistics 1 CircuitBuildTimeout 200 CircuitsAvailableTimeout 10 CircuitStreamTimeout 20 -ClientAutoIPv6ORPort 1 ClientOnly 1 ClientPreferIPv6DirPort 1 ClientPreferIPv6ORPort 1 diff --git a/src/test/conf_examples/large_1/torrc b/src/test/conf_examples/large_1/torrc index e99acd9fb..20ddf00e1 100644 --- a/src/test/conf_examples/large_1/torrc +++ b/src/test/conf_examples/large_1/torrc @@ -16,7 +16,6 @@ CircuitBuildTimeout 200 CircuitPadding 1 CircuitsAvailableTimeout 10 CircuitStreamTimeout 20 -ClientAutoIPv6ORPort 1 ClientOnly 1 ClientPreferIPv6DirPort 1 ClientPreferIPv6ORPort 1 diff --git a/src/test/test_policy.c b/src/test/test_policy.c index 762241249..3895c345e 100644 --- a/src/test/test_policy.c +++ b/src/test/test_policy.c @@ -2124,20 +2124,6 @@ test_policies_fascist_firewall_allows_address(void *arg) teardown_capture_of_logs(); \ STMT_END
-/** Mock the preferred address function to return zero (prefer IPv4). */ -static int -mock_fascist_firewall_rand_prefer_ipv6_addr_use_ipv4(void) -{ - return 0; -} - -/** Mock the preferred address function to return one (prefer IPv6). */ -static int -mock_fascist_firewall_rand_prefer_ipv6_addr_use_ipv6(void) -{ - return 1; -} - /** Run unit tests for fascist_firewall_choose_address */ static void test_policies_fascist_firewall_choose_address(void *arg) @@ -2536,42 +2522,6 @@ test_policies_fascist_firewall_choose_address(void *arg) CHECK_CHOSEN_ADDR_RN(fake_rs, fake_node, FIREWALL_DIR_CONNECTION, 1, 1, ipv4_dir_ap);
- /* Test ClientAutoIPv6ORPort and pretend we prefer IPv4. */ - memset(&mock_options, 0, sizeof(or_options_t)); - mock_options.ClientAutoIPv6ORPort = 1; - mock_options.ClientUseIPv4 = 1; - mock_options.ClientUseIPv6 = 1; - MOCK(fascist_firewall_rand_prefer_ipv6_addr, - mock_fascist_firewall_rand_prefer_ipv6_addr_use_ipv4); - /* Simulate the initialisation of fake_node.ipv6_preferred */ - fake_node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport( - &mock_options); - - CHECK_CHOSEN_ADDR_RN(fake_rs, fake_node, FIREWALL_OR_CONNECTION, 0, 1, - ipv4_or_ap); - CHECK_CHOSEN_ADDR_RN(fake_rs, fake_node, FIREWALL_OR_CONNECTION, 1, 1, - ipv4_or_ap); - - UNMOCK(fascist_firewall_rand_prefer_ipv6_addr); - - /* Test ClientAutoIPv6ORPort and pretend we prefer IPv6. */ - memset(&mock_options, 0, sizeof(or_options_t)); - mock_options.ClientAutoIPv6ORPort = 1; - mock_options.ClientUseIPv4 = 1; - mock_options.ClientUseIPv6 = 1; - MOCK(fascist_firewall_rand_prefer_ipv6_addr, - mock_fascist_firewall_rand_prefer_ipv6_addr_use_ipv6); - /* Simulate the initialisation of fake_node.ipv6_preferred */ - fake_node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport( - &mock_options); - - CHECK_CHOSEN_ADDR_RN(fake_rs, fake_node, FIREWALL_OR_CONNECTION, 0, 1, - ipv6_or_ap); - CHECK_CHOSEN_ADDR_RN(fake_rs, fake_node, FIREWALL_OR_CONNECTION, 1, 1, - ipv6_or_ap); - - UNMOCK(fascist_firewall_rand_prefer_ipv6_addr); - /* Test firewall_choose_address_ls(). To do this, we make a fake link * specifier. */ smartlist_t *lspecs = smartlist_new(),