Repository navigation
Certificate Processing Bug: Verifification Issue - #12024
aidankeefe2022 wants to merge 2 commits into
Conversation
with ```c ./configure --enable-opensslextra --enable-altcertchains ``` If the user loads a verifiy callback that delays peer auth and the server sends a chain that looks like this: - Attacker-ca -> has all data for us to find a signer in our trust store but is sign by attacker controlled key. - Intermediate_ca -> selfsigned - leaf -> produced by attacker ca We accept the connection and trust the identity of the leaf cert. This is becuase we incorrectly wipe the error produced by the bad root CA when we come across the intermediate with the error we rightfuly don't care about. this mean we get to the point where we have forgotten out root is bad and check the leaf against it which returns OK due to the leaf being produced from the attacker root.
aidankeefe2022
changed the title
|
There was a problem hiding this comment.
🟡 Changes recommended
The PEM-dependent test and callback helper lack WOLFSSL_PEM_TO_DER guards, causing fixture loading to fail in DER-only builds before certificate verification is exercised.
1 open issue
What changed in this PR
This PR fixes certificate-chain processing so bypassing one certificate’s CA error does not erase verification errors from earlier certificates.
Changes:
- Preserve prior verification errors in alternate-chain and Apple native-validation paths.
- Add a forged-CA regression test covering three verification-callback scenarios.
- Include certificate fixtures, a regeneration script, and distribution entries.
| File | Description |
|---|---|
tests/api/test_ssl_cert.h |
Declares and registers the regression test. |
tests/api/test_ssl_cert.c |
Tests forged-chain handling and preservation of verification errors. |
src/internal.c |
Preserves earlier errors when bypassing a CA error. |
certs/test/include.am |
Includes the new fixtures and script in distributions. |
certs/test/alt-chain-forged-ca/server-chain.pem |
Provides the forged certificate chain. |
certs/test/alt-chain-forged-ca/realRoot-cert.pem |
Provides the trusted root fixture. |
certs/test/alt-chain-forged-ca/leaf-key.pem |
Provides the test server’s private key. |
certs/test/alt-chain-forged-ca/gen-alt-chain-forged-ca.sh |
Generates the regression fixtures. |
Review effort: Balanced
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12024
Scan targets checked: wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): certs/test/alt-chain-forged-ca/gen-alt-chain-forged-ca.sh, certs/test/alt-chain-forged-ca/leaf-key.pem, certs/test/alt-chain-forged-ca/realRoot-cert.pem, certs/test/alt-chain-forged-ca/server-chain.pem, certs/test/include.am, tests/api/test_ssl_cert.h
Coverage: 2 of 2 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Max
bd5e810 to
19868a5
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The regression covers only alternate chains, leaving security-sensitive Apple native trust fallback and callback interactions in need of maintainer validation on macOS.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12024
Scan targets checked: wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): certs/test/alt-chain-forged-ca/gen-alt-chain-forged-ca.sh, certs/test/alt-chain-forged-ca/leaf-key.pem, certs/test/alt-chain-forged-ca/realRoot-cert.pem, certs/test/alt-chain-forged-ca/server-chain.pem, certs/test/include.am, tests/api/test_ssl_cert.h
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
|
retest this please |
Description
Write-up of the issue is here: https://wolfsslinc.slack.com/files/U0B68E1LLHG/F0CFA3L0KTQ/vulnerabilitywriteup.md
Testing
Reproduced the issue and confirmed it. Added a regression test and confirmed that the nginx build is affected
Checklist