[PATCH v6 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface
Ilias Apalodimas
ilias.apalodimas at linaro.org
Fri Nov 4 22:46:37 CET 2022
Hi Kojima-san
On Wed, Oct 26, 2022 at 07:43:44PM +0900, 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 v6:
> - use efi_secure_boot_enabled()
> - replace with WIN_CERT_REVISION_2_0 from pe.h
> - call efi_build_signature_store() to check the valid EFI Signature List
> - update comment
>
> Changes in v4:
> - add CONFIG_EFI_MM_COMM_TEE dependency
> - fix error handling
>
> Changes in v3:
> - fix error handling
>
> Changes in v2:
> - allow to enroll .esl file
> - fix typos
> - add function comments
>
> cmd/Makefile | 5 +
> cmd/eficonfig.c | 3 +
> cmd/eficonfig_sbkey.c | 333 ++++++++++++++++++++++++++++++++++++++++++
> include/efi_config.h | 5 +
> 4 files changed, 346 insertions(+)
> create mode 100644 cmd/eficonfig_sbkey.c
>
> diff --git a/cmd/Makefile b/cmd/Makefile
> index c95e09d058..e43ef22e98 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -66,6 +66,11 @@ 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
> +ifdef CONFIG_EFI_MM_COMM_TEE
> +obj-$(CONFIG_EFI_SECURE_BOOT) += eficonfig_sbkey.o
> +endif
> +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 c765b795d0..0b643a046c 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -2447,6 +2447,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) && CONFIG_IS_ENABLED(EFI_MM_COMM_TEE))
> + {"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..e4a3573f1b
> --- /dev/null
> +++ b/cmd/eficonfig_sbkey.c
> @@ -0,0 +1,333 @@
> +// 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}, */
> +};
> +
> +/**
> + * 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;
Can't we do struct efi_time time = {0}?
Unless you explicitly need daylight to be set to zero
> + memcpy(&auth->time_stamp, &time, sizeof(time));
> + auth->auth_info.hdr.dwLength = sizeof(struct win_certificate_uefi_guid);
> + auth->auth_info.hdr.wRevision = WIN_CERT_REVISION_2_0;
> + 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;
> +}
> +
> +/**
> + * 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;
> + 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);
You can replace most of this code with efi_file_size()
> +
> + 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;
> + }
> +
> + if (!file_have_auth_header(buf, size)) {
Can you explain why we need this? I would expect the user to prepare an
.esl file with ./tools/efivar.py
> + struct efi_signature_store *sigstore;
> + char *tmp_buf;
> +
> + /* Check if the file is valid EFI Signature List(s) */
> + tmp_buf = calloc(1, size);
> + if (!tmp_buf) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> + memcpy(tmp_buf, buf, size);
> + /* tmp_buf is freed in efi_build_signature_store() */
> + sigstore = efi_build_signature_store(tmp_buf, size);
> + if (!sigstore) {
> + eficonfig_print_msg("ERROR! Invalid file format.");
> + ret = EFI_INVALID_PARAMETER;
> + goto out;
> + }
> + efi_sigstore_free(sigstore);
> +
> + 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;
> +
[...]
Thanks
/Ilias
More information about the U-Boot
mailing list