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

Jump to bottom

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
Closed

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

gotwarlost opened this issue May 2, 2017 · 6 comments
Labels
FrozenDueToAge

Comments

@gotwarlost
Copy link

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
Copy link
Contributor

bradfitz commented May 2, 2017

/cc @tombergan

@dmitris
Copy link
Contributor

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
Copy link

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

@dmitris
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
@bradfitz
net/http: update bundled http2
724fd13 
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
@dmitris @bradfitz
http2: add all bad ciphers, use package constants
ccb9e00 
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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bradfitz @dmitris @gotwarlost @gopherbot