From 023697870bcd4372a142a606546253d719a81024 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Mon, 23 Dec 2019 17:37:17 +0100 Subject: Refactor (without semantic changes) crypto/x509/{v3_purp.c,x509_vfy.c} This prepares some corrections and improves readability (coding style). Among others, it adds the static function check_sig_alg_match() and the internal functions x509_likely_issued() and x509_signing_allowed(). Reviewed-by: Viktor Dukhovni (Merged from https://github.com/openssl/openssl/pull/10587) --- crypto/x509/v3_purp.c | 71 ++++++++++++++++++++++++++++++++---------------- crypto/x509/x509_local.h | 3 ++ crypto/x509/x509_txt.c | 3 ++ crypto/x509/x509_vfy.c | 55 ++++++++++++++++--------------------- 4 files changed, 77 insertions(+), 55 deletions(-) (limited to 'crypto') diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index 5d9b947a39..1c0fba2743 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -14,6 +14,7 @@ #include #include "crypto/x509.h" #include "internal/tsan_assist.h" +#include "x509_local.h" DEFINE_STACK_OF(GENERAL_NAME) DEFINE_STACK_OF(DIST_POINT) @@ -346,6 +347,21 @@ static int setup_crldp(X509 *x) return 1; } +/* Check that issuer public key algorithm matches subject signature algorithm */ +static int check_sig_alg_match(const EVP_PKEY *pkey, const X509 *subject) +{ + int pkey_nid; + + if (pkey == NULL) + return X509_V_ERR_NO_ISSUER_PUBLIC_KEY; + if (OBJ_find_sigid_algs(OBJ_obj2nid(subject->cert_info.signature.algorithm), + NULL, &pkey_nid) == 0) + return X509_V_ERR_UNSUPPORTED_SIGNATURE_ALGORITHM; + if (EVP_PKEY_type(pkey_nid) != EVP_PKEY_base_id(pkey)) + return X509_V_ERR_SIGNATURE_ALGORITHM_MISMATCH; + return X509_V_OK; +} + #define V1_ROOT (EXFLAG_V1|EXFLAG_SS) #define ku_reject(x, usage) \ (((x)->ex_flags & EXFLAG_KUSAGE) && !((x)->ex_kusage & (usage))) @@ -815,39 +831,47 @@ static int no_check(const X509_PURPOSE *xp, const X509 *x, int ca) * Returns 0 for OK, or positive for reason for mismatch * where reason codes match those for X509_verify_cert(). */ +int x509_check_issued_int(X509 *issuer, X509 *subject, + OPENSSL_CTX *libctx, const char *propq) +{ + int ret; -int x509_check_issued_int(X509 *issuer, X509 *subject, OPENSSL_CTX *libctx, - const char *propq) + if ((ret = x509_likely_issued(issuer, subject, libctx, propq)) != X509_V_OK) + return ret; + return x509_signing_allowed(issuer, subject); +} + +/* do the checks 1., 2., and 3. as described above for X509_check_issued() */ +int x509_likely_issued(X509 *issuer, X509 *subject, + OPENSSL_CTX *libctx, const char *propq) { + int ret; + if (X509_NAME_cmp(X509_get_subject_name(issuer), - X509_get_issuer_name(subject))) + X509_get_issuer_name(subject)) != 0) return X509_V_ERR_SUBJECT_ISSUER_MISMATCH; if (!X509v3_cache_extensions(issuer, libctx, propq) || !X509v3_cache_extensions(subject, libctx, propq)) return X509_V_ERR_UNSPECIFIED; - if (subject->akid) { - int ret = X509_check_akid(issuer, subject->akid); - if (ret != X509_V_OK) - return ret; - } + ret = X509_check_akid(issuer, subject->akid); + if (ret != X509_V_OK) + return ret; /* check if the subject signature alg matches the issuer's PUBKEY alg */ - { - EVP_PKEY *i_pkey = X509_get0_pubkey(issuer); - X509_ALGOR *s_algor = &subject->cert_info.signature; - int s_pknid = NID_undef, s_mdnid = NID_undef; - - if (i_pkey == NULL) - return X509_V_ERR_NO_ISSUER_PUBLIC_KEY; - - if (!OBJ_find_sigid_algs(OBJ_obj2nid(s_algor->algorithm), - &s_mdnid, &s_pknid) - || EVP_PKEY_type(s_pknid) != EVP_PKEY_base_id(i_pkey)) - return X509_V_ERR_SIGNATURE_ALGORITHM_MISMATCH; - } + return check_sig_alg_match(X509_get0_pubkey(issuer), subject); +} +/*- + * Check if certificate I is allowed to issue certificate I + * according to the B field of I if present + * depending on any proxyCertInfo extension of I. + * Returns 0 for OK, or positive for reason for rejection + * where reason codes match those for X509_verify_cert(). + */ +int x509_signing_allowed(const X509 *issuer, const X509 *subject) +{ if (subject->ex_flags & EXFLAG_PROXY) { if (ku_reject(issuer, KU_DIGITAL_SIGNATURE)) return X509_V_ERR_KEYUSAGE_NO_DIGITAL_SIGNATURE; @@ -863,8 +887,7 @@ int X509_check_issued(X509 *issuer, X509 *subject) int X509_check_akid(X509 *issuer, AUTHORITY_KEYID *akid) { - - if (!akid) + if (akid == NULL) return X509_V_OK; /* Check key ids (if present) */ @@ -894,7 +917,7 @@ int X509_check_akid(X509 *issuer, AUTHORITY_KEYID *akid) break; } } - if (nm && X509_NAME_cmp(nm, X509_get_issuer_name(issuer))) + if (nm != NULL && X509_NAME_cmp(nm, X509_get_issuer_name(issuer)) != 0) return X509_V_ERR_AKID_ISSUER_SERIAL_MISMATCH; } return X509_V_OK; diff --git a/crypto/x509/x509_local.h b/crypto/x509/x509_local.h index e174ae7611..a1fe4203b9 100644 --- a/crypto/x509/x509_local.h +++ b/crypto/x509/x509_local.h @@ -149,3 +149,6 @@ DEFINE_STACK_OF(STACK_OF_X509_NAME_ENTRY) void x509_set_signature_info(X509_SIG_INFO *siginf, const X509_ALGOR *alg, const ASN1_STRING *sig); +int x509_likely_issued(X509 *issuer, X509 *subject, + OPENSSL_CTX *libctx, const char *propq); +int x509_signing_allowed(const X509 *issuer, const X509 *subject); diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c index 6ce8a722cc..63d8d95f3f 100644 --- a/crypto/x509/x509_txt.c +++ b/crypto/x509/x509_txt.c @@ -178,6 +178,9 @@ const char *X509_verify_cert_error_string(long n) return "subject signature algorithm and issuer public key algorithm mismatch"; case X509_V_ERR_NO_ISSUER_PUBLIC_KEY: return "issuer certificate doesn't have a public key"; + case X509_V_ERR_UNSUPPORTED_SIGNATURE_ALGORITHM: + return "Cannot find certificate signature algorithm"; + default: /* Printing an error number into a static buffer is not thread-safe */ return "unknown certificate verification error"; diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index ef149a2e28..a7541d8572 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -344,15 +344,9 @@ static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x) static int check_issued(X509_STORE_CTX *ctx, X509 *x, X509 *issuer) { int ret; - int ss; - - if (x == issuer) { - ss = cert_self_signed(ctx, x); - if (ss < 0) - return 0; - return ss; - } + 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) { int i; @@ -367,7 +361,7 @@ static int check_issued(X509_STORE_CTX *ctx, X509 *x, X509 *issuer) 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)) { + if (ch == issuer || X509_cmp(ch, issuer) == 0) { ret = X509_V_ERR_PATH_LOOP; break; } @@ -1779,8 +1773,6 @@ static int internal_verify(X509_STORE_CTX *ctx) * is allowed to reset errors (at its own peril). */ while (n >= 0) { - EVP_PKEY *pkey; - /* * Skip signature check for self-signed certificates unless explicitly * asked for because it does not add any security and just wastes time. @@ -1788,9 +1780,12 @@ static int internal_verify(X509_STORE_CTX *ctx) * 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); + 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)) + if (!verify_cb_cert(ctx, xi, issuer_depth, + X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY)) return 0; } else if (X509_verify_ex(xs, pkey, ctx->libctx, ctx->propq) <= 0) { if (!verify_cb_cert(ctx, xs, n, @@ -2962,7 +2957,7 @@ static int build_chain(X509_STORE_CTX *ctx) SSL_DANE *dane = ctx->dane; int num = sk_X509_num(ctx->chain); X509 *cert = sk_X509_value(ctx->chain, num - 1); - int ss; + int self_signed = cert_self_signed(ctx, cert); STACK_OF(X509) *sktmp = NULL; unsigned int search; int may_trusted = 0; @@ -2980,9 +2975,8 @@ static int build_chain(X509_STORE_CTX *ctx) return 0; } - ss = cert_self_signed(ctx, cert); - if (ss < 0) { - X509err(X509_F_BUILD_CHAIN, ERR_R_INTERNAL_ERROR); + self_signed = cert_self_signed(ctx, cert); + if (self_signed < 0) { ctx->error = X509_V_ERR_UNSPECIFIED; return 0; } @@ -3122,7 +3116,7 @@ static int build_chain(X509_STORE_CTX *ctx) * certificate among the ones from the trust store. */ if ((search & S_DOALTERNATE) != 0) { - if (!ossl_assert(num > i && i > 0 && ss == 0)) { + if (!ossl_assert(num > i && i > 0 && !self_signed)) { X509err(X509_F_BUILD_CHAIN, ERR_R_INTERNAL_ERROR); X509_free(xtmp); trust = X509_TRUST_REJECTED; @@ -3150,7 +3144,7 @@ static int build_chain(X509_STORE_CTX *ctx) * Self-signed untrusted certificates get replaced by their * trusted matching issuer. Otherwise, grow the chain. */ - if (ss == 0) { + if (!self_signed) { if (!sk_X509_push(ctx->chain, x = xtmp)) { X509_free(xtmp); X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE); @@ -3159,9 +3153,8 @@ static int build_chain(X509_STORE_CTX *ctx) search = 0; continue; } - ss = cert_self_signed(ctx, x); - if (ss < 0) { - X509err(X509_F_BUILD_CHAIN, ERR_R_INTERNAL_ERROR); + self_signed = cert_self_signed(ctx, x); + if (self_signed < 0) { ctx->error = X509_V_ERR_UNSPECIFIED; return 0; } @@ -3211,7 +3204,7 @@ static int build_chain(X509_STORE_CTX *ctx) search = 0; continue; } - if (ss == 0) + if (!self_signed) continue; } } @@ -3233,7 +3226,7 @@ static int build_chain(X509_STORE_CTX *ctx) /* Search for a trusted issuer of a shorter chain */ search |= S_DOALTERNATE; alt_untrusted = ctx->num_untrusted - 1; - ss = 0; + self_signed = 0; } } @@ -3255,7 +3248,8 @@ static int build_chain(X509_STORE_CTX *ctx) * Once we run out of untrusted issuers, we stop looking for more * and start looking only in the trust store if enabled. */ - xtmp = (ss || depth < num) ? NULL : find_issuer(ctx, sktmp, x); + xtmp = (self_signed || depth < num) ? NULL + : find_issuer(ctx, sktmp, x); if (xtmp == NULL) { search &= ~S_DOUNTRUSTED; if (may_trusted) @@ -3285,11 +3279,10 @@ static int build_chain(X509_STORE_CTX *ctx) x = xtmp; ++ctx->num_untrusted; - ss = cert_self_signed(ctx, xtmp); - if (ss < 0) { - X509err(X509_F_BUILD_CHAIN, ERR_R_INTERNAL_ERROR); - ctx->error = X509_V_ERR_UNSPECIFIED; + self_signed = cert_self_signed(ctx, xtmp); + if (self_signed < 0) { sk_X509_free(sktmp); + ctx->error = X509_V_ERR_UNSPECIFIED; return 0; } @@ -3333,10 +3326,10 @@ static int build_chain(X509_STORE_CTX *ctx) if (DANETLS_ENABLED(dane) && (!DANETLS_HAS_PKIX(dane) || dane->pdpth >= 0)) return verify_cb_cert(ctx, NULL, num-1, X509_V_ERR_DANE_NO_MATCH); - if (ss && sk_X509_num(ctx->chain) == 1) + if (self_signed && sk_X509_num(ctx->chain) == 1) return verify_cb_cert(ctx, NULL, num-1, X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT); - if (ss) + if (self_signed) return verify_cb_cert(ctx, NULL, num-1, X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN); if (ctx->num_untrusted < num) -- cgit v1.2.3