net/http: check GetConfigForClient in server.ServeTLS

Just like for tls.Config.GetCertificate the http.Server.ServeTLS method
should be checking tls.Config.GetConfigForClient before trying top open
the specified certFile/keyFile.

This was previously fixed for crypto/tls when using tls.Listen in
CL205059, but the same change for net/http was missed. I've added a
comment src/crypto/tls/tls.go in the relevant section in the hope that
any future changes of a similar nature consider will consider updating
net/http as needed as well.

Change-Id: I312303bc497d92aa2f4627fe2620c70779cbcc99
GitHub-Last-Rev: 6ed29a900816a13690a9f3e26476d9bc1055a6f7
GitHub-Pull-Request: golang/go#66795
Reviewed-on: https://go-review.googlesource.com/c/go/+/578396
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>

diff --git a/src/crypto/tls/tls.go b/src/crypto/tls/tls.go
index b30f0b8fe4fdd700d36f4aaa6263d13cbb3ea698..f3089f0ed68ddaa8867a0e83eb98465ad2d9de39 100644 (file)
--- a/src/crypto/tls/tls.go
+++ b/src/crypto/tls/tls.go
@@ -87,6 +87,7 @@ func NewListener(inner net.Listener, config *Config) net.Listener {
 // The configuration config must be non-nil and must include
 // at least one certificate or else set GetCertificate.
 func Listen(network, laddr string, config *Config) (net.Listener, error) {
+       // If this condition changes, consider updating http.Server.ServeTLS too.
        if config == nil || len(config.Certificates) == 0 &&
                config.GetCertificate == nil && config.GetConfigForClient == nil {
                return nil, errors.New("tls: neither Certificates, GetCertificate, nor GetConfigForClient set in Config")

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 34b7d57f4014072e3853926a30d3fbdc9a74ca7c..06bf5089d8fe63ae48627f429d8a827cf72a6789 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -1748,6 +1748,24 @@ func TestAutomaticHTTP2_ListenAndServe_GetCertificate(t *testing.T) {
        })
 }
 
+func TestAutomaticHTTP2_ListenAndServe_GetConfigForClient(t *testing.T) {
+       cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey)
+       if err != nil {
+               t.Fatal(err)
+       }
+       conf := &tls.Config{
+               // GetConfigForClient requires specifying a full tls.Config so we must set
+               // NextProtos ourselves.
+               NextProtos:   []string{"h2"},
+               Certificates: []tls.Certificate{cert},
+       }
+       testAutomaticHTTP2_ListenAndServe(t, &tls.Config{
+               GetConfigForClient: func(clientHello *tls.ClientHelloInfo) (*tls.Config, error) {
+                       return conf, nil
+               },
+       })
+}
+
 func testAutomaticHTTP2_ListenAndServe(t *testing.T, tlsConf *tls.Config) {
        CondSkipHTTP2(t)
        // Not parallel: uses global test hooks.

diff --git a/src/net/http/server.go b/src/net/http/server.go
index b9a6edd7ad2508bf1d15b12d77c996c7042564d4..190f56501379b0eb43af91a2ff7ba8a5b6d3ad8c 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -3370,7 +3370,8 @@ func (srv *Server) Serve(l net.Listener) error {
 //
 // Files containing a certificate and matching private key for the
 // server must be provided if neither the [Server]'s
-// TLSConfig.Certificates nor TLSConfig.GetCertificate are populated.
+// TLSConfig.Certificates, TLSConfig.GetCertificate nor
+// config.GetConfigForClient are populated.
 // If the certificate is signed by a certificate authority, the
 // certFile should be the concatenation of the server's certificate,
 // any intermediates, and the CA's certificate.
@@ -3389,7 +3390,7 @@ func (srv *Server) ServeTLS(l net.Listener, certFile, keyFile string) error {
                config.NextProtos = append(config.NextProtos, "http/1.1")
        }
 
-       configHasCert := len(config.Certificates) > 0 || config.GetCertificate != nil
+       configHasCert := len(config.Certificates) > 0 || config.GetCertificate != nil || config.GetConfigForClient != nil
        if !configHasCert || certFile != "" || keyFile != "" {
                var err error
                config.Certificates = make([]tls.Certificate, 1)