linux-kernel - Re: [PATCH crypto v2] 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]

Date:   Fri, 27 May 2022 20:59:29 -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>, stable@...r.kernel.org
Subject: Re: [PATCH crypto v2] crypto: blake2s - remove shash module

On Fri, May 27, 2022 at 10:11:06AM +0200, Jason A. Donenfeld wrote:
> BLAKE2s has no use as an shash, with no users of it.

"no use" => "no known current use".

> diff --git a/lib/crypto/blake2s-selftest.c b/lib/crypto/blake2s-selftest.c
> index 409e4b728770..38996ee73a64 100644
> --- a/lib/crypto/blake2s-selftest.c
> +++ b/lib/crypto/blake2s-selftest.c
> @@ -4,6 +4,8 @@
>   */
>  
>  #include <crypto/internal/blake2s.h>
> +#include <linux/kernel.h>
> +#include <linux/random.h>
>  #include <linux/string.h>
>  
>  /*
> @@ -548,7 +550,8 @@ bool __init blake2s_selftest(void)
>  	u8 key[BLAKE2S_KEY_SIZE];
>  	u8 buf[ARRAY_SIZE(blake2s_testvecs)];
>  	u8 hash[BLAKE2S_HASH_SIZE];
> -	struct blake2s_state state;
> +	u8 blocks[BLAKE2S_BLOCK_SIZE * 4];
> +	struct blake2s_state state, state1, state2;
>  	bool success = true;
>  	int i, l;
>  
> @@ -587,5 +590,32 @@ bool __init blake2s_selftest(void)
>  		}
>  	}
>  
> +	for (i = 0; i < 2048; ++i) {
> +		get_random_bytes(blocks, sizeof(blocks));
> +		get_random_bytes(&state, sizeof(state));
> +
> +		memcpy(&state1, &state, sizeof(state1));
> +		memcpy(&state2, &state, sizeof(state2));
> +		blake2s_compress(&state1, blocks, 4, sizeof(blocks));
> +		blake2s_compress_generic(&state2, blocks, 4, sizeof(blocks));
> +		if (memcmp(&state1, &state2, sizeof(state1))) {
> +			pr_err("blake2s random compress self-test %d: FAIL\n",
> +			       i + 1);
> +			success = false;
> +		}
> +
> +		for (l = 1; l < 8; ++l) {
> +			memcpy(&state1, &state, sizeof(state1));
> +			memcpy(&state2, &state, sizeof(state2));
> +			blake2s_compress(&state1, blocks + l, 3, sizeof(blocks) - BLAKE2S_BLOCK_SIZE);
> +			blake2s_compress_generic(&state2, blocks + l, 3, sizeof(blocks) - BLAKE2S_BLOCK_SIZE);
> +			if (memcmp(&state1, &state2, sizeof(state1))) {
> +				pr_err("blake2s random compress align %d self-test %d: FAIL\n",
> +				       l, i + 1);
> +				success = false;
> +			}
> +		}
> +	}

This doesn't compile on arm, since blake2s_compress_generic() isn't defined.

Also, the wrong value is being passed for the 'inc' argument.

2048 iterations is also a lot.  Doing a lot of iterations here doesn't
meaningfully increase the test coverage.

And please run checkpatch; those are some very long lines :-(

- Eric