Fix partial chain verification

[embhorn]

Description

The chain is now only accepted at a partial-chain terminal if ctx->current_cert is itself in the original trust set.

Fixes zd21566

Testing

Added new test in test_wolfSSL_X509_STORE_CTX_ex

Checklist

• added tests
• updated/added doxygen
• updated appropriate READMEs
• Updated manual and documentation

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Tightens X509_V_FLAG_PARTIAL_CHAIN handling so verification only succeeds when the terminal certificate is actually trusted by the caller’s original trust set, and adds a regression test to prevent accepting chains that never reach a trust anchor.

Changes:

• Add a trusted-certificate membership check (X509StoreCertIsTrusted) and use it in the partial-chain fallback path.
• Track the pre-injection “trusted” slice size to avoid treating injected intermediates as trusted.
• Add a negative partial-chain test ensuring verification fails when no chain cert is trusted.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
tests/api/test_ossl_x509_str.c	Adds a negative regression test for partial-chain verification behavior.
src/x509_str.c	Fixes partial-chain fallback logic by verifying the terminal cert is in the original trust set.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dgarske]

🐺 Skoll Code Review

Overall recommendation: APPROVE
Findings: 3 total — 3 posted, 1 skipped

Posted findings

• [Medium] origCertsNum boundary can become stale during retry cycles — src/x509_str.c:610,738-739
• [Medium] No test for mixed trusted-store + untrusted-chain with partial chain — tests/api/test_ossl_x509_str.c:788-822
• [Low] Unused variable n declared at function scope — src/x509_str.c:565

Skipped findings

• [Medium] origCertsNum boundary can become stale during retry cycles

Review generated by Skoll via openclaw

[dgarske]

🟡 [Medium] No test for mixed trusted-store + untrusted-chain with partial chain
💡 SUGGEST test

The new negative test verifies that partial-chain fails when no certificate is trusted (empty store + untrusted intermediates). However, there is no corresponding test for the mixed scenario: a store that contains some trusted intermediates combined with additional untrusted intermediates provided via the chain argument. This scenario exercises the origCertsNum boundary logic most directly. The existing positive tests (ex10, ex11) place intermediates in the store but do not provide extra untrusted intermediates through X509_STORE_CTX_init, so the boundary between trusted and untrusted entries in certs is never tested.

Suggestion:

Suggested change

	static int test_wolfSSL_X509_STORE_CTX_ex_partial_chain_neg(
	Add a test where one intermediate (e.g., x509CaInt) is added to the store, another (e.g., x509CaInt2) is passed as untrusted, PARTIAL_CHAIN is set, and verify: (a) the chain verifies successfully because x509CaInt is trusted, and (b) if *only* x509CaInt2 is in the store and x509CaInt is untrusted, verify the chain still succeeds with the correct terminal cert.

[dgarske]

🔵 [Low] Unused variable n declared at function scope
🔧 NIT style

The variable int n is declared at the top of X509StoreCertIsTrusted but is only used inside the if (store != NULL && store->trusted != NULL) block. While this follows C89 declaration-at-top style (common in the wolfSSL codebase), declaring n at the top and only assigning it conditionally may produce compiler warnings about uninitialized use on some static analysis tools. This is purely cosmetic since the usage is correct.

Recommendation: No action needed — this follows the existing C89-style convention in the codebase. If you want to silence picky analyzers, initialize n = 0 at declaration.