[PATCH v6 0/9] Integrate EFI capsule tasks into u-boot's build flow

This patchset aims to bring two capsule related tasks under the u-boot build flow.
One is the embedding of the public key into the platform's dtb. The public key is in the form of an EFI Signature List(ESL) file and is used for capsule authentication. This is being achieved by adding the signature node containing the capsule public key in the architecture's u-boot.dtsi file. Currently, the u-boot.dtsi file has been added for the sandbox and arm architectures. The path to the ESL file is being provided through a Kconfig symbol(CONFIG_EFI_CAPSULE_ESL_FILE).
Changes have also been made to the test flow so that the keys used for signing the capsule, and the ESL file, are generated prior to invoking the u-boot's build, which enables embedding the ESL file into the dtb as part of the u-boot build.
The other task is related to generation of capsules. The capsules can be generated as part of u-boot build, and this is being achieved through binman, by adding a capsule entry type. The capsules can be generated by specifying the capsule parameters as properties under the capsule entry node.
Changes have also been made to the efi capsule update feature testing setup on the sandbox variants. Currently, the capsule files and the public key ESL file are generated after u-boot has been built. This logic has been changed so that the capsule input files along with the keys needed for capsule signing and authentication are generated prior to initiation of the u-boot build. The placement of all the files needed for generation of capsules is under the test/py/tests/test_efi_capsule/test_files/ directory.
The document has been updated to reflect the above changes.
Changes since V5: This series drops the changes for generating capsules by reading the params from a config file. This was suggested by Simon Glass. The config file changes would be submitted separately once these changes get merged.
* Get rid of the logic of keeping the files under the /tmp/capsules/ directory from earlier versions. * New patch which introduces the input files and certs needed for EFI capsule update testing in the tree. * The capsule input files and certs are put under the test/py/tests/test_efi_capsule/test_files/ directory. * Add support for the oemflag parameter used in FWU A/B updates. This was missed in the earlier version. * Use a single function, generate_capsule in the mkeficapsule bintool, instead of the multiple functions in earlier version. * Remove the logic for generating capsules from config file as suggested by Simon. * Use required_props for image index and GUID parameters. * Use a subnode for the capsule payload instead of using a filename for the payload, as suggested by Simon. * Add a capsule generation test with oemflag parameter being passed. * Remove the documentation for generating the capsule through config file, as that functionality is not added through this series. * Use the public key ESL file from the tree instead of the /tmp/capsules/ directory being used in previous version. * Use the public key ESL file and other input files from the tree instead of the /tmp/capsules/ directory being used in previous version. * Use macros for other input files and certs.
Sughosh Ganu (9): binman: bintool: Build a tool from a list of commands nuvoton: npcm845-evb: Add a newline at the end of file capsule: authenticate: Add capsule public key in platform's dtb doc: capsule: Document the new mechanism to embed ESL file into dtb test: capsule: Add files needed for testing EFI capsule updates binman: capsule: Add support for generating EFI capsules doc: Add documentation to highlight capsule generation related updates test: capsule: Remove public key embed logic from capsule update test sandbox: capsule: Generate capsule related files through binman
arch/arm/dts/nuvoton-npcm845-evb.dts | 2 +- arch/arm/dts/u-boot.dtsi | 14 + arch/sandbox/dts/u-boot.dtsi | 364 ++++++++++++++++++ configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + doc/develop/uefi/uefi.rst | 40 +- lib/efi_loader/Kconfig | 9 + test/py/tests/test_efi_capsule/conftest.py | 165 +------- test/py/tests/test_efi_capsule/signature.dts | 10 - .../test_efi_capsule/test_files/SIGNER.crt | 19 + .../test_efi_capsule/test_files/SIGNER.esl | Bin 0 -> 829 bytes .../test_efi_capsule/test_files/SIGNER.key | 28 ++ .../test_efi_capsule/test_files/SIGNER2.crt | 19 + .../test_efi_capsule/test_files/SIGNER2.key | 28 ++ .../test_files/u-boot.bin.new | 1 + .../test_files/u-boot.bin.old | 1 + .../test_files/u-boot.env.new | 1 + .../test_files/u-boot.env.old | 1 + .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- tools/binman/bintool.py | 19 +- tools/binman/btool/mkeficapsule.py | 101 +++++ tools/binman/entries.rst | 64 +++ tools/binman/etype/efi_capsule.py | 160 ++++++++ tools/binman/ftest.py | 122 ++++++ tools/binman/test/307_capsule.dts | 21 + tools/binman/test/308_capsule_signed.dts | 23 ++ tools/binman/test/309_capsule_version.dts | 22 ++ tools/binman/test/310_capsule_signed_ver.dts | 24 ++ tools/binman/test/311_capsule_oemflags.dts | 22 ++ tools/binman/test/312_capsule_missing_key.dts | 22 ++ .../binman/test/313_capsule_missing_index.dts | 20 + .../binman/test/314_capsule_missing_guid.dts | 19 + .../test/315_capsule_missing_payload.dts | 17 + 34 files changed, 1172 insertions(+), 225 deletions(-) create mode 100644 arch/arm/dts/u-boot.dtsi create mode 100644 arch/sandbox/dts/u-boot.dtsi delete mode 100644 test/py/tests/test_efi_capsule/signature.dts create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER.crt create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER.esl create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER.key create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER2.crt create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER2.key create mode 100644 test/py/tests/test_efi_capsule/test_files/u-boot.bin.new create mode 100644 test/py/tests/test_efi_capsule/test_files/u-boot.bin.old create mode 100644 test/py/tests/test_efi_capsule/test_files/u-boot.env.new create mode 100644 test/py/tests/test_efi_capsule/test_files/u-boot.env.old delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its create mode 100644 tools/binman/btool/mkeficapsule.py create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/307_capsule.dts create mode 100644 tools/binman/test/308_capsule_signed.dts create mode 100644 tools/binman/test/309_capsule_version.dts create mode 100644 tools/binman/test/310_capsule_signed_ver.dts create mode 100644 tools/binman/test/311_capsule_oemflags.dts create mode 100644 tools/binman/test/312_capsule_missing_key.dts create mode 100644 tools/binman/test/313_capsule_missing_index.dts create mode 100644 tools/binman/test/314_capsule_missing_guid.dts create mode 100644 tools/binman/test/315_capsule_missing_payload.dts

Add support to build a tool from source with a list of commands. This is useful when a tool can be built with multiple commands instead of a single command.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Simon Glass sjg@chromium.org --- Changes since V5: None
tools/binman/bintool.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 0b0f56dbbb..3c4ad1adbb 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -328,7 +328,7 @@ class Bintool: return result.stdout
@classmethod - def build_from_git(cls, git_repo, make_target, bintool_path, flags=None): + def build_from_git(cls, git_repo, make_targets, bintool_path, flags=None): """Build a bintool from a git repo
This clones the repo in a temporary directory, builds it with 'make', @@ -336,7 +336,8 @@ class Bintool:
Args: git_repo (str): URL of git repo - make_target (str): Target to pass to 'make' to build the tool + make_targets (list of str): List of targets to pass to 'make' to build + the tool bintool_path (str): Relative path of the tool in the repo, after build is complete flags (list of str): Flags or variables to pass to make, or None @@ -350,12 +351,14 @@ class Bintool: tmpdir = tempfile.mkdtemp(prefix='binmanf.') print(f"- clone git repo '{git_repo}' to '{tmpdir}'") tools.run('git', 'clone', '--depth', '1', git_repo, tmpdir) - print(f"- build target '{make_target}'") - cmd = ['make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}', - make_target] - if flags: - cmd += flags - tools.run(*cmd) + for target in make_targets: + print(f"- build target '{target}'") + cmd = ['make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}', + target] + if flags: + cmd += flags + tools.run(*cmd) + fname = os.path.join(tmpdir, bintool_path) if not os.path.exists(fname): print(f"- File '{fname}' was not produced")

Add a newline at the end of the dts, without which the build fails when including the u-boot.dtsi file.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- Changes since V5: None
arch/arm/dts/nuvoton-npcm845-evb.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/nuvoton-npcm845-evb.dts b/arch/arm/dts/nuvoton-npcm845-evb.dts index 3cab7807e3..a93666cb41 100644 --- a/arch/arm/dts/nuvoton-npcm845-evb.dts +++ b/arch/arm/dts/nuvoton-npcm845-evb.dts @@ -354,4 +354,4 @@ &r1en_pins &r1oen_pins >; -}; \ No newline at end of file +};

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 */ 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

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

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

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_GV2oS6...
Regards, Simon

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_GV2oS6...
Regards, Simon

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.

Update the document to specify how the EFI Signature List(ESL) file can be embedded into the platform's dtb as part of the u-boot build.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Simon Glass sjg@chromium.org --- Changes since V5: None
doc/develop/uefi/uefi.rst | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index a7a41f2fac..b2854b52a6 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -522,20 +522,16 @@ and used by the steps highlighted below. ... }
-You can do step-4 manually with +You can perform step-4 by defining the Kconfig symbol +CONFIG_EFI_CAPSULE_ESL_FILE. Once this has been done, the signature +node can be added to the u-boot.dtsi file. For reference, check the +u-boot.dtsi file for the sandbox architecture. If this node has not +been added to the architecture's u-boot.dtsi file, this needs to be +done. The node has currently been added for the sandbox and arm +architectures' in the u-boot.dtsi file. Once the u-boot.dtsi file has +been added with the signature node, the esl file will automatically +get embedded into the platform's dtb as part of u-boot build.
-.. code-block:: console - - $ dtc -@ -I dts -O dtb -o signature.dtbo signature.dts - $ fdtoverlay -i orig.dtb -o new.dtb -v signature.dtbo - -where signature.dts looks like:: - - &{/} { - signature { - capsule-key = /incbin/("CRT.esl"); - }; - };
Anti-rollback Protection ************************

Add the files that would be needed for testing the EFI capsule update functionality. These include the keys needed for signing and authenticating the capsules with the capsule authentication functionality enabled. This includes the public key in form of the EFI Signature List(ESL) file which will be embedded in the platform's DTB.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V5: * Get rid of the logic of keeping the files under the /tmp/capsules/ directory from earlier versions. * New patch which introduces the input files and certs needed for EFI capsule update testing in the tree. * The capsule input files and certs are put under the test/py/tests/test_efi_capsule/test_files/ directory.
.../test_efi_capsule/test_files/SIGNER.crt | 19 ++++++++++++ .../test_efi_capsule/test_files/SIGNER.esl | Bin 0 -> 829 bytes .../test_efi_capsule/test_files/SIGNER.key | 28 ++++++++++++++++++ .../test_efi_capsule/test_files/SIGNER2.crt | 19 ++++++++++++ .../test_efi_capsule/test_files/SIGNER2.key | 28 ++++++++++++++++++ .../test_files/u-boot.bin.new | 1 + .../test_files/u-boot.bin.old | 1 + .../test_files/u-boot.env.new | 1 + .../test_files/u-boot.env.old | 1 + 9 files changed, 98 insertions(+) create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER.crt create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER.esl create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER.key create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER2.crt create mode 100644 test/py/tests/test_efi_capsule/test_files/SIGNER2.key create mode 100644 test/py/tests/test_efi_capsule/test_files/u-boot.bin.new create mode 100644 test/py/tests/test_efi_capsule/test_files/u-boot.bin.old create mode 100644 test/py/tests/test_efi_capsule/test_files/u-boot.env.new create mode 100644 test/py/tests/test_efi_capsule/test_files/u-boot.env.old
diff --git a/test/py/tests/test_efi_capsule/test_files/SIGNER.crt b/test/py/tests/test_efi_capsule/test_files/SIGNER.crt new file mode 100644 index 0000000000..722a4e2483 --- /dev/null +++ b/test/py/tests/test_efi_capsule/test_files/SIGNER.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDDTCCAfWgAwIBAgIUGGjxXEUS+sBJaSOBz4u0MJRWdcowDQYJKoZIhvcNAQEL +BQAwFjEUMBIGA1UEAwwLVEVTVF9TSUdORVIwHhcNMjMwNzI3MDY1NjQzWhcNMzMw +NzI0MDY1NjQzWjAWMRQwEgYDVQQDDAtURVNUX1NJR05FUjCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAMUEYpFf8i/zLX6/4bhUAIrLWCxaLCqGscZ85DSU +SagMu+9DpCDrJSzgZQFj2+YSc4JSoBDs9u/JN+HNH+hK255Slzf8+Pl2YeRjTyCA +7k6u0s2nFpLJkMPBzqyrEYP+fNrGsTtIlvutef2MPs8WfgyzB5CSRx/K40PirQHE +Lt5HfLJ8WOvPAbdZ4z+PDm5LrZReewJOYHVKQepAY8z3Dsy3ZBnXGI/1ZYgMfTU0 +sBCfTtEBJb+ja+eKepw93IuxPLdN1ZXW1YUiBTs7h+BUAJr+Qjt/zvWl2ms1+sQf +dHtsMa+WmTLu0XHCGOEfgX/fdTWv1GaelMTxl9Lzqug/+8cCAwEAAaNTMFEwHQYD +VR0OBBYEFLH9hYGrnXfQ/CfAMaMAh64xJxTCMB8GA1UdIwQYMBaAFLH9hYGrnXfQ +/CfAMaMAh64xJxTCMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEB +ACSmt+2O8mNT5K232P2BOPt3r2v3M+HZFsjb54s+wSiY5tS1KBFJbMehGKhwNZRz +uEIp1RJnsh9Kg8Cnzh2Hpgwnycx2sipbTXN0Frz8QuC3zbAqhrcalaKFOuSXbY6O +mBBJDJENj+d57yzHxT3XvSmAu62UMf2hwJqIqQfA7+wrV2VGEbrY4O9KY6O4Mj2f +vS9WjN3840xQCqsohwbU9u9GpHYb6eFQ+jiit/yqYVlJCSDN812Fv1TYOPzLqG8L +VCCqqpPRJX0E13kPAafoNK5UA2OKglgosvufWzwJ3Mn0Al8BQnv5rRnBoTxJLDef +n+uX7jeUW6LfIH5s8cMrA2Y= +-----END CERTIFICATE----- diff --git a/test/py/tests/test_efi_capsule/test_files/SIGNER.esl b/test/py/tests/test_efi_capsule/test_files/SIGNER.esl new file mode 100644 index 0000000000000000000000000000000000000000..3d584cd44a22e0b9c5414f54a3a5bc24504f0a71 GIT binary patch literal 829 zcmZ1&d0^?2Da*aux2_hA(f&|m%gg`<iclKDgOCPI%)ACoj9(WpGchtTiAZF8jByqE zb-*)Ix$%7W7K15arKb#d**LY@JlekVGBR?rG8l*%iWms7F^94+^Kgf_28YB4d%F9% z1{uhS^BNf&m>ZcJn3<ZH7)Oco8iTkdP%ch=O^iy&Rxq+MFgG#sGXP!1#ni;e$Z(V; zX=3~*{m;5}`ycKIVdy#?p%bN})wc0i%@dO;o-25Ezjt1u@LE;pK`LYN?Po&8O+gC; z-h6w1(){6B`4?Wd=LJnS|MTN#S>lsqe}#s3e(NrsT`o51<b=Zq&#hT4*!-{N*0GJ& z9@BoWt^C_#cV4WHXEXbRN$&Ee9y>o;%XmcZo_o!vnuyou8MjA1w(sZ5^Ikh8uA0d& zq14Oql|%BG?|f&rr$}Cx=>MA9!BcB$vO!?J-$h2%{fo1ocU8@?z0<wXX1njzsn@Qy zDzREyw?7DBnDx)ey8hhPrMI$8e;tu8sm?K6KW(PbyNiW~Bp%8)*55BRU4JEQ-jpLB zr(gQK>V^I9<4nwq42+9|4FU~hfuSzT$0Eievhi<g<LbHP7yhUpFkH;gzRplx<dA_p zNLrai!a%G6y8?cY0%1nR|17Kq%s>h`*nu$%40c8am1W!C_I*kYezJD^jlYc+zsuKW ze>Z-3Q|!d;=iPP(HD)}!vQ<OSGw1k1i4_H=Q;K&uX<ijd-z4wVd|>%G+4f~T>L<^X zZPJSNEiMt;^T+AI_Olzb+O|tgUDRszWO{C2-wXjyo{7Bu&nw^S96xG%eXnN2?zK}4 z|1LZ*t79emf%k8;!&BV^cinjK-Ya?W4kO$7d-cP5?*4h~6Tr1vqn+)_xA$&K%A{XD z4ESZSX#1a4iIJY13THpZw(bwPVe#kmihS-6g;lF2UsSDSxn9Z7xcr65x)A2%uBHf$ wO~2<y+i>1F`GqN-(W(09TFHY8Z9H|%=g)sV{hj%g=tcJx>T*6F)@Duv0DX*DtN;K2
literal 0 HcmV?d00001
diff --git a/test/py/tests/test_efi_capsule/test_files/SIGNER.key b/test/py/tests/test_efi_capsule/test_files/SIGNER.key new file mode 100644 index 0000000000..e124cd35f0 --- /dev/null +++ b/test/py/tests/test_efi_capsule/test_files/SIGNER.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDFBGKRX/Iv8y1+ +v+G4VACKy1gsWiwqhrHGfOQ0lEmoDLvvQ6Qg6yUs4GUBY9vmEnOCUqAQ7PbvyTfh +zR/oStueUpc3/Pj5dmHkY08ggO5OrtLNpxaSyZDDwc6sqxGD/nzaxrE7SJb7rXn9 +jD7PFn4MsweQkkcfyuND4q0BxC7eR3yyfFjrzwG3WeM/jw5uS62UXnsCTmB1SkHq +QGPM9w7Mt2QZ1xiP9WWIDH01NLAQn07RASW/o2vninqcPdyLsTy3TdWV1tWFIgU7 +O4fgVACa/kI7f871pdprNfrEH3R7bDGvlpky7tFxwhjhH4F/33U1r9RmnpTE8ZfS +86roP/vHAgMBAAECggEAMvsIAIM52cOM1bwUTgzamQ+2UL/Cpvx0ux5tNNfcWXJ2 +HRs9ONFwLLUiHeJ3sAi9QA9eYRLYcUL5xWG9bHAWdVj8zV2WFYNXIHC8NHZ4c/7U +CKhAdJpY7fbUIqUfoq6zIy+ABA2sGBMTOpNUW2UAGAwpnHTll6n59gKNbyQTVqvn +swoHqutRaveshZeiCvOTEjoMUaBkMG5FWmsOI8qRrzJZx+K6A4hJxMQOnNN75wa/ +RtTf91Howw9/mxDzHjT42LTfWKPrJ0H3zoIG+4cgS7cRftNKi8L5OZnw6LvyJb6l +wHJWPFklCDTQMq/NNMldgqtN5JuRDyUi8a24LcKI9QKBgQDiKkTr2wrzMXIEud14 +7x/0OY8fOwstKdkaeZ9VxsQg7AurVheEbjNYZXiragakg2vk5AWFJrwyv1RTlBnT +IvvFIIshNiy3Oz6WJBG4WOwsGVrr47T2T3atiB/N8NBfb2jwHahMzjXZIvCA+FaE +XJ9xfqH3uNAEamdI7Li7fFs8iwKBgQDfAcVeF2KxLLYoyfgWFoabchXx+FumjYgO +tdv46kyKFWofss+W0KMIXx8KjDETbm0RL0IY1NW9wghTuLi581ie0eMTkwnlXDSG +5Y+6sEKo3+9qkp+ZX/V62PhSwCt3O959nJZJsjICRfW73/Blz01LUiRxuEithdq2 +xoTvt1S5NQKBgH2kbdV5QoQIHAd0Gg1tCptqvo/jBTp53RpQJqxIV/zSJUlx8m6n +qe6ZsIfJxxbty6rE4iwucK7gi8BCrnYVITlJ8wDoT78bMpHGR/HZtJprG4+gWI/d +ZVjSHpkSBzB9fBao4y6IAHI4btO3Ipk1u34Zk3FDQoyxb9+bYqUFWMoxAoGBAJd0 +H3PYlAlaIViwWlG9+KtHnwnXr3787iN3dS6nCVZaVtmyWfPGPIMp/u3t6kKVI3Oh +UdWFbqhSR898S9DWKSCr0PlxSi5AIdhfve5/WLZSZ8pMTCIhHpnRE004AA0ZVvCe +UR8562bJ1qtC2oR6drcp0WB+VLWsi67IQm5/ZwXlAoGBAM09vlTKsVeKpMGly34m +GkzapC921p7SHddAALhTt3vfUFnVkIYrmyWCtHmuTTQljm5ZQlS/YE4664j5VaJb +7yeQKFhPi9B848+WVdnEJspmz11BZorS0TMhYn3/eArXKalLMhhRq0HqWJjc+8vu +M1o40Gn1NdsvzQgiwi0JwAwS +-----END PRIVATE KEY----- diff --git a/test/py/tests/test_efi_capsule/test_files/SIGNER2.crt b/test/py/tests/test_efi_capsule/test_files/SIGNER2.crt new file mode 100644 index 0000000000..25da91a873 --- /dev/null +++ b/test/py/tests/test_efi_capsule/test_files/SIGNER2.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDDTCCAfWgAwIBAgIUA+466jn5wSD1ufVxtNZUYbC2eyUwDQYJKoZIhvcNAQEL +BQAwFjEUMBIGA1UEAwwLVEVTVF9TSUdORVIwHhcNMjMwNzI3MDY1NjU5WhcNMzMw +NzI0MDY1NjU5WjAWMRQwEgYDVQQDDAtURVNUX1NJR05FUjCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAJvgRltjaxviomhFdY2V0hrLw9MxsxvuCGhEW7H0 +Xh7wg/7qufBVO7hY/f0H3+Q1TEKRH2/j6sio2l+2CC+Pf4e4KxWJl1pLe3RgjtMB +zfuRkEqoF4rQyYRBL47P8fqSCvSdap+olj2O7K0iiYgrsJjLeq7zpOFRTu3Mxy6a +ePw3by8OqLVLlkpEXuC2nZIpUCvaBtp2hi0qbbnaPOnEn4HH9d7l8C9NvIGd7IXf +knW0+NJna7aMgjfI+Em+ZIfHed+s2mXgG5dzgMK+iPWjuePFGTRhXsJAG1jw4lVn +haYJ0LgGdwSSoxx/cES/kGzRKik0zFACT9ke1u+jLQC17KMCAwEAAaNTMFEwHQYD +VR0OBBYEFK8pSvdztiocjuXQ6M37rgtQQjTfMB8GA1UdIwQYMBaAFK8pSvdztioc +juXQ6M37rgtQQjTfMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEB +ABWO4IiahT5yukHgOwHIr7yj0OeBuH8dVlM8KCWUk1Vr6+vJNkHClvWbsiLuT7ns +o1M27aCeagHWdtB1OmtaHKiFz+UpxIeVV1Ti6+1oshqg71xVCr6BY8+PzZiEj0r7 +auC1PEr4UtizmVAAyodf8lUaHGmU6zLzcCr9RtPhr9/5gq86V0IsXNn3p74CVf3k +E8++FpNNBm43tmsWWKV4n4GyJHPNm1W/8P6HnFx4RKHuyNyUngA5axUFX/CvJ4jC +RmE4Rxb2lncOumXx0/N4iC9SpfL78IcVxOyIpErnx3GLSvsEt5+TfLnLd9Y4IiOV +mFrCCJRGrqGwmlZoDwKf2ZA= +-----END CERTIFICATE----- diff --git a/test/py/tests/test_efi_capsule/test_files/SIGNER2.key b/test/py/tests/test_efi_capsule/test_files/SIGNER2.key new file mode 100644 index 0000000000..73b7d7d26f --- /dev/null +++ b/test/py/tests/test_efi_capsule/test_files/SIGNER2.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQCb4EZbY2sb4qJo +RXWNldIay8PTMbMb7ghoRFux9F4e8IP+6rnwVTu4WP39B9/kNUxCkR9v4+rIqNpf +tggvj3+HuCsViZdaS3t0YI7TAc37kZBKqBeK0MmEQS+Oz/H6kgr0nWqfqJY9juyt +IomIK7CYy3qu86ThUU7tzMcumnj8N28vDqi1S5ZKRF7gtp2SKVAr2gbadoYtKm25 +2jzpxJ+Bx/Xe5fAvTbyBneyF35J1tPjSZ2u2jII3yPhJvmSHx3nfrNpl4BuXc4DC +voj1o7njxRk0YV7CQBtY8OJVZ4WmCdC4BncEkqMcf3BEv5Bs0SopNMxQAk/ZHtbv +oy0AteyjAgMBAAECggEAA2Rypr63qbz/2gzk8MAzErIFLYP8r2qgFGpxPaK69HZs +kPz/ySfs64ER6gW+JpCblV+xYCiE+nmX4xLEA9deKBCAhr2RvzChKH7veZuMJZT2 +uUE5d4pxHRrXRtOzIvs2fmz5avpRM4uR20TDDhhRUn0r/g6MZvqNIRmRt8U7eKKD +524QheUKxgnz6GjWbAowHpi5XCnpl8nw4AjjOoxnquGkxgjSGo3BgRBWin4Uj051 +UVpQejTN05QKacoNpvC+hrsAcU1N75Lm1vPlkbK2EIk/flcm//WiK4TvKtNmHnxl +L9ZL/LNSwX/9Hczox+b5Z+JR7I/l8/jA+TVOO4KUXQKBgQDU2u02wiszmh9dkgB7 +tUTmPmOOmKbuHslv5KBT3zVtJKjVkJHzKKkQqnQ2NcWUYz2QhZrPz26UblDigy3e +Gf93GXJHJCgfLwNzrJrGE4zttBX4qmST1huueerTVuncVsTJhHhOiw8yV3RKNslc +PGHqTlpCw0lpA5hYXQxxerx/bwKBgQC7eLKMrrHq/iBP6S+1w0u7nTmYmj+QkkbP +o0Oghc5Roe8s/+BEbgi5VZEhemcEqV/fXl13WxqMdIrg8p3DgkLf4wyT9L82Yrkr +yWKc9XWEy+NQ7isAv3r3w/TdxLh4fgp8IVvBF3DV8QJsN6I1Q5f9vW5uilTDm0Ng +cZ7/3lnMDQKBgQCN8PwU8wCKJbHa3PzTgfrTKzGKqsNOsVsU8bn0lDl1cefgmsqp +AylSwshCSjNak58/W8jz4VjVRIdNtbqFjIKuMlrhk/vpZ5l+rtB7XBgzf07ThxUQ +/Mty2zw7+I5076vE0kDD57mXkXgr8ULv7hhBfkR0lvPCQrJ54nrkxbsjowKBgQC5 +g84Eg1dS+NlW2qW406Lc7NAzH+jpEqd6D9D7R44MoBeDy03NyaleZbtxiqPpLAbQ +jpwlYYUbGrTXt57A+uVckl0/CMIzemxNVUL9mbUKjYzL6HOrkNCJ4GMvFd2Kdwe/ +IG+g26ZwP8gq+L7OwK3mjY97We5ZhwqcpLM39nub/QKBgQCTYW0YizRU8cNnC1Qv ++5SpSNjS04OLjKto1hOPzHnJMj5A2lp9ZObzjrMbyAxbzQMcwUe/V4PKiaNwypPE +uLWkoH9QrS9qV4b5qbtCn3PAyJODscFkn+VFxgGizZvd48Ze5V2U4JwS13QMYMCe +VHiogc9HvLI5XzE/YAfE4C7hKA== +-----END PRIVATE KEY----- diff --git a/test/py/tests/test_efi_capsule/test_files/u-boot.bin.new b/test/py/tests/test_efi_capsule/test_files/u-boot.bin.new new file mode 100644 index 0000000000..798bfcb5e9 --- /dev/null +++ b/test/py/tests/test_efi_capsule/test_files/u-boot.bin.new @@ -0,0 +1 @@ +u-boot:New \ No newline at end of file diff --git a/test/py/tests/test_efi_capsule/test_files/u-boot.bin.old b/test/py/tests/test_efi_capsule/test_files/u-boot.bin.old new file mode 100644 index 0000000000..cd6427b0f7 --- /dev/null +++ b/test/py/tests/test_efi_capsule/test_files/u-boot.bin.old @@ -0,0 +1 @@ +u-boot:Old \ No newline at end of file diff --git a/test/py/tests/test_efi_capsule/test_files/u-boot.env.new b/test/py/tests/test_efi_capsule/test_files/u-boot.env.new new file mode 100644 index 0000000000..b2c4bd4cee --- /dev/null +++ b/test/py/tests/test_efi_capsule/test_files/u-boot.env.new @@ -0,0 +1 @@ +u-boot-env:New \ No newline at end of file diff --git a/test/py/tests/test_efi_capsule/test_files/u-boot.env.old b/test/py/tests/test_efi_capsule/test_files/u-boot.env.old new file mode 100644 index 0000000000..04ad4c0ad4 --- /dev/null +++ b/test/py/tests/test_efi_capsule/test_files/u-boot.env.old @@ -0,0 +1 @@ +u-boot-env:Old \ No newline at end of file

Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V5: * Add support for the oemflag parameter used in FWU A/B updates. This was missed in the earlier version. * Use a single function, generate_capsule in the mkeficapsule bintool, instead of the multiple functions in earlier version. * Remove the logic for generating capsules from config file as suggested by Simon. * Use required_props for image index and GUID parameters. * Use a subnode for the capsule payload instead of using a filename for the payload, as suggested by Simon. * Add a capsule generation test with oemflag parameter being passed.
configs/sandbox_spl_defconfig | 1 + tools/binman/btool/mkeficapsule.py | 101 +++++++++++ tools/binman/entries.rst | 64 +++++++ tools/binman/etype/efi_capsule.py | 160 ++++++++++++++++++ tools/binman/ftest.py | 122 +++++++++++++ tools/binman/test/307_capsule.dts | 21 +++ tools/binman/test/308_capsule_signed.dts | 23 +++ tools/binman/test/309_capsule_version.dts | 22 +++ tools/binman/test/310_capsule_signed_ver.dts | 24 +++ tools/binman/test/311_capsule_oemflags.dts | 22 +++ tools/binman/test/312_capsule_missing_key.dts | 22 +++ .../binman/test/313_capsule_missing_index.dts | 20 +++ .../binman/test/314_capsule_missing_guid.dts | 19 +++ .../test/315_capsule_missing_payload.dts | 17 ++ 14 files changed, 638 insertions(+) create mode 100644 tools/binman/btool/mkeficapsule.py create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/307_capsule.dts create mode 100644 tools/binman/test/308_capsule_signed.dts create mode 100644 tools/binman/test/309_capsule_version.dts create mode 100644 tools/binman/test/310_capsule_signed_ver.dts create mode 100644 tools/binman/test/311_capsule_oemflags.dts create mode 100644 tools/binman/test/312_capsule_missing_key.dts create mode 100644 tools/binman/test/313_capsule_missing_index.dts create mode 100644 tools/binman/test/314_capsule_missing_guid.dts create mode 100644 tools/binman/test/315_capsule_missing_payload.dts
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 8d50162b27..65223475ab 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -249,3 +249,4 @@ CONFIG_UNIT_TEST=y CONFIG_SPL_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y +CONFIG_TOOLS_MKEFICAPSULE=y diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py new file mode 100644 index 0000000000..da1d5de873 --- /dev/null +++ b/tools/binman/btool/mkeficapsule.py @@ -0,0 +1,101 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2023 Linaro Limited +# +"""Bintool implementation for mkeficapsule tool + +mkeficapsule is a tool used for generating EFI capsules. + +The following are the command-line options to be provided +to the tool +Usage: mkeficapsule [options] <image blob> <output file> +Options: + -g, --guid <guid string> guid for image blob type + -i, --index <index> update image index + -I, --instance <instance> update hardware instance + -v, --fw-version <version> firmware version + -p, --private-key <privkey file> private key file + -c, --certificate <cert file> signer's certificate file + -m, --monotonic-count <count> monotonic count + -d, --dump_sig dump signature (*.p7) + -A, --fw-accept firmware accept capsule, requires GUID, no image blob + -R, --fw-revert firmware revert capsule, takes no GUID, no image blob + -o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff + -h, --help print a help message +""" + +from binman import bintool + +class Bintoolmkeficapsule(bintool.Bintool): + """Handles the 'mkeficapsule' tool + + This bintool is used for generating the EFI capsules. The + capsule generation parameters can either be specified through + command-line, or through a config file. + """ + def __init__(self, name): + super().__init__(name, 'mkeficapsule tool for generating capsules') + + def generate_capsule(self, image_index, image_guid, hardware_instance, + payload, output_fname, priv_key, pub_key, + monotonic_count=0, version=0, oemflags=0): + """Generate a capsule through commandline provided parameters + + Args: + image_index (int): Unique number for identifying payload image + image_guid (str): GUID used for identifying the image + hardware_instance (int): Optional unique hardware instance of + a device in the system. 0 if not being used + payload (str): Path to the input payload image + output_fname (str): Path to the output capsule file + priv_key (str): Path to the private key + pub_key(str): Path to the public key + monotonic_count (int): Count used when signing an image + version (int): Image version (Optional) + oemflags (int): Optional 16 bit OEM flags + + Returns: + str: Tool output + """ + args = [ + f'--index={image_index}', + f'--guid={image_guid}', + f'--instance={hardware_instance}' + ] + + if version: + args += [f'--fw-version={version}'] + if oemflags: + args += [f'--capoemflag={oemflags}'] + if priv_key and pub_key: + args += [ + f'--monotonic-count={monotonic_count}', + f'--private-key={priv_key}', + f'--certificate={pub_key}' + ] + + args += [ + payload, + output_fname + ] + + return self.run_cmd(*args) + + def fetch(self, method): + """Fetch handler for mkeficapsule + + This builds the tool from source + + Returns: + tuple: + str: Filename of fetched file to copy to a suitable directory + str: Name of temp directory to remove, or None + """ + if method != bintool.FETCH_BUILD: + return None + + cmd = ['tools-only_defconfig', 'tools'] + result = self.build_from_git( + 'https://source.denx.de/u-boot/u-boot.git', + cmd, + 'tools/mkeficapsule') + return result diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index f2376932be..cfe699361c 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -468,6 +468,70 @@ updating the EC on startup via software sync.
+.. _etype_efi_capsule: + +Entry: capsule: Entry for generating EFI Capsule files +------------------------------------------------------ + +This is an entry for generating EFI capsules. + + This is an entry for generating EFI capsules. + + The parameters needed for generation of the capsules can + either be provided as properties in the entry. + +Properties / Entry arguments: + - image-index: Unique number for identifying corresponding + payload image. Number between 1 and descriptor count, i.e. + the total number of firmware images that can be updated. + - image-type-id: Image GUID which will be used for identifying the + updatable image on the board. + - hardware-instance: Optional number for identifying unique + hardware instance of a device in the system. Default value of 0 + for images where value is not to be used. + - fw-version: Optional value of image version that can be put on + the capsule through the Firmware Management Protocol(FMP) header. + - monotonic-count: Count used when signing an image. + - private-key: Path to PEM formatted .key private key file. + - pub-key-cert: Path to PEM formatted .crt public key certificate + file. + - capoemflags - Optional OEM flags to be passed through capsule header. + - filename: Optional path to the output capsule file. A capsule is a + continuous set of data as defined by the EFI specification. Refer + to the specification for more details. + + For more details on the description of the capsule format, and the capsule + update functionality, refer Section 8.5 and Chapter 23 in the UEFI + specification. + https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf + + The capsule parameters like image index and image GUID are passed as + properties in the entry. The payload to be used in the capsule is to be + provided as a subnode of the capsule entry. + +A typical capsule entry node would then look something like this:: + + capsule { + type = "efi-capsule"; + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + private-key = "tools/binman/test/key.key"; + pub-key-cert = "tools/binman/test/key.pem"; + filename = "test.capsule"; + + u-boot { + }; + }; + +In the above example, the capsule payload is the u-boot image. The +capsule entry would read the contents of the payload and put them +into the capsule. Any external file can also be specified as the +payload using the blob-ext subnode. + + + .. _etype_encrypted:
Entry: encrypted: Externally built encrypted binary blob diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py new file mode 100644 index 0000000000..b8a269e247 --- /dev/null +++ b/tools/binman/etype/efi_capsule.py @@ -0,0 +1,160 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2023 Linaro Limited +# +# Entry-type module for producing a EFI capsule +# + +from collections import OrderedDict +import os + +from binman.entry import Entry +from binman.etype.section import Entry_section +from dtoc import fdt_util +from u_boot_pylib import tools + +class Entry_efi_capsule(Entry_section): + """Entry for generating EFI capsules + + This is an entry for generating EFI capsules. + + The parameters needed for generation of the capsules can + either be provided as properties in the entry. + + Properties / Entry arguments: + - image-index: Unique number for identifying corresponding + payload image. Number between 1 and descriptor count, i.e. + the total number of firmware images that can be updated. + - image-type-id: Image GUID which will be used for identifying the + updatable image on the board. + - hardware-instance: Optional number for identifying unique + hardware instance of a device in the system. Default value of 0 + for images where value is not to be used. + - fw-version: Optional value of image version that can be put on + the capsule through the Firmware Management Protocol(FMP) header. + - monotonic-count: Count used when signing an image. + - private-key: Path to PEM formatted .key private key file. + - pub-key-cert: Path to PEM formatted .crt public key certificate + file. + - capoemflags - Optional OEM flags to be passed through capsule header. + - filename: Optional path to the output capsule file. A capsule is a + continuous set of data as defined by the EFI specification. Refer + to the specification for more details. + + For more details on the description of the capsule format, and the capsule + update functionality, refer Section 8.5 and Chapter 23 in the UEFI + specification. + https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf + + The capsule parameters like image index and image GUID are passed as + properties in the entry. The payload to be used in the capsule is to be + provided as a subnode of the capsule entry. + + A typical capsule entry node would then look something like this + + capsule { + type = "efi-capsule"; + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + private-key = "tools/binman/test/key.key"; + pub-key-cert = "tools/binman/test/key.pem"; + capoemflags = <0x8000>; + + u-boot { + }; + }; + + In the above example, the capsule payload is the u-boot image. The + capsule entry would read the contents of the payload and put them + into the capsule. Any external file can also be specified as the + payload using the blob-ext subnode. + """ + def __init__(self, section, etype, node): + super().__init__(section, etype, node) + self.required_props = ['image-index', 'image-type-id'] + self.image_index = 0 + self.image_guid = '' + self.hardware_instance = 0 + self.monotonic_count = 0 + self.fw_version = 0 + self.capoemflags = 0 + self.private_key = '' + self.pub_key_cert = '' + self.auth = 0 + self.capsule_fname = '' + self._entries = OrderedDict() + + def ReadNode(self): + self.ReadEntries() + super().ReadNode() + + self.image_index = fdt_util.GetInt(self._node, 'image-index') + self.image_guid = fdt_util.GetString(self._node, 'image-type-id') + self.fw_version = fdt_util.GetInt(self._node, 'fw-version') + self.hardware_instance = fdt_util.GetInt(self._node, 'hardware-instance') + self.monotonic_count = fdt_util.GetInt(self._node, 'monotonic-count') + self.capoemflags = fdt_util.GetInt(self._node, 'capoemflags') + + self.private_key = fdt_util.GetString(self._node, 'private-key') + self.pub_key_cert = fdt_util.GetString(self._node, 'pub-key-cert') + if ((self.private_key and not self.pub_key_cert) or (self.pub_key_cert and not self.private_key)): + self.Raise('Both private key and public key certificate need to be provided') + elif not (self.private_key and self.pub_key_cert): + self.auth = 0 + else: + self.auth = 1 + + def ReadEntries(self): + """Read the subnode to get the payload for this capsule""" + # We can have a single payload per capsule + if len(self._node.subnodes) != 1: + self.Raise('The capsule entry expects a single subnode for payload') + + for node in self._node.subnodes: + entry = Entry.Create(self, node) + entry.ReadNode() + self._entries[entry.name] = entry + + def _GenCapsule(self): + return self.mkeficapsule.generate_capsule(self.image_index, + self.image_guid, + self.hardware_instance, + self.payload, + self.capsule_fname, + self.private_key, + self.pub_key_cert, + self.monotonic_count, + self.fw_version, + self.capoemflags) + + def BuildSectionData(self, required): + if self.auth: + if not os.path.isabs(self.private_key): + self.private_key = tools.get_input_filename(self.private_key) + if not os.path.isabs(self.pub_key_cert): + self.pub_key_cert = tools.get_input_filename(self.pub_key_cert) + data, self.payload, _ = self.collect_contents_to_file( + self._entries.values(), 'capsule_payload') + outfile = self._filename if self._filename else self._node.name + self.capsule_fname = tools.get_output_filename(outfile) + if self._GenCapsule() is not None: + os.remove(self.payload) + return tools.read_file(self.capsule_fname) + + def ProcessContents(self): + ok = super().ProcessContents() + data = self.BuildSectionData(True) + ok2 = self.ProcessContentsUpdate(data) + return ok and ok2 + + def SetImagePos(self, image_pos): + upto = 0 + for entry in self.GetEntries().values(): + entry.SetOffsetSize(upto, None) + upto += entry.size + + super().SetImagePos(image_pos) + + def AddBintools(self, btools): + self.mkeficapsule = self.AddBintool(btools, 'mkeficapsule') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1cfa349d38..7e7926b607 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -48,6 +48,7 @@ U_BOOT_VPL_DATA = b'vpl76543210fedcbazywxyz_' BLOB_DATA = b'89' ME_DATA = b'0abcd' VGA_DATA = b'vga' +EFI_CAPSULE_DATA = b'efi' U_BOOT_DTB_DATA = b'udtb' U_BOOT_SPL_DTB_DATA = b'spldtb' U_BOOT_TPL_DTB_DATA = b'tpldtb' @@ -119,6 +120,11 @@ COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz', 'zstd']
TEE_ADDR = 0x5678
+# Firmware Management Protocol(FMP) GUID +FW_MGMT_GUID = 'edd5cb6d2de8444cbda17194199ad92a' +# Image GUID specified in the DTS +CAPSULE_IMAGE_GUID = '52cfd7092007104791d108469b7fe9c8' + class TestFunctional(unittest.TestCase): """Functional tests for binman
@@ -215,6 +221,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('scp.bin', SCP_DATA) TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA) TestFunctional._MakeInputFile('ti_unsecure.bin', TI_UNSECURE_DATA) + TestFunctional._MakeInputFile('efi_capsule_payload.bin', EFI_CAPSULE_DATA)
# Add a few .dtb files for testing TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR, @@ -7087,5 +7094,120 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertEqual(fdt_util.GetString(key_node, "key-name-hint"), "key")
+ def _CheckCapsule(self, data, signed_capsule=False, version_check=False, + capoemflags=False): + fmp_signature = "4d535331" # 'M', 'S', 'S', '1' + fmp_size = "10" + fmp_fw_version = "02" + oemflag = "0080" + + payload_data = EFI_CAPSULE_DATA + + # Firmware Management Protocol(FMP) GUID - offset(0 - 32) + self.assertEqual(FW_MGMT_GUID, data.hex()[:32]) + # Image GUID - offset(96 - 128) + self.assertEqual(CAPSULE_IMAGE_GUID, data.hex()[96:128]) + + if capoemflags: + # OEM Flags - offset(40 - 44) + self.assertEqual(oemflag, data.hex()[40:44]) + if signed_capsule and version_check: + # FMP header signature - offset(4770 - 4778) + self.assertEqual(fmp_signature, data.hex()[4770:4778]) + # FMP header size - offset(4778 - 4780) + self.assertEqual(fmp_size, data.hex()[4778:4780]) + # firmware version - offset(4786 - 4788) + self.assertEqual(fmp_fw_version, data.hex()[4786:4788]) + # payload offset signed capsule(4802 - 4808) + self.assertEqual(payload_data.hex(), data.hex()[4802:4808]) + elif signed_capsule: + # payload offset signed capsule(4770 - 4776) + self.assertEqual(payload_data.hex(), data.hex()[4770:4776]) + elif version_check: + # FMP header signature - offset(184 - 192) + self.assertEqual(fmp_signature, data.hex()[184:192]) + # FMP header size - offset(192 - 194) + self.assertEqual(fmp_size, data.hex()[192:194]) + # firmware version - offset(200 - 202) + self.assertEqual(fmp_fw_version, data.hex()[200:202]) + # payload offset for non-signed capsule with version header(216 - 222) + self.assertEqual(payload_data.hex(), data.hex()[216:222]) + else: + # payload offset for non-signed capsule with no version header(184 - 190) + self.assertEqual(payload_data.hex(), data.hex()[184:190]) + + def testCapsuleGen(self): + """Test generation of EFI capsule""" + data = self._DoReadFile('307_capsule.dts') + + self._CheckCapsule(data) + + def testSignedCapsuleGen(self): + """Test generation of EFI capsule""" + data = tools.read_file(self.TestFile("key.key")) + self._MakeInputFile("key.key", data) + data = tools.read_file(self.TestFile("key.pem")) + self._MakeInputFile("key.crt", data) + + data = self._DoReadFile('308_capsule_signed.dts') + + self._CheckCapsule(data, signed_capsule=True) + + def testCapsuleGenVersionSupport(self): + """Test generation of EFI capsule with version support""" + data = self._DoReadFile('309_capsule_version.dts') + + self._CheckCapsule(data, version_check=True) + + def testCapsuleGenSignedVer(self): + """Test generation of signed EFI capsule with version information""" + data = tools.read_file(self.TestFile("key.key")) + self._MakeInputFile("key.key", data) + data = tools.read_file(self.TestFile("key.pem")) + self._MakeInputFile("key.crt", data) + + data = self._DoReadFile('310_capsule_signed_ver.dts') + + self._CheckCapsule(data, signed_capsule=True, version_check=True) + + def testCapsuleGenCapOemFlags(self): + """Test generation of EFI capsule with OEM Flags set""" + data = self._DoReadFile('311_capsule_oemflags.dts') + + self._CheckCapsule(data, capoemflags=True) + + + def testCapsuleGenKeyMissing(self): + """Test that binman errors out on missing key""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('312_capsule_missing_key.dts') + + self.assertIn("Both private key and public key certificate need to be provided", + str(e.exception)) + + def testCapsuleGenIndexMissing(self): + """Test that binman errors out on missing image index""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('313_capsule_missing_index.dts') + + self.assertIn("entry is missing properties: image-index", + str(e.exception)) + + def testCapsuleGenGuidMissing(self): + """Test that binman errors out on missing image GUID""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('314_capsule_missing_guid.dts') + + self.assertIn("entry is missing properties: image-type-id", + str(e.exception)) + + def testCapsuleGenPayloadMissing(self): + """Test that binman errors out on missing input(payload)image""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('315_capsule_missing_payload.dts') + + self.assertIn("The capsule entry expects a single subnode for payload", + str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/307_capsule.dts b/tools/binman/test/307_capsule.dts new file mode 100644 index 0000000000..dd5b6f1c11 --- /dev/null +++ b/tools/binman/test/307_capsule.dts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + + blob-ext { + filename = "efi_capsule_payload.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/308_capsule_signed.dts b/tools/binman/test/308_capsule_signed.dts new file mode 100644 index 0000000000..2765dd4140 --- /dev/null +++ b/tools/binman/test/308_capsule_signed.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + private-key = "key.key"; + pub-key-cert = "key.crt"; + + blob-ext { + filename = "efi_capsule_payload.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/309_capsule_version.dts b/tools/binman/test/309_capsule_version.dts new file mode 100644 index 0000000000..21d6e2bd26 --- /dev/null +++ b/tools/binman/test/309_capsule_version.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + fw-version = <0x2>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + + blob-ext { + filename = "efi_capsule_payload.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/310_capsule_signed_ver.dts b/tools/binman/test/310_capsule_signed_ver.dts new file mode 100644 index 0000000000..f3606e6520 --- /dev/null +++ b/tools/binman/test/310_capsule_signed_ver.dts @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + fw-version = <0x2>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + private-key = "key.key"; + pub-key-cert = "key.crt"; + + blob-ext { + filename = "efi_capsule_payload.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/311_capsule_oemflags.dts b/tools/binman/test/311_capsule_oemflags.dts new file mode 100644 index 0000000000..9f535f8712 --- /dev/null +++ b/tools/binman/test/311_capsule_oemflags.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + capoemflags = <0x8000>; + + blob-ext { + filename = "efi_capsule_payload.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/312_capsule_missing_key.dts b/tools/binman/test/312_capsule_missing_key.dts new file mode 100644 index 0000000000..6a6fc59b6d --- /dev/null +++ b/tools/binman/test/312_capsule_missing_key.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + private-key = "tools/binman/test/key.key"; + + blob-ext { + filename = "efi_capsule_payload.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/313_capsule_missing_index.dts b/tools/binman/test/313_capsule_missing_index.dts new file mode 100644 index 0000000000..7806521135 --- /dev/null +++ b/tools/binman/test/313_capsule_missing_index.dts @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + + blob-ext { + filename = "efi_capsule_payload.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/314_capsule_missing_guid.dts b/tools/binman/test/314_capsule_missing_guid.dts new file mode 100644 index 0000000000..599562bef6 --- /dev/null +++ b/tools/binman/test/314_capsule_missing_guid.dts @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + hardware-instance = <0x0>; + + blob-ext { + filename = "efi_capsule_payload.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/315_capsule_missing_payload.dts b/tools/binman/test/315_capsule_missing_payload.dts new file mode 100644 index 0000000000..86da9cd309 --- /dev/null +++ b/tools/binman/test/315_capsule_missing_payload.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + hardware-instance = <0x0>; + }; + }; +};

Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Add support for the oemflag parameter used in FWU A/B updates. This was missed in the earlier version.
- Use a single function, generate_capsule in the mkeficapsule bintool, instead of the multiple functions in earlier version.
- Remove the logic for generating capsules from config file as suggested by Simon.
- Use required_props for image index and GUID parameters.
- Use a subnode for the capsule payload instead of using a filename for the payload, as suggested by Simon.
- Add a capsule generation test with oemflag parameter being passed.
configs/sandbox_spl_defconfig | 1 +
move to a later commit
tools/binman/btool/mkeficapsule.py | 101 +++++++++++
nit: Please split this into its own commit, with the etype in the second commit.
tools/binman/entries.rst | 64 +++++++ tools/binman/etype/efi_capsule.py | 160 ++++++++++++++++++ tools/binman/ftest.py | 122 +++++++++++++ tools/binman/test/307_capsule.dts | 21 +++ tools/binman/test/308_capsule_signed.dts | 23 +++ tools/binman/test/309_capsule_version.dts | 22 +++ tools/binman/test/310_capsule_signed_ver.dts | 24 +++ tools/binman/test/311_capsule_oemflags.dts | 22 +++ tools/binman/test/312_capsule_missing_key.dts | 22 +++ .../binman/test/313_capsule_missing_index.dts | 20 +++ .../binman/test/314_capsule_missing_guid.dts | 19 +++ .../test/315_capsule_missing_payload.dts | 17 ++ 14 files changed, 638 insertions(+) create mode 100644 tools/binman/btool/mkeficapsule.py create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/307_capsule.dts create mode 100644 tools/binman/test/308_capsule_signed.dts create mode 100644 tools/binman/test/309_capsule_version.dts create mode 100644 tools/binman/test/310_capsule_signed_ver.dts create mode 100644 tools/binman/test/311_capsule_oemflags.dts create mode 100644 tools/binman/test/312_capsule_missing_key.dts create mode 100644 tools/binman/test/313_capsule_missing_index.dts create mode 100644 tools/binman/test/314_capsule_missing_guid.dts create mode 100644 tools/binman/test/315_capsule_missing_payload.dts
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 8d50162b27..65223475ab 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -249,3 +249,4 @@ CONFIG_UNIT_TEST=y CONFIG_SPL_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y +CONFIG_TOOLS_MKEFICAPSULE=y diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py new file mode 100644 index 0000000000..da1d5de873 --- /dev/null +++ b/tools/binman/btool/mkeficapsule.py @@ -0,0 +1,101 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2023 Linaro Limited +# +"""Bintool implementation for mkeficapsule tool
+mkeficapsule is a tool used for generating EFI capsules.
+The following are the command-line options to be provided +to the tool +Usage: mkeficapsule [options] <image blob> <output file> +Options:
-g, --guid <guid string> guid for image blob type
-i, --index <index> update image index
-I, --instance <instance> update hardware instance
-v, --fw-version <version> firmware version
-p, --private-key <privkey file> private key file
-c, --certificate <cert file> signer's certificate file
-m, --monotonic-count <count> monotonic count
-d, --dump_sig dump signature (*.p7)
-A, --fw-accept firmware accept capsule, requires GUID, no image blob
-R, --fw-revert firmware revert capsule, takes no GUID, no image blob
-o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff
-h, --help print a help message
+"""
+from binman import bintool
+class Bintoolmkeficapsule(bintool.Bintool):
- """Handles the 'mkeficapsule' tool
- This bintool is used for generating the EFI capsules. The
- capsule generation parameters can either be specified through
- command-line, or through a config file.
commandline (or command line, but please be consistent )
- """
- def __init__(self, name):
super().__init__(name, 'mkeficapsule tool for generating capsules')
- def generate_capsule(self, image_index, image_guid, hardware_instance,
payload, output_fname, priv_key, pub_key,
monotonic_count=0, version=0, oemflags=0):
"""Generate a capsule through commandline provided parameters
commandline-provided
Args:
image_index (int): Unique number for identifying payload image
image_guid (str): GUID used for identifying the image
hardware_instance (int): Optional unique hardware instance of
a device in the system. 0 if not being used
payload (str): Path to the input payload image
output_fname (str): Path to the output capsule file
priv_key (str): Path to the private key
pub_key(str): Path to the public key
monotonic_count (int): Count used when signing an image
version (int): Image version (Optional)
oemflags (int): Optional 16 bit OEM flags
Returns:
str: Tool output
"""
args = [
f'--index={image_index}',
f'--guid={image_guid}',
f'--instance={hardware_instance}'
]
if version:
args += [f'--fw-version={version}']
if oemflags:
args += [f'--capoemflag={oemflags}']
if priv_key and pub_key:
args += [
f'--monotonic-count={monotonic_count}',
f'--private-key={priv_key}',
f'--certificate={pub_key}'
]
args += [
payload,
output_fname
]
return self.run_cmd(*args)
- def fetch(self, method):
"""Fetch handler for mkeficapsule
This builds the tool from source
Returns:
tuple:
str: Filename of fetched file to copy to a suitable directory
str: Name of temp directory to remove, or None
"""
if method != bintool.FETCH_BUILD:
return None
cmd = ['tools-only_defconfig', 'tools']
result = self.build_from_git(
'https://source.denx.de/u-boot/u-boot.git',
cmd,
'tools/mkeficapsule')
return result
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index f2376932be..cfe699361c 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -468,6 +468,70 @@ updating the EC on startup via software sync.
The btool looks fine to me
[..]
diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py new file mode 100644 index 0000000000..b8a269e247 --- /dev/null +++ b/tools/binman/etype/efi_capsule.py @@ -0,0 +1,160 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2023 Linaro Limited +# +# Entry-type module for producing a EFI capsule +#
+from collections import OrderedDict +import os
+from binman.entry import Entry +from binman.etype.section import Entry_section +from dtoc import fdt_util +from u_boot_pylib import tools
+class Entry_efi_capsule(Entry_section):
- """Entry for generating EFI capsules
"""Generate EPI capsules
(as we know it is an entry type)
- This is an entry for generating EFI capsules.
Drop that as it repeats the above line
- The parameters needed for generation of the capsules can
- either be provided as properties in the entry.
or...?
- Properties / Entry arguments:
- image-index: Unique number for identifying corresponding
payload image. Number between 1 and descriptor count, i.e.
the total number of firmware images that can be updated.
- image-type-id: Image GUID which will be used for identifying the
updatable image on the board.
- hardware-instance: Optional number for identifying unique
hardware instance of a device in the system. Default value of 0
for images where value is not to be used.
- fw-version: Optional value of image version that can be put on
the capsule through the Firmware Management Protocol(FMP) header.
- monotonic-count: Count used when signing an image.
- private-key: Path to PEM formatted .key private key file.
Can you perhaps put these in a different order do you can say that if private-key is provided, you must also provide public-key-cert ?
- pub-key-cert: Path to PEM formatted .crt public key certificate
public, I think, since you write out private in full
file.
- capoemflags - Optional OEM flags to be passed through capsule header.
oem-flags ?
If 'cap' refers to capsule, we already know it is a capsule
- filename:
Technically this is not true...it is the section output. I think it is better to mention that it inherits the section property, which allows an output filename.
Optional path to the output capsule file. A capsule is a
continuous set of data as defined by the EFI specification. Refer
to the specification for more details.
Good comments. You can drop filename, since you subclass entry_Section which always provides this feature. But it's OK to mention it if you like, e.g. "All section properties are supported, include filename".
- For more details on the description of the capsule format, and the capsule
- update functionality, refer Section 8.5 and Chapter 23 in the UEFI
- specification.
- https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf
I suppose this is OK, but can you make it into a text link? e.g. see how doc/usage/cmd/acpi.rst does it.
- The capsule parameters like image index and image GUID are passed as
- properties in the entry. The payload to be used in the capsule is to be
- provided as a subnode of the capsule entry.
- A typical capsule entry node would then look something like this
- capsule {
type = "efi-capsule";
image-index = <0x1>;
/* Image GUID for testing capsule update */
image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
lower-case. Also please use a name here, e.g. a #fdefine above, if this isn't something already in U-Boot. I very much want to avoid people adding random UUIDs everywhere.
hardware-instance = <0x0>;
private-key = "tools/binman/test/key.key";
pub-key-cert = "tools/binman/test/key.pem";
Drop the path for these
capoemflags = <0x8000>;
u-boot {
};
- };
- In the above example, the capsule payload is the u-boot image. The
U-Boot
- capsule entry would read the contents of the payload and put them
- into the capsule. Any external file can also be specified as the
- payload using the blob-ext subnode.
- """
- def __init__(self, section, etype, node):
super().__init__(section, etype, node)
self.required_props = ['image-index', 'image-type-id']
self.image_index = 0
self.image_guid = ''
self.hardware_instance = 0
self.monotonic_count = 0
self.fw_version = 0
self.capoemflags = 0
self.private_key = ''
self.pub_key_cert = ''
self.auth = 0
self.capsule_fname = ''
Please drop
self._entries = OrderedDict()
entry_Section already does this
- def ReadNode(self):
self.ReadEntries()
super().ReadNode()
self.image_index = fdt_util.GetInt(self._node, 'image-index')
self.image_guid = fdt_util.GetString(self._node, 'image-type-id')
self.fw_version = fdt_util.GetInt(self._node, 'fw-version')
self.hardware_instance = fdt_util.GetInt(self._node, 'hardware-instance')
self.monotonic_count = fdt_util.GetInt(self._node, 'monotonic-count')
self.capoemflags = fdt_util.GetInt(self._node, 'capoemflags')
self.private_key = fdt_util.GetString(self._node, 'private-key')
self.pub_key_cert = fdt_util.GetString(self._node, 'pub-key-cert')
if ((self.private_key and not self.pub_key_cert) or (self.pub_key_cert and not self.private_key)):
self.Raise('Both private key and public key certificate need to be provided')
elif not (self.private_key and self.pub_key_cert):
self.auth = 0
else:
self.auth = 1
- def ReadEntries(self):
"""Read the subnode to get the payload for this capsule"""
# We can have a single payload per capsule
if len(self._node.subnodes) != 1:
self.Raise('The capsule entry expects a single subnode for payload')
No, we mustn't restruit this. Why would we?
for node in self._node.subnodes:
entry = Entry.Create(self, node)
entry.ReadNode()
self._entries[entry.name] = entry
- def _GenCapsule(self):
return self.mkeficapsule.generate_capsule(self.image_index,
self.image_guid,
self.hardware_instance,
self.payload,
self.capsule_fname,
self.private_key,
self.pub_key_cert,
self.monotonic_count,
self.fw_version,
self.capoemflags)
- def BuildSectionData(self, required):
if self.auth:
if not os.path.isabs(self.private_key):
self.private_key = tools.get_input_filename(self.private_key)
if not os.path.isabs(self.pub_key_cert):
self.pub_key_cert = tools.get_input_filename(self.pub_key_cert)
These should be local vars, not class vars. Also these are properties passed in by the user, so you should not be converting them to filenames.
data, self.payload, _ = self.collect_contents_to_file(
Please don't add new things to the class in the middle of the class. Doesn't pylint warn about this? This can just be a local var.
self._entries.values(), 'capsule_payload')
outfile = self._filename if self._filename else self._node.name
self.capsule_fname = tools.get_output_filename(outfile)
No, the class is for holding properties...you don't use this outside the function, so just:
fname = tools....
if self._GenCapsule() is not None:
Here you need to pass some of the params. Honestly, why not drop _GenCapsule() and just call generate_capsule() here? It seems easier.
os.remove(self.payload)
return tools.read_file(self.capsule_fname)
return tools.read_file(fname)
- def ProcessContents(self):
ok = super().ProcessContents()
data = self.BuildSectionData(True)
ok2 = self.ProcessContentsUpdate(data)
return ok and ok2
If that is the same as the entry_Section function, you can drop it
- def SetImagePos(self, image_pos):
upto = 0
for entry in self.GetEntries().values():
entry.SetOffsetSize(upto, None)
upto += entry.size
super().SetImagePos(image_pos)
This function looks the same as entry_Section. Drop it?
- def AddBintools(self, btools):
self.mkeficapsule = self.AddBintool(btools, 'mkeficapsule')
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1cfa349d38..7e7926b607 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -48,6 +48,7 @@ U_BOOT_VPL_DATA = b'vpl76543210fedcbazywxyz_' BLOB_DATA = b'89' ME_DATA = b'0abcd' VGA_DATA = b'vga' +EFI_CAPSULE_DATA = b'efi' U_BOOT_DTB_DATA = b'udtb' U_BOOT_SPL_DTB_DATA = b'spldtb' U_BOOT_TPL_DTB_DATA = b'tpldtb' @@ -119,6 +120,11 @@ COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz', 'zstd']
TEE_ADDR = 0x5678
+# Firmware Management Protocol(FMP) GUID +FW_MGMT_GUID = 'edd5cb6d2de8444cbda17194199ad92a' +# Image GUID specified in the DTS +CAPSULE_IMAGE_GUID = '52cfd7092007104791d108469b7fe9c8'
class TestFunctional(unittest.TestCase): """Functional tests for binman
@@ -215,6 +221,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('scp.bin', SCP_DATA) TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA) TestFunctional._MakeInputFile('ti_unsecure.bin', TI_UNSECURE_DATA)
TestFunctional._MakeInputFile('efi_capsule_payload.bin', EFI_CAPSULE_DATA) # Add a few .dtb files for testing TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
@@ -7087,5 +7094,120 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertEqual(fdt_util.GetString(key_node, "key-name-hint"), "key")
- def _CheckCapsule(self, data, signed_capsule=False, version_check=False,
capoemflags=False):
fmp_signature = "4d535331" # 'M', 'S', 'S', '1'
fmp_size = "10"
fmp_fw_version = "02"
oemflag = "0080"
payload_data = EFI_CAPSULE_DATA
# Firmware Management Protocol(FMP) GUID - offset(0 - 32)
self.assertEqual(FW_MGMT_GUID, data.hex()[:32])
# Image GUID - offset(96 - 128)
self.assertEqual(CAPSULE_IMAGE_GUID, data.hex()[96:128])
if capoemflags:
# OEM Flags - offset(40 - 44)
self.assertEqual(oemflag, data.hex()[40:44])
if signed_capsule and version_check:
# FMP header signature - offset(4770 - 4778)
self.assertEqual(fmp_signature, data.hex()[4770:4778])
# FMP header size - offset(4778 - 4780)
Can you mention where these values came from and how you created them? This all seems very brittle. Is there a tool that prints them out, or..?
self.assertEqual(fmp_size, data.hex()[4778:4780])
# firmware version - offset(4786 - 4788)
self.assertEqual(fmp_fw_version, data.hex()[4786:4788])
# payload offset signed capsule(4802 - 4808)
self.assertEqual(payload_data.hex(), data.hex()[4802:4808])
elif signed_capsule:
# payload offset signed capsule(4770 - 4776)
self.assertEqual(payload_data.hex(), data.hex()[4770:4776])
elif version_check:
# FMP header signature - offset(184 - 192)
self.assertEqual(fmp_signature, data.hex()[184:192])
# FMP header size - offset(192 - 194)
self.assertEqual(fmp_size, data.hex()[192:194])
# firmware version - offset(200 - 202)
self.assertEqual(fmp_fw_version, data.hex()[200:202])
# payload offset for non-signed capsule with version header(216 - 222)
self.assertEqual(payload_data.hex(), data.hex()[216:222])
else:
# payload offset for non-signed capsule with no version header(184 - 190)
self.assertEqual(payload_data.hex(), data.hex()[184:190])
- def testCapsuleGen(self):
"""Test generation of EFI capsule"""
data = self._DoReadFile('307_capsule.dts')
self._CheckCapsule(data)
- def testSignedCapsuleGen(self):
"""Test generation of EFI capsule"""
data = tools.read_file(self.TestFile("key.key"))
self._MakeInputFile("key.key", data)
data = tools.read_file(self.TestFile("key.pem"))
self._MakeInputFile("key.crt", data)
data = self._DoReadFile('308_capsule_signed.dts')
self._CheckCapsule(data, signed_capsule=True)
- def testCapsuleGenVersionSupport(self):
"""Test generation of EFI capsule with version support"""
data = self._DoReadFile('309_capsule_version.dts')
self._CheckCapsule(data, version_check=True)
- def testCapsuleGenSignedVer(self):
"""Test generation of signed EFI capsule with version information"""
data = tools.read_file(self.TestFile("key.key"))
self._MakeInputFile("key.key", data)
data = tools.read_file(self.TestFile("key.pem"))
self._MakeInputFile("key.crt", data)
data = self._DoReadFile('310_capsule_signed_ver.dts')
self._CheckCapsule(data, signed_capsule=True, version_check=True)
- def testCapsuleGenCapOemFlags(self):
"""Test generation of EFI capsule with OEM Flags set"""
data = self._DoReadFile('311_capsule_oemflags.dts')
self._CheckCapsule(data, capoemflags=True)
- def testCapsuleGenKeyMissing(self):
"""Test that binman errors out on missing key"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('312_capsule_missing_key.dts')
self.assertIn("Both private key and public key certificate need to be provided",
str(e.exception))
- def testCapsuleGenIndexMissing(self):
"""Test that binman errors out on missing image index"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('313_capsule_missing_index.dts')
self.assertIn("entry is missing properties: image-index",
str(e.exception))
- def testCapsuleGenGuidMissing(self):
"""Test that binman errors out on missing image GUID"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('314_capsule_missing_guid.dts')
self.assertIn("entry is missing properties: image-type-id",
str(e.exception))
- def testCapsuleGenPayloadMissing(self):
"""Test that binman errors out on missing input(payload)image"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('315_capsule_missing_payload.dts')
self.assertIn("The capsule entry expects a single subnode for payload",
str(e.exception))
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/307_capsule.dts b/tools/binman/test/307_capsule.dts new file mode 100644 index 0000000000..dd5b6f1c11 --- /dev/null +++ b/tools/binman/test/307_capsule.dts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;
#size-cells = <1>;
binman {
efi-capsule {
image-index = <0x1>;
/* Image GUID for testing capsule update */
image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
Please create a #include files for these (and use lower case). Please fix globally.
hardware-instance = <0x0>;
blob-ext {
filename = "efi_capsule_payload.bin";
Do you need this filename?
};
};
};
+};
[..]
Regards, Simon

hi Simon,
On Wed, 2 Aug 2023 at 18:23, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Add support for the oemflag parameter used in FWU A/B updates. This was missed in the earlier version.
- Use a single function, generate_capsule in the mkeficapsule bintool, instead of the multiple functions in earlier version.
- Remove the logic for generating capsules from config file as suggested by Simon.
- Use required_props for image index and GUID parameters.
- Use a subnode for the capsule payload instead of using a filename for the payload, as suggested by Simon.
- Add a capsule generation test with oemflag parameter being passed.
configs/sandbox_spl_defconfig | 1 +
move to a later commit
Okay. I think it'd be better to have this change before adding support for efi_capsule entry in binman.
tools/binman/btool/mkeficapsule.py | 101 +++++++++++
nit: Please split this into its own commit, with the etype in the second commit.
Okay
tools/binman/entries.rst | 64 +++++++ tools/binman/etype/efi_capsule.py | 160 ++++++++++++++++++ tools/binman/ftest.py | 122 +++++++++++++ tools/binman/test/307_capsule.dts | 21 +++ tools/binman/test/308_capsule_signed.dts | 23 +++ tools/binman/test/309_capsule_version.dts | 22 +++ tools/binman/test/310_capsule_signed_ver.dts | 24 +++ tools/binman/test/311_capsule_oemflags.dts | 22 +++ tools/binman/test/312_capsule_missing_key.dts | 22 +++ .../binman/test/313_capsule_missing_index.dts | 20 +++ .../binman/test/314_capsule_missing_guid.dts | 19 +++ .../test/315_capsule_missing_payload.dts | 17 ++ 14 files changed, 638 insertions(+) create mode 100644 tools/binman/btool/mkeficapsule.py create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/307_capsule.dts create mode 100644 tools/binman/test/308_capsule_signed.dts create mode 100644 tools/binman/test/309_capsule_version.dts create mode 100644 tools/binman/test/310_capsule_signed_ver.dts create mode 100644 tools/binman/test/311_capsule_oemflags.dts create mode 100644 tools/binman/test/312_capsule_missing_key.dts create mode 100644 tools/binman/test/313_capsule_missing_index.dts create mode 100644 tools/binman/test/314_capsule_missing_guid.dts create mode 100644 tools/binman/test/315_capsule_missing_payload.dts
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 8d50162b27..65223475ab 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -249,3 +249,4 @@ CONFIG_UNIT_TEST=y CONFIG_SPL_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y +CONFIG_TOOLS_MKEFICAPSULE=y diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py new file mode 100644 index 0000000000..da1d5de873 --- /dev/null +++ b/tools/binman/btool/mkeficapsule.py @@ -0,0 +1,101 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2023 Linaro Limited +# +"""Bintool implementation for mkeficapsule tool
+mkeficapsule is a tool used for generating EFI capsules.
+The following are the command-line options to be provided +to the tool +Usage: mkeficapsule [options] <image blob> <output file> +Options:
-g, --guid <guid string> guid for image blob type
-i, --index <index> update image index
-I, --instance <instance> update hardware instance
-v, --fw-version <version> firmware version
-p, --private-key <privkey file> private key file
-c, --certificate <cert file> signer's certificate file
-m, --monotonic-count <count> monotonic count
-d, --dump_sig dump signature (*.p7)
-A, --fw-accept firmware accept capsule, requires GUID, no image blob
-R, --fw-revert firmware revert capsule, takes no GUID, no image blob
-o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff
-h, --help print a help message
+"""
+from binman import bintool
+class Bintoolmkeficapsule(bintool.Bintool):
- """Handles the 'mkeficapsule' tool
- This bintool is used for generating the EFI capsules. The
- capsule generation parameters can either be specified through
- command-line, or through a config file.
commandline (or command line, but please be consistent
Okay
)
- """
- def __init__(self, name):
super().__init__(name, 'mkeficapsule tool for generating capsules')
- def generate_capsule(self, image_index, image_guid, hardware_instance,
payload, output_fname, priv_key, pub_key,
monotonic_count=0, version=0, oemflags=0):
"""Generate a capsule through commandline provided parameters
commandline-provided
Okay
Args:
image_index (int): Unique number for identifying payload image
image_guid (str): GUID used for identifying the image
hardware_instance (int): Optional unique hardware instance of
a device in the system. 0 if not being used
payload (str): Path to the input payload image
output_fname (str): Path to the output capsule file
priv_key (str): Path to the private key
pub_key(str): Path to the public key
monotonic_count (int): Count used when signing an image
version (int): Image version (Optional)
oemflags (int): Optional 16 bit OEM flags
Returns:
str: Tool output
"""
args = [
f'--index={image_index}',
f'--guid={image_guid}',
f'--instance={hardware_instance}'
]
if version:
args += [f'--fw-version={version}']
if oemflags:
args += [f'--capoemflag={oemflags}']
if priv_key and pub_key:
args += [
f'--monotonic-count={monotonic_count}',
f'--private-key={priv_key}',
f'--certificate={pub_key}'
]
args += [
payload,
output_fname
]
return self.run_cmd(*args)
- def fetch(self, method):
"""Fetch handler for mkeficapsule
This builds the tool from source
Returns:
tuple:
str: Filename of fetched file to copy to a suitable directory
str: Name of temp directory to remove, or None
"""
if method != bintool.FETCH_BUILD:
return None
cmd = ['tools-only_defconfig', 'tools']
result = self.build_from_git(
'https://source.denx.de/u-boot/u-boot.git',
cmd,
'tools/mkeficapsule')
return result
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index f2376932be..cfe699361c 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -468,6 +468,70 @@ updating the EC on startup via software sync.
The btool looks fine to me
[..]
diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py new file mode 100644 index 0000000000..b8a269e247 --- /dev/null +++ b/tools/binman/etype/efi_capsule.py @@ -0,0 +1,160 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2023 Linaro Limited +# +# Entry-type module for producing a EFI capsule +#
+from collections import OrderedDict +import os
+from binman.entry import Entry +from binman.etype.section import Entry_section +from dtoc import fdt_util +from u_boot_pylib import tools
+class Entry_efi_capsule(Entry_section):
- """Entry for generating EFI capsules
"""Generate EPI capsules
(as we know it is an entry type)
Okay
- This is an entry for generating EFI capsules.
Drop that as it repeats the above line
Okay
- The parameters needed for generation of the capsules can
- either be provided as properties in the entry.
or...?
Vestige from the earlier version where capsule generation was supported through a config file as well.
- Properties / Entry arguments:
- image-index: Unique number for identifying corresponding
payload image. Number between 1 and descriptor count, i.e.
the total number of firmware images that can be updated.
- image-type-id: Image GUID which will be used for identifying the
updatable image on the board.
- hardware-instance: Optional number for identifying unique
hardware instance of a device in the system. Default value of 0
for images where value is not to be used.
- fw-version: Optional value of image version that can be put on
the capsule through the Firmware Management Protocol(FMP) header.
- monotonic-count: Count used when signing an image.
- private-key: Path to PEM formatted .key private key file.
Can you perhaps put these in a different order do you can say that if private-key is provided, you must also provide public-key-cert ?
For both the private and public key properties, I have added the following line.
"This property is mandatory for generating signed capsules."
- pub-key-cert: Path to PEM formatted .crt public key certificate
public, I think, since you write out private in full
Okay
file.
- capoemflags - Optional OEM flags to be passed through capsule header.
oem-flags ?
Okay
If 'cap' refers to capsule, we already know it is a capsule
- filename:
Technically this is not true...it is the section output. I think it is better to mention that it inherits the section property, which allows an output filename.
Optional path to the output capsule file. A capsule is a
continuous set of data as defined by the EFI specification. Refer
to the specification for more details.
Good comments. You can drop filename, since you subclass entry_Section which always provides this feature. But it's OK to mention it if you like, e.g. "All section properties are supported, include filename".
I have removed the filename property, and replaced it with the following text "Since this is a subclass of Entry_section, all properties of the parent class also apply here."
- For more details on the description of the capsule format, and the capsule
- update functionality, refer Section 8.5 and Chapter 23 in the UEFI
- specification.
- https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf
I suppose this is OK, but can you make it into a text link? e.g. see how doc/usage/cmd/acpi.rst does it.
Done
- The capsule parameters like image index and image GUID are passed as
- properties in the entry. The payload to be used in the capsule is to be
- provided as a subnode of the capsule entry.
- A typical capsule entry node would then look something like this
- capsule {
type = "efi-capsule";
image-index = <0x1>;
/* Image GUID for testing capsule update */
image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
lower-case. Also please use a name here, e.g. a #fdefine above, if this isn't something already in U-Boot. I very much want to avoid people adding random UUIDs everywhere.
hardware-instance = <0x0>;
private-key = "tools/binman/test/key.key";
pub-key-cert = "tools/binman/test/key.pem";
Drop the path for these
This is just an illustration. I have removed this specific path though.
capoemflags = <0x8000>;
u-boot {
};
- };
- In the above example, the capsule payload is the u-boot image. The
U-Boot
Okay
- capsule entry would read the contents of the payload and put them
- into the capsule. Any external file can also be specified as the
- payload using the blob-ext subnode.
- """
- def __init__(self, section, etype, node):
super().__init__(section, etype, node)
self.required_props = ['image-index', 'image-type-id']
self.image_index = 0
self.image_guid = ''
self.hardware_instance = 0
self.monotonic_count = 0
self.fw_version = 0
self.capoemflags = 0
self.private_key = ''
self.pub_key_cert = ''
self.auth = 0
self.capsule_fname = ''
Please drop
Okay
self._entries = OrderedDict()
entry_Section already does this
Removed
- def ReadNode(self):
self.ReadEntries()
super().ReadNode()
self.image_index = fdt_util.GetInt(self._node, 'image-index')
self.image_guid = fdt_util.GetString(self._node, 'image-type-id')
self.fw_version = fdt_util.GetInt(self._node, 'fw-version')
self.hardware_instance = fdt_util.GetInt(self._node, 'hardware-instance')
self.monotonic_count = fdt_util.GetInt(self._node, 'monotonic-count')
self.capoemflags = fdt_util.GetInt(self._node, 'capoemflags')
self.private_key = fdt_util.GetString(self._node, 'private-key')
self.pub_key_cert = fdt_util.GetString(self._node, 'pub-key-cert')
if ((self.private_key and not self.pub_key_cert) or (self.pub_key_cert and not self.private_key)):
self.Raise('Both private key and public key certificate need to be provided')
elif not (self.private_key and self.pub_key_cert):
self.auth = 0
else:
self.auth = 1
- def ReadEntries(self):
"""Read the subnode to get the payload for this capsule"""
# We can have a single payload per capsule
if len(self._node.subnodes) != 1:
self.Raise('The capsule entry expects a single subnode for payload')
No, we mustn't restruit this. Why would we?
The mkeficapsule tool that generates the capsule expects a single image as it's input payload. Why do you see the need to support multiple entries as payload?
mkeficapsule [options] <image blob> <output file>
for node in self._node.subnodes:
entry = Entry.Create(self, node)
entry.ReadNode()
self._entries[entry.name] = entry
- def _GenCapsule(self):
return self.mkeficapsule.generate_capsule(self.image_index,
self.image_guid,
self.hardware_instance,
self.payload,
self.capsule_fname,
self.private_key,
self.pub_key_cert,
self.monotonic_count,
self.fw_version,
self.capoemflags)
- def BuildSectionData(self, required):
if self.auth:
if not os.path.isabs(self.private_key):
self.private_key = tools.get_input_filename(self.private_key)
if not os.path.isabs(self.pub_key_cert):
self.pub_key_cert = tools.get_input_filename(self.pub_key_cert)
These should be local vars, not class vars.
?
These are properties being passed by the user. Why should only these be local vars?
Also these are properties passed in by the user, so you should not be converting them to filenames.
These properties are actually paths to the private and public key cert files that a user will pass. In the normal usage, the user would be expected to provide the absolute path to the private and public key cert files. The above logic is needed for binman tests and out of tree builds with relative paths.
data, self.payload, _ = self.collect_contents_to_file(
Please don't add new things to the class in the middle of the class. Doesn't pylint warn about this? This can just be a local var.
Changed this to a local var.
self._entries.values(), 'capsule_payload')
outfile = self._filename if self._filename else self._node.name
self.capsule_fname = tools.get_output_filename(outfile)
No, the class is for holding properties...you don't use this outside the function, so just:
fname = tools....
Changed
if self._GenCapsule() is not None:
Here you need to pass some of the params. Honestly, why not drop _GenCapsule() and just call generate_capsule() here? It seems easier.
Done
os.remove(self.payload)
return tools.read_file(self.capsule_fname)
return tools.read_file(fname)
- def ProcessContents(self):
ok = super().ProcessContents()
data = self.BuildSectionData(True)
ok2 = self.ProcessContentsUpdate(data)
return ok and ok2
If that is the same as the entry_Section function, you can drop it
There are differences, so this is needed.
- def SetImagePos(self, image_pos):
upto = 0
for entry in self.GetEntries().values():
entry.SetOffsetSize(upto, None)
upto += entry.size
super().SetImagePos(image_pos)
This function looks the same as entry_Section. Drop it?
Yes
- def AddBintools(self, btools):
self.mkeficapsule = self.AddBintool(btools, 'mkeficapsule')
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1cfa349d38..7e7926b607 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -48,6 +48,7 @@ U_BOOT_VPL_DATA = b'vpl76543210fedcbazywxyz_' BLOB_DATA = b'89' ME_DATA = b'0abcd' VGA_DATA = b'vga' +EFI_CAPSULE_DATA = b'efi' U_BOOT_DTB_DATA = b'udtb' U_BOOT_SPL_DTB_DATA = b'spldtb' U_BOOT_TPL_DTB_DATA = b'tpldtb' @@ -119,6 +120,11 @@ COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz', 'zstd']
TEE_ADDR = 0x5678
+# Firmware Management Protocol(FMP) GUID +FW_MGMT_GUID = 'edd5cb6d2de8444cbda17194199ad92a' +# Image GUID specified in the DTS +CAPSULE_IMAGE_GUID = '52cfd7092007104791d108469b7fe9c8'
class TestFunctional(unittest.TestCase): """Functional tests for binman
@@ -215,6 +221,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('scp.bin', SCP_DATA) TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA) TestFunctional._MakeInputFile('ti_unsecure.bin', TI_UNSECURE_DATA)
TestFunctional._MakeInputFile('efi_capsule_payload.bin', EFI_CAPSULE_DATA) # Add a few .dtb files for testing TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
@@ -7087,5 +7094,120 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertEqual(fdt_util.GetString(key_node, "key-name-hint"), "key")
- def _CheckCapsule(self, data, signed_capsule=False, version_check=False,
capoemflags=False):
fmp_signature = "4d535331" # 'M', 'S', 'S', '1'
fmp_size = "10"
fmp_fw_version = "02"
oemflag = "0080"
payload_data = EFI_CAPSULE_DATA
# Firmware Management Protocol(FMP) GUID - offset(0 - 32)
self.assertEqual(FW_MGMT_GUID, data.hex()[:32])
# Image GUID - offset(96 - 128)
self.assertEqual(CAPSULE_IMAGE_GUID, data.hex()[96:128])
if capoemflags:
# OEM Flags - offset(40 - 44)
self.assertEqual(oemflag, data.hex()[40:44])
if signed_capsule and version_check:
# FMP header signature - offset(4770 - 4778)
self.assertEqual(fmp_signature, data.hex()[4770:4778])
# FMP header size - offset(4778 - 4780)
Can you mention where these values came from and how you created them? This all seems very brittle. Is there a tool that prints them out, or..?
Like I mentioned earlier as well, the contents of a capsule are dependent on the input parameters specified in it's generation. For e.g. the capsule generated would be different when opting for passing the version param. Similarly when generating a signed capsule -- even with a signed capsule, the contents would depend on the keys. The offsets therefore are not fixed but depend on the inputs specified. I have added comments here for better understanding. Not sure what can be done to improve this.
This is also one reason why the testing of the EFI capsule update feature should use capsules generated as part of the sandbox platform build. That makes the testing that much more robust.
self.assertEqual(fmp_size, data.hex()[4778:4780])
# firmware version - offset(4786 - 4788)
self.assertEqual(fmp_fw_version, data.hex()[4786:4788])
# payload offset signed capsule(4802 - 4808)
self.assertEqual(payload_data.hex(), data.hex()[4802:4808])
elif signed_capsule:
# payload offset signed capsule(4770 - 4776)
self.assertEqual(payload_data.hex(), data.hex()[4770:4776])
elif version_check:
# FMP header signature - offset(184 - 192)
self.assertEqual(fmp_signature, data.hex()[184:192])
# FMP header size - offset(192 - 194)
self.assertEqual(fmp_size, data.hex()[192:194])
# firmware version - offset(200 - 202)
self.assertEqual(fmp_fw_version, data.hex()[200:202])
# payload offset for non-signed capsule with version header(216 - 222)
self.assertEqual(payload_data.hex(), data.hex()[216:222])
else:
# payload offset for non-signed capsule with no version header(184 - 190)
self.assertEqual(payload_data.hex(), data.hex()[184:190])
- def testCapsuleGen(self):
"""Test generation of EFI capsule"""
data = self._DoReadFile('307_capsule.dts')
self._CheckCapsule(data)
- def testSignedCapsuleGen(self):
"""Test generation of EFI capsule"""
data = tools.read_file(self.TestFile("key.key"))
self._MakeInputFile("key.key", data)
data = tools.read_file(self.TestFile("key.pem"))
self._MakeInputFile("key.crt", data)
data = self._DoReadFile('308_capsule_signed.dts')
self._CheckCapsule(data, signed_capsule=True)
- def testCapsuleGenVersionSupport(self):
"""Test generation of EFI capsule with version support"""
data = self._DoReadFile('309_capsule_version.dts')
self._CheckCapsule(data, version_check=True)
- def testCapsuleGenSignedVer(self):
"""Test generation of signed EFI capsule with version information"""
data = tools.read_file(self.TestFile("key.key"))
self._MakeInputFile("key.key", data)
data = tools.read_file(self.TestFile("key.pem"))
self._MakeInputFile("key.crt", data)
data = self._DoReadFile('310_capsule_signed_ver.dts')
self._CheckCapsule(data, signed_capsule=True, version_check=True)
- def testCapsuleGenCapOemFlags(self):
"""Test generation of EFI capsule with OEM Flags set"""
data = self._DoReadFile('311_capsule_oemflags.dts')
self._CheckCapsule(data, capoemflags=True)
- def testCapsuleGenKeyMissing(self):
"""Test that binman errors out on missing key"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('312_capsule_missing_key.dts')
self.assertIn("Both private key and public key certificate need to be provided",
str(e.exception))
- def testCapsuleGenIndexMissing(self):
"""Test that binman errors out on missing image index"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('313_capsule_missing_index.dts')
self.assertIn("entry is missing properties: image-index",
str(e.exception))
- def testCapsuleGenGuidMissing(self):
"""Test that binman errors out on missing image GUID"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('314_capsule_missing_guid.dts')
self.assertIn("entry is missing properties: image-type-id",
str(e.exception))
- def testCapsuleGenPayloadMissing(self):
"""Test that binman errors out on missing input(payload)image"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('315_capsule_missing_payload.dts')
self.assertIn("The capsule entry expects a single subnode for payload",
str(e.exception))
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/307_capsule.dts b/tools/binman/test/307_capsule.dts new file mode 100644 index 0000000000..dd5b6f1c11 --- /dev/null +++ b/tools/binman/test/307_capsule.dts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;
#size-cells = <1>;
binman {
efi-capsule {
image-index = <0x1>;
/* Image GUID for testing capsule update */
image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
Please create a #include files for these (and use lower case). Please fix globally.
Okay. I am adding a file sandbox_efi_capsule.h which contains information related to the capsule GUIDs and input files needed for EFI capsule update functionality.
hardware-instance = <0x0>;
blob-ext {
filename = "efi_capsule_payload.bin";
Do you need this filename?
Yes, this is the payload that will be used in generating the capsule.
-sughosh
};
};
};
+};
[..]
Regards, Simon

Hi Sughosh,
On Thu, 3 Aug 2023 at 05:08, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 2 Aug 2023 at 18:23, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Add support for the oemflag parameter used in FWU A/B updates. This was missed in the earlier version.
- Use a single function, generate_capsule in the mkeficapsule bintool, instead of the multiple functions in earlier version.
- Remove the logic for generating capsules from config file as suggested by Simon.
- Use required_props for image index and GUID parameters.
- Use a subnode for the capsule payload instead of using a filename for the payload, as suggested by Simon.
- Add a capsule generation test with oemflag parameter being passed.
configs/sandbox_spl_defconfig | 1 +
move to a later commit
Okay. I think it'd be better to have this change before adding support for efi_capsule entry in binman.
OK, I see that it is currently only built for tools-only and a smattering of other boards. Tools should be built for all boards. I suppose for now you can create a patch to enable it for all sandbox boards, e.g. 'default y if SANDBOX' in the Kconfing, in a patch before this one.
tools/binman/btool/mkeficapsule.py | 101 +++++++++++
nit: Please split this into its own commit, with the etype in the second commit.
Okay
tools/binman/entries.rst | 64 +++++++ tools/binman/etype/efi_capsule.py | 160 ++++++++++++++++++ tools/binman/ftest.py | 122 +++++++++++++ tools/binman/test/307_capsule.dts | 21 +++ tools/binman/test/308_capsule_signed.dts | 23 +++ tools/binman/test/309_capsule_version.dts | 22 +++ tools/binman/test/310_capsule_signed_ver.dts | 24 +++ tools/binman/test/311_capsule_oemflags.dts | 22 +++ tools/binman/test/312_capsule_missing_key.dts | 22 +++ .../binman/test/313_capsule_missing_index.dts | 20 +++ .../binman/test/314_capsule_missing_guid.dts | 19 +++ .../test/315_capsule_missing_payload.dts | 17 ++ 14 files changed, 638 insertions(+) create mode 100644 tools/binman/btool/mkeficapsule.py create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/307_capsule.dts create mode 100644 tools/binman/test/308_capsule_signed.dts create mode 100644 tools/binman/test/309_capsule_version.dts create mode 100644 tools/binman/test/310_capsule_signed_ver.dts create mode 100644 tools/binman/test/311_capsule_oemflags.dts create mode 100644 tools/binman/test/312_capsule_missing_key.dts create mode 100644 tools/binman/test/313_capsule_missing_index.dts create mode 100644 tools/binman/test/314_capsule_missing_guid.dts create mode 100644 tools/binman/test/315_capsule_missing_payload.dts
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 8d50162b27..65223475ab 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -249,3 +249,4 @@ CONFIG_UNIT_TEST=y CONFIG_SPL_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y +CONFIG_TOOLS_MKEFICAPSULE=y
[..]
- def ReadEntries(self):
"""Read the subnode to get the payload for this capsule"""
# We can have a single payload per capsule
if len(self._node.subnodes) != 1:
self.Raise('The capsule entry expects a single subnode for payload')
No, we mustn't restruit this. Why would we?
The mkeficapsule tool that generates the capsule expects a single image as it's input payload. Why do you see the need to support multiple entries as payload?
That's how binman works...it isn't the entry type's job to decide how the image is laid out. The user could add a section in any cases and get around any restriction you put here. But the restriction isn't necessary and is just strange.
mkeficapsule [options] <image blob> <output file>
for node in self._node.subnodes:
entry = Entry.Create(self, node)
entry.ReadNode()
self._entries[entry.name] = entry
- def _GenCapsule(self):
return self.mkeficapsule.generate_capsule(self.image_index,
self.image_guid,
self.hardware_instance,
self.payload,
self.capsule_fname,
self.private_key,
self.pub_key_cert,
self.monotonic_count,
self.fw_version,
self.capoemflags)
- def BuildSectionData(self, required):
if self.auth:
if not os.path.isabs(self.private_key):
self.private_key = tools.get_input_filename(self.private_key)
if not os.path.isabs(self.pub_key_cert):
self.pub_key_cert = tools.get_input_filename(self.pub_key_cert)
These should be local vars, not class vars.
?
These are properties being passed by the user. Why should only these be local vars?
Not really.
self.private_key is a string, but here you turn it into an input filename. You are changing the original property. Just use a local var.
Also these are properties passed in by the user, so you should not be converting them to filenames.
These properties are actually paths to the private and public key cert files that a user will pass. In the normal usage, the user would be expected to provide the absolute path to the private and public key cert files. The above logic is needed for binman tests and out of tree builds with relative paths.
I'm fine with the logic, just don't update the property.
data, self.payload, _ = self.collect_contents_to_file(
Please don't add new things to the class in the middle of the class. Doesn't pylint warn about this? This can just be a local var.
Changed this to a local var.
self._entries.values(), 'capsule_payload')
outfile = self._filename if self._filename else self._node.name
self.capsule_fname = tools.get_output_filename(outfile)
No, the class is for holding properties...you don't use this outside the function, so just:
[..]
- def ProcessContents(self):
ok = super().ProcessContents()
data = self.BuildSectionData(True)
ok2 = self.ProcessContentsUpdate(data)
return ok and ok2
If that is the same as the entry_Section function, you can drop it
There are differences, so this is needed.
Why is it different? Deleting the code has no effect on the tests that I can see.
[..]
payload_data = EFI_CAPSULE_DATA
# Firmware Management Protocol(FMP) GUID - offset(0 - 32)
self.assertEqual(FW_MGMT_GUID, data.hex()[:32])
# Image GUID - offset(96 - 128)
self.assertEqual(CAPSULE_IMAGE_GUID, data.hex()[96:128])
if capoemflags:
# OEM Flags - offset(40 - 44)
self.assertEqual(oemflag, data.hex()[40:44])
if signed_capsule and version_check:
# FMP header signature - offset(4770 - 4778)
self.assertEqual(fmp_signature, data.hex()[4770:4778])
# FMP header size - offset(4778 - 4780)
Can you mention where these values came from and how you created them? This all seems very brittle. Is there a tool that prints them out, or..?
Like I mentioned earlier as well, the contents of a capsule are dependent on the input parameters specified in it's generation. For e.g. the capsule generated would be different when opting for passing the version param. Similarly when generating a signed capsule -- even with a signed capsule, the contents would depend on the keys. The offsets therefore are not fixed but depend on the inputs specified. I have added comments here for better understanding. Not sure what can be done to improve this.
This is also one reason why the testing of the EFI capsule update feature should use capsules generated as part of the sandbox platform build. That makes the testing that much more robust.
We have the same problem with mkimage. Is there a 'dump_capsule' command that can output information about it? Then you could use that in this test.
self.assertEqual(fmp_size, data.hex()[4778:4780])
# firmware version - offset(4786 - 4788)
self.assertEqual(fmp_fw_version, data.hex()[4786:4788])
# payload offset signed capsule(4802 - 4808)
self.assertEqual(payload_data.hex(), data.hex()[4802:4808])
elif signed_capsule:
# payload offset signed capsule(4770 - 4776)
self.assertEqual(payload_data.hex(), data.hex()[4770:4776])
elif version_check:
# FMP header signature - offset(184 - 192)
self.assertEqual(fmp_signature, data.hex()[184:192])
# FMP header size - offset(192 - 194)
self.assertEqual(fmp_size, data.hex()[192:194])
# firmware version - offset(200 - 202)
self.assertEqual(fmp_fw_version, data.hex()[200:202])
# payload offset for non-signed capsule with version header(216 - 222)
[..]
hardware-instance = <0x0>;
blob-ext {
filename = "efi_capsule_payload.bin";
Do you need this filename?
Yes, this is the payload that will be used in generating the capsule.
Oh, this is so confusing. I think you can just make this a 'blob' instead of a 'blob-ext'. How about capsule_input.bin as it is shorter?
[..]
Regards, Simon

hi Simon,
On Fri, 4 Aug 2023 at 08:32, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Thu, 3 Aug 2023 at 05:08, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 2 Aug 2023 at 18:23, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Add support for the oemflag parameter used in FWU A/B updates. This was missed in the earlier version.
- Use a single function, generate_capsule in the mkeficapsule bintool, instead of the multiple functions in earlier version.
- Remove the logic for generating capsules from config file as suggested by Simon.
- Use required_props for image index and GUID parameters.
- Use a subnode for the capsule payload instead of using a filename for the payload, as suggested by Simon.
- Add a capsule generation test with oemflag parameter being passed.
configs/sandbox_spl_defconfig | 1 +
move to a later commit
Okay. I think it'd be better to have this change before adding support for efi_capsule entry in binman.
OK, I see that it is currently only built for tools-only and a smattering of other boards. Tools should be built for all boards. I suppose for now you can create a patch to enable it for all sandbox boards, e.g. 'default y if SANDBOX' in the Kconfing, in a patch before this one.
Will do.
tools/binman/btool/mkeficapsule.py | 101 +++++++++++
nit: Please split this into its own commit, with the etype in the second commit.
Okay
tools/binman/entries.rst | 64 +++++++ tools/binman/etype/efi_capsule.py | 160 ++++++++++++++++++ tools/binman/ftest.py | 122 +++++++++++++ tools/binman/test/307_capsule.dts | 21 +++ tools/binman/test/308_capsule_signed.dts | 23 +++ tools/binman/test/309_capsule_version.dts | 22 +++ tools/binman/test/310_capsule_signed_ver.dts | 24 +++ tools/binman/test/311_capsule_oemflags.dts | 22 +++ tools/binman/test/312_capsule_missing_key.dts | 22 +++ .../binman/test/313_capsule_missing_index.dts | 20 +++ .../binman/test/314_capsule_missing_guid.dts | 19 +++ .../test/315_capsule_missing_payload.dts | 17 ++ 14 files changed, 638 insertions(+) create mode 100644 tools/binman/btool/mkeficapsule.py create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/307_capsule.dts create mode 100644 tools/binman/test/308_capsule_signed.dts create mode 100644 tools/binman/test/309_capsule_version.dts create mode 100644 tools/binman/test/310_capsule_signed_ver.dts create mode 100644 tools/binman/test/311_capsule_oemflags.dts create mode 100644 tools/binman/test/312_capsule_missing_key.dts create mode 100644 tools/binman/test/313_capsule_missing_index.dts create mode 100644 tools/binman/test/314_capsule_missing_guid.dts create mode 100644 tools/binman/test/315_capsule_missing_payload.dts
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 8d50162b27..65223475ab 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -249,3 +249,4 @@ CONFIG_UNIT_TEST=y CONFIG_SPL_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y +CONFIG_TOOLS_MKEFICAPSULE=y
[..]
- def ReadEntries(self):
"""Read the subnode to get the payload for this capsule"""
# We can have a single payload per capsule
if len(self._node.subnodes) != 1:
self.Raise('The capsule entry expects a single subnode for payload')
No, we mustn't restruit this. Why would we?
The mkeficapsule tool that generates the capsule expects a single image as it's input payload. Why do you see the need to support multiple entries as payload?
That's how binman works...it isn't the entry type's job to decide how the image is laid out. The user could add a section in any cases and get around any restriction you put here. But the restriction isn't necessary and is just strange.
The idea here is that if a user puts two entries, there is no clarity on how that is to be handled, since the mkeficapsule tool expects a single input image(binary in binman jargon). I am aware of the user using section type to club multiple images, and I think that is absolutely fine, since we are then left with one final binary to present to the mkeficapsule tool. In fact, I think if the user wants a combination of multiple binaries, they can use a section entry. In fact a user can even use the recently added templates in binman to add stuff to the input binary, but the final binary to be passed to the mkeficapsule tool should be one.
mkeficapsule [options] <image blob> <output file>
for node in self._node.subnodes:
entry = Entry.Create(self, node)
entry.ReadNode()
self._entries[entry.name] = entry
- def _GenCapsule(self):
return self.mkeficapsule.generate_capsule(self.image_index,
self.image_guid,
self.hardware_instance,
self.payload,
self.capsule_fname,
self.private_key,
self.pub_key_cert,
self.monotonic_count,
self.fw_version,
self.capoemflags)
- def BuildSectionData(self, required):
if self.auth:
if not os.path.isabs(self.private_key):
self.private_key = tools.get_input_filename(self.private_key)
if not os.path.isabs(self.pub_key_cert):
self.pub_key_cert = tools.get_input_filename(self.pub_key_cert)
These should be local vars, not class vars.
?
These are properties being passed by the user. Why should only these be local vars?
Not really.
self.private_key is a string, but here you turn it into an input filename. You are changing the original property. Just use a local var.
Okay, understood.
Also these are properties passed in by the user, so you should not be converting them to filenames.
These properties are actually paths to the private and public key cert files that a user will pass. In the normal usage, the user would be expected to provide the absolute path to the private and public key cert files. The above logic is needed for binman tests and out of tree builds with relative paths.
I'm fine with the logic, just don't update the property.
Understood
data, self.payload, _ = self.collect_contents_to_file(
Please don't add new things to the class in the middle of the class. Doesn't pylint warn about this? This can just be a local var.
Changed this to a local var.
self._entries.values(), 'capsule_payload')
outfile = self._filename if self._filename else self._node.name
self.capsule_fname = tools.get_output_filename(outfile)
No, the class is for holding properties...you don't use this outside the function, so just:
[..]
- def ProcessContents(self):
ok = super().ProcessContents()
data = self.BuildSectionData(True)
ok2 = self.ProcessContentsUpdate(data)
return ok and ok2
If that is the same as the entry_Section function, you can drop it
There are differences, so this is needed.
Why is it different? Deleting the code has no effect on the tests that I can see.
Umm, let me check this, but a cursory glance showed differences. This is in fact taken from the mkimage.py entry.
[..]
payload_data = EFI_CAPSULE_DATA
# Firmware Management Protocol(FMP) GUID - offset(0 - 32)
self.assertEqual(FW_MGMT_GUID, data.hex()[:32])
# Image GUID - offset(96 - 128)
self.assertEqual(CAPSULE_IMAGE_GUID, data.hex()[96:128])
if capoemflags:
# OEM Flags - offset(40 - 44)
self.assertEqual(oemflag, data.hex()[40:44])
if signed_capsule and version_check:
# FMP header signature - offset(4770 - 4778)
self.assertEqual(fmp_signature, data.hex()[4770:4778])
# FMP header size - offset(4778 - 4780)
Can you mention where these values came from and how you created them? This all seems very brittle. Is there a tool that prints them out, or..?
Like I mentioned earlier as well, the contents of a capsule are dependent on the input parameters specified in it's generation. For e.g. the capsule generated would be different when opting for passing the version param. Similarly when generating a signed capsule -- even with a signed capsule, the contents would depend on the keys. The offsets therefore are not fixed but depend on the inputs specified. I have added comments here for better understanding. Not sure what can be done to improve this.
This is also one reason why the testing of the EFI capsule update feature should use capsules generated as part of the sandbox platform build. That makes the testing that much more robust.
We have the same problem with mkimage. Is there a 'dump_capsule' command that can output information about it? Then you could use that in this test.
We currently do not have support in the mkeficapsule tool to dump the capsule contents. I can take a stab at it later. I would say, for now, let's get this in. Once I have made changes to the capsule generation tool, I will come back and change it.
self.assertEqual(fmp_size, data.hex()[4778:4780])
# firmware version - offset(4786 - 4788)
self.assertEqual(fmp_fw_version, data.hex()[4786:4788])
# payload offset signed capsule(4802 - 4808)
self.assertEqual(payload_data.hex(), data.hex()[4802:4808])
elif signed_capsule:
# payload offset signed capsule(4770 - 4776)
self.assertEqual(payload_data.hex(), data.hex()[4770:4776])
elif version_check:
# FMP header signature - offset(184 - 192)
self.assertEqual(fmp_signature, data.hex()[184:192])
# FMP header size - offset(192 - 194)
self.assertEqual(fmp_size, data.hex()[192:194])
# firmware version - offset(200 - 202)
self.assertEqual(fmp_fw_version, data.hex()[200:202])
# payload offset for non-signed capsule with version header(216 - 222)
[..]
hardware-instance = <0x0>;
blob-ext {
filename = "efi_capsule_payload.bin";
Do you need this filename?
Yes, this is the payload that will be used in generating the capsule.
Oh, this is so confusing. I think you can just make this a 'blob' instead of a 'blob-ext'. How about capsule_input.bin as it is shorter?
Heh, not sure what is confusing. This is a blob which is specifying the name of the image(binary) which goes into the capsule. If you think the name is rather long, I will make it shorter as per your suggestion.
-sughosh

Hi Sughosh,
On Fri, 4 Aug 2023 at 00:44, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Fri, 4 Aug 2023 at 08:32, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Thu, 3 Aug 2023 at 05:08, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 2 Aug 2023 at 18:23, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Add support for the oemflag parameter used in FWU A/B updates. This was missed in the earlier version.
- Use a single function, generate_capsule in the mkeficapsule bintool, instead of the multiple functions in earlier version.
- Remove the logic for generating capsules from config file as suggested by Simon.
- Use required_props for image index and GUID parameters.
- Use a subnode for the capsule payload instead of using a filename for the payload, as suggested by Simon.
- Add a capsule generation test with oemflag parameter being passed.
configs/sandbox_spl_defconfig | 1 +
[..]
[..]
- def ReadEntries(self):
"""Read the subnode to get the payload for this capsule"""
# We can have a single payload per capsule
if len(self._node.subnodes) != 1:
self.Raise('The capsule entry expects a single subnode for payload')
No, we mustn't restruit this. Why would we?
The mkeficapsule tool that generates the capsule expects a single image as it's input payload. Why do you see the need to support multiple entries as payload?
That's how binman works...it isn't the entry type's job to decide how the image is laid out. The user could add a section in any cases and get around any restriction you put here. But the restriction isn't necessary and is just strange.
The idea here is that if a user puts two entries, there is no clarity on how that is to be handled, since the mkeficapsule tool expects a single input image(binary in binman jargon). I am aware of the user
There is a single input, since binman collects everything together.
using section type to club multiple images, and I think that is absolutely fine, since we are then left with one final binary to present to the mkeficapsule tool. In fact, I think if the user wants a combination of multiple binaries, they can use a section entry. In fact a user can even use the recently added templates in binman to add stuff to the input binary, but the final binary to be passed to the mkeficapsule tool should be one.
Is there some strange magic in the tool? What does it matter where the data come from?
I really don't understand what breaks if the user adds two entries here. We should not have this limitation.
[..]
[..]
payload_data = EFI_CAPSULE_DATA
# Firmware Management Protocol(FMP) GUID - offset(0 - 32)
self.assertEqual(FW_MGMT_GUID, data.hex()[:32])
# Image GUID - offset(96 - 128)
self.assertEqual(CAPSULE_IMAGE_GUID, data.hex()[96:128])
if capoemflags:
# OEM Flags - offset(40 - 44)
self.assertEqual(oemflag, data.hex()[40:44])
if signed_capsule and version_check:
# FMP header signature - offset(4770 - 4778)
self.assertEqual(fmp_signature, data.hex()[4770:4778])
# FMP header size - offset(4778 - 4780)
Can you mention where these values came from and how you created them? This all seems very brittle. Is there a tool that prints them out, or..?
Like I mentioned earlier as well, the contents of a capsule are dependent on the input parameters specified in it's generation. For e.g. the capsule generated would be different when opting for passing the version param. Similarly when generating a signed capsule -- even with a signed capsule, the contents would depend on the keys. The offsets therefore are not fixed but depend on the inputs specified. I have added comments here for better understanding. Not sure what can be done to improve this.
This is also one reason why the testing of the EFI capsule update feature should use capsules generated as part of the sandbox platform build. That makes the testing that much more robust.
We have the same problem with mkimage. Is there a 'dump_capsule' command that can output information about it? Then you could use that in this test.
We currently do not have support in the mkeficapsule tool to dump the capsule contents. I can take a stab at it later. I would say, for now, let's get this in. Once I have made changes to the capsule generation tool, I will come back and change it.
OK, I think that is a good path.
self.assertEqual(fmp_size, data.hex()[4778:4780])
# firmware version - offset(4786 - 4788)
self.assertEqual(fmp_fw_version, data.hex()[4786:4788])
# payload offset signed capsule(4802 - 4808)
self.assertEqual(payload_data.hex(), data.hex()[4802:4808])
elif signed_capsule:
# payload offset signed capsule(4770 - 4776)
self.assertEqual(payload_data.hex(), data.hex()[4770:4776])
elif version_check:
# FMP header signature - offset(184 - 192)
self.assertEqual(fmp_signature, data.hex()[184:192])
# FMP header size - offset(192 - 194)
self.assertEqual(fmp_size, data.hex()[192:194])
# firmware version - offset(200 - 202)
self.assertEqual(fmp_fw_version, data.hex()[200:202])
# payload offset for non-signed capsule with version header(216 - 222)
[..]
hardware-instance = <0x0>;
blob-ext {
filename = "efi_capsule_payload.bin";
Do you need this filename?
Yes, this is the payload that will be used in generating the capsule.
Oh, this is so confusing. I think you can just make this a 'blob' instead of a 'blob-ext'. How about capsule_input.bin as it is shorter?
Heh, not sure what is confusing. This is a blob which is specifying the name of the image(binary) which goes into the capsule. If you think the name is rather long, I will make it shorter as per your suggestion.
Well making it blob-ext was suggesting there was some external thing that might not be available. If you change it to 'blob' it is easier.
Yes 'input' is better as 'payload' sounds like it is the capsule update file.
Regards, Smion

The EFI capsules can now be generated as part of u-boot build, through binman. Highlight these changes in the documentation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V5: * Remove the documentation for generating the capsule through config file, as that functionality is not added through this series.
doc/develop/uefi/uefi.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index b2854b52a6..3150d6fb9c 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -318,6 +318,9 @@ Run the following command --guid <image GUID> \ <capsule_file_name>
+Capsule with firmware version +***************************** + The UEFI specification does not define the firmware versioning mechanism. EDK II reference implementation inserts the FMP Payload Header right before the payload. It coutains the fw_version and lowest supported version, @@ -345,6 +348,21 @@ add --fw-version option in mkeficapsule tool. If the --fw-version option is not set, FMP Payload Header is not inserted and fw_version is set as 0.
+ +Capsule Generation through binman +********************************* + +Support has also been added to generate capsules during u-boot build +through binman. This requires the platform's DTB to be populated with +the capsule entry nodes for binman. The capsules then can be generated +by specifying the capsule parameters either through a config file, or +by specifying them as properties in the capsule entry node. + +Check the arch/sandbox/dts/u-boot.dtsi file for the sandbox platform +as reference for how to generate capsules through binman as part of +u-boot build. + + Performing the update *********************

The embedding of the public key EFI Signature List(ESL) file into the platform's DTB is now done at the time of u-boot build. Remove this logic from the capsule update test' configuration.
Include the public key for the sandbox and sandbox_flattree variant as part of the build.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V5: * Use the public key ESL file from the tree instead of the /tmp/capsules/ directory being used in previous version.
configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + test/py/tests/test_efi_capsule/conftest.py | 37 ++++---------------- test/py/tests/test_efi_capsule/signature.dts | 10 ------ 4 files changed, 9 insertions(+), 40 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/signature.dts
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index b6c4f735f2..3d9e59a11d 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y CONFIG_EFI_CAPSULE_ON_DISK=y CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y CONFIG_EFI_CAPSULE_AUTHENTICATE=y +CONFIG_EFI_CAPSULE_ESL_FILE="../../../test/py/tests/test_efi_capsule/test_files/SIGNER.esl" CONFIG_EFI_SECURE_BOOT=y CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 8aa295686d..325e162ad9 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -227,6 +227,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y CONFIG_EFI_CAPSULE_ON_DISK=y CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y CONFIG_EFI_CAPSULE_AUTHENTICATE=y +CONFIG_EFI_CAPSULE_ESL_FILE="../../../test/py/tests/test_efi_capsule/test_files/SIGNER.esl" CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py index 054be1ee97..bee3050282 100644 --- a/test/py/tests/test_efi_capsule/conftest.py +++ b/test/py/tests/test_efi_capsule/conftest.py @@ -25,48 +25,25 @@ def efi_capsule_data(request, u_boot_config): image_path = u_boot_config.persistent_data_dir + '/test_efi_capsule.img'
try: + capsules_path_dir = u_boot_config.source_dir + '/test/py/tests/test_efi_capsule/test_files/' # Create a target device check_call('dd if=/dev/zero of=./spi.bin bs=1MiB count=16', shell=True)
check_call('rm -rf %s' % mnt_point, shell=True) check_call('mkdir -p %s' % data_dir, shell=True) check_call('mkdir -p %s' % install_dir, shell=True) - - capsule_auth_enabled = u_boot_config.buildconfig.get( - 'config_efi_capsule_authenticate') - if capsule_auth_enabled: - # Create private key (SIGNER.key) and certificate (SIGNER.crt) - check_call('cd %s; ' - 'openssl req -x509 -sha256 -newkey rsa:2048 ' - '-subj /CN=TEST_SIGNER/ -keyout SIGNER.key ' - '-out SIGNER.crt -nodes -days 365' - % data_dir, shell=True) - check_call('cd %s; %scert-to-efi-sig-list SIGNER.crt SIGNER.esl' - % (data_dir, EFITOOLS_PATH), shell=True) - - # Update dtb adding capsule certificate - check_call('cd %s; ' - 'cp %s/test/py/tests/test_efi_capsule/signature.dts .' - % (data_dir, u_boot_config.source_dir), shell=True) - check_call('cd %s; ' - 'dtc -@ -I dts -O dtb -o signature.dtbo signature.dts; ' - 'fdtoverlay -i %s/arch/sandbox/dts/test.dtb ' - '-o test_sig.dtb signature.dtbo' - % (data_dir, u_boot_config.build_dir), shell=True) - - # Create *malicious* private key (SIGNER2.key) and certificate - # (SIGNER2.crt) - check_call('cd %s; ' - 'openssl req -x509 -sha256 -newkey rsa:2048 ' - '-subj /CN=TEST_SIGNER/ -keyout SIGNER2.key ' - '-out SIGNER2.crt -nodes -days 365' - % data_dir, shell=True) + check_call('cp %s/* %s ' % (capsules_path_dir, data_dir), shell=True)
# Update dtb to add the version information check_call('cd %s; ' 'cp %s/test/py/tests/test_efi_capsule/version.dts .' % (data_dir, u_boot_config.source_dir), shell=True) + + capsule_auth_enabled = u_boot_config.buildconfig.get( + 'config_efi_capsule_authenticate') if capsule_auth_enabled: + check_call('cp %s/arch/sandbox/dts/test.dtb %s/test_sig.dtb' % + (u_boot_config.build_dir, data_dir), shell=True) check_call('cd %s; ' 'dtc -@ -I dts -O dtb -o version.dtbo version.dts; ' 'fdtoverlay -i test_sig.dtb ' diff --git a/test/py/tests/test_efi_capsule/signature.dts b/test/py/tests/test_efi_capsule/signature.dts deleted file mode 100644 index 078cfc76c9..0000000000 --- a/test/py/tests/test_efi_capsule/signature.dts +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ - -/dts-v1/; -/plugin/; - -&{/} { - signature { - capsule-key = /incbin/("SIGNER.esl"); - }; -};

The EFI capsule files can now be generated as part of u-boot build. This is done through binman. Add capsule entry nodes in the u-boot.dtsi for the sandbox architecture for generating the capsules. Remove the corresponding generation of capsules from the capsule update conftest file.
The capsules are generated through the config file for the sandbox variant, and through explicit parameters for the sandbox_flattree variant.
Also generate the FIT image used for testing the capsule update feature on the sandbox_flattree variant through binman. Remove the now superfluous its file which was used for generating this FIT image.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V5: * Use the public key ESL file and other input files from the tree instead of the /tmp/capsules/ directory being used in previous version. * Use macros for other input files and certs.
arch/sandbox/dts/u-boot.dtsi | 347 ++++++++++++++++++ test/py/tests/test_efi_capsule/conftest.py | 128 +------ .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 3 files changed, 348 insertions(+), 163 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi index 60bd004937..ae798660de 100644 --- a/arch/sandbox/dts/u-boot.dtsi +++ b/arch/sandbox/dts/u-boot.dtsi @@ -7,11 +7,358 @@ */
#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT + +#define SANDBOX_UBOOT_IMAGE_GUID "09d7cf52-0720-4710-91d1-08469b7fe9c8" +#define SANDBOX_UBOOT_ENV_IMAGE_GUID "5a7021f5-fef2-48b4-aaba-832e777418c0" +#define SANDBOX_FIT_IMAGE_GUID "3673b45d-6a7c-46f3-9e60-adabb03f7937" +#define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4" + +#define UBOOT_BIN_IMAGE "test/py/tests/test_efi_capsule/test_files/u-boot.bin.new" +#define UBOOT_ENV_IMAGE "test/py/tests/test_efi_capsule/test_files/u-boot.env.new" +#define UBOOT_FIT_IMAGE "u-boot_bin_env.itb" + +#define CAPSULE_PRIV_KEY "test/py/tests/test_efi_capsule/test_files/SIGNER.key" +#define CAPSULE_PUB_KEY "test/py/tests/test_efi_capsule/test_files/SIGNER.crt" +#define CAPSULE_INVAL_KEY "test/py/tests/test_efi_capsule/test_files/SIGNER2.key" +#define CAPSULE_INVAL_PUB_KEY "test/py/tests/test_efi_capsule/test_files/SIGNER2.crt" + / { #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE signature { capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); }; #endif + + binman: binman { + multiple-images; + }; +}; + +&binman { + itb { + filename = UBOOT_FIT_IMAGE; + + fit { + description = "Automatic U-Boot environment update"; + #address-cells = <2>; + + images { + u-boot-bin { + description = "U-Boot binary on SPI Flash"; + compression = "none"; + type = "firmware"; + arch = "sandbox"; + load = <0>; + blob { + filename = UBOOT_BIN_IMAGE; + }; + + hash-1 { + algo = "sha1"; + }; + }; + u-boot-env { + description = "U-Boot environment on SPI Flash"; + compression = "none"; + type = "firmware"; + arch = "sandbox"; + load = <0>; + blob { + filename = UBOOT_ENV_IMAGE; + }; + + hash-1 { + algo = "sha1"; + }; + }; + }; + }; + }; + + capsule1 { + filename = "Test01"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + + blob-ext { + filename = UBOOT_BIN_IMAGE; + }; + }; + }; + + capsule2 { + filename = "Test02"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; + + blob-ext { + filename = UBOOT_ENV_IMAGE; + }; + }; + }; + + capsule3 { + filename = "Test03"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_INCORRECT_GUID; + + blob-ext { + filename = UBOOT_BIN_IMAGE; + }; + }; + }; + + capsule4 { + filename = "Test04"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + + blob-ext { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule5 { + filename = "Test05"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_INCORRECT_GUID; + + blob-ext { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule6 { + filename = "Test101"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + + blob-ext { + filename = UBOOT_BIN_IMAGE; + }; + }; + }; + + capsule7 { + filename = "Test102"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + fw-version = <0xa>; + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; + + blob-ext { + filename = UBOOT_ENV_IMAGE; + }; + }; + }; + + capsule8 { + filename = "Test103"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + + blob-ext { + filename = UBOOT_BIN_IMAGE; + }; + }; + }; + + capsule9 { + filename = "Test104"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + + blob-ext { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule10 { + filename = "Test105"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + + blob-ext { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE + capsule11 { + filename = "Test11"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + pub-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_BIN_IMAGE; + }; + }; + }; + + capsule12 { + filename = "Test12"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_INVAL_KEY; + pub-key-cert = CAPSULE_INVAL_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_BIN_IMAGE; + }; + }; + }; + + capsule13 { + filename = "Test13"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + pub-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule14 { + filename = "Test14"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + private-key = CAPSULE_INVAL_KEY; + pub-key-cert = CAPSULE_INVAL_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule15 { + filename = "Test111"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + pub-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_BIN_IMAGE; + }; + }; + }; + + capsule16 { + filename = "Test112"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + fw-version = <0xa>; + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + pub-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_ENV_IMAGE; + }; + }; + }; + + capsule17 { + filename = "Test113"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + pub-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_BIN_IMAGE; + }; + }; + }; + + capsule18 { + filename = "Test114"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + pub-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule19 { + filename = "Test115"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + pub-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob-ext { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */ }; #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py index bee3050282..bc0f188609 100644 --- a/test/py/tests/test_efi_capsule/conftest.py +++ b/test/py/tests/test_efi_capsule/conftest.py @@ -33,6 +33,7 @@ def efi_capsule_data(request, u_boot_config): check_call('mkdir -p %s' % data_dir, shell=True) check_call('mkdir -p %s' % install_dir, shell=True) check_call('cp %s/* %s ' % (capsules_path_dir, data_dir), shell=True) + check_call('cp %s/Test* %s ' % (u_boot_config.build_dir, data_dir), shell=True)
# Update dtb to add the version information check_call('cd %s; ' @@ -56,133 +57,6 @@ def efi_capsule_data(request, u_boot_config): '-o test_ver.dtb version.dtbo' % (data_dir, u_boot_config.build_dir), shell=True)
- # Create capsule files - # two regions: one for u-boot.bin and the other for u-boot.env - check_call('cd %s; echo -n u-boot:Old > u-boot.bin.old; echo -n u-boot:New > u-boot.bin.new; echo -n u-boot-env:Old > u-boot.env.old; echo -n u-boot-env:New > u-boot.env.new' % data_dir, - shell=True) - check_call('sed -e "s?BINFILE1?u-boot.bin.new?" -e "s?BINFILE2?u-boot.env.new?" %s/test/py/tests/test_efi_capsule/uboot_bin_env.its > %s/uboot_bin_env.its' % - (u_boot_config.source_dir, data_dir), - shell=True) - check_call('cd %s; %s/tools/mkimage -f uboot_bin_env.its uboot_bin_env.itb' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 u-boot.bin.new Test01' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 2 --guid 5A7021F5-FEF2-48B4-AABA-832E777418C0 u-boot.env.new Test02' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 058B7D83-50D5-4C47-A195-60D86AD341C4 u-boot.bin.new Test03' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 uboot_bin_env.itb Test04' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 058B7D83-50D5-4C47-A195-60D86AD341C4 uboot_bin_env.itb Test05' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --fw-version 5 ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 u-boot.bin.new Test101' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 2 --fw-version 10 ' - '--guid 5A7021F5-FEF2-48B4-AABA-832E777418C0 u-boot.env.new Test102' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --fw-version 2 ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 u-boot.bin.new Test103' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --fw-version 5 ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 uboot_bin_env.itb Test104' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --fw-version 2 ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 uboot_bin_env.itb Test105' % - (data_dir, u_boot_config.build_dir), - shell=True) - - if capsule_auth_enabled: - # raw firmware signed with proper key - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 ' - 'u-boot.bin.new Test11' - % (data_dir, u_boot_config.build_dir), - shell=True) - # raw firmware signed with *mal* key - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--private-key SIGNER2.key ' - '--certificate SIGNER2.crt ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 ' - 'u-boot.bin.new Test12' - % (data_dir, u_boot_config.build_dir), - shell=True) - # FIT firmware signed with proper key - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 ' - 'uboot_bin_env.itb Test13' - % (data_dir, u_boot_config.build_dir), - shell=True) - # FIT firmware signed with *mal* key - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--private-key SIGNER2.key ' - '--certificate SIGNER2.crt ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 ' - 'uboot_bin_env.itb Test14' - % (data_dir, u_boot_config.build_dir), - shell=True) - # raw firmware signed with proper key with version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--fw-version 5 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 ' - 'u-boot.bin.new Test111' - % (data_dir, u_boot_config.build_dir), - shell=True) - # raw firmware signed with proper key with version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 2 --monotonic-count 1 ' - '--fw-version 10 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 5A7021F5-FEF2-48B4-AABA-832E777418C0 ' - 'u-boot.env.new Test112' - % (data_dir, u_boot_config.build_dir), - shell=True) - # raw firmware signed with proper key with lower version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--fw-version 2 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 ' - 'u-boot.bin.new Test113' - % (data_dir, u_boot_config.build_dir), - shell=True) - # FIT firmware signed with proper key with version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--fw-version 5 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 ' - 'uboot_bin_env.itb Test114' - % (data_dir, u_boot_config.build_dir), - shell=True) - # FIT firmware signed with proper key with lower version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--fw-version 2 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 ' - 'uboot_bin_env.itb Test115' - % (data_dir, u_boot_config.build_dir), - shell=True) - # Create a disk image with EFI system partition check_call('virt-make-fs --partition=gpt --size=+1M --type=vfat %s %s' % (mnt_point, image_path), shell=True) diff --git a/test/py/tests/test_efi_capsule/uboot_bin_env.its b/test/py/tests/test_efi_capsule/uboot_bin_env.its deleted file mode 100644 index fc65907481..0000000000 --- a/test/py/tests/test_efi_capsule/uboot_bin_env.its +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Automatic software update for U-Boot - * Make sure the flashing addresses ('load' prop) is correct for your board! - */ - -/dts-v1/; - -/ { - description = "Automatic U-Boot environment update"; - #address-cells = <2>; - - images { - u-boot-bin { - description = "U-Boot binary on SPI Flash"; - data = /incbin/("BINFILE1"); - compression = "none"; - type = "firmware"; - arch = "sandbox"; - load = <0>; - hash-1 { - algo = "sha1"; - }; - }; - u-boot-env { - description = "U-Boot environment on SPI Flash"; - data = /incbin/("BINFILE2"); - compression = "none"; - type = "firmware"; - arch = "sandbox"; - load = <0>; - hash-1 { - algo = "sha1"; - }; - }; - }; -};

Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The EFI capsule files can now be generated as part of u-boot build. This is done through binman. Add capsule entry nodes in the u-boot.dtsi for the sandbox architecture for generating the capsules. Remove the corresponding generation of capsules from the capsule update conftest file.
The capsules are generated through the config file for the sandbox variant, and through explicit parameters for the sandbox_flattree variant.
Also generate the FIT image used for testing the capsule update feature on the sandbox_flattree variant through binman. Remove the now superfluous its file which was used for generating this FIT image.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Use the public key ESL file and other input files from the tree instead of the /tmp/capsules/ directory being used in previous version.
- Use macros for other input files and certs.
arch/sandbox/dts/u-boot.dtsi | 347 ++++++++++++++++++ test/py/tests/test_efi_capsule/conftest.py | 128 +------ .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 3 files changed, 348 insertions(+), 163 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
I want to get the binman stuff right before diving into this, but the binman stuff seems fairly close, so I'll just mention...do you really need all these combinations of tests? It seems to me that one test is enough. You know that the binman tests will protect the code there, so why test it all over again here?
Regards, Simon

hi Simon,
On Wed, 2 Aug 2023 at 18:23, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The EFI capsule files can now be generated as part of u-boot build. This is done through binman. Add capsule entry nodes in the u-boot.dtsi for the sandbox architecture for generating the capsules. Remove the corresponding generation of capsules from the capsule update conftest file.
The capsules are generated through the config file for the sandbox variant, and through explicit parameters for the sandbox_flattree variant.
Also generate the FIT image used for testing the capsule update feature on the sandbox_flattree variant through binman. Remove the now superfluous its file which was used for generating this FIT image.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Use the public key ESL file and other input files from the tree instead of the /tmp/capsules/ directory being used in previous version.
- Use macros for other input files and certs.
arch/sandbox/dts/u-boot.dtsi | 347 ++++++++++++++++++ test/py/tests/test_efi_capsule/conftest.py | 128 +------ .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 3 files changed, 348 insertions(+), 163 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
I want to get the binman stuff right before diving into this, but the binman stuff seems fairly close, so I'll just mention...do you really need all these combinations of tests? It seems to me that one test is enough. You know that the binman tests will protect the code there, so why test it all over again here?
These are capsules that are needed for testing the EFI capsule update functionality. Currently, the capsules used for testing the feature are generated after u-boot has been built. Same for embedding the public key in the dtb. I think it is better to have the same flow of generating capsules and the associated logic(public key embedding) that is being supported in u-boot rather than having two divergent flows. This also serves as an example for potential users who would want to generate capsules as part of the build flow.
-sughosh
Regards, Simon

Hi Sughosh,
On Thu, 3 Aug 2023 at 05:18, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 2 Aug 2023 at 18:23, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The EFI capsule files can now be generated as part of u-boot build. This is done through binman. Add capsule entry nodes in the u-boot.dtsi for the sandbox architecture for generating the capsules. Remove the corresponding generation of capsules from the capsule update conftest file.
The capsules are generated through the config file for the sandbox variant, and through explicit parameters for the sandbox_flattree variant.
Also generate the FIT image used for testing the capsule update feature on the sandbox_flattree variant through binman. Remove the now superfluous its file which was used for generating this FIT image.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Use the public key ESL file and other input files from the tree instead of the /tmp/capsules/ directory being used in previous version.
- Use macros for other input files and certs.
arch/sandbox/dts/u-boot.dtsi | 347 ++++++++++++++++++ test/py/tests/test_efi_capsule/conftest.py | 128 +------ .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 3 files changed, 348 insertions(+), 163 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
I want to get the binman stuff right before diving into this, but the binman stuff seems fairly close, so I'll just mention...do you really need all these combinations of tests? It seems to me that one test is enough. You know that the binman tests will protect the code there, so why test it all over again here?
These are capsules that are needed for testing the EFI capsule update functionality. Currently, the capsules used for testing the feature are generated after u-boot has been built. Same for embedding the public key in the dtb. I think it is better to have the same flow of generating capsules and the associated logic(public key embedding) that is being supported in u-boot rather than having two divergent flows. This also serves as an example for potential users who would want to generate capsules as part of the build flow.
But my question was why you need more than one test here? Are you testing that U-Boot can decode a capsule file of various types? That should be done in unit tests.
Now I see the tests you are referring to in test_capsule_firmware_signed_raw.py (please shorten the name!)
These tests all have the reboot problem we need to fix, but anyway, at least I understand it.
It looks like you are writing the test files into the source tree? They should be written to the output tree.
Regards, Simon

hi Simon,
On Fri, 4 Aug 2023 at 08:32, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Thu, 3 Aug 2023 at 05:18, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 2 Aug 2023 at 18:23, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The EFI capsule files can now be generated as part of u-boot build. This is done through binman. Add capsule entry nodes in the u-boot.dtsi for the sandbox architecture for generating the capsules. Remove the corresponding generation of capsules from the capsule update conftest file.
The capsules are generated through the config file for the sandbox variant, and through explicit parameters for the sandbox_flattree variant.
Also generate the FIT image used for testing the capsule update feature on the sandbox_flattree variant through binman. Remove the now superfluous its file which was used for generating this FIT image.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Use the public key ESL file and other input files from the tree instead of the /tmp/capsules/ directory being used in previous version.
- Use macros for other input files and certs.
arch/sandbox/dts/u-boot.dtsi | 347 ++++++++++++++++++ test/py/tests/test_efi_capsule/conftest.py | 128 +------ .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 3 files changed, 348 insertions(+), 163 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
I want to get the binman stuff right before diving into this, but the binman stuff seems fairly close, so I'll just mention...do you really need all these combinations of tests? It seems to me that one test is enough. You know that the binman tests will protect the code there, so why test it all over again here?
These are capsules that are needed for testing the EFI capsule update functionality. Currently, the capsules used for testing the feature are generated after u-boot has been built. Same for embedding the public key in the dtb. I think it is better to have the same flow of generating capsules and the associated logic(public key embedding) that is being supported in u-boot rather than having two divergent flows. This also serves as an example for potential users who would want to generate capsules as part of the build flow.
But my question was why you need more than one test here? Are you testing that U-Boot can decode a capsule file of various types? That should be done in unit tests.
The tests are the same. They are not being changed. What is changed is the stage at which the capsules are being generated. Currently, the capsules get generated only when the tests are invoked, as part of the test setup. Same for embedding of the public key cert EFI Signature List(ESL) file. This patch results in the capsules getting generated as part of the u-boot build. Same for embedding of the public key ESL. If we don't follow this flow, we would have support for generating capsules as part of the u-boot build, but that flow would not be used at all. I understand that binman tests the generation of capsules, but we would then have this divergence between the flow that is supported, and what is actually used in the tests.
One alternative, which I think is a middle ground for this would be to add a Kconfig symbol and use that for generating capsules. We can then use that symbol in CI. This is similar to how the trace testing happens in CI on the sandbox platform. In that scenario, we would not have the capsules getting generated during normal builds.
Now I see the tests you are referring to in test_capsule_firmware_signed_raw.py (please shorten the name!)
These tests all have the reboot problem we need to fix, but anyway, at least I understand it.
It looks like you are writing the test files into the source tree? They should be written to the output tree.
If we are to generate the capsules, and embed the key as part of the u-boot build, these input files are needed. Btw, I do see a few places which have input files in the source, including inside binman. What issue do you see having these in the source?
I had discussed this with Tom over irc and he had suggested this location for the files.
-sughosh

Hi Sughosh,
On Fri, 4 Aug 2023 at 01:03, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Fri, 4 Aug 2023 at 08:32, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Thu, 3 Aug 2023 at 05:18, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 2 Aug 2023 at 18:23, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The EFI capsule files can now be generated as part of u-boot build. This is done through binman. Add capsule entry nodes in the u-boot.dtsi for the sandbox architecture for generating the capsules. Remove the corresponding generation of capsules from the capsule update conftest file.
The capsules are generated through the config file for the sandbox variant, and through explicit parameters for the sandbox_flattree variant.
Also generate the FIT image used for testing the capsule update feature on the sandbox_flattree variant through binman. Remove the now superfluous its file which was used for generating this FIT image.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V5:
- Use the public key ESL file and other input files from the tree instead of the /tmp/capsules/ directory being used in previous version.
- Use macros for other input files and certs.
arch/sandbox/dts/u-boot.dtsi | 347 ++++++++++++++++++ test/py/tests/test_efi_capsule/conftest.py | 128 +------ .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 3 files changed, 348 insertions(+), 163 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
I want to get the binman stuff right before diving into this, but the binman stuff seems fairly close, so I'll just mention...do you really need all these combinations of tests? It seems to me that one test is enough. You know that the binman tests will protect the code there, so why test it all over again here?
These are capsules that are needed for testing the EFI capsule update functionality. Currently, the capsules used for testing the feature are generated after u-boot has been built. Same for embedding the public key in the dtb. I think it is better to have the same flow of generating capsules and the associated logic(public key embedding) that is being supported in u-boot rather than having two divergent flows. This also serves as an example for potential users who would want to generate capsules as part of the build flow.
But my question was why you need more than one test here? Are you testing that U-Boot can decode a capsule file of various types? That should be done in unit tests.
The tests are the same. They are not being changed. What is changed is the stage at which the capsules are being generated. Currently, the capsules get generated only when the tests are invoked, as part of the test setup. Same for embedding of the public key cert EFI Signature List(ESL) file. This patch results in the capsules getting generated as part of the u-boot build. Same for embedding of the public key ESL. If we don't follow this flow, we would have support for generating capsules as part of the u-boot build, but that flow would not be used at all. I understand that binman tests the generation of capsules, but we would then have this divergence between the flow that is supported, and what is actually used in the tests.
OK let's discuss the tests later.
One alternative, which I think is a middle ground for this would be to add a Kconfig symbol and use that for generating capsules. We can then use that symbol in CI. This is similar to how the trace testing happens in CI on the sandbox platform. In that scenario, we would not have the capsules getting generated during normal builds.
Here's what I suggest:
- rely on binman tests for capsule generation - once you have a dump_capsule tool you can use that to check that things look OK - rely on unit tests for testing decoding capsules in U-Boot - have a few functional tests as a sanity check for overall behaviour
Now I see the tests you are referring to in test_capsule_firmware_signed_raw.py (please shorten the name!)
These tests all have the reboot problem we need to fix, but anyway, at least I understand it.
It looks like you are writing the test files into the source tree? They should be written to the output tree.
If we are to generate the capsules, and embed the key as part of the u-boot build, these input files are needed. Btw, I do see a few places which have input files in the source, including inside binman. What issue do you see having these in the source?
OK so long as the tests don't write to the source tree, this is fine. Do they?
Rather than writing out the test/py/tests/xxx/xxx dir, just use the leaf name. Then you can use the input dir. If you put the certificants in board/sandbox/ that should work. Please do use 'blobl' instead of 'blob-ext' as the latter is confusing.
For the u-boot.bin.new file, it is just text, so you can use the 'text' etype that binman provides and then you don't need a file.
Basically, don't have paths in the .dtsi file. We try to make sure that the includes, etc. are correct so that everything just works.
I had discussed this with Tom over irc and he had suggested this location for the files.
It's fine for tests, but since this is now part of the build, we should not be pulling in files from there.
Regards, Simon
participants (3)
-
Simon Glass
-
Sughosh Ganu
-
Tom Rini