diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c index fcb5de4100..4eccf72e05 100644 --- a/boot/bootutil/src/loader.c +++ b/boot/bootutil/src/loader.c @@ -81,6 +81,13 @@ static struct image_max_size image_max_sizes[BOOT_IMAGE_NUMBER] = {0}; #define TARGET_STATIC #endif +#if defined(MCUBOOT_VERIFY_IMG_ADDRESS) && defined(MCUBOOT_CHECK_HEADER_LOAD_ADDRESS) +#warning MCUBOOT_CHECK_HEADER_LOAD_ADDRESS takes precedence over MCUBOOT_VERIFY_IMG_ADDRESS +#endif + +/* Valid only for ARM Cortext M */ +#define RESET_OFFSET (2 * sizeof(uin32_t)) + #if BOOT_MAX_ALIGN > 1024 #define BUF_SZ BOOT_MAX_ALIGN #else @@ -1002,25 +1009,43 @@ boot_validate_slot(struct boot_loader_state *state, int slot, goto out; } -#if MCUBOOT_IMAGE_NUMBER > 1 && !defined(MCUBOOT_ENC_IMAGES) && defined(MCUBOOT_VERIFY_IMG_ADDRESS) +#if defined(MCUBOOT_VERIFY_IMG_ADDRESS) && !defined(MCUBOOT_ENC_IMAGES) || \ + defined(MCUBOOT_CHECK_HEADER_LOAD_ADDRESS) /* Verify that the image in the secondary slot has a reset address * located in the primary slot. This is done to avoid users incorrectly * overwriting an application written to the incorrect slot. * This feature is only supported by ARM platforms. */ if (fap == BOOT_IMG_AREA(state, BOOT_SLOT_SECONDARY)) { - const struct flash_area *pri_fa = BOOT_IMG_AREA(state, BOOT_SLOT_PRIMARY); struct image_header *secondary_hdr = boot_img_hdr(state, slot); - uint32_t reset_value = 0; - uint32_t reset_addr = secondary_hdr->ih_hdr_size + sizeof(reset_value); + uint32_t internal_img_addr = 0; /* either the reset handler addres or the image beginning addres */ + uint32_t min_addr; + uint32_t max_addr; + + min_addr = flash_area_get_off(BOOT_IMG_AREA(state, BOOT_SLOT_PRIMARY)); + max_addr = flash_area_get_size(BOOT_IMG_AREA(state, BOOT_SLOT_PRIMARY)) + min_addr; - if (flash_area_read(fap, reset_addr, &reset_value, sizeof(reset_value)) != 0) { +/* MCUBOOT_CHECK_HEADER_LOAD_ADDRESS takes priority over MCUBOOT_VERIFY_IMG_ADDRESS */ +#ifdef MCUBOOT_CHECK_HEADER_LOAD_ADDRESS + internal_img_addr = secondary_hdr->ih_load_addr; +#else + /* This is platform specific code that should not be here */ + const uint32_t offset = secondary_hdr->ih_hdr_size + RESET_OFFSET; + BOOT_LOG_DBG("Getting image %d internal addr from offset %u", + BOOT_CURR_IMG(state), offset); + if (flash_area_read(fap, offset, &internal_img_addr, sizeof(internal_img_addr)) != 0) + BOOT_LOG_ERR("Failed to read image %d load address", BOOT_CURR_IMG(state)); fih_rc = FIH_NO_BOOTABLE_IMAGE; goto out; } +#endif - if (reset_value < pri_fa->fa_off || reset_value> (pri_fa->fa_off + pri_fa->fa_size)) { - BOOT_LOG_ERR("Reset address of image in secondary slot is not in the primary slot"); + BOOT_LOG_DBG("Image %d expected load address 0x%x", BOOT_CURR_IMG(state), internal_img_addr); + BOOT_LOG_DBG("Check 0x%x is within [min_addr, max_addr] = [0x%x, 0x%x)", + internal_img_addr, min_addr, max_addr); + if (internal_img_addr < min_addr || internal_img_addr >= max_addr) { + BOOT_LOG_ERR("Binary in secondary slot of image %d is not designated for the primary slot", + BOOT_CURR_IMG(state)); BOOT_LOG_ERR("Erasing image from secondary slot"); /* The vector table in the image located in the secondary diff --git a/boot/zephyr/Kconfig b/boot/zephyr/Kconfig index 467b848dc2..87f80b5bf5 100644 --- a/boot/zephyr/Kconfig +++ b/boot/zephyr/Kconfig @@ -1308,13 +1308,24 @@ config USB_DEVICE_PRODUCT config MCUBOOT_BOOTUTIL_LIB_OWN_LOG bool +config MCUBOOT_CHECK_HEADER_LOAD_ADDRESS + bool "Use load address to verify application is in proper slot" + help + The bootloader will use the load address, from the image header, + to verify that binary is in slot designated for its execution. + When not selected reset vector, read from image, is used for + the same purpose. + config MCUBOOT_VERIFY_IMG_ADDRESS - bool "Verify reset address of image in secondary slot" + bool "Verify reset address of image in secondary slot [DEPRECATED]" + select DEPRECATED depends on UPDATEABLE_IMAGE_NUMBER > 1 depends on !BOOT_ENCRYPT_IMAGE depends on ARM default y if BOOT_UPGRADE_ONLY help + This option is deprecated, please use MCUBOOT_CHECK_HEADER_LOAD_ADDRESS + instead. Verify that the reset address in the image located in the secondary slot is contained within the corresponding primary slot. This is recommended if swapping is not used (that is, BOOT_UPGRADE_ONLY is set). If a user diff --git a/boot/zephyr/include/mcuboot_config/mcuboot_config.h b/boot/zephyr/include/mcuboot_config/mcuboot_config.h index b35a11f97e..4feca5b1a6 100644 --- a/boot/zephyr/include/mcuboot_config/mcuboot_config.h +++ b/boot/zephyr/include/mcuboot_config/mcuboot_config.h @@ -307,6 +307,10 @@ #define MCUBOOT_FIND_NEXT_SLOT_HOOKS #endif +#ifdef CONFIG_MCUBOOT_CHECK_HEADER_LOAD_ADDRESS +#define MCUBOOT_CHECK_HEADER_LOAD_ADDRESS +#endif + #ifdef CONFIG_MCUBOOT_VERIFY_IMG_ADDRESS #define MCUBOOT_VERIFY_IMG_ADDRESS #endif diff --git a/sim/Cargo.toml b/sim/Cargo.toml index 6d2262d90b..f7d3505a52 100644 --- a/sim/Cargo.toml +++ b/sim/Cargo.toml @@ -34,6 +34,7 @@ direct-xip = ["mcuboot-sys/direct-xip"] downgrade-prevention = ["mcuboot-sys/downgrade-prevention"] max-align-32 = ["mcuboot-sys/max-align-32"] hw-rollback-protection = ["mcuboot-sys/hw-rollback-protection"] +check-load-addr = ["mcuboot-sys/check-load-addr"] [dependencies] byteorder = "1.4" diff --git a/sim/mcuboot-sys/Cargo.toml b/sim/mcuboot-sys/Cargo.toml index f2eb706342..b3e46082f4 100644 --- a/sim/mcuboot-sys/Cargo.toml +++ b/sim/mcuboot-sys/Cargo.toml @@ -99,6 +99,9 @@ hw-rollback-protection = [] # Enable the PSA Crypto APIs where supported for cryptography related operations. psa-crypto-api = [] +# Test for ih_load_addr in upgrade/next boot slot +check-load-addr = [] + [build-dependencies] cc = "1.0.25" diff --git a/sim/mcuboot-sys/build.rs b/sim/mcuboot-sys/build.rs index b2595689b7..b04bab404e 100644 --- a/sim/mcuboot-sys/build.rs +++ b/sim/mcuboot-sys/build.rs @@ -39,6 +39,7 @@ fn main() { let direct_xip = env::var("CARGO_FEATURE_DIRECT_XIP").is_ok(); let max_align_32 = env::var("CARGO_FEATURE_MAX_ALIGN_32").is_ok(); let hw_rollback_protection = env::var("CARGO_FEATURE_HW_ROLLBACK_PROTECTION").is_ok(); + let check_load_addr = env::var("CARGO_FEATURE_CHECK_LOAD_ADDR").is_ok(); let mut conf = CachedBuild::new(); conf.conf.define("__BOOTSIM__", None); @@ -64,6 +65,10 @@ fn main() { conf.conf.define("MCUBOOT_OVERWRITE_ONLY_FAST", None); } + if check_load_addr { + conf.conf.define("MCUBOOT_CHECK_HEADER_LOAD_ADDRESS", None); + } + if validate_primary_slot { conf.conf.define("MCUBOOT_VALIDATE_PRIMARY_SLOT", None); } diff --git a/sim/src/image.rs b/sim/src/image.rs index 9703714b34..227617d7d8 100644 --- a/sim/src/image.rs +++ b/sim/src/image.rs @@ -297,7 +297,7 @@ impl ImagesBuilder { images } - pub fn make_bad_secondary_slot_image(self) -> Images { + pub fn make_bad_secondary_slot_image(self, img_manipulation : ImageManipulation) -> Images { let mut bad_flash = self.flash; let ram = self.ram.clone(); // TODO: Avoid this clone. let images = self.slots.into_iter().enumerate().map(|(image_num, slots)| { @@ -305,7 +305,7 @@ impl ImagesBuilder { let primaries = install_image(&mut bad_flash, &self.areadesc, &slots, 0, maximal(32784), &ram, &dep, ImageManipulation::None, Some(0)); let upgrades = install_image(&mut bad_flash, &self.areadesc, &slots, 1, - maximal(41928), &ram, &dep, ImageManipulation::BadSignature, Some(0)); + maximal(41928), &ram, &dep, img_manipulation, Some(0)); OneImage { slots, primaries, @@ -899,8 +899,8 @@ impl Images { fails > 0 } - // Test taht too big upgrade image will be rejected - pub fn run_oversizefail_upgrade(&self) -> bool { + // Test expecting failed upgrade and primary slot left untouched + pub fn run_fail_upgrade_primary_intact(&self) -> bool { let mut flash = self.flash.clone(); let mut fails = 0; @@ -940,7 +940,7 @@ impl Images { } if fails > 0 { - error!("Expected an upgrade failure when image has to big size"); + error!("Expected an upgrade failure and primary slot left untouched"); } fails > 0 @@ -1930,7 +1930,21 @@ fn install_image(flash: &mut SimMultiFlash, areadesc: &AreaDesc, slots: &[SlotIn _ => place.offset } } else { - 0 + if cfg!(feature = "check-load-addr") { + let wrong_off = match img_manipulation { + ImageManipulation::WrongOffset => true, + _ => false + }; + if wrong_off { + u32::MAX + } else if cfg!(feature = "direct-xip") { + slots[slot_ind].base_off as u32 + } else { + slots[0].base_off as u32 + } + } else { + 0 + } }; let len = match len { diff --git a/sim/src/lib.rs b/sim/src/lib.rs index 8c648ca630..a496db9dbe 100644 --- a/sim/src/lib.rs +++ b/sim/src/lib.rs @@ -202,7 +202,7 @@ impl RunStatus { // Creates a badly signed image in the secondary slot to check that // it is not upgraded to - let bad_secondary_slot_image = run.clone().make_bad_secondary_slot_image(); + let bad_secondary_slot_image = run.clone().make_bad_secondary_slot_image(ImageManipulation::BadSignature); failed |= bad_secondary_slot_image.run_signfail_upgrade(); diff --git a/sim/tests/core.rs b/sim/tests/core.rs index 69745ab263..b7be7530d4 100644 --- a/sim/tests/core.rs +++ b/sim/tests/core.rs @@ -49,7 +49,7 @@ macro_rules! sim_test { }; } -sim_test!(bad_secondary_slot, make_bad_secondary_slot_image(), run_signfail_upgrade()); +sim_test!(bad_secondary_slot, make_bad_secondary_slot_image(ImageManipulation::BadSignature), run_signfail_upgrade()); sim_test!(secondary_trailer_leftover, make_erased_secondary_image(), run_secondary_leftover_trailer()); sim_test!(bootstrap, make_bootstrap_image(), run_bootstrap()); sim_test!(oversized_bootstrap, make_oversized_bootstrap_image(), run_oversized_bootstrap()); @@ -59,23 +59,33 @@ sim_test!(revert_with_fails, make_image(&NO_DEPS, false), run_revert_with_fails( sim_test!(perm_with_fails, make_image(&NO_DEPS, true), run_perm_with_fails()); sim_test!(perm_with_random_fails, make_image(&NO_DEPS, true), run_perm_with_random_fails(5)); sim_test!(norevert, make_image(&NO_DEPS, true), run_norevert()); -sim_test!(oversized_secondary_slot, make_oversized_secondary_slot_image(), run_oversizefail_upgrade()); +sim_test!(oversized_secondary_slot, make_oversized_secondary_slot_image(), run_fail_upgrade_primary_intact()); +#[cfg(feature = "check-load-addr")] +sim_test!(wrong_load_addr, make_bad_secondary_slot_image(ImageManipulation::WrongOffset), run_fail_upgrade_primary_intact()); sim_test!(status_write_fails_complete, make_image(&NO_DEPS, true), run_with_status_fails_complete()); sim_test!(status_write_fails_with_reset, make_image(&NO_DEPS, true), run_with_status_fails_with_reset()); sim_test!(downgrade_prevention, make_image(&REV_DEPS, true), run_nodowngrade()); sim_test!(direct_xip_first, make_no_upgrade_image(&NO_DEPS, ImageManipulation::None), run_direct_xip()); +#[cfg(not(feature = "check-load-addr"))] sim_test!(ram_load_first, make_no_upgrade_image(&NO_DEPS, ImageManipulation::None), run_ram_load()); +#[cfg(not(feature = "check-load-addr"))] sim_test!(ram_load_split, make_no_upgrade_image(&NO_DEPS, ImageManipulation::None), run_split_ram_load()); +#[cfg(not(feature = "check-load-addr"))] sim_test!(ram_load_from_flash, make_no_upgrade_image(&NO_DEPS, ImageManipulation::None), run_ram_load_from_flash()); -sim_test!(hw_prot_failed_security_cnt_check, make_image_with_security_counter(Some(0)), run_hw_rollback_prot()); -sim_test!(hw_prot_missing_security_cnt, make_image_with_security_counter(None), run_hw_rollback_prot()); +#[cfg(not(feature = "check-load-addr"))] sim_test!(ram_load_out_of_bounds, make_no_upgrade_image(&NO_DEPS, ImageManipulation::WrongOffset), run_ram_load_boot_with_result(false)); +#[cfg(not(feature = "check-load-addr"))] sim_test!(ram_load_missing_header_flag, make_no_upgrade_image(&NO_DEPS, ImageManipulation::IgnoreRamLoadFlag), run_ram_load_boot_with_result(false)); +#[cfg(not(feature = "check-load-addr"))] sim_test!(ram_load_failed_validation, make_no_upgrade_image(&NO_DEPS, ImageManipulation::BadSignature), run_ram_load_boot_with_result(false)); +#[cfg(not(feature = "check-load-addr"))] sim_test!(ram_load_corrupt_higher_version_image, make_no_upgrade_image(&NO_DEPS, ImageManipulation::CorruptHigherVersionImage), run_ram_load_boot_with_result(true)); +sim_test!(hw_prot_missing_security_cnt, make_image_with_security_counter(None), run_hw_rollback_prot()); +sim_test!(hw_prot_failed_security_cnt_check, make_image_with_security_counter(Some(0)), run_hw_rollback_prot()); + #[cfg(feature = "multiimage")] sim_test!(ram_load_overlapping_images_same_base, make_no_upgrade_image(&NO_DEPS, ImageManipulation::OverlapImages(true)), run_ram_load_boot_with_result(false)); #[cfg(feature = "multiimage")]