Page MenuHomeFreeBSD

Don't set a harvest_mask by default.
Needs ReviewPublic

Authored by obrien on Oct 27 2017, 2:36 AM.

Details

Reviewers
stevek
delphij
Group Reviewers
O3: Kernel Random Numbers Generator(Owns No Changed Paths)
Summary

Don't set a harvest_mask by default.
It is a bad idea and does not keep up with additions to the harvesting sources.
Otherwise 'harvest_mask' is yet another thing that has to be kept in sync with
sys/sys/random.h:random_entropy_source and
sys/dev/random/random_harvestq.c:random_source_descr[].

Do not depend on r324394's random_check_uint_harvestmask()'s disallowing
userspace modification (e.g., disabling by '511') of pure entropy sources.
That is, forcing a policy on all users instead of letting them dictate policy.
Instead this should be wrapped by raised securelevel or MAC deemed it so.
[We intend to submit that change, since that would allow policy to dictate what
should be allowed.]

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 12258
Build 12549: arc lint + arc unit

Event Timeline

obrien created this revision.Oct 27 2017, 2:36 AM

The proposed patch would effectively disable all entropy gathering sources by default. Thus, systems would boot up without any entropy, save the cached entropy from last reboot. On freshly installed systems, there is no cached entropy. The state of the entropy subsystem would be subpar.

The proposed patch would effectively disable all entropy gathering sources by default. Thus, systems would boot up without any entropy, save the cached entropy from last reboot. On freshly installed systems, there is no cached entropy. The state of the entropy subsystem would be subpar.

Actually, that's not true, harvest_mask set to 0 means don't change whatever is the default in the kernel already.

See etc/rc.d/random:

48	        if [ ${harvest_mask} -gt 0 ]; then
49	                echo -n 'Setting up harvesting: '
50	                ${SYSCTL} kern.random.harvest.mask=${harvest_mask} > /dev/null
51	                ${SYSCTL_N} kern.random.harvest.mask_symbolic
52	        fi

I removed my last comment, because reviewing code on cold medicine isn't a good idea. Stevek is right -- When I talked with lattera earlier this morning, I had an "off by one" error. Setting the mask to 1 leaves you with only cached entropy enabled. 0 leaves it unchanged. My bad and sorry for confusion.

That said, it means that the change is essentially a nop and it doesn't really matter whether it is done or not. However, I still do have some word regarding pure sources of entropy.

  1. pure sources have not been getting mixed in for the last two years due to the mask not having been set, and having no ability to set the mask value for sources beyond the environmental ones. The patch supplied by me and oliver pinter fixed that.
  2. the envrionemtal sources result in less than a quarter bit of min-entropy per byte when an SP800-90B non-iid entropy analysis tool is run over a 1000000 byte+ sample file collected from these sources alone, as demonstrated by jmg@ and myself at vbsdcon. Again, the patch fixed that some
  3. we could think of no reason why we would want to disable RDRND as it is of sufficient rate and value in terms of min-entropy. Conspiracy theories aside, the environmental entropy sources provide enough jitter than any supposed subversion of the hardware rng should not be an issue.
  4. So long as there remains a method of setting the mask for pure sources, and they are enabled by default as well, then I guess my major objection would be overcome.

Others (clearly) may disagree with my assessment that there is almost no value in letting someone disable pure sources in the harvest mask. The harvest mask doesn't even actually affect whether something is harvested, only whether or not it is given to random_fortuna_process_event, hashed and stored in one of the pools. You'd still have all the overhead of collecting the entropy. While tying the ability to adjust harvest mask to secure levels or MAC settings may be the better long-term fix, just not letting people disable the only thing that can be counted on to provide high-value entropy made sense to get the ball moving as part of at least fixing the actual bug related to pure entropy sources.

Due to the follow-up conversation with badfilemagic, my last comment should be retracted as well.