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

[Simon Glass]

Hi Ludwig,

On 2026-05-07T12:06:22, 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.
>
> Consistently use log_err for errors in fit_config_verify_required_keys()
> while at it
>
> Signed-off-by: Ludwig Nussel <ludwig.nussel at siemens.com>
>
> boot/Kconfig         | 10 ++++++++++
>  boot/image-fit-sig.c | 22 ++++++++++++++--------
>  2 files changed, 24 insertions(+), 8 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
> +     help
> +       This option requires that FIT uImages are signed or boot will fail.
> +       That means the U-Boot device tree must contain a '/signature' node
> +       and at least one public key with required='conf'. All configuration
> +       sections of a FIT file must be signed using those keys based on
> +       'required-mode' policy.

Just a few more thoughts.

The commit message says the default is enabled - please update to
something like 'defaults to off so existing setups are not affected;
boards wanting fail-closed behaviour must enable it explicitly'.

Also, since fail-closed is the safer default, can you say a word in
the help text about why a user might want to leave it off? Right now
it reads as 'always turn this on'.

> diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
> @@ -632,15 +632,16 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset,
>       /* Work out what we need to verify */
>       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));
> +             log_err("No signature node found: %s\n", fdt_strerror(key_node));
> +             if (IS_ENABLED(CONFIG_FIT_SIGNATURE_REQUIRED))
> +                     return -EPERM;
>               return 0;
>       }

This is a behaviour change for users without FIT_SIGNATURE_REQUIRED -
what was a quiet debug() now prints an error on every FIT boot when
the control DT has no /signature node, even though boot proceeds.
Please only log_err() in the REQUIRED case and keep log_debug() for
the permissive case.

> diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
> @@ -632,15 +632,16 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset,
> +             if (IS_ENABLED(CONFIG_FIT_SIGNATURE_REQUIRED))
> +                     return -EPERM;

In U-Boot the convention for code that may be compiled into SPL/TPL is
CONFIG_IS_ENABLED(FIT_SIGNATURE_REQUIRED), which expands to the
SPL_/TPL_ variant in those builds. Please use that here (and at the
second site below) for consistency with the surrounding FIT_SIGNATURE
handling — see CONFIG_IS_ENABLED(FIT_SIGNATURE) usage in cmd/bootm.c,
boot/image-fit.c, common/spl/spl_fit.c

You may also want to consider whether an SPL_FIT_SIGNATURE_REQUIRED
Kconfig is needed, given SPL_FIT_SIGNATURE exists.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon