Re: [PATCH v2 12/19] keytoc: add ecdsa support

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 01.08.24 07:57, Sascha Hauer wrote:
> This extends the keytoc utility to also handle ecdsa keys.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  scripts/keytoc.c | 191 +++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 161 insertions(+), 30 deletions(-)
> 
> diff --git a/scripts/keytoc.c b/scripts/keytoc.c
> index 891425cecc..0883ca64f7 100644
> --- a/scripts/keytoc.c
> +++ b/scripts/keytoc.c
> @@ -20,6 +20,8 @@
>  #include <openssl/engine.h>
>  #include <openssl/provider.h>
>  #include <openssl/core_names.h>
> +#include <openssl/param_build.h>
> +#include <openssl/store.h>
>  
>  static int dts, standalone;
>  
> @@ -377,44 +379,131 @@ static int print_bignum(BIGNUM *num, int num_bits)
>  	return 0;
>  }
>  
> -static int gen_key(const char *keyname, const char *path)
> +/*
> + * When imported from a HSM the key doesn't have the EC point parameters,

Can you add a TODO: at the start, so it's more apparent, that there is something
to do?

> + * only the pubkey itself exists. Exporting the pubkey and creating a new
> + * pkey from it resolves this. This can likely (hopefully) be improved, but
> + * I don't have an idea how.
> + */
> +static EVP_PKEY *reimport_key(EVP_PKEY *pkey)
>  {
> -	BIGNUM *modulus, *r_squared;
> -	uint64_t exponent = 0;
> -	uint32_t n0_inv;
> +	char group[128] = {};
> +	size_t outlen;
> +	OSSL_PARAM *params;
> +	OSSL_PARAM_BLD *param_bld = NULL;
> +	unsigned char pub[128] = {};
> +	size_t len;
> +	EVP_PKEY *pkey_out = NULL;
> +	EVP_PKEY_CTX *pkey_ctx;
>  	int ret;
> -	int bits;
> -	EVP_PKEY *key;
> -	char *tmp, *key_name_c;
>  
> -	tmp = key_name_c = strdup(keyname);
> +	ret = EVP_PKEY_get_utf8_string_param(pkey, "group", group, sizeof(group), &outlen);

Just pass in NULL for the final parameter if it's unused?

> +	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");

Hmm, why no key-name-hint?

>  	} else {
> -		ret = pem_get_pub_key(path, &key);
> -		if (ret)
> -			exit(1);
> +		fprintf(outfilep, "\nstatic uint32_t %s_x[] = {", key_name_c);
> +		print_bignum(key_x, bits);
> +		fprintf(outfilep, "\n};\n\n");
> +
> +		fprintf(outfilep, "static uint32_t %s_y[] = {", key_name_c);
> +		print_bignum(key_y, bits);
> +		fprintf(outfilep, "\n};\n\n");
> +
> +		fprintf(outfilep, "static struct ecdsa_public_key %s = {\n", key_name_c);
> +
> +		fprintf(outfilep, "\t.curve_name = \"%s\",\n", group);
> +		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",
> +				key_name_c, key_name_c, key_name_c);
>  	}

> +	ret = gen_key_ecdsa(key, keyname, key_name_c);
> +	if (ret)
> +		ret = gen_key_rsa(key, keyname, key_name_c);
> +
> +	return 0;
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	char *path, *keyname;
> @@ -515,6 +645,7 @@ int main(int argc, char *argv[])
>  		else
>  			fprintf(outfilep, "\tsignature {\n");
>  	} else if (standalone) {
> +		fprintf(outfilep, "#include <ecdsa.h>\n");

This is being used before it was added.

>  		fprintf(outfilep, "#include <rsa.h>\n");
>  	}
>  

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