Skip to content

Fix: X509 honor explicit depth 0 - #11440

Open
aidankeefe2022 wants to merge 1 commit into
wolfSSL:masterfrom
aidankeefe2022:x509-depth-fix
Open

Fix: X509 honor explicit depth 0#11440
aidankeefe2022 wants to merge 1 commit into
wolfSSL:masterfrom
aidankeefe2022:x509-depth-fix

Conversation

@aidankeefe2022

@aidankeefe2022 aidankeefe2022 commented Sep 11, 2026

Copy link
Copy Markdown
Member

Description

Fenrir 12740: X509_STORE_CTX_set_depth(ctx, 0) was treated as unset and silently go WOLFSSL_X509_STORE_DEFAULT_MAX_DEPTH.

  • set_depth() now always marks the depth as explicit (new depthSet bit), so 0 means no intermediate CAs are allowed.
  • A negative depth is clamped to 0 (fail closed) instead of falling back to the default.
  • X509_STORE_CTX_init() resets the depth, so a reused context no longer keeps a previous limit.
  • A positive ctx->depth written directly to the struct is still honored.

Testing

asserted all behavior changes with test making sure all depth behavior is tested.

Checklist

  • added tests

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #11440

Scan targets checked: wolfssl-src, wolfssl-bugs

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Comment thread src/x509_str.c Outdated
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown

…depth

Fenrir 12740: X509_STORE_CTX_set_depth(ctx, 0) was treated as unset
and silently got WOLFSSL_X509_STORE_DEFAULT_MAX_DEPTH.
- set_depth() now always marks the depth as explicit (new depthSet
  bit), so 0 means no intermediate CAs are allowed.
- An explicit negative depth rejects every chain with
  CERT_CHAIN_TOO_LONG before any path is built, instead of falling
  back to the default.
- X509_STORE_CTX_init() resets the depth, so a reused context no longer
  keeps a previous limit.
- A positive ctx->depth written directly to the struct (no setter) is
  still honored. The budget is clamped so INT_MAX cannot overflow it.

A trust anchor from X509_STORE_CTX_trusted_stack() or store->certs is
found by the same lookup as an untrusted intermediate and cost one extra
pass, so explicit depth N only allowed N-1 intermediates there (and
depth 0 rejected a leaf issued directly by the anchor). When the budget
runs out on a caller-trusted anchor that would end the path (self-issued
or PARTIAL_CHAIN), accept it instead of reporting CHAIN_TOO_LONG.

An issuer cycle among the untrusted certs (A <- B <- A) was followed
until the depth budget ran out, ~2^31 signature checks at INT_MAX. The
untrusted-issuer lookup now skips certs already on the path, so the
cycle ends the search with UNABLE_TO_GET_ISSUER_CERT_LOCALLY. Adds
loop-a/loop-b/loop-leaf fixtures to untrusted_anchor/ for the test.

Also fetch the verify callback once in X509_verify_cert() and pass it
to X509StoreVerifyCert(), X509StoreCheckPathLen() and the host/IP
checks, and factor the PARTIAL_CHAIN test into
X509StoreCertIsPartialChain().
@aidankeefe2022 aidankeefe2022 changed the title Fix: X509 honor explicit depth 0, run verify cb on depth error Fix: X509 honor explicit depth 0 Sep 11, 2026

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #11440

Scan targets checked: wolfssl-src, wolfssl-bugs

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Comment thread src/x509_str.c
}

if (anchorEndsPath) {
ret = X509StoreChainPush(ctx->chain, ctx->current_cert);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-depth partial-chain acceptance bypasses the verify callback · Certificate and trust chain validation bypass

The new PARTIAL_CHAIN shortcut accepts the terminal trusted certificate without a success callback, so a callback cannot veto it. Unlike known #10740's error-path omission, this path returns success against application policy.

Related known finding #10740 (similar but distinct): Both are in wolfSSL_X509_verify_cert and omit verification-callback handling, but #10740 omits an error-path callback after chain-depth exhaustion while this accepts a trusted terminal through a PARTIAL_CHAIN success path without an ok=1 callback. The operations, root causes, and required patches differ.

Suggested fix: Route exact-depth terminal-anchor acceptance through per-certificate callback handling and abort when the callback returns zero.
Basis: OpenSSL X509_verify_cert(3), “The verification callback”: each successfully checked certificate triggers ok = 1, and a zero return aborts verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants