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

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