Some more fixes#10714
Open
Frauschi wants to merge 3 commits into
Open
Conversation
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens correctness and fail-closed behavior in two security-sensitive parsing paths: (1) OpenSSL-style cipher-list exclusions (!aNULL, !eNULL) now remove explicitly-listed suites (not just defaults), and (2) X.509 extension decoding now rejects duplicate certificatePolicies extensions consistently across relevant build configurations. It also adds targeted regression coverage for negative-length d2i_* inputs.
Changes:
- Apply OpenSSL-style exclusion categories to explicitly-listed cipher suites, and fail when exclusions empty the list.
- Make
certificatePoliciesduplicate rejection unconditional forWOLFSSL_SEP/WOLFSSL_CERT_EXTbuilds and exposeDecodeExtensionTypefor API tests. - Add regression tests for negative-length
d2i_*decoders and duplicatecertificatePolicieshandling.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
wolfssl/wolfcrypt/asn.h |
Adjusts DecodedCert bitfield gating and exposes DecodeExtensionType for tests (incl. prefix-map). |
wolfcrypt/src/asn.c |
Enforces non-repeatable certificatePolicies duplicate rejection during extension decode. |
src/internal.c |
Implements removal of excluded suites from explicit cipher lists and updates DEFAULT/ALL exclusion semantics. |
tests/api.c |
Adds cipher exclusion regression tests and negative-length d2i_* coverage for PKCS#8-related decoders. |
tests/api/test_ossl_asn1.c |
Adds negative-length guard regression tests for ASN1 string d2i_* wrappers. |
tests/api/test_asn.h |
Registers new ASN extension-duplication regression test. |
tests/api/test_asn.c |
Adds regression test ensuring duplicate certificatePolicies extensions are rejected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Fixed the Copilot warning |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10714
Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
ParseCipherList() only cleared the InitSuites mask for "!aNULL"/"!eNULL", which governs generated defaults, so an explicitly listed ADH or NULL-cipher suite survived (e.g. "ADH-AES128-SHA:!aNULL" still offered an unauthenticated suite). Scrub the explicit suites after parsing; exclusions are order- independent and sticky (a later "ALL" cannot re-enable them). Add test_wolfSSL_set_cipher_list_exclusions.
…ilds DecodeExtensionType() guarded the certificatePolicies duplicate check (VERIFY_AND_SET_OID) under WOLFSSL_SEP only, because the extCertPolicySet tracking bit was SEP-only. In a WOLFSSL_CERT_EXT-without-WOLFSSL_SEP build a cert with two certificatePolicies extensions was accepted and the second silently overwrote the first (RFC 5280 4.2 forbids repeats). Make the bit and the guard available under WOLFSSL_CERT_EXT too, matching every other non-repeatable extension. Add test_DecodeCertExtensions_dup_certpol (DecodeExtensionType now WOLFSSL_TEST_VIS).
Contributor
Author
|
Fixed the second Copilot finding |
dgarske
approved these changes
Jun 17, 2026
Member
|
Jenkins retest this please: "Build 'wolfSSL/PRB-fips-repo-and-harness-test-v3-part2' failed with result: FAILURE" |
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.
A few correctness and robustness fixes in TLS cipher-list handling and certificate parsing, with regression tests.
Changes:
!aNULL/!eNULL) so explicitly-listed suites are treated consistently with generated defaults.d2i_*decoder paths.All existing tests pass; new tests added for the above. Fixes findings 33, 34, 44, 60, and 61 of zd21992.