All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] mkimage: Add a 'keyfile' argument for image signing
@ 2021-02-04 19:57 Alexandru Gagniuc
  2021-02-04 19:57 ` [PATCH 1/4] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-02-04 19:57 UTC (permalink / raw)
  To: u-boot

This series is a continuation of the following series
 *    [PATCH v5 0/6] Add support for ECDSA image signing (with test)

There were some requests in the previous series to be more consistent
between RSA and ECDSA paths, particularly in the area of handling the
'keydir' argument.

This series adds a 'keyfile' argument to be used in lieu of the
antiquated 'keydir'. Using 'keyfile' allows signing images that do not
contain a "key-name-hint" property.

Other issues, such as automatically adding the 'signature' node were
discussed. Such features are beyond the scope of this series. I am
going to halt any further work on mkimage because of time constraints.
I hope the corrections in this series are sufficient to finally allow 
users to sign their images with ECDSA.


Alexandru Gagniuc (4):
  doc: signature.txt: Document the keydir and keyfile arguments
  mkimage: Add a 'keyfile' argument for image signing
  lib/rsa: Use the 'keyfile' argument from mkimage
  lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate

 doc/uImage.FIT/signature.txt | 13 +++++++++
 include/image.h              |  8 ++++--
 lib/ecdsa/ecdsa-libcrypto.c  | 10 ++++++-
 lib/rsa/rsa-sign.c           | 34 +++++++++++++++++------
 tools/fit_image.c            |  3 +-
 tools/image-host.c           | 54 ++++++++++++++++++++----------------
 tools/imagetool.h            |  1 +
 tools/mkimage.c              |  6 +++-
 8 files changed, 91 insertions(+), 38 deletions(-)

-- 
2.26.2

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] doc: signature.txt: Document the keydir and keyfile arguments
  2021-02-04 19:57 [PATCH 0/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
@ 2021-02-04 19:57 ` Alexandru Gagniuc
  2021-02-07 14:37   ` Simon Glass
  2021-02-04 19:57 ` [PATCH 2/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-02-04 19:57 UTC (permalink / raw)
  To: u-boot

After lots of debating, this documents how we'd like mkimage to treat
'keydir' and 'keyfile' arguments. The rest is in the docs.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 doc/uImage.FIT/signature.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt
index 0139295d33..d9a9121190 100644
--- a/doc/uImage.FIT/signature.txt
+++ b/doc/uImage.FIT/signature.txt
@@ -472,6 +472,19 @@ Test Verified Boot Run: signed config with bad hash: OK
 Test passed
 
 
+Software signing: keydir vs keyfile
+-----------------------------------
+
+In the simplest case, signing is done by giving mkimage the 'keyfile'. This is
+the path to a file containing the signing key.
+
+The alternative is to pass the 'keydir' argument. In this case the filename of
+the key is derived from the 'keydir' and the "key-name-hint" property in the
+FIT. In this case the "key-name-hint" property is mandatory, and the key must
+exist in "<keydir>/<key-name-hint>.<ext>" Here the extension "ext" is
+specific to the signing algorithm.
+
+
 Hardware Signing with PKCS#11 or with HSM
 -----------------------------------------
 
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] mkimage: Add a 'keyfile' argument for image signing
  2021-02-04 19:57 [PATCH 0/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
  2021-02-04 19:57 ` [PATCH 1/4] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
@ 2021-02-04 19:57 ` Alexandru Gagniuc
  2021-02-07 14:37   ` Simon Glass
  2021-02-04 19:57 ` [PATCH 3/4] lib/rsa: Use the 'keyfile' argument from mkimage Alexandru Gagniuc
  2021-02-04 19:57 ` [PATCH 4/4] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate Alexandru Gagniuc
  3 siblings, 1 reply; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-02-04 19:57 UTC (permalink / raw)
  To: u-boot

It's not always desirable to use 'keydir' and some ad-hoc heuristics
to get the filename of the signing key. More often, just passing the
filename is the simpler, easier, and logical thing to do.

Since mkimage doesn't use long options, we're slowly running out of
letters. I've chosen '-G' because it was available.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 include/image.h    |  8 ++++---
 tools/fit_image.c  |  3 ++-
 tools/image-host.c | 58 +++++++++++++++++++++++++---------------------
 tools/imagetool.h  |  1 +
 tools/mkimage.c    |  6 ++++-
 5 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/include/image.h b/include/image.h
index 2447321023..9bc8b8d179 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1128,9 +1128,10 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
  *     0, on success
  *     libfdt error code, on failure
  */
-int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
-			      const char *comment, int require_keys,
-			      const char *engine_id, const char *cmdname);
+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);
 
 int fit_image_verify_with_data(const void *fit, int image_noffset,
 			       const void *data, size_t size);
@@ -1236,6 +1237,7 @@ void image_set_host_blob(void *host_blob);
 struct image_sign_info {
 	const char *keydir;		/* Directory conaining keys */
 	const char *keyname;		/* Name of key to use */
+	const char *keyfile;		/* Filename of private or public key */
 	void *fit;			/* Pointer to FIT blob */
 	int node_offset;		/* Offset of signature node */
 	const char *name;		/* Algorithm name */
diff --git a/tools/fit_image.c b/tools/fit_image.c
index 06faeda7c2..0091545bb1 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -68,7 +68,8 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
 	}
 
 	if (!ret) {
-		ret = fit_add_verification_data(params->keydir, dest_blob, ptr,
+		ret = fit_add_verification_data(params->keydir,
+						params->keyfile, dest_blob, ptr,
 						params->comment,
 						params->require_keys,
 						params->engine_id,
diff --git a/tools/image-host.c b/tools/image-host.c
index 33a224129a..270d36fe45 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -153,8 +153,9 @@ 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, void *fit, const char *image_name,
-		int noffset, const char *require_keys, const char *engine_id)
+		const char *keydir, const char *keyfile, void *fit,
+		const char *image_name, int noffset, const char *require_keys,
+		const char *engine_id)
 {
 	const char *node_name;
 	char *algo_name;
@@ -171,6 +172,7 @@ static int fit_image_setup_sig(struct image_sign_info *info,
 
 	memset(info, '\0', sizeof(*info));
 	info->keydir = keydir;
+	info->keyfile = keyfile;
 	info->keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL);
 	info->fit = fit;
 	info->node_offset = noffset;
@@ -207,8 +209,8 @@ static int fit_image_setup_sig(struct image_sign_info *info,
  * @engine_id:	Engine to use for signing
  * @return 0 if ok, -1 on error
  */
-static int fit_image_process_sig(const char *keydir, void *keydest,
-		void *fit, const char *image_name,
+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)
@@ -220,8 +222,9 @@ static int fit_image_process_sig(const char *keydir, void *keydest,
 	uint value_len;
 	int ret;
 
-	if (fit_image_setup_sig(&info, keydir, fit, image_name, noffset,
-				require_keys ? "image" : NULL, engine_id))
+	if (fit_image_setup_sig(&info, keydir, keyfile, fit, image_name,
+				noffset, require_keys ? "image" : NULL,
+				engine_id))
 		return -1;
 
 	node_name = fit_get_name(fit, noffset, NULL);
@@ -598,9 +601,10 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
  * @engine_id:	Engine to use for signing
  * @return: 0 on success, <0 on failure
  */
-int fit_image_add_verification_data(const char *keydir, void *keydest,
-		void *fit, int image_noffset, const char *comment,
-		int require_keys, const char *engine_id, const char *cmdname)
+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 *image_name;
 	const void *data;
@@ -632,10 +636,10 @@ int fit_image_add_verification_data(const char *keydir, void *keydest,
 			     strlen(FIT_HASH_NODENAME))) {
 			ret = fit_image_process_hash(fit, image_name, noffset,
 						data, size);
-		} else if (IMAGE_ENABLE_SIGN && keydir &&
+		} else if (IMAGE_ENABLE_SIGN && (keydir || keyfile) &&
 			   !strncmp(node_name, FIT_SIG_NODENAME,
 				strlen(FIT_SIG_NODENAME))) {
-			ret = fit_image_process_sig(keydir, keydest,
+			ret = fit_image_process_sig(keydir, keyfile, keydest,
 				fit, image_name, noffset, data, size,
 				comment, require_keys, engine_id, cmdname);
 		}
@@ -918,10 +922,10 @@ static int fit_config_get_data(void *fit, int conf_noffset, int noffset,
 	return 0;
 }
 
-static int fit_config_process_sig(const char *keydir, 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)
+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)
 {
 	struct image_sign_info info;
 	const char *node_name;
@@ -938,7 +942,7 @@ static int fit_config_process_sig(const char *keydir, void *keydest,
 				&region_count, &region_prop, &region_proplen))
 		return -1;
 
-	if (fit_image_setup_sig(&info, keydir, fit, conf_name, noffset,
+	if (fit_image_setup_sig(&info, keydir, keyfile, fit, conf_name, noffset,
 				require_keys ? "conf" : NULL, engine_id))
 		return -1;
 
@@ -983,9 +987,10 @@ static int fit_config_process_sig(const char *keydir, void *keydest,
 	return 0;
 }
 
-static int fit_config_add_verification_data(const char *keydir, void *keydest,
-		void *fit, int conf_noffset, const char *comment,
-		int require_keys, const char *engine_id, const char *cmdname)
+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 *conf_name;
 	int noffset;
@@ -1002,7 +1007,7 @@ static int fit_config_add_verification_data(const char *keydir, void *keydest,
 		node_name = fit_get_name(fit, noffset, NULL);
 		if (!strncmp(node_name, FIT_SIG_NODENAME,
 			     strlen(FIT_SIG_NODENAME))) {
-			ret = fit_config_process_sig(keydir, keydest,
+			ret = fit_config_process_sig(keydir, keyfile, keydest,
 				fit, conf_name, conf_noffset, noffset, comment,
 				require_keys, engine_id, cmdname);
 		}
@@ -1048,9 +1053,10 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
 	return 0;
 }
 
-int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
-			      const char *comment, int require_keys,
-			      const char *engine_id, const char *cmdname)
+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)
 {
 	int images_noffset, confs_noffset;
 	int noffset;
@@ -1072,7 +1078,7 @@ int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
 		 * Direct child node of the images parent node,
 		 * i.e. component image node.
 		 */
-		ret = fit_image_add_verification_data(keydir, keydest,
+		ret = fit_image_add_verification_data(keydir, keyfile, keydest,
 				fit, noffset, comment, require_keys, engine_id,
 				cmdname);
 		if (ret)
@@ -1080,7 +1086,7 @@ int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
 	}
 
 	/* If there are no keys, we can't sign configurations */
-	if (!IMAGE_ENABLE_SIGN || !keydir)
+	if (!IMAGE_ENABLE_SIGN || !(keydir || keyfile))
 		return 0;
 
 	/* Find configurations parent node offset */
@@ -1095,7 +1101,7 @@ int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
 	for (noffset = fdt_first_subnode(fit, confs_noffset);
 	     noffset >= 0;
 	     noffset = fdt_next_subnode(fit, noffset)) {
-		ret = fit_config_add_verification_data(keydir, keydest,
+		ret = fit_config_add_verification_data(keydir, keyfile, keydest,
 						       fit, noffset, comment,
 						       require_keys,
 						       engine_id, cmdname);
diff --git a/tools/imagetool.h b/tools/imagetool.h
index 8726792c8c..8400e87e62 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -67,6 +67,7 @@ struct image_tool_params {
 	const char *outfile;	/* Output filename */
 	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 *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 */
diff --git a/tools/mkimage.c b/tools/mkimage.c
index 68d5206cb4..cc7b242faf 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -108,6 +108,7 @@ static void usage(const char *msg)
 		"Signing / verified boot options: [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r] [-N engine]\n"
 		"          -k => set directory containing private keys\n"
 		"          -K => write public keys to this .dtb file\n"
+		"          -G => use this signing key (in lieu of -k)\n"
 		"          -c => add comment in signature node\n"
 		"          -F => re-sign existing FIT image\n"
 		"          -p => place external data at a static position\n"
@@ -151,7 +152,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:Fk: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:rR:qstT:vVx")) != -1) {
 		switch (opt) {
 		case 'a':
 			params.addr = strtoull(optarg, &ptr, 16);
@@ -226,6 +227,9 @@ static void process_args(int argc, char **argv)
 			params.type = IH_TYPE_FLATDT;
 			params.fflag = 1;
 			break;
+		case 'G':
+			params.keyfile = optarg;
+			break;
 		case 'i':
 			params.fit_ramdisk = optarg;
 			break;
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] lib/rsa: Use the 'keyfile' argument from mkimage
  2021-02-04 19:57 [PATCH 0/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
  2021-02-04 19:57 ` [PATCH 1/4] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
  2021-02-04 19:57 ` [PATCH 2/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
@ 2021-02-04 19:57 ` Alexandru Gagniuc
  2021-02-07 14:37   ` Simon Glass
  2021-02-04 19:57 ` [PATCH 4/4] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate Alexandru Gagniuc
  3 siblings, 1 reply; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-02-04 19:57 UTC (permalink / raw)
  To: u-boot

Keys can be derived from keydir, and the "key-name-hint" property of
the FIT. They can also be specified ad-literam via 'keyfile'. Update
the RSA signing path to use the appropriate one.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 lib/rsa/rsa-sign.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
index 557c690a6d..65c6d4490c 100644
--- a/lib/rsa/rsa-sign.c
+++ b/lib/rsa/rsa-sign.c
@@ -210,14 +210,20 @@ static int rsa_get_pub_key(const char *keydir, const char *name,
  * @return 0 if ok, -ve on error (in which case *rsap will be set to NULL)
  */
 static int rsa_pem_get_priv_key(const char *keydir, const char *name,
-				RSA **rsap)
+				const char *keyfile, RSA **rsap)
 {
 	char path[1024];
 	RSA *rsa;
 	FILE *f;
 
 	*rsap = NULL;
-	snprintf(path, sizeof(path), "%s/%s.key", keydir, name);
+	if (keydir && name)
+		snprintf(path, sizeof(path), "%s/%s.key", keydir, name);
+	else if (keyfile)
+		snprintf(path, sizeof(path), "%s", keyfile);
+	else
+		return -EINVAL;
+
 	f = fopen(path, "r");
 	if (!f) {
 		fprintf(stderr, "Couldn't open RSA private key: '%s': %s\n",
@@ -247,6 +253,7 @@ static int rsa_pem_get_priv_key(const char *keydir, const char *name,
  * @return 0 if ok, -ve on error (in which case *rsap will be set to NULL)
  */
 static int rsa_engine_get_priv_key(const char *keydir, const char *name,
+				   const char *keyfile,
 				   ENGINE *engine, RSA **rsap)
 {
 	const char *engine_id;
@@ -260,6 +267,10 @@ static int rsa_engine_get_priv_key(const char *keydir, const char *name,
 	engine_id = ENGINE_get_id(engine);
 
 	if (engine_id && !strcmp(engine_id, "pkcs11")) {
+		if (!keydir && !name) {
+			fprintf(stderr, "Please use 'keydir' with PKCS11\n");
+			return -EINVAL;
+		}
 		if (keydir)
 			if (strstr(keydir, "object="))
 				snprintf(key_id, sizeof(key_id),
@@ -274,14 +285,19 @@ static int rsa_engine_get_priv_key(const char *keydir, const char *name,
 				 "pkcs11:object=%s;type=private",
 				 name);
 	} else if (engine_id) {
-		if (keydir)
+		if (keydir && name)
 			snprintf(key_id, sizeof(key_id),
 				 "%s%s",
 				 keydir, name);
-		else
+		else if (keydir)
 			snprintf(key_id, sizeof(key_id),
 				 "%s",
 				 name);
+		else if (keyfile)
+			snprintf(key_id, sizeof(key_id), "%s", keyfile);
+		else
+			return -EINVAL;
+
 	} else {
 		fprintf(stderr, "Engine not supported\n");
 		return -ENOTSUP;
@@ -319,11 +335,12 @@ err_rsa:
  * @return 0 if ok, -ve on error (in which case *rsap will be set to NULL)
  */
 static int rsa_get_priv_key(const char *keydir, const char *name,
-			    ENGINE *engine, RSA **rsap)
+			    const char *keyfile, ENGINE *engine, RSA **rsap)
 {
 	if (engine)
-		return rsa_engine_get_priv_key(keydir, name, engine, rsap);
-	return rsa_pem_get_priv_key(keydir, name, rsap);
+		return rsa_engine_get_priv_key(keydir, name, keyfile, engine,
+					       rsap);
+	return rsa_pem_get_priv_key(keydir, name, keyfile, rsap);
 }
 
 static int rsa_init(void)
@@ -534,7 +551,8 @@ int rsa_sign(struct image_sign_info *info,
 			goto err_engine;
 	}
 
-	ret = rsa_get_priv_key(info->keydir, info->keyname, e, &rsa);
+	ret = rsa_get_priv_key(info->keydir, info->keyname, info->keyfile,
+			       e, &rsa);
 	if (ret)
 		goto err_priv;
 	ret = rsa_sign_with_key(rsa, info->padding, info->checksum, region,
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate
  2021-02-04 19:57 [PATCH 0/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
                   ` (2 preceding siblings ...)
  2021-02-04 19:57 ` [PATCH 3/4] lib/rsa: Use the 'keyfile' argument from mkimage Alexandru Gagniuc
@ 2021-02-04 19:57 ` Alexandru Gagniuc
  2021-02-07 14:37   ` Simon Glass
  3 siblings, 1 reply; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-02-04 19:57 UTC (permalink / raw)
  To: u-boot

Keys can be derived from keydir, and the "key-name-hint" property of
the FIT. They can also be specified ad-literam via 'keyfile'. Update
the ECDSA signing path to use the appropriate one.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 lib/ecdsa/ecdsa-libcrypto.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c
index 322880963f..1757a14562 100644
--- a/lib/ecdsa/ecdsa-libcrypto.c
+++ b/lib/ecdsa/ecdsa-libcrypto.c
@@ -140,8 +140,20 @@ static int read_key(struct signer *ctx, const char *key_name)
 /* Prepare a 'signer' context that's ready to sign and verify. */
 static int prepare_ctx(struct signer *ctx, const struct image_sign_info *info)
 {
-	const char *kname = info->keydir;
 	int key_len_bytes, ret;
+	char kname[1024];
+
+	memset(ctx, 0, sizeof(*ctx));
+
+	if (info->keyfile) {
+		snprintf(kname,  sizeof(kname), "%s", info->keyfile);
+	} else if (info->keydir && info->keyname) {
+		snprintf(kname, sizeof(kname), "%s/%s.pem", info->keydir,
+			 info->keyname);
+	} else {
+		fprintf(stderr, "keyfile, keyname, or key-name-hint missing\n");
+		return -EINVAL;
+	}
 
 	ret = alloc_ctx(ctx, info);
 	if (ret)
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] mkimage: Add a 'keyfile' argument for image signing
  2021-02-04 19:57 ` [PATCH 2/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
@ 2021-02-07 14:37   ` Simon Glass
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2021-02-07 14:37 UTC (permalink / raw)
  To: u-boot

Hi Alexandru,

On Thu, 4 Feb 2021 at 12:57, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> It's not always desirable to use 'keydir' and some ad-hoc heuristics
> to get the filename of the signing key. More often, just passing the
> filename is the simpler, easier, and logical thing to do.
>
> Since mkimage doesn't use long options, we're slowly running out of
> letters. I've chosen '-G' because it was available.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  include/image.h    |  8 ++++---
>  tools/fit_image.c  |  3 ++-
>  tools/image-host.c | 58 +++++++++++++++++++++++++---------------------
>  tools/imagetool.h  |  1 +
>  tools/mkimage.c    |  6 ++++-
>  5 files changed, 45 insertions(+), 31 deletions(-)
>

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

See below.

> diff --git a/include/image.h b/include/image.h
> index 2447321023..9bc8b8d179 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1128,9 +1128,10 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
>   *     0, on success
>   *     libfdt error code, on failure
>   */
> -int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
> -                             const char *comment, int require_keys,
> -                             const char *engine_id, const char *cmdname);
> +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);
>
>  int fit_image_verify_with_data(const void *fit, int image_noffset,
>                                const void *data, size_t size);
> @@ -1236,6 +1237,7 @@ void image_set_host_blob(void *host_blob);
>  struct image_sign_info {
>         const char *keydir;             /* Directory conaining keys */
>         const char *keyname;            /* Name of key to use */
> +       const char *keyfile;            /* Filename of private or public key */

Please also document the semantics of this...can it be NULL? I think
you intend that either keydir or keyfile is used but not both, right?

Regards,
Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] doc: signature.txt: Document the keydir and keyfile arguments
  2021-02-04 19:57 ` [PATCH 1/4] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
@ 2021-02-07 14:37   ` Simon Glass
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2021-02-07 14:37 UTC (permalink / raw)
  To: u-boot

On Thu, 4 Feb 2021 at 12:57, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> After lots of debating, this documents how we'd like mkimage to treat
> 'keydir' and 'keyfile' arguments. The rest is in the docs.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  doc/uImage.FIT/signature.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] lib/rsa: Use the 'keyfile' argument from mkimage
  2021-02-04 19:57 ` [PATCH 3/4] lib/rsa: Use the 'keyfile' argument from mkimage Alexandru Gagniuc
@ 2021-02-07 14:37   ` Simon Glass
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2021-02-07 14:37 UTC (permalink / raw)
  To: u-boot

On Thu, 4 Feb 2021 at 12:57, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> Keys can be derived from keydir, and the "key-name-hint" property of
> the FIT. They can also be specified ad-literam via 'keyfile'. Update
> the RSA signing path to use the appropriate one.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  lib/rsa/rsa-sign.c | 34 ++++++++++++++++++++++++++--------
>  1 file changed, 26 insertions(+), 8 deletions(-)

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate
  2021-02-04 19:57 ` [PATCH 4/4] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate Alexandru Gagniuc
@ 2021-02-07 14:37   ` Simon Glass
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2021-02-07 14:37 UTC (permalink / raw)
  To: u-boot

On Thu, 4 Feb 2021 at 12:57, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> Keys can be derived from keydir, and the "key-name-hint" property of
> the FIT. They can also be specified ad-literam via 'keyfile'. Update
> the ECDSA signing path to use the appropriate one.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  lib/ecdsa/ecdsa-libcrypto.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-02-07 14:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04 19:57 [PATCH 0/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
2021-02-04 19:57 ` [PATCH 1/4] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
2021-02-07 14:37   ` Simon Glass
2021-02-04 19:57 ` [PATCH 2/4] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
2021-02-07 14:37   ` Simon Glass
2021-02-04 19:57 ` [PATCH 3/4] lib/rsa: Use the 'keyfile' argument from mkimage Alexandru Gagniuc
2021-02-07 14:37   ` Simon Glass
2021-02-04 19:57 ` [PATCH 4/4] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate Alexandru Gagniuc
2021-02-07 14:37   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.