All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 00/11] Add support for ECDSA image signing
@ 2021-02-19 18:45 Alexandru Gagniuc
  2021-02-19 18:45 ` [PATCH v6 01/11] lib: Rename rsa-checksum.c to hash-checksum.c Alexandru Gagniuc
                   ` (10 more replies)
  0 siblings, 11 replies; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 UTC (permalink / raw)
  To: u-boot

## Purpose and intent

The purpose of this series is to enable ECDSA as an alternative to RSA
for FIT signing. As new chips have built-in support for ECDSA verified
boot, it makes sense to stick to one signing algorithm, instead of
resorting to RSA for u-boot images.

The focus of this series is signing an existing FIT image:

	mkimage -F some-existing.fit -G some/key.pem

Signing while assembling a FIT is not a tested use case.
This concatenates two series:

  * Add support for ECDSA image signing (with test)
  * mkimage: Add a 'keyfile' argument for image signing


# Testing

test/py/tests/test_fit_ecdsa.py is implemented withe the goal to check
that the signing is done correctly, and that the signature is encoded
in the proper raw format. Verification is done with pyCryptodomex, so
this test will catch both coding errors and openssl bugs. This is the
only scope of testing proposed here.


Changes since v5:
 - Include "mkimage: Add a 'keyfile' argument for image signing" series
 - Update python test to use 'keyfile' (-G) mkinage argument
 - document which strings can be null in image.h
 - Fix typo in python test (invokations -> invocations)

Changes since v4:
 - Fixed tools/ build issue with # FIT_SIGNATURE is not selected

Changes since v3:
 - Don't use 'log_msg_ret()', as it's not available host-side

Changes since v1 and v2:
 - Added lots of function comments
 - Replaced hardcoded error numbers with more meaningful errno numbers
 - Changed some error paths to use 'return log_msg_ret'

Alexandru Gagniuc (11):
  lib: Rename rsa-checksum.c to hash-checksum.c
  lib/rsa: Make fdt_add_bignum() available outside of RSA code
  lib: Add support for ECDSA image signing
  doc: signature.txt: Document devicetree format for ECDSA keys
  test/py: Add pycryptodomex to list of required pakages
  test/py: ecdsa: Add test for mkimage ECDSA signing
  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
  test/py: ecdsa: Use mkimage keyfile instead of keydir argument

 common/image-fit-sig.c                        |   2 +-
 common/image-sig.c                            |  13 +-
 doc/uImage.FIT/signature.txt                  |  20 +-
 include/image.h                               |  21 +-
 include/u-boot/ecdsa.h                        |  94 ++++++
 include/u-boot/fdt-libcrypto.h                |  27 ++
 .../{rsa-checksum.h => hash-checksum.h}       |   0
 lib/Makefile                                  |   1 +
 lib/crypto/pkcs7_verify.c                     |   2 +-
 lib/crypto/x509_public_key.c                  |   2 +-
 lib/ecdsa/ecdsa-libcrypto.c                   | 318 ++++++++++++++++++
 lib/fdt-libcrypto.c                           |  72 ++++
 lib/{rsa/rsa-checksum.c => hash-checksum.c}   |   3 +-
 lib/rsa/Makefile                              |   2 +-
 lib/rsa/rsa-sign.c                            |  99 ++----
 test/py/requirements.txt                      |   1 +
 test/py/tests/test_fit_ecdsa.py               | 111 ++++++
 tools/Makefile                                |  11 +-
 tools/fit_image.c                             |   3 +-
 tools/image-host.c                            |  58 ++--
 tools/imagetool.h                             |   1 +
 tools/mkimage.c                               |   6 +-
 22 files changed, 752 insertions(+), 115 deletions(-)
 create mode 100644 include/u-boot/ecdsa.h
 create mode 100644 include/u-boot/fdt-libcrypto.h
 rename include/u-boot/{rsa-checksum.h => hash-checksum.h} (100%)
 create mode 100644 lib/ecdsa/ecdsa-libcrypto.c
 create mode 100644 lib/fdt-libcrypto.c
 rename lib/{rsa/rsa-checksum.c => hash-checksum.c} (96%)
 create mode 100644 test/py/tests/test_fit_ecdsa.py

-- 
2.26.2

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

* [PATCH v6 01/11] lib: Rename rsa-checksum.c to hash-checksum.c
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:24   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 02/11] lib/rsa: Make fdt_add_bignum() available outside of RSA code Alexandru Gagniuc
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 UTC (permalink / raw)
  To: u-boot

rsa-checksum.c sontains the hash_calculate() implementations. Despite
the "rsa-" file prefix, this function is useful for other algorithms.

To prevent confusion, move this file to lib/, and rename it to
hash-checksum.c, to give it a more "generic" feel.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 common/image-fit-sig.c                             | 2 +-
 common/image-sig.c                                 | 2 +-
 include/image.h                                    | 2 +-
 include/u-boot/{rsa-checksum.h => hash-checksum.h} | 0
 lib/Makefile                                       | 1 +
 lib/crypto/pkcs7_verify.c                          | 2 +-
 lib/crypto/x509_public_key.c                       | 2 +-
 lib/{rsa/rsa-checksum.c => hash-checksum.c}        | 3 ++-
 lib/rsa/Makefile                                   | 2 +-
 tools/Makefile                                     | 3 ++-
 10 files changed, 11 insertions(+), 8 deletions(-)
 rename include/u-boot/{rsa-checksum.h => hash-checksum.h} (100%)
 rename lib/{rsa/rsa-checksum.c => hash-checksum.c} (96%)

diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index 34ebb8edfe..55ddf1879e 100644
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -16,7 +16,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #include <fdt_region.h>
 #include <image.h>
 #include <u-boot/rsa.h>
-#include <u-boot/rsa-checksum.h>
+#include <u-boot/hash-checksum.h>
 
 #define IMAGE_MAX_HASHED_NODES		100
 
diff --git a/common/image-sig.c b/common/image-sig.c
index 4abd3c080f..54f0eb2019 100644
--- a/common/image-sig.c
+++ b/common/image-sig.c
@@ -17,7 +17,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #endif /* !USE_HOSTCC*/
 #include <image.h>
 #include <u-boot/rsa.h>
-#include <u-boot/rsa-checksum.h>
+#include <u-boot/hash-checksum.h>
 
 #define IMAGE_MAX_HASHED_NODES		100
 
diff --git a/include/image.h b/include/image.h
index d5a940313a..fbe9537c00 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1278,7 +1278,7 @@ struct image_region {
 };
 
 #if IMAGE_ENABLE_VERIFY
-# include <u-boot/rsa-checksum.h>
+# include <u-boot/hash-checksum.h>
 #endif
 struct checksum_algo {
 	const char *name;
diff --git a/include/u-boot/rsa-checksum.h b/include/u-boot/hash-checksum.h
similarity index 100%
rename from include/u-boot/rsa-checksum.h
rename to include/u-boot/hash-checksum.h
diff --git a/lib/Makefile b/lib/Makefile
index edc1c3dd4f..1d4b7d3aad 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -60,6 +60,7 @@ endif
 obj-$(CONFIG_$(SPL_)ACPIGEN) += acpi/
 obj-$(CONFIG_$(SPL_)MD5) += md5.o
 obj-$(CONFIG_$(SPL_)RSA) += rsa/
+obj-$(CONFIG_FIT_SIGNATURE) += hash-checksum.o
 obj-$(CONFIG_SHA1) += sha1.o
 obj-$(CONFIG_SHA256) += sha256.o
 obj-$(CONFIG_SHA512_ALGO) += sha512.o
diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
index 58683ef614..82c5c745d4 100644
--- a/lib/crypto/pkcs7_verify.c
+++ b/lib/crypto/pkcs7_verify.c
@@ -15,7 +15,7 @@
 #include <linux/bitops.h>
 #include <linux/compat.h>
 #include <linux/asn1.h>
-#include <u-boot/rsa-checksum.h>
+#include <u-boot/hash-checksum.h>
 #include <crypto/public_key.h>
 #include <crypto/pkcs7_parser.h>
 #else
diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c
index 91810a8640..d557ab27ae 100644
--- a/lib/crypto/x509_public_key.c
+++ b/lib/crypto/x509_public_key.c
@@ -19,7 +19,7 @@
 #include <linux/kernel.h>
 #ifdef __UBOOT__
 #include <crypto/x509_parser.h>
-#include <u-boot/rsa-checksum.h>
+#include <u-boot/hash-checksum.h>
 #else
 #include <linux/slab.h>
 #include <keys/asymmetric-subtype.h>
diff --git a/lib/rsa/rsa-checksum.c b/lib/hash-checksum.c
similarity index 96%
rename from lib/rsa/rsa-checksum.c
rename to lib/hash-checksum.c
index e60debb7df..d732ecc38f 100644
--- a/lib/rsa/rsa-checksum.c
+++ b/lib/hash-checksum.c
@@ -13,7 +13,8 @@
 #else
 #include "fdt_host.h"
 #endif
-#include <u-boot/rsa.h>
+#include <hash.h>
+#include <image.h>
 
 int hash_calculate(const char *name,
 		    const struct image_region region[],
diff --git a/lib/rsa/Makefile b/lib/rsa/Makefile
index 8b75d41f04..c9ac72c1e2 100644
--- a/lib/rsa/Makefile
+++ b/lib/rsa/Makefile
@@ -5,6 +5,6 @@
 # (C) Copyright 2000-2007
 # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
 
-obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY) += rsa-verify.o rsa-checksum.o
+obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY) += rsa-verify.o
 obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY_WITH_PKEY) += rsa-keyprop.o
 obj-$(CONFIG_RSA_SOFTWARE_EXP) += rsa-mod-exp.o
diff --git a/tools/Makefile b/tools/Makefile
index 2d550432ba..96316ed729 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -67,7 +67,7 @@ LIBFDT_OBJS := $(addprefix libfdt/, fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o \
 		fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o)
 
 RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
-					rsa-sign.o rsa-verify.o rsa-checksum.o \
+					rsa-sign.o rsa-verify.o \
 					rsa-mod-exp.o)
 
 AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
@@ -106,6 +106,7 @@ dumpimage-mkimage-objs := aisimage.o \
 			socfpgaimage.o \
 			sunxi_egon.o \
 			lib/crc16.o \
+			lib/hash-checksum.o \
 			lib/sha1.o \
 			lib/sha256.o \
 			lib/sha512.o \
-- 
2.26.2

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

* [PATCH v6 02/11] lib/rsa: Make fdt_add_bignum() available outside of RSA code
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
  2021-02-19 18:45 ` [PATCH v6 01/11] lib: Rename rsa-checksum.c to hash-checksum.c Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:24   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 03/11] lib: Add support for ECDSA image signing Alexandru Gagniuc
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 UTC (permalink / raw)
  To: u-boot

fdt_add_bignum() is useful for algorithms other than just RSA. To
allow its use for ECDSA, move it to a common file under lib/.

The new file is suffixed with '-libcrypto' because it has a direct
dependency on openssl. This is due to the use of the "BIGNUM *" type.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 include/u-boot/fdt-libcrypto.h | 27 +++++++++++++
 lib/fdt-libcrypto.c            | 72 ++++++++++++++++++++++++++++++++++
 lib/rsa/rsa-sign.c             | 65 +-----------------------------
 tools/Makefile                 |  5 +++
 4 files changed, 105 insertions(+), 64 deletions(-)
 create mode 100644 include/u-boot/fdt-libcrypto.h
 create mode 100644 lib/fdt-libcrypto.c

diff --git a/include/u-boot/fdt-libcrypto.h b/include/u-boot/fdt-libcrypto.h
new file mode 100644
index 0000000000..5142f37039
--- /dev/null
+++ b/include/u-boot/fdt-libcrypto.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me@gmail.com>
+ * Copyright (c) 2013, Google Inc.
+ */
+
+#ifndef _FDT_LIBCRYPTO_H
+#define _FDT_LIBCRYPTO_H
+
+#include <openssl/bn.h>
+
+/**
+ * fdt_add_bignum() - Write a libcrypto BIGNUM as an FDT property
+ *
+ * Convert a libcrypto BIGNUM * into a big endian array of integers.
+ *
+ * @blob:	FDT blob to modify
+ * @noffset:	Offset of the FDT node
+ * @prop_name:	What to call the property in the FDT
+ * @num:	pointer to a libcrypto big number
+ * @num_bits:	How big is 'num' in bits?
+ * @return 0 if all good all working, -ve on horror
+ */
+int fdt_add_bignum(void *blob, int noffset, const char *prop_name,
+		   BIGNUM *num, int num_bits);
+
+#endif /* _FDT_LIBCRYPTO_H */
diff --git a/lib/fdt-libcrypto.c b/lib/fdt-libcrypto.c
new file mode 100644
index 0000000000..ecb0344c8f
--- /dev/null
+++ b/lib/fdt-libcrypto.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me@gmail.com>
+ * Copyright (c) 2013, Google Inc.
+ */
+
+#include <libfdt.h>
+#include <u-boot/fdt-libcrypto.h>
+
+int fdt_add_bignum(void *blob, int noffset, const char *prop_name,
+		   BIGNUM *num, int num_bits)
+{
+	int nwords = num_bits / 32;
+	int size;
+	uint32_t *buf, *ptr;
+	BIGNUM *tmp, *big2, *big32, *big2_32;
+	BN_CTX *ctx;
+	int ret;
+
+	tmp = BN_new();
+	big2 = BN_new();
+	big32 = BN_new();
+	big2_32 = BN_new();
+
+	/*
+	 * Note: This code assumes that all of the above succeed, or all fail.
+	 * In practice memory allocations generally do not fail (unless the
+	 * process is killed), so it does not seem worth handling each of these
+	 * as a separate case. Technicaly this could leak memory on failure,
+	 * but a) it won't happen in practice, and b) it doesn't matter as we
+	 * will immediately exit with a failure code.
+	 */
+	if (!tmp || !big2 || !big32 || !big2_32) {
+		fprintf(stderr, "Out of memory (bignum)\n");
+		return -ENOMEM;
+	}
+	ctx = BN_CTX_new();
+	if (!ctx) {
+		fprintf(stderr, "Out of memory (bignum context)\n");
+		return -ENOMEM;
+	}
+	BN_set_word(big2, 2L);
+	BN_set_word(big32, 32L);
+	BN_exp(big2_32, big2, big32, ctx); /* B = 2^32 */
+
+	size = nwords * sizeof(uint32_t);
+	buf = malloc(size);
+	if (!buf) {
+		fprintf(stderr, "Out of memory (%d bytes)\n", size);
+		return -ENOMEM;
+	}
+
+	/* Write out modulus as big endian array of integers */
+	for (ptr = buf + nwords - 1; ptr >= buf; ptr--) {
+		BN_mod(tmp, num, big2_32, ctx); /* n = N mod B */
+		*ptr = cpu_to_fdt32(BN_get_word(tmp));
+		BN_rshift(num, num, 32); /*  N = N/B */
+	}
+
+	/*
+	 * We try signing with successively increasing size values, so this
+	 * might fail several times
+	 */
+	ret = fdt_setprop(blob, noffset, prop_name, buf, size);
+	free(buf);
+	BN_free(tmp);
+	BN_free(big2);
+	BN_free(big32);
+	BN_free(big2_32);
+
+	return ret ? -FDT_ERR_NOSPACE : 0;
+}
diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
index 1f0d81bd7a..557c690a6d 100644
--- a/lib/rsa/rsa-sign.c
+++ b/lib/rsa/rsa-sign.c
@@ -9,6 +9,7 @@
 #include <string.h>
 #include <image.h>
 #include <time.h>
+#include <u-boot/fdt-libcrypto.h>
 #include <openssl/bn.h>
 #include <openssl/rsa.h>
 #include <openssl/pem.h>
@@ -680,70 +681,6 @@ int rsa_get_params(RSA *key, uint64_t *exponent, uint32_t *n0_invp,
 	return ret;
 }
 
-static int fdt_add_bignum(void *blob, int noffset, const char *prop_name,
-			  BIGNUM *num, int num_bits)
-{
-	int nwords = num_bits / 32;
-	int size;
-	uint32_t *buf, *ptr;
-	BIGNUM *tmp, *big2, *big32, *big2_32;
-	BN_CTX *ctx;
-	int ret;
-
-	tmp = BN_new();
-	big2 = BN_new();
-	big32 = BN_new();
-	big2_32 = BN_new();
-
-	/*
-	 * Note: This code assumes that all of the above succeed, or all fail.
-	 * In practice memory allocations generally do not fail (unless the
-	 * process is killed), so it does not seem worth handling each of these
-	 * as a separate case. Technicaly this could leak memory on failure,
-	 * but a) it won't happen in practice, and b) it doesn't matter as we
-	 * will immediately exit with a failure code.
-	 */
-	if (!tmp || !big2 || !big32 || !big2_32) {
-		fprintf(stderr, "Out of memory (bignum)\n");
-		return -ENOMEM;
-	}
-	ctx = BN_CTX_new();
-	if (!ctx) {
-		fprintf(stderr, "Out of memory (bignum context)\n");
-		return -ENOMEM;
-	}
-	BN_set_word(big2, 2L);
-	BN_set_word(big32, 32L);
-	BN_exp(big2_32, big2, big32, ctx); /* B = 2^32 */
-
-	size = nwords * sizeof(uint32_t);
-	buf = malloc(size);
-	if (!buf) {
-		fprintf(stderr, "Out of memory (%d bytes)\n", size);
-		return -ENOMEM;
-	}
-
-	/* Write out modulus as big endian array of integers */
-	for (ptr = buf + nwords - 1; ptr >= buf; ptr--) {
-		BN_mod(tmp, num, big2_32, ctx); /* n = N mod B */
-		*ptr = cpu_to_fdt32(BN_get_word(tmp));
-		BN_rshift(num, num, 32); /*  N = N/B */
-	}
-
-	/*
-	 * We try signing with successively increasing size values, so this
-	 * might fail several times
-	 */
-	ret = fdt_setprop(blob, noffset, prop_name, buf, size);
-	free(buf);
-	BN_free(tmp);
-	BN_free(big2);
-	BN_free(big32);
-	BN_free(big2_32);
-
-	return ret ? -FDT_ERR_NOSPACE : 0;
-}
-
 int rsa_add_verify_data(struct image_sign_info *info, void *keydest)
 {
 	BIGNUM *modulus, *r_squared;
diff --git a/tools/Makefile b/tools/Makefile
index 96316ed729..58b13eaf12 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -73,6 +73,10 @@ RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
 AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
 					aes-encrypt.o aes-decrypt.o)
 
+# Cryptographic helpers that depend on openssl/libcrypto
+LIBCRYPTO_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/, \
+					fdt-libcrypto.o)
+
 ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
 
 # common objs for dumpimage and mkimage
@@ -115,6 +119,7 @@ dumpimage-mkimage-objs := aisimage.o \
 			zynqimage.o \
 			zynqmpimage.o \
 			zynqmpbif.o \
+			$(LIBCRYPTO_OBJS-y) \
 			$(LIBFDT_OBJS) \
 			gpimage.o \
 			gpimage-common.o \
-- 
2.26.2

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

* [PATCH v6 03/11] lib: Add support for ECDSA image signing
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
  2021-02-19 18:45 ` [PATCH v6 01/11] lib: Rename rsa-checksum.c to hash-checksum.c Alexandru Gagniuc
  2021-02-19 18:45 ` [PATCH v6 02/11] lib/rsa: Make fdt_add_bignum() available outside of RSA code Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:24   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 04/11] doc: signature.txt: Document devicetree format for ECDSA keys Alexandru Gagniuc
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 UTC (permalink / raw)
  To: u-boot

mkimage supports rsa2048, and rsa4096 signatures. With newer silicon
now supporting hardware-accelerated ECDSA, it makes sense to expand
signing support to elliptic curves.

Implement host-side ECDSA signing and verification with libcrypto.
Device-side implementation of signature verification is beyond the
scope of this patch.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 common/image-sig.c          |  11 +-
 include/image.h             |   3 +
 include/u-boot/ecdsa.h      |  94 +++++++++++
 lib/ecdsa/ecdsa-libcrypto.c | 306 ++++++++++++++++++++++++++++++++++++
 tools/Makefile              |   3 +
 5 files changed, 415 insertions(+), 2 deletions(-)
 create mode 100644 include/u-boot/ecdsa.h
 create mode 100644 lib/ecdsa/ecdsa-libcrypto.c

diff --git a/common/image-sig.c b/common/image-sig.c
index 54f0eb2019..0f8e592aba 100644
--- a/common/image-sig.c
+++ b/common/image-sig.c
@@ -16,6 +16,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 #endif /* !USE_HOSTCC*/
 #include <image.h>
+#include <u-boot/ecdsa.h>
 #include <u-boot/rsa.h>
 #include <u-boot/hash-checksum.h>
 
@@ -83,8 +84,14 @@ struct crypto_algo crypto_algos[] = {
 		.sign = rsa_sign,
 		.add_verify_data = rsa_add_verify_data,
 		.verify = rsa_verify,
-	}
-
+	},
+	{
+		.name = "ecdsa256",
+		.key_len = ECDSA256_BYTES,
+		.sign = ecdsa_sign,
+		.add_verify_data = ecdsa_add_verify_data,
+		.verify = ecdsa_verify,
+	},
 };
 
 struct padding_algo padding_algos[] = {
diff --git a/include/image.h b/include/image.h
index fbe9537c00..37feb5d56f 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1219,16 +1219,19 @@ int calculate_hash(const void *data, int data_len, const char *algo,
 # if defined(CONFIG_FIT_SIGNATURE)
 #  define IMAGE_ENABLE_SIGN	1
 #  define IMAGE_ENABLE_VERIFY	1
+#  define IMAGE_ENABLE_VERIFY_ECDSA	1
 #  define FIT_IMAGE_ENABLE_VERIFY	1
 #  include <openssl/evp.h>
 # else
 #  define IMAGE_ENABLE_SIGN	0
 #  define IMAGE_ENABLE_VERIFY	0
+# define IMAGE_ENABLE_VERIFY_ECDSA	0
 #  define FIT_IMAGE_ENABLE_VERIFY	0
 # endif
 #else
 # define IMAGE_ENABLE_SIGN	0
 # define IMAGE_ENABLE_VERIFY		CONFIG_IS_ENABLED(RSA_VERIFY)
+# define IMAGE_ENABLE_VERIFY_ECDSA	0
 # define FIT_IMAGE_ENABLE_VERIFY	CONFIG_IS_ENABLED(FIT_SIGNATURE)
 #endif
 
diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h
new file mode 100644
index 0000000000..979690d966
--- /dev/null
+++ b/include/u-boot/ecdsa.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me@gmail.com>.
+ */
+
+#ifndef _ECDSA_H
+#define _ECDSA_H
+
+#include <errno.h>
+#include <image.h>
+#include <linux/kconfig.h>
+
+/**
+ * crypto_algo API impementation for ECDSA;
+ * @see "struct crypto_algo"
+ * @{
+ */
+#if IMAGE_ENABLE_SIGN
+/**
+ * sign() - calculate and return signature for given input data
+ *
+ * @info:	Specifies key and FIT information
+ * @data:	Pointer to the input data
+ * @data_len:	Data length
+ * @sigp:	Set to an allocated buffer holding the signature
+ * @sig_len:	Set to length of the calculated hash
+ *
+ * This computes input data signature according to selected algorithm.
+ * Resulting signature value is placed in an allocated buffer, the
+ * pointer is returned as *sigp. The length of the calculated
+ * signature is returned via the sig_len pointer argument. The caller
+ * should free *sigp.
+ *
+ * @return: 0, on success, -ve on error
+ */
+int ecdsa_sign(struct image_sign_info *info, const struct image_region region[],
+	       int region_count, uint8_t **sigp, uint *sig_len);
+
+/**
+ * add_verify_data() - Add verification information to FDT
+ *
+ * Add public key information to the FDT node, suitable for
+ * verification at run-time. The information added depends on the
+ * algorithm being used. I just copypasted this from rsa.h.
+ *
+ * @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
+ */
+int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest);
+#else
+static inline
+int ecdsa_sign(struct image_sign_info *info, const struct image_region region[],
+	       int region_count, uint8_t **sigp, uint *sig_len)
+{
+	return -ENXIO;
+}
+
+static inline
+int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest)
+{
+	return -ENXIO;
+}
+#endif
+
+#if IMAGE_ENABLE_VERIFY_ECDSA
+/**
+ * verify() - Verify a signature against some data
+ *
+ * @info:	Specifies key and FIT information
+ * @data:	Pointer to the input data
+ * @data_len:	Data length
+ * @sig:	Signature
+ * @sig_len:	Number of bytes in signature
+ * @return 0 if verified, -ve on error
+ */
+int ecdsa_verify(struct image_sign_info *info,
+		 const struct image_region region[], int region_count,
+		 uint8_t *sig, uint sig_len);
+#else
+static inline
+int ecdsa_verify(struct image_sign_info *info,
+		 const struct image_region region[], int region_count,
+		 uint8_t *sig, uint sig_len)
+{
+	return -ENXIO;
+}
+#endif
+/** @} */
+
+#define ECDSA256_BYTES	(256 / 8)
+
+#endif
diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c
new file mode 100644
index 0000000000..322880963f
--- /dev/null
+++ b/lib/ecdsa/ecdsa-libcrypto.c
@@ -0,0 +1,306 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * ECDSA image signing implementation using libcrypto backend
+ *
+ * The signature is a binary representation of the (R, S) points, padded to the
+ * key size. The signature will be (2 * key_size_bits) / 8 bytes.
+ *
+ * Deviations from behavior of RSA equivalent:
+ *  - Verification uses private key. This is not technically required, but a
+ *    limitation on how clumsy the openssl API is to use.
+ *  - Handling of keys and key paths:
+ *    - The '-K' key directory option must contain path to the key file,
+ *      instead of the key directory.
+ *    - No assumptions are made about the file extension of the key
+ *    - The 'key-name-hint' property is only used for naming devicetree nodes,
+ *      but is not used for looking up keys on the filesystem.
+ *
+ * Copyright (c) 2020,2021, Alexandru Gagniuc <mr.nuke.me@gmail.com>
+ */
+
+#include <u-boot/ecdsa.h>
+#include <u-boot/fdt-libcrypto.h>
+#include <openssl/ssl.h>
+#include <openssl/ec.h>
+#include <openssl/bn.h>
+
+/* Image signing context for openssl-libcrypto */
+struct signer {
+	EVP_PKEY *evp_key;	/* Pointer to EVP_PKEY object */
+	EC_KEY *ecdsa_key;	/* Pointer to EC_KEY object */
+	void *hash;		/* Pointer to hash used for verification */
+	void *signature;	/* Pointer to output signature. Do not free()!*/
+};
+
+static int alloc_ctx(struct signer *ctx, const struct image_sign_info *info)
+{
+	memset(ctx, 0, sizeof(*ctx));
+
+	if (!OPENSSL_init_ssl(0, NULL)) {
+		fprintf(stderr, "Failure to init SSL library\n");
+		return -1;
+	}
+
+	ctx->hash = malloc(info->checksum->checksum_len);
+	ctx->signature = malloc(info->crypto->key_len * 2);
+
+	if (!ctx->hash || !ctx->signature)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void free_ctx(struct signer *ctx)
+{
+	if (ctx->ecdsa_key)
+		EC_KEY_free(ctx->ecdsa_key);
+
+	if (ctx->evp_key)
+		EVP_PKEY_free(ctx->evp_key);
+
+	if (ctx->hash)
+		free(ctx->hash);
+}
+
+/*
+ * Convert an ECDSA signature to raw format
+ *
+ * openssl DER-encodes 'binary' signatures. We want the signature in a raw
+ * (R, S) point pair. So we have to dance a bit.
+ */
+static void ecdsa_sig_encode_raw(void *buf, const ECDSA_SIG *sig, size_t order)
+{
+	int point_bytes = order;
+	const BIGNUM *r, *s;
+	uintptr_t s_buf;
+
+	ECDSA_SIG_get0(sig, &r, &s);
+	s_buf = (uintptr_t)buf + point_bytes;
+	BN_bn2binpad(r, buf, point_bytes);
+	BN_bn2binpad(s, (void *)s_buf, point_bytes);
+}
+
+/* Get a signature from a raw encoding */
+static ECDSA_SIG *ecdsa_sig_from_raw(void *buf, size_t order)
+{
+	int point_bytes = order;
+	uintptr_t s_buf;
+	ECDSA_SIG *sig;
+	BIGNUM *r, *s;
+
+	sig = ECDSA_SIG_new();
+	if (!sig)
+		return NULL;
+
+	s_buf = (uintptr_t)buf + point_bytes;
+	r = BN_bin2bn(buf, point_bytes, NULL);
+	s = BN_bin2bn((void *)s_buf, point_bytes, NULL);
+	ECDSA_SIG_set0(sig, r, s);
+
+	return sig;
+}
+
+/* ECDSA key size in bytes */
+static size_t ecdsa_key_size_bytes(const EC_KEY *key)
+{
+	const EC_GROUP *group;
+
+	group = EC_KEY_get0_group(key);
+	return EC_GROUP_order_bits(group) / 8;
+}
+
+static int read_key(struct signer *ctx, const char *key_name)
+{
+	FILE *f = fopen(key_name, "r");
+
+	if (!f) {
+		fprintf(stderr, "Can not get key file '%s'\n", key_name);
+		return -ENOENT;
+	}
+
+	ctx->evp_key = PEM_read_PrivateKey(f, NULL, NULL, NULL);
+	fclose(f);
+	if (!ctx->evp_key) {
+		fprintf(stderr, "Can not read key from '%s'\n", key_name);
+		return -EIO;
+	}
+
+	if (EVP_PKEY_id(ctx->evp_key) != EVP_PKEY_EC) {
+		fprintf(stderr, "'%s' is not an ECDSA key\n", key_name);
+		return -EINVAL;
+	}
+
+	ctx->ecdsa_key = EVP_PKEY_get1_EC_KEY(ctx->evp_key);
+	if (!ctx->ecdsa_key)
+		fprintf(stderr, "Can not extract ECDSA key\n");
+
+	return (ctx->ecdsa_key) ? 0 : -EINVAL;
+}
+
+/* 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;
+
+	ret = alloc_ctx(ctx, info);
+	if (ret)
+		return ret;
+
+	ret = read_key(ctx, kname);
+	if (ret)
+		return ret;
+
+	key_len_bytes = ecdsa_key_size_bytes(ctx->ecdsa_key);
+	if (key_len_bytes != info->crypto->key_len) {
+		fprintf(stderr, "Expected a %u-bit key, got %u-bit key\n",
+			info->crypto->key_len * 8, key_len_bytes * 8);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int do_sign(struct signer *ctx, struct image_sign_info *info,
+		   const struct image_region region[], int region_count)
+{
+	const struct checksum_algo *algo = info->checksum;
+	ECDSA_SIG *sig;
+
+	algo->calculate(algo->name, region, region_count, ctx->hash);
+	sig = ECDSA_do_sign(ctx->hash, algo->checksum_len, ctx->ecdsa_key);
+
+	ecdsa_sig_encode_raw(ctx->signature, sig, info->crypto->key_len);
+
+	return 0;
+}
+
+static int ecdsa_check_signature(struct signer *ctx, struct image_sign_info *info)
+{
+	ECDSA_SIG *sig;
+	int okay;
+
+	sig = ecdsa_sig_from_raw(ctx->signature, info->crypto->key_len);
+	if (!sig)
+		return -ENOMEM;
+
+	okay = ECDSA_do_verify(ctx->hash, info->checksum->checksum_len,
+			       sig, ctx->ecdsa_key);
+	if (!okay)
+		fprintf(stderr, "WARNING: Signature is fake news!\n");
+
+	ECDSA_SIG_free(sig);
+	return !okay;
+}
+
+static int do_verify(struct signer *ctx, struct image_sign_info *info,
+		     const struct image_region region[], int region_count,
+		     uint8_t *raw_sig, uint sig_len)
+{
+	const struct checksum_algo *algo = info->checksum;
+
+	if (sig_len != info->crypto->key_len * 2) {
+		fprintf(stderr, "Signature has wrong length\n");
+		return -EINVAL;
+	}
+
+	memcpy(ctx->signature, raw_sig, sig_len);
+	algo->calculate(algo->name, region, region_count, ctx->hash);
+
+	return ecdsa_check_signature(ctx, info);
+}
+
+int ecdsa_sign(struct image_sign_info *info, const struct image_region region[],
+	       int region_count, uint8_t **sigp, uint *sig_len)
+{
+	struct signer ctx;
+	int ret;
+
+	ret = prepare_ctx(&ctx, info);
+	if (ret >= 0) {
+		do_sign(&ctx, info, region, region_count);
+		*sigp = ctx.signature;
+		*sig_len = info->crypto->key_len * 2;
+
+		ret = ecdsa_check_signature(&ctx, info);
+	}
+
+	free_ctx(&ctx);
+	return ret;
+}
+
+int ecdsa_verify(struct image_sign_info *info,
+		 const struct image_region region[], int region_count,
+		 uint8_t *sig, uint sig_len)
+{
+	struct signer ctx;
+	int ret;
+
+	ret = prepare_ctx(&ctx, info);
+	if (ret >= 0)
+		ret = do_verify(&ctx, info, region, region_count, sig, sig_len);
+
+	free_ctx(&ctx);
+	return ret;
+}
+
+static int do_add(struct signer *ctx, void *fdt, const char *key_node_name)
+{
+	int signature_node, key_node, ret, key_bits;
+	const char *curve_name;
+	const EC_GROUP *group;
+	const EC_POINT *point;
+	BIGNUM *x, *y;
+
+	signature_node = fdt_subnode_offset(fdt, 0, FIT_SIG_NODENAME);
+	if (signature_node < 0) {
+		fprintf(stderr, "Could not find 'signature node: %s\n",
+			fdt_strerror(signature_node));
+		return signature_node;
+	}
+
+	key_node = fdt_add_subnode(fdt, signature_node, key_node_name);
+	if (key_node < 0) {
+		fprintf(stderr, "Could not create '%s' node: %s\n",
+			key_node_name, fdt_strerror(key_node));
+		return key_node;
+	}
+
+	group = EC_KEY_get0_group(ctx->ecdsa_key);
+	key_bits = EC_GROUP_order_bits(group);
+	curve_name = OBJ_nid2sn(EC_GROUP_get_curve_name(group));
+	/* Let 'x' and 'y' memory leak by not BN_free()'ing them. */
+	x = BN_new();
+	y = BN_new();
+	point = EC_KEY_get0_public_key(ctx->ecdsa_key);
+	EC_POINT_get_affine_coordinates(group, point, x, y, NULL);
+
+	ret = fdt_setprop_string(fdt, key_node, "ecdsa,curve", curve_name);
+	if (ret < 0)
+		return ret;
+
+	ret = fdt_add_bignum(fdt, key_node, "ecdsa,x-point", x, key_bits);
+	if (ret < 0)
+		return ret;
+
+	ret = fdt_add_bignum(fdt, key_node, "ecdsa,y-point", y, key_bits);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+int ecdsa_add_verify_data(struct image_sign_info *info, void *fdt)
+{
+	const char *fdt_key_name;
+	struct signer ctx;
+	int ret;
+
+	fdt_key_name = info->keyname ? info->keyname : "default-key";
+	ret = prepare_ctx(&ctx, info);
+	if (ret >= 0)
+		do_add(&ctx, fdt, fdt_key_name);
+
+	free_ctx(&ctx);
+	return ret;
+}
diff --git a/tools/Makefile b/tools/Makefile
index 58b13eaf12..90f4c90576 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -70,6 +70,8 @@ RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
 					rsa-sign.o rsa-verify.o \
 					rsa-mod-exp.o)
 
+ECDSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
+
 AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
 					aes-encrypt.o aes-decrypt.o)
 
@@ -124,6 +126,7 @@ dumpimage-mkimage-objs := aisimage.o \
 			gpimage.o \
 			gpimage-common.o \
 			mtk_image.o \
+			$(ECDSA_OBJS-y) \
 			$(RSA_OBJS-y) \
 			$(AES_OBJS-y)
 
-- 
2.26.2

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

* [PATCH v6 04/11] doc: signature.txt: Document devicetree format for ECDSA keys
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
                   ` (2 preceding siblings ...)
  2021-02-19 18:45 ` [PATCH v6 03/11] lib: Add support for ECDSA image signing Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:24   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 05/11] test/py: Add pycryptodomex to list of required pakages Alexandru Gagniuc
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 doc/uImage.FIT/signature.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt
index a3455889ed..0139295d33 100644
--- a/doc/uImage.FIT/signature.txt
+++ b/doc/uImage.FIT/signature.txt
@@ -142,7 +142,7 @@ public key in U-Boot's control FDT (using CONFIG_OF_CONTROL).
 Public keys should be stored as sub-nodes in a /signature node. Required
 properties are:
 
-- algo: Algorithm name (e.g. "sha1,rsa2048")
+- algo: Algorithm name (e.g. "sha1,rsa2048" or "sha256,ecdsa256")
 
 Optional properties are:
 
@@ -167,6 +167,11 @@ For RSA the following are mandatory:
 - rsa,r-squared: (2^num-bits)^2 as a big-endian multi-word integer
 - rsa,n0-inverse: -1 / modulus[0] mod 2^32
 
+For ECDSA the following are mandatory:
+- ecdsa,curve: Name of ECDSA curve (e.g. "prime256v1")
+- ecdsa,x-point: Public key X coordinate as a big-endian multi-word integer
+- ecdsa,y-point: Public key Y coordinate as a big-endian multi-word integer
+
 These parameters can be added to a binary device tree using parameter -K of the
 mkimage command::
 
-- 
2.26.2

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

* [PATCH v6 05/11] test/py: Add pycryptodomex to list of required pakages
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
                   ` (3 preceding siblings ...)
  2021-02-19 18:45 ` [PATCH v6 04/11] doc: signature.txt: Document devicetree format for ECDSA keys Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:24   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 06/11] test/py: ecdsa: Add test for mkimage ECDSA signing Alexandru Gagniuc
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 UTC (permalink / raw)
  To: u-boot

We wish to use pycryptodomex to verify code paths involving ECDSA
signatures. Add it to requirements.txt so that they get picked up
automatically .gitlab and .azure tasks

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 test/py/requirements.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/py/requirements.txt b/test/py/requirements.txt
index 926bccad69..9dea9415a7 100644
--- a/test/py/requirements.txt
+++ b/test/py/requirements.txt
@@ -10,6 +10,7 @@ packaging==19.2
 pbr==5.4.3
 pluggy==0.13.0
 py==1.8.0
+pycryptodomex==3.9.8
 pyelftools==0.27
 pygit2==1.4.0
 pyparsing==2.4.2
-- 
2.26.2

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

* [PATCH v6 06/11] test/py: ecdsa: Add test for mkimage ECDSA signing
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
                   ` (4 preceding siblings ...)
  2021-02-19 18:45 ` [PATCH v6 05/11] test/py: Add pycryptodomex to list of required pakages Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:24   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 07/11] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 UTC (permalink / raw)
  To: u-boot

Add a test to make sure that the ECDSA signatures generated by
mkimage can be verified successfully. pyCryptodomex was chosen as the
crypto library because it integrates much better with python code.
Using openssl would have been unnecessarily painful.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 test/py/tests/test_fit_ecdsa.py | 111 ++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)
 create mode 100644 test/py/tests/test_fit_ecdsa.py

diff --git a/test/py/tests/test_fit_ecdsa.py b/test/py/tests/test_fit_ecdsa.py
new file mode 100644
index 0000000000..f597570281
--- /dev/null
+++ b/test/py/tests/test_fit_ecdsa.py
@@ -0,0 +1,111 @@
+# SPDX-License-Identifier:	GPL-2.0+
+#
+# Copyright (c) 2020,2021 Alexandru Gagniuc <mr.nuke.me@gmail.com>
+
+"""
+Test ECDSA signing of FIT images
+
+This test uses mkimage to sign an existing FIT image with an ECDSA key. The
+signature is then extracted, and verified against pyCryptodome.
+This test doesn't run the sandbox. It only checks the host tool 'mkimage'
+"""
+
+import pytest
+import u_boot_utils as util
+from Cryptodome.Hash import SHA256
+from Cryptodome.PublicKey import ECC
+from Cryptodome.Signature import DSS
+
+class SignableFitImage(object):
+    """ Helper to manipulate a FIT image on disk """
+    def __init__(self, cons, file_name):
+        self.fit = file_name
+        self.cons = cons
+        self.signable_nodes = set()
+
+    def __fdt_list(self, path):
+        return util.run_and_log(self.cons, f'fdtget -l {self.fit} {path}')
+
+    def __fdt_set(self, node, **prop_value):
+        for prop, value in prop_value.items():
+            util.run_and_log(self.cons, f'fdtput -ts {self.fit} {node} {prop} {value}')
+
+    def __fdt_get_binary(self, node, prop):
+        numbers = util.run_and_log(self.cons, f'fdtget -tbi {self.fit} {node} {prop}')
+
+        bignum = bytearray()
+        for little_num in numbers.split():
+            bignum.append(int(little_num))
+
+        return bignum
+
+    def find_signable_image_nodes(self):
+        for node in self.__fdt_list('/images').split():
+            image = f'/images/{node}'
+            if 'signature' in self.__fdt_list(image):
+                self.signable_nodes.add(image)
+
+        return self.signable_nodes
+
+    def change_signature_algo_to_ecdsa(self):
+        for image in self.signable_nodes:
+            self.__fdt_set(f'{image}/signature', algo='sha256,ecdsa256')
+
+    def sign(self, mkimage, key_file):
+        util.run_and_log(self.cons, [mkimage, '-F', self.fit, f'-k{key_file}'])
+
+    def check_signatures(self, key):
+        for image in self.signable_nodes:
+            raw_sig = self.__fdt_get_binary(f'{image}/signature', 'value')
+            raw_bin = self.__fdt_get_binary(image, 'data')
+
+            sha = SHA256.new(raw_bin)
+            verifier = DSS.new(key, 'fips-186-3')
+            verifier.verify(sha, bytes(raw_sig))
+
+
+ at pytest.mark.buildconfigspec('fit_signature')
+ at pytest.mark.requiredtool('dtc')
+ at pytest.mark.requiredtool('fdtget')
+ at pytest.mark.requiredtool('fdtput')
+def test_fit_ecdsa(u_boot_console):
+    """ Test that signatures generated by mkimage are legible. """
+    def generate_ecdsa_key():
+        return ECC.generate(curve='prime256v1')
+
+    def assemble_fit_image(dest_fit, its, destdir):
+        dtc_args = f'-I dts -O dtb -i {destdir}'
+        util.run_and_log(cons, [mkimage, '-D', dtc_args, '-f', its, dest_fit])
+
+    def dtc(dts):
+        dtb = dts.replace('.dts', '.dtb')
+        util.run_and_log(cons, f'dtc {datadir}/{dts} -O dtb -o {tempdir}/{dtb}')
+
+    cons = u_boot_console
+    mkimage = cons.config.build_dir + '/tools/mkimage'
+    datadir = cons.config.source_dir + '/test/py/tests/vboot/'
+    tempdir = cons.config.result_dir
+    key_file = f'{tempdir}/ecdsa-test-key.pem'
+    fit_file = f'{tempdir}/test.fit'
+    dtc('sandbox-kernel.dts')
+
+    key = generate_ecdsa_key()
+
+    # Create a fake kernel image -- zeroes will do just fine
+    with open(f'{tempdir}/test-kernel.bin', 'w') as fd:
+        fd.write(500 * chr(0))
+
+    # invocations of mkimage expect to read the key from disk
+    with open(key_file, 'w') as f:
+        f.write(key.export_key(format='PEM'))
+
+    assemble_fit_image(fit_file, f'{datadir}/sign-images-sha256.its', tempdir)
+
+    fit = SignableFitImage(cons, fit_file)
+    nodes = fit.find_signable_image_nodes()
+    if len(nodes) == 0:
+        raise ValueError('FIT image has no "/image" nodes with "signature"')
+
+    fit.change_signature_algo_to_ecdsa()
+    fit.sign(mkimage, key_file)
+    fit.check_signatures(key)
-- 
2.26.2

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

* [PATCH v6 07/11] doc: signature.txt: Document the keydir and keyfile arguments
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
                   ` (5 preceding siblings ...)
  2021-02-19 18:45 ` [PATCH v6 06/11] test/py: ecdsa: Add test for mkimage ECDSA signing Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:25   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 08/11] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 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] 24+ messages in thread

* [PATCH v6 08/11] mkimage: Add a 'keyfile' argument for image signing
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
                   ` (6 preceding siblings ...)
  2021-02-19 18:45 ` [PATCH v6 07/11] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:25   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 09/11] lib/rsa: Use the 'keyfile' argument from mkimage Alexandru Gagniuc
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 include/image.h    | 16 +++++++++----
 tools/fit_image.c  |  3 ++-
 tools/image-host.c | 58 +++++++++++++++++++++++++---------------------
 tools/imagetool.h  |  1 +
 tools/mkimage.c    |  6 ++++-
 5 files changed, 52 insertions(+), 32 deletions(-)

diff --git a/include/image.h b/include/image.h
index 37feb5d56f..5bb7922a66 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1131,9 +1131,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);
@@ -1251,10 +1252,17 @@ void image_set_host_blob(void *host_blob);
 #endif
 #endif /* IMAGE_ENABLE_FIT */
 
-/* Information passed to the signing routines */
+/*
+ * Information passed to the signing routines
+ *
+ * Either 'keydir',  'keyname', or 'keyfile' can be NULL. However, either
+ * 'keyfile', or both 'keydir' and 'keyname' should have valid values. If
+ * neither are valid, some operations might fail with EINVAL.
+ */
 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 d440d143c6..ae30f80783 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] 24+ messages in thread

* [PATCH v6 09/11] lib/rsa: Use the 'keyfile' argument from mkimage
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
                   ` (7 preceding siblings ...)
  2021-02-19 18:45 ` [PATCH v6 08/11] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:25   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 10/11] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate Alexandru Gagniuc
  2021-02-19 18:45 ` [PATCH v6 11/11] test/py: ecdsa: Use mkimage keyfile instead of keydir argument Alexandru Gagniuc
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 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] 24+ messages in thread

* [PATCH v6 10/11] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
                   ` (8 preceding siblings ...)
  2021-02-19 18:45 ` [PATCH v6 09/11] lib/rsa: Use the 'keyfile' argument from mkimage Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-04-16 12:25   ` Tom Rini
  2021-02-19 18:45 ` [PATCH v6 11/11] test/py: ecdsa: Use mkimage keyfile instead of keydir argument Alexandru Gagniuc
  10 siblings, 1 reply; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 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] 24+ messages in thread

* [PATCH v6 11/11] test/py: ecdsa: Use mkimage keyfile instead of keydir argument
  2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
                   ` (9 preceding siblings ...)
  2021-02-19 18:45 ` [PATCH v6 10/11] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate Alexandru Gagniuc
@ 2021-02-19 18:45 ` Alexandru Gagniuc
  2021-02-20 11:55   ` Simon Glass
  2021-04-16 12:25   ` Tom Rini
  10 siblings, 2 replies; 24+ messages in thread
From: Alexandru Gagniuc @ 2021-02-19 18:45 UTC (permalink / raw)
  To: u-boot

Originally, the ECDSA code path used 'keydir' as the key filename.
mkimage has since been updated to include a new 'keyfile' argument.
Use the new argument for passing in the key.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 test/py/tests/test_fit_ecdsa.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/py/tests/test_fit_ecdsa.py b/test/py/tests/test_fit_ecdsa.py
index f597570281..87b6081222 100644
--- a/test/py/tests/test_fit_ecdsa.py
+++ b/test/py/tests/test_fit_ecdsa.py
@@ -52,7 +52,7 @@ class SignableFitImage(object):
             self.__fdt_set(f'{image}/signature', algo='sha256,ecdsa256')
 
     def sign(self, mkimage, key_file):
-        util.run_and_log(self.cons, [mkimage, '-F', self.fit, f'-k{key_file}'])
+        util.run_and_log(self.cons, [mkimage, '-F', self.fit, f'-G{key_file}'])
 
     def check_signatures(self, key):
         for image in self.signable_nodes:
-- 
2.26.2

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

* [PATCH v6 11/11] test/py: ecdsa: Use mkimage keyfile instead of keydir argument
  2021-02-19 18:45 ` [PATCH v6 11/11] test/py: ecdsa: Use mkimage keyfile instead of keydir argument Alexandru Gagniuc
@ 2021-02-20 11:55   ` Simon Glass
  2021-04-16 12:25   ` Tom Rini
  1 sibling, 0 replies; 24+ messages in thread
From: Simon Glass @ 2021-02-20 11:55 UTC (permalink / raw)
  To: u-boot

On Fri, 19 Feb 2021 at 11:45, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> Originally, the ECDSA code path used 'keydir' as the key filename.
> mkimage has since been updated to include a new 'keyfile' argument.
> Use the new argument for passing in the key.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  test/py/tests/test_fit_ecdsa.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

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

* [PATCH v6 01/11] lib: Rename rsa-checksum.c to hash-checksum.c
  2021-02-19 18:45 ` [PATCH v6 01/11] lib: Rename rsa-checksum.c to hash-checksum.c Alexandru Gagniuc
@ 2021-04-16 12:24   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:10PM -0600, Alexandru Gagniuc wrote:

> rsa-checksum.c sontains the hash_calculate() implementations. Despite
> the "rsa-" file prefix, this function is useful for other algorithms.
> 
> To prevent confusion, move this file to lib/, and rename it to
> hash-checksum.c, to give it a more "generic" feel.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/b46480cf/attachment.sig>

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

* [PATCH v6 02/11] lib/rsa: Make fdt_add_bignum() available outside of RSA code
  2021-02-19 18:45 ` [PATCH v6 02/11] lib/rsa: Make fdt_add_bignum() available outside of RSA code Alexandru Gagniuc
@ 2021-04-16 12:24   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:11PM -0600, Alexandru Gagniuc wrote:

> fdt_add_bignum() is useful for algorithms other than just RSA. To
> allow its use for ECDSA, move it to a common file under lib/.
> 
> The new file is suffixed with '-libcrypto' because it has a direct
> dependency on openssl. This is due to the use of the "BIGNUM *" type.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/e54496e2/attachment.sig>

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

* [PATCH v6 03/11] lib: Add support for ECDSA image signing
  2021-02-19 18:45 ` [PATCH v6 03/11] lib: Add support for ECDSA image signing Alexandru Gagniuc
@ 2021-04-16 12:24   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:12PM -0600, Alexandru Gagniuc wrote:

> mkimage supports rsa2048, and rsa4096 signatures. With newer silicon
> now supporting hardware-accelerated ECDSA, it makes sense to expand
> signing support to elliptic curves.
> 
> Implement host-side ECDSA signing and verification with libcrypto.
> Device-side implementation of signature verification is beyond the
> scope of this patch.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/0e08c375/attachment.sig>

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

* [PATCH v6 04/11] doc: signature.txt: Document devicetree format for ECDSA keys
  2021-02-19 18:45 ` [PATCH v6 04/11] doc: signature.txt: Document devicetree format for ECDSA keys Alexandru Gagniuc
@ 2021-04-16 12:24   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:13PM -0600, Alexandru Gagniuc wrote:

> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/9903cac7/attachment.sig>

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

* [PATCH v6 05/11] test/py: Add pycryptodomex to list of required pakages
  2021-02-19 18:45 ` [PATCH v6 05/11] test/py: Add pycryptodomex to list of required pakages Alexandru Gagniuc
@ 2021-04-16 12:24   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:14PM -0600, Alexandru Gagniuc wrote:

> We wish to use pycryptodomex to verify code paths involving ECDSA
> signatures. Add it to requirements.txt so that they get picked up
> automatically .gitlab and .azure tasks
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/69ad42e2/attachment.sig>

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

* [PATCH v6 06/11] test/py: ecdsa: Add test for mkimage ECDSA signing
  2021-02-19 18:45 ` [PATCH v6 06/11] test/py: ecdsa: Add test for mkimage ECDSA signing Alexandru Gagniuc
@ 2021-04-16 12:24   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:15PM -0600, Alexandru Gagniuc wrote:

> Add a test to make sure that the ECDSA signatures generated by
> mkimage can be verified successfully. pyCryptodomex was chosen as the
> crypto library because it integrates much better with python code.
> Using openssl would have been unnecessarily painful.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/a4f965ac/attachment.sig>

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

* [PATCH v6 07/11] doc: signature.txt: Document the keydir and keyfile arguments
  2021-02-19 18:45 ` [PATCH v6 07/11] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
@ 2021-04-16 12:25   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:25 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:16PM -0600, Alexandru Gagniuc 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>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/1cf2db16/attachment.sig>

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

* [PATCH v6 08/11] mkimage: Add a 'keyfile' argument for image signing
  2021-02-19 18:45 ` [PATCH v6 08/11] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
@ 2021-04-16 12:25   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:25 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:17PM -0600, Alexandru Gagniuc 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>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/3ffc6fc3/attachment.sig>

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

* [PATCH v6 09/11] lib/rsa: Use the 'keyfile' argument from mkimage
  2021-02-19 18:45 ` [PATCH v6 09/11] lib/rsa: Use the 'keyfile' argument from mkimage Alexandru Gagniuc
@ 2021-04-16 12:25   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:25 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:18PM -0600, Alexandru Gagniuc 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>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/82c4cb1f/attachment.sig>

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

* [PATCH v6 10/11] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate
  2021-02-19 18:45 ` [PATCH v6 10/11] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate Alexandru Gagniuc
@ 2021-04-16 12:25   ` Tom Rini
  0 siblings, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:25 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:19PM -0600, Alexandru Gagniuc 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>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/92f05818/attachment.sig>

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

* [PATCH v6 11/11] test/py: ecdsa: Use mkimage keyfile instead of keydir argument
  2021-02-19 18:45 ` [PATCH v6 11/11] test/py: ecdsa: Use mkimage keyfile instead of keydir argument Alexandru Gagniuc
  2021-02-20 11:55   ` Simon Glass
@ 2021-04-16 12:25   ` Tom Rini
  1 sibling, 0 replies; 24+ messages in thread
From: Tom Rini @ 2021-04-16 12:25 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 19, 2021 at 12:45:20PM -0600, Alexandru Gagniuc wrote:

> Originally, the ECDSA code path used 'keydir' as the key filename.
> mkimage has since been updated to include a new 'keyfile' argument.
> Use the new argument for passing in the key.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/2bfce8d9/attachment.sig>

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

end of thread, other threads:[~2021-04-16 12:25 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19 18:45 [PATCH v6 00/11] Add support for ECDSA image signing Alexandru Gagniuc
2021-02-19 18:45 ` [PATCH v6 01/11] lib: Rename rsa-checksum.c to hash-checksum.c Alexandru Gagniuc
2021-04-16 12:24   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 02/11] lib/rsa: Make fdt_add_bignum() available outside of RSA code Alexandru Gagniuc
2021-04-16 12:24   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 03/11] lib: Add support for ECDSA image signing Alexandru Gagniuc
2021-04-16 12:24   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 04/11] doc: signature.txt: Document devicetree format for ECDSA keys Alexandru Gagniuc
2021-04-16 12:24   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 05/11] test/py: Add pycryptodomex to list of required pakages Alexandru Gagniuc
2021-04-16 12:24   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 06/11] test/py: ecdsa: Add test for mkimage ECDSA signing Alexandru Gagniuc
2021-04-16 12:24   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 07/11] doc: signature.txt: Document the keydir and keyfile arguments Alexandru Gagniuc
2021-04-16 12:25   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 08/11] mkimage: Add a 'keyfile' argument for image signing Alexandru Gagniuc
2021-04-16 12:25   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 09/11] lib/rsa: Use the 'keyfile' argument from mkimage Alexandru Gagniuc
2021-04-16 12:25   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 10/11] lib/ecdsa: Use the 'keydir' argument from mkimage if appropriate Alexandru Gagniuc
2021-04-16 12:25   ` Tom Rini
2021-02-19 18:45 ` [PATCH v6 11/11] test/py: ecdsa: Use mkimage keyfile instead of keydir argument Alexandru Gagniuc
2021-02-20 11:55   ` Simon Glass
2021-04-16 12:25   ` Tom Rini

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.