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: Disable DES ciphers in TLS 1.1+ #21144

Closed
bdarnell opened this issue Jul 24, 2017 · 5 comments
Closed

crypto/tls: Disable DES ciphers in TLS 1.1+ #21144

bdarnell opened this issue Jul 24, 2017 · 5 comments
Assignees
Milestone

Comments

@bdarnell
Copy link

@bdarnell bdarnell commented Jul 24, 2017

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8.3 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/go/src/github.com/cockroachdb/cockroach/artifacts/go-build278107966=/tmp/go-build -gno-record-gcc-switches"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

  1. Run a simple HTTPS server, with tls.Config.MinVersion set to 1.2:
package main

import (
	"crypto/tls"
	"log"
	"net/http"
)

func main() {
	server := &http.Server{
		Addr: "127.0.0.1:8443",
		TLSConfig: &tls.Config{
			MinVersion: tls.VersionTLS12,
		},
	}
	err := server.ListenAndServeTLS("server.crt", "server.key")
	if err != nil {
		log.Fatal("ListenAndServe: ", err)
	}
}
  1. Run a TLS security scanner on it to show the supported cipher suites:
$ sslscan 127.0.0.1:8443
Version: 1.11.10
OpenSSL 1.0.2l  25 May 2017

OpenSSL version does not support SSLv2
SSLv2 ciphers will not be detected

Testing SSL server 127.0.0.1 on port 8443 using SNI name 127.0.0.1

  TLS Fallback SCSV:
Server does not support TLS Fallback SCSV

  TLS renegotiation:
Session renegotiation not supported

  TLS Compression:
Compression disabled

  Heartbleed:
TLS 1.2 not vulnerable to heartbleed
TLS 1.1 not vulnerable to heartbleed
TLS 1.0 not vulnerable to heartbleed

  Supported Server Cipher(s):
Preferred TLSv1.2  128 bits  ECDHE-RSA-AES128-GCM-SHA256   Curve P-256 DHE 256
Accepted  TLSv1.2  256 bits  ECDHE-RSA-AES256-GCM-SHA384   Curve P-256 DHE 256
Accepted  TLSv1.2  128 bits  ECDHE-RSA-AES128-SHA          Curve P-256 DHE 256
Accepted  TLSv1.2  256 bits  ECDHE-RSA-AES256-SHA          Curve P-256 DHE 256
Accepted  TLSv1.2  128 bits  AES128-GCM-SHA256            
Accepted  TLSv1.2  256 bits  AES256-GCM-SHA384            
Accepted  TLSv1.2  128 bits  AES128-SHA                   
Accepted  TLSv1.2  256 bits  AES256-SHA                   
Accepted  TLSv1.2  112 bits  ECDHE-RSA-DES-CBC3-SHA        Curve P-256 DHE 256
Accepted  TLSv1.2  112 bits  DES-CBC3-SHA                 

  SSL Certificate:
Signature Algorithm: sha256WithRSAEncryption
RSA Key Strength:    2048

Subject:  node
Altnames: DNS:localhost, DNS:*.local, IP Address:127.0.0.1, IP Address:0:0:0:0:0:0:0:1
Issuer:   Cockroach CA

Not valid before: Mar 29 17:47:45 2017 GMT
Not valid after:  Jan  1 00:00:00 9999 GMT

What did you expect to see?

Only secure cipher suites should be enabled by default.

What did you see instead?

Two DES-based cipher suites are enabled by default. This is flagged as vulnerable to the Sweet32 attack. According to cloudflare's docs on the subject, there is no legitimate need for DES ciphers in TLS 1.2 (or 1.1), so disabling these suites in TLS 1.1+ should be a straightforward win for security. Or, if there are other reasons that this is not a concern in Go's implementation, this should be documented to put our users' minds at ease.

We can use the tls.Config.CipherSuites option to disable the DES-based ciphers ourselves, but we'd prefer to delegate the knowledge of which ciphers are safe to the Go crypto team.

This is a subset of #13385, because the DES ciphers also use CBC.

@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Jul 24, 2017

cc @agl

@ALTree ALTree added this to the Go1.10 milestone Jul 24, 2017
@agl agl self-assigned this Jul 24, 2017
@agl

This comment has been minimized.

Copy link
Contributor

@agl agl commented Jul 24, 2017

Are you suggesting that setting the MinVersion should also change the default cipher suites? Or that servers should never negotiate 3DES with a client that offers TLS >= 1.1 despite the cipher suite config? Both of these seem rather surprising cross-behaviours. As you note, if you wish to disable cipher suites you are able to.

Also, keep in mind that disabling 3DES does not improve the first-order security of the system—it just breaks clients that cannot do better. That might lead to helpful second-order effects (e.g. they update their clients), but the benefit is indirect.

I would prefer to disable 3DES by default. That's simple and clear. However, 3DES is what IE on Windows XP is clinging onto and I suspect there is a constituency of Go users who care about that. At some point the balance will shift and they'll have to explicitly enable 3DES rather than the other set explicitly disabling it. I don't know whether 1.10 is the point where that happens.

@groob

This comment has been minimized.

Copy link
Contributor

@groob groob commented Jul 24, 2017

The Mozilla wiki has a good list of CipherSuites and compatibility with various browsers.
https://wiki.mozilla.org/Security/Server_Side_TLS#Recommended_configurations

@bdarnell

This comment has been minimized.

Copy link
Author

@bdarnell bdarnell commented Jul 24, 2017

Are you suggesting that setting the MinVersion should also change the default cipher suites? Or that servers should never negotiate 3DES with a client that offers TLS >= 1.1 despite the cipher suite config?

Yes, that's my suggestion. RFC 5246 says "Removed IDEA and DES cipher suites. They are now deprecated and will be documented in a separate document." This language suggests that these ciphers should not be used when negotiating TLS 1.2 (or is that only referring to single DES?) If there's no precedent for this sort of "cross-behavior", though, I can see why you might be reluctant to introduce it.

Also, keep in mind that disabling 3DES does not improve the first-order security of the system—it just breaks clients that cannot do better. That might lead to helpful second-order effects (e.g. they update their clients), but the benefit is indirect.

So in that case should the report from Nessus that this configuration is vulnerable to Sweet32 be regarded as a false positive?

What I'm really looking for is an answer to "i'm willing to abandon browsers older than X; what is the most secure way to configure my TLS server?". I was hoping that setting MinVersion to 1.2 would be a reasonable proxy for this, but maybe that's not the right way to look at it. Maybe there should be a package (probably outside the standard library so it can update on its own schedule) containing factory functions that construct tls.Configs from higher-level parameters (like Python's ssl.create_default_context, which incidentally just dropped 3DES from the default configuration in Python 3.6)

@agl

This comment has been minimized.

Copy link
Contributor

@agl agl commented Jul 25, 2017

RFC 5246 says "Removed IDEA and DES cipher suites. They are now deprecated and will be documented in a separate document." This language suggests that these ciphers should not be used when negotiating TLS 1.2 (or is that only referring to single DES?)

It means single-DES. 3DES is still described in that RFC. There's no standards-based prohibition with using 3DES with TLS 1.2.

So in that case should the report from Nessus that this configuration is vulnerable to Sweet32 be regarded as a false positive?

It's technically true, but the implications are perhaps non-obvious. It's not the case that the whole user population is vulnerable because 3DES is enabled. Rather, users who otherwise couldn't connect can do so, but only with 3DES, and that means that they'll be subject to birthday-bound limitations inherent with any 64-bit cipher.

What I'm really looking for is an answer to "i'm willing to abandon browsers older than X; what is the most secure way to configure my TLS server?"

As linked above, there are several resources that have opinions on how you should configure TLS servers. But the MinVersion controls the minimum version and I think it would be unwise for these low-level controls to have non-obvious effects, like also changing the cipher list. So, on that question, I'm going to mark this bug as closed.

But you can certainly create a package that returns tls.Config objects that reflect specific policies as a baseline for people to configure Go servers.

@agl agl closed this Jul 25, 2017
@golang golang locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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