Re: [PATCH v3 08/15] crypto: rsa: encapsulate rsa keys in public keys struct

[Sascha Hauer <s.hauer@pengutronix.de>]

On Wed, Sep 11, 2024 at 10:33:41AM +0200, Ahmad Fatoum wrote:
> On 06.09.24 12:40, Sascha Hauer wrote:
> > 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.
> 
> Ah, the option is renamed anyway. Still disallowing pkcs11 as hint-name
> should make migration easier a bit, I think.

As said, keytoc won't allow pkcs11: as a key name hint.

Sascha

> 
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  crypto/Kconfig                    | 11 ++++-----
> >  crypto/Makefile                   | 17 +++++++-------
> >  crypto/public-keys.c              | 39 +++++++++++++++++++++++++++++++
> >  crypto/rsa.c                      | 23 ++----------------
> >  include/asm-generic/barebox.lds.h | 10 ++++----
> >  include/crypto/public_key.h       | 20 ++++++++++++++++
> >  include/rsa.h                     | 16 +++++++++++++
> >  scripts/keytoc.c                  | 18 ++++++++++----
> >  8 files changed, 110 insertions(+), 44 deletions(-)
> >  create mode 100644 crypto/public-keys.c
> >  create mode 100644 include/crypto/public_key.h
> > 
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index 64a016eb2c..612e6f33fc 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -118,14 +118,13 @@ config CRYPTO_PBKDF2
> >  config CRYPTO_RSA
> >  	bool
> >  
> > -config CRYPTO_RSA_BUILTIN_KEYS
> > -	bool
> > -	default y if CRYPTO_RSA_KEY != ""
> > +config CRYPTO_BUILTIN_KEYS
> > +	bool "builtin keys"
> >  	select KEYTOC
> >  
> > -config CRYPTO_RSA_KEY
> > -	depends on CRYPTO_RSA
> > -	string "RSA key to compile in"
> > +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
> > diff --git a/crypto/Makefile b/crypto/Makefile
> > index f3e49ab7ba..b07e5dd8d4 100644
> > --- a/crypto/Makefile
> > +++ b/crypto/Makefile
> > @@ -18,20 +18,21 @@ obj-y	+= memneq.o
> >  obj-$(CONFIG_CRYPTO_PBKDF2)	+= pbkdf2.o
> >  obj-$(CONFIG_CRYPTO_RSA)	+= rsa.o
> >  obj-$(CONFIG_CRYPTO_KEYSTORE)	+= keystore.o
> > +obj-$(CONFIG_CRYPTO_BUILTIN_KEYS)      += public-keys.o
> >  
> >  obj-$(CONFIG_JWT)		+= jwt.o
> >  
> > -extra-$(CONFIG_CRYPTO_RSA_BUILTIN_KEYS) += rsa-keys.h
> > +extra-$(CONFIG_CRYPTO_BUILTIN_KEYS) += public-keys.h
> >  
> > -ifdef CONFIG_CRYPTO_RSA_BUILTIN_KEYS
> > +ifdef CONFIG_CRYPTO_BUILTIN_KEYS
> >  
> > -$(obj)/rsa.o: $(obj)/rsa-keys.h
> > +$(obj)/public-keys.o: $(obj)/public-keys.h
> >  
> > -CONFIG_CRYPTO_RSA_KEY := $(CONFIG_CRYPTO_RSA_KEY:"%"=%)
> > +CONFIG_CRYPTO_PUBLIC_KEYS := $(CONFIG_CRYPTO_PUBLIC_KEYS:"%"=%)
> >  
> > -RSA_DEP := $(filter-out pkcs11:% __ENV__%, $(CONFIG_CRYPTO_RSA_KEY))
> > -RSA_DEP := $(shell echo $(RSA_DEP) | sed -e "s/[[:alnum:]]*://g")
> > +PUBLIC_KEYS_DEP := $(filter-out pkcs11:% __ENV__%, $(CONFIG_CRYPTO_PUBLIC_KEYS))
> > +PUBLIC_KEYS_DEP := $(shell echo $(PUBLIC_KEYS_DEP) | sed -e "s/[[:alnum:]]*://g")
> >  
> > -$(obj)/rsa-keys.h: $(RSA_DEP) FORCE
> > -	$(call cmd,public_keys,$(CONFIG_CRYPTO_RSA_KEY))
> > +$(obj)/public-keys.h: $(PUBLIC_KEYS_DEP) FORCE
> > +	$(call cmd,public_keys,$(CONFIG_CRYPTO_PUBLIC_KEYS))
> >  endif
> > diff --git a/crypto/public-keys.c b/crypto/public-keys.c
> > new file mode 100644
> > index 0000000000..a3ef3bafc8
> > --- /dev/null
> > +++ b/crypto/public-keys.c
> > @@ -0,0 +1,39 @@
> > +#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);
> > +
> > +#ifdef CONFIG_CRYPTO_BUILTIN_KEYS
> > +#include "public-keys.h"
> > +#endif
> > diff --git a/crypto/rsa.c b/crypto/rsa.c
> > index a379b77c9a..8eab07beed 100644
> > --- a/crypto/rsa.c
> > +++ b/crypto/rsa.c
> > @@ -469,7 +469,7 @@ const struct rsa_public_key *rsa_get_key(const char *name)
> >  	return NULL;
> >  }
> >  
> > -static int rsa_key_add(struct rsa_public_key *key)
> > +int rsa_key_add(struct rsa_public_key *key)
> >  {
> >  	if (rsa_get_key(key->key_name_hint))
> >  		return -EEXIST;
> > @@ -479,7 +479,7 @@ static int rsa_key_add(struct rsa_public_key *key)
> >  	return 0;
> >  }
> >  
> > -static struct rsa_public_key *rsa_key_dup(const struct rsa_public_key *key)
> > +struct rsa_public_key *rsa_key_dup(const struct rsa_public_key *key)
> >  {
> >  	struct rsa_public_key *new;
> >  
> > @@ -490,9 +490,6 @@ static struct rsa_public_key *rsa_key_dup(const struct rsa_public_key *key)
> >  	return new;
> >  }
> >  
> > -extern const struct rsa_public_key * const __rsa_keys_start;
> > -extern const struct rsa_public_key * const __rsa_keys_end;
> > -
> >  static void rsa_init_keys_of(void)
> >  {
> >  	struct device_node *sigs, *sig;
> > @@ -523,25 +520,9 @@ static void rsa_init_keys_of(void)
> >  
> >  static int rsa_init_keys(void)
> >  {
> > -	const struct rsa_public_key * const *iter;
> > -	struct rsa_public_key *key;
> > -	int ret;
> > -
> > -	for (iter = &__rsa_keys_start; iter != &__rsa_keys_end; iter++) {
> > -		key = rsa_key_dup(*iter);
> > -		ret = rsa_key_add(key);
> > -		if (ret)
> > -			pr_err("Cannot add rsa key %s: %s\n",
> > -			       key->key_name_hint, strerror(-ret));
> > -	}
> > -
> >  	rsa_init_keys_of();
> >  
> >  	return 0;
> >  }
> >  
> >  device_initcall(rsa_init_keys);
> > -
> > -#ifdef CONFIG_CRYPTO_RSA_BUILTIN_KEYS
> > -#include "rsa-keys.h"
> > -#endif
> > diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
> > index d3736ebaed..eb4c42ca5b 100644
> > --- a/include/asm-generic/barebox.lds.h
> > +++ b/include/asm-generic/barebox.lds.h
> > @@ -106,11 +106,11 @@
> >  #define BAREBOX_PCI_FIXUP
> >  #endif
> >  
> > -#define BAREBOX_RSA_KEYS			\
> > +#define BAREBOX_PUBLIC_KEYS			\
> >  	STRUCT_ALIGN();				\
> > -	__rsa_keys_start = .;			\
> > -	KEEP(*(.rsa_keys.rodata.*));		\
> > -	__rsa_keys_end = .;			\
> > +	__public_keys_start = .;		\
> > +	KEEP(*(.public_keys.rodata.*));		\
> > +	__public_keys_end = .;			\
> >  
> >  #define BAREBOX_DEEP_PROBE			\
> >  	STRUCT_ALIGN();				\
> > @@ -140,7 +140,7 @@
> >  	BAREBOX_MAGICVARS			\
> >  	BAREBOX_CLK_TABLE			\
> >  	BAREBOX_DTB				\
> > -	BAREBOX_RSA_KEYS			\
> > +	BAREBOX_PUBLIC_KEYS			\
> >  	BAREBOX_PCI_FIXUP			\
> >  	BAREBOX_DEEP_PROBE
> >  
> > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> > new file mode 100644
> > index 0000000000..83e8401aed
> > --- /dev/null
> > +++ b/include/crypto/public_key.h
> > @@ -0,0 +1,20 @@
> > +#ifndef __CRYPTO_PUBLIC_KEY_H
> > +#define __CRYPTO_PUBLIC_KEY_H
> > +
> > +struct rsa_public_key;
> > +struct ecdsa_public_key;
> > +
> > +enum pulic_key_type {
> > +	PUBLIC_KEY_TYPE_RSA,
> > +};
> > +
> > +struct public_key {
> > +	enum pulic_key_type type;
> > +
> > +	union {
> > +		struct rsa_public_key *rsa;
> > +		struct ecdsa_public_key *ecdsa;
> > +	};
> > +};
> > +
> > +#endif /* __CRYPTO_PUBLIC_KEY_H */
> > diff --git a/include/rsa.h b/include/rsa.h
> > index f1e3c1b6c3..ecb2f42957 100644
> > --- a/include/rsa.h
> > +++ b/include/rsa.h
> > @@ -62,4 +62,20 @@ 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;
> > +}
> > +#endif
> > +
> >  #endif
> > diff --git a/scripts/keytoc.c b/scripts/keytoc.c
> > index 85a7cd7319..e66c5989bf 100644
> > --- a/scripts/keytoc.c
> > +++ b/scripts/keytoc.c
> > @@ -488,9 +488,14 @@ static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *key_na
> >  		fprintf(outfilep, "\t.x = %s_x,\n", key_name_c);
> >  		fprintf(outfilep, "\t.y = %s_y,\n", key_name_c);
> >  		fprintf(outfilep, "};\n");
> > -		if (!standalone)
> > -			fprintf(outfilep, "\nstruct ecdsa_public_key *%s_ecdsa_p __attribute__((section(\".ecdsa_keys.rodata.%s\"))) = &%s;\n",
> > +		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.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",
> >  				key_name_c, key_name_c, key_name_c);
> > +		}
> >  	}
> >  
> >  	return 0;
> > @@ -549,9 +554,14 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *key_name
> >  		fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", key_name);
> >  		fprintf(outfilep, "};\n");
> >  
> > -		if (!standalone)
> > -			fprintf(outfilep, "\nstruct rsa_public_key *%sp __attribute__((section(\".rsa_keys.rodata.%s\"))) = &%s;\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.rsa = &%s,\n", key_name_c);
> > +			fprintf(outfilep, "};");
> > +			fprintf(outfilep, "\nstruct public_key *%sp __attribute__((section(\".public_keys.rodata.%s\"))) = &%s_public_key;\n",
> >  				key_name_c, key_name_c, key_name_c);
> > +		}
> >  	}
> >  
> >  	return 0;
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |