Thor Lancelot Simon <t...@panix.com> wrote:
> Attached are the changes from the tls-earlyentropy branch, which tries
> to make the output of /dev/random less predictable -- particularly for
> an attacker outside the box -- earlier.
This is a very positive work for the cases when system is used as a server
or other workloads which may involve cryptographic activity. However, you
seem to assume that aggressive entropy collection is always preferable even
if it has extra costs. This is simply not true. There are small embedded
systems where aggressive entropy collection is not required and is actually
costly. There are embedded systems where I happily run rlogin, because ssh
is simply not needed in that environment.
Please make this tunable. Also, please make this tunable *before* merge,
otherwise this will just never happen. Like cprng_fast/strong functions
were never optimised to not use spin-lock (at least they no longer use
IPL_HIGH, which should have been considered simply as a bug).
Few fragments which caught my eye while skimming through the diff..
> #if defined(__HAVE_CPU_COUNTER)
> - if (cpu_hascounter())
> - return (cpu_counter32());
> + if (cpu_hascounter() && sizeof(cpu_counter() == sizeof(uint64_t))) {
> + return (cpu_counter());
> + }
> #endif
??
> - rnd_add_uint32(&skewsrc, rnd_counter());
> - callout_schedule(&skew_callout, hz);
> + rnd_add_uint64(&skewsrc, rnd_counter());
> + callout_schedule(&skew_callout, hz / 10);
Do we really need to run 10 extra callouts per second to get an extra bit?
I did not investigate, but we have plenty of already existing callouts.
Is, or can, flipflop logic be used with them? Or deltas between *all*
callouts in the system?
> +static void kprintf_rnd_get(size_t bytes, void *priv)
> +{
> + if (mutex_tryenter(&kprintf_mtx)) {
> + if (kprnd_added) {
> + SHA512_Final(kprnd_accum, &kprnd_sha);
> + rnd_add_data(&rnd_printf_source,
> + kprnd_accum, sizeof(kprnd_accum), 0);
> + kprnd_added = 0;
> + /* This, we must do, since we called _Final. */
> + SHA512_Init(&kprnd_sha);
> + /* This is optional but seems useful. */
> + SHA512_Update(&kprnd_sha, kprnd_accum,
> + sizeof(kprnd_accum));
> + }
> + mutex_exit(&kprintf_mtx);
> + }
> +}
Pre-checking kprnd_added first rather than locking/unlocking mutex all
the time would be a start.. but how about not performing SHA-512 while
holding IPL_HIGH mutex? Not only embedded systems, but perhaps our VAX
or m68k users would also prefer to not have micro-freezes every second
while printing to a console.
> + kr = rnd_sources.lh_first;
> + start = rset->start;
> + while (kr != NULL && start > 1) {
> + kr = kr->list.le_next;
> + start--;
> + }
LIST_FIRST(), LIST_NEXT()?
--
Mindaugas
