diff mbox series

[3/5] lib: ecdsa: Implement signature verification for crypto_algo API

Message ID 20210111154137.621732-4-mr.nuke.me@gmail.com
State New
Delegated to: Patrick Delaunay
Headers show
Series Enable ECDSA FIT verification for stm32mp | expand

Commit Message

Alexandru Gagniuc Jan. 11, 2021, 3:41 p.m. UTC
Implement the crypto_algo .verify() function for ecdsa256. Because
it backends on UCLASS_ECDSA, this change is focused on parsing the
keys from devicetree and passing this information to the specific
UCLASS driver.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 lib/ecdsa/ecdsa-verify.c | 117 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 1 deletion(-)

Comments

Simon Glass Jan. 13, 2021, 4:10 p.m. UTC | #1
On Mon, 11 Jan 2021 at 08:41, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> Implement the crypto_algo .verify() function for ecdsa256. Because
> it backends on UCLASS_ECDSA, this change is focused on parsing the
> keys from devicetree and passing this information to the specific
> UCLASS driver.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  lib/ecdsa/ecdsa-verify.c | 117 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 116 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Patrick DELAUNAY Feb. 9, 2021, 3:56 p.m. UTC | #2
Hi,

On 1/11/21 4:41 PM, Alexandru Gagniuc wrote:
> Implement the crypto_algo .verify() function for ecdsa256. Because
> it backends on UCLASS_ECDSA, this change is focused on parsing the
> keys from devicetree and passing this information to the specific
> UCLASS driver.
>
> Signed-off-by: Alexandru Gagniuc<mr.nuke.me@gmail.com>
> ---
>   lib/ecdsa/ecdsa-verify.c | 117 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c
> index d2e6a40f4a..d84f6eb093 100644
> --- a/lib/ecdsa/ecdsa-verify.c
> +++ b/lib/ecdsa/ecdsa-verify.c
> @@ -1,13 +1,128 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   /*
> + * ECDSA signature verification for u-boot
> + *
> + * This implements the firmware-side wrapper for ECDSA verification. It bridges
> + * the struct crypto_algo API to the ECDSA uclass implementations.
> + *
>    * Copyright (c) 2020, Alexandru Gagniuc<mr.nuke.me@gmail.com>
>    */
>   

http://www.denx.de/wiki/U-Boot/CodingStyle #Include files

#include <crypto/ecdsa-uclass.h>

it is normally the first in alphabetic order of directory

> +#include <dm/uclass.h>
>   #include <u-boot/ecdsa.h>
> +#include <crypto/ecdsa-uclass.h>
> +
> +/*
> + * Derive size of an ECDSA key from the curve name
> + *
> + * While it's possible to extract the key size by using string manipulation,
> + * use a list of known curves for the time being.
> + */
> +static int ecdsa_key_size(const char *curve_name)
> +{
> +	if (!strcmp(curve_name, "prime256v1"))
> +		return 256;
> +	else
> +		return 0;
> +}
> +


To prepare the future can you parse a array of supported curves with 
associated ID

used as parameter of ECDSA parameter = enum ECDSA_CURVES

for example: char * name, int size, enum ECDSA_CURVES

const [] = {
{"prime256v1", 256, ECDSA_PRIME256V1 },

}


> +static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node)
> +{
> +	int x_len, y_len;
> +
> +	key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL);
> +	key->size_bits = ecdsa_key_size(key->curve_name);
> +	if (key->size_bits == 0) {
> +		debug("Unknown ECDSA curve '%s'", key->curve_name);
> +		return -EINVAL;
> +	}
> +
> +	key->x = fdt_getprop(fdt, node, "ecdsa,x-point", &x_len);
> +	key->y = fdt_getprop(fdt, node, "ecdsa,y-point", &y_len);
> +
> +	if (!key->x || !key->y)
> +		return -EINVAL;
> +
> +	if (x_len != (key->size_bits / 8) || y_len != (key->size_bits / 8)) {
> +		printf("%s: node=%d, curve@%p x@%p+%i y@%p+%i\n", __func__,
> +		       node, key->curve_name, key->x, x_len, key->y, y_len);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ecdsa_verify_hash(struct udevice *dev,
> +			     const struct image_sign_info *info,
> +			     const void *hash, const void *sig, uint sig_len)
> +{
> +	const struct ecdsa_ops *ops = device_get_ops(dev);
> +	const struct checksum_algo *algo = info->checksum;
> +	struct ecdsa_public_key key;
> +	int sig_node, key_node, ret;
> +
> +	if (!ops || !ops->verify)
> +		return -ENODEV;
> +
> +	if (info->required_keynode > 0) {
> +		ret = fdt_get_key(&key, info->fdt_blob, info->required_keynode);
> +		if (ret < 0)
> +			return ret;
> +
> +		return ops->verify(dev, &key, hash, algo->checksum_len,
> +				   sig, sig_len);


Need to indicate the used curve here as parameter of the verify opts ?


> +	}
> +
> +	sig_node = fdt_subnode_offset(info->fdt_blob, 0, FIT_SIG_NODENAME);
> +	if (sig_node < 0)
> +		return -ENOENT;
> +
> +	/* Try all possible keys under the "/signature" node */
> +	fdt_for_each_subnode(key_node, info->fdt_blob, sig_node) {
> +		ret = fdt_get_key(&key, info->fdt_blob, key_node);
> +		if (ret < 0)
> +			continue;
> +
> +		ret = ops->verify(dev, &key, hash, algo->checksum_len,
> +				  sig, sig_len);
> +
> +		/* On success, don't worry about remaining keys */
> +		if (ret == 0)


issue raised by chekpatch I think

if (!ret)


> +			return 0;
> +	}
> +
> +	return -EPERM;
> +}
>   
>   int ecdsa_verify(struct image_sign_info *info,
>   		 const struct image_region region[], int region_count,
>   		 uint8_t *sig, uint sig_len)
>   {
> -	return -EOPNOTSUPP;
> +	const struct checksum_algo *algo = info->checksum;
> +	uint8_t hash[algo->checksum_len];
> +	struct udevice *dev;
> +	int ret;
> +
> +	ret = uclass_first_device(UCLASS_ECDSA, &dev);
> +	if (ret) {
> +		debug("ECDSA: Could not find ECDSA implementation: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = algo->calculate(algo->name, region, region_count, hash);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	return ecdsa_verify_hash(dev, info, hash, sig, sig_len);
>   }
> +
> +/*
> + * uclass definition for ECDSA API
> + *
> + * We don't implement any wrappers around ecdsa_ops->verify() because it's
> + * trivial to call ops->verify().
> + */
> +UCLASS_DRIVER(ecdsa) = {
> +	.id		= UCLASS_ECDSA,
> +	.name		= "ecdsa_verifier",
> +};


Regards

Patrick
Alexandru Gagniuc Feb. 9, 2021, 10:58 p.m. UTC | #3
On 2/9/21 9:56 AM, Patrick DELAUNAY wrote:
> Hi,

[snip]

>> diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c
>> index d2e6a40f4a..d84f6eb093 100644
>> --- a/lib/ecdsa/ecdsa-verify.c
>> +++ b/lib/ecdsa/ecdsa-verify.c
>> @@ -1,13 +1,128 @@
>>   // SPDX-License-Identifier: GPL-2.0+
>>   /*
>> + * ECDSA signature verification for u-boot
>> + *
>> + * This implements the firmware-side wrapper for ECDSA verification. 
>> It bridges
>> + * the struct crypto_algo API to the ECDSA uclass implementations.
>> + *
>>    * Copyright (c) 2020, Alexandru Gagniuc<mr.nuke.me@gmail.com>
>>    */
> 
> http://www.denx.de/wiki/U-Boot/CodingStyle #Include files
> 
> #include <crypto/ecdsa-uclass.h>
> 
> it is normally the first in alphabetic order of directory

Thank your for catching that. I will have it fixed in the next iteration 
of this series.

>> +#include <dm/uclass.h>
>>   #include <u-boot/ecdsa.h>
>> +#include <crypto/ecdsa-uclass.h>
>> +
>> +/*
>> + * Derive size of an ECDSA key from the curve name
>> + *
>> + * While it's possible to extract the key size by using string 
>> manipulation,
>> + * use a list of known curves for the time being.
>> + */
>> +static int ecdsa_key_size(const char *curve_name)
>> +{
>> +    if (!strcmp(curve_name, "prime256v1"))
>> +        return 256;
>> +    else
>> +        return 0;
>> +}
>> +
> 
> 
> To prepare the future can you parse a array of supported curves with 
> associated ID
> 
> used as parameter of ECDSA parameter = enum ECDSA_CURVES
> 
> for example: char * name, int size, enum ECDSA_CURVES
> 
> const [] = {
> {"prime256v1", 256, ECDSA_PRIME256V1 },
> 
> }

That is possible. If I were to have a longer list of curve names, I 
change things to extract the key length from the name itself. So instead 
of running strncmp() of N keyname strings, I would extract the digits 
from 'curve_name'.

I chose not to do that here because I want this patch to be didactic. 
That is, I'm trying to achieve the goal clearly, with the lowest number 
of lines of code.

[snip]

>> +static int ecdsa_verify_hash(struct udevice *dev,
>> +                 const struct image_sign_info *info,
>> +                 const void *hash, const void *sig, uint sig_len)
>> +{
>> +    const struct ecdsa_ops *ops = device_get_ops(dev);
>> +    const struct checksum_algo *algo = info->checksum;
>> +    struct ecdsa_public_key key;
>> +    int sig_node, key_node, ret;
>> +
>> +    if (!ops || !ops->verify)
>> +        return -ENODEV;
>> +
>> +    if (info->required_keynode > 0) {
>> +        ret = fdt_get_key(&key, info->fdt_blob, info->required_keynode);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        return ops->verify(dev, &key, hash, algo->checksum_len,
>> +                   sig, sig_len);
> 
> 
> Need to indicate the used curve here as parameter of the verify opts ?

The curve name is part of the key (struct ecdsa_public_key).


[snip]
>> +        ret = ops->verify(dev, &key, hash, algo->checksum_len,
>> +                  sig, sig_len);
>> +
>> +        /* On success, don't worry about remaining keys */
>> +        if (ret == 0)
> 
> 
> issue raised by chekpatch I think
> 
> if (!ret)

Oh! I'll get this fixed. Thanks!

Alex
diff mbox series

Patch

diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c
index d2e6a40f4a..d84f6eb093 100644
--- a/lib/ecdsa/ecdsa-verify.c
+++ b/lib/ecdsa/ecdsa-verify.c
@@ -1,13 +1,128 @@ 
 // SPDX-License-Identifier: GPL-2.0+
 /*
+ * ECDSA signature verification for u-boot
+ *
+ * This implements the firmware-side wrapper for ECDSA verification. It bridges
+ * the struct crypto_algo API to the ECDSA uclass implementations.
+ *
  * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me@gmail.com>
  */
 
+#include <dm/uclass.h>
 #include <u-boot/ecdsa.h>
+#include <crypto/ecdsa-uclass.h>
+
+/*
+ * Derive size of an ECDSA key from the curve name
+ *
+ * While it's possible to extract the key size by using string manipulation,
+ * use a list of known curves for the time being.
+ */
+static int ecdsa_key_size(const char *curve_name)
+{
+	if (!strcmp(curve_name, "prime256v1"))
+		return 256;
+	else
+		return 0;
+}
+
+static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node)
+{
+	int x_len, y_len;
+
+	key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL);
+	key->size_bits = ecdsa_key_size(key->curve_name);
+	if (key->size_bits == 0) {
+		debug("Unknown ECDSA curve '%s'", key->curve_name);
+		return -EINVAL;
+	}
+
+	key->x = fdt_getprop(fdt, node, "ecdsa,x-point", &x_len);
+	key->y = fdt_getprop(fdt, node, "ecdsa,y-point", &y_len);
+
+	if (!key->x || !key->y)
+		return -EINVAL;
+
+	if (x_len != (key->size_bits / 8) || y_len != (key->size_bits / 8)) {
+		printf("%s: node=%d, curve@%p x@%p+%i y@%p+%i\n", __func__,
+		       node, key->curve_name, key->x, x_len, key->y, y_len);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ecdsa_verify_hash(struct udevice *dev,
+			     const struct image_sign_info *info,
+			     const void *hash, const void *sig, uint sig_len)
+{
+	const struct ecdsa_ops *ops = device_get_ops(dev);
+	const struct checksum_algo *algo = info->checksum;
+	struct ecdsa_public_key key;
+	int sig_node, key_node, ret;
+
+	if (!ops || !ops->verify)
+		return -ENODEV;
+
+	if (info->required_keynode > 0) {
+		ret = fdt_get_key(&key, info->fdt_blob, info->required_keynode);
+		if (ret < 0)
+			return ret;
+
+		return ops->verify(dev, &key, hash, algo->checksum_len,
+				   sig, sig_len);
+	}
+
+	sig_node = fdt_subnode_offset(info->fdt_blob, 0, FIT_SIG_NODENAME);
+	if (sig_node < 0)
+		return -ENOENT;
+
+	/* Try all possible keys under the "/signature" node */
+	fdt_for_each_subnode(key_node, info->fdt_blob, sig_node) {
+		ret = fdt_get_key(&key, info->fdt_blob, key_node);
+		if (ret < 0)
+			continue;
+
+		ret = ops->verify(dev, &key, hash, algo->checksum_len,
+				  sig, sig_len);
+
+		/* On success, don't worry about remaining keys */
+		if (ret == 0)
+			return 0;
+	}
+
+	return -EPERM;
+}
 
 int ecdsa_verify(struct image_sign_info *info,
 		 const struct image_region region[], int region_count,
 		 uint8_t *sig, uint sig_len)
 {
-	return -EOPNOTSUPP;
+	const struct checksum_algo *algo = info->checksum;
+	uint8_t hash[algo->checksum_len];
+	struct udevice *dev;
+	int ret;
+
+	ret = uclass_first_device(UCLASS_ECDSA, &dev);
+	if (ret) {
+		debug("ECDSA: Could not find ECDSA implementation: %d\n", ret);
+		return ret;
+	}
+
+	ret = algo->calculate(algo->name, region, region_count, hash);
+	if (ret < 0)
+		return -EINVAL;
+
+	return ecdsa_verify_hash(dev, info, hash, sig, sig_len);
 }
+
+/*
+ * uclass definition for ECDSA API
+ *
+ * We don't implement any wrappers around ecdsa_ops->verify() because it's
+ * trivial to call ops->verify().
+ */
+UCLASS_DRIVER(ecdsa) = {
+	.id		= UCLASS_ECDSA,
+	.name		= "ecdsa_verifier",
+};