diff mbox series

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

Message ID 20221014065705.5249-4-masahisa.kojima@linaro.org
State Superseded
Headers show
Series eficonfig: add UEFI Secure Boot key maintenance interface | expand

Commit Message

Masahisa Kojima Oct. 14, 2022, 6:56 a.m. UTC 
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@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

Comments

Heinrich Schuchardt Oct. 23, 2022, 8:17 a.m. UTC | #1 
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@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
Heinrich Schuchardt Oct. 23, 2022, 9:45 a.m. UTC | #2 
On 10/23/22 10:17, Heinrich Schuchardt wrote:
> 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@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

Key enrollment does not make sense if the keys are not persisted across
boots. The new feature should depend on CONFIG_EFI_MM_COMM_TEE.

>> +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)

ret = EFI_OUT_OF_RESOURCES; is missing here.

Best regards

Heinrich

>> +        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
>
Masahisa Kojima Oct. 24, 2022, 12:52 a.m. UTC | #3 
On Sun, 23 Oct 2022 at 18:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/23/22 10:17, Heinrich Schuchardt wrote:
> > 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@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
>
> Key enrollment does not make sense if the keys are not persisted across
> boots. The new feature should depend on CONFIG_EFI_MM_COMM_TEE.

Yes, persistent storage is required.
The problem is that not many platforms support CONFIG_EFI_MM_COMM_TEE and
this secure boot key menu feature can not be tested on sandbox.

>
> >> +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)
>
> ret = EFI_OUT_OF_RESOURCES; is missing here.

Yes, this will fix the compilation error in clang.

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> >> +        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
> >
>
diff mbox series

Patch

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;
+		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;
+	}
+
+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