Reject non-CA peer intermediate certs#1021
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds stricter rules for promoting peer-supplied intermediate certificates into the cert manager trust store, preventing promotion of non-CA intermediates and adding regression coverage for issue F-5851.
Changes:
- Introduces
CertManIntermediateIsCA()to gate intermediate promotion on BasicConstraints CA=TRUE (and KeyUsage keyCertSign when present). - Updates certificate chain verification to avoid trusting non-CA intermediates.
- Adds unit tests that reproduce the non-CA intermediate promotion attack and validate promotion of legitimate CA intermediates.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/unit.c | Adds regression/positive tests that exercise intermediate promotion behavior using forged ECC certificates. |
| src/certman.c | Adds CA-check helper and gates intermediate promotion during chain verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1021
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1021
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
- Add CertManIntermediateIsCA: require isCA and, for non-self-signed intermediates that carry a KeyUsage extension, the keyCertSign bit before promoting a cert. - Only promote a verified intermediate into the trust store when it is actually a CA; otherwise fail with WS_CERT_NO_SIGNER_E. - Prevents a peer-supplied end-entity cert at an intermediate position from being trusted to issue certs for arbitrary SSH principals. - Gate keyCertSign on ALLOW_INVALID_CERTSIGN and on the KeyUsage extension being present, matching wolfSSL's AddCA loader. - Add regression tests: non-CA intermediate is not promoted, and a valid CA intermediate (with and without KeyUsage) still is. Issue: F-5851
OCSP_NEED_URL meant the cert has no AIA OCSP URL and no default responder is set, so OCSP cannot run. Treat it as not-revoked instead of failing the whole verification. This softens revocation from hard-fail to soft-fail for that case only. The AIA URL is part of the signed certificate and is not attacker-strippable, so a malicious peer cannot use this to bypass an otherwise-enforced OCSP check.
Issue: F-5851