| Message ID | 20230815162623.1824357-3-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | capsule: Embed the public key ESL as part of build | expand |
Hi Sughosh, On Tue, 15 Aug 2023 at 10:26, 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 target for generating a dtsi file which contains the signature > node with the ESL file included as a property under the signature > node. Include the dtsi file in the dtb. 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> > --- > Changes since RFC series: > * Remove the default value of the config symbol. > * s/include_files/dtsi_include_list > * Add all the dtsi files being included as dependency for the dtb > target. > > lib/efi_loader/Kconfig | 8 ++++++++ > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > scripts/Makefile.lib | 18 +++++++++++++++++- > 3 files changed, 36 insertions(+), 1 deletion(-) > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index 9989e3f384..d20aaab6db 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -272,6 +272,14 @@ 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" Do we need this, or could we name it as we do with the .env file? It seems confusing to have to set this for each board - it might be better to have it in a defined location. Another idea is that we could use binman to pull this in, e.g. with an option to insert the capsule key during the build. Then it can be anywhere on the binman path. As you know I am not a fan of these opaque binaries when we have a nice self-describing format like devicetre. But we can worry about that problem another time. > + depends on EFI_CAPSULE_AUTHENTICATE > + help > + Provides the 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/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in > new file mode 100644 > index 0000000000..61a9f2b25e > --- /dev/null > +++ b/lib/efi_loader/capsule_esl.dtsi.in > @@ -0,0 +1,11 @@ > +// 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/scripts/Makefile.lib b/scripts/Makefile.lib > index 368b5a3e28..2e71f190bc 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -334,7 +334,23 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > ; \ > sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ > +cmd_capsule_esl_gen = \ > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) > + > +$(obj)/.capsule_esl.dtsi: > + $(call cmd_capsule_esl_gen) > + > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in > +capsule_esl_dtsi = .capsule_esl.dtsi > +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) > +dtsi_include_list += $(capsule_esl_dtsi) > +endif > + > +dtsi_include_list_deps = $(addprefix $(obj)/,$(subst $(quote),,$(dtsi_include_list))) > + > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(dtsi_include_list_deps) FORCE > $(call if_changed_dep,dtc) > > pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp) > -- > 2.34.1 > Regards, Simon
On Tue, Aug 15, 2023 at 09:56:20PM +0530, Sughosh Ganu 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 target for generating a dtsi file which contains the signature > node with the ESL file included as a property under the signature > node. Include the dtsi file in the dtb. 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> [snip] > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 368b5a3e28..2e71f190bc 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -334,7 +334,23 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > ; \ > sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ > +cmd_capsule_esl_gen = \ > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) > + > +$(obj)/.capsule_esl.dtsi: > + $(call cmd_capsule_esl_gen) > + > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in > +capsule_esl_dtsi = .capsule_esl.dtsi > +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) > +dtsi_include_list += $(capsule_esl_dtsi) > +endif We should only need to ifdef around appending to dtsi_include_list. The rest we can / should just leave always there, that's cleaner reading. > + > +dtsi_include_list_deps = $(addprefix $(obj)/,$(subst $(quote),,$(dtsi_include_list))) > + > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(dtsi_include_list_deps) FORCE This part here is a separate bugfix and we should do that as patch 2, and the rest of the changes here as patch 3.
hi Simon, On Wed, 16 Aug 2023 at 00:09, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Tue, 15 Aug 2023 at 10:26, 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 target for generating a dtsi file which contains the signature > > node with the ESL file included as a property under the signature > > node. Include the dtsi file in the dtb. 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> > > --- > > Changes since RFC series: > > * Remove the default value of the config symbol. > > * s/include_files/dtsi_include_list > > * Add all the dtsi files being included as dependency for the dtb > > target. > > > > lib/efi_loader/Kconfig | 8 ++++++++ > > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > > scripts/Makefile.lib | 18 +++++++++++++++++- > > 3 files changed, 36 insertions(+), 1 deletion(-) > > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index 9989e3f384..d20aaab6db 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -272,6 +272,14 @@ 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" > > Do we need this, or could we name it as we do with the .env file? It > seems confusing to have to set this for each board - it might be > better to have it in a defined location. The reason I put this is because I thought this gave the user the flexibility to provide the location and name of the ESL. But I suppose that the board directory would be a good location to expect this file. Then this file can have a name like capsule_pub_key,esl. Tom, what are your thoughts on this? > > Another idea is that we could use binman to pull this in, e.g. with an > option to insert the capsule key during the build. Then it can be > anywhere on the binman path. So, if I understand you right, I believe you are suggesting using binman for this task? Which, just to make it clear, I pretty much understand what would be needed to be done to get this working in binman. A typical binman image for for this task would look like &binman { u-boot-capsule-esl { u-boot-no-dtb { }; fdt-esl { u-boot-dtb { }; }; }; }; The fdt-esl entry type would then call the fdt_add_pubkey tool to do the needful. We also support capsule update for FIT images, and that would mean adding some logic to call the fdt_add_pubkey for the fdt_list. So implementing this is not a difficult thing at all. But the main issue, or rather the only issue with this is that we are getting an image with a different name, like u-boot-capsule-esl.bin as the resulting image. But like I had mentioned earlier in our discussions, we have platforms which boot the u-boot.bin binary. We also have the ST boards which generate the fip image and use the u-boot.dtb as the BL33_CFG. So generating a new image binary, or a new DTB would break these boards. Not to mention that these platforms would not be interested in using a different image just because they are enabling some functionality on the platform. > > As you know I am not a fan of these opaque binaries when we have a > nice self-describing format like devicetre. But we can worry about > that problem another time. I know, but this is what we have when we follow the UEFI specification. It is not our design. -sughosh > > > + depends on EFI_CAPSULE_AUTHENTICATE > > + help > > + Provides the 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/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in > > new file mode 100644 > > index 0000000000..61a9f2b25e > > --- /dev/null > > +++ b/lib/efi_loader/capsule_esl.dtsi.in > > @@ -0,0 +1,11 @@ > > +// 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/scripts/Makefile.lib b/scripts/Makefile.lib > > index 368b5a3e28..2e71f190bc 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -334,7 +334,23 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > ; \ > > sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > > > -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ > > +cmd_capsule_esl_gen = \ > > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) > > + > > +$(obj)/.capsule_esl.dtsi: > > + $(call cmd_capsule_esl_gen) > > + > > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in > > +capsule_esl_dtsi = .capsule_esl.dtsi > > +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) > > +dtsi_include_list += $(capsule_esl_dtsi) > > +endif > > + > > +dtsi_include_list_deps = $(addprefix $(obj)/,$(subst $(quote),,$(dtsi_include_list))) > > + > > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(dtsi_include_list_deps) FORCE > > $(call if_changed_dep,dtc) > > > > pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp) > > -- > > 2.34.1 > > > > Regards, > Simon
hi Tom, On Wed, 16 Aug 2023 at 01:54, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Aug 15, 2023 at 09:56:20PM +0530, Sughosh Ganu 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 target for generating a dtsi file which contains the signature > > node with the ESL file included as a property under the signature > > node. Include the dtsi file in the dtb. 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> > [snip] > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index 368b5a3e28..2e71f190bc 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -334,7 +334,23 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > ; \ > > sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > > > -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ > > +cmd_capsule_esl_gen = \ > > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) > > + > > +$(obj)/.capsule_esl.dtsi: > > + $(call cmd_capsule_esl_gen) > > + > > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in > > +capsule_esl_dtsi = .capsule_esl.dtsi > > +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) > > +dtsi_include_list += $(capsule_esl_dtsi) > > +endif > > We should only need to ifdef around appending to dtsi_include_list. The > rest we can / should just leave always there, that's cleaner reading. Okay > > > + > > +dtsi_include_list_deps = $(addprefix $(obj)/,$(subst $(quote),,$(dtsi_include_list))) > > + > > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(dtsi_include_list_deps) FORCE > > This part here is a separate bugfix and we should do that as patch 2, > and the rest of the changes here as patch 3. Will put this in a separate patch. Thanks. -sughosh
On Wed, Aug 16, 2023 at 09:58:42AM +0530, Sughosh Ganu wrote: > hi Simon, > > On Wed, 16 Aug 2023 at 00:09, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Tue, 15 Aug 2023 at 10:26, 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 target for generating a dtsi file which contains the signature > > > node with the ESL file included as a property under the signature > > > node. Include the dtsi file in the dtb. 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> > > > --- > > > Changes since RFC series: > > > * Remove the default value of the config symbol. > > > * s/include_files/dtsi_include_list > > > * Add all the dtsi files being included as dependency for the dtb > > > target. > > > > > > lib/efi_loader/Kconfig | 8 ++++++++ > > > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > > > scripts/Makefile.lib | 18 +++++++++++++++++- > > > 3 files changed, 36 insertions(+), 1 deletion(-) > > > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > index 9989e3f384..d20aaab6db 100644 > > > --- a/lib/efi_loader/Kconfig > > > +++ b/lib/efi_loader/Kconfig > > > @@ -272,6 +272,14 @@ 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" > > > > Do we need this, or could we name it as we do with the .env file? It > > seems confusing to have to set this for each board - it might be > > better to have it in a defined location. > > The reason I put this is because I thought this gave the user the > flexibility to provide the location and name of the ESL. But I suppose > that the board directory would be a good location to expect this file. > Then this file can have a name like capsule_pub_key,esl. Tom, what are > your thoughts on this? I feel like an automatic name we can guess isn't likely how this will be used in the real world, so we should leave this as configurable.
Hi Tom, On Wed, 16 Aug 2023 at 15:26, Tom Rini <trini@konsulko.com> wrote: > > On Wed, Aug 16, 2023 at 09:58:42AM +0530, Sughosh Ganu wrote: > > hi Simon, > > > > On Wed, 16 Aug 2023 at 00:09, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Sughosh, > > > > > > On Tue, 15 Aug 2023 at 10:26, 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 target for generating a dtsi file which contains the signature > > > > node with the ESL file included as a property under the signature > > > > node. Include the dtsi file in the dtb. 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> > > > > --- > > > > Changes since RFC series: > > > > * Remove the default value of the config symbol. > > > > * s/include_files/dtsi_include_list > > > > * Add all the dtsi files being included as dependency for the dtb > > > > target. > > > > > > > > lib/efi_loader/Kconfig | 8 ++++++++ > > > > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > > > > scripts/Makefile.lib | 18 +++++++++++++++++- > > > > 3 files changed, 36 insertions(+), 1 deletion(-) > > > > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > index 9989e3f384..d20aaab6db 100644 > > > > --- a/lib/efi_loader/Kconfig > > > > +++ b/lib/efi_loader/Kconfig > > > > @@ -272,6 +272,14 @@ 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" > > > > > > Do we need this, or could we name it as we do with the .env file? It > > > seems confusing to have to set this for each board - it might be > > > better to have it in a defined location. > > > > The reason I put this is because I thought this gave the user the > > flexibility to provide the location and name of the ESL. But I suppose > > that the board directory would be a good location to expect this file. > > Then this file can have a name like capsule_pub_key,esl. Tom, what are > > your thoughts on this? > > I feel like an automatic name we can guess isn't likely how this will be > used in the real world, so we should leave this as configurable. Are we expecting these files to end up in the source tree? Where would they go? Regards, Simon
On Thu, Aug 17, 2023 at 07:41:33AM -0600, Simon Glass wrote: > Hi Tom, > > On Wed, 16 Aug 2023 at 15:26, Tom Rini <trini@konsulko.com> wrote: > > > > On Wed, Aug 16, 2023 at 09:58:42AM +0530, Sughosh Ganu wrote: > > > hi Simon, > > > > > > On Wed, 16 Aug 2023 at 00:09, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Tue, 15 Aug 2023 at 10:26, 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 target for generating a dtsi file which contains the signature > > > > > node with the ESL file included as a property under the signature > > > > > node. Include the dtsi file in the dtb. 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> > > > > > --- > > > > > Changes since RFC series: > > > > > * Remove the default value of the config symbol. > > > > > * s/include_files/dtsi_include_list > > > > > * Add all the dtsi files being included as dependency for the dtb > > > > > target. > > > > > > > > > > lib/efi_loader/Kconfig | 8 ++++++++ > > > > > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > > > > > scripts/Makefile.lib | 18 +++++++++++++++++- > > > > > 3 files changed, 36 insertions(+), 1 deletion(-) > > > > > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > > > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > > index 9989e3f384..d20aaab6db 100644 > > > > > --- a/lib/efi_loader/Kconfig > > > > > +++ b/lib/efi_loader/Kconfig > > > > > @@ -272,6 +272,14 @@ 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" > > > > > > > > Do we need this, or could we name it as we do with the .env file? It > > > > seems confusing to have to set this for each board - it might be > > > > better to have it in a defined location. > > > > > > The reason I put this is because I thought this gave the user the > > > flexibility to provide the location and name of the ESL. But I suppose > > > that the board directory would be a good location to expect this file. > > > Then this file can have a name like capsule_pub_key,esl. Tom, what are > > > your thoughts on this? > > > > I feel like an automatic name we can guess isn't likely how this will be > > used in the real world, so we should leave this as configurable. > > Are we expecting these files to end up in the source tree? Where would they go? Yes, they should be board/vendor/common/whatever-vendor-uses-internally.esl or so. As I think I mentioned on IRC, in theory someone like Asus should be using the same file here for their rockchip-based tinker board and their x8664 based motherboards too. And it's a public key, not a private key. But we still need to ask here because a vendor may care more about "security" and so have the key /over/somewhere/else more than reproducible builds.
Hi Tom, On Thu, 17 Aug 2023 at 09:10, Tom Rini <trini@konsulko.com> wrote: > > On Thu, Aug 17, 2023 at 07:41:33AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 16 Aug 2023 at 15:26, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Wed, Aug 16, 2023 at 09:58:42AM +0530, Sughosh Ganu wrote: > > > > hi Simon, > > > > > > > > On Wed, 16 Aug 2023 at 00:09, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > Hi Sughosh, > > > > > > > > > > On Tue, 15 Aug 2023 at 10:26, 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 target for generating a dtsi file which contains the signature > > > > > > node with the ESL file included as a property under the signature > > > > > > node. Include the dtsi file in the dtb. 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> > > > > > > --- > > > > > > Changes since RFC series: > > > > > > * Remove the default value of the config symbol. > > > > > > * s/include_files/dtsi_include_list > > > > > > * Add all the dtsi files being included as dependency for the dtb > > > > > > target. > > > > > > > > > > > > lib/efi_loader/Kconfig | 8 ++++++++ > > > > > > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > > > > > > scripts/Makefile.lib | 18 +++++++++++++++++- > > > > > > 3 files changed, 36 insertions(+), 1 deletion(-) > > > > > > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > > > > > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > > > index 9989e3f384..d20aaab6db 100644 > > > > > > --- a/lib/efi_loader/Kconfig > > > > > > +++ b/lib/efi_loader/Kconfig > > > > > > @@ -272,6 +272,14 @@ 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" > > > > > > > > > > Do we need this, or could we name it as we do with the .env file? It > > > > > seems confusing to have to set this for each board - it might be > > > > > better to have it in a defined location. > > > > > > > > The reason I put this is because I thought this gave the user the > > > > flexibility to provide the location and name of the ESL. But I suppose > > > > that the board directory would be a good location to expect this file. > > > > Then this file can have a name like capsule_pub_key,esl. Tom, what are > > > > your thoughts on this? > > > > > > I feel like an automatic name we can guess isn't likely how this will be > > > used in the real world, so we should leave this as configurable. > > > > Are we expecting these files to end up in the source tree? Where would they go? > > Yes, they should be > board/vendor/common/whatever-vendor-uses-internally.esl or so. As I > think I mentioned on IRC, in theory someone like Asus should be using > the same file here for their rockchip-based tinker board and their x8664 > based motherboards too. And it's a public key, not a private key. But > we still need to ask here because a vendor may care more about > "security" and so have the key /over/somewhere/else more than > reproducible builds. OK. Regards, Simon
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 9989e3f384..d20aaab6db 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -272,6 +272,14 @@ 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" + depends on EFI_CAPSULE_AUTHENTICATE + help + Provides the 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/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in new file mode 100644 index 0000000000..61a9f2b25e --- /dev/null +++ b/lib/efi_loader/capsule_esl.dtsi.in @@ -0,0 +1,11 @@ +// 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/scripts/Makefile.lib b/scripts/Makefile.lib index 368b5a3e28..2e71f190bc 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -334,7 +334,23 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ ; \ sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ +cmd_capsule_esl_gen = \ + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) + +$(obj)/.capsule_esl.dtsi: + $(call cmd_capsule_esl_gen) + +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in +capsule_esl_dtsi = .capsule_esl.dtsi +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) +dtsi_include_list += $(capsule_esl_dtsi) +endif + +dtsi_include_list_deps = $(addprefix $(obj)/,$(subst $(quote),,$(dtsi_include_list))) + +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(dtsi_include_list_deps) FORCE $(call if_changed_dep,dtc) pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp)
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 target for generating a dtsi file which contains the signature node with the ESL file included as a property under the signature node. Include the dtsi file in the dtb. 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> --- Changes since RFC series: * Remove the default value of the config symbol. * s/include_files/dtsi_include_list * Add all the dtsi files being included as dependency for the dtb target. lib/efi_loader/Kconfig | 8 ++++++++ lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ scripts/Makefile.lib | 18 +++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 lib/efi_loader/capsule_esl.dtsi.in