linux-kernel - Re: [PATCH crypto] crypto: blake2s

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: <Yo/Afs61tFwnaOV8@sol.localdomain>
Date:   Thu, 26 May 2022 11:01:34 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
        herbert@...dor.apana.org.au, gaochao <gaochao49@...wei.com>,
        Ard Biesheuvel <ardb@...nel.org>
Subject: Re: [PATCH crypto] crypto: blake2s - remove shash module

On Thu, May 26, 2022 at 11:20:26AM +0200, Jason A. Donenfeld wrote:
> BLAKE2s has no use as an shash and no use is likely to ever come up.

I'm not sure about that.  Anyone who is already using shash (for supporting
multiple algorithms) and wants to add blake2s support would need it.  dm-verity,
dm-integrity, fs-verity, UBIFS authentication, IMA, btrfs checksums, etc.  A
couple of these are already using blake2b, even.  I guess you might as well
remove blake2s until someone explicitly says they want it, but I can easily see
it coming back later.

> Just remove all of this unnecessary plumbing. Removing this shash was
> something we talked about back when we were making BLAKE2s a built-in,
> but I simply never got around to doing it. So this completes that
> project.
> 
> This also helps fix a bug in which the lib code depends on
> crypto_simd_disabled_for_test, which is now unnecessary.
> 
> Cc: gaochao <gaochao49@...wei.com>
> Cc: Eric Biggers <ebiggers@...nel.org>
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>

More importantly, this is removing quite a bit of test coverage because the
extra self-tests in crypto/testmgr.c are more comprehensive than what
lib/crypto/blake2s-selftest.c does.  For example they test the case where the
input data is misaligned, as well as the case where the code is executed in a
context where SIMD instructions are unavailable.

In order for this to be acceptable, I think you'd need to update
blake2s-selftest.c to be more comprehensive.

- Eric