From 2e2d22d29a1829550d60c4115ad2a9822945a799 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 16 Dec 2016 22:30:55 +1100 Subject: [PATCH 1/5] Make fascist_firewall_use_ipv6() check ORPort & DirPort IP preferences This makes clients try harder to find an IPv6 address when searching for a directory server. Required for #19608. --- src/or/policies.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/or/policies.c b/src/or/policies.c index f4c0cddbcc..dcee65301c 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -297,8 +297,8 @@ parse_reachable_addresses(void) } else if (fascist_firewall_use_ipv6(options) && (policy_is_reject_star(reachable_or_addr_policy, AF_INET6, 0) || policy_is_reject_star(reachable_dir_addr_policy, AF_INET6, 0))) { - log_warn(LD_CONFIG, "You have configured tor to use IPv6 " - "(ClientUseIPv6 1 or UseBridges 1), but " + log_warn(LD_CONFIG, "You have configured tor to use or prefer IPv6 " + "(or UseBridges 1), but " "ReachableAddresses, ReachableORAddresses, or " "ReachableDirAddresses reject all IPv6 addresses. " "Tor will not connect using IPv6."); @@ -426,6 +426,9 @@ fascist_firewall_allows_address(const tor_addr_t *addr, } /** Is this client configured to use IPv6? + * Returns true if the client might use IPv6 for some of its connections + * (including dual-stack and IPv6-only clients), and false if it will never + * use IPv6 for any connections. * Use node_ipv6_or/dir_preferred() when checking a specific node and OR/Dir * port: it supports bridge client per-node IPv6 preferences. */ @@ -433,9 +436,11 @@ int fascist_firewall_use_ipv6(const or_options_t *options) { /* Clients use IPv6 if it's set, or they use bridges, or they don't use - * IPv4 */ - return (options->ClientUseIPv6 == 1 || options->UseBridges == 1 - || options->ClientUseIPv4 == 0); + * IPv4, or they prefer it. + * ClientPreferIPv6DirPort is deprecated, but check it anyway. */ + return (options->ClientUseIPv6 == 1 || options->ClientUseIPv4 == 0 || + options->ClientPreferIPv6ORPort == 1 || + options->ClientPreferIPv6DirPort == 1 || options->UseBridges == 1); } /** Do we prefer to connect to IPv6, ignoring ClientPreferIPv6ORPort and From 5227ff4aadcb41f8758d426bf8ddfb4fb2d82cdf Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 16 Dec 2016 22:31:06 +1100 Subject: [PATCH 2/5] Remove redundant options checks for IPv6 preference conflicts It is no longer possible for the IPv6 preference options to differ from the IPv6 usage: preferring IPv6 implies possibly using IPv6. Also remove the corresponding unit test warning message checks. (But keep the unit tests themselves - they now run without warnings.) --- src/or/config.c | 17 ----------------- src/test/test_options.c | 10 ---------- 2 files changed, 27 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 6948bdbdb4..bbe93a5ba1 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -3305,23 +3305,6 @@ options_validate(or_options_t *old_options, or_options_t *options, "of the Internet, so they must not set Reachable*Addresses " "or FascistFirewall or FirewallPorts or ClientUseIPv4 0."); - /* We check if Reachable*Addresses blocks all addresses in - * parse_reachable_addresses(). */ - -#define WARN_PLEASE_USE_IPV6_LOG_MSG \ - "ClientPreferIPv6%sPort 1 is ignored unless tor is using IPv6. " \ - "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges." - - if (!fascist_firewall_use_ipv6(options) - && options->ClientPreferIPv6ORPort == 1) - log_warn(LD_CONFIG, WARN_PLEASE_USE_IPV6_LOG_MSG, "OR"); - - if (!fascist_firewall_use_ipv6(options) - && options->ClientPreferIPv6DirPort == 1) - log_warn(LD_CONFIG, WARN_PLEASE_USE_IPV6_LOG_MSG, "Dir"); - -#undef WARN_PLEASE_USE_IPV6_LOG_MSG - if (options->UseBridges && server_mode(options)) REJECT("Servers must be able to freely connect to the rest " diff --git a/src/test/test_options.c b/src/test/test_options.c index e85e11805b..febd3928da 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -1794,14 +1794,6 @@ test_options_validate__reachable_addresses(void *ignored) /* Test IPv4-only clients setting IPv6 preferences */ -#define WARN_PLEASE_USE_IPV6_OR_LOG_MSG \ - "ClientPreferIPv6ORPort 1 is ignored unless tor is using IPv6. " \ - "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges.\n" - -#define WARN_PLEASE_USE_IPV6_DIR_LOG_MSG \ - "ClientPreferIPv6DirPort 1 is ignored unless tor is using IPv6. " \ - "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges.\n" - free_options_test_data(tdata); tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "ClientUseIPv4 1\n" @@ -1811,7 +1803,6 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - expect_log_msg(WARN_PLEASE_USE_IPV6_OR_LOG_MSG); tor_free(msg); free_options_test_data(tdata); @@ -1823,7 +1814,6 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - expect_log_msg(WARN_PLEASE_USE_IPV6_DIR_LOG_MSG); tor_free(msg); /* Now test an IPv4/IPv6 client setting IPv6 preferences */ From 0417dae5808ddf86aed3a9b9a6883fa8f0922d2e Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 16 Dec 2016 22:34:42 +1100 Subject: [PATCH 3/5] When IPv6 addresses have not been downloaded, use hard-coded address info The microdesc consensus does not contain any IPv6 addresses. When a client has a microdesc consensus but no microdescriptor, make it use the hard-coded IPv6 address for the node (if available). (Hard-coded addresses can come from authorities, fallback directories, or configured bridges.) If there is no hard-coded address, log a BUG message, and fail the connection attempt. (All existing code checks for a hard-coded address before choosing a node address.) Fixes 20996, fix on b167e82 from 19608 in 0.2.8.5-alpha. --- src/or/nodelist.c | 21 ++++++++++++++++----- src/or/policies.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- src/or/routerlist.c | 2 +- 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 3f2e111077..b29f217efd 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -1126,6 +1126,9 @@ node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out) node_assert_ok(node); tor_assert(ap_out); + /* Check ri first, because rewrite_node_address_for_bridge() updates + * node->ri with the configured bridge address. */ + RETURN_IPV4_AP(node->ri, or_port, ap_out); RETURN_IPV4_AP(node->rs, or_port, ap_out); /* Microdescriptors only have an IPv6 address */ @@ -1156,9 +1159,11 @@ node_get_pref_ipv6_orport(const node_t *node, tor_addr_port_t *ap_out) node_assert_ok(node); tor_assert(ap_out); - /* Prefer routerstatus over microdesc for consistency with the - * fascist_firewall_* functions. Also check if the address or port are valid, - * and try another alternative if they are not. */ + /* Check ri first, because rewrite_node_address_for_bridge() updates + * node->ri with the configured bridge address. + * Prefer rs over md for consistency with the fascist_firewall_* functions. + * Check if the address or port are valid, and try another alternative + * if they are not. */ if (node->ri && tor_addr_port_is_valid(&node->ri->ipv6_addr, node->ri->ipv6_orport, 0)) { @@ -1218,6 +1223,9 @@ node_get_prim_dirport(const node_t *node, tor_addr_port_t *ap_out) node_assert_ok(node); tor_assert(ap_out); + /* Check ri first, because rewrite_node_address_for_bridge() updates + * node->ri with the configured bridge address. */ + RETURN_IPV4_AP(node->ri, dir_port, ap_out); RETURN_IPV4_AP(node->rs, dir_port, ap_out); /* Microdescriptors only have an IPv6 address */ @@ -1250,8 +1258,11 @@ node_get_pref_ipv6_dirport(const node_t *node, tor_addr_port_t *ap_out) node_assert_ok(node); tor_assert(ap_out); - /* Check if the address or port are valid, and try another alternative if - * they are not. Note that microdescriptors have no dir_port. */ + /* Check ri first, because rewrite_node_address_for_bridge() updates + * node->ri with the configured bridge address. + * Prefer rs over md for consistency with the fascist_firewall_* functions. + * Check if the address or port are valid, and try another alternative + * if they are not. */ /* Assume IPv4 and IPv6 dirports are the same */ if (node->ri && tor_addr_port_is_valid(&node->ri->ipv6_addr, diff --git a/src/or/policies.c b/src/or/policies.c index dcee65301c..ac57e7209f 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -20,6 +20,7 @@ #include "or.h" #include "config.h" #include "dirserv.h" +#include "microdesc.h" #include "networkstatus.h" #include "nodelist.h" #include "policies.h" @@ -893,6 +894,33 @@ fascist_firewall_choose_address_ipv4h(uint32_t ipv4h_addr, pref_ipv6, ap); } +/* The microdescriptor consensus has no IPv6 addresses in rs: they are in + * the microdescriptors. This means we can't rely on the node's IPv6 address + * until its microdescriptor is available (when using microdescs). + * But for bridges, rewrite_node_address_for_bridge() updates node->ri with + * the configured address, so we can trust bridge addresses. + * (Bridges could gain an IPv6 address if their microdescriptor arrives, but + * this will never be their preferred address: that is in the config.) + * Returns true if the node needs a microdescriptor for its IPv6 address, and + * false if the addresses in the node are already up-to-date. + */ +static int +node_awaiting_ipv6(const or_options_t* options, const node_t *node) +{ + tor_assert(node); + + /* There's no point waiting for an IPv6 address if we'd never use it */ + if (!fascist_firewall_use_ipv6(options)) { + return 0; + } + + /* We are waiting if we_use_microdescriptors_for_circuits() and we have no + * md. Bridges have a ri based on their config. They would never use the + * address from their md, so there's no need to wait for it. */ + return (!node->md && we_use_microdescriptors_for_circuits(options) && + !node->ri); +} + /** Like fascist_firewall_choose_address_base(), but takes rs. * Consults the corresponding node, then falls back to rs if node is NULL. * This should only happen when there's no valid consensus, and rs doesn't @@ -909,15 +937,15 @@ fascist_firewall_choose_address_rs(const routerstatus_t *rs, tor_assert(ap); + const or_options_t *options = get_options(); const node_t *node = node_get_by_id(rs->identity_digest); - if (node) { + if (node && !node_awaiting_ipv6(options, node)) { return fascist_firewall_choose_address_node(node, fw_connection, pref_only, ap); } else { /* There's no node-specific IPv6 preference, so use the generic IPv6 * preference instead. */ - const or_options_t *options = get_options(); int pref_ipv6 = (fw_connection == FIREWALL_OR_CONNECTION ? fascist_firewall_prefer_ipv6_orport(options) : fascist_firewall_prefer_ipv6_dirport(options)); @@ -951,6 +979,18 @@ fascist_firewall_choose_address_node(const node_t *node, node_assert_ok(node); + /* Calling fascist_firewall_choose_address_node() when the node is missing + * IPv6 information breaks IPv6-only clients. + * If the node is a hard-coded fallback directory or authority, call + * fascist_firewall_choose_address_rs() on the fake (hard-coded) routerstatus + * for the node. + * If it is not hard-coded, check that the node has a microdescriptor, full + * descriptor (routerinfo), or is one of our configured bridges before + * calling this function. */ + if (BUG(node_awaiting_ipv6(get_options(), node))) { + return 0; + } + const int pref_ipv6_node = (fw_connection == FIREWALL_OR_CONNECTION ? node_ipv6_or_preferred(node) : node_ipv6_dir_preferred(node)); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 6bac57d4db..043419562c 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -948,7 +948,7 @@ authority_certs_fetch_resource_impl(const char *resource, /* If we've just downloaded a consensus from a bridge, re-use that * bridge */ - if (options->UseBridges && node && !get_via_tor) { + if (options->UseBridges && node && node->ri && !get_via_tor) { /* clients always make OR connections to bridges */ tor_addr_port_t or_ap; /* we are willing to use a non-preferred address if we need to */ From 2debcc869fa6672d85b238a03ab0988ac74547e4 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 16 Dec 2016 22:43:46 +1100 Subject: [PATCH 4/5] Remove redundant boolean expression from firewall_is_fascist_impl() Let A = UseBridges Let B = ClientUseIPv4 Then firewall_is_fascist_impl expands and simplifies to: B || (!(A || ...) && A) B || (!A && ... && A) B || 0 B --- src/or/policies.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/or/policies.c b/src/or/policies.c index ac57e7209f..84600f7ef8 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -317,10 +317,8 @@ firewall_is_fascist_impl(void) const or_options_t *options = get_options(); /* Assume every non-bridge relay has an IPv4 address. * Clients which use bridges may only know the IPv6 address of their - * bridge. */ - return (options->ClientUseIPv4 == 0 - || (!fascist_firewall_use_ipv6(options) - && options->UseBridges == 1)); + * bridge, but they will connect regardless of the ClientUseIPv6 setting. */ + return options->ClientUseIPv4 == 0; } /** Return true iff the firewall options, including ClientUseIPv4 0 and From 8a7e5c3ba082c5a90db2f4e1bc701fcdcfb86cd8 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 16 Dec 2016 22:57:17 +1100 Subject: [PATCH 5/5] Changes file for 20996 --- changes/bug20996 | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changes/bug20996 diff --git a/changes/bug20996 b/changes/bug20996 new file mode 100644 index 0000000000..b2be805b0a --- /dev/null +++ b/changes/bug20996 @@ -0,0 +1,9 @@ + o Minor bugfixes (IPv6): + - Make IP6-using clients try harder to find an IPv6 directory server. + Fixes bug 20999; bugfix on 77a9de0 from 17840 in 0.2.8.2-alpha. + - When IPv6 addresses have not been downloaded, use hard-coded address + info for authorities, fallbacks, and configured bridges. + (When IPv6-only clients receive a microdesc consensus, it has no IPv6 + addresses, so they can't use it until microdescs are downloaded.) + This allows IPv6-only clients to use microdescriptors. + Fixes bug 20996; bugfix on b167e82 from 19608 in 0.2.8.5-alpha.