From 5edc59bfd185a894ae2b72984e87b480531963a4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 08:11:41 -0400 Subject: [PATCH 1/9] Increment MIN_SUPPORTED_CONSENSUS_METHOD to 32. Per proposal 290, all earlier consensus methods are obsolete, since 32 is the highest method supported by 0.4.7.7. --- src/feature/dirauth/dirvote.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index 6d02ef3635..9faea988d0 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -50,7 +50,7 @@ ((MIN_VOTE_SECONDS_TESTING)+(MIN_DIST_SECONDS_TESTING)+1) /** The lowest consensus method that we currently support. */ -#define MIN_SUPPORTED_CONSENSUS_METHOD 28 +#define MIN_SUPPORTED_CONSENSUS_METHOD 32 /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 34 From a937648022546e8b0953e2bc918bb609e59acaf3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 08:41:09 -0400 Subject: [PATCH 2/9] Update consensus methods in dir_umbw tests. These tests had previously listed methods that we no longer support. --- src/test/test_dir.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 248fd8ab5d..87aaef7fe6 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -4078,7 +4078,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 25,26,27,28 " + "m 32,33 " "sha256=xyzajkldsdsajdadlsdjaslsdksdjlsdjsdaskdaaa%d\n", idx); } @@ -4102,9 +4102,8 @@ vote_tweaks_for_umbw(networkstatus_t *v, int voter, time_t now) tt_assert(v->supported_methods); SMARTLIST_FOREACH(v->supported_methods, char *, c, tor_free(c)); smartlist_clear(v->supported_methods); - /* Method 17 is MIN_METHOD_TO_CLIP_UNMEASURED_BW_KB */ smartlist_split_string(v->supported_methods, - "25 26 27 28", + "32 33", NULL, 0, -1); /* If we're using a non-default clip bandwidth, add it to net_params */ if (alternate_clip_bw > 0) { From a7993bbd72e44868384832af67c03595918b4bfb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 08:59:51 -0400 Subject: [PATCH 3/9] Whoops: test the "wrong" name for maxunmeasurdbw (sic). Now that we never use an earlier consensus method, our tests actually hit this, and we find that we have misspelled "maxunmeasurdbw" (sic) in dirvote.c. I have opened ticket #40869 to track this misspelling. --- src/test/test_dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 87aaef7fe6..baf67be4f8 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -4107,7 +4107,7 @@ vote_tweaks_for_umbw(networkstatus_t *v, int voter, time_t now) NULL, 0, -1); /* If we're using a non-default clip bandwidth, add it to net_params */ if (alternate_clip_bw > 0) { - tor_asprintf(&maxbw_param, "maxunmeasuredbw=%u", alternate_clip_bw); + tor_asprintf(&maxbw_param, "maxunmeasurdbw=%u", alternate_clip_bw); tt_assert(maxbw_param); if (maxbw_param) { smartlist_add(v->net_params, maxbw_param); From a2ab949a13106fcafe1caaa6699424e53255525a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 08:14:36 -0400 Subject: [PATCH 4/9] Remove MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS --- src/feature/dirauth/dirvote.c | 14 ++++---------- src/feature/dirauth/dirvote.h | 6 ------ 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 81d8cffd25..fa919fbf48 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -3945,14 +3945,10 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) } if (family) { - if (consensus_method < MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS) { - smartlist_add_asprintf(chunks, "family %s\n", family); - } else { - const uint8_t *id = (const uint8_t *)ri->cache_info.identity_digest; - char *canonical_family = nodefamily_canonicalize(family, id, 0); - smartlist_add_asprintf(chunks, "family %s\n", canonical_family); - tor_free(canonical_family); - } + const uint8_t *id = (const uint8_t *)ri->cache_info.identity_digest; + char *canonical_family = nodefamily_canonicalize(family, id, 0); + smartlist_add_asprintf(chunks, "family %s\n", canonical_family); + tor_free(canonical_family); } if (summary && strcmp(summary, "reject 1-65535")) @@ -4050,8 +4046,6 @@ static const struct consensus_method_range_t { int high; } microdesc_consensus_methods[] = { {MIN_SUPPORTED_CONSENSUS_METHOD, - MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS - 1}, - {MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS, MIN_METHOD_FOR_UNPADDED_NTOR_KEY - 1}, {MIN_METHOD_FOR_UNPADDED_NTOR_KEY, MAX_SUPPORTED_CONSENSUS_METHOD}, diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index 9faea988d0..64afc2612c 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -55,12 +55,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 34 -/** - * Lowest consensus method where microdescriptor lines are put in canonical - * form for improved compressibility and ease of storage. See proposal 298. - **/ -#define MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS 29 - /** Lowest consensus method where an unpadded base64 onion-key-ntor is allowed * See #7869 */ #define MIN_METHOD_FOR_UNPADDED_NTOR_KEY 30 From 8ebb726d4d6b83e08a8fb3ac2ed97d54506c4b03 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 08:16:57 -0400 Subject: [PATCH 5/9] Remove MIN_METHOD_FOR_UNPADDED_NTOR_KEY --- src/feature/dirauth/dirvote.c | 6 ++---- src/feature/dirauth/dirvote.h | 4 ---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index fa919fbf48..682e2b19cd 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -3921,6 +3921,7 @@ dirvote_get_vote(const char *fp, int flags) STATIC microdesc_t * dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) { + (void) consensus_method; // Currently unneeded... microdesc_t *result = NULL; char *key = NULL, *summary = NULL, *family = NULL; size_t keylen; @@ -3939,8 +3940,7 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) if (ri->onion_curve25519_pkey) { char kbuf[CURVE25519_BASE64_PADDED_LEN + 1]; - bool add_padding = (consensus_method < MIN_METHOD_FOR_UNPADDED_NTOR_KEY); - curve25519_public_to_base64(kbuf, ri->onion_curve25519_pkey, add_padding); + curve25519_public_to_base64(kbuf, ri->onion_curve25519_pkey, false); smartlist_add_asprintf(chunks, "ntor-onion-key %s\n", kbuf); } @@ -4046,8 +4046,6 @@ static const struct consensus_method_range_t { int high; } microdesc_consensus_methods[] = { {MIN_SUPPORTED_CONSENSUS_METHOD, - MIN_METHOD_FOR_UNPADDED_NTOR_KEY - 1}, - {MIN_METHOD_FOR_UNPADDED_NTOR_KEY, MAX_SUPPORTED_CONSENSUS_METHOD}, {-1, -1} }; diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index 64afc2612c..b879990462 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -55,10 +55,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 34 -/** Lowest consensus method where an unpadded base64 onion-key-ntor is allowed - * See #7869 */ -#define MIN_METHOD_FOR_UNPADDED_NTOR_KEY 30 - /** Lowest consensus method for which we use the correct algorithm for * extracting the bwweightscale= and maxunmeasuredbw= parameters. See #19011. */ From 940a4c7eaad3b93069a0dd8dc7d539b466cfe980 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 08:25:44 -0400 Subject: [PATCH 6/9] Remove tests that checked for obsolete microdesc encoding. --- src/test/test_microdesc.c | 40 ++++----------------------------------- 1 file changed, 4 insertions(+), 36 deletions(-) diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index c564805ecf..315a38f257 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -366,37 +366,14 @@ static const char test_ri[] = "iFJkKxxDx7ksxX0zdl7aPT4ORFEuRhCYS6el7YJmoyg=\n" "-----END SIGNATURE-----\n"; -static const char test_md2_25[] = +static const char test_md2_withfamily_33[] = "onion-key\n" "-----BEGIN RSA PUBLIC KEY-----\n" "MIGJAoGBAMvEJ/JVNK7I38PPWhQMuCgkET/ki4WIas4tj5Kmqfb9kHqxMR+EunRD\n" "83k4pel1yB7QdV+iTd/4SZOI8RpZP+BO1KnOTWfpztAU1lDGr19/PwdwcHaILpBD\n" "nNzm6otk4/bKUQ0vqpOfJljtg0DfAm4uMAQ6BMFy6uEAF7+JupuPAgMBAAE=\n" "-----END RSA PUBLIC KEY-----\n" - "ntor-onion-key FChIfm77vrWB7JsxQ+jMbN6VSSp1P0DYbw/2aqey4iA=\n" - "p accept 1-65535\n" - "id ed25519 J5lkRqyL6qW+CpN3E4RIlgJZeLgwjtmOOrjZvVhuwLQ\n"; - -static const char test_md2_withfamily_28[] = - "onion-key\n" - "-----BEGIN RSA PUBLIC KEY-----\n" - "MIGJAoGBAMvEJ/JVNK7I38PPWhQMuCgkET/ki4WIas4tj5Kmqfb9kHqxMR+EunRD\n" - "83k4pel1yB7QdV+iTd/4SZOI8RpZP+BO1KnOTWfpztAU1lDGr19/PwdwcHaILpBD\n" - "nNzm6otk4/bKUQ0vqpOfJljtg0DfAm4uMAQ6BMFy6uEAF7+JupuPAgMBAAE=\n" - "-----END RSA PUBLIC KEY-----\n" - "ntor-onion-key FChIfm77vrWB7JsxQ+jMbN6VSSp1P0DYbw/2aqey4iA=\n" - "family OtherNode !Strange\n" - "p accept 1-65535\n" - "id ed25519 J5lkRqyL6qW+CpN3E4RIlgJZeLgwjtmOOrjZvVhuwLQ\n"; - -static const char test_md2_withfamily_29[] = - "onion-key\n" - "-----BEGIN RSA PUBLIC KEY-----\n" - "MIGJAoGBAMvEJ/JVNK7I38PPWhQMuCgkET/ki4WIas4tj5Kmqfb9kHqxMR+EunRD\n" - "83k4pel1yB7QdV+iTd/4SZOI8RpZP+BO1KnOTWfpztAU1lDGr19/PwdwcHaILpBD\n" - "nNzm6otk4/bKUQ0vqpOfJljtg0DfAm4uMAQ6BMFy6uEAF7+JupuPAgMBAAE=\n" - "-----END RSA PUBLIC KEY-----\n" - "ntor-onion-key FChIfm77vrWB7JsxQ+jMbN6VSSp1P0DYbw/2aqey4iA=\n" + "ntor-onion-key FChIfm77vrWB7JsxQ+jMbN6VSSp1P0DYbw/2aqey4iA\n" "family !Strange $D219590AC9513BCDEBBA9AB721007A4CC01BBAE3 othernode\n" "p accept 1-65535\n" "id ed25519 J5lkRqyL6qW+CpN3E4RIlgJZeLgwjtmOOrjZvVhuwLQ\n"; @@ -411,21 +388,12 @@ 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, 25); - tt_str_op(md->body, OP_EQ, test_md2_25); - tt_assert(ed25519_pubkey_eq(md->ed25519_identity_pkey, - &ri->cache_info.signing_key_cert->signing_key)); - // Try family encoding. microdesc_free(md); ri->declared_family = smartlist_new(); smartlist_add_strdup(ri->declared_family, "OtherNode !Strange"); - md = dirvote_create_microdescriptor(ri, 28); - tt_str_op(md->body, OP_EQ, test_md2_withfamily_28); - - microdesc_free(md); - md = dirvote_create_microdescriptor(ri, 29); - tt_str_op(md->body, OP_EQ, test_md2_withfamily_29); + md = dirvote_create_microdescriptor(ri, 33); + tt_str_op(md->body, OP_EQ, test_md2_withfamily_33); done: microdesc_free(md); From a62ea322464d4e1f8b0c8d48e3ad779a80fafac6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 08:17:59 -0400 Subject: [PATCH 7/9] Remove MIN_METHOD_FOR_CORRECT_BWWEIGHTSCALE This also lets us discard extract_param_buggy, which we've been wanting to do. --- src/feature/dirauth/dirvote.c | 73 ++++------------------------------- src/feature/dirauth/dirvote.h | 8 ---- src/test/test_dirvote.c | 25 ------------ 3 files changed, 8 insertions(+), 98 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 682e2b19cd..36025b88a9 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -1780,15 +1780,10 @@ networkstatus_compute_consensus(smartlist_t *votes, } { - if (consensus_method < MIN_METHOD_FOR_CORRECT_BWWEIGHTSCALE) { - max_unmeasured_bw_kb = (int32_t) extract_param_buggy( - params, "maxunmeasuredbw", DEFAULT_MAX_UNMEASURED_BW_KB); - } else { - max_unmeasured_bw_kb = dirvote_get_intermediate_param_value( - param_list, "maxunmeasurdbw", DEFAULT_MAX_UNMEASURED_BW_KB); - if (max_unmeasured_bw_kb < 1) - max_unmeasured_bw_kb = 1; - } + max_unmeasured_bw_kb = dirvote_get_intermediate_param_value( + param_list, "maxunmeasurdbw", DEFAULT_MAX_UNMEASURED_BW_KB); + if (max_unmeasured_bw_kb < 1) + max_unmeasured_bw_kb = 1; } /* Add the actual router entries. */ @@ -2371,15 +2366,10 @@ networkstatus_compute_consensus(smartlist_t *votes, { int64_t weight_scale; - if (consensus_method < MIN_METHOD_FOR_CORRECT_BWWEIGHTSCALE) { - weight_scale = extract_param_buggy(params, "bwweightscale", - BW_WEIGHT_SCALE); - } else { - weight_scale = dirvote_get_intermediate_param_value( - param_list, "bwweightscale", BW_WEIGHT_SCALE); - if (weight_scale < 1) - weight_scale = 1; - } + weight_scale = dirvote_get_intermediate_param_value( + param_list, "bwweightscale", BW_WEIGHT_SCALE); + if (weight_scale < 1) + weight_scale = 1; added_weights = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T, weight_scale); } @@ -2481,53 +2471,6 @@ networkstatus_compute_consensus(smartlist_t *votes, return result; } -/** Extract the value of a parameter from a string encoding a list of - * parameters, badly. - * - * This is a deliberately buggy implementation, for backward compatibility - * with versions of Tor affected by #19011. Once all authorities have - * upgraded to consensus method 31 or later, then we can throw away this - * function. */ -STATIC int64_t -extract_param_buggy(const char *params, - const char *param_name, - int64_t default_value) -{ - int64_t value = default_value; - const char *param_str = NULL; - - if (params) { - char *prefix1 = NULL, *prefix2=NULL; - tor_asprintf(&prefix1, "%s=", param_name); - tor_asprintf(&prefix2, " %s=", param_name); - if (strcmpstart(params, prefix1) == 0) - param_str = params; - else - param_str = strstr(params, prefix2); - tor_free(prefix1); - tor_free(prefix2); - } - - if (param_str) { - int ok=0; - char *eq = strchr(param_str, '='); - if (eq) { - value = tor_parse_long(eq+1, 10, 1, INT32_MAX, &ok, NULL); - if (!ok) { - log_warn(LD_DIR, "Bad element '%s' in %s", - escaped(param_str), param_name); - value = default_value; - } - } else { - log_warn(LD_DIR, "Bad element '%s' in %s", - escaped(param_str), param_name); - value = default_value; - } - } - - return value; -} - /** Given a list of networkstatus_t for each vote, return a newly allocated * string containing the "package" lines for the vote. */ STATIC char * diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index b879990462..4790180256 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -55,11 +55,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 34 -/** Lowest consensus method for which we use the correct algorithm for - * extracting the bwweightscale= and maxunmeasuredbw= parameters. See #19011. - */ -#define MIN_METHOD_FOR_CORRECT_BWWEIGHTSCALE 31 - /** Lowest consensus method for which we handle the MiddleOnly flag specially. */ #define MIN_METHOD_FOR_MIDDLEONLY 32 @@ -270,9 +265,6 @@ STATIC char *networkstatus_get_detached_signatures(smartlist_t *consensuses); STATIC microdesc_t *dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method); -STATIC int64_t extract_param_buggy(const char *params, - const char *param_name, - int64_t default_value); #endif /* defined(DIRVOTE_PRIVATE) */ diff --git a/src/test/test_dirvote.c b/src/test/test_dirvote.c index 2b53955107..bb7e6fdf10 100644 --- a/src/test/test_dirvote.c +++ b/src/test/test_dirvote.c @@ -656,30 +656,6 @@ done: ROUTER_FREE(pppp); } -static void -test_dirvote_parse_param_buggy(void *arg) -{ - (void)arg; - - /* Tests for behavior with bug emulation to migrate away from bug 19011. */ - tt_i64_op(extract_param_buggy("blah blah", "bwweightscale", 10000), - OP_EQ, 10000); - tt_i64_op(extract_param_buggy("bwweightscale=7", "bwweightscale", 10000), - OP_EQ, 7); - tt_i64_op(extract_param_buggy("bwweightscale=7 foo=9", - "bwweightscale", 10000), - OP_EQ, 10000); - tt_i64_op(extract_param_buggy("foo=7 bwweightscale=777 bar=9", - "bwweightscale", 10000), - OP_EQ, 10000); - tt_i64_op(extract_param_buggy("foo=7 bwweightscale=1234", - "bwweightscale", 10000), - OP_EQ, 1234); - - done: - ; -} - #define NODE(name, flags) \ { \ #name, test_dirvote_##name, (flags), NULL, NULL \ @@ -692,5 +668,4 @@ struct testcase_t dirvote_tests[] = { NODE(get_sybil_by_ip_version_ipv4, TT_FORK), NODE(get_sybil_by_ip_version_ipv6, TT_FORK), NODE(get_all_possible_sybil, TT_FORK), - NODE(parse_param_buggy, 0), END_OF_TESTCASES}; From 5b80a8f509ca7fbc8d141acc754685439519fcf1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 08:25:01 -0400 Subject: [PATCH 8/9] Remove MIN_METHOD_FOR_MIDDLEONLY --- src/feature/dirauth/dirvote.c | 2 +- src/feature/dirauth/dirvote.h | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 36025b88a9..2839c6db15 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -2129,7 +2129,7 @@ networkstatus_compute_consensus(smartlist_t *votes, /* Starting with consensus method 32, we handle the middle-only * flag specially: when it is present, we clear some flags, and * set others. */ - if (is_middle_only && consensus_method >= MIN_METHOD_FOR_MIDDLEONLY) { + if (is_middle_only) { remove_flag(chosen_flags, "Exit"); remove_flag(chosen_flags, "V2Dir"); remove_flag(chosen_flags, "Guard"); diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index 4790180256..6ac07f171a 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -55,10 +55,6 @@ /** The highest consensus method that we currently support. */ #define MAX_SUPPORTED_CONSENSUS_METHOD 34 -/** Lowest consensus method for which we handle the MiddleOnly flag specially. - */ -#define MIN_METHOD_FOR_MIDDLEONLY 32 - /** * Lowest consensus method for which we suppress the published time in * microdescriptor consensuses. From d2e03322a9fe83e270cd2b3ab4d41a49aa4ff1ec Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Oct 2023 12:34:15 -0400 Subject: [PATCH 9/9] Add a changes file for removing pre-32 consensus methods (#40835) --- changes/ticket40835 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/ticket40835 diff --git a/changes/ticket40835 b/changes/ticket40835 new file mode 100644 index 0000000000..cda51a5d28 --- /dev/null +++ b/changes/ticket40835 @@ -0,0 +1,3 @@ + o Removed features: + - Directory authorities no longer support consensus methods + before method 32. Closes ticket 40835.