[PATCH v3 02/11] capsule: authenticate: Add capsule public key in platform's dtb

Mon Jul 10 23:38:32 CEST 2023

Hi Sughosh, Heinrich,

On Mon, 10 Jul 2023 at 01:24, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> hi Heinrich,
>
> On Sun, 9 Jul 2023 at 19:22, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> >
> >
> > Am 9. Juli 2023 15:33:17 MESZ schrieb Sughosh Ganu <sughosh.ganu at linaro.org>:
> > >The EFI capsule authentication logic in u-boot expects the public key
> > >in the form of an EFI Signature List(ESL) to be provided as part of
> > >the platform's dtb. Currently, the embedding of the ESL file into the
> > >dtb needs to be done manually.
> > >
> > >Add a signature node in the u-boot dtsi file and include the public
> > >key through the capsule-key property. This file is per architecture,
> > >and is currently being added for sandbox and arm architectures. It
> >
> > The device-tree compiler can pick up files from /include/. If the dtsi file is not architecture specific, we should avoid code duplication.
>
> The idea here was to have the dtsi file get included automatically
> during build, and that happens only when the dtsi file, with a
> specific name, is in the same directory as the dts being built. The
> include directory is just a location where files would be searched for
> when building, but the corresponding dtsi file will have to be
> explicitly included in the dts being built. So this would mean every
> board dts file needing to include the capsule dtsi file, instead of
> automatic inclusion. I believe putting this dtsi file in the dts
> directory of an architecture which is using capsule updates is easier
> than including it in every board's dts.
>
> The initial implementation was generating the signature node in a dtbo
> on the fly, and including it into the corresponding dtb as it was
> built. This is the approach that was taken in v1 [1], but Simon did
> not like it, and Jonas suggested this approach [2].
>
> -sughosh
>
> [1] - https://lists.denx.de/pipermail/u-boot/2023-June/520121.html
> [2] - https://lists.denx.de/pipermail/u-boot/2023-June/521195.html
>
> >
> > We should treat all EFI architectures the same.

I'm not saying this is a great solution. I would prefer that people
add the public key to their board themselves, but we are still
struggling with the concept of the DT being used for such things...

For now this seems OK. We can always clean it up later.

> >
> > Best regards
> >
> > Heinrich
> >
> > >will have to be added for other architectures which need to enable
> > >capsule authentication support.
> > >
> > >The path to the ESL file is specified through the
> > >CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > >
> > >Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > >---
> > >Changes since V2:
> > >* Add the public key ESL file through the u-boot.dtsi.
> > >* Add the dtsi files for sandbox and arm architectures.
> > >* Add a check in the Makefile that the ESL file path is not empty.
> > >
> > > arch/arm/dts/u-boot.dtsi     | 17 +++++++++++++++++
> > > arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
> > > lib/efi_loader/Kconfig       | 11 +++++++++++
> > > lib/efi_loader/Makefile      |  7 +++++++
> > > 4 files changed, 52 insertions(+)
> > > create mode 100644 arch/arm/dts/u-boot.dtsi
> > > create mode 100644 arch/sandbox/dts/u-boot.dtsi
> > >
> > >diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > >new file mode 100644
> > >index 0000000000..60bd004937
> > >--- /dev/null
> > >+++ b/arch/arm/dts/u-boot.dtsi
> > >@@ -0,0 +1,17 @@
> > >+// SPDX-License-Identifier: GPL-2.0+
> > >+/*
> > >+ * Devicetree file with miscellaneous nodes that will be included
> > >+ * at build time into the DTB. Currently being used for including
> > >+ * capsule related information.
> > >+ *
> > >+ */
> > >+
> > >+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > >+/ {
> > >+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE

May as well put both #ifdefs together as there is no point in
declaring an empty node.

> > >+      signature {
> > >+              capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > >+      };
> > >+#endif
> > >+};
> > >+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > >diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > >new file mode 100644
> > >index 0000000000..60bd004937
> > >--- /dev/null
> > >+++ b/arch/sandbox/dts/u-boot.dtsi
> > >@@ -0,0 +1,17 @@
> > >+// SPDX-License-Identifier: GPL-2.0+
> > >+/*
> > >+ * Devicetree file with miscellaneous nodes that will be included
> > >+ * at build time into the DTB. Currently being used for including
> > >+ * capsule related information.
> > >+ *
> > >+ */
> > >+
> > >+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > >+/ {
> > >+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > >+      signature {
> > >+              capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > >+      };
> > >+#endif
> > >+};
> > >+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > >diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > >index c5835e6ef6..1326a1d109 100644
> > >--- a/lib/efi_loader/Kconfig
> > >+++ b/lib/efi_loader/Kconfig
> > >@@ -234,6 +234,17 @@ config EFI_CAPSULE_MAX
> > >         Select the max capsule index value used for capsule report
> > >         variables. This value is used to create CapsuleMax variable.
> > >
> > >+config EFI_CAPSULE_ESL_FILE
> > >+      string "Path to the EFI Signature List File"
> > >+      default ""
> > >+      depends on EFI_CAPSULE_AUTHENTICATE
> > >+      help
> > >+        Provides the absolute path to the EFI Signature List
> > >+        file which will be embedded in the platform's device
> > >+        tree and used for capsule authentication at the time
> > >+        of capsule update.
> > >+
> > >+

Drop extra blank line.

> > > 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 13a35eae6c..9fb04720d9 100644
> > >--- a/lib/efi_loader/Makefile
> > >+++ b/lib/efi_loader/Makefile
> > >@@ -86,3 +86,10 @@ 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)
> > >+EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE))
> > >+ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> > >+$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE)
> > >+endif
> > >+endif

Regards,
Simon