Re: [PATCH] random Remove setting of chacha state to constant values.

[Jason A. Donenfeld]

Hi Sandy,

On Thu, Jun 16, 2022 at 01:18:23PM +0800, Sandy Harris wrote:
> Setting parts of the state to known constants is needed in
> some Chacha applications to ensure that blocks can be processed
> in parallel and that when needed (e.g. when encrypting disk
> blocks) the algorithm can jump to an arbitrary part of the
> output stream. In an RNG these are not required, and setting
> the constants wastes cycles.
> 
> If (as we hope) the enemy does not know the state, then
> this is more secure since it makes the chacha outputs
> depend on more unknown bits.
> 
> If they can peek at the state or infer parts of it from
> outputs, knowable values cannot possibly be worse than
> known ones. This at least prevents them from using
> pre-computed tables based on the known constants.
> 
> Signed-off-by: Sandy Harris <sandyinchina@xxxxxxxxx>
> ---
>  drivers/char/random.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 655e327d425e..6df9e656a157 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -249,9 +249,7 @@ static void crng_fast_key_erasure(u8 key[CHACHA_KEY_SIZE],
> 
>      BUG_ON(random_data_len > 32);
> 
> -    chacha_init_consts(chacha_state);
>      memcpy(&chacha_state[4], key, CHACHA_KEY_SIZE);
> -    memset(&chacha_state[12], 0, sizeof(u32) * 4);
>      chacha20_block(chacha_state, first_block);
> 
>      memcpy(key, first_block, CHACHA_KEY_SIZE);

Hard NACK here, sorry. You proposed removing the constants used with
BLAKE2s, also, and Eric and I told you the same then:

https://lore.kernel.org/all/YfLtrrB+140KkiN0@sol.localdomain/
https://lore.kernel.org/all/CAHmME9pyj-ejZn8KpVKqhELYB=-5bVYTeNhLk4SZOnBM1zeidA@xxxxxxxxxxxxxx/

Same sort of justification here. ChaCha is a permutation that requires
those constants.

Jason