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 <[email protected]>

Author: FiloSottile

src/crypto/tls/common.go

@@ -92,6 +92,7 @@ const (
 	extensionSupportedVersions       uint16 = 43
 	extensionCookie                  uint16 = 44
 	extensionPSKModes                uint16 = 45
+	extensionCertificateAuthorities  uint16 = 47
 	extensionSignatureAlgorithmsCert uint16 = 50
 	extensionKeyShare                uint16 = 51
 	extensionNextProtoNeg            uint16 = 13172 // not IANA assigned

src/crypto/tls/conn.go

@@ -57,8 +57,8 @@ type Conn struct {
 	secureRenegotiation bool
 	// ekm is a closure for exporting keying material.
 	ekm func(label string, context []byte, length int) ([]byte, error)
-	// resumptionSecret is the resumption_master_secret for generating or
-	// handling NewSessionTicket messages. nil if config.SessionTicketsDisabled.
+	// resumptionSecret is the resumption_master_secret for handling
+	// NewSessionTicket messages. nil if config.SessionTicketsDisabled.
 	resumptionSecret []byte
 
 	// clientFinishedIsFirst is true if the client sent the first Finished

src/crypto/tls/handshake_client.go

@@ -519,7 +519,8 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 		certRequested = true
 		hs.finishedHash.Write(certReq.marshal())
 
-		if chainToSend, err = hs.getCertificate(certReq); err != nil {
+		cri := certificateRequestInfoFromMsg(certReq)
+		if chainToSend, err = c.getClientCertificate(cri); err != nil {
 			c.sendAlert(alertInternalError)
 			return err
 		}
@@ -863,20 +864,15 @@ func (c *Conn) verifyServerCertificate(certificates [][]byte) error {
 
 // tls11SignatureSchemes contains the signature schemes that we synthesise for
 // a TLS <= 1.1 connection, based on the supported certificate types.
-var tls11SignatureSchemes = []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithP384AndSHA384, ECDSAWithP521AndSHA512, PKCS1WithSHA256, PKCS1WithSHA384, PKCS1WithSHA512, PKCS1WithSHA1}
-
-const (
-	// tls11SignatureSchemesNumECDSA is the number of initial elements of
-	// tls11SignatureSchemes that use ECDSA.
-	tls11SignatureSchemesNumECDSA = 3
-	// tls11SignatureSchemesNumRSA is the number of trailing elements of
-	// tls11SignatureSchemes that use RSA.
-	tls11SignatureSchemesNumRSA = 4
+var (
+	tls11SignatureSchemes      = []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithP384AndSHA384, ECDSAWithP521AndSHA512, PKCS1WithSHA256, PKCS1WithSHA384, PKCS1WithSHA512, PKCS1WithSHA1}
+	tls11SignatureSchemesECDSA = tls11SignatureSchemes[:3]
+	tls11SignatureSchemesRSA   = tls11SignatureSchemes[3:]
 )
 
-func (hs *clientHandshakeState) getCertificate(certReq *certificateRequestMsg) (*Certificate, error) {
-	c := hs.c
-
+// certificateRequestInfoFromMsg generates a CertificateRequestInfo from a TLS
+// <= 1.2 CertificateRequest, making an effort to fill in missing information.
+func certificateRequestInfoFromMsg(certReq *certificateRequestMsg) *CertificateRequestInfo {
 	var rsaAvail, ecdsaAvail bool
 	for _, certType := range certReq.certificateTypes {
 		switch certType {
@@ -887,77 +883,84 @@ func (hs *clientHandshakeState) getCertificate(certReq *certificateRequestMsg) (
 		}
 	}
 
-	if c.config.GetClientCertificate != nil {
-		var signatureSchemes []SignatureScheme
-
-		if !certReq.hasSignatureAlgorithm {
-			// Prior to TLS 1.2, the signature schemes were not
-			// included in the certificate request message. In this
-			// case we use a plausible list based on the acceptable
-			// certificate types.
-			signatureSchemes = tls11SignatureSchemes
-			if !ecdsaAvail {
-				signatureSchemes = signatureSchemes[tls11SignatureSchemesNumECDSA:]
+	cri := &CertificateRequestInfo{
+		AcceptableCAs: certReq.certificateAuthorities,
+	}
+
+	if !certReq.hasSignatureAlgorithm {
+		// Prior to TLS 1.2, the signature schemes were not
+		// included in the certificate request message. In this
+		// case we use a plausible list based on the acceptable
+		// certificate types.
+		switch {
+		case rsaAvail && ecdsaAvail:
+			cri.SignatureSchemes = tls11SignatureSchemes
+		case rsaAvail:
+			cri.SignatureSchemes = tls11SignatureSchemesRSA
+		case ecdsaAvail:
+			cri.SignatureSchemes = tls11SignatureSchemesECDSA
+		}
+		return cri
+	}
+
+	// In TLS 1.2, the signature schemes apply to both the certificate chain and
+	// the leaf key, while the certificate types only apply to the leaf key.
+	// See RFC 5246, Section 7.4.4 (where it calls this "somewhat complicated").
+	// Filter the signature schemes based on the certificate type.
+	cri.SignatureSchemes = make([]SignatureScheme, 0, len(certReq.supportedSignatureAlgorithms))
+	for _, sigScheme := range certReq.supportedSignatureAlgorithms {
+		switch signatureFromSignatureScheme(sigScheme) {
+		case signatureECDSA:
+			if ecdsaAvail {
+				cri.SignatureSchemes = append(cri.SignatureSchemes, sigScheme)
 			}
-			if !rsaAvail {
-				signatureSchemes = signatureSchemes[:len(signatureSchemes)-tls11SignatureSchemesNumRSA]
+		case signatureRSAPSS, signaturePKCS1v15:
+			if rsaAvail {
+				cri.SignatureSchemes = append(cri.SignatureSchemes, sigScheme)
 			}
-		} else {
-			signatureSchemes = certReq.supportedSignatureAlgorithms
 		}
-
-		return c.config.GetClientCertificate(&CertificateRequestInfo{
-			AcceptableCAs:    certReq.certificateAuthorities,
-			SignatureSchemes: signatureSchemes,
-		})
 	}
 
-	// RFC 4346 on the certificateAuthorities field: A list of the
-	// distinguished names of acceptable certificate authorities.
-	// These distinguished names may specify a desired
-	// distinguished name for a root CA or for a subordinate CA;
-	// thus, this message can be used to describe both known roots
-	// and a desired authorization space. If the
-	// certificate_authorities list is empty then the client MAY
-	// send any certificate of the appropriate
-	// ClientCertificateType, unless there is some external
-	// arrangement to the contrary.
+	return cri
+}
+
+func (c *Conn) getClientCertificate(cri *CertificateRequestInfo) (*Certificate, error) {
+	if c.config.GetClientCertificate != nil {
+		return c.config.GetClientCertificate(cri)
+	}
 
 	// We need to search our list of client certs for one
 	// where SignatureAlgorithm is acceptable to the server and the
-	// Issuer is in certReq.certificateAuthorities
-findCert:
+	// Issuer is in AcceptableCAs.
 	for i, chain := range c.config.Certificates {
-		if !rsaAvail && !ecdsaAvail {
+		sigOK := false
+		for _, alg := range signatureSchemesForCertificate(&chain) {
+			if isSupportedSignatureAlgorithm(alg, cri.SignatureSchemes) {
+				sigOK = true
+				break
+			}
+		}
+		if !sigOK {
 			continue
 		}
 
+		if len(cri.AcceptableCAs) == 0 {
+			return &chain, nil
+		}
+
 		for j, cert := range chain.Certificate {
 			x509Cert := chain.Leaf
-			// parse the certificate if this isn't the leaf
-			// node, or if chain.Leaf was nil
+			// Parse the certificate if this isn't the leaf node, or if
+			// chain.Leaf was nil.
 			if j != 0 || x509Cert == nil {
 				var err error
 				if x509Cert, err = x509.ParseCertificate(cert); err != nil {
 					c.sendAlert(alertInternalError)
-					return nil, errors.New("tls: failed to parse client certificate #" + strconv.Itoa(i) + ": " + err.Error())
+					return nil, errors.New("tls: failed to parse configured certificate chain #" + strconv.Itoa(i) + ": " + err.Error())
 				}
 			}
 
-			switch {
-			case rsaAvail && x509Cert.PublicKeyAlgorithm == x509.RSA:
-			case ecdsaAvail && x509Cert.PublicKeyAlgorithm == x509.ECDSA:
-			default:
-				continue findCert
-			}
-
-			if len(certReq.certificateAuthorities) == 0 {
-				// they gave us an empty list, so just take the
-				// first cert from c.config.Certificates
-				return &chain, nil
-			}
-
-			for _, ca := range certReq.certificateAuthorities {
+			for _, ca := range cri.AcceptableCAs {
 				if bytes.Equal(x509Cert.RawIssuer, ca) {
 					return &chain, nil
 				}

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)
-
-		// 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
+	testResumeState("WithClientCertificates", true)
+	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)