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

[Sughosh Ganu]

hi Simon,

On Mon, 19 Jun 2023 at 18:07, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Thu, 15 Jun 2023 at 17:11, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Thu, 15 Jun 2023 at 14:44, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 13 Jun 2023 at 11:41, Sughosh Ganu <sughosh.ganu at 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 at 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?
>
> The best way to do that IMO is to generate a second file, .e.g.
> u-boot-capsule.bin

That would break the scripts for platforms which might be using
u-boot.bin as the image to boot from. I know that the ST platform
which does enable capsule updates uses the u-boot-nodtb.bin as the
BL33 image and the u-boot.dtb as BL33_CFG. Hence my question, if we
have to use binman, is there a way to 1) modify the u-boot.dtb and
then 2) regenerate u-boot.bin image.

I know this is software, and everything can be done in a hacky way.
But I was exploring using the u-boot node as a section entry, so that
the u-boot.dtb can be modified and then binman would
repackage/regenerate the u-boot.bin. But this is not working.

>
> I don't think it is a good idea to add other junk to u-boot.bin. It
> should just be U-Boot + dtb.

No junk is being added to u-boot.bin. Just that, as the platform
builds dtb's, the ESL file gets embedded into them as a property under
the signature node. There is no additional image being added to the
the u-boot.bin.

-sughosh

>
> >
> > 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.
>
> In binman terminology, it collects a set of 'binaries' to produce
> firmware 'images'.
>
> Regards,
> Simon