Hi Ayoub,
On 2026-09-18T17:21:23, Ayoub Zaki <[email protected]> wrote:
> test: add ML-DSA verification tests
>
> Adapt the Linux KUnit coverage introduced by commit
> ed894faccb8de55cd755e093c4b0971f190d384d to the U-Boot unit-test
> framework. Import its fixed vectors for ML-DSA-44, ML-DSA-65 and
> ML-DSA-87.
Please can you use the usual form, i.e. commit ed894faccb8d
('subject'), so that people can see what the commit is without looking
it up? The same applies to patch 2.
>
> Use deterministic mutation positions for reproducible U-Boot tests.
> Test valid signatures for every parameter set. Test signature and
> public-key length errors plus a changed message length. Test malformed
> encodings and every valid UseHint input.
>
> Add an invalid-algorithm test beyond the Linux coverage. Test SHAKE128
> and SHAKE256 against FIPS 202 known answers.
>
> Signed-off-by: Ayoub Zaki <[email protected]>
>
> test/Kconfig | 9 +
> test/lib/Makefile | 1 +
> test/lib/mldsa-testvecs.h | 1896
> +++++++++++++++++++++++++++++++++++++++++++++
> test/lib/mldsa.c | 383 +++++++++
> 4 files changed, 2289 insertions(+)
> diff --git a/test/Kconfig b/test/Kconfig
> @@ -75,6 +75,15 @@ config UT_LIB_RSA
> +config UT_LIB_MLDSA
> + bool "Unit tests for ML-DSA verification"
> + default y
> + select MLDSA_VERIFY
With 'default y' and 'select', any board with UNIT_TEST now builds the
ML-DSA verifier, SHAKE and about 9KB of test vectors. That includes
QEMU boards (qemu_arm64, qemu-riscv64, malta, etc.), not just sandbox.
Enabling unit tests should not silently turn on a crypto feature.
Please can you use 'depends on MLDSA_VERIFY' instead, as UT_LIB_RSA
does, and enable MLDSA_VERIFY for sandbox (e.g. imply it from SANDBOX
or add it to sandbox_defconfig)? Then the test runs wherever the
feature is actually used.
BTW the tests all pass on sandbox. lib_mldsa_use_hint takes about
150ms on my machine there, which is fine, though it will be much
slower under QEMU emulation.
> diff --git a/test/lib/mldsa.c b/test/lib/mldsa.c
> @@ -0,0 +1,383 @@
> +static int lib_shake(struct unit_test_state *uts)
> +{
> + static const u8 shake128_empty[32] = {
SHAKE comes in patch 1 and is meant to be generic, so its test belongs
with it, in patch 1 or a separate patch straight after it. Please put
it in its own file (e.g. test/lib/sha3.c) controlled by CONFIG_SHAKE
or its own option. At present it is only built when the ML-DSA test is
enabled.
The coverage is also thin. Only the empty message is tested, so the
absorb path never processes a full block (168 bytes for SHAKE128, 136
for SHAKE256) and shake_update() never handles partial blocks. Linux
has lib/crypto/tests/sha3_kunit.c with vectors you could port,
including multi-block and incremental cases. Please can you add those?
Regards,
Simon