diff mbox series

[u-boot,v2019.04-aspeed-openbmc,4/7] image: Check hash-nodes when checking configurations

Message ID 20210128105304.401058-5-joel@jms.id.au
State New
Headers show
Series Blackport SHA512 for FIT | expand

Commit Message

Joel Stanley Jan. 28, 2021, 10:53 a.m. UTC 
From: Simon Glass <sjg@chromium.org>

It is currently possible to use a different configuration's signature and
thus bypass the configuration check. Make sure that the configuration node
that was hashed matches the one being checked, to catch this problem.

Also add a proper function comment to fit_config_check_sig() and make it
static.

Signed-off-by: Simon Glass <sjg@chromium.org>
(cherry picked from commit 67acad3db71bb372458fbb8a77749f5eb88aa324)
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 common/image-sig.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

Comments

Klaus Heinrich Kiwi Jan. 28, 2021, 9:30 p.m. UTC | #1 
On 1/28/2021 7:53 AM, Joel Stanley wrote:
> From: Simon Glass<sjg@chromium.org>
> 
> It is currently possible to use a different configuration's signature and
> thus bypass the configuration check. Make sure that the configuration node
> that was hashed matches the one being checked, to catch this problem.
> 
> Also add a proper function comment to fit_config_check_sig() and make it
> static.
> 
> Signed-off-by: Simon Glass<sjg@chromium.org>
> (cherry picked from commit 67acad3db71bb372458fbb8a77749f5eb88aa324)
> Signed-off-by: Joel Stanley<joel@jms.id.au>
Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
Klaus Heinrich Kiwi Jan. 28, 2021, 10:13 p.m. UTC | #2 
On 1/28/2021 7:53 AM, Joel Stanley wrote:
> thus bypass the configuration check. Make sure that the configuration node
> that was hashed matches the one being checked, to catch this problem.
> 
> Also add a proper function comment to fit_config_check_sig() and make it
> static.
> 
> Signed-off-by: Simon Glass<sjg@chromium.org>
> (cherry picked from commit 67acad3db71bb372458fbb8a77749f5eb88aa324)
> Signed-off-by: Joel Stanley<joel@jms.id.au>

Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
diff mbox series

Patch

diff --git a/common/image-sig.c b/common/image-sig.c
index 48532b15a31a..44c797530d0e 100644
--- a/common/image-sig.c
+++ b/common/image-sig.c
@@ -330,20 +330,39 @@  int fit_image_verify_required_sigs(const void *fit, int image_noffset,
 	return 0;
 }
 
-int fit_config_check_sig(const void *fit, int noffset, int required_keynode,
-			 char **err_msgp)
+/**
+ * fit_config_check_sig() - Check the signature of a config
+ *
+ * @fit: FIT to check
+ * @noffset: Offset of configuration node (e.g. /configurations/conf-1)
+ * @required_keynode:	Offset in the control FDT of the required key node,
+ *			if any. If this is given, then the configuration wil not
+ *			pass verification unless that key is used. If this is
+ *			-1 then any signature will do.
+ * @conf_noffset: Offset of the configuration subnode being checked (e.g.
+ *	 /configurations/conf-1/kernel)
+ * @err_msgp:		In the event of an error, this will be pointed to a
+ *			help error string to display to the user.
+ * @return 0 if all verified ok, <0 on error
+ */
+static int fit_config_check_sig(const void *fit, int noffset,
+				int required_keynode, int conf_noffset,
+				char **err_msgp)
 {
 	char * const exc_prop[] = {"data"};
 	const char *prop, *end, *name;
 	struct image_sign_info info;
 	const uint32_t *strings;
+	const char *config_name;
 	uint8_t *fit_value;
 	int fit_value_len;
+	bool found_config;
 	int max_regions;
 	int i, prop_len;
 	char path[200];
 	int count;
 
+	config_name = fit_get_name(fit, conf_noffset, NULL);
 	debug("%s: fdt=%p, conf='%s', sig='%s'\n", __func__, gd_fdt_blob(),
 	      fit_get_name(fit, noffset, NULL),
 	      fit_get_name(gd_fdt_blob(), required_keynode, NULL));
@@ -384,9 +403,20 @@  int fit_config_check_sig(const void *fit, int noffset, int required_keynode,
 	char *node_inc[count];
 
 	debug("Hash nodes (%d):\n", count);
+	found_config = false;
 	for (name = prop, i = 0; name < end; name += strlen(name) + 1, i++) {
 		debug("   '%s'\n", name);
 		node_inc[i] = (char *)name;
+		if (!strncmp(FIT_CONFS_PATH, name, strlen(FIT_CONFS_PATH)) &&
+		    name[sizeof(FIT_CONFS_PATH) - 1] == '/' &&
+		    !strcmp(name + sizeof(FIT_CONFS_PATH), config_name)) {
+			debug("      (found config node %s)", config_name);
+			found_config = true;
+		}
+	}
+	if (!found_config) {
+		*err_msgp = "Selected config not in hashed nodes";
+		return -1;
 	}
 
 	/*
@@ -454,7 +484,7 @@  static int fit_config_verify_sig(const void *fit, int conf_noffset,
 		if (!strncmp(name, FIT_SIG_NODENAME,
 			     strlen(FIT_SIG_NODENAME))) {
 			ret = fit_config_check_sig(fit, noffset, sig_offset,
-						   &err_msg);
+						   conf_noffset, &err_msg);
 			if (ret) {
 				puts("- ");
 			} else {