Merge branch 'obsolete_consensus_methods' into 'main'

Remove consensus methods before #32

Closes #40835

See merge request tpo/core/tor!771

changes/ticket40835

@@ -0,0 +1,3 @@
+  o Removed features:
+    - Directory authorities no longer support consensus methods
+      before method 32. Closes ticket 40835.

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. */
@@ -2134,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");
@@ -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 *
@@ -3921,6 +3864,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,20 +3883,15 @@ 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);
   }
 
   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,10 +3989,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},
   {-1, -1}
 };

src/feature/dirauth/dirvote.h

@@ -50,30 +50,11 @@
                 ((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
 
-/**
- * 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
-
-/** 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
-
 /**
  * Lowest consensus method for which we suppress the published time in
  * microdescriptor consensuses.
@@ -280,9 +261,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) */
 

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,13 +4102,12 @@ 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) {
-    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);

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};

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);