Opened 3 years ago

Closed 3 years ago

#19998 closed defect (implemented)

Stop allowing 3DES in TLS ciphersuites

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-spec, review-group-8
Cc: Actual Points: 0
Parent ID: Points: .2
Reviewer: Sponsor:

Description

Thanks to the SWEET32 attack, 3des is getting lots of attention.

Right now, Tor is willing in principle to negotiate a 3DES TLS connection.

But the good news is (I think) that two non-obsolete Tor instances will never actually do so. Here is my reasoning:

  • Our source code has always preferred AES to 3DES. So the only way to get 3DES would be if one party didn't support AES.
  • OpenSSL began supporting AES in version 0.9.7.
  • Tor has required OpenSSL 0.9.7 or later since 7da93b80ca7a6ba , which was in 0.2.0.10-alpha.

So this cipher shouldn't get negotiated, unless you're doing something very very weird.

I suggest that the best fix is to stop servers from ever choosing it.

I suggest that as an additional fix, clients should reject a connection to any server that _does_ choose it.

Child Tickets

Change History (8)

comment:1 Changed 3 years ago by yawning

Keywords: tor-spec added

I agree with the suggested fixes.

This will require a tor-spec update as well since 3DES is listed as mandatory in a few locations (primarily in relation to outdated link handshakes), and although it is unlikely that someone will implement a tor that only supports 3DES, the spec should reflect the code.

While we are revisiting the allowed cipher suites, should we proscribe the RC4 ones? It's even less likely that any of those will be negotiated, and they're just as flawed as 3DES.... (Probably a separate ticket?)

comment:2 Changed 3 years ago by nickm

AFAICT the RC4 suites are no longer advertised or accepted by any supported Tor. The only reason our code still mentions "RC4" is to detect versions of the cipher list that are lies.

So yeah, adding to our spec "no RC4 either" would be a fine thing!

comment:3 Changed 3 years ago by nickm

Actual Points: 0
Status: newneeds_review

See branch bug19998 in my torspec repository and branch ticket19998 in my tor repository.

comment:4 Changed 3 years ago by nickm

Owner: set to nickm
Status: needs_reviewaccepted

comment:5 Changed 3 years ago by nickm

Status: acceptedneeds_review

comment:6 Changed 3 years ago by nickm

Keywords: review-group-8 added

comment:7 Changed 3 years ago by asn

Status: needs_reviewmerge_ready

Both torspec and tor branches look good to me.

comment:8 Changed 3 years ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged. Thank you for the review!

Note: See TracTickets for help on using tickets.