dirauth: Sybil flag found its way into the consensus
The Sybil
flag was added very recently and should only be present in votes in order to simply signal that a relay was flagged a sybil by an authority. (#40255 (closed))
However, at 19:00 UTC today (May 5th, 2021), a relay ended up with this flag in the consensus:
r cadory gMmJS6puG3OkMhV1tMhMMRUg+pE S/FEdoR+O5xERKON4HX+OMupfTI 2021-05-05 03:29:40 94.114.193.136 19004 4447
s Fast Running Stable Sybil V2Dir Valid
v Tor 0.3.5.14
pr Cons=1-2 Desc=1-2 DirCache=1-2 HSDir=1-2 HSIntro=3-4 HSRend=1-2 Link=1-5 LinkAuth=1,3 Microdesc=1-2 Relay=1-2
w Bandwidth=634
p reject 1-65535
For this consensus, only maatuska
voted for Sybil
out of all authorities, it was the only one with Sybil
in the known-flags
list and a bunch of relays were voted with Sybil
.
known-flags Authority Exit Fast Guard HSDir Running Stable StaleDesc Sybil V2Dir Valid
And here is its vote for that relay:
r cadory gMmJS6puG3OkMhV1tMhMMRUg+pE S/FEdoR+O5xERKON4HX+OMupfTI 2021-05-05 03:29:40 94.114.193.136 19004 4447
s Sybil
v Tor 0.3.5.14
pr Cons=1-2 Desc=1-2 DirCache=1-2 HSDir=1-2 HSIntro=3-4 HSRend=1-2 Link=1-5 LinkAuth=1,3 Microdesc=1-2 Relay=1-2
w Bandwidth=1048 Measured=360
p reject 1-65535
id ed25519 LnXUcIyIrqgOOIcvfhA2kH/xmvBtgrwgJf3fXf95gIU
stats wfu=0.000000 tk=4539335 mtbf=0
m 28,29 sha256=Wy+G6ULbeul7dxE43/bAKJ1V89WHdUJG+q9HsIUUVxc
m 30,31 sha256=cWhmH6p8B1Z63joisGLrqsph7VRT7SQAKD3MqwrnCWA
So somehow, only 1 dirauth was enough to put that flag in the consensus for this relay.
The function that computes the consensus out of all votes is: networkstatus_compute_consensus()
. It crunches all votes and creates an array of all flags per relay and an array of which voters care about which flags (n_flag_voters
).
int *n_flag_voters; /* n_flag_voters[f] is the number of votes that care
* about flags[f]. */
Thus, I think that this array, for the Sybil
flag, will be set to 1 because only maatuska
cares about it.
And here comes our bug, later in the function, the flags appearing in the votes are put in the chosen_flags
list and then put in the consensus:
if (flag_counts[fl_sl_idx] > n_flag_voters[fl_sl_idx]/2) {
smartlist_add(chosen_flags, (char*)fl);
So I think what is happening here is that flag_counts[Sybil] == 1
and n_flag_voters[Sybil] == 1
and thus 1 > (1/2)
resulting in the flag being put in the consensus.
It appears there is a logical flaw here where we are assuming that known-flags
are the same for each dirauth but in reality they are not with this very example.
The known-flags
are constructed with:
for all votes:
SMARTLIST_FOREACH(v->known_flags, const char *, cp,
smartlist_add_strdup(flags, cp));
smartlist_sort_strings(flags);
smartlist_uniq_strings(flags);
And so Sybil
gets in there but is only known by 1 dirauth and thus it ends up in the consensus. I'm thinking that we either should have trimmed Sybil
from that list considering that not a majority of dirauth knows about it or we need to change somehow the logic on how a flag is put in the consensus.