Merge cherrypicks of [5704859, 5705300, 5704034, 5704195, 5705082, 5704058, 5704059, 5704932, 5705340, 5705341, 5705342, 5705343, 5705344, 5705361, 5705362, 5705363, 5705364, 5704870, 5704196, 5705083, 5701785, 5701786, 5701787, 5704035, 5705261, 5705281, 5704036, 5704037, 5704038, 5704871, 5704933, 5704872, 5705347, 5705262, 5704934] into pi-qpr2-release
Change-Id: I47efd4c1e1ed383d23fbda98b8f7a672f51d703b
diff --git a/src/crypto/x509/x509_test.cc b/src/crypto/x509/x509_test.cc
index b4cecca..89c20e2 100644
--- a/src/crypto/x509/x509_test.cc
+++ b/src/crypto/x509/x509_test.cc
@@ -422,6 +422,155 @@
"recgVPpVS7B+d9g4EwtZXIh4lodTBDHBBw==\n"
"-----END CERTIFICATE-----\n";
+static const char kSANTypesRoot[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIICTTCCAbagAwIBAgIIAj5CwoHlWuYwDQYJKoZIhvcNAQELBQAwKzEXMBUGA1UE\n"
+ "ChMOQm9yaW5nU1NMIFRlc3QxEDAOBgNVBAMTB1Jvb3QgQ0EwHhcNMTUwMTAxMDAw\n"
+ "MDAwWhcNMjUwMTAxMDAwMDAwWjArMRcwFQYDVQQKEw5Cb3JpbmdTU0wgVGVzdDEQ\n"
+ "MA4GA1UEAxMHUm9vdCBDQTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA6Q5/\n"
+ "EQzmWuaGg3D2UQcuAngR9bIkkjjuJmICx5TxPqF3asCP1SJotl3iTNrghRE1wpJy\n"
+ "SY2BtIiXa7f8skRb2U0GcPkMxo/ps9+jaoRsQ1m+nbLQdpvD1/qZWcO45fNTA71J\n"
+ "1rPMokP+rcILuQG4VimUAySnDSghKamulFtK+Z8CAwEAAaN6MHgwDgYDVR0PAQH/\n"
+ "BAQDAgIEMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAPBgNVHRMBAf8E\n"
+ "BTADAQH/MBkGA1UdDgQSBBBAN9cB+0AvuBx+VAQnjFkBMBsGA1UdIwQUMBKAEEA3\n"
+ "1wH7QC+4HH5UBCeMWQEwDQYJKoZIhvcNAQELBQADgYEAc4N6hTE62/3gwg+kyc2f\n"
+ "c/Jj1mHrOt+0NRaBnmvbmNpsEjHS96Ef4Wt/ZlPXPkkv1C1VosJnOIMF3Q522wRH\n"
+ "bqaxARldS12VAa3gcWisDWD+SqSyDxjyojz0XDiJkTrFuCTCUiZO+1GLB7SO10Ms\n"
+ "d5YVX0c90VMnUhF/dlrqS9U=\n"
+ "-----END CERTIFICATE-----\n";
+
+// The following four certificates were generated with this Go program, varying
+// |includeNetscapeExtension| and defining rootKeyPEM and rootCertPEM to be
+// strings containing the kSANTypesRoot, above.
+
+// package main
+
+// import (
+// "crypto/ecdsa"
+// "crypto/elliptic"
+// "crypto/rand"
+// "crypto/x509"
+// "crypto/x509/pkix"
+// "encoding/asn1"
+// "encoding/pem"
+// "math/big"
+// "os"
+// "time"
+// )
+
+// const includeNetscapeExtension = true
+
+// func main() {
+// block, _ := pem.Decode([]byte(rootKeyPEM))
+// rootPriv, _ := x509.ParsePKCS1PrivateKey(block.Bytes)
+// block, _ = pem.Decode([]byte(rootCertPEM))
+// root, _ := x509.ParseCertificate(block.Bytes)
+
+// interTemplate := &x509.Certificate{
+// SerialNumber: big.NewInt(2),
+// Subject: pkix.Name{
+// CommonName: "No Basic Constraints (Netscape)",
+// },
+// NotBefore: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
+// NotAfter: time.Date(2099, time.January, 1, 0, 0, 0, 0, time.UTC),
+// }
+
+// if includeNetscapeExtension {
+// interTemplate.ExtraExtensions = []pkix.Extension{
+// pkix.Extension{
+// Id: asn1.ObjectIdentifier([]int{2, 16, 840, 1, 113730, 1, 1}),
+// Value: []byte{0x03, 0x02, 2, 0x04},
+// },
+// }
+// } else {
+// interTemplate.KeyUsage = x509.KeyUsageCertSign
+// }
+
+// interKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+
+// interDER, err := x509.CreateCertificate(rand.Reader, interTemplate, root, &interKey.PublicKey, rootPriv)
+// if err != nil {
+// panic(err)
+// }
+
+// pem.Encode(os.Stdout, &pem.Block{Type: "CERTIFICATE", Bytes: interDER})
+
+// inter, _ := x509.ParseCertificate(interDER)
+
+// leafTemplate := &x509.Certificate{
+// SerialNumber: big.NewInt(3),
+// Subject: pkix.Name{
+// CommonName: "Leaf from CA with no Basic Constraints",
+// },
+// NotBefore: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
+// NotAfter: time.Date(2099, time.January, 1, 0, 0, 0, 0, time.UTC),
+// BasicConstraintsValid: true,
+// }
+// leafKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+
+// leafDER, err := x509.CreateCertificate(rand.Reader, leafTemplate, inter, &leafKey.PublicKey, interKey)
+// if err != nil {
+// panic(err)
+// }
+
+// pem.Encode(os.Stdout, &pem.Block{Type: "CERTIFICATE", Bytes: leafDER})
+// }
+
+// kNoBasicConstraintsCertSignIntermediate doesn't have isCA set, but contains
+// certSign in the keyUsage.
+static const char kNoBasicConstraintsCertSignIntermediate[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBqjCCAROgAwIBAgIBAjANBgkqhkiG9w0BAQsFADArMRcwFQYDVQQKEw5Cb3Jp\n"
+ "bmdTU0wgVGVzdDEQMA4GA1UEAxMHUm9vdCBDQTAgFw0wMDAxMDEwMDAwMDBaGA8y\n"
+ "MDk5MDEwMTAwMDAwMFowHzEdMBsGA1UEAxMUTm8gQmFzaWMgQ29uc3RyYWludHMw\n"
+ "WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASEFMblfxIEDO8My7wHtHWTuDzNyID1\n"
+ "OsPkMGkn32O/pSyXxXuAqDeFoMVffUMTyfm8JcYugSEbrv2qEXXM4bZRoy8wLTAO\n"
+ "BgNVHQ8BAf8EBAMCAgQwGwYDVR0jBBQwEoAQQDfXAftAL7gcflQEJ4xZATANBgkq\n"
+ "hkiG9w0BAQsFAAOBgQC1Lh6hIAm3K5kRh5iIydU0YAEm7eV6ZSskERDUq3DLJyl9\n"
+ "ZUZCHUzvb464dkwZjeNzaUVS1pdElJslwX3DtGgeJLJGCnk8zUjBjaNrrDm0kzPW\n"
+ "xKt/6oif1ci/KCKqKNXJAIFbc4e+IiBpenwpxHk3If4NM+Ek0nKoO8Uj0NkgTQ==\n"
+ "-----END CERTIFICATE-----\n";
+
+static const char kNoBasicConstraintsCertSignLeaf[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBUDCB96ADAgECAgEDMAoGCCqGSM49BAMCMB8xHTAbBgNVBAMTFE5vIEJhc2lj\n"
+ "IENvbnN0cmFpbnRzMCAXDTAwMDEwMTAwMDAwMFoYDzIwOTkwMTAxMDAwMDAwWjAx\n"
+ "MS8wLQYDVQQDEyZMZWFmIGZyb20gQ0Egd2l0aCBubyBCYXNpYyBDb25zdHJhaW50\n"
+ "czBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABEsYPMwzdJKjB+2gpC90ib2ilHoB\n"
+ "w/arQ6ikUX0CNUDDaKaOu/jF39ogzVlg4lDFrjCKShSfCCcrwgONv70IZGijEDAO\n"
+ "MAwGA1UdEwEB/wQCMAAwCgYIKoZIzj0EAwIDSAAwRQIgbV7R99yM+okXSIs6Fp3o\n"
+ "eCOXiDL60IBxaTOcLS44ywcCIQDbn87Gj5cFgHBYAkzdHqDsyGXkxQTHDq9jmX24\n"
+ "Djy3Zw==\n"
+ "-----END CERTIFICATE-----\n";
+
+// kNoBasicConstraintsNetscapeCAIntermediate doesn't have isCA set, but contains
+// a Netscape certificate-type extension that asserts a type of "SSL CA".
+static const char kNoBasicConstraintsNetscapeCAIntermediate[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBuDCCASGgAwIBAgIBAjANBgkqhkiG9w0BAQsFADArMRcwFQYDVQQKEw5Cb3Jp\n"
+ "bmdTU0wgVGVzdDEQMA4GA1UEAxMHUm9vdCBDQTAgFw0wMDAxMDEwMDAwMDBaGA8y\n"
+ "MDk5MDEwMTAwMDAwMFowKjEoMCYGA1UEAxMfTm8gQmFzaWMgQ29uc3RyYWludHMg\n"
+ "KE5ldHNjYXBlKTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABCeMbmCaOtMzXBqi\n"
+ "PrCdNOH23CkaawUA+pAezitAN4RXS1O2CGK5sJjGPVVeogROU8G7/b+mU+ciZIzH\n"
+ "1PP8FJKjMjAwMBsGA1UdIwQUMBKAEEA31wH7QC+4HH5UBCeMWQEwEQYJYIZIAYb4\n"
+ "QgEBBAQDAgIEMA0GCSqGSIb3DQEBCwUAA4GBAAgNWjh7cfBTClTAk+Ml//5xb9Ju\n"
+ "tkBhG6Rm+kkMD+qiSMO6t7xS7CsA0+jIBjkdEYaLZ3oxtQCBdZsVNxUvRxZ0AUfF\n"
+ "G3DtRFTsrI1f7IQhpMuqEMF4shPW+5x54hrq0Fo6xMs6XoinJZcTUaaB8EeXRF6M\n"
+ "P9p6HuyLrmn0c/F0\n"
+ "-----END CERTIFICATE-----\n";
+
+static const char kNoBasicConstraintsNetscapeCALeaf[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBXDCCAQKgAwIBAgIBAzAKBggqhkjOPQQDAjAqMSgwJgYDVQQDEx9ObyBCYXNp\n"
+ "YyBDb25zdHJhaW50cyAoTmV0c2NhcGUpMCAXDTAwMDEwMTAwMDAwMFoYDzIwOTkw\n"
+ "MTAxMDAwMDAwWjAxMS8wLQYDVQQDEyZMZWFmIGZyb20gQ0Egd2l0aCBubyBCYXNp\n"
+ "YyBDb25zdHJhaW50czBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABDlJKolDu3R2\n"
+ "tPqSDycr0QJcWhxdBv76V0EEVflcHRxED6vAioTEcnQszt1OfKtBZvjlo0yp6i6Q\n"
+ "DaYit0ZInmWjEDAOMAwGA1UdEwEB/wQCMAAwCgYIKoZIzj0EAwIDSAAwRQIhAJsh\n"
+ "aZL6BHeEfoUBj1oZ2Ln91qzj3UCVMJ+vrmwAFdYyAiA3wp2JphgchvmoUFuzPXwj\n"
+ "XyPwWPbymSTpzKhB4xB7qQ==\n"
+ "-----END CERTIFICATE-----\n";
+
// CertFromPEM parses the given, NUL-terminated pem block and returns an
// |X509*|.
static bssl::UniquePtr<X509> CertFromPEM(const char *pem) {
@@ -1035,3 +1184,35 @@
}
}
}
+
+TEST(X509Test, NoBasicConstraintsCertSign) {
+ bssl::UniquePtr<X509> root(CertFromPEM(kSANTypesRoot));
+ bssl::UniquePtr<X509> intermediate(
+ CertFromPEM(kNoBasicConstraintsCertSignIntermediate));
+ bssl::UniquePtr<X509> leaf(CertFromPEM(kNoBasicConstraintsCertSignLeaf));
+
+ ASSERT_TRUE(root);
+ ASSERT_TRUE(intermediate);
+ ASSERT_TRUE(leaf);
+
+ // The intermediate has keyUsage certSign, but is not marked as a CA in the
+ // basicConstraints.
+ EXPECT_EQ(X509_V_ERR_INVALID_CA,
+ Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, 0));
+}
+
+TEST(X509Test, NoBasicConstraintsNetscapeCA) {
+ bssl::UniquePtr<X509> root(CertFromPEM(kSANTypesRoot));
+ bssl::UniquePtr<X509> intermediate(
+ CertFromPEM(kNoBasicConstraintsNetscapeCAIntermediate));
+ bssl::UniquePtr<X509> leaf(CertFromPEM(kNoBasicConstraintsNetscapeCALeaf));
+
+ ASSERT_TRUE(root);
+ ASSERT_TRUE(intermediate);
+ ASSERT_TRUE(leaf);
+
+ // The intermediate has a Netscape certificate type of "SSL CA", but is not
+ // marked as a CA in the basicConstraints.
+ EXPECT_EQ(X509_V_ERR_INVALID_CA,
+ Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, 0));
+}
diff --git a/src/crypto/x509/x509_vfy.c b/src/crypto/x509/x509_vfy.c
index aff2ee9..f3cda0d 100644
--- a/src/crypto/x509/x509_vfy.c
+++ b/src/crypto/x509/x509_vfy.c
@@ -578,7 +578,7 @@
static int check_chain_extensions(X509_STORE_CTX *ctx)
{
- int i, ok = 0, must_be_ca, plen = 0;
+ int i, ok = 0, plen = 0;
X509 *x;
int (*cb) (int xok, X509_STORE_CTX *xctx);
int proxy_path_length = 0;
@@ -586,15 +586,13 @@
int allow_proxy_certs;
cb = ctx->verify_cb;
- /*
- * must_be_ca can have 1 of 3 values: -1: we accept both CA and non-CA
- * certificates, to allow direct use of self-signed certificates (which
- * are marked as CA). 0: we only accept non-CA certificates. This is
- * currently not used, but the possibility is present for future
- * extensions. 1: we only accept CA certificates. This is currently used
- * for all certificates in the chain except the leaf certificate.
- */
- must_be_ca = -1;
+ enum {
+ // ca_or_leaf allows either type of certificate so that direct use of
+ // self-signed certificates works.
+ ca_or_leaf,
+ must_be_ca,
+ must_not_be_ca,
+ } ca_requirement;
/* CRL path validation */
if (ctx->parent) {
@@ -606,6 +604,8 @@
purpose = ctx->param->purpose;
}
+ ca_requirement = ca_or_leaf;
+
/* Check all untrusted certificates */
for (i = 0; i < ctx->last_untrusted; i++) {
int ret;
@@ -627,33 +627,30 @@
if (!ok)
goto end;
}
- ret = X509_check_ca(x);
- switch (must_be_ca) {
- case -1:
- if ((ctx->param->flags & X509_V_FLAG_X509_STRICT)
- && (ret != 1) && (ret != 0)) {
- ret = 0;
- ctx->error = X509_V_ERR_INVALID_CA;
- } else
- ret = 1;
+
+ switch (ca_requirement) {
+ case ca_or_leaf:
+ ret = 1;
break;
- case 0:
- if (ret != 0) {
+ case must_not_be_ca:
+ if (X509_check_ca(x)) {
ret = 0;
ctx->error = X509_V_ERR_INVALID_NON_CA;
} else
ret = 1;
break;
- default:
- if ((ret == 0)
- || ((ctx->param->flags & X509_V_FLAG_X509_STRICT)
- && (ret != 1))) {
+ case must_be_ca:
+ if (!X509_check_ca(x)) {
ret = 0;
ctx->error = X509_V_ERR_INVALID_CA;
} else
ret = 1;
break;
+ default:
+ // impossible.
+ ret = 0;
}
+
if (ret == 0) {
ctx->error_depth = i;
ctx->current_cert = x;
@@ -662,10 +659,9 @@
goto end;
}
if (ctx->param->purpose > 0) {
- ret = X509_check_purpose(x, purpose, must_be_ca > 0);
- if ((ret == 0)
- || ((ctx->param->flags & X509_V_FLAG_X509_STRICT)
- && (ret != 1))) {
+ ret = X509_check_purpose(x, purpose, ca_requirement == must_be_ca);
+ if (ret != 1) {
+ ret = 0;
ctx->error = X509_V_ERR_INVALID_PURPOSE;
ctx->error_depth = i;
ctx->current_cert = x;
@@ -703,9 +699,10 @@
goto end;
}
proxy_path_length++;
- must_be_ca = 0;
- } else
- must_be_ca = 1;
+ ca_requirement = must_not_be_ca;
+ } else {
+ ca_requirement = must_be_ca;
+ }
}
ok = 1;
end:
diff --git a/src/crypto/x509v3/v3_purp.c b/src/crypto/x509v3/v3_purp.c
index 324de85..0327037 100644
--- a/src/crypto/x509v3/v3_purp.c
+++ b/src/crypto/x509v3/v3_purp.c
@@ -80,7 +80,6 @@
static void x509v3_cache_extensions(X509 *x);
-static int check_ssl_ca(const X509 *x);
static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x,
int ca);
static int check_purpose_ssl_server(const X509_PURPOSE *xp, const X509 *x,
@@ -562,39 +561,20 @@
CRYPTO_MUTEX_unlock_write(&x->lock);
}
-/*
- * CA checks common to all purposes return codes: 0 not a CA 1 is a CA 2
- * basicConstraints absent so "maybe" a CA 3 basicConstraints absent but self
- * signed V1. 4 basicConstraints absent but keyUsage present and keyCertSign
- * asserted.
- */
-
+/* check_ca returns one if |x| should be considered a CA certificate and zero
+ * otherwise. */
static int check_ca(const X509 *x)
{
/* keyUsage if present should allow cert signing */
if (ku_reject(x, KU_KEY_CERT_SIGN))
return 0;
- if (x->ex_flags & EXFLAG_BCONS) {
- if (x->ex_flags & EXFLAG_CA)
- return 1;
- /* If basicConstraints says not a CA then say so */
- else
- return 0;
- } else {
- /* we support V1 roots for... uh, I don't really know why. */
- if ((x->ex_flags & V1_ROOT) == V1_ROOT)
- return 3;
- /*
- * If key usage present it must have certSign so tolerate it
- */
- else if (x->ex_flags & EXFLAG_KUSAGE)
- return 4;
- /* Older certificates could have Netscape-specific CA types */
- else if (x->ex_flags & EXFLAG_NSCERT && x->ex_nscert & NS_ANY_CA)
- return 5;
- /* can this still be regarded a CA certificate? I doubt it */
- return 0;
+ /* Version 1 certificates are considered CAs and don't have extensions. */
+ if ((x->ex_flags & V1_ROOT) == V1_ROOT) {
+ return 1;
}
+ /* Otherwise, it's only a CA if basicConstraints says so. */
+ return ((x->ex_flags & EXFLAG_BCONS) &&
+ (x->ex_flags & EXFLAG_CA));
}
int X509_check_ca(X509 *x)
@@ -603,27 +583,13 @@
return check_ca(x);
}
-/* Check SSL CA: common checks for SSL client and server */
-static int check_ssl_ca(const X509 *x)
-{
- int ca_ret;
- ca_ret = check_ca(x);
- if (!ca_ret)
- return 0;
- /* check nsCertType if present */
- if (ca_ret != 5 || x->ex_nscert & NS_SSL_CA)
- return ca_ret;
- else
- return 0;
-}
-
static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x,
int ca)
{
if (xku_reject(x, XKU_SSL_CLIENT))
return 0;
if (ca)
- return check_ssl_ca(x);
+ return check_ca(x);
/* We need to do digital signatures or key agreement */
if (ku_reject(x, KU_DIGITAL_SIGNATURE | KU_KEY_AGREEMENT))
return 0;
@@ -647,7 +613,7 @@
if (xku_reject(x, XKU_SSL_SERVER | XKU_SGC))
return 0;
if (ca)
- return check_ssl_ca(x);
+ return check_ca(x);
if (ns_reject(x, NS_SSL_SERVER))
return 0;
@@ -671,29 +637,23 @@
return ret;
}
-/* common S/MIME checks */
+/* purpose_smime returns one if |x| is a valid S/MIME leaf (|ca| is zero) or CA
+ * (|ca| is one) certificate, and zero otherwise. */
static int purpose_smime(const X509 *x, int ca)
{
if (xku_reject(x, XKU_SMIME))
return 0;
if (ca) {
- int ca_ret;
- ca_ret = check_ca(x);
- if (!ca_ret)
- return 0;
/* check nsCertType if present */
- if (ca_ret != 5 || x->ex_nscert & NS_SMIME_CA)
- return ca_ret;
- else
- return 0;
+ if ((x->ex_flags & EXFLAG_NSCERT) &&
+ (x->ex_nscert & NS_SMIME_CA) == 0) {
+ return 0;
+ }
+
+ return check_ca(x);
}
if (x->ex_flags & EXFLAG_NSCERT) {
- if (x->ex_nscert & NS_SMIME)
- return 1;
- /* Workaround for some buggy certificates */
- if (x->ex_nscert & NS_SSL_CLIENT)
- return 2;
- return 0;
+ return (x->ex_nscert & NS_SMIME) == NS_SMIME;
}
return 1;
}
@@ -726,11 +686,7 @@
int ca)
{
if (ca) {
- int ca_ret;
- if ((ca_ret = check_ca(x)) != 2)
- return ca_ret;
- else
- return 0;
+ return check_ca(x);
}
if (ku_reject(x, KU_CRL_SIGN))
return 0;
@@ -744,10 +700,6 @@
static int ocsp_helper(const X509_PURPOSE *xp, const X509 *x, int ca)
{
- /*
- * Must be a valid CA. Should we really support the "I don't know" value
- * (2)?
- */
if (ca)
return check_ca(x);
/* leaf certificate is checked in OCSP_verify() */
diff --git a/src/include/openssl/x509_vfy.h b/src/include/openssl/x509_vfy.h
index 208a380..9e0e0e6 100644
--- a/src/include/openssl/x509_vfy.h
+++ b/src/include/openssl/x509_vfy.h
@@ -366,8 +366,8 @@
#define X509_V_FLAG_CRL_CHECK_ALL 0x8
/* Ignore unhandled critical extensions */
#define X509_V_FLAG_IGNORE_CRITICAL 0x10
-/* Disable workarounds for broken certificates */
-#define X509_V_FLAG_X509_STRICT 0x20
+/* Does nothing as its functionality has been enabled by default. */
+#define X509_V_FLAG_X509_STRICT 0x00
/* Enable proxy certificate validation */
#define X509_V_FLAG_ALLOW_PROXY_CERTS 0x40
/* Enable policy checking */
