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

Jump to bottom

crypto/tls: Disable CBC Ciphers by default #13385

Open
pquerna opened this issue Nov 24, 2015 · 9 comments
Open

crypto/tls: Disable CBC Ciphers by default #13385

pquerna opened this issue Nov 24, 2015 · 9 comments
Labels
Security
Milestone
Unplanned

Comments

@pquerna
Copy link

pquerna commented Nov 24, 2015

Per @agl on Twitter, the only way to be safe with Go and TLS if you are worried about Lucky13-style attacks is to disable CBC mode ciphers:

https://twitter.com/agl__/status/669182140244824064

Currently none of the CBC ciphers are marked with suiteDefaultOff: https://github.com/golang/go/blob/master/src/crypto/tls/cipher_suites.go#L75-L95

TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
TLS_RSA_WITH_AES_128_CBC_SHA
TLS_RSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
TLS_RSA_WITH_3DES_EDE_CBC_SHA

It seems wrong for Go to have insecure defaults for a class of attacks that is becoming more well documented. If there is no willingness to implement countermeasures, shouldn't the vulnerable class of ciphers be disabled by default?
@pquerna pquerna changed the title Disable CBC Ciphers for TLS by default crypto/tls: Disable CBC Ciphers by default Nov 24, 2015
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Nov 24, 2015
@timcooijmans
Copy link
Contributor

Disabling CBC so will break compatibility with many operating systems and devices. In practice all public-facing Go-services would have to re-enable the CBC-ciphersuites. So the question is wether implementing countermeasures isn't the only viable solution.

@bradfitz
Copy link
Contributor

Perhaps it just needs to be easier to change the global, default ciphersuites without re-wiring up a *tls.Config, *http.Server, *http.Transport, etc.

@pquerna
Copy link
Author

pquerna commented Nov 24, 2015

@bradfitz such a method would be a good start; http2 is already has a blacklist of ciphers, making a way to more easily control that behavior in the various invocations of cipher configurations / global default would be a great start.

@DemiMarie
Copy link

If disabling CBC is not an option, Go needs to implement the appropriate countermeasures in its cryptography (or call out to a third-party library that does so).

Also AEAD ciphers should automatically be preferred when possible.

@timcooijmans
Copy link
Contributor

I agree, as long as CBC is needed to support all commonly used browsers and libraries, fixing CBC is the only solution, no matter how ugly and complex it may be. While disabling CBC ensures a safe-default. The fact that in practice any public service needs to re-enable it, still leads to unsafe situations that could lead to breaches and negative publicity for Go.

@gopherbot
Copy link

CL https://golang.org/cl/17532 mentions this issue.

bradfitz added a commit that referenced this issue Dec 14, 2015
@bradfitz
crypto/tls: document lack of Lucky13 hardening
91abab0 
Updates #13385

Change-Id: I9c2edf8c02adc388c48760b29e63dfa2966262d6
Reviewed-on: https://go-review.googlesource.com/17532
Reviewed-by: Tim Cooijmans <timcooijmans@gmail.com>
Reviewed-by: Adam Langley <agl@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/18130 mentions this issue.

@davecheney davecheney mentioned this issue Mar 13, 2016
Closed
gopherbot pushed a commit that referenced this issue Oct 4, 2016
@FiloSottile @rsc
crypto/tls: implement countermeasures against CBC padding oracles
f28cf83 
The aim is to make the decrypt() timing profile constant, irrespective of
the CBC padding length or correctness.  The old algorithm, on valid padding,
would only MAC bytes up to the padding length threshold, making CBC
ciphersuites vulnerable to plaintext recovery attacks as presented in the
"Lucky Thirteen" paper.

The new algorithm Write()s to the MAC all supposed payload, performs a
constant time Sum()---which required implementing a constant time Sum() in
crypto/sha1, see the "Lucky Microseconds" paper---and then Write()s the rest
of the data. This is performed whether the padding is good or not.

This should have no explicit secret-dependent timings, but it does NOT
attempt to normalize memory accesses to prevent cache timing leaks.

Updates #13385

Change-Id: I15d91dc3cc6eefc1d44f317f72ff8feb0a9888f7
Reviewed-on: https://go-review.googlesource.com/18130
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/33665 mentions this issue.

gopherbot pushed a commit that referenced this issue Nov 29, 2016
@bradfitz
crypto/tls: update CBC Lucky13 warning a bit
4822e76 
Some countermeasures were implemented in https://golang.org/cl/18130

Updates #13385

Change-Id: I723e1e3be0fa6d13767b65b145d90c89e92b2774
Reviewed-on: https://go-review.googlesource.com/33665
Reviewed-by: Adam Langley <agl@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/35290 mentions this issue.

gopherbot pushed a commit that referenced this issue Jan 17, 2017
@FiloSottile @bradfitz
crypto/tls: disable CBC cipher suites with SHA-256 by default
48d8edb 
As is, they were fully vulnerable to the Lucky13 attack. The SHA1
variants implement limited countermeasures (see f28cf83) but the
SHA256 ones are apparently used rarely enough (see 8741504) that
it's not worth the extra code.

Instead, disable them by default and update the warning.

Updates #13385
Updates #15487

Change-Id: I45b8b716001e2fa0811b17e25be76e2512e5abb2
Reviewed-on: https://go-review.googlesource.com/35290
Reviewed-by: Adam Langley <alangley@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@bdarnell bdarnell mentioned this issue Jul 24, 2017
Closed
@FiloSottile FiloSottile mentioned this issue Aug 26, 2017
Closed
@harshavardhana harshavardhana mentioned this issue Nov 28, 2017
Merged
9 tasks
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@bradfitz
crypto/tls: document lack of Lucky13 hardening
e2901e8 
Updates golang#13385

Change-Id: I9c2edf8c02adc388c48760b29e63dfa2966262d6
Reviewed-on: https://go-review.googlesource.com/17532
Reviewed-by: Tim Cooijmans <timcooijmans@gmail.com>
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile added a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@FiloSottile @rsc
crypto/tls: implement countermeasures against CBC padding oracles
4536ac7 
The aim is to make the decrypt() timing profile constant, irrespective of
the CBC padding length or correctness.  The old algorithm, on valid padding,
would only MAC bytes up to the padding length threshold, making CBC
ciphersuites vulnerable to plaintext recovery attacks as presented in the
"Lucky Thirteen" paper.

The new algorithm Write()s to the MAC all supposed payload, performs a
constant time Sum()---which required implementing a constant time Sum() in
crypto/sha1, see the "Lucky Microseconds" paper---and then Write()s the rest
of the data. This is performed whether the padding is good or not.

This should have no explicit secret-dependent timings, but it does NOT
attempt to normalize memory accesses to prevent cache timing leaks.

Updates golang#13385

Change-Id: I15d91dc3cc6eefc1d44f317f72ff8feb0a9888f7
Reviewed-on: https://go-review.googlesource.com/18130
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@bradfitz
crypto/tls: update CBC Lucky13 warning a bit
50eb6d7 
Some countermeasures were implemented in https://golang.org/cl/18130

Updates golang#13385

Change-Id: I723e1e3be0fa6d13767b65b145d90c89e92b2774
Reviewed-on: https://go-review.googlesource.com/33665
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile added a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@FiloSottile @bradfitz
crypto/tls: disable CBC cipher suites with SHA-256 by default
0fcebb3 
As is, they were fully vulnerable to the Lucky13 attack. The SHA1
variants implement limited countermeasures (see f28cf83) but the
SHA256 ones are apparently used rarely enough (see 8741504) that
it's not worth the extra code.

Instead, disable them by default and update the warning.

Updates golang#13385
Updates golang#15487

Change-Id: I45b8b716001e2fa0811b17e25be76e2512e5abb2
Reviewed-on: https://go-review.googlesource.com/35290
Reviewed-by: Adam Langley <alangley@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@bradfitz
crypto/tls: document lack of Lucky13 hardening
75bdcc3 
Updates golang#13385

Change-Id: I9c2edf8c02adc388c48760b29e63dfa2966262d6
Reviewed-on: https://go-review.googlesource.com/17532
Reviewed-by: Tim Cooijmans <timcooijmans@gmail.com>
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile added a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@FiloSottile @rsc
crypto/tls: implement countermeasures against CBC padding oracles
318ec0c 
The aim is to make the decrypt() timing profile constant, irrespective of
the CBC padding length or correctness.  The old algorithm, on valid padding,
would only MAC bytes up to the padding length threshold, making CBC
ciphersuites vulnerable to plaintext recovery attacks as presented in the
"Lucky Thirteen" paper.

The new algorithm Write()s to the MAC all supposed payload, performs a
constant time Sum()---which required implementing a constant time Sum() in
crypto/sha1, see the "Lucky Microseconds" paper---and then Write()s the rest
of the data. This is performed whether the padding is good or not.

This should have no explicit secret-dependent timings, but it does NOT
attempt to normalize memory accesses to prevent cache timing leaks.

Updates golang#13385

Change-Id: I15d91dc3cc6eefc1d44f317f72ff8feb0a9888f7
Reviewed-on: https://go-review.googlesource.com/18130
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@bradfitz
crypto/tls: update CBC Lucky13 warning a bit
c2df6cc 
Some countermeasures were implemented in https://golang.org/cl/18130

Updates golang#13385

Change-Id: I723e1e3be0fa6d13767b65b145d90c89e92b2774
Reviewed-on: https://go-review.googlesource.com/33665
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile added a commit to FiloSottile/go that referenced this issue Oct 12, 2018
@FiloSottile @bradfitz
crypto/tls: disable CBC cipher suites with SHA-256 by default
b5fd625 
As is, they were fully vulnerable to the Lucky13 attack. The SHA1
variants implement limited countermeasures (see f28cf83) but the
SHA256 ones are apparently used rarely enough (see 8741504) that
it's not worth the extra code.

Instead, disable them by default and update the warning.

Updates golang#13385
Updates golang#15487

Change-Id: I45b8b716001e2fa0811b17e25be76e2512e5abb2
Reviewed-on: https://go-review.googlesource.com/35290
Reviewed-by: Adam Langley <alangley@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
rchincha added a commit to rchincha/zot that referenced this issue Mar 1, 2022
@rchincha
tls: restrict tls cipher suites
4255165 
golang will not turn them off by default:
golang/go#13385

Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
rchincha added a commit to rchincha/zot that referenced this issue Mar 1, 2022
@rchincha
tls: restrict tls cipher suites
42db587 
golang will not turn them off by default:
golang/go#13385

Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
@rchincha rchincha mentioned this issue Mar 1, 2022
Closed
rchincha added a commit to rchincha/zot that referenced this issue Mar 2, 2022
@rchincha
tls: restrict tls cipher suites
57bcae3 
golang will not turn them off by default:
golang/go#13385

Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
@rsc rsc unassigned agl Jun 23, 2022
@ainar-g ainar-g mentioned this issue Sep 21, 2022
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Security
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@bradfitz @agl @pquerna @ianlancetaylor @timcooijmans @DemiMarie @gopherbot