Skip to content

Commit

Permalink
crypto/tls: add Config.Clone
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@bradfitz
bradfitz committed Sep 1, 2016
1 parent cd0ba4c commit d24f446
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 71 deletions.
5 changes: 3 additions & 2 deletions src/crypto/tls/common.go
View file
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions src/crypto/tls/conn_test.go
View file
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
}
20 changes: 10 additions & 10 deletions src/crypto/tls/handshake_client_test.go
View file
Expand Up @@ -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}

Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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 {
Expand Down Expand Up @@ -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"},
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down
34 changes: 17 additions & 17 deletions src/crypto/tls/handshake_server_test.go
View file
Expand Up @@ -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
Expand All @@ -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}
Expand All @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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

Expand All @@ -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{
{
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1090,7 +1090,7 @@ func TestClientAuth(t *testing.T) {
defer os.Remove(ecdsaKeyPath)
}

config := testConfig.clone()
config := testConfig.Clone()
config.ClientAuth = RequestClientCert

test := &serverTest{
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/crypto/tls/tls.go
View file
Expand Up @@ -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
}
Expand Down
22 changes: 11 additions & 11 deletions src/crypto/tls/tls_test.go
View file
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down

0 comments on commit d24f446

Please sign in to comment.