Skip to content

Commit

Permalink
http2: require either ECDSA or RSA ciphersuite
Browse files Browse the repository at this point in the history
The HTTP/2 RFC does indeed mandate TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
but in practice, people are also using TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
becuase they are only using an ECDSA certificate. This is the case in acme/autocert.

It doesn't make sense to enforce only RSA in cipher suites if it will
never be used because they are using a ECDSA certificate.

Change-Id: I86dac192a3eb9b74e4268310a3b550b3bd88a37f
Reviewed-on: https://go-review.googlesource.com/30721
Reviewed-by: Tom Bergan <tombergan@google.com>
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
nhooyr authored and tombergan committed Nov 7, 2017
1 parent 01c1902 commit a337091
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
11 changes: 7 additions & 4 deletions http2/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,15 @@ func ConfigureServer(s *http.Server, conf *Server) error {
} else if s.TLSConfig.CipherSuites != nil {
// If they already provided a CipherSuite list, return
// an error if it has a bad order or is missing
// ECDHE_RSA_WITH_AES_128_GCM_SHA256.
const requiredCipher = tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
// ECDHE_RSA_WITH_AES_128_GCM_SHA256 or ECDHE_ECDSA_WITH_AES_128_GCM_SHA256.
haveRequired := false
sawBad := false
for i, cs := range s.TLSConfig.CipherSuites {
if cs == requiredCipher {
switch cs {
case tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
// Alternative MTI cipher to not discourage ECDSA-only servers.
// See http://golang.org/cl/30721 for further information.
tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256:
haveRequired = true
}
if isBadCipher(cs) {
Expand All @@ -235,7 +238,7 @@ func ConfigureServer(s *http.Server, conf *Server) error {
}
}
if !haveRequired {
return fmt.Errorf("http2: TLSConfig.CipherSuites is missing HTTP/2-required TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256")
return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher.")
}
}

Expand Down
8 changes: 7 additions & 1 deletion http2/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3189,12 +3189,18 @@ func TestConfigureServer(t *testing.T) {
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
},
},
{
name: "just the alternative required cipher suite",
tlsConfig: &tls.Config{
CipherSuites: []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
},
},
{
name: "missing required cipher suite",
tlsConfig: &tls.Config{
CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384},
},
wantErr: "is missing HTTP/2-required TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
wantErr: "is missing an HTTP/2-required AES_128_GCM_SHA256 cipher.",
},
{
name: "required after bad",
Expand Down

0 comments on commit a337091

Please sign in to comment.