Commit ea6781bc authored by Mike Danese's avatar Mike Danese Committed by Brad Fitzpatrick

[release-branch.go1.8] crypto/tls: make Config.Clone also clone the GetClientCertificate field

Using GetClientCertificate with the http client is currently completely
broken because inside the transport we clone the tls.Config and pass it
off to the tls.Client. Since tls.Config.Clone() does not pass forward
the GetClientCertificate field, GetClientCertificate is ignored in this
context.

Fixes #19264

Change-Id: Ie214f9f0039ac7c3a2dab8ffd14d30668bdb4c71
Signed-off-by: 's avatarMike Danese <mikedanese@google.com>
Reviewed-on: https://go-review.googlesource.com/37541Reviewed-by: 's avatarFilippo Valsorda <hi@filippo.io>
Reviewed-by: 's avatarAdam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 87649d32)
Reviewed-on: https://go-review.googlesource.com/37946
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: 's avatarTom Bergan <tombergan@google.com>
parent 2327d696
...@@ -563,6 +563,7 @@ func (c *Config) Clone() *Config { ...@@ -563,6 +563,7 @@ func (c *Config) Clone() *Config {
Certificates: c.Certificates, Certificates: c.Certificates,
NameToCertificate: c.NameToCertificate, NameToCertificate: c.NameToCertificate,
GetCertificate: c.GetCertificate, GetCertificate: c.GetCertificate,
GetClientCertificate: c.GetClientCertificate,
GetConfigForClient: c.GetConfigForClient, GetConfigForClient: c.GetConfigForClient,
VerifyPeerCertificate: c.VerifyPeerCertificate, VerifyPeerCertificate: c.VerifyPeerCertificate,
RootCAs: c.RootCAs, RootCAs: c.RootCAs,
......
...@@ -13,13 +13,11 @@ import ( ...@@ -13,13 +13,11 @@ import (
"io" "io"
"io/ioutil" "io/ioutil"
"math" "math"
"math/rand"
"net" "net"
"os" "os"
"reflect" "reflect"
"strings" "strings"
"testing" "testing"
"testing/quick"
"time" "time"
) )
...@@ -568,11 +566,50 @@ func TestConnCloseWrite(t *testing.T) { ...@@ -568,11 +566,50 @@ func TestConnCloseWrite(t *testing.T) {
} }
} }
func TestClone(t *testing.T) { func TestCloneFuncFields(t *testing.T) {
const expectedCount = 5
called := 0
c1 := Config{
Time: func() time.Time {
called |= 1 << 0
return time.Time{}
},
GetCertificate: func(*ClientHelloInfo) (*Certificate, error) {
called |= 1 << 1
return nil, nil
},
GetClientCertificate: func(*CertificateRequestInfo) (*Certificate, error) {
called |= 1 << 2
return nil, nil
},
GetConfigForClient: func(*ClientHelloInfo) (*Config, error) {
called |= 1 << 3
return nil, nil
},
VerifyPeerCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
called |= 1 << 4
return nil
},
}
c2 := c1.Clone()
c2.Time()
c2.GetCertificate(nil)
c2.GetClientCertificate(nil)
c2.GetConfigForClient(nil)
c2.VerifyPeerCertificate(nil, nil)
if called != (1<<expectedCount)-1 {
t.Fatalf("expected %d calls but saw calls %b", expectedCount, called)
}
}
func TestCloneNonFuncFields(t *testing.T) {
var c1 Config var c1 Config
v := reflect.ValueOf(&c1).Elem() v := reflect.ValueOf(&c1).Elem()
rnd := rand.New(rand.NewSource(time.Now().Unix()))
typ := v.Type() typ := v.Type()
for i := 0; i < typ.NumField(); i++ { for i := 0; i < typ.NumField(); i++ {
f := v.Field(i) f := v.Field(i)
...@@ -581,40 +618,49 @@ func TestClone(t *testing.T) { ...@@ -581,40 +618,49 @@ func TestClone(t *testing.T) {
continue continue
} }
// testing/quick can't handle functions or interfaces. // testing/quick can't handle functions or interfaces and so
fn := typ.Field(i).Name // isn't used here.
switch fn { switch fn := typ.Field(i).Name; fn {
case "Rand": case "Rand":
f.Set(reflect.ValueOf(io.Reader(os.Stdin))) f.Set(reflect.ValueOf(io.Reader(os.Stdin)))
continue
case "Time", "GetCertificate", "GetConfigForClient", "VerifyPeerCertificate", "GetClientCertificate": case "Time", "GetCertificate", "GetConfigForClient", "VerifyPeerCertificate", "GetClientCertificate":
// DeepEqual can't compare functions. // DeepEqual can't compare functions. If you add a
continue // function field to this list, you must also change
// TestCloneFuncFields to ensure that the func field is
// cloned.
case "Certificates": case "Certificates":
f.Set(reflect.ValueOf([]Certificate{ f.Set(reflect.ValueOf([]Certificate{
{Certificate: [][]byte{{'b'}}}, {Certificate: [][]byte{{'b'}}},
})) }))
continue
case "NameToCertificate": case "NameToCertificate":
f.Set(reflect.ValueOf(map[string]*Certificate{"a": nil})) f.Set(reflect.ValueOf(map[string]*Certificate{"a": nil}))
continue
case "RootCAs", "ClientCAs": case "RootCAs", "ClientCAs":
f.Set(reflect.ValueOf(x509.NewCertPool())) f.Set(reflect.ValueOf(x509.NewCertPool()))
continue
case "ClientSessionCache": case "ClientSessionCache":
f.Set(reflect.ValueOf(NewLRUClientSessionCache(10))) f.Set(reflect.ValueOf(NewLRUClientSessionCache(10)))
continue
case "KeyLogWriter": case "KeyLogWriter":
f.Set(reflect.ValueOf(io.Writer(os.Stdout))) f.Set(reflect.ValueOf(io.Writer(os.Stdout)))
continue case "NextProtos":
f.Set(reflect.ValueOf([]string{"a", "b"}))
} case "ServerName":
f.Set(reflect.ValueOf("b"))
q, ok := quick.Value(f.Type(), rnd) case "ClientAuth":
if !ok { f.Set(reflect.ValueOf(VerifyClientCertIfGiven))
t.Fatalf("quick.Value failed on field %s", fn) case "InsecureSkipVerify", "SessionTicketsDisabled", "DynamicRecordSizingDisabled", "PreferServerCipherSuites":
f.Set(reflect.ValueOf(true))
case "MinVersion", "MaxVersion":
f.Set(reflect.ValueOf(uint16(VersionTLS12)))
case "SessionTicketKey":
f.Set(reflect.ValueOf([32]byte{}))
case "CipherSuites":
f.Set(reflect.ValueOf([]uint16{1, 2}))
case "CurvePreferences":
f.Set(reflect.ValueOf([]CurveID{CurveP256}))
case "Renegotiation":
f.Set(reflect.ValueOf(RenegotiateOnceAsClient))
default:
t.Errorf("all fields must be accounted for, but saw unknown field %q", fn)
} }
f.Set(q)
} }
c2 := c1.Clone() c2 := c1.Clone()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment