[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 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)
> +{
> +	/* 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 |