Opened 9 years ago

Last modified 5 months ago

#1299 new defect (None)

Tor should verify signatures before parsing

Reported by: mikeperry Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: 0.2.1.24
Severity: Normal Keywords: tor-security, tor-crypto, tor-client, parse, safety, 035-removed-20180711
Cc: mikeperry, ioerror, Sebastian, nickm, Samdney Actual Points:
Parent ID: Points: 2
Reviewer: Sponsor:

Description (last modified by nickm)

Right now Tor parses both consensus documents and router descriptors before verifying their
signature. This exposes us to all sorts of potential MITM tampering and code execution bugs, of which
we have recently had several. Right now, an adversary who finds a parsing exploit needs only to
sign up as a directory mirror, or MITM 0.2.0.x clients that are not using tunnelled directory connections.

Such an adversary can custom-craft payloads based on the fingerprint of the OS of the client that
connects to them, and can also target specific clients for precision attacks.

If we verify signatures before parsing, the adversary loses their ability to target specific clients
by OS or by IP, and can at best publish a malicious router descriptor signed by them to everyone.
This leaves us with a clear audit trail of where the exploit came from, and a record of all such
attempts in the descriptor archives. This would be a considerably better position to be in than
we are now.

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

TicketStatusOwnerSummaryComponent
#26037closedSamdneyDirAuths should check vote signatures before parsingCore Tor/Tor

Change History (9)

comment:1 Changed 8 years ago by nickm

One problem here is that, for certain documents, you need to parse them in order to learn what key they were supposed to be signed with. We could change our procedure from the current "parse, extract key from parsed document, check signature" to "locate key in raw document, parse key only, check signature, parse document"... but historically we've had headaches stemming from the "locate XYZ in raw document" step not following exactly the same rules as the "parse document" step.

Some of the code I did for bug #1270 tries to make parsing in general more verifiably and obviously correct; that could make us feel better about this if we merged it, but it wouldn't resolve the issue.

comment:2 Changed 8 years ago by nickm

Description: modified (diff)
Milestone: Tor: unspecified
Priority: majornormal

Moving to "unspecified" milestone.

comment:3 Changed 6 years ago by nickm

Keywords: tor-client added

comment:4 Changed 6 years ago by nickm

Component: Tor ClientTor

comment:5 Changed 22 months ago by nickm

Cc: mikeperry,ioerror,Sebastian,nickmmikeperry, ioerror, Sebastian, nickm
Priority: MediumHigh
Severity: Normal

comment:6 Changed 19 months ago by nickm

Keywords: parse safety added
Priority: HighMedium

comment:7 Changed 5 months ago by teor

Keywords: tor-security tor-crypto added
Milestone: Tor: unspecifiedTor: 0.3.5.x-final
Points: 2

Moved to 0.3.5 to replace #26037.

comment:8 Changed 5 months ago by Samdney

Cc: Samdney added

Hi, added me as observer here after we closed the ticket #26037 as duplicat which was assigned to me. Currently, I'm experiementing with coding some possible solution ways.

comment:9 Changed 5 months ago by nickm

Keywords: 035-removed-20180711 added
Milestone: Tor: 0.3.5.x-finalTor: unspecified

These tickets are being triaged out of 0.3.5. The ones marked "035-roadmap-proposed" may return.

Note: See TracTickets for help on using tickets.