Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 16 Jun 2022 11:58:04 +0200
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
To:     Sandy Harris <sandyinchina@gmail.com>
Cc:     Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, Ted Ts'o <tytso@mit.edu>
Subject: Re: [PATCH] random Remove setting of chacha state to constant values.
Message-ID: <Yqr+rFeixFuHzyHD@zx2c4.com>
References: <CACXcFmmw8bzSr-pmTauMS7a=036eW0=1KLdwAD1MOB_fY-7VRg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CACXcFmmw8bzSr-pmTauMS7a=036eW0=1KLdwAD1MOB_fY-7VRg@mail.gmail.com>
Precedence: bulk

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@gmail.com>
> ---
>  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@mail.gmail.com/

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

Jason