| Message ID | 20211215120926.1696302-1-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/arm: add control knob to disable kaslr_seed via DTB | expand |
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>
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>
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
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
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>
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);
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(-)