Repository navigation
Conversation
With WOLFSSL_SP_MATH_ALL, wc_ecc_mulmod_ex() always runs the generic point multiplication; only WOLFSSL_SP_MATH builds hand it to the SP code. Key generation, signing, verification and shared secrets reach the SP code by their own paths, but callers of the public point API do not, so for example hostapd/wpa_supplicant's WPA3-SAE got no benefit from SP ECC. Use the SP code when the curve is one it implements and the result is mapped back to affine coordinates. Only the modulus and a are passed in, and other curves of the same size may be compiled in, so the curve is identified by both rather than by its bit length. Add an API test that checks k*G against the public key on every curve compiled in, including the Koblitz and Brainpool curves of the same sizes.
4 tasks
|
Member
Author
|
@claude review |
wolfSSL-Fenrir-bot
requested changes
Oct 28, 2026
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11912
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-bugs
Not reviewed (no scan target covers them): tests/api/test_ecc.h
Coverage: 1 of 2 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_ecc.c
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
Review tier: Lite
The SP code takes a scalar and coordinates no wider than the curve and truncates anything wider, so a base point with a coordinate not reduced modulo the prime gave a wrong result, where the generic code reduces it. Use the SP code only for a non-negative scalar and non-negative coordinates below the modulus; leave anything else to the generic code. Test it with x + prime in place of the base point's x.
The generic code does not handle an unreduced P-521 coordinate correctly: y + prime gives a wrong point every time and x + prime now and then, so test_wc_ecc_mulmod_curves failed intermittently. That is independent of the SP dispatch. An unreduced z still exercises the fallback on every curve (the SP code would truncate it) and the generic code handles it.
1 of 4 tasks
# Conflicts: # tests/api/test_ecc.c # tests/api/test_ecc.h
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
With
WOLFSSL_SP_MATH_ALL,wc_ecc_mulmod_ex()always runs the generic point multiplication: the dispatch tosp_ecc_mulmod_256/384/521()is only in theWOLFSSL_SP_MATHbranch. wolfSSL's own key generation, sign, verify and shared secret functions reach the SP code by their own paths, so this only shows up for callers of the public point API. hostapd/wpa_supplicant's WPA3-SAE is one: it callswc_ecc_mulmod()directly, so building with SP ECC did not speed SAE up at all.This makes the generic
wc_ecc_mulmod_ex()use the SP code when:a(other curves of the same size, such as secp256k1 or brainpoolP256r1, may be compiled in, so the bit length alone is not enough);kis no longer than the curve;mapset), since without mapping callers expect the generic code's representation.Otherwise the generic code runs as before.
WOLFSSL_SP_MATHbuilds are unchanged.Measured on a MIPS 24Kc at 650 MHz (32-bit C SP code,
--enable-sp=ec256), with hostapd and wpa_supplicant from the same build doing SAE on P-256, median of 10 connections:The "before" row is slower than the generic build because SP P-256 also drops
sp_int.c's fixed size 256-bit routines, which SAE's field arithmetic still uses; #11911 addresses that separately.Testing
test_wc_ecc_mulmod_curves: for every curve compiled in (P-256/384/521, secp256k1, brainpoolP256r1/384r1/512r1),wc_ecc_mulmod()of the private key and the base point must equal the public key. With the curve identification removed (dispatch by bit length only), it fails on the non-NIST 256-bit curves, as expected../tests/unit.test --api --group eccandtestwolfcryptpass, on macOS arm64, with:--enable-sp=ec256 --disable-sp-asm --enable-ecccustcurves=all--enable-sp=ec256,ec384,ec521 --enable-sp-asm --enable-ecccustcurves=all--enable-sp=smallec256,smallec384,smallec521 --disable-sp-asm --enable-ecccustcurves=all--enable-sp=ec256 --enable-fpecc --enable-smallstack --enable-ecccustcurves=all--enable-sp-math --enable-sp=yes,384,521--disable-sp --enable-ecccustcurves=all--enable-sp=ec256and--enable-sp=ec256,ec384,ec521):testwolfcryptpasses, and WPA3-SAE and OWE on P-256, P-384 and P-521 interoperate with a hostapd/wpa_supplicant built against another crypto library, in both directions (SAE with hunting-and-pecking only and with hash-to-element only).Checklist