Re: [PATCH] FIT: don't verify signature of non-signature nodes

[Sascha Hauer <sha@pengutronix.de>]

On Thu, Jul 27, 2023 at 05:57:26PM +0200, Ahmad Fatoum wrote:
> One would expect that all children of a configuration node are signature
> nodes, but OpenEmbedded's core kernel-fitimage.bbclass always generates
> a dummy hash-1 node into configurations with just an algo and no digest,
> which barebox would try to interpret as a FIT configuration leading to
> an error verifying the FIT image:
> 
>   ERROR: FIT: hashed-strings start not found in
>   /configurations/conf-something/hash-1
> 
> Make it possible to boot such FIT images by only verifying nodes that
> are supposed to be signatures. This aligns us with U-Boot behavior, but
> introduces theoretical breakage for FIT images that have signature nodes
> with funny names. Given that everyone uses signature@1 or signature-1 and we even
> hardcode it as places and that the failure mode is to refuse boot of old
> images with new barebox version when FIT image verification is required,
> this is deemed acceptable.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/image-fit.c |  4 ++++
>  drivers/of/base.c  | 15 +++++++++++++++
>  include/of.h       |  6 ++++++
>  3 files changed, 25 insertions(+)

Applied, thanks

Sascha

> 
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 9bea62bb34a0..e73ed581a2be 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -670,8 +670,12 @@ static int fit_config_verify_signature(struct fit_handle *handle, struct device_
>  	}
>  
>  	for_each_child_of_node(conf_node, sig_node) {
> +		if (!of_node_has_prefix(sig_node, "signature"))
> +			continue;
> +
>  		if (handle->verbose)
>  			of_print_nodes(sig_node, 0, ~0);
> +
>  		ret = fit_verify_signature(sig_node, handle->fit);
>  		if (ret < 0)
>  			return ret;
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 4dc1c76b136d..03cceeffc0df 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -24,6 +24,21 @@
>  
>  static struct device_node *root_node;
>  
> +/**
> + * of_node_has_prefix - Test if a node name has a given prefix
> + * @np: The node name to test
> + * @prefix: The prefix to see if @np starts with
> + *
> + * Returns:
> + * * strlen(@prefix) if @np starts with @prefix
> + * * 0 if @np does not start with @prefix
> + */
> +size_t of_node_has_prefix(const struct device_node *np, const char *prefix)
> +{
> +	return np ? str_has_prefix(kbasename(np->full_name), prefix) : 0;
> +}
> +EXPORT_SYMBOL(of_node_has_prefix);
> +
>  bool of_node_name_eq(const struct device_node *np, const char *name)
>  {
>  	const char *node_name;
> diff --git a/include/of.h b/include/of.h
> index 92a15f5c4a13..b3e4d4699248 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -131,6 +131,7 @@ extern int of_n_addr_cells(struct device_node *np);
>  extern int of_bus_n_size_cells(struct device_node *np);
>  extern int of_n_size_cells(struct device_node *np);
>  extern bool of_node_name_eq(const struct device_node *np, const char *name);
> +extern size_t of_node_has_prefix(const struct device_node *np, const char *prefix);
>  
>  extern struct property *of_find_property(const struct device_node *np,
>  					const char *name, int *lenp);
> @@ -377,6 +378,11 @@ static inline bool of_node_name_eq(const struct device_node *np, const char *nam
>  	return false;
>  }
>  
> +static inline size_t of_node_has_prefix(const struct device_node *np, const char *prefix)
> +{
> +	return 0;
> +}
> +
>  static inline int of_parse_partitions(struct cdev *cdev,
>  					  struct device_node *node)
>  {
> -- 
> 2.39.2
> 
> 
> 

-- 
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 |