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

[Daniel P . Berrangé]

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

> Obviously this still leaves the legacy bios case broken for at least
> measured boot, but I don't think anyone cares about that now.
> 
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> ---
>  hw/i386/pc_q35.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 83c57c6eb1..11e8dd7ca7 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -357,6 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
>      pcmc->default_nic_model = "e1000e";
>      pcmc->pci_root_uid = 0;
>      pcmc->default_cpu_version = 1;
> +    pcmc->legacy_no_rng_seed = true;
>  
>      m->family = "pc_q35";
>      m->desc = "Standard PC (Q35 + ICH9, 2009)";
> @@ -394,9 +395,7 @@ DEFINE_Q35_MACHINE(v7_2, "pc-q35-7.2", NULL,
>  
>  static void pc_q35_7_1_machine_options(MachineClass *m)
>  {
> -    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>      pc_q35_7_2_machine_options(m);
> -    pcmc->legacy_no_rng_seed = true;
>      compat_props_add(m->compat_props, hw_compat_7_1,
> hw_compat_7_1_len);
>      compat_props_add(m->compat_props, pc_compat_7_1,
> pc_compat_7_1_len);
>  }

This patch changes behaviour of the pc-q35-7.2 machine type. Any
change will need to be in latest development 8.0 machine type only

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