diff options
author | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2019-12-24 11:25:15 +0100 |
---|---|---|
committer | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2020-07-01 11:14:54 +0200 |
commit | 0e7b1383e138ce3fa66c5bd0ea4a9cb35487436c (patch) | |
tree | 9ddb274339a9a7da79ba23c1c85af72e57ef6dc8 /crypto | |
parent | d18c7ad66aaaebe10c86127d966f5401bc414d2a (diff) |
Fix issue 1418 by moving check of KU_KEY_CERT_SIGN and weakening check_issued()
Move check that cert signing is allowed from x509v3_cache_extensions() to
where it belongs: internal_verify(), generalize it for proxy cert signing.
Correct and simplify check_issued(), now checking self-issued (not: self-signed).
Add test case to 25-test_verify.t that demonstrates successful fix
Fixes #1418
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10587)
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/x509/v3_purp.c | 3 | ||||
-rw-r--r-- | crypto/x509/x509_vfy.c | 39 |
2 files changed, 18 insertions, 24 deletions
diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index 1c0fba2743..e9f8823ce2 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -521,7 +521,8 @@ int X509v3_cache_extensions(X509 *x, OPENSSL_CTX *libctx, const char *propq) if (!X509_NAME_cmp(X509_get_subject_name(x), X509_get_issuer_name(x))) { x->ex_flags |= EXFLAG_SI; /* cert is self-issued */ if (X509_check_akid(x, x->akid) == X509_V_OK /* SKID matches AKID */ - && !ku_reject(x, KU_KEY_CERT_SIGN)) + /* .. and the signature alg matches the PUBKEY alg: */ + && check_sig_alg_match(X509_get0_pubkey(x), x) == X509_V_OK) x->ex_flags |= EXFLAG_SS; /* indicate self-signed */ } x->altname = X509_get_ext_d2i(x, NID_subject_alt_name, &i, NULL); diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index ba36bafdfc..122d6f8a3b 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -116,7 +116,6 @@ static int null_callback(int ok, X509_STORE_CTX *e) * This does not verify self-signedness but relies on x509v3_cache_extensions() * matching issuer and subject names (i.e., the cert being self-issued) and any * present authority key identifier matching the subject key identifier, etc. - * Moreover the key usage (if present) must allow certificate signing - TODO correct this wrong semantics of x509v3_cache_extensions() */ static int cert_self_signed(X509_STORE_CTX *ctx, X509 *x) { @@ -343,36 +342,26 @@ static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x) return rv; } -/* Given a possible certificate and issuer check them */ - +/* + * Check that the given certificate 'x' is issued by the certificate 'issuer' + * and the issuer is not yet in ctx->chain, where the exceptional case + * that 'x' is self-issued and ctx->chain has just one element is allowed. + */ static int check_issued(X509_STORE_CTX *ctx, X509 *x, X509 *issuer) { - int ret; - - if (x == issuer) - return cert_self_signed(ctx, x) == 1; - ret = x509_check_issued_int(issuer, x, ctx->libctx, ctx->propq); - if (ret == X509_V_OK) { + if (x509_likely_issued(issuer, x, ctx->libctx, ctx->propq) != X509_V_OK) + return 0; + if ((x->ex_flags & EXFLAG_SI) == 0 || sk_X509_num(ctx->chain) != 1) { int i; X509 *ch; - ss = cert_self_signed(ctx, x); - if (ss < 0) - return 0; - - /* Special case: single (likely) self-signed certificate */ - if (ss > 0 && sk_X509_num(ctx->chain) == 1) - return 1; for (i = 0; i < sk_X509_num(ctx->chain); i++) { ch = sk_X509_value(ctx->chain, i); - if (ch == issuer || X509_cmp(ch, issuer) == 0) { - ret = X509_V_ERR_PATH_LOOP; - break; - } + if (ch == issuer || X509_cmp(ch, issuer) == 0) + return 0; } } - - return (ret == X509_V_OK); + return 1; } /* Alternative lookup method: look from a STACK stored in other_ctx */ @@ -1780,13 +1769,17 @@ static int internal_verify(X509_STORE_CTX *ctx) /* * 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 unusable, report the issuer certificate + * 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)) { EVP_PKEY *pkey; int issuer_depth = n + (xi == xs ? 0 : 1); + int ret = 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, issuer_depth, X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY)) |