Opened 8 months ago

Closed 7 months ago

#24978 closed defect (implemented)

Tor doesn't work when built with (unreleased) OpenSSL 1.1.1 built with enable-tls1_3

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 029-backport, 031-backport, 032-backport, openssl, review-group-31
Cc: Actual Points:
Parent ID: Points:
Reviewer: isis Sponsor:

Description

From https://www.openssl.org/blog/blog/2017/05/04/tlsv1.3/ :

If you explicitly configure your ciphersuites then care should be taken to ensure that you are not inadvertently excluding all TLSv1.3 compatible ciphersuites. If a client has TLSv1.3 enabled but no TLSv1.3 ciphersuites configured then it will immediately fail (even if the server does not support TLSv1.3) with an error message

That's the situation we're in now. When OpenSSL 1.1.1 releases in April, current Tor versions just won't work with it at all, since they have neither disabled TLS1.3 nor enabled any TLS1.3 ciphers.

We have two options for fixing this: I'll implement both and we can see what we like.

Child Tickets

Change History (7)

comment:1 Changed 8 months ago by nickm

Status: assignedneeds_review

See bug24978_029_enable and bug24978_029_disable for our options. We should merge only one IMO.

I'd argue against a backport to 0.2.5, since 0.2.5 is EOL in May, and since it won't build with OpenSSL 1.1.1 at all.

comment:2 Changed 8 months ago by nickm

(If you go to test these out with openssl git master, expect a bunch of warnings. I have a PR for that in openssl: https://github.com/openssl/openssl/pull/5150 .)

comment:3 Changed 8 months ago by nickm

Keywords: review-group-31 added

comment:4 Changed 7 months ago by isis

Reviewer: isis

comment:5 Changed 7 months ago by isis

Status: needs_reviewmerge_ready

IMHO we should merge bug24978_029_enable, because opportunistically speaking the cleaner, better-designed TLS protocol with the nicer ciphers would be preferable to simply disabling it (assuming everything about our current link protocol will still function in a TLS 1.3 context).

One note:

  • "TLS13-CHACHA20-POLY1305-SHA256:TLS13-AES-128-GCM-SHA256:TLS13-AES-256-GCM-SHA384:" is the default ciphersuite list/ordering for OpenSSL 1.1.1. Ours is going to now be "TLS13-AES-256-GCM-SHA384:TLS13-CHACHA20-POLY1305-SHA256:TLS13-AES-128-GCM-SHA256:TLS13-AES-128-CCM-SHA256:[…]" (plus some other stuff). I don't know if or how much we should care about what will probably eventually result in a preference reordering. This means older link protocol tors will be distinguishable from newer ones, but they'll look different anyway, so merge at your call.

comment:6 Changed 7 months ago by nickm

Thanks for the review! I've merged this to 0.2.9 and forward. I'm happy to revise the client and server cipher lists however much is requested, but I would need a suggested list of what to change to. That might be something to think about on an 0.3.4 timeframe, and backport as appropriate.

comment:7 Changed 7 months ago by nickm

Resolution: implemented
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.