Re: [PATCH v3 5/8] lib/crypto: Add SHA3 kunit tests

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Sep 26, 2025 at 03:19:48PM +0100, David Howells wrote:
> +config CRYPTO_LIB_SHA3_KUNIT_TEST
> +	tristate "KUnit tests for SHA-3" if !KUNIT_ALL_TESTS
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS || CRYPTO_SELFTESTS
> +	select CRYPTO_LIB_BENCHMARK_VISIBLE
> +	select CRYPTO_LIB_SHA3
> +	help
> +	  KUnit tests for the SHA3 cryptographic hash functions, including
> +	  SHA3-224, SHA3-256, SHA3-386, SHA3-512, SHAKE128 and SHAKE256.  Note

SHA3-386 => SHA3-384

> +	  that whilst the SHAKE* hash functions can support arbitrary-length
> +	  digests, these tests only check the nominal digest sizes for now.

Arbitrary-length output support needs to be tested.  It looks like it is
now, and you just forgot to update this help text?

> +static const u8 test_sha3_sample[] =
> +	"The quick red fox jumped over the lazy brown dog!\n"
> +	"The quick red fox jumped over the lazy brown dog!\n"
> +	"The quick red fox jumped over the lazy brown dog!\n"
> +	"The quick red fox jumped over the lazy brown dog!\n";
> +
> +static const u8 test_sha3_224[8 + SHA3_224_DIGEST_SIZE + 8] = {
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-before guard */
> +	0xd6, 0xe8, 0xd8, 0x80, 0xfa, 0x42, 0x80, 0x70,
> +	0x7e, 0x7f, 0xd7, 0xd2, 0xd7, 0x7a, 0x35, 0x65,
> +	0xf0, 0x0b, 0x4f, 0x9f, 0x2a, 0x33, 0xca, 0x0a,
> +	0xef, 0xa6, 0x4c, 0xb8,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-after guard */
> +};
> +
> +static const u8 test_sha3_256[8 + SHA3_256_DIGEST_SIZE + 8] = {
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-before guard */
> +	0xdb, 0x3b, 0xb0, 0xb8, 0x8d, 0x15, 0x78, 0xe5,
> +	0x78, 0x76, 0x8e, 0x39, 0x7e, 0x89, 0x86, 0xb9,
> +	0x14, 0x3a, 0x1e, 0xe7, 0x96, 0x7c, 0xf3, 0x25,
> +	0x70, 0xbd, 0xc3, 0xa9, 0xae, 0x63, 0x71, 0x1d,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-after guard */
> +};
> +
> +static const u8 test_sha3_384[8 + SHA3_384_DIGEST_SIZE + 8] = {
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-before guard */
> +	0x2d, 0x4b, 0x29, 0x85, 0x19, 0x94, 0xaa, 0x31,
> +	0x9b, 0x04, 0x9d, 0x6e, 0x79, 0x66, 0xc7, 0x56,
> +	0x8a, 0x2e, 0x99, 0x84, 0x06, 0xcf, 0x10, 0x2d,
> +	0xec, 0xf0, 0x03, 0x04, 0x1f, 0xd5, 0x99, 0x63,
> +	0x2f, 0xc3, 0x2b, 0x0d, 0xd9, 0x45, 0xf7, 0xbb,
> +	0x0a, 0xc3, 0x46, 0xab, 0xfe, 0x4d, 0x94, 0xc2,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-after guard */
> +};
> +
> +static const u8 test_sha3_512[8 + SHA3_512_DIGEST_SIZE + 8] = {
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-before guard */
> +	0xdd, 0x71, 0x3b, 0x44, 0xb6, 0x6c, 0xd7, 0x78,
> +	0xe7, 0x93, 0xa1, 0x4c, 0xd7, 0x24, 0x16, 0xf1,
> +	0xfd, 0xa2, 0x82, 0x4e, 0xed, 0x59, 0xe9, 0x83,
> +	0x15, 0x38, 0x89, 0x7d, 0x39, 0x17, 0x0c, 0xb2,
> +	0xcf, 0x12, 0x80, 0x78, 0xa1, 0x78, 0x41, 0xeb,
> +	0xed, 0x21, 0x4c, 0xa4, 0x4a, 0x5f, 0x30, 0x1a,
> +	0x70, 0x98, 0x4f, 0x14, 0xa2, 0xd1, 0x64, 0x1b,
> +	0xc2, 0x0a, 0xff, 0x3b, 0xe8, 0x26, 0x41, 0x8f,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-after guard */
> +};
> +
> +static const u8 test_shake128[8 + SHAKE128_DEFAULT_SIZE + 8] = {
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-before guard */
> +	0x41, 0xd6, 0xb8, 0x9c, 0xf8, 0xe8, 0x54, 0xf2,
> +	0x5c, 0xde, 0x51, 0x12, 0xaf, 0x9e, 0x0d, 0x91,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-after guard */
> +};
> +
> +static const u8 test_shake256[8 + SHAKE256_DEFAULT_SIZE + 8] = {
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-before guard */
> +	0xab, 0x06, 0xd4, 0xf9, 0x8b, 0xfd, 0xb2, 0xc4,
> +	0xfe, 0xf1, 0xcc, 0xe2, 0x40, 0x45, 0xdd, 0x15,
> +	0xcb, 0xdd, 0x02, 0x8d, 0xb7, 0x9f, 0x1e, 0x67,
> +	0xd6, 0x7f, 0x98, 0x5e, 0x1b, 0x19, 0xf8, 0x01,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Write-after guard */
> +};

If these expected outputs are from an external source, then that source
needs to be documented.  If they aren't, then the way in which they were
generated needs to be easily reproducible and documented, e.g. by adding
support for generating them to gen-hash-testvecs.py.

> +MODULE_DESCRIPTION("KUnit tests and benchmark for SHA3-256");

SHA3-256 => SHA3

> diff --git a/lib/crypto/tests/sha3_testvecs.h b/lib/crypto/tests/sha3_testvecs.h
> new file mode 100644
> index 000000000000..9c4c403cc6e0
> --- /dev/null
> +++ b/lib/crypto/tests/sha3_testvecs.h
> @@ -0,0 +1,231 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* This file was generated by: ./scripts/crypto/gen-hash-testvecs.py sha3-256 */

If that's the case, then running "./scripts/crypto/gen-hash-testvecs.py
sha3-256 > lib/crypto/tests/sha3_testvecs.h" should reproduce this file
exactly.  But it doesn't, so you must have manually edited this file.

It should match exactly.  That can be done by tweaking
gen-hash-testvecs.py to use the correct *_DIGEST_SIZE constant and
skipping the HMAC test if sha3-256 is requested.

> diff --git a/scripts/crypto/gen-hash-testvecs.py b/scripts/crypto/gen-hash-testvecs.py
> index fc063f2ee95f..a5b0abd8dabe 100755
> --- a/scripts/crypto/gen-hash-testvecs.py
> +++ b/scripts/crypto/gen-hash-testvecs.py
> @@ -61,6 +61,10 @@ def hash_update(ctx, data):
>      ctx.update(data)
>  
>  def hash_final(ctx):
> +    if ctx.name == "shake_128":
> +        return ctx.digest(16)
> +    if ctx.name == "shake_256":
> +        return ctx.digest(32)

This addition is unnecessary.

>      return ctx.digest()
>  
>  def compute_hash(alg, data):
> @@ -122,7 +126,7 @@ def gen_hmac_testvecs(alg):
>          ctx.update(mac)
>      print_static_u8_array_definition(
>              f'hmac_testvec_consolidated[{alg.upper()}_DIGEST_SIZE]',
> -            ctx.digest())
> +            hash_final(ctx))
>  
>  BLAKE2S_KEY_SIZE = 32
>  BLAKE2S_HASH_SIZE = 32
> @@ -164,5 +168,5 @@ if alg == 'blake2s':
>      gen_additional_blake2s_testvecs()
>  elif alg == 'poly1305':
>      gen_additional_poly1305_testvecs()
> -else:
> +elif alg != 'shake128' and alg != 'shake256':
>      gen_hmac_testvecs(alg)

Likewise.  Except it really needs to be 'sha3-256', as I mentioned.

- Eric