From cd52956357f91c9b6e36e7a6340f2b9ba115dae6 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Mon, 29 Nov 2004 11:18:00 +0000 Subject: [PATCH] Make an explicit check during certificate validation to see that the CA setting in each certificate on the chain is correct. As a side- effect always do the following basic checks on extensions, not just when there's an associated purpose to the check: - if there is an unhandled critical extension (unless the user has chosen to ignore this fault) - if the path length has been exceeded (if one is set at all) - that certain extensions fit the associated purpose (if one has been given) --- apps/verify.c | 1 + crypto/x509/x509_txt.c | 2 ++ crypto/x509/x509_vfy.c | 74 +++++++++++++++++++++++++++++++++++------ crypto/x509/x509_vfy.h | 1 + crypto/x509v3/v3_purp.c | 50 +++++++++++++--------------- crypto/x509v3/x509v3.h | 1 + 6 files changed, 92 insertions(+), 37 deletions(-) diff --git a/apps/verify.c b/apps/verify.c index 6a93c018b8..d73280cdd0 100644 --- a/apps/verify.c +++ b/apps/verify.c @@ -354,6 +354,7 @@ static int MS_CALLBACK cb(int ok, X509_STORE_CTX *ctx) if (ctx->error == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) ok=1; /* Continue after extension errors too */ if (ctx->error == X509_V_ERR_INVALID_CA) ok=1; + if (ctx->error == X509_V_ERR_INVALID_NON_CA) ok=1; if (ctx->error == X509_V_ERR_PATH_LENGTH_EXCEEDED) ok=1; if (ctx->error == X509_V_ERR_INVALID_PURPOSE) ok=1; if (ctx->error == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) ok=1; diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c index e31ebc6741..b8f5bc0cd7 100644 --- a/crypto/x509/x509_txt.c +++ b/crypto/x509/x509_txt.c @@ -122,6 +122,8 @@ const char *X509_verify_cert_error_string(long n) return("certificate revoked"); case X509_V_ERR_INVALID_CA: return ("invalid CA certificate"); + case X509_V_ERR_INVALID_NON_CA: + return ("invalid non-CA certificate (has CA markings)"); case X509_V_ERR_PATH_LENGTH_EXCEEDED: return ("path length constraint exceeded"); case X509_V_ERR_INVALID_PURPOSE: diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index e24e10259d..028139bb03 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -73,7 +73,7 @@ static int null_callback(int ok,X509_STORE_CTX *e); static int check_issued(X509_STORE_CTX *ctx, X509 *x, X509 *issuer); static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x); -static int check_chain_purpose(X509_STORE_CTX *ctx); +static int check_chain_extensions(X509_STORE_CTX *ctx); static int check_trust(X509_STORE_CTX *ctx); static int check_revocation(X509_STORE_CTX *ctx); static int check_cert(X509_STORE_CTX *ctx); @@ -281,7 +281,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) } /* We have the chain complete: now we need to check its purpose */ - if (ctx->purpose > 0) ok = check_chain_purpose(ctx); + ok = check_chain_extensions(ctx); if (!ok) goto end; @@ -371,15 +371,25 @@ static int get_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) * with the supplied purpose */ -static int check_chain_purpose(X509_STORE_CTX *ctx) +static int check_chain_extensions(X509_STORE_CTX *ctx) { #ifdef OPENSSL_NO_CHAIN_VERIFY return 1; #else - int i, ok=0; + int i, ok=0, must_be_ca; X509 *x; int (*cb)(); cb=ctx->verify_cb; + + /* must_be_ca can have 1 of 3 values: + -1: we accept both CA and non-CA certificates, to allow direct + use of self-signed certificates (which are marked as CA). + 0: we only accept non-CA certificates. This is currently not + used, but the possibility is present for future extensions. + 1: we only accept CA certificates. This is currently used for + all certificates in the chain except the leaf certificate. + */ + must_be_ca = -1; /* Check all untrusted certificates */ for (i = 0; i < ctx->last_untrusted; i++) { @@ -394,20 +404,62 @@ static int check_chain_purpose(X509_STORE_CTX *ctx) ok=cb(0,ctx); if (!ok) goto end; } - ret = X509_check_purpose(x, ctx->purpose, i); - if ((ret == 0) - || ((ctx->flags & X509_V_FLAG_X509_STRICT) - && (ret != 1))) + ret = X509_check_ca(x); + switch(must_be_ca) { - if (i) + case -1: + if ((ctx->flags & X509_V_FLAG_X509_STRICT) + && (ret != 1) && (ret != 0)) + { + ret = 0; ctx->error = X509_V_ERR_INVALID_CA; + } else - ctx->error = X509_V_ERR_INVALID_PURPOSE; + ret = 1; + break; + case 0: + if (ret != 0) + { + ret = 0; + ctx->error = X509_V_ERR_INVALID_NON_CA; + } + else + ret = 1; + break; + default: + if ((ret == 0) + || ((ctx->flags & X509_V_FLAG_X509_STRICT) + && (ret != 1))) + { + ret = 0; + ctx->error = X509_V_ERR_INVALID_CA; + } + else + ret = 1; + break; + } + if (ret == 0) + { ctx->error_depth = i; ctx->current_cert = x; ok=cb(0,ctx); if (!ok) goto end; } + if (ctx->purpose > 0) + { + ret = X509_check_purpose(x, ctx->purpose, + must_be_ca > 0); + if ((ret == 0) + || ((ctx->flags & X509_V_FLAG_X509_STRICT) + && (ret != 1))) + { + ctx->error = X509_V_ERR_INVALID_PURPOSE; + ctx->error_depth = i; + ctx->current_cert = x; + ok=cb(0,ctx); + if (!ok) goto end; + } + } /* Check pathlen */ if ((i > 1) && (x->ex_pathlen != -1) && (i > (x->ex_pathlen + 1))) @@ -418,6 +470,8 @@ static int check_chain_purpose(X509_STORE_CTX *ctx) ok=cb(0,ctx); if (!ok) goto end; } + /* The next certificate must be a CA */ + must_be_ca = 1; } ok = 1; end: diff --git a/crypto/x509/x509_vfy.h b/crypto/x509/x509_vfy.h index 198495884c..cb725b64d8 100644 --- a/crypto/x509/x509_vfy.h +++ b/crypto/x509/x509_vfy.h @@ -306,6 +306,7 @@ struct x509_store_ctx_st /* X509_STORE_CTX */ #define X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION 34 #define X509_V_ERR_KEYUSAGE_NO_CRL_SIGN 35 #define X509_V_ERR_UNHANDLED_CRITICAL_CRL_EXTENSION 36 +#define X509_V_ERR_INVALID_NON_CA 37 /* The application is not happy */ #define X509_V_ERR_APPLICATION_VERIFICATION 50 diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c index b3d1ae5d1c..2b71b4ae43 100644 --- a/crypto/x509v3/v3_purp.c +++ b/crypto/x509v3/v3_purp.c @@ -63,7 +63,6 @@ static void x509v3_cache_extensions(X509 *x); -static int ca_check(const X509 *x); static int check_ssl_ca(const X509 *x); static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x, int ca); static int check_purpose_ssl_server(const X509_PURPOSE *xp, const X509 *x, int ca); @@ -426,8 +425,14 @@ static void x509v3_cache_extensions(X509 *x) #define ns_reject(x, usage) \ (((x)->ex_flags & EXFLAG_NSCERT) && !((x)->ex_nscert & (usage))) -static int ca_check(const X509 *x) +int X509_check_ca(X509 *x) { + if(!(x->ex_flags & EXFLAG_SET)) { + CRYPTO_w_lock(CRYPTO_LOCK_X509); + x509v3_cache_extensions(x); + CRYPTO_w_unlock(CRYPTO_LOCK_X509); + } + /* keyUsage if present should allow cert signing */ if(ku_reject(x, KU_KEY_CERT_SIGN)) return 0; if(x->ex_flags & EXFLAG_BCONS) { @@ -435,10 +440,17 @@ static int ca_check(const X509 *x) /* If basicConstraints says not a CA then say so */ else return 0; } else { + /* we support V1 roots for... uh, I don't really know why. */ if((x->ex_flags & V1_ROOT) == V1_ROOT) return 3; /* If key usage present it must have certSign so tolerate it */ else if (x->ex_flags & EXFLAG_KUSAGE) return 4; - else return 2; + /* Older certificates could have Netscape-specific CA types */ + else if (x->ex_flags & EXFLAG_NSCERT + && x->ex_nscert & NS_ANY_CA) return 5; + /* 2 means "I don't know...", which is legal for V1 and V2 */ + else if (x->ex_flags & EXFLAG_V1) return 2; + /* can this still be regarded a CA certificate? I doubt it */ + return 0; } } @@ -446,14 +458,10 @@ static int ca_check(const X509 *x) static int check_ssl_ca(const X509 *x) { int ca_ret; - ca_ret = ca_check(x); + ca_ret = X509_check_ca(x); if(!ca_ret) return 0; /* check nsCertType if present */ - if(x->ex_flags & EXFLAG_NSCERT) { - if(x->ex_nscert & NS_SSL_CA) return ca_ret; - return 0; - } - if(ca_ret != 2) return ca_ret; + if(ca_ret != 5 || x->ex_nscert & NS_SSL_CA) return ca_ret; else return 0; } @@ -498,14 +506,10 @@ static int purpose_smime(const X509 *x, int ca) if(xku_reject(x,XKU_SMIME)) return 0; if(ca) { int ca_ret; - ca_ret = ca_check(x); + ca_ret = X509_check_ca(x); if(!ca_ret) return 0; /* check nsCertType if present */ - if(x->ex_flags & EXFLAG_NSCERT) { - if(x->ex_nscert & NS_SMIME_CA) return ca_ret; - return 0; - } - if(ca_ret != 2) return ca_ret; + if(ca_ret != 5 || x->ex_nscert & NS_SMIME_CA) return ca_ret; else return 0; } if(x->ex_flags & EXFLAG_NSCERT) { @@ -539,7 +543,7 @@ static int check_purpose_crl_sign(const X509_PURPOSE *xp, const X509 *x, int ca) { if(ca) { int ca_ret; - if((ca_ret = ca_check(x)) != 2) return ca_ret; + if((ca_ret = X509_check_ca(x)) != 2) return ca_ret; else return 0; } if(ku_reject(x, KU_CRL_SIGN)) return 0; @@ -552,17 +556,9 @@ static int check_purpose_crl_sign(const X509_PURPOSE *xp, const X509 *x, int ca) static int ocsp_helper(const X509_PURPOSE *xp, const X509 *x, int ca) { - /* Must be a valid CA */ - if(ca) { - int ca_ret; - ca_ret = ca_check(x); - if(ca_ret != 2) return ca_ret; - if(x->ex_flags & EXFLAG_NSCERT) { - if(x->ex_nscert & NS_ANY_CA) return ca_ret; - return 0; - } - return 0; - } + /* Must be a valid CA. Should we really support the "I don't know" + value (2)? */ + if(ca) return X509_check_ca(x); /* leaf certificate is checked in OCSP_verify() */ return 1; } diff --git a/crypto/x509v3/x509v3.h b/crypto/x509v3/x509v3.h index fb07a19016..d0f559eae0 100644 --- a/crypto/x509v3/x509v3.h +++ b/crypto/x509v3/x509v3.h @@ -527,6 +527,7 @@ int X509V3_EXT_print_fp(FILE *out, X509_EXTENSION *ext, int flag, int indent); int X509V3_extensions_print(BIO *out, char *title, STACK_OF(X509_EXTENSION) *exts, unsigned long flag, int indent); +int X509_check_ca(X509 *x); int X509_check_purpose(X509 *x, int id, int ca); int X509_supported_extension(X509_EXTENSION *ex); int X509_PURPOSE_set(int *p, int purpose); -- 2.25.1