lkml.org 
[lkml]   [2022]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH RFC v1 1/2] random: add mechanism for VM forks to reinitialize crng
    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@mit.edu>
    > Cc: Jann Horn <jannh@google.com>
    > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
    > ---
    > 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

    \
     
     \ /
      Last update: 2022-02-24 00:16    [W:4.078 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site