diff options
author | Viktor Dukhovni <openssl-users@dukhovni.org> | 2016-02-02 04:35:27 -0500 |
---|---|---|
committer | Viktor Dukhovni <openssl-users@dukhovni.org> | 2016-02-05 10:54:11 -0500 |
commit | a3baa171053547488475709c7197592c66e427cf (patch) | |
tree | be6605870d9229a17c93242b5fc4f50d8d433661 /crypto/x509 | |
parent | 093d20a8cb74e64d627fcd03532ba6b3150f1d1f (diff) |
Fix missing ok=0 with locally blacklisted CAs
Also in X509_verify_cert() avoid using "i" not only as a loop
counter, but also as a trust outcome and as an error ordinal.
Finally, make sure that all "goto end" jumps return an error, with
"end" renamed to "err" accordingly.
[ The 1.1.0 version of X509_verify_cert() is major rewrite,
which addresses these issues in a more systemic way. ]
Reviewed-by: Rich Salz <rsalz@openssl.org>
Diffstat (limited to 'crypto/x509')
-rw-r--r-- | crypto/x509/x509_vfy.c | 70 |
1 files changed, 40 insertions, 30 deletions
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 0429767032..4d34dbac93 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -194,6 +194,9 @@ int X509_verify_cert(X509_STORE_CTX *ctx) int num, j, retry; int (*cb) (int xok, X509_STORE_CTX *xctx); STACK_OF(X509) *sktmp = NULL; + int trust = X509_TRUST_UNTRUSTED; + int err; + if (ctx->cert == NULL) { X509err(X509_F_X509_VERIFY_CERT, X509_R_NO_CERT_SET_FOR_US_TO_VERIFY); return -1; @@ -216,7 +219,8 @@ int X509_verify_cert(X509_STORE_CTX *ctx) if (((ctx->chain = sk_X509_new_null()) == NULL) || (!sk_X509_push(ctx->chain, ctx->cert))) { X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE); - goto end; + ok = -1; + goto err; } CRYPTO_add(&ctx->cert->references, 1, CRYPTO_LOCK_X509); ctx->last_untrusted = 1; @@ -225,7 +229,8 @@ int X509_verify_cert(X509_STORE_CTX *ctx) if (ctx->untrusted != NULL && (sktmp = sk_X509_dup(ctx->untrusted)) == NULL) { X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE); - goto end; + ok = -1; + goto err; } num = sk_X509_num(ctx->chain); @@ -249,7 +254,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) if (ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) { ok = ctx->get_issuer(&xtmp, ctx, x); if (ok < 0) - goto end; + goto err; /* * If successful for now free up cert so it will be picked up * again later. @@ -266,7 +271,8 @@ int X509_verify_cert(X509_STORE_CTX *ctx) if (xtmp != NULL) { if (!sk_X509_push(ctx->chain, xtmp)) { X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE); - goto end; + ok = -1; + goto err; } CRYPTO_add(&xtmp->references, 1, CRYPTO_LOCK_X509); (void)sk_X509_delete_ptr(sktmp, xtmp); @@ -314,7 +320,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) bad_chain = 1; ok = cb(0, ctx); if (!ok) - goto end; + goto err; } else { /* * We have a match: replace certificate with store @@ -347,25 +353,26 @@ int X509_verify_cert(X509_STORE_CTX *ctx) ok = ctx->get_issuer(&xtmp, ctx, x); if (ok < 0) - goto end; + goto err; if (ok == 0) break; x = xtmp; if (!sk_X509_push(ctx->chain, x)) { X509_free(xtmp); X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE); - ok = 0; - goto end; + ok = -1; + goto err; } num++; } /* we now have our chain, lets check it... */ - i = check_trust(ctx); + if ((trust = check_trust(ctx)) == X509_TRUST_REJECTED) { + /* Callback already issued */ + ok = 0; + goto err; + } - /* If explicitly rejected error */ - if (i == X509_TRUST_REJECTED) - goto end; /* * If it's not explicitly trusted then check if there is an alternative * chain that could be used. We only do this if we haven't already @@ -373,14 +380,14 @@ int X509_verify_cert(X509_STORE_CTX *ctx) * chain checking */ retry = 0; - if (i != X509_TRUST_TRUSTED + if (trust != X509_TRUST_TRUSTED && !(ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) && !(ctx->param->flags & X509_V_FLAG_NO_ALT_CHAINS)) { while (j-- > 1) { xtmp2 = sk_X509_value(ctx->chain, j - 1); ok = ctx->get_issuer(&xtmp, ctx, xtmp2); if (ok < 0) - goto end; + goto err; /* Check if we found an alternate chain */ if (ok > 0) { /* @@ -410,7 +417,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) * self signed certificate in which case we've indicated an error already * and set bad_chain == 1 */ - if (i != X509_TRUST_TRUSTED && !bad_chain) { + if (trust != X509_TRUST_TRUSTED && !bad_chain) { if ((chain_ss == NULL) || !ctx->check_issued(ctx, x, chain_ss)) { if (ctx->last_untrusted >= num) ctx->error = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY; @@ -431,26 +438,26 @@ int X509_verify_cert(X509_STORE_CTX *ctx) bad_chain = 1; ok = cb(0, ctx); if (!ok) - goto end; + goto err; } /* We have the chain complete: now we need to check its purpose */ ok = check_chain_extensions(ctx); if (!ok) - goto end; + goto err; /* Check name constraints */ ok = check_name_constraints(ctx); if (!ok) - goto end; + goto err; ok = check_id(ctx); if (!ok) - goto end; + goto err; /* We may as well copy down any DSA parameters that are required */ X509_get_pubkey_parameters(NULL, ctx->chain); @@ -462,16 +469,16 @@ int X509_verify_cert(X509_STORE_CTX *ctx) ok = ctx->check_revocation(ctx); if (!ok) - goto end; + goto err; - i = X509_chain_check_suiteb(&ctx->error_depth, NULL, ctx->chain, - ctx->param->flags); - if (i != X509_V_OK) { - ctx->error = i; + err = X509_chain_check_suiteb(&ctx->error_depth, NULL, ctx->chain, + ctx->param->flags); + if (err != X509_V_OK) { + ctx->error = err; ctx->current_cert = sk_X509_value(ctx->chain, ctx->error_depth); ok = cb(0, ctx); if (!ok) - goto end; + goto err; } /* At this point, we have a chain and need to verify it */ @@ -480,25 +487,28 @@ int X509_verify_cert(X509_STORE_CTX *ctx) else ok = internal_verify(ctx); if (!ok) - goto end; + goto err; #ifndef OPENSSL_NO_RFC3779 /* RFC 3779 path validation, now that CRL check has been done */ ok = v3_asid_validate_path(ctx); if (!ok) - goto end; + goto err; ok = v3_addr_validate_path(ctx); if (!ok) - goto end; + goto err; #endif /* If we get this far evaluate policies */ if (!bad_chain && (ctx->param->flags & X509_V_FLAG_POLICY_CHECK)) ok = ctx->check_policy(ctx); if (!ok) - goto end; + goto err; if (0) { - end: + err: + /* Ensure we return an error */ + if (ok > 0) + ok = 0; X509_get_pubkey_parameters(NULL, ctx->chain); } if (sktmp != NULL) |