From 477754024dbac6f472148e2f3b7948300b552c2b Mon Sep 17 00:00:00 2001 From: aidan garske Date: Thu, 4 Jun 2026 15:24:10 -0700 Subject: [PATCH 1/3] ML-KEM: FIPS 203 modulus check - reject non-reduced private key vector on decode --- wolfcrypt/src/wc_mlkem.c | 21 ++++++++++++++++----- wolfcrypt/src/wc_mlkem_poly.c | 9 ++++++--- wolfcrypt/test/test.c | 16 +++++++++++++++- wolfssl/wolfcrypt/wc_mlkem.h | 2 +- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/wolfcrypt/src/wc_mlkem.c b/wolfcrypt/src/wc_mlkem.c index 89f647ebefb..e217e5ebedf 100644 --- a/wolfcrypt/src/wc_mlkem.c +++ b/wolfcrypt/src/wc_mlkem.c @@ -2043,7 +2043,9 @@ static void mlkemkey_decode_public(sword16* pub, byte* pubSeed, const byte* p, * @return BAD_FUNC_ARG when key or in is NULL. * @return NOT_COMPILED_IN when key type is not supported. * @return BUFFER_E when len is not the correct size. - * @return PUBLIC_KEY_E when public key data doesn't match parameters. + * @return PUBLIC_KEY_E when the private or public vector has a coefficient + * that is not reduced modulo q, or public key data doesn't match + * parameters. * @return MLKEM_PUB_HASH_E when public key hash doesn't match stored hash. * @return MEMORY_E when dynamic memory allocation failed. */ @@ -2130,15 +2132,24 @@ int wc_MlKemKey_DecodePrivateKey(MlKemKey* key, const unsigned char* in, } #endif if (ret == 0) { + /* Clear the key-set flags first so any failure below (size, reduction + * check, or hash) leaves a reused key object consistently unusable + * rather than flagged-set with zeroed material. */ + key->flags &= ~(MLKEM_FLAG_BOTH_SET | MLKEM_FLAG_H_SET); + /* Decode private key that is vector of polynomials. * Alg 18 Step 1: dk_PKE <- dk[0 : 384k] * Alg 15 Step 5: s_hat <- ByteDecode_12(dk_PKE) */ mlkem_from_bytes(key->priv, p, (int)k); p += k * WC_ML_KEM_POLY_SIZE; - /* Decode the public key that is after the private key. */ - mlkemkey_decode_public(key->pub, key->pubSeed, p, k); - ret = mlkem_check_public(key->pub, (int)k); + /* Both vectors must decode to coefficients reduced modulo q. */ + ret = mlkem_check_reduced(key->priv, (int)k); + if (ret == 0) { + /* Decode the public key that is after the private key. */ + mlkemkey_decode_public(key->pub, key->pubSeed, p, k); + ret = mlkem_check_reduced(key->pub, (int)k); + } if (ret != 0) { ForceZero(key->priv, k * MLKEM_N * sizeof(sword16)); } @@ -2263,7 +2274,7 @@ int wc_MlKemKey_DecodePublicKey(MlKemKey* key, const unsigned char* in, if (ret == 0) { /* Decode public key and check public key matches parameters. */ mlkemkey_decode_public(key->pub, key->pubSeed, p, k); - ret = mlkem_check_public(key->pub, (int)k); + ret = mlkem_check_reduced(key->pub, (int)k); } if (ret == 0) { /* Calculate public hash. */ diff --git a/wolfcrypt/src/wc_mlkem_poly.c b/wolfcrypt/src/wc_mlkem_poly.c index 1a0f3e2213d..aa3d7835d5d 100644 --- a/wolfcrypt/src/wc_mlkem_poly.c +++ b/wolfcrypt/src/wc_mlkem_poly.c @@ -6096,14 +6096,17 @@ void mlkem_to_bytes(byte* b, sword16* p, int k) } /** - * Check the public key values are smaller than the modulus. + * Check the vector coefficients are reduced modulo q. * - * @param [in] p Public key - vector. + * FIPS 203, Sections 7.2 and 7.3: encapsulation and decapsulation keys must + * decode to coefficients in Z_q; reject any that are not reduced. + * + * @param [in] p Key - vector of polynomials. * @param [in] k Number of polynomials in vector. * @return 0 when all values are in range. * @return PUBLIC_KEY_E when at least one value is out of range. */ -int mlkem_check_public(const sword16* p, int k) +int mlkem_check_reduced(const sword16* p, int k) { int ret = 0; int i; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index d76dd112c99..b004d3a7b73 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -52299,9 +52299,23 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t mlkem_test(void) if (ret != 0) ERROR_OUT(WC_TEST_RET_ENC_I(i), out); - if (XMEMCMP(priv, priv2, testData[i][2]) != 0) + if (XMEMCMP(priv, priv2, testData[i][1]) != 0) ERROR_OUT(WC_TEST_RET_ENC_I(i), out); + /* FIPS 203 modulus check: a private key whose first coefficient is + * not reduced (>= q) must be rejected on decode. Free first so the + * reinit does not leak the decoded dynamic priv/pub buffers. */ + wc_MlKemKey_Free(key); + ret = wc_MlKemKey_Init(key, testData[i][0], HEAP_HINT, devId); + if (ret != 0) + ERROR_OUT(WC_TEST_RET_ENC_I(i), out); + priv[0] = 0xff; + priv[1] |= 0x0f; + ret = wc_MlKemKey_DecodePrivateKey(key, priv, testData[i][1]); + if (ret != PUBLIC_KEY_E) + ERROR_OUT(WC_TEST_RET_ENC_I(i), out); + ret = 0; + #if !defined(WOLFSSL_NO_MALLOC) && !defined(WC_NO_CONSTRUCTORS) tmpKey = wc_MlKemKey_New(testData[i][0], HEAP_HINT, devId); if (tmpKey == NULL) diff --git a/wolfssl/wolfcrypt/wc_mlkem.h b/wolfssl/wolfcrypt/wc_mlkem.h index 4d02a6252a3..3dad3a5e942 100644 --- a/wolfssl/wolfcrypt/wc_mlkem.h +++ b/wolfssl/wolfcrypt/wc_mlkem.h @@ -576,7 +576,7 @@ void mlkem_from_bytes(sword16* p, const byte* b, int k); WOLFSSL_LOCAL void mlkem_to_bytes(byte* b, sword16* p, int k); WOLFSSL_LOCAL -int mlkem_check_public(const sword16* p, int k); +int mlkem_check_reduced(const sword16* p, int k); #ifdef USE_INTEL_SPEEDUP WOLFSSL_LOCAL From a1bdacbb837d0331706867ae86b665d03851962b Mon Sep 17 00:00:00 2001 From: aidan garske Date: Thu, 4 Jun 2026 15:36:32 -0700 Subject: [PATCH 2/3] ML-DSA: reject private key with out-of-range s1/s2 coefficients on decode --- wolfcrypt/src/wc_mldsa.c | 59 ++++++++++++++++++++++++++++++++++++++++ wolfcrypt/test/test.c | 19 +++++++++++++ 2 files changed, 78 insertions(+) diff --git a/wolfcrypt/src/wc_mldsa.c b/wolfcrypt/src/wc_mldsa.c index 73b7a9b1182..ad53f667e50 100644 --- a/wolfcrypt/src/wc_mldsa.c +++ b/wolfcrypt/src/wc_mldsa.c @@ -11669,6 +11669,55 @@ int wc_MlDsaKey_ImportPubRaw(wc_MlDsaKey* key, const byte* in, word32 inLen) #ifdef WOLFSSL_MLDSA_PRIVATE_KEY +/* Check the s1 and s2 vectors of a private key are in range [-eta, eta]. + * + * FIPS 204, Algorithm 25 skDecode: s1 and s2 are BitPack encodings of + * (eta - coeff), so each packed value must be no greater than 2*eta. Reject + * a private key with any value outside this range rather than silently + * accepting a non-conforming key. + * + * @param [in] p Encoded s1 followed by s2. + * @param [in] eta Coefficient range specifier (2 or 4). + * @param [in] len Number of encoded bytes covering s1 and s2. + * @return 0 when all values are in range. + * @return PUBLIC_KEY_E when at least one value is out of range. + */ +static int mldsa_check_eta_range(const byte* p, byte eta, word32 len) +{ + int ret = 0; + word32 i; + word32 j; + word32 bits; + byte max = (byte)(2 * eta); + + if (eta == MLDSA_ETA_4) { + /* 4 bits per coefficient, two coefficients per byte. */ + for (i = 0; i < len; i++) { + if (((p[i] & 0xf) > max) || ((p[i] >> 4) > max)) { + ret = PUBLIC_KEY_E; + break; + } + } + } + else { + /* 3 bits per coefficient, eight coefficients per three bytes. len + * (s1EncSz + s2EncSz) is always a multiple of 3, so no trailing + * partial group is skipped. */ + for (i = 0; (ret == 0) && (i + 3 <= len); i += 3) { + bits = (word32)p[i] | ((word32)p[i + 1] << 8) | + ((word32)p[i + 2] << 16); + for (j = 0; j < 8; j++) { + if (((bits >> (3 * j)) & 0x7) > max) { + ret = PUBLIC_KEY_E; + break; + } + } + } + } + + return ret; +} + /* Set the private key data into key. * * @param [in] priv Private key data. @@ -11677,6 +11726,7 @@ int wc_MlDsaKey_ImportPubRaw(wc_MlDsaKey* key, const byte* in, word32 inLen) * @return 0 on success. * @return BAD_FUNC_ARG when private key size is invalid. * @return MEMORY_E when dynamic memory allocation fails. + * @return PUBLIC_KEY_E when an s1 or s2 coefficient is out of range. * @return Other negative on hash error. */ static int mldsa_set_priv_key(const byte* priv, word32 privSz, @@ -11706,6 +11756,15 @@ static int mldsa_set_priv_key(const byte* priv, word32 privSz, } #endif + if (ret == 0) { + /* Reject a private key whose s1 or s2 coefficients are out of range + * before copying it in, so a failed import never overwrites an + * existing key or leaves the object in an inconsistent state. */ + const byte* s1p = priv + MLDSA_PUB_SEED_SZ + MLDSA_K_SZ + MLDSA_TR_SZ; + ret = mldsa_check_eta_range(s1p, key->params->eta, + (word32)key->params->s1EncSz + key->params->s2EncSz); + } + if (ret == 0) { /* Copy the private key data in or copy pointer. */ #ifdef WOLFSSL_MLDSA_ASSIGN_KEY diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index b004d3a7b73..38bfee7416b 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -55860,6 +55860,25 @@ static wc_test_ret_t test_mldsa_decode_level(const byte* rawKey, ret = WC_TEST_RET_ENC_NC; } #endif /* !WOLFSSL_MLDSA_FIPS204_DRAFT */ + +#ifdef WOLFSSL_MLDSA_PRIVATE_KEY + /* Negative: a private key with an out-of-range s1 coefficient must be + * rejected. s1 follows rho || K || tr; force its first byte out of range. */ + if ((ret == 0) && (!isPublicOnlyKey)) { + XMEMCPY(der, rawKey, rawKeySz); + der[MLDSA_PUB_SEED_SZ + MLDSA_K_SZ + MLDSA_TR_SZ] = 0xff; + wc_MlDsaKey_Free(key); + ret = wc_MlDsaKey_Init(key, NULL, devId); + if (ret == 0) { + ret = wc_MlDsaKey_SetParams(key, expectedLevel); + } + if (ret == 0) { + if (wc_MlDsaKey_ImportPrivRaw(key, der, rawKeySz) != PUBLIC_KEY_E) { + ret = WC_TEST_RET_ENC_NC; + } + } + } +#endif #endif /* !WOLFSSL_MLDSA_NO_ASN1 && WOLFSSL_ASN_TEMPLATE */ /* Cleanup */ From 514e39e2f5cd0f7f924f78cdad83e299de85a014 Mon Sep 17 00:00:00 2001 From: aidan garske Date: Thu, 11 Jun 2026 09:35:03 -0700 Subject: [PATCH 3/3] test: wrap PUBLIC_KEY_E comparisons in WC_NO_ERR_TRACE for ML-KEM/ML-DSA decode tests --- wolfcrypt/test/test.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 38bfee7416b..102964470d4 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -52312,7 +52312,7 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t mlkem_test(void) priv[0] = 0xff; priv[1] |= 0x0f; ret = wc_MlKemKey_DecodePrivateKey(key, priv, testData[i][1]); - if (ret != PUBLIC_KEY_E) + if (ret != WC_NO_ERR_TRACE(PUBLIC_KEY_E)) ERROR_OUT(WC_TEST_RET_ENC_I(i), out); ret = 0; @@ -55873,7 +55873,8 @@ static wc_test_ret_t test_mldsa_decode_level(const byte* rawKey, ret = wc_MlDsaKey_SetParams(key, expectedLevel); } if (ret == 0) { - if (wc_MlDsaKey_ImportPrivRaw(key, der, rawKeySz) != PUBLIC_KEY_E) { + if (wc_MlDsaKey_ImportPrivRaw(key, der, rawKeySz) != + WC_NO_ERR_TRACE(PUBLIC_KEY_E)) { ret = WC_TEST_RET_ENC_NC; } }