| 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
| ||
|
Message-ID: <20251001160827.GD1592@sol>
Date: Wed, 1 Oct 2025 09:08:27 -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 Wed, Oct 01, 2025 at 09:04:35AM -0700, Eric Biggers wrote:
> 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.
Ah sorry, I meant to leave this feedback on v4. This patch is identical
in v3 and v4, though, so all this still applies.
- Eric
Powered by blists - more mailing lists