From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E80ECC4332F for ; Fri, 4 Nov 2022 21:46:48 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2C220850F8; Fri, 4 Nov 2022 22:46:46 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="n5VYSE60"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 379FC84F61; Fri, 4 Nov 2022 22:46:45 +0100 (CET) Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 3B758850F8 for ; Fri, 4 Nov 2022 22:46:41 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-ej1-x62a.google.com with SMTP id n12so16533358eja.11 for ; Fri, 04 Nov 2022 14:46:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=cnFHnC/aww1Ud3EXFiDYJzGrteqQwaopBWSycWYKMTY=; b=n5VYSE609zFERVAC5KDEY/v0a5Rqnx34B5qMyuIfHAfr3Md2BKG1bVIasjmxl1MjK3 shZXTmpddylcF9dpFBeGelu/wzC4jnLespSaFtsk3pLxBe/K9snqnG9K4ngASFcxEtIZ ZakMg7wuOD0c7MCTxFika/PsQYODM3w9x8BRRwvq7+1ED+CF8Tm+PeeqU7luhCuvnd0I axX9qTOYXHyRbxOzt5ynQXGZ/m+inQPiZSoEzbPcVpgllCbOV5VapSUwAs68lKXgPHnH OsBLiUl6e0snYAb0nOjMGiU3uqUV1qm3LTHf7O7ejY0BXoPbLOdZHeoAjTAfWXXSlqzw 4NUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=cnFHnC/aww1Ud3EXFiDYJzGrteqQwaopBWSycWYKMTY=; b=PXi3KcSV+3e/ni/qgfbtjLrzmvMgQSH/YTkUYcuMgPCWFF7bzTpt1xEzq24pDpzs+7 D/5Z9zltKv8ChR9RO0B55L+quF2Teg4QJjQZWpAhZ07dQSNN3SDvasHhD2BqwhVNQaD6 xAX/kCLbGHFwnam6JcP8u20y7rIvBEAxf3lcrUJccyHTLScjAPEssRMtV4VhH3XeV50P 3m7Zgn1pk2/9TGKo6MuJAJ16YBEEVnVJZGPNx7c19UOVY8Bbi+jMDf623nxbvgn0K9Rh 1dBVh0usXqIxLlc2IiBwkmDeESpszTSSIkRG+I238HDoErbLKuEWR8WQzy0wFiGoiCmE KD/Q== X-Gm-Message-State: ACrzQf2/DQ7E43HxKiCVn7wbLIimrmZFpseyCmpcbeunsBg9qDq39si7 1lB/MDMFoqWFrhhhy5zazSMPpg== X-Google-Smtp-Source: AMsMyM5umjr0QYjPYyfwUkGkJHrtgpyBRYgf/om2uLwVpKyX3zFB2FjejPL+Ba2X6A/bDB9HVjiv+w== X-Received: by 2002:a17:906:899d:b0:7ad:cf09:96be with SMTP id gg29-20020a170906899d00b007adcf0996bemr29995535ejc.221.1667598400823; Fri, 04 Nov 2022 14:46:40 -0700 (PDT) Received: from hera (ppp046103046165.access.hol.gr. [46.103.46.165]) by smtp.gmail.com with ESMTPSA id w21-20020a170906131500b007addcbd402esm31518ejb.215.2022.11.04.14.46.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Nov 2022 14:46:40 -0700 (PDT) Date: Fri, 4 Nov 2022 23:46:37 +0200 From: Ilias Apalodimas To: Masahisa Kojima Cc: u-boot@lists.denx.de, Heinrich Schuchardt , Simon Glass , Takahiro Akashi , Etienne Carriere , Roger Knecht , Chris Morgan , Stefan Roese , Ovidiu Panait , Ashok Reddy Soma Subject: Re: [PATCH v6 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface Message-ID: References: <20221026104345.28714-1-masahisa.kojima@linaro.org> <20221026104345.28714-5-masahisa.kojima@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221026104345.28714-5-masahisa.kojima@linaro.org> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean 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 > --- > 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +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