lib/crypto: mldsa: Address some minor nits
Subsystems (2)
Eric Biggers
maintainercover lib/crypto: mldsa: Address some minor nits
A few pedantic fixes for issues found by LLMs.
Eric Biggers (3):
lib/crypto: mldsa: Check pk_len before doing pointer arithmetic lib/crypto: mldsa: Explicitly check that 'alg' is a valid value lib/crypto: mldsa: Add __must_check to mldsa_verify()
include/crypto/mldsa.h | 6 ++++-- lib/crypto/mldsa.c | 15 +++++++++++---- lib/crypto/tests/mldsa_kunit.c | 9 +++++++++ 3 files changed, 24 insertions(+), 6 deletions(-)
base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
Signature · 1 line
2.56.0
Eric Biggers
maintainerPATCH 1/3 lib/crypto: mldsa: Check pk_len before doing pointer arithmetic
Though common in the kernel and benign in practice (not even triggering UBSAN warnings), taking an address past the end of an array is undefined behavior according to the C specification.
Therefore, compute &pk[RHO_LEN] only after pk_len has been validated, since pk_len could be less than RHO_LEN if an incorrectly sized public key is provided.
(Also fix a typo that isn't worth its own commit.)
--- lib/crypto/mldsa.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
lib/crypto/mldsa.c +3 -2
@@ -306,7 +306,7 @@ static const u8 *decode_t1_elem(struct mldsa_ring_elem *out,* Reference: FIPS 204 Algorithm 27, sigDecode. * * This also validates that the coefficients of z are in range, corresponding -* the infinity norm check at the end of Algorithm 8, ML-DSA.Verify_internal. +* to the infinity norm check at the end of Algorithm 8, ML-DSA.Verify_internal. * * Finally, this also converts z to NTT form. */
@@ -533,7 +533,7 @@ int mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,/* For now this just does pure ML-DSA with an empty context string. */ static const u8 msg_prefix[2 ] = {/* dom_sep= */ 0 ,/* ctx_len= */ 0 };const u8 *ctilde;/* The signer's commitment hash */ -const u8 *t1_encoded = &pk[RHO_LEN ];/* Next encoded element of t_1 */ +const u8 *t1_encoded;/* Next encoded element of t_1 */ u8 *h;/* The signer's hint vector, length k * N */ size_t w1_enc_len;
@@ -587,6 +587,7 @@ int mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,* The computation is the same for each of the k rows. Just do each row * before moving on to the next, resulting in only one loop over k. */ + t1_encoded = &pk[RHO_LEN ];for (int i =0 ; i < k; i++) {/* * tmp = NTT(A) * NTT(z) * 2^-32
Signature · 1 line
2.56.0
Eric Biggers
maintainerPATCH 2/3 lib/crypto: mldsa: Explicitly check that 'alg' is a valid value
In reply to Eric BiggersC doesn't guarantee at the language level that an enum actually has one of the declared values.
For robustness, make mldsa_verify() handle invalid values of 'alg'.
No effect on current callers, who always pass a valid value for 'alg'. (Except for the KUnit test case that this commit adds.)
--- include/crypto/mldsa.h | 1 + lib/crypto/mldsa.c | 10 ++++++++-- lib/crypto/tests/mldsa_kunit.c | 9 +++++++++ 3 files changed, 18 insertions(+), 2 deletions(-)
include/crypto/mldsa.h +1 -0
@@ -48,6 +48,7 @@ enum mldsa_alg {* Return: * * 0 if the signature is valid * * -EBADMSG if the signature and/or public key is malformed +* * -EINVAL if @alg is not a valid value * * -EKEYREJECTED if the signature is invalid but otherwise well-formed * * -ENOMEM if out of memory so the validity of the signature is unknown */
lib/crypto/mldsa.c +8 -2
@@ -528,8 +528,8 @@ static size_t encode_w1(u8 out[MAX_W1_ENCODED_LEN],int mldsa_verify (enum mldsa_alg alg,const u8 *sig,size_t sig_len,const u8 *msg,size_t msg_len,const u8 *pk,size_t pk_len) { -const struct mldsa_parameter_set *params = &mldsa_parameter_sets[alg]; -const int k = params->k, l = params->l; +const struct mldsa_parameter_set *params; +int k, l;/* For now this just does pure ML-DSA with an empty context string. */ static const u8 msg_prefix[2 ] = {/* dom_sep= */ 0 ,/* ctx_len= */ 0 };const u8 *ctilde;/* The signer's commitment hash */
@@ -537,6 +537,12 @@ int mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,u8 *h;/* The signer's hint vector, length k * N */ size_t w1_enc_len; +if (alg !=MLDSA44 && alg !=MLDSA65 && alg !=MLDSA87 ) +return -EINVAL ; + params = &mldsa_parameter_sets[alg]; + k = params->k; + l = params->l; +/* Validate the public key and signature lengths. */ if (pk_len != params->pk_len || sig_len != params->sig_len)return -EBADMSG ;
lib/crypto/tests/mldsa_kunit.c +9 -0
@@ -372,6 +372,14 @@ static void test_mldsa_use_hint(struct kunit *test) } } +static void test_mldsa_invalid_alg (struct kunit *test) +{ +KUNIT_ASSERT_EQ (test, -EINVAL , +mldsa_verify (MLDSA44 -1 ,NULL ,0 ,NULL ,0 ,NULL ,0 )); +KUNIT_ASSERT_EQ (test, -EINVAL , +mldsa_verify (MLDSA87 +1 ,NULL ,0 ,NULL ,0 ,NULL ,0 )); +} +static void benchmark_mldsa (struct kunit *test,const struct mldsa_testvector *tv) {
@@ -414,6 +422,7 @@ static struct kunit_case mldsa_kunit_cases[] = {KUNIT_CASE (test_mldsa65),KUNIT_CASE (test_mldsa87),KUNIT_CASE (test_mldsa_use_hint), +KUNIT_CASE (test_mldsa_invalid_alg),KUNIT_CASE (benchmark_mldsa44),KUNIT_CASE (benchmark_mldsa65), KUNIT_CASE(benchmark_mldsa87),
Signature · 1 line
2.56.0
Eric Biggers
maintainerPATCH 3/3 lib/crypto: mldsa: Add __must_check to mldsa_verify()
In reply to Eric BiggersSignature verification status always must be checked. Add __must_check, for consistency with e.g. the AES-GCM and ChaCha20Poly1305 functions.
No effect on current callers, who always check the return value.
--- include/crypto/mldsa.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
include/crypto/mldsa.h +3 -2
@@ -52,8 +52,9 @@ enum mldsa_alg {* * -EKEYREJECTED if the signature is invalid but otherwise well-formed * * -ENOMEM if out of memory so the validity of the signature is unknown */ -int mldsa_verify (enum mldsa_alg alg,const u8 *sig,size_t sig_len, -const u8 *msg,size_t msg_len,const u8 *pk,size_t pk_len); +int __must_check mldsa_verify (enum mldsa_alg alg,const u8 *sig,size_t sig_len, +const u8 *msg,size_t msg_len,const u8 *pk, +size_t pk_len);#if IS_ENABLED (CONFIG_CRYPTO_LIB_MLDSA_KUNIT_TEST )/* Internal function, exposed only for unit testing */
Signature · 1 line
2.56.0
Quoted 7 lines · Eric Biggers
A few pedantic fixes for issues found by LLMs.
Eric Biggers (3):
lib/crypto: mldsa: Checkpk_lenbefore doing pointer arithmetic lib/crypto: mldsa: Explicitly check that 'alg' is a valid value lib/crypto: mldsa: Add__must_checktomldsa_verify()
Quoted 6 lines
include/crypto/mldsa.h | 6 ++++-- lib/crypto/mldsa.c | 15 +++++++++++---- lib/crypto/tests/mldsa_kunit.c | 9 +++++++++ 3 files changed, 24 insertions(+), 6 deletions(-)
Quoted 10 lines · Ard Biesheuvel
Quoted 7 lines · Eric Biggers
A few pedantic fixes for issues found by LLMs.
Eric Biggers (3):
lib/crypto: mldsa: Checkpk_lenbefore doing pointer arithmetic lib/crypto: mldsa: Explicitly check that 'alg' is a valid value lib/crypto: mldsa: Add__must_checktomldsa_verify()
Quoted 8 lines
Quoted 6 lines
include/crypto/mldsa.h | 6 ++++-- lib/crypto/mldsa.c | 15 +++++++++++---- lib/crypto/tests/mldsa_kunit.c | 9 +++++++++ 3 files changed, 24 insertions(+), 6 deletions(-)Reviewed-by Ard Biesheuvel <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-fixes
- Eric