Never vote for an ed key twice.

When generating a vote, and we have two routerinfos with the same ed
key, omit the one published earlier.

This was supposed to have been solved by key pinning, but when I
made key pinning optional, I didn't realize that this would jump up
and bite us.  It is part of bug 18318, and the root cause of 17668.

diff --git a/changes/bug18318_ed b/changes/bug18318_ed
new file mode 100644 (file)
index 0000000..af39234
--- /dev/null
+++ b/changes/bug18318_ed
@@ -0,0 +1,7 @@
+  o Major bugfixes:
+    - When generating a vote with keypinning disabled, never include two
+      entries for the same ed25519 identity.  This bug was causing
+      authorities to generate votes that they could not parse when a router
+      violated key pinning by changing its RSA identity but keeping its
+      Ed25519 identity.  Fixes bug 17668; fixes part of bug 18318.  Bugfix on
+      0.2.7.2-alpha.

diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 8d9f1665564f5dfdb9019173b033e229aae97d12..016514f10f118a3648c0abc79ba87676a4942bfa 100644 (file)
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -2126,6 +2126,44 @@ get_possible_sybil_list(const smartlist_t *routers)
   return omit_as_sybil;
 }
 
+/** If there are entries in <b>routers</b> with exactly the same ed25519 keys,
+ * remove the older one.  May alter the order of the list. */
+static void
+routers_make_ed_keys_unique(smartlist_t *routers)
+{
+  routerinfo_t *ri2;
+  digest256map_t *by_ed_key = digest256map_new();
+
+  SMARTLIST_FOREACH_BEGIN(routers, routerinfo_t *, ri) {
+    ri->omit_from_vote = 0;
+    if (ri->signing_key_cert == NULL)
+      continue; /* No ed key */
+    const uint8_t *pk = ri->signing_key_cert->signing_key.pubkey;
+    if ((ri2 = digest256map_get(by_ed_key, pk))) {
+      /* Duplicate; must omit one.  Set the omit_from_vote flag in whichever
+       * one has the earlier published_on. */
+      if (ri2->cache_info.published_on < ri->cache_info.published_on) {
+        digest256map_set(by_ed_key, pk, ri);
+        ri2->omit_from_vote = 1;
+      } else {
+        ri->omit_from_vote = 1;
+      }
+    } else {
+      /* Add to map */
+      digest256map_set(by_ed_key, pk, ri);
+    }
+  } SMARTLIST_FOREACH_END(ri);
+
+  digest256map_free(by_ed_key, NULL);
+
+  /* Now remove every router where the omit_from_vote flag got set. */
+  SMARTLIST_FOREACH_BEGIN(routers, const routerinfo_t *, ri) {
+    if (ri->omit_from_vote) {
+      SMARTLIST_DEL_CURRENT(routers, ri);
+    }
+  } SMARTLIST_FOREACH_END(ri);
+}
+
 /** Extract status information from <b>ri</b> and from other authority
  * functions and store it in <b>rs</b>>.
  *
@@ -2815,6 +2853,7 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key,
 
   routers = smartlist_new();
   smartlist_add_all(routers, rl->routers);
+  routers_make_ed_keys_unique(routers);
   routers_sort_by_identity(routers);
   omit_as_sybil = get_possible_sybil_list(routers);
 

diff --git a/src/or/or.h b/src/or/or.h
index 4496cbcec3319a79b60bf09cbc094aa28c37ca38..b6d6ec074f4c6b6372461670bd276389c0ec75cf 100644 (file)
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2081,6 +2081,10 @@ typedef struct {
    * tests for it. */
   unsigned int needs_retest_if_added:1;
 
+  /** Used during voting to indicate that we should not include an entry for
+   * this routerinfo. Used only during voting. */
+  unsigned int omit_from_vote:1;
+
 /** Tor can use this router for general positions in circuits; we got it
  * from a directory server as usual, or we're an authority and a server
  * uploaded it. */