[PATCH v6 00/11] Add support for ECDSA image signing

## 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

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@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 \

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!

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 \

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!

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)

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!

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::

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!

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

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!

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)) + + +@pytest.mark.buildconfigspec('fit_signature') +@pytest.mark.requiredtool('dtc') +@pytest.mark.requiredtool('fdtget') +@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)

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!

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 -----------------------------------------

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!

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, ®ion_count, ®ion_prop, ®ion_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;

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!

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,

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!

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)

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!

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:

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

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!
participants (3)
-
Alexandru Gagniuc
-
Simon Glass
-
Tom Rini