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>
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 7423726..12a4a26 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -17,7 +17,6 @@
"io"
"math/big"
"net"
- "strconv"
"time"
"./ed25519"
@@ -1678,78 +1677,17 @@
// 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
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 56814d3..7ae441c 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -138,6 +138,7 @@
ecdsaP384Certificate Certificate
ecdsaP521Certificate Certificate
ed25519Certificate Certificate
+ garbageCertificate Certificate
)
var testCerts = []struct {
@@ -236,6 +237,9 @@
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 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 @@
"-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",
+ })
}
}
