Re: [PATCH] x86: fix q35 kernel measurements broken due to rng seeding

[Jason A. Donenfeld]

This patch is not needed. It is already fixed in a pending pull. Do not merge.

On Wed, Feb 1, 2023, 09:57 James Bottomley <jejb@linux.ibm.com> wrote:

On Wed, 2023-02-01 at 14:35 +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 01, 2023 at 08:57:10AM -0500, James Bottomley wrote:
> > The origin commit for rng seeding 67f7e426e5 ("hw/i386: pass RNG
> > seed
> > via setup_data entry") modifies the kernel image file to append a
> > random seed.  Obviously this makes the hash of the kernel file
> > non-deterministic and so breaks both measured and some signed
> > boots.
>
> I recall raising that at the time
>
>   https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg00710.html
>
> and Jason pointed me to a followup which I tested and believe
> fixed it for SEV:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg00601.html
>
> but it doesn't look like that second patch ever merged. We went
> through so many patches I think it probably got obsoleted by
> something else, and no one rechecked SEV again.

The kernel file problem is a pretty huge one.  OVMF lays it down on an
internal file system and without the second patch, it now contains
random junk at the end.  Anything that hashes the whole file (which
includes not only the measured direct boot but also grub signatures and
probably other bootloader signing mechanisms) will have an issue.

> > The commit notes it's only for non-EFI (because EFI has a different
> > RNG seeding mechanism) so, since there are no non-EFI q35 systems,
> > this should be disabled for the whole of the q35 machine type to
> > bring back deterministic kernel file hashes.
>
> SeaBIOS is the default firmware for both q35 and i440fx. The
> majority of systems using q35 will be non-EFI today, and that
> is what the random seed was intended to address. I don't think
> we can just disable this for the whole of q35.
>
> When you say it breaks measured / signed boots, I presume you
> are specifically referring to SEV kernel hashes measurements ?
> Or is there a more general problem to solve ?

No it generally breaks measured/signed boots because it adds random
junk to the kernel file.  The second patch will fix this if you apply
it because setup data isn't measured or signed (yet ... however see the
linux-coco debate about how it should be).

I also note there was a v3 of the patch and considerable discussion
saying it couldn't work:

https://lore.kernel.org/qemu-devel/20220804230411.17720-1-Jason@zx2c4.com/

Which is likely why it never went in ... although the discussion does
seem to resolve towards the end.

James