Re: [PATCH 2/8] fit: Don't allow verification of images with @ nodes

2021-02-15 Thread Tom Rini
On Mon, Feb 15, 2021 at 05:08:06PM -0700, Simon Glass wrote:

> 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 
> Reported-by: Bruce Monroe 
> Reported-by: Arie Haenel 
> Reported-by: Julien Lenoir 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


[PATCH 2/8] fit: Don't allow verification of images with @ nodes

2021-02-15 Thread Simon Glass
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 
Reported-by: Bruce Monroe 
Reported-by: Arie Haenel 
Reported-by: Julien Lenoir 
---

 common/image-fit-sig.c   | 22 --
 common/image-fit.c   | 20 +++-
 test/py/tests/test_fit.py| 24 
 test/py/tests/vboot_forge.py | 12 ++--
 4 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index 897e04c7a38..34ebb8edfe2 100644
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -149,6 +149,14 @@ static int fit_image_verify_sig(const void *fit, int 
image_noffset,
fdt_for_each_subnode(noffset, fit, image_noffset) {
const char *name = fit_get_name(fit, noffset, NULL);
 
+   /*
+* We don't support this since libfdt considers names with the
+* name root but different @ suffix to be equal
+*/
+   if (strchr(name, '@')) {
+   err_msg = "Node name contains @";
+   goto error;
+   }
if (!strncmp(name, FIT_SIG_NODENAME,
 strlen(FIT_SIG_NODENAME))) {
ret = fit_image_check_sig(fit, noffset, data,
@@ -398,9 +406,10 @@ error:
return -EPERM;
 }
 
-int fit_config_verify_required_sigs(const void *fit, int conf_noffset,
-   const void *sig_blob)
+static int fit_config_verify_required_sigs(const void *fit, int conf_noffset,
+  const void *sig_blob)
 {
+   const char *name = fit_get_name(fit, conf_noffset, NULL);
int noffset;
int sig_node;
int verified = 0;
@@ -408,6 +417,15 @@ int fit_config_verify_required_sigs(const void *fit, int 
conf_noffset,
bool reqd_policy_all = true;
const char *reqd_mode;
 
+   /*
+* We don't support this since libfdt considers names with the
+* name root but different @ suffix to be equal
+*/
+   if (strchr(name, '@')) {
+   printf("Configuration node '%s' contains '@'\n", name);
+   return -EPERM;
+   }
+
/* Work out what we need to verify */
sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
if (sig_node < 0) {
diff --git a/common/image-fit.c b/common/image-fit.c
index adc3e551de9..c3dc814115f 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1369,21 +1369,31 @@ error:
  */
 int fit_image_verify(const void *fit, int image_noffset)
 {
+   const char *name = fit_get_name(fit, image_noffset, NULL);
const void  *data;
size_t  size;
-   int noffset = 0;
char*err_msg = "";
 
+   if (strchr(name, '@')) {
+   /*
+* We don't support this since libfdt considers names with the
+* name root but different @ suffix to be equal
+*/
+   err_msg = "Node name contains @";
+   goto err;
+   }
/* Get image data and data length */
if (fit_image_get_data_and_size(fit, image_noffset, , )) {
err_msg = "Can't get image data/size";
-   printf("error!\n%s for '%s' hash node in '%s' image node\n",
-  err_msg, fit_get_name(fit, noffset, NULL),
-  fit_get_name(fit, image_noffset, NULL));
-   return 0;
+   goto err;
}
 
return fit_image_verify_with_data(fit, image_noffset, data, size);
+
+err:
+   printf("error!\n%s in '%s' image node\n", err_msg,
+  fit_get_name(fit, image_noffset, NULL));
+   return 0;
 }
 
 /**
diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py
index 84b3f958505..6d5b43c3bab 100755
--- a/test/py/tests/test_fit.py
+++ b/test/py/tests/test_fit.py
@@ -17,7 +17,7 @@ base_its = '''
 #address-cells = <1>;
 
 images {
-kernel@1 {
+kernel-1 {
 data = /incbin/("%(kernel)s");
 type = "kernel";
 arch = "sandbox";
@@ -26,7 +26,7 @@ base_its = '''
 load = <0x4>;
 entry = <0x8>;
 };
-kernel@2 {
+kernel-2 {
 data = /incbin/("%(loadables1)s");
 type = "kernel";
 arch = "sandbox";
@@ -35,19 +35,19 @@ base_its = '''
 %(loadables1_load)s