On Wed, Feb 23, 2022 at 02:12:30PM +0100, Jason A. Donenfeld wrote:
> When a VM forks, we must immediately mix in additional information to
> the stream of random output so that two forks or a rollback don't
> produce the same stream of random numbers, which could have catastrophic
> cryptographic consequences. This commit adds a simple API, add_vmfork_
> randomness(), for that.
>
> Cc: Theodore Ts'o <tytso@xxxxxxx>
> Cc: Jann Horn <jannh@xxxxxxxxxx>
> Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
> ---
> drivers/char/random.c | 58 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/random.h | 1 +
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 536237a0f073..29d6ce484d15 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -344,6 +344,46 @@ static void crng_reseed(void)
> }
> }
>
> +/*
> + * This mixes unique_vm_id directly into the base_crng key as soon as
> + * possible, similarly to crng_pre_init_inject(), even if the crng is
> + * already running, in order to immediately branch streams from prior
> + * VM instances.
> + */
> +static void crng_vm_fork_inject(const void *unique_vm_id, size_t len)
> +{
> + unsigned long flags, next_gen;
> + struct blake2s_state hash;
> +
> + /*
> + * Unlike crng_reseed(), we take the lock as early as possible,
> + * since we don't want the RNG to be used until it's updated.
> + */
> + spin_lock_irqsave(&base_crng.lock, flags);
> +
> + /*
> + * Also update the generation, while locked, as early as
> + * possible. This will mean unlocked reads of the generation
> + * will cause a reseeding of per-cpu crngs, and those will
> + * spin on the base_crng lock waiting for the rest of this
> + * operation to complete, which achieves the goal of blocking
> + * the production of new output until this is done.
> + */
> + next_gen = base_crng.generation + 1;
> + if (next_gen == ULONG_MAX)
> + ++next_gen;
> + WRITE_ONCE(base_crng.generation, next_gen);
> + WRITE_ONCE(base_crng.birth, jiffies);
> +
> + /* This is the same formulation used by crng_pre_init_inject(). */
> + blake2s_init(&hash, sizeof(base_crng.key));
> + blake2s_update(&hash, base_crng.key, sizeof(base_crng.key));
> + blake2s_update(&hash, unique_vm_id, len);
> + blake2s_final(&hash, base_crng.key);
> +
> + spin_unlock_irqrestore(&base_crng.lock, flags);
> +}
[...]
> +/*
> + * Handle a new unique VM ID, which is unique, not secret, so we
> + * don't credit it, but we do mix it into the entropy pool and
> + * inject it into the crng.
> + */
> +void add_vmfork_randomness(const void *unique_vm_id, size_t size)
> +{
> + add_device_randomness(unique_vm_id, size);
> + crng_vm_fork_inject(unique_vm_id, size);
> +}
> +EXPORT_SYMBOL_GPL(add_vmfork_randomness);
I think we should be removing cases where the base_crng key is changed directly
besides extraction from the input_pool, not adding new ones. Why not implement
this as add_device_randomness() followed by crng_reseed(force=true), where the
'force' argument forces a reseed to occur even if the entropy_count is too low?
- Eric
