Fix handling of otherName in ConfirmNameConstraints#10339
Fix handling of otherName in ConfirmNameConstraints#10339embhorn wants to merge 6 commits intowolfSSL:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR fixes RFC 5280 nameConstraints enforcement for otherName SAN entries and ensures critical nameConstraints behavior is correctly propagated and enforced during verification.
Changes:
- Add byte-exact
otherNamematching for permitted/excluded nameConstraints subtrees. - Track and fail-closed on critical nameConstraints that contain unsupported GeneralName forms.
- Propagate
extNameConstraintCrit(+ new unsupported-form flag) intoSigner, and add a regression test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
wolfssl/wolfcrypt/asn.h |
Adds flags to track critical nameConstraints and presence of unsupported constraint forms in DecodedCert/Signer. |
wolfcrypt/src/asn.c |
Implements otherName constraint matching, records unsupported forms during decode, propagates flags, and enforces fail-closed behavior for critical constraints. |
tests/api.c |
Adds a new test that builds a chain using otherName SAN + nameConstraints to verify enforcement. |
Comments suppressed due to low confidence (3)
wolfcrypt/src/asn.c:1
- New fail-closed behavior is introduced for critical nameConstraints containing unsupported GeneralName forms, but the added test suite only covers
otherName. Please add a regression test that constructs a critical nameConstraints with an unsupported form (e.g.,registeredID,x400Address, orediPartyName) and asserts verification rejects; also add a non-critical variant asserting verification continues (since the fail-closed requirement is for critical constraints).
wolfcrypt/src/asn.c:1 - The
otherNamebyte-exact matching logic (including theWOLFSSL_FPKIoidSumguard) is duplicated in bothPermittedListOkandIsInExcludedList. Consider extracting a small helper (e.g.,MatchOtherNameConstraint(name, current)) so future updates don’t accidentally diverge between permitted vs excluded handling.
wolfcrypt/src/asn.c:1 - The
otherNamebyte-exact matching logic (including theWOLFSSL_FPKIoidSumguard) is duplicated in bothPermittedListOkandIsInExcludedList. Consider extracting a small helper (e.g.,MatchOtherNameConstraint(name, current)) so future updates don’t accidentally diverge between permitted vs excluded handling.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10339
Scan targets checked: wolfcrypt-bugs, wolfcrypt-src
No new issues found in the changed files. ✅
|
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10339
Scan targets checked: wolfcrypt-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
Description
ConfirmNameConstraintsignored theotherNameGeneralName form (and theextNameConstraintCritflag was never propagated to Signer)WOLFSSL_X509_STORE_ALLOW_NON_CA_INTERMEDIATEfor backward compatibility.Fixes
Testing
Added
test_NameConstraints_OtherNameChecklist