On Thu, Feb 26, 2026 at 10:05:38AM -0800, Eric Biggers wrote: > On Thu, Feb 26, 2026 at 02:09:47PM +0100, Geert Uytterhoeven wrote: > > On Sun, 14 Dec 2025 at 19:18, Eric Biggers <[email protected]> wrote: > > > Add a KUnit test suite for ML-DSA verification, including the following > > > for each ML-DSA parameter set (ML-DSA-44, ML-DSA-65, and ML-DSA-87): > > > > > > - Positive test (valid signature), using vector imported from leancrypto > > > - Various negative tests: > > > - Wrong length for signature, message, or public key > > > - Out-of-range coefficients in z vector > > > - Invalid encoded hint vector > > > - Any bit flipped in signature, message, or public key > > > - Unit test for the internal function use_hint() > > > - A benchmark > > > > > > ML-DSA inputs and outputs are very large. To keep the size of the tests > > > down, use just one valid test vector per parameter set, and generate the > > > negative tests at runtime by mutating the valid test vector. > > > > > > I also considered importing the test vectors from Wycheproof. I've > > > tested that mldsa_verify() indeed passes all of Wycheproof's ML-DSA test > > > vectors that use an empty context string. However, importing these > > > permanently would add over 6 MB of source. That's too much to be a > > > reasonable addition to the Linux kernel tree for one algorithm. It also > > > wouldn't actually provide much better test coverage than this commit. > > > Another potential issue is that Wycheproof uses the Apache license. > > > > > > Similarly, this also differs from the earlier proposal to import a long > > > list of test vectors from leancrypto. I retained only one valid > > > signature for each algorithm, and I also added (runtime-generated) > > > negative tests which were missing. I think this is a better tradeoff. > > > > > > Reviewed-by: David Howells <[email protected]> > > > Tested-by: David Howells <[email protected]> > > > Signed-off-by: Eric Biggers <[email protected]> > > > > Thanks for your patch, which is now commit ed894faccb8de55c > > ("lib/crypto: tests: Add KUnit tests for ML-DSA verification") > > in v7.0-rc1. > > > > > --- a/lib/crypto/tests/Kconfig > > > +++ b/lib/crypto/tests/Kconfig > > > @@ -36,10 +36,19 @@ config CRYPTO_LIB_MD5_KUNIT_TEST > > > select CRYPTO_LIB_MD5 > > > help > > > KUnit tests for the MD5 cryptographic hash function and its > > > corresponding HMAC. > > > > > > +config CRYPTO_LIB_MLDSA_KUNIT_TEST > > > + tristate "KUnit tests for ML-DSA" if !KUNIT_ALL_TESTS > > > + depends on KUNIT > > > + default KUNIT_ALL_TESTS || CRYPTO_SELFTESTS > > > + select CRYPTO_LIB_BENCHMARK_VISIBLE > > > + select CRYPTO_LIB_MLDSA > > > > These two selects mean that enabling KUNIT_ALL_TESTS also enables > > extra functionality, which may not be desirable in a production system. > > Fortunately CRYPTO_LIB_MLDSA is tristate, so in the modular case > > the extra functionality is a module, too, and not part of the running system > > by default. Unfortunately CRYPTO_LIB_MLDSA is invisible, so this cannot > > just be changed from "select" to "depends on". But as CRYPTO_MLDSA > > also selects it, perhaps the test can be made dependent on CRYPTO_MLDSA? > > "depends on CRYPTO_MLDSA" doesn't make sense, since the test is for the > code in CRYPTO_LIB_MLDSA, not CRYPTO_MLDSA. CRYPTO_MLDSA just happens > to be one of the users of CRYPTO_LIB_MLDSA. In this case the names > happen to be similar, but consider e.g. CRYPTO_LIB_AESGCM which is > selected by AMD_MEM_ENCRYPT. If we added a test for CRYPTO_LIB_AESGCM, > it clearly shouldn't use "depends on AMD_MEM_ENCRYPT". > > So, "depends on CRYPTO_LIB_MLDSA" would be the correct way to switch it > from a selection to a dependency. > > It's just a bit annoying to do this for hidden symbols, given that it > makes it so that anyone who wants to unconditionally enable the test, > like what I do to test all the crypto library code, has to find and > enable some other random kconfig symbol that enables the code. > > Also, the series that originally added CRYPTO_LIB_MLDSA and its test > (https://lore.kernel.org/linux-crypto/[email protected]/) > didn't add any user of CRYPTO_LIB_MLDSA besides the test, as the real > user came a bit later. So if I had used "depends on CRYPTO_LIB_MLDSA", > my series wouldn't have received any build bot coverage, and I'd have > needed to temporarily carry local patches to build and test the code. > > But if this is really the convention for KUnit, as it seems to be, I > will follow it and work around it for my own testing. So I'll plan to > change the crypto library and CRC tests to use "depends on". > > But any thoughts from the KUnit maintainers would also be appreciated. > Is it indeed intended that the tests for library modules depend on those > modules rather than selecting them, despite their symbols being hidden?
I sent the following patch to consider: https://lore.kernel.org/linux-crypto/[email protected]/ - Eric
