[PATCH v3 3/6] eficonfig: add UEFI Secure Boot Key enrollment interface
Heinrich Schuchardt
xypron.glpk at gmx.de
Sun Oct 23 11:45:24 CEST 2022
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 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
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
>
More information about the U-Boot
mailing list