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: mitigate "Sweet32" #41476

Open
r10r opened this issue Sep 18, 2020 · 7 comments
Open

crypto/tls: mitigate "Sweet32" #41476

r10r opened this issue Sep 18, 2020 · 7 comments
Assignees
Milestone

Comments

@r10r
Copy link

@r10r r10r commented Sep 18, 2020

What version of Go are you using (go version)?

$ go version go1.15.2 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"

What did you do?

The godoc for https://golang.org/pkg/crypto/tls/#CipherSuites states

CipherSuites returns a list of cipher suites currently implemented by this package, excluding those with security issues, which are returned by InsecureCipherSuites.

https://play.golang.org/p/1RmZ0n-CKbT

What did you expect to see?

No insecure ciphers listed.

What did you see instead?

TLS_RSA_WITH_3DES_EDE_CBC_SHA is vulnerable to Sweet32 CVE-2016-2183
TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA is vulnerable to Sweet32 CVE-2016-2183
@r10r
Copy link
Author

@r10r r10r commented Sep 18, 2020

testssl.sh reports this in the section Testing vulnerabilities

@ALTree ALTree changed the title tls: Remove vulnerable DES and Triple DES ciphers - CVE-2016-2183 "Sweet32" crypto/tls: remove vulnerable DES and Triple DES ciphers - CVE-2016-2183 "Sweet32" Sep 18, 2020
@ALTree
Copy link
Member

@ALTree ALTree commented Sep 18, 2020

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Sep 18, 2020

3DES ciphers are picked as the last fallback by peers that can't do anything better, and the only vulnerability they have is the birthday bound inherent to the block size. The attack is impractical, but I had already planned to drop a counter in there to mitigate it.

@FiloSottile FiloSottile changed the title crypto/tls: remove vulnerable DES and Triple DES ciphers - CVE-2016-2183 "Sweet32" crypto/tls: mitigate "Sweet32" Sep 18, 2020
@r10r
Copy link
Author

@r10r r10r commented Sep 18, 2020

Thanks for your response.

but I had already planned to drop a counter in there to mitigate it.

So you're going to make some changes to the algorithm but keep the ciphers enabled ?

A note in the godoc would be good here. It's easy to drop these ciphers If you know that you have to.
I would like to keep going with the cipher list as sane defaults - but we got complaints from our customers.
It's hard to argue with customers that this can't be exploited in reality - they see a vulerability in their security scanner and complain about it.

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Sep 18, 2020

Since we can implement these ciphers securely, we're not going to break old clients that can still establish a secure connection only to appease security scanners. You should probably open an issue with the scanner if possible.

@r10r
Copy link
Author

@r10r r10r commented Sep 18, 2020

Ok that's good to know. So for secure implementation you "restrict the number of blocks in a key bundle" ?
Can you give me a short pointer how the scanner could possibly distinguish a secure from an insecure implementation?

I trust your words but maybe dropping a note in the godoc about this would be a good thing.

This is just for the records:

as consequence Triple DES has been deprecated by NIST in 2017

OpenSSL does not include 3DES by default since version 1.1.0 (August 2016) and considers it a "weak cipher"

@r10r
Copy link
Author

@r10r r10r commented Sep 18, 2020

Shall I update the documentation and send you a pull request ?

@FiloSottile FiloSottile added this to the Go1.16 milestone Sep 22, 2020
@FiloSottile FiloSottile self-assigned this Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.