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

2021-02-09 Thread Alex G.



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


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

#include 

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 
  #include 
+#include 
+
+/*
+ * 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(, info->fdt_blob, info->required_keynode);
+    if (ret < 0)
+    return ret;
+
+    return ops->verify(dev, , 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, , 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


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

2021-02-09 Thread Patrick DELAUNAY

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


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

#include 

it is normally the first in alphabetic order of directory


+#include 
  #include 
+#include 
+
+/*
+ * 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", _len);
+   key->y = fdt_getprop(fdt, node, "ecdsa,y-point", _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(, info->fdt_blob, info->required_keynode);
+   if (ret < 0)
+   return ret;
+
+   return ops->verify(dev, , 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(, info->fdt_blob, key_node);
+   if (ret < 0)
+   continue;
+
+   ret = ops->verify(dev, , 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, );
+   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 

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

2021-01-13 Thread Simon Glass
On Mon, 11 Jan 2021 at 08:41, 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 
> ---
>  lib/ecdsa/ecdsa-verify.c | 117 ++-
>  1 file changed, 116 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass 


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

2021-01-11 Thread Alexandru Gagniuc
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 
---
 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 
  */
 
+#include 
 #include 
+#include 
+
+/*
+ * 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", _len);
+   key->y = fdt_getprop(fdt, node, "ecdsa,y-point", _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(, info->fdt_blob, info->required_keynode);
+   if (ret < 0)
+   return ret;
+
+   return ops->verify(dev, , 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(, info->fdt_blob, key_node);
+   if (ret < 0)
+   continue;
+
+   ret = ops->verify(dev, , 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, );
+   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",
+};
-- 
2.26.2