From 3ae19b5ef4e82c09ee6cc9d93efa6d2e7612f933 Mon Sep 17 00:00:00 2001 From: Joe Doyle Date: Thu, 25 Jun 2026 15:57:19 -0400 Subject: [PATCH 1/2] Reject malformed ML-DSA signing inputs Reject short external-mu inputs before copying mu in internal signing and verification, and reject nonzero context length with a NULL context pointer at the domain-separation prefix boundary. Add focused unit regression tests for the malformed external-mu and domain-prefix cases without bundling unrelated SHAKE/output-cleanup tests. Verification: - pre-fix `make run_unit_44` failed after adding the targeted malformed-input regression test - make run_unit_44 - make run_unit - make run_func Co-authored-by: Codex Signed-off-by: Joe Doyle --- mldsa/src/sign.c | 58 ++++++++++++++++------ test/src/test_unit.c | 116 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 14 deletions(-) diff --git a/mldsa/src/sign.c b/mldsa/src/sign.c index 0b08e5f95d..f8020eb0cc 100644 --- a/mldsa/src/sign.c +++ b/mldsa/src/sign.c @@ -45,6 +45,7 @@ #define mld_validate_hash_length MLD_ADD_PARAM_SET(mld_validate_hash_length) #define mld_get_hash_oid MLD_ADD_PARAM_SET(mld_get_hash_oid) #define mld_H MLD_ADD_PARAM_SET(mld_H) +#define mld_prepare_verify_mu MLD_ADD_PARAM_SET(mld_prepare_verify_mu) #define mld_compute_pack_z MLD_ADD_PARAM_SET(mld_compute_pack_z) #define mld_attempt_signature_generation \ MLD_ADD_PARAM_SET(mld_attempt_signature_generation) MLD_CONTEXT_PARAMETERS_8 @@ -489,6 +490,36 @@ __contract__( } #endif /* !MLD_CONFIG_NO_SIGN_API || !MLD_CONFIG_NO_VERIFY_API */ +#if !defined(MLD_CONFIG_NO_VERIFY_API) +MLD_MUST_CHECK_RETURN_VALUE +static int mld_prepare_verify_mu(uint8_t mu[MLDSA_CRHBYTES], const uint8_t *m, + size_t mlen, const uint8_t *pre, size_t prelen, + const uint8_t pk[MLDSA_CRYPTO_PUBLICKEYBYTES], + int externalmu) +{ + MLD_ALIGN uint8_t hpk[MLDSA_CRHBYTES]; + + if (externalmu) + { + if (mlen != MLDSA_CRHBYTES) + { + return MLD_ERR_FAIL; + } + mld_memcpy(mu, m, MLDSA_CRHBYTES); + return 0; + } + + /* Compute CRH(H(rho, t1), pre, msg) */ + mld_H(hpk, MLDSA_TRBYTES, pk, MLDSA_CRYPTO_PUBLICKEYBYTES, NULL, 0, NULL, 0); + mld_H(mu, MLDSA_CRHBYTES, hpk, MLDSA_TRBYTES, pre, prelen, m, mlen); + + /* @[FIPS204, Section 3.6.3] Destruction of intermediate values. */ + mld_zeroize(hpk, sizeof(hpk)); + + return 0; +} +#endif /* !MLD_CONFIG_NO_VERIFY_API */ + #if !defined(MLD_CONFIG_NO_SIGN_API) /* Sampling y from counter kappa uses nonces kappa, ..., kappa+L-1, which fit in * uint16_t iff kappa <= UINT16_MAX - MLDSA_L. */ @@ -951,6 +982,11 @@ int mld_sign_signature_internal(uint8_t sig[MLDSA_CRYPTO_BYTES], size_t *siglen, { /* mu has been provided directly (external-mu variant; line 6 done by the * caller in a separate cryptographic module). */ + if (mlen != MLDSA_CRHBYTES) + { + ret = MLD_ERR_FAIL; + goto cleanup; + } mld_memcpy(mu, m, MLDSA_CRHBYTES); } @@ -1191,21 +1227,10 @@ int mld_sign_verify_internal(const uint8_t *sig, size_t siglen, goto cleanup; } - if (!externalmu) - { - /* Compute CRH(H(rho, t1), pre, msg) */ - MLD_ALIGN uint8_t hpk[MLDSA_CRHBYTES]; - mld_H(hpk, MLDSA_TRBYTES, pk, MLDSA_CRYPTO_PUBLICKEYBYTES, NULL, 0, NULL, - 0); - mld_H(mu, MLDSA_CRHBYTES, hpk, MLDSA_TRBYTES, pre, prelen, m, mlen); - - /* @[FIPS204, Section 3.6.3] Destruction of intermediate values. */ - mld_zeroize(hpk, sizeof(hpk)); - } - else + ret = mld_prepare_verify_mu(mu, m, mlen, pre, prelen, pk, externalmu); + if (ret != 0) { - /* mu has been provided directly */ - mld_memcpy(mu, m, MLDSA_CRHBYTES); + goto cleanup; } /* Matrix-vector multiplication and per-row reconstruction of w1. */ @@ -1536,6 +1561,10 @@ size_t mld_prepare_domain_separation_prefix( { return 0; } + if (ctxlen > 0 && ctx == NULL) + { + return 0; + } if (hashalg != MLD_PREHASH_NONE) { @@ -1661,6 +1690,7 @@ int mld_sign_pk_from_sk(uint8_t pk[MLDSA_CRYPTO_PUBLICKEYBYTES], #undef mld_validate_hash_length #undef mld_get_hash_oid #undef mld_H +#undef mld_prepare_verify_mu #undef mld_compute_pack_z #undef mld_attempt_signature_generation #undef mld_compute_pack_t0_t1 diff --git a/test/src/test_unit.c b/test/src/test_unit.c index 87d3dbc2b4..8af58c1198 100644 --- a/test/src/test_unit.c +++ b/test/src/test_unit.c @@ -13,6 +13,7 @@ #include "../../mldsa/src/poly_kl.h" #include "../../mldsa/src/polyvec.h" #include "../../mldsa/src/polyvec_lazy.h" +#include "../../mldsa/src/sign.h" #ifndef NUM_RANDOM_TESTS #ifdef MLDSA_DEBUG @@ -1402,16 +1403,131 @@ static int test_polyvec_lazy_eager(void) } #endif /* !MLD_CONFIG_NO_SIGN_API */ +#if !defined(MLD_CONFIG_NO_KEYPAIR_API) && !defined(MLD_CONFIG_NO_SIGN_API) +static int test_signature_internal_rejects_short_external_mu(void) +{ + size_t siglen = 0; + uint8_t seed[MLDSA_SEEDBYTES] = {0}; + uint8_t rnd[MLDSA_RNDBYTES] = {0}; + uint8_t mu[MLDSA_CRHBYTES] = {0}; + uint8_t short_mu[1] = {0}; + MLD_ALLOC(pk, uint8_t, MLDSA_CRYPTO_PUBLICKEYBYTES, NULL); + MLD_ALLOC(sk, uint8_t, MLDSA_CRYPTO_SECRETKEYBYTES, NULL); + MLD_ALLOC(sig, uint8_t, MLDSA_CRYPTO_BYTES, NULL); + int ret = 1; + + if (pk == NULL || sk == NULL || sig == NULL) + { + goto cleanup; + } + + CHECK(mld_sign_keypair_internal(pk, sk, seed, NULL) == 0); + CHECK(mld_sign_signature_internal(sig, &siglen, short_mu, sizeof(short_mu), + NULL, 0, rnd, sk, 1, NULL) == MLD_ERR_FAIL); + CHECK(mld_sign_signature_internal(sig, &siglen, mu, sizeof(mu), NULL, 0, rnd, + sk, 1, NULL) == 0); + CHECK(mld_sign_verify_internal(sig, siglen, short_mu, sizeof(short_mu), NULL, + 0, pk, 1, NULL) == MLD_ERR_FAIL); + + ret = 0; + +cleanup: + MLD_FREE(sig, uint8_t, MLDSA_CRYPTO_BYTES, NULL); + MLD_FREE(sk, uint8_t, MLDSA_CRYPTO_SECRETKEYBYTES, NULL); + MLD_FREE(pk, uint8_t, MLDSA_CRYPTO_PUBLICKEYBYTES, NULL); + return ret; +} +#endif /* !MLD_CONFIG_NO_KEYPAIR_API && !MLD_CONFIG_NO_SIGN_API */ + +#if !defined(MLD_CONFIG_NO_SIGN_API) || !defined(MLD_CONFIG_NO_VERIFY_API) +#define TEST_PRE_HASH_OID_LEN 11 + +static int test_domain_separation_prefix_boundaries(void) +{ + static const struct + { + int hashalg; + size_t phlen; + } cases[] = { + {MLD_PREHASH_SHA2_224, 224 / 8}, {MLD_PREHASH_SHA2_256, 256 / 8}, + {MLD_PREHASH_SHA2_384, 384 / 8}, {MLD_PREHASH_SHA2_512, 512 / 8}, + {MLD_PREHASH_SHA2_512_224, 224 / 8}, {MLD_PREHASH_SHA2_512_256, 256 / 8}, + {MLD_PREHASH_SHA3_224, 224 / 8}, {MLD_PREHASH_SHA3_256, 256 / 8}, + {MLD_PREHASH_SHA3_384, 384 / 8}, {MLD_PREHASH_SHA3_512, 512 / 8}, + {MLD_PREHASH_SHAKE_128, 256 / 8}, {MLD_PREHASH_SHAKE_256, 512 / 8}, + }; + uint8_t prefix[MLD_DOMAIN_SEPARATION_MAX_BYTES]; + uint8_t ph[64]; + uint8_t ctx[256]; + size_t i, len; + + for (i = 0; i < sizeof(ph); i++) + { + ph[i] = (uint8_t)(0x80u + i); + } + + for (i = 0; i < sizeof(ctx); i++) + { + ctx[i] = (uint8_t)i; + } + + CHECK(mld_prepare_domain_separation_prefix(prefix, NULL, 0, NULL, 1, + MLD_PREHASH_NONE) == 0); + len = mld_prepare_domain_separation_prefix(prefix, NULL, 0, ctx, 255, + MLD_PREHASH_NONE); + CHECK(len == 2 + 255); + CHECK(prefix[0] == 0); + CHECK(prefix[1] == 255); + CHECK(memcmp(prefix + 2, ctx, 255) == 0); + CHECK(mld_prepare_domain_separation_prefix(prefix, NULL, 0, ctx, 256, + MLD_PREHASH_NONE) == 0); + + for (i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) + { + len = mld_prepare_domain_separation_prefix(prefix, ph, cases[i].phlen, ctx, + 3, cases[i].hashalg); + CHECK(len == 2 + 3 + TEST_PRE_HASH_OID_LEN + cases[i].phlen); + CHECK(prefix[0] == 1); + CHECK(prefix[1] == 3); + CHECK(memcmp(prefix + 2, ctx, 3) == 0); + CHECK(memcmp(prefix + 2 + 3 + TEST_PRE_HASH_OID_LEN, ph, cases[i].phlen) == + 0); + + CHECK(mld_prepare_domain_separation_prefix(prefix, ph, cases[i].phlen - 1, + ctx, 3, cases[i].hashalg) == 0); + CHECK(mld_prepare_domain_separation_prefix(prefix, ph, cases[i].phlen + 1, + ctx, 3, cases[i].hashalg) == 0); + CHECK(mld_prepare_domain_separation_prefix(prefix, NULL, cases[i].phlen, + ctx, 3, cases[i].hashalg) == 0); + } + + CHECK(mld_prepare_domain_separation_prefix(prefix, ph, sizeof(ph), ctx, 3, + MLD_PREHASH_SHAKE_256 + 1) == 0); + + return 0; +} + +#undef TEST_PRE_HASH_OID_LEN +#endif /* !MLD_CONFIG_NO_SIGN_API || !MLD_CONFIG_NO_VERIFY_API */ + /* Prototype for a re-#define'd main, to satisfy -Wmissing-prototypes. */ #if defined(main) int main(void); #endif + int main(void) { /* WARNING: Test-only * Normally, you would want to seed a PRNG with trustworthy entropy here. */ randombytes_reset(); +#if !defined(MLD_CONFIG_NO_KEYPAIR_API) && !defined(MLD_CONFIG_NO_SIGN_API) + CHECK(test_signature_internal_rejects_short_external_mu() == 0); +#endif +#if !defined(MLD_CONFIG_NO_SIGN_API) || !defined(MLD_CONFIG_NO_VERIFY_API) + CHECK(test_domain_separation_prefix_boundaries() == 0); +#endif + #if !defined(MLD_CONFIG_NO_SIGN_API) CHECK(test_polyvec_lazy_eager() == 0); #endif From 83c27aad387b95ecae474ee98fcf8d5bcfd33f5d Mon Sep 17 00:00:00 2001 From: Joe Doyle Date: Tue, 30 Jun 2026 12:51:16 -0400 Subject: [PATCH 2/2] Keep malformed-input unit test reduced-api safe Keep the signing-side short external-mu unit regression test enabled when the build exposes keygen and sign APIs but compiles out verification. Guard only the verify-side malformed external-mu assertion so reduced-api keygen+sign builds do not reference verify_internal. Verification: - pre-fix focused reduced-api unit check failed with an implicit declaration of verify_internal under MLD_CONFIG_NO_VERIFY_API - focused reduced-api unit check passed for keygen+sign/no-verify - focused reduced-api-reduce-ram unit check passed for keygen+sign/no-verify - nix develop .#ci -c lint Co-authored-by: Codex Signed-off-by: Joe Doyle --- test/src/test_unit.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/src/test_unit.c b/test/src/test_unit.c index 8af58c1198..ca33486b4a 100644 --- a/test/src/test_unit.c +++ b/test/src/test_unit.c @@ -1426,8 +1426,10 @@ static int test_signature_internal_rejects_short_external_mu(void) NULL, 0, rnd, sk, 1, NULL) == MLD_ERR_FAIL); CHECK(mld_sign_signature_internal(sig, &siglen, mu, sizeof(mu), NULL, 0, rnd, sk, 1, NULL) == 0); +#if !defined(MLD_CONFIG_NO_VERIFY_API) CHECK(mld_sign_verify_internal(sig, siglen, short_mu, sizeof(short_mu), NULL, 0, pk, 1, NULL) == MLD_ERR_FAIL); +#endif ret = 0;