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

Hardening: TLS >= 1.2, limit cipher suites #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cstsw
Copy link

@cstsw cstsw commented Jun 24, 2022

In order to provide a reasonably secure TLS configuration, the following defaults have been set:

  • Don't use TLS versions below 1.2 as those are vulnerable to attacks such as BEAST (CVE-2011-3389) and FREAK (CVE-2015-0204)
  • Exclude ciphers known to be vulnerable, i.e. (3)DES, RC4, CBC ciphers

In order to provide a reasonably secure TLS configuration, the following defaults have been set:
- Don't use TLS versions below 1.2 as those are vulnerable to attacks such as BEAST (CVE-2011-3389) and FREAK (CVE-2015-0204)
- Exclude ciphers known to be vulnerable, i.e. (3)DES, RC4, CBC ciphers
@cstsw
Copy link
Author

cstsw commented Jun 24, 2022

My suggestion for a reasonably secure TLS configuration in order to address #42

Copy link
Owner

@suyashkumar suyashkumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for identifying and for the contribution! Just had a quick question I commented inline in the code.

// Limit cipher suites available as of go 1.13
// - List according to crypto/tls constants - in reverse order (i.e. prefer stronger over weaker ciphers)
// - Filtered out: RC4, (3)DES, CBC suites
tlsCfg.CipherSuites = []uint16{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of specifying our own can we rely on the "safe default" list chosen by the Go authors? As vulnerabilities emerge, we may need to specify our own list or upgrade Go, but at the moment upgrading seems sufficient and limits complexity on our end?

It also appears in go1.18.3 the tls1.3 cipher suites are not settable, so do they need to be included? (As an aside, I'm going to send a PR to update the repo to go1.18.3, so we are up to date with default lists)

https://cs.opensource.google/go/go/+/refs/tags/go1.18.3:src/crypto/tls/common.go;l=649-654;drc=2580d0e08d5e9f979b943758d3c49877fb2324cb

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just sent #44 to update go).

If we can just go with the safe default list, we can not specify this list but we can still specify the min tls version and go from there. wdyt?

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

Successfully merging this pull request may close these issues.

2 participants