[PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[Ilias Apalodimas]

The capsule signature is now part of our DTB.  This is problematic when a
user is allowed to change/fixup that DTB from U-Boots command line since he
can overwrite the signature as well.
So Instead of adding the key on the DTB, embed it in the u-boot binary it
self as part of it's .rodata.  This assumes that the U-Boot binary we load
is authenticated by a previous boot stage loader.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 board/emulation/common/Makefile       |  1 -
 board/emulation/common/qemu_capsule.c | 43 ---------------------------
 include/asm-generic/sections.h        |  2 ++
 lib/efi_loader/Kconfig                |  6 ++++
 lib/efi_loader/Makefile               |  8 +++++
 lib/efi_loader/efi_capsule.c          | 18 +++++++++--
 lib/efi_loader/efi_capsule_key.S      |  8 +++++
 7 files changed, 39 insertions(+), 47 deletions(-)
 delete mode 100644 board/emulation/common/qemu_capsule.c
 create mode 100644 lib/efi_loader/efi_capsule_key.S

diff --git a/board/emulation/common/Makefile b/board/emulation/common/Makefile
index 7ed447a69dce..c5b452e7e341 100644
--- a/board/emulation/common/Makefile
+++ b/board/emulation/common/Makefile
@@ -2,4 +2,3 @@
 
 obj-$(CONFIG_SYS_MTDPARTS_RUNTIME) += qemu_mtdparts.o
 obj-$(CONFIG_SET_DFU_ALT_INFO) += qemu_dfu.o
-obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT) += qemu_capsule.o
diff --git a/board/emulation/common/qemu_capsule.c b/board/emulation/common/qemu_capsule.c
deleted file mode 100644
index 6b8a87022a4c..000000000000
--- a/board/emulation/common/qemu_capsule.c
+++ /dev/null
@@ -1,43 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright (c) 2020 Linaro Limited
- */
-
-#include <common.h>
-#include <efi_api.h>
-#include <efi_loader.h>
-#include <env.h>
-#include <fdtdec.h>
-#include <asm/global_data.h>
-
-DECLARE_GLOBAL_DATA_PTR;
-
-int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
-{
-	const void *fdt_blob = gd->fdt_blob;
-	const void *blob;
-	const char *cnode_name = "capsule-key";
-	const char *snode_name = "signature";
-	int sig_node;
-	int len;
-
-	sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);
-	if (sig_node < 0) {
-		EFI_PRINT("Unable to get signature node offset\n");
-		return -FDT_ERR_NOTFOUND;
-	}
-
-	blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len);
-
-	if (!blob || len < 0) {
-		EFI_PRINT("Unable to get capsule-key value\n");
-		*pkey = NULL;
-		*pkey_len = 0;
-		return -FDT_ERR_NOTFOUND;
-	}
-
-	*pkey = (void *)blob;
-	*pkey_len = len;
-
-	return 0;
-}
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 267f1db73f23..ec992b0c2e3f 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -27,6 +27,8 @@ extern char __efi_helloworld_begin[];
 extern char __efi_helloworld_end[];
 extern char __efi_var_file_begin[];
 extern char __efi_var_file_end[];
+extern char __efi_capsule_sig_begin[];
+extern char __efi_capsule_sig_end[];
 
 /* Private data used by of-platdata devices/uclasses */
 extern char __priv_data_start[], __priv_data_end[];
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 156b39152112..42f1292fa04b 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -213,6 +213,12 @@ config EFI_CAPSULE_AUTHENTICATE
 	  Select this option if you want to enable capsule
 	  authentication
 
+config EFI_CAPSULE_KEY_PATH
+	string "Path to .esl file for capsule authentication"
+	depends on EFI_CAPSULE_AUTHENTICATE
+	help
+	  Provide the .esl file used for capsule authentication
+
 config EFI_DEVICE_PATH_TO_TEXT
 	bool "Device path to text protocol"
 	default y
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index fd344cea29b0..9b369430e258 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -20,11 +20,19 @@ always += helloworld.efi
 targets += helloworld.o
 endif
 
+ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
+EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_KEY_PATH))
+ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
+$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_KEY_PATH)
+endif
+endif
+
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
 obj-y += efi_boottime.o
 obj-y += efi_helper.o
 obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
+obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
 obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
 obj-y += efi_console.o
 obj-y += efi_device_path.o
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index b878e71438b8..50e93cad4ee5 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -16,6 +16,7 @@
 #include <mapmem.h>
 #include <sort.h>
 
+#include <asm/sections.h>
 #include <crypto/pkcs7.h>
 #include <crypto/pkcs7_parser.h>
 #include <linux/err.h>
@@ -222,12 +223,23 @@ skip:
 const efi_guid_t efi_guid_capsule_root_cert_guid =
 	EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
 
+int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
+{
+	const void *blob = __efi_capsule_sig_begin;
+	const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
+
+	*pkey = (void *)blob;
+	*pkey_len = len;
+
+	return 0;
+}
+
 efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size,
 				      void **image, efi_uintn_t *image_size)
 {
 	u8 *buf;
 	int ret;
-	void *fdt_pkey, *pkey;
+	void *stored_pkey, *pkey;
 	efi_uintn_t pkey_len;
 	uint64_t monotonic_count;
 	struct efi_signature_store *truststore;
@@ -286,7 +298,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
 		goto out;
 	}
 
-	ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
+	ret = efi_get_public_key_data(&stored_pkey, &pkey_len);
 	if (ret < 0)
 		goto out;
 
@@ -294,7 +306,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
 	if (!pkey)
 		goto out;
 
-	memcpy(pkey, fdt_pkey, pkey_len);
+	memcpy(pkey, stored_pkey, pkey_len);
 	truststore = efi_build_signature_store(pkey, pkey_len);
 	if (!truststore)
 		goto out;
diff --git a/lib/efi_loader/efi_capsule_key.S b/lib/efi_loader/efi_capsule_key.S
new file mode 100644
index 000000000000..f7047a42e39d
--- /dev/null
+++ b/lib/efi_loader/efi_capsule_key.S
@@ -0,0 +1,8 @@
+.section .rodata.capsule_key.init,"a"
+.balign 16
+.global __efi_capsule_sig_begin
+__efi_capsule_sig_begin:
+.incbin CONFIG_EFI_CAPSULE_KEY_PATH
+__efi_capsule_sig_end:
+.global __efi_capsule_sig_end
+.balign 16
-- 
2.32.0.rc0

[PATCH 2/3] mkeficapsule: Remove dtb related options

[Ilias Apalodimas]

commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
added a bunch of options enabling the addition of the capsule public key
in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
this functionality anymore

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 tools/mkeficapsule.c | 226 ++-----------------------------------------
 1 file changed, 7 insertions(+), 219 deletions(-)

diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index de0a62898886..214dc38e46e3 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -4,14 +4,12 @@
  *		Author: AKASHI Takahiro
  */
 
-#include <errno.h>
 #include <getopt.h>
 #include <malloc.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
 #include <linux/types.h>
 
 #include <sys/mman.h>
@@ -29,9 +27,6 @@ typedef __s32 s32;
 
 #define aligned_u64 __aligned_u64
 
-#define SIGNATURE_NODENAME	"signature"
-#define OVERLAY_NODENAME	"__overlay__"
-
 #ifndef __packed
 #define __packed __attribute__((packed))
 #endif
@@ -52,9 +47,6 @@ static struct option options[] = {
 	{"raw", required_argument, NULL, 'r'},
 	{"index", required_argument, NULL, 'i'},
 	{"instance", required_argument, NULL, 'I'},
-	{"dtb", required_argument, NULL, 'D'},
-	{"public key", required_argument, NULL, 'K'},
-	{"overlay", no_argument, NULL, 'O'},
 	{"help", no_argument, NULL, 'h'},
 	{NULL, 0, NULL, 0},
 };
@@ -68,187 +60,10 @@ static void print_usage(void)
 	       "\t-r, --raw <raw image>       new raw image file\n"
 	       "\t-i, --index <index>         update image index\n"
 	       "\t-I, --instance <instance>   update hardware instance\n"
-	       "\t-K, --public-key <key file> public key esl file\n"
-	       "\t-D, --dtb <dtb file>        dtb file\n"
-	       "\t-O, --overlay               the dtb file is an overlay\n"
 	       "\t-h, --help                  print a help message\n",
 	       tool_name);
 }
 
-static int fdt_add_pub_key_data(void *sptr, void *dptr, size_t key_size,
-				bool overlay)
-{
-	int parent;
-	int ov_node;
-	int frag_node;
-	int ret = 0;
-
-	if (overlay) {
-		/*
-		 * The signature would be stored in the
-		 * first fragment node of the overlay
-		 */
-		frag_node = fdt_first_subnode(dptr, 0);
-		if (frag_node == -FDT_ERR_NOTFOUND) {
-			fprintf(stderr,
-				"Couldn't find the fragment node: %s\n",
-				fdt_strerror(frag_node));
-			goto done;
-		}
-
-		ov_node = fdt_subnode_offset(dptr, frag_node, OVERLAY_NODENAME);
-		if (ov_node == -FDT_ERR_NOTFOUND) {
-			fprintf(stderr,
-				"Couldn't find the __overlay__ node: %s\n",
-				fdt_strerror(ov_node));
-			goto done;
-		}
-	} else {
-		ov_node = 0;
-	}
-
-	parent = fdt_subnode_offset(dptr, ov_node, SIGNATURE_NODENAME);
-	if (parent == -FDT_ERR_NOTFOUND) {
-		parent = fdt_add_subnode(dptr, ov_node, SIGNATURE_NODENAME);
-		if (parent < 0) {
-			ret = parent;
-			if (ret != -FDT_ERR_NOSPACE) {
-				fprintf(stderr,
-					"Couldn't create signature node: %s\n",
-					fdt_strerror(parent));
-			}
-		}
-	}
-	if (ret)
-		goto done;
-
-	/* Write the key to the FDT node */
-	ret = fdt_setprop(dptr, parent, "capsule-key",
-			  sptr, key_size);
-
-done:
-	if (ret)
-		ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
-
-	return ret;
-}
-
-static int add_public_key(const char *pkey_file, const char *dtb_file,
-			  bool overlay)
-{
-	int ret;
-	int srcfd = -1;
-	int destfd = -1;
-	void *sptr = NULL;
-	void *dptr = NULL;
-	off_t src_size;
-	struct stat pub_key;
-	struct stat dtb;
-
-	/* Find out the size of the public key */
-	srcfd = open(pkey_file, O_RDONLY);
-	if (srcfd == -1) {
-		fprintf(stderr, "%s: Can't open %s: %s\n",
-			__func__, pkey_file, strerror(errno));
-		ret = -1;
-		goto err;
-	}
-
-	ret = fstat(srcfd, &pub_key);
-	if (ret == -1) {
-		fprintf(stderr, "%s: Can't stat %s: %s\n",
-			__func__, pkey_file, strerror(errno));
-		ret = -1;
-		goto err;
-	}
-
-	src_size = pub_key.st_size;
-
-	/* mmap the public key esl file */
-	sptr = mmap(0, src_size, PROT_READ, MAP_SHARED, srcfd, 0);
-	if (sptr == MAP_FAILED) {
-		fprintf(stderr, "%s: Failed to mmap %s:%s\n",
-			__func__, pkey_file, strerror(errno));
-		ret = -1;
-		goto err;
-	}
-
-	/* Open the dest FDT */
-	destfd = open(dtb_file, O_RDWR);
-	if (destfd == -1) {
-		fprintf(stderr, "%s: Can't open %s: %s\n",
-			__func__, dtb_file, strerror(errno));
-		ret = -1;
-		goto err;
-	}
-
-	ret = fstat(destfd, &dtb);
-	if (ret == -1) {
-		fprintf(stderr, "%s: Can't stat %s: %s\n",
-			__func__, dtb_file, strerror(errno));
-		goto err;
-	}
-
-	dtb.st_size += src_size + 0x30;
-	if (ftruncate(destfd, dtb.st_size)) {
-		fprintf(stderr, "%s: Can't expand %s: %s\n",
-			__func__, dtb_file, strerror(errno));
-		ret = -1;
-		goto err;
-	}
-
-	errno = 0;
-	/* mmap the dtb file */
-	dptr = mmap(0, dtb.st_size, PROT_READ | PROT_WRITE, MAP_SHARED,
-		    destfd, 0);
-	if (dptr == MAP_FAILED) {
-		fprintf(stderr, "%s: Failed to mmap %s:%s\n",
-			__func__, dtb_file, strerror(errno));
-		ret = -1;
-		goto err;
-	}
-
-	if (fdt_check_header(dptr)) {
-		fprintf(stderr, "%s: Invalid FDT header\n", __func__);
-		ret = -1;
-		goto err;
-	}
-
-	ret = fdt_open_into(dptr, dptr, dtb.st_size);
-	if (ret) {
-		fprintf(stderr, "%s: Cannot expand FDT: %s\n",
-			__func__, fdt_strerror(ret));
-		ret = -1;
-		goto err;
-	}
-
-	/* Copy the esl file to the expanded FDT */
-	ret = fdt_add_pub_key_data(sptr, dptr, src_size, overlay);
-	if (ret < 0) {
-		fprintf(stderr, "%s: Unable to add public key to the FDT\n",
-			__func__);
-		ret = -1;
-		goto err;
-	}
-
-	ret = 0;
-
-err:
-	if (sptr)
-		munmap(sptr, src_size);
-
-	if (dptr)
-		munmap(dptr, dtb.st_size);
-
-	if (srcfd != -1)
-		close(srcfd);
-
-	if (destfd != -1)
-		close(destfd);
-
-	return ret;
-}
-
 static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 			unsigned long index, unsigned long instance)
 {
@@ -366,22 +181,16 @@ err_1:
 int main(int argc, char **argv)
 {
 	char *file;
-	char *pkey_file;
-	char *dtb_file;
 	efi_guid_t *guid;
 	unsigned long index, instance;
 	int c, idx;
-	int ret;
-	bool overlay = false;
 
 	file = NULL;
-	pkey_file = NULL;
-	dtb_file = NULL;
 	guid = NULL;
 	index = 0;
 	instance = 0;
 	for (;;) {
-		c = getopt_long(argc, argv, "f:r:i:I:v:D:K:Oh", options, &idx);
+		c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx);
 		if (c == -1)
 			break;
 
@@ -408,43 +217,22 @@ int main(int argc, char **argv)
 		case 'I':
 			instance = strtoul(optarg, NULL, 0);
 			break;
-		case 'K':
-			if (pkey_file) {
-				printf("Public Key already specified\n");
-				return -1;
-			}
-			pkey_file = optarg;
-			break;
-		case 'D':
-			if (dtb_file) {
-				printf("DTB file already specified\n");
-				return -1;
-			}
-			dtb_file = optarg;
-			break;
-		case 'O':
-			overlay = true;
-			break;
 		case 'h':
 			print_usage();
 			return 0;
 		}
 	}
 
-	/* need a fit image file or raw image file */
-	if (!file && !pkey_file && !dtb_file) {
+	/* need a output file */
+	if (argc != optind + 1) {
 		print_usage();
 		exit(EXIT_FAILURE);
 	}
 
-	if (pkey_file && dtb_file) {
-		ret = add_public_key(pkey_file, dtb_file, overlay);
-		if (ret == -1) {
-			printf("Adding public key to the dtb failed\n");
-			exit(EXIT_FAILURE);
-		} else {
-			exit(EXIT_SUCCESS);
-		}
+	/* need a fit image file or raw image file */
+	if (!file) {
+		print_usage();
+		exit(EXIT_SUCCESS);
 	}
 
 	if (create_fwbin(argv[optind], file, guid, index, instance)
-- 
2.32.0.rc0

[PATCH 3/3] doc: Update CapsuleUpdate READMEs

[Ilias Apalodimas]

Since we removed embeddingg the capsule key into a .dtb and fixed
authenticated capsule updates for all boards, move the relevant
documentation in the efi file and update it accordingly

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 doc/board/emulation/qemu_capsule_update.rst | 203 --------------------
 doc/develop/uefi/uefi.rst                   | 125 ++++++++++++
 2 files changed, 125 insertions(+), 203 deletions(-)
 delete mode 100644 doc/board/emulation/qemu_capsule_update.rst

diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst
deleted file mode 100644
index 0a2286d039d9..000000000000
--- a/doc/board/emulation/qemu_capsule_update.rst
+++ /dev/null
@@ -1,203 +0,0 @@
-.. SPDX-License-Identifier: GPL-2.0+
-.. Copyright (C) 2020, Linaro Limited
-
-Enabling UEFI Capsule Update feature
-------------------------------------
-
-Support has been added for the UEFI capsule update feature which
-enables updating the U-Boot image using the UEFI firmware management
-protocol (fmp). The capsules are not passed to the firmware through
-the UpdateCapsule runtime service. Instead, capsule-on-disk
-functionality is used for fetching the capsule from the EFI System
-Partition (ESP) by placing the capsule file under the
-\EFI\UpdateCapsule directory.
-
-Currently, support has been added on the QEMU ARM64 virt platform for
-updating the U-Boot binary as a raw image when the platform is booted
-in non-secure mode, i.e. with CONFIG_TFABOOT disabled. For this
-configuration, the QEMU platform needs to be booted with
-'secure=off'. The U-Boot binary placed on the first bank of the NOR
-flash at offset 0x0. The U-Boot environment is placed on the second
-NOR flash bank at offset 0x4000000.
-
-The capsule update feature is enabled with the following configuration
-settings::
-
-    CONFIG_MTD=y
-    CONFIG_FLASH_CFI_MTD=y
-    CONFIG_CMD_MTDPARTS=y
-    CONFIG_CMD_DFU=y
-    CONFIG_DFU_MTD=y
-    CONFIG_PCI_INIT_R=y
-    CONFIG_EFI_CAPSULE_ON_DISK=y
-    CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
-    CONFIG_EFI_CAPSULE_FIRMWARE=y
-    CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
-    CONFIG_EFI_CAPSULE_FMP_HEADER=y
-
-In addition, the following config needs to be disabled(QEMU ARM specific)::
-
-    CONFIG_TFABOOT
-
-The capsule file can be generated by using the tools/mkeficapsule::
-
-    $ mkeficapsule --raw <u-boot.bin> --index 1 <capsule_file_name>
-
-As per the UEFI specification, the capsule file needs to be placed on
-the EFI System Partition, under the \EFI\UpdateCapsule directory. The
-EFI System Partition can be a virtio-blk-device.
-
-Before initiating the firmware update, the efi variables BootNext,
-BootXXXX and OsIndications need to be set. The BootXXXX variable needs
-to be pointing to the EFI System Partition which contains the capsule
-file. The BootNext, BootXXXX and OsIndications variables can be set
-using the following commands::
-
-    => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
-    => efidebug boot next 0
-    => setenv -e -nv -bs -rt -v OsIndications =0x04
-    => saveenv
-
-Finally, the capsule update can be initiated with the following
-command::
-
-    => efidebug capsule disk-update
-
-The updated U-Boot image will be booted on subsequent boot.
-
-Enabling Capsule Authentication
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-The UEFI specification defines a way of authenticating the capsule to
-be updated by verifying the capsule signature. The capsule signature
-is computed and prepended to the capsule payload at the time of
-capsule generation. This signature is then verified by using the
-public key stored as part of the X509 certificate. This certificate is
-in the form of an efi signature list (esl) file, which is embedded as
-part of the platform's device tree blob using the mkeficapsule
-utility.
-
-On the QEMU virt platforms, the device-tree is generated on the fly
-based on the devices configured. This device tree is then passed on to
-the various software components booting on the platform, including
-U-Boot. Therefore, on the QEMU virt platform, the signatute is
-embedded on an overlay. This overlay is then applied at runtime to the
-base platform device-tree. Steps needed for embedding the esl file in
-the overlay are highlighted below.
-
-The capsule authentication feature can be enabled through the
-following config, in addition to the configs listed above for capsule
-update::
-
-    CONFIG_EFI_CAPSULE_AUTHENTICATE=y
-
-The public and private keys used for the signing process are generated
-and used by the steps highlighted below::
-
-    1. Install utility commands on your host
-       * OPENSSL
-       * efitools
-
-    2. Create signing keys and certificate files on your host
-
-        $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
-            -keyout CRT.key -out CRT.crt -nodes -days 365
-        $ cert-to-efi-sig-list CRT.crt CRT.esl
-
-        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
-        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
-
-        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
-        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
-
-The capsule file can be generated by using the GenerateCapsule.py
-script in EDKII::
-
-    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
-      <capsule_file_name> --monotonic-count <val> --fw-version \
-      <val> --lsv <val> --guid \
-      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
-      --update-image-index <val> --signer-private-cert \
-      /path/to/CRT.pem --trusted-public-cert \
-      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
-      <u-boot.bin>
-
-Place the capsule generated in the above step on the EFI System
-Partition under the EFI/UpdateCapsule directory
-
-For embedding the public key certificate, the following steps need to
-be followed::
-
-    1. Generate a skeleton overlay dts file, with a single fragment
-       node and an empty __overlay__ node
-
-       A typical skeleton overlay file will look like this
-
-       /dts-v1/;
-       /plugin/;
-
-       / {
-               fragment@0 {
-                       target-path = "/";
-                       __overlay__ {
-                       };
-               };
-       };
-
-
-    2. Convert the dts to a corresponding dtb with the following
-       command
-        ./scripts/dtc/dtc -@ -I dts -O dtb -o <ov_dtb_file_name> \
-        <dts_file>
-
-    3. Run the dtb file generated above through the mkeficapsule tool
-       in U-Boot
-        ./tools/mkeficapsule -O <pub_key.esl> -D <ov_dtb>
-
-Running the above command results in the creation of a 'signature'
-node in the dtb, under which the public key is stored as a
-'capsule-key' property. The '-O' option is to be used since the
-public key certificate(esl) file is being embedded in an overlay.
-
-The dtb file embedded with the certificate is now to be placed on an
-EFI System Partition. This would then be loaded and "merged" with the
-base platform flattened device-tree(dtb) at runtime.
-
-Build U-Boot with the following steps(QEMU ARM64)::
-
-    $ make qemu_arm64_defconfig
-    $ make menuconfig
-        Disable CONFIG_TFABOOT
-        Enable CONFIG_EFI_CAPSULE_AUTHENTICATE
-        Enable all configs needed for capsule update(listed above)
-    $ make all
-
-Boot the platform and perform the following steps on the U-Boot
-command line::
-
-    1. Enable capsule authentication by setting the following env
-       variable
-
-        => setenv capsule_authentication_enabled 1
-        => saveenv
-
-    2. Load the overlay dtb to memory and merge it with the base fdt
-
-        => fatload virtio 0:1 <$fdtovaddr> EFI/<ov_dtb_file>
-        => fdt addr $fdtcontroladdr
-        => fdt resize <size_of_ov_dtb_file>
-        => fdt apply <$fdtovaddr>
-
-    3. Set the following environment and UEFI boot variables
-
-        => setenv -e -nv -bs -rt -v OsIndications =0x04
-        => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
-        => efidebug boot next 0
-        => saveenv
-
-    4. Finally, the capsule update can be initiated with the following
-       command
-
-        => efidebug capsule disk-update
-
-On subsequent reboot, the platform should boot the updated U-Boot binary.
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index 4f2b8b036db8..3d04228e8188 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -277,6 +277,131 @@ Enable ``CONFIG_OPTEE``, ``CONFIG_CMD_OPTEE_RPMB`` and ``CONFIG_EFI_MM_COMM_TEE`
 
 [1] https://optee.readthedocs.io/en/latest/building/efi_vars/stmm.html
 
+Enabling UEFI Capsule Update feature
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Support has been added for the UEFI capsule update feature which
+enables updating the U-Boot image using the UEFI firmware management
+protocol (FMP). The capsules are not passed to the firmware through
+the UpdateCapsule runtime service. Instead, capsule-on-disk
+functionality is used for fetching the capsule from the EFI System
+Partition (ESP) by placing the capsule file under the
+\EFI\UpdateCapsule directory.
+
+The directory \EFI\UpdateCapsule is checked for capsules only within the
+EFI system partition on the device specified in the active boot option
+determine by reference to BootNext variable or BootOrder variable processing.
+The active Boot Variable is the variable with highest priority BootNext or
+within BootOrder that refers to a device found to be present. Boot variables
+in BootOrder but referring to devices not present are ignored when determining
+active boot variable.
+Before starting a capsule update make sure your capsules are installed in the
+correct ESP partition or set BootNext.
+
+Performing the update
+*********************
+
+Since U-boot doesn't currently support SetVariable at runtime there's a Kconfig
+option (CONFIG_EFI_IGNORE_OSINDICATIONS) to disable the OsIndications variable
+check. If that option is enabled just copy your capsule to \EFI\UpdateCapsule.
+
+If that option is disabled, you'll need to set the OsIndications variable with::
+
+    => setenv -e -nv -bs -rt -v OsIndications =0x04
+
+Finally, the capsule update can be initiated either by rebooting the board,
+which is the preferred method, or by issuing the following command::
+
+    => efidebug capsule disk-update
+
+**The efidebug command is should only be used during debugging/development.**
+
+Enabling Capsule Authentication
+*******************************
+
+The UEFI specification defines a way of authenticating the capsule to
+be updated by verifying the capsule signature. The capsule signature
+is computed and prepended to the capsule payload at the time of
+capsule generation. This signature is then verified by using the
+public key stored as part of the X509 certificate. This certificate is
+in the form of an efi signature list (esl) file, which is embedded as
+part of U-Boot.
+
+The capsule authentication feature can be enabled through the
+following config, in addition to the configs listed above for capsule
+update::
+
+    CONFIG_EFI_CAPSULE_AUTHENTICATE=y
+    CONFIG_EFI_CAPSULE_KEY_PATH=<path to .esl cert>
+
+The public and private keys used for the signing process are generated
+and used by the steps highlighted below::
+
+    1. Install utility commands on your host
+       * OPENSSL
+       * efitools
+
+    2. Create signing keys and certificate files on your host
+
+        $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
+            -keyout CRT.key -out CRT.crt -nodes -days 365
+        $ cert-to-efi-sig-list CRT.crt CRT.esl
+
+        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
+        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
+
+        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
+        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
+
+The capsule file can be generated by using the GenerateCapsule.py
+script in EDKII::
+
+    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
+      <capsule_file_name> --monotonic-count <val> --fw-version \
+      <val> --lsv <val> --guid \
+      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
+      --update-image-index <val> --signer-private-cert \
+      /path/to/CRT.pem --trusted-public-cert \
+      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
+      <u-boot.bin>
+
+Place the capsule generated in the above step on the EFI System
+Partition under the EFI/UpdateCapsule directory
+
+Testing on QEMU
+***************
+
+Currently, support has been added on the QEMU ARM64 virt platform for
+updating the U-Boot binary as a raw image when the platform is booted
+in non-secure mode, i.e. with CONFIG_TFABOOT disabled. For this
+configuration, the QEMU platform needs to be booted with
+'secure=off'. The U-Boot binary placed on the first bank of the NOR
+flash at offset 0x0. The U-Boot environment is placed on the second
+NOR flash bank at offset 0x4000000.
+
+The capsule update feature is enabled with the following configuration
+settings::
+
+    CONFIG_MTD=y
+    CONFIG_FLASH_CFI_MTD=y
+    CONFIG_CMD_MTDPARTS=y
+    CONFIG_CMD_DFU=y
+    CONFIG_DFU_MTD=y
+    CONFIG_PCI_INIT_R=y
+    CONFIG_EFI_CAPSULE_ON_DISK=y
+    CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
+    CONFIG_EFI_CAPSULE_FIRMWARE=y
+    CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
+    CONFIG_EFI_CAPSULE_FMP_HEADER=y
+
+In addition, the following config needs to be disabled(QEMU ARM specific)::
+
+    CONFIG_TFABOOT
+
+The capsule file can be generated by using the tools/mkeficapsule::
+
+    $ mkeficapsule --raw <u-boot.bin> --index 1 <capsule_file_name>
+
 Executing the boot manager
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.32.0.rc0

Re: [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[Masami Hiramatsu]

2021年7月16日(金) 2:00 Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>
> The capsule signature is now part of our DTB.  This is problematic when a
> user is allowed to change/fixup that DTB from U-Boots command line since he
> can overwrite the signature as well.
> So Instead of adding the key on the DTB, embed it in the u-boot binary it
> self as part of it's .rodata.  This assumes that the U-Boot binary we load
> is authenticated by a previous boot stage loader.
>

I have tested this patch on DeveloperBox.
Without signing the capsule,

----
** Unrecognized filesystem type **
Capsule authentication check failed. Aborting update
Firmware update failed: <NULL>
Applying capsule UBOOT1024.Cap failed
PlatformLang:
----

With signing the capsule (by Takahiro's patch*)
(*) https://lists.denx.de/pipermail/u-boot/2021-May/449878.html
----
** Unrecognized filesystem type **
####################################################################################################
###########################################################################PlatformLang:
----

Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

IMHO, we should have a command to show that the current U-Boot
has what certification from the console for debugging.
(also, need to import Takahiro's patch)

Thank you,

> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  board/emulation/common/Makefile       |  1 -
>  board/emulation/common/qemu_capsule.c | 43 ---------------------------
>  include/asm-generic/sections.h        |  2 ++
>  lib/efi_loader/Kconfig                |  6 ++++
>  lib/efi_loader/Makefile               |  8 +++++
>  lib/efi_loader/efi_capsule.c          | 18 +++++++++--
>  lib/efi_loader/efi_capsule_key.S      |  8 +++++
>  7 files changed, 39 insertions(+), 47 deletions(-)
>  delete mode 100644 board/emulation/common/qemu_capsule.c
>  create mode 100644 lib/efi_loader/efi_capsule_key.S
>
> diff --git a/board/emulation/common/Makefile b/board/emulation/common/Makefile
> index 7ed447a69dce..c5b452e7e341 100644
> --- a/board/emulation/common/Makefile
> +++ b/board/emulation/common/Makefile
> @@ -2,4 +2,3 @@
>
>  obj-$(CONFIG_SYS_MTDPARTS_RUNTIME) += qemu_mtdparts.o
>  obj-$(CONFIG_SET_DFU_ALT_INFO) += qemu_dfu.o
> -obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT) += qemu_capsule.o
> diff --git a/board/emulation/common/qemu_capsule.c b/board/emulation/common/qemu_capsule.c
> deleted file mode 100644
> index 6b8a87022a4c..000000000000
> --- a/board/emulation/common/qemu_capsule.c
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Copyright (c) 2020 Linaro Limited
> - */
> -
> -#include <common.h>
> -#include <efi_api.h>
> -#include <efi_loader.h>
> -#include <env.h>
> -#include <fdtdec.h>
> -#include <asm/global_data.h>
> -
> -DECLARE_GLOBAL_DATA_PTR;
> -
> -int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> -{
> -       const void *fdt_blob = gd->fdt_blob;
> -       const void *blob;
> -       const char *cnode_name = "capsule-key";
> -       const char *snode_name = "signature";
> -       int sig_node;
> -       int len;
> -
> -       sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);
> -       if (sig_node < 0) {
> -               EFI_PRINT("Unable to get signature node offset\n");
> -               return -FDT_ERR_NOTFOUND;
> -       }
> -
> -       blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len);
> -
> -       if (!blob || len < 0) {
> -               EFI_PRINT("Unable to get capsule-key value\n");
> -               *pkey = NULL;
> -               *pkey_len = 0;
> -               return -FDT_ERR_NOTFOUND;
> -       }
> -
> -       *pkey = (void *)blob;
> -       *pkey_len = len;
> -
> -       return 0;
> -}
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index 267f1db73f23..ec992b0c2e3f 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -27,6 +27,8 @@ extern char __efi_helloworld_begin[];
>  extern char __efi_helloworld_end[];
>  extern char __efi_var_file_begin[];
>  extern char __efi_var_file_end[];
> +extern char __efi_capsule_sig_begin[];
> +extern char __efi_capsule_sig_end[];
>
>  /* Private data used by of-platdata devices/uclasses */
>  extern char __priv_data_start[], __priv_data_end[];
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 156b39152112..42f1292fa04b 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -213,6 +213,12 @@ config EFI_CAPSULE_AUTHENTICATE
>           Select this option if you want to enable capsule
>           authentication
>
> +config EFI_CAPSULE_KEY_PATH
> +       string "Path to .esl file for capsule authentication"
> +       depends on EFI_CAPSULE_AUTHENTICATE
> +       help
> +         Provide the .esl file used for capsule authentication
> +
>  config EFI_DEVICE_PATH_TO_TEXT
>         bool "Device path to text protocol"
>         default y
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index fd344cea29b0..9b369430e258 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -20,11 +20,19 @@ always += helloworld.efi
>  targets += helloworld.o
>  endif
>
> +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_KEY_PATH))
> +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> +$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_KEY_PATH)
> +endif
> +endif
> +
>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>  obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
>  obj-y += efi_boottime.o
>  obj-y += efi_helper.o
>  obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
>  obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
>  obj-y += efi_console.o
>  obj-y += efi_device_path.o
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index b878e71438b8..50e93cad4ee5 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -16,6 +16,7 @@
>  #include <mapmem.h>
>  #include <sort.h>
>
> +#include <asm/sections.h>
>  #include <crypto/pkcs7.h>
>  #include <crypto/pkcs7_parser.h>
>  #include <linux/err.h>
> @@ -222,12 +223,23 @@ skip:
>  const efi_guid_t efi_guid_capsule_root_cert_guid =
>         EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>
> +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> +{
> +       const void *blob = __efi_capsule_sig_begin;
> +       const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
> +
> +       *pkey = (void *)blob;
> +       *pkey_len = len;
> +
> +       return 0;
> +}
> +
>  efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size,
>                                       void **image, efi_uintn_t *image_size)
>  {
>         u8 *buf;
>         int ret;
> -       void *fdt_pkey, *pkey;
> +       void *stored_pkey, *pkey;
>         efi_uintn_t pkey_len;
>         uint64_t monotonic_count;
>         struct efi_signature_store *truststore;
> @@ -286,7 +298,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
>                 goto out;
>         }
>
> -       ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> +       ret = efi_get_public_key_data(&stored_pkey, &pkey_len);
>         if (ret < 0)
>                 goto out;
>
> @@ -294,7 +306,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
>         if (!pkey)
>                 goto out;
>
> -       memcpy(pkey, fdt_pkey, pkey_len);
> +       memcpy(pkey, stored_pkey, pkey_len);
>         truststore = efi_build_signature_store(pkey, pkey_len);
>         if (!truststore)
>                 goto out;
> diff --git a/lib/efi_loader/efi_capsule_key.S b/lib/efi_loader/efi_capsule_key.S
> new file mode 100644
> index 000000000000..f7047a42e39d
> --- /dev/null
> +++ b/lib/efi_loader/efi_capsule_key.S
> @@ -0,0 +1,8 @@
> +.section .rodata.capsule_key.init,"a"
> +.balign 16
> +.global __efi_capsule_sig_begin
> +__efi_capsule_sig_begin:
> +.incbin CONFIG_EFI_CAPSULE_KEY_PATH
> +__efi_capsule_sig_end:
> +.global __efi_capsule_sig_end
> +.balign 16
> --
> 2.32.0.rc0
>


--
Masami Hiramatsu

Re: [PATCH 2/3] mkeficapsule: Remove dtb related options

[Masami Hiramatsu]

2021年7月16日(金) 2:00 Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>
> commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
> added a bunch of options enabling the addition of the capsule public key
> in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
> this functionality anymore

This looks good to me.

Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

Thanks,

>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  tools/mkeficapsule.c | 226 ++-----------------------------------------
>  1 file changed, 7 insertions(+), 219 deletions(-)
>
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index de0a62898886..214dc38e46e3 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -4,14 +4,12 @@
>   *             Author: AKASHI Takahiro
>   */
>
> -#include <errno.h>
>  #include <getopt.h>
>  #include <malloc.h>
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> -#include <unistd.h>
>  #include <linux/types.h>
>
>  #include <sys/mman.h>
> @@ -29,9 +27,6 @@ typedef __s32 s32;
>
>  #define aligned_u64 __aligned_u64
>
> -#define SIGNATURE_NODENAME     "signature"
> -#define OVERLAY_NODENAME       "__overlay__"
> -
>  #ifndef __packed
>  #define __packed __attribute__((packed))
>  #endif
> @@ -52,9 +47,6 @@ static struct option options[] = {
>         {"raw", required_argument, NULL, 'r'},
>         {"index", required_argument, NULL, 'i'},
>         {"instance", required_argument, NULL, 'I'},
> -       {"dtb", required_argument, NULL, 'D'},
> -       {"public key", required_argument, NULL, 'K'},
> -       {"overlay", no_argument, NULL, 'O'},
>         {"help", no_argument, NULL, 'h'},
>         {NULL, 0, NULL, 0},
>  };
> @@ -68,187 +60,10 @@ static void print_usage(void)
>                "\t-r, --raw <raw image>       new raw image file\n"
>                "\t-i, --index <index>         update image index\n"
>                "\t-I, --instance <instance>   update hardware instance\n"
> -              "\t-K, --public-key <key file> public key esl file\n"
> -              "\t-D, --dtb <dtb file>        dtb file\n"
> -              "\t-O, --overlay               the dtb file is an overlay\n"
>                "\t-h, --help                  print a help message\n",
>                tool_name);
>  }
>
> -static int fdt_add_pub_key_data(void *sptr, void *dptr, size_t key_size,
> -                               bool overlay)
> -{
> -       int parent;
> -       int ov_node;
> -       int frag_node;
> -       int ret = 0;
> -
> -       if (overlay) {
> -               /*
> -                * The signature would be stored in the
> -                * first fragment node of the overlay
> -                */
> -               frag_node = fdt_first_subnode(dptr, 0);
> -               if (frag_node == -FDT_ERR_NOTFOUND) {
> -                       fprintf(stderr,
> -                               "Couldn't find the fragment node: %s\n",
> -                               fdt_strerror(frag_node));
> -                       goto done;
> -               }
> -
> -               ov_node = fdt_subnode_offset(dptr, frag_node, OVERLAY_NODENAME);
> -               if (ov_node == -FDT_ERR_NOTFOUND) {
> -                       fprintf(stderr,
> -                               "Couldn't find the __overlay__ node: %s\n",
> -                               fdt_strerror(ov_node));
> -                       goto done;
> -               }
> -       } else {
> -               ov_node = 0;
> -       }
> -
> -       parent = fdt_subnode_offset(dptr, ov_node, SIGNATURE_NODENAME);
> -       if (parent == -FDT_ERR_NOTFOUND) {
> -               parent = fdt_add_subnode(dptr, ov_node, SIGNATURE_NODENAME);
> -               if (parent < 0) {
> -                       ret = parent;
> -                       if (ret != -FDT_ERR_NOSPACE) {
> -                               fprintf(stderr,
> -                                       "Couldn't create signature node: %s\n",
> -                                       fdt_strerror(parent));
> -                       }
> -               }
> -       }
> -       if (ret)
> -               goto done;
> -
> -       /* Write the key to the FDT node */
> -       ret = fdt_setprop(dptr, parent, "capsule-key",
> -                         sptr, key_size);
> -
> -done:
> -       if (ret)
> -               ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
> -
> -       return ret;
> -}
> -
> -static int add_public_key(const char *pkey_file, const char *dtb_file,
> -                         bool overlay)
> -{
> -       int ret;
> -       int srcfd = -1;
> -       int destfd = -1;
> -       void *sptr = NULL;
> -       void *dptr = NULL;
> -       off_t src_size;
> -       struct stat pub_key;
> -       struct stat dtb;
> -
> -       /* Find out the size of the public key */
> -       srcfd = open(pkey_file, O_RDONLY);
> -       if (srcfd == -1) {
> -               fprintf(stderr, "%s: Can't open %s: %s\n",
> -                       __func__, pkey_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       ret = fstat(srcfd, &pub_key);
> -       if (ret == -1) {
> -               fprintf(stderr, "%s: Can't stat %s: %s\n",
> -                       __func__, pkey_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       src_size = pub_key.st_size;
> -
> -       /* mmap the public key esl file */
> -       sptr = mmap(0, src_size, PROT_READ, MAP_SHARED, srcfd, 0);
> -       if (sptr == MAP_FAILED) {
> -               fprintf(stderr, "%s: Failed to mmap %s:%s\n",
> -                       __func__, pkey_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       /* Open the dest FDT */
> -       destfd = open(dtb_file, O_RDWR);
> -       if (destfd == -1) {
> -               fprintf(stderr, "%s: Can't open %s: %s\n",
> -                       __func__, dtb_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       ret = fstat(destfd, &dtb);
> -       if (ret == -1) {
> -               fprintf(stderr, "%s: Can't stat %s: %s\n",
> -                       __func__, dtb_file, strerror(errno));
> -               goto err;
> -       }
> -
> -       dtb.st_size += src_size + 0x30;
> -       if (ftruncate(destfd, dtb.st_size)) {
> -               fprintf(stderr, "%s: Can't expand %s: %s\n",
> -                       __func__, dtb_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       errno = 0;
> -       /* mmap the dtb file */
> -       dptr = mmap(0, dtb.st_size, PROT_READ | PROT_WRITE, MAP_SHARED,
> -                   destfd, 0);
> -       if (dptr == MAP_FAILED) {
> -               fprintf(stderr, "%s: Failed to mmap %s:%s\n",
> -                       __func__, dtb_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       if (fdt_check_header(dptr)) {
> -               fprintf(stderr, "%s: Invalid FDT header\n", __func__);
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       ret = fdt_open_into(dptr, dptr, dtb.st_size);
> -       if (ret) {
> -               fprintf(stderr, "%s: Cannot expand FDT: %s\n",
> -                       __func__, fdt_strerror(ret));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       /* Copy the esl file to the expanded FDT */
> -       ret = fdt_add_pub_key_data(sptr, dptr, src_size, overlay);
> -       if (ret < 0) {
> -               fprintf(stderr, "%s: Unable to add public key to the FDT\n",
> -                       __func__);
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       ret = 0;
> -
> -err:
> -       if (sptr)
> -               munmap(sptr, src_size);
> -
> -       if (dptr)
> -               munmap(dptr, dtb.st_size);
> -
> -       if (srcfd != -1)
> -               close(srcfd);
> -
> -       if (destfd != -1)
> -               close(destfd);
> -
> -       return ret;
> -}
> -
>  static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>                         unsigned long index, unsigned long instance)
>  {
> @@ -366,22 +181,16 @@ err_1:
>  int main(int argc, char **argv)
>  {
>         char *file;
> -       char *pkey_file;
> -       char *dtb_file;
>         efi_guid_t *guid;
>         unsigned long index, instance;
>         int c, idx;
> -       int ret;
> -       bool overlay = false;
>
>         file = NULL;
> -       pkey_file = NULL;
> -       dtb_file = NULL;
>         guid = NULL;
>         index = 0;
>         instance = 0;
>         for (;;) {
> -               c = getopt_long(argc, argv, "f:r:i:I:v:D:K:Oh", options, &idx);
> +               c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx);
>                 if (c == -1)
>                         break;
>
> @@ -408,43 +217,22 @@ int main(int argc, char **argv)
>                 case 'I':
>                         instance = strtoul(optarg, NULL, 0);
>                         break;
> -               case 'K':
> -                       if (pkey_file) {
> -                               printf("Public Key already specified\n");
> -                               return -1;
> -                       }
> -                       pkey_file = optarg;
> -                       break;
> -               case 'D':
> -                       if (dtb_file) {
> -                               printf("DTB file already specified\n");
> -                               return -1;
> -                       }
> -                       dtb_file = optarg;
> -                       break;
> -               case 'O':
> -                       overlay = true;
> -                       break;
>                 case 'h':
>                         print_usage();
>                         return 0;
>                 }
>         }
>
> -       /* need a fit image file or raw image file */
> -       if (!file && !pkey_file && !dtb_file) {
> +       /* need a output file */
> +       if (argc != optind + 1) {
>                 print_usage();
>                 exit(EXIT_FAILURE);
>         }
>
> -       if (pkey_file && dtb_file) {
> -               ret = add_public_key(pkey_file, dtb_file, overlay);
> -               if (ret == -1) {
> -                       printf("Adding public key to the dtb failed\n");
> -                       exit(EXIT_FAILURE);
> -               } else {
> -                       exit(EXIT_SUCCESS);
> -               }
> +       /* need a fit image file or raw image file */
> +       if (!file) {
> +               print_usage();
> +               exit(EXIT_SUCCESS);
>         }
>
>         if (create_fwbin(argv[optind], file, guid, index, instance)
> --
> 2.32.0.rc0
>


-- 
Masami Hiramatsu

Re: [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[Sughosh Ganu]

On Thu, 15 Jul 2021 at 22:30, Ilias Apalodimas <ilias.apalodimas@linaro.org>
wrote:

> The capsule signature is now part of our DTB.  This is problematic when a
> user is allowed to change/fixup that DTB from U-Boots command line since he
> can overwrite the signature as well.
> So Instead of adding the key on the DTB, embed it in the u-boot binary it
> self as part of it's .rodata.  This assumes that the U-Boot binary we load
> is authenticated by a previous boot stage loader.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  board/emulation/common/Makefile       |  1 -
>  board/emulation/common/qemu_capsule.c | 43 ---------------------------
>  include/asm-generic/sections.h        |  2 ++
>  lib/efi_loader/Kconfig                |  6 ++++
>  lib/efi_loader/Makefile               |  8 +++++
>  lib/efi_loader/efi_capsule.c          | 18 +++++++++--
>  lib/efi_loader/efi_capsule_key.S      |  8 +++++
>  7 files changed, 39 insertions(+), 47 deletions(-)
>  delete mode 100644 board/emulation/common/qemu_capsule.c
>  create mode 100644 lib/efi_loader/efi_capsule_key.S
>

Tested the changes on Qemu arm64 virt platform.

Tested-by: Sughosh Ganu <sughosh.ganu@linaro.org>

-sughosh


>
> diff --git a/board/emulation/common/Makefile
> b/board/emulation/common/Makefile
> index 7ed447a69dce..c5b452e7e341 100644
> --- a/board/emulation/common/Makefile
> +++ b/board/emulation/common/Makefile
> @@ -2,4 +2,3 @@
>
>  obj-$(CONFIG_SYS_MTDPARTS_RUNTIME) += qemu_mtdparts.o
>  obj-$(CONFIG_SET_DFU_ALT_INFO) += qemu_dfu.o
> -obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT) += qemu_capsule.o
> diff --git a/board/emulation/common/qemu_capsule.c
> b/board/emulation/common/qemu_capsule.c
> deleted file mode 100644
> index 6b8a87022a4c..000000000000
> --- a/board/emulation/common/qemu_capsule.c
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Copyright (c) 2020 Linaro Limited
> - */
> -
> -#include <common.h>
> -#include <efi_api.h>
> -#include <efi_loader.h>
> -#include <env.h>
> -#include <fdtdec.h>
> -#include <asm/global_data.h>
> -
> -DECLARE_GLOBAL_DATA_PTR;
> -
> -int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> -{
> -       const void *fdt_blob = gd->fdt_blob;
> -       const void *blob;
> -       const char *cnode_name = "capsule-key";
> -       const char *snode_name = "signature";
> -       int sig_node;
> -       int len;
> -
> -       sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);
> -       if (sig_node < 0) {
> -               EFI_PRINT("Unable to get signature node offset\n");
> -               return -FDT_ERR_NOTFOUND;
> -       }
> -
> -       blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len);
> -
> -       if (!blob || len < 0) {
> -               EFI_PRINT("Unable to get capsule-key value\n");
> -               *pkey = NULL;
> -               *pkey_len = 0;
> -               return -FDT_ERR_NOTFOUND;
> -       }
> -
> -       *pkey = (void *)blob;
> -       *pkey_len = len;
> -
> -       return 0;
> -}
> diff --git a/include/asm-generic/sections.h
> b/include/asm-generic/sections.h
> index 267f1db73f23..ec992b0c2e3f 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -27,6 +27,8 @@ extern char __efi_helloworld_begin[];
>  extern char __efi_helloworld_end[];
>  extern char __efi_var_file_begin[];
>  extern char __efi_var_file_end[];
> +extern char __efi_capsule_sig_begin[];
> +extern char __efi_capsule_sig_end[];
>
>  /* Private data used by of-platdata devices/uclasses */
>  extern char __priv_data_start[], __priv_data_end[];
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 156b39152112..42f1292fa04b 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -213,6 +213,12 @@ config EFI_CAPSULE_AUTHENTICATE
>           Select this option if you want to enable capsule
>           authentication
>
> +config EFI_CAPSULE_KEY_PATH
> +       string "Path to .esl file for capsule authentication"
> +       depends on EFI_CAPSULE_AUTHENTICATE
> +       help
> +         Provide the .esl file used for capsule authentication
> +
>  config EFI_DEVICE_PATH_TO_TEXT
>         bool "Device path to text protocol"
>         default y
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index fd344cea29b0..9b369430e258 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -20,11 +20,19 @@ always += helloworld.efi
>  targets += helloworld.o
>  endif
>
> +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_KEY_PATH))
> +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> +$(error .esl cerificate not found. Configure your
> CONFIG_EFI_CAPSULE_KEY_PATH)
> +endif
> +endif
> +
>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>  obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
>  obj-y += efi_boottime.o
>  obj-y += efi_helper.o
>  obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
>  obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
>  obj-y += efi_console.o
>  obj-y += efi_device_path.o
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index b878e71438b8..50e93cad4ee5 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -16,6 +16,7 @@
>  #include <mapmem.h>
>  #include <sort.h>
>
> +#include <asm/sections.h>
>  #include <crypto/pkcs7.h>
>  #include <crypto/pkcs7_parser.h>
>  #include <linux/err.h>
> @@ -222,12 +223,23 @@ skip:
>  const efi_guid_t efi_guid_capsule_root_cert_guid =
>         EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>
> +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> +{
> +       const void *blob = __efi_capsule_sig_begin;
> +       const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
> +
> +       *pkey = (void *)blob;
> +       *pkey_len = len;
> +
> +       return 0;
> +}
> +
>  efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t
> capsule_size,
>                                       void **image, efi_uintn_t
> *image_size)
>  {
>         u8 *buf;
>         int ret;
> -       void *fdt_pkey, *pkey;
> +       void *stored_pkey, *pkey;
>         efi_uintn_t pkey_len;
>         uint64_t monotonic_count;
>         struct efi_signature_store *truststore;
> @@ -286,7 +298,7 @@ efi_status_t efi_capsule_authenticate(const void
> *capsule, efi_uintn_t capsule_s
>                 goto out;
>         }
>
> -       ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> +       ret = efi_get_public_key_data(&stored_pkey, &pkey_len);
>         if (ret < 0)
>                 goto out;
>
> @@ -294,7 +306,7 @@ efi_status_t efi_capsule_authenticate(const void
> *capsule, efi_uintn_t capsule_s
>         if (!pkey)
>                 goto out;
>
> -       memcpy(pkey, fdt_pkey, pkey_len);
> +       memcpy(pkey, stored_pkey, pkey_len);
>         truststore = efi_build_signature_store(pkey, pkey_len);
>         if (!truststore)
>                 goto out;
> diff --git a/lib/efi_loader/efi_capsule_key.S
> b/lib/efi_loader/efi_capsule_key.S
> new file mode 100644
> index 000000000000..f7047a42e39d
> --- /dev/null
> +++ b/lib/efi_loader/efi_capsule_key.S
> @@ -0,0 +1,8 @@
> +.section .rodata.capsule_key.init,"a"
> +.balign 16
> +.global __efi_capsule_sig_begin
> +__efi_capsule_sig_begin:
> +.incbin CONFIG_EFI_CAPSULE_KEY_PATH
> +__efi_capsule_sig_end:
> +.global __efi_capsule_sig_end
> +.balign 16
> --
> 2.32.0.rc0
>
>

Re: [PATCH 3/3] doc: Update CapsuleUpdate READMEs

[Heinrich Schuchardt]

On 7/15/21 7:00 PM, Ilias Apalodimas wrote:
> Since we removed embeddingg the capsule key into a .dtb and fixed
> authenticated capsule updates for all boards, move the relevant
> documentation in the efi file and update it accordingly
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   doc/board/emulation/qemu_capsule_update.rst | 203 --------------------
>   doc/develop/uefi/uefi.rst                   | 125 ++++++++++++
>   2 files changed, 125 insertions(+), 203 deletions(-)
>   delete mode 100644 doc/board/emulation/qemu_capsule_update.rst
>
> diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst
> deleted file mode 100644
> index 0a2286d039d9..000000000000
> --- a/doc/board/emulation/qemu_capsule_update.rst
> +++ /dev/null
> @@ -1,203 +0,0 @@
> -.. SPDX-License-Identifier: GPL-2.0+
> -.. Copyright (C) 2020, Linaro Limited
> -
> -Enabling UEFI Capsule Update feature
> -------------------------------------
> -
> -Support has been added for the UEFI capsule update feature which
> -enables updating the U-Boot image using the UEFI firmware management
> -protocol (fmp). The capsules are not passed to the firmware through
> -the UpdateCapsule runtime service. Instead, capsule-on-disk
> -functionality is used for fetching the capsule from the EFI System
> -Partition (ESP) by placing the capsule file under the
> -\EFI\UpdateCapsule directory.
> -
> -Currently, support has been added on the QEMU ARM64 virt platform for
> -updating the U-Boot binary as a raw image when the platform is booted
> -in non-secure mode, i.e. with CONFIG_TFABOOT disabled. For this
> -configuration, the QEMU platform needs to be booted with
> -'secure=off'. The U-Boot binary placed on the first bank of the NOR
> -flash at offset 0x0. The U-Boot environment is placed on the second
> -NOR flash bank at offset 0x4000000.
> -
> -The capsule update feature is enabled with the following configuration
> -settings::
> -
> -    CONFIG_MTD=y
> -    CONFIG_FLASH_CFI_MTD=y
> -    CONFIG_CMD_MTDPARTS=y
> -    CONFIG_CMD_DFU=y
> -    CONFIG_DFU_MTD=y
> -    CONFIG_PCI_INIT_R=y
> -    CONFIG_EFI_CAPSULE_ON_DISK=y
> -    CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
> -    CONFIG_EFI_CAPSULE_FIRMWARE=y
> -    CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> -    CONFIG_EFI_CAPSULE_FMP_HEADER=y
> -
> -In addition, the following config needs to be disabled(QEMU ARM specific)::
> -
> -    CONFIG_TFABOOT
> -
> -The capsule file can be generated by using the tools/mkeficapsule::
> -
> -    $ mkeficapsule --raw <u-boot.bin> --index 1 <capsule_file_name>
> -
> -As per the UEFI specification, the capsule file needs to be placed on
> -the EFI System Partition, under the \EFI\UpdateCapsule directory. The
> -EFI System Partition can be a virtio-blk-device.
> -
> -Before initiating the firmware update, the efi variables BootNext,
> -BootXXXX and OsIndications need to be set. The BootXXXX variable needs
> -to be pointing to the EFI System Partition which contains the capsule
> -file. The BootNext, BootXXXX and OsIndications variables can be set
> -using the following commands::
> -
> -    => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
> -    => efidebug boot next 0
> -    => setenv -e -nv -bs -rt -v OsIndications =0x04
> -    => saveenv
> -
> -Finally, the capsule update can be initiated with the following
> -command::
> -
> -    => efidebug capsule disk-update
> -
> -The updated U-Boot image will be booted on subsequent boot.
> -
> -Enabling Capsule Authentication
> -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> -
> -The UEFI specification defines a way of authenticating the capsule to
> -be updated by verifying the capsule signature. The capsule signature
> -is computed and prepended to the capsule payload at the time of
> -capsule generation. This signature is then verified by using the
> -public key stored as part of the X509 certificate. This certificate is
> -in the form of an efi signature list (esl) file, which is embedded as
> -part of the platform's device tree blob using the mkeficapsule
> -utility.
> -
> -On the QEMU virt platforms, the device-tree is generated on the fly
> -based on the devices configured. This device tree is then passed on to
> -the various software components booting on the platform, including
> -U-Boot. Therefore, on the QEMU virt platform, the signatute is
> -embedded on an overlay. This overlay is then applied at runtime to the
> -base platform device-tree. Steps needed for embedding the esl file in
> -the overlay are highlighted below.
> -
> -The capsule authentication feature can be enabled through the
> -following config, in addition to the configs listed above for capsule
> -update::
> -
> -    CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> -
> -The public and private keys used for the signing process are generated
> -and used by the steps highlighted below::
> -
> -    1. Install utility commands on your host
> -       * OPENSSL
> -       * efitools
> -
> -    2. Create signing keys and certificate files on your host
> -
> -        $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
> -            -keyout CRT.key -out CRT.crt -nodes -days 365
> -        $ cert-to-efi-sig-list CRT.crt CRT.esl
> -
> -        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
> -        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
> -
> -        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
> -        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
> -
> -The capsule file can be generated by using the GenerateCapsule.py
> -script in EDKII::
> -
> -    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> -      <capsule_file_name> --monotonic-count <val> --fw-version \
> -      <val> --lsv <val> --guid \
> -      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
> -      --update-image-index <val> --signer-private-cert \
> -      /path/to/CRT.pem --trusted-public-cert \
> -      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
> -      <u-boot.bin>
> -
> -Place the capsule generated in the above step on the EFI System
> -Partition under the EFI/UpdateCapsule directory
> -
> -For embedding the public key certificate, the following steps need to
> -be followed::
> -
> -    1. Generate a skeleton overlay dts file, with a single fragment
> -       node and an empty __overlay__ node
> -
> -       A typical skeleton overlay file will look like this
> -
> -       /dts-v1/;
> -       /plugin/;
> -
> -       / {
> -               fragment@0 {
> -                       target-path = "/";
> -                       __overlay__ {
> -                       };
> -               };
> -       };
> -
> -
> -    2. Convert the dts to a corresponding dtb with the following
> -       command
> -        ./scripts/dtc/dtc -@ -I dts -O dtb -o <ov_dtb_file_name> \
> -        <dts_file>
> -
> -    3. Run the dtb file generated above through the mkeficapsule tool
> -       in U-Boot
> -        ./tools/mkeficapsule -O <pub_key.esl> -D <ov_dtb>
> -
> -Running the above command results in the creation of a 'signature'
> -node in the dtb, under which the public key is stored as a
> -'capsule-key' property. The '-O' option is to be used since the
> -public key certificate(esl) file is being embedded in an overlay.
> -
> -The dtb file embedded with the certificate is now to be placed on an
> -EFI System Partition. This would then be loaded and "merged" with the
> -base platform flattened device-tree(dtb) at runtime.
> -
> -Build U-Boot with the following steps(QEMU ARM64)::
> -
> -    $ make qemu_arm64_defconfig
> -    $ make menuconfig
> -        Disable CONFIG_TFABOOT
> -        Enable CONFIG_EFI_CAPSULE_AUTHENTICATE
> -        Enable all configs needed for capsule update(listed above)
> -    $ make all
> -
> -Boot the platform and perform the following steps on the U-Boot
> -command line::
> -
> -    1. Enable capsule authentication by setting the following env
> -       variable
> -
> -        => setenv capsule_authentication_enabled 1
> -        => saveenv
> -
> -    2. Load the overlay dtb to memory and merge it with the base fdt
> -
> -        => fatload virtio 0:1 <$fdtovaddr> EFI/<ov_dtb_file>
> -        => fdt addr $fdtcontroladdr
> -        => fdt resize <size_of_ov_dtb_file>
> -        => fdt apply <$fdtovaddr>
> -
> -    3. Set the following environment and UEFI boot variables
> -
> -        => setenv -e -nv -bs -rt -v OsIndications =0x04
> -        => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
> -        => efidebug boot next 0
> -        => saveenv
> -
> -    4. Finally, the capsule update can be initiated with the following
> -       command
> -
> -        => efidebug capsule disk-update
> -
> -On subsequent reboot, the platform should boot the updated U-Boot binary.
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index 4f2b8b036db8..3d04228e8188 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -277,6 +277,131 @@ Enable ``CONFIG_OPTEE``, ``CONFIG_CMD_OPTEE_RPMB`` and ``CONFIG_EFI_MM_COMM_TEE`
>
>   [1] https://optee.readthedocs.io/en/latest/building/efi_vars/stmm.html
>
> +Enabling UEFI Capsule Update feature
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Support has been added for the UEFI capsule update feature which
> +enables updating the U-Boot image using the UEFI firmware management
> +protocol (FMP). The capsules are not passed to the firmware through
> +the UpdateCapsule runtime service. Instead, capsule-on-disk
> +functionality is used for fetching the capsule from the EFI System
> +Partition (ESP) by placing the capsule file under the
> +\EFI\UpdateCapsule directory.
> +
> +The directory \EFI\UpdateCapsule is checked for capsules only within the
> +EFI system partition on the device specified in the active boot option
> +determine by reference to BootNext variable or BootOrder variable processing.

%s/determine/determined/

> +The active Boot Variable is the variable with highest priority BootNext or

Does only the device have to be present or also the file?
Should we check only the binary or also the initrd?

Best regards

Heinrich

> +within BootOrder that refers to a device found to be present. Boot variables
> +in BootOrder but referring to devices not present are ignored when determining
> +active boot variable.
> +Before starting a capsule update make sure your capsules are installed in the
> +correct ESP partition or set BootNext.
> +
> +Performing the update
> +*********************
> +
> +Since U-boot doesn't currently support SetVariable at runtime there's a Kconfig
> +option (CONFIG_EFI_IGNORE_OSINDICATIONS) to disable the OsIndications variable
> +check. If that option is enabled just copy your capsule to \EFI\UpdateCapsule.
> +
> +If that option is disabled, you'll need to set the OsIndications variable with::
> +
> +    => setenv -e -nv -bs -rt -v OsIndications =0x04
> +
> +Finally, the capsule update can be initiated either by rebooting the board,
> +which is the preferred method, or by issuing the following command::
> +
> +    => efidebug capsule disk-update
> +
> +**The efidebug command is should only be used during debugging/development.**
> +
> +Enabling Capsule Authentication
> +*******************************
> +
> +The UEFI specification defines a way of authenticating the capsule to
> +be updated by verifying the capsule signature. The capsule signature
> +is computed and prepended to the capsule payload at the time of
> +capsule generation. This signature is then verified by using the
> +public key stored as part of the X509 certificate. This certificate is
> +in the form of an efi signature list (esl) file, which is embedded as
> +part of U-Boot.
> +
> +The capsule authentication feature can be enabled through the
> +following config, in addition to the configs listed above for capsule
> +update::
> +
> +    CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> +    CONFIG_EFI_CAPSULE_KEY_PATH=<path to .esl cert>
> +
> +The public and private keys used for the signing process are generated
> +and used by the steps highlighted below::
> +
> +    1. Install utility commands on your host
> +       * OPENSSL
> +       * efitools
> +
> +    2. Create signing keys and certificate files on your host
> +
> +        $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
> +            -keyout CRT.key -out CRT.crt -nodes -days 365
> +        $ cert-to-efi-sig-list CRT.crt CRT.esl
> +
> +        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
> +        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
> +
> +        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
> +        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
> +
> +The capsule file can be generated by using the GenerateCapsule.py
> +script in EDKII::
> +
> +    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> +      <capsule_file_name> --monotonic-count <val> --fw-version \
> +      <val> --lsv <val> --guid \
> +      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
> +      --update-image-index <val> --signer-private-cert \
> +      /path/to/CRT.pem --trusted-public-cert \
> +      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
> +      <u-boot.bin>
> +
> +Place the capsule generated in the above step on the EFI System
> +Partition under the EFI/UpdateCapsule directory
> +
> +Testing on QEMU
> +***************
> +
> +Currently, support has been added on the QEMU ARM64 virt platform for
> +updating the U-Boot binary as a raw image when the platform is booted
> +in non-secure mode, i.e. with CONFIG_TFABOOT disabled. For this
> +configuration, the QEMU platform needs to be booted with
> +'secure=off'. The U-Boot binary placed on the first bank of the NOR
> +flash at offset 0x0. The U-Boot environment is placed on the second
> +NOR flash bank at offset 0x4000000.
> +
> +The capsule update feature is enabled with the following configuration
> +settings::
> +
> +    CONFIG_MTD=y
> +    CONFIG_FLASH_CFI_MTD=y
> +    CONFIG_CMD_MTDPARTS=y
> +    CONFIG_CMD_DFU=y
> +    CONFIG_DFU_MTD=y
> +    CONFIG_PCI_INIT_R=y
> +    CONFIG_EFI_CAPSULE_ON_DISK=y
> +    CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
> +    CONFIG_EFI_CAPSULE_FIRMWARE=y
> +    CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> +    CONFIG_EFI_CAPSULE_FMP_HEADER=y
> +
> +In addition, the following config needs to be disabled(QEMU ARM specific)::
> +
> +    CONFIG_TFABOOT
> +
> +The capsule file can be generated by using the tools/mkeficapsule::
> +
> +    $ mkeficapsule --raw <u-boot.bin> --index 1 <capsule_file_name>
> +
>   Executing the boot manager
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>

Re: [PATCH 3/3] doc: Update CapsuleUpdate READMEs

[Ilias Apalodimas]

> > +
> > +The directory \EFI\UpdateCapsule is checked for capsules only within the
> > +EFI system partition on the device specified in the active boot option
> > +determine by reference to BootNext variable or BootOrder variable processing.
> 
> %s/determine/determined/
> 

sure 

> > +The active Boot Variable is the variable with highest priority BootNext or
> 
> Does only the device have to be present or also the file?
> Should we check only the binary or also the initrd?

I don't follow on the initrd.
The whole paragraph is copied verbatim from the EFI spec. Basically you
need a valid boot option (with priority) that points to the ESP partition
your capsule is.  Akashi's code is also doing the right thing following the
spec.

Regards
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> > +within BootOrder that refers to a device found to be present. Boot variables
> > +in BootOrder but referring to devices not present are ignored when determining
> > +active boot variable.
> > +Before starting a capsule update make sure your capsules are installed in the
> > +correct ESP partition or set BootNext.
> > +
> > +Performing the update
> > +*********************
> > +
> > +Since U-boot doesn't currently support SetVariable at runtime there's a Kconfig
> > +option (CONFIG_EFI_IGNORE_OSINDICATIONS) to disable the OsIndications variable
> > +check. If that option is enabled just copy your capsule to \EFI\UpdateCapsule.
> > +
> > +If that option is disabled, you'll need to set the OsIndications variable with::
> > +
> > +    => setenv -e -nv -bs -rt -v OsIndications =0x04
> > +
> > +Finally, the capsule update can be initiated either by rebooting the board,
> > +which is the preferred method, or by issuing the following command::
> > +
> > +    => efidebug capsule disk-update
> > +
> > +**The efidebug command is should only be used during debugging/development.**
> > +
> > +Enabling Capsule Authentication
> > +*******************************
> > +
> > +The UEFI specification defines a way of authenticating the capsule to
> > +be updated by verifying the capsule signature. The capsule signature
> > +is computed and prepended to the capsule payload at the time of
> > +capsule generation. This signature is then verified by using the
> > +public key stored as part of the X509 certificate. This certificate is
> > +in the form of an efi signature list (esl) file, which is embedded as
> > +part of U-Boot.
> > +
> > +The capsule authentication feature can be enabled through the
> > +following config, in addition to the configs listed above for capsule
> > +update::
> > +
> > +    CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > +    CONFIG_EFI_CAPSULE_KEY_PATH=<path to .esl cert>
> > +
> > +The public and private keys used for the signing process are generated
> > +and used by the steps highlighted below::
> > +
> > +    1. Install utility commands on your host
> > +       * OPENSSL
> > +       * efitools
> > +
> > +    2. Create signing keys and certificate files on your host
> > +
> > +        $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
> > +            -keyout CRT.key -out CRT.crt -nodes -days 365
> > +        $ cert-to-efi-sig-list CRT.crt CRT.esl
> > +
> > +        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
> > +        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
> > +
> > +        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
> > +        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
> > +
> > +The capsule file can be generated by using the GenerateCapsule.py
> > +script in EDKII::
> > +
> > +    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> > +      <capsule_file_name> --monotonic-count <val> --fw-version \
> > +      <val> --lsv <val> --guid \
> > +      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
> > +      --update-image-index <val> --signer-private-cert \
> > +      /path/to/CRT.pem --trusted-public-cert \
> > +      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
> > +      <u-boot.bin>
> > +
> > +Place the capsule generated in the above step on the EFI System
> > +Partition under the EFI/UpdateCapsule directory
> > +
> > +Testing on QEMU
> > +***************
> > +
> > +Currently, support has been added on the QEMU ARM64 virt platform for
> > +updating the U-Boot binary as a raw image when the platform is booted
> > +in non-secure mode, i.e. with CONFIG_TFABOOT disabled. For this
> > +configuration, the QEMU platform needs to be booted with
> > +'secure=off'. The U-Boot binary placed on the first bank of the NOR
> > +flash at offset 0x0. The U-Boot environment is placed on the second
> > +NOR flash bank at offset 0x4000000.
> > +
> > +The capsule update feature is enabled with the following configuration
> > +settings::
> > +
> > +    CONFIG_MTD=y
> > +    CONFIG_FLASH_CFI_MTD=y
> > +    CONFIG_CMD_MTDPARTS=y
> > +    CONFIG_CMD_DFU=y
> > +    CONFIG_DFU_MTD=y
> > +    CONFIG_PCI_INIT_R=y
> > +    CONFIG_EFI_CAPSULE_ON_DISK=y
> > +    CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
> > +    CONFIG_EFI_CAPSULE_FIRMWARE=y
> > +    CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > +    CONFIG_EFI_CAPSULE_FMP_HEADER=y
> > +
> > +In addition, the following config needs to be disabled(QEMU ARM specific)::
> > +
> > +    CONFIG_TFABOOT
> > +
> > +The capsule file can be generated by using the tools/mkeficapsule::
> > +
> > +    $ mkeficapsule --raw <u-boot.bin> --index 1 <capsule_file_name>
> > +
> >   Executing the boot manager
> >   ~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > 
> 

Re: [PATCH 3/3] doc: Update CapsuleUpdate READMEs

[AKASHI Takahiro]

On Fri, Jul 16, 2021 at 10:09:40AM +0300, Ilias Apalodimas wrote:
> > > +
> > > +The directory \EFI\UpdateCapsule is checked for capsules only within the
> > > +EFI system partition on the device specified in the active boot option
> > > +determine by reference to BootNext variable or BootOrder variable processing.
> > 
> > %s/determine/determined/
> > 
> 
> sure 
> 
> > > +The active Boot Variable is the variable with highest priority BootNext or
> > 
> > Does only the device have to be present or also the file?
> > Should we check only the binary or also the initrd?
> 
> I don't follow on the initrd.
> The whole paragraph is copied verbatim from the EFI spec. Basically you
> need a valid boot option (with priority) that points to the ESP partition
> your capsule is.  Akashi's code is also doing the right thing following the
> spec.

I *guess* that Heinrich thinks of all the device paths
contained in an boot option, including "initrd" that
you added a support for.
IIRC, all the paths in one option must point to the same device.
(So no for Heinrich's concern?)

-Takahiro Akashi

> Regards
> /Ilias
> > 
> > Best regards
> > 
> > Heinrich
> > 
> > > +within BootOrder that refers to a device found to be present. Boot variables
> > > +in BootOrder but referring to devices not present are ignored when determining
> > > +active boot variable.
> > > +Before starting a capsule update make sure your capsules are installed in the
> > > +correct ESP partition or set BootNext.
> > > +
> > > +Performing the update
> > > +*********************
> > > +
> > > +Since U-boot doesn't currently support SetVariable at runtime there's a Kconfig
> > > +option (CONFIG_EFI_IGNORE_OSINDICATIONS) to disable the OsIndications variable
> > > +check. If that option is enabled just copy your capsule to \EFI\UpdateCapsule.
> > > +
> > > +If that option is disabled, you'll need to set the OsIndications variable with::
> > > +
> > > +    => setenv -e -nv -bs -rt -v OsIndications =0x04
> > > +
> > > +Finally, the capsule update can be initiated either by rebooting the board,
> > > +which is the preferred method, or by issuing the following command::
> > > +
> > > +    => efidebug capsule disk-update
> > > +
> > > +**The efidebug command is should only be used during debugging/development.**
> > > +
> > > +Enabling Capsule Authentication
> > > +*******************************
> > > +
> > > +The UEFI specification defines a way of authenticating the capsule to
> > > +be updated by verifying the capsule signature. The capsule signature
> > > +is computed and prepended to the capsule payload at the time of
> > > +capsule generation. This signature is then verified by using the
> > > +public key stored as part of the X509 certificate. This certificate is
> > > +in the form of an efi signature list (esl) file, which is embedded as
> > > +part of U-Boot.
> > > +
> > > +The capsule authentication feature can be enabled through the
> > > +following config, in addition to the configs listed above for capsule
> > > +update::
> > > +
> > > +    CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > > +    CONFIG_EFI_CAPSULE_KEY_PATH=<path to .esl cert>
> > > +
> > > +The public and private keys used for the signing process are generated
> > > +and used by the steps highlighted below::
> > > +
> > > +    1. Install utility commands on your host
> > > +       * OPENSSL
> > > +       * efitools
> > > +
> > > +    2. Create signing keys and certificate files on your host
> > > +
> > > +        $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
> > > +            -keyout CRT.key -out CRT.crt -nodes -days 365
> > > +        $ cert-to-efi-sig-list CRT.crt CRT.esl
> > > +
> > > +        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
> > > +        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
> > > +
> > > +        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
> > > +        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
> > > +
> > > +The capsule file can be generated by using the GenerateCapsule.py
> > > +script in EDKII::
> > > +
> > > +    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> > > +      <capsule_file_name> --monotonic-count <val> --fw-version \
> > > +      <val> --lsv <val> --guid \
> > > +      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
> > > +      --update-image-index <val> --signer-private-cert \
> > > +      /path/to/CRT.pem --trusted-public-cert \
> > > +      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
> > > +      <u-boot.bin>
> > > +
> > > +Place the capsule generated in the above step on the EFI System
> > > +Partition under the EFI/UpdateCapsule directory
> > > +
> > > +Testing on QEMU
> > > +***************
> > > +
> > > +Currently, support has been added on the QEMU ARM64 virt platform for
> > > +updating the U-Boot binary as a raw image when the platform is booted
> > > +in non-secure mode, i.e. with CONFIG_TFABOOT disabled. For this
> > > +configuration, the QEMU platform needs to be booted with
> > > +'secure=off'. The U-Boot binary placed on the first bank of the NOR
> > > +flash at offset 0x0. The U-Boot environment is placed on the second
> > > +NOR flash bank at offset 0x4000000.
> > > +
> > > +The capsule update feature is enabled with the following configuration
> > > +settings::
> > > +
> > > +    CONFIG_MTD=y
> > > +    CONFIG_FLASH_CFI_MTD=y
> > > +    CONFIG_CMD_MTDPARTS=y
> > > +    CONFIG_CMD_DFU=y
> > > +    CONFIG_DFU_MTD=y
> > > +    CONFIG_PCI_INIT_R=y
> > > +    CONFIG_EFI_CAPSULE_ON_DISK=y
> > > +    CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
> > > +    CONFIG_EFI_CAPSULE_FIRMWARE=y
> > > +    CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > > +    CONFIG_EFI_CAPSULE_FMP_HEADER=y
> > > +
> > > +In addition, the following config needs to be disabled(QEMU ARM specific)::
> > > +
> > > +    CONFIG_TFABOOT
> > > +
> > > +The capsule file can be generated by using the tools/mkeficapsule::
> > > +
> > > +    $ mkeficapsule --raw <u-boot.bin> --index 1 <capsule_file_name>
> > > +
> > >   Executing the boot manager
> > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > > 
> > 

Re: [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[Takahiro Akashi]

Just a few minor comments:

On Fri, Jul 16, 2021 at 02:57:00PM +0900, Masami Hiramatsu wrote:
> 2021年7月16日(金) 2:00 Ilias Apalodimas <ilias.apalodimas@linaro.org>:
> >
> > The capsule signature is now part of our DTB.  This is problematic when a
> > user is allowed to change/fixup that DTB from U-Boots command line since he
> > can overwrite the signature as well.
> > So Instead of adding the key on the DTB, embed it in the u-boot binary it
> > self as part of it's .rodata.  This assumes that the U-Boot binary we load
> > is authenticated by a previous boot stage loader.
> >
> 
> I have tested this patch on DeveloperBox.
> Without signing the capsule,
> 
> ----
> ** Unrecognized filesystem type **
> Capsule authentication check failed. Aborting update
> Firmware update failed: <NULL>
> Applying capsule UBOOT1024.Cap failed
> PlatformLang:
> ----
> 
> With signing the capsule (by Takahiro's patch*)
> (*) https://lists.denx.de/pipermail/u-boot/2021-May/449878.html
> ----
> ** Unrecognized filesystem type **
> ####################################################################################################
> ###########################################################################PlatformLang:
> ----
> 
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> 
> IMHO, we should have a command to show that the current U-Boot
> has what certification from the console for debugging.
> (also, need to import Takahiro's patch)
> 
> Thank you,
> 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >  board/emulation/common/Makefile       |  1 -
> >  board/emulation/common/qemu_capsule.c | 43 ---------------------------
> >  include/asm-generic/sections.h        |  2 ++
> >  lib/efi_loader/Kconfig                |  6 ++++
> >  lib/efi_loader/Makefile               |  8 +++++
> >  lib/efi_loader/efi_capsule.c          | 18 +++++++++--
> >  lib/efi_loader/efi_capsule_key.S      |  8 +++++
> >  7 files changed, 39 insertions(+), 47 deletions(-)
> >  delete mode 100644 board/emulation/common/qemu_capsule.c
> >  create mode 100644 lib/efi_loader/efi_capsule_key.S
> >
> > diff --git a/board/emulation/common/Makefile b/board/emulation/common/Makefile
> > index 7ed447a69dce..c5b452e7e341 100644
> > --- a/board/emulation/common/Makefile
> > +++ b/board/emulation/common/Makefile
> > @@ -2,4 +2,3 @@
> >
> >  obj-$(CONFIG_SYS_MTDPARTS_RUNTIME) += qemu_mtdparts.o
> >  obj-$(CONFIG_SET_DFU_ALT_INFO) += qemu_dfu.o
> > -obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT) += qemu_capsule.o
> > diff --git a/board/emulation/common/qemu_capsule.c b/board/emulation/common/qemu_capsule.c
> > deleted file mode 100644
> > index 6b8a87022a4c..000000000000
> > --- a/board/emulation/common/qemu_capsule.c
> > +++ /dev/null
> > @@ -1,43 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0+
> > -/*
> > - * Copyright (c) 2020 Linaro Limited
> > - */
> > -
> > -#include <common.h>
> > -#include <efi_api.h>
> > -#include <efi_loader.h>
> > -#include <env.h>
> > -#include <fdtdec.h>
> > -#include <asm/global_data.h>
> > -
> > -DECLARE_GLOBAL_DATA_PTR;
> > -
> > -int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> > -{
> > -       const void *fdt_blob = gd->fdt_blob;
> > -       const void *blob;
> > -       const char *cnode_name = "capsule-key";
> > -       const char *snode_name = "signature";
> > -       int sig_node;
> > -       int len;
> > -
> > -       sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);
> > -       if (sig_node < 0) {
> > -               EFI_PRINT("Unable to get signature node offset\n");
> > -               return -FDT_ERR_NOTFOUND;
> > -       }
> > -
> > -       blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len);
> > -
> > -       if (!blob || len < 0) {
> > -               EFI_PRINT("Unable to get capsule-key value\n");
> > -               *pkey = NULL;
> > -               *pkey_len = 0;
> > -               return -FDT_ERR_NOTFOUND;
> > -       }
> > -
> > -       *pkey = (void *)blob;
> > -       *pkey_len = len;
> > -
> > -       return 0;
> > -}
> > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> > index 267f1db73f23..ec992b0c2e3f 100644
> > --- a/include/asm-generic/sections.h
> > +++ b/include/asm-generic/sections.h
> > @@ -27,6 +27,8 @@ extern char __efi_helloworld_begin[];
> >  extern char __efi_helloworld_end[];
> >  extern char __efi_var_file_begin[];
> >  extern char __efi_var_file_end[];
> > +extern char __efi_capsule_sig_begin[];
> > +extern char __efi_capsule_sig_end[];
> >
> >  /* Private data used by of-platdata devices/uclasses */
> >  extern char __priv_data_start[], __priv_data_end[];
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 156b39152112..42f1292fa04b 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -213,6 +213,12 @@ config EFI_CAPSULE_AUTHENTICATE
> >           Select this option if you want to enable capsule
> >           authentication
> >
> > +config EFI_CAPSULE_KEY_PATH
> > +       string "Path to .esl file for capsule authentication"
> > +       depends on EFI_CAPSULE_AUTHENTICATE
> > +       help
> > +         Provide the .esl file used for capsule authentication

We might be friendly if we add what "esl" means here.
More importantly, we are able to contain more than one signatures
if we want.

> > +
> >  config EFI_DEVICE_PATH_TO_TEXT
> >         bool "Device path to text protocol"
> >         default y
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index fd344cea29b0..9b369430e258 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -20,11 +20,19 @@ always += helloworld.efi
> >  targets += helloworld.o
> >  endif
> >
> > +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> > +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_KEY_PATH))
> > +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> > +$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_KEY_PATH)
> > +endif
> > +endif
> > +
> >  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> >  obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
> >  obj-y += efi_boottime.o
> >  obj-y += efi_helper.o
> >  obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> > +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o

We should give users another choice here to allow them to add their
own solution for key storage.
Or only enable this line if "CONFIG_EFI_CAPSULE_KEY_PATH" != null?

> >  obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> >  obj-y += efi_console.o
> >  obj-y += efi_device_path.o
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index b878e71438b8..50e93cad4ee5 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -16,6 +16,7 @@
> >  #include <mapmem.h>
> >  #include <sort.h>
> >
> > +#include <asm/sections.h>
> >  #include <crypto/pkcs7.h>
> >  #include <crypto/pkcs7_parser.h>
> >  #include <linux/err.h>
> > @@ -222,12 +223,23 @@ skip:
> >  const efi_guid_t efi_guid_capsule_root_cert_guid =
> >         EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> >
> > +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)

static?

> > +{
> > +       const void *blob = __efi_capsule_sig_begin;
> > +       const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;

It seems that the length can be calculated at compile time.

> > +       *pkey = (void *)blob;
> > +       *pkey_len = len;
> > +
> > +       return 0;
> > +}
> > +
> >  efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size,
> >                                       void **image, efi_uintn_t *image_size)
> >  {
> >         u8 *buf;
> >         int ret;
> > -       void *fdt_pkey, *pkey;
> > +       void *stored_pkey, *pkey;
> >         efi_uintn_t pkey_len;
> >         uint64_t monotonic_count;
> >         struct efi_signature_store *truststore;
> > @@ -286,7 +298,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> >                 goto out;
> >         }
> >
> > -       ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> > +       ret = efi_get_public_key_data(&stored_pkey, &pkey_len);
> >         if (ret < 0)
> >                 goto out;
> >
> > @@ -294,7 +306,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> >         if (!pkey)
> >                 goto out;
> >
> > -       memcpy(pkey, fdt_pkey, pkey_len);
> > +       memcpy(pkey, stored_pkey, pkey_len);
> >         truststore = efi_build_signature_store(pkey, pkey_len);
> >         if (!truststore)
> >                 goto out;
> > diff --git a/lib/efi_loader/efi_capsule_key.S b/lib/efi_loader/efi_capsule_key.S
> > new file mode 100644
> > index 000000000000..f7047a42e39d
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_capsule_key.S
> > @@ -0,0 +1,8 @@

Should we have "#include <config.h>" here?
Otherwise it looks good.

-Takahiro Akashi

> > +.section .rodata.capsule_key.init,"a"
> > +.balign 16
> > +.global __efi_capsule_sig_begin
> > +__efi_capsule_sig_begin:
> > +.incbin CONFIG_EFI_CAPSULE_KEY_PATH
> > +__efi_capsule_sig_end:
> > +.global __efi_capsule_sig_end
> > +.balign 16
> > --
> > 2.32.0.rc0
> >
> 
> 
> --
> Masami Hiramatsu

Re: [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[Simon Glass]

Hi Ilias,

On Thu, 15 Jul 2021 at 11:00, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> The capsule signature is now part of our DTB.  This is problematic when a
> user is allowed to change/fixup that DTB from U-Boots command line since he
> can overwrite the signature as well.

Do you mean with the 'fdt' command?

If you mean the FDT fixups, they happen to a different DT, the one
being passed to Linux.

> So Instead of adding the key on the DTB, embed it in the u-boot binary it
> self as part of it's .rodata.  This assumes that the U-Boot binary we load
> is authenticated by a previous boot stage loader.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  board/emulation/common/Makefile       |  1 -
>  board/emulation/common/qemu_capsule.c | 43 ---------------------------
>  include/asm-generic/sections.h        |  2 ++
>  lib/efi_loader/Kconfig                |  6 ++++
>  lib/efi_loader/Makefile               |  8 +++++
>  lib/efi_loader/efi_capsule.c          | 18 +++++++++--
>  lib/efi_loader/efi_capsule_key.S      |  8 +++++
>  7 files changed, 39 insertions(+), 47 deletions(-)
>  delete mode 100644 board/emulation/common/qemu_capsule.c
>  create mode 100644 lib/efi_loader/efi_capsule_key.S

Regards,
Simon

Re: [PATCH 2/3] mkeficapsule: Remove dtb related options

[Takahiro Akashi]

On Fri, Jul 16, 2021 at 02:57:54PM +0900, Masami Hiramatsu wrote:
> 2021年7月16日(金) 2:00 Ilias Apalodimas <ilias.apalodimas@linaro.org>:
> >
> > commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
> > added a bunch of options enabling the addition of the capsule public key
> > in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
> > this functionality anymore
> 
> This looks good to me.
> 
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> 
> Thanks,
> 
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >  tools/mkeficapsule.c | 226 ++-----------------------------------------
> >  1 file changed, 7 insertions(+), 219 deletions(-)
> >
> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > index de0a62898886..214dc38e46e3 100644
> > --- a/tools/mkeficapsule.c
> > +++ b/tools/mkeficapsule.c
> > @@ -4,14 +4,12 @@
> >   *             Author: AKASHI Takahiro
> >   */
> >
> > -#include <errno.h>
> >  #include <getopt.h>
> >  #include <malloc.h>
> >  #include <stdbool.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> > -#include <unistd.h>
> >  #include <linux/types.h>
> >
> >  #include <sys/mman.h>

I didn't try the compilation, but I don't think
we need neither <sys/mman.h> nor "fdt_host.h".

-Takahiro Akashi

> > @@ -29,9 +27,6 @@ typedef __s32 s32;
> >
> >  #define aligned_u64 __aligned_u64
> >
> > -#define SIGNATURE_NODENAME     "signature"
> > -#define OVERLAY_NODENAME       "__overlay__"
> > -
> >  #ifndef __packed
> >  #define __packed __attribute__((packed))
> >  #endif
> > @@ -52,9 +47,6 @@ static struct option options[] = {
> >         {"raw", required_argument, NULL, 'r'},
> >         {"index", required_argument, NULL, 'i'},
> >         {"instance", required_argument, NULL, 'I'},
> > -       {"dtb", required_argument, NULL, 'D'},
> > -       {"public key", required_argument, NULL, 'K'},
> > -       {"overlay", no_argument, NULL, 'O'},
> >         {"help", no_argument, NULL, 'h'},
> >         {NULL, 0, NULL, 0},
> >  };
> > @@ -68,187 +60,10 @@ static void print_usage(void)
> >                "\t-r, --raw <raw image>       new raw image file\n"
> >                "\t-i, --index <index>         update image index\n"
> >                "\t-I, --instance <instance>   update hardware instance\n"
> > -              "\t-K, --public-key <key file> public key esl file\n"
> > -              "\t-D, --dtb <dtb file>        dtb file\n"
> > -              "\t-O, --overlay               the dtb file is an overlay\n"
> >                "\t-h, --help                  print a help message\n",
> >                tool_name);
> >  }
> >
> > -static int fdt_add_pub_key_data(void *sptr, void *dptr, size_t key_size,
> > -                               bool overlay)
> > -{
> > -       int parent;
> > -       int ov_node;
> > -       int frag_node;
> > -       int ret = 0;
> > -
> > -       if (overlay) {
> > -               /*
> > -                * The signature would be stored in the
> > -                * first fragment node of the overlay
> > -                */
> > -               frag_node = fdt_first_subnode(dptr, 0);
> > -               if (frag_node == -FDT_ERR_NOTFOUND) {
> > -                       fprintf(stderr,
> > -                               "Couldn't find the fragment node: %s\n",
> > -                               fdt_strerror(frag_node));
> > -                       goto done;
> > -               }
> > -
> > -               ov_node = fdt_subnode_offset(dptr, frag_node, OVERLAY_NODENAME);
> > -               if (ov_node == -FDT_ERR_NOTFOUND) {
> > -                       fprintf(stderr,
> > -                               "Couldn't find the __overlay__ node: %s\n",
> > -                               fdt_strerror(ov_node));
> > -                       goto done;
> > -               }
> > -       } else {
> > -               ov_node = 0;
> > -       }
> > -
> > -       parent = fdt_subnode_offset(dptr, ov_node, SIGNATURE_NODENAME);
> > -       if (parent == -FDT_ERR_NOTFOUND) {
> > -               parent = fdt_add_subnode(dptr, ov_node, SIGNATURE_NODENAME);
> > -               if (parent < 0) {
> > -                       ret = parent;
> > -                       if (ret != -FDT_ERR_NOSPACE) {
> > -                               fprintf(stderr,
> > -                                       "Couldn't create signature node: %s\n",
> > -                                       fdt_strerror(parent));
> > -                       }
> > -               }
> > -       }
> > -       if (ret)
> > -               goto done;
> > -
> > -       /* Write the key to the FDT node */
> > -       ret = fdt_setprop(dptr, parent, "capsule-key",
> > -                         sptr, key_size);
> > -
> > -done:
> > -       if (ret)
> > -               ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
> > -
> > -       return ret;
> > -}
> > -
> > -static int add_public_key(const char *pkey_file, const char *dtb_file,
> > -                         bool overlay)
> > -{
> > -       int ret;
> > -       int srcfd = -1;
> > -       int destfd = -1;
> > -       void *sptr = NULL;
> > -       void *dptr = NULL;
> > -       off_t src_size;
> > -       struct stat pub_key;
> > -       struct stat dtb;
> > -
> > -       /* Find out the size of the public key */
> > -       srcfd = open(pkey_file, O_RDONLY);
> > -       if (srcfd == -1) {
> > -               fprintf(stderr, "%s: Can't open %s: %s\n",
> > -                       __func__, pkey_file, strerror(errno));
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       ret = fstat(srcfd, &pub_key);
> > -       if (ret == -1) {
> > -               fprintf(stderr, "%s: Can't stat %s: %s\n",
> > -                       __func__, pkey_file, strerror(errno));
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       src_size = pub_key.st_size;
> > -
> > -       /* mmap the public key esl file */
> > -       sptr = mmap(0, src_size, PROT_READ, MAP_SHARED, srcfd, 0);
> > -       if (sptr == MAP_FAILED) {
> > -               fprintf(stderr, "%s: Failed to mmap %s:%s\n",
> > -                       __func__, pkey_file, strerror(errno));
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       /* Open the dest FDT */
> > -       destfd = open(dtb_file, O_RDWR);
> > -       if (destfd == -1) {
> > -               fprintf(stderr, "%s: Can't open %s: %s\n",
> > -                       __func__, dtb_file, strerror(errno));
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       ret = fstat(destfd, &dtb);
> > -       if (ret == -1) {
> > -               fprintf(stderr, "%s: Can't stat %s: %s\n",
> > -                       __func__, dtb_file, strerror(errno));
> > -               goto err;
> > -       }
> > -
> > -       dtb.st_size += src_size + 0x30;
> > -       if (ftruncate(destfd, dtb.st_size)) {
> > -               fprintf(stderr, "%s: Can't expand %s: %s\n",
> > -                       __func__, dtb_file, strerror(errno));
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       errno = 0;
> > -       /* mmap the dtb file */
> > -       dptr = mmap(0, dtb.st_size, PROT_READ | PROT_WRITE, MAP_SHARED,
> > -                   destfd, 0);
> > -       if (dptr == MAP_FAILED) {
> > -               fprintf(stderr, "%s: Failed to mmap %s:%s\n",
> > -                       __func__, dtb_file, strerror(errno));
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       if (fdt_check_header(dptr)) {
> > -               fprintf(stderr, "%s: Invalid FDT header\n", __func__);
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       ret = fdt_open_into(dptr, dptr, dtb.st_size);
> > -       if (ret) {
> > -               fprintf(stderr, "%s: Cannot expand FDT: %s\n",
> > -                       __func__, fdt_strerror(ret));
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       /* Copy the esl file to the expanded FDT */
> > -       ret = fdt_add_pub_key_data(sptr, dptr, src_size, overlay);
> > -       if (ret < 0) {
> > -               fprintf(stderr, "%s: Unable to add public key to the FDT\n",
> > -                       __func__);
> > -               ret = -1;
> > -               goto err;
> > -       }
> > -
> > -       ret = 0;
> > -
> > -err:
> > -       if (sptr)
> > -               munmap(sptr, src_size);
> > -
> > -       if (dptr)
> > -               munmap(dptr, dtb.st_size);
> > -
> > -       if (srcfd != -1)
> > -               close(srcfd);
> > -
> > -       if (destfd != -1)
> > -               close(destfd);
> > -
> > -       return ret;
> > -}
> > -
> >  static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> >                         unsigned long index, unsigned long instance)
> >  {
> > @@ -366,22 +181,16 @@ err_1:
> >  int main(int argc, char **argv)
> >  {
> >         char *file;
> > -       char *pkey_file;
> > -       char *dtb_file;
> >         efi_guid_t *guid;
> >         unsigned long index, instance;
> >         int c, idx;
> > -       int ret;
> > -       bool overlay = false;
> >
> >         file = NULL;
> > -       pkey_file = NULL;
> > -       dtb_file = NULL;
> >         guid = NULL;
> >         index = 0;
> >         instance = 0;
> >         for (;;) {
> > -               c = getopt_long(argc, argv, "f:r:i:I:v:D:K:Oh", options, &idx);
> > +               c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx);
> >                 if (c == -1)
> >                         break;
> >
> > @@ -408,43 +217,22 @@ int main(int argc, char **argv)
> >                 case 'I':
> >                         instance = strtoul(optarg, NULL, 0);
> >                         break;
> > -               case 'K':
> > -                       if (pkey_file) {
> > -                               printf("Public Key already specified\n");
> > -                               return -1;
> > -                       }
> > -                       pkey_file = optarg;
> > -                       break;
> > -               case 'D':
> > -                       if (dtb_file) {
> > -                               printf("DTB file already specified\n");
> > -                               return -1;
> > -                       }
> > -                       dtb_file = optarg;
> > -                       break;
> > -               case 'O':
> > -                       overlay = true;
> > -                       break;
> >                 case 'h':
> >                         print_usage();
> >                         return 0;
> >                 }
> >         }
> >
> > -       /* need a fit image file or raw image file */
> > -       if (!file && !pkey_file && !dtb_file) {
> > +       /* need a output file */
> > +       if (argc != optind + 1) {
> >                 print_usage();
> >                 exit(EXIT_FAILURE);
> >         }
> >
> > -       if (pkey_file && dtb_file) {
> > -               ret = add_public_key(pkey_file, dtb_file, overlay);
> > -               if (ret == -1) {
> > -                       printf("Adding public key to the dtb failed\n");
> > -                       exit(EXIT_FAILURE);
> > -               } else {
> > -                       exit(EXIT_SUCCESS);
> > -               }
> > +       /* need a fit image file or raw image file */
> > +       if (!file) {
> > +               print_usage();
> > +               exit(EXIT_SUCCESS);
> >         }
> >
> >         if (create_fwbin(argv[optind], file, guid, index, instance)
> > --
> > 2.32.0.rc0
> >
> 
> 
> -- 
> Masami Hiramatsu

Re: [PATCH 2/3] mkeficapsule: Remove dtb related options

[Simon Glass]

Hi Ilias,

On Thu, 15 Jul 2021 at 11:00, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
> added a bunch of options enabling the addition of the capsule public key
> in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
> this functionality anymore
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  tools/mkeficapsule.c | 226 ++-----------------------------------------
>  1 file changed, 7 insertions(+), 219 deletions(-)

Here again I see EFI diverging from the impl in U-Boot. WIth U-Boot
you can add the public key after the build step, e.g. in a key-signing
server. With EFI and this change you will have to rebuild U-Boot (from
source) every time you sign something. Seems like a pain.

Regards,
Simon

Re: [PATCH 2/3] mkeficapsule: Remove dtb related options

[Ilias Apalodimas]

On Fri, Jul 16, 2021 at 08:03:23AM -0600, Simon Glass wrote:
> Hi Ilias,
> 
> On Thu, 15 Jul 2021 at 11:00, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
> > added a bunch of options enabling the addition of the capsule public key
> > in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
> > this functionality anymore
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >  tools/mkeficapsule.c | 226 ++-----------------------------------------
> >  1 file changed, 7 insertions(+), 219 deletions(-)
> 
> Here again I see EFI diverging from the impl in U-Boot. WIth U-Boot
> you can add the public key after the build step, e.g. in a key-signing
> server. With EFI and this change you will have to rebuild U-Boot (from
> source) every time you sign something. Seems like a pain.

I don't see why either of this is a problem.  You need the public key to
update the binary it self, so rebuilding from source is a prerequisite. 

Apart from a signing server, you can also have special hardware that provides 
the public key you need (which is not implemented yet).  So this is the bare 
minimum functionality you need  for authenticated capsule updates.


Regards
/Ilias

> 
> Regards,
> Simon

Re: [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[Ilias Apalodimas]

> > >

[...]

> > > +config EFI_CAPSULE_KEY_PATH
> > > +       string "Path to .esl file for capsule authentication"
> > > +       depends on EFI_CAPSULE_AUTHENTICATE
> > > +       help
> > > +         Provide the .esl file used for capsule authentication
> 
> We might be friendly if we add what "esl" means here.
> More importantly, we are able to contain more than one signatures
> if we want.

Sure, I'll replace it on the help message

> 
> > > +
> > >  config EFI_DEVICE_PATH_TO_TEXT
> > >         bool "Device path to text protocol"
> > >         default y
> > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > > index fd344cea29b0..9b369430e258 100644
> > > --- a/lib/efi_loader/Makefile
> > > +++ b/lib/efi_loader/Makefile
> > > @@ -20,11 +20,19 @@ always += helloworld.efi
> > >  targets += helloworld.o
> > >  endif
> > >
> > > +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> > > +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_KEY_PATH))
> > > +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> > > +$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_KEY_PATH)
> > > +endif
> > > +endif
> > > +
> > >  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> > >  obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
> > >  obj-y += efi_boottime.o
> > >  obj-y += efi_helper.o
> > >  obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> > > +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
> 
> We should give users another choice here to allow them to add their
> own solution for key storage.
> Or only enable this line if "CONFIG_EFI_CAPSULE_KEY_PATH" != null?
> 
> > >  obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> > >  obj-y += efi_console.o
> > >  obj-y += efi_device_path.o
> > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > index b878e71438b8..50e93cad4ee5 100644
> > > --- a/lib/efi_loader/efi_capsule.c
> > > +++ b/lib/efi_loader/efi_capsule.c
> > > @@ -16,6 +16,7 @@
> > >  #include <mapmem.h>
> > >  #include <sort.h>
> > >
> > > +#include <asm/sections.h>
> > >  #include <crypto/pkcs7.h>
> > >  #include <crypto/pkcs7_parser.h>
> > >  #include <linux/err.h>
> > > @@ -222,12 +223,23 @@ skip:
> > >  const efi_guid_t efi_guid_capsule_root_cert_guid =
> > >         EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> > >
> > > +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> 
> static?

yea

> 
> > > +{
> > > +       const void *blob = __efi_capsule_sig_begin;
> > > +       const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
> 
> It seems that the length can be calculated at compile time.
> 

Yea but you still need the __efi_capsule_sig_begin.  What's the proposal
here? Replace __efi_capsule_sig_end with the size on the .S file?

> > > +       *pkey = (void *)blob;
> > > +       *pkey_len = len;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size,
> > >                                       void **image, efi_uintn_t *image_size)
> > >  {
> > >         u8 *buf;
> > >         int ret;
> > > -       void *fdt_pkey, *pkey;
> > > +       void *stored_pkey, *pkey;
> > >         efi_uintn_t pkey_len;
> > >         uint64_t monotonic_count;
> > >         struct efi_signature_store *truststore;
> > > @@ -286,7 +298,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> > >                 goto out;
> > >         }
> > >
> > > -       ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> > > +       ret = efi_get_public_key_data(&stored_pkey, &pkey_len);
> > >         if (ret < 0)
> > >                 goto out;
> > >
> > > @@ -294,7 +306,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> > >         if (!pkey)
> > >                 goto out;
> > >
> > > -       memcpy(pkey, fdt_pkey, pkey_len);
> > > +       memcpy(pkey, stored_pkey, pkey_len);
> > >         truststore = efi_build_signature_store(pkey, pkey_len);
> > >         if (!truststore)
> > >                 goto out;
> > > diff --git a/lib/efi_loader/efi_capsule_key.S b/lib/efi_loader/efi_capsule_key.S
> > > new file mode 100644
> > > index 000000000000..f7047a42e39d
> > > --- /dev/null
> > > +++ b/lib/efi_loader/efi_capsule_key.S
> > > @@ -0,0 +1,8 @@
> 
> Should we have "#include <config.h>" here?

Hmm maybe. Compiling didn't cause any problems, but it seems we can add
that include


> Otherwise it looks good.
> 
> -Takahiro Akashi

Thanks
/Ilias
> 
> > > +.section .rodata.capsule_key.init,"a"
> > > +.balign 16
> > > +.global __efi_capsule_sig_begin
> > > +__efi_capsule_sig_begin:
> > > +.incbin CONFIG_EFI_CAPSULE_KEY_PATH
> > > +__efi_capsule_sig_end:
> > > +.global __efi_capsule_sig_end
> > > +.balign 16
> > > --
> > > 2.32.0.rc0
> > >
> > 
> > 
> > --
> > Masami Hiramatsu

Re: [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[Ilias Apalodimas]

On Fri, Jul 16, 2021 at 07:49:09AM -0600, Simon Glass wrote:
> Hi Ilias,
> 
> On Thu, 15 Jul 2021 at 11:00, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > The capsule signature is now part of our DTB.  This is problematic when a
> > user is allowed to change/fixup that DTB from U-Boots command line since he
> > can overwrite the signature as well.
> 
> Do you mean with the 'fdt' command?
> 
> If you mean the FDT fixups, they happen to a different DT, the one
> being passed to Linux.
> 

This was only usable in QEMU pre-patch.  I think Sughosh replaced the
entire DTB (including the signature) on his tests.

Sughosh?

Cheers
/Ilias

> > So Instead of adding the key on the DTB, embed it in the u-boot binary it
> > self as part of it's .rodata.  This assumes that the U-Boot binary we load
> > is authenticated by a previous boot stage loader.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >  board/emulation/common/Makefile       |  1 -
> >  board/emulation/common/qemu_capsule.c | 43 ---------------------------
> >  include/asm-generic/sections.h        |  2 ++
> >  lib/efi_loader/Kconfig                |  6 ++++
> >  lib/efi_loader/Makefile               |  8 +++++
> >  lib/efi_loader/efi_capsule.c          | 18 +++++++++--
> >  lib/efi_loader/efi_capsule_key.S      |  8 +++++
> >  7 files changed, 39 insertions(+), 47 deletions(-)
> >  delete mode 100644 board/emulation/common/qemu_capsule.c
> >  create mode 100644 lib/efi_loader/efi_capsule_key.S
> 
> Regards,
> Simon

Re: [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata

[Ilias Apalodimas]

On Sat, 17 Jul 2021 at 14:35, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> > > >

[...]

> > > >  obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> > > > +obj-$(CONFIG_EFI_CAPSULE_AUTHENTICATE) += efi_capsule_key.o
> >
> > We should give users another choice here to allow them to add their
> > own solution for key storage.
> > Or only enable this line if "CONFIG_EFI_CAPSULE_KEY_PATH" != null?

Actually once you enable the capsule authentication compilation fails
now if a file is not provided, with a message asking for the use to
provide a valid filepath.  I'd prefer leaving it as is and once we get
a hardware that can override the embedded key, we can add an extra
Kconfig option.

> >
> > > >  obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> > > >  obj-y += efi_console.o
> > > >  obj-y += efi_device_path.o
> > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > > index b878e71438b8..50e93cad4ee5 100644
> > > > --- a/lib/efi_loader/efi_capsule.c
> > > > +++ b/lib/efi_loader/efi_capsule.c
> > > > @@ -16,6 +16,7 @@
> > > >  #include <mapmem.h>
> > > >  #include <sort.h>
> > > >
> > > > +#include <asm/sections.h>
> > > >  #include <crypto/pkcs7.h>
> > > >  #include <crypto/pkcs7_parser.h>
> > > >  #include <linux/err.h>
> > > > @@ -222,12 +223,23 @@ skip:
> > > >  const efi_guid_t efi_guid_capsule_root_cert_guid =
> > > >         EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> > > >
> > > > +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> >
> > static?
>
> yea

Once we do get support for hardware stored signatures, this can be
changed to a __weak function.

>
> >
> > > > +{
> > > > +       const void *blob = __efi_capsule_sig_begin;
> > > > +       const int len = __efi_capsule_sig_end - __efi_capsule_sig_begin;
> >
> > It seems that the length can be calculated at compile time.
> >
>
> Yea but you still need the __efi_capsule_sig_begin.  What's the proposal
> here? Replace __efi_capsule_sig_end with the size on the .S file?
>
> > > > +       *pkey = (void *)blob;
> > > > +       *pkey_len = len;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > >  efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size,
> > > >                                       void **image, efi_uintn_t *image_size)
> > > >  {
> > > >         u8 *buf;
> > > >         int ret;
> > > > -       void *fdt_pkey, *pkey;
> > > > +       void *stored_pkey, *pkey;
> > > >         efi_uintn_t pkey_len;
> > > >         uint64_t monotonic_count;
> > > >         struct efi_signature_store *truststore;
> > > > @@ -286,7 +298,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> > > >                 goto out;
> > > >         }
> > > >
> > > > -       ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
> > > > +       ret = efi_get_public_key_data(&stored_pkey, &pkey_len);
> > > >         if (ret < 0)
> > > >                 goto out;
> > > >
> > > > @@ -294,7 +306,7 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> > > >         if (!pkey)
> > > >                 goto out;
> > > >
> > > > -       memcpy(pkey, fdt_pkey, pkey_len);
> > > > +       memcpy(pkey, stored_pkey, pkey_len);
> > > >         truststore = efi_build_signature_store(pkey, pkey_len);
> > > >         if (!truststore)
> > > >                 goto out;
> > > > diff --git a/lib/efi_loader/efi_capsule_key.S b/lib/efi_loader/efi_capsule_key.S
> > > > new file mode 100644
> > > > index 000000000000..f7047a42e39d
> > > > --- /dev/null
> > > > +++ b/lib/efi_loader/efi_capsule_key.S
> > > > @@ -0,0 +1,8 @@
> >
> > Should we have "#include <config.h>" here?
>
> Hmm maybe. Compiling didn't cause any problems, but it seems we can add
> that include
>
>
> > Otherwise it looks good.
> >
> > -Takahiro Akashi
>
> Thanks
> /Ilias
> >
> > > > +.section .rodata.capsule_key.init,"a"
> > > > +.balign 16
> > > > +.global __efi_capsule_sig_begin
> > > > +__efi_capsule_sig_begin:
> > > > +.incbin CONFIG_EFI_CAPSULE_KEY_PATH
> > > > +__efi_capsule_sig_end:
> > > > +.global __efi_capsule_sig_end
> > > > +.balign 16
> > > > --
> > > > 2.32.0.rc0
> > > >
> > >
> > >
> > > --
> > > Masami Hiramatsu

Re: [PATCH 2/3] mkeficapsule: Remove dtb related options

[Simon Glass]

Hi Ilias,

On Sat, 17 Jul 2021 at 01:24, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Fri, Jul 16, 2021 at 08:03:23AM -0600, Simon Glass wrote:
> > Hi Ilias,
> >
> > On Thu, 15 Jul 2021 at 11:00, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
> > > added a bunch of options enabling the addition of the capsule public key
> > > in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
> > > this functionality anymore
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > ---
> > >  tools/mkeficapsule.c | 226 ++-----------------------------------------
> > >  1 file changed, 7 insertions(+), 219 deletions(-)
> >
> > Here again I see EFI diverging from the impl in U-Boot. WIth U-Boot
> > you can add the public key after the build step, e.g. in a key-signing
> > server. With EFI and this change you will have to rebuild U-Boot (from
> > source) every time you sign something. Seems like a pain.
>
> I don't see why either of this is a problem.  You need the public key to
> update the binary it self, so rebuilding from source is a prerequisite.

Please can you have a look at binman and the concept of packaging
separate from building? Rebuilding from source is definitely not
needed to update a binary.

>
> Apart from a signing server, you can also have special hardware that provides
> the public key you need (which is not implemented yet).  So this is the bare
> minimum functionality you need  for authenticated capsule updates.

As discussed on the mailing list you have not included the motivation
for this. Now that I understand the motivation, which is to avoid
someone changing the key at runtime, I believe that this change does
not actually help...I've replied separately on the mailing list.

Regards,
Simon

Re: [PATCH 2/3] mkeficapsule: Remove dtb related options

[Ilias Apalodimas]

On Tue, 20 Jul 2021 at 21:33, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ilias,
>
> On Sat, 17 Jul 2021 at 01:24, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > On Fri, Jul 16, 2021 at 08:03:23AM -0600, Simon Glass wrote:
> > > Hi Ilias,
> > >
> > > On Thu, 15 Jul 2021 at 11:00, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
> > > > added a bunch of options enabling the addition of the capsule public key
> > > > in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
> > > > this functionality anymore
> > > >
> > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > ---
> > > >  tools/mkeficapsule.c | 226 ++-----------------------------------------
> > > >  1 file changed, 7 insertions(+), 219 deletions(-)
> > >
> > > Here again I see EFI diverging from the impl in U-Boot. WIth U-Boot
> > > you can add the public key after the build step, e.g. in a key-signing
> > > server. With EFI and this change you will have to rebuild U-Boot (from
> > > source) every time you sign something. Seems like a pain.
> >
> > I don't see why either of this is a problem.  You need the public key to
> > update the binary it self, so rebuilding from source is a prerequisite.
>
> Please can you have a look at binman and the concept of packaging
> separate from building? Rebuilding from source is definitely not
> needed to update a binary.

Sure I'll take a look. We already have an mkeficapsule.c though, which
in theory could take care of the capsule signing.  The point is that
we don't uses that key to sign anything, we use it to authenticate the
capsule that tries to update the firmware.

>
> >
> > Apart from a signing server, you can also have special hardware that provides
> > the public key you need (which is not implemented yet).  So this is the bare
> > minimum functionality you need  for authenticated capsule updates.
>
> As discussed on the mailing list you have not included the motivation
> for this.

To be fair, I did on patch 1/3.

> Now that I understand the motivation, which is to avoid
> someone changing the key at runtime, I believe that this change does
> not actually help...I've replied separately on the mailing list.

It does help, but you need combined code which doesn't exist in either
case.  Anyway, I'll reply on the other thread

Cheers
/Ilias
>
> Regards,
> Simon

Re: [PATCH 2/3] mkeficapsule: Remove dtb related options

[Simon Glass]

Hi Ilias,

On Tue, 20 Jul 2021 at 12:43, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Tue, 20 Jul 2021 at 21:33, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Sat, 17 Jul 2021 at 01:24, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > On Fri, Jul 16, 2021 at 08:03:23AM -0600, Simon Glass wrote:
> > > > Hi Ilias,
> > > >
> > > > On Thu, 15 Jul 2021 at 11:00, Ilias Apalodimas
> > > > <ilias.apalodimas@linaro.org> wrote:
> > > > >
> > > > > commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
> > > > > added a bunch of options enabling the addition of the capsule public key
> > > > > in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
> > > > > this functionality anymore
> > > > >
> > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > ---
> > > > >  tools/mkeficapsule.c | 226 ++-----------------------------------------
> > > > >  1 file changed, 7 insertions(+), 219 deletions(-)
> > > >
> > > > Here again I see EFI diverging from the impl in U-Boot. WIth U-Boot
> > > > you can add the public key after the build step, e.g. in a key-signing
> > > > server. With EFI and this change you will have to rebuild U-Boot (from
> > > > source) every time you sign something. Seems like a pain.
> > >
> > > I don't see why either of this is a problem.  You need the public key to
> > > update the binary it self, so rebuilding from source is a prerequisite.
> >
> > Please can you have a look at binman and the concept of packaging
> > separate from building? Rebuilding from source is definitely not
> > needed to update a binary.
>
> Sure I'll take a look. We already have an mkeficapsule.c though, which
> in theory could take care of the capsule signing.  The point is that
> we don't uses that key to sign anything, we use it to authenticate the
> capsule that tries to update the firmware.

That is not the key point IMO :-)

FIT signing works the same way...it is the public key. So I fully
understand that is how it works.

>
> >
> > >
> > > Apart from a signing server, you can also have special hardware that provides
> > > the public key you need (which is not implemented yet).  So this is the bare
> > > minimum functionality you need  for authenticated capsule updates.
> >
> > As discussed on the mailing list you have not included the motivation
> > for this.
>
> To be fair, I did on patch 1/3.

OK I see. Then I believe the motivation is misplaced / incorrect for
reasons mentioned on IRC...you have bigger problems than just the key
in the DT and you yourself mention the power of the command line if
the user has access.

>
> > Now that I understand the motivation, which is to avoid
> > someone changing the key at runtime, I believe that this change does
> > not actually help...I've replied separately on the mailing list.
>
> It does help, but you need combined code which doesn't exist in either
> case.  Anyway, I'll reply on the other thread

I still don't think this helps at all.

Regards,
Simon