Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/tls: deprecate PreferServerCipherSuites and CipherSuites ordering #45430

Closed
FiloSottile opened this issue Apr 7, 2021 · 5 comments
Closed
Labels
Milestone

Comments

@FiloSottile
Copy link
Contributor

Background

Cipher suite ordering and selection is complex, nuanced, and security relevant. Mechanically, the client sends a list of cipher suites in preference order, and the server picks one arbitrarily. In Go, Config.CipherSuites is in preference order, and PreferServerCipherSuites (default false) dictates whether on the server-side we prioritize the client's or the server's preferences.

There are two reasons to manipulate cipher suite setting: one is for filtering, to add off-by-default suites or turn off on-by-default ones; the other is for ordering, which has security and sometimes performance reasons. Filtering sets the minimum security bar: it's a statement of the kind "I am not willing to make a connection with less than these security properties". Ordering is particularly nuanced and has security semantics, because if implemented correctly it guarantees that peers will benefit from the highest security they support, thanks to downgrade protection. Properly implemented ordering influences filtering, because i.e. 3DES might be acceptable as a last resort only if it can be guaranteed that peers that support better options won't end up using 3DES.

The ordering logic is already not very linear: if we get to choose, we'll prioritize AES only if both sides (seem to) support hardware AES instructions, because software AES is slow and unsafe. Due to Sweet32 attacks (#41476) we would also like to put 3DES at the end of the list, so it only affects connections that would have broken otherwise. Expecting applications to implement all this logic and stay up to date on it is unreasonable.

The other angle is an ecosystem angle. Historically, clients were the ones that would know best, so a default behavior of deferring to the client's preference had the ecosystem benefit that clients could keep less secure cipher suites enabled as a fallback, counting on the server to pick their higher preferences. Servers were expected to have fossilized cipher suite preference lists, not informed by the latest attacks. We can do much better in Go if we are always in charge of ordering: we can fix the logic in security releases based on the latest attack. This also serves better legacy clients like old devices that can't get security updates anymore.

This proposal would finally solve the problem of how to expose our order logic so that applications can replicate it with a different filtering logic: we don't need to, as applications can now configure filtering without affecting ordering.

Proposal

I propose that we deprecate Config.PreferServerCipherSuites, effectively considering it always true, and remove the ordering semantics from Config.CipherSuites, considering it only a filtering list specifying which cipher suites to enable.

Ordering of client-advertised ciphersuites and server ciphersuite selection will be automatically handled by crypto/tls with logic evolving based on the changes in the ecosystem.

@FiloSottile FiloSottile added the Proposal-Crypto Proposal related to crypto packages or other security issues label Apr 7, 2021
@gopherbot gopherbot added this to the Proposal milestone Apr 7, 2021
@rsc
Copy link
Contributor

rsc commented Apr 7, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 14, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 21, 2021

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: crypto/tls: deprecate PreferServerCipherSuites and CipherSuites ordering crypto/tls: deprecate PreferServerCipherSuites and CipherSuites ordering Apr 21, 2021
@rsc rsc modified the milestones: Proposal, Backlog Apr 21, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/314609 mentions this issue: crypto/tls: make cipher suite preference ordering automatic

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/314611 mentions this issue: http2: remove TLSConfig.CipherSuites order check

gopherbot pushed a commit to golang/net that referenced this issue May 8, 2021
The order of CipherSuites is ignored as of Go 1.17, and the effective
order will always put good ciphers first, as tested by TestCipherSuites
in crypto/tls.

Updates golang/go#45430

Change-Id: I54424f68bffe805f504b57eb025601c08c6357f6
Reviewed-on: https://go-review.googlesource.com/c/net/+/314611
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
dteh pushed a commit to dteh/fhttp that referenced this issue Jun 22, 2022
The order of CipherSuites is ignored as of Go 1.17, and the effective
order will always put good ciphers first, as tested by TestCipherSuites
in crypto/tls.

Updates golang/go#45430

Change-Id: I54424f68bffe805f504b57eb025601c08c6357f6
Reviewed-on: https://go-review.googlesource.com/c/net/+/314611
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
@rsc rsc moved this to Accepted in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@golang golang locked and limited conversation to collaborators Sep 5, 2022
@rsc rsc removed this from Proposals Oct 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants