2022-03-22 01:19:14

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH] random: skip fast_init if hwrng provides large chunk of entropy

At boot time, EFI calls add_bootloader_randomness(), which in turn calls
add_hwgenerator_randomness(). Currently add_hwgenerator_randomness()
feeds the first 64 bytes of randomness to the "fast init"
non-crypto-grade phase. But if add_hwgenerator_randomness() gets called
with more than POOL_MIN_BITS of entropy, there's no point in passing it
off to the "fast init" stage, since that's enough entropy to bootstrap
the real RNG. The "fast init" stage is just there to provide _something_
in the case where we don't have enough entropy to properly bootstrap the
RNG. But if we do have enough entropy to bootstrap the RNG, the current
logic doesn't serve a purpose. So, in the case where we're passed
greater than or equal to POOL_MIN_BITS of entropy, this commit makes us
skip the "fast init" phase.

Cc: Dominik Brodowski <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
drivers/char/random.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0bdefada7453..78e0ed46a7cb 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1118,7 +1118,7 @@ void rand_initialize_disk(struct gendisk *disk)
void add_hwgenerator_randomness(const void *buffer, size_t count,
size_t entropy)
{
- if (unlikely(crng_init == 0)) {
+ if (unlikely(crng_init == 0 && entropy < POOL_MIN_BITS)) {
size_t ret = crng_pre_init_inject(buffer, count, true);
mix_pool_bytes(buffer, ret);
count -= ret;
--
2.35.1


2022-03-22 07:14:56

by Dominik Brodowski

[permalink] [raw]
Subject: Re: [PATCH] random: skip fast_init if hwrng provides large chunk of entropy

Am Mon, Mar 21, 2022 at 06:52:56PM -0600 schrieb Jason A. Donenfeld:
> At boot time, EFI calls add_bootloader_randomness(), which in turn calls
> add_hwgenerator_randomness(). Currently add_hwgenerator_randomness()
> feeds the first 64 bytes of randomness to the "fast init"
> non-crypto-grade phase. But if add_hwgenerator_randomness() gets called
> with more than POOL_MIN_BITS of entropy, there's no point in passing it
> off to the "fast init" stage, since that's enough entropy to bootstrap
> the real RNG.

Well, so far, we need 64 bytes input to the fast init stage, and then
further 32 bytes of randomness to proceed to full init, and we used to mix
the former into the latter, which provided for some sort of extra margin.
But as we don't seem to do that any more (mixing some of base_crng back into
the input_pool), that exercise may have become pointless.

However, it's noteworthy that then CONFIG_RANDOM_TRUST_BOOTLOADER really
means trusting it to possibly being the only source for the first generation
of base_crng. In the past, EFI-provided randnomness never was sufficient to
progress crng_init to 2.

Therefore, I am a bit torn about this patch.

Thanks,
Dominik


NB: As POOL_MIN_BITS equals POOL_BITS, there's some room for cleanup. For
example, entropy_count cannot become larger than POOL_MIN_BITS in
credit_entropy_bits(), AFAICS.

2022-03-24 17:54:38

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH] random: skip fast_init if hwrng provides large chunk of entropy

Hi Dominik,

On Tue, Mar 22, 2022 at 12:45 AM Dominik Brodowski
<[email protected]> wrote:
> Well, so far, we need 64 bytes input to the fast init stage, and then
> further 32 bytes of randomness to proceed to full init, and we used to mix
> the former into the latter, which provided for some sort of extra margin.
> But as we don't seem to do that any more (mixing some of base_crng back into
> the input_pool), that exercise may have become pointless.

"Some extra margin" but you're comparing 512 bits to 768 bits? That
makes no sense. 256 bits alone would be sufficient here. The whole
point of CONFIG_RANDOM_TRUST_BOOTLOADER=y is that the kernel builder
has chosen to trust the seed that comes from the bootloader. If it's
not trusted, then it goes through add_device_randomness(), which
doesn't have anything to do with fast init or the main init.

The purpose of "fast init" being separate from the full thing is so
that you can't brute force inputs bit by bit. Having a massive tranche
of 512 bits of entropy makes that brute forcing impossible; therefore
it doesn't make sense to do the fast init thing in that case.

Jason