Repository navigation
Conversation
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Certificate checks can incorrectly skip bad signatures for issuer/serial-only AKIDs or differently encoded identical ECC keys, and the new PEM-based tests fail in supported builds without PEM-to-DER support.
3 open issues
What changed in this PR
This PR updates wolfSSL’s alternate certificate-chain handling to tolerate extra same-name roots with different keys, while still requiring the peer certificate to verify through a trusted CA.
Changes:
- Adds conditional skipping of signature failures for self-issued chain CAs without an authority key identifier.
- Adds positive and negative handshake regression tests.
- Adds certificate fixtures, generation commands, and distribution entries.
| File | Description |
|---|---|
tests/api/test_ssl_cert.h |
Declares and registers the regression tests. |
tests/api/test_ssl_cert.c |
Tests tolerated extra roots and rejected bad signatures. |
src/internal.c |
Adds the alternate-chain root-skipping logic. |
certs/intermediate/server-chain-alt-same-key-ecc.pem |
Negative chain fixture retaining the trusted key. |
certs/intermediate/server-chain-alt-same-dn-rsa.pem |
Chain fixture with an extra same-name RSA root. |
certs/intermediate/server-chain-alt-same-dn-int-ecc.pem |
Negative chain fixture with a non-self-issued CA. |
certs/intermediate/server-chain-alt-same-dn-ecc.pem |
Chain fixture with an extra same-name ECC root. |
certs/intermediate/server-chain-alt-same-dn-akid-ecc.pem |
Negative chain fixture carrying an authority key identifier. |
certs/intermediate/include.am |
Includes the new fixtures in source distributions. |
certs/intermediate/genintcerts.sh |
Generates the new certificates and chains. |
certs/intermediate/ca-ecc-same-key-cert.pem |
Bad-signature CA fixture using the trusted key. |
certs/intermediate/ca-ecc-same-dn-rsa-cert.pem |
Same-name root fixture using an RSA key. |
certs/intermediate/ca-ecc-same-dn-int-cert.pem |
Non-self-issued CA fixture. |
certs/intermediate/ca-ecc-same-dn-cert.pem |
Same-name root fixture using a different ECC key. |
certs/intermediate/ca-ecc-same-dn-akid-cert.pem |
Same-name root fixture with an authority key identifier. |
Review effort: Balanced
Frauschi
force-pushed
the
alt_chain_same_dn_root
branch
from
November 10, 2026 11:03
05a35f9 to
d3815de
Compare
Member
Author
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12077
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): certs/intermediate/ca-ecc-same-dn-akid-cert.pem, certs/intermediate/ca-ecc-same-dn-akid-name-cert.pem, certs/intermediate/ca-ecc-same-dn-cert.pem, certs/intermediate/ca-ecc-same-dn-int-cert.pem, certs/intermediate/ca-ecc-same-dn-rsa-cert.pem, certs/intermediate/ca-ecc-same-key-cert.pem, certs/intermediate/genintcerts.sh, certs/intermediate/include.am, certs/intermediate/server-chain-alt-same-dn-akid-ecc.pem, certs/intermediate/server-chain-alt-same-dn-akid-name-ecc.pem, certs/intermediate/server-chain-alt-same-dn-ecc.pem, certs/intermediate/server-chain-alt-same-dn-int-ecc.pem, certs/intermediate/server-chain-alt-same-dn-rsa.pem, certs/intermediate/server-chain-alt-same-key-ecc.pem, tests/api/test_ssl_cert.h
Coverage: 4 of 4 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
With WOLFSSL_ALT_CERT_CHAINS a chain CA that fails with ASN_NO_SIGNER_E or ASN_SELF_SIGNED_E is skipped, as long as the peer certificate still verifies to a trusted CA. An extra CA that carries no Authority Key Identifier and shares its issuer name with a trusted CA is looked up by name, finds that trusted CA and then fails with ASN_SIG_CONFIRM_E, or with ASN_SIG_OID_E when the key types differ. Those errors aborted the handshake even in alternate chain mode. NSS can send such a chain: its own leaf and intermediate followed by the peer's trust anchor from another PKI. RFC 8446 section 4.4.2 asks implementations to tolerate extraneous certificates. Treat both signature errors like the missing signer case when the chain CA is self-issued, has no AKID, and the CA found for it has a different public key. Only an error from parsing the chain CA counts, not one from its OCSP or CRL check. A tampered intermediate, or a tampered copy of a trusted root found by its own SKID, still fails on its own signature. A CA with an AKID names its issuer, so a bad signature still fails the handshake, as tests/test-fails.conf expects for server-cert-rsa-badsig.pem. DecodedCert gains extAuthKeyIdPresent for this, since extAuthKeyIdSet only records a key identifier and an AKID with just issuer and serial would otherwise count as absent. Requiring a found issuer keeps out the ASN_SIG_OID_E that DecodeCert returns when signatureAlgorithm differs from the TBS signature; the original ASN decoder sets isCA before that check, so such a malformed CA would otherwise be skipped. The skipped certificate is not added to the certificate manager, so it never becomes trusted and the peer certificate still has to verify on its own. Add ECC test chains, generated by genintcerts.sh from existing keys, that each end in an extra CA named after ca-ecc-cert.pem: roots with an ECC and an RSA key and no key ids, a root with the key and SKID of ca-ecc-cert.pem but signed by another key, roots with an AKID naming ca-ecc-cert.pem by key id and by issuer and serial, and a CA that is not self-issued. API tests expect the first two to pass on an alternate chain when enabled and otherwise to fail with ASN_SIG_CONFIRM_E and ASN_SIG_OID_E respectively. The other four, and server-cert-rsa-badsig.pem whose chain CA carries an AKID, fail with ASN_SIG_CONFIRM_E in both modes. The tests set WOLFSSL_VERIFY_PEER on the client, since builds with OPENSSL_COMPATIBLE_DEFAULTS default to no peer verification. They need WOLFSSL_PEM_TO_DER for the PEM fixtures and are skipped with WOLFSSL_TEST_APPLE_NATIVE_CERT_VALIDATION, where Security.framework revalidates the chain.
Frauschi
force-pushed
the
alt_chain_same_dn_root
branch
from
November 10, 2026 14:50
d3815de to
b2322fd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
With
WOLFSSL_ALT_CERT_CHAINS, a peer chain ending in an extra self-signed root that shares a trusted CA's name, but has another key and no AKID, failed withASN_SIG_CONFIRM_EorASN_SIG_OID_E. NSS sometimes sends such chains, and RFC 8446 section 4.4.2 asks implementations to tolerate extraneous certificates.Changes to handle this:
SelfIssuedCaKeyDiffers()).genintcerts.shgenerates the new test chains from existing keys.Security
A skipped certificate is never trusted. It is not added to the certificate manager, and the peer certificate must still verify to a CA the client already trusts, so a forged root gains nothing. A chain CA with an AKID, one that is not self-issued, or one carrying the trusted CA's own key still fails on a bad signature, as before.
Testing
test_wolfSSL_alt_chain_same_dn_root: same-name roots with an ECC and an RSA key pass with alternate chains and fail withASN_SIG_CONFIRM_E/ASN_SIG_OID_Ewithout them.test_wolfSSL_alt_chain_bad_sig: a root with the trusted CA's key, a root with an AKID naming that CA, a non-self-issued CA andserver-cert-rsa-badsig.pemfail withASN_SIG_CONFIRM_Ein both modes.