[PATCH v5 01/15] ecdsa: fix support of secp521r1
Raymond Mao
raymondmaoca at gmail.com
Wed Apr 22 20:32:22 CEST 2026
Hi Philippe,
On Tue, Apr 21, 2026 at 5:10 PM Philippe Reynes
<philippe.reynes at softathome.com> wrote:
>
> Current implementation of ecdsa only supports key len aligned on
> 8 bits. But the curve secp521r1 uses a key of 521 bits which is not
> aligned on 8 bits. In this commit, we update the keys management
> for ecdsa to support keys that are not aligned on 8 bits.
>
> Signed-off-by: Philippe Reynes <philippe.reynes at softathome.com>
> ---
> v2:
> - intitial version
> v3:
> - fix typo in comments
> v4:
> - fix commit message
> - clean code with DIV_ROUND_UP-
> - duplicate data before shifting
> - support ecdsa521 and secp521r1
> - clean code
> v5:
> - check that x and y are not null before using them
> - free keys when keys not found
>
> lib/ecdsa/ecdsa-libcrypto.c | 63 ++++++++++++++++++++++++++++++-
> lib/ecdsa/ecdsa-verify.c | 75 +++++++++++++++++++++++++++++++++----
> lib/fdt-libcrypto.c | 2 +-
> tools/image-sig-host.c | 7 ++++
> 4 files changed, 137 insertions(+), 10 deletions(-)
>
> diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c
> index c4bfb2cec61..41590d22a51 100644
> --- a/lib/ecdsa/ecdsa-libcrypto.c
> +++ b/lib/ecdsa/ecdsa-libcrypto.c
> @@ -26,6 +26,8 @@
> #include <openssl/ec.h>
> #include <openssl/bn.h>
>
> +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> +
> /* Image signing context for openssl-libcrypto */
> struct signer {
> EVP_PKEY *evp_key; /* Pointer to EVP_PKEY object */
> @@ -41,10 +43,26 @@ struct ecdsa_public_key {
> int size_bits;
> };
>
> +static char *memdup(char *buf, size_t size)
> +{
> + char *dup;
> +
> + dup = malloc(size);
> + if (dup)
> + memcpy(dup, buf, size);
> +
> + return dup;
> +}
> +
> static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node)
> {
> + const char *x;
> + const char *y;
> int x_len;
> int y_len;
> + int expected_len;
> +
> + memset(key, 0, sizeof(*key));
>
> key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL);
> if (!key->curve_name)
> @@ -54,6 +72,8 @@ static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node)
> key->size_bits = 256;
> else if (!strcmp(key->curve_name, "secp384r1"))
> key->size_bits = 384;
> + else if (!strcmp(key->curve_name, "secp521r1"))
> + key->size_bits = 521;
> else
> return -EINVAL;
>
> @@ -63,12 +83,43 @@ static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node)
> if (!key->x || !key->y)
> return -EINVAL;
>
> - if (x_len != key->size_bits / 8 || y_len != key->size_bits / 8)
> + /*
> + * The public key is stored as an array of u32, so if the key size is
> + * not a multiple of 32 (for example 521), we may have extra bytes.
> + * To avoid any issue later, we shift the x and y pointer to the first
> + * useful byte.
> + */
> + expected_len = DIV_ROUND_UP(key->size_bits, 8);
> +
> + if (x_len < expected_len || y_len < expected_len)
> return -EINVAL;
>
> + x = memdup((char *)key->x + (x_len - expected_len), expected_len);
> + if (!x) {
> + fprintf(stderr, "Cannot allocate memory for point X");
> + return -ENOMEM;
> + }
> + key->x = (const uint8_t *)x;
> +
> + y = memdup((char *)key->y + (y_len - expected_len), expected_len);
> + if (!y) {
> + fprintf(stderr, "Cannot allocate memory for point Y");
> + free((char *)x);
> + return -ENOMEM;
> + }
> + key->y = (const uint8_t *)y;
> +
> return 0;
> }
>
> +static void fdt_free_key(struct ecdsa_public_key *key)
> +{
> + if (!key)
> + return;
> + free((char *)key->x);
It is safer to be:
if (key->x)
free((char *)key->x);
> + free((char *)key->y);
Ditto.
> +}
> +
> static int read_key_from_fdt(struct signer *ctx, const void *fdt, int node)
> {
> struct ecdsa_public_key pubkey;
> @@ -89,8 +140,11 @@ static int read_key_from_fdt(struct signer *ctx, const void *fdt, int node)
> nid = NID_X9_62_prime256v1;
> } else if (!strcmp(pubkey.curve_name, "secp384r1")) {
> nid = NID_secp384r1;
> + } else if (!strcmp(pubkey.curve_name, "secp521r1")) {
> + nid = NID_secp521r1;
> } else {
> fprintf(stderr, "Unsupported curve name: '%s'\n", pubkey.curve_name);
> + fdt_free_key(&pubkey);
> return -EINVAL;
> }
>
> @@ -100,6 +154,7 @@ static int read_key_from_fdt(struct signer *ctx, const void *fdt, int node)
> ec_key = EC_KEY_new_by_curve_name(nid);
> if (!ec_key) {
> fprintf(stderr, "Failed to allocate EC_KEY for curve %s\n", pubkey.curve_name);
> + fdt_free_key(&pubkey);
> return -ENOMEM;
> }
>
> @@ -108,10 +163,11 @@ static int read_key_from_fdt(struct signer *ctx, const void *fdt, int node)
> if (!point) {
> fprintf(stderr, "Failed to allocate EC_POINT\n");
> EC_KEY_free(ec_key);
> + fdt_free_key(&pubkey);
> return -ENOMEM;
> }
>
> - len = pubkey.size_bits / 8;
> + len = DIV_ROUND_UP(pubkey.size_bits, 8);
>
> uint8_t buf[1 + len * 2];
>
> @@ -123,6 +179,7 @@ static int read_key_from_fdt(struct signer *ctx, const void *fdt, int node)
> fprintf(stderr, "Failed to convert (x,y) point to EC_POINT\n");
> EC_POINT_free(point);
> EC_KEY_free(ec_key);
> + fdt_free_key(&pubkey);
> return -EINVAL;
> }
>
> @@ -130,11 +187,13 @@ static int read_key_from_fdt(struct signer *ctx, const void *fdt, int node)
> fprintf(stderr, "Failed to set EC_POINT as public key\n");
> EC_POINT_free(point);
> EC_KEY_free(ec_key);
> + fdt_free_key(&pubkey);
> return -EINVAL;
> }
>
> fprintf(stderr, "Successfully loaded ECDSA key from FDT node %d\n", node);
> EC_POINT_free(point);
> + fdt_free_key(&pubkey);
> ctx->ecdsa_key = ec_key;
>
> return 0;
> diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c
> index 629b662cf6c..e503627f490 100644
> --- a/lib/ecdsa/ecdsa-verify.c
> +++ b/lib/ecdsa/ecdsa-verify.c
> @@ -10,6 +10,7 @@
>
> #include <crypto/ecdsa-uclass.h>
> #include <dm/uclass.h>
> +#include <malloc.h>
> #include <u-boot/ecdsa.h>
>
> /*
> @@ -24,13 +25,19 @@ static int ecdsa_key_size(const char *curve_name)
> return 256;
> else if (!strcmp(curve_name, "secp384r1"))
> return 384;
> + else if (!strcmp(curve_name, "secp521r1"))
> + return 521;
>
> return 0;
> }
>
> static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node)
> {
> - int x_len, y_len;
> + int expected_len, x_len, y_len;
> + const char *x;
> + const char *y;
> +
> + memset(key, 0, sizeof(*key));
>
> key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL);
> if (!key->curve_name) {
> @@ -50,15 +57,46 @@ static int fdt_get_key(struct ecdsa_public_key *key, const void *fdt, int node)
> if (!key->x || !key->y)
> return -EINVAL;
>
> - if (x_len != (key->size_bits / 8) || y_len != (key->size_bits / 8)) {
> + /*
> + * The public key is stored as an array of u32, so if the key size is
> + * not a multiple of 32 (for example 521), we may have extra bytes.
> + * To avoid any issue later, we shift the x and y pointer to the first
> + * useful byte.
> + */
> + expected_len = DIV_ROUND_UP(key->size_bits, 8);
> +
> + if (x_len < expected_len || y_len < expected_len) {
> 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;
> }
>
> + x = memdup((char *)key->x + (x_len - expected_len), expected_len);
> + if (!x) {
> + debug("Cannot allocate memory for point X");
> + return -ENOMEM;
> + }
> + key->x = (const uint8_t *)x;
> +
> + y = memdup((char *)key->y + (y_len - expected_len), expected_len);
> + if (!y) {
> + debug("Cannot allocate memory for point Y");
> + free((char *)x);
> + return -ENOMEM;
> + }
> + key->y = (const uint8_t *)y;
> +
> return 0;
> }
>
> +static void fdt_free_key(struct ecdsa_public_key *key)
> +{
> + if (!key)
> + return;
> + free((char *)key->x);
> + free((char *)key->y);
> +}
> +
> static int ecdsa_verify_hash(struct udevice *dev,
> const struct image_sign_info *info,
> const void *hash, const void *sig, uint sig_len)
> @@ -73,11 +111,16 @@ static int ecdsa_verify_hash(struct udevice *dev,
>
> if (info->required_keynode > 0) {
> ret = fdt_get_key(&key, info->fdt_blob, info->required_keynode);
> - if (ret < 0)
> + if (ret < 0) {
> + fdt_free_key(&key);
> return ret;
> + }
> +
> + ret = ops->verify(dev, &key, hash, algo->checksum_len,
> + sig, sig_len);
> + fdt_free_key(&key);
>
> - return ops->verify(dev, &key, hash, algo->checksum_len,
> - sig, sig_len);
> + return ret;
> }
>
> sig_node = fdt_subnode_offset(info->fdt_blob, 0, FIT_SIG_NODENAME);
> @@ -87,15 +130,21 @@ static int ecdsa_verify_hash(struct udevice *dev,
> /* 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)
> + if (ret < 0) {
'if (ret)' is fine. Moreover, when fdt_get_key() returns a non-zero
value, the key is already free.
> + fdt_free_key(&key);
> continue;
> + }
>
> ret = ops->verify(dev, &key, hash, algo->checksum_len,
> sig, sig_len);
>
> /* On success, don't worry about remaining keys */
> - if (!ret)
> + if (!ret) {
> + fdt_free_key(&key);
> return 0;
> + }
> +
> + fdt_free_key(&key);
Above few lines can be simplified as:
```
fdt_free_key(&key);
if (!ret)
return 0;
```
Regards,
Raymond
> }
>
> return -EPERM;
> @@ -135,6 +184,18 @@ U_BOOT_CRYPTO_ALGO(ecdsa384) = {
> .verify = ecdsa_verify,
> };
>
> +U_BOOT_CRYPTO_ALGO(ecdsa521) = {
> + .name = "ecdsa521",
> + .key_len = ECDSA521_BYTES,
> + .verify = ecdsa_verify,
> +};
> +
> +U_BOOT_CRYPTO_ALGO(secp521r1) = {
> + .name = "secp521r1",
> + .key_len = ECDSA521_BYTES,
> + .verify = ecdsa_verify,
> +};
> +
> /*
> * uclass definition for ECDSA API
> *
> diff --git a/lib/fdt-libcrypto.c b/lib/fdt-libcrypto.c
> index ecb0344c8f6..090246b44e9 100644
> --- a/lib/fdt-libcrypto.c
> +++ b/lib/fdt-libcrypto.c
> @@ -10,7 +10,7 @@
> int fdt_add_bignum(void *blob, int noffset, const char *prop_name,
> BIGNUM *num, int num_bits)
> {
> - int nwords = num_bits / 32;
> + int nwords = (num_bits + 31) / 32;
> int size;
> uint32_t *buf, *ptr;
> BIGNUM *tmp, *big2, *big32, *big2_32;
> diff --git a/tools/image-sig-host.c b/tools/image-sig-host.c
> index 5285263c616..285547994ca 100644
> --- a/tools/image-sig-host.c
> +++ b/tools/image-sig-host.c
> @@ -83,6 +83,13 @@ struct crypto_algo crypto_algos[] = {
> .add_verify_data = ecdsa_add_verify_data,
> .verify = ecdsa_verify,
> },
> + {
> + .name = "ecdsa521",
> + .key_len = ECDSA521_BYTES,
> + .sign = ecdsa_sign,
> + .add_verify_data = ecdsa_add_verify_data,
> + .verify = ecdsa_verify,
> + },
> {
> .name = "secp521r1",
> .key_len = ECDSA521_BYTES,
> --
> 2.43.0
>
More information about the U-Boot
mailing list