From 69347f48e00ae31222ce10d66c507293954e4fcc Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 14:31:51 -0400 Subject: [PATCH 1/9] Disable consensus methods before 25. Consensus method 25 is the oldest one supported by any stable version of 0.2.9, which is our current most-recent LTS. Thus, by proposal 290, they should be removed. This commit does not actually remove the code to implement these methods: it only makes it so authorities will no longer support them. I'll remove the backend code for them in later commits. --- src/or/dirvote.c | 8 +------- src/or/dirvote.h | 2 +- src/test/test_dir.c | 14 ++++++++------ 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index c3cd0d3cd1..2657155054 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -3951,13 +3951,7 @@ static const struct consensus_method_range_t { int low; int high; } microdesc_consensus_methods[] = { - {MIN_SUPPORTED_CONSENSUS_METHOD, MIN_METHOD_FOR_A_LINES - 1}, - {MIN_METHOD_FOR_A_LINES, MIN_METHOD_FOR_P6_LINES - 1}, - {MIN_METHOD_FOR_P6_LINES, MIN_METHOD_FOR_NTOR_KEY - 1}, - {MIN_METHOD_FOR_NTOR_KEY, MIN_METHOD_FOR_ID_HASH_IN_MD - 1}, - {MIN_METHOD_FOR_ID_HASH_IN_MD, MIN_METHOD_FOR_ED25519_ID_IN_MD - 1}, - {MIN_METHOD_FOR_ED25519_ID_IN_MD, - MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC - 1}, + {MIN_SUPPORTED_CONSENSUS_METHOD, MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC - 1}, {MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC, MAX_SUPPORTED_CONSENSUS_METHOD}, {-1, -1} }; diff --git a/src/or/dirvote.h b/src/or/dirvote.h index deeb27bfe1..91da4edeef 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -56,7 +56,7 @@ #define ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD 0 /** The lowest consensus method that we currently support. */ -#define MIN_SUPPORTED_CONSENSUS_METHOD 13 +#define MIN_SUPPORTED_CONSENSUS_METHOD 25 /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 28 diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 5fac045b26..6cafa7cb35 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -2917,8 +2917,9 @@ gen_routerstatus_for_umbw(int idx, time_t now) rs->addr = 0x99008801; rs->or_port = 443; rs->dir_port = 8000; - /* all flags but running cleared */ + /* all flags but running and valid cleared */ rs->is_flagged_running = 1; + rs->is_valid = 1; /* * This one has measured bandwidth below the clip cutoff, and * so shouldn't be clipped; we'll have to test that it isn't @@ -2991,8 +2992,9 @@ gen_routerstatus_for_umbw(int idx, time_t now) rs->addr = 0xC0000203; rs->or_port = 500; rs->dir_port = 1999; - /* all flags but running cleared */ + /* all flags but running and valid cleared */ rs->is_flagged_running = 1; + rs->is_valid = 1; /* * This one has unmeasured bandwidth below the clip cutoff, and * so shouldn't be clipped; we'll have to test that it isn't @@ -3014,7 +3016,7 @@ gen_routerstatus_for_umbw(int idx, time_t now) if (vrs) { vrs->microdesc = tor_malloc_zero(sizeof(vote_microdesc_hash_t)); tor_asprintf(&vrs->microdesc->microdesc_hash_line, - "m 9,10,11,12,13,14,15,16,17 " + "m 25,26,27,28 " "sha256=xyzajkldsdsajdadlsdjaslsdksdjlsdjsdaskdaaa%d\n", idx); } @@ -3040,7 +3042,7 @@ vote_tweaks_for_umbw(networkstatus_t *v, int voter, time_t now) smartlist_clear(v->supported_methods); /* Method 17 is MIN_METHOD_TO_CLIP_UNMEASURED_BW_KB */ smartlist_split_string(v->supported_methods, - "1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17", + "25 26 27 28", NULL, 0, -1); /* If we're using a non-default clip bandwidth, add it to net_params */ if (alternate_clip_bw > 0) { @@ -3202,9 +3204,9 @@ test_routerstatus_for_umbw(routerstatus_t *rs, time_t now) tt_assert(!rs->is_fast); tt_assert(!rs->is_possible_guard); tt_assert(!rs->is_stable); - /* (If it wasn't running it wouldn't be here) */ + /* (If it wasn't running and valid it wouldn't be here) */ tt_assert(rs->is_flagged_running); - tt_assert(!rs->is_valid); + tt_assert(rs->is_valid); tt_assert(!rs->is_named); /* This one should have measured bandwidth below the clip cutoff */ tt_assert(rs->has_bandwidth); From 08373467b10a3e3c4765835d1820f86741b9f307 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 14:44:03 -0400 Subject: [PATCH 2/9] Remove MIN_METHOD_FOR and MIN_METHOD_FOR_A_LINES Also, in networkstatus.c, remove client code for recognizing pre- MIN_METHOD_FOR_A_LINES consensuses, and corresponding unit tests in test_dir.c. --- src/or/dirvote.c | 15 ++++++--------- src/or/dirvote.h | 7 ------- src/or/networkstatus.c | 2 +- src/test/test_dir.c | 18 +++++------------- 4 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 2657155054..1985d90679 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -549,12 +549,12 @@ compute_routerstatus_consensus(smartlist_t *votes, int consensus_method, tor_assert(most); - /* If we're producing "a" lines, vote on potential alternative (sets - * of) OR port(s) in the winning routerstatuses. + /* Vote on potential alternative (sets of) OR port(s) in the winning + * routerstatuses. * * XXX prop186 There's at most one alternative OR port (_the_ IPv6 * port) for now. */ - if (consensus_method >= MIN_METHOD_FOR_A_LINES && best_alt_orport_out) { + if (best_alt_orport_out) { smartlist_t *alt_orports = smartlist_new(); const tor_addr_port_t *most_alt_orport = NULL; @@ -1890,10 +1890,8 @@ networkstatus_compute_consensus(smartlist_t *votes, rs_out.published_on = rs->status.published_on; rs_out.dir_port = rs->status.dir_port; rs_out.or_port = rs->status.or_port; - if (consensus_method >= MIN_METHOD_FOR_A_LINES) { - tor_addr_copy(&rs_out.ipv6_addr, &alt_orport.addr); - rs_out.ipv6_orport = alt_orport.port; - } + tor_addr_copy(&rs_out.ipv6_addr, &alt_orport.addr); + rs_out.ipv6_orport = alt_orport.port; rs_out.has_bandwidth = 0; rs_out.has_exitsummary = 0; @@ -3846,8 +3844,7 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) /* 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_A_LINES && - consensus_method < MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC && + 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)); diff --git a/src/or/dirvote.h b/src/or/dirvote.h index 91da4edeef..9c7916a9be 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -61,13 +61,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 28 -/** Lowest consensus method where microdesc consensuses omit any entry - * with no microdesc. */ -#define MIN_METHOD_FOR_MANDATORY_MICRODESC 13 - -/** Lowest consensus method that contains "a" lines. */ -#define MIN_METHOD_FOR_A_LINES 14 - /** Lowest consensus method where microdescs may include a "p6" line. */ #define MIN_METHOD_FOR_P6_LINES 15 diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 235b95b704..d7088a1670 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1528,7 +1528,7 @@ networkstatus_consensus_has_ipv6(const or_options_t* options) return cons->consensus_method >= MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS; } else { - return cons->consensus_method >= MIN_METHOD_FOR_A_LINES; + return 1; } } diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 6cafa7cb35..e5355069a3 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -5771,22 +5771,10 @@ test_dir_networkstatus_consensus_has_ipv6(void *arg) /* Test the bounds for A lines in the NS consensus */ mock_options->UseMicrodescriptors = 0; - mock_networkstatus->consensus_method = MIN_METHOD_FOR_A_LINES; + mock_networkstatus->consensus_method = MIN_SUPPORTED_CONSENSUS_METHOD; has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); tt_assert(has_ipv6); - mock_networkstatus->consensus_method = MIN_METHOD_FOR_A_LINES + 1; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(has_ipv6); - - mock_networkstatus->consensus_method = MIN_METHOD_FOR_A_LINES + 20; - has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); - tt_assert(has_ipv6); - - mock_networkstatus->consensus_method = MIN_METHOD_FOR_A_LINES - 1; - 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; @@ -5795,6 +5783,10 @@ test_dir_networkstatus_consensus_has_ipv6(void *arg) 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_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS + 1; has_ipv6 = networkstatus_consensus_has_ipv6(get_options()); From 93380db8333609b8613a1e987894c6f1fd8be6aa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 14:51:36 -0400 Subject: [PATCH 3/9] Remove MIN_METHOD_{FOR_P6_LINES,FOR_NTOR_KEY,TO_CLIP_UNMEASURED_BW} Also remove a unit test for pre-MIN_METHOD_FOR_NTOR_KEY consensuses. --- src/or/dirvote.c | 14 +++++--------- src/or/dirvote.h | 11 ----------- src/test/test_microdesc.c | 11 ----------- 3 files changed, 5 insertions(+), 31 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 1985d90679..334ebfb7f7 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1601,7 +1601,7 @@ networkstatus_compute_consensus(smartlist_t *votes, smartlist_free(dir_sources); } - if (consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW) { + { char *max_unmeasured_param = NULL; /* XXXX Extract this code into a common function. Or don't! see #19011 */ if (params) { @@ -1989,8 +1989,7 @@ networkstatus_compute_consensus(smartlist_t *votes, rs_out.has_bandwidth = 1; rs_out.bw_is_unmeasured = 1; rs_out.bandwidth_kb = median_uint32(bandwidths_kb, num_bandwidths); - if (consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW && - n_authorities_measuring_bandwidth > 2) { + if (n_authorities_measuring_bandwidth > 2) { /* Cap non-measured bandwidths. */ if (rs_out.bandwidth_kb > max_unmeasured_bw_kb) { rs_out.bandwidth_kb = max_unmeasured_bw_kb; @@ -2130,8 +2129,7 @@ networkstatus_compute_consensus(smartlist_t *votes, /* Now the weight line. */ if (rs_out.has_bandwidth) { char *guardfraction_str = NULL; - int unmeasured = rs_out.bw_is_unmeasured && - consensus_method >= MIN_METHOD_TO_CLIP_UNMEASURED_BW; + int unmeasured = rs_out.bw_is_unmeasured; /* If we have guardfraction info, include it in the 'w' line. */ if (rs_out.has_guardfraction) { @@ -3833,8 +3831,7 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) smartlist_add_asprintf(chunks, "onion-key\n%s", key); - if (consensus_method >= MIN_METHOD_FOR_NTOR_KEY && - ri->onion_curve25519_pkey) { + if (ri->onion_curve25519_pkey) { char kbuf[128]; base64_encode(kbuf, sizeof(kbuf), (const char*)ri->onion_curve25519_pkey->public_key, @@ -3855,8 +3852,7 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) if (summary && strcmp(summary, "reject 1-65535")) smartlist_add_asprintf(chunks, "p %s\n", summary); - if (consensus_method >= MIN_METHOD_FOR_P6_LINES && - ri->ipv6_exit_policy) { + if (ri->ipv6_exit_policy) { /* XXXX+++ This doesn't match proposal 208, which says these should * be taken unchanged from the routerinfo. That's bogosity, IMO: * the proposal should have said to do this instead.*/ diff --git a/src/or/dirvote.h b/src/or/dirvote.h index 9c7916a9be..311bc61e16 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -61,17 +61,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 28 -/** Lowest consensus method where microdescs may include a "p6" line. */ -#define MIN_METHOD_FOR_P6_LINES 15 - -/** Lowest consensus method where microdescs may include an onion-key-ntor - * line */ -#define MIN_METHOD_FOR_NTOR_KEY 16 - -/** Lowest consensus method that ensures that authorities output an - * Unmeasured=1 flag for unmeasured bandwidths */ -#define MIN_METHOD_TO_CLIP_UNMEASURED_BW 17 - /** Lowest consensus method where authorities may include an "id" line in * microdescriptors. */ #define MIN_METHOD_FOR_ID_HASH_IN_MD 18 diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index 59b28f7580..c0eced1bbb 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -385,15 +385,6 @@ static const char test_ri2[] = "cf34GXHv61XReJF3AlzNHFpbrPOYmowmhrTULKyMqow=\n" "-----END SIGNATURE-----\n"; -static const char test_md_8[] = - "onion-key\n" - "-----BEGIN RSA PUBLIC KEY-----\n" - "MIGJAoGBANBJz8Vldl12aFeSMPLiA4nOetLDN0oxU8bB1SDhO7Uu2zdWYVYAF5J0\n" - "st7WvrVy/jA9v/fsezNAPskBanecHRSkdMTpkcgRPMHE7CTGEwIy1Yp1X4bPgDlC\n" - "VCnbs5Pcts5HnWEYNK7qHDAUn+IlmjOO+pTUY8uyq+GQVz6H9wFlAgMBAAE=\n" - "-----END RSA PUBLIC KEY-----\n" - "p reject 25,119,135-139,445,563,1214,4661-4666,6346-6429,6699,6881-6999\n"; - static const char test_md_16[] = "onion-key\n" "-----BEGIN RSA PUBLIC KEY-----\n" @@ -444,8 +435,6 @@ test_md_generate(void *arg) ri = router_parse_entry_from_string(test_ri, NULL, 0, 0, NULL, NULL); tt_assert(ri); - md = dirvote_create_microdescriptor(ri, 8); - tt_str_op(md->body, OP_EQ, test_md_8); /* XXXX test family lines. */ /* XXXX test method 14 for A lines. */ From caf766991dfd82dd1599ce90adc8afb9e749f468 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 14:54:46 -0400 Subject: [PATCH 4/9] Remove MIN_METHOD_FOR_ID_HASH_IN_MD and a test for running without it. --- src/or/dirvote.c | 2 +- src/or/dirvote.h | 4 ---- src/test/test_microdesc.c | 19 ------------------- 3 files changed, 1 insertion(+), 24 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 334ebfb7f7..1a21def463 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -3862,7 +3862,7 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) tor_free(p6); } - if (consensus_method >= MIN_METHOD_FOR_ID_HASH_IN_MD) { + { char idbuf[ED25519_BASE64_LEN+1]; const char *keytype; if (consensus_method >= MIN_METHOD_FOR_ED25519_ID_IN_MD && diff --git a/src/or/dirvote.h b/src/or/dirvote.h index 311bc61e16..334dc07321 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -61,10 +61,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 28 -/** Lowest consensus method where authorities may include an "id" line in - * microdescriptors. */ -#define MIN_METHOD_FOR_ID_HASH_IN_MD 18 - /** Lowest consensus method where we include "package" lines*/ #define MIN_METHOD_FOR_PACKAGE_LINES 19 diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index c0eced1bbb..fc565ebf31 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -385,16 +385,6 @@ static const char test_ri2[] = "cf34GXHv61XReJF3AlzNHFpbrPOYmowmhrTULKyMqow=\n" "-----END SIGNATURE-----\n"; -static const char test_md_16[] = - "onion-key\n" - "-----BEGIN RSA PUBLIC KEY-----\n" - "MIGJAoGBANBJz8Vldl12aFeSMPLiA4nOetLDN0oxU8bB1SDhO7Uu2zdWYVYAF5J0\n" - "st7WvrVy/jA9v/fsezNAPskBanecHRSkdMTpkcgRPMHE7CTGEwIy1Yp1X4bPgDlC\n" - "VCnbs5Pcts5HnWEYNK7qHDAUn+IlmjOO+pTUY8uyq+GQVz6H9wFlAgMBAAE=\n" - "-----END RSA PUBLIC KEY-----\n" - "ntor-onion-key Gg73xH7+kTfT6bi1uNVx9gwQdQas9pROIfmc4NpAdC4=\n" - "p reject 25,119,135-139,445,563,1214,4661-4666,6346-6429,6699,6881-6999\n"; - static const char test_md_18[] = "onion-key\n" "-----BEGIN RSA PUBLIC KEY-----\n" @@ -436,15 +426,6 @@ test_md_generate(void *arg) ri = router_parse_entry_from_string(test_ri, NULL, 0, 0, NULL, NULL); tt_assert(ri); - /* XXXX test family lines. */ - /* XXXX test method 14 for A lines. */ - /* XXXX test method 15 for P6 lines. */ - - microdesc_free(md); - md = NULL; - md = dirvote_create_microdescriptor(ri, 16); - tt_str_op(md->body, OP_EQ, test_md_16); - microdesc_free(md); md = NULL; md = dirvote_create_microdescriptor(ri, 18); From 4d6f21bb6b43fb79bb4a659ced8aa898a42389a8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 14:59:34 -0400 Subject: [PATCH 5/9] Remove MIN_METHOD_FOR_{PACKAGE_LINES,GUARDFRACTION,ED25519_ID_IN_MD} Also remove a rest for pre-19 microdesc versions. --- src/or/dirvote.c | 19 +++---------------- src/or/dirvote.h | 12 ------------ src/test/test_microdesc.c | 15 --------------- 3 files changed, 3 insertions(+), 43 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 1a21def463..80db32eb52 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -664,13 +664,6 @@ compute_consensus_method(smartlist_t *votes) static int consensus_method_is_supported(int method) { - if (method == MIN_METHOD_FOR_ED25519_ID_IN_MD) { - /* This method was broken due to buggy code accidentally left in - * dircollate.c; do not actually use it. - */ - return 0; - } - return (method >= MIN_SUPPORTED_CONSENSUS_METHOD) && (method <= MAX_SUPPORTED_CONSENSUS_METHOD); } @@ -1455,11 +1448,7 @@ networkstatus_compute_consensus(smartlist_t *votes, n_versioning_servers); client_versions = compute_consensus_versions_list(combined_client_versions, n_versioning_clients); - if (consensus_method >= MIN_METHOD_FOR_PACKAGE_LINES) { - packages = compute_consensus_package_lines(votes); - } else { - packages = tor_strdup(""); - } + packages = compute_consensus_package_lines(votes); SMARTLIST_FOREACH(combined_server_versions, char *, cp, tor_free(cp)); SMARTLIST_FOREACH(combined_client_versions, char *, cp, tor_free(cp)); @@ -1971,8 +1960,7 @@ networkstatus_compute_consensus(smartlist_t *votes, /* If it's a guard and we have enough guardfraction votes, calculate its consensus guardfraction value. */ - if (is_guard && num_guardfraction_inputs > 2 && - consensus_method >= MIN_METHOD_FOR_GUARDFRACTION) { + if (is_guard && num_guardfraction_inputs > 2) { rs_out.has_guardfraction = 1; rs_out.guardfraction_percentage = median_uint32(measured_guardfraction, num_guardfraction_inputs); @@ -3865,8 +3853,7 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) { char idbuf[ED25519_BASE64_LEN+1]; const char *keytype; - if (consensus_method >= MIN_METHOD_FOR_ED25519_ID_IN_MD && - ri->cache_info.signing_key_cert && + if (ri->cache_info.signing_key_cert && ri->cache_info.signing_key_cert->signing_key_included) { keytype = "ed25519"; ed25519_public_to_base64(idbuf, diff --git a/src/or/dirvote.h b/src/or/dirvote.h index 334dc07321..14fa8e0ad2 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -61,18 +61,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 28 -/** Lowest consensus method where we include "package" lines*/ -#define MIN_METHOD_FOR_PACKAGE_LINES 19 - -/** Lowest consensus method where authorities may include - * GuardFraction information in microdescriptors. */ -#define MIN_METHOD_FOR_GUARDFRACTION 20 - -/** Lowest consensus method where authorities may include an "id" line for - * ed25519 identities in microdescriptors. (Broken; see - * consensus_method_is_supported() for more info.) */ -#define MIN_METHOD_FOR_ED25519_ID_IN_MD 21 - /** Lowest consensus method where authorities vote on ed25519 ids and ensure * ed25519 id consistency. */ #define MIN_METHOD_FOR_ED25519_ID_VOTING 22 diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index fc565ebf31..3d02fc1a59 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -396,16 +396,6 @@ static const char test_md_18[] = "p reject 25,119,135-139,445,563,1214,4661-4666,6346-6429,6699,6881-6999\n" "id rsa1024 Cd47okjCHD83YGzThGBDptXs9Z4\n"; -static const char test_md2_18[] = - "onion-key\n" - "-----BEGIN RSA PUBLIC KEY-----\n" - "MIGJAoGBAL2R8EfubUcahxha4u02P4VAR0llQIMwFAmrHPjzcK7apcQgDOf2ovOA\n" - "+YQnJFxlpBmCoCZC6ssCi+9G0mqo650lFuTMP5I90BdtjotfzESfTykHLiChyvhd\n" - "l0dlqclb2SU/GKem/fLRXH16aNi72CdSUu/1slKs/70ILi34QixRAgMBAAE=\n" - "-----END RSA PUBLIC KEY-----\n" - "ntor-onion-key hbxdRnfVUJJY7+KcT4E3Rs7/zuClbN3hJrjSBiEGMgI=\n" - "id rsa1024 t+J/EEITw28T5+mCkYKEXklZl6A\n"; - static const char test_md2_21[] = "onion-key\n" "-----BEGIN RSA PUBLIC KEY-----\n" @@ -439,11 +429,6 @@ test_md_generate(void *arg) routerinfo_free(ri); ri = router_parse_entry_from_string(test_ri2, NULL, 0, 0, NULL, NULL); - microdesc_free(md); - md = NULL; - md = dirvote_create_microdescriptor(ri, 18); - tt_str_op(md->body, OP_EQ, test_md2_18); - microdesc_free(md); md = NULL; md = dirvote_create_microdescriptor(ri, 21); From 73c9c16faab299b483716c1b8e76c433282d3d50 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 15:02:57 -0400 Subject: [PATCH 6/9] Remove MIN_METHOD_FOR_ED25519_ID_VOTING This also lets us remove the old rsa-based routerstatus collator. --- src/or/dircollate.c | 38 +++----------------------------------- src/or/dirvote.c | 7 ++----- src/or/dirvote.h | 4 ---- 3 files changed, 5 insertions(+), 44 deletions(-) diff --git a/src/or/dircollate.c b/src/or/dircollate.c index ce4534ff6c..dec6f75154 100644 --- a/src/or/dircollate.c +++ b/src/or/dircollate.c @@ -25,7 +25,6 @@ #include "dircollate.h" #include "dirvote.h" -static void dircollator_collate_by_rsa(dircollator_t *dc); static void dircollator_collate_by_ed25519(dircollator_t *dc); /** Hashtable entry mapping a pair of digests (actually an ed25519 key and an @@ -208,48 +207,17 @@ dircollator_add_vote(dircollator_t *dc, networkstatus_t *v) void dircollator_collate(dircollator_t *dc, int consensus_method) { + (void) consensus_method; + tor_assert(!dc->is_collated); dc->all_rsa_sha1_lst = smartlist_new(); - if (consensus_method < MIN_METHOD_FOR_ED25519_ID_VOTING) - dircollator_collate_by_rsa(dc); - else - dircollator_collate_by_ed25519(dc); + dircollator_collate_by_ed25519(dc); smartlist_sort_digests(dc->all_rsa_sha1_lst); dc->is_collated = 1; } -/** - * Collation function for RSA-only consensuses: collate the votes for each - * entry in dc by their RSA keys. - * - * The rule is: - * If an RSA identity key is listed by more than half of the authorities, - * include that identity, and treat all descriptors with that RSA identity - * as describing the same router. - */ -static void -dircollator_collate_by_rsa(dircollator_t *dc) -{ - const int total_authorities = dc->n_authorities; - - DIGESTMAP_FOREACH(dc->by_rsa_sha1, k, vote_routerstatus_t **, vrs_lst) { - int n = 0, i; - for (i = 0; i < dc->n_votes; ++i) { - if (vrs_lst[i] != NULL) - ++n; - } - - if (n <= total_authorities / 2) - continue; - - smartlist_add(dc->all_rsa_sha1_lst, (char *)k); - } DIGESTMAP_FOREACH_END; - - dc->by_collated_rsa_sha1 = dc->by_rsa_sha1; -} - /** * Collation function for ed25519 consensuses: collate the votes for each * entry in dc by ed25519 key and by RSA key. diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 80db32eb52..6b553083b1 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1455,8 +1455,7 @@ networkstatus_compute_consensus(smartlist_t *votes, smartlist_free(combined_server_versions); smartlist_free(combined_client_versions); - if (consensus_method >= MIN_METHOD_FOR_ED25519_ID_VOTING) - smartlist_add_strdup(flags, "NoEdConsensus"); + smartlist_add_strdup(flags, "NoEdConsensus"); smartlist_sort_strings(flags); smartlist_uniq_strings(flags); @@ -1852,7 +1851,6 @@ networkstatus_compute_consensus(smartlist_t *votes, continue; if (ed_consensus > 0) { - tor_assert(consensus_method >= MIN_METHOD_FOR_ED25519_ID_VOTING); if (ed_consensus <= total_authorities / 2) { log_warn(LD_BUG, "Not enough entries had ed_consensus set; how " "can we have a consensus of %d?", ed_consensus); @@ -1910,8 +1908,7 @@ networkstatus_compute_consensus(smartlist_t *votes, } else if (!strcmp(fl, "Unnamed")) { if (is_unnamed) smartlist_add(chosen_flags, (char*)fl); - } else if (!strcmp(fl, "NoEdConsensus") && - consensus_method >= MIN_METHOD_FOR_ED25519_ID_VOTING) { + } else if (!strcmp(fl, "NoEdConsensus")) { if (ed_consensus <= total_authorities/2) smartlist_add(chosen_flags, (char*)fl); } else { diff --git a/src/or/dirvote.h b/src/or/dirvote.h index 14fa8e0ad2..defcdf5754 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -61,10 +61,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 28 -/** Lowest consensus method where authorities vote on ed25519 ids and ensure - * ed25519 id consistency. */ -#define MIN_METHOD_FOR_ED25519_ID_VOTING 22 - /** Lowest consensus method where authorities may include a shared random * value(s). */ #define MIN_METHOD_FOR_SHARED_RANDOM 23 From 5f90d28c01960921ad1cd680ba7e1275a44345c0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 15:07:51 -0400 Subject: [PATCH 7/9] Remove MIN_METHOD_FOR_{SHARED_RANDOM,EXCLUDING_INVALID_NODES} Also remove client detection for pre-EXCLUDING_INVALID_NODES consensuses, and a test for that detection. --- src/or/dirvote.c | 5 ++--- src/or/dirvote.h | 8 -------- src/or/routerparse.c | 3 +-- src/test/test_dir.c | 16 ++-------------- 4 files changed, 5 insertions(+), 27 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 6b553083b1..25ca961fa6 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1524,7 +1524,7 @@ networkstatus_compute_consensus(smartlist_t *votes, smartlist_add_strdup(chunks, "\n"); } - if (consensus_method >= MIN_METHOD_FOR_SHARED_RANDOM) { + { int num_dirauth = get_n_authorities(V3_DIRINFO); /* Default value of this is 2/3 of the total number of authorities. For * instance, if we have 9 dirauth, the default value is 6. The following @@ -1935,8 +1935,7 @@ networkstatus_compute_consensus(smartlist_t *votes, /* Starting with consensus method 24, we don't list servers * that are not valid in a consensus. See Proposal 272 */ - if (!is_valid && - consensus_method >= MIN_METHOD_FOR_EXCLUDING_INVALID_NODES) + if (!is_valid) continue; /* Pick the version. */ diff --git a/src/or/dirvote.h b/src/or/dirvote.h index defcdf5754..8a317deb47 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -61,14 +61,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 28 -/** Lowest consensus method where authorities may include a shared random - * value(s). */ -#define MIN_METHOD_FOR_SHARED_RANDOM 23 - -/** Lowest consensus method where authorities drop all nodes that don't get - * the Valid flag. */ -#define MIN_METHOD_FOR_EXCLUDING_INVALID_NODES 24 - /** Lowest consensus method where authorities vote on required/recommended * protocols. */ #define MIN_METHOD_FOR_RECOMMENDED_PROTOCOLS 25 diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 79499f2e6f..1834cfad24 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -2743,8 +2743,7 @@ routerstatus_parse_entry_from_string(memarea_t *area, /* These are implied true by having been included in a consensus made * with a given method */ rs->is_flagged_running = 1; /* Starting with consensus method 4. */ - if (consensus_method >= MIN_METHOD_FOR_EXCLUDING_INVALID_NODES) - rs->is_valid = 1; + rs->is_valid = 1; /* Starting with consensus method 24. */ } { const char *protocols = NULL, *version = NULL; diff --git a/src/test/test_dir.c b/src/test/test_dir.c index e5355069a3..df5ae2d594 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -5617,9 +5617,8 @@ test_dir_assumed_flags(void *arg) memarea_t *area = memarea_new(); routerstatus_t *rs = NULL; - /* First, we should always assume that the Running flag is set, even - * when it isn't listed, since the consensus method is always - * higher than 4. */ + /* We can assume that consensus method is higher than 24, so Running and + * Valid are always implicitly set */ const char *str1 = "r example hereiswhereyouridentitygoes 2015-08-30 12:00:00 " "192.168.0.1 9001 0\n" @@ -5627,17 +5626,6 @@ test_dir_assumed_flags(void *arg) "s Fast Guard Stable\n"; const char *cp = str1; - rs = routerstatus_parse_entry_from_string(area, &cp, tokens, NULL, NULL, - 23, FLAV_MICRODESC); - tt_assert(rs); - tt_assert(rs->is_flagged_running); - tt_assert(! rs->is_valid); - tt_assert(! rs->is_exit); - tt_assert(rs->is_fast); - routerstatus_free(rs); - - /* With method 24 or later, we can assume "valid" is set. */ - cp = str1; rs = routerstatus_parse_entry_from_string(area, &cp, tokens, NULL, NULL, 24, FLAV_MICRODESC); tt_assert(rs); From 2609a8be81c744f1c3568c83cce035836ec50758 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 15:10:27 -0400 Subject: [PATCH 8/9] Require MIN_METHOD_FOR_RECOMMENDED_PROTOCOLS (Remove support for running without this method.) --- src/or/dirvote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 25ca961fa6..f3b8a19f00 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1504,7 +1504,7 @@ networkstatus_compute_consensus(smartlist_t *votes, tor_free(flaglist); } - if (consensus_method >= MIN_METHOD_FOR_RECOMMENDED_PROTOCOLS) { + { int num_dirauth = get_n_authorities(V3_DIRINFO); int idx; for (idx = 0; idx < 4; ++idx) { From ade3c7400761229a54c9a1280833672e35452176 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2018 15:13:37 -0400 Subject: [PATCH 9/9] Add a changes file for removing old consensus methods (Ticket 24378; proposal 290.) --- changes/24378 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changes/24378 diff --git a/changes/24378 b/changes/24378 new file mode 100644 index 0000000000..663d27e969 --- /dev/null +++ b/changes/24378 @@ -0,0 +1,8 @@ + o Removed features: + + - Directory authorities will no longer support voting according to any + consensus method before consensus method 25. This keeps authorities + compatible with all authorities running 0.2.9.8 and later, and does + not break any clients or relays. Implements ticket 24378 and + proposal 290. +