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 X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1DD1C4338F for ; Mon, 2 Aug 2021 02:29:01 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id A1BAE6018A for ; Mon, 2 Aug 2021 02:29:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A1BAE6018A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3C2D08330F; Mon, 2 Aug 2021 04:28:58 +0200 (CEST) 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="Hqp32MIN"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id DF3488330F; Mon, 2 Aug 2021 04:28:56 +0200 (CEST) Received: from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com [IPv6:2a00:1450:4864:20::32d]) (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 3233A8330F for ; Mon, 2 Aug 2021 04:28:53 +0200 (CEST) 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-wm1-x32d.google.com with SMTP id f14-20020a05600c154eb02902519e4abe10so12766150wmg.4 for ; Sun, 01 Aug 2021 19:28:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=SdYr3eSBnSYVWPQAPPhR/OewW+RgAJmz2bP5QDDDVLk=; b=Hqp32MINXJYRoMBy5bZuOcU7z8++QPoqvWX/WPe1Bel4OoaKcKnAE1wnBXg+cWfQ8i vVbnUcR56/8ggnen4LaRghzxEMlaPhcL046Sby18uXid8saacYrUu//vftsfnPDEF5ew OmBFZVnXsWV426AuYQeRAElSS+SypfBO98zkLi4EqupJNfnBs4f0L+l4FUUqVeBKDJ5A FUNUeuO3Mdc2k6K6So/uu5bLEFYRkyouBhRQWeixA1TayGkPAYgleeMY9s3ywc+JVzxz 4RecJToL6aAlXcrCTP9Ehyd56J27ZvOdibdQ/YKfjm3BM7+SU+kH0rVHfJGkyQv4xbRK fLiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=SdYr3eSBnSYVWPQAPPhR/OewW+RgAJmz2bP5QDDDVLk=; b=QO/QaMzqMe+ytbd5fjU9ohEy/QShj5AIn2wYaHftfj2D6AarHUJTh7TwJEIkp7lUjG niCcEhK8v0DiKZE8/Ye0ytHgKURW/m8iwalU7kEw/uUeT3BKRWYE9+2PtuyxpaXkNEMe WPSkyChg2vpmDygTyTJoRgLZIJSYBszTq2CRF1SzPEZx8PLS+u2XAtMG7DprRmX2Refd 0bWPhBnjQIJ0PjWCDR2HEqq45FiBO/msfAozEs6WSbgQaE9RnBcWikLFWH69F/j6qtMz Gk9WeRmCNFKz6zZHK5Zwu+Xii6y3XRI8O4dj+VCD7tRDfP8xG08ZlM3f3lxSlcokVptr I50A== X-Gm-Message-State: AOAM531jpkarUVlGN23L0tOqigwfYa/QBB1wBbBHgE7pDve7tFQmiBh7 xis5uIswpqUBuvxUkbp9jMmaag== X-Google-Smtp-Source: ABdhPJzna4IFqR2/zfVwozlQsdq43qoQnIpsKlCbv7xiDuLomE1YVEgwpwMmXJl2UUzOuvRwajWbaQ== X-Received: by 2002:a1c:7208:: with SMTP id n8mr13739991wmc.89.1627871332715; Sun, 01 Aug 2021 19:28:52 -0700 (PDT) Received: from enceladus (ppp-94-66-251-219.home.otenet.gr. [94.66.251.219]) by smtp.gmail.com with ESMTPSA id r133sm9183163wma.18.2021.08.01.19.28.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Aug 2021 19:28:52 -0700 (PDT) Date: Mon, 2 Aug 2021 05:28:49 +0300 From: Ilias Apalodimas To: Simon Glass Cc: U-Boot Mailing List , Heinrich Schuchardt , KASHI Takahiro , Alexander Graf , Masami Hiramatsu , Sughosh Ganu Subject: Re: [PATCH] Revert "efi_capsule: Move signature from DTB to .rodata" Message-ID: References: <20210802014621.2280899-1-sjg@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210802014621.2280899-1-sjg@chromium.org> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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.2 at phobos.denx.de X-Virus-Status: Clean Hi Simon, On Sun, Aug 01, 2021 at 07:46:21PM -0600, Simon Glass wrote: > This was unfortunately applied despite much discussion about it being > the wrong way to implement this feature. No this was applied *before* the discussion, not despite. > > Revert it before too many other things are built on top of it. I don't really mind if this gets reverted but there's things that haven't been answered on that discussion [1] and my concern is what happens if CONFIG_OF_EMBED is not selected. Also you need to revert the entire series, not just one of the patches, as it changes the QEMU documentation for enabling authenticated capsule updates, as well as the mkeficapsule app. [1] https://lore.kernel.org/u-boot/YPna8Aiaoov6h50K@enceladus/ Regards /Ilias > > This reverts commit ddf67daac39de76d2697d587148f4c2cb768f492. > > Signed-off-by: Simon Glass > --- > > board/emulation/common/Makefile | 1 + > board/emulation/common/qemu_capsule.c | 43 +++++++++++++++++++++++++++ > include/asm-generic/sections.h | 2 -- > lib/efi_loader/Kconfig | 7 ----- > lib/efi_loader/Makefile | 8 ----- > lib/efi_loader/efi_capsule.c | 18 ++--------- > lib/efi_loader/efi_capsule_key.S | 17 ----------- > 7 files changed, 47 insertions(+), 49 deletions(-) > create mode 100644 board/emulation/common/qemu_capsule.c > delete mode 100644 lib/efi_loader/efi_capsule_key.S > > diff --git a/board/emulation/common/Makefile b/board/emulation/common/Makefile > index c5b452e7e34..7ed447a69dc 100644 > --- a/board/emulation/common/Makefile > +++ b/board/emulation/common/Makefile > @@ -2,3 +2,4 @@ > > obj-$(CONFIG_SYS_MTDPARTS_RUNTIME) += qemu_mtdparts.o > obj-$(CONFIG_SET_DFU_ALT_INFO) += qemu_dfu.o > +obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT) += qemu_capsule.o > diff --git a/board/emulation/common/qemu_capsule.c b/board/emulation/common/qemu_capsule.c > new file mode 100644 > index 00000000000..6b8a87022a4 > --- /dev/null > +++ b/board/emulation/common/qemu_capsule.c > @@ -0,0 +1,43 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2020 Linaro Limited > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +DECLARE_GLOBAL_DATA_PTR; > + > +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) > +{ > + const void *fdt_blob = gd->fdt_blob; > + const void *blob; > + const char *cnode_name = "capsule-key"; > + const char *snode_name = "signature"; > + int sig_node; > + int len; > + > + sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name); > + if (sig_node < 0) { > + EFI_PRINT("Unable to get signature node offset\n"); > + return -FDT_ERR_NOTFOUND; > + } > + > + blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len); > + > + if (!blob || len < 0) { > + EFI_PRINT("Unable to get capsule-key value\n"); > + *pkey = NULL; > + *pkey_len = 0; > + return -FDT_ERR_NOTFOUND; > + } > + > + *pkey = (void *)blob; > + *pkey_len = len; > + > + return 0; > +} > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > index ec992b0c2e3..267f1db73f2 100644 > --- a/include/asm-generic/sections.h > +++ b/include/asm-generic/sections.h > @@ -27,8 +27,6 @@ extern char __efi_helloworld_begin[]; > extern char __efi_helloworld_end[]; > extern char __efi_var_file_begin[]; > extern char __efi_var_file_end[]; > -extern char __efi_capsule_sig_begin[]; > -extern char __efi_capsule_sig_end[]; > > /* Private data used by of-platdata devices/uclasses */ > extern char __priv_data_start[], __priv_data_end[]; > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index dacc3b58810..7a469f22721 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -214,13 +214,6 @@ config EFI_CAPSULE_AUTHENTICATE > Select this option if you want to enable capsule > authentication > > -config EFI_CAPSULE_KEY_PATH > - string "Path to .esl cert for capsule authentication" > - depends on EFI_CAPSULE_AUTHENTICATE > - help > - Provide the EFI signature list (esl) certificate used for capsule > - authentication > - > config EFI_DEVICE_PATH_TO_TEXT > bool "Device path to text protocol" > default y > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile > index 9b369430e25..fd344cea29b 100644 > --- a/lib/efi_loader/Makefile > +++ b/lib/efi_loader/Makefile > @@ -20,19 +20,11 @@ always += helloworld.efi > targets += helloworld.o > endif > > -ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y) > -EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_KEY_PATH)) > -ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","") > -$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_KEY_PATH) > -endif > -endif > - > obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o > obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o > obj-y += efi_boottime.o > obj-y += efi_helper.o > obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o > -obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o > obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o > obj-y += efi_console.o > obj-y += efi_device_path.o > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 26990bc2df4..b75e4bcba1a 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -16,7 +16,6 @@ > #include > #include > > -#include > #include > #include > #include > @@ -253,23 +252,12 @@ out: > > #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > > -static int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) > -{ > - const void *blob = __efi_capsule_sig_begin; > - const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin; > - > - *pkey = (void *)blob; > - *pkey_len = len; > - > - return 0; > -} > - > efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size, > void **image, efi_uintn_t *image_size) > { > u8 *buf; > int ret; > - void *stored_pkey, *pkey; > + void *fdt_pkey, *pkey; > efi_uintn_t pkey_len; > uint64_t monotonic_count; > struct efi_signature_store *truststore; > @@ -322,7 +310,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s > goto out; > } > > - ret = efi_get_public_key_data(&stored_pkey, &pkey_len); > + ret = efi_get_public_key_data(&fdt_pkey, &pkey_len); > if (ret < 0) > goto out; > > @@ -330,7 +318,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s > if (!pkey) > goto out; > > - memcpy(pkey, stored_pkey, pkey_len); > + memcpy(pkey, fdt_pkey, pkey_len); > truststore = efi_build_signature_store(pkey, pkey_len); > if (!truststore) > goto out; > diff --git a/lib/efi_loader/efi_capsule_key.S b/lib/efi_loader/efi_capsule_key.S > deleted file mode 100644 > index 58f00b8e4bc..00000000000 > --- a/lib/efi_loader/efi_capsule_key.S > +++ /dev/null > @@ -1,17 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0+ */ > -/* > - * .esl cert for capsule authentication > - * > - * Copyright (c) 2021, Ilias Apalodimas > - */ > - > -#include > - > -.section .rodata.capsule_key.init,"a" > -.balign 16 > -.global __efi_capsule_sig_begin > -__efi_capsule_sig_begin: > -.incbin CONFIG_EFI_CAPSULE_KEY_PATH > -__efi_capsule_sig_end: > -.global __efi_capsule_sig_end > -.balign 16 > -- > 2.32.0.554.ge1b32706d8-goog >