[PATCH] hw/arm: add control knob to disable kaslr_seed via DTB

[PATCH] hw/arm: add control knob to disable kaslr_seed via DTB

[Alex Bennée]

Generally a guest needs an external source of randomness to properly
enable things like address space randomisation. However in a trusted
boot environment where the firmware will cryptographically verify
components having random data in the DTB will cause verification to
fail. Add a control knob so we can prevent this being added to the
system DTB.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Jerome Forissier <jerome@forissier.org>
---
 docs/system/arm/virt.rst |  7 +++++++
 include/hw/arm/virt.h    |  1 +
 hw/arm/virt.c            | 32 ++++++++++++++++++++++++++++++--
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 850787495b..c86a4808df 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -121,6 +121,13 @@ ras
   Set ``on``/``off`` to enable/disable reporting host memory errors to a guest
   using ACPI and guest external abort exceptions. The default is off.
 
+kaslr-dtb-seed
+  Set ``on``/``off`` to pass a random seed via the guest dtb to use for features
+  like address space randomisation. The default is ``on``. You will want
+  to disable it if your trusted boot chain will verify the DTB it is
+  passed. It would be the responsibility of the firmware to come up
+  with a seed and pass it on if it wants to.
+
 Linux guest kernel configuration
 """"""""""""""""""""""""""""""""
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index dc6b66ffc8..acd0665fe7 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -148,6 +148,7 @@ struct VirtMachineState {
     bool virt;
     bool ras;
     bool mte;
+    bool kaslr_dtb_seed;
     OnOffAuto acpi;
     VirtGICType gic_version;
     VirtIOMMUType iommu;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 30da05dfe0..4496612e89 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -248,11 +248,15 @@ static void create_fdt(VirtMachineState *vms)
 
     /* /chosen must exist for load_dtb to fill in necessary properties later */
     qemu_fdt_add_subnode(fdt, "/chosen");
-    create_kaslr_seed(ms, "/chosen");
+    if (vms->kaslr_dtb_seed) {
+        create_kaslr_seed(ms, "/chosen");
+    }
 
     if (vms->secure) {
         qemu_fdt_add_subnode(fdt, "/secure-chosen");
-        create_kaslr_seed(ms, "/secure-chosen");
+        if (vms->kaslr_dtb_seed) {
+            create_kaslr_seed(ms, "/secure-chosen");
+        }
     }
 
     /* Clock node, for the benefit of the UART. The kernel device tree
@@ -2236,6 +2240,20 @@ static void virt_set_its(Object *obj, bool value, Error **errp)
     vms->its = value;
 }
 
+static bool virt_get_kaslr_dtb_seed(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return vms->kaslr_dtb_seed;
+}
+
+static void virt_set_kaslr_dtb_seed(Object *obj, bool value, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->kaslr_dtb_seed = value;
+}
+
 static char *virt_get_oem_id(Object *obj, Error **errp)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -2765,6 +2783,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
                                           "Set on/off to enable/disable "
                                           "ITS instantiation");
 
+    object_class_property_add_bool(oc, "kaslr-dtb-seed",
+                                   virt_get_kaslr_dtb_seed,
+                                   virt_set_kaslr_dtb_seed);
+    object_class_property_set_description(oc, "kaslr-dtb-seed",
+                                          "Set off to disable passing of kaslr "
+                                          "dtb node to guest");
+
     object_class_property_add_str(oc, "x-oem-id",
                                   virt_get_oem_id,
                                   virt_set_oem_id);
@@ -2829,6 +2854,9 @@ static void virt_instance_init(Object *obj)
     /* MTE is disabled by default.  */
     vms->mte = false;
 
+    /* Supply a kaslr-seed by default */
+    vms->kaslr_dtb_seed = true;
+
     vms->irqmap = a15irqmap;
 
     virt_flash_create(vms);
-- 
2.30.2

Re: [PATCH] hw/arm: add control knob to disable kaslr_seed via DTB

[Ilias Apalodimas]

Hi Alex,

On Wed, 15 Dec 2021 at 14:09, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Generally a guest needs an external source of randomness to properly
> enable things like address space randomisation. However in a trusted
> boot environment where the firmware will cryptographically verify
> components having random data in the DTB will cause verification to
> fail. Add a control knob so we can prevent this being added to the
> system DTB.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Jerome Forissier <jerome@forissier.org>
> ---
>  docs/system/arm/virt.rst |  7 +++++++
>  include/hw/arm/virt.h    |  1 +
>  hw/arm/virt.c            | 32 ++++++++++++++++++++++++++++++--
>  3 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> index 850787495b..c86a4808df 100644
> --- a/docs/system/arm/virt.rst
> +++ b/docs/system/arm/virt.rst
> @@ -121,6 +121,13 @@ ras
>    Set ``on``/``off`` to enable/disable reporting host memory errors to a guest
>    using ACPI and guest external abort exceptions. The default is off.
>
> +kaslr-dtb-seed
> +  Set ``on``/``off`` to pass a random seed via the guest dtb to use for features
> +  like address space randomisation. The default is ``on``. You will want
> +  to disable it if your trusted boot chain will verify the DTB it is
> +  passed. It would be the responsibility of the firmware to come up
> +  with a seed and pass it on if it wants to.
> +
>  Linux guest kernel configuration
>  """"""""""""""""""""""""""""""""
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index dc6b66ffc8..acd0665fe7 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -148,6 +148,7 @@ struct VirtMachineState {
>      bool virt;
>      bool ras;
>      bool mte;
> +    bool kaslr_dtb_seed;
>      OnOffAuto acpi;
>      VirtGICType gic_version;
>      VirtIOMMUType iommu;
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 30da05dfe0..4496612e89 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -248,11 +248,15 @@ static void create_fdt(VirtMachineState *vms)
>
>      /* /chosen must exist for load_dtb to fill in necessary properties later */
>      qemu_fdt_add_subnode(fdt, "/chosen");
> -    create_kaslr_seed(ms, "/chosen");
> +    if (vms->kaslr_dtb_seed) {
> +        create_kaslr_seed(ms, "/chosen");
> +    }
>
>      if (vms->secure) {
>          qemu_fdt_add_subnode(fdt, "/secure-chosen");
> -        create_kaslr_seed(ms, "/secure-chosen");
> +        if (vms->kaslr_dtb_seed) {
> +            create_kaslr_seed(ms, "/secure-chosen");
> +        }
>      }
>
>      /* Clock node, for the benefit of the UART. The kernel device tree
> @@ -2236,6 +2240,20 @@ static void virt_set_its(Object *obj, bool value, Error **errp)
>      vms->its = value;
>  }
>
> +static bool virt_get_kaslr_dtb_seed(Object *obj, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    return vms->kaslr_dtb_seed;
> +}
> +
> +static void virt_set_kaslr_dtb_seed(Object *obj, bool value, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    vms->kaslr_dtb_seed = value;
> +}
> +
>  static char *virt_get_oem_id(Object *obj, Error **errp)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(obj);
> @@ -2765,6 +2783,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>                                            "Set on/off to enable/disable "
>                                            "ITS instantiation");
>
> +    object_class_property_add_bool(oc, "kaslr-dtb-seed",
> +                                   virt_get_kaslr_dtb_seed,
> +                                   virt_set_kaslr_dtb_seed);
> +    object_class_property_set_description(oc, "kaslr-dtb-seed",
> +                                          "Set off to disable passing of kaslr "
> +                                          "dtb node to guest");
> +
>      object_class_property_add_str(oc, "x-oem-id",
>                                    virt_get_oem_id,
>                                    virt_set_oem_id);
> @@ -2829,6 +2854,9 @@ static void virt_instance_init(Object *obj)
>      /* MTE is disabled by default.  */
>      vms->mte = false;
>
> +    /* Supply a kaslr-seed by default */
> +    vms->kaslr_dtb_seed = true;
> +
>      vms->irqmap = a15irqmap;
>
>      virt_flash_create(vms);
> --
> 2.30.2
>

This is particularly useful if the bootloader uses a TPM to measures
the DTB and end up with a measured boot flow.

Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH] hw/arm: add control knob to disable kaslr_seed via DTB

[Jerome Forissier]

On 12/15/21 14:15, Ilias Apalodimas wrote:
> Hi Alex,
> 
> On Wed, 15 Dec 2021 at 14:09, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Generally a guest needs an external source of randomness to properly
>> enable things like address space randomisation. However in a trusted
>> boot environment where the firmware will cryptographically verify
>> components having random data in the DTB will cause verification to
>> fail. Add a control knob so we can prevent this being added to the
>> system DTB.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> Cc: Jerome Forissier <jerome@forissier.org>
>> ---
>>  docs/system/arm/virt.rst |  7 +++++++
>>  include/hw/arm/virt.h    |  1 +
>>  hw/arm/virt.c            | 32 ++++++++++++++++++++++++++++++--
>>  3 files changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
>> index 850787495b..c86a4808df 100644
>> --- a/docs/system/arm/virt.rst
>> +++ b/docs/system/arm/virt.rst
>> @@ -121,6 +121,13 @@ ras
>>    Set ``on``/``off`` to enable/disable reporting host memory errors to a guest
>>    using ACPI and guest external abort exceptions. The default is off.
>>
>> +kaslr-dtb-seed
>> +  Set ``on``/``off`` to pass a random seed via the guest dtb to use for features
>> +  like address space randomisation. The default is ``on``. You will want
>> +  to disable it if your trusted boot chain will verify the DTB it is
>> +  passed. It would be the responsibility of the firmware to come up
>> +  with a seed and pass it on if it wants to.
>> +
>>  Linux guest kernel configuration
>>  """"""""""""""""""""""""""""""""
>>
>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>> index dc6b66ffc8..acd0665fe7 100644
>> --- a/include/hw/arm/virt.h
>> +++ b/include/hw/arm/virt.h
>> @@ -148,6 +148,7 @@ struct VirtMachineState {
>>      bool virt;
>>      bool ras;
>>      bool mte;
>> +    bool kaslr_dtb_seed;
>>      OnOffAuto acpi;
>>      VirtGICType gic_version;
>>      VirtIOMMUType iommu;
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 30da05dfe0..4496612e89 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -248,11 +248,15 @@ static void create_fdt(VirtMachineState *vms)
>>
>>      /* /chosen must exist for load_dtb to fill in necessary properties later */
>>      qemu_fdt_add_subnode(fdt, "/chosen");
>> -    create_kaslr_seed(ms, "/chosen");
>> +    if (vms->kaslr_dtb_seed) {
>> +        create_kaslr_seed(ms, "/chosen");
>> +    }
>>
>>      if (vms->secure) {
>>          qemu_fdt_add_subnode(fdt, "/secure-chosen");
>> -        create_kaslr_seed(ms, "/secure-chosen");
>> +        if (vms->kaslr_dtb_seed) {
>> +            create_kaslr_seed(ms, "/secure-chosen");
>> +        }
>>      }
>>
>>      /* Clock node, for the benefit of the UART. The kernel device tree
>> @@ -2236,6 +2240,20 @@ static void virt_set_its(Object *obj, bool value, Error **errp)
>>      vms->its = value;
>>  }
>>
>> +static bool virt_get_kaslr_dtb_seed(Object *obj, Error **errp)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    return vms->kaslr_dtb_seed;
>> +}
>> +
>> +static void virt_set_kaslr_dtb_seed(Object *obj, bool value, Error **errp)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    vms->kaslr_dtb_seed = value;
>> +}
>> +
>>  static char *virt_get_oem_id(Object *obj, Error **errp)
>>  {
>>      VirtMachineState *vms = VIRT_MACHINE(obj);
>> @@ -2765,6 +2783,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>                                            "Set on/off to enable/disable "
>>                                            "ITS instantiation");
>>
>> +    object_class_property_add_bool(oc, "kaslr-dtb-seed",
>> +                                   virt_get_kaslr_dtb_seed,
>> +                                   virt_set_kaslr_dtb_seed);
>> +    object_class_property_set_description(oc, "kaslr-dtb-seed",
>> +                                          "Set off to disable passing of kaslr "
>> +                                          "dtb node to guest");
>> +
>>      object_class_property_add_str(oc, "x-oem-id",
>>                                    virt_get_oem_id,
>>                                    virt_set_oem_id);
>> @@ -2829,6 +2854,9 @@ static void virt_instance_init(Object *obj)
>>      /* MTE is disabled by default.  */
>>      vms->mte = false;
>>
>> +    /* Supply a kaslr-seed by default */
>> +    vms->kaslr_dtb_seed = true;
>> +
>>      vms->irqmap = a15irqmap;
>>
>>      virt_flash_create(vms);
>> --
>> 2.30.2
>>
> 
> This is particularly useful if the bootloader uses a TPM to measures
> the DTB and end up with a measured boot flow.
> 
> Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> 

Fine with me too. FWIW:

Acked-by: Jerome Forissier <jerome@forissier.org>

Re: [PATCH] hw/arm: add control knob to disable kaslr_seed via DTB

[Peter Maydell]

On Wed, 15 Dec 2021 at 12:09, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Generally a guest needs an external source of randomness to properly
> enable things like address space randomisation. However in a trusted
> boot environment where the firmware will cryptographically verify
> components having random data in the DTB will cause verification to
> fail. Add a control knob so we can prevent this being added to the
> system DTB.

Given that the DTB is automatically generated for the virt board,
firmware has no way to guarantee that it's the same every time
anyway, surely ?

-- PMM

Re: [PATCH] hw/arm: add control knob to disable kaslr_seed via DTB

[Ilias Apalodimas]

Hi Peter

On Wed, Dec 15, 2021 at 01:36:07PM +0000, Peter Maydell wrote:
> On Wed, 15 Dec 2021 at 12:09, Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> > Generally a guest needs an external source of randomness to properly
> > enable things like address space randomisation. However in a trusted
> > boot environment where the firmware will cryptographically verify
> > components having random data in the DTB will cause verification to
> > fail. Add a control knob so we can prevent this being added to the
> > system DTB.
> 
> Given that the DTB is automatically generated for the virt board,
> firmware has no way to guarantee that it's the same every time
> anyway, surely ?

The firmware needs hardware assistance to do this. In order to have some 
guarantees on the loaded DTB, the firmware measures and extends the TPM PCRs.
In that case you'd expect the measurements to match across reboots assuming 
the command line hasn't been changed.  The kaslr-seed is obviously a deal
breaker for this. 

Thanks
/Ilias

Re: [PATCH] hw/arm: add control knob to disable kaslr_seed via DTB

[Heinrich Schuchardt]

On 12/15/21 13:09, Alex Bennée wrote:
> Generally a guest needs an external source of randomness to properly
> enable things like address space randomisation. However in a trusted
> boot environment where the firmware will cryptographically verify
> components having random data in the DTB will cause verification to
> fail. Add a control knob so we can prevent this being added to the
> system DTB.
>
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> Cc: Ilias Apalodimas<ilias.apalodimas@linaro.org>
> Cc: Jerome Forissier<jerome@forissier.org>
> ---
>   docs/system/arm/virt.rst |  7 +++++++
>   include/hw/arm/virt.h    |  1 +
>   hw/arm/virt.c            | 32 ++++++++++++++++++++++++++++++--
>   3 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> index 850787495b..c86a4808df 100644
> --- a/docs/system/arm/virt.rst
> +++ b/docs/system/arm/virt.rst
> @@ -121,6 +121,13 @@ ras
>     Set ``on``/``off`` to enable/disable reporting host memory errors to a guest
>     using ACPI and guest external abort exceptions. The default is off.
>

Tested on top of QEMU v6.1.0

Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>