diff --git a/tests/api/test_mlkem.c b/tests/api/test_mlkem.c index db6644e6e4..759f25f644 100644 --- a/tests/api/test_mlkem.c +++ b/tests/api/test_mlkem.c @@ -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), diff --git a/wolfcrypt/src/ge_448.c b/wolfcrypt/src/ge_448.c index 643611f2fa..f3a895f23e 100644 --- a/wolfcrypt/src/ge_448.c +++ b/wolfcrypt/src/ge_448.c @@ -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++) { diff --git a/wolfssl/wolfcrypt/fe_operations.h b/wolfssl/wolfcrypt/fe_operations.h index bfac7c04f9..5c7654ce7e 100644 --- a/wolfssl/wolfcrypt/fe_operations.h +++ b/wolfssl/wolfcrypt/fe_operations.h @@ -29,52 +29,10 @@ #include -#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. diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index e0d13ec017..dbb847e23b 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -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) diff --git a/wolfssl/wolfcrypt/wc_mlkem.h b/wolfssl/wolfcrypt/wc_mlkem.h index edbc17a077..852b802f7b 100644 --- a/wolfssl/wolfcrypt/wc_mlkem.h +++ b/wolfssl/wolfcrypt/wc_mlkem.h @@ -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);