Re: [PATCH v4 03/12] capsule: authenticate: Add capsule public key in platform's dtb

2023-07-17 Thread Sughosh Ganu 
hi Simon,

On Sun, 16 Jul 2023 at 05:12, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Sat, 15 Jul 2023 at 07:46, 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 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 
> > ---
> > Changes since V3:
> > * Put the two ifdef statements together in arm architecture's
> >   u-boot.dtsi file.
> > * Remove the extra blank line in the Kconfig.
> >
> >  arch/arm/dts/u-boot.dtsi | 17 +
> >  arch/sandbox/dts/u-boot.dtsi | 17 +
> >  lib/efi_loader/Kconfig   | 10 ++
> >  lib/efi_loader/Makefile  |  7 +++
> >  4 files changed, 51 insertions(+)
> >  create mode 100644 arch/arm/dts/u-boot.dtsi
> >  create mode 100644 arch/sandbox/dts/u-boot.dtsi
>
> This approach seems OK to me for now. It is a bit strange to specify a
> CONFIG option to add something to the DT, but we can always adjust it
> later if needed.
>
> >
> > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > new file mode 100644
> > index 00..2a9359c43c
> > --- /dev/null
> > +++ b/arch/arm/dts/u-boot.dtsi
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
>
> /**
>
> for multi-line comments

Okay

>
> > + * Devicetree file with miscellaneous nodes that will be included
> > + * at build time into the DTB. Currently being used for including
> > + * capsule related information.
> > + *
>
> drop blank line

Okay

>
> > + */
> > +
> > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
>
> Can you combine these, or can you omit the first one?

I will drop the first line. Should build for all platforms I believe.

>
> > +/ {
> > +   signature {
> > +   capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > +   };
> > +};
> > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > new file mode 100644
> > index 00..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..9abb9a4db3 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -235,6 +235,16 @@ 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.
>
> Can you wrap to 72 chars or so?

Okay

-sughosh

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

Re: [PATCH v4 03/12] capsule: authenticate: Add capsule public key in platform's dtb

2023-07-15 Thread Simon Glass 
Hi Sughosh,

On Sat, 15 Jul 2023 at 07:46, 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 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 
> ---
> Changes since V3:
> * Put the two ifdef statements together in arm architecture's
>   u-boot.dtsi file.
> * Remove the extra blank line in the Kconfig.
>
>  arch/arm/dts/u-boot.dtsi | 17 +
>  arch/sandbox/dts/u-boot.dtsi | 17 +
>  lib/efi_loader/Kconfig   | 10 ++
>  lib/efi_loader/Makefile  |  7 +++
>  4 files changed, 51 insertions(+)
>  create mode 100644 arch/arm/dts/u-boot.dtsi
>  create mode 100644 arch/sandbox/dts/u-boot.dtsi

This approach seems OK to me for now. It is a bit strange to specify a
CONFIG option to add something to the DT, but we can always adjust it
later if needed.

>
> diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> new file mode 100644
> index 00..2a9359c43c
> --- /dev/null
> +++ b/arch/arm/dts/u-boot.dtsi
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*

/**

for multi-line comments

> + * Devicetree file with miscellaneous nodes that will be included
> + * at build time into the DTB. Currently being used for including
> + * capsule related information.
> + *

drop blank line

> + */
> +
> +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE

Can you combine these, or can you omit the first one?

> +/ {
> +   signature {
> +   capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> +   };
> +};
> +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> new file mode 100644
> index 00..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..9abb9a4db3 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -235,6 +235,16 @@ 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.

Can you wrap to 72 chars or so?

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

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

2023-07-15 Thread Sughosh Ganu 
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 
---
Changes since V3:
* Put the two ifdef statements together in arm architecture's
  u-boot.dtsi file.
* Remove the extra blank line in the Kconfig.

 arch/arm/dts/u-boot.dtsi | 17 +
 arch/sandbox/dts/u-boot.dtsi | 17 +
 lib/efi_loader/Kconfig   | 10 ++
 lib/efi_loader/Makefile  |  7 +++
 4 files changed, 51 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 00..2a9359c43c
--- /dev/null
+++ b/arch/arm/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 /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
new file mode 100644
index 00..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..9abb9a4db3 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -235,6 +235,16 @@ 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