Update wolfSSL_X509_verify_cert to retry all certs until a valid chain is found. #8692
+138
−3
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR updates the wolfSSL certificate verification function to retry alternative certificate chains until a valid one is found, addressing issues with multiple CA certs having the same subject key.
- Added a RemoveCA function prototype in wolfssl/internal.h and its implementation in src/ssl.c.
- Updated wolfSSL_X509_verify_cert in src/x509_str.c to include retry logic that removes invalid certs and manages a failedCerts stack.
- Introduced X509StorePopCert to extract a failing cert from the candidate stack.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| wolfssl/internal.h | Added prototype for RemoveCA to support cert removal |
| src/x509_str.c | Updated verify_cert to retry with alternative cert chains |
| src/ssl.c | Implemented RemoveCA to remove a CA cert by subject hash |
Comments suppressed due to low confidence (4)
wolfssl/internal.h:4262
- [nitpick] Consider using 'const byte* hash' if the hash parameter is not modified within RemoveCA to enhance const-correctness and clarify intent.
WOLFSSL_LOCAL int RemoveCA(WOLFSSL_CERT_MANAGER* cm, byte* hash, byte type);
src/x509_str.c:409
- Ensure that the updated return value documentation for wolfSSL_X509_verify_cert aligns with all call sites and the handling of WOLFSSL_SUCCESS/WOLFSSL_FAILURE macros.
* returns 1 on success or 0 on failure.
src/x509_str.c:511
- Review the retry logic condition and the manipulation of 'added' and 'depth' to ensure that it does not inadvertently lead to infinite loops or skipped retries when processing alternative cert chains.
if ((origDepth - depth) <= 1)
src/x509_str.c:1099
- Verify that using pointer equality to identify the certificate in X509StorePopCert is the intended behavior and that it correctly handles cases with duplicate certificate pointers.
if (wolfSSL_sk_X509_value(certs_stack, i) == cert) {
|
Looks like it's currently failing unit tests with this configure |
kareem-wolfssl
force-pushed
the
zd19563_verify
branch
2 times, most recently
from
c0be320 to
e02811f
Compare
…alid chain is found, rather than failing out on the first invalid chain. This allows for registering multiple certs with the same subject key, ie. alt cert chains.
WOLFSSL_X509's calculated subject key hash is not guaranteed to match the cert's, ie. in the case that NO_SHA is defined. Use the same logic as AddCa, parsing the DER cert and using the decoded cert's subject key hash.
kareem-wolfssl
force-pushed
the
zd19563_verify
branch
from
e02811f to
e08125f
Compare
…as optional, it is possible there is no temp CA to remove.
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
Previously wolfSSL_X509_verify_cert was failing out after finding an invalid chain, this commit updates it to remove the invalid cert and retry until a valid chain is found or all certs are exhausted. This matches OpenSSL's behavior according to the documentation and testing.
This allows wolfSSL_X509_verify_cert to work with multiple CA certs with the same subject key, ie. alt cert chains.
Fixes zd#19563
Testing
Tested with customer provided test.
Unit tests to be added. Confirming it passes CI/CD tests first.
Checklist