All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ppc/spapr: advertise secure boot in the guest device tree
@ 2021-05-10 12:07 Daniel Axtens
  2021-05-11  4:50 ` David Gibson
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Axtens @ 2021-05-10 12:07 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, Daniel Axtens

The ibm,secure-boot property of the / node determines how firmware
and the operating system should enforce secure boot. The meaning
of the various values are:

 0   - secure boot is disabled
 1   - secure boot in log-only mode
 2   - secure boot enabled and enforced
 3-9 - secure boot enabled and enforced; requirements at the
         discretion of the operating system

We expose this as two properties:

 - secure-boot: determines whether the property is advertised in the
                guest device tree. The default is false.

 - secure-boot-level: what value is advertised if enabled?
                      The default is 2.

This doesn't make the firmware or OS actually _do_ any verification, it
just advises them that they should.

Signed-off-by: Daniel Axtens <dja@axtens.net>

---

Linux already reads this property. Versions of SLOF and grub that do
verification are available on my GitHub:
 - github.com/daxtens/SLOF branch ibm,secure-boot (not production ready!)
 - github.com/daxtens/grub branch appendedsig-2.06
---
 hw/ppc/spapr.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  4 ++++
 2 files changed, 46 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3b1a5ed86518..544a412c3d18 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1157,6 +1157,20 @@ static void spapr_dt_hypervisor(SpaprMachineState *spapr, void *fdt)
     }
 }
 
+static void spapr_dt_stb(SpaprMachineState *spapr, void *fdt)
+{
+    /*
+     * PowerVM may provide fw-secure-boot, which purports to tell a partition
+     * if the underlying firmware was booted securely. It's not meaningful
+     * for KVM as there are no agreed semantics for what it would mean (host
+     * secure boot only gives you integrity for the host kernel, not host
+     * qemu). So we omit the property for now.
+     */
+    if (spapr->secure_boot)
+        _FDT(fdt_setprop_cell(fdt, 0, "ibm,secure-boot",
+            spapr->secure_boot_level));
+}
+
 void *spapr_build_fdt(SpaprMachineState *spapr, bool reset, size_t space)
 {
     MachineState *machine = MACHINE(spapr);
@@ -1263,6 +1277,9 @@ void *spapr_build_fdt(SpaprMachineState *spapr, bool reset, size_t space)
         spapr_dt_hypervisor(spapr, fdt);
     }
 
+    /* /ibm,secureboot */
+    spapr_dt_stb(spapr, fdt);
+
     /* Build memory reserve map */
     if (reset) {
         if (spapr->kernel_size) {
@@ -3298,6 +3315,20 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp)
     spapr->host_serial = g_strdup(value);
 }
 
+static bool spapr_get_secure_boot(Object *obj, Error **errp)
+{
+    SpaprMachineState *spapr = SPAPR_MACHINE(obj);
+
+    return spapr->secure_boot;
+}
+
+static void spapr_set_secure_boot(Object *obj, bool value, Error **errp)
+{
+    SpaprMachineState *spapr = SPAPR_MACHINE(obj);
+
+    spapr->secure_boot = value;
+}
+
 static void spapr_instance_init(Object *obj)
 {
     SpaprMachineState *spapr = SPAPR_MACHINE(obj);
@@ -3353,6 +3384,17 @@ static void spapr_instance_init(Object *obj)
         spapr_get_host_serial, spapr_set_host_serial);
     object_property_set_description(obj, "host-serial",
         "Host serial number to advertise in guest device tree");
+
+    /* If we have secure boot, the default level is 2: enable and enforce */
+    spapr->secure_boot_level = 2;
+    object_property_add_bool(obj, "secure-boot",
+        spapr_get_secure_boot, spapr_set_secure_boot);
+    object_property_set_description(obj, "secure-boot",
+        "Advertise secure boot in the guest device tree");
+    object_property_add_uint8_ptr(obj, "secure-boot-level",
+        &spapr->secure_boot_level, OBJ_PROP_FLAG_READWRITE);
+    object_property_set_description(obj, "secure-boot-level",
+        "Level of secure boot advertised in the guest device tree");
 }
 
 static void spapr_machine_finalizefn(Object *obj)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index c421410e3fb8..d829d0c27011 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -210,6 +210,10 @@ struct SpaprMachineState {
     int fwnmi_machine_check_interlock;
     QemuCond fwnmi_machine_check_interlock_cond;
 
+    /* Secure Boot */
+    bool secure_boot;
+    uint8_t secure_boot_level;
+
     /*< public >*/
     char *kvm_type;
     char *host_model;
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ppc/spapr: advertise secure boot in the guest device tree
  2021-05-10 12:07 [PATCH] ppc/spapr: advertise secure boot in the guest device tree Daniel Axtens
@ 2021-05-11  4:50 ` David Gibson
  2021-05-12  3:47   ` Daniel Axtens
  0 siblings, 1 reply; 3+ messages in thread
From: David Gibson @ 2021-05-11  4:50 UTC (permalink / raw)
  To: Daniel Axtens; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 5081 bytes --]

On Mon, May 10, 2021 at 10:07:13PM +1000, Daniel Axtens wrote:
> The ibm,secure-boot property of the / node determines how firmware
> and the operating system should enforce secure boot. The meaning
> of the various values are:
> 
>  0   - secure boot is disabled
>  1   - secure boot in log-only mode
>  2   - secure boot enabled and enforced
>  3-9 - secure boot enabled and enforced; requirements at the
>          discretion of the operating system
> 
> We expose this as two properties:
> 
>  - secure-boot: determines whether the property is advertised in the
>                 guest device tree. The default is false.
> 
>  - secure-boot-level: what value is advertised if enabled?
>                       The default is 2.
> 
> This doesn't make the firmware or OS actually _do_ any verification, it
> just advises them that they should.

So.. what's the point?  AFAIK we have no secure boot support in SLOF,
so what would advertising it in the device tree accomplish?

> 
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> 
> ---
> 
> Linux already reads this property. Versions of SLOF and grub that do
> verification are available on my GitHub:
>  - github.com/daxtens/SLOF branch ibm,secure-boot (not production ready!)
>  - github.com/daxtens/grub branch appendedsig-2.06
> ---
>  hw/ppc/spapr.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |  4 ++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3b1a5ed86518..544a412c3d18 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1157,6 +1157,20 @@ static void spapr_dt_hypervisor(SpaprMachineState *spapr, void *fdt)
>      }
>  }
>  
> +static void spapr_dt_stb(SpaprMachineState *spapr, void *fdt)
> +{
> +    /*
> +     * PowerVM may provide fw-secure-boot, which purports to tell a partition
> +     * if the underlying firmware was booted securely. It's not meaningful
> +     * for KVM as there are no agreed semantics for what it would mean (host
> +     * secure boot only gives you integrity for the host kernel, not host
> +     * qemu). So we omit the property for now.
> +     */
> +    if (spapr->secure_boot)
> +        _FDT(fdt_setprop_cell(fdt, 0, "ibm,secure-boot",
> +            spapr->secure_boot_level));
> +}
> +
>  void *spapr_build_fdt(SpaprMachineState *spapr, bool reset, size_t space)
>  {
>      MachineState *machine = MACHINE(spapr);
> @@ -1263,6 +1277,9 @@ void *spapr_build_fdt(SpaprMachineState *spapr, bool reset, size_t space)
>          spapr_dt_hypervisor(spapr, fdt);
>      }
>  
> +    /* /ibm,secureboot */
> +    spapr_dt_stb(spapr, fdt);
> +
>      /* Build memory reserve map */
>      if (reset) {
>          if (spapr->kernel_size) {
> @@ -3298,6 +3315,20 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp)
>      spapr->host_serial = g_strdup(value);
>  }
>  
> +static bool spapr_get_secure_boot(Object *obj, Error **errp)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(obj);
> +
> +    return spapr->secure_boot;
> +}
> +
> +static void spapr_set_secure_boot(Object *obj, bool value, Error **errp)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(obj);
> +
> +    spapr->secure_boot = value;
> +}
> +
>  static void spapr_instance_init(Object *obj)
>  {
>      SpaprMachineState *spapr = SPAPR_MACHINE(obj);
> @@ -3353,6 +3384,17 @@ static void spapr_instance_init(Object *obj)
>          spapr_get_host_serial, spapr_set_host_serial);
>      object_property_set_description(obj, "host-serial",
>          "Host serial number to advertise in guest device tree");
> +
> +    /* If we have secure boot, the default level is 2: enable and enforce */
> +    spapr->secure_boot_level = 2;
> +    object_property_add_bool(obj, "secure-boot",
> +        spapr_get_secure_boot, spapr_set_secure_boot);
> +    object_property_set_description(obj, "secure-boot",
> +        "Advertise secure boot in the guest device tree");
> +    object_property_add_uint8_ptr(obj, "secure-boot-level",
> +        &spapr->secure_boot_level, OBJ_PROP_FLAG_READWRITE);
> +    object_property_set_description(obj, "secure-boot-level",
> +        "Level of secure boot advertised in the guest device tree");
>  }
>  
>  static void spapr_machine_finalizefn(Object *obj)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c421410e3fb8..d829d0c27011 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -210,6 +210,10 @@ struct SpaprMachineState {
>      int fwnmi_machine_check_interlock;
>      QemuCond fwnmi_machine_check_interlock_cond;
>  
> +    /* Secure Boot */
> +    bool secure_boot;
> +    uint8_t secure_boot_level;
> +
>      /*< public >*/
>      char *kvm_type;
>      char *host_model;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ppc/spapr: advertise secure boot in the guest device tree
  2021-05-11  4:50 ` David Gibson
@ 2021-05-12  3:47   ` Daniel Axtens
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Axtens @ 2021-05-12  3:47 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel


> So.. what's the point?  AFAIK we have no secure boot support in SLOF,
> so what would advertising it in the device tree accomplish?

Linux reads the property and enters secure boot mode:
commit 61f879d97ce4 ("powerpc/pseries: Detect secure and trusted boot state of the system.")

grub patches to read the property and enter lockdown are on the list:
https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00359.html
(patch 19)

I have very basic SLOF support:
>>  - github.com/daxtens/SLOF branch ibm,secure-boot (not production ready!)

The property is extremely useful in developing and testing secure boot
support all the way up the stack.

Kind regards,
Daniel


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-12  3:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10 12:07 [PATCH] ppc/spapr: advertise secure boot in the guest device tree Daniel Axtens
2021-05-11  4:50 ` David Gibson
2021-05-12  3:47   ` Daniel Axtens

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.