crypto/tls: add Config.Clone

In Go 1.0, the Config struct consisted only of exported fields.

In Go 1.1, it started to grow private, uncopyable fields (sync.Once,
sync.Mutex, etc).

Ever since, people have been writing their own private Config.Clone
methods, or risking it and doing a language-level shallow copy and
copying the unexported sync variables.

Clean this up and export the Config.clone method as Config.Clone.
This matches the convention of Template.Clone from text/template and
html/template at least.

Fixes #15771
Updates #16228 (needs update in x/net/http2 before fixed)
Updates #16492 (not sure whether @agl wants to do more)

Change-Id: I48c2825d4fef55a75d2f99640a7079c56fce39ca
Reviewed-on: https://go-review.googlesource.com/28075
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index 60f47b49bac662b6899c4f01146821d050c36487..46bc2aa03a03a0f199e5867baf95b413eb8eaa1c 100644 (file)
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -430,8 +430,9 @@ func ticketKeyFromBytes(b [32]byte) (key ticketKey) {
        return key
 }
 
-// clone returns a copy of c. Only the exported fields are copied.
-func (c *Config) clone() *Config {
+// Clone returns a shallow clone of c.
+// Only the exported fields are copied.
+func (c *Config) Clone() *Config {
        return &Config{
                Rand:                        c.Rand,
                Time:                        c.Time,

diff --git a/src/crypto/tls/conn_test.go b/src/crypto/tls/conn_test.go
index 645f13b8cfc67bd851f295f9feb73860649e482b..15397d607eb0593c166176a349e43fc9cb2432b9 100644 (file)
--- a/src/crypto/tls/conn_test.go
+++ b/src/crypto/tls/conn_test.go
@@ -124,7 +124,7 @@ func TestCertificateSelection(t *testing.T) {
 func runDynamicRecordSizingTest(t *testing.T, config *Config) {
        clientConn, serverConn := net.Pipe()
 
-       serverConfig := config.clone()
+       serverConfig := config.Clone()
        serverConfig.DynamicRecordSizingDisabled = false
        tlsConn := Server(serverConn, serverConfig)
 
@@ -225,19 +225,19 @@ func runDynamicRecordSizingTest(t *testing.T, config *Config) {
 }
 
 func TestDynamicRecordSizingWithStreamCipher(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.CipherSuites = []uint16{TLS_RSA_WITH_RC4_128_SHA}
        runDynamicRecordSizingTest(t, config)
 }
 
 func TestDynamicRecordSizingWithCBC(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.CipherSuites = []uint16{TLS_RSA_WITH_AES_256_CBC_SHA}
        runDynamicRecordSizingTest(t, config)
 }
 
 func TestDynamicRecordSizingWithAEAD(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.CipherSuites = []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}
        runDynamicRecordSizingTest(t, config)
 }

diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go
index 45a4544e120270debdad721e2d05e3ce22e43244..a5491bcdf36053978b3594d068c16015eef9d1b6 100644 (file)
--- a/src/crypto/tls/handshake_client_test.go
+++ b/src/crypto/tls/handshake_client_test.go
@@ -535,7 +535,7 @@ func TestHandshakeClientECDHEECDSAAES128CBCSHA256(t *testing.T) {
 }
 
 func TestHandshakeClientCertRSA(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        cert, _ := X509KeyPair([]byte(clientCertificatePEM), []byte(clientKeyPEM))
        config.Certificates = []Certificate{cert}
 
@@ -571,7 +571,7 @@ func TestHandshakeClientCertRSA(t *testing.T) {
 }
 
 func TestHandshakeClientCertECDSA(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        cert, _ := X509KeyPair([]byte(clientECDSACertificatePEM), []byte(clientECDSAKeyPEM))
        config.Certificates = []Certificate{cert}
 
@@ -728,7 +728,7 @@ func TestLRUClientSessionCache(t *testing.T) {
 }
 
 func TestHandshakeClientKeyLog(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        buf := &bytes.Buffer{}
        config.KeyLogWriter = buf
 
@@ -769,7 +769,7 @@ func TestHandshakeClientKeyLog(t *testing.T) {
 }
 
 func TestHandshakeClientALPNMatch(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.NextProtos = []string{"proto2", "proto1"}
 
        test := &clientTest{
@@ -790,7 +790,7 @@ func TestHandshakeClientALPNMatch(t *testing.T) {
 }
 
 func TestHandshakeClientALPNNoMatch(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.NextProtos = []string{"proto3"}
 
        test := &clientTest{
@@ -814,7 +814,7 @@ func TestHandshakeClientALPNNoMatch(t *testing.T) {
 const sctsBase64 = "ABIBaQFnAHUApLkJkLQYWBSHuxOizGdwCjw1mAT5G9+443fNDsgN3BAAAAFHl5nuFgAABAMARjBEAiAcS4JdlW5nW9sElUv2zvQyPoZ6ejKrGGB03gjaBZFMLwIgc1Qbbn+hsH0RvObzhS+XZhr3iuQQJY8S9G85D9KeGPAAdgBo9pj4H2SCvjqM7rkoHUz8cVFdZ5PURNEKZ6y7T0/7xAAAAUeX4bVwAAAEAwBHMEUCIDIhFDgG2HIuADBkGuLobU5a4dlCHoJLliWJ1SYT05z6AiEAjxIoZFFPRNWMGGIjskOTMwXzQ1Wh2e7NxXE1kd1J0QsAdgDuS723dc5guuFCaR+r4Z5mow9+X7By2IMAxHuJeqj9ywAAAUhcZIqHAAAEAwBHMEUCICmJ1rBT09LpkbzxtUC+Hi7nXLR0J+2PmwLp+sJMuqK+AiEAr0NkUnEVKVhAkccIFpYDqHOlZaBsuEhWWrYpg2RtKp0="
 
 func TestHandshakClientSCTs(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
 
        scts, err := base64.StdEncoding.DecodeString(sctsBase64)
        if err != nil {
@@ -849,7 +849,7 @@ func TestHandshakClientSCTs(t *testing.T) {
 }
 
 func TestRenegotiationRejected(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        test := &clientTest{
                name:                        "RenegotiationRejected",
                command:                     []string{"openssl", "s_server", "-state"},
@@ -871,7 +871,7 @@ func TestRenegotiationRejected(t *testing.T) {
 }
 
 func TestRenegotiateOnce(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.Renegotiation = RenegotiateOnceAsClient
 
        test := &clientTest{
@@ -885,7 +885,7 @@ func TestRenegotiateOnce(t *testing.T) {
 }
 
 func TestRenegotiateTwice(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.Renegotiation = RenegotiateFreelyAsClient
 
        test := &clientTest{
@@ -899,7 +899,7 @@ func TestRenegotiateTwice(t *testing.T) {
 }
 
 func TestRenegotiateTwiceRejected(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.Renegotiation = RenegotiateOnceAsClient
 
        test := &clientTest{

diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go
index a266f6754280a3a3d8742c620995864024f81e90..f42bad3a999d9f2ef0c1b6810cbecae1b756e5a6 100644 (file)
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -130,7 +130,7 @@ func TestNoRC4ByDefault(t *testing.T) {
                cipherSuites:       []uint16{TLS_RSA_WITH_RC4_128_SHA},
                compressionMethods: []uint8{compressionNone},
        }
-       serverConfig := testConfig.clone()
+       serverConfig := testConfig.Clone()
        // Reset the enabled cipher suites to nil in order to test the
        // defaults.
        serverConfig.CipherSuites = nil
@@ -147,7 +147,7 @@ func TestDontSelectECDSAWithRSAKey(t *testing.T) {
                supportedCurves:    []CurveID{CurveP256},
                supportedPoints:    []uint8{pointFormatUncompressed},
        }
-       serverConfig := testConfig.clone()
+       serverConfig := testConfig.Clone()
        serverConfig.CipherSuites = clientHello.cipherSuites
        serverConfig.Certificates = make([]Certificate, 1)
        serverConfig.Certificates[0].Certificate = [][]byte{testECDSACertificate}
@@ -172,7 +172,7 @@ func TestDontSelectRSAWithECDSAKey(t *testing.T) {
                supportedCurves:    []CurveID{CurveP256},
                supportedPoints:    []uint8{pointFormatUncompressed},
        }
-       serverConfig := testConfig.clone()
+       serverConfig := testConfig.Clone()
        serverConfig.CipherSuites = clientHello.cipherSuites
        // First test that it *does* work when the server's key is RSA.
        testClientHello(t, serverConfig, clientHello)
@@ -265,7 +265,7 @@ func TestTLS12OnlyCipherSuites(t *testing.T) {
                reply, clientErr = cli.readHandshake()
                c.Close()
        }()
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.CipherSuites = clientHello.cipherSuites
        Server(s, config).Handshake()
        s.Close()
@@ -732,7 +732,7 @@ func TestHandshakeServerAES256GCMSHA384(t *testing.T) {
 }
 
 func TestHandshakeServerECDHEECDSAAES(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.Certificates = make([]Certificate, 1)
        config.Certificates[0].Certificate = [][]byte{testECDSACertificate}
        config.Certificates[0].PrivateKey = testECDSAPrivateKey
@@ -748,7 +748,7 @@ func TestHandshakeServerECDHEECDSAAES(t *testing.T) {
 }
 
 func TestHandshakeServerKeyLog(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        buf := &bytes.Buffer{}
        config.KeyLogWriter = buf
 
@@ -785,7 +785,7 @@ func TestHandshakeServerKeyLog(t *testing.T) {
 }
 
 func TestHandshakeServerALPN(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.NextProtos = []string{"proto1", "proto2"}
 
        test := &serverTest{
@@ -806,7 +806,7 @@ func TestHandshakeServerALPN(t *testing.T) {
 }
 
 func TestHandshakeServerALPNNoMatch(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.NextProtos = []string{"proto3"}
 
        test := &serverTest{
@@ -841,7 +841,7 @@ func TestHandshakeServerSNI(t *testing.T) {
 // TestHandshakeServerSNICertForName is similar to TestHandshakeServerSNI, but
 // tests the dynamic GetCertificate method
 func TestHandshakeServerSNIGetCertificate(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
 
        // Replace the NameToCertificate map with a GetCertificate function
        nameToCert := config.NameToCertificate
@@ -863,7 +863,7 @@ func TestHandshakeServerSNIGetCertificate(t *testing.T) {
 // GetCertificate method doesn't return a cert, we fall back to what's in
 // the NameToCertificate map.
 func TestHandshakeServerSNIGetCertificateNotFound(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
 
        config.GetCertificate = func(clientHello *ClientHelloInfo) (*Certificate, error) {
                return nil, nil
@@ -881,7 +881,7 @@ func TestHandshakeServerSNIGetCertificateNotFound(t *testing.T) {
 func TestHandshakeServerSNIGetCertificateError(t *testing.T) {
        const errMsg = "TestHandshakeServerSNIGetCertificateError error"
 
-       serverConfig := testConfig.clone()
+       serverConfig := testConfig.Clone()
        serverConfig.GetCertificate = func(clientHello *ClientHelloInfo) (*Certificate, error) {
                return nil, errors.New(errMsg)
        }
@@ -900,7 +900,7 @@ func TestHandshakeServerSNIGetCertificateError(t *testing.T) {
 func TestHandshakeServerEmptyCertificates(t *testing.T) {
        const errMsg = "TestHandshakeServerEmptyCertificates error"
 
-       serverConfig := testConfig.clone()
+       serverConfig := testConfig.Clone()
        serverConfig.GetCertificate = func(clientHello *ClientHelloInfo) (*Certificate, error) {
                return nil, errors.New(errMsg)
        }
@@ -928,7 +928,7 @@ func TestHandshakeServerEmptyCertificates(t *testing.T) {
 // TestCipherSuiteCertPreferance ensures that we select an RSA ciphersuite with
 // an RSA certificate and an ECDSA ciphersuite with an ECDSA certificate.
 func TestCipherSuiteCertPreferenceECDSA(t *testing.T) {
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.CipherSuites = []uint16{TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA}
        config.PreferServerCipherSuites = true
 
@@ -938,7 +938,7 @@ func TestCipherSuiteCertPreferenceECDSA(t *testing.T) {
        }
        runServerTestTLS12(t, test)
 
-       config = testConfig.clone()
+       config = testConfig.Clone()
        config.CipherSuites = []uint16{TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA}
        config.Certificates = []Certificate{
                {
@@ -977,7 +977,7 @@ func TestResumptionDisabled(t *testing.T) {
        sessionFilePath := tempFile("")
        defer os.Remove(sessionFilePath)
 
-       config := testConfig.clone()
+       config := testConfig.Clone()
 
        test := &serverTest{
                name:    "IssueTicketPreDisable",
@@ -1090,7 +1090,7 @@ func TestClientAuth(t *testing.T) {
                defer os.Remove(ecdsaKeyPath)
        }
 
-       config := testConfig.clone()
+       config := testConfig.Clone()
        config.ClientAuth = RequestClientCert
 
        test := &serverTest{
@@ -1127,7 +1127,7 @@ func TestSNIGivenOnFailure(t *testing.T) {
                serverName:         expectedServerName,
        }
 
-       serverConfig := testConfig.clone()
+       serverConfig := testConfig.Clone()
        // Erase the server's cipher suites to ensure the handshake fails.
        serverConfig.CipherSuites = nil
 

diff --git a/src/crypto/tls/tls.go b/src/crypto/tls/tls.go
index e11e7dd5672184b81581c35dc671ebe0bcf3acda..fc864288519a7a5ed196e222a6c524e5d3e36c7b 100644 (file)
--- a/src/crypto/tls/tls.go
+++ b/src/crypto/tls/tls.go
@@ -135,7 +135,7 @@ func DialWithDialer(dialer *net.Dialer, network, addr string, config *Config) (*
        // from the hostname we're connecting to.
        if config.ServerName == "" {
                // Make a copy to avoid polluting argument or default.
-               c := config.clone()
+               c := config.Clone()
                c.ServerName = hostname
                config = c
        }

diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go
index 9305e3ae1ea5b578e4c24bfda94f7d5fb82f7e5e..8b8dfa4e1e6ab7cbc4abb278e2e8df75f965bfbf 100644 (file)
--- a/src/crypto/tls/tls_test.go
+++ b/src/crypto/tls/tls_test.go
@@ -241,7 +241,7 @@ func testConnReadNonzeroAndEOF(t *testing.T, delay time.Duration) error {
                        srvCh <- nil
                        return
                }
-               serverConfig := testConfig.clone()
+               serverConfig := testConfig.Clone()
                srv := Server(sconn, serverConfig)
                if err := srv.Handshake(); err != nil {
                        serr = fmt.Errorf("handshake: %v", err)
@@ -251,7 +251,7 @@ func testConnReadNonzeroAndEOF(t *testing.T, delay time.Duration) error {
                srvCh <- srv
        }()
 
-       clientConfig := testConfig.clone()
+       clientConfig := testConfig.Clone()
        conn, err := Dial("tcp", ln.Addr().String(), clientConfig)
        if err != nil {
                t.Fatal(err)
@@ -295,7 +295,7 @@ func TestTLSUniqueMatches(t *testing.T) {
                        if err != nil {
                                t.Fatal(err)
                        }
-                       serverConfig := testConfig.clone()
+                       serverConfig := testConfig.Clone()
                        srv := Server(sconn, serverConfig)
                        if err := srv.Handshake(); err != nil {
                                t.Fatal(err)
@@ -304,7 +304,7 @@ func TestTLSUniqueMatches(t *testing.T) {
                }
        }()
 
-       clientConfig := testConfig.clone()
+       clientConfig := testConfig.Clone()
        clientConfig.ClientSessionCache = NewLRUClientSessionCache(1)
        conn, err := Dial("tcp", ln.Addr().String(), clientConfig)
        if err != nil {
@@ -394,7 +394,7 @@ func TestConnCloseBreakingWrite(t *testing.T) {
                        srvCh <- nil
                        return
                }
-               serverConfig := testConfig.clone()
+               serverConfig := testConfig.Clone()
                srv := Server(sconn, serverConfig)
                if err := srv.Handshake(); err != nil {
                        serr = fmt.Errorf("handshake: %v", err)
@@ -414,7 +414,7 @@ func TestConnCloseBreakingWrite(t *testing.T) {
                Conn: cconn,
        }
 
-       clientConfig := testConfig.clone()
+       clientConfig := testConfig.Clone()
        tconn := Client(conn, clientConfig)
        if err := tconn.Handshake(); err != nil {
                t.Fatal(err)
@@ -507,7 +507,7 @@ func TestClone(t *testing.T) {
                f.Set(q)
        }
 
-       c2 := c1.clone()
+       c2 := c1.Clone()
 
        if !reflect.DeepEqual(&c1, c2) {
                t.Errorf("clone failed to copy a field")
@@ -555,7 +555,7 @@ func throughput(b *testing.B, totalBytes int64, dynamicRecordSizingDisabled bool
                                // (cannot call b.Fatal in goroutine)
                                panic(fmt.Errorf("accept: %v", err))
                        }
-                       serverConfig := testConfig.clone()
+                       serverConfig := testConfig.Clone()
                        serverConfig.DynamicRecordSizingDisabled = dynamicRecordSizingDisabled
                        srv := Server(sconn, serverConfig)
                        if err := srv.Handshake(); err != nil {
@@ -568,7 +568,7 @@ func throughput(b *testing.B, totalBytes int64, dynamicRecordSizingDisabled bool
        }()
 
        b.SetBytes(totalBytes)
-       clientConfig := testConfig.clone()
+       clientConfig := testConfig.Clone()
        clientConfig.DynamicRecordSizingDisabled = dynamicRecordSizingDisabled
 
        buf := make([]byte, bufsize)
@@ -645,7 +645,7 @@ func latency(b *testing.B, bps int, dynamicRecordSizingDisabled bool) {
                                // (cannot call b.Fatal in goroutine)
                                panic(fmt.Errorf("accept: %v", err))
                        }
-                       serverConfig := testConfig.clone()
+                       serverConfig := testConfig.Clone()
                        serverConfig.DynamicRecordSizingDisabled = dynamicRecordSizingDisabled
                        srv := Server(&slowConn{sconn, bps}, serverConfig)
                        if err := srv.Handshake(); err != nil {
@@ -655,7 +655,7 @@ func latency(b *testing.B, bps int, dynamicRecordSizingDisabled bool) {
                }
        }()
 
-       clientConfig := testConfig.clone()
+       clientConfig := testConfig.Clone()
        clientConfig.DynamicRecordSizingDisabled = dynamicRecordSizingDisabled
 
        buf := make([]byte, 16384)

diff --git a/src/net/http/httptest/server.go b/src/net/http/httptest/server.go
index 8608077bd1eb9b49fa9070fefd5484eca188e330..e67b7145bec488aea25ebdbdf44ea0414111667f 100644 (file)
--- a/src/net/http/httptest/server.go
+++ b/src/net/http/httptest/server.go
@@ -114,9 +114,10 @@ func (s *Server) StartTLS() {
        }
 
        existingConfig := s.TLS
-       s.TLS = new(tls.Config)
        if existingConfig != nil {
-               *s.TLS = *existingConfig
+               s.TLS = existingConfig.Clone()
+       } else {
+               s.TLS = new(tls.Config)
        }
        if s.TLS.NextProtos == nil {
                s.TLS.NextProtos = []string{"http/1.1"}

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 65465e25c1cb6628c28b067fe1e5d0dafd20eaf4..44e29c642f1a7c12b228155298d0600cd0241d70 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -2087,30 +2087,7 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config {
        if cfg == nil {
                return &tls.Config{}
        }
-       return &tls.Config{
-               Rand:                        cfg.Rand,
-               Time:                        cfg.Time,
-               Certificates:                cfg.Certificates,
-               NameToCertificate:           cfg.NameToCertificate,
-               GetCertificate:              cfg.GetCertificate,
-               RootCAs:                     cfg.RootCAs,
-               NextProtos:                  cfg.NextProtos,
-               ServerName:                  cfg.ServerName,
-               ClientAuth:                  cfg.ClientAuth,
-               ClientCAs:                   cfg.ClientCAs,
-               InsecureSkipVerify:          cfg.InsecureSkipVerify,
-               CipherSuites:                cfg.CipherSuites,
-               PreferServerCipherSuites:    cfg.PreferServerCipherSuites,
-               SessionTicketsDisabled:      cfg.SessionTicketsDisabled,
-               SessionTicketKey:            cfg.SessionTicketKey,
-               ClientSessionCache:          cfg.ClientSessionCache,
-               MinVersion:                  cfg.MinVersion,
-               MaxVersion:                  cfg.MaxVersion,
-               CurvePreferences:            cfg.CurvePreferences,
-               DynamicRecordSizingDisabled: cfg.DynamicRecordSizingDisabled,
-               Renegotiation:               cfg.Renegotiation,
-               KeyLogWriter:                cfg.KeyLogWriter,
-       }
+       return cfg.Clone()
 }
 
 // cloneTLSClientConfig is like cloneTLSConfig but omits