lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251001160435.GC1592@sol>
Date: Wed, 1 Oct 2025 09:04:35 -0700
From: Eric Biggers <ebiggers@...nel.org>
To: David Howells <dhowells@...hat.com>
Cc: "Jason A . Donenfeld" <Jason@...c4.com>,
	Ard Biesheuvel <ardb@...nel.org>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Stephan Mueller <smueller@...onox.de>, linux-crypto@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 5/8] lib/crypto: Add SHA3 kunit tests

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ