Re: [PATCH] random: do not pretend to handle premature next security model

[Eric Biggers <ebiggers@kernel.org>]

On Wed, May 04, 2022 at 01:30:25PM +0200, Jason A. Donenfeld wrote:
> Per the thread linked below, "premature next" is not considered to be a
> realistic threat model, and leads to more serious security problems.
> 
> "Premature next" is the scenario in which:
> 
> - Attacker compromises the current state of a fully initialized RNG via
>   some kind of infoleak.
> - New bits of entropy are added directly to the key used to generate the
>   /dev/urandom stream, without any buffering or pooling.
> - Attacker then, somehow having read access to /dev/urandom, samples RNG
>   output and brute forces the individual new bits that were added.
> - Result: the RNG never "recovers" from the initial compromise, a
>   so-called violation of what academics term "post-compromise security".
> 
> The usual solutions to this involve some form of delaying when entropy
> gets mixed into the crng. With Fortuna, this involves multiple input
> buckets. With what the Linux RNG was trying to do prior, this involves
> entropy estimation.
> 
> However, by delaying when entropy gets mixed in, it also means that RNG
> compromises are extremely dangerous during the window of time before
> the RNG has gathered enough entropy, during which time nonces may become
> predictable (or repeated), ephemeral keys may not be secret, and so
> forth. Moreover, it's unclear how realistic "premature next" is from an
> attack perspective, if these attacks even make sense in practice.
> 
> Put together -- and discussed in more detail in the thread below --
> these constitute grounds for just doing away with the current code that
> pretends to handle premature next. I say "pretends" because it wasn't
> doing an especially great job at it either; should we change our mind
> about this direction, we would probably implement Fortuna to "fix" the
> "problem", in which case, removing the pretend solution still makes
> sense.
> 
> This also reduces the crng reseed period from 5 minutes down to 1
> minute. The rationale from the thread might lead us toward reducing that
> even further in the future (or even eliminating it), but that remains a
> topic of a future commit.
> 
> At a high level, this patch changes semantics from:
> 
>     Before: Seed for the first time after 256 "bits" of estimated
>     entropy have been accumulated since the system booted. Thereafter,
>     reseed once every five minutes, but only if 256 new "bits" have been
>     accumulated since the last reseeding.
> 
>     After: Seed for the first time after 256 "bits" of estimated entropy
>     have been accumulated since the system booted. Thereafter, reseed
>     once every minute.
> 
> Most of this patch is renaming and removing: POOL_MIN_BITS becomes
> POOL_INIT_BITS, credit_entropy_bits() becomes credit_init_bits(),
> crng_reseed() loses its "force" parameter since it's now always true,
> the drain_entropy() function no longer has any use so it's removed,
> entropy estimation is skipped if we've already init'd, the various
> notifiers for "low on entropy" are now only active prior to init, and
> finally, some documentation comments are cleaned up here and there.
> 
> Link: https://lore.kernel.org/lkml/YmlMGx6+uigkGiZ0@zx2c4.com/
> Cc: Theodore Ts'o <tytso@mit.edu>
> Cc: Nadia Heninger <nadiah@cs.ucsd.edu>
> Cc: Tom Ristenpart <ristenpart@cornell.edu>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

This looks good to me; thanks for cleaning this up!  Feel free to add:

	Reviewed-by: Eric Biggers <ebiggers@google.com>

A couple very minor comments:

>   * The high level overview is that there is one input pool, into which
> - * various pieces of data are hashed. Some of that data is then "credited" as
> - * having a certain number of bits of entropy. When enough bits of entropy are
> - * available, the hash is finalized and handed as a key to a stream cipher that
> - * expands it indefinitely for various consumers. This key is periodically
> - * refreshed as the various entropy collectors, described below, add data to the
> - * input pool and credit it. There is currently no Fortuna-like scheduler
> - * involved, which can lead to malicious entropy sources causing a premature
> - * reseed, and the entropy estimates are, at best, conservative guesses.
> + * various pieces of data are hashed. Prior to initialization, some of that
> + * data is then "credited" as having a certain number of bits of entropy.
> + * When enough bits of entropy are available, the hash is finalized and
> + * handed as a key to a stream cipher that expands it indefinitely for
> + * various consumers. This key is periodically refreshed as the various
> + * entropy collectors, described below, add data to the input pool and
> + * credit it.

The words "and credit it" at the end of this paragraph shouldn't be there.

> +      /*
> +       * If the base_crng is old enough, we try to reseed, which in turn
> +       * bumps the generation counter that we check below.
> +       */

This should say "reseed" instead of "try to reseed".

- Eric