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

Add option to configure TLS cipher suites #1193

Closed
coderanger opened this issue Mar 9, 2016 · 12 comments
Closed

Add option to configure TLS cipher suites #1193

coderanger opened this issue Mar 9, 2016 · 12 comments

Comments

@coderanger
Copy link

The default Go cipher configs are a bit wonky, it would be nice to be able to put in a cipher string in the config to override it. I know bupkiss about Go's TLS listener API, but a quick look over of the docs made this seem non-trivial.

@jefferai
Copy link
Member

jefferai commented Mar 9, 2016

Any chance you can define "wonky"?

@coderanger
Copy link
Author

Ciphersuite order is chosen so that ... RC4 comes before AES (because of the Lucky13 attack).

Granted I can't really blame them, CBC vs. RC4 is likely choosing which finger you would like blown off. Would rather just disable both, and maybe 3DES while I'm at it because 😿 downgrade attacks are sadmaking.

@coderanger
Copy link
Author

For reference https://github.com/golang/go/blob/master/src/crypto/tls/cipher_suites.go#L78-L94 is Go's default cipher ordering I think.

@jefferai
Copy link
Member

jefferai commented Mar 9, 2016

Note that you can easily disable this on the client side as well.

@coderanger
Copy link
Author

"easily" would depend on what TLS client library you were using, and from a compliance PoV it is nice to just knock out everything related to MitM downgrades at once (like you can already do with TLS versions).

@jefferai
Copy link
Member

I think Go's defaults are pretty sane here, and they update them with each release based on current thinking in the security community. But I'm happy to look at a PR if one pops up...shouldn't be too bad, since Go has constants that map to the uint16 values. It would just need to basically have a string array and logic to do the mapping.

@pearkes pearkes closed this as completed Apr 19, 2016
@coderanger
Copy link
Author

Was this closed for an actual reason?

@jefferai jefferai reopened this Apr 19, 2016
@vishalnayak
Copy link
Member

@coderanger

Some of the Vault issues were closed by mistake from a syncing software. We apologize for the confusion caused. We have reopened the issues that were closed and are investigating the problem that caused this. Again, we are very sorry for the inconvenience.

@rhuddleston
Copy link

+1

I just got this from a pen tester:

Sweet32: Birthday attacks on 64-bit block ciphers

https://sweet32.info/
https://www.openssl.org/blog/blog/2016/08/24/sweet32/

nmap --script +ssl-enum-ciphers -p 8200 node1.dev.xxx.corp

PORT STATE SERVICE
8200/tcp open trivnet1
| ssl-enum-ciphers:
| TLSv1.2:
| ciphers:
| TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA (secp256r1) - C
| TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
| TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
| TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
| TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
| TLS_RSA_WITH_3DES_EDE_CBC_SHA (rsa 2048) - C
| TLS_RSA_WITH_AES_128_CBC_SHA (rsa 2048) - A
| TLS_RSA_WITH_AES_128_GCM_SHA256 (rsa 2048) - A
| TLS_RSA_WITH_AES_256_CBC_SHA (rsa 2048) - A
| TLS_RSA_WITH_AES_256_GCM_SHA384 (rsa 2048) - A
| compressors:
| NULL
| cipher preference: client
| warnings:
| 64-bit block cipher 3DES vulnerable to SWEET32 attack
|_ least strength: C

So at the least we need to disable these ciphers:

TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
TLS_RSA_WITH_3DES_EDE_CBC_SHA

@jefferai
Copy link
Member

@rhuddleston if you're willing to do the work it'll get done much faster than leaving it to us :-) if so let me know and we can figure out the design. Shouldn't be too bad.

@roman-vynar
Copy link
Contributor

@jefferai I have added PR on this if you have a chance to take a look.

@jefferai
Copy link
Member

Closed via #2293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants
@jefferai @coderanger @pearkes @roman-vynar @vishalnayak @rhuddleston and others