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

Reply via email to