crypto/tls: Improve TLS Client Authentication

Fix incorrect marshal/unmarshal of certificateRequest.
Add support for configuring client-auth on the server side.
Fix the certificate selection in the client side.
Update generate_cert.go to new time package

Fixes #2521.

R=krautz, agl, bradfitz
CC=golang-dev, mikkel
https://golang.org/cl/5448093

Author: jeffallen

src/pkg/crypto/tls/common.go

@@ -111,6 +111,18 @@ type ConnectionState struct {
 	VerifiedChains [][]*x509.Certificate
 }
 
+// ClientAuthType declares the policy the server will follow for
+// TLS Client Authentication.
+type ClientAuthType int
+
+const (
+	NoClientCert ClientAuthType = iota
+	RequestClientCert
+	RequireAnyClientCert
+	VerifyClientCertIfGiven
+	RequireAndVerifyClientCert
+)
+
 // A Config structure is used to configure a TLS client or server. After one
 // has been passed to a TLS function it must not be modified.
 type Config struct {
@@ -120,7 +132,7 @@ type Config struct {
 	Rand io.Reader
 
 	// Time returns the current time as the number of seconds since the epoch.
-	// If Time is nil, TLS uses the system time.Seconds.
+	// If Time is nil, TLS uses time.Now.
 	Time func() time.Time
 
 	// Certificates contains one or more certificate chains
@@ -148,11 +160,14 @@ type Config struct {
 	// hosting.
 	ServerName string
 
-	// AuthenticateClient controls whether a server will request a certificate
-	// from the client. It does not require that the client send a
-	// certificate nor does it require that the certificate sent be
-	// anything more than self-signed.
-	AuthenticateClient bool
+	// ClientAuth determines the server's policy for
+	// TLS Client Authentication. The default is NoClientCert.
+	ClientAuth ClientAuthType
+
+	// ClientCAs defines the set of root certificate authorities
+	// that servers use if required to verify a client certificate
+	// by the policy in ClientAuth.
+	ClientCAs *x509.CertPool
 
 	// InsecureSkipVerify controls whether a client verifies the
 	// server's certificate chain and host name.
@@ -259,6 +274,11 @@ type Certificate struct {
 	// OCSPStaple contains an optional OCSP response which will be served
 	// to clients that request it.
 	OCSPStaple []byte
+	// Leaf is the parsed form of the leaf certificate, which may be
+	// initialized using x509.ParseCertificate to reduce per-handshake
+	// processing for TLS clients doing client authentication. If nil, the
+	// leaf certificate will be parsed as needed.
+	Leaf *x509.Certificate
 }
 
 // A TLS record.

src/pkg/crypto/tls/handshake_client.go

@@ -5,12 +5,14 @@
 package tls
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/rsa"
 	"crypto/subtle"
 	"crypto/x509"
 	"errors"
 	"io"
+	"strconv"
 )
 
 func (c *Conn) clientHandshake() error {
@@ -162,10 +164,23 @@ func (c *Conn) clientHandshake() error {
 		}
 	}
 
-	transmitCert := false
+	var certToSend *Certificate
 	certReq, ok := msg.(*certificateRequestMsg)
 	if ok {
-		// We only accept certificates with RSA keys.
+		// 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.
+
+		finishedHash.Write(certReq.marshal())
+
+		// For now, we only know how to sign challenges with RSA
 		rsaAvail := false
 		for _, certType := range certReq.certificateTypes {
 			if certType == certTypeRSASign {
@@ -174,23 +189,41 @@ func (c *Conn) clientHandshake() error {
 			}
 		}
 
-		// For now, only send a certificate back if the server gives us an
-		// empty list of certificateAuthorities.
-		//
-		// 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.
-		if rsaAvail && len(certReq.certificateAuthorities) == 0 {
-			transmitCert = true
-		}
+		// We need to search our list of client certs for one
+		// where SignatureAlgorithm is RSA and the Issuer is in
+		// certReq.certificateAuthorities
+	findCert:
+		for i, cert := range c.config.Certificates {
+			if !rsaAvail {
+				continue
+			}
 
-		finishedHash.Write(certReq.marshal())
+			leaf := cert.Leaf
+			if leaf == nil {
+				if leaf, err = x509.ParseCertificate(cert.Certificate[0]); err != nil {
+					c.sendAlert(alertInternalError)
+					return errors.New("tls: failed to parse client certificate #" + strconv.Itoa(i) + ": " + err.Error())
+				}
+			}
+
+			if leaf.PublicKeyAlgorithm != x509.RSA {
+				continue
+			}
+
+			if len(certReq.certificateAuthorities) == 0 {
+				// they gave us an empty list, so just take the
+				// first RSA cert from c.config.Certificates
+				certToSend = &cert
+				break
+			}
+
+			for _, ca := range certReq.certificateAuthorities {
+				if bytes.Equal(leaf.RawIssuer, ca) {
+					certToSend = &cert
+					break findCert
+				}
+			}
+		}
 
 		msg, err = c.readHandshake()
 		if err != nil {
@@ -204,17 +237,9 @@ func (c *Conn) clientHandshake() error {
 	}
 	finishedHash.Write(shd.marshal())
 
-	var cert *x509.Certificate
-	if transmitCert {
+	if certToSend != nil {
 		certMsg = new(certificateMsg)
-		if len(c.config.Certificates) > 0 {
-			cert, err = x509.ParseCertificate(c.config.Certificates[0].Certificate[0])
-			if err == nil && cert.PublicKeyAlgorithm == x509.RSA {
-				certMsg.certificates = c.config.Certificates[0].Certificate
-			} else {
-				cert = nil
-			}
-		}
+		certMsg.certificates = certToSend.Certificate
 		finishedHash.Write(certMsg.marshal())
 		c.writeRecord(recordTypeHandshake, certMsg.marshal())
 	}
@@ -229,7 +254,7 @@ func (c *Conn) clientHandshake() error {
 		c.writeRecord(recordTypeHandshake, ckx.marshal())
 	}
 
-	if cert != nil {
+	if certToSend != nil {
 		certVerify := new(certificateVerifyMsg)
 		digest := make([]byte, 0, 36)
 		digest = finishedHash.serverMD5.Sum(digest)

src/pkg/crypto/tls/handshake_messages.go

@@ -881,9 +881,11 @@ func (m *certificateRequestMsg) marshal() (x []byte) {
 
 	// See http://tools.ietf.org/html/rfc4346#section-7.4.4
 	length := 1 + len(m.certificateTypes) + 2
+	casLength := 0
 	for _, ca := range m.certificateAuthorities {
-		length += 2 + len(ca)
+		casLength += 2 + len(ca)
 	}
+	length += casLength
 
 	x = make([]byte, 4+length)
 	x[0] = typeCertificateRequest
@@ -895,10 +897,8 @@ func (m *certificateRequestMsg) marshal() (x []byte) {
 
 	copy(x[5:], m.certificateTypes)
 	y := x[5+len(m.certificateTypes):]
-
-	numCA := len(m.certificateAuthorities)
-	y[0] = uint8(numCA >> 8)
-	y[1] = uint8(numCA)
+	y[0] = uint8(casLength >> 8)
+	y[1] = uint8(casLength)
 	y = y[2:]
 	for _, ca := range m.certificateAuthorities {
 		y[0] = uint8(len(ca) >> 8)
@@ -909,7 +909,6 @@ func (m *certificateRequestMsg) marshal() (x []byte) {
 	}
 
 	m.raw = x
-
 	return
 }
 
@@ -937,31 +936,34 @@ func (m *certificateRequestMsg) unmarshal(data []byte) bool {
 	}
 
 	data = data[numCertTypes:]
+
 	if len(data) < 2 {
 		return false
 	}
-
-	numCAs := uint16(data[0])<<16 | uint16(data[1])
+	casLength := uint16(data[0])<<8 | uint16(data[1])
 	data = data[2:]
+	if len(data) < int(casLength) {
+		return false
+	}
+	cas := make([]byte, casLength)
+	copy(cas, data)
+	data = data[casLength:]
 
-	m.certificateAuthorities = make([][]byte, numCAs)
-	for i := uint16(0); i < numCAs; i++ {
-		if len(data) < 2 {
+	m.certificateAuthorities = nil
+	for len(cas) > 0 {
+		if len(cas) < 2 {
 			return false
 		}
-		caLen := uint16(data[0])<<16 | uint16(data[1])
+		caLen := uint16(cas[0])<<8 | uint16(cas[1])
+		cas = cas[2:]
 
-		data = data[2:]
-		if len(data) < int(caLen) {
+		if len(cas) < int(caLen) {
 			return false
 		}
 
-		ca := make([]byte, caLen)
-		copy(ca, data)
-		m.certificateAuthorities[i] = ca
-		data = data[caLen:]
+		m.certificateAuthorities = append(m.certificateAuthorities, cas[:caLen])
+		cas = cas[caLen:]
 	}
-
 	if len(data) > 0 {
 		return false
 	}