Repository navigation
ecc: keep ECC_MAXSIZE at least MAX_ECC_BYTES - #11690
Merged
Merged
Conversation
ECC_MAXSIZE was fixed at 66 bytes without SAKKE, while MAX_ECC_BYTES follows MAX_ECC_BITS, which --with-max-ecc-bits and the CMake WOLFSSL_MAX_ECC_BITS option can raise to 1024. Such a build had MAX_ECC_BYTES of 128 against an ECC_MAXSIZE of 66, so curves the build was sized for were still refused by wc_ecc_set_curve() and the ECC_MAXSIZE buffers were smaller than the curve size. Grow ECC_MAXSIZE and ECC_MAXSIZE_GEN with MAX_ECC_BYTES when it exceeds 66, and add a static assert so the two cannot drift apart again, for example through a MAX_ECC_BITS override past SAKKE's 128 bytes.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused sizing change is consistent with existing ECC buffer usage and preserves default behavior.
0 open findings
What changed in this PR
Ensures ECC buffer sizing tracks configured maximum curve sizes.
Changes:
- Expands
ECC_MAXSIZEand generation buffers above 66 bytes. - Adds a compile-time sizing invariant.
| File | Description |
|---|---|
wolfssl/wolfcrypt/ecc.h |
Updates ECC size constants and adds a static assertion. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
SparkiDev
approved these changes
Oct 8, 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
Follow-up to review feedback on #11484:
MAX_ECC_BYTESshould never be larger thanECC_MAXSIZE, but nothing enforced that.Without SAKKE,
ECC_MAXSIZEis fixed at 66 bytes, whileMAX_ECC_BYTESfollowsMAX_ECC_BITS.--with-max-ecc-bitsand the CMakeWOLFSSL_MAX_ECC_BITSoption accept up to 1024, which givesMAX_ECC_BYTES= 128 againstECC_MAXSIZE= 66. In that buildwc_ecc_set_curve()still refuses curves the build was sized for, and theECC_MAXSIZEbuffers are smaller than the curve size.This grows
ECC_MAXSIZEandECC_MAXSIZE_GENwithMAX_ECC_BYTESwhen it exceeds 66, and addswc_static_assert(MAX_ECC_BYTES <= ECC_MAXSIZE)so the two cannot drift apart again (e.g. aMAX_ECC_BITSoverride past SAKKE's 128 bytes). Default builds are unchanged.Testing
--enable-ecccustcurves --with-max-ecc-bits=1024: the static assert alone fails to compile on master; with this change it builds, and the ecc API tests andtestwolfcryptpass.--enable-all(SAKKE) and a default./configure: build, ecc API tests andtestwolfcryptpass.