diff options
author | Tom Rini <trini@konsulko.com> | 2021-02-15 19:19:56 -0500 |
---|---|---|
committer | Tom Rini <trini@konsulko.com> | 2021-02-15 22:31:54 -0500 |
commit | b6f4c757959f8850e1299a77c8e5713da78e8ec0 (patch) | |
tree | 2de8580b23f833e100a186448625721d71625521 /common | |
parent | 6144438fb5c9059dc87cf219bed0c992f70b3509 (diff) | |
parent | 3f04db891a353f4b127ed57279279f851c6b4917 (diff) |
Merge branch '2021-02-15-fix-CVE-2021-27097-CVE-2021-27138'
Fix CVE-2021-27097 and CVE-2021-27138. For more details see
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-27097 and
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-27138
Diffstat (limited to 'common')
-rw-r--r-- | common/Kconfig.boot | 20 | ||||
-rw-r--r-- | common/fdt_region.c | 11 | ||||
-rw-r--r-- | common/image-fdt.c | 2 | ||||
-rw-r--r-- | common/image-fit-sig.c | 22 | ||||
-rw-r--r-- | common/image-fit.c | 126 | ||||
-rw-r--r-- | common/splash_source.c | 6 | ||||
-rw-r--r-- | common/update.c | 4 |
7 files changed, 154 insertions, 37 deletions
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 5eaabdfc27..7532e55edb 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -63,6 +63,15 @@ config FIT_ENABLE_SHA512_SUPPORT SHA512 checksum is a 512-bit (64-byte) hash value used to check that the image contents have not been corrupted. +config FIT_FULL_CHECK + bool "Do a full check of the FIT before using it" + default y + help + Enable this do a full check of the FIT to make sure it is valid. This + helps to protect against carefully crafted FITs which take advantage + of bugs or omissions in the code. This includes a bad structure, + multiple root nodes and the like. + config FIT_SIGNATURE bool "Enable signature verification of FIT uImages" depends on DM @@ -70,6 +79,7 @@ config FIT_SIGNATURE select RSA select RSA_VERIFY select IMAGE_SIGN_INFO + select FIT_FULL_CHECK help This option enables signature verification of FIT uImages, using a hash signed and verified using RSA. If @@ -159,6 +169,15 @@ config SPL_FIT_PRINT help Support printing the content of the fitImage in a verbose manner in SPL. +config SPL_FIT_FULL_CHECK + bool "Do a full check of the FIT before using it" + help + Enable this do a full check of the FIT to make sure it is valid. This + helps to protect against carefully crafted FITs which take advantage + of bugs or omissions in the code. This includes a bad structure, + multiple root nodes and the like. + + config SPL_FIT_SIGNATURE bool "Enable signature verification of FIT firmware within SPL" depends on SPL_DM @@ -168,6 +187,7 @@ config SPL_FIT_SIGNATURE select SPL_RSA select SPL_RSA_VERIFY select SPL_IMAGE_SIGN_INFO + select SPL_FIT_FULL_CHECK config SPL_LOAD_FIT bool "Enable SPL loading U-Boot as a FIT (basic fitImage features)" diff --git a/common/fdt_region.c b/common/fdt_region.c index ff12c518e9..e4ef0ca770 100644 --- a/common/fdt_region.c +++ b/common/fdt_region.c @@ -43,6 +43,7 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, int depth = -1; int want = 0; int base = fdt_off_dt_struct(fdt); + bool expect_end = false; end = path; *end = '\0'; @@ -59,6 +60,10 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, tag = fdt_next_tag(fdt, offset, &nextoffset); stop_at = nextoffset; + /* If we see two root nodes, something is wrong */ + if (expect_end && tag != FDT_END) + return -FDT_ERR_BADLAYOUT; + switch (tag) { case FDT_PROP: include = want >= 2; @@ -81,6 +86,10 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, if (depth == FDT_MAX_DEPTH) return -FDT_ERR_BADSTRUCTURE; name = fdt_get_name(fdt, offset, &len); + + /* The root node must have an empty name */ + if (!depth && *name) + return -FDT_ERR_BADLAYOUT; if (end - path + 2 + len >= path_len) return -FDT_ERR_NOSPACE; if (end != path + 1) @@ -108,6 +117,8 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, while (end > path && *--end != '/') ; *end = '\0'; + if (depth == -1) + expect_end = true; break; case FDT_END: diff --git a/common/image-fdt.c b/common/image-fdt.c index 0157cce32d..61ce6e5779 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -400,7 +400,7 @@ int boot_get_fdt(int flag, int argc, char *const argv[], uint8_t arch, */ #if CONFIG_IS_ENABLED(FIT) /* check FDT blob vs FIT blob */ - if (fit_check_format(buf)) { + if (!fit_check_format(buf, IMAGE_SIZE_INVAL)) { ulong load, len; fdt_noffset = boot_get_fdt_fit(images, 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..28b3d2b191 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -8,6 +8,8 @@ * Wolfgang Denk, DENX Software Engineering, wd@denx.de. */ +#define LOG_CATEGORY LOGC_BOOT + #ifdef USE_HOSTCC #include "mkimage.h" #include <time.h> @@ -1369,21 +1371,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; } /** @@ -1557,48 +1569,101 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp) } /** - * fit_check_format - sanity check FIT image format - * @fit: pointer to the FIT format image header + * fdt_check_no_at() - Check for nodes whose names contain '@' * - * fit_check_format() runs a basic sanity FIT image verification. - * Routine checks for mandatory properties, nodes, etc. + * This checks the parent node and all subnodes recursively * - * returns: - * 1, on success - * 0, on failure + * @fit: FIT to check + * @parent: Parent node to check + * @return 0 if OK, -EADDRNOTAVAIL is a node has a name containing '@' */ -int fit_check_format(const void *fit) +static int fdt_check_no_at(const void *fit, int parent) { + const char *name; + int node; + int ret; + + name = fdt_get_name(fit, parent, NULL); + if (!name || strchr(name, '@')) + return -EADDRNOTAVAIL; + + fdt_for_each_subnode(node, fit, parent) { + ret = fdt_check_no_at(fit, node); + if (ret) + return ret; + } + + return 0; +} + +int fit_check_format(const void *fit, ulong size) +{ + int ret; + /* A FIT image must be a valid FDT */ - if (fdt_check_header(fit)) { - debug("Wrong FIT format: not a flattened device tree\n"); - return 0; + ret = fdt_check_header(fit); + if (ret) { + log_debug("Wrong FIT format: not a flattened device tree (err=%d)\n", + ret); + return -ENOEXEC; + } + + if (CONFIG_IS_ENABLED(FIT_FULL_CHECK)) { + /* + * If we are not given the size, make do wtih calculating it. + * This is not as secure, so we should consider a flag to + * control this. + */ + if (size == IMAGE_SIZE_INVAL) + size = fdt_totalsize(fit); + ret = fdt_check_full(fit, size); + if (ret) + ret = -EINVAL; + + /* + * U-Boot stopped using unit addressed in 2017. Since libfdt + * can match nodes ignoring any unit address, signature + * verification can see the wrong node if one is inserted with + * the same name as a valid node but with a unit address + * attached. Protect against this by disallowing unit addresses. + */ + if (!ret && CONFIG_IS_ENABLED(FIT_SIGNATURE)) { + ret = fdt_check_no_at(fit, 0); + + if (ret) { + log_debug("FIT check error %d\n", ret); + return ret; + } + } + if (ret) { + log_debug("FIT check error %d\n", ret); + return ret; + } } /* mandatory / node 'description' property */ - if (fdt_getprop(fit, 0, FIT_DESC_PROP, NULL) == NULL) { - debug("Wrong FIT format: no description\n"); - return 0; + if (!fdt_getprop(fit, 0, FIT_DESC_PROP, NULL)) { + log_debug("Wrong FIT format: no description\n"); + return -ENOMSG; } if (IMAGE_ENABLE_TIMESTAMP) { /* mandatory / node 'timestamp' property */ - if (fdt_getprop(fit, 0, FIT_TIMESTAMP_PROP, NULL) == NULL) { - debug("Wrong FIT format: no timestamp\n"); - return 0; + if (!fdt_getprop(fit, 0, FIT_TIMESTAMP_PROP, NULL)) { + log_debug("Wrong FIT format: no timestamp\n"); + return -ENODATA; } } /* mandatory subimages parent '/images' node */ if (fdt_path_offset(fit, FIT_IMAGES_PATH) < 0) { - debug("Wrong FIT format: no images parent node\n"); - return 0; + log_debug("Wrong FIT format: no images parent node\n"); + return -ENOENT; } - return 1; + return 0; } - /** * fit_conf_find_compat * @fit: pointer to the FIT format image header @@ -1935,10 +2000,13 @@ int fit_image_load(bootm_headers_t *images, ulong addr, printf("## Loading %s from FIT Image at %08lx ...\n", prop_name, addr); bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT); - if (!fit_check_format(fit)) { - printf("Bad FIT %s image format!\n", prop_name); + ret = fit_check_format(fit, IMAGE_SIZE_INVAL); + if (ret) { + printf("Bad FIT %s image format! (err=%d)\n", prop_name, ret); + if (CONFIG_IS_ENABLED(FIT_SIGNATURE) && ret == -EADDRNOTAVAIL) + printf("Signature checking prevents use of unit addresses (@) in nodes\n"); bootstage_error(bootstage_id + BOOTSTAGE_SUB_FORMAT); - return -ENOEXEC; + return ret; } bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT_OK); if (fit_uname) { diff --git a/common/splash_source.c b/common/splash_source.c index 2737fc6e7f..d7f179e3ea 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -337,10 +337,10 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr) if (res < 0) return res; - res = fit_check_format(fit_header); - if (!res) { + res = fit_check_format(fit_header, IMAGE_SIZE_INVAL); + if (res) { debug("Could not find valid FIT image\n"); - return -EINVAL; + return res; } /* Get the splash image node */ diff --git a/common/update.c b/common/update.c index a5879cb52c..f0848954e5 100644 --- a/common/update.c +++ b/common/update.c @@ -286,7 +286,7 @@ int update_tftp(ulong addr, char *interface, char *devstring) got_update_file: fit = map_sysmem(addr, 0); - if (!fit_check_format((void *)fit)) { + if (fit_check_format((void *)fit, IMAGE_SIZE_INVAL)) { printf("Bad FIT format of the update file, aborting " "auto-update\n"); return 1; @@ -363,7 +363,7 @@ int fit_update(const void *fit) if (!fit) return -EINVAL; - if (!fit_check_format((void *)fit)) { + if (fit_check_format((void *)fit, IMAGE_SIZE_INVAL)) { printf("Bad FIT format of the update file, aborting auto-update\n"); return -EINVAL; } |