Opened 3 months ago

Closed 2 months ago

#26485 closed defect (implemented)

June mystery: the microdesc consensus is getting 9 sigs, but the ns consensus is getting only 5

Reported by: arma Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-dirauth, fast-fix, 035-triaged-in-20180711 029-backport 032-backport 033-backport 034-backport
Cc: ln5, sebastian Actual Points:
Parent ID: Points:
Reviewer: catalyst Sponsor:

Description

Lately (in the past few days) the dir auths are not getting all 9 signatures on the same document. Frequently we're getting 5 that sign one and 4 that sign a different one.

Some more digging shows that it's the ns consensus that they're disagreeing about, and the microdesc consensus is actually doing fine:

Jun 24 18:00:00.337 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet moria1 dizum Faravahar). 3 (tor26 maatuska gabelmoo) of the authorities we know didn't sign it.
Jun 24 18:00:00.538 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet moria1 dizum Faravahar). 3 (tor26 maatuska gabelmoo) of the authorities we know didn't sign it.
Jun 24 18:00:00.643 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This microdesc one has 9 (dannenberg tor26 longclaw bastet maatuska moria1 dizum gabelmoo Faravahar).
Jun 24 18:00:00.871 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This microdesc one has 9 (dannenberg tor26 longclaw bastet maatuska moria1 dizum gabelmoo Faravahar).

I've hacked up moria1 to write out the consensus document that it generates, for each flavor, in #4363. The theory is that we need to figure out how the ns consensus that we generated differs from the one that the other dir auths generated.

Next step is for moria1 to be the minority voter and me to notice it in time, or for more people to start running the patch in #4363 and try to catch the bug in action for themselves.

Child Tickets

Change History (26)

comment:1 Changed 3 months ago by arma

Cc: sebastian added

moria1 has always been in the majority since I started tracking:

Jun 24 19:00:00.338 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 5 (dannenberg longclaw bastet moria1 Faravahar). 4 (tor26 maatuska dizum gabelmoo) of the authorities we know didn't sign it.
Jun 24 20:00:00.340 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 7 (dannenberg tor26 longclaw bastet moria1 dizum Faravahar). 2 (maatuska gabelmoo) of the authorities we know didn't sign it.
Jun 24 21:00:00.341 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet maatuska moria1 Faravahar). 3 (tor26 dizum gabelmoo) of the authorities we know didn't sign it.
Jun 24 22:00:00.343 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 7 (dannenberg longclaw bastet maatuska moria1 dizum Faravahar). 2 (tor26 gabelmoo) of the authorities we know didn't sign it.
Jun 24 23:00:00.344 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 7 (dannenberg tor26 longclaw bastet maatuska moria1 Faravahar). 2 (dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 00:00:00.346 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg tor26 longclaw bastet moria1 Faravahar). 3 (maatuska dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 01:00:00.348 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 5 (tor26 longclaw bastet moria1 Faravahar). 4 (dannenberg maatuska dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 02:00:00.350 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet maatuska moria1 Faravahar). 3 (tor26 dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 03:00:00.351 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 5 (tor26 longclaw bastet moria1 Faravahar). 4 (dannenberg maatuska dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 04:00:00.354 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet maatuska moria1 Faravahar). 3 (tor26 dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 05:00:00.355 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet maatuska moria1 Faravahar). 3 (tor26 dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 06:00:00.357 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg tor26 longclaw bastet moria1 Faravahar). 3 (maatuska dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 07:00:00.358 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet maatuska moria1 Faravahar). 3 (tor26 dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 08:00:00.359 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 5 (dannenberg longclaw bastet moria1 Faravahar). 4 (tor26 maatuska dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 09:00:00.362 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet maatuska moria1 Faravahar). 3 (tor26 dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 10:00:00.363 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet maatuska moria1 Faravahar). 3 (tor26 dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 11:00:00.363 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg longclaw bastet maatuska moria1 Faravahar). 3 (tor26 dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 12:00:00.364 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 6 (dannenberg tor26 longclaw bastet moria1 Faravahar). 3 (maatuska dizum gabelmoo) of the authorities we know didn't sign it.
Jun 25 13:00:00.583 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 7 (dannenberg tor26 longclaw bastet maatuska moria1 Faravahar). 2 (dizum gabelmoo) of the authorities we know didn't sign it.

Meanwhile the microdesc consensus has been rock solid at 9 signatures each time.

comment:2 Changed 3 months ago by arma

Sounds like getting gabelmoo or dizum to apply the #4363 patch, or maatuska or tor26 and then lots of patience, is the way forward here.

comment:3 Changed 3 months ago by arma

Got one!

Jun 25 14:00:00.367 [warn] A consensus needs 5 good signatures from recognized authorities for us to accept it. This ns one has 4 (longclaw bastet moria1 Faravahar). 5 (dannenberg tor26 maatuska dizum gabelmoo) of the authorities we know didn't sign it.

And...drum roll please... the diff, ignoring the missing signatures, is:

-client-versions 0.2.9.14,0.2.9.15,0.3.1.9,0.3.1.10,0.3.2.6-alpha,0.3.2.7-rc,0.3.2.8-rc,0.3.2.9,0.3.2.10,0.3.3.1-alpha,0.3.3.2-alpha,0.3.3.3-alpha,0.3.3.4-alpha,0.3.3.5-rc,0.3.3.6,0.3.3.7,0.3.3.8,0.3.4.1-alpha,0.3.4.2-alpha,0.3.4.3-alpha
+client-versions 0.2.9.14,0.2.9.15,0.3.1.9,0.3.1.10,0.3.2.6-alpha,0.3.2.7-rc,0.3.2.9,0.3.2.10,0.3.3.1-alpha,0.3.3.2-alpha,0.3.3.3-alpha,0.3.3.4-alpha,0.3.3.5-rc,0.3.3.6,0.3.3.7,0.3.3.8,0.3.4.1-alpha,0.3.4.2-alpha,0.3.4.3-alpha

comment:4 Changed 3 months ago by nickm

If that's the only difference, my guess would be that the authorities are running different versions of Tor, and that there is somehow a difference in how these versions generate the client-versions lines.

That said, I can see no reason in 0.2.9 or 0.3.5 why an authority would ever have different versions lines in the ns and md consensuses.

Maybe it would be helpful to know the votes that led to that particular situation above? And to say the exact Tor versions that the authorities were running at the time?

comment:5 Changed 3 months ago by arma

http://freehaven.net/~arma/bug26485-2018-06-25.tar.bz2
now includes

-rw-r--r-- arma/arma   2188185 2018-06-25 14:37 cached-consensus
-rw-r--r-- arma/arma   2009418 2018-06-25 14:37 cached-microdesc-consensus
-rw-r--r-- arma/arma   2005176 2018-06-25 14:37 my-consensus-microdesc
-rw-r--r-- arma/arma   2186018 2018-06-25 14:37 my-consensus-ns
-rw-r--r-- arma/arma  41782684 2018-06-25 14:36 v3-status-votes

v3-status-votes is the nine votes that went into making cached-consensus and cached-microdesc-consensus. Whereas my-consensus-* are the two files that moria1 wanted to sign.

comment:6 in reply to:  4 Changed 3 months ago by arma

Replying to nickm:

If that's the only difference, my guess would be that the authorities are running different versions of Tor, and that there is somehow a difference in how these versions generate the client-versions lines.

That said, I can see no reason in 0.2.9 or 0.3.5 why an authority would ever have different versions lines in the ns and md consensuses.

The three dir auths voting about versions were moria1 (running 0.3.5.0), tor26 (running 0.3.3.7), gabelmoo (running 0.3.3.7).

comment:7 Changed 3 months ago by nickm

Okay. And what are the versions for the authorities that disagree about the versions in the final consensus?

comment:8 in reply to:  7 Changed 3 months ago by arma

Replying to nickm:

Okay. And what are the versions for the authorities that disagree about the versions in the final consensus?

Faravahar is on 0.3.3.6, and everybody else besides moria1 is on 0.3.3.7.

(You can learn this by looking for the string Authority in the cached-consensus file.)

comment:9 Changed 3 months ago by arma

I moved moria1 back to 0.3.3.7-dev, and the problems remain.

comment:10 Changed 3 months ago by nickm

I have narrowed the bug down to some problem with version sorting and comparison. See my branch is_version_sort_reliable for a test that demonstrates the problem.

comment:11 Changed 3 months ago by nickm

The upshot of the test is that all 3 of these values are possible sorts of the same list:

0: 0.2.5.16 0.2.5.17 0.2.9.14 0.2.9.14 0.2.9.14 0.2.9.15 0.2.9.15 0.2.9.15 0.3.1.9 0.3.1.9 0.3.1.9 0.3.1.10 0.3.1.10 0.3.1.10 0.3.2.6-alpha 0.3.2.6-alpha 0.3.2.7-rc 0.3.2.7-rc 0.3.2.8-rc 0.3.2.7-rc 0.3.3.1-alpha 0.3.2.6-alpha 0.3.2.9 0.3.2.8-rc 0.3.2.8-rc 0.3.2.9 0.3.2.9 0.3.2.10 0.3.2.10 0.3.2.10 0.3.3.1-alpha 0.3.3.1-alpha 0.3.3.2-alpha 0.3.3.2-alpha 0.3.3.2-alpha 0.3.3.3-alpha 0.3.3.3-alpha 0.3.3.3-alpha 0.3.3.4-alpha 0.3.3.4-alpha 0.3.3.4-alpha 0.3.3.5-rc 0.3.3.5-rc 0.3.3.5-rc 0.3.3.6 0.3.3.6 0.3.3.6 0.3.3.7 0.3.3.7 0.3.3.7 0.3.3.8 0.3.3.8 0.3.3.8 0.3.4.1-alpha 0.3.4.1-alpha 0.3.4.1-alpha 0.3.4.2-alpha 0.3.4.2-alpha 0.3.4.2-alpha 0.3.4.3-alpha 0.3.4.3-alpha 0.3.4.3-alpha
1: 0.2.5.16 0.2.5.17 0.2.9.14 0.2.9.14 0.2.9.14 0.2.9.15 0.2.9.15 0.2.9.15 0.3.1.9 0.3.1.9 0.3.1.9 0.3.1.10 0.3.1.10 0.3.1.10 0.3.2.6-alpha 0.3.2.6-alpha 0.3.2.7-rc 0.3.2.7-rc 0.3.2.8-rc 0.3.2.8-rc 0.3.2.8-rc 0.3.2.7-rc 0.3.3.1-alpha 0.3.2.6-alpha 0.3.2.9 0.3.2.9 0.3.2.9 0.3.2.10 0.3.2.10 0.3.2.10 0.3.3.1-alpha 0.3.3.1-alpha 0.3.3.2-alpha 0.3.3.2-alpha 0.3.3.2-alpha 0.3.3.3-alpha 0.3.3.3-alpha 0.3.3.3-alpha 0.3.3.4-alpha 0.3.3.4-alpha 0.3.3.4-alpha 0.3.3.5-rc 0.3.3.5-rc 0.3.3.5-rc 0.3.3.6 0.3.3.6 0.3.3.6 0.3.3.7 0.3.3.7 0.3.3.7 0.3.3.8 0.3.3.8 0.3.3.8 0.3.4.1-alpha 0.3.4.1-alpha 0.3.4.1-alpha 0.3.4.2-alpha 0.3.4.2-alpha 0.3.4.2-alpha 0.3.4.3-alpha 0.3.4.3-alpha 0.3.4.3-alpha
2: 0.2.5.16 0.2.5.17 0.2.9.14 0.2.9.14 0.2.9.14 0.2.9.15 0.2.9.15 0.2.9.15 0.3.1.9 0.3.1.9 0.3.1.9 0.3.1.10 0.3.1.10 0.3.1.10 0.3.2.6-alpha 0.3.2.6-alpha 0.3.2.7-rc 0.3.2.7-rc 0.3.2.8-rc 0.3.2.8-rc 0.3.2.7-rc 0.3.3.1-alpha 0.3.2.6-alpha 0.3.2.9 0.3.2.8-rc 0.3.2.9 0.3.2.9 0.3.2.10 0.3.2.10 0.3.2.10 0.3.3.1-alpha 0.3.3.1-alpha 0.3.3.2-alpha 0.3.3.2-alpha 0.3.3.2-alpha 0.3.3.3-alpha 0.3.3.3-alpha 0.3.3.3-alpha 0.3.3.4-alpha 0.3.3.4-alpha 0.3.3.4-alpha 0.3.3.5-rc 0.3.3.5-rc 0.3.3.5-rc 0.3.3.6 0.3.3.6 0.3.3.6 0.3.3.7 0.3.3.7 0.3.3.7 0.3.3.8 0.3.3.8 0.3.3.8 0.3.4.1-alpha 0.3.4.1-alpha 0.3.4.1-alpha 0.3.4.2-alpha 0.3.4.2-alpha 0.3.4.2-alpha 0.3.4.3-alpha 0.3.4.3-alpha 0.3.4.3-alpha

comment:12 Changed 3 months ago by nickm

Look at the client-versions lines in the votes!

The one from moria has spaces in it!!!!1!

comment:13 Changed 3 months ago by cypherpunks

!!!!1!

Sign of jerk. CoC, p0.

comment:14 Changed 3 months ago by dgoulet

Component: Core Tor/DirAuthCore Tor/Tor
Keywords: tor-dirauth added
Status: newneeds_review

This seems to affect "tor" and has a branch from nickm so going in Core Tor/Tor component.

comment:15 Changed 3 months ago by dgoulet

Reviewer: nickm

comment:16 Changed 3 months ago by dgoulet

Milestone: Tor: 0.3.5.x-final

comment:17 Changed 3 months ago by nickm

Owner: set to nickm
Reviewer: nickm
Status: needs_reviewaccepted

This actually didn't belong in needs_review -- the branch I made above is diagnostic, but not actually a fix. I'll take on the fix, though, unless somebody else would like to do it.

comment:18 Changed 3 months ago by nickm

Keywords: fast-fix added

comment:19 Changed 3 months ago by nickm

Keywords: 035-triaged-in-20180711 added

comment:20 Changed 3 months ago by nickm

Keywords: 029-backport 032-backport 033-backport 034-backport added
Status: acceptedneeds_review

I've done a minimal version of this in bug26485_029, PR at https://github.com/torproject/tor/pull/228

All this does is:

  • warn the voters if any authority has voted for a version with a space in it
  • warn the authority operator if they configure an invalid-looking version list
  • when generating a vote, to split any versions that seem to have spaces in them

I considered other changes as well, but I want to avoid changes that would affect the output of the consensus algorithm -- especially in a branch we're thinking of backporting to 0.2.9.

comment:21 Changed 2 months ago by dgoulet

Reviewer: catalyst

comment:22 Changed 2 months ago by catalyst

Thanks! I've left an inline comment on github about a minor comment accuracy thing, and asked about tests.

comment:23 Changed 2 months ago by nickm

Status: needs_reviewneeds_revision

You're correct on both counts -- I'll revise.

comment:24 Changed 2 months ago by nickm

Status: needs_revisionneeds_review

Okay, revised, and added tests. Better now?

comment:25 in reply to:  24 Changed 2 months ago by catalyst

Status: needs_reviewmerge_ready

Replying to nickm:

Okay, revised, and added tests. Better now?

Looks good; thanks!

comment:26 Changed 2 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

squashed and merged to 0.2.9 and forward!

Note: See TracTickets for help on using tickets.