[resent][PATCH 0/3] mkimage: allow to specify signing algorithm

[resent][PATCH 0/3] mkimage: allow to specify signing algorithm

[Jan Kiszka]

[resent as requested by Simon]

Another step to decouple the FIT image specification from the actual
signing: With these changes, the signature nodes can leave out an algo
property, mkimage will initialize that as well while signing. This way,
in-tree FIT source files can be prepared for gaining signatures without
defining the key type or size upfront, forcing users to patch the code
to change that.

Patch 1 is preparatory for this, patch 2 a drive-by cleanup.

A better solution would actually be if the algorithm was derived from
the provided key. But the underlying crypto layer seems to be rather
unprepared for that.

Jan

Jan Kiszka (3):
  image-fit: Make string of algo parameter constant
  mkimage: Drop unused OPT_STRING constant
  mkimage: Allow to specify the signature algorithm on the command line

 boot/image-fit-sig.c |  2 +-
 boot/image-fit.c     |  8 +++----
 doc/mkimage.1        |  5 +++++
 include/image.h      |  5 +++--
 tools/fit_image.c    |  3 ++-
 tools/image-host.c   | 50 +++++++++++++++++++++++++-------------------
 tools/imagetool.h    |  1 +
 tools/mkimage.c      |  6 ++++--
 8 files changed, 49 insertions(+), 31 deletions(-)

-- 
2.31.1

[resent][PATCH 1/3] image-fit: Make string of algo parameter constant

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Modifications would be invalid.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 boot/image-fit-sig.c | 2 +-
 boot/image-fit.c     | 8 ++++----
 include/image.h      | 2 +-
 tools/image-host.c   | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 63e5423c92..47d7633568 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -67,7 +67,7 @@ static int fit_image_setup_verify(struct image_sign_info *info,
 				  const void *fit, int noffset,
 				  int required_keynode, char **err_msgp)
 {
-	char *algo_name;
+	const char *algo_name;
 	const char *padding_name;
 
 	if (fdt_totalsize(fit) > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)) {
diff --git a/boot/image-fit.c b/boot/image-fit.c
index b629339f4e..d06a68c025 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -191,7 +191,7 @@ static void fit_image_print_data(const void *fit, int noffset, const char *p,
 	const char *keyname;
 	uint8_t *value;
 	int value_len;
-	char *algo;
+	const char *algo;
 	const char *padding;
 	bool required;
 	int ret, i;
@@ -1063,11 +1063,11 @@ int fit_image_get_data_and_size(const void *fit, int noffset,
  *     0, on success
  *     -1, on failure
  */
-int fit_image_hash_get_algo(const void *fit, int noffset, char **algo)
+int fit_image_hash_get_algo(const void *fit, int noffset, const char **algo)
 {
 	int len;
 
-	*algo = (char *)fdt_getprop(fit, noffset, FIT_ALGO_PROP, &len);
+	*algo = (const char *)fdt_getprop(fit, noffset, FIT_ALGO_PROP, &len);
 	if (*algo == NULL) {
 		fit_get_debug(fit, noffset, FIT_ALGO_PROP, len);
 		return -1;
@@ -1265,7 +1265,7 @@ static int fit_image_check_hash(const void *fit, int noffset, const void *data,
 {
 	uint8_t value[FIT_MAX_HASH_LEN];
 	int value_len;
-	char *algo;
+	const char *algo;
 	uint8_t *fit_value;
 	int fit_value_len;
 	int ignore;
diff --git a/include/image.h b/include/image.h
index fd662e74b4..16ccc5b10f 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1011,7 +1011,7 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
 int fit_image_get_data_and_size(const void *fit, int noffset,
 				const void **data, size_t *size);
 
-int fit_image_hash_get_algo(const void *fit, int noffset, char **algo);
+int fit_image_hash_get_algo(const void *fit, int noffset, const char **algo);
 int fit_image_hash_get_value(const void *fit, int noffset, uint8_t **value,
 				int *value_len);
 
diff --git a/tools/image-host.c b/tools/image-host.c
index a6b0a94420..a027155f3c 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -63,7 +63,7 @@ static int fit_image_process_hash(void *fit, const char *image_name,
 	uint8_t value[FIT_MAX_HASH_LEN];
 	const char *node_name;
 	int value_len;
-	char *algo;
+	const char *algo;
 	int ret;
 
 	node_name = fit_get_name(fit, noffset, NULL);
@@ -160,7 +160,7 @@ static int fit_image_setup_sig(struct image_sign_info *info,
 		const char *engine_id)
 {
 	const char *node_name;
-	char *algo_name;
+	const char *algo_name;
 	const char *padding_name;
 
 	node_name = fit_get_name(fit, noffset, NULL);
-- 
2.31.1

[resent][PATCH 2/3] mkimage: Drop unused OPT_STRING constant

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

The actual opt string is inlined - and different. Seems this was a
left-over from older versions of 603e26f76346.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 tools/mkimage.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index fbe883ce36..a4844d0f18 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -146,7 +146,6 @@ static int add_content(int type, const char *fname)
 	return 0;
 }
 
-#define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
 static void process_args(int argc, char **argv)
 {
 	char *ptr;
-- 
2.31.1

[resent][PATCH 3/3] mkimage: Allow to specify the signature algorithm on the command line

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

This permits to prepare FIT image description that do not hard-code the
final choice of the signature algorithm, possibly requiring the user to
patch the sources.

When -o <algo> is specified, this information is used in favor of the
'algo' property in the signature node. Furthermore, that property is set
accordingly when writing the image.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 doc/mkimage.1      |  5 +++++
 include/image.h    |  3 ++-
 tools/fit_image.c  |  3 ++-
 tools/image-host.c | 48 +++++++++++++++++++++++++++-------------------
 tools/imagetool.h  |  1 +
 tools/mkimage.c    |  5 ++++-
 6 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/doc/mkimage.1 b/doc/mkimage.1
index fea5288784..0734bd36a1 100644
--- a/doc/mkimage.1
+++ b/doc/mkimage.1
@@ -155,6 +155,11 @@ the corresponding public key is written into this file for for run-time
 verification. Typically the file here is the device tree binary used by
 CONFIG_OF_CONTROL in U-Boot.
 
+.TP
+.BI "\-o [" "signing algorithm" "]"
+Specifies the algorithm to be used for signing a FIT image. The default is
+taken from the target signature nodes 'algo' properties.
+
 .TP
 .BI "\-p [" "external position" "]"
 Place external data at a static external position. See \-E. Instead of writing
diff --git a/include/image.h b/include/image.h
index 16ccc5b10f..4a7e9bc9a1 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1031,6 +1031,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
+ * @algo_name:	Algorithm name, or NULL if to be read from FIT
  *
  * Adds hash values for all component images in the FIT blob.
  * Hashes are calculated for all component images which have hash subnodes
@@ -1045,7 +1046,7 @@ 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, const char *algo_name);
 
 int fit_image_verify_with_data(const void *fit, int image_noffset,
 			       const void *data, size_t size);
diff --git a/tools/fit_image.c b/tools/fit_image.c
index f4f372ba62..428ddcf881 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->algo_name);
 	}
 
 	if (dest_blob) {
diff --git a/tools/image-host.c b/tools/image-host.c
index a027155f3c..d2e67a06aa 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -107,7 +107,7 @@ static int fit_image_process_hash(void *fit, const char *image_name,
  */
 static int fit_image_write_sig(void *fit, int noffset, uint8_t *value,
 		int value_len, const char *comment, const char *region_prop,
-		int region_proplen, const char *cmdname)
+		int region_proplen, const char *cmdname, const char *algo_name)
 {
 	int string_size;
 	int ret;
@@ -150,6 +150,8 @@ static int fit_image_write_sig(void *fit, int noffset, uint8_t *value,
 					  strdata, sizeof(strdata));
 		}
 	}
+	if (algo_name && !ret)
+		ret = fdt_setprop_string(fit, noffset, "algo", algo_name);
 
 	return ret;
 }
@@ -157,17 +159,18 @@ static int fit_image_write_sig(void *fit, int noffset, uint8_t *value,
 static int fit_image_setup_sig(struct image_sign_info *info,
 		const char *keydir, const char *keyfile, void *fit,
 		const char *image_name, int noffset, const char *require_keys,
-		const char *engine_id)
+		const char *engine_id, const char *algo_name)
 {
 	const char *node_name;
-	const char *algo_name;
 	const char *padding_name;
 
 	node_name = fit_get_name(fit, noffset, NULL);
-	if (fit_image_hash_get_algo(fit, noffset, &algo_name)) {
-		printf("Can't get algo property for '%s' signature node in '%s' image node\n",
-		       node_name, image_name);
-		return -1;
+	if (!algo_name) {
+		if (fit_image_hash_get_algo(fit, noffset, &algo_name)) {
+			printf("Can't get algo property for '%s' signature node in '%s' image node\n",
+			       node_name, image_name);
+			return -1;
+		}
 	}
 
 	padding_name = fdt_getprop(fit, noffset, "padding", NULL);
@@ -215,7 +218,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,
 		void *keydest, void *fit, const char *image_name,
 		int noffset, const void *data, size_t size,
 		const char *comment, int require_keys, const char *engine_id,
-		const char *cmdname)
+		const char *cmdname, const char *algo_name)
 {
 	struct image_sign_info info;
 	struct image_region region;
@@ -226,7 +229,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,
 
 	if (fit_image_setup_sig(&info, keydir, keyfile, fit, image_name,
 				noffset, require_keys ? "image" : NULL,
-				engine_id))
+				engine_id, algo_name))
 		return -1;
 
 	node_name = fit_get_name(fit, noffset, NULL);
@@ -244,7 +247,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,
 	}
 
 	ret = fit_image_write_sig(fit, noffset, value, value_len, comment,
-			NULL, 0, cmdname);
+			NULL, 0, cmdname, algo_name);
 	if (ret) {
 		if (ret == -FDT_ERR_NOSPACE)
 			return -ENOSPC;
@@ -606,7 +609,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
 int fit_image_add_verification_data(const char *keydir, const char *keyfile,
 		void *keydest, void *fit, int image_noffset,
 		const char *comment, int require_keys, const char *engine_id,
-		const char *cmdname)
+		const char *cmdname, const char* algo_name)
 {
 	const char *image_name;
 	const void *data;
@@ -643,7 +646,8 @@ int fit_image_add_verification_data(const char *keydir, const char *keyfile,
 				strlen(FIT_SIG_NODENAME))) {
 			ret = fit_image_process_sig(keydir, keyfile, keydest,
 				fit, image_name, noffset, data, size,
-				comment, require_keys, engine_id, cmdname);
+				comment, require_keys, engine_id, cmdname,
+				algo_name);
 		}
 		if (ret)
 			return ret;
@@ -927,7 +931,8 @@ 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,
 		int conf_noffset, int noffset, const char *comment,
-		int require_keys, const char *engine_id, const char *cmdname)
+		int require_keys, const char *engine_id, const char *cmdname,
+		const char *algo_name)
 {
 	struct image_sign_info info;
 	const char *node_name;
@@ -945,7 +950,8 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile,
 		return -1;
 
 	if (fit_image_setup_sig(&info, keydir, keyfile, fit, conf_name, noffset,
-				require_keys ? "conf" : NULL, engine_id))
+				require_keys ? "conf" : NULL, engine_id,
+				algo_name))
 		return -1;
 
 	ret = info.crypto->sign(&info, region, region_count, &value,
@@ -962,7 +968,8 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile,
 	}
 
 	ret = fit_image_write_sig(fit, noffset, value, value_len, comment,
-				region_prop, region_proplen, cmdname);
+				  region_prop, region_proplen, cmdname,
+				  algo_name);
 	if (ret) {
 		if (ret == -FDT_ERR_NOSPACE)
 			return -ENOSPC;
@@ -992,7 +999,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, const char *algo_name)
 {
 	const char *conf_name;
 	int noffset;
@@ -1011,7 +1018,7 @@ static int fit_config_add_verification_data(const char *keydir,
 			     strlen(FIT_SIG_NODENAME))) {
 			ret = fit_config_process_sig(keydir, keyfile, keydest,
 				fit, conf_name, conf_noffset, noffset, comment,
-				require_keys, engine_id, cmdname);
+				require_keys, engine_id, cmdname, algo_name);
 		}
 		if (ret)
 			return ret;
@@ -1058,7 +1065,7 @@ 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, const char *algo_name)
 {
 	int images_noffset, confs_noffset;
 	int noffset;
@@ -1082,7 +1089,7 @@ int fit_add_verification_data(const char *keydir, const char *keyfile,
 		 */
 		ret = fit_image_add_verification_data(keydir, keyfile, keydest,
 				fit, noffset, comment, require_keys, engine_id,
-				cmdname);
+				cmdname, algo_name);
 		if (ret)
 			return ret;
 	}
@@ -1106,7 +1113,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,
+						       algo_name);
 		if (ret)
 			return ret;
 	}
diff --git a/tools/imagetool.h b/tools/imagetool.h
index e229a34ffc..d71027317f 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -69,6 +69,7 @@ struct image_tool_params {
 	const char *keydest;	/* Destination .dtb for public key */
 	const char *keyfile;	/* Filename of private or public key */
 	const char *comment;	/* Comment to add to signature node */
+	const char *algo_name;	/* Algorithm name to use hashing/signing */
 	int require_keys;	/* 1 to mark signing keys as 'required' */
 	int file_size;		/* Total size of output file */
 	int orig_file_size;	/* Original size for file before padding */
diff --git a/tools/mkimage.c b/tools/mkimage.c
index a4844d0f18..ddb79331a6 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -154,7 +154,7 @@ static void process_args(int argc, char **argv)
 	int opt;
 
 	while ((opt = getopt(argc, argv,
-		   "a:A:b:B:c:C:d:D:e:Ef:FG:k:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
+		   "a:A:b:B:c:C:d:D:e:Ef:FG:k:i:K:ln:N:p:o:O:rR:qstT:vVx")) != -1) {
 		switch (opt) {
 		case 'a':
 			params.addr = strtoull(optarg, &ptr, 16);
@@ -250,6 +250,9 @@ static void process_args(int argc, char **argv)
 		case 'N':
 			params.engine_id = optarg;
 			break;
+		case 'o':
+			params.algo_name = optarg;
+			break;
 		case 'O':
 			params.os = genimg_get_os_id(optarg);
 			if (params.os < 0) {
-- 
2.31.1

Re: [resent][PATCH 1/3] image-fit: Make string of algo parameter constant

[Simon Glass]

On Fri, 14 Jan 2022 at 02:21, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Modifications would be invalid.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  boot/image-fit-sig.c | 2 +-
>  boot/image-fit.c     | 8 ++++----
>  include/image.h      | 2 +-
>  tools/image-host.c   | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)

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

Re: [resent][PATCH 2/3] mkimage: Drop unused OPT_STRING constant

[Simon Glass]

On Fri, 14 Jan 2022 at 02:21, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> The actual opt string is inlined - and different. Seems this was a
> left-over from older versions of 603e26f76346.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  tools/mkimage.c | 1 -
>  1 file changed, 1 deletion(-)

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

Re: [resent][PATCH 3/3] mkimage: Allow to specify the signature algorithm on the command line

[Simon Glass]

Hi Jan,

On Fri, 14 Jan 2022 at 02:21, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> This permits to prepare FIT image description that do not hard-code the
> final choice of the signature algorithm, possibly requiring the user to
> patch the sources.
>
> When -o <algo> is specified, this information is used in favor of the
> 'algo' property in the signature node. Furthermore, that property is set
> accordingly when writing the image.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  doc/mkimage.1      |  5 +++++
>  include/image.h    |  3 ++-
>  tools/fit_image.c  |  3 ++-
>  tools/image-host.c | 48 +++++++++++++++++++++++++++-------------------
>  tools/imagetool.h  |  1 +
>  tools/mkimage.c    |  5 ++++-
>  6 files changed, 42 insertions(+), 23 deletions(-)

Please add a test to test_vboot for this case.

>
> diff --git a/doc/mkimage.1 b/doc/mkimage.1
> index fea5288784..0734bd36a1 100644
> --- a/doc/mkimage.1
> +++ b/doc/mkimage.1
> @@ -155,6 +155,11 @@ the corresponding public key is written into this file for for run-time
>  verification. Typically the file here is the device tree binary used by
>  CONFIG_OF_CONTROL in U-Boot.
>
> +.TP
> +.BI "\-o [" "signing algorithm" "]"
> +Specifies the algorithm to be used for signing a FIT image. The default is
> +taken from the target signature nodes 'algo' properties.

What does 'target' mean in this case?

Perhaps 'taken from the signature node's 'algo' properties' ?

> +
>  .TP
>  .BI "\-p [" "external position" "]"
>  Place external data at a static external position. See \-E. Instead of writing
> diff --git a/include/image.h b/include/image.h
> index 16ccc5b10f..4a7e9bc9a1 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1031,6 +1031,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
> + * @algo_name: Algorithm name, or NULL if to be read from FIT
>   *
>   * Adds hash values for all component images in the FIT blob.
>   * Hashes are calculated for all component images which have hash subnodes
> @@ -1045,7 +1046,7 @@ 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, const char *algo_name);
>
>  int fit_image_verify_with_data(const void *fit, int image_noffset,
>                                const void *data, size_t size);
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index f4f372ba62..428ddcf881 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->algo_name);
>         }
>
>         if (dest_blob) {
> diff --git a/tools/image-host.c b/tools/image-host.c
> index a027155f3c..d2e67a06aa 100644
> --- a/tools/image-host.c
> +++ b/tools/image-host.c
> @@ -107,7 +107,7 @@ static int fit_image_process_hash(void *fit, const char *image_name,
>   */
>  static int fit_image_write_sig(void *fit, int noffset, uint8_t *value,
>                 int value_len, const char *comment, const char *region_prop,
> -               int region_proplen, const char *cmdname)
> +               int region_proplen, const char *cmdname, const char *algo_name)
>  {
>         int string_size;
>         int ret;
> @@ -150,6 +150,8 @@ static int fit_image_write_sig(void *fit, int noffset, uint8_t *value,
>                                           strdata, sizeof(strdata));
>                 }
>         }
> +       if (algo_name && !ret)
> +               ret = fdt_setprop_string(fit, noffset, "algo", algo_name);
>
>         return ret;
>  }
> @@ -157,17 +159,18 @@ static int fit_image_write_sig(void *fit, int noffset, uint8_t *value,
>  static int fit_image_setup_sig(struct image_sign_info *info,
>                 const char *keydir, const char *keyfile, void *fit,
>                 const char *image_name, int noffset, const char *require_keys,
> -               const char *engine_id)
> +               const char *engine_id, const char *algo_name)
>  {
>         const char *node_name;
> -       const char *algo_name;
>         const char *padding_name;
>
>         node_name = fit_get_name(fit, noffset, NULL);
> -       if (fit_image_hash_get_algo(fit, noffset, &algo_name)) {
> -               printf("Can't get algo property for '%s' signature node in '%s' image node\n",
> -                      node_name, image_name);
> -               return -1;
> +       if (!algo_name) {
> +               if (fit_image_hash_get_algo(fit, noffset, &algo_name)) {
> +                       printf("Can't get algo property for '%s' signature node in '%s' image node\n",
> +                              node_name, image_name);
> +                       return -1;
> +               }
>         }
>
>         padding_name = fdt_getprop(fit, noffset, "padding", NULL);
> @@ -215,7 +218,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,
>                 void *keydest, void *fit, const char *image_name,
>                 int noffset, const void *data, size_t size,
>                 const char *comment, int require_keys, const char *engine_id,
> -               const char *cmdname)
> +               const char *cmdname, const char *algo_name)
>  {
>         struct image_sign_info info;
>         struct image_region region;
> @@ -226,7 +229,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,
>
>         if (fit_image_setup_sig(&info, keydir, keyfile, fit, image_name,
>                                 noffset, require_keys ? "image" : NULL,
> -                               engine_id))
> +                               engine_id, algo_name))
>                 return -1;
>
>         node_name = fit_get_name(fit, noffset, NULL);
> @@ -244,7 +247,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,
>         }
>
>         ret = fit_image_write_sig(fit, noffset, value, value_len, comment,
> -                       NULL, 0, cmdname);
> +                       NULL, 0, cmdname, algo_name);
>         if (ret) {
>                 if (ret == -FDT_ERR_NOSPACE)
>                         return -ENOSPC;
> @@ -606,7 +609,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
>  int fit_image_add_verification_data(const char *keydir, const char *keyfile,
>                 void *keydest, void *fit, int image_noffset,
>                 const char *comment, int require_keys, const char *engine_id,
> -               const char *cmdname)
> +               const char *cmdname, const char* algo_name)
>  {
>         const char *image_name;
>         const void *data;
> @@ -643,7 +646,8 @@ int fit_image_add_verification_data(const char *keydir, const char *keyfile,
>                                 strlen(FIT_SIG_NODENAME))) {
>                         ret = fit_image_process_sig(keydir, keyfile, keydest,
>                                 fit, image_name, noffset, data, size,
> -                               comment, require_keys, engine_id, cmdname);
> +                               comment, require_keys, engine_id, cmdname,
> +                               algo_name);
>                 }
>                 if (ret)
>                         return ret;
> @@ -927,7 +931,8 @@ 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,
>                 int conf_noffset, int noffset, const char *comment,
> -               int require_keys, const char *engine_id, const char *cmdname)
> +               int require_keys, const char *engine_id, const char *cmdname,
> +               const char *algo_name)
>  {
>         struct image_sign_info info;
>         const char *node_name;
> @@ -945,7 +950,8 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile,
>                 return -1;
>
>         if (fit_image_setup_sig(&info, keydir, keyfile, fit, conf_name, noffset,
> -                               require_keys ? "conf" : NULL, engine_id))
> +                               require_keys ? "conf" : NULL, engine_id,
> +                               algo_name))
>                 return -1;
>
>         ret = info.crypto->sign(&info, region, region_count, &value,
> @@ -962,7 +968,8 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile,
>         }
>
>         ret = fit_image_write_sig(fit, noffset, value, value_len, comment,
> -                               region_prop, region_proplen, cmdname);
> +                                 region_prop, region_proplen, cmdname,
> +                                 algo_name);
>         if (ret) {
>                 if (ret == -FDT_ERR_NOSPACE)
>                         return -ENOSPC;
> @@ -992,7 +999,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, const char *algo_name)
>  {
>         const char *conf_name;
>         int noffset;
> @@ -1011,7 +1018,7 @@ static int fit_config_add_verification_data(const char *keydir,
>                              strlen(FIT_SIG_NODENAME))) {
>                         ret = fit_config_process_sig(keydir, keyfile, keydest,
>                                 fit, conf_name, conf_noffset, noffset, comment,
> -                               require_keys, engine_id, cmdname);
> +                               require_keys, engine_id, cmdname, algo_name);
>                 }
>                 if (ret)
>                         return ret;
> @@ -1058,7 +1065,7 @@ 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, const char *algo_name)
>  {
>         int images_noffset, confs_noffset;
>         int noffset;
> @@ -1082,7 +1089,7 @@ int fit_add_verification_data(const char *keydir, const char *keyfile,
>                  */
>                 ret = fit_image_add_verification_data(keydir, keyfile, keydest,
>                                 fit, noffset, comment, require_keys, engine_id,
> -                               cmdname);
> +                               cmdname, algo_name);
>                 if (ret)
>                         return ret;
>         }
> @@ -1106,7 +1113,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,
> +                                                      algo_name);
>                 if (ret)
>                         return ret;
>         }
> diff --git a/tools/imagetool.h b/tools/imagetool.h
> index e229a34ffc..d71027317f 100644
> --- a/tools/imagetool.h
> +++ b/tools/imagetool.h
> @@ -69,6 +69,7 @@ struct image_tool_params {
>         const char *keydest;    /* Destination .dtb for public key */
>         const char *keyfile;    /* Filename of private or public key */
>         const char *comment;    /* Comment to add to signature node */
> +       const char *algo_name;  /* Algorithm name to use hashing/signing */

NULL to use the one in the .its ?


>         int require_keys;       /* 1 to mark signing keys as 'required' */
>         int file_size;          /* Total size of output file */
>         int orig_file_size;     /* Original size for file before padding */
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index a4844d0f18..ddb79331a6 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -154,7 +154,7 @@ static void process_args(int argc, char **argv)
>         int opt;
>
>         while ((opt = getopt(argc, argv,
> -                  "a:A:b:B:c:C:d:D:e:Ef:FG:k:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
> +                  "a:A:b:B:c:C:d:D:e:Ef:FG:k:i:K:ln:N:p:o:O:rR:qstT:vVx")) != -1) {
>                 switch (opt) {
>                 case 'a':
>                         params.addr = strtoull(optarg, &ptr, 16);
> @@ -250,6 +250,9 @@ static void process_args(int argc, char **argv)
>                 case 'N':
>                         params.engine_id = optarg;
>                         break;
> +               case 'o':
> +                       params.algo_name = optarg;
> +                       break;
>                 case 'O':
>                         params.os = genimg_get_os_id(optarg);
>                         if (params.os < 0) {
> --
> 2.31.1
>

Re: [resent][PATCH 1/3] image-fit: Make string of algo parameter constant

[Tom Rini]

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

On Fri, Jan 14, 2022 at 10:21:17AM +0100, Jan Kiszka wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Modifications would be invalid.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

Re: [resent][PATCH 2/3] mkimage: Drop unused OPT_STRING constant

[Tom Rini]

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

On Fri, Jan 14, 2022 at 10:21:18AM +0100, Jan Kiszka wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> The actual opt string is inlined - and different. Seems this was a
> left-over from older versions of 603e26f76346.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

Re: [resent][PATCH 3/3] mkimage: Allow to specify the signature algorithm on the command line

[Tom Rini]

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

On Fri, Jan 14, 2022 at 10:21:19AM +0100, Jan Kiszka wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This permits to prepare FIT image description that do not hard-code the
> final choice of the signature algorithm, possibly requiring the user to
> patch the sources.
> 
> When -o <algo> is specified, this information is used in favor of the
> 'algo' property in the signature node. Furthermore, that property is set
> accordingly when writing the image.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Applied to u-boot/master, thanks!

-- 
Tom

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