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

Reply via email to