Hi James, On 2026-07-02T01:46:51, James Hilliard <[email protected]> wrote: > lib: ecdsa: support additional curve sizes > > U-Boot's ECDSA FIT code is tied closely to prime256v1 and secp384r1. > Hardware verifiers may support a wider set of curves, and FIT public key > properties need to preserve the fixed coordinate width of those curves. > > Add common curve-size handling for secp224r1, prime256v1, secp384r1 and > secp521r1. Use fixed-width big-endian byte arrays when writing libcrypto > BIGNUM values into the control FDT so non-32-bit-aligned coordinates such > as secp521r1 are encoded correctly. > > Register the corresponding host and target ECDSA algorithm names and clean > up the libcrypto signing context so raw signatures are owned and freed > consistently on error paths. > > Signed-off-by: James Hilliard <[email protected]> > > doc/mkimage.1 | 3 + > doc/usage/fit/signature.rst | 3 +- > include/u-boot/ecdsa.h | 16 +++++ > include/u-boot/fdt-libcrypto.h | 6 +- > lib/ecdsa/Kconfig | 2 +- > lib/ecdsa/ecdsa-libcrypto.c | 141 > ++++++++++++++++++++++++++--------------- > lib/ecdsa/ecdsa-verify.c | 39 ++++++------ > lib/fdt-libcrypto.c | 59 ++++------------- > tools/image-sig-host.c | 7 ++ > 9 files changed, 152 insertions(+), 124 deletions(-)
> Add common curve-size handling for secp224r1, prime256v1, secp384r1 and > secp521r1. Use fixed-width big-endian byte arrays when writing libcrypto > BIGNUM values into the control FDT so non-32-bit-aligned coordinates such > as secp521r1 are encoded correctly. The fdt_add_bignum() rewrite also changes shared code used by lib/rsa/rsa-sign.c for 'rsa,modulus' and 'rsa,r-squared', but neither the subject nor the message mentions RSA. Please can you note that the encoding is unchanged for RSA (key sizes are a multiple of 32 bits, so the byte array is identical to the old word array)? Better still, split the fdt_add_bignum() change into its own patch, since it stands alone and affects both signature schemes. This patch also bundles the new curves with quite a lot of unrelated error-path and memory-ownership cleanup in ecdsa-libcrypto.c, which would be easier to review as a separate patch. > diff --git a/lib/ecdsa/Kconfig b/lib/ecdsa/Kconfig > @@ -17,7 +17,7 @@ config ECDSA_VERIFY > config SPL_ECDSA_VERIFY > bool "Enable ECDSA verification support in SPL" > - depends on SPL > + depends on SPL && SPL_DM This has nothing to do with curve sizes and is not mentioned in the commit message. I suspect it is a build fix, since ecdsa-verify.c uses the uclass API, but please can you either explain it in the message or move it to a separate patch? > diff --git a/doc/usage/fit/signature.rst b/doc/usage/fit/signature.rst > @@ -132,7 +132,8 @@ rsa,n0-inverse > ecdsa,x-point > Public key X coordinate as a big-endian multi-word integer Since fdt_add_bignum() now writes a fixed-width byte array, the 'multi-word integer' wording for 'ecdsa,x-point' and 'ecdsa,y-point' is no longer accurate for secp521r1 (66 bytes). Please can you update these to say the width is the curve size rounded up to whole bytes? > diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h > @@ -64,8 +65,23 @@ int ecdsa_verify(struct image_sign_info *info, > +static inline unsigned int ecdsa_curve_size(const char *curve_name) Please can you add a function comment in the style of the other declarations in this file, mentioning the parameter and that 0 is returned for an unknown curve? > diff --git a/lib/fdt-libcrypto.c b/lib/fdt-libcrypto.c > @@ -5,68 +5,35 @@ > - return ret ? -FDT_ERR_NOSPACE : 0; > + return ret; This mixes error namespaces: the function can return -EINVAL and -ENOMEM as well as raw libfdt codes from fdt_setprop(). Some values collide (-ENOMEM is -12, the same as -FDT_ERR_BADLAYOUT), so a caller cannot reliably interpret the result. It works today because callers only check for -FDT_ERR_NOSPACE, but it would be cleaner to normalise here, e.g. map -FDT_ERR_NOSPACE to -ENOSPC and other libfdt errors to -EIO, adjusting the callers to match. What do you think? Regards, Simon
