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

x/net/http2: incomplete list of bad HTTP/2 cipher suites causes a TLS server to not start up with a correct cipher suite list #20213

Closed
gotwarlost opened this issue May 2, 2017 · 6 comments

Comments

@gotwarlost
Copy link

@gotwarlost gotwarlost commented May 2, 2017

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ananthk"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build037787161=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
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?

https://play.golang.org/p/ucaLECZznh

What did you expect to see?

Server should start correctly

What did you see instead?

Server produces error:

http2: TLSConfig.CipherSuites index 7 contains an HTTP/2-approved cipher suite (0x003c), but it comes after unapproved cipher suites. With this configuration, clients that don't support previous, approved cipher suites may be given an unapproved one and reject the connection.

My explanation in the code comments in the playground link.

@bradfitz bradfitz changed the title Incomplete list of bad HTTP/2 cipher suites causes a TLS server to not start up with a correct cipher suite list x/net/http2: incomplete list of bad HTTP/2 cipher suites causes a TLS server to not start up with a correct cipher suite list May 2, 2017
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented May 2, 2017

@dmitris

This comment has been minimized.

Copy link
Contributor

@dmitris dmitris commented May 2, 2017

confirmed also with go1.8.1. With the diff below, I am not getting the TLSConfig.CipherSuites error in the test program:

$ git diff
diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go
index 57bc0a5..859752c 100644
--- a/src/net/http/h2_bundle.go
+++ b/src/net/http/h2_bundle.go
@@ -2175,6 +2175,7 @@ func http2isBadCipher(cipher uint16) bool {
                tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA,
                tls.TLS_RSA_WITH_AES_128_CBC_SHA,
                tls.TLS_RSA_WITH_AES_256_CBC_SHA,
+               tls.TLS_RSA_WITH_AES_128_CBC_SHA256,
                tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
                tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
                tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented May 2, 2017

CL https://golang.org/cl/42510 mentions this issue.

@dmitris

This comment has been minimized.

Copy link
Contributor

@dmitris dmitris commented May 3, 2017

With the change https://golang.org/cl/42510 imported into go master, getting a different, expected error on the test program:

2017/05/03 12:50:19 open cert.pem: no such file or directory
exit status 1
@dmitris

This comment has been minimized.

Copy link
Contributor

@dmitris dmitris commented May 3, 2017

@bradfitz - should I make another change to import the current x/net into stdlib's net/http (h2_bundle.go), or does it happen automatically at some point? When I run go generate, I'm getting a generated file where all references to the current http package are prepended with http. - dmitris/go@master...tlscerts - am I doing something wrong with the go generate invocation?

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented May 3, 2017

@dmitris, I already sent you the CL. You can approve https://go-review.googlesource.com/c/42494/

gopherbot pushed a commit that referenced this issue May 3, 2017
Updates bundled http2 to x/net/http2 git rev feeb485 for:

    http2: add all bad ciphers, use package constants
    https://golang.org/cl/42510

Updates #20213

Change-Id: I851453e3785e6b126db7a5c5eec2ebbbf61358ae
Reviewed-on: https://go-review.googlesource.com/42494
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Savintsev <dsavints@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
Make all the ciphers from
https://www.iana.org/assignments/tls-parameters/tls-parameters.txt
available as package constants (no longer relying on
crypto/tls).

Number of bad ciphers such as TLS_RSA_WITH_AES_128_CBC_SHA256
from https://tools.ietf.org/html/rfc7540#appendix-A
are added to the HTTP/2 blacklist
(also listed in https://http2.github.io/http2-spec/#BadCipherSuites).
The zero CipherSuite TLS_NULL_WITH_NULL_NULL (0x00) is now explicitly
marked as a bad one which required change of some test mocks.

Fixes golang/go#20213

Change-Id: I6b02061603cce4cf469998606400ed6729199238
Reviewed-on: https://go-review.googlesource.com/42510
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators May 3, 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
4 participants
You can’t perform that action at this time.