linux-kernel - Re: [PATCH v2 7/9] random: use simpler fast key erasure flow on per-cpu keys

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: <YgN7mOTtQ03etVJX@owl.dominikbrodowski.net>
Date:   Wed, 9 Feb 2022 09:30:16 +0100
From:   Dominik Brodowski <linux@...inikbrodowski.net>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
        tytso@....edu, ebiggers@...nel.org,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: [PATCH v2 7/9] random: use simpler fast key erasure flow on
 per-cpu keys

Nice work, just a few minor queries:

> -static void extract_crng(u8 out[CHACHA_BLOCK_SIZE])
> +/*
> + * The general form here is based on a "fast key erasure RNG" from
> + * <https://blog.cr.yp.to/20170723-random.html>. It generates a ChaCha
> + * block using the provided key, and then immediately overwites that
> + * key with half the block. It returns the resultant ChaCha state to the
> + * user, along with the second half of the block containing 32 bytes of
> + * random data that may be used; random_data_len may not be greater than
> + * 32.
> + */
> +static void crng_fast_key_erasure(u8 key[CHACHA_KEY_SIZE],
> +				  u32 chacha_state[CHACHA_STATE_WORDS],
> +				  u8 *random_data, size_t random_data_len)
>  {
> -	unsigned long flags, init_time;
> +	u8 first_block[CHACHA_BLOCK_SIZE];

Do we need a BUG_ON(random_data_len > 32) here?

>  
> -	if (crng_ready()) {
> -		init_time = READ_ONCE(primary_crng.init_time);
> -		if (time_after(jiffies, init_time + CRNG_RESEED_INTERVAL))
> -			crng_reseed();
> -	}
> -	spin_lock_irqsave(&primary_crng.lock, flags);
> -	chacha20_block(&primary_crng.state[0], out);
> -	if (primary_crng.state[12] == 0)
> -		primary_crng.state[13]++;
> -	spin_unlock_irqrestore(&primary_crng.lock, flags);
> +	chacha_init_consts(chacha_state);
> +	memcpy(&chacha_state[4], key, CHACHA_KEY_SIZE);
> +	memset(&chacha_state[12], 0, sizeof(u32) * 4);

No IV, no generation counter here? As you already have a generation counter
in use for other purposes, why not use it here as well as some non-zero
starting point?

Otherwise, it looks really nice.

Thanks,
	Dominik