Harden dev mode value

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<C: McxaConfig + BootStatePolicy> Board for Mcxa<C> {
         #[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<C: McxaConfig + BootStatePolicy> Board for Mcxa<C> {
             }
 
             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<C: McxaConfig + BootStatePolicy> Mcxa<C> {
         ec_slimloader::start::<Self, JOURNAL_BUFFER_SIZE>(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, u32>(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
+        );
+    }
 }

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<NbootImgAuthParms, BootError> {
@@ -74,7 +75,7 @@ fn load_nboot_auth_parms_from_ifr(ifr: &IFR) -> Result<NbootImgAuthParms, BootEr
 pub fn verify_authenticity<'d>(
     mut peri: Peri<'d, peripherals::SGI0>,
     image_base: *const u8,
-    dev_mode: impl FnOnce() -> u32,
+    boot_mode: impl FnOnce() -> BootMode,
 ) -> Result<NbootBoolValue, BootError> {
     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?,

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<const TRUE: u32, R, E: FnOnce() -> u32, B: FnOnce() -> R>(
     expression: E,
     body: B,