[Skiboot] [RFC 7/8] secvar/backend: add a switchable-mode edk2-based backend

[Daniel Axtens]

Eric Richter <erichte at linux.ibm.com> writes:

> This patch introduces a new backend driver that wraps the existing
> edk2-compat-v1 driver and the new edk2-compat-static driver, to switch
> functionality based on the secure boot mode as exposed by the secboot_tpm
> storage driver.
>
> If the secure boot mode is set to SYSTEM_MODE, all hooks will be passed to
> the edk2-compat-static driver. Otherwise, if the mode is USER_MODE, the
> regular edk2-compat-v1 driver hooks are called.
>
> As with the secboot_tpm_switchable driver, asserting physical presence
> will switch between modes: clear-os-keys will disable secure boot and enter
> USER_MODE, while reset-default-keys will enforce secure boot with default
> keys via the edk2-compat-static driver.
>
> RFC NOTE: I don't like how this driver has to directly interact with a
> specific storage driver. I am considering instead leaving a crumb in the
> device tree for this driver to inspect. That way, if documented as an
> interface, a future "switchable" storage driver may also be compatible
> with this switchable backend driver.
>
> The name "switchable" is subject to change, I did not put a lot of time
> into coming up with good names, so consider all of them placeholders.
>
> Signed-off-by: Eric Richter <erichte at linux.ibm.com>
> ---
>  include/secvar.h                        |  1 +
>  libstb/secvar/backend/Makefile.inc      |  2 +-
>  libstb/secvar/backend/edk2-switchable.c | 46 +++++++++++++++++++++++++
>  3 files changed, 48 insertions(+), 1 deletion(-)
>  create mode 100644 libstb/secvar/backend/edk2-switchable.c
>
> diff --git a/include/secvar.h b/include/secvar.h
> index 3b439eaf..2aa52ec2 100644
> --- a/include/secvar.h
> +++ b/include/secvar.h
> @@ -41,6 +41,7 @@ extern struct secvar_storage_driver secboot_tpm_driver;
>  extern struct secvar_storage_driver secboot_tpm_switchable_driver;
>  extern struct secvar_backend_driver edk2_compatible_v1;
>  extern struct secvar_backend_driver edk2_compatible_v1_static;
> +extern struct secvar_backend_driver edk2_switchable_driver;
>  
>  int secvar_main(struct secvar_storage_driver, struct secvar_backend_driver);
>  
> diff --git a/libstb/secvar/backend/Makefile.inc b/libstb/secvar/backend/Makefile.inc
> index b929769f..b60efe83 100644
> --- a/libstb/secvar/backend/Makefile.inc
> +++ b/libstb/secvar/backend/Makefile.inc
> @@ -5,7 +5,7 @@ SECVAR_BACKEND_DIR = libstb/secvar/backend
>  
>  SUBDIRS += $(SECVAR_BACKEND_DIR)
>  
> -SECVAR_BACKEND_OBJS = edk2-compat.o edk2-compat-process.o edk2-compat-reset.o edk2-compat-static.o
> +SECVAR_BACKEND_OBJS = edk2-compat.o edk2-compat-process.o edk2-compat-reset.o edk2-compat-static.o edk2-switchable.o
>  SECVAR_BACKEND = $(SECVAR_BACKEND_DIR)/built-in.a
>  
>  $(SECVAR_BACKEND): $(SECVAR_BACKEND_OBJS:%=$(SECVAR_BACKEND_DIR)/%)
> diff --git a/libstb/secvar/backend/edk2-switchable.c b/libstb/secvar/backend/edk2-switchable.c
> new file mode 100644
> index 00000000..f2883928
> --- /dev/null
> +++ b/libstb/secvar/backend/edk2-switchable.c
> @@ -0,0 +1,46 @@
> +#include <opal.h>
> +#include <secvar.h>
> +#include "../storage/secboot_tpm_switchable.h"
> +#include "../secvar.h"
> +
> +static int switchable_pre_process(struct list_head *variable_bank,
> +                                  struct list_head *update_bank)
> +{
> +        if (mode_value == USER_MODE)
> +                return edk2_compatible_v1.pre_process(variable_bank, update_bank);
> +        return edk2_compatible_v1_static.pre_process(variable_bank, update_bank);
> +}
> +
> +
> +static int switchable_process(struct list_head *variable_bank,
> +                              struct list_head *update_bank)
> +{
> +        if (mode_value == USER_MODE)
> +                return edk2_compatible_v1.process(variable_bank, update_bank);
> +        return edk2_compatible_v1_static.process(variable_bank, update_bank);
> +}
> +
> +static int switchable_post_process(struct list_head *variable_bank,
> +                                   struct list_head *update_bank)
> +{
> +        if (mode_value == USER_MODE)
> +                return edk2_compatible_v1.post_process(variable_bank, update_bank);
> +        return edk2_compatible_v1_static.post_process(variable_bank, update_bank);
> +}
> +
> +static int switchable_validate(struct secvar *var)
> +{
> +        if (mode_value == USER_MODE)
> +                return edk2_compatible_v1.validate(var);
> +        return edk2_compatible_v1_static.validate(var);
> +};
> +
> +struct secvar_backend_driver edk2_switchable_driver = {
> +	.pre_process = switchable_pre_process,
> +	.process = switchable_process,
> +	.post_process = switchable_post_process,
> +	.validate = switchable_validate,
> +        // TODO: perhaps this should also be a composite compatible?
> +        // The interface isn't changing, but it might be useful to know if using a switchable backend
> +	.compatible = "ibm,edk2-compat-v1",

As I suggested in the previous patch I don't think static keys is
`compatible` with ibm,edk2-compat-v1.

So I think this should change at boot time. If you are in user mode, it
should be
 compatible = "ibm,edk2-compat-switchable-v1", "ibm,edk2-compat-v1"
and in static mode
 compatible = "ibm,edk2-compat-switchable-v1", "ibm,edk2-compat-static-v1"

(this is in DT notation per
https://elinux.org/Device_Tree_Usage#Understanding_the_compatible_Property ,
idk how we express this in skiboot)

Kind regards,
Daniel