[PATCH v3 3/6] eficonfig: add UEFI Secure Boot Key enrollment interface

Heinrich Schuchardt xypron.glpk at gmx.de
Sun Oct 23 10:17:53 CEST 2022


On 10/14/22 08:56, Masahisa Kojima wrote:
> This commit adds the menu-driven UEFI Secure Boot Key
> enrollment interface. User can enroll the PK, KEK, db
> and dbx by selecting EFI Signature Lists file.
> After the PK is enrolled, UEFI Secure Boot is enabled and
> EFI Signature Lists file must be signed by KEK or PK.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> ---
> Changes in v3:
> - fix error handling
>
> Changes in v2:
> - allow to enroll .esl file
> - fix typos
> - add function comments
>
>   cmd/Makefile          |   3 +
>   cmd/eficonfig.c       |   3 +
>   cmd/eficonfig_sbkey.c | 357 ++++++++++++++++++++++++++++++++++++++++++
>   include/efi_config.h  |   5 +
>   4 files changed, 368 insertions(+)
>   create mode 100644 cmd/eficonfig_sbkey.c
>
> diff --git a/cmd/Makefile b/cmd/Makefile
> index c95e09d058..f2f2857146 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -66,6 +66,9 @@ obj-$(CONFIG_CMD_EEPROM) += eeprom.o
>   obj-$(CONFIG_EFI) += efi.o
>   obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o
>   obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o
> +ifdef CONFIG_CMD_EFICONFIG
> +obj-$(CONFIG_EFI_SECURE_BOOT) += eficonfig_sbkey.o
> +endif
>   obj-$(CONFIG_CMD_ELF) += elf.o
>   obj-$(CONFIG_CMD_EROFS) += erofs.o
>   obj-$(CONFIG_HUSH_PARSER) += exit.o
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 0cb0770ac3..a72f07e671 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -2442,6 +2442,9 @@ static const struct eficonfig_item maintenance_menu_items[] = {
>   	{"Edit Boot Option", eficonfig_process_edit_boot_option},
>   	{"Change Boot Order", eficonfig_process_change_boot_order},
>   	{"Delete Boot Option", eficonfig_process_delete_boot_option},
> +#if (CONFIG_IS_ENABLED(EFI_SECURE_BOOT))
> +	{"Secure Boot Configuration", eficonfig_process_secure_boot_config},
> +#endif
>   	{"Quit", eficonfig_process_quit},
>   };
>
> diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
> new file mode 100644
> index 0000000000..cc27f78e66
> --- /dev/null
> +++ b/cmd/eficonfig_sbkey.c
> @@ -0,0 +1,357 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Menu-driven UEFI Secure Boot Key Maintenance
> + *
> + *  Copyright (c) 2022 Masahisa Kojima, Linaro Limited
> + */
> +
> +#include <ansi.h>
> +#include <common.h>
> +#include <charset.h>
> +#include <hexdump.h>
> +#include <log.h>
> +#include <malloc.h>
> +#include <menu.h>
> +#include <efi_loader.h>
> +#include <efi_config.h>
> +#include <efi_variable.h>
> +#include <crypto/pkcs7_parser.h>
> +
> +enum efi_sbkey_signature_type {
> +	SIG_TYPE_X509 = 0,
> +	SIG_TYPE_HASH,
> +	SIG_TYPE_CRL,
> +	SIG_TYPE_RSA2048,
> +};
> +
> +struct eficonfig_sigtype_to_str {
> +	efi_guid_t sig_type;
> +	char *str;
> +	enum efi_sbkey_signature_type type;
> +};
> +
> +static const struct eficonfig_sigtype_to_str sigtype_to_str[] = {
> +	{EFI_CERT_X509_GUID,		"X509",			SIG_TYPE_X509},
> +	{EFI_CERT_SHA256_GUID,		"SHA256",		SIG_TYPE_HASH},
> +	{EFI_CERT_X509_SHA256_GUID,	"X509_SHA256 CRL",	SIG_TYPE_CRL},
> +	{EFI_CERT_X509_SHA384_GUID,	"X509_SHA384 CRL",	SIG_TYPE_CRL},
> +	{EFI_CERT_X509_SHA512_GUID,	"X509_SHA512 CRL",	SIG_TYPE_CRL},
> +	/* U-Boot does not support the following signature types */
> +/*	{EFI_CERT_RSA2048_GUID,		"RSA2048",		SIG_TYPE_RSA2048}, */
> +/*	{EFI_CERT_RSA2048_SHA256_GUID,	"RSA2048_SHA256",	SIG_TYPE_RSA2048}, */
> +/*	{EFI_CERT_SHA1_GUID,		"SHA1",			SIG_TYPE_HASH}, */
> +/*	{EFI_CERT_RSA2048_SHA_GUID,	"RSA2048_SHA",		SIG_TYPE_RSA2048 }, */
> +/*	{EFI_CERT_SHA224_GUID,		"SHA224",		SIG_TYPE_HASH}, */
> +/*	{EFI_CERT_SHA384_GUID,		"SHA384",		SIG_TYPE_HASH}, */
> +/*	{EFI_CERT_SHA512_GUID,		"SHA512",		SIG_TYPE_HASH}, */
> +};
> +
> +/**
> + * is_secureboot_enabled() - check UEFI Secure Boot is enabled
> + *
> + * Return:	true when UEFI Secure Boot is enabled, false otherwise
> + */
> +static bool is_secureboot_enabled(void)
> +{
> +	efi_status_t ret;
> +	u8 secure_boot;
> +	efi_uintn_t size;
> +
> +	size = sizeof(secure_boot);
> +	ret = efi_get_variable_int(u"SecureBoot", &efi_global_variable_guid,
> +				   NULL, &size, &secure_boot, NULL);
> +
> +	return secure_boot == 1;
> +}
> +
> +/**
> + * create_time_based_payload() - create payload for time based authenticate variable
> + *
> + * @db:		pointer to the original signature database
> + * @new_db:	pointer to the authenticated variable payload
> + * @size:	pointer to payload size
> + * Return:	status code
> + */
> +static efi_status_t create_time_based_payload(void *db, void **new_db, efi_uintn_t *size)
> +{
> +	efi_status_t ret;
> +	struct efi_time time;
> +	efi_uintn_t total_size;
> +	struct efi_variable_authentication_2 *auth;
> +
> +	*new_db = NULL;
> +
> +	/*
> +	 * SetVariable() call with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
> +	 * attribute requires EFI_VARIABLE_AUTHENTICATED_2 descriptor, prepare it
> +	 * without certificate data in it.
> +	 */
> +	total_size = sizeof(struct efi_variable_authentication_2) + *size;
> +
> +	auth = calloc(1, total_size);
> +	if (!auth)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL));
> +	if (ret != EFI_SUCCESS) {
> +		free(auth);
> +		return EFI_OUT_OF_RESOURCES;
> +	}
> +	time.pad1 = 0;
> +	time.nanosecond = 0;
> +	time.timezone = 0;
> +	time.daylight = 0;
> +	time.pad2 = 0;
> +	memcpy(&auth->time_stamp, &time, sizeof(time));
> +	auth->auth_info.hdr.dwLength = sizeof(struct win_certificate_uefi_guid);
> +	auth->auth_info.hdr.wRevision = 0x0200;
> +	auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID;
> +	guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7);
> +	if (db)
> +		memcpy((u8 *)auth + sizeof(struct efi_variable_authentication_2), db, *size);
> +
> +	*new_db = auth;
> +	*size = total_size;
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * file_have_auth_header() - check file has EFI_VARIABLE_AUTHENTICATION_2 header
> + * @buf:	pointer to file
> + * @size:	file size
> + * Return:	true if file has auth header, false otherwise
> + */
> +static bool file_have_auth_header(void *buf, efi_uintn_t size)
> +{
> +	struct efi_variable_authentication_2 *auth = buf;
> +
> +	if (auth->auth_info.hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID)
> +		return false;
> +
> +	if (guidcmp(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7))
> +		return false;
> +
> +	return true;
> +}
> +
> +/**
> + * file_is_efi_signature_list() - check the file is efi signature list
> + * @buf:	pointer to file
> + * Return:	true if file is efi signature list, false otherwise
> + */
> +static bool file_is_efi_signature_list(void *buf)
> +{
> +	u32 i;
> +	struct efi_signature_list *sig_list = buf;
> +
> +	for (i = 0; i < ARRAY_SIZE(sigtype_to_str); i++) {
> +		if (!guidcmp(&sig_list->signature_type, &sigtype_to_str[i].sig_type))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + * eficonfig_process_enroll_key() - enroll key into signature database
> + *
> + * @data:	pointer to the data for each entry
> + * Return:	status code
> + */
> +static efi_status_t eficonfig_process_enroll_key(void *data)
> +{
> +	u32 attr;
> +	char *buf = NULL;
> +	efi_uintn_t size;
> +	efi_status_t ret;
> +	void *new_db = NULL;
> +	struct efi_file_handle *f;
> +	struct efi_file_handle *root;
> +	struct eficonfig_select_file_info file_info;
> +
> +	file_info.current_path = calloc(1, EFICONFIG_FILE_PATH_BUF_SIZE);
> +	if (!file_info.current_path)
> +		goto out;
> +
> +	ret = eficonfig_select_file_handler(&file_info);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	ret = efi_open_volume_int(file_info.current_volume, &root);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	ret = efi_file_open_int(root, &f, file_info.current_path, EFI_FILE_MODE_READ, 0);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	size = 0;
> +	ret = EFI_CALL(f->getinfo(f, &efi_file_info_guid, &size, NULL));
> +	if (ret != EFI_BUFFER_TOO_SMALL)
> +		goto out;
> +
> +	buf = calloc(1, size);
> +	if (!buf) {
> +		ret = EFI_OUT_OF_RESOURCES;
> +		goto out;
> +	}
> +	ret = EFI_CALL(f->getinfo(f, &efi_file_info_guid, &size, buf));
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	size = ((struct efi_file_info *)buf)->file_size;
> +	free(buf);
> +
> +	buf = calloc(1, size);
> +	if (!buf) {
> +		ret = EFI_OUT_OF_RESOURCES;

This leaves ret undefined and you get an error when compiling with clang:

+cmd/eficonfig_sbkey.c:208:6: error: variable 'ret' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
+        if (!file_info.current_path)
+            ^~~~~~~~~~~~~~~~~~~~~~~
+cmd/eficonfig_sbkey.c:294:9: note: uninitialized use occurs here
+        ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
+               ^~~
+cmd/eficonfig_sbkey.c:208:2: note: remove the 'if' if its condition is
always false
+        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
+cmd/eficonfig_sbkey.c:201:18: note: initialize the variable 'ret' to
silence this warning
+        efi_status_t ret;
+                        ^
+                         = 0

> +		goto out;
> +	}
> +
> +	ret = efi_file_read_int(f, &size, buf);
> +	if (ret != EFI_SUCCESS) {
> +		eficonfig_print_msg("ERROR! Failed to read file.");
> +		goto out;
> +	}
> +	if (size == 0) {
> +		eficonfig_print_msg("ERROR! File is empty.");
> +		goto out;
> +	}
> +
> +	/* We expect that file is EFI Signature Lists or signed EFI Signature Lists */
> +	if (!file_have_auth_header(buf, size)) {
> +		if (!file_is_efi_signature_list(buf)) {
> +			eficonfig_print_msg("ERROR! Invalid file format.");
> +			ret = EFI_INVALID_PARAMETER;
> +			goto out;
> +		}
> +
> +		ret = create_time_based_payload(buf, &new_db, &size);
> +		if (ret != EFI_SUCCESS) {
> +			eficonfig_print_msg("ERROR! Failed to create payload with timestamp.");
> +			goto out;
> +		}
> +
> +		free(buf);
> +		buf = new_db;
> +	}
> +
> +	attr = EFI_VARIABLE_NON_VOLATILE |
> +	       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +	       EFI_VARIABLE_RUNTIME_ACCESS |
> +	       EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> +
> +	/* PK can enroll only one certificate */
> +	if (u16_strcmp(data, u"PK")) {
> +		efi_uintn_t db_size = 0;
> +
> +		/* check the variable exists. If exists, add APPEND_WRITE attribute */
> +		ret = efi_get_variable_int(data, efi_auth_var_get_guid(data), NULL,
> +					   &db_size, NULL,  NULL);
> +		if (ret == EFI_BUFFER_TOO_SMALL)
> +			attr |= EFI_VARIABLE_APPEND_WRITE;
> +	}
> +
> +	ret = efi_set_variable_int((u16 *)data, efi_auth_var_get_guid((u16 *)data),
> +				   attr, size, buf, false);
> +	if (ret != EFI_SUCCESS) {
> +		eficonfig_print_msg("ERROR! Failed to update signature database");
> +		goto out;

This goto is superfluous.

Best regards

Heinrich

> +	}
> +
> +out:
> +	free(file_info.current_path);
> +	free(buf);
> +
> +	/* to stay the parent menu */
> +	ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
> +
> +	return ret;
> +}
> +
> +static struct eficonfig_item key_config_menu_items[] = {
> +	{"Enroll New Key", eficonfig_process_enroll_key},
> +	{"Quit", eficonfig_process_quit},
> +};
> +
> +/**
> + * eficonfig_process_set_secure_boot_key() - display the key configuration menu
> + *
> + * @data:	pointer to the data for each entry
> + * Return:	status code
> + */
> +static efi_status_t eficonfig_process_set_secure_boot_key(void *data)
> +{
> +	u32 i;
> +	efi_status_t ret;
> +	char header_str[32];
> +	struct efimenu *efi_menu;
> +
> +	for (i = 0; i < ARRAY_SIZE(key_config_menu_items); i++)
> +		key_config_menu_items[i].data = data;
> +
> +	snprintf(header_str, sizeof(header_str), "  ** Configure %ls **", (u16 *)data);
> +
> +	while (1) {
> +		efi_menu = eficonfig_create_fixed_menu(key_config_menu_items,
> +						       ARRAY_SIZE(key_config_menu_items));
> +
> +		ret = eficonfig_process_common(efi_menu, header_str);
> +		eficonfig_destroy(efi_menu);
> +
> +		if (ret == EFI_ABORTED)
> +			break;
> +	}
> +
> +	/* to stay the parent menu */
> +	ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
> +
> +	return ret;
> +}
> +

> +static const struct eficonfig_item secure_boot_menu_items[] = {
> +	{"PK", eficonfig_process_set_secure_boot_key, u"PK"},
> +	{"KEK", eficonfig_process_set_secure_boot_key, u"KEK"},
> +	{"db", eficonfig_process_set_secure_boot_key, u"db"},
> +	{"dbx", eficonfig_process_set_secure_boot_key, u"dbx"},
> +	{"Quit", eficonfig_process_quit} > +};
> +
> +/**
> + * eficonfig_process_secure_boot_config() - display the key list menu
> + *
> + * @data:	pointer to the data for each entry
> + * Return:	status code
> + */
> +efi_status_t eficonfig_process_secure_boot_config(void *data)
> +{
> +	efi_status_t ret;
> +	struct efimenu *efi_menu;
> +
> +	while (1) {
> +		char header_str[64];
> +
> +		snprintf(header_str, sizeof(header_str),
> +			 "  ** UEFI Secure Boot Key Configuration (SecureBoot : %s) **",
> +			 (is_secureboot_enabled() ? "ON" : "OFF"));
> +
> +		efi_menu = eficonfig_create_fixed_menu(secure_boot_menu_items,
> +						       ARRAY_SIZE(secure_boot_menu_items));
> +		if (!efi_menu) {
> +			ret = EFI_OUT_OF_RESOURCES;
> +			break;
> +		}
> +
> +		ret = eficonfig_process_common(efi_menu, header_str);
> +		eficonfig_destroy(efi_menu);
> +
> +		if (ret == EFI_ABORTED)
> +			break;
> +	}
> +
> +	/* to stay the parent menu */
> +	ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
> +
> +	return ret;
> +}
> diff --git a/include/efi_config.h b/include/efi_config.h
> index 86bc801211..6db8e123f0 100644
> --- a/include/efi_config.h
> +++ b/include/efi_config.h
> @@ -99,5 +99,10 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
>   					 char *title, eficonfig_entry_func func,
>   					 void *data);
>   efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu);
> +void *eficonfig_create_fixed_menu(const struct eficonfig_item *items, int count);
> +
> +#ifdef CONFIG_EFI_SECURE_BOOT
> +efi_status_t eficonfig_process_secure_boot_config(void *data);
> +#endif
>
>   #endif



More information about the U-Boot mailing list