Merge branch 'safer_params_squashed'

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.
+

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);
+      /* Make sure enough authorities voted on this param, unless the
-      char *out_string = tor_malloc(64+cur_param_len);
+       * the consensus method we use is too old for that. */
-      memcpy(out_string, param, cur_param_len);
+      if (method < MIN_METHOD_FOR_MAJORITY_PARAMS ||
-      tor_snprintf(out_string+cur_param_len,64, "%ld", (long)median);
+          i > total_authorities/2 ||
-      smartlist_add(output, out_string);
+          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);

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

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);
+  res = dirvote_compute_params(votes, 11, 4);
-  test_streq(res,
+  test_streq(res, "ab=90 abcd=20 c=1 cw=50 x-yz=-9 zzzzz=101");
-             "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.*/