Skip to content

boot: bootutil: Fix security counter updated before revert after resuming upgrade #2282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
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
28 changes: 18 additions & 10 deletions boot/bootutil/src/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -2316,18 +2316,26 @@ boot_update_hw_rollback_protection(struct boot_loader_state *state)
{
#ifdef MCUBOOT_HW_ROLLBACK_PROT
int rc;
uint8_t image_index;
struct boot_swap_state swap_state;

image_index = BOOT_CURR_IMG(state);

rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_PRIMARY(image_index), &swap_state);
if (rc != 0) {
return rc;
}

/* Update the stored security counter with the active image's security
* counter value. It will only be updated if the new security counter is
* greater than the stored value.
*
* In case of a successful image swapping when the swap type is TEST the
* security counter can be increased only after a reset, when the swap
* type is NONE and the image has marked itself "OK" (the image_ok flag
* has been set). This way a "revert" can be performed when it's
* necessary.
*/
if (BOOT_SWAP_TYPE(state) == BOOT_SWAP_TYPE_NONE) {
* counter value. It will only be updated if the new security counter is
* greater than the stored value.
*
* In case of a successful image swapping when the swap type is TEST the
* security counter can be increased only after a reset, when the image has
* marked itself "OK" (the image_ok flag has been set). This way a "revert"
* can be performed when it's necessary.
*/
if (swap_state.magic != BOOT_MAGIC_GOOD || swap_state.image_ok == BOOT_FLAG_SET) {
rc = boot_update_security_counter(state, BOOT_PRIMARY_SLOT, BOOT_PRIMARY_SLOT);
if (rc != 0) {
BOOT_LOG_ERR("Security counter update failed after image "
Expand Down
10 changes: 10 additions & 0 deletions sim/mcuboot-sys/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,3 +388,13 @@ pub extern "C" fn sim_get_nv_counter_for_image(image_index: u32, security_counte
});
return rc;
}

pub fn sim_reset_nv_counters() {
NV_COUNTER_CTX.with(|ctx| {
let mut counter_storage = ctx.borrow_mut();

for i in 0..counter_storage.storage.len() {
counter_storage.storage[i] = 0;
}
});
}
4 changes: 4 additions & 0 deletions sim/mcuboot-sys/src/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ pub fn get_security_counter(image_index: u32) -> u32 {
return counter_val;
}

pub fn reset_security_counters() {
api::sim_reset_nv_counters();
}

mod raw {
use crate::area::CAreaDesc;
use crate::api::{BootRsp, CSimContext};
Expand Down
8 changes: 6 additions & 2 deletions sim/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ impl ImagesBuilder {
let upgr = match deps.depends[image_num] {
DepType::NoUpgrade => install_no_image(),
_ => install_image(&mut flash, &self.areadesc, &slots[1],
maximal(46928), &ram, &*dep, ImageManipulation::BadSignature, Some(0), true)
maximal(46928), &ram, &*dep, ImageManipulation::BadSignature, Some(1), true)
};
(prim, upgr)
} else {
Expand All @@ -248,7 +248,7 @@ impl ImagesBuilder {
let upgr = match deps.depends[image_num] {
DepType::NoUpgrade => install_no_image(),
_ => install_image(&mut flash, &self.areadesc, &slots[1],
maximal(46928), &ram, &*dep, img_manipulation, Some(0), true)
maximal(46928), &ram, &*dep, img_manipulation, Some(1), true)
};
(prim, upgr)
};
Expand Down Expand Up @@ -289,6 +289,10 @@ impl ImagesBuilder {
}
};

// As a side effect, the upgrade performed above has updated the security counters. Reset
// them to their original value.
c::reset_security_counters();

images.total_count = Some(total_count);
images
}
Expand Down
2 changes: 2 additions & 0 deletions sim/tests/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use std::{
env,
sync::atomic::{AtomicUsize, Ordering},
};
use mcuboot_sys::c;

/// A single test, after setting up logging and such. Within the $body,
/// $arg will be bound to each device.
Expand Down Expand Up @@ -92,6 +93,7 @@ test_shell!(dependency_combos, r, {
let image = r.clone().make_image(&dep, true);
dump_image(&image, "dependency_combos");
assert!(!image.run_check_deps(&dep));
c::reset_security_counters();
}
});

Expand Down
Loading