Gate secp192r1/P-192 TLS-GROUP advertisement on curve availability#432
Open
yosuke-wolfssl wants to merge 1 commit into
Open
Gate secp192r1/P-192 TLS-GROUP advertisement on curve availability#432yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request prevents wolfProvider from advertising the weak/non-FIPS P-192 (secp192r1) TLS group unless the curve is both compiled/usable and the build is non-FIPS, avoiding negotiation of unsupported or too-weak groups and aligning advertised capabilities with actual provider behavior.
Changes:
- Gate
secp192r1/P-192TLS-GROUP capability advertisement behindWP_HAVE_EC_P192and!HAVE_FIPS. - Add a regression test that enumerates
TLS-GROUPcapabilities and asserts P-192 presence/absence matches the same compile-time guard. - Register the new test in the unit test harness.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/wp_tls_capa.c |
Conditionally excludes P-192 TLS group entries from the advertised capability list in FIPS or when the curve isn’t available. |
test/test_ecc.c |
Adds a capability-walk test validating whether P-192 is advertised matches build-time gating. |
test/unit.c |
Registers the new ECC TLS-GROUP advertisement test when ECC is enabled. |
test/unit.h |
Declares the new unit test entry point. |
💡 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 #432
Scan targets checked: wolfprovider-bugs, wolfprovider-src
No new issues found in the changed files. ✅
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.
Gate secp192r1/P-192 TLS-GROUP advertisement on curve availability
Fixes finding f_6497.
Problem
When libssl queries wolfProvider for
TLS-GROUPcapabilities,wp_tls_group_capability()walkedwp_param_group_list[]andunconditionally advertised
secp192r1andP-192. That entry declaresonly 80 bits of security, which is below the 112-bit modern/NIST minimum
and below the P-224 floor used by wolfCrypt FIPS.
Two consequences followed:
blocks P-192 private key operations (
WP_FIPS_CHECK_P192). The provideradvertised it anyway, so libssl could select a group the underlying
wolfCrypt cannot perform, failing at key generation rather than at
negotiation. The brainpool curves in the same table were already gated
with
#ifndef HAVE_FIPS, so the intent to FIPS-gate non-approved curveswas established; P-192 was simply missed.
secp192r1was actually built into wolfCrypt, so the provider could advertise a
curve it cannot satisfy.
A peer configured to accept the advertised set could therefore negotiate an
80-bit ECDHE group with no error surfaced to the application.
Fix
Wrap the two P-192 entries in the curve's own availability macro, plus an
explicit FIPS exclusion:
WP_HAVE_EC_P192already exists ininclude/wolfprovider/settings.hand isdefined as
(HAVE_ECC192 || HAVE_ALL_CURVES) && ECC_MIN_KEY_SZ <= 192. Itcovers the "curve compiled in and usable" half, including hardened builds
that raise the minimum ECC key size.
Both terms are needed.
ECC_MIN_KEY_SZdefaults to 224 in a plain non-FIPSbuild, so
WP_HAVE_EC_P192is already off by default there. However,wolfSSL defaults
ECC_MIN_KEY_SZto 192 underFIPS_VERSION_GE(2,0),so a FIPS build that does not separately override it would leave
WP_HAVE_EC_P192defined. The explicit!defined(HAVE_FIPS)termguarantees P-192 is never advertised in a FIPS build regardless of the
configured minimum key size, and matches the existing brainpool gating
convention in the same file.
Scope
This change is intentionally limited to P-192, the subject of the finding.
The
P-224,P-256,P-384, andP-521entries remain unconditional eventhough each has a corresponding
WP_HAVE_EC_Pxxxmacro. Those curves allmeet the 112-bit floor and are FIPS approved. Gating them on their
availability macros is a pre-existing inconsistency and is left as a
separate follow-up rather than widening this fix.
Testing
New regression test
test_ec_tls_group_p192intest/test_ecc.c(registeredin
test/unit.candtest/unit.h). It callsOSSL_PROVIDER_get_capabilities(prov, "TLS-GROUP", ...), walks the advertisedgroup list, and asserts that P-192 presence matches the guard, so it stays
correct in both FIPS and non-FIPS builds.
Verified at runtime against the actual built provider across three
configurations:
WP_HAVE_EC_P192on, non-FIPSWP_HAVE_EC_P192forced offHAVE_FIPSforced onWP_HAVE_EC_P192off (pre-fix)The last row is the negative control: it confirms the test fails without the
fix rather than passing vacuously.