From 4308349c01a33c5d9483add5f562be306d5b8138 Mon Sep 17 00:00:00 2001 From: kaleb-himes Date: Mon, 31 Aug 2026 16:57:13 -0600 Subject: [PATCH] Trim FIPS comments and share one ML-DSA seed entry point --- tests/api/test_ossl_rsa.c | 16 ++++---- wolfcrypt/benchmark/fips_cast_bench.c | 31 +++++--------- wolfcrypt/src/rsa.c | 29 ++++++------- wolfcrypt/src/wc_mldsa.c | 59 +++++++++++++-------------- wolfcrypt/src/wc_mlkem.c | 4 +- wolfcrypt/src/wc_slhdsa.c | 9 ++-- 6 files changed, 67 insertions(+), 81 deletions(-) diff --git a/tests/api/test_ossl_rsa.c b/tests/api/test_ossl_rsa.c index eb8c22fecb..7c30e5ce07 100644 --- a/tests/api/test_ossl_rsa.c +++ b/tests/api/test_ossl_rsa.c @@ -488,6 +488,13 @@ int test_wolfSSL_RSA_print(void) return EXPECT_RESULT(); } + +int test_wolfSSL_RSA_padding_add_PKCS1_PSS(void) +{ + EXPECT_DECLS; +#ifndef NO_RSA +#if defined(OPENSSL_ALL) && defined(WC_RSA_PSS) && !defined(WC_NO_RNG) +#if !defined(HAVE_FIPS) || (defined(HAVE_FIPS_VERSION) && (HAVE_FIPS_VERSION>2)) /* These ask for the longest salt the modulus allows: 222 bytes here, once * long salts are compiled in. FIPS 186-5 sec 5.4(g) caps the salt at the * hash length, so a v7 module must refuse it and a success is the defect. @@ -498,13 +505,6 @@ int test_wolfSSL_RSA_print(void) #else #define TEST_PSS_MAX_SALT_RESULT 1 #endif - -int test_wolfSSL_RSA_padding_add_PKCS1_PSS(void) -{ - EXPECT_DECLS; -#ifndef NO_RSA -#if defined(OPENSSL_ALL) && defined(WC_RSA_PSS) && !defined(WC_NO_RNG) -#if !defined(HAVE_FIPS) || (defined(HAVE_FIPS_VERSION) && (HAVE_FIPS_VERSION>2)) RSA *rsa = NULL; const unsigned char *derBuf = client_key_der_2048; unsigned char em[256] = {0}; /* len = 2048/8 */ @@ -570,6 +570,7 @@ int test_wolfSSL_RSA_padding_add_PKCS1_PSS(void) RSA_PSS_SALTLEN_MAX), TEST_PSS_MAX_SALT_RESULT); ExpectIntEQ(RSA_verify_PKCS1_PSS(rsa, mHash, EVP_sha256(), em, RSA_PSS_SALTLEN_MAX), TEST_PSS_MAX_SALT_RESULT); +#undef TEST_PSS_MAX_SALT_RESULT ExpectIntEQ(RSA_padding_add_PKCS1_PSS(rsa, em, mHash, EVP_sha256(), 10), 1); ExpectIntEQ(RSA_verify_PKCS1_PSS(rsa, mHash, EVP_sha256(), em, 10), 1); @@ -580,7 +581,6 @@ int test_wolfSSL_RSA_padding_add_PKCS1_PSS(void) #endif return EXPECT_RESULT(); } -#undef TEST_PSS_MAX_SALT_RESULT int test_wolfSSL_RSA_sign_sha3(void) { diff --git a/wolfcrypt/benchmark/fips_cast_bench.c b/wolfcrypt/benchmark/fips_cast_bench.c index 1b05100e47..1c6b2d2d3a 100644 --- a/wolfcrypt/benchmark/fips_cast_bench.c +++ b/wolfcrypt/benchmark/fips_cast_bench.c @@ -20,11 +20,10 @@ */ /* Times the module's self-tests so an operator can budget start-up on slow - * hardware. Two kinds, costing very different amounts: - * per-algorithm known-answer tests, run once each (FIPS 140-3 IG 10.3.A) - * -p key-pair tests, run on EVERY key generation - * (ISO/IEC 19790:2012 sec 7.10.3.3) - */ + * hardware. Two kinds: + * known-answer tests, once per algorithm (FIPS 140-3 IG 10.3.A) + * -p key-pair tests, on EVERY key generation + * (ISO/IEC 19790:2012 sec 7.10.3.3) */ #ifdef HAVE_CONFIG_H #include @@ -192,17 +191,13 @@ static int run_one_cast(int id, int iters, } -/* Pairwise consistency tests. - * - * A CAST runs once at start-up. This test runs on every key generation, so - * the application keeps paying it and none of it shows in the CAST numbers. +/* Pairwise consistency tests. A CAST runs once at start-up; these run on + * every key generation, so none of the cost shows in the CAST numbers. * ISO/IEC 19790:2012 sec 7.10.3.3. * * KeyGen+PCT what a caller pays today. MEASURED. * PCT alone the same test repeated on the finished key. MEASURED. - * KeyGen raw the difference. DERIVED: no build generates a key without - * the test, so it cannot be measured directly. - */ + * KeyGen raw the difference. DERIVED: no build skips the test. */ #define BENCH_PCT_DEFAULT_ITERS 1 @@ -599,14 +594,10 @@ static int bench_pct_slhdsa(int iters) double a = a_s / (double)iters * 1000.0; double c = c_s / (double)iters * 1000.0; - /* The elected test must be the cheap one. If someone puts sign - * and verify back inside key generation, KeyGen+PCT swallows it - * and stops being the smaller number. - * - * Demand a 2x margin rather than a bare comparison: the elected - * option measures 3.8x to 13.8x cheaper, while a reverted one - * costs about 1.1x sign+verify, so 2x separates them with room - * for a loaded machine to move both numbers. */ + /* The elected test must stay the cheap one. A 2x margin, not a + * bare comparison: elected measures 3.8x to 13.8x cheaper and a + * reverted one costs about 1.1x sign+verify, so 2x separates + * them even on a loaded machine. */ if (a > (kg * 2.0)) { printf("%-15s | %10.3f | %8.3f | %7.4f | %8.1fx\n", names[t], kg, a, c, (kg > 0.0) ? (a / kg) : 0.0); diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index 4ea3b22140..4861ba129c 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -159,15 +159,11 @@ static void wc_RsaCleanup(RsaKey* key) #ifndef WOLFSSL_RSA_PUBLIC_ONLY #if FIPS_VERSION3_GE(7,0,0) /* Erase the recovered plaintext on the way out, success or failure. - * SP 800-56B Rev2 sec 7.2.2.4. - * - * Only a buffer we allocated: when the caller supplies its own, - * key->data points at it (dataIsAlloc is 0) and erasing would destroy - * the answer. No key->type test: it only ever holds RSA_PRIVATE, - * RSA_PUBLIC or RSA_TYPE_UNKNOWN, never RSA_PRIVATE_DECRYPT (3) or - * RSA_PRIVATE_ENCRYPT (2), which belong to the operation-type family - * sharing that enum (rsa.h:176-183). Comparing against them is - * always false, so the buffer was being freed unwiped. */ + * SP 800-56B Rev2 sec 7.2.2.4. Only a buffer we allocated: a + * caller-supplied one is the answer itself. No key->type test: + * it never holds RSA_PRIVATE_DECRYPT/ENCRYPT, which belong to the + * operation-type half of that enum (rsa.h:176-183), so the old test + * was always false and the buffer was freed unwiped. */ if (key->dataIsAlloc && key->data != NULL && key->dataLen > 0) { ForceZero(key->data, key->dataLen); } @@ -945,8 +941,9 @@ int wc_CheckRsaKey(RsaKey* key) } /* Primes: right size, coprime to e, far enough apart, and actually - * prime (steps 5a to 5g). p alone first, then p with q so the - * |p - q| separation can be tested. */ + * prime (steps 5a to 5g). Two calls because steps 5f/5g want a + * primality test on each prime: the first tests p, the second tests + * q and the |p - q| separation. */ if (ret == 0) { ret = _CheckProbablePrime(&key->p, NULL, &key->e, nBits, &isPrime, rng); @@ -986,12 +983,10 @@ int wc_CheckRsaKey(RsaKey* key) /* Check dP, dQ and u if they exist */ if (ret == 0 && !mp_iszero(&key->dP)) { #if FIPS_VERSION3_GE(7,0,0) - /* Guarded on the version alone, unlike the block above: this needs - * no WOLFSSL_KEY_GEN because it calls no key-generation helper. - * - * Each CRT component must be greater than 1. - * SP 800-56B Rev2 sec 6.4.1.4.3 item F, steps 7a/7b/7c. - * Their upper bounds are checked just below. */ + /* Each CRT component must be greater than 1; upper bounds are + * checked just below. SP 800-56B Rev2 sec 6.4.1.4.3 item F, steps + * 7a/7b/7c. No WOLFSSL_KEY_GEN in the guard: unlike the block + * above this calls no key-generation helper. */ if ((mp_cmp_d(&key->dP, 1) != MP_GT) || (mp_cmp_d(&key->dQ, 1) != MP_GT) || (mp_cmp_d(&key->u, 1) != MP_GT)) { diff --git a/wolfcrypt/src/wc_mldsa.c b/wolfcrypt/src/wc_mldsa.c index b0edc7ad94..6dab0496fa 100644 --- a/wolfcrypt/src/wc_mldsa.c +++ b/wolfcrypt/src/wc_mldsa.c @@ -9120,8 +9120,7 @@ static int mldsa_make_key_from_seed(wc_MlDsaKey* key, const byte* seed) key-pair test required by ISO/IEC 19790:2012 sec 7.10.3.3" #endif /* Test every new key pair by signing and verifying with it. - * ISO/IEC 19790:2012 sec 7.10.3.3. Called from both generation paths. - * Fixed rnd because the decode path has no RNG. + * ISO/IEC 19790:2012 sec 7.10.3.3. Fixed rnd: no RNG on this path. * * @param [in, out] key ML-DSA key pair to test. Freed on failure. * @return 0 on success. @@ -11290,7 +11289,9 @@ int wc_MlDsaKey_MakeKey(wc_MlDsaKey* key, WC_RNG* rng) } /* No key-pair test here: wc_MlDsaKey_MakeKeyFromSeed(), reached from - * mldsa_make_key() above, already runs it on every generation path. */ + * mldsa_make_key() above, already runs it on every generation path. + * Guarded on the version, not HAVE_FIPS: src/include.am only compiles + * this file under BUILD_FIPS_V7_PLUS, so the two are equivalent here. */ return ret; } @@ -11302,37 +11303,45 @@ int wc_MlDsaKey_MakeKey(wc_MlDsaKey* key, WC_RNG* rng) * @return 0 on success. * @return BAD_FUNC_ARG when key or seed is NULL. * @return BAD_STATE_E when the parameters have not been set. - * @return ML_DSA_PCT_E when the key pair fails its consistency test. The - * key is freed in that case and must be re-initialised before reuse. + * @return ML_DSA_PCT_E on a failed test; the key is freed and must be + * re-initialised before reuse. */ -int wc_MlDsaKey_MakeKeyFromSeed(wc_MlDsaKey* key, const byte* seed) +/* Expand a seed into a key pair, validating arguments first. + * + * runPct is 0 for the ASN.1 decode path: FIPS 140-3 IG 10.3.A Additional + * Comment 1 does not require a key-pair test on a key imported from outside + * the module. + */ +static int mldsa_key_from_seed_checked(wc_MlDsaKey* key, const byte* seed, + int runPct) { int ret = 0; - /* Validate parameters. */ if ((key == NULL) || (seed == NULL)) { ret = BAD_FUNC_ARG; } - - if (ret == 0) { - /* Check the level or parameters have been set. */ - if (key->params == NULL) { - ret = BAD_STATE_E; - } - else { - /* Make the key. */ - ret = mldsa_make_key_from_seed(key, seed); - } + else if (key->params == NULL) { + ret = BAD_STATE_E; + } + else { + ret = mldsa_make_key_from_seed(key, seed); } #if FIPS_VERSION3_GE(7,0,0) - if (ret == 0) { + if ((ret == 0) && runPct) { ret = mldsa_pct(key); } +#else + (void)runPct; #endif return ret; } + +int wc_MlDsaKey_MakeKeyFromSeed(wc_MlDsaKey* key, const byte* seed) +{ + return mldsa_key_from_seed_checked(key, seed, 1); +} #endif #ifndef WOLFSSL_MLDSA_NO_SIGN @@ -13441,18 +13450,8 @@ int wc_MlDsaKey_PrivateKeyDecode(wc_MlDsaKey* key, const byte* input, if (seedLen != 0) { #if !defined(WOLFSSL_MLDSA_NO_MAKE_KEY) if (seedLen == MLDSA_SEED_SZ) { - /* No key-pair test: FIPS 140-3 IG 10.3.A Additional Comment 1 - * does not require one for a key imported from outside the - * module. */ - if (seed == NULL) { - ret = BAD_FUNC_ARG; - } - else if (key->params == NULL) { - ret = BAD_STATE_E; - } - else { - ret = mldsa_make_key_from_seed(key, seed); - } + /* runPct 0: this is an import, not a generation. */ + ret = mldsa_key_from_seed_checked(key, seed, 0); } else { ret = ASN_PARSE_E; diff --git a/wolfcrypt/src/wc_mlkem.c b/wolfcrypt/src/wc_mlkem.c index 8224580377..a1424e7b17 100644 --- a/wolfcrypt/src/wc_mlkem.c +++ b/wolfcrypt/src/wc_mlkem.c @@ -708,7 +708,9 @@ int wc_MlKemKey_MakeKey(MlKemKey* key, WC_RNG* rng) } /* No key-pair test here: wc_MlKemKey_MakeKeyWithRandom(), called above, - * already runs it on every generation path. */ + * already runs it on every generation path. Guarded on the version, not + * HAVE_FIPS: src/include.am only compiles this file under + * BUILD_FIPS_V7_PLUS, so the two are equivalent here. */ /* Ensure seeds are zeroized. */ ForceZero((void*)rand, (word32)sizeof(rand)); diff --git a/wolfcrypt/src/wc_slhdsa.c b/wolfcrypt/src/wc_slhdsa.c index f34d8a28e5..ec9a6e9d2b 100644 --- a/wolfcrypt/src/wc_slhdsa.c +++ b/wolfcrypt/src/wc_slhdsa.c @@ -7133,11 +7133,10 @@ int wc_SlhDsaKey_MakeKeyWithRandom(SlhDsaKey* key, const byte* sk_seed, /* Test every new key pair. ISO/IEC 19790:2012 sec 7.10.3.3. Here * because every generation path reaches this function. * - * The PK.SEED check is the one FIPS 140-3 IG 10.3.A Additional Comment 1 - * names for SLH-DSA, but it always passes here: the public key is a - * slice of the private one. So the root recompute does the real work. - * It is the cheapest check that can fail; signing and verifying costs - * about ten times the key generation. */ + * PK.SEED is the check FIPS 140-3 IG 10.3.A Additional Comment 1 names, + * but it always passes here: the public key is a slice of the private + * one. So the root recompute does the real work, at a tenth the cost of + * signing and verifying. */ if (ret == 0) { byte n = key->params->n; byte pct_root[SLHDSA_MAX_N];