From 6eae03e136f031649683d359a0879f6d6ae5e023 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Thu, 20 Oct 2016 09:48:24 -0700 Subject: [PATCH] net/http: drop custom tls.Config cloning code. Now that we have the Clone method on tls.Config, net/http doesn't need any custom functions to do that any more. Change-Id: Ib60707d37f1a7f9a7d7723045f83e59eceffd026 Reviewed-on: https://go-review.googlesource.com/31595 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/crypto/tls/common.go | 8 +++++-- src/crypto/tls/tls_test.go | 4 ++++ src/net/http/transport.go | 48 ++++---------------------------------- 3 files changed, 14 insertions(+), 46 deletions(-) diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index 7199cd9d71..fc898e202a 100644 --- a/src/crypto/tls/common.go +++ b/src/crypto/tls/common.go @@ -455,9 +455,13 @@ func ticketKeyFromBytes(b [32]byte) (key ticketKey) { return key } -// Clone returns a shallow clone of c. -// Only the exported fields are copied. +// Clone returns a shallow clone of c. It is safe to clone a Config that is +// being used concurrently by a TLS client or server. func (c *Config) Clone() *Config { + // Running serverInit ensures that it's safe to read + // SessionTicketsDisabled. + c.serverInitOnce.Do(c.serverInit) + var sessionTicketKeys []ticketKey c.mutex.RLock() sessionTicketKeys = c.sessionTicketKeys diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go index d2a674d08b..87cfa3f7f1 100644 --- a/src/crypto/tls/tls_test.go +++ b/src/crypto/tls/tls_test.go @@ -508,6 +508,10 @@ func TestClone(t *testing.T) { } c2 := c1.Clone() + // DeepEqual also compares unexported fields, thus c2 needs to have run + // serverInit in order to be DeepEqual to c1. Cloning it and discarding + // the result is sufficient. + c2.Clone() if !reflect.DeepEqual(&c1, c2) { t.Errorf("clone failed to copy a field") diff --git a/src/net/http/transport.go b/src/net/http/transport.go index bce9e34de1..8162f9a998 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -1040,7 +1040,7 @@ func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (*persistCon if cm.targetScheme == "https" && !tlsDial { // Initiate TLS and check remote host name against certificate. - cfg := cloneTLSClientConfig(t.TLSClientConfig) + cfg := cloneTLSConfig(t.TLSClientConfig) if cfg.ServerName == "" { cfg.ServerName = cm.tlsHost() } @@ -2099,17 +2099,9 @@ type fakeLocker struct{} func (fakeLocker) Lock() {} func (fakeLocker) Unlock() {} -// cloneTLSConfig returns a shallow clone of the exported -// fields of cfg, ignoring the unexported sync.Once, which -// contains a mutex and must not be copied. -// -// The cfg must not be in active use by tls.Server, or else -// there can still be a race with tls.Server updating SessionTicketKey -// and our copying it, and also a race with the server setting -// SessionTicketsDisabled=false on failure to set the random -// ticket key. -// -// If cfg is nil, a new zero tls.Config is returned. +// clneTLSConfig returns a shallow clone of cfg, or a new zero tls.Config if +// cfg is nil. This is safe to call even if cfg is in active use by a TLS +// client or server. func cloneTLSConfig(cfg *tls.Config) *tls.Config { if cfg == nil { return &tls.Config{} @@ -2117,38 +2109,6 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config { return cfg.Clone() } -// cloneTLSClientConfig is like cloneTLSConfig but omits -// the fields SessionTicketsDisabled and SessionTicketKey. -// This makes it safe to call cloneTLSClientConfig on a config -// in active use by a server. -func cloneTLSClientConfig(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, - ClientSessionCache: cfg.ClientSessionCache, - MinVersion: cfg.MinVersion, - MaxVersion: cfg.MaxVersion, - CurvePreferences: cfg.CurvePreferences, - DynamicRecordSizingDisabled: cfg.DynamicRecordSizingDisabled, - Renegotiation: cfg.Renegotiation, - KeyLogWriter: cfg.KeyLogWriter, - } -} - type connLRU struct { ll *list.List // list.Element.Value type is of *persistConn m map[*persistConn]*list.Element -- 2.48.1