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.
This commit is contained in:
Nick Mathewson 2016-02-22 11:43:59 -05:00
parent c20e34e189
commit 13a31e72db
3 changed files with 50 additions and 0 deletions

7
changes/bug18318_ed Normal file
View File

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

View File

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

View File

@ -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. */