Re: [PATCH 1/7] capsule: authenticate: Embed capsule public key in platform's dtb

Sughosh Ganu
2023-06-15T16:11:55Z

hi Simon,

On Thu, 15 Jun 2023 at 14:44, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Tue, 13 Jun 2023 at 11:41, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > 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 script for embedding the ESL used for capsule authentication in
> > the platform's dtb, and call this as part of building the dtb(s). This
> > brings the embedding of the ESL in the dtb into the u-boot build flow.
> >
> > The path to the ESL file is specified through the
> > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > lib/efi_loader/Kconfig | 11 +++++++++++
> > scripts/Makefile.lib | 8 ++++++++
> > scripts/embed_capsule_key.sh | 25 +++++++++++++++++++++++++
> > 3 files changed, 44 insertions(+)
> > create mode 100755 scripts/embed_capsule_key.sh
> >
> > 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.
> > +
> > +
> > config EFI_DEVICE_PATH_TO_TEXT
> > bool "Device path to text protocol"
> > default y
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 7b27224b5d..a4083d0a26 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -192,6 +192,8 @@ dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc \
> > -D__ASSEMBLY__ \
> > -undef -D__DTS__
> >
> > +export dtc_cpp_flags
> > +
> > # Finds the multi-part object the current object will be linked into
> > modname-multi = $(sort $(foreach m,$(multi-used),\
> > $(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))
> > @@ -315,6 +317,9 @@ ifeq ($(CONFIG_OF_LIBFDT_OVERLAY),y)
> > DTC_FLAGS += -@
> > endif
> >
> > +quiet_cmd_embedcapsulekey = EMBEDCAPSULEKEY $@
> > +cmd_embedcapsulekey = $(srctree)/scripts/embed_capsule_key.sh $@
> > +
> > quiet_cmd_dtc = DTC $@
> > # Modified for U-Boot
> > # Bring in any U-Boot-specific include at the end of the file
> > @@ -333,6 +338,9 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> >
> > $(obj)/%.dtb: $(src)/%.dts FORCE
> > $(call if_changed_dep,dtc)
> > +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> > + $(call cmd,embedcapsulekey,$@)
> > +endif
> >
> > pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp)
> > dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
> > diff --git a/scripts/embed_capsule_key.sh b/scripts/embed_capsule_key.sh
> > new file mode 100755
> > index 0000000000..1c2e45f758
> > --- /dev/null
> > +++ b/scripts/embed_capsule_key.sh
> > @@ -0,0 +1,25 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright (C) 2023, Linaro Limited
> > +#
> > +
> > +gen_capsule_signature_file() {
> > +cat >> $1 << EOF
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +&{/} {
> > + signature {
> > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > + };
> > +};
> > +EOF
> > +}
> > +
> > +gen_capsule_signature_file signature.$$.dts > /dev/null 2>&1
> > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o signature.$$.tmp signature.$$.dts > /dev/null 2>&1
> > +dtc -@ -O dtb -o signature.$$.dtbo signature.$$.tmp > /dev/null 2>&1
> > +fdtoverlay -i $1 -o temp.$$.dtb -v signature.$$.dtbo > /dev/null 2>&1
> > +mv temp.$$.dtb $1 > /dev/null 2>&1
> > +rm -f signature.$$.* > /dev/null 2>&1
> > --
> > 2.34.1
> >
>
> Can you please add this to binman instead?

I had looked at using binman for this work earlier because I very much
expected this comment from you :). Having said that, I am very much
open to using binman instead if it turns out to be the better way of
achieving this. What this patch does is that, with capsule
authentication enabled, it embeds the public key esl file into the
dtb's as they get built. As per my understanding, binman gets called
at the end of the u-boot build, once the constituent images( e..g
u-boot.bin = u-boot-no-dtb.bin + dtb) have been generated. So, if we
call binman _after_ the requisite image(s) have been generated, we
would need to 1) identify the dtb's in which the esl needs to be
embedded, and then 2) generate the final image all over again. Don't
you think this is non optimal? Or is there a way of generating the
constituent images(including the dtb's) through binman instead?

My understanding of binman is that it is a tool of packaging
constituent images together. But the constituent images are still
being built through make targets.

-sughosh