Re: [PATCH v6 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface

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@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