[PATCH 00/16] tools: Add support for signing devicetree blobs

[PATCH 00/16] tools: Add support for signing devicetree blobs

[Simon Glass]

At present mkimage supports signing FITs, the standard U-Boot image type.

Various people are opposed to using FIT since:

a) it requires adding support for FIT into other bootloaders, notably
   UEFI
b) it requires packaging a kernel in this standard U-Boot format, meaning
   that distros must run 'mkimage' and deal with the kernel and initrd
   being inside a FIT

The kernel and initrd can be dealt with in other ways. But without FIT,
we have no standard way of signing and grouping FDT files. Instead we must
include them in the distro as separate files.

In particular, some sort of mechanism for verifying FDT files is needed.
One option would be to tack a signature on before or after the file,
processing it accordingly. But due to the nature of the FDT binary format,
it is possible to embed a signature inside the FDT itself, which is very
convenient.

This series provides a tool, fdt_sign, which can add a signature to an
FDT. The signature can be checked later, preventing any change to the FDT,
other than in permitted nodes (e.g. /chosen).

This series also provides a fdt_check_sign tool, used to check signatures.

Both of these tools are stand-alone do not require mkimage or FIT.

As with FIT signing, multiple signatures are possible, but in this case
that requires that fit_sign be called once for each signature. To make the
check fail if a signature does not match, it should be marked as
'required' using the -r flag to fdt_sign.

Run-time support for checking FDT signatures could be added to U-Boot
fairly easily, but needs further discussion as the correct plumbing needs
to be determined.

For now there is absolutely no configurability in the signature mechanism.
It would of course be possible to adjust which nodes are signed, as is
done for FIT, but that needs further discussion also. The omission of the
/chosen node is implemented in h_exclude_nodes() like this:

   if (type == FDT_IS_NODE) {
      /* Ignore the chosen node as well as /signature and subnodes */
      if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
         return 0;
   }

Man pages are provided with example usage of the tools. Use this to view
them:

   man -l doc/fdt_check_sign.1

This series also includes various clean-ups noticed along the way, as well
as refactoring to avoid code duplication with the new tools. The last four
patches are the new code.

This series is available at u-boot-dm/fdt-sign-working :

  https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working


Simon Glass (16):
  rsa: Add debugging for failure cases
  fit_check_sign: Update help to mention the key is in a dtb
  tools: Move copyfile() into a common file
  tools: Avoid leaving extra data at the end of copied files
  tools: Improve comments in signing functions
  tools: Drop unused name in image-host
  tools: Avoid confusion between keys and signatures
  tools: Tidy up argument order in fit_config_check_sig()
  tools: Pass the key blob around
  image: Return destination node for add_verify_data() method
  tools: Pass public-key node through to caller
  tools: mkimage: Show where signatures/keys are written
  tools: Add a new tool to sign FDT blobs
  tools: Add a new tool to check FDT-blob signatures
  test: Add a test for FDT signing
  tools: Add man pages for fdt_sign and fdt_check_sign

 MAINTAINERS                      |   7 +
 boot/image-fit-sig.c             | 151 +++++++++----
 boot/image-fit.c                 |  12 +-
 common/spl/spl_fit.c             |   3 +-
 doc/fdt_check_sign.1             |  74 +++++++
 doc/fdt_sign.1                   | 111 ++++++++++
 include/image.h                  |  80 ++++++-
 include/u-boot/ecdsa.h           |   5 +-
 include/u-boot/rsa.h             |   5 +-
 lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
 lib/rsa/rsa-sign.c               |   5 +-
 lib/rsa/rsa-verify.c             |  13 +-
 test/py/tests/test_fdt_sign.py   |  83 ++++++++
 test/py/tests/test_vboot.py      |  21 +-
 test/py/tests/vboot/sign-fdt.dts |  23 ++
 test/py/tests/vboot_comm.py      |  22 ++
 tools/Makefile                   |  10 +-
 tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
 tools/fdt_check_sign.c           |  85 ++++++++
 tools/fdt_host.h                 |  46 ++++
 tools/fdt_sign.c                 | 210 ++++++++++++++++++
 tools/fit_check_sign.c           |   4 +-
 tools/fit_common.c               |  69 ++++++
 tools/fit_common.h               |  23 ++
 tools/fit_image.c                |  59 +-----
 tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
 tools/image-host.c               | 155 +++++++++++---
 tools/imagetool.h                |   4 +
 tools/mkimage.c                  |   4 +
 29 files changed, 1714 insertions(+), 181 deletions(-)
 create mode 100644 doc/fdt_check_sign.1
 create mode 100644 doc/fdt_sign.1
 create mode 100644 test/py/tests/test_fdt_sign.py
 create mode 100644 test/py/tests/vboot/sign-fdt.dts
 create mode 100644 test/py/tests/vboot_comm.py
 create mode 100644 tools/fdt-host.c
 create mode 100644 tools/fdt_check_sign.c
 create mode 100644 tools/fdt_sign.c
 create mode 100644 tools/image-fdt-sig.c

-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 01/16] rsa: Add debugging for failure cases

[Simon Glass]

Add some more debugging to make it easier to see what is being tried and
what fails. Fix a few comment styles while here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/rsa/rsa-verify.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 83f7564101c..26ee256cf4c 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -79,14 +79,14 @@ int padding_pkcs_15_verify(struct image_sign_info *info,
 	struct checksum_algo *checksum = info->checksum;
 	int ret, pad_len = msg_len - checksum->checksum_len;
 
-	/* Check pkcs1.5 padding bytes. */
+	/* Check pkcs1.5 padding bytes */
 	ret = rsa_verify_padding(msg, pad_len, checksum);
 	if (ret) {
 		debug("In RSAVerify(): Padding check failed!\n");
 		return -EINVAL;
 	}
 
-	/* Check hash. */
+	/* Check hash */
 	if (memcmp((uint8_t *)msg + pad_len, hash, msg_len - pad_len)) {
 		debug("In RSAVerify(): Hash check failed!\n");
 		return -EACCES;
@@ -502,7 +502,8 @@ int rsa_verify_hash(struct image_sign_info *info,
 	if (CONFIG_IS_ENABLED(RSA_VERIFY_WITH_PKEY) && !info->fdt_blob) {
 		/* don't rely on fdt properties */
 		ret = rsa_verify_with_pkey(info, hash, sig, sig_len);
-
+		if (ret)
+			debug("%s: rsa_verify_with_pkey() failed\n", __func__);
 		return ret;
 	}
 
@@ -522,6 +523,9 @@ int rsa_verify_hash(struct image_sign_info *info,
 		if (info->required_keynode != -1) {
 			ret = rsa_verify_with_keynode(info, hash, sig, sig_len,
 						      info->required_keynode);
+			if (ret)
+				debug("%s: Failed to verify required_keynode\n",
+				      __func__);
 			return ret;
 		}
 
@@ -531,6 +535,8 @@ int rsa_verify_hash(struct image_sign_info *info,
 		ret = rsa_verify_with_keynode(info, hash, sig, sig_len, node);
 		if (!ret)
 			return ret;
+		debug("%s: Could not verify key '%s', trying all\n", __func__,
+		      name);
 
 		/* No luck, so try each of the keys in turn */
 		for (ndepth = 0, noffset = fdt_next_node(blob, sig_node,
@@ -546,6 +552,7 @@ int rsa_verify_hash(struct image_sign_info *info,
 			}
 		}
 	}
+	debug("%s: Failed to verify by any means\n", __func__);
 
 	return ret;
 }
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 02/16] fit_check_sign: Update help to mention the key is in a dtb

[Simon Glass]

The key is inside a dtb file, so tweak the help to make that clear.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/fit_check_sign.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/fit_check_sign.c b/tools/fit_check_sign.c
index 5573842d251..3d1d33fdab1 100644
--- a/tools/fit_check_sign.c
+++ b/tools/fit_check_sign.c
@@ -27,7 +27,7 @@ void usage(char *cmdname)
 {
 	fprintf(stderr, "Usage: %s -f fit file -k key file -c config name\n"
 			 "          -f ==> set fit file which should be checked'\n"
-			 "          -k ==> set key file which contains the key'\n"
+			 "          -k ==> set key .dtb file which contains the key'\n"
 			 "          -c ==> set the configuration name'\n",
 		cmdname);
 	exit(EXIT_FAILURE);
@@ -89,7 +89,7 @@ int main(int argc, char **argv)
 		fprintf(stderr, "Signature check OK\n");
 	} else {
 		ret = EXIT_FAILURE;
-		fprintf(stderr, "Signature check Bad (error %d)\n", ret);
+		fprintf(stderr, "Signature check bad (error %d)\n", ret);
 	}
 
 	(void) munmap((void *)fit_blob, fsbuf.st_size);
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 03/16] tools: Move copyfile() into a common file

[Simon Glass]

This function is useful in other places. Move it to a common file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/fit_common.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/fit_common.h | 11 +++++++++
 tools/fit_image.c  | 56 ----------------------------------------------
 3 files changed, 67 insertions(+), 56 deletions(-)

diff --git a/tools/fit_common.c b/tools/fit_common.c
index 52b63296f89..c161f23b0f7 100644
--- a/tools/fit_common.c
+++ b/tools/fit_common.c
@@ -111,3 +111,59 @@ err:
 
 	return -1;
 }
+
+int copyfile(const char *src, const char *dst)
+{
+	int fd_src = -1, fd_dst = -1;
+	void *buf = NULL;
+	ssize_t size;
+	size_t count;
+	int ret = -1;
+
+	fd_src = open(src, O_RDONLY);
+	if (fd_src < 0) {
+		printf("Can't open file %s (%s)\n", src, strerror(errno));
+		goto out;
+	}
+
+	fd_dst = open(dst, O_WRONLY | O_CREAT, 0666);
+	if (fd_dst < 0) {
+		printf("Can't open file %s (%s)\n", dst, strerror(errno));
+		goto out;
+	}
+
+	buf = calloc(1, 512);
+	if (!buf) {
+		printf("Can't allocate buffer to copy file\n");
+		goto out;
+	}
+
+	while (1) {
+		size = read(fd_src, buf, 512);
+		if (size < 0) {
+			printf("Can't read file %s\n", src);
+			goto out;
+		}
+		if (!size)
+			break;
+
+		count = size;
+		size = write(fd_dst, buf, count);
+		if (size < 0) {
+			printf("Can't write file %s\n", dst);
+			goto out;
+		}
+	}
+
+	ret = 0;
+
+ out:
+	if (fd_src >= 0)
+		close(fd_src);
+	if (fd_dst >= 0)
+		close(fd_dst);
+	if (buf)
+		free(buf);
+
+	return ret;
+}
diff --git a/tools/fit_common.h b/tools/fit_common.h
index 1e81d4c68b6..a76c4001c59 100644
--- a/tools/fit_common.h
+++ b/tools/fit_common.h
@@ -39,4 +39,15 @@ int mmap_fdt(const char *cmdname, const char *fname, size_t size_inc,
 	     void **blobp, struct stat *sbuf, bool delete_on_error,
 	     bool read_only);
 
+/**
+ * copyfile() - Copy a file
+ *
+ * This uses read()/write() to copy file @src to file @dst
+ *
+ * @src: Filename to read from
+ * @dst: Filename to write to
+ * @return 0 if OK, -1 on error
+ */
+int copyfile(const char *src, const char *dst);
+
 #endif /* _FIT_COMMON_H_ */
diff --git a/tools/fit_image.c b/tools/fit_image.c
index f4f372ba62f..c4f56bb6967 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -654,62 +654,6 @@ err:
 	return ret;
 }
 
-static int copyfile(const char *src, const char *dst)
-{
-	int fd_src = -1, fd_dst = -1;
-	void *buf = NULL;
-	ssize_t size;
-	size_t count;
-	int ret = -1;
-
-	fd_src = open(src, O_RDONLY);
-	if (fd_src < 0) {
-		printf("Can't open file %s (%s)\n", src, strerror(errno));
-		goto out;
-	}
-
-	fd_dst = open(dst, O_WRONLY | O_CREAT, 0666);
-	if (fd_dst < 0) {
-		printf("Can't open file %s (%s)\n", dst, strerror(errno));
-		goto out;
-	}
-
-	buf = calloc(1, 512);
-	if (!buf) {
-		printf("Can't allocate buffer to copy file\n");
-		goto out;
-	}
-
-	while (1) {
-		size = read(fd_src, buf, 512);
-		if (size < 0) {
-			printf("Can't read file %s\n", src);
-			goto out;
-		}
-		if (!size)
-			break;
-
-		count = size;
-		size = write(fd_dst, buf, count);
-		if (size < 0) {
-			printf("Can't write file %s\n", dst);
-			goto out;
-		}
-	}
-
-	ret = 0;
-
- out:
-	if (fd_src >= 0)
-		close(fd_src);
-	if (fd_dst >= 0)
-		close(fd_dst);
-	if (buf)
-		free(buf);
-
-	return ret;
-}
-
 /**
  * fit_handle_file - main FIT file processing function
  *
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 04/16] tools: Avoid leaving extra data at the end of copied files

[Simon Glass]

The copyfile() implementation has strange behaviour if the destination
file already exists. Update it to ensure that any existing data in the
destination file is dropped.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/fit_common.c | 2 +-
 tools/fit_common.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/fit_common.c b/tools/fit_common.c
index c161f23b0f7..d13e5ebf1ae 100644
--- a/tools/fit_common.c
+++ b/tools/fit_common.c
@@ -126,7 +126,7 @@ int copyfile(const char *src, const char *dst)
 		goto out;
 	}
 
-	fd_dst = open(dst, O_WRONLY | O_CREAT, 0666);
+	fd_dst = open(dst, O_WRONLY | O_CREAT | O_TRUNC, 0666);
 	if (fd_dst < 0) {
 		printf("Can't open file %s (%s)\n", dst, strerror(errno));
 		goto out;
diff --git a/tools/fit_common.h b/tools/fit_common.h
index a76c4001c59..55f3f6acd4e 100644
--- a/tools/fit_common.h
+++ b/tools/fit_common.h
@@ -44,6 +44,8 @@ int mmap_fdt(const char *cmdname, const char *fname, size_t size_inc,
  *
  * This uses read()/write() to copy file @src to file @dst
  *
+ * If @dst exists, it is overwritten and truncated to the correct size.
+ *
  * @src: Filename to read from
  * @dst: Filename to write to
  * @return 0 if OK, -1 on error
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 05/16] tools: Improve comments in signing functions

[Simon Glass]

Add some more comments to explain what is going on in the signing
functions. Fix two repeated typos.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/image.h    |  2 +-
 tools/fdt_host.h   |  8 ++++
 tools/image-host.c | 98 +++++++++++++++++++++++++++++++++++-----------
 3 files changed, 85 insertions(+), 23 deletions(-)

diff --git a/include/image.h b/include/image.h
index fd662e74b41..533c23e2002 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1025,7 +1025,7 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
  * fit_add_verification_data() - add verification data to FIT image nodes
  *
  * @keydir:	Directory containing keys
- * @kwydest:	FDT blob to write public key information to
+ * @kwydest:	FDT blob to write public key information to (NULL if none)
  * @fit:	Pointer to the FIT format image header
  * @comment:	Comment to add to signature nodes
  * @require_keys: Mark all keys as 'required'
diff --git a/tools/fdt_host.h b/tools/fdt_host.h
index 15c07c7a96e..bc42306c9e5 100644
--- a/tools/fdt_host.h
+++ b/tools/fdt_host.h
@@ -27,6 +27,14 @@
  */
 int fdt_remove_unused_strings(const void *old, void *new);
 
+/**
+ * fit_check_sign() - Check a signature in a FIT
+ *
+ * @fit: FIT to check
+ * @key: Key FDT blob to check against
+ * @fit_uname_config: Name of configuration to check (NULL for default)
+ * @return 0 if OK, -ve if signature failed
+ */
 int fit_check_sign(const void *fit, const void *key,
 		   const char *fit_uname_config);
 
diff --git a/tools/image-host.c b/tools/image-host.c
index a6b0a944205..73d35f3eee3 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -48,10 +48,10 @@ static int fit_set_hash_value(void *fit, int noffset, uint8_t *value,
  * fit_image_process_hash - Process a single subnode of the images/ node
  *
  * Check each subnode and process accordingly. For hash nodes we generate
- * a hash of the supplised data and store it in the node.
+ * a hash of the supplied data and store it in the node.
  *
  * @fit:	pointer to the FIT format image header
- * @image_name:	name of image being processes (used to display errors)
+ * @image_name:	name of image being processed (used to display errors)
  * @noffset:	subnode offset
  * @data:	data to process
  * @size:	size of data in bytes
@@ -197,12 +197,12 @@ static int fit_image_setup_sig(struct image_sign_info *info,
  * fit_image_process_sig- Process a single subnode of the images/ node
  *
  * Check each subnode and process accordingly. For signature nodes we
- * generate a signed hash of the supplised data and store it in the node.
+ * generate a signed hash of the supplied data and store it in the node.
  *
  * @keydir:	Directory containing keys to use for signing
- * @keydest:	Destination FDT blob to write public keys into
+ * @keydest:	Destination FDT blob to write public keys into (NULL if none)
  * @fit:	pointer to the FIT format image header
- * @image_name:	name of image being processes (used to display errors)
+ * @image_name:	name of image being processed (used to display errors)
  * @noffset:	subnode offset
  * @data:	data to process
  * @size:	size of data in bytes
@@ -685,14 +685,14 @@ static int strlist_add(struct strlist *list, const char *str)
 	return 0;
 }
 
-static const char *fit_config_get_image_list(void *fit, int noffset,
-		int *lenp, int *allow_missingp)
+static const char *fit_config_get_image_list(const void *fit, int noffset,
+					     int *lenp, int *allow_missingp)
 {
 	static const char default_list[] = FIT_KERNEL_PROP "\0"
 			FIT_FDT_PROP;
 	const char *prop;
 
-	/* If there is an "image" property, use that */
+	/* If there is an "sign-image" property, use that */
 	prop = fdt_getprop(fit, noffset, "sign-images", lenp);
 	if (prop) {
 		*allow_missingp = 0;
@@ -706,8 +706,24 @@ static const char *fit_config_get_image_list(void *fit, int noffset,
 	return default_list;
 }
 
-static int fit_config_add_hash(void *fit, const char *conf_name, const char *sig_name,
-			       struct strlist *node_inc, const char *iname, int image_noffset)
+/**
+ * fit_config_add_hash() - Add a list of nodes to hash for an image
+ *
+ * This adds a list of paths to image nodes (as referred to by a particular
+ * offset) that need to be hashed, to protect a configuration
+ *
+ * @fit:	Pointer to the FIT format image header
+ * @image_noffset: Offset of image to process (e.g. /images/kernel-1)
+ * @node_inc:	List of nodes to add to
+ * @conf_name	Configuration-node name, child of /configurations node (only
+ *	used for error messages)
+ * @sig_name	Signature-node name (only used for error messages)
+ * @iname:	Name of image being processed (e.g. "kernel-1" (only used
+ *	for error messages)
+ */
+static int fit_config_add_hash(const void *fit, int image_noffset,
+			       struct strlist *node_inc, const char *conf_name,
+			       const char *sig_name, const char *iname)
 {
 	char name[200], path[200];
 	int noffset;
@@ -777,7 +793,21 @@ err_path:
 	return -ENOENT;
 }
 
-static int fit_config_get_hash_list(void *fit, int conf_noffset,
+/**
+ * fit_config_get_hash_list() - Get the regions to sign
+ *
+ * This calculates a list of nodes to hash for this particular configuration,
+ * returning it as a string list (struct strlist, not a devicetree string list)
+ *
+ * @fit:	Pointer to the FIT format image header
+ * @conf_noffset: Offset of configuration node to sign (child of
+ *	/configurations node)
+ * @sig_offset:	Offset of signature node containing info about how to sign it
+ *	(child of 'signatures' node)
+ * @return 0 if OK, -ENOENT if an image referred to by the configuration cannot
+ *	be found, -ENOMSG if ther were no images in the configuration
+ */
+static int fit_config_get_hash_list(const void *fit, int conf_noffset,
 				    int sig_offset, struct strlist *node_inc)
 {
 	int allow_missing;
@@ -828,9 +858,8 @@ static int fit_config_get_hash_list(void *fit, int conf_noffset,
 				return -ENOENT;
 			}
 
-			ret = fit_config_add_hash(fit, conf_name,
-						  sig_name, node_inc,
-						  iname, image_noffset);
+			ret = fit_config_add_hash(fit, image_noffset, node_inc,
+						  conf_name, sig_name, iname);
 			if (ret < 0)
 				return ret;
 
@@ -852,9 +881,32 @@ err_mem:
 	return -ENOMEM;
 }
 
-static int fit_config_get_data(void *fit, int conf_noffset, int noffset,
-		struct image_region **regionp, int *region_countp,
-		char **region_propp, int *region_proplen)
+/**
+ * fit_config_get_regions() - Get the regions to sign
+ *
+ * This calculates a list of node to hash for this particular configuration,
+ * then finds which regions of the devicetree they correspond to.
+ *
+ * @fit:	Pointer to the FIT format image header
+ * @conf_noffset: Offset of configuration node to sign (child of
+ *	/configurations node)
+ * @sig_offset:	Offset of signature node containing info about how to sign it
+ *	(child of 'signatures' node)
+ * @regionp: Returns list of regions that need to be hashed (allocated; must be
+ *	freed by the caller)
+ * @region_count: Returns number of regions
+ * @region_propp: Returns string-list property containing the list of nodes
+ *	that correspond to the regions. Each entry is a full path to the node.
+ *	This is in devicetree format, i.e. a \0 between each string. This is
+ *	allocated and must be freed by the caller.
+ * @region_proplen: Returns length of *@@region_propp in bytes
+ * @return 0 if OK, -ENOMEM if out of memory, -EIO if the regions to hash could
+ * not be found, -EINVAL if no registers were found to hash
+ */
+static int fit_config_get_regions(const void *fit, int conf_noffset,
+				  int sig_offset, struct image_region **regionp,
+				  int *region_countp, char **region_propp,
+				  int *region_proplen)
 {
 	char * const exc_prop[] = {"data"};
 	struct strlist node_inc;
@@ -867,11 +919,12 @@ static int fit_config_get_data(void *fit, int conf_noffset, int noffset,
 	int ret, len;
 
 	conf_name = fit_get_name(fit, conf_noffset, NULL);
-	sig_name = fit_get_name(fit, noffset, NULL);
+	sig_name = fit_get_name(fit, sig_offset, NULL);
 	debug("%s: conf='%s', sig='%s'\n", __func__, conf_name, sig_name);
 
 	/* Get a list of nodes we want to hash */
-	ret = fit_config_get_hash_list(fit, conf_noffset, noffset, &node_inc);
+	ret = fit_config_get_hash_list(fit, conf_noffset, sig_offset,
+				       &node_inc);
 	if (ret)
 		return ret;
 
@@ -925,7 +978,7 @@ static int fit_config_get_data(void *fit, int conf_noffset, int noffset,
 }
 
 static int fit_config_process_sig(const char *keydir, const char *keyfile,
-		void *keydest,	void *fit, const char *conf_name,
+		void *keydest, void *fit, const char *conf_name,
 		int conf_noffset, int noffset, const char *comment,
 		int require_keys, const char *engine_id, const char *cmdname)
 {
@@ -940,8 +993,9 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile,
 	int ret;
 
 	node_name = fit_get_name(fit, noffset, NULL);
-	if (fit_config_get_data(fit, conf_noffset, noffset, &region,
-				&region_count, &region_prop, &region_proplen))
+	if (fit_config_get_regions(fit, conf_noffset, noffset, &region,
+				   &region_count, &region_prop,
+				   &region_proplen))
 		return -1;
 
 	if (fit_image_setup_sig(&info, keydir, keyfile, fit, conf_name, noffset,
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 06/16] tools: Drop unused name in image-host

[Simon Glass]

The name is created but never used. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/image-host.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/image-host.c b/tools/image-host.c
index 73d35f3eee3..e4322d323a2 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -725,7 +725,7 @@ static int fit_config_add_hash(const void *fit, int image_noffset,
 			       struct strlist *node_inc, const char *conf_name,
 			       const char *sig_name, const char *iname)
 {
-	char name[200], path[200];
+	char path[200];
 	int noffset;
 	int hash_count;
 	int ret;
@@ -736,9 +736,6 @@ static int fit_config_add_hash(const void *fit, int image_noffset,
 	if (strlist_add(node_inc, path))
 		goto err_mem;
 
-	snprintf(name, sizeof(name), "%s/%s", FIT_CONFS_PATH,
-		 conf_name);
-
 	/* Add all this image's hashes */
 	hash_count = 0;
 	for (noffset = fdt_first_subnode(fit, image_noffset);
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 07/16] tools: Avoid confusion between keys and signatures

[Simon Glass]

We should be consistent in using the term 'signature' to describe a value
added to sign something and 'key' to describe the key that can be used to
verify the signature.

Tidy up the code to stick to this.

Add some comments to fit_config_verify_key() and its callers while we are
here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/image-fit-sig.c | 97 ++++++++++++++++++++++++++++++++------------
 1 file changed, 72 insertions(+), 25 deletions(-)

diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 63e5423c925..f0ccd9b5227 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -135,7 +135,7 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data,
 
 static int fit_image_verify_sig(const void *fit, int image_noffset,
 				const char *data, size_t size,
-				const void *sig_blob, int sig_offset)
+				const void *key_blob, int key_offset)
 {
 	int noffset;
 	char *err_msg = "";
@@ -184,34 +184,34 @@ error:
 
 int fit_image_verify_required_sigs(const void *fit, int image_noffset,
 				   const char *data, size_t size,
-				   const void *sig_blob, int *no_sigsp)
+				   const void *key_blob, int *no_sigsp)
 {
 	int verify_count = 0;
 	int noffset;
-	int sig_node;
+	int key_node;
 
 	/* Work out what we need to verify */
 	*no_sigsp = 1;
-	sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
-	if (sig_node < 0) {
+	key_node = fdt_subnode_offset(key_blob, 0, FIT_SIG_NODENAME);
+	if (key_node < 0) {
 		debug("%s: No signature node found: %s\n", __func__,
-		      fdt_strerror(sig_node));
+		      fdt_strerror(key_node));
 		return 0;
 	}
 
-	fdt_for_each_subnode(noffset, sig_blob, sig_node) {
+	fdt_for_each_subnode(noffset, key_blob, key_node) {
 		const char *required;
 		int ret;
 
-		required = fdt_getprop(sig_blob, noffset, FIT_KEY_REQUIRED,
+		required = fdt_getprop(key_blob, noffset, FIT_KEY_REQUIRED,
 				       NULL);
 		if (!required || strcmp(required, "image"))
 			continue;
 		ret = fit_image_verify_sig(fit, image_noffset, data, size,
-					   sig_blob, noffset);
+					   key_blob, noffset);
 		if (ret) {
 			printf("Failed to verify required signature '%s'\n",
-			       fit_get_name(sig_blob, noffset, NULL));
+			       fit_get_name(key_blob, noffset, NULL));
 			return ret;
 		}
 		verify_count++;
@@ -368,8 +368,35 @@ static int fit_config_check_sig(const void *fit, int noffset,
 	return 0;
 }
 
-static int fit_config_verify_sig(const void *fit, int conf_noffset,
-				 const void *sig_blob, int sig_offset)
+/**
+ * fit_config_verify_key() - Verify that a configuration is signed with a key
+ *
+ * Here we are looking at a particular configuration that needs verification:
+ *
+ *	configurations {
+ *		default = "conf-1";
+ *		conf-1 {
+ *			kernel = "kernel-1";
+ *			fdt = "fdt-1";
+ *			signature-1 {
+ *				algo = "sha1,rsa2048";
+ *				value = <...conf 1 signature...>;
+ *			};
+ *		};
+ *
+ * We must check each of the signature subnodes of conf-1. Hopefully one of them
+ * will match the key at key_offset.
+ *
+ * @fit: FIT to check
+ * @conf_noffset: Offset of the configuration node to check (e.g.
+ *	/configurations/conf-1)
+ * @key_blob: Blob containing the keys to check against
+ * @key_offset: Offset of the key to check within @key_blob
+ * @return 0 if OK, -EPERM if any signatures did not verify, or the
+ *	configuration node has an invalid name
+ */
+static int fit_config_verify_key(const void *fit, int conf_noffset,
+				 const void *key_blob, int key_offset)
 {
 	int noffset;
 	char *err_msg = "No 'signature' subnode found";
@@ -382,7 +409,7 @@ static int fit_config_verify_sig(const void *fit, int conf_noffset,
 
 		if (!strncmp(name, FIT_SIG_NODENAME,
 			     strlen(FIT_SIG_NODENAME))) {
-			ret = fit_config_check_sig(fit, noffset, sig_offset,
+			ret = fit_config_check_sig(fit, noffset, key_offset,
 						   conf_noffset, &err_msg);
 			if (ret) {
 				puts("- ");
@@ -409,12 +436,25 @@ error:
 	return -EPERM;
 }
 
-static int fit_config_verify_required_sigs(const void *fit, int conf_noffset,
-					   const void *sig_blob)
+/**
+ * fit_config_verify_required_keys() - verify any required signatures for config
+ *
+ * This looks through all the signatures we expect and verifies that at least
+ * all the required ones are valid signatures for the configuration
+ *
+ * @fit: FIT to check
+ * @conf_noffset: Offset of the configuration node to check (e.g.
+ *	/configurations/conf-1)
+ * @key_blob: Blob containing the keys to check against
+ * @return 0 if OK, -EPERM if any signatures did not verify, or the
+ *	configuration node has an invalid name
+ */
+static int fit_config_verify_required_keys(const void *fit, int conf_noffset,
+					   const void *key_blob)
 {
 	const char *name = fit_get_name(fit, conf_noffset, NULL);
 	int noffset;
-	int sig_node;
+	int key_node;
 	int verified = 0;
 	int reqd_sigs = 0;
 	bool reqd_policy_all = true;
@@ -430,38 +470,45 @@ static int fit_config_verify_required_sigs(const void *fit, int conf_noffset,
 	}
 
 	/* Work out what we need to verify */
-	sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
-	if (sig_node < 0) {
+	key_node = fdt_subnode_offset(key_blob, 0, FIT_SIG_NODENAME);
+	if (key_node < 0) {
 		debug("%s: No signature node found: %s\n", __func__,
-		      fdt_strerror(sig_node));
+		      fdt_strerror(key_node));
 		return 0;
 	}
 
 	/* Get required-mode policy property from DTB */
-	reqd_mode = fdt_getprop(sig_blob, sig_node, "required-mode", NULL);
+	reqd_mode = fdt_getprop(key_blob, key_node, "required-mode", NULL);
 	if (reqd_mode && !strcmp(reqd_mode, "any"))
 		reqd_policy_all = false;
 
 	debug("%s: required-mode policy set to '%s'\n", __func__,
 	      reqd_policy_all ? "all" : "any");
 
-	fdt_for_each_subnode(noffset, sig_blob, sig_node) {
+	/*
+	 * The algorithm here is a little convoluted due to how we want it to
+	 * work. Here we work through each of the signature nodes in the
+	 * public-key area. These are in the U-Boot control devicetree. Each
+	 * node was created by signing a configuration, so we check if it is
+	 * 'required' and if so, request that it be verified.
+	 */
+	fdt_for_each_subnode(noffset, key_blob, key_node) {
 		const char *required;
 		int ret;
 
-		required = fdt_getprop(sig_blob, noffset, FIT_KEY_REQUIRED,
+		required = fdt_getprop(key_blob, noffset, FIT_KEY_REQUIRED,
 				       NULL);
 		if (!required || strcmp(required, "conf"))
 			continue;
 
 		reqd_sigs++;
 
-		ret = fit_config_verify_sig(fit, conf_noffset, sig_blob,
+		ret = fit_config_verify_key(fit, conf_noffset, key_blob,
 					    noffset);
 		if (ret) {
 			if (reqd_policy_all) {
 				printf("Failed to verify required signature '%s'\n",
-				       fit_get_name(sig_blob, noffset, NULL));
+				       fit_get_name(key_blob, noffset, NULL));
 				return ret;
 			}
 		} else {
@@ -481,6 +528,6 @@ static int fit_config_verify_required_sigs(const void *fit, int conf_noffset,
 
 int fit_config_verify(const void *fit, int conf_noffset)
 {
-	return fit_config_verify_required_sigs(fit, conf_noffset,
+	return fit_config_verify_required_keys(fit, conf_noffset,
 					       gd_fdt_blob());
 }
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 08/16] tools: Tidy up argument order in fit_config_check_sig()

[Simon Glass]

Put the parent node first in the parameters as this is more natural. Also
add a comment to explain what is going on.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/image-fit-sig.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index f0ccd9b5227..253748ef649 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -226,21 +226,34 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset,
 /**
  * fit_config_check_sig() - Check the signature of a config
  *
+ * Here we are looking at a particular signature that needs verification (here
+ * signature-1):
+ *
+ *	configurations {
+ *		default = "conf-1";
+ *		conf-1 {
+ *			kernel = "kernel-1";
+ *			fdt = "fdt-1";
+ *			signature-1 {
+ *				algo = "sha1,rsa2048";
+ *				value = <...conf 1 signature...>;
+ *			};
+ *		};
+ *
  * @fit: FIT to check
- * @noffset: Offset of configuration node (e.g. /configurations/conf-1)
- * @required_keynode:	Offset in the control FDT of the required key node,
+ * @noffset: Offset of the signature node being checked (e.g.
+ *	 /configurations/conf-1/signature-1)
+ * @conf_noffset: Offset of configuration node (e.g. /configurations/conf-1)
+ * @required_keynode:	Offset in @key_blob of the required key node,
  *			if any. If this is given, then the configuration wil not
  *			pass verification unless that key is used. If this is
  *			-1 then any signature will do.
- * @conf_noffset: Offset of the configuration subnode being checked (e.g.
- *	 /configurations/conf-1/kernel)
  * @err_msgp:		In the event of an error, this will be pointed to a
  *			help error string to display to the user.
  * @return 0 if all verified ok, <0 on error
  */
-static int fit_config_check_sig(const void *fit, int noffset,
-				int required_keynode, int conf_noffset,
-				char **err_msgp)
+static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
+				int required_keynode, char **err_msgp)
 {
 	static char * const exc_prop[] = {
 		"data",
@@ -409,8 +422,8 @@ static int fit_config_verify_key(const void *fit, int conf_noffset,
 
 		if (!strncmp(name, FIT_SIG_NODENAME,
 			     strlen(FIT_SIG_NODENAME))) {
-			ret = fit_config_check_sig(fit, noffset, key_offset,
-						   conf_noffset, &err_msg);
+			ret = fit_config_check_sig(fit, noffset, conf_noffset,
+						   key_offset, &err_msg);
 			if (ret) {
 				puts("- ");
 			} else {
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 09/16] tools: Pass the key blob around

[Simon Glass]

At present we rely on the key blob being in the global_data fdt_blob
pointer. This is true in U-Boot but not with tools. For clarity, pass the
parameter around.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/image-fit-sig.c | 31 ++++++++++++++++++-------------
 boot/image-fit.c     | 12 +++++++-----
 common/spl/spl_fit.c |  3 ++-
 include/image.h      | 23 ++++++++++++++++++-----
 4 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 253748ef649..a751c12d174 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -65,7 +65,8 @@ struct image_region *fit_region_make_list(const void *fit,
 
 static int fit_image_setup_verify(struct image_sign_info *info,
 				  const void *fit, int noffset,
-				  int required_keynode, char **err_msgp)
+				  const void *key_blob, int required_keynode,
+				  char **err_msgp)
 {
 	char *algo_name;
 	const char *padding_name;
@@ -91,7 +92,7 @@ static int fit_image_setup_verify(struct image_sign_info *info,
 	info->checksum = image_get_checksum_algo(algo_name);
 	info->crypto = image_get_crypto_algo(algo_name);
 	info->padding = image_get_padding_algo(padding_name);
-	info->fdt_blob = gd_fdt_blob();
+	info->fdt_blob = key_blob;
 	info->required_keynode = required_keynode;
 	printf("%s:%s", algo_name, info->keyname);
 
@@ -104,7 +105,8 @@ static int fit_image_setup_verify(struct image_sign_info *info,
 }
 
 int fit_image_check_sig(const void *fit, int noffset, const void *data,
-			size_t size, int required_keynode, char **err_msgp)
+			size_t size, const void *key_blob, int required_keynode,
+			char **err_msgp)
 {
 	struct image_sign_info info;
 	struct image_region region;
@@ -112,8 +114,8 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data,
 	int fit_value_len;
 
 	*err_msgp = NULL;
-	if (fit_image_setup_verify(&info, fit, noffset, required_keynode,
-				   err_msgp))
+	if (fit_image_setup_verify(&info, fit, noffset, key_blob,
+				   required_keynode, err_msgp))
 		return -1;
 
 	if (fit_image_hash_get_value(fit, noffset, &fit_value,
@@ -156,8 +158,8 @@ static int fit_image_verify_sig(const void *fit, int image_noffset,
 		}
 		if (!strncmp(name, FIT_SIG_NODENAME,
 			     strlen(FIT_SIG_NODENAME))) {
-			ret = fit_image_check_sig(fit, noffset, data,
-						  size, -1, &err_msg);
+			ret = fit_image_check_sig(fit, noffset, data, size,
+						  key_blob, -1, &err_msg);
 			if (ret) {
 				puts("- ");
 			} else {
@@ -244,6 +246,7 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset,
  * @noffset: Offset of the signature node being checked (e.g.
  *	 /configurations/conf-1/signature-1)
  * @conf_noffset: Offset of configuration node (e.g. /configurations/conf-1)
+ * @key_blob: Blob containing the keys to check against
  * @required_keynode:	Offset in @key_blob of the required key node,
  *			if any. If this is given, then the configuration wil not
  *			pass verification unless that key is used. If this is
@@ -253,7 +256,8 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset,
  * @return 0 if all verified ok, <0 on error
  */
 static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
-				int required_keynode, char **err_msgp)
+				const void *key_blob, int required_keynode,
+				char **err_msgp)
 {
 	static char * const exc_prop[] = {
 		"data",
@@ -275,12 +279,12 @@ static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
 	int count;
 
 	config_name = fit_get_name(fit, conf_noffset, NULL);
-	debug("%s: fdt=%p, conf='%s', sig='%s'\n", __func__, gd_fdt_blob(),
+	debug("%s: fdt=%p, conf='%s', sig='%s'\n", __func__, key_blob,
 	      fit_get_name(fit, noffset, NULL),
-	      fit_get_name(gd_fdt_blob(), required_keynode, NULL));
+	      fit_get_name(key_blob, required_keynode, NULL));
 	*err_msgp = NULL;
-	if (fit_image_setup_verify(&info, fit, noffset, required_keynode,
-				   err_msgp))
+	if (fit_image_setup_verify(&info, fit, noffset, key_blob,
+				   required_keynode, err_msgp))
 		return -1;
 
 	if (fit_image_hash_get_value(fit, noffset, &fit_value,
@@ -423,7 +427,8 @@ static int fit_config_verify_key(const void *fit, int conf_noffset,
 		if (!strncmp(name, FIT_SIG_NODENAME,
 			     strlen(FIT_SIG_NODENAME))) {
 			ret = fit_config_check_sig(fit, noffset, conf_noffset,
-						   key_offset, &err_msg);
+						   key_blob, key_offset,
+						   &err_msg);
 			if (ret) {
 				puts("- ");
 			} else {
diff --git a/boot/image-fit.c b/boot/image-fit.c
index 33b4a46028b..59191a5486c 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1309,7 +1309,8 @@ static int fit_image_check_hash(const void *fit, int noffset, const void *data,
 }
 
 int fit_image_verify_with_data(const void *fit, int image_noffset,
-			       const void *data, size_t size)
+			       const void *key_blob, const void *data,
+			       size_t size)
 {
 	int		noffset = 0;
 	char		*err_msg = "";
@@ -1319,7 +1320,7 @@ int fit_image_verify_with_data(const void *fit, int image_noffset,
 	/* Verify all required signatures */
 	if (FIT_IMAGE_ENABLE_VERIFY &&
 	    fit_image_verify_required_sigs(fit, image_noffset, data, size,
-					   gd_fdt_blob(), &verify_all)) {
+					   key_blob, &verify_all)) {
 		err_msg = "Unable to verify required signature";
 		goto error;
 	}
@@ -1342,8 +1343,8 @@ int fit_image_verify_with_data(const void *fit, int image_noffset,
 		} else if (FIT_IMAGE_ENABLE_VERIFY && verify_all &&
 				!strncmp(name, FIT_SIG_NODENAME,
 					strlen(FIT_SIG_NODENAME))) {
-			ret = fit_image_check_sig(fit, noffset, data,
-							size, -1, &err_msg);
+			ret = fit_image_check_sig(fit, noffset, data, size,
+						  gd_fdt_blob(), -1, &err_msg);
 
 			/*
 			 * Show an indication on failure, but do not return
@@ -1406,7 +1407,8 @@ int fit_image_verify(const void *fit, int image_noffset)
 		goto err;
 	}
 
-	return fit_image_verify_with_data(fit, image_noffset, data, size);
+	return fit_image_verify_with_data(fit, image_noffset, gd_fdt_blob(),
+					  data, size);
 
 err:
 	printf("error!\n%s in '%s' image node\n", err_msg,
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 5fe0273d66d..55133ceea6f 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -314,7 +314,8 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
 	if (CONFIG_IS_ENABLED(FIT_SIGNATURE)) {
 		printf("## Checking hash(es) for Image %s ... ",
 		       fit_get_name(fit, node, NULL));
-		if (!fit_image_verify_with_data(fit, node, src, length))
+		if (!fit_image_verify_with_data(fit, node, gd_fdt_blob(), src,
+						length))
 			return -EPERM;
 		puts("OK\n");
 	}
diff --git a/include/image.h b/include/image.h
index 533c23e2002..d5598cec461 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1047,8 +1047,19 @@ int fit_add_verification_data(const char *keydir, const char *keyfile,
 			      int require_keys, const char *engine_id,
 			      const char *cmdname);
 
+/**
+ * fit_image_verify_with_data() - Verify an image with given data
+ *
+ * @fit:	Pointer to the FIT format image header
+ * @image_offset: Offset in @fit of image to verify
+ * @key_blob:	FDT containing public keys
+ * @data:	Image data to verify
+ * @size:	Size of image data
+ */
 int fit_image_verify_with_data(const void *fit, int image_noffset,
-			       const void *data, size_t size);
+			       const void *key_blob, const void *data,
+			       size_t size);
+
 int fit_image_verify(const void *fit, int noffset);
 int fit_config_verify(const void *fit, int conf_noffset);
 int fit_all_image_verify(const void *fit);
@@ -1296,7 +1307,7 @@ struct padding_algo *image_get_padding_algo(const char *name);
  * @image_noffset:	Offset of image node to check
  * @data:		Image data to check
  * @size:		Size of image data
- * @sig_blob:		FDT containing public keys
+ * @key_blob:		FDT containing public keys
  * @no_sigsp:		Returns 1 if no signatures were required, and
  *			therefore nothing was checked. The caller may wish
  *			to fall back to other mechanisms, or refuse to
@@ -1304,7 +1315,7 @@ struct padding_algo *image_get_padding_algo(const char *name);
  * @return 0 if all verified ok, <0 on error
  */
 int fit_image_verify_required_sigs(const void *fit, int image_noffset,
-		const char *data, size_t size, const void *sig_blob,
+		const char *data, size_t size, const void *key_blob,
 		int *no_sigsp);
 
 /**
@@ -1314,7 +1325,8 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset,
  * @noffset:		Offset of signature node to check
  * @data:		Image data to check
  * @size:		Size of image data
- * @required_keynode:	Offset in the control FDT of the required key node,
+ * @keyblob:		Key blob to check (typically the control FDT)
+ * @required_keynode:	Offset in the keyblob of the required key node,
  *			if any. If this is given, then the image wil not
  *			pass verification unless that key is used. If this is
  *			-1 then any signature will do.
@@ -1323,7 +1335,8 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset,
  * @return 0 if all verified ok, <0 on error
  */
 int fit_image_check_sig(const void *fit, int noffset, const void *data,
-		size_t size, int required_keynode, char **err_msgp);
+			size_t size, const void *key_blob, int required_keynode,
+			char **err_msgp);
 
 int fit_image_decrypt_data(const void *fit,
 			   int image_noffset, int cipher_noffset,
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 10/16] image: Return destination node for add_verify_data() method

[Simon Glass]

It is useful to know where the verification data was written. Update the
API to return this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/image.h             | 3 ++-
 include/u-boot/ecdsa.h      | 5 +++--
 include/u-boot/rsa.h        | 5 +++--
 lib/ecdsa/ecdsa-libcrypto.c | 4 ++--
 lib/rsa/rsa-sign.c          | 5 ++++-
 tools/image-host.c          | 5 ++---
 6 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/include/image.h b/include/image.h
index d5598cec461..0f5e037a192 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1242,7 +1242,8 @@ struct crypto_algo {
 	 *
 	 * @info:	Specifies key and FIT information
 	 * @keydest:	Destination FDT blob for public key data
-	 * @return: 0, on success, -ve on error
+	 * @return: node offset within the FDT blob where the data was written,
+	 *	or -ve on error
 	 */
 	int (*add_verify_data)(struct image_sign_info *info, void *keydest);
 
diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h
index f6951c7346d..f1fa176d916 100644
--- a/include/u-boot/ecdsa.h
+++ b/include/u-boot/ecdsa.h
@@ -44,8 +44,9 @@ int ecdsa_sign(struct image_sign_info *info, const struct image_region region[],
  *
  * @info:	Specifies key and FIT information
  * @keydest:	Destination FDT blob for public key data
- * @return: 0, on success, -ENOSPC if the keydest FDT blob ran out of space,
- * other -ve value on error
+ * @return: node offset within the FDT blob where the data was written on
+ *	success, -ENOSPC if the keydest FDT blob ran out of space, other -ve
+ *	value on other error
  */
 int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest);
 
diff --git a/include/u-boot/rsa.h b/include/u-boot/rsa.h
index 7556aa5b4b7..fafefeedd80 100644
--- a/include/u-boot/rsa.h
+++ b/include/u-boot/rsa.h
@@ -61,8 +61,9 @@ int rsa_sign(struct image_sign_info *info,
  *
  * @info:	Specifies key and FIT information
  * @keydest:	Destination FDT blob for public key data
- * @return: 0, on success, -ENOSPC if the keydest FDT blob ran out of space,
-		other -ve value on error
+ * @return: node offset within the FDT blob where the data was written on
+ *	success, -ENOSPC if the keydest FDT blob ran out of space, other -ve
+ *	value on other error
 */
 int rsa_add_verify_data(struct image_sign_info *info, void *keydest);
 
diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c
index 1757a145623..aa9a226d031 100644
--- a/lib/ecdsa/ecdsa-libcrypto.c
+++ b/lib/ecdsa/ecdsa-libcrypto.c
@@ -299,7 +299,7 @@ static int do_add(struct signer *ctx, void *fdt, const char *key_node_name)
 	if (ret < 0)
 		return ret;
 
-	return 0;
+	return key_node;
 }
 
 int ecdsa_add_verify_data(struct image_sign_info *info, void *fdt)
@@ -311,7 +311,7 @@ int ecdsa_add_verify_data(struct image_sign_info *info, void *fdt)
 	fdt_key_name = info->keyname ? info->keyname : "default-key";
 	ret = prepare_ctx(&ctx, info);
 	if (ret >= 0)
-		do_add(&ctx, fdt, fdt_key_name);
+		ret = do_add(&ctx, fdt, fdt_key_name);
 
 	free_ctx(&ctx);
 	return ret;
diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
index 0579e5294ee..e8b06c857f2 100644
--- a/lib/rsa/rsa-sign.c
+++ b/lib/rsa/rsa-sign.c
@@ -701,5 +701,8 @@ err_get_pub_key:
 	if (info->engine_id)
 		rsa_engine_remove(e);
 
-	return ret;
+	if (ret)
+		return ret;
+
+	return node;
 }
diff --git a/tools/image-host.c b/tools/image-host.c
index e4322d323a2..e53fe4bbbed 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -264,7 +264,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,
 	 */
 	if (keydest) {
 		ret = info.crypto->add_verify_data(&info, keydest);
-		if (ret) {
+		if (ret < 0) {
 			printf("Failed to add verification data for '%s' signature node in '%s' image node\n",
 			       node_name, image_name);
 			return ret;
@@ -1030,11 +1030,10 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile,
 	/* Write the public key into the supplied FDT file */
 	if (keydest) {
 		ret = info.crypto->add_verify_data(&info, keydest);
-		if (ret) {
+		if (ret < 0) {
 			printf("Failed to add verification data for '%s' signature node in '%s' configuration node\n",
 			       node_name, conf_name);
 		}
-		return ret;
 	}
 
 	return 0;
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 11/16] tools: Pass public-key node through to caller

[Simon Glass]

Update the two functions that call add_verify_data() so that the caller
can see the node that was written to.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/image-host.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/image-host.c b/tools/image-host.c
index e53fe4bbbed..e2b120ce532 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -209,7 +209,8 @@ static int fit_image_setup_sig(struct image_sign_info *info,
  * @comment:	Comment to add to signature nodes
  * @require_keys: Mark all keys as 'required'
  * @engine_id:	Engine to use for signing
- * @return 0 if ok, -1 on error
+ * @return keydest node if @keydest is non-NULL, else 0 if none; -ve error code
+ *	on failure
  */
 static int fit_image_process_sig(const char *keydir, const char *keyfile,
 		void *keydest, void *fit, const char *image_name,
@@ -269,6 +270,8 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,
 			       node_name, image_name);
 			return ret;
 		}
+		/* Return the node that was written to */
+		return ret;
 	}
 
 	return 0;
@@ -645,7 +648,7 @@ int fit_image_add_verification_data(const char *keydir, const char *keyfile,
 				fit, image_name, noffset, data, size,
 				comment, require_keys, engine_id, cmdname);
 		}
-		if (ret)
+		if (ret < 0)
 			return ret;
 	}
 
@@ -974,6 +977,24 @@ static int fit_config_get_regions(const void *fit, int conf_noffset,
 	return 0;
 }
 
+/**
+ * fit_config_process_sig - Process a single subnode of the configurations/ node
+ *
+ * Generate a signed hash of the supplied data and store it in the node.
+ *
+ * @keydir:	Directory containing keys to use for signing
+ * @keydest:	Destination FDT blob to write public keys into (NULL if none)
+ * @fit:	pointer to the FIT format image header
+ * @conf_name	name of config being processed (used to display errors)
+ * @conf_noffset: Offset of configuration node, e.g. '/configurations/conf-1'
+ * @noffset:	subnode offset, e.g. '/configurations/conf-1/sig-1'
+ * @comment:	Comment to add to signature nodes
+ * @require_keys: Mark all keys as 'required'
+ * @engine_id:	Engine to use for signing
+ * @cmdname:	Command name used when reporting errors
+ * @return keydest node if @keydest is non-NULL, else 0 if none; -ve error code
+ *	on failure
+ */
 static int fit_config_process_sig(const char *keydir, const char *keyfile,
 		void *keydest, void *fit, const char *conf_name,
 		int conf_noffset, int noffset, const char *comment,
@@ -1034,6 +1055,7 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile,
 			printf("Failed to add verification data for '%s' signature node in '%s' configuration node\n",
 			       node_name, conf_name);
 		}
+		return ret;
 	}
 
 	return 0;
@@ -1063,7 +1085,7 @@ static int fit_config_add_verification_data(const char *keydir,
 				fit, conf_name, conf_noffset, noffset, comment,
 				require_keys, engine_id, cmdname);
 		}
-		if (ret)
+		if (ret < 0)
 			return ret;
 	}
 
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 12/16] tools: mkimage: Show where signatures/keys are written

[Simon Glass]

At present mkimage displays the node information but it is not clear what
signing action was taken. Add a message that shows it. For now it only
supports showing a single signing action, since that is the common case.

Sample:

   Signature written to 'sha1-basic/test.fit',
       node '/configurations/conf-1/signature'
   Public key written to 'sha1-basic/sandbox-u-boot.dtb',
       node '/signature/key-dev'

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/image.h    | 23 ++++++++++++++++++++++-
 tools/fit_common.c | 13 +++++++++++++
 tools/fit_common.h | 10 ++++++++++
 tools/fit_image.c  |  3 ++-
 tools/image-host.c | 23 ++++++++++++++++++-----
 tools/imagetool.h  |  3 +++
 tools/mkimage.c    |  4 ++++
 7 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/include/image.h b/include/image.h
index 0f5e037a192..733fa016694 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1021,6 +1021,25 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
 		    const char *comment, int require_keys,
 		    const char *engine_id, const char *cmdname);
 
+#define NODE_MAX_NAME_LEN	80
+
+/**
+ * struct image_summary  - Provides information about signing info added
+ *
+ * @sig_offset: Offset of the node in the blob devicetree where the signature
+ *	was wriiten
+ * @sig_path: Path to @sig_offset
+ * @keydest_offset: Offset of the node in the keydest devicetree where the
+ *	public key was written (-1 if none)
+ * @keydest_path: Path to @keydest_offset
+ */
+struct image_summary {
+	int sig_offset;
+	char sig_path[NODE_MAX_NAME_LEN];
+	int keydest_offset;
+	char keydest_path[NODE_MAX_NAME_LEN];
+};
+
 /**
  * fit_add_verification_data() - add verification data to FIT image nodes
  *
@@ -1031,6 +1050,7 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
  * @require_keys: Mark all keys as 'required'
  * @engine_id:	Engine to use for signing
  * @cmdname:	Command name used when reporting errors
+ * @summary:	Returns information about what data was written
  *
  * Adds hash values for all component images in the FIT blob.
  * Hashes are calculated for all component images which have hash subnodes
@@ -1045,7 +1065,8 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
 int fit_add_verification_data(const char *keydir, const char *keyfile,
 			      void *keydest, void *fit, const char *comment,
 			      int require_keys, const char *engine_id,
-			      const char *cmdname);
+			      const char *cmdname,
+			      struct image_summary *summary);
 
 /**
  * fit_image_verify_with_data() - Verify an image with given data
diff --git a/tools/fit_common.c b/tools/fit_common.c
index d13e5ebf1ae..defdc9d5688 100644
--- a/tools/fit_common.c
+++ b/tools/fit_common.c
@@ -167,3 +167,16 @@ int copyfile(const char *src, const char *dst)
 
 	return ret;
 }
+
+void summary_show(struct image_summary *summary, const char *imagefile,
+		  const char *keydest)
+{
+	if (summary->sig_offset) {
+		printf("Signature written to '%s', node '%s'\n", imagefile,
+		       summary->sig_path);
+		if (keydest) {
+			printf("Public key written to '%s', node '%s'\n",
+			       keydest, summary->keydest_path);
+		}
+	}
+}
diff --git a/tools/fit_common.h b/tools/fit_common.h
index 55f3f6acd4e..07fb718ae3a 100644
--- a/tools/fit_common.h
+++ b/tools/fit_common.h
@@ -52,4 +52,14 @@ int mmap_fdt(const char *cmdname, const char *fname, size_t size_inc,
  */
 int copyfile(const char *src, const char *dst);
 
+/**
+ * summary_show() - Show summary information about the signing process
+ *
+ * @summary: Summary info to show
+ * @imagefile: Filename of the output image
+ * @keydest: Filename where the key information is written (NULL if none)
+ */
+void summary_show(struct image_summary *summary, const char *imagefile,
+		  const char *keydest);
+
 #endif /* _FIT_COMMON_H_ */
diff --git a/tools/fit_image.c b/tools/fit_image.c
index c4f56bb6967..aff27d0ffcb 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -73,7 +73,8 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
 						params->comment,
 						params->require_keys,
 						params->engine_id,
-						params->cmdname);
+						params->cmdname,
+						&params->summary);
 	}
 
 	if (dest_blob) {
diff --git a/tools/image-host.c b/tools/image-host.c
index e2b120ce532..fb9aa7493cc 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -1064,7 +1064,7 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile,
 static int fit_config_add_verification_data(const char *keydir,
 		const char *keyfile, void *keydest, void *fit, int conf_noffset,
 		const char *comment, int require_keys, const char *engine_id,
-		const char *cmdname)
+		const char *cmdname, struct image_summary *summary)
 {
 	const char *conf_name;
 	int noffset;
@@ -1084,9 +1084,20 @@ static int fit_config_add_verification_data(const char *keydir,
 			ret = fit_config_process_sig(keydir, keyfile, keydest,
 				fit, conf_name, conf_noffset, noffset, comment,
 				require_keys, engine_id, cmdname);
+			if (ret < 0)
+				return ret;
+
+			summary->sig_offset = noffset;
+			fdt_get_path(fit, noffset, summary->sig_path,
+				     sizeof(summary->sig_path));
+
+			if (keydest) {
+				summary->keydest_offset = ret;
+				fdt_get_path(keydest, ret,
+					     summary->keydest_path,
+					     sizeof(summary->keydest_path));
+			}
 		}
-		if (ret < 0)
-			return ret;
 	}
 
 	return 0;
@@ -1130,7 +1141,8 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
 int fit_add_verification_data(const char *keydir, const char *keyfile,
 			      void *keydest, void *fit, const char *comment,
 			      int require_keys, const char *engine_id,
-			      const char *cmdname)
+			      const char *cmdname,
+			      struct image_summary *summary)
 {
 	int images_noffset, confs_noffset;
 	int noffset;
@@ -1178,7 +1190,8 @@ int fit_add_verification_data(const char *keydir, const char *keyfile,
 		ret = fit_config_add_verification_data(keydir, keyfile, keydest,
 						       fit, noffset, comment,
 						       require_keys,
-						       engine_id, cmdname);
+						       engine_id, cmdname,
+						       summary);
 		if (ret)
 			return ret;
 	}
diff --git a/tools/imagetool.h b/tools/imagetool.h
index e229a34ffc5..c0579c8c93c 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -21,6 +21,8 @@
 #include <unistd.h>
 #include <u-boot/sha1.h>
 
+#include <image.h>
+
 #include "fdt_host.h"
 
 #define ARRAY_SIZE(x)		(sizeof(x) / sizeof((x)[0]))
@@ -83,6 +85,7 @@ struct image_tool_params {
 	int bl_len;		/* Block length in byte for external data */
 	const char *engine_id;	/* Engine to use for signing */
 	bool reset_timestamp;	/* Reset the timestamp on an existing image */
+	struct image_summary summary;	/* results of signing process */
 };
 
 /*
diff --git a/tools/mkimage.c b/tools/mkimage.c
index fbe883ce362..566607c489e 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -10,6 +10,7 @@
 #include "imagetool.h"
 #include "mkimage.h"
 #include "imximage.h"
+#include <fit_common.h>
 #include <image.h>
 #include <version.h>
 #ifdef __linux__
@@ -469,6 +470,9 @@ int main(int argc, char **argv)
 
 		(void) munmap((void *)ptr, sbuf.st_size);
 		(void) close (ifd);
+		if (!retval)
+			summary_show(&params.summary, params.imagefile,
+				     params.keydest);
 
 		exit (retval);
 	}
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 13/16] tools: Add a new tool to sign FDT blobs

[Simon Glass]

This works similarly to FIT signing except that it signs the entire FDT
(with a few exclusions), rather than just FIT configurations or images.

Also it is a stand-alone tool rather than being part of mkimage.

The signature is added to the FDT so it can be checked later, to ensure
that no modifications were made to the FDT.

The /chosen node is excluded, since U-Boot may want to update it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/image.h   |  29 ++++
 tools/Makefile    |   8 +-
 tools/fdt-host.c  | 333 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/fdt_host.h  |  19 +++
 tools/fdt_sign.c  | 210 +++++++++++++++++++++++++++++
 tools/imagetool.h |   1 +
 6 files changed, 598 insertions(+), 2 deletions(-)
 create mode 100644 tools/fdt-host.c
 create mode 100644 tools/fdt_sign.c

diff --git a/include/image.h b/include/image.h
index 733fa016694..b2767883636 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1154,6 +1154,35 @@ int fit_check_ramdisk(const void *fit, int os_noffset,
 int calculate_hash(const void *data, int data_len, const char *algo,
 			uint8_t *value, int *value_len);
 
+/**
+ * fdt_add_verif_data() - add verification data to an FDT blob
+ *
+ * @keydir:	Directory containing keys
+ * @keyfile:	Filename containing .key file
+ * @kwydest:	FDT blob to write public key information to (NULL if none)
+ * @fit:	Pointer to the FIT format image header
+ * @key_name:	Name of key used to sign (used for node name and .crt file)
+ * @comment:	Comment to add to signature nodes
+ * @require_keys: Mark all keys as 'required'
+ * @engine_id:	Engine to use for signing
+ * @cmdname:	Command name used when reporting errors
+ * @summary:	Returns information about what data was written
+ *
+ * Adds hash values for all component images in the FIT blob.
+ * Hashes are calculated for all component images which have hash subnodes
+ * with algorithm property set to one of the supported hash algorithms.
+ *
+ * Also add signatures if signature nodes are present.
+ *
+ * returns
+ *     0, on success
+ *     libfdt error code, on failure
+ */
+int fdt_add_verif_data(const char *keydir, const char *keyfile, void *keydest,
+		       void *blob, const char *key_name, const char *comment,
+		       bool require_keys, const char *engine_id,
+		       const char *cmdname, struct image_summary *summary);
+
 /*
  * At present we only support signing on the host, and verification on the
  * device
diff --git a/tools/Makefile b/tools/Makefile
index 1763f44cac4..07eca631cb0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -72,12 +72,14 @@ hostprogs-y += mkenvimage
 mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
 hostprogs-y += dumpimage mkimage
-hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
+hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign \
+	fdt_sign
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
 FIT_OBJS-y := fit_common.o fit_image.o image-host.o boot/image-fit.o
-FIT_SIG_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := image-sig-host.o boot/image-fit-sig.o
+FIT_SIG_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := image-sig-host.o \
+	boot/image-fit-sig.o fdt-host.o
 FIT_CIPHER_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := boot/image-cipher.o
 
 # The following files are synced with upstream DTC.
@@ -154,6 +156,7 @@ dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
 mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
 fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
+fdt_sign-objs   := $(dumpimage-mkimage-objs) fdt_sign.o
 file2include-objs := file2include.o
 
 ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
@@ -191,6 +194,7 @@ HOSTCFLAGS_fit_image.o += -DMKIMAGE_DTC=\"$(CONFIG_MKIMAGE_DTC_PATH)\"
 HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_info := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_check_sign := $(HOSTLDLIBS_mkimage)
+HOSTLDLIBS_fdt_sign := $(HOSTLDLIBS_mkimage)
 
 hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
 hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
diff --git a/tools/fdt-host.c b/tools/fdt-host.c
new file mode 100644
index 00000000000..8aa0e099f2f
--- /dev/null
+++ b/tools/fdt-host.c
@@ -0,0 +1,333 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 Google, LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include "mkimage.h"
+#include <fdt_region.h>
+#include <image.h>
+#include <version.h>
+
+/**
+ * fdt_setup_sig() - Setup the signing info ready for use
+ *
+ * @info: Info to set up
+ * @keydir: Directory holding private keys
+ * @keyfile: Filename of private or public key (NULL to find it in @keydir)
+ * @keyname: Name to use for key node if added
+ * @blob: FDT blob to sign
+ * @algo_name: Algorithm name to use for signing, e.g. "sha256,rsa2048"
+ * @padding_name: Padding method to use for signing, NULL if none
+ * @require_keys: Mark signing keys as 'required' (else they are optional)
+ * @engine_id: Engine to use for signin, NULL for default
+ * @return 0 if OK, -ENOMEM if out of memory, -ENOSYS if unsupported algorithm
+ */
+static int fdt_setup_sig(struct image_sign_info *info, const char *keydir,
+			 const char *keyfile, const char *keyname, void *blob,
+			 const char *algo_name, const char *padding_name,
+			 const char *require_keys, const char *engine_id)
+{
+	memset(info, '\0', sizeof(*info));
+	info->keydir = keydir;
+	info->keyfile = keyfile;
+	info->keyname = keyname;
+	info->fit = blob;
+	info->name = strdup(algo_name);
+	if (!info->name) {
+		printf("Out of memory for algo_name\n");
+		return -ENOMEM;
+	}
+	info->checksum = image_get_checksum_algo(algo_name);
+	info->crypto = image_get_crypto_algo(algo_name);
+	info->padding = image_get_padding_algo(padding_name);
+	info->require_keys = require_keys;
+	info->engine_id = engine_id;
+	if (!info->checksum || !info->crypto) {
+		printf("Unsupported signature algorithm (%s)\n", algo_name);
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+/**
+ * h_exclude_nodes() - Handles excluding certain nodes from the FDT
+ *
+ * This is called by fdt_next_region() when it wants to find out if a node or
+ * property should be included in the hash.
+ *
+ * This function simply omits the /chosen node as well as /signature and any
+ * subnodes.
+ *
+ * @return 0 if the node should be excluded, -1 for everything else (meaning it
+ *	has no opinion)
+ */
+static int h_exclude_nodes(void *priv, const void *fdt, int offset, int type,
+			   const char *data, int size)
+{
+	if (type == FDT_IS_NODE) {
+		/* Ignore the chosen node as well as /signature and subnodes */
+		if (!strcmp("/chosen", data) ||
+		    !strncmp("/signature", data, 10))
+			return 0;
+	}
+
+	return -1;
+}
+
+/**
+ * run_find_regions() - Use the fdt region calculation to get a list of regions
+ *
+ * This finds the regions of the FDT that need to be hashed so that it can be
+ * protected against modification by a signature
+ *
+ * @fdt: FDT blob to check
+ * @include_func: Function to call to determine whether to include an element of
+ * the devicetree
+ * @priv: Private data, set to NULL for now
+ * @region: Region list to fill in
+ * @max_regions: Max size of region list, e.g. 100
+ * @path: Place to keep the path to each node (used by this function)
+ * @path_len: Max size of @path, 200 bytes is recommended
+ * @return 0 if OK, -FDT_ERR_... on error
+ */
+static int run_find_regions(const void *fdt,
+			    int (*include_func)(void *priv, const void *fdt,
+						int offset, int type,
+						const char *data, int size),
+			    void *priv, struct fdt_region *region,
+			    int max_regions, char *path, int path_len,
+			    int flags)
+{
+	struct fdt_region_state state;
+	int count;
+	int ret;
+
+	count = 0;
+	ret = fdt_first_region(fdt, include_func, NULL, &region[count++], path,
+			       path_len, flags, &state);
+	while (ret == 0) {
+		ret = fdt_next_region(fdt, include_func, NULL,
+				      count < max_regions ? &region[count] :
+				      NULL, path, path_len, flags, &state);
+		if (!ret)
+			count++;
+	}
+	if (ret && ret != -FDT_ERR_NOTFOUND)
+		return ret;
+
+	return count;
+}
+
+int fdt_get_regions(const void *blob, int strtab_len,
+		    struct image_region **regionp, int *region_countp)
+{
+	struct image_region *region;
+	struct fdt_region fdt_regions[100];
+	char path[200];
+	int count;
+
+	/* Get a list of regions to hash */
+	count = run_find_regions(blob, h_exclude_nodes, NULL, fdt_regions,
+				 ARRAY_SIZE(fdt_regions), path, sizeof(path),
+				 FDT_REG_SUPERNODES | FDT_REG_ADD_MEM_RSVMAP |
+				 FDT_REG_ADD_STRING_TAB);
+	if (count < 0) {
+		fprintf(stderr, "Failed to hash device tree\n");
+		return -EIO;
+	}
+	if (count == 0) {
+		fprintf(stderr, "No data to hash for device tree");
+		return -EINVAL;
+	}
+
+	/* Limit the string table to what was hashed */
+	if (strtab_len != -1) {
+		if (strtab_len < 0 ||
+		    strtab_len > fdt_regions[count - 1].size) {
+			fprintf(stderr, "Invalid string-table offset\n");
+			return -EINVAL;
+		}
+		fdt_regions[count - 1].size = strtab_len;
+	}
+
+	/* Build our list of data blocks */
+	region = fit_region_make_list(blob, fdt_regions, count, NULL);
+	if (!region) {
+		fprintf(stderr, "Out of memory making region list'\n");
+		return -ENOMEM;
+	}
+#ifdef DEBUG
+	int i;
+
+	printf("Regions:\n");
+	for (i = 0; i < count; i++) {
+		printf("region %d: %x %x %x\n", i, fdt_regions[i].offset,
+		       fdt_regions[i].size,
+		       fdt_regions[i].offset + fdt_regions[i].size);
+	}
+#endif
+	*region_countp = count;
+	*regionp = region;
+
+	return 0;
+}
+
+/**
+ * fdt_write_sig() - write the signature to an FDT
+ *
+ * This writes the signature and signer data to the FDT
+ *
+ * @blob: pointer to the FDT header
+ * @noffset: hash node offset
+ * @value: signature value to be set
+ * @value_len: signature value length
+ * @comment: Text comment to write (NULL for none)
+ *
+ * returns
+ *     offset of node where things were added, on success
+ *     -FDT_ERR_..., on failure
+ */
+static int fdt_write_sig(void *blob, uint8_t *value, int value_len,
+			 const char *algo_name, const char *sig_name,
+			 const char *comment, const char *cmdname)
+{
+	uint32_t strdata[2];
+	int string_size;
+	int sigs_node, noffset;
+	int ret;
+
+	/*
+	 * Get the current string size, before we update the FIT and add
+	 * more
+	 */
+	string_size = fdt_size_dt_strings(blob);
+
+	sigs_node = fdt_subnode_offset(blob, 0, FIT_SIG_NODENAME);
+	if (sigs_node == -FDT_ERR_NOTFOUND)
+		sigs_node = fdt_add_subnode(blob, 0, FIT_SIG_NODENAME);
+	if (sigs_node < 0)
+		return sigs_node;
+
+	/* Create a node for this signature */
+	noffset = fdt_subnode_offset(blob, sigs_node, sig_name);
+	if (noffset == -FDT_ERR_NOTFOUND)
+		noffset = fdt_add_subnode(blob, sigs_node, sig_name);
+	if (noffset < 0)
+		return noffset;
+
+	ret = fdt_setprop(blob, noffset, FIT_VALUE_PROP, value, value_len);
+	if (!ret) {
+		ret = fdt_setprop_string(blob, noffset, FIT_ALGO_PROP,
+					 algo_name);
+	}
+	if (!ret) {
+		ret = fdt_setprop_string(blob, noffset, "signer-name",
+					 "fdt_sign");
+	}
+	if (!ret) {
+		ret = fdt_setprop_string(blob, noffset, "signer-version",
+					 PLAIN_VERSION);
+	}
+	if (comment && !ret)
+		ret = fdt_setprop_string(blob, noffset, "comment", comment);
+	if (!ret) {
+		time_t timestamp = imagetool_get_source_date(cmdname,
+							     time(NULL));
+		uint32_t t = cpu_to_uimage(timestamp);
+
+		ret = fdt_setprop(blob, noffset, FIT_TIMESTAMP_PROP, &t,
+				  sizeof(uint32_t));
+	}
+
+	/* This is a legacy offset, it is unused, and must remain 0. */
+	strdata[0] = 0;
+	strdata[1] = cpu_to_fdt32(string_size);
+	if (!ret) {
+		ret = fdt_setprop(blob, noffset, "hashed-strings",
+				  strdata, sizeof(strdata));
+	}
+	if (ret)
+		return ret;
+
+	return noffset;
+}
+
+static int fdt_process_sig(const char *keydir, const char *keyfile,
+			   void *keydest, void *blob, const char *keyname,
+			   const char *comment, int require_keys,
+			   const char *engine_id, const char *cmdname,
+			   struct image_summary *summary)
+{
+	struct image_sign_info info;
+	struct image_region *region;
+	int region_count;
+	uint8_t *value;
+	uint value_len;
+	int ret;
+
+	ret = fdt_get_regions(blob, -1, &region, &region_count);
+	if (ret)
+		return ret;
+	ret = fdt_setup_sig(&info, keydir, keyfile, keyname, blob,
+			    "sha256,rsa2048", NULL, require_keys ? "fdt" : NULL,
+			    engine_id);
+	if (ret)
+		return ret;
+
+	ret = info.crypto->sign(&info, region, region_count, &value,
+				&value_len);
+	free(region);
+	if (ret) {
+		fprintf(stderr, "Failed to sign FDT\n");
+
+		/* We allow keys to be missing */
+		if (ret == -ENOENT)
+			return 0;
+		return -1;
+	}
+
+	ret = fdt_write_sig(blob, value, value_len, info.name, keyname, comment,
+			    cmdname);
+	if (ret < 0) {
+		if (ret == -FDT_ERR_NOSPACE)
+			return -ENOSPC;
+		printf("Can't write signature: %s\n", fdt_strerror(ret));
+		return -1;
+	}
+	summary->sig_offset = ret;
+	fdt_get_path(blob, ret, summary->sig_path,
+		     sizeof(summary->sig_path));
+	free(value);
+
+	/* Write the public key into the supplied FDT file */
+	if (keydest) {
+		ret = info.crypto->add_verify_data(&info, keydest);
+		if (ret < 0) {
+			if (ret != -ENOSPC)
+				fprintf(stderr, "Failed to add verification data (err=%d)\n",
+					ret);
+			return ret;
+		}
+		summary->keydest_offset = ret;
+		fdt_get_path(keydest, ret, summary->keydest_path,
+			     sizeof(summary->keydest_path));
+	}
+
+	return 0;
+}
+
+/* This function exists just to mirror fit_image_add_verification_data() */
+int fdt_add_verif_data(const char *keydir, const char *keyfile, void *keydest,
+		       void *blob, const char *keyname, const char *comment,
+		       bool require_keys, const char *engine_id,
+		       const char *cmdname, struct image_summary *summary)
+{
+	int ret;
+
+	ret = fdt_process_sig(keydir, keyfile, keydest, blob, keyname, comment,
+			      require_keys, engine_id, cmdname, summary);
+
+	return ret;
+}
diff --git a/tools/fdt_host.h b/tools/fdt_host.h
index bc42306c9e5..877f098676b 100644
--- a/tools/fdt_host.h
+++ b/tools/fdt_host.h
@@ -10,6 +10,8 @@
 #include "../include/linux/libfdt.h"
 #include "../include/fdt_support.h"
 
+struct image_region;
+
 /**
  * fdt_remove_unused_strings() - Remove any unused strings from an FDT
  *
@@ -38,4 +40,21 @@ int fdt_remove_unused_strings(const void *old, void *new);
 int fit_check_sign(const void *fit, const void *key,
 		   const char *fit_uname_config);
 
+/**
+ * fdt_get_regions() - Get the regions to sign
+ *
+ * This calculates a list of node to hash for this particular configuration,
+ * then finds which regions of the devicetree they correspond to.
+ *
+ * @blob:	Pointer to the FDT blob to sign
+ * @strtab_len:	Length in bytes of the string table to sign, -1 to sign it all
+ * @regionp: Returns list of regions that need to be hashed (allocated; must be
+ *	freed by the caller)
+ * @region_count: Returns number of regions
+ * @return 0 if OK, -ENOMEM if out of memory, -EIO if the regions to hash could
+ * not be found, -EINVAL if no registers were found to hash
+ */
+int fdt_get_regions(const void *blob, int strtab_len,
+		    struct image_region **regionp, int *region_countp);
+
 #endif /* __FDT_HOST_H__ */
diff --git a/tools/fdt_sign.c b/tools/fdt_sign.c
new file mode 100644
index 00000000000..b3a7fb4f8bf
--- /dev/null
+++ b/tools/fdt_sign.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Sign an FDT file
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include "mkimage.h"
+#include "fit_common.h"
+#include <image.h>
+
+void usage(char *cmdname)
+{
+	fprintf(stderr,
+		"Usage: %s -f dtb_file -G file.key -k dir [-K pub.dtb] [-o out_file]\n"
+		"          -f ==> set dtb file which should be signed\n"
+		"          -G ==> set signing key to use\n"
+		"          -k ==> set directory containing private keys\n"
+		"          -K ==> set DTB file to receive signing key\n"
+		"          -o ==> if not provided, dtb file is updated\n"
+		"          -S ==> name to use for signaure (defaults to -G)\n"
+		"          -r ==> mark keys as required to be verified\n"
+		"          -q ==> quiet mode\n",
+		cmdname);
+	exit(EXIT_FAILURE);
+}
+
+/**
+ * sign_fdt() - Sign an FDT
+ *
+ * @params: Image parameters
+ * @destfile: Filename of FDT being signed (only used for messages)
+ * @size_inc: Amount to expand the FDT blob by, before adding signing data
+ * @blob: FDT blob to sign
+ * @size: Size of blob in bytes
+ * @return 0 if OK, -ve on error
+ */
+static int sign_fdt(struct image_tool_params *params, const char *destfile,
+		    size_t size_inc, char *blob, int size)
+{
+	struct image_summary summary;
+	void *dest_blob = NULL;
+	struct stat dest_sbuf;
+	char def_name[80];
+	int destfd = 0;
+	int ret;
+
+	/*
+	 * If we don't have a signature name, try to make one from the keyfile.
+	 * '/path/to/dir/name.key' becomes 'name'
+	 */
+	if (!params->keyname) {
+		const char *p = strrchr(params->keyfile, '/');
+		char *q;
+
+		if (p)
+			p++;
+		else
+			p = params->keyfile;
+		strncpy(def_name, p, sizeof(def_name));
+		def_name[sizeof(def_name) - 1] = '\0';
+		q = strstr(def_name, ".key");
+		if (q && q[strlen(q)] == '\0')
+			*q = '\0';
+		params->keyname = def_name;
+	}
+
+	if (params->keydest) {
+		destfd = mmap_fdt(params->cmdname, params->keydest, size_inc,
+				  &dest_blob, &dest_sbuf, false,
+				  false);
+		if (destfd < 0) {
+			ret = -EIO;
+			fprintf(stderr, "Cannot open keydest file '%s'\n",
+				params->keydest);
+		}
+	}
+
+	ret = fdt_add_verif_data(params->keydir, params->keyfile, dest_blob,
+				 blob, params->keyname, params->comment,
+				 params->require_keys, params->engine_id,
+				 params->cmdname, &summary);
+	if (!ret && !params->quiet)
+		summary_show(&summary, destfile, params->keydest);
+
+	if (params->keydest) {
+		(void)munmap(dest_blob, dest_sbuf.st_size);
+		close(destfd);
+	}
+	if (ret < 0) {
+		if (ret != -ENOSPC)
+			fprintf(stderr, "Failed to add signature\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+/**
+ * do_fdt_sign() - Sign an FDT, expanding if needed
+ *
+ * If a separate output file is specified, the FDT blob is copied to that first.
+ *
+ * If there is not space in the FDT to add the signature, it is expanded
+ * slightly and the operation is retried.
+ *
+ * @params: Image parameters
+ * @cmdname: Name of tool (e.g. argv[0]), to use in error messages
+ * @fdtfile: Filename of FDT blob to sign
+ * @return 0 if OK, -ve on error
+ */
+static int do_fdt_sign(struct image_tool_params *params, const char *cmdname,
+		       const char *fdtfile)
+{
+	const char *destfile;
+	struct stat fsbuf;
+	int ffd, size_inc;
+	void *blob;
+	int ret;
+
+	destfile = params->outfile ? params->outfile : fdtfile;
+	for (size_inc = 0; size_inc < 64 * 1024;) {
+		if (params->outfile) {
+			if (copyfile(fdtfile, params->outfile) < 0) {
+				printf("Can't copy %s to %s\n", fdtfile,
+				       params->outfile);
+				return -EIO;
+			}
+		}
+		ffd = mmap_fdt(cmdname, destfile, size_inc, &blob, &fsbuf,
+			       params->outfile, false);
+		if (ffd < 0)
+			return -1;
+
+		ret = sign_fdt(params, destfile, 0, blob, fsbuf.st_size);
+		(void)munmap((void *)blob, fsbuf.st_size);
+		close(ffd);
+
+		if (ret >= 0 || ret != -ENOSPC)
+			break;
+		size_inc += 512;
+		debug("Not enough space in FDT '%s', trying size_inc=%#x\n",
+		      destfile, size_inc);
+	}
+
+	if (ret < 0) {
+		fprintf(stderr, "Failed to sign '%s' (error %d)\n",
+			destfile, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	struct image_tool_params params;
+	char *fdtfile = NULL;
+	char cmdname[256];
+	int ret;
+	int c;
+
+	memset(&params, '\0', sizeof(params));
+	strncpy(cmdname, *argv, sizeof(cmdname) - 1);
+	cmdname[sizeof(cmdname) - 1] = '\0';
+	while ((c = getopt(argc, argv, "f:G:k:K:o:qrS:")) != -1)
+		switch (c) {
+		case 'f':
+			fdtfile = optarg;
+			break;
+		case 'G':
+			params.keyfile = optarg;
+			break;
+		case 'k':
+			params.keydir = optarg;
+			break;
+		case 'K':
+			params.keydest = optarg;
+			break;
+		case 'o':
+			params.outfile = optarg;
+			break;
+		case 'q':
+			params.quiet = true;
+			break;
+		case 'r':
+			params.require_keys = true;
+			break;
+		case 'S':
+			params.keyname = optarg;
+			break;
+		default:
+			usage(cmdname);
+			break;
+	}
+
+	if (!fdtfile) {
+		fprintf(stderr, "%s: Missing fdt file\n", *argv);
+		usage(*argv);
+	}
+	if (!params.keyfile) {
+		fprintf(stderr, "%s: Missing key file\n", *argv);
+		usage(*argv);
+	}
+
+	ret = do_fdt_sign(&params, cmdname, fdtfile);
+
+	exit(ret);
+}
diff --git a/tools/imagetool.h b/tools/imagetool.h
index c0579c8c93c..4f9626fd1e0 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -70,6 +70,7 @@ struct image_tool_params {
 	const char *keydir;	/* Directory holding private keys */
 	const char *keydest;	/* Destination .dtb for public key */
 	const char *keyfile;	/* Filename of private or public key */
+	const char *keyname;	/* Name to use for key node if added */
 	const char *comment;	/* Comment to add to signature node */
 	int require_keys;	/* 1 to mark signing keys as 'required' */
 	int file_size;		/* Total size of output file */
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 14/16] tools: Add a new tool to check FDT-blob signatures

[Simon Glass]

Add a stand-alone (from mkimage) tool that checks signatures in an FDT
blob.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/Makefile         |   6 +-
 tools/fdt-host.c       |  20 ++++
 tools/fdt_check_sign.c |  85 ++++++++++++++
 tools/fdt_host.h       |  19 +++
 tools/image-fdt-sig.c  | 254 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 382 insertions(+), 2 deletions(-)
 create mode 100644 tools/fdt_check_sign.c
 create mode 100644 tools/image-fdt-sig.c

diff --git a/tools/Makefile b/tools/Makefile
index 07eca631cb0..b7857d86fac 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -73,13 +73,13 @@ mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
 hostprogs-y += dumpimage mkimage
 hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign \
-	fdt_sign
+	fdt_sign fdt_check_sign
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
 FIT_OBJS-y := fit_common.o fit_image.o image-host.o boot/image-fit.o
 FIT_SIG_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := image-sig-host.o \
-	boot/image-fit-sig.o fdt-host.o
+	boot/image-fit-sig.o fdt-host.o image-fdt-sig.o
 FIT_CIPHER_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := boot/image-cipher.o
 
 # The following files are synced with upstream DTC.
@@ -157,6 +157,7 @@ mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
 fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
 fdt_sign-objs   := $(dumpimage-mkimage-objs) fdt_sign.o
+fdt_check_sign-objs   := $(dumpimage-mkimage-objs) fdt_check_sign.o
 file2include-objs := file2include.o
 
 ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
@@ -195,6 +196,7 @@ HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_info := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_check_sign := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fdt_sign := $(HOSTLDLIBS_mkimage)
+HOSTLDLIBS_fdt_check_sign := $(HOSTLDLIBS_mkimage)
 
 hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
 hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
diff --git a/tools/fdt-host.c b/tools/fdt-host.c
index 8aa0e099f2f..142d486091b 100644
--- a/tools/fdt-host.c
+++ b/tools/fdt-host.c
@@ -331,3 +331,23 @@ int fdt_add_verif_data(const char *keydir, const char *keyfile, void *keydest,
 
 	return ret;
 }
+
+#ifdef CONFIG_FIT_SIGNATURE
+int fdt_check_sign(const void *blob, const void *key)
+{
+	int fdt_sigs;
+	int ret;
+
+	fdt_sigs = fdt_subnode_offset(blob, 0, FIT_SIG_NODENAME);
+	if (fdt_sigs < 0) {
+		printf("No %s node found (err=%d)\n", FIT_SIG_NODENAME,
+		       fdt_sigs);
+		return fdt_sigs;
+	}
+
+	ret = fdt_sig_verify(blob, fdt_sigs, key);
+	fprintf(stderr, "Verify %s\n", ret ? "failed" : "OK");
+
+	return ret;
+}
+#endif
diff --git a/tools/fdt_check_sign.c b/tools/fdt_check_sign.c
new file mode 100644
index 00000000000..943d01d5167
--- /dev/null
+++ b/tools/fdt_check_sign.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Check a signature in an FDT file
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include "mkimage.h"
+#include "fit_common.h"
+#include <image.h>
+
+void usage(char *cmdname)
+{
+	fprintf(stderr,
+		"Usage: %s -f dtb_file -k key file\n"
+		"          -f ==> set dtb file which should be checked\n"
+		"          -k ==> set key .dtb file which should be checked\n",
+		cmdname);
+	exit(EXIT_FAILURE);
+}
+
+int main(int argc, char **argv)
+{
+	struct image_tool_params params;
+	char *fdtfile = NULL;
+	char *keyfile = NULL;
+	char cmdname[256];
+	struct stat fsbuf;
+	struct stat ksbuf;
+	void *fdt_blob;
+	void *key_blob;
+	int ffd = -1;
+	int kfd = -1;
+	int ret;
+	int c;
+
+	memset(&params, '\0', sizeof(params));
+	strncpy(cmdname, *argv, sizeof(cmdname) - 1);
+	cmdname[sizeof(cmdname) - 1] = '\0';
+	while ((c = getopt(argc, argv, "f:k:")) != -1)
+		switch (c) {
+		case 'f':
+			fdtfile = optarg;
+			break;
+		case 'k':
+			keyfile = optarg;
+			break;
+		default:
+			usage(cmdname);
+			break;
+	}
+
+	if (!fdtfile) {
+		fprintf(stderr, "%s: Missing fdt file\n", *argv);
+		usage(*argv);
+	}
+	if (!keyfile) {
+		fprintf(stderr, "%s: Missing key file\n", *argv);
+		usage(*argv);
+	}
+
+	ffd = mmap_fdt(cmdname, fdtfile, 0, &fdt_blob, &fsbuf, false, true);
+	if (ffd < 0)
+		return EXIT_FAILURE;
+	kfd = mmap_fdt(cmdname, keyfile, 0, &key_blob, &ksbuf, false, true);
+	if (kfd < 0)
+		return EXIT_FAILURE;
+
+	ret = fdt_check_sign(fdt_blob, key_blob);
+	if (!ret) {
+		ret = EXIT_SUCCESS;
+		printf("Signature check OK\n");
+	} else {
+		ret = EXIT_FAILURE;
+		fprintf(stderr, "Signature check bad (error %d)\n", ret);
+	}
+
+	(void)munmap((void *)fdt_blob, fsbuf.st_size);
+	(void)munmap((void *)key_blob, ksbuf.st_size);
+
+	close(ffd);
+	close(kfd);
+	exit(ret);
+}
diff --git a/tools/fdt_host.h b/tools/fdt_host.h
index 877f098676b..b653efcb5d3 100644
--- a/tools/fdt_host.h
+++ b/tools/fdt_host.h
@@ -57,4 +57,23 @@ int fit_check_sign(const void *fit, const void *key,
 int fdt_get_regions(const void *blob, int strtab_len,
 		    struct image_region **regionp, int *region_countp);
 
+/**
+ * fdt_check_sign() - Check signatures in an FDT blob
+ *
+ * @fdt: FDT blob to check
+ * @key: Key FDT blob to check against
+ * @return 0 if OK, -ve if any required signature failed
+ */
+int fdt_check_sign(const void *blob, const void *key);
+
+/**
+ * fdt_sig_verify() - Check signatures in an FDT blob
+ *
+ * @fdt: FDT blob to check
+ * @fit_sigs: Offset of /signatures node in the FDT
+ * @key: Key FDT blob to check against
+ * @return 0 if OK, -ve if any required signature failed
+ */
+int fdt_sig_verify(const void *blob, int fdt_sigs, const void *key);
+
 #endif /* __FDT_HOST_H__ */
diff --git a/tools/image-fdt-sig.c b/tools/image-fdt-sig.c
new file mode 100644
index 00000000000..8b2570cbb06
--- /dev/null
+++ b/tools/image-fdt-sig.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 Google, LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include "mkimage.h"
+#include <image.h>
+#include <u-boot/rsa.h>
+
+/**
+ * fdt_setup_sig() - Setup the signing info ready for use
+ *
+ * @info: Info to set up
+ * @blob: FDT blob to check
+ * @noffset: Offset of signature to verify, e.g. '/signatures/sig-1'
+ * @key_blob: FDT blob containing keys to verify against
+ * @required_keynode: Node containing the key to verify
+ * @err_msgp: Returns the error messages if something goes wrong
+ * @return 0 if OK, -E2BIG if the FDT is too large, -ENOSYS if unsupported
+ *	algorithm, -EINVAL if the algorithm name is not present in the signature
+ */
+static int fdt_image_setup_verify(struct image_sign_info *info,
+				  const void *blob, int noffset,
+				  const void *key_blob, int required_keynode,
+				  char **err_msgp)
+{
+	char *algo_name;
+	const char *padding_name;
+
+	if (fdt_totalsize(blob) > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)) {
+		*err_msgp = "Total size too large";
+		return -E2BIG;
+	}
+	if (fit_image_hash_get_algo(blob, noffset, &algo_name)) {
+		*err_msgp = "Can't get hash algo property";
+		return -EINVAL;
+	}
+
+	padding_name = fdt_getprop(blob, noffset, "padding", NULL);
+	if (!padding_name)
+		padding_name = RSA_DEFAULT_PADDING_NAME;
+
+	memset(info, '\0', sizeof(*info));
+	info->keyname = fdt_getprop(blob, noffset, FIT_KEY_HINT, NULL);
+	info->fit = blob;
+	info->node_offset = noffset;
+	info->name = algo_name;
+	info->checksum = image_get_checksum_algo(algo_name);
+	info->crypto = image_get_crypto_algo(algo_name);
+	info->padding = image_get_padding_algo(padding_name);
+	info->fdt_blob = key_blob;
+	info->required_keynode = required_keynode;
+
+	if (!info->checksum || !info->crypto || !info->padding) {
+		*err_msgp = "Unknown signature algorithm";
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+/**
+ * fdt_verify_sig() - Verify that a signature can be verified with a key
+ *
+ * This checks a particular key to see if it verifies the given signature
+ *
+ * @blob: FDT blob to verify
+ * @noffset: Offset of signature to verify, e.g. '/signatures/sig-1'
+ * @key_blob: FDT blob containing keys to verify against
+ * @required_keynode: Node containing the key to verify
+ * @err_msgp: Returns the error messages if something goes wrong
+ * @return 0 if OK, -EPERM if the key could not be verified
+ */
+static int fdt_check_sig(const void *blob, int noffset, const void *key_blob,
+			 int required_keynode, char **err_msgp)
+{
+	int region_count, strtab_len;
+	struct image_sign_info info;
+	struct image_region *region;
+	const uint32_t *strings;
+	uint8_t *fdt_value;
+	int fdt_value_len;
+	int size;
+	int ret;
+
+	if (fdt_image_setup_verify(&info, blob, noffset, key_blob,
+				   required_keynode, err_msgp))
+		return -EPERM;
+
+	if (fit_image_hash_get_value(blob, noffset, &fdt_value,
+				     &fdt_value_len)) {
+		*err_msgp = "Can't get hash value property";
+		return -EPERM;
+	}
+
+	/* Add the strings */
+	strings = fdt_getprop(blob, noffset, "hashed-strings", &size);
+	if (!strings) {
+		*err_msgp = "Missing 'hashed-strings' property";
+		return -EINVAL;
+	}
+	if (size != sizeof(u32) * 2) {
+		*err_msgp = "Invalid 'hashed-strings' property";
+		return -EINVAL;
+	}
+	strtab_len = fdt32_to_cpu(strings[1]);
+	debug("%s: strtab_len=%x\n", __func__, strtab_len);
+
+	ret = fdt_get_regions(blob, strtab_len, &region, &region_count);
+	if (ret) {
+		*err_msgp = "Cannot get regions";
+		return ret;
+	}
+
+	ret = info.crypto->verify(&info, region, region_count, fdt_value,
+				  fdt_value_len);
+	free(region);
+	if (ret) {
+		*err_msgp = "Verification failed";
+		return -EPERM;
+	}
+
+	return 0;
+}
+
+/**
+ * fdt_verify_sig() - Verify that a signature exists for a key
+ *
+ * This checks a particular key to see if it verifies. It tries all signatures
+ * to find a match.
+ *
+ * @blob: FDT blob to verify
+ * @fdt_sigs: Offset of /signatures node in blob
+ * @key_blob: FDT blob containing keys to verify against
+ * @required_keynode: Node containing the key to verify
+ * @return 0 if OK, -EPERM if the key could not be verified
+ */
+static int fdt_verify_sig(const void *blob, int fdt_sigs,
+			  const void *key_blob, int required_keynode)
+{
+	char *err_msg = "No 'signature' subnode found";
+	int bad_noffset = -1;
+	int noffset;
+	int verified = 0;
+	int ret;
+
+	/* Process all subnodes of the /signature node */
+	fdt_for_each_subnode(noffset, blob, fdt_sigs) {
+		printf("%s", fdt_get_name(blob, noffset, NULL));
+		ret = fdt_check_sig(blob, noffset, key_blob, required_keynode,
+				    &err_msg);
+		if (ret) {
+			printf("- ");
+			bad_noffset = noffset;
+		} else {
+			printf("+ ");
+			verified = 1;
+			break;
+		}
+	}
+
+	if (noffset == -FDT_ERR_TRUNCATED || noffset == -FDT_ERR_BADSTRUCTURE) {
+		err_msg = "Corrupted or truncated tree";
+		goto error;
+	}
+
+	if (verified) {
+		printf("\n");
+		return 0;
+	}
+
+error:
+	printf(" error!\n%s for node '%s'\n", err_msg,
+	       fdt_get_name(blob, bad_noffset, NULL));
+	return -EPERM;
+}
+
+/**
+ * fdt_verify_required_sigs() - Verify that required signatures are valid
+ *
+ * This works through all the provided keys, checking for a signature that uses
+ * that key. If the key is required, then it must verify correctly. If it is not
+ * required, then the failure is ignored, just displayed for informational
+ * purposes.
+ *
+ * If the "required-mode" property is present and set to "any" then only one of
+ * the required keys needs to be verified
+ *
+ * @blob: FDT blob to verify
+ * @fdt_sigs: Offset of /signatures node in blob
+ * @key_blob: FDT blob containing keys to verify against
+ * @return 0 if OK, -EPERM if required keys could not be verified (either any or
+ *	all), -EINVAL if the FDT is missing something
+ */
+static int fdt_verify_required_sigs(const void *blob, int fdt_sigs,
+				    const void *key_blob)
+{
+	int key_node;
+	int keys_node;
+	int verified = 0;
+	int reqd_sigs = 0;
+	bool reqd_policy_all = true;
+	const char *reqd_mode;
+
+	/* Work out what we need to verify */
+	keys_node = fdt_subnode_offset(key_blob, 0, FIT_SIG_NODENAME);
+	if (keys_node < 0) {
+		debug("%s: No signature node found: %s\n", __func__,
+		      fdt_strerror(keys_node));
+		return 0;
+	}
+
+	/* Get required-mode policy property from DTB */
+	reqd_mode = fdt_getprop(key_blob, keys_node, "required-mode", NULL);
+	if (reqd_mode && !strcmp(reqd_mode, "any"))
+		reqd_policy_all = false;
+
+	debug("%s: required-mode policy set to '%s'\n", __func__,
+	      reqd_policy_all ? "all" : "any");
+
+	/* Check each key node */
+	fdt_for_each_subnode(key_node, key_blob, keys_node) {
+		const char *required_prop;
+		bool required;
+		int ret;
+
+		required_prop = fdt_getprop(key_blob, key_node, FIT_KEY_REQUIRED,
+					    NULL);
+		required = required_prop && !strcmp(required_prop, "fdt");
+
+		if (required)
+			reqd_sigs++;
+
+		ret = fdt_verify_sig(blob, fdt_sigs, key_blob, key_node);
+		if (!ret && required)
+			verified++;
+	}
+
+	if (reqd_policy_all && reqd_sigs != verified) {
+		fprintf(stderr, "Failed to verify all required signature\n");
+		return -EPERM;
+	} else if (reqd_sigs && !verified) {
+		fprintf(stderr, "Failed to verify 'any' of the required signature(s)\n");
+		return -EPERM;
+	}
+
+	return 0;
+}
+
+int fdt_sig_verify(const void *blob, int fdt_sigs, const void *key)
+{
+	return fdt_verify_required_sigs(blob, fdt_sigs, key);
+}
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 15/16] test: Add a test for FDT signing

[Simon Glass]

Add a test which checks that signing and checking work, including signing
an FDT twice.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 test/py/tests/test_fdt_sign.py   | 83 ++++++++++++++++++++++++++++++++
 test/py/tests/test_vboot.py      | 21 ++------
 test/py/tests/vboot/sign-fdt.dts | 23 +++++++++
 test/py/tests/vboot_comm.py      | 22 +++++++++
 4 files changed, 131 insertions(+), 18 deletions(-)
 create mode 100644 test/py/tests/test_fdt_sign.py
 create mode 100644 test/py/tests/vboot/sign-fdt.dts
 create mode 100644 test/py/tests/vboot_comm.py

diff --git a/test/py/tests/test_fdt_sign.py b/test/py/tests/test_fdt_sign.py
new file mode 100644
index 00000000000..72d58211159
--- /dev/null
+++ b/test/py/tests/test_fdt_sign.py
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2021 Google LLC
+#
+# U-Boot FDT-signing test
+
+import os
+
+import u_boot_utils as util
+import vboot_comm
+
+def test_fdt_sign(u_boot_console):
+    def dtc(dts):
+        """Run the device tree compiler to compile a .dts file
+
+        The output file will be the same as the input file but with a .dtb
+        extension.
+
+        Args:
+            dts: Device tree file to compile.
+        """
+        dtb = dts.replace('.dts', '.dtb')
+        util.run_and_log(cons, 'dtc %s %s%s -O dtb -p 0x1000 '
+                         '-o %s%s' % (dtc_args, datadir, dts, tmpdir, dtb))
+
+    cons = u_boot_console
+    datadir = os.path.join(cons.config.source_dir, 'test/py/tests/vboot/')
+    fdt_sign = os.path.join(cons.config.build_dir, 'tools/fdt_sign')
+    fdt_check_sign = os.path.join(cons.config.build_dir, 'tools/fdt_check_sign')
+
+    tmpdir = os.path.join(cons.config.result_dir, 'fdt_sign') + '/'
+    if not os.path.exists(tmpdir):
+        os.mkdir(tmpdir)
+
+    dtb = '%ssandbox-u-boot.dtb' % tmpdir
+    dtc_args = '-I dts -O dtb -i %s' % tmpdir
+    dtc('sign-fdt.dts')
+    dtc('sandbox-u-boot.dts')
+
+    vboot_comm.create_rsa_pair(cons, tmpdir, 'dev')
+
+    # Sign and check that it verifies
+    signed = os.path.join(tmpdir, 'sign-fdt.dtb')
+    cmd = [fdt_sign, '-f', signed, '-G', os.path.join(tmpdir, 'dev.key'),
+           '-K', dtb, '-k', tmpdir, '-r']
+    util.run_and_log(cons, ' '.join(cmd))
+
+    cmd = [fdt_check_sign, '-f', signed, '-k', dtb]
+    util.run_and_log(cons, ' '.join(cmd))
+
+    # Update the chosen node, which dpes not affect things since the signature
+    # omits that node
+    util.run_and_log(cons, 'fdtput -t bx %s /chosen fred 1' % signed)
+
+    cmd = [fdt_check_sign, '-f', signed, '-k', dtb]
+    util.run_and_log(cons, ' '.join(cmd))
+
+    # Update the alias node, which should break things because that is included
+    # in the signature
+    util.run_and_log(cons, 'fdtput -t bx %s /aliases fred 1' % signed)
+
+    cmd = [fdt_check_sign, '-f', signed, '-k', dtb]
+    util.run_and_log_expect_exception(cons, cmd, 1, 'Verification failed')
+
+    # Regenerate the original devictree and sign it
+    dtc('sign-fdt.dts')
+    dtc('sandbox-u-boot.dts')
+    out = os.path.join(tmpdir, 'out-fdt.dtb')
+    cmd = [fdt_sign, '-f', signed, '-G', os.path.join(tmpdir, 'dev.key'),
+           '-K', dtb, '-k', tmpdir, '-r', '-o', out]
+    util.run_and_log(cons, ' '.join(cmd))
+
+    cmd = [fdt_check_sign, '-f', out, '-k', dtb]
+    util.run_and_log(cons, ' '.join(cmd))
+
+    # Create a new key and sign with that too
+    vboot_comm.create_rsa_pair(cons, tmpdir, 'prod')
+    cmd = [fdt_sign, '-f', out, '-G', os.path.join(tmpdir, 'prod.key'),
+           '-K', dtb, '-k', tmpdir, '-r']
+    util.run_and_log(cons, ' '.join(cmd))
+
+    # Now check that both signatures are valid
+    cmd = [fdt_check_sign, '-f', out, '-k', dtb]
+    util.run_and_log(cons, ' '.join(cmd))
diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 095e00cce36..8458210691d 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -29,6 +29,7 @@ import shutil
 import struct
 import pytest
 import u_boot_utils as util
+import vboot_comm
 import vboot_forge
 import vboot_evil
 
@@ -173,22 +174,6 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
             handle.write(struct.pack(">I", size))
         return struct.unpack(">I", total_size)[0]
 
-    def create_rsa_pair(name):
-        """Generate a new RSA key paid and certificate
-
-        Args:
-            name: Name of of the key (e.g. 'dev')
-        """
-        public_exponent = 65537
-        util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %s%s.key '
-                     '-pkeyopt rsa_keygen_bits:2048 '
-                     '-pkeyopt rsa_keygen_pubexp:%d' %
-                     (tmpdir, name, public_exponent))
-
-        # Create a certificate containing the public key
-        util.run_and_log(cons, 'openssl req -batch -new -x509 -key %s%s.key '
-                         '-out %s%s.crt' % (tmpdir, name, tmpdir, name))
-
     def test_with_algo(sha_algo, padding, sign_options):
         """Test verified boot with the given hash algorithm.
 
@@ -377,8 +362,8 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
     dtb = '%ssandbox-u-boot.dtb' % tmpdir
     sig_node = '/configurations/conf-1/signature'
 
-    create_rsa_pair('dev')
-    create_rsa_pair('prod')
+    vboot_comm.create_rsa_pair(cons, tmpdir, 'dev')
+    vboot_comm.create_rsa_pair(cons, tmpdir, 'prod')
 
     # Create a number kernel image with zeroes
     with open('%stest-kernel.bin' % tmpdir, 'wb') as fd:
diff --git a/test/py/tests/vboot/sign-fdt.dts b/test/py/tests/vboot/sign-fdt.dts
new file mode 100644
index 00000000000..2c5b84976b1
--- /dev/null
+++ b/test/py/tests/vboot/sign-fdt.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	model = "Google Snow";
+	chosen {
+		bootargs = "console=tty1";
+		stdout-path = "serial3:115200n8";
+	};
+	aliases {
+		serail0 = "/serial@12c30000";
+	};
+
+	serial@12c30000 {
+		compatible = "samsung,exynos4210-uart";
+		reg = <0x12c30000 0x00000100>;
+		u-boot,dm-pre-reloc;
+	};
+};
diff --git a/test/py/tests/vboot_comm.py b/test/py/tests/vboot_comm.py
new file mode 100644
index 00000000000..7e1690e89f8
--- /dev/null
+++ b/test/py/tests/vboot_comm.py
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2021 Google LLC
+#
+# Common functions
+
+import u_boot_utils as util
+
+def create_rsa_pair(cons, tmpdir, name):
+    """Generate a new RSA key pair and certificate
+
+    Args:
+        name: Name of the key (e.g. 'dev')
+    """
+    public_exponent = 65537
+    util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %s%s.key '
+                 '-pkeyopt rsa_keygen_bits:2048 '
+                 '-pkeyopt rsa_keygen_pubexp:%d' %
+                 (tmpdir, name, public_exponent))
+
+    # Create a certificate containing the public key
+    util.run_and_log(cons, 'openssl req -batch -new -x509 -key %s%s.key '
+                     '-out %s%s.crt' % (tmpdir, name, tmpdir, name))
-- 
2.34.0.rc1.387.gb447b232ab-goog

[PATCH 16/16] tools: Add man pages for fdt_sign and fdt_check_sign

[Simon Glass]

Add some documentation for these tools, along with a MAINTAINER entry.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 MAINTAINERS          |   7 +++
 doc/fdt_check_sign.1 |  74 +++++++++++++++++++++++++++++
 doc/fdt_sign.1       | 111 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 192 insertions(+)
 create mode 100644 doc/fdt_check_sign.1
 create mode 100644 doc/fdt_sign.1

diff --git a/MAINTAINERS b/MAINTAINERS
index 00ff572d4d2..9f476d03eb0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -780,6 +780,13 @@ F:	cmd/fdt.c
 F:	common/fdt_support.c
 F:	scripts/dtc-version.sh
 
+FDT SIGNING
+M:	Simon Glass <sjg@chromium.org>
+S:	Maintained
+F:	doc/fdt_*sign.1
+F:	tools/fdt_check_sign.c
+F:	tools/fdt_sign.c
+
 FREEBSD
 M:	Rafal Jaworowski <raj@semihalf.com>
 S:	Maintained
diff --git a/doc/fdt_check_sign.1 b/doc/fdt_check_sign.1
new file mode 100644
index 00000000000..ca4d338a92b
--- /dev/null
+++ b/doc/fdt_check_sign.1
@@ -0,0 +1,74 @@
+.TH FDT_CHECK_SIGN 1 "2021-11-11"
+
+.SH NAME
+fdt_check_sign \- Check the signature in a flat device tree (FDT) file
+.SH SYNOPSIS
+.B fdt_check_sign
+.RB [\fIoptions\fP] " \-f [" "device tree binary file" "] " \-K " [" "key file" "]"
+
+.SH "DESCRIPTION"
+The
+.B fdt_check_sign
+command is used to check the signatures in a device tree binary file (.dtb
+file) to ensure it has not been modified since it was signed.
+
+Permitted modifications are to the
+.B /chosen
+node and
+.B /signature
+node/subnodes only.
+
+.SH "OPTIONS"
+
+.TP
+.BI "\-f [" "device tree binary file" "]"
+Selects the .dtb file to check.
+
+.TP
+.BI "\-k [" "key directory" "]"
+Selects the device tree binary file containing the public key to check against.
+
+.SH EXAMPLES
+
+.P
+Generate a key pair with openssl:
+
+.nf
+   $ openssl genpkey -algorithm RSA -out name.key \\
+                 -pkeyopt rsa_keygen_bits:2048 \\
+                 -pkeyopt rsa_keygen_pubexp:65537
+   $ openssl req -batch -new -x509 -key name.key -out name.crt
+.fi
+
+.P
+Create a .dtb file to accept the public key, called pubkey.dtb in this example.
+
+   # echo "/dts-v1/; /{};" | dtc - -o pubkey.dtb
+
+Sign a device tree binary:
+
+.nf
+   $ fdt_sign -f input.dtb -G name.key -K pubkey.dtb -k . -r
+   Signature written to 'input.dtb', node '/signature/name'
+   Public key written to 'pubkey.dtb', node '/signature/key-name
+.fi
+
+.P
+Check the signatures:
+
+.nf
+   $ fdt_check_sign -f input.dtb -k pubkey.dtb
+   name+
+   Verify OK
+   Signature check OK
+.fi
+
+.SH SEE ALSO
+
+fdt_sign
+
+.SH HOMEPAGE
+http://www.denx.de/wiki/U-Boot/WebHome
+.PP
+.SH AUTHOR
+This manual page was written by Simon Glass <sjg@chromium.org>.
diff --git a/doc/fdt_sign.1 b/doc/fdt_sign.1
new file mode 100644
index 00000000000..39f8bdc2a40
--- /dev/null
+++ b/doc/fdt_sign.1
@@ -0,0 +1,111 @@
+.TH FDT_SIGN 1 "2021-11-11"
+
+.SH NAME
+fdt_sign \- Sign a flat device tree (FDT) file
+.SH SYNOPSIS
+.B fdt_sign
+.RB [\fIoptions\fP] " \-f [" "device tree binary file" "] " \-G " [" "key file" "]"
+
+.SH "DESCRIPTION"
+The
+.B fdt_sign
+command is used to sign device tree binary files (.dtb files) so that they be
+protected against unwanted modification.
+
+.B fdt_sign
+adds a /signature node to the FDT, if not already present, then adds a node with
+the signature itself, in a simplified format. It uses a private key to generate
+the signature.
+
+.B fdt_check_sign
+can then be used to verify the signature against a public key.
+
+The /chosen node is ignored when signing. A /signature node is added containing
+the signature.
+
+.SH "OPTIONS"
+
+.TP
+.BI "\-f [" "device tree binary file" "]"
+Selects the .dtb file to sign.
+
+.TP
+.BI "\-G [" "key file" "]"
+Selects the key file to sign with. This should be a .key file containing an
+RSA private key.
+
+.TP
+.BI "\-k [" "key directory" "]"
+Selects the directory containing the key file.
+
+.TP
+.BI "\-K [" "key .dtb file" "]"
+Selects the device tree binary file to write the public key into. This is
+updated with a /signature subnode containing the public key the can be used to
+verify the signed device tree.
+
+.TP
+.BI "\-o [" "output file" "]"
+Selects the output file to write to. If omitted, the device tree binary file is
+updated in-place and only the
+.B -f
+parameter is used.
+
+.TP
+.BI "\-q
+Selects quiet mode which supresses non-error output.
+
+.TP
+.BI "\-r
+Marks the key as 'required', meaning that it must be used to check a signature
+in the device tree, when fdt_check_sign is used.
+
+.TP
+.BI "\-S
+Selects the key name to sign with. This is optional and defaults to the base
+name of the key file.
+
+.SH EXAMPLES
+
+.P
+Generate a key pair with openssl:
+
+.nf
+   $ openssl genpkey -algorithm RSA -out name.key \\
+                 -pkeyopt rsa_keygen_bits:2048 \\
+                 -pkeyopt rsa_keygen_pubexp:65537
+   $ openssl req -batch -new -x509 -key name.key -out name.crt
+.fi
+
+.P
+Create a .dtb file to accept the public key, called pubkey.dtb in this example.
+
+   # echo "/dts-v1/; /{};" | dtc - -o pubkey.dtb
+
+Sign a device tree binary:
+
+.nf
+   $ fdt_sign -f input.dtb -G name.key -K pubkey.dtb -k . -r
+   Signature written to 'input.dtb', node '/signature/name'
+   Public key written to 'pubkey.dtb', node '/signature/key-name
+.fi
+
+.P
+Check the signatures:
+
+.nf
+   $ fdt_check_sign -f input.dtb -k pubkey.dtb
+   name+
+   Verify OK
+   Signature check OK
+.fi
+
+.SH SEE ALSO
+
+fdt_check_sign
+
+.SH HOMEPAGE
+http://www.denx.de/wiki/U-Boot/WebHome
+.PP
+.SH AUTHOR
+This manual page was written by Simon Glass <sjg@chromium.org>.
-- 
2.34.0.rc1.387.gb447b232ab-goog

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[François Ozog]

Hi Simon

Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org> a écrit :

> At present mkimage supports signing FITs, the standard U-Boot image type.
>
> Various people are opposed to using FIT since:
>
just to be sure: I am not one of those.

>
> a) it requires adding support for FIT into other bootloaders, notably
>    UEFI

whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
FIT can evolve,  U-Boot UEFI does not have to change.

>
> b) it requires packaging a kernel in this standard U-Boot format, meaning
>    that distros must run 'mkimage' and deal with the kernel and initrd
>    being inside a FIT
>
> The kernel and initrd can be dealt with in other ways. But without FIT,
> we have no standard way of signing and grouping FDT files. Instead we must
> include them in the distro as separate files.
>
> In particular, some sort of mechanism for verifying FDT files is needed.
> One option would be to tack a signature on before or after the file,
> processing it accordingly. But due to the nature of the FDT binary format,
> it is possible to embed a signature inside the FDT itself, which is very
> convenient.
>
> This series provides a tool, fdt_sign, which can add a signature to an
> FDT. The signature can be checked later, preventing any change to the FDT,
> other than in permitted nodes (e.g. /chosen).
>
> This series also provides a fdt_check_sign tool, used to check signatures.
>
> Both of these tools are stand-alone do not require mkimage or FIT.
>
> As with FIT signing, multiple signatures are possible, but in this case
> that requires that fit_sign be called once for each signature. To make the
> check fail if a signature does not match, it should be marked as
> 'required' using the -r flag to fdt_sign.
>
> Run-time support for checking FDT signatures could be added to U-Boot
> fairly easily, but needs further discussion as the correct plumbing needs
> to be determined.
>
> For now there is absolutely no configurability in the signature mechanism.
> It would of course be possible to adjust which nodes are signed, as is
> done for FIT, but that needs further discussion also. The omission of the
> /chosen node is implemented in h_exclude_nodes() like this:
>
>    if (type == FDT_IS_NODE) {
>       /* Ignore the chosen node as well as /signature and subnodes */
>       if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>          return 0;
>    }
>
> Man pages are provided with example usage of the tools. Use this to view
> them:
>
>    man -l doc/fdt_check_sign.1
>
> This series also includes various clean-ups noticed along the way, as well
> as refactoring to avoid code duplication with the new tools. The last four
> patches are the new code.
>
> This series is available at u-boot-dm/fdt-sign-working :
>
>
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>
>
> Simon Glass (16):
>   rsa: Add debugging for failure cases
>   fit_check_sign: Update help to mention the key is in a dtb
>   tools: Move copyfile() into a common file
>   tools: Avoid leaving extra data at the end of copied files
>   tools: Improve comments in signing functions
>   tools: Drop unused name in image-host
>   tools: Avoid confusion between keys and signatures
>   tools: Tidy up argument order in fit_config_check_sig()
>   tools: Pass the key blob around
>   image: Return destination node for add_verify_data() method
>   tools: Pass public-key node through to caller
>   tools: mkimage: Show where signatures/keys are written
>   tools: Add a new tool to sign FDT blobs
>   tools: Add a new tool to check FDT-blob signatures
>   test: Add a test for FDT signing
>   tools: Add man pages for fdt_sign and fdt_check_sign
>
>  MAINTAINERS                      |   7 +
>  boot/image-fit-sig.c             | 151 +++++++++----
>  boot/image-fit.c                 |  12 +-
>  common/spl/spl_fit.c             |   3 +-
>  doc/fdt_check_sign.1             |  74 +++++++
>  doc/fdt_sign.1                   | 111 ++++++++++
>  include/image.h                  |  80 ++++++-
>  include/u-boot/ecdsa.h           |   5 +-
>  include/u-boot/rsa.h             |   5 +-
>  lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>  lib/rsa/rsa-sign.c               |   5 +-
>  lib/rsa/rsa-verify.c             |  13 +-
>  test/py/tests/test_fdt_sign.py   |  83 ++++++++
>  test/py/tests/test_vboot.py      |  21 +-
>  test/py/tests/vboot/sign-fdt.dts |  23 ++
>  test/py/tests/vboot_comm.py      |  22 ++
>  tools/Makefile                   |  10 +-
>  tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>  tools/fdt_check_sign.c           |  85 ++++++++
>  tools/fdt_host.h                 |  46 ++++
>  tools/fdt_sign.c                 | 210 ++++++++++++++++++
>  tools/fit_check_sign.c           |   4 +-
>  tools/fit_common.c               |  69 ++++++
>  tools/fit_common.h               |  23 ++
>  tools/fit_image.c                |  59 +-----
>  tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>  tools/image-host.c               | 155 +++++++++++---
>  tools/imagetool.h                |   4 +
>  tools/mkimage.c                  |   4 +
>  29 files changed, 1714 insertions(+), 181 deletions(-)
>  create mode 100644 doc/fdt_check_sign.1
>  create mode 100644 doc/fdt_sign.1
>  create mode 100644 test/py/tests/test_fdt_sign.py
>  create mode 100644 test/py/tests/vboot/sign-fdt.dts
>  create mode 100644 test/py/tests/vboot_comm.py
>  create mode 100644 tools/fdt-host.c
>  create mode 100644 tools/fdt_check_sign.c
>  create mode 100644 tools/fdt_sign.c
>  create mode 100644 tools/image-fdt-sig.c
>
> --
> 2.34.0.rc1.387.gb447b232ab-goog
>
> --
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Heinrich Schuchardt]

On 11/12/21 20:49, François Ozog wrote:
> Hi Simon
>
> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> <mailto:sjg@chromium.org>> a écrit :
>
>     At present mkimage supports signing FITs, the standard U-Boot image
>     type.
>
>     Various people are opposed to using FIT since:
>
> just to be sure: I am not one of those.
>
>
>     a) it requires adding support for FIT into other bootloaders, notably
>         UEFI
>
> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
> FIT can evolve,  U-Boot UEFI does not have to change.

We already can create signed FIT images containing a UEFI binary and a
devcie-tree and launch the FIT image with the bootm command.

Cf.
CONFIG_BOOTM_EFI
test/py/tests/test_efi_fit.py
doc/usage/bootefi.rst

Simon, what are you missing?

>
>
>     b) it requires packaging a kernel in this standard U-Boot format,
>     meaning
>         that distros must run 'mkimage' and deal with the kernel and initrd
>         being inside a FIT

U-Boot tools are contained in distros like Debian and Ubuntu.
Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
a similar purpose. The same hook directory can be used to create a FIT
image with a simple bash script.

Why do we need a new tool for signing device-trees?

The real problem to solve is the protection of the private key used for
signing any file containing an initrd.

>
>     The kernel and initrd can be dealt with in other ways. But without FIT,

How can the initrd be checked without FIT?

>     we have no standard way of signing and grouping FDT files. Instead
>     we must
>     include them in the distro as separate files.
>
>     In particular, some sort of mechanism for verifying FDT files is needed.
>     One option would be to tack a signature on before or after the file,
>     processing it accordingly. But due to the nature of the FDT binary
>     format,
>     it is possible to embed a signature inside the FDT itself, which is very
>     convenient.
>
>     This series provides a tool, fdt_sign, which can add a signature to an
>     FDT. The signature can be checked later, preventing any change to
>     the FDT,
>     other than in permitted nodes (e.g. /chosen).

I am not aware of any standard defining which nodes may be changed in
the FDT fixup and which may not be changed.

How can we discover which nodes were excluded from the signature after
the signature?

>
>     This series also provides a fdt_check_sign tool, used to check
>     signatures.
>
>     Both of these tools are stand-alone do not require mkimage or FIT.
>
>     As with FIT signing, multiple signatures are possible, but in this case
>     that requires that fit_sign be called once for each signature. To
>     make the
>     check fail if a signature does not match, it should be marked as
>     'required' using the -r flag to fdt_sign.
>
>     Run-time support for checking FDT signatures could be added to U-Boot
>     fairly easily, but needs further discussion as the correct plumbing
>     needs
>     to be determined.
>
>     For now there is absolutely no configurability in the signature
>     mechanism.

I would have expected a description of what a signature looks like. I
don't wont to reverse engineer your patches.

Please, describe this in doc/develop/ and in this cover-letter.

This series should have been sent as RFC.

Best regards

Heinrich

>     It would of course be possible to adjust which nodes are signed, as is
>     done for FIT, but that needs further discussion also. The omission
>     of the
>     /chosen node is implemented in h_exclude_nodes() like this:
>
>         if (type == FDT_IS_NODE) {
>            /* Ignore the chosen node as well as /signature and subnodes */
>            if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>               return 0;
>         }
>
>     Man pages are provided with example usage of the tools. Use this to view
>     them:
>
>         man -l doc/fdt_check_sign.1
>
>     This series also includes various clean-ups noticed along the way,
>     as well
>     as refactoring to avoid code duplication with the new tools. The
>     last four
>     patches are the new code.
>
>     This series is available at u-boot-dm/fdt-sign-working :
>
>     https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>     <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
>
>
>     Simon Glass (16):
>        rsa: Add debugging for failure cases
>        fit_check_sign: Update help to mention the key is in a dtb
>        tools: Move copyfile() into a common file
>        tools: Avoid leaving extra data at the end of copied files
>        tools: Improve comments in signing functions
>        tools: Drop unused name in image-host
>        tools: Avoid confusion between keys and signatures
>        tools: Tidy up argument order in fit_config_check_sig()
>        tools: Pass the key blob around
>        image: Return destination node for add_verify_data() method
>        tools: Pass public-key node through to caller
>        tools: mkimage: Show where signatures/keys are written
>        tools: Add a new tool to sign FDT blobs
>        tools: Add a new tool to check FDT-blob signatures
>        test: Add a test for FDT signing
>        tools: Add man pages for fdt_sign and fdt_check_sign
>
>       MAINTAINERS                      |   7 +
>       boot/image-fit-sig.c             | 151 +++++++++----
>       boot/image-fit.c                 |  12 +-
>       common/spl/spl_fit.c             |   3 +-
>       doc/fdt_check_sign.1             |  74 +++++++
>       doc/fdt_sign.1                   | 111 ++++++++++
>       include/image.h                  |  80 ++++++-
>       include/u-boot/ecdsa.h           |   5 +-
>       include/u-boot/rsa.h             |   5 +-
>       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>       lib/rsa/rsa-sign.c               |   5 +-
>       lib/rsa/rsa-verify.c             |  13 +-
>       test/py/tests/test_fdt_sign.py   |  83 ++++++++
>       test/py/tests/test_vboot.py      |  21 +-
>       test/py/tests/vboot/sign-fdt.dts |  23 ++
>       test/py/tests/vboot_comm.py      |  22 ++
>       tools/Makefile                   |  10 +-
>       tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>       tools/fdt_check_sign.c           |  85 ++++++++
>       tools/fdt_host.h                 |  46 ++++
>       tools/fdt_sign.c                 | 210 ++++++++++++++++++
>       tools/fit_check_sign.c           |   4 +-
>       tools/fit_common.c               |  69 ++++++
>       tools/fit_common.h               |  23 ++
>       tools/fit_image.c                |  59 +-----
>       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>       tools/image-host.c               | 155 +++++++++++---
>       tools/imagetool.h                |   4 +
>       tools/mkimage.c                  |   4 +
>       29 files changed, 1714 insertions(+), 181 deletions(-)
>       create mode 100644 doc/fdt_check_sign.1
>       create mode 100644 doc/fdt_sign.1
>       create mode 100644 test/py/tests/test_fdt_sign.py
>       create mode 100644 test/py/tests/vboot/sign-fdt.dts
>       create mode 100644 test/py/tests/vboot_comm.py
>       create mode 100644 tools/fdt-host.c
>       create mode 100644 tools/fdt_check_sign.c
>       create mode 100644 tools/fdt_sign.c
>       create mode 100644 tools/image-fdt-sig.c
>
>     --
>     2.34.0.rc1.387.gb447b232ab-goog
>
> --
>
> François-Frédéric Ozog | /Director Business Development/
> T: +33.67221.6485
> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
>
>

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Simon Glass]

Hi Heinrich,

On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 11/12/21 20:49, François Ozog wrote:
> > Hi Simon
> >
> > Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> > <mailto:sjg@chromium.org>> a écrit :
> >
> >     At present mkimage supports signing FITs, the standard U-Boot image
> >     type.
> >
> >     Various people are opposed to using FIT since:
> >
> > just to be sure: I am not one of those.
> >
> >
> >     a) it requires adding support for FIT into other bootloaders, notably
> >         UEFI
> >
> > whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
> > FIT can evolve,  U-Boot UEFI does not have to change.
>
> We already can create signed FIT images containing a UEFI binary and a
> devcie-tree and launch the FIT image with the bootm command.
>
> Cf.
> CONFIG_BOOTM_EFI
> test/py/tests/test_efi_fit.py
> doc/usage/bootefi.rst
>
> Simon, what are you missing?

Some people don't want to use FIT.

>
> >
> >
> >     b) it requires packaging a kernel in this standard U-Boot format,
> >     meaning
> >         that distros must run 'mkimage' and deal with the kernel and initrd
> >         being inside a FIT
>
> U-Boot tools are contained in distros like Debian and Ubuntu.
> Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
> a similar purpose. The same hook directory can be used to create a FIT
> image with a simple bash script.
>
> Why do we need a new tool for signing device-trees?
>
> The real problem to solve is the protection of the private key used for
> signing any file containing an initrd.

Well FIT already solves that one. Either FIT is not being used, in
which case this series is useful, or it is being used, in which case
the initrd problem is solved.

>
> >
> >     The kernel and initrd can be dealt with in other ways. But without FIT,
>
> How can the initrd be checked without FIT?

I don't know. Please check with the EFI people.

>
> >     we have no standard way of signing and grouping FDT files. Instead
> >     we must
> >     include them in the distro as separate files.
> >
> >     In particular, some sort of mechanism for verifying FDT files is needed.
> >     One option would be to tack a signature on before or after the file,
> >     processing it accordingly. But due to the nature of the FDT binary
> >     format,
> >     it is possible to embed a signature inside the FDT itself, which is very
> >     convenient.
> >
> >     This series provides a tool, fdt_sign, which can add a signature to an
> >     FDT. The signature can be checked later, preventing any change to
> >     the FDT,
> >     other than in permitted nodes (e.g. /chosen).
>
> I am not aware of any standard defining which nodes may be changed in
> the FDT fixup and which may not be changed.
>
> How can we discover which nodes were excluded from the signature after
> the signature?

There is no way at present. I decided against adding a list of signed
nodes. We can of course add whatever is useful here.

>
> >
> >     This series also provides a fdt_check_sign tool, used to check
> >     signatures.
> >
> >     Both of these tools are stand-alone do not require mkimage or FIT.
> >
> >     As with FIT signing, multiple signatures are possible, but in this case
> >     that requires that fit_sign be called once for each signature. To
> >     make the
> >     check fail if a signature does not match, it should be marked as
> >     'required' using the -r flag to fdt_sign.
> >
> >     Run-time support for checking FDT signatures could be added to U-Boot
> >     fairly easily, but needs further discussion as the correct plumbing
> >     needs
> >     to be determined.
> >
> >     For now there is absolutely no configurability in the signature
> >     mechanism.
>
> I would have expected a description of what a signature looks like. I
> don't wont to reverse engineer your patches.
>
> Please, describe this in doc/develop/ and in this cover-letter.

It is the same format as the FIT signature, an RSA signature. See here:

https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162

>
> This series should have been sent as RFC.

The last time I did that it disappeared without trace. You can of
course make comments on any series I send.

Regards,
Simon

>
> Best regards
>
> Heinrich
>
> >     It would of course be possible to adjust which nodes are signed, as is
> >     done for FIT, but that needs further discussion also. The omission
> >     of the
> >     /chosen node is implemented in h_exclude_nodes() like this:
> >
> >         if (type == FDT_IS_NODE) {
> >            /* Ignore the chosen node as well as /signature and subnodes */
> >            if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
> >               return 0;
> >         }
> >
> >     Man pages are provided with example usage of the tools. Use this to view
> >     them:
> >
> >         man -l doc/fdt_check_sign.1
> >
> >     This series also includes various clean-ups noticed along the way,
> >     as well
> >     as refactoring to avoid code duplication with the new tools. The
> >     last four
> >     patches are the new code.
> >
> >     This series is available at u-boot-dm/fdt-sign-working :
> >
> >     https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >     <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
> >
> >
> >     Simon Glass (16):
> >        rsa: Add debugging for failure cases
> >        fit_check_sign: Update help to mention the key is in a dtb
> >        tools: Move copyfile() into a common file
> >        tools: Avoid leaving extra data at the end of copied files
> >        tools: Improve comments in signing functions
> >        tools: Drop unused name in image-host
> >        tools: Avoid confusion between keys and signatures
> >        tools: Tidy up argument order in fit_config_check_sig()
> >        tools: Pass the key blob around
> >        image: Return destination node for add_verify_data() method
> >        tools: Pass public-key node through to caller
> >        tools: mkimage: Show where signatures/keys are written
> >        tools: Add a new tool to sign FDT blobs
> >        tools: Add a new tool to check FDT-blob signatures
> >        test: Add a test for FDT signing
> >        tools: Add man pages for fdt_sign and fdt_check_sign
> >
> >       MAINTAINERS                      |   7 +
> >       boot/image-fit-sig.c             | 151 +++++++++----
> >       boot/image-fit.c                 |  12 +-
> >       common/spl/spl_fit.c             |   3 +-
> >       doc/fdt_check_sign.1             |  74 +++++++
> >       doc/fdt_sign.1                   | 111 ++++++++++
> >       include/image.h                  |  80 ++++++-
> >       include/u-boot/ecdsa.h           |   5 +-
> >       include/u-boot/rsa.h             |   5 +-
> >       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> >       lib/rsa/rsa-sign.c               |   5 +-
> >       lib/rsa/rsa-verify.c             |  13 +-
> >       test/py/tests/test_fdt_sign.py   |  83 ++++++++
> >       test/py/tests/test_vboot.py      |  21 +-
> >       test/py/tests/vboot/sign-fdt.dts |  23 ++
> >       test/py/tests/vboot_comm.py      |  22 ++
> >       tools/Makefile                   |  10 +-
> >       tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
> >       tools/fdt_check_sign.c           |  85 ++++++++
> >       tools/fdt_host.h                 |  46 ++++
> >       tools/fdt_sign.c                 | 210 ++++++++++++++++++
> >       tools/fit_check_sign.c           |   4 +-
> >       tools/fit_common.c               |  69 ++++++
> >       tools/fit_common.h               |  23 ++
> >       tools/fit_image.c                |  59 +-----
> >       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> >       tools/image-host.c               | 155 +++++++++++---
> >       tools/imagetool.h                |   4 +
> >       tools/mkimage.c                  |   4 +
> >       29 files changed, 1714 insertions(+), 181 deletions(-)
> >       create mode 100644 doc/fdt_check_sign.1
> >       create mode 100644 doc/fdt_sign.1
> >       create mode 100644 test/py/tests/test_fdt_sign.py
> >       create mode 100644 test/py/tests/vboot/sign-fdt.dts
> >       create mode 100644 test/py/tests/vboot_comm.py
> >       create mode 100644 tools/fdt-host.c
> >       create mode 100644 tools/fdt_check_sign.c
> >       create mode 100644 tools/fdt_sign.c
> >       create mode 100644 tools/image-fdt-sig.c
> >
> >     --
> >     2.34.0.rc1.387.gb447b232ab-goog
> >
> > --
> >
> > François-Frédéric Ozog | /Director Business Development/
> > T: +33.67221.6485
> > francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
> >
> >

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[François Ozog]

Hi Simon,

Le sam. 13 nov. 2021 à 04:31, Simon Glass <sjg@chromium.org> a écrit :

> Hi Heinrich,
>
> On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> >
> >
> >
> > On 11/12/21 20:49, François Ozog wrote:
> > > Hi Simon
> > >
> > > Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> > > <mailto:sjg@chromium.org>> a écrit :
> > >
> > >     At present mkimage supports signing FITs, the standard U-Boot image
> > >     type.
> > >
> > >     Various people are opposed to using FIT since:
> > >
> > > just to be sure: I am not one of those.
> > >
> > >
> > >     a) it requires adding support for FIT into other bootloaders,
> notably
> > >         UEFI
> > >
> > > whatever happens to FIT is entirely orthogonal to U-Boot UEFI
> subsystem.
> > > FIT can evolve,  U-Boot UEFI does not have to change.
> >
> > We already can create signed FIT images containing a UEFI binary and a
> > devcie-tree and launch the FIT image with the bootm command.
> >
> > Cf.
> > CONFIG_BOOTM_EFI
> > test/py/tests/test_efi_fit.py
> > doc/usage/bootefi.rst
> >
> > Simon, what are you missing?
>
> Some people don't want to use FIT.
>
> >
> > >
> > >
> > >     b) it requires packaging a kernel in this standard U-Boot format,
> > >     meaning
> > >         that distros must run 'mkimage' and deal with the kernel and
> initrd
> > >         being inside a FIT
> >
> > U-Boot tools are contained in distros like Debian and Ubuntu.
> > Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
> > a similar purpose. The same hook directory can be used to create a FIT
> > image with a simple bash script.
> >
> > Why do we need a new tool for signing device-trees?
> >
> > The real problem to solve is the protection of the private key used for
> > signing any file containing an initrd.
>
> Well FIT already solves that one. Either FIT is not being used, in
> which case this series is useful, or it is being used, in which case
> the initrd problem is solved.
>
> >
> > >
> > >     The kernel and initrd can be dealt with in other ways. But without
> FIT,
> >
> > How can the initrd be checked without FIT?
>
> I don't know. Please check with the EFI people.
>
Ilias has made a proposal to “measure” files that do not have signatures
until files include signature.

>
> >
> > >     we have no standard way of signing and grouping FDT files. Instead
> > >     we must
> > >     include them in the distro as separate files.
> > >
> > >     In particular, some sort of mechanism for verifying FDT files is
> needed.
> > >     One option would be to tack a signature on before or after the
> file,
> > >     processing it accordingly. But due to the nature of the FDT binary
> > >     format,
> > >     it is possible to embed a signature inside the FDT itself, which
> is very
> > >     convenient.
> > >
> > >     This series provides a tool, fdt_sign, which can add a signature
> to an
> > >     FDT. The signature can be checked later, preventing any change to
> > >     the FDT,
> > >     other than in permitted nodes (e.g. /chosen).
> >

Shouldn’t devicetree.org and kernel.org folks involved in this DT signing
effort ? I believe there are parrallel efforts in this area. Or is it a
private <u-boot FDT sign>? in that case, they do not need to be involved.
Depending on the case, you may want to split the patch series in a number
of chunks.

>
> > I am not aware of any standard defining which nodes may be changed in
> > the FDT fixup and which may not be changed.
> >
> > How can we discover which nodes were excluded from the signature after
> > the signature?
>
> There is no way at present. I decided against adding a list of signed
> nodes. We can of course add whatever is useful here.
>
> >
> > >
> > >     This series also provides a fdt_check_sign tool, used to check
> > >     signatures.
> > >
> > >     Both of these tools are stand-alone do not require mkimage or FIT.
> > >
> > >     As with FIT signing, multiple signatures are possible, but in this
> case
> > >     that requires that fit_sign be called once for each signature. To
> > >     make the
> > >     check fail if a signature does not match, it should be marked as
> > >     'required' using the -r flag to fdt_sign.
> > >
> > >     Run-time support for checking FDT signatures could be added to
> U-Boot
> > >     fairly easily, but needs further discussion as the correct plumbing
> > >     needs
> > >     to be determined.
> > >
> > >     For now there is absolutely no configurability in the signature
> > >     mechanism.
> >
> > I would have expected a description of what a signature looks like. I
> > don't wont to reverse engineer your patches.
> >
> > Please, describe this in doc/develop/ and in this cover-letter.
>
> It is the same format as the FIT signature, an RSA signature. See here:
>
>
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
>
> >
> > This series should have been sent as RFC.
>
> The last time I did that it disappeared without trace. You can of
> course make comments on any series I send.
>
> Regards,
> Simon
>
> >
> > Best regards
> >
> > Heinrich
> >
> > >     It would of course be possible to adjust which nodes are signed,
> as is
> > >     done for FIT, but that needs further discussion also. The omission
> > >     of the
> > >     /chosen node is implemented in h_exclude_nodes() like this:
> > >
> > >         if (type == FDT_IS_NODE) {
> > >            /* Ignore the chosen node as well as /signature and
> subnodes */
> > >            if (!strcmp("/chosen", data) || !strncmp("/signature",
> data, 10))
> > >               return 0;
> > >         }
> > >
> > >     Man pages are provided with example usage of the tools. Use this
> to view
> > >     them:
> > >
> > >         man -l doc/fdt_check_sign.1
> > >
> > >     This series also includes various clean-ups noticed along the way,
> > >     as well
> > >     as refactoring to avoid code duplication with the new tools. The
> > >     last four
> > >     patches are the new code.
> > >
> > >     This series is available at u-boot-dm/fdt-sign-working :
> > >
> > >
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> > >     <
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >
> > >
> > >
> > >     Simon Glass (16):
> > >        rsa: Add debugging for failure cases
> > >        fit_check_sign: Update help to mention the key is in a dtb
> > >        tools: Move copyfile() into a common file
> > >        tools: Avoid leaving extra data at the end of copied files
> > >        tools: Improve comments in signing functions
> > >        tools: Drop unused name in image-host
> > >        tools: Avoid confusion between keys and signatures
> > >        tools: Tidy up argument order in fit_config_check_sig()
> > >        tools: Pass the key blob around
> > >        image: Return destination node for add_verify_data() method
> > >        tools: Pass public-key node through to caller
> > >        tools: mkimage: Show where signatures/keys are written
> > >        tools: Add a new tool to sign FDT blobs
> > >        tools: Add a new tool to check FDT-blob signatures
> > >        test: Add a test for FDT signing
> > >        tools: Add man pages for fdt_sign and fdt_check_sign
> > >
> > >       MAINTAINERS                      |   7 +
> > >       boot/image-fit-sig.c             | 151 +++++++++----
> > >       boot/image-fit.c                 |  12 +-
> > >       common/spl/spl_fit.c             |   3 +-
> > >       doc/fdt_check_sign.1             |  74 +++++++
> > >       doc/fdt_sign.1                   | 111 ++++++++++
> > >       include/image.h                  |  80 ++++++-
> > >       include/u-boot/ecdsa.h           |   5 +-
> > >       include/u-boot/rsa.h             |   5 +-
> > >       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> > >       lib/rsa/rsa-sign.c               |   5 +-
> > >       lib/rsa/rsa-verify.c             |  13 +-
> > >       test/py/tests/test_fdt_sign.py   |  83 ++++++++
> > >       test/py/tests/test_vboot.py      |  21 +-
> > >       test/py/tests/vboot/sign-fdt.dts |  23 ++
> > >       test/py/tests/vboot_comm.py      |  22 ++
> > >       tools/Makefile                   |  10 +-
> > >       tools/fdt-host.c                 | 353
> +++++++++++++++++++++++++++++++
> > >       tools/fdt_check_sign.c           |  85 ++++++++
> > >       tools/fdt_host.h                 |  46 ++++
> > >       tools/fdt_sign.c                 | 210 ++++++++++++++++++
> > >       tools/fit_check_sign.c           |   4 +-
> > >       tools/fit_common.c               |  69 ++++++
> > >       tools/fit_common.h               |  23 ++
> > >       tools/fit_image.c                |  59 +-----
> > >       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> > >       tools/image-host.c               | 155 +++++++++++---
> > >       tools/imagetool.h                |   4 +
> > >       tools/mkimage.c                  |   4 +
> > >       29 files changed, 1714 insertions(+), 181 deletions(-)
> > >       create mode 100644 doc/fdt_check_sign.1
> > >       create mode 100644 doc/fdt_sign.1
> > >       create mode 100644 test/py/tests/test_fdt_sign.py
> > >       create mode 100644 test/py/tests/vboot/sign-fdt.dts
> > >       create mode 100644 test/py/tests/vboot_comm.py
> > >       create mode 100644 tools/fdt-host.c
> > >       create mode 100644 tools/fdt_check_sign.c
> > >       create mode 100644 tools/fdt_sign.c
> > >       create mode 100644 tools/image-fdt-sig.c
> > >
> > >     --
> > >     2.34.0.rc1.387.gb447b232ab-goog
> > >
> > > --
> > >
> > > François-Frédéric Ozog | /Director Business Development/
> > > T: +33.67221.6485
> > > francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype:
> ffozog
> > >
> > >
>
-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Heinrich Schuchardt]

On 11/13/21 04:30, Simon Glass wrote:
> Hi Heinrich,
>
> On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>>
>>
>> On 11/12/21 20:49, François Ozog wrote:
>>> Hi Simon
>>>
>>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
>>> <mailto:sjg@chromium.org>> a écrit :
>>>
>>>      At present mkimage supports signing FITs, the standard U-Boot image
>>>      type.
>>>
>>>      Various people are opposed to using FIT since:
>>>
>>> just to be sure: I am not one of those.
>>>
>>>
>>>      a) it requires adding support for FIT into other bootloaders, notably
>>>          UEFI
>>>
>>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
>>> FIT can evolve,  U-Boot UEFI does not have to change.
>>
>> We already can create signed FIT images containing a UEFI binary and a
>> devcie-tree and launch the FIT image with the bootm command.
>>
>> Cf.
>> CONFIG_BOOTM_EFI
>> test/py/tests/test_efi_fit.py
>> doc/usage/bootefi.rst
>>
>> Simon, what are you missing?
>
> Some people don't want to use FIT.

The problem with FIT is that other firmware like EDK II does not support it.

Why do you expect those people to like your new tool better?

>
>>
>>>
>>>
>>>      b) it requires packaging a kernel in this standard U-Boot format,
>>>      meaning
>>>          that distros must run 'mkimage' and deal with the kernel and initrd
>>>          being inside a FIT
>>
>> U-Boot tools are contained in distros like Debian and Ubuntu.
>> Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
>> a similar purpose. The same hook directory can be used to create a FIT
>> image with a simple bash script.
>>
>> Why do we need a new tool for signing device-trees?
>>
>> The real problem to solve is the protection of the private key used for
>> signing any file containing an initrd.
>
> Well FIT already solves that one. Either FIT is not being used, in
> which case this series is useful, or it is being used, in which case
> the initrd problem is solved.

The question was:

How do you protect the private key used by Linux to sign the FIT image
with the updated initrd generated by intramfs-tools?

Best regards

Heinrich

>
>>
>>>
>>>      The kernel and initrd can be dealt with in other ways. But without FIT,
>>
>> How can the initrd be checked without FIT?
>
> I don't know. Please check with the EFI people.
>
>>
>>>      we have no standard way of signing and grouping FDT files. Instead
>>>      we must
>>>      include them in the distro as separate files.
>>>
>>>      In particular, some sort of mechanism for verifying FDT files is needed.
>>>      One option would be to tack a signature on before or after the file,
>>>      processing it accordingly. But due to the nature of the FDT binary
>>>      format,
>>>      it is possible to embed a signature inside the FDT itself, which is very
>>>      convenient.
>>>
>>>      This series provides a tool, fdt_sign, which can add a signature to an
>>>      FDT. The signature can be checked later, preventing any change to
>>>      the FDT,
>>>      other than in permitted nodes (e.g. /chosen).
>>
>> I am not aware of any standard defining which nodes may be changed in
>> the FDT fixup and which may not be changed.
>>
>> How can we discover which nodes were excluded from the signature after
>> the signature?
>
> There is no way at present. I decided against adding a list of signed
> nodes. We can of course add whatever is useful here.
>
>>
>>>
>>>      This series also provides a fdt_check_sign tool, used to check
>>>      signatures.
>>>
>>>      Both of these tools are stand-alone do not require mkimage or FIT.
>>>
>>>      As with FIT signing, multiple signatures are possible, but in this case
>>>      that requires that fit_sign be called once for each signature. To
>>>      make the
>>>      check fail if a signature does not match, it should be marked as
>>>      'required' using the -r flag to fdt_sign.
>>>
>>>      Run-time support for checking FDT signatures could be added to U-Boot
>>>      fairly easily, but needs further discussion as the correct plumbing
>>>      needs
>>>      to be determined.
>>>
>>>      For now there is absolutely no configurability in the signature
>>>      mechanism.
>>
>> I would have expected a description of what a signature looks like. I
>> don't wont to reverse engineer your patches.
>>
>> Please, describe this in doc/develop/ and in this cover-letter.
>
> It is the same format as the FIT signature, an RSA signature. See here:
>
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
>
>>
>> This series should have been sent as RFC.
>
> The last time I did that it disappeared without trace. You can of
> course make comments on any series I send.
>
> Regards,
> Simon
>
>>
>> Best regards
>>
>> Heinrich
>>
>>>      It would of course be possible to adjust which nodes are signed, as is
>>>      done for FIT, but that needs further discussion also. The omission
>>>      of the
>>>      /chosen node is implemented in h_exclude_nodes() like this:
>>>
>>>          if (type == FDT_IS_NODE) {
>>>             /* Ignore the chosen node as well as /signature and subnodes */
>>>             if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>>>                return 0;
>>>          }
>>>
>>>      Man pages are provided with example usage of the tools. Use this to view
>>>      them:
>>>
>>>          man -l doc/fdt_check_sign.1
>>>
>>>      This series also includes various clean-ups noticed along the way,
>>>      as well
>>>      as refactoring to avoid code duplication with the new tools. The
>>>      last four
>>>      patches are the new code.
>>>
>>>      This series is available at u-boot-dm/fdt-sign-working :
>>>
>>>      https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>>>      <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
>>>
>>>
>>>      Simon Glass (16):
>>>         rsa: Add debugging for failure cases
>>>         fit_check_sign: Update help to mention the key is in a dtb
>>>         tools: Move copyfile() into a common file
>>>         tools: Avoid leaving extra data at the end of copied files
>>>         tools: Improve comments in signing functions
>>>         tools: Drop unused name in image-host
>>>         tools: Avoid confusion between keys and signatures
>>>         tools: Tidy up argument order in fit_config_check_sig()
>>>         tools: Pass the key blob around
>>>         image: Return destination node for add_verify_data() method
>>>         tools: Pass public-key node through to caller
>>>         tools: mkimage: Show where signatures/keys are written
>>>         tools: Add a new tool to sign FDT blobs
>>>         tools: Add a new tool to check FDT-blob signatures
>>>         test: Add a test for FDT signing
>>>         tools: Add man pages for fdt_sign and fdt_check_sign
>>>
>>>        MAINTAINERS                      |   7 +
>>>        boot/image-fit-sig.c             | 151 +++++++++----
>>>        boot/image-fit.c                 |  12 +-
>>>        common/spl/spl_fit.c             |   3 +-
>>>        doc/fdt_check_sign.1             |  74 +++++++
>>>        doc/fdt_sign.1                   | 111 ++++++++++
>>>        include/image.h                  |  80 ++++++-
>>>        include/u-boot/ecdsa.h           |   5 +-
>>>        include/u-boot/rsa.h             |   5 +-
>>>        lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>>>        lib/rsa/rsa-sign.c               |   5 +-
>>>        lib/rsa/rsa-verify.c             |  13 +-
>>>        test/py/tests/test_fdt_sign.py   |  83 ++++++++
>>>        test/py/tests/test_vboot.py      |  21 +-
>>>        test/py/tests/vboot/sign-fdt.dts |  23 ++
>>>        test/py/tests/vboot_comm.py      |  22 ++
>>>        tools/Makefile                   |  10 +-
>>>        tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>>>        tools/fdt_check_sign.c           |  85 ++++++++
>>>        tools/fdt_host.h                 |  46 ++++
>>>        tools/fdt_sign.c                 | 210 ++++++++++++++++++
>>>        tools/fit_check_sign.c           |   4 +-
>>>        tools/fit_common.c               |  69 ++++++
>>>        tools/fit_common.h               |  23 ++
>>>        tools/fit_image.c                |  59 +-----
>>>        tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>>>        tools/image-host.c               | 155 +++++++++++---
>>>        tools/imagetool.h                |   4 +
>>>        tools/mkimage.c                  |   4 +
>>>        29 files changed, 1714 insertions(+), 181 deletions(-)
>>>        create mode 100644 doc/fdt_check_sign.1
>>>        create mode 100644 doc/fdt_sign.1
>>>        create mode 100644 test/py/tests/test_fdt_sign.py
>>>        create mode 100644 test/py/tests/vboot/sign-fdt.dts
>>>        create mode 100644 test/py/tests/vboot_comm.py
>>>        create mode 100644 tools/fdt-host.c
>>>        create mode 100644 tools/fdt_check_sign.c
>>>        create mode 100644 tools/fdt_sign.c
>>>        create mode 100644 tools/image-fdt-sig.c
>>>
>>>      --
>>>      2.34.0.rc1.387.gb447b232ab-goog
>>>
>>> --
>>>
>>> François-Frédéric Ozog | /Director Business Development/
>>> T: +33.67221.6485
>>> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
>>>
>>>

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Simon Glass]

Hi François,

On Sat, 13 Nov 2021 at 03:18, François Ozog <francois.ozog@linaro.org> wrote:
>
> Hi Simon,
>
> Le sam. 13 nov. 2021 à 04:31, Simon Glass <sjg@chromium.org> a écrit :
>>
>> Hi Heinrich,
>>
>> On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> >
>> >
>> >
>> > On 11/12/21 20:49, François Ozog wrote:
>> > > Hi Simon
>> > >
>> > > Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
>> > > <mailto:sjg@chromium.org>> a écrit :
>> > >
>> > >     At present mkimage supports signing FITs, the standard U-Boot image
>> > >     type.
>> > >
>> > >     Various people are opposed to using FIT since:
>> > >
>> > > just to be sure: I am not one of those.
>> > >
>> > >
>> > >     a) it requires adding support for FIT into other bootloaders, notably
>> > >         UEFI
>> > >
>> > > whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
>> > > FIT can evolve,  U-Boot UEFI does not have to change.
>> >
>> > We already can create signed FIT images containing a UEFI binary and a
>> > devcie-tree and launch the FIT image with the bootm command.
>> >
>> > Cf.
>> > CONFIG_BOOTM_EFI
>> > test/py/tests/test_efi_fit.py
>> > doc/usage/bootefi.rst
>> >
>> > Simon, what are you missing?
>>
>> Some people don't want to use FIT.
>>
>> >
>> > >
>> > >
>> > >     b) it requires packaging a kernel in this standard U-Boot format,
>> > >     meaning
>> > >         that distros must run 'mkimage' and deal with the kernel and initrd
>> > >         being inside a FIT
>> >
>> > U-Boot tools are contained in distros like Debian and Ubuntu.
>> > Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
>> > a similar purpose. The same hook directory can be used to create a FIT
>> > image with a simple bash script.
>> >
>> > Why do we need a new tool for signing device-trees?
>> >
>> > The real problem to solve is the protection of the private key used for
>> > signing any file containing an initrd.
>>
>> Well FIT already solves that one. Either FIT is not being used, in
>> which case this series is useful, or it is being used, in which case
>> the initrd problem is solved.
>>
>> >
>> > >
>> > >     The kernel and initrd can be dealt with in other ways. But without FIT,
>> >
>> > How can the initrd be checked without FIT?
>>
>> I don't know. Please check with the EFI people.
>
> Ilias has made a proposal to “measure” files that do not have signatures until files include signature.

OK

>>
>>
>> >
>> > >     we have no standard way of signing and grouping FDT files. Instead
>> > >     we must
>> > >     include them in the distro as separate files.
>> > >
>> > >     In particular, some sort of mechanism for verifying FDT files is needed.
>> > >     One option would be to tack a signature on before or after the file,
>> > >     processing it accordingly. But due to the nature of the FDT binary
>> > >     format,
>> > >     it is possible to embed a signature inside the FDT itself, which is very
>> > >     convenient.
>> > >
>> > >     This series provides a tool, fdt_sign, which can add a signature to an
>> > >     FDT. The signature can be checked later, preventing any change to
>> > >     the FDT,
>> > >     other than in permitted nodes (e.g. /chosen).
>> >
>
> Shouldn’t devicetree.org and kernel.org folks involved in this DT signing effort ? I believe there are parrallel efforts in this area. Or is it a private <u-boot FDT sign>? in that case, they do not need to be involved. Depending on the case, you may want to split the patch series in a number of chunks.

Yes indeed these people should be involved, but I also think that
interested parties should be part of the boot-architecture effort. The
meeting notes where this tool was discussed are here:

https://lists.linaro.org/pipermail/boot-architecture/2021-October/001955.html

My slides are available there, too.

Here is what I have done so far: I sent a v4 series to
devicetree-compiler a week ago with the fdt_region and fdtgrep parts.
Rob and Bill are copied on this series, as you can see. I was not sure
about sending U-Boot patches to the boot-architecture list, but I will
cover it in a future meeting.

But if there are others that should be copied, please feel free to add
them, or point me to them.

Regards,
Simon


>>
>>
>> > I am not aware of any standard defining which nodes may be changed in
>> > the FDT fixup and which may not be changed.
>> >
>> > How can we discover which nodes were excluded from the signature after
>> > the signature?
>>
>> There is no way at present. I decided against adding a list of signed
>> nodes. We can of course add whatever is useful here.
>>
>> >
>> > >
>> > >     This series also provides a fdt_check_sign tool, used to check
>> > >     signatures.
>> > >
>> > >     Both of these tools are stand-alone do not require mkimage or FIT.
>> > >
>> > >     As with FIT signing, multiple signatures are possible, but in this case
>> > >     that requires that fit_sign be called once for each signature. To
>> > >     make the
>> > >     check fail if a signature does not match, it should be marked as
>> > >     'required' using the -r flag to fdt_sign.
>> > >
>> > >     Run-time support for checking FDT signatures could be added to U-Boot
>> > >     fairly easily, but needs further discussion as the correct plumbing
>> > >     needs
>> > >     to be determined.
>> > >
>> > >     For now there is absolutely no configurability in the signature
>> > >     mechanism.
>> >
>> > I would have expected a description of what a signature looks like. I
>> > don't wont to reverse engineer your patches.
>> >
>> > Please, describe this in doc/develop/ and in this cover-letter.
>>
>> It is the same format as the FIT signature, an RSA signature. See here:
>>
>> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
>>
>> >
>> > This series should have been sent as RFC.
>>
>> The last time I did that it disappeared without trace. You can of
>> course make comments on any series I send.
>>
>> Regards,
>> Simon
>>
>> >
>> > Best regards
>> >
>> > Heinrich
>> >
>> > >     It would of course be possible to adjust which nodes are signed, as is
>> > >     done for FIT, but that needs further discussion also. The omission
>> > >     of the
>> > >     /chosen node is implemented in h_exclude_nodes() like this:
>> > >
>> > >         if (type == FDT_IS_NODE) {
>> > >            /* Ignore the chosen node as well as /signature and subnodes */
>> > >            if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>> > >               return 0;
>> > >         }
>> > >
>> > >     Man pages are provided with example usage of the tools. Use this to view
>> > >     them:
>> > >
>> > >         man -l doc/fdt_check_sign.1
>> > >
>> > >     This series also includes various clean-ups noticed along the way,
>> > >     as well
>> > >     as refactoring to avoid code duplication with the new tools. The
>> > >     last four
>> > >     patches are the new code.
>> > >
>> > >     This series is available at u-boot-dm/fdt-sign-working :
>> > >
>> > >     https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>> > >     <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
>> > >
>> > >
>> > >     Simon Glass (16):
>> > >        rsa: Add debugging for failure cases
>> > >        fit_check_sign: Update help to mention the key is in a dtb
>> > >        tools: Move copyfile() into a common file
>> > >        tools: Avoid leaving extra data at the end of copied files
>> > >        tools: Improve comments in signing functions
>> > >        tools: Drop unused name in image-host
>> > >        tools: Avoid confusion between keys and signatures
>> > >        tools: Tidy up argument order in fit_config_check_sig()
>> > >        tools: Pass the key blob around
>> > >        image: Return destination node for add_verify_data() method
>> > >        tools: Pass public-key node through to caller
>> > >        tools: mkimage: Show where signatures/keys are written
>> > >        tools: Add a new tool to sign FDT blobs
>> > >        tools: Add a new tool to check FDT-blob signatures
>> > >        test: Add a test for FDT signing
>> > >        tools: Add man pages for fdt_sign and fdt_check_sign
>> > >
>> > >       MAINTAINERS                      |   7 +
>> > >       boot/image-fit-sig.c             | 151 +++++++++----
>> > >       boot/image-fit.c                 |  12 +-
>> > >       common/spl/spl_fit.c             |   3 +-
>> > >       doc/fdt_check_sign.1             |  74 +++++++
>> > >       doc/fdt_sign.1                   | 111 ++++++++++
>> > >       include/image.h                  |  80 ++++++-
>> > >       include/u-boot/ecdsa.h           |   5 +-
>> > >       include/u-boot/rsa.h             |   5 +-
>> > >       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>> > >       lib/rsa/rsa-sign.c               |   5 +-
>> > >       lib/rsa/rsa-verify.c             |  13 +-
>> > >       test/py/tests/test_fdt_sign.py   |  83 ++++++++
>> > >       test/py/tests/test_vboot.py      |  21 +-
>> > >       test/py/tests/vboot/sign-fdt.dts |  23 ++
>> > >       test/py/tests/vboot_comm.py      |  22 ++
>> > >       tools/Makefile                   |  10 +-
>> > >       tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>> > >       tools/fdt_check_sign.c           |  85 ++++++++
>> > >       tools/fdt_host.h                 |  46 ++++
>> > >       tools/fdt_sign.c                 | 210 ++++++++++++++++++
>> > >       tools/fit_check_sign.c           |   4 +-
>> > >       tools/fit_common.c               |  69 ++++++
>> > >       tools/fit_common.h               |  23 ++
>> > >       tools/fit_image.c                |  59 +-----
>> > >       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>> > >       tools/image-host.c               | 155 +++++++++++---
>> > >       tools/imagetool.h                |   4 +
>> > >       tools/mkimage.c                  |   4 +
>> > >       29 files changed, 1714 insertions(+), 181 deletions(-)
>> > >       create mode 100644 doc/fdt_check_sign.1
>> > >       create mode 100644 doc/fdt_sign.1
>> > >       create mode 100644 test/py/tests/test_fdt_sign.py
>> > >       create mode 100644 test/py/tests/vboot/sign-fdt.dts
>> > >       create mode 100644 test/py/tests/vboot_comm.py
>> > >       create mode 100644 tools/fdt-host.c
>> > >       create mode 100644 tools/fdt_check_sign.c
>> > >       create mode 100644 tools/fdt_sign.c
>> > >       create mode 100644 tools/image-fdt-sig.c
>> > >
>> > >     --
>> > >     2.34.0.rc1.387.gb447b232ab-goog
>> > >
>> > > --
>> > >
>> > > François-Frédéric Ozog | /Director Business Development/
>> > > T: +33.67221.6485
>> > > francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
>> > >
>> > >
>
> --
> François-Frédéric Ozog | Director Business Development
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Simon Glass]

Hi Heinrich,

On Sat, 13 Nov 2021 at 04:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 11/13/21 04:30, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >>
> >>
> >> On 11/12/21 20:49, François Ozog wrote:
> >>> Hi Simon
> >>>
> >>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> >>> <mailto:sjg@chromium.org>> a écrit :
> >>>
> >>>      At present mkimage supports signing FITs, the standard U-Boot image
> >>>      type.
> >>>
> >>>      Various people are opposed to using FIT since:
> >>>
> >>> just to be sure: I am not one of those.
> >>>
> >>>
> >>>      a) it requires adding support for FIT into other bootloaders, notably
> >>>          UEFI
> >>>
> >>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
> >>> FIT can evolve,  U-Boot UEFI does not have to change.
> >>
> >> We already can create signed FIT images containing a UEFI binary and a
> >> devcie-tree and launch the FIT image with the bootm command.
> >>
> >> Cf.
> >> CONFIG_BOOTM_EFI
> >> test/py/tests/test_efi_fit.py
> >> doc/usage/bootefi.rst
> >>
> >> Simon, what are you missing?
> >
> > Some people don't want to use FIT.
>
> The problem with FIT is that other firmware like EDK II does not support it.
>
> Why do you expect those people to like your new tool better?

I believe the EDK decision is not so much that it *does* not support
FIT, which is after all not a lot of code, but that it *should* not.
If I have that wrong, please let me know.

The goal here is to support signing in an FDT without FIT. I believe
EDK supports FDT, at least.

>
> >
> >>
> >>>
> >>>
> >>>      b) it requires packaging a kernel in this standard U-Boot format,
> >>>      meaning
> >>>          that distros must run 'mkimage' and deal with the kernel and initrd
> >>>          being inside a FIT
> >>
> >> U-Boot tools are contained in distros like Debian and Ubuntu.
> >> Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
> >> a similar purpose. The same hook directory can be used to create a FIT
> >> image with a simple bash script.
> >>
> >> Why do we need a new tool for signing device-trees?
> >>
> >> The real problem to solve is the protection of the private key used for
> >> signing any file containing an initrd.
> >
> > Well FIT already solves that one. Either FIT is not being used, in
> > which case this series is useful, or it is being used, in which case
> > the initrd problem is solved.
>
> The question was:
>
> How do you protect the private key used by Linux to sign the FIT image
> with the updated initrd generated by intramfs-tools?

Well, if it is a FIT we can add a seperate signature at the time the
initramfs-tools runs. It does support multiple signatures. If that is
not suitable I am sure something else can be devised. What are the
constraints / requirements?

Regards,
Simon

>
> Best regards
>
> Heinrich
>
> >
> >>
> >>>
> >>>      The kernel and initrd can be dealt with in other ways. But without FIT,
> >>
> >> How can the initrd be checked without FIT?
> >
> > I don't know. Please check with the EFI people.
> >
> >>
> >>>      we have no standard way of signing and grouping FDT files. Instead
> >>>      we must
> >>>      include them in the distro as separate files.
> >>>
> >>>      In particular, some sort of mechanism for verifying FDT files is needed.
> >>>      One option would be to tack a signature on before or after the file,
> >>>      processing it accordingly. But due to the nature of the FDT binary
> >>>      format,
> >>>      it is possible to embed a signature inside the FDT itself, which is very
> >>>      convenient.
> >>>
> >>>      This series provides a tool, fdt_sign, which can add a signature to an
> >>>      FDT. The signature can be checked later, preventing any change to
> >>>      the FDT,
> >>>      other than in permitted nodes (e.g. /chosen).
> >>
> >> I am not aware of any standard defining which nodes may be changed in
> >> the FDT fixup and which may not be changed.
> >>
> >> How can we discover which nodes were excluded from the signature after
> >> the signature?
> >
> > There is no way at present. I decided against adding a list of signed
> > nodes. We can of course add whatever is useful here.
> >
> >>
> >>>
> >>>      This series also provides a fdt_check_sign tool, used to check
> >>>      signatures.
> >>>
> >>>      Both of these tools are stand-alone do not require mkimage or FIT.
> >>>
> >>>      As with FIT signing, multiple signatures are possible, but in this case
> >>>      that requires that fit_sign be called once for each signature. To
> >>>      make the
> >>>      check fail if a signature does not match, it should be marked as
> >>>      'required' using the -r flag to fdt_sign.
> >>>
> >>>      Run-time support for checking FDT signatures could be added to U-Boot
> >>>      fairly easily, but needs further discussion as the correct plumbing
> >>>      needs
> >>>      to be determined.
> >>>
> >>>      For now there is absolutely no configurability in the signature
> >>>      mechanism.
> >>
> >> I would have expected a description of what a signature looks like. I
> >> don't wont to reverse engineer your patches.
> >>
> >> Please, describe this in doc/develop/ and in this cover-letter.
> >
> > It is the same format as the FIT signature, an RSA signature. See here:
> >
> > https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
> >
> >>
> >> This series should have been sent as RFC.
> >
> > The last time I did that it disappeared without trace. You can of
> > course make comments on any series I send.
> >
> > Regards,
> > Simon
> >
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>      It would of course be possible to adjust which nodes are signed, as is
> >>>      done for FIT, but that needs further discussion also. The omission
> >>>      of the
> >>>      /chosen node is implemented in h_exclude_nodes() like this:
> >>>
> >>>          if (type == FDT_IS_NODE) {
> >>>             /* Ignore the chosen node as well as /signature and subnodes */
> >>>             if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
> >>>                return 0;
> >>>          }
> >>>
> >>>      Man pages are provided with example usage of the tools. Use this to view
> >>>      them:
> >>>
> >>>          man -l doc/fdt_check_sign.1
> >>>
> >>>      This series also includes various clean-ups noticed along the way,
> >>>      as well
> >>>      as refactoring to avoid code duplication with the new tools. The
> >>>      last four
> >>>      patches are the new code.
> >>>
> >>>      This series is available at u-boot-dm/fdt-sign-working :
> >>>
> >>>      https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >>>      <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
> >>>
> >>>
> >>>      Simon Glass (16):
> >>>         rsa: Add debugging for failure cases
> >>>         fit_check_sign: Update help to mention the key is in a dtb
> >>>         tools: Move copyfile() into a common file
> >>>         tools: Avoid leaving extra data at the end of copied files
> >>>         tools: Improve comments in signing functions
> >>>         tools: Drop unused name in image-host
> >>>         tools: Avoid confusion between keys and signatures
> >>>         tools: Tidy up argument order in fit_config_check_sig()
> >>>         tools: Pass the key blob around
> >>>         image: Return destination node for add_verify_data() method
> >>>         tools: Pass public-key node through to caller
> >>>         tools: mkimage: Show where signatures/keys are written
> >>>         tools: Add a new tool to sign FDT blobs
> >>>         tools: Add a new tool to check FDT-blob signatures
> >>>         test: Add a test for FDT signing
> >>>         tools: Add man pages for fdt_sign and fdt_check_sign
> >>>
> >>>        MAINTAINERS                      |   7 +
> >>>        boot/image-fit-sig.c             | 151 +++++++++----
> >>>        boot/image-fit.c                 |  12 +-
> >>>        common/spl/spl_fit.c             |   3 +-
> >>>        doc/fdt_check_sign.1             |  74 +++++++
> >>>        doc/fdt_sign.1                   | 111 ++++++++++
> >>>        include/image.h                  |  80 ++++++-
> >>>        include/u-boot/ecdsa.h           |   5 +-
> >>>        include/u-boot/rsa.h             |   5 +-
> >>>        lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> >>>        lib/rsa/rsa-sign.c               |   5 +-
> >>>        lib/rsa/rsa-verify.c             |  13 +-
> >>>        test/py/tests/test_fdt_sign.py   |  83 ++++++++
> >>>        test/py/tests/test_vboot.py      |  21 +-
> >>>        test/py/tests/vboot/sign-fdt.dts |  23 ++
> >>>        test/py/tests/vboot_comm.py      |  22 ++
> >>>        tools/Makefile                   |  10 +-
> >>>        tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
> >>>        tools/fdt_check_sign.c           |  85 ++++++++
> >>>        tools/fdt_host.h                 |  46 ++++
> >>>        tools/fdt_sign.c                 | 210 ++++++++++++++++++
> >>>        tools/fit_check_sign.c           |   4 +-
> >>>        tools/fit_common.c               |  69 ++++++
> >>>        tools/fit_common.h               |  23 ++
> >>>        tools/fit_image.c                |  59 +-----
> >>>        tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> >>>        tools/image-host.c               | 155 +++++++++++---
> >>>        tools/imagetool.h                |   4 +
> >>>        tools/mkimage.c                  |   4 +
> >>>        29 files changed, 1714 insertions(+), 181 deletions(-)
> >>>        create mode 100644 doc/fdt_check_sign.1
> >>>        create mode 100644 doc/fdt_sign.1
> >>>        create mode 100644 test/py/tests/test_fdt_sign.py
> >>>        create mode 100644 test/py/tests/vboot/sign-fdt.dts
> >>>        create mode 100644 test/py/tests/vboot_comm.py
> >>>        create mode 100644 tools/fdt-host.c
> >>>        create mode 100644 tools/fdt_check_sign.c
> >>>        create mode 100644 tools/fdt_sign.c
> >>>        create mode 100644 tools/image-fdt-sig.c
> >>>
> >>>      --
> >>>      2.34.0.rc1.387.gb447b232ab-goog
> >>>
> >>> --
> >>>
> >>> François-Frédéric Ozog | /Director Business Development/
> >>> T: +33.67221.6485
> >>> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
> >>>
> >>>

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[François Ozog]

Hi Simon

Le sam. 13 nov. 2021 à 14:57, Simon Glass <sjg@chromium.org> a écrit :

> Hi François,
> On Sat, 13 Nov 2021 at 03:18, François Ozog <francois.ozog@linaro.org>
> wrote:
> >
> > Hi Simon,
> >
> > Le sam. 13 nov. 2021 à 04:31, Simon Glass <sjg@chromium.org> a écrit :
> >>
> >> Hi Heinrich,
> >>
> >> On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> >> >
> >> >
> >> >
> >> > On 11/12/21 20:49, François Ozog wrote:
> >> > > Hi Simon
> >> > >
> >> > > Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> >> > > <mailto:sjg@chromium.org>> a écrit :
> >> > >
> >> > >     At present mkimage supports signing FITs, the standard U-Boot
> image
> >> > >     type.
> >> > >
> >> > >     Various people are opposed to using FIT since:
> >> > >
> >> > > just to be sure: I am not one of those.
> >> > >
> >> > >
> >> > >     a) it requires adding support for FIT into other bootloaders,
> notably
> >> > >         UEFI
> >> > >
> >> > > whatever happens to FIT is entirely orthogonal to U-Boot UEFI
> subsystem.
> >> > > FIT can evolve,  U-Boot UEFI does not have to change.
> >> >
> >> > We already can create signed FIT images containing a UEFI binary and a
> >> > devcie-tree and launch the FIT image with the bootm command.
> >> >
> >> > Cf.
> >> > CONFIG_BOOTM_EFI
> >> > test/py/tests/test_efi_fit.py
> >> > doc/usage/bootefi.rst
> >> >
> >> > Simon, what are you missing?
> >>
> >> Some people don't want to use FIT.
> >>
> >> >
> >> > >
> >> > >
> >> > >     b) it requires packaging a kernel in this standard U-Boot
> format,
> >> > >     meaning
> >> > >         that distros must run 'mkimage' and deal with the kernel
> and initrd
> >> > >         being inside a FIT
> >> >
> >> > U-Boot tools are contained in distros like Debian and Ubuntu.
> >> > Package flash-kernel uses a script in /etc/initramfs/post-update.d/
> for
> >> > a similar purpose. The same hook directory can be used to create a FIT
> >> > image with a simple bash script.
> >> >
> >> > Why do we need a new tool for signing device-trees?
> >> >
> >> > The real problem to solve is the protection of the private key used
> for
> >> > signing any file containing an initrd.
> >>
> >> Well FIT already solves that one. Either FIT is not being used, in
> >> which case this series is useful, or it is being used, in which case
> >> the initrd problem is solved.
> >>
> >> >
> >> > >
> >> > >     The kernel and initrd can be dealt with in other ways. But
> without FIT,
> >> >
> >> > How can the initrd be checked without FIT?
> >>
> >> I don't know. Please check with the EFI people.
> >
> > Ilias has made a proposal to “measure” files that do not have signatures
> until files include signature.
>
> OK
>
> >>
> >>
> >> >
> >> > >     we have no standard way of signing and grouping FDT files.
> Instead
> >> > >     we must
> >> > >     include them in the distro as separate files.
> >> > >
> >> > >     In particular, some sort of mechanism for verifying FDT files
> is needed.
> >> > >     One option would be to tack a signature on before or after the
> file,
> >> > >     processing it accordingly. But due to the nature of the FDT
> binary
> >> > >     format,
> >> > >     it is possible to embed a signature inside the FDT itself,
> which is very
> >> > >     convenient.
> >> > >
> >> > >     This series provides a tool, fdt_sign, which can add a
> signature to an
> >> > >     FDT. The signature can be checked later, preventing any change
> to
> >> > >     the FDT,
> >> > >     other than in permitted nodes (e.g. /chosen).
> >> >
> >
> > Shouldn’t devicetree.org and kernel.org folks involved in this DT
> signing effort ? I believe there are parrallel efforts in this area. Or is
> it a private <u-boot FDT sign>? in that case, they do not need to be
> involved. Depending on the case, you may want to split the patch series in
> a number of chunks.
>
> Yes indeed these people should be involved, but I also think that
> interested parties should be part of the boot-architecture effort. The
> meeting notes where this tool was discussed are here:
>
>
> https://lists.linaro.org/pipermail/boot-architecture/2021-October/001955.html
>
> My slides are available there, too.
>
> Here is what I have done so far: I sent a v4 series to
> devicetree-compiler a week ago with the fdt_region and fdtgrep parts.
> Rob and Bill are copied on this series, as you can see. I was not sure
> about sending U-Boot patches to the boot-architecture list, but I will
> cover it in a future meeting.
>
> But if there are others that should be copied, please feel free to add
> them, or point me to them.
>
Sorry. I missed those. Now the reference is made that’s good.

>
> Regards,
> Simon
>
>
> >>
> >>
> >> > I am not aware of any standard defining which nodes may be changed in
> >> > the FDT fixup and which may not be changed.
> >> >
> >> > How can we discover which nodes were excluded from the signature after
> >> > the signature?
> >>
> >> There is no way at present. I decided against adding a list of signed
> >> nodes. We can of course add whatever is useful here.
> >>
> >> >
> >> > >
> >> > >     This series also provides a fdt_check_sign tool, used to check
> >> > >     signatures.
> >> > >
> >> > >     Both of these tools are stand-alone do not require mkimage or
> FIT.
> >> > >
> >> > >     As with FIT signing, multiple signatures are possible, but in
> this case
> >> > >     that requires that fit_sign be called once for each signature.
> To
> >> > >     make the
> >> > >     check fail if a signature does not match, it should be marked as
> >> > >     'required' using the -r flag to fdt_sign.
> >> > >
> >> > >     Run-time support for checking FDT signatures could be added to
> U-Boot
> >> > >     fairly easily, but needs further discussion as the correct
> plumbing
> >> > >     needs
> >> > >     to be determined.
> >> > >
> >> > >     For now there is absolutely no configurability in the signature
> >> > >     mechanism.
> >> >
> >> > I would have expected a description of what a signature looks like. I
> >> > don't wont to reverse engineer your patches.
> >> >
> >> > Please, describe this in doc/develop/ and in this cover-letter.
> >>
> >> It is the same format as the FIT signature, an RSA signature. See here:
> >>
> >>
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
> >>
> >> >
> >> > This series should have been sent as RFC.
> >>
> >> The last time I did that it disappeared without trace. You can of
> >> course make comments on any series I send.
> >>
> >> Regards,
> >> Simon
> >>
> >> >
> >> > Best regards
> >> >
> >> > Heinrich
> >> >
> >> > >     It would of course be possible to adjust which nodes are
> signed, as is
> >> > >     done for FIT, but that needs further discussion also. The
> omission
> >> > >     of the
> >> > >     /chosen node is implemented in h_exclude_nodes() like this:
> >> > >
> >> > >         if (type == FDT_IS_NODE) {
> >> > >            /* Ignore the chosen node as well as /signature and
> subnodes */
> >> > >            if (!strcmp("/chosen", data) || !strncmp("/signature",
> data, 10))
> >> > >               return 0;
> >> > >         }
> >> > >
> >> > >     Man pages are provided with example usage of the tools. Use
> this to view
> >> > >     them:
> >> > >
> >> > >         man -l doc/fdt_check_sign.1
> >> > >
> >> > >     This series also includes various clean-ups noticed along the
> way,
> >> > >     as well
> >> > >     as refactoring to avoid code duplication with the new tools. The
> >> > >     last four
> >> > >     patches are the new code.
> >> > >
> >> > >     This series is available at u-boot-dm/fdt-sign-working :
> >> > >
> >> > >
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >> > >     <
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >
> >> > >
> >> > >
> >> > >     Simon Glass (16):
> >> > >        rsa: Add debugging for failure cases
> >> > >        fit_check_sign: Update help to mention the key is in a dtb
> >> > >        tools: Move copyfile() into a common file
> >> > >        tools: Avoid leaving extra data at the end of copied files
> >> > >        tools: Improve comments in signing functions
> >> > >        tools: Drop unused name in image-host
> >> > >        tools: Avoid confusion between keys and signatures
> >> > >        tools: Tidy up argument order in fit_config_check_sig()
> >> > >        tools: Pass the key blob around
> >> > >        image: Return destination node for add_verify_data() method
> >> > >        tools: Pass public-key node through to caller
> >> > >        tools: mkimage: Show where signatures/keys are written
> >> > >        tools: Add a new tool to sign FDT blobs
> >> > >        tools: Add a new tool to check FDT-blob signatures
> >> > >        test: Add a test for FDT signing
> >> > >        tools: Add man pages for fdt_sign and fdt_check_sign
> >> > >
> >> > >       MAINTAINERS                      |   7 +
> >> > >       boot/image-fit-sig.c             | 151 +++++++++----
> >> > >       boot/image-fit.c                 |  12 +-
> >> > >       common/spl/spl_fit.c             |   3 +-
> >> > >       doc/fdt_check_sign.1             |  74 +++++++
> >> > >       doc/fdt_sign.1                   | 111 ++++++++++
> >> > >       include/image.h                  |  80 ++++++-
> >> > >       include/u-boot/ecdsa.h           |   5 +-
> >> > >       include/u-boot/rsa.h             |   5 +-
> >> > >       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> >> > >       lib/rsa/rsa-sign.c               |   5 +-
> >> > >       lib/rsa/rsa-verify.c             |  13 +-
> >> > >       test/py/tests/test_fdt_sign.py   |  83 ++++++++
> >> > >       test/py/tests/test_vboot.py      |  21 +-
> >> > >       test/py/tests/vboot/sign-fdt.dts |  23 ++
> >> > >       test/py/tests/vboot_comm.py      |  22 ++
> >> > >       tools/Makefile                   |  10 +-
> >> > >       tools/fdt-host.c                 | 353
> +++++++++++++++++++++++++++++++
> >> > >       tools/fdt_check_sign.c           |  85 ++++++++
> >> > >       tools/fdt_host.h                 |  46 ++++
> >> > >       tools/fdt_sign.c                 | 210 ++++++++++++++++++
> >> > >       tools/fit_check_sign.c           |   4 +-
> >> > >       tools/fit_common.c               |  69 ++++++
> >> > >       tools/fit_common.h               |  23 ++
> >> > >       tools/fit_image.c                |  59 +-----
> >> > >       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> >> > >       tools/image-host.c               | 155 +++++++++++---
> >> > >       tools/imagetool.h                |   4 +
> >> > >       tools/mkimage.c                  |   4 +
> >> > >       29 files changed, 1714 insertions(+), 181 deletions(-)
> >> > >       create mode 100644 doc/fdt_check_sign.1
> >> > >       create mode 100644 doc/fdt_sign.1
> >> > >       create mode 100644 test/py/tests/test_fdt_sign.py
> >> > >       create mode 100644 test/py/tests/vboot/sign-fdt.dts
> >> > >       create mode 100644 test/py/tests/vboot_comm.py
> >> > >       create mode 100644 tools/fdt-host.c
> >> > >       create mode 100644 tools/fdt_check_sign.c
> >> > >       create mode 100644 tools/fdt_sign.c
> >> > >       create mode 100644 tools/image-fdt-sig.c
> >> > >
> >> > >     --
> >> > >     2.34.0.rc1.387.gb447b232ab-goog
> >> > >
> >> > > --
> >> > >
> >> > > François-Frédéric Ozog | /Director Business Development/
> >> > > T: +33.67221.6485
> >> > > francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> |
> Skype: ffozog
> >> > >
> >> > >
> >
> > --
> > François-Frédéric Ozog | Director Business Development
> > T: +33.67221.6485
> > francois.ozog@linaro.org | Skype: ffozog
> >
>
-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[François Ozog]

Hi Simon

Le sam. 13 nov. 2021 à 14:57, Simon Glass <sjg@chromium.org> a écrit :

> Hi Heinrich,
>
> On Sat, 13 Nov 2021 at 04:57, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> >
> >
> >
> > On 11/13/21 04:30, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> > >>
> > >>
> > >>
> > >> On 11/12/21 20:49, François Ozog wrote:
> > >>> Hi Simon
> > >>>
> > >>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> > >>> <mailto:sjg@chromium.org>> a écrit :
> > >>>
> > >>>      At present mkimage supports signing FITs, the standard U-Boot
> image
> > >>>      type.
> > >>>
> > >>>      Various people are opposed to using FIT since:
> > >>>
> > >>> just to be sure: I am not one of those.
> > >>>
> > >>>
> > >>>      a) it requires adding support for FIT into other bootloaders,
> notably
> > >>>          UEFI
> > >>>
> > >>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI
> subsystem.
> > >>> FIT can evolve,  U-Boot UEFI does not have to change.
> > >>
> > >> We already can create signed FIT images containing a UEFI binary and a
> > >> devcie-tree and launch the FIT image with the bootm command.
> > >>
> > >> Cf.
> > >> CONFIG_BOOTM_EFI
> > >> test/py/tests/test_efi_fit.py
> > >> doc/usage/bootefi.rst
> > >>
> > >> Simon, what are you missing?
> > >
> > > Some people don't want to use FIT.
> >
> > The problem with FIT is that other firmware like EDK II does not support
> it.
> >
> > Why do you expect those people to like your new tool better?
>
> I believe the EDK decision is not so much that it *does* not support
> FIT, which is after all not a lot of code, but that it *should* not.
> If I have that wrong, please let me know.
>
UEFI (EDK2 and now U-Boot) design choice is that it does not include *any*
technologies for OS: UEFI launches an OS loader that know about those.
A possible way to introduce a FIT is by adding a protocol that can register
a vendor media file , assign a UUID to a FIT. Add support for this in the
UEFI stub of the kernel.
Another way would be a minimalistic UEFI app that will fetch the second
parameter as a FIT. The FIT format parsing code is only in this
minimalistic loader. U-Boot can play this role based on your efforts to
have U-Boot as UEFI application.
You could also promote FIT in the systemd-boot UeFi application. I believe
this is a very good option BTW.

>
> The goal here is to support signing in an FDT without FIT. I believe
> EDK supports FDT, at least.
>
> >
> > >
> > >>
> > >>>
> > >>>
> > >>>      b) it requires packaging a kernel in this standard U-Boot
> format,
> > >>>      meaning
> > >>>          that distros must run 'mkimage' and deal with the kernel
> and initrd
> > >>>          being inside a FIT
> > >>
> > >> U-Boot tools are contained in distros like Debian and Ubuntu.
> > >> Package flash-kernel uses a script in /etc/initramfs/post-update.d/
> for
> > >> a similar purpose. The same hook directory can be used to create a FIT
> > >> image with a simple bash script.
> > >>
> > >> Why do we need a new tool for signing device-trees?
> > >>
> > >> The real problem to solve is the protection of the private key used
> for
> > >> signing any file containing an initrd.
> > >
> > > Well FIT already solves that one. Either FIT is not being used, in
> > > which case this series is useful, or it is being used, in which case
> > > the initrd problem is solved.
> >
> > The question was:
> >
> > How do you protect the private key used by Linux to sign the FIT image
> > with the updated initrd generated by intramfs-tools?
>
> Well, if it is a FIT we can add a seperate signature at the time the
> initramfs-tools runs. It does support multiple signatures. If that is
> not suitable I am sure something else can be devised. What are the
> constraints / requirements?
>
> Regards,
> Simon
>
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > >>
> > >>>
> > >>>      The kernel and initrd can be dealt with in other ways. But
> without FIT,
> > >>
> > >> How can the initrd be checked without FIT?
> > >
> > > I don't know. Please check with the EFI people.
> > >
> > >>
> > >>>      we have no standard way of signing and grouping FDT files.
> Instead
> > >>>      we must
> > >>>      include them in the distro as separate files.
> > >>>
> > >>>      In particular, some sort of mechanism for verifying FDT files
> is needed.
> > >>>      One option would be to tack a signature on before or after the
> file,
> > >>>      processing it accordingly. But due to the nature of the FDT
> binary
> > >>>      format,
> > >>>      it is possible to embed a signature inside the FDT itself,
> which is very
> > >>>      convenient.
> > >>>
> > >>>      This series provides a tool, fdt_sign, which can add a
> signature to an
> > >>>      FDT. The signature can be checked later, preventing any change
> to
> > >>>      the FDT,
> > >>>      other than in permitted nodes (e.g. /chosen).
> > >>
> > >> I am not aware of any standard defining which nodes may be changed in
> > >> the FDT fixup and which may not be changed.
> > >>
> > >> How can we discover which nodes were excluded from the signature after
> > >> the signature?
> > >
> > > There is no way at present. I decided against adding a list of signed
> > > nodes. We can of course add whatever is useful here.
> > >
> > >>
> > >>>
> > >>>      This series also provides a fdt_check_sign tool, used to check
> > >>>      signatures.
> > >>>
> > >>>      Both of these tools are stand-alone do not require mkimage or
> FIT.
> > >>>
> > >>>      As with FIT signing, multiple signatures are possible, but in
> this case
> > >>>      that requires that fit_sign be called once for each signature.
> To
> > >>>      make the
> > >>>      check fail if a signature does not match, it should be marked as
> > >>>      'required' using the -r flag to fdt_sign.
> > >>>
> > >>>      Run-time support for checking FDT signatures could be added to
> U-Boot
> > >>>      fairly easily, but needs further discussion as the correct
> plumbing
> > >>>      needs
> > >>>      to be determined.
> > >>>
> > >>>      For now there is absolutely no configurability in the signature
> > >>>      mechanism.
> > >>
> > >> I would have expected a description of what a signature looks like. I
> > >> don't wont to reverse engineer your patches.
> > >>
> > >> Please, describe this in doc/develop/ and in this cover-letter.
> > >
> > > It is the same format as the FIT signature, an RSA signature. See here:
> > >
> > >
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
> > >
> > >>
> > >> This series should have been sent as RFC.
> > >
> > > The last time I did that it disappeared without trace. You can of
> > > course make comments on any series I send.
> > >
> > > Regards,
> > > Simon
> > >
> > >>
> > >> Best regards
> > >>
> > >> Heinrich
> > >>
> > >>>      It would of course be possible to adjust which nodes are
> signed, as is
> > >>>      done for FIT, but that needs further discussion also. The
> omission
> > >>>      of the
> > >>>      /chosen node is implemented in h_exclude_nodes() like this:
> > >>>
> > >>>          if (type == FDT_IS_NODE) {
> > >>>             /* Ignore the chosen node as well as /signature and
> subnodes */
> > >>>             if (!strcmp("/chosen", data) || !strncmp("/signature",
> data, 10))
> > >>>                return 0;
> > >>>          }
> > >>>
> > >>>      Man pages are provided with example usage of the tools. Use
> this to view
> > >>>      them:
> > >>>
> > >>>          man -l doc/fdt_check_sign.1
> > >>>
> > >>>      This series also includes various clean-ups noticed along the
> way,
> > >>>      as well
> > >>>      as refactoring to avoid code duplication with the new tools. The
> > >>>      last four
> > >>>      patches are the new code.
> > >>>
> > >>>      This series is available at u-boot-dm/fdt-sign-working :
> > >>>
> > >>>
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> > >>>      <
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >
> > >>>
> > >>>
> > >>>      Simon Glass (16):
> > >>>         rsa: Add debugging for failure cases
> > >>>         fit_check_sign: Update help to mention the key is in a dtb
> > >>>         tools: Move copyfile() into a common file
> > >>>         tools: Avoid leaving extra data at the end of copied files
> > >>>         tools: Improve comments in signing functions
> > >>>         tools: Drop unused name in image-host
> > >>>         tools: Avoid confusion between keys and signatures
> > >>>         tools: Tidy up argument order in fit_config_check_sig()
> > >>>         tools: Pass the key blob around
> > >>>         image: Return destination node for add_verify_data() method
> > >>>         tools: Pass public-key node through to caller
> > >>>         tools: mkimage: Show where signatures/keys are written
> > >>>         tools: Add a new tool to sign FDT blobs
> > >>>         tools: Add a new tool to check FDT-blob signatures
> > >>>         test: Add a test for FDT signing
> > >>>         tools: Add man pages for fdt_sign and fdt_check_sign
> > >>>
> > >>>        MAINTAINERS                      |   7 +
> > >>>        boot/image-fit-sig.c             | 151 +++++++++----
> > >>>        boot/image-fit.c                 |  12 +-
> > >>>        common/spl/spl_fit.c             |   3 +-
> > >>>        doc/fdt_check_sign.1             |  74 +++++++
> > >>>        doc/fdt_sign.1                   | 111 ++++++++++
> > >>>        include/image.h                  |  80 ++++++-
> > >>>        include/u-boot/ecdsa.h           |   5 +-
> > >>>        include/u-boot/rsa.h             |   5 +-
> > >>>        lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> > >>>        lib/rsa/rsa-sign.c               |   5 +-
> > >>>        lib/rsa/rsa-verify.c             |  13 +-
> > >>>        test/py/tests/test_fdt_sign.py   |  83 ++++++++
> > >>>        test/py/tests/test_vboot.py      |  21 +-
> > >>>        test/py/tests/vboot/sign-fdt.dts |  23 ++
> > >>>        test/py/tests/vboot_comm.py      |  22 ++
> > >>>        tools/Makefile                   |  10 +-
> > >>>        tools/fdt-host.c                 | 353
> +++++++++++++++++++++++++++++++
> > >>>        tools/fdt_check_sign.c           |  85 ++++++++
> > >>>        tools/fdt_host.h                 |  46 ++++
> > >>>        tools/fdt_sign.c                 | 210 ++++++++++++++++++
> > >>>        tools/fit_check_sign.c           |   4 +-
> > >>>        tools/fit_common.c               |  69 ++++++
> > >>>        tools/fit_common.h               |  23 ++
> > >>>        tools/fit_image.c                |  59 +-----
> > >>>        tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> > >>>        tools/image-host.c               | 155 +++++++++++---
> > >>>        tools/imagetool.h                |   4 +
> > >>>        tools/mkimage.c                  |   4 +
> > >>>        29 files changed, 1714 insertions(+), 181 deletions(-)
> > >>>        create mode 100644 doc/fdt_check_sign.1
> > >>>        create mode 100644 doc/fdt_sign.1
> > >>>        create mode 100644 test/py/tests/test_fdt_sign.py
> > >>>        create mode 100644 test/py/tests/vboot/sign-fdt.dts
> > >>>        create mode 100644 test/py/tests/vboot_comm.py
> > >>>        create mode 100644 tools/fdt-host.c
> > >>>        create mode 100644 tools/fdt_check_sign.c
> > >>>        create mode 100644 tools/fdt_sign.c
> > >>>        create mode 100644 tools/image-fdt-sig.c
> > >>>
> > >>>      --
> > >>>      2.34.0.rc1.387.gb447b232ab-goog
> > >>>
> > >>> --
> > >>>
> > >>> François-Frédéric Ozog | /Director Business Development/
> > >>> T: +33.67221.6485
> > >>> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype:
> ffozog
> > >>>
> > >>>
>
-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Simon Glass]

Hi François,

On Sat, 13 Nov 2021 at 11:53, François Ozog <francois.ozog@linaro.org> wrote:
>
> Hi Simon
>
> Le sam. 13 nov. 2021 à 14:57, Simon Glass <sjg@chromium.org> a écrit :
>>
>> Hi Heinrich,
>>
>> On Sat, 13 Nov 2021 at 04:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> >
>> >
>> >
>> > On 11/13/21 04:30, Simon Glass wrote:
>> > > Hi Heinrich,
>> > >
>> > > On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> > >>
>> > >>
>> > >>
>> > >> On 11/12/21 20:49, François Ozog wrote:
>> > >>> Hi Simon
>> > >>>
>> > >>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
>> > >>> <mailto:sjg@chromium.org>> a écrit :
>> > >>>
>> > >>>      At present mkimage supports signing FITs, the standard U-Boot image
>> > >>>      type.
>> > >>>
>> > >>>      Various people are opposed to using FIT since:
>> > >>>
>> > >>> just to be sure: I am not one of those.
>> > >>>
>> > >>>
>> > >>>      a) it requires adding support for FIT into other bootloaders, notably
>> > >>>          UEFI
>> > >>>
>> > >>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
>> > >>> FIT can evolve,  U-Boot UEFI does not have to change.
>> > >>
>> > >> We already can create signed FIT images containing a UEFI binary and a
>> > >> devcie-tree and launch the FIT image with the bootm command.
>> > >>
>> > >> Cf.
>> > >> CONFIG_BOOTM_EFI
>> > >> test/py/tests/test_efi_fit.py
>> > >> doc/usage/bootefi.rst
>> > >>
>> > >> Simon, what are you missing?
>> > >
>> > > Some people don't want to use FIT.
>> >
>> > The problem with FIT is that other firmware like EDK II does not support it.
>> >
>> > Why do you expect those people to like your new tool better?
>>
>> I believe the EDK decision is not so much that it *does* not support
>> FIT, which is after all not a lot of code, but that it *should* not.
>> If I have that wrong, please let me know.
>
> UEFI (EDK2 and now U-Boot) design choice is that it does not include *any* technologies for OS: UEFI launches an OS loader that know about those.
> A possible way to introduce a FIT is by adding a protocol that can register a vendor media file , assign a UUID to a FIT. Add support for this in the UEFI stub of the kernel.
> Another way would be a minimalistic UEFI app that will fetch the second parameter as a FIT. The FIT format parsing code is only in this minimalistic loader. U-Boot can play this role based on your efforts to have U-Boot as UEFI application.
> You could also promote FIT in the systemd-boot UeFi application. I believe this is a very good option BTW.

I think UEFI is making things very complicated. If we define a
standard boot flow we should not need grub.

U-Boot already supports FIT so we don't have a problem there.

Since FIT is a container format, and UEFI supports other container
formats (capsule update for example), there is no obvious
philosophical reason why UEFI should not support FIT.

Anyway, at least you can understand why I sent this patch.

Regards,
SImon


>>
>>
>> The goal here is to support signing in an FDT without FIT. I believe
>> EDK supports FDT, at least.
>>
>> >
>> > >
>> > >>
>> > >>>
>> > >>>
>> > >>>      b) it requires packaging a kernel in this standard U-Boot format,
>> > >>>      meaning
>> > >>>          that distros must run 'mkimage' and deal with the kernel and initrd
>> > >>>          being inside a FIT
>> > >>
>> > >> U-Boot tools are contained in distros like Debian and Ubuntu.
>> > >> Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
>> > >> a similar purpose. The same hook directory can be used to create a FIT
>> > >> image with a simple bash script.
>> > >>
>> > >> Why do we need a new tool for signing device-trees?
>> > >>
>> > >> The real problem to solve is the protection of the private key used for
>> > >> signing any file containing an initrd.
>> > >
>> > > Well FIT already solves that one. Either FIT is not being used, in
>> > > which case this series is useful, or it is being used, in which case
>> > > the initrd problem is solved.
>> >
>> > The question was:
>> >
>> > How do you protect the private key used by Linux to sign the FIT image
>> > with the updated initrd generated by intramfs-tools?
>>
>> Well, if it is a FIT we can add a seperate signature at the time the
>> initramfs-tools runs. It does support multiple signatures. If that is
>> not suitable I am sure something else can be devised. What are the
>> constraints / requirements?
>>
>> Regards,
>> Simon
>>
>> >
>> > Best regards
>> >
>> > Heinrich
>> >
>> > >
>> > >>
>> > >>>
>> > >>>      The kernel and initrd can be dealt with in other ways. But without FIT,
>> > >>
>> > >> How can the initrd be checked without FIT?
>> > >
>> > > I don't know. Please check with the EFI people.
>> > >
>> > >>
>> > >>>      we have no standard way of signing and grouping FDT files. Instead
>> > >>>      we must
>> > >>>      include them in the distro as separate files.
>> > >>>
>> > >>>      In particular, some sort of mechanism for verifying FDT files is needed.
>> > >>>      One option would be to tack a signature on before or after the file,
>> > >>>      processing it accordingly. But due to the nature of the FDT binary
>> > >>>      format,
>> > >>>      it is possible to embed a signature inside the FDT itself, which is very
>> > >>>      convenient.
>> > >>>
>> > >>>      This series provides a tool, fdt_sign, which can add a signature to an
>> > >>>      FDT. The signature can be checked later, preventing any change to
>> > >>>      the FDT,
>> > >>>      other than in permitted nodes (e.g. /chosen).
>> > >>
>> > >> I am not aware of any standard defining which nodes may be changed in
>> > >> the FDT fixup and which may not be changed.
>> > >>
>> > >> How can we discover which nodes were excluded from the signature after
>> > >> the signature?
>> > >
>> > > There is no way at present. I decided against adding a list of signed
>> > > nodes. We can of course add whatever is useful here.
>> > >
>> > >>
>> > >>>
>> > >>>      This series also provides a fdt_check_sign tool, used to check
>> > >>>      signatures.
>> > >>>
>> > >>>      Both of these tools are stand-alone do not require mkimage or FIT.
>> > >>>
>> > >>>      As with FIT signing, multiple signatures are possible, but in this case
>> > >>>      that requires that fit_sign be called once for each signature. To
>> > >>>      make the
>> > >>>      check fail if a signature does not match, it should be marked as
>> > >>>      'required' using the -r flag to fdt_sign.
>> > >>>
>> > >>>      Run-time support for checking FDT signatures could be added to U-Boot
>> > >>>      fairly easily, but needs further discussion as the correct plumbing
>> > >>>      needs
>> > >>>      to be determined.
>> > >>>
>> > >>>      For now there is absolutely no configurability in the signature
>> > >>>      mechanism.
>> > >>
>> > >> I would have expected a description of what a signature looks like. I
>> > >> don't wont to reverse engineer your patches.
>> > >>
>> > >> Please, describe this in doc/develop/ and in this cover-letter.
>> > >
>> > > It is the same format as the FIT signature, an RSA signature. See here:
>> > >
>> > > https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
>> > >
>> > >>
>> > >> This series should have been sent as RFC.
>> > >
>> > > The last time I did that it disappeared without trace. You can of
>> > > course make comments on any series I send.
>> > >
>> > > Regards,
>> > > Simon
>> > >
>> > >>
>> > >> Best regards
>> > >>
>> > >> Heinrich
>> > >>
>> > >>>      It would of course be possible to adjust which nodes are signed, as is
>> > >>>      done for FIT, but that needs further discussion also. The omission
>> > >>>      of the
>> > >>>      /chosen node is implemented in h_exclude_nodes() like this:
>> > >>>
>> > >>>          if (type == FDT_IS_NODE) {
>> > >>>             /* Ignore the chosen node as well as /signature and subnodes */
>> > >>>             if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>> > >>>                return 0;
>> > >>>          }
>> > >>>
>> > >>>      Man pages are provided with example usage of the tools. Use this to view
>> > >>>      them:
>> > >>>
>> > >>>          man -l doc/fdt_check_sign.1
>> > >>>
>> > >>>      This series also includes various clean-ups noticed along the way,
>> > >>>      as well
>> > >>>      as refactoring to avoid code duplication with the new tools. The
>> > >>>      last four
>> > >>>      patches are the new code.
>> > >>>
>> > >>>      This series is available at u-boot-dm/fdt-sign-working :
>> > >>>
>> > >>>      https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>> > >>>      <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
>> > >>>
>> > >>>
>> > >>>      Simon Glass (16):
>> > >>>         rsa: Add debugging for failure cases
>> > >>>         fit_check_sign: Update help to mention the key is in a dtb
>> > >>>         tools: Move copyfile() into a common file
>> > >>>         tools: Avoid leaving extra data at the end of copied files
>> > >>>         tools: Improve comments in signing functions
>> > >>>         tools: Drop unused name in image-host
>> > >>>         tools: Avoid confusion between keys and signatures
>> > >>>         tools: Tidy up argument order in fit_config_check_sig()
>> > >>>         tools: Pass the key blob around
>> > >>>         image: Return destination node for add_verify_data() method
>> > >>>         tools: Pass public-key node through to caller
>> > >>>         tools: mkimage: Show where signatures/keys are written
>> > >>>         tools: Add a new tool to sign FDT blobs
>> > >>>         tools: Add a new tool to check FDT-blob signatures
>> > >>>         test: Add a test for FDT signing
>> > >>>         tools: Add man pages for fdt_sign and fdt_check_sign
>> > >>>
>> > >>>        MAINTAINERS                      |   7 +
>> > >>>        boot/image-fit-sig.c             | 151 +++++++++----
>> > >>>        boot/image-fit.c                 |  12 +-
>> > >>>        common/spl/spl_fit.c             |   3 +-
>> > >>>        doc/fdt_check_sign.1             |  74 +++++++
>> > >>>        doc/fdt_sign.1                   | 111 ++++++++++
>> > >>>        include/image.h                  |  80 ++++++-
>> > >>>        include/u-boot/ecdsa.h           |   5 +-
>> > >>>        include/u-boot/rsa.h             |   5 +-
>> > >>>        lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>> > >>>        lib/rsa/rsa-sign.c               |   5 +-
>> > >>>        lib/rsa/rsa-verify.c             |  13 +-
>> > >>>        test/py/tests/test_fdt_sign.py   |  83 ++++++++
>> > >>>        test/py/tests/test_vboot.py      |  21 +-
>> > >>>        test/py/tests/vboot/sign-fdt.dts |  23 ++
>> > >>>        test/py/tests/vboot_comm.py      |  22 ++
>> > >>>        tools/Makefile                   |  10 +-
>> > >>>        tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>> > >>>        tools/fdt_check_sign.c           |  85 ++++++++
>> > >>>        tools/fdt_host.h                 |  46 ++++
>> > >>>        tools/fdt_sign.c                 | 210 ++++++++++++++++++
>> > >>>        tools/fit_check_sign.c           |   4 +-
>> > >>>        tools/fit_common.c               |  69 ++++++
>> > >>>        tools/fit_common.h               |  23 ++
>> > >>>        tools/fit_image.c                |  59 +-----
>> > >>>        tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>> > >>>        tools/image-host.c               | 155 +++++++++++---
>> > >>>        tools/imagetool.h                |   4 +
>> > >>>        tools/mkimage.c                  |   4 +
>> > >>>        29 files changed, 1714 insertions(+), 181 deletions(-)
>> > >>>        create mode 100644 doc/fdt_check_sign.1
>> > >>>        create mode 100644 doc/fdt_sign.1
>> > >>>        create mode 100644 test/py/tests/test_fdt_sign.py
>> > >>>        create mode 100644 test/py/tests/vboot/sign-fdt.dts
>> > >>>        create mode 100644 test/py/tests/vboot_comm.py
>> > >>>        create mode 100644 tools/fdt-host.c
>> > >>>        create mode 100644 tools/fdt_check_sign.c
>> > >>>        create mode 100644 tools/fdt_sign.c
>> > >>>        create mode 100644 tools/image-fdt-sig.c
>> > >>>
>> > >>>      --
>> > >>>      2.34.0.rc1.387.gb447b232ab-goog
>> > >>>
>> > >>> --
>> > >>>
>> > >>> François-Frédéric Ozog | /Director Business Development/
>> > >>> T: +33.67221.6485
>> > >>> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
>> > >>>
>> > >>>
>
> --
> François-Frédéric Ozog | Director Business Development
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3207 bytes --]

On Wed, Nov 24, 2021 at 05:12:12PM -0700, Simon Glass wrote:
> Hi François,
> 
> On Sat, 13 Nov 2021 at 11:53, François Ozog <francois.ozog@linaro.org> wrote:
> >
> > Hi Simon
> >
> > Le sam. 13 nov. 2021 à 14:57, Simon Glass <sjg@chromium.org> a écrit :
> >>
> >> Hi Heinrich,
> >>
> >> On Sat, 13 Nov 2021 at 04:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >> >
> >> >
> >> >
> >> > On 11/13/21 04:30, Simon Glass wrote:
> >> > > Hi Heinrich,
> >> > >
> >> > > On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >> > >>
> >> > >>
> >> > >>
> >> > >> On 11/12/21 20:49, François Ozog wrote:
> >> > >>> Hi Simon
> >> > >>>
> >> > >>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> >> > >>> <mailto:sjg@chromium.org>> a écrit :
> >> > >>>
> >> > >>>      At present mkimage supports signing FITs, the standard U-Boot image
> >> > >>>      type.
> >> > >>>
> >> > >>>      Various people are opposed to using FIT since:
> >> > >>>
> >> > >>> just to be sure: I am not one of those.
> >> > >>>
> >> > >>>
> >> > >>>      a) it requires adding support for FIT into other bootloaders, notably
> >> > >>>          UEFI
> >> > >>>
> >> > >>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
> >> > >>> FIT can evolve,  U-Boot UEFI does not have to change.
> >> > >>
> >> > >> We already can create signed FIT images containing a UEFI binary and a
> >> > >> devcie-tree and launch the FIT image with the bootm command.
> >> > >>
> >> > >> Cf.
> >> > >> CONFIG_BOOTM_EFI
> >> > >> test/py/tests/test_efi_fit.py
> >> > >> doc/usage/bootefi.rst
> >> > >>
> >> > >> Simon, what are you missing?
> >> > >
> >> > > Some people don't want to use FIT.
> >> >
> >> > The problem with FIT is that other firmware like EDK II does not support it.
> >> >
> >> > Why do you expect those people to like your new tool better?
> >>
> >> I believe the EDK decision is not so much that it *does* not support
> >> FIT, which is after all not a lot of code, but that it *should* not.
> >> If I have that wrong, please let me know.
> >
> > UEFI (EDK2 and now U-Boot) design choice is that it does not include *any* technologies for OS: UEFI launches an OS loader that know about those.
> > A possible way to introduce a FIT is by adding a protocol that can register a vendor media file , assign a UUID to a FIT. Add support for this in the UEFI stub of the kernel.
> > Another way would be a minimalistic UEFI app that will fetch the second parameter as a FIT. The FIT format parsing code is only in this minimalistic loader. U-Boot can play this role based on your efforts to have U-Boot as UEFI application.
> > You could also promote FIT in the systemd-boot UeFi application. I believe this is a very good option BTW.
> 
> I think UEFI is making things very complicated. If we define a
> standard boot flow we should not need grub.

To be clear, UEFI doesn't require grub (nor systemd-boot) but off the
shelf distributions tend to really really like that, rather than
modifying UEFI variables more directly to implement some sort of boot
menu and other related features.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Rasmus Villemoes]

On 12/11/2021 20.28, Simon Glass wrote:
> At present mkimage supports signing FITs, the standard U-Boot image type.
> 
> Various people are opposed to using FIT since:
> 
> a) it requires adding support for FIT into other bootloaders, notably
>    UEFI
> b) it requires packaging a kernel in this standard U-Boot format, meaning
>    that distros must run 'mkimage' and deal with the kernel and initrd
>    being inside a FIT
> 
> The kernel and initrd can be dealt with in other ways. But without FIT,
> we have no standard way of signing and grouping FDT files. Instead we must
> include them in the distro as separate files.
> 
> In particular, some sort of mechanism for verifying FDT files is needed.
> One option would be to tack a signature on before or after the file,
> processing it accordingly. But due to the nature of the FDT binary format,
> it is possible to embed a signature inside the FDT itself, which is very
> convenient.
> 
> This series provides a tool, fdt_sign, which can add a signature to an
> FDT. The signature can be checked later, preventing any change to the FDT,
> other than in permitted nodes (e.g. /chosen).
> 
> This series also provides a fdt_check_sign tool, used to check signatures.

This could be useful. Not because I'm interested in signing device-tree
blobs as such, but as a replacement for the current incomprehensible way
FIT images are signed and verified. This seems to be a much better and
simpler scheme, that avoids (can avoid) the overly simplistic approach
of signing just image nodes, and the way too complex
signing-configurations-and-then-add-a-property-saying-which-nodes-I-signed-and-sign-that-as-well.

In order not to hard-code anything in the tool about /chosen or whatnot,
make the rule be that the blob (I'll use that word, because it can as
well be a FIT image containing kernel image+initramfs etc., or a FIT
containing a U-Boot script) has a property in the top node

  unsigned-nodes = "/chosen", "/signatures";

(maybe it could be

  unsigned-nodes = <&chosen &sigs>

but that may make the implementation a little more complex). That
property itself is by definition part of what gets signed. The verifier
can and should choose to reject the whole thing if "/" is mentioned.

Then, please, for debugging, inside the /signatures node, beside the
signature data, also add a copy of the offset/len pairs that were
actually hashed, and that hash value.

  /signatures {
    hashed-regions = <offset0 len0 offset1 len1 ...>;
    hash-1 {
      value = 0xabcd;
      algo = "sha256";
    };
    signature-1 {
       ...
    };
    ...

And make the tool start by adding dummy properties, so those strings
"hashed-regions", "value", "algo", "signer" and whatever other property
names are used already exist in the string table. After that, fill in
the actual values - and have the rule that the entire string table is
hashed, not just some initial part of it.

The verifier will of course not rely on /signatures/hashed-regions, but
will compute the regions itself based on /unsigned-nodes (and implicitly
add a region consisting of the string table and the memreserve table).

Then we'd have a generic scheme for signing blobs, disentangled from
whether they are actually describing hardware or used as a generic
container format. And instead of "image" or "conf", we could mark a
public key as being required for "whole-blob" (bikeshedding welcome),
using that same scheme for verifying a container with a boot script and
a container with kernel+initramfs+dtbs.
> For now there is absolutely no configurability in the signature mechanism.
> It would of course be possible to adjust which nodes are signed, as is

Yes, let the blob itself define that. And don't add any ad hoc "oh, skip
a property if it is called data-offset or data or...".

Rasmus

Re: [PATCH 00/16] tools: Add support for signing devicetree blobs

[Simon Glass]

Hi Rasmus,

On Fri, 26 Nov 2021 at 01:36, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 12/11/2021 20.28, Simon Glass wrote:
> > At present mkimage supports signing FITs, the standard U-Boot image type.
> >
> > Various people are opposed to using FIT since:
> >
> > a) it requires adding support for FIT into other bootloaders, notably
> >    UEFI
> > b) it requires packaging a kernel in this standard U-Boot format, meaning
> >    that distros must run 'mkimage' and deal with the kernel and initrd
> >    being inside a FIT
> >
> > The kernel and initrd can be dealt with in other ways. But without FIT,
> > we have no standard way of signing and grouping FDT files. Instead we must
> > include them in the distro as separate files.
> >
> > In particular, some sort of mechanism for verifying FDT files is needed.
> > One option would be to tack a signature on before or after the file,
> > processing it accordingly. But due to the nature of the FDT binary format,
> > it is possible to embed a signature inside the FDT itself, which is very
> > convenient.
> >
> > This series provides a tool, fdt_sign, which can add a signature to an
> > FDT. The signature can be checked later, preventing any change to the FDT,
> > other than in permitted nodes (e.g. /chosen).
> >
> > This series also provides a fdt_check_sign tool, used to check signatures.
>
> This could be useful. Not because I'm interested in signing device-tree
> blobs as such, but as a replacement for the current incomprehensible way
> FIT images are signed and verified. This seems to be a much better and
> simpler scheme, that avoids (can avoid) the overly simplistic approach
> of signing just image nodes, and the way too complex
> signing-configurations-and-then-add-a-property-saying-which-nodes-I-signed-and-sign-that-as-well.

That property is not actually signed. It is just a hint.

>
> In order not to hard-code anything in the tool about /chosen or whatnot,
> make the rule be that the blob (I'll use that word, because it can as
> well be a FIT image containing kernel image+initramfs etc., or a FIT
> containing a U-Boot script) has a property in the top node
>
>   unsigned-nodes = "/chosen", "/signatures";
>
> (maybe it could be
>
>   unsigned-nodes = <&chosen &sigs>
>
> but that may make the implementation a little more complex). That
> property itself is by definition part of what gets signed. The verifier
> can and should choose to reject the whole thing if "/" is mentioned.

If you list unsigned nodes, then the signature will become invalid if
a new image or config is added later.

>
> Then, please, for debugging, inside the /signatures node, beside the
> signature data, also add a copy of the offset/len pairs that were
> actually hashed, and that hash value.
>
>   /signatures {
>     hashed-regions = <offset0 len0 offset1 len1 ...>;
>     hash-1 {
>       value = 0xabcd;
>       algo = "sha256";
>     };
>     signature-1 {
>        ...
>     };
>     ...

The offsets may change whenever the devicetree is modified.

>
> And make the tool start by adding dummy properties, so those strings
> "hashed-regions", "value", "algo", "signer" and whatever other property
> names are used already exist in the string table. After that, fill in
> the actual values - and have the rule that the entire string table is
> hashed, not just some initial part of it.

The hashed part of the string table is the part used for each
signature. It is in fact the whole table at the time that signing
takes place. It's just that the table may expand late.

>
> The verifier will of course not rely on /signatures/hashed-regions, but
> will compute the regions itself based on /unsigned-nodes (and implicitly
> add a region consisting of the string table and the memreserve table).
>
> Then we'd have a generic scheme for signing blobs, disentangled from
> whether they are actually describing hardware or used as a generic
> container format. And instead of "image" or "conf", we could mark a
> public key as being required for "whole-blob" (bikeshedding welcome),
> using that same scheme for verifying a container with a boot script and
> a container with kernel+initramfs+dtbs.

What does this 'whole blob' help with?

> > For now there is absolutely no configurability in the signature mechanism.
> > It would of course be possible to adjust which nodes are signed, as is
>
> Yes, let the blob itself define that. And don't add any ad hoc "oh, skip
> a property if it is called data-offset or data or...".

We don't want to sign the data because it is already hashed once.
There is no point and it just makes verification slower.

For data-offset we want tools to be able to move the data around, e.g.
for memory-allocation reasons.

Overall I can grant that you are describing another way of doing
things, but I'm not sure what it would bring us.

The scheme in this series is aimed at a different purpose: signing a
devicetree blob, rather than a FIT.

Regards,
Simon

Re: [PATCH 12/16] tools: mkimage: Show where signatures/keys are written

[Simon Glass]

At present mkimage displays the node information but it is not clear what
signing action was taken. Add a message that shows it. For now it only
supports showing a single signing action, since that is the common case.

Sample:

   Signature written to 'sha1-basic/test.fit',
       node '/configurations/conf-1/signature'
   Public key written to 'sha1-basic/sandbox-u-boot.dtb',
       node '/signature/key-dev'

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/image.h    | 23 ++++++++++++++++++++++-
 tools/fit_common.c | 13 +++++++++++++
 tools/fit_common.h | 10 ++++++++++
 tools/fit_image.c  |  3 ++-
 tools/image-host.c | 23 ++++++++++++++++++-----
 tools/imagetool.h  |  3 +++
 tools/mkimage.c    |  4 ++++
 7 files changed, 72 insertions(+), 7 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 11/16] tools: Pass public-key node through to caller

[Simon Glass]

Update the two functions that call add_verify_data() so that the caller
can see the node that was written to.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/image-host.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 09/16] tools: Pass the key blob around

[Simon Glass]

At present we rely on the key blob being in the global_data fdt_blob
pointer. This is true in U-Boot but not with tools. For clarity, pass the
parameter around.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/image-fit-sig.c | 31 ++++++++++++++++++-------------
 boot/image-fit.c     | 12 +++++++-----
 common/spl/spl_fit.c |  3 ++-
 include/image.h      | 23 ++++++++++++++++++-----
 4 files changed, 45 insertions(+), 24 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 08/16] tools: Tidy up argument order in fit_config_check_sig()

[Simon Glass]

Put the parent node first in the parameters as this is more natural. Also
add a comment to explain what is going on.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/image-fit-sig.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 07/16] tools: Avoid confusion between keys and signatures

[Simon Glass]

We should be consistent in using the term 'signature' to describe a value
added to sign something and 'key' to describe the key that can be used to
verify the signature.

Tidy up the code to stick to this.

Add some comments to fit_config_verify_key() and its callers while we are
here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/image-fit-sig.c | 97 ++++++++++++++++++++++++++++++++------------
 1 file changed, 72 insertions(+), 25 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 05/16] tools: Improve comments in signing functions

[Simon Glass]

Add some more comments to explain what is going on in the signing
functions. Fix two repeated typos.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/image.h    |  2 +-
 tools/fdt_host.h   |  8 ++++
 tools/image-host.c | 98 +++++++++++++++++++++++++++++++++++-----------
 3 files changed, 85 insertions(+), 23 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 04/16] tools: Avoid leaving extra data at the end of copied files

[Simon Glass]

The copyfile() implementation has strange behaviour if the destination
file already exists. Update it to ensure that any existing data in the
destination file is dropped.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/fit_common.c | 2 +-
 tools/fit_common.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 06/16] tools: Drop unused name in image-host

[Simon Glass]

The name is created but never used. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/image-host.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 02/16] fit_check_sign: Update help to mention the key is in a dtb

[Simon Glass]

The key is inside a dtb file, so tweak the help to make that clear.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/fit_check_sign.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 03/16] tools: Move copyfile() into a common file

[Simon Glass]

This function is useful in other places. Move it to a common file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/fit_common.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/fit_common.h | 11 +++++++++
 tools/fit_image.c  | 56 ----------------------------------------------
 3 files changed, 67 insertions(+), 56 deletions(-)

Applied to u-boot-dm, thanks!

Re: [PATCH 01/16] rsa: Add debugging for failure cases

[Simon Glass]

Add some more debugging to make it easier to see what is being tried and
what fails. Fix a few comment styles while here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/rsa/rsa-verify.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Applied to u-boot-dm, thanks!