diff mbox series

[v6,3/9] capsule: authenticate: Add capsule public key in platform's dtb

Message ID 20230801174018.1342555-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 Aug. 1, 2023, 5:40 p.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 V5:
* None

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

Comments

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

On Tue, 1 Aug 2023 at 11:40, 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 V5:
> * None
>
>  arch/arm/dts/u-boot.dtsi     | 14 ++++++++++++++
>  arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
>  lib/efi_loader/Kconfig       |  9 +++++++++
>  3 files changed, 40 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 */

For sandbox at least, you don't need these #ifdefs as I doubt anything
cares about any new properties.

Somehow we are not on the same page even on the inner #ifdef, so I
think it is best to remove both.

> 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
> --
> 2.34.1
>

Regards,
Simon
Simon Glass Aug. 2, 2023, 1:34 p.m. UTC | #2 
Hi Sughosh,

On Wed, 2 Aug 2023 at 06:52, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Tue, 1 Aug 2023 at 11:40, 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 V5:
> > * None
> >
> >  arch/arm/dts/u-boot.dtsi     | 14 ++++++++++++++
> >  arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
> >  lib/efi_loader/Kconfig       |  9 +++++++++
> >  3 files changed, 40 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 */

Ilias mentioned that this binding can cause problems if it not
upstream, causing the platform to fail validation. So we need to agree
a binding for it in dt-schema. Can you send a patch to we can discuss
it there?

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

On Wed, 2 Aug 2023 at 19:04, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 2 Aug 2023 at 06:52, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 1 Aug 2023 at 11:40, 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 V5:
> > > * None
> > >
> > >  arch/arm/dts/u-boot.dtsi     | 14 ++++++++++++++
> > >  arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
> > >  lib/efi_loader/Kconfig       |  9 +++++++++
> > >  3 files changed, 40 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 */
>
> Ilias mentioned that this binding can cause problems if it not
> upstream, causing the platform to fail validation. So we need to agree
> a binding for it in dt-schema. Can you send a patch to we can discuss
> it there?

There was an effort from Jassi earlier [1] to upstream one such
binding for the FWU node. But Rob had asked to remove this in u-boot
before the DT was passed over to the kernel.

-sughosh

[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6cg-5qg@mail.gmail.com/

>
> Regards,
> Simon
Simon Glass Aug. 4, 2023, 12:18 a.m. UTC | #4 
Hi Sughosh,

On Thu, 3 Aug 2023 at 05:12, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Wed, 2 Aug 2023 at 19:04, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Wed, 2 Aug 2023 at 06:52, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 1 Aug 2023 at 11:40, 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 V5:
> > > > * None
> > > >
> > > >  arch/arm/dts/u-boot.dtsi     | 14 ++++++++++++++
> > > >  arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
> > > >  lib/efi_loader/Kconfig       |  9 +++++++++
> > > >  3 files changed, 40 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 */
> >
> > Ilias mentioned that this binding can cause problems if it not
> > upstream, causing the platform to fail validation. So we need to agree
> > a binding for it in dt-schema. Can you send a patch to we can discuss
> > it there?
>
> There was an effort from Jassi earlier [1] to upstream one such
> binding for the FWU node. But Rob had asked to remove this in u-boot
> before the DT was passed over to the kernel.

I don't like that idea...U-Boot's DT needs to pass validation too, of course!


- Simon

>
> -sughosh
>
> [1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6cg-5qg@mail.gmail.com/
>
> >
> > Regards,
> > Simon
Tom Rini Aug. 4, 2023, 7:05 p.m. UTC | #5 
On Thu, Aug 03, 2023 at 06:18:24PM -0600, Simon Glass wrote:
> Hi Sughosh,
> 
> On Thu, 3 Aug 2023 at 05:12, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Wed, 2 Aug 2023 at 19:04, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Wed, 2 Aug 2023 at 06:52, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Tue, 1 Aug 2023 at 11:40, 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 V5:
> > > > > * None
> > > > >
> > > > >  arch/arm/dts/u-boot.dtsi     | 14 ++++++++++++++
> > > > >  arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
> > > > >  lib/efi_loader/Kconfig       |  9 +++++++++
> > > > >  3 files changed, 40 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 */
> > >
> > > Ilias mentioned that this binding can cause problems if it not
> > > upstream, causing the platform to fail validation. So we need to agree
> > > a binding for it in dt-schema. Can you send a patch to we can discuss
> > > it there?
> >
> > There was an effort from Jassi earlier [1] to upstream one such
> > binding for the FWU node. But Rob had asked to remove this in u-boot
> > before the DT was passed over to the kernel.
> 
> I don't like that idea...U-Boot's DT needs to pass validation too, of course!

We're about 30 steps away from being able to just validate our trees
too.  We shouldn't make things intentionally harder but we will need to
have some local schemas or something for cases where they've already
been intentionally submitted and rejected by dt-schema for being a
purely internal need.
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