Hi Philippe, On Tue, Mar 31, 2026 at 6:00 AM Philippe Reynes <[email protected]> wrote: > > Current implementation of ecdsa only supports key len aligned on > 8 bits. But the curve secp521r1 use a key of 521 bits which is not
s/use/uses > aligned on 8 bits. In this commit, we update the key management > for ecdsa to support key of any lenght. s/key/keys s/lenght/length 'any' is too arbitrary. > > Signed-off-by: Philippe Reynes <[email protected]> > --- > v2: > - intitial version > v3: > - fix typo in comments > > lib/ecdsa/ecdsa-libcrypto.c | 21 +++++++++++++++++++-- > lib/ecdsa/ecdsa-verify.c | 24 ++++++++++++++++++++++-- > lib/fdt-libcrypto.c | 2 +- > tools/image-sig-host.c | 2 +- > 4 files changed, 43 insertions(+), 6 deletions(-) > > diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c > index c4bfb2cec61..f8ed694595c 100644 > --- a/lib/ecdsa/ecdsa-libcrypto.c > +++ b/lib/ecdsa/ecdsa-libcrypto.c > @@ -45,6 +45,7 @@ static int fdt_get_key(struct ecdsa_public_key *key, const > void *fdt, int node) > { > int x_len; > int y_len; > + int expected_len; > > key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL); > if (!key->curve_name) > @@ -54,6 +55,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,7 +66,19 @@ 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 = (key->size_bits + 7) / 8; DIV_ROUND_UP(key->size_bits, 8) > + if (x_len > expected_len) > + key->x += x_len - expected_len; Please don't move the key->x directly, instead, use local variables to store adjusted pointers. > + if (y_len > expected_len) > + key->y += y_len - expected_len; Ditto. > + > + if (x_len < expected_len || y_len < expected_len) > return -EINVAL; > > return 0; > @@ -89,6 +104,8 @@ 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); > return -EINVAL; > @@ -111,7 +128,7 @@ static int read_key_from_fdt(struct signer *ctx, const > void *fdt, int node) > return -ENOMEM; > } > > - len = pubkey.size_bits / 8; > + len = (pubkey.size_bits + 7) / 8; > > uint8_t buf[1 + len * 2]; > > diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c > index 629b662cf6c..b90571e9fff 100644 > --- a/lib/ecdsa/ecdsa-verify.c > +++ b/lib/ecdsa/ecdsa-verify.c > @@ -24,13 +24,15 @@ 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; > > key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL); > if (!key->curve_name) { > @@ -50,7 +52,19 @@ 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 = (key->size_bits + 7) / 8; > + if (x_len > expected_len) > + key->x += x_len - expected_len; Ditto. > + if (y_len > expected_len) > + key->y += y_len - expected_len; Ditto. > + > + 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; > @@ -135,6 +149,12 @@ U_BOOT_CRYPTO_ALGO(ecdsa384) = { > .verify = ecdsa_verify, > }; > > +U_BOOT_CRYPTO_ALGO(ecdsa521) = { > + .name = "ecdsa521", > + .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; DIV_ROUND_UP(num_bits, 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..a2272b196e7 100644 > --- a/tools/image-sig-host.c > +++ b/tools/image-sig-host.c > @@ -84,7 +84,7 @@ struct crypto_algo crypto_algos[] = { > .verify = ecdsa_verify, > }, > { > - .name = "secp521r1", > + .name = "ecdsa521", Does the renaming cause an ABI change and comply with FIT / image signing naming conventions? Regards, Raymond > .key_len = ECDSA521_BYTES, > .sign = ecdsa_sign, > .add_verify_data = ecdsa_add_verify_data, > -- > 2.43.0 >
