aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--changes/bug190117
-rw-r--r--src/feature/dirauth/dirvote.c115
-rw-r--r--src/feature/dirauth/dirvote.h10
-rw-r--r--src/test/test_dirvote.c25
4 files changed, 105 insertions, 52 deletions
diff --git a/changes/bug19011 b/changes/bug19011
new file mode 100644
index 0000000000..de178fd438
--- /dev/null
+++ b/changes/bug19011
@@ -0,0 +1,7 @@
+ o Minor bugfixes (directory authorities, voting):
+ - Add a new consensus method (31) to support any future changes that
+ authorities decide to make to the value of bwweightscale or
+ maxunmeasuredbw. Previously, there was a bug that prevented the
+ authorities from parsing these consensus parameters correctly under
+ most circumstances.
+ Fixes bug 19011; bugfix on 0.2.2.10-alpha.
diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c
index a1f9bb28ae..4f494a75a3 100644
--- a/src/feature/dirauth/dirvote.c
+++ b/src/feature/dirauth/dirvote.c
@@ -1757,26 +1757,14 @@ networkstatus_compute_consensus(smartlist_t *votes,
}
{
- char *max_unmeasured_param = NULL;
- /* XXXX Extract this code into a common function. Or don't! see #19011 */
- if (params) {
- if (strcmpstart(params, "maxunmeasuredbw=") == 0)
- max_unmeasured_param = params;
- else
- max_unmeasured_param = strstr(params, " maxunmeasuredbw=");
- }
- if (max_unmeasured_param) {
- int ok = 0;
- char *eq = strchr(max_unmeasured_param, '=');
- if (eq) {
- max_unmeasured_bw_kb = (uint32_t)
- tor_parse_ulong(eq+1, 10, 1, UINT32_MAX, &ok, NULL);
- if (!ok) {
- log_warn(LD_DIR, "Bad element '%s' in max unmeasured bw param",
- escaped(max_unmeasured_param));
- max_unmeasured_bw_kb = DEFAULT_MAX_UNMEASURED_BW_KB;
- }
- }
+ 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;
}
}
@@ -2326,38 +2314,16 @@ networkstatus_compute_consensus(smartlist_t *votes,
smartlist_add_strdup(chunks, "directory-footer\n");
{
- int64_t weight_scale = BW_WEIGHT_SCALE;
- char *bw_weight_param = NULL;
-
- // Parse params, extract BW_WEIGHT_SCALE if present
- // DO NOT use consensus_param_bw_weight_scale() in this code!
- // The consensus is not formed yet!
- /* XXXX Extract this code into a common function. Or not: #19011. */
- if (params) {
- if (strcmpstart(params, "bwweightscale=") == 0)
- bw_weight_param = params;
- else
- bw_weight_param = strstr(params, " bwweightscale=");
- }
-
- if (bw_weight_param) {
- int ok=0;
- char *eq = strchr(bw_weight_param, '=');
- if (eq) {
- weight_scale = tor_parse_long(eq+1, 10, 1, INT32_MAX, &ok,
- NULL);
- if (!ok) {
- log_warn(LD_DIR, "Bad element '%s' in bw weight param",
- escaped(bw_weight_param));
- weight_scale = BW_WEIGHT_SCALE;
- }
- } else {
- log_warn(LD_DIR, "Bad element '%s' in bw weight param",
- escaped(bw_weight_param));
- weight_scale = BW_WEIGHT_SCALE;
- }
+ 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;
}
-
added_weights = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D,
T, weight_scale);
}
@@ -2459,6 +2425,53 @@ 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 4f48e45dc3..7d30a51d08 100644
--- a/src/feature/dirauth/dirvote.h
+++ b/src/feature/dirauth/dirvote.h
@@ -53,7 +53,7 @@
#define MIN_SUPPORTED_CONSENSUS_METHOD 28
/** The highest consensus method that we currently support. */
-#define MAX_SUPPORTED_CONSENSUS_METHOD 30
+#define MAX_SUPPORTED_CONSENSUS_METHOD 31
/**
* Lowest consensus method where microdescriptor lines are put in canonical
@@ -65,6 +65,11 @@
* 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
+
/** Default bandwidth to clip unmeasured bandwidths to using method >=
* MIN_METHOD_TO_CLIP_UNMEASURED_BW. (This is not a consensus method; do not
* get confused with the above macros.) */
@@ -256,6 +261,9 @@ 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);
/** The recommended relay protocols for this authority's votes.
* Recommending a new protocol causes old tor versions to log a warning.
diff --git a/src/test/test_dirvote.c b/src/test/test_dirvote.c
index b5e57ad071..d92d1aaf90 100644
--- a/src/test/test_dirvote.c
+++ b/src/test/test_dirvote.c
@@ -656,6 +656,30 @@ 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 \
@@ -668,4 +692,5 @@ 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};