Repository navigation
Reject ECC groups in TLSX_IsGroupSupported when ECC is off - #11760
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The guards consistently match curve availability, and the corresponding test updates reveal no blocking issues.
0 open findings
What changed in this PR
Aligns wolfSSL’s TLS group selection with compiled ECC support, rejecting unavailable curves and hybrids before key generation.
Changes:
- Applies consistent SECP availability checks to validation, preferences, and defaults.
- Consolidates ML-KEM and Kyber support returns.
- Updates API expectations and hybrid test guards.
| File | Description |
|---|---|
| tests/api/test_tls13.c | Updates key-share errors and curve-dependent test guards. |
| tests/api.c | Checks rejection of unavailable P-256 curves and hybrids. |
| src/tls.c | Gates ECC groups and SECP hybrids on compiled curve availability. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Frauschi
force-pushed
the
fix-group-supported-have-ecc
branch
2 times, most recently
from
October 15, 2026 20:49
4f8a08f to
fbb3aeb
Compare
Member
Author
|
Jenkins retest this please |
TLSX_IsGroupSupported gated the SECP, Koblitz, Brainpool and SM2 groups on curve size macros only, and the SECP-based ML-KEM and Kyber hybrids not at all. A build without HAVE_ECC therefore accepted secp256r1 in wolfSSL_UseSupportedCurve() and wolfSSL_UseKeyShare(), advertised it, and only failed later in TLSX_KeyShare_GenEccKey() with NOT_COMPILED_IN. Gate the ECC groups on HAVE_ECC. Add TLSX_HAVE_SECP256R1/384R1/521R1, true when the plain SECP curve is usable (HAVE_ECC, !NO_ECC_SECP, curve size, ECC_MIN_KEY_SZ), and use them for every SECP-based hybrid: in TLSX_IsGroupSupported, preferredGroup[], the default key share group and TLSX_PopulateSupportedGroups. Those callers did not check NO_ECC_SECP, so an ECC build without the SECP curves (Brainpool or SM2 only) offered SECP hybrids by default and every TLS 1.3 client handshake failed with BAD_FUNC_ARG in key generation; such builds now handshake. All ML-KEM and Kyber cases in TLSX_IsGroupSupported now share one return, so no return is left without a case label when a block's groups are compiled out; the default build (standalone ML-KEM off) already had such a return. test_wolfSSL_UseSupportedCurve expected secp256r1 to be accepted in every build; it now expects BAD_FUNC_ARG when the curve is not compiled in, and checks that the SECP hybrid is rejected then too. test_tls13_apis expected NOT_COMPILED_IN from wolfSSL_UseKeyShare() with secp256r1 and no ECC; the call now fails up front with BAD_FUNC_ARG. The TLS 1.3 tests that picked a SECP P-256 hybrid under HAVE_ECC now use TEST_HAVE_SECP256R1, the same condition the library applies. test_wolfSSL_dh_ctx_params_reuse offered secp256r1 to keep the server off FFDHE and on its own DH parameters, which failed in builds without ECC. It now offers the first compiled-in group of secp256r1, X25519 and ML-KEM-768.
The tests that pick a SECP or X25519/X448 hybrid group still checked HAVE_ECC or HAVE_CURVE25519 alone, so with --with-eccminsz=384 or 521, or without the SECP curves, they offered groups that TLSX_IsGroupSupported() now rejects and failed. tests/suites.c gets SUITES_HAVE_SECP256R1/384R1/521R1, the library's TLSX_HAVE_SECP* conditions plus the existing SP math checks, for the hybrid group filter. The X25519 and X448 hybrids also need ECC_MIN_KEY_SZ to allow them. test_tls13_apis, test_tls13_pq_groups and the hybrid truncated, malformed and async tests use TEST_HAVE_SECP256R1 and the same X25519 check, and test_dtls_rtx_across_epoch_change gates its group choice the same way. The two DTLS 1.3 ClientHello fragmentation tests now share one group choice, which falls back to SecP384r1MLKEM1024 and skips the tests when no group fits. At --with-eccminsz=384 and 521 the pq-hybrid suite cases and five hybrid API tests that failed on master now pass or are skipped.
Frauschi
force-pushed
the
fix-group-supported-have-ecc
branch
from
November 2, 2026 10:32
fbb3aeb to
346f5e1
Compare
Member
Author
|
Jenkins retest this please |
SparkiDev
approved these changes
Nov 3, 2026
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
TLSX_IsGroupSupported()gated the SECP, Koblitz, Brainpool and SM2 groups on the curve size macros only, and the SECP-based ML-KEM and Kyber hybrids not at all. A build withoutHAVE_ECCaccepted secp256r1 inwolfSSL_UseSupportedCurve()andwolfSSL_UseKeyShare(), advertised it, and only failed later inTLSX_KeyShare_GenEccKey()withNOT_COMPILED_IN. The same happened for the SECP hybrids with--with-eccminsz=384or521.HAVE_ECC: the classic ECC cases inTLSX_IsGroupSupported()are now underHAVE_ECC.TLSX_HAVE_SECP256R1/384R1/521R1hold when the plain SECP curve is usable (HAVE_ECC,!NO_ECC_SECP, curve size,ECC_MIN_KEY_SZ).TLSX_IsGroupSupported(),preferredGroup[], the default key share group andTLSX_PopulateSupportedGroups()all use them. The callers never checkedNO_ECC_SECP, so an ECC build without the SECP curves (Brainpool or SM2 only) offered SECP hybrids by default and every TLS 1.3 client handshake failed withBAD_FUNC_ARG; such builds now handshake.TLSX_IsGroupSupported()share onereturn, so none is left without a case label when its groups are compiled out.tests/suites.c(newSUITES_HAVE_SECP*),test_tls13_apis,test_tls13_pq_groups, the hybrid truncated/malformed/async tests andtest_dtls_rtx_across_epoch_changenow only pick a hybrid whose classical curve is offered. The two DTLS 1.3 ClientHello fragmentation tests share one group choice that falls back to SecP384r1MLKEM1024 and skips when no group fits.Default and
--enable-allbuilds behave the same; only builds where a group's ECC part is missing change, from a lateNOT_COMPILED_IN/BAD_FUNC_ARGat key generation toBAD_FUNC_ARGfrom the API call.Testing
--enable-allmake checkpassesmake checkpasses--enable-all+ Kyber + extra PQC hybrids +WOLFSSL_ML_KEM_USE_OLD_IDS--with-eccminsz=384, DTLS 1.3 + frag CH (unit.test --api)NO_ECC_SECP, ML-KEM hybrids (--api)The ML-KEM-only and Brainpool rows are from the first revision; the library change is unchanged since. At eccminsz 384 the six tests fixed are
test_wolfSSL_UseSupportedCurve,test_tls13_pq_groups, the hybrid truncated and malformed tests and both DTLS 1.3 fragmented-ClientHello tests. The remaining failures there come from the P-256 test certificates and keys.Test changes:
test_wolfSSL_UseSupportedCurve: expectsBAD_FUNC_ARGfor secp256r1 and the SECP hybrid when the curve is not compiled in.test_tls13_apis:wolfSSL_UseKeyShare()with secp256r1 and no ECC now returnsBAD_FUNC_ARGinstead ofNOT_COMPILED_IN.test_wolfSSL_dh_ctx_params_reuse: offers the first compiled-in group of secp256r1, X25519 and ML-KEM-768 instead of secp256r1 alone.