Harden dev mode value by diondokter · Pull Request #56 · OpenDevicePartnership/ec-slimloader

diondokter · 2026-10-21T09:17:19Z

Supercedes #54

Instead of collapsing dev mode into a bool, we keep it a very specific u32.

The secure_boot_cfg is also read twice.
This hopefully hardens against fault injections that saturate registers.
Everything after that uses a u32 that when saturated will not enable dev mode, so that should be good

felipebalbi · 2026-10-21T16:01:41Z

    /// 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 {
+    /// Dev mode is active when the returned value is [DEV_VAL]
+    fn dev_mode() -> u32 {


I don't like returning a raw u32 here. Can we have a Mode struct with PartialEq, Eq, named secure-boot and lifecycle fields, and constants built from the existing enums instead?

Something like below ought to do it? Maybe add a to_raw() -> u32 method to it for the boundary?

#[derive(PartialEq, Eq)] struct Mode { secure_boot: [SecBootEn; 2], lifecycle: Result<LifeCycleState, u8>, inverse_lifecycle: Result<InvLifeCycleState, u8>, }

Ideally, you would return a raw constant here, a specific made-up pattern that meets certain requirements e.g. the likes of NbootBool values.

A specific fault-resistant pattern makes sense, I agree. I’m not proposing replacing it with an ordinary Boolean. My concern is preserving the contract for future maintainers: a typed wrapper with private construction can enforce sample agreement and lifecycle/inverse consistency, while retaining the observed encoding for protected_if.

My original struct sketch does not enforce those relationships by itself, it needs a little more work :-). The goal is to validate at the boundary and preserve those invariants downstream, rather than make every caller understand an unrestricted u32. The exact representation can be designed in parallel, but the point remains: I would like the invariants to preserved at a constructor that refuses invalid representation centrally. Downstream users use the validated, readable representation only.

Ah yes, I see what you mean. Agreed.

felipebalbi · 2026-10-21T16:01:41Z

    }
 }

+pub const DEV_VAL: u32 = 0b00000011_00000011_00000011_11111100;


isn't this still the opposite of what #54 fixes? Both secure-boot bytes here are SecBootEn::OnlyPki, so a Develop device takes the self-derived-key path precisely when PKI enforcement is configured. We won't be testing the flashed keys in that configuration. See MCXA577 RM (MCXAP172M240F70RM, Rev. 1), section 8.4.1 for SEC_BOOT_EN.

Also, #54 allows every value except OnlyPki; one constant only matches one exact configuration. Do we want AllAllowed only, or all three non-OnlyPki configurations? Just changing these bytes to zero would not fully carry over #54.

isn't this still the opposite of what #54 fixes? Both secure-boot bytes here are SecBootEn::OnlyPki, so a Develop device takes the self-derived-key path precisely when PKI enforcement is configured. We won't be testing the flashed keys in that configuration. See MCXA577 RM (MCXAP172M240F70RM, Rev. 1), section 8.4.1 for SEC_BOOT_EN.

@felipebalbi is correct here (thanks!!), it reverses the fix from PR 54. We want DEV mode when NOT secure boot on with OnlyPki, the hybrid SB mode.

Also, #54 allows every value except OnlyPki; one constant only matches one exact configuration. Do we want AllAllowed only, or all three non-OnlyPki configurations? Just changing these bytes to zero would not fully carry over #54.

We need "NOT OnlyPki" because OnlyPki is the only one that will authenticate the SBL itself with ECDSA + MLDSA signatures via boot ROM on boot. So OnlyPki = no DEV MODE, other three = DEV mode.

felipebalbi · 2026-10-21T16:01:41Z

+        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_boot1 = unsafe { secure_boot_cfg_ptr.read_volatile() }


What fault model are we protecting against here? The two volatile reads will remain separate accesses, but what are we assuming about the duration and scope of the fault you're protecting against?

All zero attractors on the bus/register would mimic an AllAllowed for SB level. Idea is to read twice and cross check to guard against sustained glitches, against which the MCU built in GDET sensors can also activate.

All one faults would pass, though. No?

That's why it's read twice since supposedly the hardware would catch it if the injection was strong enough to do it twice in a row

Correct, they would "pass". But all ones would trigger SEC_BOOT_EN as OnlyPki, at which point DEV mode is NOT enabled, which means no root of trust bypass, which means an attacker trust chain cannot authenticate the image. ROM only trusts what is in IFR. So that attack fails safe to exactly what we need.

The whole point of the glitch hardening here is NOT to let an attacker glitch into DEV mode and have their RoT take over.

To clarify my earlier “all ones” question: I meant saturation of the secure-boot samples while the lifecycle bytes remain unaffected, not saturation of the entire packed word. With the policy corrected, all-ones samples should prevent dev mode.

My remaining question is about common-mode faults: two reads can agree on the same incorrect value. The RM documents ROM-configured GDET/aGDET protection, but §63.1 also requires system characterization, and §63.1.3 requires reconfiguration when core voltage changes. That does not establish that every fault affecting both reads will be detected.

Can we just document the assumed fault scope/duration and detector configuration? I’m asking for a bounded hardening claim, not immunity to every injection: redundant sampling helps against faults confined to one sample; coverage of faults affecting both needs separate justification.

Can we just document the assumed fault scope/duration and detector configuration?

IIRC boot is configured on Normal mode (from SYSCON), so GDET should be on VDD_CORE normal mode too. The SBL will operate as such (please ignore the current board.rs config, it's too fast for flexSPI anyway and I have PR coming changing all that back to Normal). But I have asked NXP to clarify and then we can document the boundaries.

Thank you, I think that should be a good addition here.

felipebalbi · 2026-10-21T16:01:41Z

+            Err(e) => e,
+        } as u32;
+
+        (secure_boot1 << 24) | (secure_boot2 << 16) | (lifecycle << 8) | (inv_lifecycle)


Can we split the reads from the decoding/packing so we can actually test this without dereferencing IFR? At least cover the intended dev configurations, Develop + OnlyPki keeping the programmed keys, disagreement between the two secure-boot samples, and invalid or mismatched lifecycle/inverse bytes. MCXA577 RM (MCXAP172M240F70RM, Rev. 1), section 4.1.3, table 18 gives the lifecycle pairs.

The tests need to exercise the helper used here, not a separate copy of the packing expression.

felipebalbi · 2026-10-21T16:01:41Z

    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, || {
+    let result = protected_if::<DEV_VAL, Result<(), BootError>, _, _>(dev_mode, || {


This also changes the meaning of the public callback without changing its type. A caller returning 1 still compiles, but no longer selects dev mode. A typed value would make that contract clearer.

We can still encode the observed fields into a word at the protected_if boundary, though.

DEV mode is a very specific scenario that MSFT needs, a normal bootloader ideally should not bypass its secure roots of trust. So, passing a 1 which compiles and bypasses DEV mode here would be international I believe, as you'd want to NOT use DEV mode.

Only the select pattern lets you though, all else verifies against the devices fused root of trust.

alamfarjadf · 2026-10-21T18:13:25Z

+            Err(e) => e,
+        } as u32;
+
+        (secure_boot1 << 24) | (secure_boot2 << 16) | (lifecycle << 8) | (inv_lifecycle)


This is a question that sort of circles back to the double read in my original PR: read twice via volatile then compare and if mismatch, return Err. The concern was although read_volatile contractually obligates the compiler to perform two reads, there is no obligation for it to use both values attained (if it can ascertain they might match). Isn't there such a concern here too? Why wouldn't the compiler just repeat the first read for the second one without any explicit guidance or asm guards?

If the compiler trully follows its own documentation and that of LLVM, I don't think we can get a situation where the compiler reuses the first value for the second. We will definitely get two reads and two fresh values. @diondokter?

alamfarjadf · 2026-10-21T18:20:36Z

#55 (comment)

And last open question / concern, but IMO not sure what else can be done (other than doing a check asm again, but where do we draw the line)

alamfarjadf · 2026-10-23T21:48:15Z

@felipebalbi @diondokter would 0b11111100_11111100_00000011_11111100 work for the pattern? Basically !OnlyPki when computing and if it matches, we know OnlyPki not set (DEV mode can be enabled).

Harden dev mode value

a8e8c53

diondokter force-pushed the devmode-sensitivity branch from e22cfe6 to a8e8c53 Compare October 21, 2026 09:45

felipebalbi reviewed Oct 21, 2026

View reviewed changes

alamfarjadf reviewed Oct 21, 2026

View reviewed changes

jerrysxie added this to EC Oct 22, 2026

jerrysxie moved this to In Progress in EC Oct 22, 2026

jerrysxie assigned felipebalbi Oct 22, 2026

jerrysxie requested a review from JamesHuard October 22, 2026 15:51

Conversation

diondokter commented Oct 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamfarjadf commented Oct 21, 2026

Uh oh!

alamfarjadf commented Oct 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants