http2: remove TLSConfig.CipherSuites order check

The order of CipherSuites is ignored as of Go 1.17, and the effective order will always put good ciphers first, as tested by TestCipherSuites in crypto/tls. Updates golang/go#45430 Change-Id: I54424f68bffe805f504b57eb025601c08c6357f6 Reviewed-on: https://go-review.googlesource.com/c/net/+/314611 Run-TryBot: Filippo Valsorda <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Filippo Valsorda <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]>
dteh · May 8, 2021 · 984f4d5 · 984f4d5
1 parent 27beaea
commit 984f4d5
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 13 deletions.
diff --git a/server.go b/server.go
@@ -231,28 +231,22 @@ func ConfigureServer(s *http.Server, conf *Server) error {
 
 	if s.TLSConfig == nil {
 		s.TLSConfig = new(tls.Config)
-	} 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 or ECDHE_ECDSA_WITH_AES_128_GCM_SHA256.
+	} else if s.TLSConfig.CipherSuites != nil && s.TLSConfig.MinVersion < tls.VersionTLS13 {
+		// If they already provided a TLS 1.0–1.2 CipherSuite list, return an
+		// error if it is missing 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 {
+		for _, cs := range s.TLSConfig.CipherSuites {
 			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) {
-				sawBad = true
-			} else if sawBad {
-				return fmt.Errorf("http2: TLSConfig.CipherSuites index %d contains an HTTP/2-approved cipher suite (%#04x), 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.", i, cs)
-			}
 		}
 		if !haveRequired {
-			return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 or TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256).")
+			return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 or TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)")
 		}
 	}
 

diff --git a/server_test.go b/server_test.go
@@ -3385,6 +3385,17 @@ func TestConfigureServer(t *testing.T) {
 		{
 			name: "empty server",
 		},
+		{
+			name:      "empty CipherSuites",
+			tlsConfig: &tls.Config{},
+		},
+		{
+			name: "bad CipherSuites but MinVersion TLS 1.3",
+			tlsConfig: &tls.Config{
+				MinVersion:   tls.VersionTLS13,
+				CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384},
+			},
+		},
 		{
 			name: "just the required cipher suite",
 			tlsConfig: &tls.Config{
@@ -3409,7 +3420,6 @@ func TestConfigureServer(t *testing.T) {
 			tlsConfig: &tls.Config{
 				CipherSuites: []uint16{tls.TLS_RSA_WITH_RC4_128_SHA, tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
 			},
-			wantErr: "contains an HTTP/2-approved cipher suite (0xc02f), but it comes after",
 		},
 		{
 			name: "bad after required",