Re: [PATCH] hw/arm/virt: dt: add rng-seed property

[Alex Bennée]

"Jason A. Donenfeld" <Jason@zx2c4.com> writes:

> On Wed, Jun 29, 2022 at 11:18:23AM +0100, Alex Bennée wrote:
>> 
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> 
>> > On Tue, 28 Jun 2022 at 19:45, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> >>
>> >> On 6/27/22, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> >> > On 6/27/22, Peter Maydell <peter.maydell@linaro.org> wrote:
>> >> >> On Mon, 27 Jun 2022 at 17:07, Jason A. Donenfeld <Jason@zx2c4.com> 
>> >> >> wrote:
>> >> >>>
>> >> >>> In 60592cfed2 ("hw/arm/virt: dt: add kaslr-seed property"), the
>> >> >>> kaslr-seed property was added, but the equally as important rng-seed
>> >> >>> property was forgotten about, which has identical semantics for a
>> >> >>> similar purpose. This commit implements it in exactly the same way as
>> >> >>> kaslr-seed.
>> >> >>
>> >> >> Not an objection, since if this is what the dtb spec says we need
>> >> >> to provide then I guess we need to provide it, but:
>> >> >> Why do we need to give the kernel two separate random seeds?
>> >> >> Isn't one sufficient for the kernel to seed its RNG and generate
>> >> >> whatever randomness it needs for whatever purposes it wants it?
>> >> >>
>> >> >
>> >> > Seems a bit silly to me too. `rng-seed` alone ought to be sufficient.
>> >> > After the kernel calls add_bootloader_randomness() on it,
>> >> > get_random_long() can be used for kaslr'ing and everything else too.
>> >> > So I'm not sure what's up, but here we are. Maybe down the line I'll
>> >> > look into the details and formulate a plan to remove `kaslr-seed` if
>> >> > my supposition is correct.
>> 
>> Sorry now I've had my coffee and read properly I see you are already
>> aware of kaslr-seed. However my point about suppression would still
>> stand because for the secure boot flow you need checksum-able DTBs.
>
> Please read the patch. Maybe take a sip of coffee first. There's a knob
> for this too.

I was obviously not paying enough attention this morning. Sorry about that.

> The code is exactly the same for kaslr-seed and rng-seed. Everytime
> there's some kaslr-seed thing, there is now the same rng-seed thing.

The duplication is annoying but specs are specs - where is this written
by the way?

Given the use case for the dtb-kaslr-seed knob I wonder if we should
have a common property and deprecate the kaslr one? As of this patch
existing workflows will break until command lines are updated to suppress
the second source of randomness.

Maybe it would be better to have a single a new property
(dtb-rng-seeds?) which suppresses both dtb entries and make
dtb-kaslr-seed an alias and mark it as deprecated.

-- 
Alex Bennée