X509 validation fixes#10737
Open
kareem-wolfssl wants to merge 4 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses X.509 validation correctness issues reported by x509-limbo (zd#21998) by tightening hostname wildcard matching rules and enforcing BasicConstraints pathLenConstraint during wolfSSL_X509_verify_cert() chain validation.
Changes:
- Enforce RFC 6125/RFC 9525 + CA/Browser Forum wildcard placement restrictions in
MatchDomainName(). - Add
pathLenConstraintenforcement over the assembled chain inwolfSSL_X509_verify_cert()(OpenSSL-compat path). - Add regression tests for wildcard placement and
pathLenConstraintbehavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/api/test_ossl_x509.h | Registers a new API test entry for wildcard placement regression coverage. |
| tests/api/test_ossl_x509.c | Adds wildcard placement regression tests for MatchDomainName(). |
| tests/api/test_ossl_x509_str.c | Adds regression tests ensuring wolfSSL_X509_verify_cert() enforces BasicConstraints path length. |
| src/x509_str.c | Implements pathLenConstraint checking over the built verification chain and integrates it into wolfSSL_X509_verify_cert(). |
| src/internal.c | Adds wildcard placement validation and applies it within MatchDomainName(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
- Break out of the chain-build loop after the partial-chain fallback accepts a caller-trusted terminus, so it is pushed to ctx->chain once instead of twice; X509StoreCheckPathLen's anchor-skip is now defensive, not load-bearing. - Drop the now-dead cert == anchor guard and refresh the comment. - Rework the pathLen regression tests: reuse the existing certs/test-pathlen chains (chainF rejects, chainB verifies) instead of inlined report certs.
X509StoreCheckPathLen() consumed a unit of the issuer's path length budget for any non-self-issued intermediate. Gate the RFC 5280 sec. 6.1.4 (l) decrement on cert->isCa so only CA certificates count, matching ParseCertRelative() (wolfcrypt/src/asn.c) and the (m) tightening step. This prevents a false PATH_LENGTH_EXCEEDED when a non-CA intermediate is tolerated via verify_cb.
Contributor
|
Jenkins retest this please |
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
Fixes zd#21998
Testing
Built in tests + added tests
Checklist