Opened 23 months ago

Closed 23 months ago

Last modified 19 months ago

#6007 closed defect (fixed)

Be more strict about rejecting pre-handshake data in 0.2.2

Reported by: nickm Owned by:
Priority: major Milestone: Tor: 0.2.2.x-final
Component: Tor Version:
Keywords: tor-relay Cc:
Actual Points: Parent ID:
Points:

Description

In 0.2.2, while in state OR_CONN_STATE_TLS_SERVER_RENEGOTIATING, if we get any data we just let it sit in the inbuf. We should instead close the connection if the data arrives before the handshake, since this is either a failed attempt at a v3 handshake, or an attempt to bloat our buffer and make us waste ram.

On 0.2.3, this fix isn't necessary, since any data arriving will get treated as an attempt at a v3 handshake.

Found by pseudnymous user on IRC.

Child Tickets

Change History (12)

comment:1 Changed 23 months ago by cypherpunks

See #5934 for example of exploit. It triggers waiting of v2 handshake by (standard) cipher list.

comment:2 Changed 23 months ago by nickm

  • Status changed from new to needs_review

Branch "bug6007" should fix this.

comment:3 Changed 23 months ago by nickm

The branch mentioned above imposes an 8k limit on the inbuf size; we could drop it to something much smaller, or even 0, if we trust that any data received in such a state really and truly is erroneous.

comment:4 Changed 23 months ago by nickm

(It would be great if somebody could test this on an active 0.2.2.x relay, with and without the MAX_OR_INBUF_WHEN_NONOPEN value set to 0.)

comment:5 Changed 23 months ago by nickm

For the alternative version, see branch "bug6007_strict". *That* one sure needs some testing!

comment:6 Changed 23 months ago by arma

From irc discussion, it sounds like 6007_strict is the one to pick, and it should go into 0.2.2.37.

With, as you say, some testing.

comment:7 Changed 23 months ago by arma

boboper points out that we don't actually want to make it a warn. It is common for browsers to send extra stuff. Who knows what else does. Sounds more like a protocol_warn.

comment:8 Changed 23 months ago by nickm

Updated bug6007_strict to log at protocol_warn.

comment:9 Changed 23 months ago by nickm

I can confirm that this doesn't stop a mixed network of 0.2.2 and 0.2.3 nodes from bootstrapping with chutney.

We should run it on a bridge and/or a relay and make sure our stuff can connect to it, then call it "good enough to merge"

comment:10 Changed 23 months ago by nickm

  • Resolution set to fixed
  • Status changed from needs_review to closed

Squashed into "bug6007_strict_squashed" and moved it into 0.2.2.x and forward.

comment:11 Changed 19 months ago by nickm

  • Keywords tor-relay added

comment:12 Changed 19 months ago by nickm

  • Component changed from Tor Relay to Tor
Note: See TracTickets for help on using tickets.