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=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 57BF0C636CA for ; Sat, 17 Jul 2021 11:35:45 +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 E62A66128D for ; Sat, 17 Jul 2021 11:35:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E62A66128D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 76FA080C8F; Sat, 17 Jul 2021 13:35:41 +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="cfCrezBg"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C888080796; Sat, 17 Jul 2021 13:35:39 +0200 (CEST) Received: from mail-ej1-x629.google.com (mail-ej1-x629.google.com [IPv6:2a00:1450:4864:20::629]) (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 8AD7B80796 for ; Sat, 17 Jul 2021 13:35:36 +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-ej1-x629.google.com with SMTP id dt7so19370373ejc.12 for ; Sat, 17 Jul 2021 04:35:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=NjgvsJDkXB/hgNcu/23506EUO51L1yuBnfhlngAlwnA=; b=cfCrezBg+JC/00pjon4kfMiwvitJPoAfiuHbkpcl6MuwW++6hOGKvWuDmr8IFUx28S WG4Qz+9wXGAzfuvwriAi+SB7jh/Sr5QYY4HknUd7qh4+EcmFi72FGQI/+hwakkJBgX1d /JLf7Wx/jaVI46ZbL5zoGagExSsMXnY3fy4+Zv3REeAoxLGivZBtuGAKMThe3gKHxujS UQZfmA/5t/IFOKxH9hgbdqtG0DxIdGVb6IaBkjGGyNe3bK6kK5tvo3BtAhRuI3tJkeZY 0JrKXCS/xHTUJ/27l39uZ2M9Ni1NjIad07JK6uEzjiK8+U5kmn3H8SApXKNHU0FQSRmI pAXA== 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:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=NjgvsJDkXB/hgNcu/23506EUO51L1yuBnfhlngAlwnA=; b=nZJYdZdNAnbj9JDl6yFN4rn3t4EyisRGBMuvHk2AMzsHn5/cktCjP0t34nWfm+eRaz xxTK5Hp3XRVv7hxQHQiSa/UmRgTbVvmKLQzdGo58PVdtB2yzxtA4vGBIlNYT0oGxRg7x tfMDFJYbulT+VdngNCM6QqYDojQovsfm5Fhz+EcgHSZN9eetRAYdzoHB7VXW3lefEywZ 9QmOKTKI2OqE8qljBVHhQ3N9+SDiLIumgM4FBlBctmUiae3UmkNBFhI/ajMcmiKWqnfg M80IZVSIIFxb6JvoNz9euN3OEtjhv7AZbBh4cCkcy0wKMY4Oh9eE0pm+jRVCq06wi2nB 3GBg== X-Gm-Message-State: AOAM531+isXBPQz7Kh+KABiAkMkUIMCQRXz/9uwRAh7lohdXrsY1Lff7 Zj8mKWCaHTA9hh58AXRUgaRMag== X-Google-Smtp-Source: ABdhPJz5vRkSE0x1FIB1lDwtyBTh2RBAj/HWDfuPYsdcGDmOdi9u41zLllDnYeuVYs1MiTPbR9Du4Q== X-Received: by 2002:a17:906:a185:: with SMTP id s5mr16938336ejy.96.1626521736030; Sat, 17 Jul 2021 04:35:36 -0700 (PDT) Received: from apalos.home (ppp-94-66-220-20.home.otenet.gr. [94.66.220.20]) by smtp.gmail.com with ESMTPSA id eb9sm3845332ejc.32.2021.07.17.04.35.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Jul 2021 04:35:35 -0700 (PDT) Date: Sat, 17 Jul 2021 14:35:33 +0300 From: Ilias Apalodimas To: Takahiro Akashi , Masami Hiramatsu , Heinrich Schuchardt , Alexander Graf , Sughosh Ganu , Simon Glass , U-Boot Mailing List Subject: Re: [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata Message-ID: References: <20210715170030.97758-1-ilias.apalodimas@linaro.org> <20210716133903.GB68948@laputa> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210716133903.GB68948@laputa> 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 > > > [...] > > > +config EFI_CAPSULE_KEY_PATH > > > + string "Path to .esl file for capsule authentication" > > > + depends on EFI_CAPSULE_AUTHENTICATE > > > + help > > > + Provide the .esl file used for capsule authentication > > We might be friendly if we add what "esl" means here. > More importantly, we are able to contain more than one signatures > if we want. Sure, I'll replace it on the help message > > > > + > > > 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 fd344cea29b0..9b369430e258 100644 > > > --- a/lib/efi_loader/Makefile > > > +++ b/lib/efi_loader/Makefile > > > @@ -20,11 +20,19 @@ 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 > > We should give users another choice here to allow them to add their > own solution for key storage. > Or only enable this line if "CONFIG_EFI_CAPSULE_KEY_PATH" != null? > > > > 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 b878e71438b8..50e93cad4ee5 100644 > > > --- a/lib/efi_loader/efi_capsule.c > > > +++ b/lib/efi_loader/efi_capsule.c > > > @@ -16,6 +16,7 @@ > > > #include > > > #include > > > > > > +#include > > > #include > > > #include > > > #include > > > @@ -222,12 +223,23 @@ skip: > > > const efi_guid_t efi_guid_capsule_root_cert_guid = > > > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > > > > > +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) > > static? yea > > > > +{ > > > + const void *blob = __efi_capsule_sig_begin; > > > + const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin; > > It seems that the length can be calculated at compile time. > Yea but you still need the __efi_capsule_sig_begin. What's the proposal here? Replace __efi_capsule_sig_end with the size on the .S file? > > > + *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 *fdt_pkey, *pkey; > > > + void *stored_pkey, *pkey; > > > efi_uintn_t pkey_len; > > > uint64_t monotonic_count; > > > struct efi_signature_store *truststore; > > > @@ -286,7 +298,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s > > > goto out; > > > } > > > > > > - ret = efi_get_public_key_data(&fdt_pkey, &pkey_len); > > > + ret = efi_get_public_key_data(&stored_pkey, &pkey_len); > > > if (ret < 0) > > > goto out; > > > > > > @@ -294,7 +306,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s > > > if (!pkey) > > > goto out; > > > > > > - memcpy(pkey, fdt_pkey, pkey_len); > > > + memcpy(pkey, stored_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 > > > new file mode 100644 > > > index 000000000000..f7047a42e39d > > > --- /dev/null > > > +++ b/lib/efi_loader/efi_capsule_key.S > > > @@ -0,0 +1,8 @@ > > Should we have "#include " here? Hmm maybe. Compiling didn't cause any problems, but it seems we can add that include > Otherwise it looks good. > > -Takahiro Akashi Thanks /Ilias > > > > +.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.rc0 > > > > > > > > > -- > > Masami Hiramatsu