Opened 6 years ago

Closed 5 years ago

#10163 closed enhancement (implemented)

Raise minimum consensus method to 13

Reported by: nickm Owned by:
Priority: High Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-auth prop215 026-triaged-1
Cc: tom@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Very old directory consensus methods simply won't generate results that work on the Tor network. There's a security flaw in consensus methods 7 through 11. According to proposal 215, it would be fine to simply require method 13 as a minimum for all authorities, since it's supported by 0.2.3 and later.

We should do this.

Child Tickets

Change History (11)

comment:1 Changed 6 years ago by tom

Cc: tom@… added

comment:2 Changed 5 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

comment:3 Changed 5 years ago by andrea

Keywords: 026-triaged-1 added

This is easy and should be done in 0.2.6.

comment:4 Changed 5 years ago by nickm

Status: newneeds_review

See branch "bug10163" in my public repository.

 5 files changed, 56 insertions(+), 390 deletions(-)

Isn't it nice when old code goes away?

Somebody should run this under chutney before we merge it. We also need a torspec patch.

comment:5 Changed 5 years ago by nickm

Branch 'bug10163_spec' in my public torspec repository has requisite dir-spec changes.

comment:6 Changed 5 years ago by sysrqb

Minor aside.

src/or/dirvote.c: In function ‘networkstatus_compute_consensus’:
src/or/dirvote.c:1272:9: warning: variable ‘chosen_named_idx’ set but not used [-Wunused-but-set-variable]
     int chosen_named_idx;
         ^

As for chutney, the branch looks ok.

5 dirauths, all running bug10163. I'll test a mixed network next.

network-status-version 3
vote-status vote
consensus-methods 13 14 15 16 17 18
published 2014-08-25 05:04:21
valid-after 2014-08-25 05:05:00
fresh-until 2014-08-25 05:10:00
valid-until 2014-08-25 05:20:00
voting-delay 20 20
network-status-version 3
vote-status consensus
consensus-method 18
valid-after 2014-08-25 05:05:00
fresh-until 2014-08-25 05:10:00
valid-until 2014-08-25 05:20:00
voting-delay 20 20

comment:7 Changed 5 years ago by nickm

Fixed the warning; thanks! I'll wait to hear about a mixed network, to make sure nothing explodes.

comment:8 Changed 5 years ago by sysrqb

7 authorities, 5 running branch10163, 3 running 0.2.4.23

0.2.4.23

network-status-version 3
vote-status vote
consensus-methods 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
published 2014-08-25 23:39:21
valid-after 2014-08-25 23:40:00
fresh-until 2014-08-25 23:45:00
valid-until 2014-08-25 23:55:00
voting-delay 20 20

branch bug1013

network-status-version 3
vote-status vote
consensus-methods 13 14 15 16 17 18
published 2014-08-25 23:39:21
valid-after 2014-08-25 23:40:00
fresh-until 2014-08-25 23:45:00
valid-until 2014-08-25 23:55:00
voting-delay 20 20

As expected, the 5 authorities running bug10163 outvoted the 2 running 0.2.4.23 for the consensus method:

Aug 25 23:45:01.000 [info] A consensus needs 4 good signatures from
recognized authorities for us to accept it. This one has 5 (test000a 
test002a test004a test001a test006a). 2 (test003a test005a) of the 
authorities we know didn't sign it.

consensus

network-status-version 3
vote-status consensus
consensus-method 18
valid-after 2014-08-25 23:40:00
fresh-until 2014-08-25 23:45:00
valid-until 2014-08-25 23:55:00
voting-delay 20 20

comment:9 in reply to:  8 Changed 5 years ago by sysrqb

Replying to sysrqb:

7 authorities, 5 running branch10163, 3 running 0.2.4.23

New math. 2 running 0.2.4.23

comment:10 Changed 5 years ago by sysrqb

7 authorities, 2 running branch10163, 5 running 0.2.4.23

0.2.4.23

network-status-version 3
vote-status vote
consensus-methods 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
published 2014-08-26 00:29:21
valid-after 2014-08-26 00:30:00
fresh-until 2014-08-26 00:35:00
valid-until 2014-08-26 00:45:00
voting-delay 20 20

branch bug1013

network-status-version 3
vote-status vote
consensus-methods 13 14 15 16 17 18
published 2014-08-26 00:29:21
valid-after 2014-08-26 00:30:00
fresh-until 2014-08-26 00:35:00
valid-until 2014-08-26 00:45:00
voting-delay 20 20

consensus

network-status-version 3
vote-status consensus
consensus-method 17
valid-after 2014-08-26 00:30:00
fresh-until 2014-08-26 00:35:00
valid-until 2014-08-26 00:45:00
voting-delay 20 20

Both versions, by authorities, relays, and clients:

Aug 26 00:33:46.000 [info] A consensus needs 4 good signatures from 
recognized authorities for us to accept it. This one has 7 (test000a 
test003a test002a test004a test005a test001a test006a).

comment:11 Changed 5 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed
Type: defectenhancement

Thanks for testing this! I've merged it to master.

Note: See TracTickets for help on using tickets.