Test invalid certificates.

The fuzzer should discover this instantly, but it's a sufficiently
important failure case (don't accidentally drop the certificate on the
floor or anything weird like that) that it's probably worth testing.

Change-Id: I684932c2e8a88fcf9b2318bf46980d312c66f6ef
Reviewed-on: https://boringssl-review.googlesource.com/19744
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/test/runner/handshake_client.go

@@ -17,7 +17,6 @@ import (
 	"io"
 	"math/big"
 	"net"
-	"strconv"
 	"time"
 
 	"./ed25519"
@@ -1678,78 +1677,17 @@ func (hs *clientHandshakeState) writeHash(msg []byte, seqno uint16) {
 // certificate, or none if none match. It may return a particular certificate or
 // nil on success, or an error on internal error.
 func selectClientCertificate(c *Conn, certReq *certificateRequestMsg) (*Certificate, error) {
-	// 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.
-
-	var rsaAvail, ecdsaAvail bool
-	if !certReq.hasRequestContext {
-		for _, certType := range certReq.certificateTypes {
-			switch certType {
-			case CertTypeRSASign:
-				rsaAvail = true
-			case CertTypeECDSASign:
-				ecdsaAvail = true
-			}
-		}
+	if len(c.config.Certificates) == 0 {
+		return nil, nil
 	}
 
-	// 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, chain := range c.config.Certificates {
-		if !certReq.hasRequestContext && !rsaAvail && !ecdsaAvail {
-			continue
-		}
-
-		// Ensure the private key supports one of the advertised
-		// signature algorithms.
-		if certReq.hasSignatureAlgorithm {
-			if _, err := selectSignatureAlgorithm(c.vers, chain.PrivateKey, c.config, certReq.signatureAlgorithms); err != nil {
-				continue
-			}
-		}
-
-		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
-			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())
-				}
-			}
-
-			if !certReq.hasRequestContext {
-				switch {
-				case rsaAvail && x509Cert.PublicKeyAlgorithm == x509.RSA:
-				case ecdsaAvail && x509Cert.PublicKeyAlgorithm == x509.ECDSA:
-				case ecdsaAvail && isEd25519Certificate(x509Cert):
-				default:
-					continue findCert
-				}
-			}
-
-			if expected := c.config.Bugs.ExpectCertificateReqNames; expected != nil {
-				if !eqByteSlices(expected, certReq.certificateAuthorities) {
-					return nil, fmt.Errorf("tls: CertificateRequest names differed, got %#v but expected %#v", certReq.certificateAuthorities, expected)
-				}
-			}
-
-			return &chain, nil
-		}
+	// The test is assumed to have configured the certificate it meant to
+	// send.
+	if len(c.config.Certificates) > 1 {
+		return nil, errors.New("tls: multiple certificates configured")
 	}
 
-	return nil, nil
+	return &c.config.Certificates[0], nil
 }
 
 // clientSessionCacheKey returns a key used to cache sessionTickets that could

ssl/test/runner/runner.go

@@ -138,6 +138,7 @@ var (
 	ecdsaP384Certificate Certificate
 	ecdsaP521Certificate Certificate
 	ed25519Certificate   Certificate
+	garbageCertificate   Certificate
 )
 
 var testCerts = []struct {
@@ -236,6 +237,9 @@ func initCertificates() {
 	channelIDBytes = make([]byte, 64)
 	writeIntPadded(channelIDBytes[:32], channelIDKey.X)
 	writeIntPadded(channelIDBytes[32:], channelIDKey.Y)
+
+	garbageCertificate.Certificate = [][]byte{[]byte("GARBAGE")}
+	garbageCertificate.PrivateKey = rsaCertificate.PrivateKey
 }
 
 func getRunnerCertificate(t testCert) Certificate {
@@ -12241,9 +12245,9 @@ func addRecordVersionTests() {
 }
 
 func addCertificateTests() {
-	// Test that a certificate chain with intermediate may be sent and
-	// received as both client and server.
 	for _, ver := range tlsVersions {
+		// Test that a certificate chain with intermediate may be sent
+		// and received as both client and server.
 		testCases = append(testCases, testCase{
 			testType: clientTest,
 			name:     "SendReceiveIntermediate-Client-" + ver.name,
@@ -12279,6 +12283,36 @@ func addCertificateTests() {
 				"-expect-peer-cert-file", path.Join(*resourceDir, rsaChainCertificateFile),
 			},
 		})
+
+		// Test that garbage leaf certificates are properly rejected.
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			name:     "GarbageCertificate-Client-" + ver.name,
+			config: Config{
+				MinVersion:   ver.version,
+				MaxVersion:   ver.version,
+				Certificates: []Certificate{garbageCertificate},
+			},
+			tls13Variant:       ver.tls13Variant,
+			shouldFail:         true,
+			expectedError:      ":CANNOT_PARSE_LEAF_CERT:",
+			expectedLocalError: "remote error: error decoding message",
+		})
+
+		testCases = append(testCases, testCase{
+			testType: serverTest,
+			name:     "GarbageCertificate-Server-" + ver.name,
+			config: Config{
+				MinVersion:   ver.version,
+				MaxVersion:   ver.version,
+				Certificates: []Certificate{garbageCertificate},
+			},
+			tls13Variant:       ver.tls13Variant,
+			flags:              []string{"-require-any-client-certificate"},
+			shouldFail:         true,
+			expectedError:      ":CANNOT_PARSE_LEAF_CERT:",
+			expectedLocalError: "remote error: error decoding message",
+		})
 	}
 }