From 441a048a3a135a9405f4fb8463fb60a0f11e42d2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 9 Jan 2020 12:41:56 -0500 Subject: [PATCH 1/3] Remove support for now-obsolete consensus methods before 28. Closes ticket 32695. --- changes/ticket32695 | 6 ++++++ src/feature/dirauth/dirvote.c | 18 +++--------------- src/feature/dirauth/dirvote.h | 22 +--------------------- src/feature/nodelist/fmt_routerstatus.c | 8 ++------ src/feature/nodelist/networkstatus.c | 10 +++------- src/test/test_dir.c | 13 ++++--------- 6 files changed, 19 insertions(+), 58 deletions(-) create mode 100644 changes/ticket32695 diff --git a/changes/ticket32695 b/changes/ticket32695 new file mode 100644 index 0000000000..2df53144eb --- /dev/null +++ b/changes/ticket32695 @@ -0,0 +1,6 @@ + o Removed features: + - We no longer support consensus methods before method 28; these + methods were only used by authorities running versions of Tor that + are now at end-of-life. In effect, this means that clients and + relays, and authorities now assume that authorities will be + running version 0.3.5.x or later. Closes ticket 32695. diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 9889170a26..c1d4ff1a7d 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -1540,14 +1540,11 @@ networkstatus_compute_consensus(smartlist_t *votes, consensus_method = MAX_SUPPORTED_CONSENSUS_METHOD; } - if (consensus_method >= MIN_METHOD_FOR_INIT_BW_WEIGHTS_ONE) { + { /* It's smarter to initialize these weights to 1, so that later on, * we can't accidentally divide by zero. */ G = M = E = D = 1; T = 4; - } else { - /* ...but originally, they were set to zero. */ - G = M = E = D = T = 0; } /* Compute medians of time-related things, and figure out how many @@ -2268,8 +2265,7 @@ networkstatus_compute_consensus(smartlist_t *votes, smartlist_add_strdup(chunks, chosen_version); } smartlist_add_strdup(chunks, "\n"); - if (chosen_protocol_list && - consensus_method >= MIN_METHOD_FOR_RS_PROTOCOLS) { + if (chosen_protocol_list) { smartlist_add_asprintf(chunks, "pr %s\n", chosen_protocol_list); } /* Now the weight line. */ @@ -3805,13 +3801,6 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) smartlist_add_asprintf(chunks, "ntor-onion-key %s", kbuf); } - /* We originally put a lines in the micrdescriptors, but then we worked out - * that we needed them in the microdesc consensus. See #20916. */ - if (consensus_method < MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC && - !tor_addr_is_null(&ri->ipv6_addr) && ri->ipv6_orport) - smartlist_add_asprintf(chunks, "a %s\n", - fmt_addrport(&ri->ipv6_addr, ri->ipv6_orport)); - if (family) { if (consensus_method < MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS) { smartlist_add_asprintf(chunks, "family %s\n", family); @@ -3917,8 +3906,7 @@ static const struct consensus_method_range_t { int low; int high; } microdesc_consensus_methods[] = { - {MIN_SUPPORTED_CONSENSUS_METHOD, MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC - 1}, - {MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC, + {MIN_SUPPORTED_CONSENSUS_METHOD, MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS - 1}, {MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS, MAX_SUPPORTED_CONSENSUS_METHOD}, diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index 063977d025..b614f303dc 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -54,31 +54,11 @@ #define ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD 0 /** The lowest consensus method that we currently support. */ -#define MIN_SUPPORTED_CONSENSUS_METHOD 25 +#define MIN_SUPPORTED_CONSENSUS_METHOD 28 /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 29 -/** Lowest consensus method where authorities vote on required/recommended - * protocols. */ -#define MIN_METHOD_FOR_RECOMMENDED_PROTOCOLS 25 - -/** Lowest consensus method where authorities add protocols to routerstatus - * entries. */ -#define MIN_METHOD_FOR_RS_PROTOCOLS 25 - -/** Lowest consensus method where authorities initialize bandwidth weights to 1 - * instead of 0. See #14881 */ -#define MIN_METHOD_FOR_INIT_BW_WEIGHTS_ONE 26 - -/** Lowest consensus method where the microdesc consensus contains relay IPv6 - * addresses. See #23826 and #20916. */ -#define MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS 27 - -/** Lowest consensus method where microdescriptors do not contain relay IPv6 - * addresses. See #23828 and #20916. */ -#define MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC 28 - /** * Lowest consensus method where microdescriptor lines are put in canonical * form for improved compressibility and ease of storage. See proposal 298. diff --git a/src/feature/nodelist/fmt_routerstatus.c b/src/feature/nodelist/fmt_routerstatus.c index 8dde0088de..e9b00939e6 100644 --- a/src/feature/nodelist/fmt_routerstatus.c +++ b/src/feature/nodelist/fmt_routerstatus.c @@ -50,6 +50,8 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version, int consensus_method, const vote_routerstatus_t *vrs) { + (void) consensus_method; + char *summary; char *result = NULL; @@ -78,12 +80,6 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version, * networkstatus_type_t values, with an additional control port value * added -MP */ - /* V3 microdesc consensuses only have "a" lines in later consensus methods - */ - if (format == NS_V3_CONSENSUS_MICRODESC && - consensus_method < MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS) - goto done; - /* Possible "a" line. At most one for now. */ if (!tor_addr_is_null(&rs->ipv6_addr)) { smartlist_add_asprintf(chunks, "a %s\n", diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index 7868020477..f88b5e6d65 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -1585,6 +1585,7 @@ networkstatus_consensus_is_already_downloading(const char *resource) int networkstatus_consensus_has_ipv6(const or_options_t* options) { + (void) options; const networkstatus_t *cons = networkstatus_get_reasonably_live_consensus( approx_time(), usable_consensus_flavor()); @@ -1594,13 +1595,8 @@ networkstatus_consensus_has_ipv6(const or_options_t* options) return 0; } - /* Different flavours of consensus gained IPv6 at different times */ - if (we_use_microdescriptors_for_circuits(options)) { - return - cons->consensus_method >= MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS; - } else { - return 1; - } + /* All supported consensus methods have IPv6 addresses. */ + return 1; } /** Given two router status entries for the same router identity, return 1 if diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 3a7ba4292e..535c7bb69a 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -7108,7 +7108,7 @@ test_dir_networkstatus_consensus_has_ipv6(void *arg) mock_options->UseMicrodescriptors = 1; mock_networkstatus->consensus_method = - MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS; + MIN_SUPPORTED_CONSENSUS_METHOD; has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); tt_assert(has_ipv6); @@ -7117,24 +7117,19 @@ test_dir_networkstatus_consensus_has_ipv6(void *arg) tt_assert(has_ipv6); mock_networkstatus->consensus_method = - MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS + 1; + MIN_SUPPORTED_CONSENSUS_METHOD + 1; has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); tt_assert(has_ipv6); mock_networkstatus->consensus_method = - MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS + 20; + MIN_SUPPORTED_CONSENSUS_METHOD + 20; has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); tt_assert(has_ipv6); - mock_networkstatus->consensus_method = - MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS - 1; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(!has_ipv6); - /* Test the edge cases */ mock_options->UseMicrodescriptors = 1; mock_networkstatus->consensus_method = - MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS; + MIN_SUPPORTED_CONSENSUS_METHOD; /* Reasonably live */ mock_networkstatus->valid_until = approx_time() - 60; From 8d94bcbf8c7c34a6e2d683982ef22ab6949c34ea Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 15 Jan 2020 11:20:30 -0500 Subject: [PATCH 2/3] Remove routerstatus_format_entry() consensus_method argument as unused --- src/feature/dirauth/dirvote.c | 3 +-- src/feature/dirauth/dirvote.h | 4 ---- src/feature/nodelist/fmt_routerstatus.c | 7 ------- src/feature/nodelist/fmt_routerstatus.h | 1 - src/feature/nodelist/networkstatus.c | 1 - 5 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index c1d4ff1a7d..97fe66c1a2 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -384,7 +384,6 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key, rsf = routerstatus_format_entry(&vrs->status, vrs->version, vrs->protocols, NS_V3_VOTE, - ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD, vrs); if (rsf) smartlist_add(chunks, rsf); @@ -2245,7 +2244,7 @@ networkstatus_compute_consensus(smartlist_t *votes, /* Okay!! Now we can write the descriptor... */ /* First line goes into "buf". */ buf = routerstatus_format_entry(&rs_out, NULL, NULL, - rs_format, consensus_method, NULL); + rs_format, NULL); if (buf) smartlist_add(chunks, buf); } diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index b614f303dc..f695e93abf 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -49,10 +49,6 @@ #define MIN_VOTE_INTERVAL_TESTING_INITIAL \ ((MIN_VOTE_SECONDS_TESTING)+(MIN_DIST_SECONDS_TESTING)+1) -/* A placeholder for routerstatus_format_entry() when the consensus method - * argument is not applicable. */ -#define ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD 0 - /** The lowest consensus method that we currently support. */ #define MIN_SUPPORTED_CONSENSUS_METHOD 28 diff --git a/src/feature/nodelist/fmt_routerstatus.c b/src/feature/nodelist/fmt_routerstatus.c index e9b00939e6..b3f1c6962d 100644 --- a/src/feature/nodelist/fmt_routerstatus.c +++ b/src/feature/nodelist/fmt_routerstatus.c @@ -27,10 +27,6 @@ * allocated character buffer. Use the same format as in network-status * documents. If version is non-NULL, add a "v" line for the platform. * - * consensus_method is the current consensus method when format is - * NS_V3_CONSENSUS or NS_V3_CONSENSUS_MICRODESC. It is ignored for other - * formats: pass ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD. - * * Return 0 on success, -1 on failure. * * The format argument has one of the following values: @@ -47,11 +43,8 @@ char * routerstatus_format_entry(const routerstatus_t *rs, const char *version, const char *protocols, routerstatus_format_type_t format, - int consensus_method, const vote_routerstatus_t *vrs) { - (void) consensus_method; - char *summary; char *result = NULL; diff --git a/src/feature/nodelist/fmt_routerstatus.h b/src/feature/nodelist/fmt_routerstatus.h index 7a50027a31..a007989af3 100644 --- a/src/feature/nodelist/fmt_routerstatus.h +++ b/src/feature/nodelist/fmt_routerstatus.h @@ -35,7 +35,6 @@ char *routerstatus_format_entry( const char *version, const char *protocols, routerstatus_format_type_t format, - int consensus_method, const vote_routerstatus_t *vrs); #endif /* !defined(TOR_FMT_ROUTERSTATUS_H) */ diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index f88b5e6d65..d0fada44d7 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -2360,7 +2360,6 @@ char * networkstatus_getinfo_helper_single(const routerstatus_t *rs) { return routerstatus_format_entry(rs, NULL, NULL, NS_CONTROL_PORT, - ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD, NULL); } From ceacda44f1064a5cc06a7eb29f31de19844ca6a7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 15 Jan 2020 11:26:14 -0500 Subject: [PATCH 3/3] Remove functions that checked for pre-ipv6 consensus. We no longer need or need to test: * node_awaiting_ipv6() * networkstatus_consensus_has_ipv6(). --- src/core/or/policies.c | 56 +----------------- src/feature/nodelist/networkstatus.c | 22 ------- src/feature/nodelist/networkstatus.h | 1 - src/test/test_dir.c | 88 ---------------------------- 4 files changed, 1 insertion(+), 166 deletions(-) diff --git a/src/core/or/policies.c b/src/core/or/policies.c index 0f7cc5057d..a82995fe12 100644 --- a/src/core/or/policies.c +++ b/src/core/or/policies.c @@ -933,49 +933,6 @@ fascist_firewall_choose_address_ipv4h(uint32_t ipv4h_addr, pref_ipv6, ap); } -/* Some microdescriptor consensus methods have no IPv6 addresses in rs: they - * are in the microdescriptors. For these consensus methods, 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; - } - - /* If the node has an IPv6 address, we're not waiting */ - if (node_has_ipv6_addr(node)) { - return 0; - } - - /* If the current consensus method and flavour has IPv6 addresses, we're not - * waiting */ - if (networkstatus_consensus_has_ipv6(options)) { - return 0; - } - - /* Bridge clients never use the address from a bridge's md, so there's no - * need to wait for it. */ - if (node_is_a_configured_bridge(node)) { - return 0; - } - - /* We are waiting if we_use_microdescriptors_for_circuits() and we have no - * md. */ - return (!node->md && we_use_microdescriptors_for_circuits(options)); -} - /** 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 @@ -998,7 +955,7 @@ fascist_firewall_choose_address_rs(const routerstatus_t *rs, const or_options_t *options = get_options(); const node_t *node = node_get_by_id(rs->identity_digest); - if (node && !node_awaiting_ipv6(options, node)) { + if (node) { fascist_firewall_choose_address_node(node, fw_connection, pref_only, ap); } else { /* There's no node-specific IPv6 preference, so use the generic IPv6 @@ -1111,17 +1068,6 @@ 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; - } const int pref_ipv6_node = (fw_connection == FIREWALL_OR_CONNECTION ? node_ipv6_or_preferred(node) diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index d0fada44d7..868812aaa8 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -1577,28 +1577,6 @@ networkstatus_consensus_is_already_downloading(const char *resource) return answer; } -/* Does the current, reasonably live consensus have IPv6 addresses? - * Returns 1 if there is a reasonably live consensus and its consensus method - * includes IPv6 addresses in the consensus. - * Otherwise, if there is no consensus, or the method does not include IPv6 - * addresses, returns 0. */ -int -networkstatus_consensus_has_ipv6(const or_options_t* options) -{ - (void) options; - const networkstatus_t *cons = networkstatus_get_reasonably_live_consensus( - approx_time(), - usable_consensus_flavor()); - - /* If we have no consensus, we have no IPv6 in it */ - if (!cons) { - return 0; - } - - /* All supported consensus methods have IPv6 addresses. */ - return 1; -} - /** Given two router status entries for the same router identity, return 1 if * if the contents have changed between them. Otherwise, return 0. */ static int diff --git a/src/feature/nodelist/networkstatus.h b/src/feature/nodelist/networkstatus.h index b8430088c9..38929fa6b6 100644 --- a/src/feature/nodelist/networkstatus.h +++ b/src/feature/nodelist/networkstatus.h @@ -104,7 +104,6 @@ int networkstatus_consensus_can_use_multiple_directories( MOCK_DECL(int, networkstatus_consensus_can_use_extra_fallbacks,( const or_options_t *options)); int networkstatus_consensus_is_already_downloading(const char *resource); -int networkstatus_consensus_has_ipv6(const or_options_t* options); #define NSSET_FROM_CACHE 1 #define NSSET_WAS_WAITING_FOR_CERTS 2 diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 535c7bb69a..f16b054c62 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -7065,93 +7065,6 @@ test_dir_platform_str(void *arg) ; } -static networkstatus_t *mock_networkstatus; - -static networkstatus_t * -mock_networkstatus_get_latest_consensus_by_flavor(consensus_flavor_t f) -{ - (void)f; - return mock_networkstatus; -} - -static void -test_dir_networkstatus_consensus_has_ipv6(void *arg) -{ - (void)arg; - - int has_ipv6 = 0; - - /* Init options and networkstatus */ - or_options_t our_options; - mock_options = &our_options; - reset_options(mock_options, &mock_get_options_calls); - MOCK(get_options, mock_get_options); - - networkstatus_t our_networkstatus; - mock_networkstatus = &our_networkstatus; - memset(mock_networkstatus, 0, sizeof(*mock_networkstatus)); - MOCK(networkstatus_get_latest_consensus_by_flavor, - mock_networkstatus_get_latest_consensus_by_flavor); - - /* A live consensus */ - mock_networkstatus->valid_after = time(NULL) - 3600; - mock_networkstatus->valid_until = time(NULL) + 3600; - - /* Test the bounds for A lines in the NS consensus */ - mock_options->UseMicrodescriptors = 0; - - mock_networkstatus->consensus_method = MIN_SUPPORTED_CONSENSUS_METHOD; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(has_ipv6); - - /* Test the bounds for A lines in the microdesc consensus */ - mock_options->UseMicrodescriptors = 1; - - mock_networkstatus->consensus_method = - MIN_SUPPORTED_CONSENSUS_METHOD; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(has_ipv6); - - mock_networkstatus->consensus_method = MAX_SUPPORTED_CONSENSUS_METHOD + 20; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(has_ipv6); - - mock_networkstatus->consensus_method = - MIN_SUPPORTED_CONSENSUS_METHOD + 1; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(has_ipv6); - - mock_networkstatus->consensus_method = - MIN_SUPPORTED_CONSENSUS_METHOD + 20; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(has_ipv6); - - /* Test the edge cases */ - mock_options->UseMicrodescriptors = 1; - mock_networkstatus->consensus_method = - MIN_SUPPORTED_CONSENSUS_METHOD; - - /* Reasonably live */ - mock_networkstatus->valid_until = approx_time() - 60; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(has_ipv6); - - /* Not reasonably live */ - mock_networkstatus->valid_after = approx_time() - 24*60*60 - 3600; - mock_networkstatus->valid_until = approx_time() - 24*60*60 - 60; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(!has_ipv6); - - /* NULL consensus */ - mock_networkstatus = NULL; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(!has_ipv6); - - done: - UNMOCK(get_options); - UNMOCK(networkstatus_get_latest_consensus_by_flavor); -} - static void test_dir_format_versions_list(void *arg) { @@ -7290,7 +7203,6 @@ struct testcase_t dir_tests[] = { DIR(matching_flags, 0), DIR(networkstatus_compute_bw_weights_v10, 0), DIR(platform_str, 0), - DIR(networkstatus_consensus_has_ipv6, TT_FORK), DIR(format_versions_list, TT_FORK), END_OF_TESTCASES };