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

[Daniel P . Berrangé]

On Wed, Feb 01, 2023 at 09:56:35AM -0500, James Bottomley 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).

BTW, I can't find a reference now, but I recall being told that when
using -kernel,  OVMF won't enforce SecureBoot. ie it'll do the checks
but ignore any failure and carry on. It should still be reflected in
the vTPM measurements though.

> 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.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|