There is a flaw in how U-Boot verifies and generates signatures for FIT images. To prevent mix and match style attacks, it is recommended to use signed configurations. How this is supposed to work is documented in doc/usage/fit/signature.rst.
Crucially, the `hashed-nodes` property of the `signature` node contains which nodes of the FIT device tree were hashed as part of the signature and should be verified. However, this property itself is not part of the hash and can therefore be modified by an attacker. Furthermore, the signature only contains the name of each node and not the path in the device tree to the node. This patch reworks the code to address this specific oversight. Thanks to Apple Security Engineering and Architecture (SEAR) for reporting this issue and then coming up with a fix. Reported-by: Apple Security Engineering and Architecture (SEAR) Signed-off-by: Tom Rini <[email protected]> --- I assume one question will be about adding a test case. I have been asked to not share the testcase for a few releases yet, so plan to add that later in the year. --- boot/image-fit-sig.c | 31 +++++++++++++++++++++++-------- include/image.h | 12 ++++++++++++ tools/image-host.c | 9 +++++++-- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c index f23e9d5d0b0f..dce8bbe30e93 100644 --- a/boot/image-fit-sig.c +++ b/boot/image-fit-sig.c @@ -44,12 +44,6 @@ struct image_region *fit_region_make_list(const void *fit, debug("Hash regions:\n"); debug("%10s %10s\n", "Offset", "Size"); - /* - * Use malloc() except in SPL (to save code size). In SPL the caller - * must allocate the array. - */ - if (!IS_ENABLED(CONFIG_XPL_BUILD) && !region) - region = calloc(sizeof(*region), count); if (!region) return NULL; for (i = 0; i < count; i++) { @@ -62,6 +56,23 @@ struct image_region *fit_region_make_list(const void *fit, return region; } +int fit_region_add_hashed_nodes(struct image_region *region, int count, + const char* hashed_nodes, int hashed_nodes_len) +{ + /* Add the spacer to ensure the hashed strings and hashed-nodes cannot "overlap". */ + const char* HASHED_NODES_SPACER = "hashed-nodes"; + region[count].data = HASHED_NODES_SPACER; + region[count].size = strlen(HASHED_NODES_SPACER); + count++; + + region[count].data = hashed_nodes; + region[count].size = hashed_nodes_len; + count++; + + /* Now add the actual hashed-nodes value. */ + return count; +} + static int fit_image_setup_verify(struct image_sign_info *info, const void *fit, int noffset, const void *key_blob, int required_keynode, @@ -376,10 +387,14 @@ static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset, count++; } - /* Allocate the region list on the stack */ - struct image_region region[count]; + /* Allocate the region list on the stack, +2 for the hashed-nodes */ + struct image_region region[count+2]; fit_region_make_list(fit, fdt_regions, count, region); + + /* Add the hashed-nodes */ + count = fit_region_add_hashed_nodes(region, count, prop, prop_len); + if (info.crypto->verify(&info, region, count, fit_value, fit_value_len)) { *err_msgp = "Verification failed"; diff --git a/include/image.h b/include/image.h index 34efac6056dd..5b847527b586 100644 --- a/include/image.h +++ b/include/image.h @@ -1771,6 +1771,18 @@ struct image_region *fit_region_make_list(const void *fit, struct fdt_region *fdt_regions, int count, struct image_region *region); +/** + * fit_region_add_hashed_nodes() - Add a list of hashed nodes to the regions + * + * @region: List of regions to add to + * @count: Number of existing regions (Note: region needs to have space for an additional two regions) + * @hashed_nodes: List of hashed nodes + * @hashed_nodes_len: Length of list + * Return: The updated count value (i.e. count+2). + */ +int fit_region_add_hashed_nodes(struct image_region *region, int count, + const char* hashed_nodes, int hashed_nodes_len); + static inline int fit_image_check_target_arch(const void *fdt, int node) { #ifndef USE_HOSTCC diff --git a/tools/image-host.c b/tools/image-host.c index 48d69191c921..1e7c38a9a211 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -1018,13 +1018,15 @@ static int fit_config_get_regions(const void *fit, int conf_noffset, return -EINVAL; } - /* Build our list of data blocks */ - region = fit_region_make_list(fit, fdt_regions, count, NULL); + /* Allocate the region array, +2 for the hashed-nodes region. */ + region = calloc(count+2, sizeof(*region)); if (!region) { fprintf(stderr, "Out of memory hashing configuration '%s/%s'\n", conf_name, sig_name); return -ENOMEM; } + /* Build our list of data blocks */ + fit_region_make_list(fit, fdt_regions, count, region); /* Create a list of all hashed properties */ debug("Hash nodes:\n"); @@ -1043,6 +1045,9 @@ static int fit_config_get_regions(const void *fit, int conf_noffset, strcpy(region_prop + len, node_inc.strings[i]); strlist_free(&node_inc); + /* Add the hashed-nodes. */ + count = fit_region_add_hashed_nodes(region, count, region_prop, len); + *region_countp = count; *regionp = region; *region_propp = region_prop; -- 2.43.0
