From 4d9e33acb23472566ba0ae15d63c5562a0abf7a2 Mon Sep 17 00:00:00 2001
From: Viktor Dukhovni <openssl-users@dukhovni.org>
Date: Tue, 29 Mar 2016 19:40:03 -0400
Subject: [PATCH] Require intermediate CAs to have basicConstraints CA:true.

Previously, it was sufficient to have certSign in keyUsage when the
basicConstraints extension was missing.  That is still accepted in
a trust anchor, but is no longer accepted in an intermediate CA.

Reviewed-by: Rich Salz <rsalz@openssl.org>
---
 crypto/x509/x509_vfy.c        |  3 ++-
 test/certs/ca-nonbc.pem       | 18 ++++++++++++++++++
 test/certs/mkcert.sh          | 21 +++++++++++++++++++++
 test/certs/setup.sh           |  1 +
 test/recipes/25-test_verify.t |  8 ++++++--
 5 files changed, 48 insertions(+), 3 deletions(-)
 create mode 100644 test/certs/ca-nonbc.pem

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index afd8299bb4..ffa211badb 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -484,8 +484,9 @@ static int check_chain_extensions(X509_STORE_CTX *ctx)
                 ret = 1;
             break;
         default:
+            /* X509_V_FLAG_X509_STRICT is implicit for intermediate CAs */
             if ((ret == 0)
-                || ((ctx->param->flags & X509_V_FLAG_X509_STRICT)
+                || ((i + 1 < num || ctx->param->flags & X509_V_FLAG_X509_STRICT)
                     && (ret != 1))) {
                 ret = 0;
                 ctx->error = X509_V_ERR_INVALID_CA;
diff --git a/test/certs/ca-nonbc.pem b/test/certs/ca-nonbc.pem
new file mode 100644
index 0000000000..013775b965
--- /dev/null
+++ b/test/certs/ca-nonbc.pem
@@ -0,0 +1,18 @@
+-----BEGIN CERTIFICATE-----
+MIIC6zCCAdOgAwIBAgIBAjANBgkqhkiG9w0BAQsFADASMRAwDgYDVQQDDAdSb290
+IENBMCAXDTE2MDMzMDAwMDE1N1oYDzIxMTYwMzMxMDAwMTU3WjANMQswCQYDVQQD
+DAJDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAJadpD0ASxxfxsvd
+j9IxsogVzMSGLFziaYuE9KejU9+R479RifvwfBANO62sNWJ19X//9G5UjwWmkiOz
+n1k50DkYsBBA3mJzik6wjt/c58lBIlSEgAgpvDU8ht8w3t20JP9+YqXAeugqFj/W
+l9rFQtsvaWSRywjXVlp5fxuEQelNnXcJEKhsKTNExsBUZebo4/J1BWpklWzA9P0l
+YW5INvDAAwcF1nzlEf0Y6Eot03IMNyg2MTE4hehxjdgCSci8GYnFirE/ojXqqpAc
+ZGh7r2dqWgZUD1Dh+bT2vjrUzj8eTH3GdzI+oljt29102JIUaqj3yzRYkah8FLF9
+CLNNsUcCAwEAAaNPME0wHQYDVR0OBBYEFLQRM/HX4l73U54gIhBPhga/H8leMB8G
+A1UdIwQYMBaAFI71Ja8em2uEPXyAmslTnE1y96NSMAsGA1UdDwQEAwIBBjANBgkq
+hkiG9w0BAQsFAAOCAQEAPo7bKKFLbwT3x7dw+OPZMDxwyG1pk5x+5SD7iv45mOzS
+5lZ2ByaOH+jnjTfG6beNmTCbfq6RcHqTvD6LXYex5z9KliIL9Fpwh507uGDXmKDN
+lM0zmbYhXiWGRwP5NkbB/EppbiSk42l5/ky4gmCH/a9kQfiBW+Gwe3aBwRX6v+5p
+BLwH12YrM46DdEL4RHd2H/9rjSaX4X3aaZd9kZsf/yaOU65iQX15cNDfxkKncYQK
+K+xjT2S/NLcwslkPzQLCWeWZVBV4Vd+TEjjZA1tFpu5e1oNlJYvGbqjIuUurpoxv
+IhsVUfWJEf7KjpFy+kgPyijNYRUBFrMspdb6x771RQ==
+-----END CERTIFICATE-----
diff --git a/test/certs/mkcert.sh b/test/certs/mkcert.sh
index 7b892d2fed..99e7d2a342 100755
--- a/test/certs/mkcert.sh
+++ b/test/certs/mkcert.sh
@@ -114,6 +114,27 @@ genca() {
 	    -set_serial 2 -days "${DAYS}"
 }
 
+gen_nonbc_ca() {
+    local cn=$1; shift
+    local key=$1; shift
+    local cert=$1; shift
+    local cakey=$1; shift
+    local cacert=$1; shift
+    local skid="subjectKeyIdentifier = hash"
+    local akid="authorityKeyIdentifier = keyid"
+
+    exts=$(printf "%s\n%s\n%s\n" "$skid" "$akid")
+    exts=$(printf "%s\nkeyUsage = %s\n" "$exts" "keyCertSign, cRLSign")
+    for eku in "$@"
+    do
+        exts=$(printf "%s\nextendedKeyUsage = %s\n" "$exts" "$eku")
+    done
+    csr=$(req "$key" "$cn") || return 1
+    echo "$csr" |
+        cert "$cert" "$exts" -CA "${cacert}.pem" -CAkey "${cakey}.pem" \
+	    -set_serial 2 -days "${DAYS}"
+}
+
 genee() {
     local OPTIND=1
     local purpose=serverAuth
diff --git a/test/certs/setup.sh b/test/certs/setup.sh
index 8cf27eebf5..9606c77bb7 100755
--- a/test/certs/setup.sh
+++ b/test/certs/setup.sh
@@ -74,6 +74,7 @@ openssl x509 -in sroot-cert.pem -trustout \
 #
 ./mkcert.sh genca "CA" ca-key ca-cert root-key root-cert
 ./mkcert.sh genee "CA" ca-key ca-nonca root-key root-cert
+./mkcert.sh gen_nonbc_ca "CA" ca-key ca-nonbc root-key root-cert
 ./mkcert.sh genca "CA" ca-key2 ca-cert2 root-key root-cert
 ./mkcert.sh genca "CA2" ca-key ca-name2 root-key root-cert
 ./mkcert.sh genca "CA" ca-key ca-root2 root-key2 root-cert2
diff --git a/test/recipes/25-test_verify.t b/test/recipes/25-test_verify.t
index c1d222bb80..d4131ccd3e 100644
--- a/test/recipes/25-test_verify.t
+++ b/test/recipes/25-test_verify.t
@@ -19,7 +19,7 @@ sub verify {
     run(app([@args]));
 }
 
-plan tests => 81;
+plan tests => 83;
 
 # Canonical success
 ok(verify("ee-cert", "sslserver", ["root-cert"], ["ca-cert"]),
@@ -104,8 +104,12 @@ ok(!verify("ee-cert", "sslserver", [qw(root-cert root2+clientAuth ca-root2)],
 # CA variants
 ok(!verify("ee-cert", "sslserver", [qw(root-cert)], [qw(ca-nonca)]),
    "fail non-CA untrusted intermediate");
+ok(!verify("ee-cert", "sslserver", [qw(root-cert)], [qw(ca-nonbc)]),
+   "fail non-CA untrusted intermediate");
 ok(!verify("ee-cert", "sslserver", [qw(root-cert ca-nonca)], []),
-   "fail non-CA trusted intermediate");
+   "fail non-CA trust-store intermediate");
+ok(!verify("ee-cert", "sslserver", [qw(root-cert ca-nonbc)], []),
+   "fail non-CA trust-store intermediate");
 ok(!verify("ee-cert", "sslserver", [qw(root-cert nca+serverAuth)], []),
    "fail non-CA server trust intermediate");
 ok(!verify("ee-cert", "sslserver", [qw(root-cert nca+anyEKU)], []),
-- 
2.25.1