x509_vfy.c: Fix a regression in find_issuer()

...in case the candidate issuer cert is identical to the target cert.

This is the v3.0.0 variant of #13749 fixing #13739 for v1.1.1.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13762)
This commit is contained in:
Dr. David von Oheimb 2020-12-28 11:25:59 +01:00 committed by Dr. David von Oheimb
parent 0cbb3602f5
commit 4dd009180a
2 changed files with 31 additions and 46 deletions

View File

@ -316,9 +316,10 @@ static int sk_X509_contains(STACK_OF(X509) *sk, X509 *cert)
}
/*
* Find in given STACK_OF(X509) sk a non-expired issuer cert (if any) of given cert x.
* The issuer must not be the same as x and must not yet be in ctx->chain, where the
* exceptional case x is self-issued and ctx->chain has just one element is allowed.
* Find in given STACK_OF(X509) sk an issuer cert of given cert x.
* The issuer must not yet be in ctx->chain, where the exceptional case
* that x is self-issued and ctx->chain has just one element is allowed.
* Prefer the first one that is not expired, else take the last expired one.
*/
static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x)
{
@ -327,16 +328,12 @@ static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x)
for (i = 0; i < sk_X509_num(sk); i++) {
issuer = sk_X509_value(sk, i);
/*
* Below check 'issuer != x' is an optimization and safety precaution:
* Candidate issuer cert cannot be the same as the subject cert 'x'.
*/
if (issuer != x && ctx->check_issued(ctx, x, issuer)
if (ctx->check_issued(ctx, x, issuer)
&& (((x->ex_flags & EXFLAG_SI) != 0 && sk_X509_num(ctx->chain) == 1)
|| !sk_X509_contains(ctx->chain, issuer))) {
if (x509_check_cert_time(ctx, issuer, -1))
return issuer;
rv = issuer;
if (x509_check_cert_time(ctx, rv, -1))
break;
}
}
return rv;

View File

@ -24,7 +24,7 @@ static const char *req_f;
#define load_cert_from_file(file) load_cert_pem(file, NULL)
/*
/*-
* Test for CVE-2015-1793 (Alternate Chains Certificate Forgery)
*
* Chain is as follows:
@ -99,37 +99,6 @@ static int test_alt_chains_cert_forgery(void)
return ret;
}
static int test_store_ctx(void)
{
X509_STORE_CTX *sctx = NULL;
X509 *x = NULL;
int testresult = 0, ret;
x = load_cert_from_file(bad_f);
if (x == NULL)
goto err;
sctx = X509_STORE_CTX_new();
if (sctx == NULL)
goto err;
if (!X509_STORE_CTX_init(sctx, NULL, x, NULL))
goto err;
/* Verifying a cert where we have no trusted certs should fail */
ret = X509_verify_cert(sctx);
if (ret == 0) {
/* This is the result we were expecting: Test passed */
testresult = 1;
}
err:
X509_STORE_CTX_free(sctx);
X509_free(x);
return testresult;
}
OPT_TEST_DECLARE_USAGE("roots.pem untrusted.pem bad.pem\n")
static int test_distinguishing_id(void)
@ -206,30 +175,49 @@ static int test_req_distinguishing_id(void)
return ret;
}
static int test_self_signed(const char *filename, int expected)
static int test_self_signed(const char *filename, int use_trusted, int expected)
{
X509 *cert;
STACK_OF(X509) *trusted = sk_X509_new_null();
X509_STORE_CTX *ctx = X509_STORE_CTX_new();
int ret;
cert = load_cert_from_file(filename); /* may result in NULL */
ret = TEST_int_eq(X509_self_signed(cert, 1), expected);
if (cert != NULL) {
if (use_trusted)
ret = ret && TEST_true(sk_X509_push(trusted, cert));
ret = ret && TEST_true(X509_STORE_CTX_init(ctx, NULL, cert, NULL));
X509_STORE_CTX_set0_trusted_stack(ctx, trusted);
ret = ret && TEST_int_eq(X509_verify_cert(ctx), expected);
}
X509_STORE_CTX_free(ctx);
sk_X509_free(trusted);
X509_free(cert);
return ret;
}
static int test_self_signed_good(void)
{
return test_self_signed(root_f, 1);
return test_self_signed(root_f, 1, 1);
}
static int test_self_signed_bad(void)
{
return test_self_signed(bad_f, 0);
return test_self_signed(bad_f, 1, 0);
}
static int test_self_signed_error(void)
{
return test_self_signed("nonexistent file name", -1);
return test_self_signed("nonexistent file name", 1, -1);
}
static int test_store_ctx(void)
{
/* Verifying a cert where we have no trusted certs should fail */
return test_self_signed(bad_f, 0, 0);
}
int setup_tests(void)