[PATCH v2 3/4] image-fit-sig: Optionally require signatures

[Simon Glass]

Hi Ludwig,

On 2026-04-30T12:25:59, Ludwig Nussel <ludwig.nussel at siemens.com> wrote:
> image-fit-sig: Optionally require signatures
>
> If U-Boot is built with signature verification but no keys are
> included in the device tree, the boot would still continue.
> Introduce FIT_SIGNATURE_REQUIRED to avoid a fail-open setup. The
> default is enabled which may break existing setups that rely on the
> insecure behavior.
>
> Signed-off-by: Ludwig Nussel <ludwig.nussel at siemens.com>
>
> boot/Kconfig         | 10 ++++++++++
>  boot/image-fit-sig.c | 12 +++++++-----
>  2 files changed, 17 insertions(+), 5 deletions(-)

> diff --git a/boot/Kconfig b/boot/Kconfig
> @@ -124,6 +124,16 @@ config FIT_SIGNATURE
> +config FIT_SIGNATURE_REQUIRED
> +     bool "Require signature verification of FIT uImages"
> +     depends on FIT_SIGNATURE
> +     default y

I don't think we can flip this to default y in one go. Any board that
enables FIT_SIGNATURE today without a populated /signature node (or
without any key marked required='conf') will stop booting after this
lands, with no build-time warning. How about making default n and let
boards opt in?

> diff --git a/boot/Kconfig b/boot/Kconfig
> @@ -124,6 +124,16 @@ config FIT_SIGNATURE
> +     help
> +       This option requires that FIT uImages are signed. That
> +       means the U-Boot device tree must contain public keys for
> +       verification and all configuration sections must be signed
> +       using those keys.

"all configuration sections must be signed using those keys" is not
quite what the code does, i.e. fit_config_verify_required_keys()
honours the required-mode policy (all vs any), and only keys with
required='conf' participate. Please reword to match, and mention that
boot fails when no /signature node is present in the control DT.

> diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
> @@ -639,9 +639,11 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset,
>       key_node = fdt_subnode_offset(key_blob, 0, FIT_SIG_NODENAME);
>       if (key_node < 0) {
> -             debug("%s: No signature node found: %s\n", __func__,
> -                   fdt_strerror(key_node));
> -             return 0;
> +             log_err("No signature node found: %s\n", fdt_strerror(key_node));
> +             if (IS_ENABLED(CONFIG_FIT_SIGNATURE_REQUIRED))
> +                     return -EPERM;
> +             else
> +                     return 0;
>       }

With REQUIRED=n we still take this path and now print an error on
every boot for systems that have FIT_SIGNATURE set but no keys. Please
only log_err() when REQUIRED is enabled (i.e. when about to fail) and
keep log_debug() otherwise. The else is also redundant after a return.

> diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
> @@ -685,8 +687,8 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset,
> -     if (reqd_sigs && !verified) {
> -             printf("Failed to verify 'any' of the required signature(s)\n");
> +     if ((reqd_sigs || IS_ENABLED(CONFIG_FIT_SIGNATURE_REQUIRED)) && !verified) {
> +             log_err("Failed to verify 'any' of the required signature(s)\n");

Just to clarify the new failure mode: with REQUIRED=y and a /signature
node that has keys but none marked required='conf', we fall through
with reqd_sigs==0 and verified==0 and now return -EPERM. This is OK,
but the "any of the required signature(s)" message is misleading since
none was required. Please use a distinct message for that case so the
user knows what to fix.

> diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
> @@ -685,8 +687,8 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset,
>               }
>       }
>
> -     if (reqd_sigs && !verified) {
> -             printf("Failed to verify 'any' of the required signature(s)\n");
> +     if ((reqd_sigs || IS_ENABLED(CONFIG_FIT_SIGNATURE_REQUIRED)) && !verified) {
> +             log_err("Failed to verify 'any' of the required signature(s)\n");
>               return -EPERM;
>       }

The other printf() calls in this function ("Failed to verify required
signature" inside the loop, and the @-name check above) are left as
printf(). Since you are converting messages here, we should probably
convert the neighbours too so the file is consistent.

Regards,
Simon