Thumb2 X25519: Fix to do full reduction#10727
Conversation
This fix when into ARM32 assembly it is now being added to Thumb2 assembly. Full reduction to ensure the number is in range at end of work.
|
Jenkins: retest this please agent offline |
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10727
No scan targets match the changed files in this PR. Review skipped.
There was a problem hiding this comment.
Pull request overview
Extends the existing ARM32 X25519 “full reduction” fix to the Thumb2 Curve25519 implementation, ensuring the field element is fully reduced (in-range) at the end of the scalar-multiply/inversion sequence.
Changes:
- Added a final constant-time reduction step after the last
fe_mul_opin the Thumb2.Simplementation. - Updated the corresponding Thumb2 inline-asm C implementation to match the assembly reduction logic.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
wolfcrypt/src/port/arm/thumb2-curve25519.S |
Adds final reduction logic to ensure the result is < p (2^255-19) before returning (both cache-resistant and non-cache-resistant paths). |
wolfcrypt/src/port/arm/thumb2-curve25519_c.c |
Mirrors the same reduction logic in the inline-asm variant to keep behavior consistent with the .S implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dgarske
left a comment
There was a problem hiding this comment.
Thumb2 X25519 full-reduction fix - STM32 hardware test
Validates wolfSSL PR #10727 ("Thumb2 X25519: Fix to do full reduction") on STM32 Cortex-M boards. Built bare-metal with the Thumb2 inline-asm config (WOLFSSL_ARMASM_THUMB2 + WOLFSSL_ARMASM_INLINE), running wolfcrypt_test.
Reproducing the bug
The default curve25519_test runs only one in-range X25519 vector and passes even with the bug. Defining CURVE25519_OVERFLOW_ALL_TESTS adds the boundary vectors whose shared secret lands in [p, 2^255); the buggy code then fails at test.c:43750 i=2. All results below use that define.
Results
PASS = CURVE25519 test passed! and ED25519 test passed!. Before = parent of the fix (buggy); After = fix applied.
| Board | Core | Before | After |
|---|---|---|---|
| NUCLEO-F767ZI | Cortex-M7 | CURVE25519 FAIL (i=2) |
PASS |
| NUCLEO-F439ZI | Cortex-M4F | CURVE25519 FAIL (i=2) |
PASS |
| NUCLEO-U385RG-Q | Cortex-M33F | - | PASS |
| NUCLEO-C5A3ZG | Cortex-M33F | - | PASS |
NO_VAR_ASSIGN_REG path (IAR/GHS operand binding)
Re-ran before/after with WOLFSSL_NO_VAR_ASSIGN_REG defined (the named-operand inline-asm path used by IAR/GHS). Scoped to the curve25519 translation unit, because a global define hits an unrelated pre-existing sp_cortexm.c ECC UNALIGNED UsageFault (CFSR=0x01000000) before curve25519 runs; the symmetric Thumb2 asm (AES/SHA/ChaCha/Poly) all passed under the global define.
| Board | Core | Before | After |
|---|---|---|---|
| NUCLEO-F767ZI | Cortex-M7 | CURVE25519 FAIL (i=2) |
PASS |
| NUCLEO-F439ZI | Cortex-M4F | CURVE25519 FAIL (i=2) |
PASS |
| NUCLEO-U385RG-Q | Cortex-M33F | - | PASS |
| NUCLEO-C5A3ZG | Cortex-M33F | - | PASS |
Description
This fix when into ARM32 assembly it is now being added to Thumb2 assembly. Full reduction to ensure the number is in range at end of work.
Testing
./autogen.sh
./configure --disable-shared LDFLAGS=--static --host=armv7m CC=arm-linux-gnueabi-gcc --enable-curve25519 --enable-armasm
./tests/unit.test -test_wc_curve25519_shared_secret_ex_kat
Test fails.
After fix, all tests pass.