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.

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.

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;

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

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;