Fix another case of 17668: Add NoEdConsensus

I had a half-built mechanism to track, during the voting process,
whether the Ed25519 value (or lack thereof) reflected a true
consensus among the authorities.  But we never actually inserted this
field in the consensus.

The key idea here is that we first attempt to match up votes by pairs
of <Ed,RSA>, where <Ed> can be NULL if we're told that there is no
Ed key.  If this succeeds, then we can treat all those votes as 'a
consensus for Ed'.  And we can include all other votes with a
matching RSA key and no statement about Ed keys as being "also about
the same relay."

After that, we look for RSA keys we haven't actually found an entry
for yet, and see if there are enough votes for them, NOT considering
Ed keys.  If there are, we match them as before, but we treat them
as "not a consensus about ed".

When we include an entry in a consensus, if it does not reflect a
consensus about ed keys, then we include a new NoEdConsensus flag on
it.

This is all only for consensus method 22 or later.

Also see corresponding dir-spec patch.

diff --git a/changes/bug17668 b/changes/bug17668
new file mode 100644 (file)
index 0000000..fa5c1c8
--- /dev/null
+++ b/changes/bug17668
@@ -0,0 +1,5 @@
+  o Major bugfixes (voting):
+    - When collating votes by Ed25519 identities, authorities now
+      include a "NoEdConsensus" flag if the ed25519 value (or lack thereof)
+      for a server does not reflect the majority consensus. Related to bug
+      17668; bugfix on 0.2.7.2-alpha.

diff --git a/src/or/dircollate.c b/src/or/dircollate.c
index eeb0c240082274ec8b37684144e3c0dc4326b3cb..ca8b7ca84757b0add167e41d976d315fd63ab937 100644 (file)
--- a/src/or/dircollate.c
+++ b/src/or/dircollate.c
@@ -81,6 +81,8 @@ dircollator_add_routerstatus(dircollator_t *dc,
 {
   const char *id = vrs->status.identity_digest;
 
+  vrs->ed25519_reflects_consensus = 0;
+
   (void) vote;
   vote_routerstatus_t **vrs_lst = digestmap_get(dc->by_rsa_sha1, id);
   if (NULL == vrs_lst) {
@@ -92,7 +94,7 @@ dircollator_add_routerstatus(dircollator_t *dc,
 
   const uint8_t *ed = vrs->ed25519_id;
 
-  if (tor_mem_is_zero((char*)ed, DIGEST256_LEN))
+  if (! vrs->has_ed25519_listing)
     return;
 
   ddmap_entry_t search, *found;

diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index be0635d92b9455c4036bf95a67eafc35a4d8a319..654d461dd660896deb951b3cd0bd5d1193a6f2b6 100644 (file)
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -1242,6 +1242,9 @@ networkstatus_compute_consensus(smartlist_t *votes,
     smartlist_free(combined_server_versions);
     smartlist_free(combined_client_versions);
 
+    if (consensus_method >= MIN_METHOD_FOR_ED25519_ID_VOTING)
+      smartlist_add(flags, tor_strdup("NoEdConsensus"));
+
     smartlist_sort_strings(flags);
     smartlist_uniq_strings(flags);
 
@@ -1539,6 +1542,8 @@ networkstatus_compute_consensus(smartlist_t *votes,
       num_bandwidths = 0;
       num_mbws = 0;
       num_guardfraction_inputs = 0;
+      int ed_consensus = 0;
+      const uint8_t *ed_consensus_val = NULL;
 
       /* Okay, go through all the entries for this digest. */
       for (int voter_idx = 0; voter_idx < smartlist_len(votes); ++voter_idx) {
@@ -1580,6 +1585,17 @@ networkstatus_compute_consensus(smartlist_t *votes,
 
         if (rs->status.has_bandwidth)
           bandwidths_kb[num_bandwidths++] = rs->status.bandwidth_kb;
+
+        /* Count number for which ed25519 is canonical. */
+        if (rs->ed25519_reflects_consensus) {
+          ++ed_consensus;
+          if (ed_consensus_val) {
+            tor_assert(fast_memeq(ed_consensus_val, rs->ed25519_id,
+                                  ED25519_PUBKEY_LEN));
+          } else {
+            ed_consensus_val = rs->ed25519_id;
+          }
+        }
       }
 
       /* We don't include this router at all unless more than half of
@@ -1587,6 +1603,14 @@ networkstatus_compute_consensus(smartlist_t *votes,
       if (n_listing <= total_authorities/2)
         continue;
 
+      if (ed_consensus > 0) {
+        tor_assert(consensus_method >= MIN_METHOD_FOR_ED25519_ID_VOTING);
+        if (ed_consensus <= total_authorities / 2) {
+          log_warn(LD_BUG, "Not enough entries had ed_consensus set; how "
+                   "can we have a consensus of %d?", ed_consensus);
+        }
+      }
+
       /* The clangalyzer can't figure out that this will never be NULL
        * if n_listing is at least 1 */
       tor_assert(current_rsa_id);
@@ -1640,6 +1664,10 @@ networkstatus_compute_consensus(smartlist_t *votes,
         } else if (!strcmp(fl, "Unnamed")) {
           if (is_unnamed)
             smartlist_add(chosen_flags, (char*)fl);
+        } else if (!strcmp(fl, "NoEdConsensus") &&
+                   consensus_method >= MIN_METHOD_FOR_ED25519_ID_VOTING) {
+          if (ed_consensus <= total_authorities/2)
+            smartlist_add(chosen_flags, (char*)fl);
         } else {
           if (flag_counts[fl_sl_idx] > n_flag_voters[fl_sl_idx]/2) {
             smartlist_add(chosen_flags, (char*)fl);

diff --git a/src/or/or.h b/src/or/or.h
index b24b6a85e4c3b3a9d6eced893fc9b235a52e6df2..431927c7e739ef49ab4e9c239e125c853147ab3a 100644 (file)
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2380,7 +2380,10 @@ typedef struct vote_routerstatus_t {
   /** True iff the vote included an entry for ed25519 ID, or included
    * "id ed25519 none" to indicate that there was no ed25519 ID. */
   unsigned int has_ed25519_listing:1;
-  unsigned int ed25519_reflects_consensus:1; /** DOCDOC */
+  /** True if the Ed25519 listing here is the consensus-opinion for the
+   * Ed25519 listing; false if there was no consensus on Ed25519 key status,
+   * or if this VRS doesn't reflect it. */
+  unsigned int ed25519_reflects_consensus:1;
   uint32_t measured_bw_kb; /**< Measured bandwidth (capacity) of the router */
   /** The hash or hashes that the authority claims this microdesc has. */
   vote_microdesc_hash_t *microdesc;