Add ECDSA support for FIT image verification

Generating dts snippets for ECDSA keys does not work properly, there are various endianess problems. As we do not need this currently for barebox drop the support for now.

Signed-off-by Sascha Hauer <[email protected]>

---
 scripts/keytoc.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

scripts/keytoc.c +5 -9

index 60e177aeee..3ac42bc6d0 100644
@@ -470,15 +470,8 @@ static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *key_na
 		return -EINVAL;
 
 	if (dts) {
-		fprintf(outfilep, "\t\tkey-%s {\n", key_name_c);
-		fprintf(outfilep, "\t\t\tecdsa,x-point = <");
-		print_bignum(key_x, bits);
-		fprintf(outfilep, ">;\n");
-		fprintf(outfilep, "\t\t\tecdsa,y-point = <");
-		print_bignum(key_y, bits);
-		fprintf(outfilep, ">;\n");
-		fprintf(outfilep, "\t\t\tecdsa,curve = \"%s\";\n", group);
-		fprintf(outfilep, "\t\t};\n");
+		fprintf(stderr, "ERROR: generating a dts snippet for ECDSA keys is not yet supported\n");
+		return -EOPNOTSUPP;
 	} else {
 		fprintf(outfilep, "\nstatic uint32_t %s_x[] = {", key_name_c);
 		print_bignum(key_x, bits);

@@ -598,6 +591,9 @@ static int gen_key(const char *keyname, const char *path)
 	}
 
 	ret = gen_key_ecdsa(key, keyname, key_name_c);
+	if (ret == -EOPNOTSUPP)
+		return ret;
+
 	if (ret)
 		ret = gen_key_rsa(key, keyname, key_name_c);
 

In case of an error in gen_key() return an error code so that the keytoc tool returns an error.

Signed-off-by Sascha Hauer <[email protected]>

---
 scripts/keytoc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/keytoc.c +1 -1

index 3ac42bc6d0..7e73c786a3 100644
@@ -597,7 +597,7 @@ static int gen_key(const char *keyname, const char *path)
 	if (ret)
 		ret = gen_key_rsa(key, keyname, key_name_c);
 
-	return 0;
+	return ret;
 }
 
 int main(int argc, char *argv[])

We print the ECDSA key values out as an uint32_t C array. They are used in barebox as a uint64_t C array, so when the endianess of the build system differs from the system barebox runs on we end up with the 32bit words swapped in the u64 array. Fix this by printing out the key values as an uint64_t array.

Signed-off-by Sascha Hauer <[email protected]>

---
 scripts/keytoc.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

scripts/keytoc.c +21 -21

index 7e73c786a3..b140160688 100644
@@ -315,12 +315,12 @@ static int rsa_get_params(EVP_PKEY *key, uint64_t *exponent, uint32_t *n0_invp,
 
 static FILE *outfilep;
 
-static int print_bignum(BIGNUM *num, int num_bits)
+static int print_bignum(BIGNUM *num, int num_bits, int width)
 {
 	BIGNUM *tmp, *big2, *big32, *big2_32;
 	BN_CTX *ctx;
 	int i;
-	uint32_t *arr;
+	uint64_t *arr;
 
 	tmp = BN_new();
 	big2 = BN_new();

@@ -345,34 +345,34 @@ static int print_bignum(BIGNUM *num, int num_bits)
 		return -ENOMEM;
 	}
 	BN_set_word(big2, 2L);
-	BN_set_word(big32, 32L);
-	BN_exp(big2_32, big2, big32, ctx); /* B = 2^32 */
+	BN_set_word(big32, width);
+	BN_exp(big2_32, big2, big32, ctx); /* B = 2^width */
 
-	arr = malloc(num_bits / 32 * sizeof(uint32_t));
+	arr = malloc(num_bits / width * sizeof(*arr));
 
-	for (i = 0; i < num_bits / 32; i++) {
+	for (i = 0; i < num_bits / width; i++) {
 		BN_mod(tmp, num, big2_32, ctx); /* n = N mod B */
 		arr[i] = BN_get_word(tmp);
-		BN_rshift(num, num, 32); /*  N = N/B */
+		BN_rshift(num, num, width); /*  N = N/B */
 	}
 
 	if (dts) {
-		for (i = 0; i < num_bits / 32; i++) {
+		for (i = 0; i < num_bits / width; i++) {
 			if (i % 4)
 				fprintf(outfilep, " ");
 			else
 				fprintf(outfilep, "\n\t\t\t\t");
-			fprintf(outfilep, "0x%08x", arr[num_bits / 32 - 1 - i]);
-			BN_rshift(num, num, 32); /*  N = N/B */
+			fprintf(outfilep, "0x%0*jx", width / 4, arr[num_bits / width - 1 - i]);
+			BN_rshift(num, num, width); /*  N = N/B */
 		}
 	} else {
-		for (i = 0; i < num_bits / 32; i++) {
+		for (i = 0; i < num_bits / width; i++) {
 			if (i % 4)
 				fprintf(outfilep, " ");
 			else
 				fprintf(outfilep, "\n\t");
-			fprintf(outfilep, "0x%08x,", arr[i]);
-			BN_rshift(num, num, 32); /*  N = N/B */
+			fprintf(outfilep, "0x%0*jx,", width / 4, arr[i]);
+			BN_rshift(num, num, width); /*  N = N/B */
 		}
 	}
 

@@ -473,12 +473,12 @@ static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *key_na
 		fprintf(stderr, "ERROR: generating a dts snippet for ECDSA keys is not yet supported\n");
 		return -EOPNOTSUPP;
 	} else {
-		fprintf(outfilep, "\nstatic uint32_t %s_x[] = {", key_name_c);
-		print_bignum(key_x, bits);
+		fprintf(outfilep, "\nstatic uint64_t %s_x[] = {", key_name_c);
+		print_bignum(key_x, bits, 64);
 		fprintf(outfilep, "\n};\n\n");
 
-		fprintf(outfilep, "static uint32_t %s_y[] = {", key_name_c);
-		print_bignum(key_y, bits);
+		fprintf(outfilep, "static uint64_t %s_y[] = {", key_name_c);
+		print_bignum(key_y, bits, 64);
 		fprintf(outfilep, "\n};\n\n");
 
 		fprintf(outfilep, "static struct ecdsa_public_key %s = {\n", key_name_c);

@@ -512,10 +512,10 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *key_name
 	if (dts) {
 		fprintf(outfilep, "\t\tkey-%s {\n", key_name_c);
 		fprintf(outfilep, "\t\t\trsa,r-squared = <");
-		print_bignum(r_squared, bits);
+		print_bignum(r_squared, bits, 32);
 		fprintf(outfilep, ">;\n");
 		fprintf(outfilep, "\t\t\trsa,modulus= <");
-		print_bignum(modulus, bits);
+		print_bignum(modulus, bits, 32);
 		fprintf(outfilep, ">;\n");
 		fprintf(outfilep, "\t\t\trsa,exponent = <0x%0lx 0x%lx>;\n",
 			(exponent >> 32) & 0xffffffff,

@@ -526,11 +526,11 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *key_name
 		fprintf(outfilep, "\t\t};\n");
 	} else {
 		fprintf(outfilep, "\nstatic uint32_t %s_modulus[] = {", key_name_c);
-		print_bignum(modulus, bits);
+		print_bignum(modulus, bits, 32);
 		fprintf(outfilep, "\n};\n\n");
 
 		fprintf(outfilep, "static uint32_t %s_rr[] = {", key_name_c);
-		print_bignum(r_squared, bits);
+		print_bignum(r_squared, bits, 32);
 		fprintf(outfilep, "\n};\n\n");
 
 		if (standalone) {

Signed-off-by Sascha Hauer <[email protected]>

---
 scripts/keytoc.c | 11 -----------
 1 file changed, 11 deletions(-)

scripts/keytoc.c +0 -11

index b140160688..ecb33cbe47 100644
@@ -664,17 +664,6 @@ int main(int argc, char *argv[])
 		*path = 0;
 		path++;
 
-		if (!strncmp(path, "__ENV__", 7)) {
-			const char *orig_path = path;
-
-			path = getenv(orig_path + 7);
-			if (!path) {
-				fprintf(stderr, "%s doesn't contain a path\n",
-					orig_path + 7);
-				exit(1);
-			}
-		}
-
 		ret = gen_key(keyname, path);
 		if (ret)
 			exit(1);

We have:

$(if $(RSA_DEP),$<,$(CONFIG_CRYPTO_RSA_KEY))

'$<' is the first dependency which is $(RSA_DEP). $(RSA_DEP) is either empty (in which case $(CONFIG_CRYPTO_RSA_KEY) is used), or set to $(CONFIG_CRYPTO_RSA_KEY) a few lines up.
so all this is a complicated way of using $(CONFIG_CRYPTO_RSA_KEY) directly.

Signed-off-by Sascha Hauer <[email protected]>

---
 crypto/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

crypto/Makefile +1 -1

index e84360a8c7..d7a06a721d 100644
@@ -34,5 +34,5 @@ RSA_DEP := $(CONFIG_CRYPTO_RSA_KEY)
 endif
 
 $(obj)/rsa-keys.h: $(RSA_DEP) FORCE
-	$(call cmd,public_keys,$(CONFIG_CRYPTO_RSA_KEY_NAME_HINT):$(if $(RSA_DEP),$<,$(CONFIG_CRYPTO_RSA_KEY)))
+	$(call cmd,public_keys,$(CONFIG_CRYPTO_RSA_KEY_NAME_HINT):$(CONFIG_CRYPTO_RSA_KEY))
 endif

Having rsa-keys.h depend on the input key files is unnecessary. They were introduced to avoid unnecessary rebuilds when the files didn't change. This is unnecessary because the public_keys cmd won't produce output when it wouldn't change the existing file.

Signed-off-by Sascha Hauer <[email protected]>

---
 crypto/Makefile | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

crypto/Makefile +1 -5

index d7a06a721d..5a484355f3 100644
@@ -29,10 +29,6 @@ $(obj)/rsa.o: $(obj)/rsa-keys.h
 
 CONFIG_CRYPTO_RSA_KEY := $(CONFIG_CRYPTO_RSA_KEY:"%"=%)
 
-ifneq ($(filter-out pkcs11:% __ENV__%, $(CONFIG_CRYPTO_RSA_KEY)),)
-RSA_DEP := $(CONFIG_CRYPTO_RSA_KEY)
-endif
-
-$(obj)/rsa-keys.h: $(RSA_DEP) FORCE
+$(obj)/rsa-keys.h: FORCE
 	$(call cmd,public_keys,$(CONFIG_CRYPTO_RSA_KEY_NAME_HINT):$(CONFIG_CRYPTO_RSA_KEY))
 endif

The key name hint is used in barebox to find a key without iterating over all keys, but it's not required. Make the key name hint optional to keytoc.

Signed-off-by Sascha Hauer <[email protected]>

---
 scripts/keytoc.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

scripts/keytoc.c +22 -9

index ecb33cbe47..85a7cd7319 100644
@@ -9,6 +9,7 @@
  *
  */
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <string.h>
 #include <time.h>

@@ -602,9 +603,9 @@ static int gen_key(const char *keyname, const char *path)
 
 int main(int argc, char *argv[])
 {
-	char *path, *keyname;
 	int i, opt, ret;
 	char *outfile = NULL;
+	int keynum = 1;
 
 	outfilep = stdout;
 

@@ -652,21 +653,33 @@ int main(int argc, char *argv[])
 	}
 
 	for (i = optind; i < argc; i++) {
-		keyname = argv[i];
+		char *keyspec = argv[i];
+		char *keyname = NULL;
+		char *path, *freep = NULL;
 
-		path = strchr(keyname, ':');
-		if (!path) {
-			fprintf(stderr,
-				"keys must be given as <key_name_hint>:<crt>\n");
-			exit(1);
+		if (!strncmp(keyspec, "pkcs11:", 7)) {
+			path = keyspec;
+		} else {
+			path = strchr(keyspec, ':');
+			if (path) {
+				*path = 0;
+				path++;
+				keyname = keyspec;
+			} else {
+				path = keyspec;
+			}
 		}
 
-		*path = 0;
-		path++;
+		if (!keyname) {
+			asprintf(&freep, "key_%d", keynum++);
+			keyname = freep;
+		}
 
 		ret = gen_key(keyname, path);
 		if (ret)
 			exit(1);
+
+		free(freep);
 	}
 
 	if (dts) {

Set the key name hint in CONFIG_CRYPTO_RSA_KEY. CONFIG_CRYPTO_RSA_KEY now has the form:

<key_name_hint>:<filename/uri>

This is done in preparation to make CONFIG_CRYPTO_RSA_KEY a list of keys.

Signed-off-by Sascha Hauer <[email protected]>

---
 crypto/Kconfig  | 9 ++-------
 crypto/Makefile | 2 +-
 2 files changed, 3 insertions(+), 8 deletions(-)

crypto/Kconfig +2 -7

index 78b499f646..64a016eb2c 100644
@@ -130,17 +130,12 @@ config CRYPTO_RSA_KEY
 	  This option should be a filename of a PEM-formatted file containing
 	  X.509 certificates to be included into barebox. If the string starts
 	  with "pkcs11:" it is interpreted as a PKCS#11 URI rather than a file.
+	  If the string starts with a <hint>: prefix, <hint> is used as a key
+	  name hint to find a key without iterating over all keys.
 
 	  This avoids the mkimage dependency of CONFIG_BOOTM_FITIMAGE_PUBKEY
 	  at the cost of an openssl build-time dependency.
 
-config CRYPTO_RSA_KEY_NAME_HINT
-	depends on CRYPTO_RSA
-	string "FIT image key name hint"
-	help
-	  In FIT images keys are identified by a key name hint string. Provide
-	  the key name hint here.
-
 config CRYPTO_KEYSTORE
 	bool "Keystore"
 	help

crypto/Makefile +1 -1

index 5a484355f3..b2bd7fae6a 100644
@@ -30,5 +30,5 @@ $(obj)/rsa.o: $(obj)/rsa-keys.h
 CONFIG_CRYPTO_RSA_KEY := $(CONFIG_CRYPTO_RSA_KEY:"%"=%)
 
 $(obj)/rsa-keys.h: FORCE
-	$(call cmd,public_keys,$(CONFIG_CRYPTO_RSA_KEY_NAME_HINT):$(CONFIG_CRYPTO_RSA_KEY))
+	$(call cmd,public_keys,$(CONFIG_CRYPTO_RSA_KEY))
 endif

This encapsulates struct rsa_public_key into a struct public_key. So far RSA keys are the only supported key type. With adding ECDSA keys we need a container struct so that we can add ECDSA keys using the same mechanisms.

Also we rename CONFIG_CRYPTO_RSA_KEY to CONFIG_CRYPTO_PUBLIC_KEYS and CONFIG_CRYPTO_RSA_BUILTIN_KEYS to CONFIG_CRYPTO_BUILTIN_KEYS as these variables will be used for all types of keys, not only RSA keys.

Signed-off-by Sascha Hauer <[email protected]>

crypto/public-keys.c new file +29 -0

+#include <common.h>
+#include <crypto/public_key.h>
+#include <rsa.h>
+
+extern const struct public_key * const __public_keys_start;
+extern const struct public_key * const __public_keys_end;
+
+static int init_public_keys(void)
+{
+	const struct public_key * const *iter;
+	int ret;
+
+	for (iter = &__public_keys_start; iter != &__public_keys_end; iter++) {
+		struct rsa_public_key *rsa_key;
+
+		switch ((*iter)->type) {
+		case PUBLIC_KEY_TYPE_RSA:
+			rsa_key = rsa_key_dup((*iter)->rsa);
+			if (!rsa_key)
+				continue;
+
+			ret = rsa_key_add(rsa_key);
+			if (ret)
+				pr_err("Cannot add rsa key: %pe\n", ERR_PTR(ret));
+			break;
+		default:
+			pr_err("Ignoring unknown key type %u\n", (*iter)->type);
+		}
+

Quoted 10 lines · Sascha Hauer

This encapsulates struct rsa_public_key into a struct public_key. So far RSA keys are the only supported key type. With adding ECDSA keys we need a container struct so that we can add ECDSA keys using the same mechanisms.

Also we rename CONFIG_CRYPTO_RSA_KEY to CONFIG_CRYPTO_PUBLIC_KEYS and CONFIG_CRYPTO_RSA_BUILTIN_KEYS to CONFIG_CRYPTO_BUILTIN_KEYS as these variables will be used for all types of keys, not only RSA keys.

Signed-off-by Sascha Hauer <[email protected]>

Reviewed-by Ahmad Fatoum <[email protected]>

But two comments below.

crypto/public-keys.c

Quoted 6 lines

crypto/public-keys.c new file +0 -0

new file mode 100644
index 0000000000..a3ef3bafc8
@@ -0,0 +1,39 @@

add a pr_fmt?

Quoted 36 lines

crypto/public-keys.c new file +36 -0

+#include <common.h>
+#include <crypto/public_key.h>
+#include <rsa.h>
+
+extern const struct public_key * const __public_keys_start;
+extern const struct public_key * const __public_keys_end;
+
+static int init_public_keys(void)
+{
+	const struct public_key * const *iter;
+	int ret;
+
+	for (iter = &__public_keys_start; iter != &__public_keys_end; iter++) {
+		struct rsa_public_key *rsa_key;
+
+		switch ((*iter)->type) {
+		case PUBLIC_KEY_TYPE_RSA:
+			rsa_key = rsa_key_dup((*iter)->rsa);
+			if (!rsa_key)
+				continue;
+
+			ret = rsa_key_add(rsa_key);
+			if (ret)
+				pr_err("Cannot add rsa key: %pe\n", ERR_PTR(ret));
+			break;
+		default:
+			pr_err("Ignoring unknown key type %u\n", (*iter)->type);
+		}
+
+	}
+
+	return 0;
+}
+
+device_initcall(init_public_keys);
+

crypto/public-keys.c

Quoted 2 lines

crypto/public-keys.c new file +0 -2

-extern const struct rsa_public_key * const __rsa_keys_start;
-extern const struct rsa_public_key * const __rsa_keys_end;

I know you are just moving these around, but defining these with [] is way less confusing IMO.

Now that we have a struct public_key as a general container for keys create and use functions making use of it. Move the list from struct rsa_public_key to struct public_key, add public_key_verify() and use it instead of rsa_verify(), move key_name_hint to struct public_key.

With this we do not need to bother the FIT image code when adding support for ECDSA keys.

Signed-off-by Sascha Hauer <[email protected]>

---
 common/image-fit.c          | 20 ++++----
 crypto/public-keys.c        | 92 ++++++++++++++++++++++++++++++-------
 crypto/rsa.c                | 74 +++++++++--------------------
 include/crypto/public_key.h | 15 ++++++
 include/rsa.h               | 16 +------
 scripts/keytoc.c            |  3 +-
 6 files changed, 126 insertions(+), 94 deletions(-)

common/image-fit.c +10 -10

index 4a69049abc..c5d2196bda 100644
@@ -20,7 +20,7 @@
 #include <errno.h>
 #include <linux/err.h>
 #include <stringlist.h>
-#include <rsa.h>
+#include <crypto/public_key.h>
 #include <uncompress.h>
 #include <image-fit.h>
 

@@ -253,10 +253,10 @@ static struct digest *fit_alloc_digest(struct device_node *sig_node,
 	return digest;
 }
 
-static int fit_check_rsa_signature(struct device_node *sig_node,
-				   enum hash_algo algo, void *hash)
+static int fit_check_signature(struct device_node *sig_node,
+			       enum hash_algo algo, void *hash)
 {
-	const struct rsa_public_key *key;
+	const struct public_key *key;
 	const char *key_name = NULL;
 	int sig_len;
 	const char *sig_value;

@@ -270,19 +270,19 @@ static int fit_check_rsa_signature(struct device_node *sig_node,
 
 	of_property_read_string(sig_node, "key-name-hint", &key_name);
 	if (key_name) {
-		key = rsa_get_key(key_name);
+		key = public_key_get(key_name);
 		if (key) {
-			ret = rsa_verify(key, sig_value, sig_len, hash, algo);
+			ret = public_key_verify(key, sig_value, sig_len, hash, algo);
 			if (!ret)
 				goto ok;
 		}
 	}
 
-	for_each_rsa_key(key) {
+	for_each_public_key(key) {
 		if (key_name && !strcmp(key->key_name_hint, key_name))
 			continue;
 
-		ret = rsa_verify(key, sig_value, sig_len, hash, algo);
+		ret = public_key_verify(key, sig_value, sig_len, hash, algo);
 		if (!ret)
 			goto ok;
 	}

@@ -341,7 +341,7 @@ static int fit_verify_signature(struct device_node *sig_node, const void *fit)
 	hash = xzalloc(digest_length(digest));
 	digest_final(digest, hash);
 
-	ret = fit_check_rsa_signature(sig_node, algo, hash);
+	ret = fit_check_signature(sig_node, algo, hash);
 	if (ret)
 		goto out_free_hash;
 

@@ -464,7 +464,7 @@ static int fit_image_verify_signature(struct fit_handle *handle,
 	hash = xzalloc(digest_length(digest));
 	digest_final(digest, hash);
 
-	ret = fit_check_rsa_signature(sig_node, algo, hash);
+	ret = fit_check_signature(sig_node, algo, hash);
 
 	free(hash);
 

crypto/public-keys.c +76 -16

index a3ef3bafc8..36c308908d 100644
@@ -2,31 +2,91 @@
 #include <crypto/public_key.h>
 #include <rsa.h>
 
+static LIST_HEAD(public_keys);
+
+const struct public_key *public_key_next(const struct public_key *prev)
+{
+	prev = list_prepare_entry(prev, &public_keys, list);
+	list_for_each_entry_continue(prev, &public_keys, list)
+		return prev;
+
+	return NULL;
+}
+
+const struct public_key *public_key_get(const char *name)
+{
+	const struct public_key *key;
+
+	list_for_each_entry(key, &public_keys, list) {
+		if (!strcmp(key->key_name_hint, name))
+			return key;
+	}
+
+	return NULL;
+}
+
+int public_key_add(struct public_key *key)
+{
+	if (public_key_get(key->key_name_hint))
+		return -EEXIST;
+
+	list_add_tail(&key->list, &public_keys);
+
+	return 0;
+}
+
+static struct public_key *public_key_dup(const struct public_key *key)
+{
+	struct public_key *k = xzalloc(sizeof(*k));
+
+	k->type = key->type;
+	if (key->key_name_hint)
+		k->key_name_hint = xstrdup(key->key_name_hint);
+
+	switch (key->type) {
+	case PUBLIC_KEY_TYPE_RSA:
+		k->rsa = rsa_key_dup(key->rsa);
+		if (!k->rsa)
+			goto err;
+		break;
+	default:
+		goto err;
+	}
+
+	return k;
+err:
+	free(k->key_name_hint);
+	free(k);
+
+	return NULL;
+}
+
+int public_key_verify(const struct public_key *key, const uint8_t *sig,
+		      const uint32_t sig_len, const uint8_t *hash,
+		      enum hash_algo algo)
+{
+	switch (key->type) {
+	case PUBLIC_KEY_TYPE_RSA:
+		return rsa_verify(key->rsa, sig, sig_len, hash, algo);
+	}
+
+	return -ENOKEY;
+}
+
 extern const struct public_key * const __public_keys_start;
 extern const struct public_key * const __public_keys_end;
 
 static int init_public_keys(void)
 {
 	const struct public_key * const *iter;
-	int ret;
 
 	for (iter = &__public_keys_start; iter != &__public_keys_end; iter++) {
-		struct rsa_public_key *rsa_key;
-
-		switch ((*iter)->type) {
-		case PUBLIC_KEY_TYPE_RSA:
-			rsa_key = rsa_key_dup((*iter)->rsa);
-			if (!rsa_key)
-				continue;
-
-			ret = rsa_key_add(rsa_key);
-			if (ret)
-				pr_err("Cannot add rsa key: %pe\n", ERR_PTR(ret));
-			break;
-		default:
-			pr_err("Ignoring unknown key type %u\n", (*iter)->type);
-		}
+		struct public_key *key = public_key_dup(*iter);
+
+		if (!key)
+			continue;
 
+		public_key_add(key);
 	}
 
 	return 0;

crypto/rsa.c +22 -52

index 8eab07beed..969dbfb32b 100644
@@ -15,6 +15,7 @@
 #include <asm/byteorder.h>
 #include <errno.h>
 #include <rsa.h>
+#include <crypto/public_key.h>
 #include <asm/unaligned.h>
 
 #define UINT64_MULT32(v, multby)  (((uint64_t)(v)) * ((uint32_t)(multby)))

@@ -380,105 +381,74 @@ static void rsa_convert_big_endian(uint32_t *dst, const uint32_t *src, int len)
 		dst[i] = fdt32_to_cpu(src[len - 1 - i]);
 }
 
-struct rsa_public_key *rsa_of_read_key(struct device_node *node)
+struct public_key *rsa_of_read_key(struct device_node *node)
 {
 	const void *modulus, *rr;
 	const uint64_t *public_exponent;
 	int length;
-	struct rsa_public_key *key;
+	struct public_key *key;
+	struct rsa_public_key *rsa;
 	int err;
 
 	if (strncmp(node->name, "key-", 4))
 		return ERR_PTR(-EINVAL);
 
 	key = xzalloc(sizeof(*key));
+	rsa = key->rsa = xzalloc(sizeof(*rsa));
 
 	key->key_name_hint = xstrdup(node->name + 4);
 
-	of_property_read_u32(node, "rsa,num-bits", &key->len);
-	of_property_read_u32(node, "rsa,n0-inverse", &key->n0inv);
+	of_property_read_u32(node, "rsa,num-bits", &rsa->len);
+	of_property_read_u32(node, "rsa,n0-inverse", &rsa->n0inv);
 
 	public_exponent = of_get_property(node, "rsa,exponent", &length);
 	if (!public_exponent || length < sizeof(*public_exponent))
-		key->exponent = RSA_DEFAULT_PUBEXP;
+		rsa->exponent = RSA_DEFAULT_PUBEXP;
 	else
-		key->exponent = fdt64_to_cpu(*public_exponent);
+		rsa->exponent = fdt64_to_cpu(*public_exponent);
 
 	modulus = of_get_property(node, "rsa,modulus", NULL);
 	rr = of_get_property(node, "rsa,r-squared", NULL);
 
-	if (!key->len || !modulus || !rr) {
+	if (!rsa->len || !modulus || !rr) {
 		pr_debug("%s: Missing RSA key info", __func__);
 		err = -EFAULT;
 		goto out;
 	}
 
 	/* Sanity check for stack size */
-	if (key->len > RSA_MAX_KEY_BITS || key->len < RSA_MIN_KEY_BITS) {
+	if (rsa->len > RSA_MAX_KEY_BITS || rsa->len < RSA_MIN_KEY_BITS) {
 		pr_debug("RSA key bits %u outside allowed range %d..%d\n",
-			 key->len, RSA_MIN_KEY_BITS, RSA_MAX_KEY_BITS);
+			 rsa->len, RSA_MIN_KEY_BITS, RSA_MAX_KEY_BITS);
 		err = -EFAULT;
 		goto out;
 	}
 
-	key->len /= sizeof(uint32_t) * 8;
+	rsa->len /= sizeof(uint32_t) * 8;
 
-	key->modulus = xzalloc(RSA_MAX_KEY_BITS / 8);
-	key->rr = xzalloc(RSA_MAX_KEY_BITS / 8);
+	rsa->modulus = xzalloc(RSA_MAX_KEY_BITS / 8);
+	rsa->rr = xzalloc(RSA_MAX_KEY_BITS / 8);
 
-	rsa_convert_big_endian(key->modulus, modulus, key->len);
-	rsa_convert_big_endian(key->rr, rr, key->len);
+	rsa_convert_big_endian(rsa->modulus, modulus, rsa->len);
+	rsa_convert_big_endian(rsa->rr, rr, rsa->len);
 
 	err = 0;
 out:
-	if (err)
+	if (err) {
 		free(key);
+		free(rsa);
+	}
 
 	return err ? ERR_PTR(err) : key;
 }
 
 void rsa_key_free(struct rsa_public_key *key)
 {
-	list_del(&key->list);
-
 	free(key->modulus);
 	free(key->rr);
 	free(key);
 }
 
-static LIST_HEAD(rsa_keys);
-
-const struct rsa_public_key *rsa_key_next(const struct rsa_public_key *prev)
-{
-	prev = list_prepare_entry(prev, &rsa_keys, list);
-	list_for_each_entry_continue(prev, &rsa_keys, list)
-		return prev;
-
-	return NULL;
-}
-
-const struct rsa_public_key *rsa_get_key(const char *name)
-{
-	const struct rsa_public_key *key;
-
-	list_for_each_entry(key, &rsa_keys, list) {
-		if (!strcmp(key->key_name_hint, name))
-			return key;
-	}
-
-	return NULL;
-}
-
-int rsa_key_add(struct rsa_public_key *key)
-{
-	if (rsa_get_key(key->key_name_hint))
-		return -EEXIST;
-
-	list_add_tail(&key->list, &rsa_keys);
-
-	return 0;
-}
-
 struct rsa_public_key *rsa_key_dup(const struct rsa_public_key *key)
 {
 	struct rsa_public_key *new;

@@ -493,7 +463,7 @@ struct rsa_public_key *rsa_key_dup(const struct rsa_public_key *key)
 static void rsa_init_keys_of(void)
 {
 	struct device_node *sigs, *sig;
-	struct rsa_public_key *key;
+	struct public_key *key;
 	int ret;
 
 	if (!IS_ENABLED(CONFIG_OFTREE))

@@ -511,7 +481,7 @@ static void rsa_init_keys_of(void)
 			continue;
 		}
 
-		ret = rsa_key_add(key);
+		ret = public_key_add(key);
 		if (ret)
 			pr_err("Cannot add rsa key %s: %s\n",
 				key->key_name_hint, strerror(-ret));

include/crypto/public_key.h +15 -0

index 83e8401aed..1b91063042 100644
@@ -1,6 +1,8 @@
 #ifndef __CRYPTO_PUBLIC_KEY_H
 #define __CRYPTO_PUBLIC_KEY_H
 
+#include <digest.h>
+
 struct rsa_public_key;
 struct ecdsa_public_key;
 

@@ -10,6 +12,8 @@ enum pulic_key_type {
 
 struct public_key {
 	enum pulic_key_type type;
+	struct list_head list;
+	char *key_name_hint;
 
 	union {
 		struct rsa_public_key *rsa;

@@ -17,4 +21,15 @@ struct public_key {
 	};
 };
 
+int public_key_add(struct public_key *key);
+const struct public_key *public_key_get(const char *name);
+const struct public_key *public_key_next(const struct public_key *prev);
+
+#define for_each_public_key(key) \
+		for (key = public_key_next(NULL); key; key = public_key_next(key))
+
+int public_key_verify(const struct public_key *key, const uint8_t *sig,
+		      const uint32_t sig_len, const uint8_t *hash,
+		      enum hash_algo algo);
+
 #endif /* __CRYPTO_PUBLIC_KEY_H */

include/rsa.h +1 -15

index ecb2f42957..ef03a925b8 100644
@@ -29,8 +29,6 @@ struct rsa_public_key {
 	uint32_t *modulus;	/* modulus as little endian array */
 	uint32_t *rr;		/* R^2 as little endian array */
 	uint64_t exponent;	/* public exponent */
-	char *key_name_hint;
-	struct list_head list;
 };
 
 /**

@@ -54,24 +52,12 @@ int rsa_verify(const struct rsa_public_key *key, const uint8_t *sig,
 
 struct device_node;
 
-struct rsa_public_key *rsa_of_read_key(struct device_node *node);
+struct public_key *rsa_of_read_key(struct device_node *node);
 void rsa_key_free(struct rsa_public_key *key);
-const struct rsa_public_key *rsa_get_key(const char *name);
-
-const struct rsa_public_key *rsa_key_next(const struct rsa_public_key *prev);
-
-#define for_each_rsa_key(key) \
-		for (key = rsa_key_next(NULL); key; key = rsa_key_next(key))
 
 #ifdef CONFIG_CRYPTO_RSA
-int rsa_key_add(struct rsa_public_key *key);
 struct rsa_public_key *rsa_key_dup(const struct rsa_public_key *key);
 #else
-static inline int rsa_key_add(struct rsa_public_key *key)
-{
-	return -ENOSYS;
-}
-
 static inline struct rsa_public_key *rsa_key_dup(const struct rsa_public_key *key);
 {
 	return NULL;

scripts/keytoc.c +2 -1

index e66c5989bf..8b29118c95 100644
@@ -491,6 +491,7 @@ static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *key_na
 		if (!standalone) {
 			fprintf(outfilep, "\nstatic struct public_key %s_public_key = {\n", key_name_c);
 			fprintf(outfilep, "\t.type = PUBLIC_KEY_TYPE_ECDSA,\n");
+			fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", key_name);
 			fprintf(outfilep, "\t.ecdsa = &%s,\n", key_name_c);
 			fprintf(outfilep, "};");
 			fprintf(outfilep, "\nstruct public_key *%s_ecdsa_p __attribute__((section(\".public_keys.rodata.%s\"))) = &%s_public_key;\n",

@@ -551,12 +552,12 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *key_name
 		fprintf(outfilep, "\t.modulus = %s_modulus,\n", key_name_c);
 		fprintf(outfilep, "\t.rr = %s_rr,\n", key_name_c);
 		fprintf(outfilep, "\t.exponent = 0x%0lx,\n", exponent);
-		fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", key_name);
 		fprintf(outfilep, "};\n");
 
 		if (!standalone) {
 			fprintf(outfilep, "\nstatic struct public_key %s_public_key = {\n", key_name_c);
 			fprintf(outfilep, "\t.type = PUBLIC_KEY_TYPE_RSA,\n");
+			fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", key_name);
 			fprintf(outfilep, "\t.rsa = &%s,\n", key_name_c);
 			fprintf(outfilep, "};");
 			fprintf(outfilep, "\nstruct public_key *%sp __attribute__((section(\".public_keys.rodata.%s\"))) = &%s_public_key;\n",

This adds the missing pieces to make CONFIG_CRYPTO_PUBLIC_KEYS a list of keys rather than a single key only.

We have to remove the quotes from the key argument in cmd_keys to pass the keys as multiple arguments to keytoc.

For removing the quotes from CONFIG_CRYPTO_PUBLIC_KEYS "%"=% no longer matches when the string contains multiple words, so we utilize shell to remove the quotes.

Signed-off-by Sascha Hauer <[email protected]>

---
 crypto/Kconfig       | 11 ++++++-----
 crypto/Makefile      |  2 +-
 scripts/Makefile.lib |  2 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

crypto/Kconfig +6 -5

index 612e6f33fc..6b2e73c503 100644
@@ -126,11 +126,12 @@ config CRYPTO_PUBLIC_KEYS
 	depends on CRYPTO_BUILTIN_KEYS
 	string "public keys to compile in"
 	help
-	  This option should be a filename of a PEM-formatted file containing
-	  X.509 certificates to be included into barebox. If the string starts
-	  with "pkcs11:" it is interpreted as a PKCS#11 URI rather than a file.
-	  If the string starts with a <hint>: prefix, <hint> is used as a key
-	  name hint to find a key without iterating over all keys.
+	  This option should be a space separated list of filenames of
+	  PEM-formatted files containing X.509 certificates to be included into
+	  barebox. If an entry starts with "pkcs11:" it is interpreted as a
+	  PKCS#11 URI rather than a file. If an entry starts with a <hint>:
+	  prefix, <hint> is used as a key name hint to find a key without
+	  iterating over all keys.
 
 	  This avoids the mkimage dependency of CONFIG_BOOTM_FITIMAGE_PUBKEY
 	  at the cost of an openssl build-time dependency.

crypto/Makefile +1 -1

index d6ce50b386..1c5709c90a 100644
@@ -28,7 +28,7 @@ ifdef CONFIG_CRYPTO_BUILTIN_KEYS
 
 $(obj)/public-keys.o: $(obj)/public-keys.h
 
-CONFIG_CRYPTO_PUBLIC_KEYS := $(CONFIG_CRYPTO_PUBLIC_KEYS:"%"=%)
+CONFIG_CRYPTO_PUBLIC_KEYS := $(shell echo $(CONFIG_CRYPTO_PUBLIC_KEYS))
 
 $(obj)/public-keys.h: FORCE
 	$(call cmd,public_keys,$(CONFIG_CRYPTO_PUBLIC_KEYS))

scripts/Makefile.lib +0 -0

index 1881e3c139..3d0a260804 100644

rsa_verify() is called from generic code, so provide a static inline no-op wrapper used when RSA support is disabled.

Signed-off-by Sascha Hauer <[email protected]>

+++ b/crypto/ecc.c

Heads-up, this barebox code was copy-pasted from the kernel, which in turn copy-pasted it from Ken MacKay's micro-ecc library. The kernel version has a bug in carry handling which barebox inherited:

https://lore.kernel.org/r/[email protected]/

There might be further bugs that were fixed in the kernel but not in barebox.

This adds ECDSA signature verification support. The code is based on the Linux code as of Linux-6.10. The Linux code expects the key to be in ASN.1 encoded format. We don't need this in barebox as directly compile the x and y key values into the binary, so this is left out.

Signed-off-by Sascha Hauer <[email protected]>

include/crypto/public_key.h +1 -0

 	PUBLIC_KEY_TYPE_RSA,
+	PUBLIC_KEY_TYPE_ECDSA,
 };
 
 struct public_key {

include/ecdsa.h new file +0 -0

new file mode 100644
index 0000000000..1d6340c645

This series implements ECDSA signature verification for FIT images. The ECDSA code itself is taken from the Kernel. Currently only supported way to specify a ECDSA key is to compile it into the binary using CONFIG_CRYPTO_PUBLIC_KEYS, taking it from a device tree is not yet supported.
Since v2 this series unifies the way public keys are passed to barebox. We now have a single Kconfig option which takes a list of public keys, which can be either RSA or ECDSA keys.

[...]

Quoted 9 lines · Sascha Hauer

This series implements ECDSA signature verification for FIT images. The ECDSA code itself is taken from the Kernel. Currently only supported way to specify a ECDSA key is to compile it into the binary using CONFIG_CRYPTO_PUBLIC_KEYS, taking it from a device tree is not yet supported.
Since v2 this series unifies the way public keys are passed to barebox. We now have a single Kconfig option which takes a list of public keys, which can be either RSA or ECDSA keys.

Quoted 1 line

[...]

Applied, thanks!