check_cert()/check_revocation() conflate internal errors with verification failures

Bug Description

check_cert() and check_revocation() in crypto/x509/x509_vfy.c return 0 for both "revocation check failed" and "internal error" (e.g., OOM, store lookup failure). Callers cannot distinguish the two cases, and the code itself acknowledges this with comments:

// Line 907
/* Sadly, returns 0 also on internal error. */
static int check_revocation(X509_STORE_CTX *ctx)

// Line 931
/* Sadly, returns 0 also on internal error. */
static int check_cert(X509_STORE_CTX *ctx)

Where the conflation happens

In check_cert(), multiple callees (ctx->get_crl, get_crl_delta, ctx->check_crl, ctx->cert_crl) can fail due to either a verification issue or an internal error (allocation failure, store lookup failure). All failures flow to return ok at line 1001 with ok == 0, regardless of cause.

The caller check_revocation() propagates this directly:

// Line 924-926
ok = check_cert(ctx);
if (!ok)
    return ok;

And verify_chain() at line 221 checks <= 0:

|| (ok = ctx->check_revocation(ctx)) <= 0)
    return ok;

Since check_revocation never returns -1 for internal error (only 0 or 1), verify_chain treats all failures as verification failures, never as internal errors warranting a -1 return.

Impact

• A transient CRL store lookup failure (network timeout, OOM during CRL fetch) is misreported to the application as a revocation verification failure rather than an internal error.
• Applications that distinguish 0 (verification failure) from -1 (internal error) in X509_verify_cert() results — as the API contract suggests — will misclassify transient failures as permanent verification rejections.
• This can cause unnecessary certificate rejection during transient resource pressure, when the correct behavior would be to signal an internal error so the application can retry.

Suggested Fix

Have check_cert() return -1 on internal error paths (allocation failure, store lookup failure) and 0 only for actual verification failures. Update check_revocation() to propagate the -1 to verify_chain(), which already handles negative returns correctly.

For example, in check_cert() around line 951-958:

if (ctx->get_crl != NULL)
    ok = ctx->get_crl(ctx, &crl, x);
else
    ok = get_crl_delta(ctx, &crl, &dcrl, x);
if (!ok) {
    // If this is a lookup/allocation error vs "no CRL available",
    // return -1 instead of falling through with ok=0
    ok = verify_cb_crl(ctx, X509_V_ERR_UNABLE_TO_GET_CRL);
    goto done;
}

The ctx->check_crl and ctx->cert_crl callbacks would similarly need to distinguish their error returns.

Affected Versions

Present in 3.0.13 and confirmed unchanged on current master. The "Sadly" comments suggest this has been a known issue for some time.

Start in crypto/x509/x509_vfy.c with check_cert() and check_revocation(), then trace their results into verify_chain(). Review the get_crl, get_crl_delta, check_crl, and cert_crl callback contracts to determine how internal errors are distinguished from verification failures. Done means internal failures can reach the existing negative-return handling without changing genuine verification failures.