Re: [PATCH v2 3/6] lib: ecdsa: Accept .key private key files as a fallback

[Simon Glass]

Hi Ayoub,

On 2026-10-28T10:31:01+11:00, Ayoub Zaki <ayoub.z...@embetrix.com> wrote:
> lib: ecdsa: Accept .key private key files as a fallback
>
> Allow mkimage -k to use a .key file when the ECDSA .pem file is absent.
> Keep .pem precedence so existing key selection is preserved.

Please can you give the reason for the change here. My understanding
is that binman only looks for '<key-name-hint>.key' when working out
the -k directory, so ECDSA signing from binman cannot currently find a
.pem key. If that is the motivation, it is worth saying so.

>
> Signed-off-by: Ayoub Zaki <ayoub.z...@embetrix.com>
>
>  lib/ecdsa/ecdsa-libcrypto.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)

> diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c
> @@ -289,12 +289,24 @@ static int default_password(char *buf, int size, int 
> rwflag, void *u)
> +     if (!f && errno == ENOENT && fallback) {
> +             f = fopen(fallback, "r");
> +             if (!f) {
> +                     ret = -errno;
> +                     fprintf(stderr, "Can not get key file '%s' or '%s': 
> %s\n",
> +                             key_name, fallback, strerror(-ret));
> +                     return ret;
> +             }
> +             key_name = fallback;
> +     }
> +
>       if (!f) {
>               ret = -errno;
>               fprintf(stderr, "Can not get key file '%s': %s\n",

This duplicates the error path just below. It would be simpler to
switch to the fallback name and let the existing check handle any
failure, something like:

    f = fopen(key_name, 'r');
    if (!f && errno == ENOENT && fallback) {
        key_name = fallback;
        f = fopen(key_name, 'r');
    }
    if (!f) {
        ret = -errno;
        ...

The message would then name only the last file tried; if you want both
names, you could print the first before trying the fallback. What do
you think?

> diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c
> @@ -408,6 +421,9 @@ static int prepare_ctx(struct signer *ctx, const struct 
> image_sign_info *info)
> +             snprintf(fallback_name, sizeof(fallback_name), "%s/%s.key", 
> info->keydir,
> +                      info->keyname);

This line is 89 columns. Please can you wrap it after the format
string, as the line above does.

Regards,
Simon