crypto/tls: implement TLS 1.3 client authentication

Note that the SignatureSchemes passed to GetClientCertificate in TLS 1.2
are now filtered by the requested certificate type. This feels like an
improvement anyway, and the full list can be surfaced as well when
support for signature_algorithms_cert is added, which actually matches
the semantics of the CertificateRequest signature_algorithms in TLS 1.2.

Also, note a subtle behavior change in server side resumption: if a
certificate is requested but not required, and the resumed session did
not include one, it used not to invoke VerifyPeerCertificate. However,
if the resumed session did include a certificate, it would. (If a
certificate was required but not in the session, the session is rejected
in checkForResumption.) This inconsistency could be unexpected, even
dangerous, so now VerifyPeerCertificate is always invoked. Still not
consistent with the client behavior, which does not ever invoke
VerifyPeerCertificate on resumption, but it felt too surprising to
entirely change either.

Updates #9671

Change-Id: Ib2b0dbc30e659208dca3ac07d6c687a407d7aaaf
Reviewed-on: https://go-review.googlesource.com/c/147599
Reviewed-by: Adam Langley <agl@golang.org>

1 files changed, 30 insertions, 20 deletions

diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go
index 29b6386e75..17e558cf64 100644
--- a/src/crypto/tls/handshake_client_test.go
+++ b/src/crypto/tls/handshake_client_test.go
@@ -777,6 +777,7 @@ func TestHandshakeClientCertRSA(t *testing.T) {
 
 	runClientTestTLS10(t, test)
 	runClientTestTLS12(t, test)
+	runClientTestTLS13(t, test)
 
 	test = &clientTest{
 		name:    "ClientCert-RSA-AES256-GCM-SHA384",
@@ -802,6 +803,7 @@ func TestHandshakeClientCertECDSA(t *testing.T) {
 
 	runClientTestTLS10(t, test)
 	runClientTestTLS12(t, test)
+	runClientTestTLS13(t, test)
 
 	test = &clientTest{
 		name:    "ClientCert-ECDSA-ECDSA",
@@ -843,6 +845,7 @@ func TestHandshakeClientCertRSAPSS(t *testing.T) {
 	}
 
 	runClientTestTLS12(t, test)
+	runClientTestTLS13(t, test)
 }
 
 func TestHandshakeClientCertRSAPKCS1v15(t *testing.T) {
@@ -917,6 +920,9 @@ func testResumption(t *testing.T, version uint16) {
 		ticketKey := clientConfig.ClientSessionCache.(*lruSessionCache).q.Front().Value.(*lruSessionCacheEntry).sessionKey
 		clientConfig.ClientSessionCache.Put(ticketKey, nil)
 	}
+	corruptTicket := func() {
+		clientConfig.ClientSessionCache.(*lruSessionCache).q.Front().Value.(*lruSessionCacheEntry).state.masterSecret[0] ^= 0xff
+	}
 	randomKey := func() [32]byte {
 		var k [32]byte
 		if _, err := io.ReadFull(serverConfig.rand(), k[:]); err != nil {
@@ -978,21 +984,18 @@ func testResumption(t *testing.T, version uint16) {
 	serverConfig.ClientAuth = RequireAndVerifyClientCert
 	clientConfig.Certificates = serverConfig.Certificates
 	testResumeState("InitialHandshake", false)
-	if version != VersionTLS13 {
-		// TODO(filippo): reenable when client authentication is implemented
-		testResumeState("WithClientCertificates", true)
+	testResumeState("WithClientCertificates", true)
+	serverConfig.ClientAuth = NoClientCert
 
-		// Tickets should be removed from the session cache on TLS handshake failure
-		farFuture := func() time.Time { return time.Unix(16725225600, 0) }
-		serverConfig.Time = farFuture
-		_, _, err = testHandshake(t, clientConfig, serverConfig)
-		if err == nil {
-			t.Fatalf("handshake did not fail after client certificate expiry")
-		}
-		serverConfig.Time = nil
-		testResumeState("AfterHandshakeFailure", false)
-		serverConfig.ClientAuth = NoClientCert
+	// Tickets should be removed from the session cache on TLS handshake
+	// failure, and the client should recover from a corrupted PSK
+	testResumeState("FetchTicketToCorrupt", false)
+	corruptTicket()
+	_, _, err = testHandshake(t, clientConfig, serverConfig)
+	if err == nil {
+		t.Fatalf("handshake did not fail with a corrupted client secret")
 	}
+	testResumeState("AfterHandshakeFailure", false)
 
 	clientConfig.ClientSessionCache = nil
 	testResumeState("WithoutSessionCache", false)
@@ -1415,6 +1418,11 @@ func TestServerSelectingUnconfiguredCipherSuite(t *testing.T) {
 }
 
 func TestVerifyPeerCertificate(t *testing.T) {
+	t.Run("TLSv12", func(t *testing.T) { testVerifyPeerCertificate(t, VersionTLS12) })
+	t.Run("TLSv13", func(t *testing.T) { testVerifyPeerCertificate(t, VersionTLS13) })
+}
+
+func testVerifyPeerCertificate(t *testing.T, version uint16) {
 	issuer, err := x509.ParseCertificate(testRSACertificateIssuer)
 	if err != nil {
 		panic(err)
@@ -1548,6 +1556,7 @@ func TestVerifyPeerCertificate(t *testing.T) {
 			config.ClientAuth = RequireAndVerifyClientCert
 			config.ClientCAs = rootCAs
 			config.Time = now
+			config.MaxVersion = version
 			test.configureServer(config, &serverCalled)
 
 			err = Server(s, config).Handshake()
@@ -1559,6 +1568,7 @@ func TestVerifyPeerCertificate(t *testing.T) {
 		config.ServerName = "example.golang"
 		config.RootCAs = rootCAs
 		config.Time = now
+		config.MaxVersion = version
 		test.configureClient(config, &clientCalled)
 		clientErr := Client(c, config).Handshake()
 		c.Close()
@@ -1757,13 +1767,6 @@ func TestHandshakeRace(t *testing.T) {
 	}
 }
 
-func TestTLS11SignatureSchemes(t *testing.T) {
-	expected := tls11SignatureSchemesNumECDSA + tls11SignatureSchemesNumRSA
-	if expected != len(tls11SignatureSchemes) {
-		t.Errorf("expected to find %d TLS 1.1 signature schemes, but found %d", expected, len(tls11SignatureSchemes))
-	}
-}
-
 var getClientCertificateTests = []struct {
 	setup               func(*Config, *Config)
 	expectedClientError string
@@ -1846,6 +1849,11 @@ var getClientCertificateTests = []struct {
 }
 
 func TestGetClientCertificate(t *testing.T) {
+	t.Run("TLSv12", func(t *testing.T) { testGetClientCertificate(t, VersionTLS12) })
+	t.Run("TLSv13", func(t *testing.T) { testGetClientCertificate(t, VersionTLS13) })
+}
+
+func testGetClientCertificate(t *testing.T, version uint16) {
 	issuer, err := x509.ParseCertificate(testRSACertificateIssuer)
 	if err != nil {
 		panic(err)
@@ -1858,8 +1866,10 @@ func TestGetClientCertificate(t *testing.T) {
 		serverConfig.RootCAs.AddCert(issuer)
 		serverConfig.ClientCAs = serverConfig.RootCAs
 		serverConfig.Time = func() time.Time { return time.Unix(1476984729, 0) }
+		serverConfig.MaxVersion = version
 
 		clientConfig := testConfig.Clone()
+		clientConfig.MaxVersion = version
 
 		test.setup(clientConfig, serverConfig)