Skip to content

Commit d24f446

Browse files
bradfitzbradfitz
committed
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>
1 parent cd0ba4c commit d24f446

File tree

8 files changed

+50
-71
lines changed

8 files changed

+50
-71
lines changed

‎src/crypto/tls/common.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,9 @@ func ticketKeyFromBytes(b [32]byte) (key ticketKey) {
430430
return key
431431
}
432432

433-
// clone returns a copy of c. Only the exported fields are copied.
434-
func (c *Config) clone() *Config {
433+
// Clone returns a shallow clone of c.
434+
// Only the exported fields are copied.
435+
func (c *Config) Clone() *Config {
435436
return &Config{
436437
Rand: c.Rand,
437438
Time: c.Time,

‎src/crypto/tls/conn_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func TestCertificateSelection(t *testing.T) {
124124
func runDynamicRecordSizingTest(t *testing.T, config *Config) {
125125
clientConn, serverConn := net.Pipe()
126126

127-
serverConfig := config.clone()
127+
serverConfig := config.Clone()
128128
serverConfig.DynamicRecordSizingDisabled = false
129129
tlsConn := Server(serverConn, serverConfig)
130130

@@ -225,19 +225,19 @@ func runDynamicRecordSizingTest(t *testing.T, config *Config) {
225225
}
226226

227227
func TestDynamicRecordSizingWithStreamCipher(t *testing.T) {
228-
config := testConfig.clone()
228+
config := testConfig.Clone()
229229
config.CipherSuites = []uint16{TLS_RSA_WITH_RC4_128_SHA}
230230
runDynamicRecordSizingTest(t, config)
231231
}
232232

233233
func TestDynamicRecordSizingWithCBC(t *testing.T) {
234-
config := testConfig.clone()
234+
config := testConfig.Clone()
235235
config.CipherSuites = []uint16{TLS_RSA_WITH_AES_256_CBC_SHA}
236236
runDynamicRecordSizingTest(t, config)
237237
}
238238

239239
func TestDynamicRecordSizingWithAEAD(t *testing.T) {
240-
config := testConfig.clone()
240+
config := testConfig.Clone()
241241
config.CipherSuites = []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}
242242
runDynamicRecordSizingTest(t, config)
243243
}

‎src/crypto/tls/handshake_client_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ func TestHandshakeClientECDHEECDSAAES128CBCSHA256(t *testing.T) {
535535
}
536536

537537
func TestHandshakeClientCertRSA(t *testing.T) {
538-
config := testConfig.clone()
538+
config := testConfig.Clone()
539539
cert, _ := X509KeyPair([]byte(clientCertificatePEM), []byte(clientKeyPEM))
540540
config.Certificates = []Certificate{cert}
541541

@@ -571,7 +571,7 @@ func TestHandshakeClientCertRSA(t *testing.T) {
571571
}
572572

573573
func TestHandshakeClientCertECDSA(t *testing.T) {
574-
config := testConfig.clone()
574+
config := testConfig.Clone()
575575
cert, _ := X509KeyPair([]byte(clientECDSACertificatePEM), []byte(clientECDSAKeyPEM))
576576
config.Certificates = []Certificate{cert}
577577

@@ -728,7 +728,7 @@ func TestLRUClientSessionCache(t *testing.T) {
728728
}
729729

730730
func TestHandshakeClientKeyLog(t *testing.T) {
731-
config := testConfig.clone()
731+
config := testConfig.Clone()
732732
buf := &bytes.Buffer{}
733733
config.KeyLogWriter = buf
734734

@@ -769,7 +769,7 @@ func TestHandshakeClientKeyLog(t *testing.T) {
769769
}
770770

771771
func TestHandshakeClientALPNMatch(t *testing.T) {
772-
config := testConfig.clone()
772+
config := testConfig.Clone()
773773
config.NextProtos = []string{"proto2", "proto1"}
774774

775775
test := &clientTest{
@@ -790,7 +790,7 @@ func TestHandshakeClientALPNMatch(t *testing.T) {
790790
}
791791

792792
func TestHandshakeClientALPNNoMatch(t *testing.T) {
793-
config := testConfig.clone()
793+
config := testConfig.Clone()
794794
config.NextProtos = []string{"proto3"}
795795

796796
test := &clientTest{
@@ -814,7 +814,7 @@ func TestHandshakeClientALPNNoMatch(t *testing.T) {
814814
const sctsBase64 = "ABIBaQFnAHUApLkJkLQYWBSHuxOizGdwCjw1mAT5G9+443fNDsgN3BAAAAFHl5nuFgAABAMARjBEAiAcS4JdlW5nW9sElUv2zvQyPoZ6ejKrGGB03gjaBZFMLwIgc1Qbbn+hsH0RvObzhS+XZhr3iuQQJY8S9G85D9KeGPAAdgBo9pj4H2SCvjqM7rkoHUz8cVFdZ5PURNEKZ6y7T0/7xAAAAUeX4bVwAAAEAwBHMEUCIDIhFDgG2HIuADBkGuLobU5a4dlCHoJLliWJ1SYT05z6AiEAjxIoZFFPRNWMGGIjskOTMwXzQ1Wh2e7NxXE1kd1J0QsAdgDuS723dc5guuFCaR+r4Z5mow9+X7By2IMAxHuJeqj9ywAAAUhcZIqHAAAEAwBHMEUCICmJ1rBT09LpkbzxtUC+Hi7nXLR0J+2PmwLp+sJMuqK+AiEAr0NkUnEVKVhAkccIFpYDqHOlZaBsuEhWWrYpg2RtKp0="
815815

816816
func TestHandshakClientSCTs(t *testing.T) {
817-
config := testConfig.clone()
817+
config := testConfig.Clone()
818818

819819
scts, err := base64.StdEncoding.DecodeString(sctsBase64)
820820
if err != nil {
@@ -849,7 +849,7 @@ func TestHandshakClientSCTs(t *testing.T) {
849849
}
850850

851851
func TestRenegotiationRejected(t *testing.T) {
852-
config := testConfig.clone()
852+
config := testConfig.Clone()
853853
test := &clientTest{
854854
name: "RenegotiationRejected",
855855
command: []string{"openssl", "s_server", "-state"},
@@ -871,7 +871,7 @@ func TestRenegotiationRejected(t *testing.T) {
871871
}
872872

873873
func TestRenegotiateOnce(t *testing.T) {
874-
config := testConfig.clone()
874+
config := testConfig.Clone()
875875
config.Renegotiation = RenegotiateOnceAsClient
876876

877877
test := &clientTest{
@@ -885,7 +885,7 @@ func TestRenegotiateOnce(t *testing.T) {
885885
}
886886

887887
func TestRenegotiateTwice(t *testing.T) {
888-
config := testConfig.clone()
888+
config := testConfig.Clone()
889889
config.Renegotiation = RenegotiateFreelyAsClient
890890

891891
test := &clientTest{
@@ -899,7 +899,7 @@ func TestRenegotiateTwice(t *testing.T) {
899899
}
900900

901901
func TestRenegotiateTwiceRejected(t *testing.T) {
902-
config := testConfig.clone()
902+
config := testConfig.Clone()
903903
config.Renegotiation = RenegotiateOnceAsClient
904904

905905
test := &clientTest{

‎src/crypto/tls/handshake_server_test.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func TestNoRC4ByDefault(t *testing.T) {
130130
cipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
131131
compressionMethods: []uint8{compressionNone},
132132
}
133-
serverConfig := testConfig.clone()
133+
serverConfig := testConfig.Clone()
134134
// Reset the enabled cipher suites to nil in order to test the
135135
// defaults.
136136
serverConfig.CipherSuites = nil
@@ -147,7 +147,7 @@ func TestDontSelectECDSAWithRSAKey(t *testing.T) {
147147
supportedCurves: []CurveID{CurveP256},
148148
supportedPoints: []uint8{pointFormatUncompressed},
149149
}
150-
serverConfig := testConfig.clone()
150+
serverConfig := testConfig.Clone()
151151
serverConfig.CipherSuites = clientHello.cipherSuites
152152
serverConfig.Certificates = make([]Certificate, 1)
153153
serverConfig.Certificates[0].Certificate = [][]byte{testECDSACertificate}
@@ -172,7 +172,7 @@ func TestDontSelectRSAWithECDSAKey(t *testing.T) {
172172
supportedCurves: []CurveID{CurveP256},
173173
supportedPoints: []uint8{pointFormatUncompressed},
174174
}
175-
serverConfig := testConfig.clone()
175+
serverConfig := testConfig.Clone()
176176
serverConfig.CipherSuites = clientHello.cipherSuites
177177
// First test that it *does* work when the server's key is RSA.
178178
testClientHello(t, serverConfig, clientHello)
@@ -265,7 +265,7 @@ func TestTLS12OnlyCipherSuites(t *testing.T) {
265265
reply, clientErr = cli.readHandshake()
266266
c.Close()
267267
}()
268-
config := testConfig.clone()
268+
config := testConfig.Clone()
269269
config.CipherSuites = clientHello.cipherSuites
270270
Server(s, config).Handshake()
271271
s.Close()
@@ -732,7 +732,7 @@ func TestHandshakeServerAES256GCMSHA384(t *testing.T) {
732732
}
733733

734734
func TestHandshakeServerECDHEECDSAAES(t *testing.T) {
735-
config := testConfig.clone()
735+
config := testConfig.Clone()
736736
config.Certificates = make([]Certificate, 1)
737737
config.Certificates[0].Certificate = [][]byte{testECDSACertificate}
738738
config.Certificates[0].PrivateKey = testECDSAPrivateKey
@@ -748,7 +748,7 @@ func TestHandshakeServerECDHEECDSAAES(t *testing.T) {
748748
}
749749

750750
func TestHandshakeServerKeyLog(t *testing.T) {
751-
config := testConfig.clone()
751+
config := testConfig.Clone()
752752
buf := &bytes.Buffer{}
753753
config.KeyLogWriter = buf
754754

@@ -785,7 +785,7 @@ func TestHandshakeServerKeyLog(t *testing.T) {
785785
}
786786

787787
func TestHandshakeServerALPN(t *testing.T) {
788-
config := testConfig.clone()
788+
config := testConfig.Clone()
789789
config.NextProtos = []string{"proto1", "proto2"}
790790

791791
test := &serverTest{
@@ -806,7 +806,7 @@ func TestHandshakeServerALPN(t *testing.T) {
806806
}
807807

808808
func TestHandshakeServerALPNNoMatch(t *testing.T) {
809-
config := testConfig.clone()
809+
config := testConfig.Clone()
810810
config.NextProtos = []string{"proto3"}
811811

812812
test := &serverTest{
@@ -841,7 +841,7 @@ func TestHandshakeServerSNI(t *testing.T) {
841841
// TestHandshakeServerSNICertForName is similar to TestHandshakeServerSNI, but
842842
// tests the dynamic GetCertificate method
843843
func TestHandshakeServerSNIGetCertificate(t *testing.T) {
844-
config := testConfig.clone()
844+
config := testConfig.Clone()
845845

846846
// Replace the NameToCertificate map with a GetCertificate function
847847
nameToCert := config.NameToCertificate
@@ -863,7 +863,7 @@ func TestHandshakeServerSNIGetCertificate(t *testing.T) {
863863
// GetCertificate method doesn't return a cert, we fall back to what's in
864864
// the NameToCertificate map.
865865
func TestHandshakeServerSNIGetCertificateNotFound(t *testing.T) {
866-
config := testConfig.clone()
866+
config := testConfig.Clone()
867867

868868
config.GetCertificate = func(clientHello *ClientHelloInfo) (*Certificate, error) {
869869
return nil, nil
@@ -881,7 +881,7 @@ func TestHandshakeServerSNIGetCertificateNotFound(t *testing.T) {
881881
func TestHandshakeServerSNIGetCertificateError(t *testing.T) {
882882
const errMsg = "TestHandshakeServerSNIGetCertificateError error"
883883

884-
serverConfig := testConfig.clone()
884+
serverConfig := testConfig.Clone()
885885
serverConfig.GetCertificate = func(clientHello *ClientHelloInfo) (*Certificate, error) {
886886
return nil, errors.New(errMsg)
887887
}
@@ -900,7 +900,7 @@ func TestHandshakeServerSNIGetCertificateError(t *testing.T) {
900900
func TestHandshakeServerEmptyCertificates(t *testing.T) {
901901
const errMsg = "TestHandshakeServerEmptyCertificates error"
902902

903-
serverConfig := testConfig.clone()
903+
serverConfig := testConfig.Clone()
904904
serverConfig.GetCertificate = func(clientHello *ClientHelloInfo) (*Certificate, error) {
905905
return nil, errors.New(errMsg)
906906
}
@@ -928,7 +928,7 @@ func TestHandshakeServerEmptyCertificates(t *testing.T) {
928928
// TestCipherSuiteCertPreferance ensures that we select an RSA ciphersuite with
929929
// an RSA certificate and an ECDSA ciphersuite with an ECDSA certificate.
930930
func TestCipherSuiteCertPreferenceECDSA(t *testing.T) {
931-
config := testConfig.clone()
931+
config := testConfig.Clone()
932932
config.CipherSuites = []uint16{TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA}
933933
config.PreferServerCipherSuites = true
934934

@@ -938,7 +938,7 @@ func TestCipherSuiteCertPreferenceECDSA(t *testing.T) {
938938
}
939939
runServerTestTLS12(t, test)
940940

941-
config = testConfig.clone()
941+
config = testConfig.Clone()
942942
config.CipherSuites = []uint16{TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA}
943943
config.Certificates = []Certificate{
944944
{
@@ -977,7 +977,7 @@ func TestResumptionDisabled(t *testing.T) {
977977
sessionFilePath := tempFile("")
978978
defer os.Remove(sessionFilePath)
979979

980-
config := testConfig.clone()
980+
config := testConfig.Clone()
981981

982982
test := &serverTest{
983983
name: "IssueTicketPreDisable",
@@ -1090,7 +1090,7 @@ func TestClientAuth(t *testing.T) {
10901090
defer os.Remove(ecdsaKeyPath)
10911091
}
10921092

1093-
config := testConfig.clone()
1093+
config := testConfig.Clone()
10941094
config.ClientAuth = RequestClientCert
10951095

10961096
test := &serverTest{
@@ -1127,7 +1127,7 @@ func TestSNIGivenOnFailure(t *testing.T) {
11271127
serverName: expectedServerName,
11281128
}
11291129

1130-
serverConfig := testConfig.clone()
1130+
serverConfig := testConfig.Clone()
11311131
// Erase the server's cipher suites to ensure the handshake fails.
11321132
serverConfig.CipherSuites = nil
11331133

‎src/crypto/tls/tls.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func DialWithDialer(dialer *net.Dialer, network, addr string, config *Config) (*
135135
// from the hostname we're connecting to.
136136
if config.ServerName == "" {
137137
// Make a copy to avoid polluting argument or default.
138-
c := config.clone()
138+
c := config.Clone()
139139
c.ServerName = hostname
140140
config = c
141141
}

‎src/crypto/tls/tls_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ func testConnReadNonzeroAndEOF(t *testing.T, delay time.Duration) error {
241241
srvCh <- nil
242242
return
243243
}
244-
serverConfig := testConfig.clone()
244+
serverConfig := testConfig.Clone()
245245
srv := Server(sconn, serverConfig)
246246
if err := srv.Handshake(); err != nil {
247247
serr = fmt.Errorf("handshake: %v", err)
@@ -251,7 +251,7 @@ func testConnReadNonzeroAndEOF(t *testing.T, delay time.Duration) error {
251251
srvCh <- srv
252252
}()
253253

254-
clientConfig := testConfig.clone()
254+
clientConfig := testConfig.Clone()
255255
conn, err := Dial("tcp", ln.Addr().String(), clientConfig)
256256
if err != nil {
257257
t.Fatal(err)
@@ -295,7 +295,7 @@ func TestTLSUniqueMatches(t *testing.T) {
295295
if err != nil {
296296
t.Fatal(err)
297297
}
298-
serverConfig := testConfig.clone()
298+
serverConfig := testConfig.Clone()
299299
srv := Server(sconn, serverConfig)
300300
if err := srv.Handshake(); err != nil {
301301
t.Fatal(err)
@@ -304,7 +304,7 @@ func TestTLSUniqueMatches(t *testing.T) {
304304
}
305305
}()
306306

307-
clientConfig := testConfig.clone()
307+
clientConfig := testConfig.Clone()
308308
clientConfig.ClientSessionCache = NewLRUClientSessionCache(1)
309309
conn, err := Dial("tcp", ln.Addr().String(), clientConfig)
310310
if err != nil {
@@ -394,7 +394,7 @@ func TestConnCloseBreakingWrite(t *testing.T) {
394394
srvCh <- nil
395395
return
396396
}
397-
serverConfig := testConfig.clone()
397+
serverConfig := testConfig.Clone()
398398
srv := Server(sconn, serverConfig)
399399
if err := srv.Handshake(); err != nil {
400400
serr = fmt.Errorf("handshake: %v", err)
@@ -414,7 +414,7 @@ func TestConnCloseBreakingWrite(t *testing.T) {
414414
Conn: cconn,
415415
}
416416

417-
clientConfig := testConfig.clone()
417+
clientConfig := testConfig.Clone()
418418
tconn := Client(conn, clientConfig)
419419
if err := tconn.Handshake(); err != nil {
420420
t.Fatal(err)
@@ -507,7 +507,7 @@ func TestClone(t *testing.T) {
507507
f.Set(q)
508508
}
509509

510-
c2 := c1.clone()
510+
c2 := c1.Clone()
511511

512512
if !reflect.DeepEqual(&c1, c2) {
513513
t.Errorf("clone failed to copy a field")
@@ -555,7 +555,7 @@ func throughput(b *testing.B, totalBytes int64, dynamicRecordSizingDisabled bool
555555
// (cannot call b.Fatal in goroutine)
556556
panic(fmt.Errorf("accept: %v", err))
557557
}
558-
serverConfig := testConfig.clone()
558+
serverConfig := testConfig.Clone()
559559
serverConfig.DynamicRecordSizingDisabled = dynamicRecordSizingDisabled
560560
srv := Server(sconn, serverConfig)
561561
if err := srv.Handshake(); err != nil {
@@ -568,7 +568,7 @@ func throughput(b *testing.B, totalBytes int64, dynamicRecordSizingDisabled bool
568568
}()
569569

570570
b.SetBytes(totalBytes)
571-
clientConfig := testConfig.clone()
571+
clientConfig := testConfig.Clone()
572572
clientConfig.DynamicRecordSizingDisabled = dynamicRecordSizingDisabled
573573

574574
buf := make([]byte, bufsize)
@@ -645,7 +645,7 @@ func latency(b *testing.B, bps int, dynamicRecordSizingDisabled bool) {
645645
// (cannot call b.Fatal in goroutine)
646646
panic(fmt.Errorf("accept: %v", err))
647647
}
648-
serverConfig := testConfig.clone()
648+
serverConfig := testConfig.Clone()
649649
serverConfig.DynamicRecordSizingDisabled = dynamicRecordSizingDisabled
650650
srv := Server(&slowConn{sconn, bps}, serverConfig)
651651
if err := srv.Handshake(); err != nil {
@@ -655,7 +655,7 @@ func latency(b *testing.B, bps int, dynamicRecordSizingDisabled bool) {
655655
}
656656
}()
657657

658-
clientConfig := testConfig.clone()
658+
clientConfig := testConfig.Clone()
659659
clientConfig.DynamicRecordSizingDisabled = dynamicRecordSizingDisabled
660660

661661
buf := make([]byte, 16384)

0 commit comments

Comments
 (0)