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: Remove Lucky13 padding oracle and deprioritize RC4 #7418

Closed
matthewg opened this issue Feb 26, 2014 · 3 comments
Closed

crypto/tls: Remove Lucky13 padding oracle and deprioritize RC4 #7418

matthewg opened this issue Feb 26, 2014 · 3 comments

Comments

@matthewg
Copy link

crypto/tls/cipher_suites.go says:
  // Ciphersuite order is chosen so that ECDHE comes before plain RSA
  // and RC4 comes before AES (because of the Lucky13 attack).

I believe this refers to this bit from crypto/tls/conn.go:
  // note that we still have a timing side-channel in the
  // MAC check, below. An attacker can align the record
  // so that a correct padding will cause one less hash
  // block to be calculated. Then they can iteratively
  // decrypt a record by breaking each byte. See
  // "Password Interception in a SSL/TLS Channel", Brice
  // Canvel et al.
  //
  // However, our behavior matches OpenSSL, so we leak
  // only as much as they do.

If I understand correctly, OpenSSL addressed this issue with change
<http://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=e130841bccfc0bb9da254dc84e23bc6a1c78a64e>.

It'd be good to apply a similar fix to Go, and then adjust the default cipher suite
order to prefer AES to RC4.

For comparison, Android's default suites
<https://android.googlesource.com/platform/external/conscrypt/+/master/src/main/java/org/conscrypt/NativeCrypto.java>;
@robpike
Copy link
Contributor

robpike commented Feb 26, 2014

Comment 1:

Owner changed to @agl.

@agl
Copy link
Contributor

agl commented Feb 26, 2014

Comment 2:

I don't plan on fixing Lucky13 in Go because I've done it in NSS and OpenSSL and it's a
nightmare - it would destroy the codebase:
https://www.imperialviolet.org/2013/02/04/luckythirteen.html
Rather, AES-GCM is the answer.

Status changed to WorkingAsIntended.

@matthewg
Copy link
Author

Comment 3:

Should the order of these three entries in the default cipher suite list be changed,
then?
  TLS_RSA_WITH_RC4_128_SHA
  TLS_RSA_WITH_AES_128_CBC_SHA
  TLS_RSA_WITH_AES_256_CBC_SHA

@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants