Page MenuHomeFreeBSD

Make Raspberry Pi RNG compatible with upstream DTBs
ClosedPublic

Authored by sylvain_sylvaingarrigues.com on Feb 28 2018, 9:26 AM.

Details

Summary

Upstream DTBs don't provide IRQ lines for the RNG. Even if they did, harvesting bytes as often as the RNG interrupt is triggered seems overkill: it was acknowledged on #bsdmips with security and RNG experts that feeding 16 bytes of randomness every 4 seconds was enough.

For these reasons, get rid of the interrupt mode and make callout mode the default, with random bits harvested every 4 seconds.

Test Plan

Tested on RPI2 v1.1 (armv7), RPI3 (armv7), RPI3 (arm64) with upstream DTBs from https://github.com/raspberrypi/firmware/tree/master/boot.

Tested on RPI-B (armv6) by Michal.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mmel accepted this revision.Mar 2 2018, 11:34 AM
mmel added a subscriber: mmel.

Tested also on RPi-B (armv6) without problem.
Please, also remove rng related lines from sys/dts/arm/rpi[2].dts stubs.

With above exception, I'm fine with this.

This revision is now accepted and ready to land.Mar 2 2018, 11:34 AM

Remove IRQ lines from our own rpi[2].dts as suggested.

This revision now requires review to proceed.Mar 2 2018, 6:56 PM
sylvain_sylvaingarrigues.com retitled this revision from Make Raspberry Pi RNG compatible with upstream DTB to Make Raspberry Pi RNG compatible with upstream DTBs.Mar 2 2018, 7:17 PM
sylvain_sylvaingarrigues.com edited the summary of this revision. (Show Details)
sylvain_sylvaingarrigues.com edited the test plan for this revision. (Show Details)
ian accepted this revision.Mar 2 2018, 10:02 PM
manu accepted this revision.Mar 3 2018, 12:53 PM
This revision is now accepted and ready to land.Mar 3 2018, 12:53 PM
gonzo added a subscriber: gonzo.Mar 3 2018, 7:14 PM

Cosmetic change: rename callback argument from "param" to "arg" as per callout_reset documentation.

This revision now requires review to proceed.Mar 4 2018, 7:46 AM
jmg added a comment.Mar 8 2018, 12:51 AM

I only briefly reviewed changes, and I don't see any issues. I did not do a security review.

Two issues that I do see is that there does not appear to be any man page for this driver.
Also, I cannot locate any third part review of the RNG. The man page should make it clear that until a third party has properly reviewed the implementation of the rng, that FreeBSD recommends using other entropy sources in addition, to this one.

For reference: https://security.stackexchange.com/a/47481

emaste added a subscriber: emaste.Mar 8 2018, 4:18 PM

Driver originally introduced in D6888. This driver should indeed have a man page, but that should not gate the change here.

imp accepted this revision.Mar 8 2018, 4:25 PM

I see no security issues here. I agree that a man page would be nice, and a note in it communicating risk would also be nice.
Neither gate this change. It doesn't change the security profile of the driver. It rearranged the deck chairs a bit, but any security problems before the change are there after the change.

The only material change is the rate we feed data to the entropy engine. It would be good to get the SO to say yes or no to that in a timely fashion. I agree that the lower rate is fine. The interrupt rate is ridiculously often.

This revision is now accepted and ready to land.Mar 8 2018, 4:25 PM
emaste added a comment.Mar 8 2018, 4:36 PM

Perhaps we could make the period for the first callout_reset lower, instead of waiting 4s for the first harvest. But I'm happy to see this committed either way.

Schedule the initial harvesting as soon as one second after the RNG is started, which should give it plenty of time to generate the first random bytes.

This revision now requires review to proceed.Mar 8 2018, 5:28 PM

delphij: as the approver for secteam of D6888 which introduced the code and logic implemented in this driver, are you comfortable to approve this one patch here, which does not introduce functional changes to the callout mode?

For reference, as discussed on #bsdmips, the interrupt rate of the RPI RNG was measured at around 87 interrupts / second on an rpi2, which makes the interrupt mode feed 16*4*87 bytes of entropy/sec: that seems overkill and jmg gave us hint that harvesting 16*4 bytes every 4 seconds was enough, hence this patch. In addition, this patch also allows to use upstream DTBs (instead of FreeBSD custom ones) as they do not specify IRQ numbers for the RNG.

ian accepted this revision.Mar 8 2018, 6:44 PM
This revision is now accepted and ready to land.Mar 8 2018, 6:44 PM
mmel accepted this revision.Mar 9 2018, 5:18 AM
emaste accepted this revision as: emaste.Mar 10 2018, 2:13 AM
This revision was automatically updated to reflect the committed changes.