Re: [PATCH] hw_random: treat default_quality as a maximum and default to 1024

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 7, 2022 at 12:14 PM Jason A. Donenfeld <Jason@xxxxxxxxx> wrote:
>
> Hi Dominik,
>
> On Mon, Nov 07, 2022 at 08:35:02AM +0100, Dominik Brodowski wrote:
> > Hi Jason,
> >
> > Am Sun, Nov 06, 2022 at 03:40:44PM +0100 schrieb Jason A. Donenfeld:
> > > On Sun, Nov 06, 2022 at 08:05:25AM +0100, Dominik Brodowski wrote:
> > > > Am Fri, Nov 04, 2022 at 04:42:30PM +0100 schrieb Jason A. Donenfeld:
> > > > > Most hw_random devices return entropy which is assumed to be of full
> > > > > quality, but driver authors don't bother setting the quality knob. Some
> > > > > hw_random devices return less than full quality entropy, and then driver
> > > > > authors set the quality knob. Therefore, the entropy crediting should be
> > > > > opt-out rather than opt-in per-driver, to reflect the actual reality on
> > > > > the ground.
> > > > >
> > > > > For example, the two Raspberry Pi RNG drivers produce full entropy
> > > > > randomness, and both EDK2 and U-Boot's drivers for these treat them as
> > > > > such. The result is that EFI then uses these numbers and passes the to
> > > > > Linux, and Linux credits them as boot, thereby initializing the RNG.
> > > > > Yet, in Linux, the quality knob was never set to anything, and so on the
> > > > > chance that Linux is booted without EFI, nothing is ever credited.
> > > > > That's annoying.
> > > > >
> > > > > The same pattern appears to repeat itself throughout various drivers. In
> > > > > fact, very very few drivers have bothered setting quality=1024.
> > > > >
> > > > > So let's invert this logic. A hw_random struct's quality knob now
> > > > > controls the maximum quality a driver can produce, or 0 to specify 1024.
> > > > > Then, the module-wide switch called "default_quality" is changed to
> > > > > represent the maximum quality of any driver. By default it's 1024, and
> > > > > the quality of any particular driver is then given by:
> > > > >
> > > > >     min(default_quality, rng->quality ?: 1024);
> > > > >
> > > > > This way, the user can still turn this off for weird reasons, yet we get
> > > > > proper crediting for relevant RNGs.
> > > >
> > > > Hm. Wouldn't we need to verify that 1024 is appropriate for all drivers
> > > > where the quality currently is not set?
> > >
> > > No, certainly not, and I think this sort of thought belies a really
> > > backwards attitude. Hardware RNGs are assumed to produce good
> > > randomness. Some manufacturers provide a caveat, "actually, we're giving
> > > raw entropy with only N bits quality", but for the ones who don't, the
> > > overarching assumption is that the bits are fully entropic.
> >
> > My point is not about the 1024 as an exact value, it's more about "do the
> > driver and the hardware really provide _something_ sensible or not". In the
> > past, the default mode as to feed the output of hw_rng devies to some
> > userspace daemon, which then tried to verify that the device works as
> > expected, and then feeded the data back to the crng core. This userspace
> > indirection is largely removed already (in particular by a patch of mine
> > which starts up the hwrng kernel thread also for devices with quality==0)
> > once the crng is fully initialized, on the rationale that even bad quality
> > data will do no harm. Yet, we may need to be a tad more careful whether or
> > not to trust devices for the initial seeding of the crng.
>
> I got your point, and I still think it's a bad one, for the reasons
> already explained to you. If it's a hardware RNG, then it's sensible to
> assume it provides hardware random bits, unless we have documentation
> that says it provides something less than perfect.
>
> Now you've moved on to talking again about entropy estimation. Stop with
> this nonsense. Entropy estimation is an impossible proposition that
> actually results in an infoleak. With that said, a self-test to make
> sure the hardware isn't completely borked would be a nice thing, but
> this applies for any device no matter what assumptions are made. So if
> you want to work on that, go ahead, but it's completely orthogonal to
> this change here.

Based on IRC discussion, following up with a v2 with a better commit message.

Jason



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux