From 4d9e33acb23472566ba0ae15d63c5562a0abf7a2 Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni 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 --- 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