kern: random: reduce the rate at which we collect from fast entropy sources

[kevans]

Summary

I have this staged as three commits, but I decided combining them for review was not too much to review and more useful to discuss the changes in the full context. I think the below is a reasonable interpretation of previous discussions, but I'm happy to be corrected. :-)

commit c55c59e65067efaa5c51d0159afdadb4c1bdce70
Author: Kyle Evans <[e-mail]>
Date:   Sun Sep 19 23:59:09 2021 -0500

    kern: random: drop read_rate and associated functionality
    
    Refer to discussion in PR 230808 for a less incomplete discussion, but
    the gist of this change is that we currently collect orders of magnitude
    more entropy than we need.
    
    The excess comes from bytes being read out of /dev/*random.  The default
    rate at which we collect entropy without the read_rate increase is
    already more than we need to recover from a compromise of an internal
    state.

commit 238ae5d7a77f24134c985cbf2caa49a04e8af275
Author: Kyle Evans <[e-mail]>
Date:   Mon Sep 20 00:46:21 2021 -0500

    kern: random: collect ~16x less from fast-entropy sources
    
    Previously, we were collecting at a base rate of:
    
    64 bits x 32 pools x 10 Hz = 2.5 kB/s
    
    This change drops it to closer to 64-ish bits per pool per second, to
    work a little better with entropy providers in virtualized environments
    without compromising the security goals of Fortuna.

[kevans]

sys/dev/random/random_harvestq.c

81

This is a little convoluted, but I included it as-is to demonstrate the goal rate...

[cem]

sys/dev/random/fortuna.c

76–83 ↗

(On Diff #95373)

I think we could drop this part -- I misspoke earlier re: reseed interval. The virtio-rng volume is coming entirely from the background thread polling fast random sources frequently, which *happens* to also be 10 Hz prior to this revision, but is orthogonal to the reseed interval.

sys/dev/random/random_harvestq.c

83–84

Maybe also static assert > 0

237

Effectively, this goes from 2 to 1, right?

265

I was imagining breaking this loop up, so we only did a subset of poolcount every 100ms iteration of random_kthread. That could get us down anywhere from 2-32x lower read rates.

[kevans]

Address comments, maybe

[cem]

Let’s re-remove read_rate_increment but otherwise it’s looking good to me.

sys/dev/random/random_harvestq.h

45

I think this got restored on accident.

[kevans]

Whoops, fix git snafu. I also had some inline review comments that remained unsubmitted, but I went ahead and nuked those because the changes mentioned have already generally been observed. :-)

[cem]

Lgtm, thanks for driving this.

[delphij]

LGTM; I'd use howmany() for npools (see suggested edit inline).

sys/dev/random/random_harvestq.c

255–256

[OPTIONAL] I'd use howmany here.

[delphij]

Another observation (unrelated to your change) is that the current RANDOM_KTHREAD_HZ was 10; we might want to choose a power of two value that is close (8 maybe?) to simplify some calculation?

[kevans]

sys/dev/random/random_harvestq.c

255–256

Oh, good call. I will switch it pre-commit. Thanks!

[kevans]

In D32021#723250, @delphij wrote:

Another observation (unrelated to your change) is that the current RANDOM_KTHREAD_HZ was 10; we might want to choose a power of two value that is close (8 maybe?) to simplify some calculation?

Sorry, I missed this one earlier. Is there a csprng@ list/alias that we can raise this to in a general context? re: calculations here, at least, I think we're largely indifferent...the slop from overshooting the target is basically insignificant.