diff --git a/libs/ec-slimloader-mcxa/src/board.rs b/libs/ec-slimloader-mcxa/src/board.rs index aa308a9..c516e8f 100644 --- a/libs/ec-slimloader-mcxa/src/board.rs +++ b/libs/ec-slimloader-mcxa/src/board.rs @@ -8,24 +8,19 @@ //! * boot journal + config/log partitions : external FlexSPI flash `0x0800_0000` //! -use core::ops::Range; +use core::ops::{Not, Range}; use ec_slimloader::{Board, BootError, BootStatePolicy}; use ec_slimloader_state::flash::FlashJournal; use ec_slimloader_state::state::Slot; #[cfg(all(target_os = "none", feature = "mcxa5xx"))] use ec_slimloader_state::state::Status; -use embassy_mcxa::rom::NbootBoolValue; use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embedded_storage_async::nor_flash::{ErrorType, NorFlash, NorFlashErrorKind, ReadNorFlash}; use heapless::Vec; -use mcxa_ifr::{LifeCycleState, SecBootEn}; -use mcxa_security_provisioning::Provisioner; use partition_manager::{Partition, PartitionManager, RO, RW}; use static_cell::StaticCell; -use crate::{jump, verification}; - /// External flash total size (2 MB). const EXTERNAL_FLASH_SIZE: usize = 0x0020_0000; const READ_ALIGNMENT: usize = 1; @@ -474,13 +469,16 @@ impl Board for Mcxa { #[cfg(not(all(target_os = "none", feature = "mcxa5xx")))] { let _ = app_base; - return BootError::Integrity; + BootError::Integrity } #[cfg(all(target_os = "none", feature = "mcxa5xx"))] { + use embassy_mcxa::rom::NbootBoolValue; use fault_rejection::protected_if; + use crate::{jump, verification}; + if !slot_has_valid_app(app_base) { return BootError::Markers; } @@ -509,8 +507,7 @@ impl Board for Mcxa { } let Err(if_result) = protected_if::<{ NbootBoolValue::True as u32 }, _, _, _>( - || match verification::verify_authenticity(self.sgi.reborrow(), image_base, || Self::dev_mode() as u32) - { + || match verification::verify_authenticity(self.sgi.reborrow(), image_base, BootMode::load_from_ifr) { Ok(result) => result as u32, Err(error) => error as u32, // Error has no overlap in value with Nboot bool, so this is safe to do }, @@ -536,29 +533,86 @@ impl Mcxa { ec_slimloader::start::(config).await } - /// Check if the device authenticity should be checked in dev mode (where the keys are self-generated instead of taken from the programmed value) - fn dev_mode() -> bool { - let ifr = unsafe { mcxa_ifr::IFR::current() }; - let secure_boot_enabled = ifr.cmpa.secure_boot_cfg.sec_boot_en() == SecBootEn::OnlyPki; - let develop_lifecycle = ifr.cfpa.header.cfpa_lc_state() == Ok(LifeCycleState::Develop); - // Don't enable dev mode if secure boot is active. That way a flashed key can be tested - develop_lifecycle && !secure_boot_enabled - } - + #[cfg(all(target_os = "none", feature = "mcxa5xx"))] /// Check if the secure boot settings are good fn follows_policy() -> bool { - if Self::dev_mode() { + if BootMode::load_from_ifr() == BootMode::DEV_VAL { true } else { - unsafe { Provisioner::new_from_page(true) } + unsafe { mcxa_security_provisioning::Provisioner::new_from_page(true) } .check_production_settings() .is_ok() } } } +/// Information about the boot mode +/// +/// Data is laid out so that a single(!) saturation fault injection doesn't produce a value +/// that can lead to disabling security protections. +/// It's also very unlikely that a skipped instruction would lead to the correct bit pattern, +/// but that hasn't been exhaustively examined. +#[repr(C)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BootMode { + inverse_lifecycle: u8, + lifecycle: u8, + inverse_secure_boot: u8, + secure_boot: u8, +} + +impl BootMode { + /// The state of bootmode that would permit the use of dev mode + pub const DEV_VAL: Self = Self { + inverse_lifecycle: 0b11111100, // InvLifeCycleState::Develop + lifecycle: 0b00000011, // LifeCycleState::Develop + inverse_secure_boot: 0b00000011, // !SecBootEn::AllAllowed + secure_boot: 0b00000000, // SecBootEn::AllAllowed + }; + + pub fn load_from_ifr() -> Self { + let ifr = unsafe { mcxa_ifr::IFR::current() }; + + let secure_boot_cfg_ptr = &raw const ifr.cmpa.secure_boot_cfg; + + // Read secure boot twice to harden against fault injection where they saturate a register + let secure_boot = unsafe { secure_boot_cfg_ptr.read_volatile() } + .sec_boot_en() + .raw_value() + .value(); + let inverse_secure_boot = unsafe { secure_boot_cfg_ptr.read_volatile() } + .sec_boot_en() + .raw_value() + .not() + .value(); + + let lifecycle = match ifr.cfpa.header.cfpa_lc_state() { + Ok(val) => val.raw_value(), + Err(e) => e, + }; + let inverse_lifecycle = match ifr.cfpa.header.inv_cfpa_lc_state() { + Ok(val) => val.raw_value(), + Err(e) => e, + }; + + Self { + inverse_lifecycle, + lifecycle, + inverse_secure_boot, + secure_boot, + } + } + + /// Get the raw value of the mode. This way it can be used with [fault_rejection::protected_if]. + pub const fn raw_value(&self) -> u32 { + unsafe { core::mem::transmute_copy::(self) } + } +} + #[cfg(test)] mod tests { + use mcxa_ifr::{InvLifeCycleState, LifeCycleState, SecBootEn}; + use super::*; #[test] @@ -584,4 +638,29 @@ mod tests { assert!(!vector_table_looks_valid(0x2004_58B0, 0x0001_0288)); assert!(!vector_table_looks_valid(0x2004_58B0, 0x0801_0001)); } + + #[test] + fn boot_mode_dev_mode_correct() { + let secure_boot = SecBootEn::AllAllowed.raw_value().value(); + let inverse_secure_boot = SecBootEn::AllAllowed.raw_value().not().value(); + + let lifecycle = match Ok(LifeCycleState::Develop) { + Ok(val) => val.raw_value(), + Err(e) => e, + }; + let inverse_lifecycle = match Ok(InvLifeCycleState::Develop) { + Ok(val) => val.raw_value(), + Err(e) => e, + }; + + assert_eq!( + BootMode { + inverse_lifecycle, + lifecycle, + inverse_secure_boot, + secure_boot, + }, + BootMode::DEV_VAL + ); + } } diff --git a/libs/ec-slimloader-mcxa/src/verification.rs b/libs/ec-slimloader-mcxa/src/verification.rs index b2575b9..e590eb8 100644 --- a/libs/ec-slimloader-mcxa/src/verification.rs +++ b/libs/ec-slimloader-mcxa/src/verification.rs @@ -9,6 +9,7 @@ use fault_rejection::protected_if; use mcxa_ifr::{IFR, RotkEn, RotkUsageVal}; use mcxa_security_provisioning::certificate::derive_image_rkth_pair; +use crate::board::BootMode; use crate::error::map_nboot_status_to_boot_error; fn load_nboot_auth_parms_from_ifr(ifr: &IFR) -> Result { @@ -74,7 +75,7 @@ fn load_nboot_auth_parms_from_ifr(ifr: &IFR) -> Result( mut peri: Peri<'d, peripherals::SGI0>, image_base: *const u8, - dev_mode: impl FnOnce() -> u32, + boot_mode: impl FnOnce() -> BootMode, ) -> Result { let ifr = unsafe { mcxa_ifr::IFR::current() }; @@ -83,30 +84,33 @@ pub fn verify_authenticity<'d>( defmt_or_log::trace!("Initializing NBOOT context"); let mut n_boot_api = embassy_mcxa::rom::get().nboot().map_err(|_| BootError::Authenticate)?; - let result = protected_if::<{ true as u32 }, Result<(), BootError>, _, _>(dev_mode, || { - // In dev mode, we're going to lie to nboot and derive the rkth's ourselves so they're always correct - // This makes the provisioning process easier. Once no longer in dev mode, this branch is not taken and - // only the flashed keys are used - - const MAX_FLASH_SLOT_SIZE: u32 = 2 * 1024 * 1024; // 2MB - let image_header = - unsafe { mcxa_security_provisioning::header::ImageHeader::from_ptr(image_base, MAX_FLASH_SLOT_SIZE) } - .map_err(|_| BootError::Integrity)?; - - // Parse AHAB container once and derive both RKTH values - let (image_rkth, pqc_rkth) = derive_image_rkth_pair( - peri.reborrow(), - image_base, - image_header.extended_header_offset(), - image_header.image_length(), - ) - .map_err(|_| BootError::Hash)?; - - parms.soc_rot_nvm.soc_rkh.copy_from_slice(&image_rkth.as_words()); - parms.soc_rot_nvm.soc_rkh_1.copy_from_slice(&pqc_rkth.as_words()); - - Ok(()) - }); + let result = protected_if::<{ BootMode::DEV_VAL.raw_value() }, Result<(), BootError>, _, _>( + || boot_mode().raw_value(), + || { + // In dev mode, we're going to lie to nboot and derive the rkth's ourselves so they're always correct + // This makes the provisioning process easier. Once no longer in dev mode, this branch is not taken and + // only the flashed keys are used + + const MAX_FLASH_SLOT_SIZE: u32 = 2 * 1024 * 1024; // 2MB + let image_header = + unsafe { mcxa_security_provisioning::header::ImageHeader::from_ptr(image_base, MAX_FLASH_SLOT_SIZE) } + .map_err(|_| BootError::Integrity)?; + + // Parse AHAB container once and derive both RKTH values + let (image_rkth, pqc_rkth) = derive_image_rkth_pair( + peri.reborrow(), + image_base, + image_header.extended_header_offset(), + image_header.image_length(), + ) + .map_err(|_| BootError::Hash)?; + + parms.soc_rot_nvm.soc_rkh.copy_from_slice(&image_rkth.as_words()); + parms.soc_rot_nvm.soc_rkh_1.copy_from_slice(&pqc_rkth.as_words()); + + Ok(()) + }, + ); match result { Ok(if_result) => if_result?, diff --git a/libs/fault-rejection/src/arm_eabi.rs b/libs/fault-rejection/src/arm_eabi.rs index 0a333d6..7790394 100644 --- a/libs/fault-rejection/src/arm_eabi.rs +++ b/libs/fault-rejection/src/arm_eabi.rs @@ -1,5 +1,22 @@ use core::mem::ManuallyDrop; +/// A very convoluted if statement. +/// +/// It consists of 3 parts: +/// - The expression callback +/// - The `TRUE` const +/// - The body callback +/// +/// The expression will be evaluated. If it returns the same value as `TRUE`, the body will be called. +/// +/// ## Claims +/// +/// The goal is to harden against fault injections. Specifically, this function protects against single(!) instruction skips. +/// If any instruction is skipped, either the body is not executed or a `udf` instruction is executed instead. +/// +/// This function does not protect against saturation faults (where a register is set to all-0 or all-1). +/// The `TRUE` value should be chosen so it's not all-0 or all-1 to mitigate this type of fault injection somewhat. +/// If that's the case, a saturation can in practise only affect some pointers which will cause UB when it's all-0 or a memfault when it's all-1. pub fn protected_if u32, B: FnOnce() -> R>( expression: E, body: B,