crypto/tls: fix and expand TestVerifyPeerCertificate and TestGetClientCertificate

TestGetClientCertificate had disabled verification, and was only passing
because it was mistakenly checking for empty verifiedChains.

Change-Id: Iea0ddbdbbdf8ac34b499569820a2e4ce543a69c7
Reviewed-on: https://go-review.googlesource.com/47430
Run-TryBot: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
This commit is contained in:
Filippo Valsorda 2017-07-04 19:46:59 +01:00 committed by Adam Langley
parent dd708a5a20
commit 50729f1a6c

View File

@ -1189,7 +1189,7 @@ func TestVerifyPeerCertificate(t *testing.T) {
// callback should still be called but // callback should still be called but
// validatedChains must be empty. // validatedChains must be empty.
if l := len(validatedChains); l != 0 { if l := len(validatedChains); l != 0 {
return errors.New("got len(validatedChains) = 0, wanted zero") return fmt.Errorf("got len(validatedChains) = %d, wanted zero", l)
} }
*called = true *called = true
return nil return nil
@ -1438,19 +1438,23 @@ func TestTLS11SignatureSchemes(t *testing.T) {
} }
var getClientCertificateTests = []struct { var getClientCertificateTests = []struct {
setup func(*Config) setup func(*Config, *Config)
expectedClientError string expectedClientError string
verify func(*testing.T, int, *ConnectionState) verify func(*testing.T, int, *ConnectionState)
}{ }{
{ {
func(clientConfig *Config) { func(clientConfig, serverConfig *Config) {
// Returning a Certificate with no certificate data // Returning a Certificate with no certificate data
// should result in an empty message being sent to the // should result in an empty message being sent to the
// server. // server.
serverConfig.ClientCAs = nil
clientConfig.GetClientCertificate = func(cri *CertificateRequestInfo) (*Certificate, error) { clientConfig.GetClientCertificate = func(cri *CertificateRequestInfo) (*Certificate, error) {
if len(cri.SignatureSchemes) == 0 { if len(cri.SignatureSchemes) == 0 {
panic("empty SignatureSchemes") panic("empty SignatureSchemes")
} }
if len(cri.AcceptableCAs) != 0 {
panic("AcceptableCAs should have been empty")
}
return new(Certificate), nil return new(Certificate), nil
} }
}, },
@ -1462,7 +1466,7 @@ var getClientCertificateTests = []struct {
}, },
}, },
{ {
func(clientConfig *Config) { func(clientConfig, serverConfig *Config) {
// With TLS 1.1, the SignatureSchemes should be // With TLS 1.1, the SignatureSchemes should be
// synthesised from the supported certificate types. // synthesised from the supported certificate types.
clientConfig.MaxVersion = VersionTLS11 clientConfig.MaxVersion = VersionTLS11
@ -1481,7 +1485,7 @@ var getClientCertificateTests = []struct {
}, },
}, },
{ {
func(clientConfig *Config) { func(clientConfig, serverConfig *Config) {
// Returning an error should abort the handshake with // Returning an error should abort the handshake with
// that error. // that error.
clientConfig.GetClientCertificate = func(cri *CertificateRequestInfo) (*Certificate, error) { clientConfig.GetClientCertificate = func(cri *CertificateRequestInfo) (*Certificate, error) {
@ -1493,14 +1497,21 @@ var getClientCertificateTests = []struct {
}, },
}, },
{ {
func(clientConfig *Config) { func(clientConfig, serverConfig *Config) {
clientConfig.GetClientCertificate = func(cri *CertificateRequestInfo) (*Certificate, error) { clientConfig.GetClientCertificate = func(cri *CertificateRequestInfo) (*Certificate, error) {
return &testConfig.Certificates[0], nil if len(cri.AcceptableCAs) == 0 {
panic("empty AcceptableCAs")
}
cert := &Certificate{
Certificate: [][]byte{testRSACertificate},
PrivateKey: testRSAPrivateKey,
}
return cert, nil
} }
}, },
"", "",
func(t *testing.T, testNum int, cs *ConnectionState) { func(t *testing.T, testNum int, cs *ConnectionState) {
if l := len(cs.VerifiedChains); l != 0 { if len(cs.VerifiedChains) == 0 {
t.Errorf("#%d: expected some verified chains, but found none", testNum) t.Errorf("#%d: expected some verified chains, but found none", testNum)
} }
}, },
@ -1515,13 +1526,15 @@ func TestGetClientCertificate(t *testing.T) {
for i, test := range getClientCertificateTests { for i, test := range getClientCertificateTests {
serverConfig := testConfig.Clone() serverConfig := testConfig.Clone()
serverConfig.ClientAuth = RequestClientCert serverConfig.ClientAuth = VerifyClientCertIfGiven
serverConfig.RootCAs = x509.NewCertPool() serverConfig.RootCAs = x509.NewCertPool()
serverConfig.RootCAs.AddCert(issuer) serverConfig.RootCAs.AddCert(issuer)
serverConfig.ClientCAs = serverConfig.RootCAs
serverConfig.Time = func() time.Time { return time.Unix(1476984729, 0) }
clientConfig := testConfig.Clone() clientConfig := testConfig.Clone()
test.setup(clientConfig) test.setup(clientConfig, serverConfig)
type serverResult struct { type serverResult struct {
cs ConnectionState cs ConnectionState
@ -1553,6 +1566,8 @@ func TestGetClientCertificate(t *testing.T) {
t.Errorf("#%d: client error: %v", i, clientErr) t.Errorf("#%d: client error: %v", i, clientErr)
} else if got := clientErr.Error(); got != test.expectedClientError { } else if got := clientErr.Error(); got != test.expectedClientError {
t.Errorf("#%d: expected client error %q, but got %q", i, test.expectedClientError, got) t.Errorf("#%d: expected client error %q, but got %q", i, test.expectedClientError, got)
} else {
test.verify(t, i, &result.cs)
} }
} else if len(test.expectedClientError) > 0 { } else if len(test.expectedClientError) > 0 {
t.Errorf("#%d: expected client error %q, but got no error", i, test.expectedClientError) t.Errorf("#%d: expected client error %q, but got no error", i, test.expectedClientError)