Mark Murray <[email protected]> writes:
> Log:
> Bring in some behind-the-scenes development, mainly By Arthur Mesh,
> the rest by me.
>
> o Namespace cleanup; the Yarrow name is now restricted to where it
> really applies; this is in anticipation of being augmented or
> replaced by Fortuna in the future. Fortuna is mentioned, but behind
> #if logic, and is ignorable for now.
>
> o The harvest queue is pulled out into its own modules.
>
> o Entropy harvesting is emproved, both by being made more conservative,
> and by separating (a bit!) the sources. Available entropy crumbs are
> marginally improved.
>
> o Selection of sources is made clearer. With recent revelations,
> this will receive more work in the weeks and months to come.
>
> Submitted by: Arthur Mesh (partly) <[email protected]>
The patch is very large, but almost exclusively structural - code that
will be shared between Yarrow and Fortuna and any other entropy mixer we
may implement in the future has been renamed and / or moved away from
Yarrow.
I didn't see anything that deviated from the plan we agreed upon in
Cambridge.
Several random_harvest() calls have been changed to reduce the entropy
estimate - that's a good thing (as long as we don't reduce it to an
unusable level, which I don't think is the case). I also see that the
network entropy harvesting bug we talked about has been fixed, which is
also a good thing. As far as I can tell, these are the only changes
which affect the quality of the output.
The renaming part made the patch hard to read - IWBNI it had been
committed separately, but it didn't kill me. Another factor that
reduces readability is that the patch needlessly unfolds
previously wrapped lines, e.g.
- if (++random_state.outputblocks >=
- random_state.gengateinterval) {
+ if (++random_state.outputblocks >=
random_state.gengateinterval) {
which doesn't actually change anything but introduces a style bug and
increases the reviewers' workload. In fact, the patch introduces quite
a few style bugs (including some that I personally aprove of but bde@
may complain about, such as s/u_char/uint8_t/), but that can be
addressed when we get a fresh batch of round tuits.
(joking aside - barring an overriding reason, we should strive to always
conform to style(9))
In Yarrow, buffer sizes are now consistently referred to by BLOCKSIZE
rather than a mix of BLOCKSIZE and (int)sizeof(whatever), which improves
code readability at the cost of patch readability. However, it appears
that the *meaning* of BLOCKSIZE has changed from bits to bytes, and if I
read the code correctly, it used to be 256 bits but is now 128.
I dislike the use of "pseudo" in sys/dev/random/pseudo_rng.c since it is
easily confused with the P in PRNG and pseudo_rng.c is actually not a
PRNG but rather a collection of fake or dummy RNGs for testing purposes.
Perhaps s/pseudo/dummy/g would be in order.
So, this is a provisional OK from my part. *However*, I did not review
the new harvesting queue in detail, and a bug there could, in the worst
case, result in all the harvested entropy being discarded and Yarrow
receiving kilobyte upon kilobyte of zeroes; so I'd like to get a second
opinion.
DES
--
Dag-Erling Smørgrav - [email protected]
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
