diff options
author | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2020-07-03 21:19:55 +0200 |
---|---|---|
committer | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2020-07-16 21:48:22 +0200 |
commit | 42bb51e59308b3ebc5cc1c35ff4822fba6b52d79 (patch) | |
tree | d6fcd1454fe15a16edddae32ecc15a50ebeaa26d /crypto/x509/x509_vfy.c | |
parent | e2590c3a162eb118c36b09c2168164283aa099b4 (diff) |
x509_vfy.c: Improve key usage checks in internal_verify() of cert chains
If a presumably self-signed cert is last in chain we verify its signature
only if X509_V_FLAG_CHECK_SS_SIGNATURE is set. Upon this request we do the
signature verification, but not in case it is a (non-conforming) self-issued
CA certificate with a key usage extension that does not include keyCertSign.
Make clear when we must verify the signature of a certificate
and when we must adhere to key usage restrictions of the 'issuing' cert.
Add some comments for making internal_verify() easier to understand.
Update the documentation of X509_V_FLAG_CHECK_SS_SIGNATURE accordingly.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/12357)
Diffstat (limited to 'crypto/x509/x509_vfy.c')
-rw-r--r-- | crypto/x509/x509_vfy.c | 46 |
1 files changed, 35 insertions, 11 deletions
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 87b51e990d..f30c0f8d38 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -1716,6 +1716,7 @@ int x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth) return 1; } +/* verify the issuer signatures and cert times of ctx->chain */ static int internal_verify(X509_STORE_CTX *ctx) { int n = sk_X509_num(ctx->chain) - 1; @@ -1734,7 +1735,7 @@ static int internal_verify(X509_STORE_CTX *ctx) } if (ctx->check_issued(ctx, xi, xi)) - xs = xi; + xs = xi; /* the typical case: last cert in the chain is self-issued */ else { if (ctx->param->flags & X509_V_FLAG_PARTIAL_CHAIN) { xs = xi; @@ -1754,26 +1755,49 @@ static int internal_verify(X509_STORE_CTX *ctx) */ while (n >= 0) { /* - * Skip signature check for self-issued certificates unless explicitly + * For each iteration of this loop: + * n is the subject depth + * xs is the subject cert, for which the signature is to be checked + * xi is the supposed issuer cert containing the public key to use + * Initially xs == xi if the last cert in the chain is self-issued. + * + * Skip signature check for self-signed certificates unless explicitly * asked for because it does not add any security and just wastes time. - * If the issuer's public key is not available or its key usage does - * not support issuing the subject cert, report the issuer certificate - * and its depth (rather than the depth of the subject). */ - if (xs != xi || (ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE)) { + if (xs != xi || ((ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE) + && (xi->ex_flags & EXFLAG_SS) != 0)) { EVP_PKEY *pkey; + /* + * If the issuer's public key is not available or its key usage + * does not support issuing the subject cert, report the issuer + * cert and its depth (rather than n, the depth of the subject). + */ int issuer_depth = n + (xs == xi ? 0 : 1); - int ret = x509_signing_allowed(xi, xs); + /* + * According to https://tools.ietf.org/html/rfc5280#section-6.1.4 + * step (n) we must check any given key usage extension in a CA cert + * when preparing the verification of a certificate issued by it. + * According to https://tools.ietf.org/html/rfc5280#section-4.2.1.3 + * we must not verify a certifiate signature if the key usage of the + * CA certificate that issued the certificate prohibits signing. + * In case the 'issuing' certificate is the last in the chain and is + * not a CA certificate but a 'self-issued' end-entity cert (i.e., + * xs == xi && !(xi->ex_flags & EXFLAG_CA)) RFC 5280 does not apply + * (see https://tools.ietf.org/html/rfc6818#section-2) and thus + * we are free to ignore any key usage restrictions on such certs. + */ + int ret = xs == xi && (xi->ex_flags & EXFLAG_CA) == 0 + ? X509_V_OK : x509_signing_allowed(xi, xs); if (ret != X509_V_OK && !verify_cb_cert(ctx, xi, issuer_depth, ret)) return 0; if ((pkey = X509_get0_pubkey(xi)) == NULL) { - if (!verify_cb_cert(ctx, xi, xi != xs ? n+1 : n, - X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY)) + ret = X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY; + if (!verify_cb_cert(ctx, xi, issuer_depth, ret)) return 0; } else if (X509_verify(xs, pkey) <= 0) { - if (!verify_cb_cert(ctx, xs, n, - X509_V_ERR_CERT_SIGNATURE_FAILURE)) + ret = X509_V_ERR_CERT_SIGNATURE_FAILURE; + if (!verify_cb_cert(ctx, xs, n, ret)) return 0; } } |