| 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> |
| |
| CVE: CVE-2021-27138 |
| Upstream-Status: Backport[https://github.com/u-boot/u-boot/commit/79af75f7776fc20b0d7eb6afe1e27c00fdb4b9b4] |
| Signed-off-by: Scott Murray <scott.murray@konsulko.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 897e04c7a3..34ebb8edfe 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 adc3e551de..c3dc814115 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, &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 84b3f95850..6d5b43c3ba 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 = <0x40000>; |
| entry = <0x8>; |
| }; |
| - kernel@2 { |
| + kernel-2 { |
| data = /incbin/("%(loadables1)s"); |
| type = "kernel"; |
| arch = "sandbox"; |
| @@ -35,19 +35,19 @@ base_its = ''' |
| %(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 @@ base_its = ''' |
| %(ramdisk_load)s |
| compression = "%(compression)s"; |
| }; |
| - ramdisk@2 { |
| + ramdisk-2 { |
| description = "snow"; |
| data = /incbin/("%(loadables2)s"); |
| type = "ramdisk"; |
| @@ -67,10 +67,10 @@ base_its = ''' |
| }; |
| }; |
| 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 test_fit(u_boot_console): |
| |
| # 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 test_fit(u_boot_console): |
| |
| # 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 0fb7ef4024..b41105bd0e 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) |
| |