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

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

[Ahmad Fatoum]

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(+)

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

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

[Sascha Hauer]

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 |