Regression testing fixes

wc_mlkem.h/test_mlkem.c: Respect WC_NO_CONSTRUCTORS guard.

settings.h, fe_operations.h: move WOLFSSL_CURVE25519_USE_ED25519 derivation into settings.h so the assembler sees it; fixes fe_cmov_table undefined on ARM32.

ge_448.c: shift the product instead of the byte in six sc448_* loops, dodging a GCC ARM32 NEON miscompile that produced wrong ed448 signatures; table shrunk [56]→[28].
pull/11001/head
Sean Parkinson 2026-07-29 14:59:47 +10:00
parent acff4d62a1
commit 119901c227
5 changed files with 85 additions and 54 deletions

View File

@ -4229,7 +4229,9 @@ int test_wc_MlkemDecisionCoverage(void)
EXPECT_DECLS;
#if defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_NO_ML_KEM)
MlKemKey* key = NULL;
#ifndef WC_NO_CONSTRUCTORS
MlKemKey* newKey = NULL;
#endif
WC_RNG rng;
word32 len = 0;
byte out[WC_ML_KEM_MAX_PRIVATE_KEY_SIZE];
@ -4266,12 +4268,14 @@ int test_wc_MlkemDecisionCoverage(void)
/* Valid init: the false side of the guards, and the object under test. */
ExpectIntEQ(wc_MlKemKey_Init(key, t, NULL, INVALID_DEVID), 0);
#ifndef WC_NO_CONSTRUCTORS
/* --- wc_MlKemKey_New / _Delete. --- */
ExpectNull(wc_MlKemKey_New(-12345, NULL, INVALID_DEVID));
ExpectNotNull(newKey = wc_MlKemKey_New(t, NULL, INVALID_DEVID));
ExpectIntEQ(wc_MlKemKey_Delete(NULL, &newKey),
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
ExpectIntEQ(wc_MlKemKey_Delete(newKey, &newKey), 0);
#endif /* !WC_NO_CONSTRUCTORS */
/* --- Size queries: (key == NULL) and (len == NULL) independence. --- */
ExpectIntEQ(wc_MlKemKey_CipherTextSize(NULL, &len),

View File

@ -85,8 +85,8 @@ static const word8 ed448_order[56] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x3f,
};
/* Part of order of ed448 that needs tp be multiplied when reducing */
static const word8 ed448_order_mul[56] = {
/* Part of order of ed448 that needs tp be multiplied when reducing. */
static const word8 ed448_order_mul[28] = {
0x0d, 0xbb, 0xa7, 0x54, 0x6d, 0x3d, 0x87, 0xdc, 0xaa, 0x70, 0x3a, 0x72,
0x8d, 0x3d, 0x93, 0xde, 0x6f, 0xc9, 0x29, 0x51, 0xb6, 0x24, 0xb1, 0x3b,
0x16, 0xdc, 0x35, 0x83,
@ -110,7 +110,8 @@ void sc448_reduce(byte* b)
}
for (i = 0; i < 58; i++) {
for (j = 0; j < 28; j++)
t[i+j] += b[i+56] * ((word32)ed448_order_mul[j] << 2);
/* Shift the product; shifting the byte miscompiles on ARM NEON. */
t[i+j] += ((word32)b[i+56] * ed448_order_mul[j]) << 2;
t[i+56] = 0;
}
for (i = 54; i < 87; i++) {
@ -119,7 +120,8 @@ void sc448_reduce(byte* b)
}
for (i = 0; i < 31; i++) {
for (j = 0; j < 28; j++)
t[i+j] += t[i+56] * ((word32)ed448_order_mul[j] << 2);
/* Shift the product; shifting the byte miscompiles on ARM NEON. */
t[i+j] += (t[i+56] * ed448_order_mul[j]) << 2;
t[i+56] = 0;
}
for (i = 54; i < 60; i++) {
@ -128,7 +130,8 @@ void sc448_reduce(byte* b)
}
for (i = 0; i < 4; i++) {
for (j = 0; j < 28; j++)
t[i+j] += t[i+56] * ((word32)ed448_order_mul[j] << 2);
/* Shift the product; shifting the byte miscompiles on ARM NEON. */
t[i+j] += (t[i+56] * ed448_order_mul[j]) << 2;
t[i+56] = 0;
}
for (i = 0; i < 55; i++) {
@ -176,7 +179,8 @@ void sc448_muladd(byte* r, const byte* a, const byte* b, const byte* d)
}
for (i = 0; i < 56; i++) {
for (j = 0; j < 28; j++)
t[i+j] += t[i+56] * ((word32)ed448_order_mul[j] << 2);
/* Shift the product; shifting the byte miscompiles on ARM NEON. */
t[i+j] += (t[i+56] * ed448_order_mul[j]) << 2;
t[i+56] = 0;
}
for (i = 54; i < 85; i++) {
@ -185,7 +189,8 @@ void sc448_muladd(byte* r, const byte* a, const byte* b, const byte* d)
}
for (i = 0; i < 29; i++) {
for (j = 0; j < 28; j++)
t[i+j] += t[i+56] * ((word32)ed448_order_mul[j] << 2);
/* Shift the product; shifting the byte miscompiles on ARM NEON. */
t[i+j] += (t[i+56] * ed448_order_mul[j]) << 2;
t[i+56] = 0;
}
for (i = 54; i < 58; i++) {
@ -194,7 +199,8 @@ void sc448_muladd(byte* r, const byte* a, const byte* b, const byte* d)
}
for (i = 0; i < 2; i++) {
for (j = 0; j < 28; j++)
t[i+j] += t[i+56] * ((word32)ed448_order_mul[j] << 2);
/* Shift the product; shifting the byte miscompiles on ARM NEON. */
t[i+j] += (t[i+56] * ed448_order_mul[j]) << 2;
t[i+56] = 0;
}
for (i = 0; i < 55; i++) {

View File

@ -29,52 +29,10 @@
#include <wolfssl/wolfcrypt/types.h>
#if defined(USE_INTEL_SPEEDUP) && defined(WOLFSSL_X86_64_BUILD) && \
!defined(NO_CURVED25519_X64)
#define CURVED25519_X64
#elif defined(HAVE___UINT128_T) && !defined(NO_CURVED25519_128BIT)
#define CURVED25519_128BIT
#endif
#if defined(CURVED25519_X64)
#define CURVED25519_ASM_64BIT
#define CURVED25519_ASM
#endif
/* The small (reduced-C) curve25519/ed25519 code and the Intel x64 assembly
* both provide the same fe_, sc_ and curve25519 symbols, so selecting both
* (for example a user_settings.h that keeps CURVE25519_SMALL/ED25519_SMALL
* while USE_INTEL_SPEEDUP enables the x64 assembly) produces duplicate-symbol
* link errors that are hard to diagnose. Detect the incompatible combination
* at compile time with a clear message instead. To keep the small
* implementation define NO_CURVED25519_X64; to use the assembly drop
* CURVE25519_SMALL / ED25519_SMALL. */
#if defined(CURVED25519_X64) && \
(defined(CURVE25519_SMALL) || defined(ED25519_SMALL))
#error "CURVE25519_SMALL/ED25519_SMALL are incompatible with the Intel x64 curve25519/ed25519 assembly (CURVED25519_X64); define NO_CURVED25519_X64 to keep the small implementation, or remove the SMALL settings to use the assembly"
#endif
#if defined(WOLFSSL_ARMASM)
#ifdef __aarch64__
#define CURVED25519_ASM_64BIT
#else
#define CURVED25519_ASM_32BIT
#endif
#define CURVED25519_ASM
#endif
/* curve25519 always uses its own field math, but on some builds it borrows
* ed25519's group math via WOLFSSL_CURVE25519_USE_ED25519 below. Under
* WOLF_CRYPTO_CB_ONLY_ED25519 ed25519's group math is removed. */
#if (defined(CURVED25519_ASM_64BIT) || defined(HAVE_ED25519)) && \
!defined(WOLFSSL_CURVE25519_BLINDING) && \
!defined(WOLFSSL_CURVE25519_NOT_USE_ED25519) && \
(!defined(WOLF_CRYPTO_CB_ONLY_ED25519) || \
(defined(HAVE_CURVE25519) && \
!defined(WOLF_CRYPTO_CB_ONLY_CURVE25519)))
#undef WOLFSSL_CURVE25519_USE_ED25519
#define WOLFSSL_CURVE25519_USE_ED25519
#endif
/* CURVED25519_X64, CURVED25519_128BIT, CURVED25519_ASM[_32BIT|_64BIT] and
* WOLFSSL_CURVE25519_USE_ED25519 are derived in settings.h, so that the
* generated assembly - which only ever sees settings.h - is guarded by the
* same macros as the C sources. */
/*
fe means field element.

View File

@ -4707,6 +4707,67 @@
#define WOLFSSL_CURVE25519_BLINDING
#endif
/* curve25519/ed25519 implementation selection.
*
* These are derived here rather than in fe_operations.h because the generated
* assembly is guarded by the same macros as the C sources, and a .S file only
* ever sees settings.h (through libwolfssl_sources_asm.h) - never a C header.
* Deriving WOLFSSL_CURVE25519_USE_ED25519 in fe_operations.h left the
* assembler with a different view of the configuration than the compiler, so
* ge_operations.c could call ed25519 group math (e.g. fe_cmov_table) that the
* assembly had guarded out. Must stay after WOLFSSL_CURVE25519_BLINDING
* above, which is an input to the selection. */
#if defined(HAVE_CURVE25519) || defined(HAVE_ED25519)
#if defined(USE_INTEL_SPEEDUP) && defined(WOLFSSL_X86_64_BUILD) && \
!defined(NO_CURVED25519_X64)
#define CURVED25519_X64
#elif defined(HAVE___UINT128_T) && !defined(NO_CURVED25519_128BIT)
#define CURVED25519_128BIT
#endif
#if defined(CURVED25519_X64)
#define CURVED25519_ASM_64BIT
#define CURVED25519_ASM
#endif
/* The small (reduced-C) curve25519/ed25519 code and the Intel x64 assembly
* both provide the same fe_, sc_ and curve25519 symbols, so selecting both
* (for example a user_settings.h that keeps CURVE25519_SMALL/ED25519_SMALL
* while USE_INTEL_SPEEDUP enables the x64 assembly) produces duplicate-symbol
* link errors that are hard to diagnose. Detect the incompatible combination
* at compile time with a clear message instead. To keep the small
* implementation define NO_CURVED25519_X64; to use the assembly drop
* CURVE25519_SMALL / ED25519_SMALL. */
#if defined(CURVED25519_X64) && \
(defined(CURVE25519_SMALL) || defined(ED25519_SMALL))
#error "CURVE25519_SMALL/ED25519_SMALL are incompatible with the Intel x64 curve25519/ed25519 assembly (CURVED25519_X64); define NO_CURVED25519_X64 to keep the small implementation, or remove the SMALL settings to use the assembly"
#endif
#if defined(WOLFSSL_ARMASM)
#ifdef __aarch64__
#define CURVED25519_ASM_64BIT
#else
#define CURVED25519_ASM_32BIT
#endif
#define CURVED25519_ASM
#endif
/* curve25519 always uses its own field math, but on some builds it borrows
* ed25519's group math via WOLFSSL_CURVE25519_USE_ED25519 below. Under
* WOLF_CRYPTO_CB_ONLY_ED25519 ed25519's group math is removed. */
#if (defined(CURVED25519_ASM_64BIT) || defined(HAVE_ED25519)) && \
!defined(WOLFSSL_CURVE25519_BLINDING) && \
!defined(WOLFSSL_CURVE25519_NOT_USE_ED25519) && \
(!defined(WOLF_CRYPTO_CB_ONLY_ED25519) || \
(defined(HAVE_CURVE25519) && \
!defined(WOLF_CRYPTO_CB_ONLY_CURVE25519)))
#undef WOLFSSL_CURVE25519_USE_ED25519
#define WOLFSSL_CURVE25519_USE_ED25519
#endif
#endif /* HAVE_CURVE25519 || HAVE_ED25519 */
/* warning for not using harden build options (default with ./configure) */
/* do not warn if big integer support is disabled */
#if !defined(WC_NO_HARDEN) && !defined(NO_BIG_INT)

View File

@ -425,8 +425,10 @@ struct MlKemKey {
#define WC_MLKEMKEY_TYPE_DEFINED
#endif
#ifndef WC_NO_CONSTRUCTORS
WOLFSSL_API MlKemKey* wc_MlKemKey_New(int type, void* heap, int devId);
WOLFSSL_API int wc_MlKemKey_Delete(MlKemKey* key, MlKemKey** key_p);
#endif /* !WC_NO_CONSTRUCTORS */
WOLFSSL_API int wc_MlKemKey_Init(MlKemKey* key, int type, void* heap,
int devId);