From 9d84d4ed5e13713c060c5fd538e2c15242aa9b9f Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Wed, 13 Aug 2008 16:00:11 +0000 Subject: [PATCH] Initial support for CRL path validation. This supports distinct certificate and CRL signing keys. --- CHANGES | 9 ++++ apps/apps.c | 2 + crypto/x509/x509_vfy.c | 116 ++++++++++++++++++++++++++++++++++------- crypto/x509/x509_vfy.h | 4 ++ 4 files changed, 112 insertions(+), 19 deletions(-) diff --git a/CHANGES b/CHANGES index cede85628b..89dc2bada1 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,15 @@ Changes between 0.9.8i and 0.9.9 [xx XXX xxxx] + *) Add support for distinct certificate and CRL paths. The CRL issuer + certificate is validated separately in this case. Only enabled if + an extended CRL support flag is set: this flag will enable additional + CRL functionality in future. + + This work was sponsored by Google. + [Steve Henson] + + *) Add support for policy mappings extension. This work was sponsored by Google. diff --git a/apps/apps.c b/apps/apps.c index 1ef1b14ed6..7a1b6f7ca4 100644 --- a/apps/apps.c +++ b/apps/apps.c @@ -2239,6 +2239,8 @@ int args_verify(char ***pargs, int *pargc, flags |= X509_V_FLAG_INHIBIT_MAP; else if (!strcmp(arg, "-x509_strict")) flags |= X509_V_FLAG_X509_STRICT; + else if (!strcmp(arg, "-extended_crl")) + flags |= X509_V_FLAG_EXTENDED_CRL_SUPPORT; else if (!strcmp(arg, "-policy_print")) flags |= X509_V_FLAG_NOTIFY_POLICY; else diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index f33d16bba9..7435781947 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -81,6 +81,10 @@ static int check_cert(X509_STORE_CTX *ctx); static int check_policy(X509_STORE_CTX *ctx); static int crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer); static int idp_check_scope(X509 *x, X509_CRL *crl); +static int check_crl_path(X509_STORE_CTX *ctx, X509 *x); +static int check_crl_chain(X509_STORE_CTX *ctx, + STACK_OF(X509) *cert_path, + STACK_OF(X509) *crl_path); static int internal_verify(X509_STORE_CTX *ctx); const char X509_version[]="X.509" OPENSSL_VERSION_PTEXT; @@ -407,8 +411,8 @@ static int check_chain_extensions(X509_STORE_CTX *ctx) X509 *x; int (*cb)(int xok,X509_STORE_CTX *xctx); int proxy_path_length = 0; - int allow_proxy_certs = - !!(ctx->param->flags & X509_V_FLAG_ALLOW_PROXY_CERTS); + int purpose; + int allow_proxy_certs; cb=ctx->verify_cb; /* must_be_ca can have 1 of 3 values: @@ -421,10 +425,22 @@ static int check_chain_extensions(X509_STORE_CTX *ctx) */ must_be_ca = -1; - /* A hack to keep people who don't want to modify their software - happy */ - if (getenv("OPENSSL_ALLOW_PROXY_CERTS")) - allow_proxy_certs = 1; + /* CRL path validation */ + if (ctx->parent) + { + allow_proxy_certs = 0; + purpose = X509_PURPOSE_CRL_SIGN; + } + else + { + allow_proxy_certs = + !!(ctx->param->flags & X509_V_FLAG_ALLOW_PROXY_CERTS); + /* A hack to keep people who don't want to modify their + software happy */ + if (getenv("OPENSSL_ALLOW_PROXY_CERTS")) + allow_proxy_certs = 1; + purpose = ctx->param->purpose; + } /* Check all untrusted certificates */ for (i = 0; i < ctx->last_untrusted; i++) @@ -491,8 +507,7 @@ static int check_chain_extensions(X509_STORE_CTX *ctx) } if (ctx->param->purpose > 0) { - ret = X509_check_purpose(x, ctx->param->purpose, - must_be_ca > 0); + ret = X509_check_purpose(x, purpose, must_be_ca > 0); if ((ret == 0) || ((ctx->param->flags & X509_V_FLAG_X509_STRICT) && (ret != 1))) @@ -795,9 +810,7 @@ static int crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer) { X509 *crl_issuer; int cidx = ctx->error_depth; -#if 0 int i; -#endif if (!crl->akid) return 1; if (cidx != sk_X509_num(ctx->chain) - 1) @@ -824,15 +837,15 @@ static int crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer) } + /* Anything else needs extended CRL support */ + + if (!(ctx->param->flags & X509_V_FLAG_EXTENDED_CRL_SUPPORT)) + return 0; + /* Otherwise the CRL issuer is not on the path. Look for it in the * set of untrusted certificates. */ -#if 0 - /* FIXME: not enabled yet because the CRL issuer certifcate is not - * validated. - */ - for (i = 0; i < sk_X509_num(ctx->untrusted); i++) { crl_issuer = sk_X509_value(ctx->untrusted, i); @@ -841,15 +854,76 @@ static int crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer) continue; if (X509_check_akid(crl_issuer, crl->akid) == X509_V_OK) { - *pissuer = crl_issuer; - return 1; + if (check_crl_path(ctx, crl_issuer)) + { + *pissuer = crl_issuer; + return 1; + } } } -#endif return 0; } +/* Check the path of a CRL issuer certificate. This creates a new + * X509_STORE_CTX and populates it with most of the parameters from the + * parent. This could be optimised somewhat since a lot of path checking + * will be duplicated by the parent, but this will rarely be used in + * practice. + */ + +static int check_crl_path(X509_STORE_CTX *ctx, X509 *x) + { + X509_STORE_CTX crl_ctx; + int ret; + if (ctx->parent) + return 0; + if (!X509_STORE_CTX_init(&crl_ctx, ctx->ctx, x, ctx->untrusted)) + return -1; + + crl_ctx.crls = ctx->crls; + /* Copy verify params across */ + X509_STORE_CTX_set0_param(&crl_ctx, ctx->param); + + crl_ctx.parent = ctx; + crl_ctx.verify_cb = ctx->verify_cb; + + /* Verify CRL issuer */ + ret = X509_verify_cert(&crl_ctx); + + /* Maybe send path check result back to parent? */ + if (!ret) + goto err; + + /* Check chain is acceptable */ + + ret = check_crl_chain(ctx, ctx->chain, crl_ctx.chain); + + err: + X509_STORE_CTX_cleanup(&crl_ctx); + return ret; + } + +/* RFC3280 says nothing about the relationship between CRL path + * and certificate path, which could lead to situations where a + * certificate could be revoked or validated by a CA not authorised + * to do so. RFC5280 is more strict and states that the two paths must + * end in the same trust anchor, though some discussions remain... + * until this is resolved we use the RFC5280 version + */ + +static int check_crl_chain(X509_STORE_CTX *ctx, + STACK_OF(X509) *cert_path, + STACK_OF(X509) *crl_path) + { + X509 *cert_ta, *crl_ta; + cert_ta = sk_X509_value(cert_path, sk_X509_num(cert_path) - 1); + crl_ta = sk_X509_value(crl_path, sk_X509_num(crl_path) - 1); + if (!X509_cmp(cert_ta, crl_ta)) + return 1; + return 0; + } + /* Check for match between two dist point names: three separate cases. * 1. Both are relative names and compare X509_NAME types. * 2. One full, one relative. Compare X509_NAME to GENERAL_NAMES. @@ -1127,6 +1201,8 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x) static int check_policy(X509_STORE_CTX *ctx) { int ret; + if (ctx->parent) + return 1; ret = X509_policy_check(&ctx->tree, &ctx->explicit_policy, ctx->chain, ctx->param->policies, ctx->param->flags); if (ret == 0) @@ -1635,6 +1711,7 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509, ctx->current_cert=NULL; ctx->current_issuer=NULL; ctx->tree = NULL; + ctx->parent = NULL; ctx->param = X509_VERIFY_PARAM_new(); @@ -1754,7 +1831,8 @@ void X509_STORE_CTX_cleanup(X509_STORE_CTX *ctx) if (ctx->cleanup) ctx->cleanup(ctx); if (ctx->param != NULL) { - X509_VERIFY_PARAM_free(ctx->param); + if (ctx->parent == NULL) + X509_VERIFY_PARAM_free(ctx->param); ctx->param=NULL; } if (ctx->tree != NULL) diff --git a/crypto/x509/x509_vfy.h b/crypto/x509/x509_vfy.h index faf641f037..a5006c2d8b 100644 --- a/crypto/x509/x509_vfy.h +++ b/crypto/x509/x509_vfy.h @@ -269,6 +269,8 @@ struct x509_store_ctx_st /* X509_STORE_CTX */ X509 *current_issuer; /* cert currently being tested as valid issuer */ X509_CRL *current_crl; /* current CRL */ + X509_STORE_CTX *parent; /* For CRL path validation: parent context */ + CRYPTO_EX_DATA ex_data; } /* X509_STORE_CTX */; @@ -377,6 +379,8 @@ void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth); #define X509_V_FLAG_INHIBIT_MAP 0x400 /* Notify callback that policy is OK */ #define X509_V_FLAG_NOTIFY_POLICY 0x800 +/* Extended CRL features such as indirect CRLs, alternate CRL signing keys */ +#define X509_V_FLAG_EXTENDED_CRL_SUPPORT 0x1000 #define X509_VP_FLAG_DEFAULT 0x1 #define X509_VP_FLAG_OVERWRITE 0x2