* [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata
@ 2025-04-01 11:27 Ilias Apalodimas
2025-04-01 11:27 ` [PATCH 2/2] doc: Update authenticated capsules documentation Ilias Apalodimas
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Ilias Apalodimas @ 2025-04-01 11:27 UTC (permalink / raw)
To: xypron.glpk
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Marek Vasut,
Peter Robinson, Jonathan Humphreys, Jerome Forissier,
Caleb Connolly, Richard Henderson, Adriano Cordova, Michal Simek,
Sughosh Ganu, Rasmus Villemoes, Prasad Kummari, Quentin Schulz,
Sam Edwards, u-boot
commit ddf67daac39d ("efi_capsule: Move signature from DTB to .rodata")
was reverted in
commit 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB to .rodata"")
because that's what U-Boot was usually doing -- using the DT to store
configuration and data. Some of the discussions can be found here [0].
(Ab)using the device tree to store random data isn't ideal though.
On top of that with new features introduced over the years, keeping
the certificates in the DT has proven to be problematic.
One of the reasons is that platforms might send U-Boot a DTB
from the previous stage loader using a transfer list which won't contain
the signatures since other loaders are not aware of internal
U-Boot ABIs. On top of that QEMU creates the DTB on the fly, so adding
the capsule certificate there does not work and requires users to dump
it and re-create it injecting the public keys.
Now that we have proper memory permissions for arm64, move the certificate
to .rodata and read it from there.
[0] https://lore.kernel.org/u-boot/CAPnjgZ2uM=n8Qo-a=DUkx5VW5Bzp5Xy8=Wgmrw8ESqUBK00YJQ@mail.gmail.com/
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Makefile | 2 +-
include/asm-generic/sections.h | 2 ++
lib/efi_loader/Makefile | 18 +++++++++++++++
lib/efi_loader/capsule_esl.dtsi.in | 11 ---------
lib/efi_loader/efi_capsule.c | 37 ++++++++----------------------
lib/efi_loader/efi_capsule_key.S | 17 ++++++++++++++
scripts/Makefile.lib | 27 ----------------------
7 files changed, 47 insertions(+), 67 deletions(-)
delete mode 100644 lib/efi_loader/capsule_esl.dtsi.in
create mode 100644 lib/efi_loader/efi_capsule_key.S
diff --git a/Makefile b/Makefile
index 620996862e6d..7a77948006a1 100644
--- a/Makefile
+++ b/Makefile
@@ -2229,7 +2229,7 @@ CLEAN_FILES += include/autoconf.mk* include/bmp_logo.h include/bmp_logo_data.h \
itb.fit.fit itb.fit.itb itb.map spl.map mkimage-out.rom.mkimage \
mkimage.rom.mkimage mkimage-in-simple-bin* rom.map simple-bin* \
idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb \
- Test* capsule*.*.efi-capsule capsule*.map
+ Test* capsule*.*.efi-capsule capsule*.map capsule_esl_file
# Directories & files removed with 'make mrproper'
MRPROPER_DIRS += include/config include/generated spl tpl vpl \
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 024b1adde270..d59787948fd1 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -28,6 +28,8 @@ 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/Makefile b/lib/efi_loader/Makefile
index 2a0b4172bd7f..dc2912148951 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -29,6 +29,7 @@ obj-y += efi_boottime.o
obj-y += efi_helper.o
obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
+obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
obj-y += efi_console.o
obj-y += efi_device_path.o
obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
@@ -73,6 +74,23 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
$(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
+ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
+capsule_crt_path=($(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE)))
+capsule_crt_full=$(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
+quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
+cmd_capsule_esl_gen = cert-to-efi-sig-list $(capsule_crt_full) $@
+$(srctree)/capsule_esl_file: FORCE
+ @if [ ! -e "$(capsule_crt_full)" ]; then \
+ echo "ERROR: path $(capsule_crt_full) is invalid." >&2; \
+ echo "EFI CONFIG_EFI_CAPSULE_CRT_FILE must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled." >&2; \
+ exit 1; \
+ fi
+ $(call cmd,capsule_esl_gen)
+
+$(obj)/efi_capsule.o: $(srctree)/capsule_esl_file FORCE
+asflags-y += -DCAPSULE_ESL_PATH=\"$(srctree)/capsule_esl_file\"
+endif
+
# Set the C flags to add and remove for each app
$(foreach f,$(apps-y),\
$(eval CFLAGS_$(f).o := $(CFLAGS_EFI) -Os -ffreestanding)\
diff --git a/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in
deleted file mode 100644
index bc7db836faa8..000000000000
--- a/lib/efi_loader/capsule_esl.dtsi.in
+++ /dev/null
@@ -1,11 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Devicetree file with the public key EFI Signature List(ESL)
- * node. This file is used to generate the dtsi file to be
- * included into the DTB.
- */
-/ {
- signature {
- capsule-key = /incbin/("ESL_BIN_FILE");
- };
-};
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index f8a4a7c6ef46..1aa52ac7bb69 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -22,6 +22,7 @@
#include <asm/global_data.h>
#include <u-boot/uuid.h>
+#include <asm/sections.h>
#include <crypto/pkcs7.h>
#include <crypto/pkcs7_parser.h>
#include <linux/err.h>
@@ -284,33 +285,12 @@ out:
}
#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
-int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
+static int efi_get_public_key_data(const 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) {
- log_err("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) {
- log_err("Unable to get capsule-key value\n");
- *pkey = NULL;
- *pkey_len = 0;
-
- return -FDT_ERR_NOTFOUND;
- }
+ const void *blob = __efi_capsule_sig_begin;
+ const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
- *pkey = (void *)blob;
+ *pkey = blob;
*pkey_len = len;
return 0;
@@ -321,7 +301,8 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
{
u8 *buf;
int ret;
- void *fdt_pkey, *pkey;
+ void *pkey;
+ const void *stored_pkey;
efi_uintn_t pkey_len;
uint64_t monotonic_count;
struct efi_signature_store *truststore;
@@ -373,7 +354,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;
@@ -381,7 +362,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..80cefbe16ae0
--- /dev/null
+++ b/lib/efi_loader/efi_capsule_key.S
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * .esl cert for capsule authentication
+ *
+ * Copyright (c) 2021, Ilias Apalodimas <ilias.apalodimas@linaro.org>
+ */
+
+#include <config.h>
+
+.section .rodata.capsule_key.init,"a"
+.balign 16
+.global __efi_capsule_sig_begin
+__efi_capsule_sig_begin:
+.incbin CAPSULE_ESL_PATH
+__efi_capsule_sig_end:
+.global __efi_capsule_sig_end
+.balign 16
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 275c308154b1..83fd5ff6c31c 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -377,35 +377,8 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
; \
sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
-capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in
-capsule_crt_file=$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
-capsule_esl_dtsi=.capsule_esl.dtsi
-
-quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
-cmd_capsule_esl_gen = cert-to-efi-sig-list $< $@
-
-$(obj)/capsule_esl_file: $(capsule_crt_file) FORCE
-ifeq ($(CONFIG_EFI_CAPSULE_CRT_FILE),"")
- $(error "CONFIG_EFI_CAPSULE_CRT_FILE is empty, EFI capsule authentication \
- public key must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled")
-else
- $(call cmd,capsule_esl_gen)
-endif
-
-quiet_cmd_capsule_dtsi_gen = CAPSULE_DTSI_GEN $@
-cmd_capsule_dtsi_gen = \
- $(shell sed "s:ESL_BIN_FILE:$(abspath $<):" $(capsule_esl_input_file) > $@)
-
-$(obj)/$(capsule_esl_dtsi): $(obj)/capsule_esl_file FORCE
- $(call cmd,capsule_dtsi_gen)
-
dtsi_include_list_deps := $(addprefix $(u_boot_dtsi_loc),$(subst $(quote),,$(dtsi_include_list)))
-ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
-dtsi_include_list += $(capsule_esl_dtsi)
-dtsi_include_list_deps += $(obj)/$(capsule_esl_dtsi)
-endif
-
ifneq ($(CHECK_DTBS),)
DT_CHECKER ?= dt-validate
DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] doc: Update authenticated capsules documentation
2025-04-01 11:27 [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata Ilias Apalodimas
@ 2025-04-01 11:27 ` Ilias Apalodimas
2025-04-01 20:47 ` [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata Raymond Mao
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Ilias Apalodimas @ 2025-04-01 11:27 UTC (permalink / raw)
To: xypron.glpk
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Marek Vasut,
Peter Robinson, Jonathan Humphreys, Caleb Connolly,
Richard Henderson, Jerome Forissier, Adriano Cordova,
Michal Simek, Sughosh Ganu, Rasmus Villemoes, Sam Edwards,
Quentin Schulz, Prasad Kummari, u-boot
Now that we moved out the capsule signature from the DTB, remove the
relevant documentation.
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
doc/develop/uefi/uefi.rst | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index 48d6110b2ad1..3ca22b572a92 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -597,21 +597,6 @@ and used by the steps highlighted below.
[--fit | --raw | --guid <guid-string] \
<image_blob> <capsule_file_name>
-4. Insert the signature list into a device tree in the following format::
-
- {
- signature {
- capsule-key = [ <binary of signature list> ];
- }
- ...
- }
-
-You can perform step-4 through the Kconfig symbol
-CONFIG_EFI_CAPSULE_CRT_FILE. This symbol points to the signing key
-generated in step-2. As part of U-Boot build, the ESL certificate file will
-be generated from the signing key and automatically get embedded into the
-platform's dtb.
-
Anti-rollback Protection
************************
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata
2025-04-01 11:27 [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata Ilias Apalodimas
2025-04-01 11:27 ` [PATCH 2/2] doc: Update authenticated capsules documentation Ilias Apalodimas
@ 2025-04-01 20:47 ` Raymond Mao
2025-04-03 14:53 ` neil.armstrong
2025-04-04 21:31 ` Jon Humphreys
3 siblings, 0 replies; 6+ messages in thread
From: Raymond Mao @ 2025-04-01 20:47 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: xypron.glpk, Tom Rini, Simon Glass, Marek Vasut, Peter Robinson,
Jonathan Humphreys, Jerome Forissier, Caleb Connolly,
Richard Henderson, Adriano Cordova, Michal Simek, Sughosh Ganu,
Rasmus Villemoes, Prasad Kummari, Quentin Schulz, Sam Edwards,
u-boot
Hi Ilias,
On Tue, 1 Apr 2025 at 07:27, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> commit ddf67daac39d ("efi_capsule: Move signature from DTB to .rodata")
> was reverted in
> commit 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB to .rodata"")
> because that's what U-Boot was usually doing -- using the DT to store
> configuration and data. Some of the discussions can be found here [0].
>
> (Ab)using the device tree to store random data isn't ideal though.
> On top of that with new features introduced over the years, keeping
> the certificates in the DT has proven to be problematic.
> One of the reasons is that platforms might send U-Boot a DTB
> from the previous stage loader using a transfer list which won't contain
> the signatures since other loaders are not aware of internal
> U-Boot ABIs. On top of that QEMU creates the DTB on the fly, so adding
> the capsule certificate there does not work and requires users to dump
> it and re-create it injecting the public keys.
>
> Now that we have proper memory permissions for arm64, move the certificate
> to .rodata and read it from there.
>
> [0] https://lore.kernel.org/u-boot/CAPnjgZ2uM=n8Qo-a=DUkx5VW5Bzp5Xy8=Wgmrw8ESqUBK00YJQ@mail.gmail.com/
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> Makefile | 2 +-
> include/asm-generic/sections.h | 2 ++
> lib/efi_loader/Makefile | 18 +++++++++++++++
> lib/efi_loader/capsule_esl.dtsi.in | 11 ---------
> lib/efi_loader/efi_capsule.c | 37 ++++++++----------------------
> lib/efi_loader/efi_capsule_key.S | 17 ++++++++++++++
> scripts/Makefile.lib | 27 ----------------------
> 7 files changed, 47 insertions(+), 67 deletions(-)
> delete mode 100644 lib/efi_loader/capsule_esl.dtsi.in
> create mode 100644 lib/efi_loader/efi_capsule_key.S
>
> diff --git a/Makefile b/Makefile
> index 620996862e6d..7a77948006a1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2229,7 +2229,7 @@ CLEAN_FILES += include/autoconf.mk* include/bmp_logo.h include/bmp_logo_data.h \
> itb.fit.fit itb.fit.itb itb.map spl.map mkimage-out.rom.mkimage \
> mkimage.rom.mkimage mkimage-in-simple-bin* rom.map simple-bin* \
> idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb \
> - Test* capsule*.*.efi-capsule capsule*.map
> + Test* capsule*.*.efi-capsule capsule*.map capsule_esl_file
>
> # Directories & files removed with 'make mrproper'
> MRPROPER_DIRS += include/config include/generated spl tpl vpl \
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index 024b1adde270..d59787948fd1 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -28,6 +28,8 @@ 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/Makefile b/lib/efi_loader/Makefile
> index 2a0b4172bd7f..dc2912148951 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -29,6 +29,7 @@ obj-y += efi_boottime.o
> obj-y += efi_helper.o
> obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
> obj-y += efi_console.o
> obj-y += efi_device_path.o
> obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
> @@ -73,6 +74,23 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
> EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
>
> +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> +capsule_crt_path=($(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE)))
> +capsule_crt_full=$(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
> +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> +cmd_capsule_esl_gen = cert-to-efi-sig-list $(capsule_crt_full) $@
> +$(srctree)/capsule_esl_file: FORCE
> + @if [ ! -e "$(capsule_crt_full)" ]; then \
> + echo "ERROR: path $(capsule_crt_full) is invalid." >&2; \
> + echo "EFI CONFIG_EFI_CAPSULE_CRT_FILE must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled." >&2; \
> + exit 1; \
> + fi
> + $(call cmd,capsule_esl_gen)
> +
> +$(obj)/efi_capsule.o: $(srctree)/capsule_esl_file FORCE
> +asflags-y += -DCAPSULE_ESL_PATH=\"$(srctree)/capsule_esl_file\"
> +endif
> +
> # Set the C flags to add and remove for each app
> $(foreach f,$(apps-y),\
> $(eval CFLAGS_$(f).o := $(CFLAGS_EFI) -Os -ffreestanding)\
> diff --git a/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in
> deleted file mode 100644
> index bc7db836faa8..000000000000
> --- a/lib/efi_loader/capsule_esl.dtsi.in
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Devicetree file with the public key EFI Signature List(ESL)
> - * node. This file is used to generate the dtsi file to be
> - * included into the DTB.
> - */
> -/ {
> - signature {
> - capsule-key = /incbin/("ESL_BIN_FILE");
> - };
> -};
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index f8a4a7c6ef46..1aa52ac7bb69 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -22,6 +22,7 @@
> #include <asm/global_data.h>
> #include <u-boot/uuid.h>
>
> +#include <asm/sections.h>
> #include <crypto/pkcs7.h>
> #include <crypto/pkcs7_parser.h>
> #include <linux/err.h>
> @@ -284,33 +285,12 @@ out:
> }
>
> #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
> -int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> +static int efi_get_public_key_data(const 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) {
> - log_err("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) {
> - log_err("Unable to get capsule-key value\n");
> - *pkey = NULL;
> - *pkey_len = 0;
> -
> - return -FDT_ERR_NOTFOUND;
> - }
> + const void *blob = __efi_capsule_sig_begin;
> + const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
>
> - *pkey = (void *)blob;
> + *pkey = blob;
> *pkey_len = len;
>
> return 0;
> @@ -321,7 +301,8 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> {
> u8 *buf;
> int ret;
> - void *fdt_pkey, *pkey;
> + void *pkey;
> + const void *stored_pkey;
> efi_uintn_t pkey_len;
> uint64_t monotonic_count;
> struct efi_signature_store *truststore;
> @@ -373,7 +354,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;
>
> @@ -381,7 +362,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..80cefbe16ae0
> --- /dev/null
> +++ b/lib/efi_loader/efi_capsule_key.S
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * .esl cert for capsule authentication
> + *
> + * Copyright (c) 2021, Ilias Apalodimas <ilias.apalodimas@linaro.org>
> + */
> +
> +#include <config.h>
> +
> +.section .rodata.capsule_key.init,"a"
> +.balign 16
> +.global __efi_capsule_sig_begin
> +__efi_capsule_sig_begin:
> +.incbin CAPSULE_ESL_PATH
> +__efi_capsule_sig_end:
> +.global __efi_capsule_sig_end
> +.balign 16
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 275c308154b1..83fd5ff6c31c 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -377,35 +377,8 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> ; \
> sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>
> -capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in
> -capsule_crt_file=$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
> -capsule_esl_dtsi=.capsule_esl.dtsi
> -
> -quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> -cmd_capsule_esl_gen = cert-to-efi-sig-list $< $@
> -
> -$(obj)/capsule_esl_file: $(capsule_crt_file) FORCE
> -ifeq ($(CONFIG_EFI_CAPSULE_CRT_FILE),"")
> - $(error "CONFIG_EFI_CAPSULE_CRT_FILE is empty, EFI capsule authentication \
> - public key must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled")
> -else
> - $(call cmd,capsule_esl_gen)
> -endif
> -
> -quiet_cmd_capsule_dtsi_gen = CAPSULE_DTSI_GEN $@
> -cmd_capsule_dtsi_gen = \
> - $(shell sed "s:ESL_BIN_FILE:$(abspath $<):" $(capsule_esl_input_file) > $@)
> -
> -$(obj)/$(capsule_esl_dtsi): $(obj)/capsule_esl_file FORCE
> - $(call cmd,capsule_dtsi_gen)
> -
> dtsi_include_list_deps := $(addprefix $(u_boot_dtsi_loc),$(subst $(quote),,$(dtsi_include_list)))
>
> -ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> -dtsi_include_list += $(capsule_esl_dtsi)
> -dtsi_include_list_deps += $(obj)/$(capsule_esl_dtsi)
> -endif
> -
> ifneq ($(CHECK_DTBS),)
> DT_CHECKER ?= dt-validate
> DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
> --
> 2.49.0
>
Tested-by: Raymond Mao <raymond.mao@linaro.org>
Regards,
Raymond
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata
2025-04-01 11:27 [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata Ilias Apalodimas
2025-04-01 11:27 ` [PATCH 2/2] doc: Update authenticated capsules documentation Ilias Apalodimas
2025-04-01 20:47 ` [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata Raymond Mao
@ 2025-04-03 14:53 ` neil.armstrong
2025-04-03 16:13 ` Neil Armstrong
2025-04-04 21:31 ` Jon Humphreys
3 siblings, 1 reply; 6+ messages in thread
From: neil.armstrong @ 2025-04-03 14:53 UTC (permalink / raw)
To: Ilias Apalodimas, xypron.glpk
Cc: Tom Rini, Simon Glass, Marek Vasut, Peter Robinson,
Jonathan Humphreys, Jerome Forissier, Caleb Connolly,
Richard Henderson, Adriano Cordova, Michal Simek, Sughosh Ganu,
Rasmus Villemoes, Prasad Kummari, Quentin Schulz, Sam Edwards,
u-boot
On 01/04/2025 13:27, Ilias Apalodimas wrote:
> commit ddf67daac39d ("efi_capsule: Move signature from DTB to .rodata")
> was reverted in
> commit 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB to .rodata"")
> because that's what U-Boot was usually doing -- using the DT to store
> configuration and data. Some of the discussions can be found here [0].
>
> (Ab)using the device tree to store random data isn't ideal though.
> On top of that with new features introduced over the years, keeping
> the certificates in the DT has proven to be problematic.
> One of the reasons is that platforms might send U-Boot a DTB
> from the previous stage loader using a transfer list which won't contain
> the signatures since other loaders are not aware of internal
> U-Boot ABIs. On top of that QEMU creates the DTB on the fly, so adding
> the capsule certificate there does not work and requires users to dump
> it and re-create it injecting the public keys.
>
> Now that we have proper memory permissions for arm64, move the certificate
> to .rodata and read it from there.
>
> [0] https://lore.kernel.org/u-boot/CAPnjgZ2uM=n8Qo-a=DUkx5VW5Bzp5Xy8=Wgmrw8ESqUBK00YJQ@mail.gmail.com/
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> Makefile | 2 +-
> include/asm-generic/sections.h | 2 ++
> lib/efi_loader/Makefile | 18 +++++++++++++++
> lib/efi_loader/capsule_esl.dtsi.in | 11 ---------
> lib/efi_loader/efi_capsule.c | 37 ++++++++----------------------
> lib/efi_loader/efi_capsule_key.S | 17 ++++++++++++++
> scripts/Makefile.lib | 27 ----------------------
> 7 files changed, 47 insertions(+), 67 deletions(-)
> delete mode 100644 lib/efi_loader/capsule_esl.dtsi.in
> create mode 100644 lib/efi_loader/efi_capsule_key.S
>
> diff --git a/Makefile b/Makefile
> index 620996862e6d..7a77948006a1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2229,7 +2229,7 @@ CLEAN_FILES += include/autoconf.mk* include/bmp_logo.h include/bmp_logo_data.h \
> itb.fit.fit itb.fit.itb itb.map spl.map mkimage-out.rom.mkimage \
> mkimage.rom.mkimage mkimage-in-simple-bin* rom.map simple-bin* \
> idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb \
> - Test* capsule*.*.efi-capsule capsule*.map
> + Test* capsule*.*.efi-capsule capsule*.map capsule_esl_file
>
> # Directories & files removed with 'make mrproper'
> MRPROPER_DIRS += include/config include/generated spl tpl vpl \
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index 024b1adde270..d59787948fd1 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -28,6 +28,8 @@ 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/Makefile b/lib/efi_loader/Makefile
> index 2a0b4172bd7f..dc2912148951 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -29,6 +29,7 @@ obj-y += efi_boottime.o
> obj-y += efi_helper.o
> obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
> obj-y += efi_console.o
> obj-y += efi_device_path.o
> obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
> @@ -73,6 +74,23 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
> EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
>
> +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> +capsule_crt_path=($(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE)))
> +capsule_crt_full=$(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
> +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> +cmd_capsule_esl_gen = cert-to-efi-sig-list $(capsule_crt_full) $@
> +$(srctree)/capsule_esl_file: FORCE
> + @if [ ! -e "$(capsule_crt_full)" ]; then \
> + echo "ERROR: path $(capsule_crt_full) is invalid." >&2; \
> + echo "EFI CONFIG_EFI_CAPSULE_CRT_FILE must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled." >&2; \
> + exit 1; \
> + fi
> + $(call cmd,capsule_esl_gen)
> +
> +$(obj)/efi_capsule.o: $(srctree)/capsule_esl_file FORCE
> +asflags-y += -DCAPSULE_ESL_PATH=\"$(srctree)/capsule_esl_file\"
> +endif
> +
> # Set the C flags to add and remove for each app
> $(foreach f,$(apps-y),\
> $(eval CFLAGS_$(f).o := $(CFLAGS_EFI) -Os -ffreestanding)\
> diff --git a/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in
> deleted file mode 100644
> index bc7db836faa8..000000000000
> --- a/lib/efi_loader/capsule_esl.dtsi.in
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Devicetree file with the public key EFI Signature List(ESL)
> - * node. This file is used to generate the dtsi file to be
> - * included into the DTB.
> - */
> -/ {
> - signature {
> - capsule-key = /incbin/("ESL_BIN_FILE");
> - };
> -};
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index f8a4a7c6ef46..1aa52ac7bb69 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -22,6 +22,7 @@
> #include <asm/global_data.h>
> #include <u-boot/uuid.h>
>
> +#include <asm/sections.h>
> #include <crypto/pkcs7.h>
> #include <crypto/pkcs7_parser.h>
> #include <linux/err.h>
> @@ -284,33 +285,12 @@ out:
> }
>
> #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
> -int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> +static int efi_get_public_key_data(const 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) {
> - log_err("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) {
> - log_err("Unable to get capsule-key value\n");
> - *pkey = NULL;
> - *pkey_len = 0;
> -
> - return -FDT_ERR_NOTFOUND;
> - }
> + const void *blob = __efi_capsule_sig_begin;
> + const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
>
> - *pkey = (void *)blob;
> + *pkey = blob;
> *pkey_len = len;
>
> return 0;
> @@ -321,7 +301,8 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> {
> u8 *buf;
> int ret;
> - void *fdt_pkey, *pkey;
> + void *pkey;
> + const void *stored_pkey;
> efi_uintn_t pkey_len;
> uint64_t monotonic_count;
> struct efi_signature_store *truststore;
> @@ -373,7 +354,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;
>
> @@ -381,7 +362,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..80cefbe16ae0
> --- /dev/null
> +++ b/lib/efi_loader/efi_capsule_key.S
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * .esl cert for capsule authentication
> + *
> + * Copyright (c) 2021, Ilias Apalodimas <ilias.apalodimas@linaro.org>
> + */
> +
> +#include <config.h>
> +
> +.section .rodata.capsule_key.init,"a"
> +.balign 16
> +.global __efi_capsule_sig_begin
> +__efi_capsule_sig_begin:
> +.incbin CAPSULE_ESL_PATH
> +__efi_capsule_sig_end:
> +.global __efi_capsule_sig_end
> +.balign 16
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 275c308154b1..83fd5ff6c31c 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -377,35 +377,8 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> ; \
> sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>
> -capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in
> -capsule_crt_file=$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
> -capsule_esl_dtsi=.capsule_esl.dtsi
> -
> -quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> -cmd_capsule_esl_gen = cert-to-efi-sig-list $< $@
> -
> -$(obj)/capsule_esl_file: $(capsule_crt_file) FORCE
> -ifeq ($(CONFIG_EFI_CAPSULE_CRT_FILE),"")
> - $(error "CONFIG_EFI_CAPSULE_CRT_FILE is empty, EFI capsule authentication \
> - public key must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled")
> -else
> - $(call cmd,capsule_esl_gen)
> -endif
> -
> -quiet_cmd_capsule_dtsi_gen = CAPSULE_DTSI_GEN $@
> -cmd_capsule_dtsi_gen = \
> - $(shell sed "s:ESL_BIN_FILE:$(abspath $<):" $(capsule_esl_input_file) > $@)
> -
> -$(obj)/$(capsule_esl_dtsi): $(obj)/capsule_esl_file FORCE
> - $(call cmd,capsule_dtsi_gen)
> -
> dtsi_include_list_deps := $(addprefix $(u_boot_dtsi_loc),$(subst $(quote),,$(dtsi_include_list)))
>
> -ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> -dtsi_include_list += $(capsule_esl_dtsi)
> -dtsi_include_list_deps += $(obj)/$(capsule_esl_dtsi)
> -endif
> -
> ifneq ($(CHECK_DTBS),)
> DT_CHECKER ?= dt-validate
> DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on AML-A311D-CC
Thanks,
Neil
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata
2025-04-03 14:53 ` neil.armstrong
@ 2025-04-03 16:13 ` Neil Armstrong
0 siblings, 0 replies; 6+ messages in thread
From: Neil Armstrong @ 2025-04-03 16:13 UTC (permalink / raw)
To: Ilias Apalodimas, xypron.glpk
Cc: Tom Rini, Simon Glass, Marek Vasut, Peter Robinson,
Jonathan Humphreys, Jerome Forissier, Caleb Connolly,
Richard Henderson, Adriano Cordova, Michal Simek, Sughosh Ganu,
Rasmus Villemoes, Prasad Kummari, Quentin Schulz, Sam Edwards,
u-boot
On 03/04/2025 16:53, neil.armstrong@linaro.org wrote:
> On 01/04/2025 13:27, Ilias Apalodimas wrote:
>> commit ddf67daac39d ("efi_capsule: Move signature from DTB to .rodata")
>> was reverted in
>> commit 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB to .rodata"")
>> because that's what U-Boot was usually doing -- using the DT to store
>> configuration and data. Some of the discussions can be found here [0].
>>
>> (Ab)using the device tree to store random data isn't ideal though.
>> On top of that with new features introduced over the years, keeping
>> the certificates in the DT has proven to be problematic.
>> One of the reasons is that platforms might send U-Boot a DTB
>> from the previous stage loader using a transfer list which won't contain
>> the signatures since other loaders are not aware of internal
>> U-Boot ABIs. On top of that QEMU creates the DTB on the fly, so adding
>> the capsule certificate there does not work and requires users to dump
>> it and re-create it injecting the public keys.
>>
>> Now that we have proper memory permissions for arm64, move the certificate
>> to .rodata and read it from there.
>>
>> [0] https://lore.kernel.org/u-boot/CAPnjgZ2uM=n8Qo-a=DUkx5VW5Bzp5Xy8=Wgmrw8ESqUBK00YJQ@mail.gmail.com/
>>
>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> ---
>> Makefile | 2 +-
>> include/asm-generic/sections.h | 2 ++
>> lib/efi_loader/Makefile | 18 +++++++++++++++
>> lib/efi_loader/capsule_esl.dtsi.in | 11 ---------
>> lib/efi_loader/efi_capsule.c | 37 ++++++++----------------------
>> lib/efi_loader/efi_capsule_key.S | 17 ++++++++++++++
>> scripts/Makefile.lib | 27 ----------------------
>> 7 files changed, 47 insertions(+), 67 deletions(-)
>> delete mode 100644 lib/efi_loader/capsule_esl.dtsi.in
>> create mode 100644 lib/efi_loader/efi_capsule_key.S
>>
>> diff --git a/Makefile b/Makefile
>> index 620996862e6d..7a77948006a1 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2229,7 +2229,7 @@ CLEAN_FILES += include/autoconf.mk* include/bmp_logo.h include/bmp_logo_data.h \
>> itb.fit.fit itb.fit.itb itb.map spl.map mkimage-out.rom.mkimage \
>> mkimage.rom.mkimage mkimage-in-simple-bin* rom.map simple-bin* \
>> idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb \
>> - Test* capsule*.*.efi-capsule capsule*.map
>> + Test* capsule*.*.efi-capsule capsule*.map capsule_esl_file
>> # Directories & files removed with 'make mrproper'
>> MRPROPER_DIRS += include/config include/generated spl tpl vpl \
>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
>> index 024b1adde270..d59787948fd1 100644
>> --- a/include/asm-generic/sections.h
>> +++ b/include/asm-generic/sections.h
>> @@ -28,6 +28,8 @@ 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/Makefile b/lib/efi_loader/Makefile
>> index 2a0b4172bd7f..dc2912148951 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -29,6 +29,7 @@ obj-y += efi_boottime.o
>> obj-y += efi_helper.o
>> obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
>> obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
>> +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
>> obj-y += efi_console.o
>> obj-y += efi_device_path.o
>> obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
>> @@ -73,6 +74,23 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
>> EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
>> $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
>> +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
>> +capsule_crt_path=($(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE)))
>> +capsule_crt_full=$(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
>> +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
>> +cmd_capsule_esl_gen = cert-to-efi-sig-list $(capsule_crt_full) $@
>> +$(srctree)/capsule_esl_file: FORCE
>> + @if [ ! -e "$(capsule_crt_full)" ]; then \
>> + echo "ERROR: path $(capsule_crt_full) is invalid." >&2; \
>> + echo "EFI CONFIG_EFI_CAPSULE_CRT_FILE must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled." >&2; \
>> + exit 1; \
>> + fi
>> + $(call cmd,capsule_esl_gen)
>> +
>> +$(obj)/efi_capsule.o: $(srctree)/capsule_esl_file FORCE
>> +asflags-y += -DCAPSULE_ESL_PATH=\"$(srctree)/capsule_esl_file\"
>> +endif
>> +
>> # Set the C flags to add and remove for each app
>> $(foreach f,$(apps-y),\
>> $(eval CFLAGS_$(f).o := $(CFLAGS_EFI) -Os -ffreestanding)\
>> diff --git a/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in
>> deleted file mode 100644
>> index bc7db836faa8..000000000000
>> --- a/lib/efi_loader/capsule_esl.dtsi.in
>> +++ /dev/null
>> @@ -1,11 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0+
>> -/*
>> - * Devicetree file with the public key EFI Signature List(ESL)
>> - * node. This file is used to generate the dtsi file to be
>> - * included into the DTB.
>> - */
>> -/ {
>> - signature {
>> - capsule-key = /incbin/("ESL_BIN_FILE");
>> - };
>> -};
>> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
>> index f8a4a7c6ef46..1aa52ac7bb69 100644
>> --- a/lib/efi_loader/efi_capsule.c
>> +++ b/lib/efi_loader/efi_capsule.c
>> @@ -22,6 +22,7 @@
>> #include <asm/global_data.h>
>> #include <u-boot/uuid.h>
>> +#include <asm/sections.h>
>> #include <crypto/pkcs7.h>
>> #include <crypto/pkcs7_parser.h>
>> #include <linux/err.h>
>> @@ -284,33 +285,12 @@ out:
>> }
>> #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
>> -int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
>> +static int efi_get_public_key_data(const 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) {
>> - log_err("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) {
>> - log_err("Unable to get capsule-key value\n");
>> - *pkey = NULL;
>> - *pkey_len = 0;
>> -
>> - return -FDT_ERR_NOTFOUND;
>> - }
>> + const void *blob = __efi_capsule_sig_begin;
>> + const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
>> - *pkey = (void *)blob;
>> + *pkey = blob;
>> *pkey_len = len;
>> return 0;
>> @@ -321,7 +301,8 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
>> {
>> u8 *buf;
>> int ret;
>> - void *fdt_pkey, *pkey;
>> + void *pkey;
>> + const void *stored_pkey;
>> efi_uintn_t pkey_len;
>> uint64_t monotonic_count;
>> struct efi_signature_store *truststore;
>> @@ -373,7 +354,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;
>> @@ -381,7 +362,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..80cefbe16ae0
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_capsule_key.S
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * .esl cert for capsule authentication
>> + *
>> + * Copyright (c) 2021, Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> + */
>> +
>> +#include <config.h>
>> +
>> +.section .rodata.capsule_key.init,"a"
>> +.balign 16
>> +.global __efi_capsule_sig_begin
>> +__efi_capsule_sig_begin:
>> +.incbin CAPSULE_ESL_PATH
>> +__efi_capsule_sig_end:
>> +.global __efi_capsule_sig_end
>> +.balign 16
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 275c308154b1..83fd5ff6c31c 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -377,35 +377,8 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
>> ; \
>> sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>> -capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in
>> -capsule_crt_file=$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
>> -capsule_esl_dtsi=.capsule_esl.dtsi
>> -
>> -quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
>> -cmd_capsule_esl_gen = cert-to-efi-sig-list $< $@
>> -
>> -$(obj)/capsule_esl_file: $(capsule_crt_file) FORCE
>> -ifeq ($(CONFIG_EFI_CAPSULE_CRT_FILE),"")
>> - $(error "CONFIG_EFI_CAPSULE_CRT_FILE is empty, EFI capsule authentication \
>> - public key must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled")
>> -else
>> - $(call cmd,capsule_esl_gen)
>> -endif
>> -
>> -quiet_cmd_capsule_dtsi_gen = CAPSULE_DTSI_GEN $@
>> -cmd_capsule_dtsi_gen = \
>> - $(shell sed "s:ESL_BIN_FILE:$(abspath $<):" $(capsule_esl_input_file) > $@)
>> -
>> -$(obj)/$(capsule_esl_dtsi): $(obj)/capsule_esl_file FORCE
>> - $(call cmd,capsule_dtsi_gen)
>> -
>> dtsi_include_list_deps := $(addprefix $(u_boot_dtsi_loc),$(subst $(quote),,$(dtsi_include_list)))
>> -ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
>> -dtsi_include_list += $(capsule_esl_dtsi)
>> -dtsi_include_list_deps += $(obj)/$(capsule_esl_dtsi)
>> -endif
>> -
>> ifneq ($(CHECK_DTBS),)
>> DT_CHECKER ?= dt-validate
>> DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on AML-A311D-CC
>
> Thanks,
> Neil
And with:
https://lore.kernel.org/all/20250403-u-boot-fix-set-dfu-alt-info-interface-v1-1-1fdd12463186@linaro.org/
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on AML-S805X-CC
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata
2025-04-01 11:27 [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata Ilias Apalodimas
` (2 preceding siblings ...)
2025-04-03 14:53 ` neil.armstrong
@ 2025-04-04 21:31 ` Jon Humphreys
3 siblings, 0 replies; 6+ messages in thread
From: Jon Humphreys @ 2025-04-04 21:31 UTC (permalink / raw)
To: Ilias Apalodimas, xypron.glpk
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Marek Vasut,
Peter Robinson, Jerome Forissier, Caleb Connolly,
Richard Henderson, Adriano Cordova, Michal Simek, Sughosh Ganu,
Rasmus Villemoes, Prasad Kummari, Quentin Schulz, Sam Edwards,
u-boot
Ilias Apalodimas <ilias.apalodimas@linaro.org> writes:
> commit ddf67daac39d ("efi_capsule: Move signature from DTB to .rodata")
> was reverted in
> commit 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB to .rodata"")
> because that's what U-Boot was usually doing -- using the DT to store
> configuration and data. Some of the discussions can be found here [0].
>
> (Ab)using the device tree to store random data isn't ideal though.
> On top of that with new features introduced over the years, keeping
> the certificates in the DT has proven to be problematic.
> One of the reasons is that platforms might send U-Boot a DTB
> from the previous stage loader using a transfer list which won't contain
> the signatures since other loaders are not aware of internal
> U-Boot ABIs. On top of that QEMU creates the DTB on the fly, so adding
> the capsule certificate there does not work and requires users to dump
> it and re-create it injecting the public keys.
>
> Now that we have proper memory permissions for arm64, move the certificate
> to .rodata and read it from there.
>
> [0] https://lore.kernel.org/u-boot/CAPnjgZ2uM=n8Qo-a=DUkx5VW5Bzp5Xy8=Wgmrw8ESqUBK00YJQ@mail.gmail.com/
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> Makefile | 2 +-
> include/asm-generic/sections.h | 2 ++
> lib/efi_loader/Makefile | 18 +++++++++++++++
> lib/efi_loader/capsule_esl.dtsi.in | 11 ---------
> lib/efi_loader/efi_capsule.c | 37 ++++++++----------------------
> lib/efi_loader/efi_capsule_key.S | 17 ++++++++++++++
> scripts/Makefile.lib | 27 ----------------------
> 7 files changed, 47 insertions(+), 67 deletions(-)
> delete mode 100644 lib/efi_loader/capsule_esl.dtsi.in
> create mode 100644 lib/efi_loader/efi_capsule_key.S
>
> diff --git a/Makefile b/Makefile
> index 620996862e6d..7a77948006a1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2229,7 +2229,7 @@ CLEAN_FILES += include/autoconf.mk* include/bmp_logo.h include/bmp_logo_data.h \
> itb.fit.fit itb.fit.itb itb.map spl.map mkimage-out.rom.mkimage \
> mkimage.rom.mkimage mkimage-in-simple-bin* rom.map simple-bin* \
> idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb \
> - Test* capsule*.*.efi-capsule capsule*.map
> + Test* capsule*.*.efi-capsule capsule*.map capsule_esl_file
>
> # Directories & files removed with 'make mrproper'
> MRPROPER_DIRS += include/config include/generated spl tpl vpl \
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index 024b1adde270..d59787948fd1 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -28,6 +28,8 @@ 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/Makefile b/lib/efi_loader/Makefile
> index 2a0b4172bd7f..dc2912148951 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -29,6 +29,7 @@ obj-y += efi_boottime.o
> obj-y += efi_helper.o
> obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
> obj-y += efi_console.o
> obj-y += efi_device_path.o
> obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
> @@ -73,6 +74,23 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
> EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
>
> +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> +capsule_crt_path=($(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE)))
> +capsule_crt_full=$(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
> +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> +cmd_capsule_esl_gen = cert-to-efi-sig-list $(capsule_crt_full) $@
> +$(srctree)/capsule_esl_file: FORCE
> + @if [ ! -e "$(capsule_crt_full)" ]; then \
> + echo "ERROR: path $(capsule_crt_full) is invalid." >&2; \
> + echo "EFI CONFIG_EFI_CAPSULE_CRT_FILE must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled." >&2; \
> + exit 1; \
> + fi
> + $(call cmd,capsule_esl_gen)
> +
> +$(obj)/efi_capsule.o: $(srctree)/capsule_esl_file FORCE
> +asflags-y += -DCAPSULE_ESL_PATH=\"$(srctree)/capsule_esl_file\"
> +endif
> +
> # Set the C flags to add and remove for each app
> $(foreach f,$(apps-y),\
> $(eval CFLAGS_$(f).o := $(CFLAGS_EFI) -Os -ffreestanding)\
> diff --git a/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in
> deleted file mode 100644
> index bc7db836faa8..000000000000
> --- a/lib/efi_loader/capsule_esl.dtsi.in
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Devicetree file with the public key EFI Signature List(ESL)
> - * node. This file is used to generate the dtsi file to be
> - * included into the DTB.
> - */
> -/ {
> - signature {
> - capsule-key = /incbin/("ESL_BIN_FILE");
> - };
> -};
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index f8a4a7c6ef46..1aa52ac7bb69 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -22,6 +22,7 @@
> #include <asm/global_data.h>
> #include <u-boot/uuid.h>
>
> +#include <asm/sections.h>
> #include <crypto/pkcs7.h>
> #include <crypto/pkcs7_parser.h>
> #include <linux/err.h>
> @@ -284,33 +285,12 @@ out:
> }
>
> #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
> -int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> +static int efi_get_public_key_data(const 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) {
> - log_err("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) {
> - log_err("Unable to get capsule-key value\n");
> - *pkey = NULL;
> - *pkey_len = 0;
> -
> - return -FDT_ERR_NOTFOUND;
> - }
> + const void *blob = __efi_capsule_sig_begin;
> + const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
>
> - *pkey = (void *)blob;
> + *pkey = blob;
> *pkey_len = len;
>
> return 0;
> @@ -321,7 +301,8 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> {
> u8 *buf;
> int ret;
> - void *fdt_pkey, *pkey;
> + void *pkey;
> + const void *stored_pkey;
> efi_uintn_t pkey_len;
> uint64_t monotonic_count;
> struct efi_signature_store *truststore;
> @@ -373,7 +354,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;
>
> @@ -381,7 +362,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..80cefbe16ae0
> --- /dev/null
> +++ b/lib/efi_loader/efi_capsule_key.S
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * .esl cert for capsule authentication
> + *
> + * Copyright (c) 2021, Ilias Apalodimas <ilias.apalodimas@linaro.org>
> + */
> +
> +#include <config.h>
> +
> +.section .rodata.capsule_key.init,"a"
> +.balign 16
> +.global __efi_capsule_sig_begin
> +__efi_capsule_sig_begin:
> +.incbin CAPSULE_ESL_PATH
> +__efi_capsule_sig_end:
> +.global __efi_capsule_sig_end
> +.balign 16
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 275c308154b1..83fd5ff6c31c 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -377,35 +377,8 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> ; \
> sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>
> -capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in
> -capsule_crt_file=$(subst $(quote),,$(CONFIG_EFI_CAPSULE_CRT_FILE))
> -capsule_esl_dtsi=.capsule_esl.dtsi
> -
> -quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> -cmd_capsule_esl_gen = cert-to-efi-sig-list $< $@
> -
> -$(obj)/capsule_esl_file: $(capsule_crt_file) FORCE
> -ifeq ($(CONFIG_EFI_CAPSULE_CRT_FILE),"")
> - $(error "CONFIG_EFI_CAPSULE_CRT_FILE is empty, EFI capsule authentication \
> - public key must be specified when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled")
> -else
> - $(call cmd,capsule_esl_gen)
> -endif
> -
> -quiet_cmd_capsule_dtsi_gen = CAPSULE_DTSI_GEN $@
> -cmd_capsule_dtsi_gen = \
> - $(shell sed "s:ESL_BIN_FILE:$(abspath $<):" $(capsule_esl_input_file) > $@)
> -
> -$(obj)/$(capsule_esl_dtsi): $(obj)/capsule_esl_file FORCE
> - $(call cmd,capsule_dtsi_gen)
> -
> dtsi_include_list_deps := $(addprefix $(u_boot_dtsi_loc),$(subst $(quote),,$(dtsi_include_list)))
>
> -ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> -dtsi_include_list += $(capsule_esl_dtsi)
> -dtsi_include_list_deps += $(obj)/$(capsule_esl_dtsi)
> -endif
> -
> ifneq ($(CHECK_DTBS),)
> DT_CHECKER ?= dt-validate
> DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
> --
> 2.49.0
Tested-by: Jonathan Humphreys <j-humphreys@ti.com> # on TI sk-am62p-lp
Jon
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-04 21:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-01 11:27 [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata Ilias Apalodimas
2025-04-01 11:27 ` [PATCH 2/2] doc: Update authenticated capsules documentation Ilias Apalodimas
2025-04-01 20:47 ` [PATCH 1/2] efi_loader: Move public cert for capsules to .rodata Raymond Mao
2025-04-03 14:53 ` neil.armstrong
2025-04-03 16:13 ` Neil Armstrong
2025-04-04 21:31 ` Jon Humphreys
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox