[PATCH] FIT: Address Secure Boot Bypass for Signed FIT Images

[Simon Glass]

Hi,

On Tue, 3 Mar 2026 at 01:09, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
>
> Hello Tom,
>
> On 3/2/26 23:09, Tom Rini wrote:
> > There is a flaw in how U-Boot verifies and generates signatures for FIT
> > images. To prevent mix and match style attacks, it is recommended to
> > use signed configurations. How this is supposed to work is documented in
> > doc/usage/fit/signature.rst.
> >
> > Crucially, the `hashed-nodes` property of the `signature` node contains
> > which nodes of the FIT device tree were hashed as part of the signature
> > and should be verified. However, this property itself is not part of the
> > hash and can therefore be modified by an attacker. Furthermore, the
> > signature only contains the name of each node and not the path in the
> > device tree to the node.
> >
> > This patch reworks the code to address this specific oversight.
>
> Do I understand correctly that this is a breaking change
> for FIT with signed configurations?
>
> - New U-Boot hashes more than intended for old FIT
> - Old U-Boot hashes less than intended for new FIT

Yes, that's right.

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

I can see how this works. Please see nit below.

>
> Thanks,
> Ahmad
>
> >
> > Thanks to Apple Security Engineering and Architecture (SEAR) for
> > reporting this issue and then coming up with a fix.
> >
> > Reported-by: Apple Security Engineering and Architecture (SEAR)
> > Signed-off-by: Tom Rini <trini at konsulko.com>
> > ---
> > I assume one question will be about adding a test case. I have been
> > asked to not share the testcase for a few releases yet, so plan to add
> > that later in the year.
> > ---
> >  boot/image-fit-sig.c | 31 +++++++++++++++++++++++--------
> >  include/image.h      | 12 ++++++++++++
> >  tools/image-host.c   |  9 +++++++--
> >  3 files changed, 42 insertions(+), 10 deletions(-)
> >
> > diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
> > index f23e9d5d0b0f..dce8bbe30e93 100644
> > --- a/boot/image-fit-sig.c
> > +++ b/boot/image-fit-sig.c
> > @@ -44,12 +44,6 @@ struct image_region *fit_region_make_list(const void *fit,
> >       debug("Hash regions:\n");
> >       debug("%10s %10s\n", "Offset", "Size");
> >
> > -     /*
> > -      * Use malloc() except in SPL (to save code size). In SPL the caller
> > -      * must allocate the array.
> > -      */
> > -     if (!IS_ENABLED(CONFIG_XPL_BUILD) && !region)
> > -             region = calloc(sizeof(*region), count);
> >       if (!region)
> >               return NULL;
> >       for (i = 0; i < count; i++) {
> > @@ -62,6 +56,23 @@ struct image_region *fit_region_make_list(const void *fit,
> >       return region;
> >  }
> >
> > +int fit_region_add_hashed_nodes(struct image_region *region, int count,
> > +                                     const char* hashed_nodes, int hashed_nodes_len)

That should line up with the previous line.

> > +{
> > +     /* Add the spacer to ensure the hashed strings and hashed-nodes cannot "overlap". */
> > +     const char* HASHED_NODES_SPACER = "hashed-nodes";
> > +     region[count].data = HASHED_NODES_SPACER;
> > +     region[count].size = strlen(HASHED_NODES_SPACER);
> > +     count++;
> > +
> > +     region[count].data = hashed_nodes;
> > +     region[count].size = hashed_nodes_len;
> > +     count++;
> > +
> > +     /* Now add the actual hashed-nodes value. */
> > +     return count;
> > +}
> > +
> >  static int fit_image_setup_verify(struct image_sign_info *info,
> >                                 const void *fit, int noffset,
> >                                 const void *key_blob, int required_keynode,
> > @@ -376,10 +387,14 @@ static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
> >               count++;
> >       }
> >
> > -     /* Allocate the region list on the stack */
> > -     struct image_region region[count];
> > +     /* Allocate the region list on the stack, +2 for the hashed-nodes */
> > +     struct image_region region[count+2];

spaces around +

> >
> >       fit_region_make_list(fit, fdt_regions, count, region);
> > +
> > +     /* Add the hashed-nodes */
> > +     count = fit_region_add_hashed_nodes(region, count, prop, prop_len);
> > +
> >       if (info.crypto->verify(&info, region, count, fit_value,
> >                               fit_value_len)) {
> >               *err_msgp = "Verification failed";
> > diff --git a/include/image.h b/include/image.h
> > index 34efac6056dd..5b847527b586 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -1771,6 +1771,18 @@ struct image_region *fit_region_make_list(const void *fit,
> >               struct fdt_region *fdt_regions, int count,
> >               struct image_region *region);
> >
> > +/**
> > + * fit_region_add_hashed_nodes() - Add a list of hashed nodes to the regions
> > + *
> > + * @region:          List of regions to add to
> > + * @count:           Number of existing regions (Note: region needs to have space for an additional two regions)

long line

> > + * @hashed_nodes:            List of hashed nodes
> > + * @hashed_nodes_len:        Length of list
> > + * Return: The updated count value (i.e. count+2).
> > + */
> > +int fit_region_add_hashed_nodes(struct image_region *region, int count,
> > +             const char* hashed_nodes, int hashed_nodes_len);
> > +
> >  static inline int fit_image_check_target_arch(const void *fdt, int node)
> >  {
> >  #ifndef USE_HOSTCC
> > diff --git a/tools/image-host.c b/tools/image-host.c
> > index 48d69191c921..1e7c38a9a211 100644
> > --- a/tools/image-host.c
> > +++ b/tools/image-host.c
> > @@ -1018,13 +1018,15 @@ static int fit_config_get_regions(const void *fit, int conf_noffset,
> >               return -EINVAL;
> >       }
> >
> > -     /* Build our list of data blocks */
> > -     region = fit_region_make_list(fit, fdt_regions, count, NULL);
> > +     /* Allocate the region array, +2 for the hashed-nodes region. */
> > +     region = calloc(count+2, sizeof(*region));
> >       if (!region) {
> >               fprintf(stderr, "Out of memory hashing configuration '%s/%s'\n",
> >                       conf_name, sig_name);
> >               return -ENOMEM;
> >       }
> > +     /* Build our list of data blocks */
> > +     fit_region_make_list(fit, fdt_regions, count, region);
> >
> >       /* Create a list of all hashed properties */
> >       debug("Hash nodes:\n");
> > @@ -1043,6 +1045,9 @@ static int fit_config_get_regions(const void *fit, int conf_noffset,
> >               strcpy(region_prop + len, node_inc.strings[i]);
> >       strlist_free(&node_inc);
> >
> > +     /* Add the hashed-nodes. */
> > +     count = fit_region_add_hashed_nodes(region, count, region_prop, len);
> > +
> >       *region_countp = count;
> >       *regionp = region;
> >       *region_propp = region_prop;

Regards,
Simon