[PATCH 1/1] efi_loader: pre-seed UEFI variables

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Heinrich,

On Wed, Jul 15, 2020 at 07:42:15PM +0200, Heinrich Schuchardt wrote:
> Include a file with the initial values for non-volatile UEFI variables
> into the U-Boot binary. If this variable is set, changes to variable PK
> will not be allowed.

Why do you want to impose this restriction while those authenticated
variables are protected properly?

At least, we'd better have a dedicated configuration option
apart from initialization with a file.

Other questions:
Is there any description about a file format?
Is there any host tool to support creating a file?

-Takahiro Akashi

> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/asm-generic/sections.h |  2 ++
>  include/efi_variable.h         |  8 ++++++++
>  lib/efi_loader/Kconfig         | 23 +++++++++++++++++++++++
>  lib/efi_loader/Makefile        |  6 +++++-
>  lib/efi_loader/efi_var_file.c  |  8 +-------
>  lib/efi_loader/efi_var_seed.S  | 17 +++++++++++++++++
>  lib/efi_loader/efi_variable.c  | 19 +++++++++++++++++--
>  7 files changed, 73 insertions(+), 10 deletions(-)
>  create mode 100644 lib/efi_loader/efi_var_seed.S
> 
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index 17a31ec788..0577238d60 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -25,6 +25,8 @@ extern char __initdata_begin[], __initdata_end[];
>  extern char __start_rodata[], __end_rodata[];
>  extern char __efi_helloworld_begin[];
>  extern char __efi_helloworld_end[];
> +extern char __efi_var_file_begin[];
> +extern char __efi_var_file_end[];
> 
>  /* Start and end of .ctors section - used for constructor calls. */
>  extern char __ctors_start[], __ctors_end[];
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 021a74f309..17f25ad7a4 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -138,6 +138,14 @@ struct efi_var_file {
>   */
>  efi_status_t efi_var_to_file(void);
> 
> +/**
> + * efi_var_restore() - restore EFI variables from buffer
> + *
> + * @buf:	buffer
> + * Return:	status code
> + */
> +efi_status_t efi_var_restore(struct efi_var_file *buf);
> +
>  /**
>   * efi_var_from_file() - read variables from file
>   *
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 8827c76cc9..6017ffe9a6 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -50,6 +50,29 @@ config EFI_MM_COMM_TEE
> 
>  endchoice
> 
> +config EFI_VARIABLES_PRESEED
> +	bool "Initial values for UEFI variables"
> +	depends on EFI_VARIABLE_FILE_STORE
> +	help
> +	  Include a file with the initial values for non-volatile UEFI variables
> +	  into the U-Boot binary. If this configuration option is set, changes
> +	  to authentication related variables (PK, KEK, db, dbx) are not
> +	  allowed.
> +
> +if EFI_VARIABLES_PRESEED
> +
> +config EFI_VAR_SEED_FILE
> +	string "File with initial values of non-volatile UEFI variables"
> +	default ubootefi.var
> +	help
> +	  File with initial values of non-volatile UEFI variables. The file must
> +	  be in the same format as the storage in the EFI system partition. The
> +	  easiest way to create it is by setting the non-volatile variables in
> +	  U-Boot. If a relative file path is used, it is relative to the source
> +	  directory.
> +
> +endif
> +
>  config EFI_GET_TIME
>  	bool "GetTime() runtime service"
>  	depends on DM_RTC
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index f81ec8d277..441ac9432e 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -6,7 +6,7 @@
>  # This file only gets included with CONFIG_EFI_LOADER set, so all
>  # object inclusion implicitly depends on it
> 
> -asflags-y += -DHOST_ARCH="$(HOST_ARCH)"
> +asflags-y += -DHOST_ARCH="$(HOST_ARCH)" -I.
>  ccflags-y += -DHOST_ARCH="$(HOST_ARCH)"
> 
>  CFLAGS_efi_boottime.o += \
> @@ -42,6 +42,7 @@ obj-y += efi_variable_tee.o
>  else
>  obj-y += efi_variable.o
>  obj-y += efi_var_file.o
> +obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
>  endif
>  obj-y += efi_watchdog.o
>  obj-$(CONFIG_LCD) += efi_gop.o
> @@ -53,3 +54,6 @@ obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
>  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
>  obj-y += efi_signature.o
> +
> +EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> +$(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> index 880c279aef..6f9d76f2a2 100644
> --- a/lib/efi_loader/efi_var_file.c
> +++ b/lib/efi_loader/efi_var_file.c
> @@ -159,13 +159,7 @@ error:
>  #endif
>  }
> 
> -/**
> - * efi_var_restore() - restore EFI variables from buffer
> - *
> - * @buf:	buffer
> - * Return:	status code
> - */
> -static efi_status_t __maybe_unused efi_var_restore(struct efi_var_file *buf)
> +efi_status_t efi_var_restore(struct efi_var_file *buf)
>  {
>  	struct efi_var_entry *var, *last_var;
>  	efi_status_t ret;
> diff --git a/lib/efi_loader/efi_var_seed.S b/lib/efi_loader/efi_var_seed.S
> new file mode 100644
> index 0000000000..e0a40cf46c
> --- /dev/null
> +++ b/lib/efi_loader/efi_var_seed.S
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Predefined UEFI variables
> + *
> + * Copyright (c) 2020, Heinrich Schuchardt <xypron.glpk@gmx.de>
> + */
> +
> +#include <config.h>
> +
> +.section .rodata.efi_seed.init,"a"
> +.balign 16
> +.global __efi_var_file_begin
> +__efi_var_file_begin:
> +.incbin CONFIG_EFI_VAR_SEED_FILE
> +.global __efi_var_file_end
> +__efi_var_file_end:
> +.balign 16
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index ecbc4f7f54..39a8482903 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -5,12 +5,15 @@
>   * Copyright (c) 2017 Rob Clark
>   */
> 
> +#define LOG_CATEGORY LOGC_EFI
> +
>  #include <common.h>
>  #include <efi_loader.h>
>  #include <efi_variable.h>
>  #include <env.h>
>  #include <env_internal.h>
>  #include <hexdump.h>
> +#include <log.h>
>  #include <malloc.h>
>  #include <rtc.h>
>  #include <search.h>
> @@ -18,7 +21,7 @@
>  #include <crypto/pkcs7_parser.h>
>  #include <linux/compat.h>
>  #include <u-boot/crc.h>
> -
> +#include <asm/sections.h>
> 
>  #ifdef CONFIG_EFI_SECURE_BOOT
>  static u8 pkcs7_hdr[] = {
> @@ -365,10 +368,16 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
>  	delete = !append && (!data_size || !attributes);
> 
>  	/* check attributes */
> +	var_type = efi_auth_var_get_type(variable_name, vendor);
>  	if (var) {
>  		if (ro_check && (var->attr & EFI_VARIABLE_READ_ONLY))
>  			return EFI_WRITE_PROTECTED;
> 
> +		if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
> +			if (var_type != EFI_AUTH_VAR_NONE)
> +				return EFI_WRITE_PROTECTED;
> +		}
> +
>  		/* attributes won't be changed */
>  		if (!delete &&
>  		    ((ro_check && var->attr != attributes) ||
> @@ -386,7 +395,6 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
>  			return EFI_NOT_FOUND;
>  	}
> 
> -	var_type = efi_auth_var_get_type(variable_name, vendor);
>  	if (var_type != EFI_AUTH_VAR_NONE) {
>  		/* authentication is mandatory */
>  		if (!(attributes &
> @@ -589,5 +597,12 @@ efi_status_t efi_init_variables(void)
>  	if (ret != EFI_SUCCESS)
>  		return ret;
> 
> +	if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
> +		ret = efi_var_restore((struct efi_var_file *)
> +				      __efi_var_file_begin);
> +		if (ret != EFI_SUCCESS)
> +			log_err("Invalid EFI variable seed\n");
> +	}
> +
>  	return efi_var_from_file();
>  }
> --
> 2.27.0
>