random(4): deduplicate explicit_bzero() in harvest

[cem]

Summary

I noticed that Fortuna was explicitly zeroing the provided event in addition
to most callers. Pull the responsibility for zeroing events, which is
general to any conceivable algorithm, out of the algorithm-specific code and
into the callers. Most callers indirect through
random_fortuna_process_event(), so add the logic there.

Add one missing bzero in randomdev_accumulate(). Also, remove a redundant
bzero in the same function -- randomdev_hash_finish() is obliged to bzero
the hash state.

Test Plan

This is mostly just an easy cleanup with no change in function. The only real
behavioral change is the missing bzero added in randomdev_accumulate() (used by
write(2) on /dev/random).

[delphij]

Looks good to me in principal.

It do beg the question: why wasn't the bzero done in ra_event_processor instead? (i.e. an event should be considered as "consumed" upon return).

sys/dev/random/randomdev.c

356 ↗

(On Diff #57569)

Nice finding!

[cem]

Thanks for taking a look, Xin!

In D20318#438316, @delphij wrote:

It do beg the question: why wasn't the bzero done in ra_event_processor instead? (i.e. an event should be considered as "consumed" upon return).

That might be reasonable. I put it here mostly because (1) it is generic to any event processor; i.e., I don't think it makes sense to duplicate in Fortuna, AlgorithmX, etc. This point is mostly hypothetical since we only support Fortuna, though. And (2), it can be done outside the CONFIG_S_LOCK for RANDOM_LOADABLE (which again is moot in the GENERIC kernel). I guess a weak (3) would be that randomdev_accumulate reuses the same event structure repeatedly and only needs to explicit_bzero it once; pushing it inside ra_event_processor duplicates work.

It would be totally acceptable to me to move it in ra_event_processor instead, if that is your preference.

One other minor thing I noticed is that our Fortuna pool accumulators (fsp_hash) do not actually use SHA_d-256(), i.e., SHA-256(SHA-256(0^512 || ·)), but instead SHA-256²(), i.e., SHA-256(SHA-256(·)). FS&K §9.5.5 RandomData() suggests accumulating pool entropy P_i with the SHA_d-256 construction. However, in this particular case, I don't think there is any security benefit to SHA-d-256 over SHA-256², nor SHA-256² over plain SHA-256 (I don't think length extension attacks are relevant to this use).

sys/dev/random/randomdev.c

356 ↗

(On Diff #57569)

Thanks :)