Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions boot/bootutil/src/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) || \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing bracket here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is. I am still testing this locally and trying to make it run with sim.

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",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would mark this as deprecated then remove it in 2 releases

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
Expand Down
13 changes: 12 additions & 1 deletion boot/zephyr/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions boot/zephyr/include/mcuboot_config/mcuboot_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions sim/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions sim/mcuboot-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
5 changes: 5 additions & 0 deletions sim/mcuboot-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
26 changes: 20 additions & 6 deletions sim/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,15 @@ 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)| {
let dep = BoringDep::new(image_num, &NO_DEPS);
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,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion sim/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
18 changes: 14 additions & 4 deletions sim/tests/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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")]
Expand Down
Loading