diff mbox series

[v5,03/12] capsule: authenticate: Add capsule public key in platform's dtb

Message ID 20230725085725.350917-4-sughosh.ganu@linaro.org
State New
Headers show
Series Integrate EFI capsule tasks into u-boot's build flow | expand

Commit Message

Sughosh Ganu July 25, 2023, 8:57 a.m. UTC
The EFI capsule authentication logic in u-boot expects the public key
in the form of an EFI Signature List(ESL) to be provided as part of
the platform's dtb. Currently, the embedding of the ESL file into the
dtb needs to be done manually.

Add a signature node in the u-boot dtsi file and include the public
key through the capsule-key property. This file is per architecture,
and is currently being added for sandbox and arm architectures. It
will have to be added for other architectures which need to enable
capsule authentication support.

The path to the ESL file is specified through the
CONFIG_EFI_CAPSULE_ESL_FILE symbol.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
Changes since V4:
* Fix multi line comment format.
* Drop additional blank line.
* Remove the check for CONFIG_EFI_HAVE_CAPSULE_SUPPORT from arm's
  u-boot.dtsi.
* Wrap the help text in the EFI_CAPSULE_ESL_FILE config at 72 chars.

 arch/arm/dts/u-boot.dtsi     | 14 ++++++++++++++
 arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
 lib/efi_loader/Kconfig       |  9 +++++++++
 lib/efi_loader/Makefile      |  7 +++++++
 4 files changed, 47 insertions(+)
 create mode 100644 arch/arm/dts/u-boot.dtsi
 create mode 100644 arch/sandbox/dts/u-boot.dtsi

Comments

Simon Glass July 25, 2023, 10:52 p.m. UTC | #1
Hi Sughosh,

On Tue, 25 Jul 2023 at 02:58, 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 signature node in the u-boot dtsi file and include the public
> key through the capsule-key property. This file is per architecture,
> and is currently being added for sandbox and arm architectures. It
> will have to be added for other architectures which need to enable
> capsule authentication support.
>
> The path to the ESL file is specified through the
> CONFIG_EFI_CAPSULE_ESL_FILE symbol.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> Changes since V4:
> * Fix multi line comment format.
> * Drop additional blank line.
> * Remove the check for CONFIG_EFI_HAVE_CAPSULE_SUPPORT from arm's
>   u-boot.dtsi.
> * Wrap the help text in the EFI_CAPSULE_ESL_FILE config at 72 chars.
>
>  arch/arm/dts/u-boot.dtsi     | 14 ++++++++++++++
>  arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
>  lib/efi_loader/Kconfig       |  9 +++++++++
>  lib/efi_loader/Makefile      |  7 +++++++
>  4 files changed, 47 insertions(+)
>  create mode 100644 arch/arm/dts/u-boot.dtsi
>  create mode 100644 arch/sandbox/dts/u-boot.dtsi
>
> diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> new file mode 100644
> index 0000000000..4f31da4521
> --- /dev/null
> +++ b/arch/arm/dts/u-boot.dtsi
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/**
> + * Devicetree file with miscellaneous nodes that will be included
> + * at build time into the DTB. Currently being used for including
> + * capsule related information.
> + */
> +
> +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> +/ {
> +       signature {
> +               capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> +       };
> +};
> +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> new file mode 100644
> index 0000000000..60bd004937
> --- /dev/null
> +++ b/arch/sandbox/dts/u-boot.dtsi
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Devicetree file with miscellaneous nodes that will be included
> + * at build time into the DTB. Currently being used for including
> + * capsule related information.
> + *
> + */
> +
> +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> +/ {
> +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> +       signature {
> +               capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> +       };
> +#endif
> +};
> +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */

You missed my comment there. You should not need the outer #ifdef, but
if you do, please combine them into one #if

> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index a22e47616f..0d559ff3a1 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -235,6 +235,15 @@ 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/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 1a8c8d7cab..c52c9d27bd 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -89,3 +89,10 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
>
>  EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
>  $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
> +
> +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE))
> +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> +$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE)
> +endif
> +endif
> --
> 2.34.1
>

Regards,
Simon
Sughosh Ganu July 26, 2023, 8:57 a.m. UTC | #2
hi Simon,

On Wed, 26 Jul 2023 at 04:22, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Tue, 25 Jul 2023 at 02:58, 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 signature node in the u-boot dtsi file and include the public
> > key through the capsule-key property. This file is per architecture,
> > and is currently being added for sandbox and arm architectures. It
> > will have to be added for other architectures which need to enable
> > capsule authentication support.
> >
> > The path to the ESL file is specified through the
> > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > Changes since V4:
> > * Fix multi line comment format.
> > * Drop additional blank line.
> > * Remove the check for CONFIG_EFI_HAVE_CAPSULE_SUPPORT from arm's
> >   u-boot.dtsi.
> > * Wrap the help text in the EFI_CAPSULE_ESL_FILE config at 72 chars.
> >

<snip>

> > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > new file mode 100644
> > index 0000000000..60bd004937
> > --- /dev/null
> > +++ b/arch/sandbox/dts/u-boot.dtsi
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Devicetree file with miscellaneous nodes that will be included
> > + * at build time into the DTB. Currently being used for including
> > + * capsule related information.
> > + *
> > + */
> > +
> > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +/ {
> > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > +       signature {
> > +               capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > +       };
> > +#endif
> > +};
> > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
>
> You missed my comment there. You should not need the outer #ifdef, but
> if you do, please combine them into one #if

I did not miss your comment. The reason I have kept both the ifdefs is
that we need to include stuff which is needed only when
CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, and then the rest of the
stuff is needed only when CONFIG_EFI_HAVE_CAPSULE_SUPPORT is enabled.
Not having both the ifdefs would result in build failures. In the
u-boot.dtsi included for the arm arch, I am using a single ifdef,
since we are including only the signature node in that file.

-sughosh

>
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index a22e47616f..0d559ff3a1 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -235,6 +235,15 @@ 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/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index 1a8c8d7cab..c52c9d27bd 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -89,3 +89,10 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
> >
> >  EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> >  $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
> > +
> > +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> > +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE))
> > +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> > +$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE)
> > +endif
> > +endif
> > --
> > 2.34.1
> >
>
> Regards,
> Simon
Simon Glass July 26, 2023, 2:26 p.m. UTC | #3
Hi Sughosh,

On Wed, 26 Jul 2023 at 02:57, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Wed, 26 Jul 2023 at 04:22, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 25 Jul 2023 at 02:58, 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 signature node in the u-boot dtsi file and include the public
> > > key through the capsule-key property. This file is per architecture,
> > > and is currently being added for sandbox and arm architectures. It
> > > will have to be added for other architectures which need to enable
> > > capsule authentication support.
> > >
> > > The path to the ESL file is specified through the
> > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > > Changes since V4:
> > > * Fix multi line comment format.
> > > * Drop additional blank line.
> > > * Remove the check for CONFIG_EFI_HAVE_CAPSULE_SUPPORT from arm's
> > >   u-boot.dtsi.
> > > * Wrap the help text in the EFI_CAPSULE_ESL_FILE config at 72 chars.
> > >
>
> <snip>
>
> > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > new file mode 100644
> > > index 0000000000..60bd004937
> > > --- /dev/null
> > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > @@ -0,0 +1,17 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Devicetree file with miscellaneous nodes that will be included
> > > + * at build time into the DTB. Currently being used for including
> > > + * capsule related information.
> > > + *
> > > + */
> > > +
> > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > +/ {
> > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > +       signature {
> > > +               capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > +       };
> > > +#endif
> > > +};
> > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> >
> > You missed my comment there. You should not need the outer #ifdef, but
> > if you do, please combine them into one #if
>
> I did not miss your comment. The reason I have kept both the ifdefs is
> that we need to include stuff which is needed only when
> CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, and then the rest of the
> stuff is needed only when CONFIG_EFI_HAVE_CAPSULE_SUPPORT is enabled.
> Not having both the ifdefs would result in build failures. In the
> u-boot.dtsi included for the arm arch, I am using a single ifdef,
> since we are including only the signature node in that file.

Well having

/ {
};

is harmless in all cases, I believe. So you should not need the outer one?

Regards,
Simon
[..]
Sughosh Ganu Aug. 1, 2023, 5:35 p.m. UTC | #4
hi Simon,

On Wed, 26 Jul 2023 at 19:56, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 26 Jul 2023 at 02:57, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Wed, 26 Jul 2023 at 04:22, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 25 Jul 2023 at 02:58, 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 signature node in the u-boot dtsi file and include the public
> > > > key through the capsule-key property. This file is per architecture,
> > > > and is currently being added for sandbox and arm architectures. It
> > > > will have to be added for other architectures which need to enable
> > > > capsule authentication support.
> > > >
> > > > The path to the ESL file is specified through the
> > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > ---
> > > > Changes since V4:
> > > > * Fix multi line comment format.
> > > > * Drop additional blank line.
> > > > * Remove the check for CONFIG_EFI_HAVE_CAPSULE_SUPPORT from arm's
> > > >   u-boot.dtsi.
> > > > * Wrap the help text in the EFI_CAPSULE_ESL_FILE config at 72 chars.
> > > >
> >
> > <snip>
> >
> > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > > new file mode 100644
> > > > index 0000000000..60bd004937
> > > > --- /dev/null
> > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > > @@ -0,0 +1,17 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > + * at build time into the DTB. Currently being used for including
> > > > + * capsule related information.
> > > > + *
> > > > + */
> > > > +
> > > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > +/ {
> > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > +       signature {
> > > > +               capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > +       };
> > > > +#endif
> > > > +};
> > > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > >
> > > You missed my comment there. You should not need the outer #ifdef, but
> > > if you do, please combine them into one #if
> >
> > I did not miss your comment. The reason I have kept both the ifdefs is
> > that we need to include stuff which is needed only when
> > CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, and then the rest of the
> > stuff is needed only when CONFIG_EFI_HAVE_CAPSULE_SUPPORT is enabled.
> > Not having both the ifdefs would result in build failures. In the
> > u-boot.dtsi included for the arm arch, I am using a single ifdef,
> > since we are including only the signature node in that file.
>
> Well having
>
> / {
> };
>
> is harmless in all cases, I believe. So you should not need the outer one?

Sorry, I missed out this comment earlier. So this would not be an
empty node but contain the capsule generation nodes. This would result
in capsules getting generated for the sandbox_vpl and sandbox_noinst
variants which do not enable the capsule functionality.

-sughosh
Simon Glass Aug. 2, 2023, 12:52 p.m. UTC | #5
Hi Sughosh,

On Tue, 1 Aug 2023 at 11:35, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Wed, 26 Jul 2023 at 19:56, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Wed, 26 Jul 2023 at 02:57, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Wed, 26 Jul 2023 at 04:22, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Tue, 25 Jul 2023 at 02:58, 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 signature node in the u-boot dtsi file and include the public
> > > > > key through the capsule-key property. This file is per architecture,
> > > > > and is currently being added for sandbox and arm architectures. It
> > > > > will have to be added for other architectures which need to enable
> > > > > capsule authentication support.
> > > > >
> > > > > The path to the ESL file is specified through the
> > > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > ---
> > > > > Changes since V4:
> > > > > * Fix multi line comment format.
> > > > > * Drop additional blank line.
> > > > > * Remove the check for CONFIG_EFI_HAVE_CAPSULE_SUPPORT from arm's
> > > > >   u-boot.dtsi.
> > > > > * Wrap the help text in the EFI_CAPSULE_ESL_FILE config at 72 chars.
> > > > >
> > >
> > > <snip>
> > >
> > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > > > new file mode 100644
> > > > > index 0000000000..60bd004937
> > > > > --- /dev/null
> > > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > > > @@ -0,0 +1,17 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > > + * at build time into the DTB. Currently being used for including
> > > > > + * capsule related information.
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > > +/ {
> > > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > > +       signature {
> > > > > +               capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > > +       };
> > > > > +#endif
> > > > > +};
> > > > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > > >
> > > > You missed my comment there. You should not need the outer #ifdef, but
> > > > if you do, please combine them into one #if
> > >
> > > I did not miss your comment. The reason I have kept both the ifdefs is
> > > that we need to include stuff which is needed only when
> > > CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, and then the rest of the
> > > stuff is needed only when CONFIG_EFI_HAVE_CAPSULE_SUPPORT is enabled.
> > > Not having both the ifdefs would result in build failures. In the
> > > u-boot.dtsi included for the arm arch, I am using a single ifdef,
> > > since we are including only the signature node in that file.
> >
> > Well having
> >
> > / {
> > };
> >
> > is harmless in all cases, I believe. So you should not need the outer one?
>
> Sorry, I missed out this comment earlier. So this would not be an
> empty node but contain the capsule generation nodes. This would result
> in capsules getting generated for the sandbox_vpl and sandbox_noinst
> variants which do not enable the capsule functionality.

If that is all it is then I think it is fine to leave them in. Those
builds won't care anyway.

Regards,
Simon
Sughosh Ganu Aug. 3, 2023, 11:21 a.m. UTC | #6
hi Simon,

On Wed, 2 Aug 2023 at 18:22, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Tue, 1 Aug 2023 at 11:35, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Wed, 26 Jul 2023 at 19:56, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Wed, 26 Jul 2023 at 02:57, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Wed, 26 Jul 2023 at 04:22, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Tue, 25 Jul 2023 at 02:58, 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 signature node in the u-boot dtsi file and include the public
> > > > > > key through the capsule-key property. This file is per architecture,
> > > > > > and is currently being added for sandbox and arm architectures. It
> > > > > > will have to be added for other architectures which need to enable
> > > > > > capsule authentication support.
> > > > > >
> > > > > > The path to the ESL file is specified through the
> > > > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > ---
> > > > > > Changes since V4:
> > > > > > * Fix multi line comment format.
> > > > > > * Drop additional blank line.
> > > > > > * Remove the check for CONFIG_EFI_HAVE_CAPSULE_SUPPORT from arm's
> > > > > >   u-boot.dtsi.
> > > > > > * Wrap the help text in the EFI_CAPSULE_ESL_FILE config at 72 chars.
> > > > > >
> > > >
> > > > <snip>
> > > >
> > > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > > > > new file mode 100644
> > > > > > index 0000000000..60bd004937
> > > > > > --- /dev/null
> > > > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > > > > @@ -0,0 +1,17 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > > > + * at build time into the DTB. Currently being used for including
> > > > > > + * capsule related information.
> > > > > > + *
> > > > > > + */
> > > > > > +
> > > > > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > > > +/ {
> > > > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > > > +       signature {
> > > > > > +               capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > > > +       };
> > > > > > +#endif
> > > > > > +};
> > > > > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > > > >
> > > > > You missed my comment there. You should not need the outer #ifdef, but
> > > > > if you do, please combine them into one #if
> > > >
> > > > I did not miss your comment. The reason I have kept both the ifdefs is
> > > > that we need to include stuff which is needed only when
> > > > CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, and then the rest of the
> > > > stuff is needed only when CONFIG_EFI_HAVE_CAPSULE_SUPPORT is enabled.
> > > > Not having both the ifdefs would result in build failures. In the
> > > > u-boot.dtsi included for the arm arch, I am using a single ifdef,
> > > > since we are including only the signature node in that file.
> > >
> > > Well having
> > >
> > > / {
> > > };
> > >
> > > is harmless in all cases, I believe. So you should not need the outer one?
> >
> > Sorry, I missed out this comment earlier. So this would not be an
> > empty node but contain the capsule generation nodes. This would result
> > in capsules getting generated for the sandbox_vpl and sandbox_noinst
> > variants which do not enable the capsule functionality.
>
> If that is all it is then I think it is fine to leave them in. Those
> builds won't care anyway.

Fair enough. Will drop the outer ifdef.

-sughosh

>
> Regards,
> Simon
diff mbox series

Patch

diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
new file mode 100644
index 0000000000..4f31da4521
--- /dev/null
+++ b/arch/arm/dts/u-boot.dtsi
@@ -0,0 +1,14 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/**
+ * Devicetree file with miscellaneous nodes that will be included
+ * at build time into the DTB. Currently being used for including
+ * capsule related information.
+ */
+
+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
+/ {
+	signature {
+		capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
+	};
+};
+#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
new file mode 100644
index 0000000000..60bd004937
--- /dev/null
+++ b/arch/sandbox/dts/u-boot.dtsi
@@ -0,0 +1,17 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Devicetree file with miscellaneous nodes that will be included
+ * at build time into the DTB. Currently being used for including
+ * capsule related information.
+ *
+ */
+
+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
+/ {
+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
+	signature {
+		capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
+	};
+#endif
+};
+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index a22e47616f..0d559ff3a1 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -235,6 +235,15 @@  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/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 1a8c8d7cab..c52c9d27bd 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -89,3 +89,10 @@  obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
 
 EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
 $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
+
+ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
+EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE))
+ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
+$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE)
+endif
+endif