241 lines
8.7 KiB
Diff
241 lines
8.7 KiB
Diff
From 79af75f7776fc20b0d7eb6afe1e27c00fdb4b9b4 Mon Sep 17 00:00:00 2001
|
|
From: Simon Glass <sjg@chromium.org>
|
|
Date: Mon, 15 Feb 2021 17:08:06 -0700
|
|
Subject: [PATCH] fit: Don't allow verification of images with @ nodes
|
|
|
|
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 <sjg@chromium.org>
|
|
Reported-by: Bruce Monroe <bruce.monroe@intel.com>
|
|
Reported-by: Arie Haenel <arie.haenel@intel.com>
|
|
Reported-by: Julien Lenoir <julien.lenoir@intel.com>
|
|
---
|
|
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 @@ static int fit_config_verify_sig(const void *fit, int conf_noffset,
|
|
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 @@ int fit_image_verify_with_data(const void *fit, int image_noffset,
|
|
*/
|
|
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, &data, &size)) {
|
|
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 @@
|
|
#address-cells = <1>;
|
|
|
|
images {
|
|
- kernel@1 {
|
|
+ kernel-1 {
|
|
data = /incbin/("%(kernel)s");
|
|
type = "kernel";
|
|
arch = "sandbox";
|
|
@@ -26,7 +26,7 @@
|
|
load = <0x40000>;
|
|
entry = <0x8>;
|
|
};
|
|
- kernel@2 {
|
|
+ kernel-2 {
|
|
data = /incbin/("%(loadables1)s");
|
|
type = "kernel";
|
|
arch = "sandbox";
|
|
@@ -35,19 +35,19 @@
|
|
%(loadables1_load)s
|
|
entry = <0x0>;
|
|
};
|
|
- fdt@1 {
|
|
+ fdt-1 {
|
|
description = "snow";
|
|
data = /incbin/("%(fdt)s");
|
|
type = "flat_dt";
|
|
arch = "sandbox";
|
|
%(fdt_load)s
|
|
compression = "%(compression)s";
|
|
- signature@1 {
|
|
+ signature-1 {
|
|
algo = "sha1,rsa2048";
|
|
key-name-hint = "dev";
|
|
};
|
|
};
|
|
- ramdisk@1 {
|
|
+ ramdisk-1 {
|
|
description = "snow";
|
|
data = /incbin/("%(ramdisk)s");
|
|
type = "ramdisk";
|
|
@@ -56,7 +56,7 @@
|
|
%(ramdisk_load)s
|
|
compression = "%(compression)s";
|
|
};
|
|
- ramdisk@2 {
|
|
+ ramdisk-2 {
|
|
description = "snow";
|
|
data = /incbin/("%(loadables2)s");
|
|
type = "ramdisk";
|
|
@@ -67,10 +67,10 @@
|
|
};
|
|
};
|
|
configurations {
|
|
- default = "conf@1";
|
|
- conf@1 {
|
|
- kernel = "kernel@1";
|
|
- fdt = "fdt@1";
|
|
+ default = "conf-1";
|
|
+ conf-1 {
|
|
+ kernel = "kernel-1";
|
|
+ fdt = "fdt-1";
|
|
%(ramdisk_config)s
|
|
%(loadables_config)s
|
|
};
|
|
@@ -410,7 +410,7 @@ def run_fit_test(mkimage):
|
|
|
|
# Try a ramdisk
|
|
with cons.log.section('Kernel + FDT + Ramdisk load'):
|
|
- params['ramdisk_config'] = 'ramdisk = "ramdisk@1";'
|
|
+ params['ramdisk_config'] = 'ramdisk = "ramdisk-1";'
|
|
params['ramdisk_load'] = 'load = <%#x>;' % params['ramdisk_addr']
|
|
fit = make_fit(mkimage, params)
|
|
cons.restart_uboot()
|
|
@@ -419,7 +419,7 @@ def run_fit_test(mkimage):
|
|
|
|
# Configuration with some Loadables
|
|
with cons.log.section('Kernel + FDT + Ramdisk load + Loadables'):
|
|
- params['loadables_config'] = 'loadables = "kernel@2", "ramdisk@2";'
|
|
+ params['loadables_config'] = 'loadables = "kernel-2", "ramdisk-2";'
|
|
params['loadables1_load'] = ('load = <%#x>;' %
|
|
params['loadables1_addr'])
|
|
params['loadables2_load'] = ('load = <%#x>;' %
|
|
diff --git a/test/py/tests/vboot_forge.py b/test/py/tests/vboot_forge.py
|
|
index 0fb7ef40247..b41105bd0e3 100644
|
|
--- a/test/py/tests/vboot_forge.py
|
|
+++ b/test/py/tests/vboot_forge.py
|
|
@@ -376,12 +376,12 @@ def manipulate(root, strblock):
|
|
"""
|
|
Maliciously manipulates the structure to create a crafted FIT file
|
|
"""
|
|
- # locate /images/kernel@1 (frankly, it just expects it to be the first one)
|
|
+ # locate /images/kernel-1 (frankly, it just expects it to be the first one)
|
|
kernel_node = root[0][0]
|
|
# clone it to save time filling all the properties
|
|
fake_kernel = kernel_node.clone()
|
|
# rename the node
|
|
- fake_kernel.name = b'kernel@2'
|
|
+ fake_kernel.name = b'kernel-2'
|
|
# get rid of signatures/hashes
|
|
fake_kernel.children = []
|
|
# NOTE: this simply replaces the first prop... either description or data
|
|
@@ -391,13 +391,13 @@ def manipulate(root, strblock):
|
|
root[0].children.append(fake_kernel)
|
|
|
|
# modify the default configuration
|
|
- root[1].props[0].value = b'conf@2\x00'
|
|
+ root[1].props[0].value = b'conf-2\x00'
|
|
# clone the first (only?) configuration
|
|
fake_conf = root[1][0].clone()
|
|
# rename and change kernel and fdt properties to select the crafted kernel
|
|
- fake_conf.name = b'conf@2'
|
|
- fake_conf.props[0].value = b'kernel@2\x00'
|
|
- fake_conf.props[1].value = b'fdt@1\x00'
|
|
+ fake_conf.name = b'conf-2'
|
|
+ fake_conf.props[0].value = b'kernel-2\x00'
|
|
+ fake_conf.props[1].value = b'fdt-1\x00'
|
|
# insert the new configuration under /configurations
|
|
root[1].children.append(fake_conf)
|
|
|