linux-kernel - Re: [PATCH v3 0/8] crypto, lib/crypto: Add SHAKE128/256 support and move SHA3 to lib/crypto

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] [day] [month] [year] [list]

Message-ID: <20251002162705.GB1697@sol>
Date: Thu, 2 Oct 2025 09:27:05 -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 0/8] crypto, lib/crypto: Add SHAKE128/256 support and
 move SHA3 to lib/crypto

On Thu, Oct 02, 2025 at 02:14:44PM +0100, David Howells wrote:
> Eric Biggers <ebiggers@...nel.org> wrote:
> 
> > Have you had a chance to read this reply?
> 
> I have.
> 
> You held up your implementation of sha256 and sha224 as an example of how it
> perhaps should be implemented:
> 
> 	It would be worth considering separating the APIs for the different
> 	algorithms that are part of SHA-3, similar to what I did with SHA-224
> 	and SHA-256.
> 
> so I have followed that.  That defines a type for each, so I'll leave it at
> that.

In v3, you were pretty clear that you don't like the six-types solution:

    I have done what Eric required and made a separate wrapper struct
    and set of wrapper functions for each algorithm, though I think this
    is excessively bureaucratic as this multiplies the API load by 7
    (and maybe 9 in the future[*]).

    [*] The Kyber algorithm also uses CSHAKE variants in the SHA3 family - and
        NIST mentions some other variants too.

    This does, however, cause a problem for what I need to do as the
    ML-DSA prehash is dynamically selectable by certificate OID, so I
    have to add SHAKE128/256 support to the crypto shash API too -
    though hopefully it will only require an output of 16 or 32 bytes
    respectively for the prehash case and won't require multiple
    squeezing.

So, I listened.  And I realized that we can address my concern about the
digest vs. XOF confusion using just two types.

And I explained I didn't intend to require that we fully split the API
into all six variants, and apologized for not being clear.

Remember, we haven't done a SHA-3 library API before.  We're both
learning as we go...

If you've now changed your mind and strongly prefer six types, I can
tolerate that too.  But I want to make sure you actually want that, and
aren't choosing it just to try to prove a point or something.

> > All I'm really requesting is that we don't create footguns, like the
> > following that the API in the v2 patch permitted:
> 
> The way you did a separate type for each removed one more footgun - and
> arguably a more important one - as the *type* enforces[1] the output buffer
> size and the sha3_*_final() function has the same sized-array as the
> convenience wrappers.
> 
> It also eliminates the need to store the digest size as this is only needed at
> the final step for the digest variant algorithms.
> 
> David
> 
> [1] Inasmuch as this is effective in C.

Well, that "Inasmuch as this is effective in C" disclaimer is really
important, because it means it doesn't actually work properly.

Effectively, array bounds in function parameters are for humans, not the
compiler.

Which is still useful, but probably not to the extent that we should
have all those extra functions and types, vs. just documenting that
sha3_final() produces output of length matching the init function that
was called.  (As I mentioned, the BLAKE2s API does something similar.)

- Eric