| Message ID | 20230613103806.812065-2-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Integrate EFI capsule tasks into u-boot's build flow | expand |
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? Regards, Simon
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
Hi Sughosh, On Thu, 15 Jun 2023 at 17:11, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > 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? The best way to do that IMO is to generate a second file, .e.g. u-boot-capsule.bin I don't think it is a good idea to add other junk to u-boot.bin. It should just be U-Boot + dtb. > > 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
hi Simon, On Mon, 19 Jun 2023 at 18:07, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Thu, 15 Jun 2023 at 17:11, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > 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? > > 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
Hi Sughosh, On Wed, 21 Jun 2023 at 05:20, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Mon, 19 Jun 2023 at 18:07, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Thu, 15 Jun 2023 at 17:11, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > 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? > > > > 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. NO, please do not do that. You should create a new file, not modify u-boot.bin or u-boot.dtb. Please just don't mess around with this, it will lead to all sorts of confusion. I thought we already had this discussion a while back? > > > > > 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. This needs to be done in a separte file, as above. Regards, Simon
Hi Simon, [...] > > > > > > + > > > > > > +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 > > This make no sense to me whatsoever. Do we have an example in u-boot generating multiple dtb versions for other reasons/subsystems? > > 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. > > NO, please do not do that. You should create a new file, not modify > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > will lead to all sorts of confusion. > > I thought we already had this discussion a while back? No we haven't. In fact I am struggling to see the confusion part. It's fine for the u-boot dtb to include all the internal nodes DM needs, but suddenly having the capsule signature is problematic? In the past the .esl file was part of the U-Boot binary and things were working perfectly fine. In fact you could update/downgrade u-boot and the signatures naturally followed along instead of having to update u-boot *and* the dtb, which we have to do today. You could also build a capsule way easier without injecting/removing signatures to the dtb. You were the one that insisted on reverting that and instead adding it on the dtb. We explained most of the downsides back then, along with some security concerns. We also mentioned that the signature in the dtb makes little sense since it's difference *per class of boards* and it's not something we could include in static dtb files, but that lead nowhere... As Sughosh already said there are platforms that use the generated u-boot dtb and the raw binary to assemble a FIP image. So why exactly adding the capsule signature to the default dtb is wrong? Why should we add an *extra* .dtb with one additional node -- which is very much needed by the design you proposed. This will only lead to extra confusion. Thanks /Ilias > > > > > > > > > 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. > > This needs to be done in a separte file, as above. > > Regards, > Simon
Hi Ilias, On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > [...] > > > > > > > > + > > > > > > > +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 > > > > > This make no sense to me whatsoever. Do we have an example in u-boot > generating multiple dtb versions for other reasons/subsystems? > > > > 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. > > > > NO, please do not do that. You should create a new file, not modify > > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > > will lead to all sorts of confusion. > > > > I thought we already had this discussion a while back? > > No we haven't. In fact I am struggling to see the confusion part. It's > fine for the u-boot dtb to include all the internal nodes DM needs, but > suddenly having the capsule signature is problematic? > > In the past the .esl file was part of the U-Boot binary and things were > working perfectly fine. In fact you could update/downgrade u-boot and the > signatures naturally followed along instead of having to update u-boot > *and* the dtb, which we have to do today. You could also build a capsule > way easier without injecting/removing signatures to the dtb. > You were the one that insisted on reverting that and instead adding it on > the dtb. We explained most of the downsides back then, along with some > security concerns. We also mentioned that the signature in the dtb makes > little sense since it's difference *per class of boards* and it's not > something we could include in static dtb files, but that lead nowhere... > > As Sughosh already said there are platforms that use the generated u-boot > dtb and the raw binary to assemble a FIP image. So why exactly adding the > capsule signature to the default dtb is wrong? Why should we add an > *extra* .dtb with one additional node -- which is very much needed by the > design you proposed. This will only lead to extra confusion. 1. I thought a capsule update was going to be a separate file, e.g. u-boot-capture.bin ? 2. You can't put the signature into U-Boot. It needs to be in the capsule update so that U-Boot can check it. If you are talking about the public key, then yes that needs to be in U-Boot 3. Where does the public key come from? With the normal verified boot flow it is created (and added to the dtb) as a separate signing step after U-Boot is built. 4. Off topic, but re FIP, can someone just kill that off and use FIT? It is such a shame that a new format was invented... Regards, Simon
Hi Simon, On Mon, 26 Jun 2023 at 14:19, Simon Glass <sjg@chromium.org> wrote: > > Hi Ilias, > > On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Hi Simon, > > > > [...] > > > > > > > > > > + > > > > > > > > +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 > > > > > > > > This make no sense to me whatsoever. Do we have an example in u-boot > > generating multiple dtb versions for other reasons/subsystems? > > > > > > 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. > > > > > > NO, please do not do that. You should create a new file, not modify > > > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > > > will lead to all sorts of confusion. > > > > > > I thought we already had this discussion a while back? > > > > No we haven't. In fact I am struggling to see the confusion part. It's > > fine for the u-boot dtb to include all the internal nodes DM needs, but > > suddenly having the capsule signature is problematic? > > > > In the past the .esl file was part of the U-Boot binary and things were > > working perfectly fine. In fact you could update/downgrade u-boot and the > > signatures naturally followed along instead of having to update u-boot > > *and* the dtb, which we have to do today. You could also build a capsule > > way easier without injecting/removing signatures to the dtb. > > You were the one that insisted on reverting that and instead adding it on > > the dtb. We explained most of the downsides back then, along with some > > security concerns. We also mentioned that the signature in the dtb makes > > little sense since it's difference *per class of boards* and it's not > > something we could include in static dtb files, but that lead nowhere... > > > > As Sughosh already said there are platforms that use the generated u-boot > > dtb and the raw binary to assemble a FIP image. So why exactly adding the > > capsule signature to the default dtb is wrong? Why should we add an > > *extra* .dtb with one additional node -- which is very much needed by the > > design you proposed. This will only lead to extra confusion. > > 1. I thought a capsule update was going to be a separate file, e.g. > u-boot-capture.bin ? Yes the capsule itself is a different file and I dont think there's any disagreement on how to generate that. On the u-boot.bin you need to flash on the board though, you need to include the public key you authenticate the incoming capsule against. That's what Sughosh wants to inject. > > 2. You can't put the signature into U-Boot. It needs to be in the > capsule update so that U-Boot can check it. If you are talking about > the public key, then yes that needs to be in U-Boot See above, > > 3. Where does the public key come from? With the normal verified boot > flow it is created (and added to the dtb) as a separate signing step > after U-Boot is built. It's per class of hardware or whatever the vendor decides it should be. > > 4. Off topic, but re FIP, can someone just kill that off and use FIT? > It is such a shame that a new format was invented... You'd have to ask the TrustedFirmware-A code owners, but I doubt it. FIP is what TF-A uses for the first stage bootloader packaging and authentication of subsequent boot stages. Thanks /Ilias > > Regards, > Simon
Hi, On Tue, 27 Jun 2023 at 10:55, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > On Mon, 26 Jun 2023 at 14:19, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Ilias, > > > > On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > Hi Simon, > > > > > > [...] > > > > > > > > > > > > + > > > > > > > > > +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 > > > > > > > > > > > This make no sense to me whatsoever. Do we have an example in u-boot > > > generating multiple dtb versions for other reasons/subsystems? > > > > > > > > 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. > > > > > > > > NO, please do not do that. You should create a new file, not modify > > > > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > > > > will lead to all sorts of confusion. > > > > > > > > I thought we already had this discussion a while back? > > > > > > No we haven't. In fact I am struggling to see the confusion part. It's > > > fine for the u-boot dtb to include all the internal nodes DM needs, but > > > suddenly having the capsule signature is problematic? > > > > > > In the past the .esl file was part of the U-Boot binary and things were > > > working perfectly fine. In fact you could update/downgrade u-boot and the > > > signatures naturally followed along instead of having to update u-boot > > > *and* the dtb, which we have to do today. You could also build a capsule > > > way easier without injecting/removing signatures to the dtb. > > > You were the one that insisted on reverting that and instead adding it on > > > the dtb. We explained most of the downsides back then, along with some > > > security concerns. We also mentioned that the signature in the dtb makes > > > little sense since it's difference *per class of boards* and it's not > > > something we could include in static dtb files, but that lead nowhere... > > > > > > As Sughosh already said there are platforms that use the generated u-boot > > > dtb and the raw binary to assemble a FIP image. So why exactly adding the > > > capsule signature to the default dtb is wrong? Why should we add an > > > *extra* .dtb with one additional node -- which is very much needed by the > > > design you proposed. This will only lead to extra confusion. > > > > 1. I thought a capsule update was going to be a separate file, e.g. > > u-boot-capture.bin ? > > Yes the capsule itself is a different file and I dont think there's > any disagreement on how to generate that. > On the u-boot.bin you need to flash on the board though, you need to > include the public key you authenticate the incoming capsule against. > That's what Sughosh wants to inject. > > > > > 2. You can't put the signature into U-Boot. It needs to be in the > > capsule update so that U-Boot can check it. If you are talking about > > the public key, then yes that needs to be in U-Boot > > See above, > > > > > 3. Where does the public key come from? With the normal verified boot > > flow it is created (and added to the dtb) as a separate signing step > > after U-Boot is built. > > It's per class of hardware or whatever the vendor decides it should be. > > > > > 4. Off topic, but re FIP, can someone just kill that off and use FIT? > > It is such a shame that a new format was invented... > > You'd have to ask the TrustedFirmware-A code owners, but I doubt it. > FIP is what TF-A uses for the first stage bootloader packaging and > authentication of subsequent boot stages. Sughosh do you happen to be at EOSS so we could talk this through? Otherwise I'll reply later on. Regards, Simon
hi Simon, On Tue, 27 Jun 2023 at 15:44, Simon Glass <sjg@chromium.org> wrote: > > Hi, > > On Tue, 27 Jun 2023 at 10:55, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Hi Simon, > > > > On Mon, 26 Jun 2023 at 14:19, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Ilias, > > > > > > On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > Hi Simon, > > > > > > > > [...] > > > > > > > > > > > > > > + > > > > > > > > > > +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 > > > > > > > > > > > > > > This make no sense to me whatsoever. Do we have an example in u-boot > > > > generating multiple dtb versions for other reasons/subsystems? > > > > > > > > > > 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. > > > > > > > > > > NO, please do not do that. You should create a new file, not modify > > > > > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > > > > > will lead to all sorts of confusion. > > > > > > > > > > I thought we already had this discussion a while back? > > > > > > > > No we haven't. In fact I am struggling to see the confusion part. It's > > > > fine for the u-boot dtb to include all the internal nodes DM needs, but > > > > suddenly having the capsule signature is problematic? > > > > > > > > In the past the .esl file was part of the U-Boot binary and things were > > > > working perfectly fine. In fact you could update/downgrade u-boot and the > > > > signatures naturally followed along instead of having to update u-boot > > > > *and* the dtb, which we have to do today. You could also build a capsule > > > > way easier without injecting/removing signatures to the dtb. > > > > You were the one that insisted on reverting that and instead adding it on > > > > the dtb. We explained most of the downsides back then, along with some > > > > security concerns. We also mentioned that the signature in the dtb makes > > > > little sense since it's difference *per class of boards* and it's not > > > > something we could include in static dtb files, but that lead nowhere... > > > > > > > > As Sughosh already said there are platforms that use the generated u-boot > > > > dtb and the raw binary to assemble a FIP image. So why exactly adding the > > > > capsule signature to the default dtb is wrong? Why should we add an > > > > *extra* .dtb with one additional node -- which is very much needed by the > > > > design you proposed. This will only lead to extra confusion. > > > > > > 1. I thought a capsule update was going to be a separate file, e.g. > > > u-boot-capture.bin ? > > > > Yes the capsule itself is a different file and I dont think there's > > any disagreement on how to generate that. > > On the u-boot.bin you need to flash on the board though, you need to > > include the public key you authenticate the incoming capsule against. > > That's what Sughosh wants to inject. > > > > > > > > 2. You can't put the signature into U-Boot. It needs to be in the > > > capsule update so that U-Boot can check it. If you are talking about > > > the public key, then yes that needs to be in U-Boot > > > > See above, > > > > > > > > 3. Where does the public key come from? With the normal verified boot > > > flow it is created (and added to the dtb) as a separate signing step > > > after U-Boot is built. > > > > It's per class of hardware or whatever the vendor decides it should be. > > > > > > > > 4. Off topic, but re FIP, can someone just kill that off and use FIT? > > > It is such a shame that a new format was invented... > > > > You'd have to ask the TrustedFirmware-A code owners, but I doubt it. > > FIP is what TF-A uses for the first stage bootloader packaging and > > authentication of subsequent boot stages. > > Sughosh do you happen to be at EOSS so we could talk this through? > Otherwise I'll reply later on. I will not be at EOSS. You can reply to my question in your free time. Basically I wanted your feedback about addressing dependencies in binman [1]. -sughosh [1] - https://lists.denx.de/pipermail/u-boot/2023-June/521148.html
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
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