From 30b415b0765b465e71262d051b7b16b604a855be Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Mon, 29 Nov 2004 11:28:08 +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 | 7 ++-- crypto/x509v3/v3_purp.c | 50 +++++++++++++--------------- crypto/x509v3/x509v3.h | 1 + 6 files changed, 95 insertions(+), 40 deletions(-) diff --git a/apps/verify.c b/apps/verify.c index f7c85b8dd6..9ff32cb068 100644 --- a/apps/verify.c +++ b/apps/verify.c @@ -348,6 +348,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 ddc3b9b355..57ff33dc19 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 1e66786bd5..5e2cc82c51 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); @@ -285,7 +285,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) } /* We have the chain complete: now we need to check its purpose */ - if (param->purpose > 0) ok = check_chain_purpose(ctx); + ok = check_chain_extensions(ctx); if (!ok) goto end; @@ -381,15 +381,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++) { @@ -404,20 +414,62 @@ static int check_chain_purpose(X509_STORE_CTX *ctx) ok=cb(0,ctx); if (!ok) goto end; } - ret = X509_check_purpose(x, ctx->param->purpose, i); - if ((ret == 0) - || ((ctx->param->flags & X509_V_FLAG_X509_STRICT) - && (ret != 1))) + ret = X509_check_ca(x); + switch(must_be_ca) { - if (i) + case -1: + if ((ctx->param->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->param->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->param->purpose > 0) + { + ret = X509_check_purpose(x, ctx->param->purpose, + must_be_ca > 0); + if ((ret == 0) + || ((ctx->param->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))) @@ -428,6 +480,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 e1bd21b802..5f49c2a8b7 100644 --- a/crypto/x509/x509_vfy.h +++ b/crypto/x509/x509_vfy.h @@ -322,10 +322,11 @@ void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth); #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 -#define X509_V_ERR_INVALID_EXTENSION 37 -#define X509_V_ERR_INVALID_POLICY_EXTENSION 38 -#define X509_V_ERR_NO_EXPLICIT_POLICY 39 +#define X509_V_ERR_INVALID_EXTENSION 38 +#define X509_V_ERR_INVALID_POLICY_EXTENSION 39 +#define X509_V_ERR_NO_EXPLICIT_POLICY 40 /* The application is not happy */ diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c index 67596862ab..d37e79481a 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 677a930f77..a6436289c0 100644 --- a/crypto/x509v3/x509v3.h +++ b/crypto/x509v3/x509v3.h @@ -578,6 +578,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);