diff --git a/changes/proposal178 b/changes/proposal178 new file mode 100644 index 0000000000..ee706952ae --- /dev/null +++ b/changes/proposal178 @@ -0,0 +1,6 @@ + o Major features: + - Implement a more secure consensus parameter voting algorithm that + ensures that at least three directory authorities or a majority of + them voted on a given parameter before including it in the + consensus. Implements proposal 178. + diff --git a/src/or/dirvote.c b/src/or/dirvote.c index bf34c62af3..01e2358c44 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -50,7 +50,7 @@ static int dirvote_publish_consensus(void); static char *make_consensus_method_list(int low, int high, const char *sep); /** The highest consensus method that we currently support. */ -#define MAX_SUPPORTED_CONSENSUS_METHOD 11 +#define MAX_SUPPORTED_CONSENSUS_METHOD 12 /** Lowest consensus method that contains a 'directory-footer' marker */ #define MIN_METHOD_FOR_FOOTER 9 @@ -64,6 +64,10 @@ static char *make_consensus_method_list(int low, int high, const char *sep); /** Lowest consensus method that generates microdescriptors */ #define MIN_METHOD_FOR_MICRODESC 8 +/** Lowest consensus method that ensures a majority of authorities voted + * for a param. */ +#define MIN_METHOD_FOR_MAJORITY_PARAMS 12 + /* ===== * Voting * =====*/ @@ -608,11 +612,16 @@ compute_consensus_versions_list(smartlist_t *lst, int n_versioning) return result; } +/** Minimum number of directory authorities voting for a parameter to + * include it in the consensus, if consensus method 12 or later is to be + * used. See proposal 178 for details. */ +#define MIN_VOTES_FOR_PARAM 3 + /** Helper: given a list of valid networkstatus_t, return a new string * containing the contents of the consensus network parameter set. */ /* private */ char * -dirvote_compute_params(smartlist_t *votes) +dirvote_compute_params(smartlist_t *votes, int method, int total_authorities) { int i; int32_t *vals; @@ -669,11 +678,17 @@ dirvote_compute_params(smartlist_t *votes) next_param = smartlist_get(param_list, param_sl_idx+1); if (!next_param || strncmp(next_param, param, cur_param_len)) { /* We've reached the end of a series. */ - int32_t median = median_int32(vals, i); - char *out_string = tor_malloc(64+cur_param_len); - memcpy(out_string, param, cur_param_len); - tor_snprintf(out_string+cur_param_len,64, "%ld", (long)median); - smartlist_add(output, out_string); + /* Make sure enough authorities voted on this param, unless the + * the consensus method we use is too old for that. */ + if (method < MIN_METHOD_FOR_MAJORITY_PARAMS || + i > total_authorities/2 || + i >= MIN_VOTES_FOR_PARAM) { + int32_t median = median_int32(vals, i); + char *out_string = tor_malloc(64+cur_param_len); + memcpy(out_string, param, cur_param_len); + tor_snprintf(out_string+cur_param_len,64, "%ld", (long)median); + smartlist_add(output, out_string); + } i = 0; if (next_param) { @@ -1496,7 +1511,8 @@ networkstatus_compute_consensus(smartlist_t *votes, } if (consensus_method >= MIN_METHOD_FOR_PARAMS) { - params = dirvote_compute_params(votes); + params = dirvote_compute_params(votes, consensus_method, + total_authorities); if (params) { smartlist_add(chunks, tor_strdup("params ")); smartlist_add(chunks, params); diff --git a/src/or/dirvote.h b/src/or/dirvote.h index d19635173f..1f4dc362b5 100644 --- a/src/or/dirvote.h +++ b/src/or/dirvote.h @@ -84,7 +84,8 @@ document_signature_t *voter_get_sig_by_algorithm( #ifdef DIRVOTE_PRIVATE char *format_networkstatus_vote(crypto_pk_env_t *private_key, networkstatus_t *v3_ns); -char *dirvote_compute_params(smartlist_t *votes); +char *dirvote_compute_params(smartlist_t *votes, int method, + int total_authorities); #endif #endif diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 205d7b577c..5b7ce5cabe 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -620,13 +620,81 @@ test_dir_param_voting(void) test_eq(0, networkstatus_get_param(&vote4, "foobar", 0, -100, 8)); smartlist_add(votes, &vote1); + + /* Do the first tests without adding all the other votes, for + * networks without many dirauths. */ + + res = dirvote_compute_params(votes, 11, 6); + test_streq(res, "ab=90 abcd=20 cw=50 x-yz=-99"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 2); + test_streq(res, ""); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 1); + test_streq(res, "ab=90 abcd=20 cw=50 x-yz=-99"); + tor_free(res); + smartlist_add(votes, &vote2); + + res = dirvote_compute_params(votes, 11, 2); + test_streq(res, "ab=27 abcd=20 cw=5 x-yz=-99"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 2); + test_streq(res, "ab=27 cw=5 x-yz=-99"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 3); + test_streq(res, "ab=27 cw=5 x-yz=-99"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 6); + test_streq(res, ""); + tor_free(res); + smartlist_add(votes, &vote3); + + res = dirvote_compute_params(votes, 11, 3); + test_streq(res, "ab=27 abcd=20 c=60 cw=50 x-yz=-9 zzzzz=101"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 3); + test_streq(res, "ab=27 abcd=20 cw=50 x-yz=-9"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 5); + test_streq(res, "cw=50 x-yz=-9"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 9); + test_streq(res, "cw=50 x-yz=-9"); + tor_free(res); + smartlist_add(votes, &vote4); - res = dirvote_compute_params(votes); - test_streq(res, - "ab=90 abcd=20 c=1 cw=50 x-yz=-9 zzzzz=101"); + res = dirvote_compute_params(votes, 11, 4); + test_streq(res, "ab=90 abcd=20 c=1 cw=50 x-yz=-9 zzzzz=101"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 4); + test_streq(res, "ab=90 abcd=20 cw=50 x-yz=-9"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 5); + test_streq(res, "ab=90 abcd=20 cw=50 x-yz=-9"); + tor_free(res); + + /* Test that the special-cased "at least three dirauths voted for + * this param" logic works as expected. */ + res = dirvote_compute_params(votes, 12, 6); + test_streq(res, "ab=90 abcd=20 cw=50 x-yz=-9"); + tor_free(res); + + res = dirvote_compute_params(votes, 12, 10); + test_streq(res, "ab=90 abcd=20 cw=50 x-yz=-9"); + tor_free(res); done: tor_free(res); @@ -1049,7 +1117,7 @@ test_dir_v3_networkstatus(void) "Running:Stable:V2Dir:Valid"); tor_free(cp); cp = smartlist_join_strings(con->net_params, ":", 0, NULL); - test_streq(cp, "bar=2000000000:circuitwindow=80:foo=660"); + test_streq(cp, "circuitwindow=80:foo=660"); tor_free(cp); test_eq(4, smartlist_len(con->voters)); /*3 voters, 1 legacy key.*/