From fc6acbbb37978f324baa32182433a26bc037361a Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Sun, 4 Oct 2026 20:14:03 +0200 Subject: [PATCH] sm: Walk the platform again after an update activates Activation only proposes the image. The device keeps running the old one until it is reset, so the machine now enters PreSupervision instead of returning to Ready: that entry quiesces every live component and verifies each image at rest before releasing it, which is the reset that boots the candidate. It also fixes a stale floor commit. CommitSvnFloor advances to verified_svn, which only the walk's VerifyFirmware records. Without the re-walk a BootConfirmed after an update committed the floor to the previous image's SVN, leaving the downgrade window the floor exists to close. The walk re-verifies the new image first, so the commit takes its SVN. The commit window now spans that walk, so every state it passes through has to answer CommitTimeout. PreSupervision is unsupervised and had no match arm, and the supervising handler had none either, so a watchdog fire during the walk was dropped and never came again: commit-or-lock went unenforced for the length of a boot. Both match arms added. The image is judged twice, by the pumped verifier before activation and by the walk after the reset. That is the point: the second one covers code at rest, in the slot the device actually booted. The effect bound still holds: this dispatch emits ActivateUpdate plus N quiesce asserts plus the entry's two walk effects, N+3 against the E >= 2N+2 the buffer is sized for. The documented bound came from the gate cascade, which is the larger path. Assisted-by: Claude --- .../orchestrator/orchestrator-machine.md | 8 +- services/orchestrator/driver/src/tests.rs | 38 +++++-- services/orchestrator/sm/src/rot.rs | 51 ++++++++- services/orchestrator/sm/src/tests.rs | 104 +++++++++++++++++- 4 files changed, 181 insertions(+), 20 deletions(-) diff --git a/docs/src/design/orchestrator/orchestrator-machine.md b/docs/src/design/orchestrator/orchestrator-machine.md index 6b109592..c3a5dc7d 100644 --- a/docs/src/design/orchestrator/orchestrator-machine.md +++ b/docs/src/design/orchestrator/orchestrator-machine.md @@ -25,7 +25,7 @@ stateDiagram-v2 state SupervisingPlatform { Ready --> Updating : UpdateRequest(id)
/ AuthenticateStageUpdate - Updating --> Ready : UpdateVerified / ActivateUpdate + Updating --> PreSupervision : UpdateVerified / ActivateUpdate Updating --> Ready : UpdateRejected / DiscardStaged Ready --> Recovering : CorruptionDetected
/ RestoreGoldenImage Updating --> Recovering : CorruptionDetected
/ RestoreGoldenImage @@ -116,6 +116,8 @@ here — they persist across re-walks so exhausted components are not re-verifie | `VerificationFailed(id)` | — | — | `Recovering(id)` — recovery is attempted first, regardless of the component's recovery-failure policy | | `CorruptionDetected(id)` | `Required`/unknown | `RestoreGoldenImage` | `Recovering(id)` | | `CorruptionDetected(id)` | `Isolable`/`Cascading` | `AssertReset` · `ReportIsolated` | `Handled` (component gated; walk continues) | +| `CommitTimeout` | `pending_commit` set | — | `Locked` (commit window spans the post-activation walk; `LatchLockdown` on entry) | +| `CommitTimeout` | `pending_commit` clear | — | `Handled` (no window open, nothing to enforce) | | anything else | — | — | `Outcome::Super` (top level — discarded) | When advancing the cursor, any component marked `Isolated` is skipped without @@ -203,7 +205,7 @@ state's payload. | Event | Guard | Effects | Next state | |---|---|---|---| -| `UpdateVerified` | — | `ActivateUpdate` | `Ready` (commit window opens on the payload) | +| `UpdateVerified` | — | `ActivateUpdate` | `PreSupervision` (re-walk; commit window spans the walk) | | `UpdateRejected` | — | `DiscardStaged` | `Ready` (INV4) | | `CorruptionDetected(id)` | `Required`/unknown | `DiscardStaged` (then `RestoreGoldenImage` on entry) | `Recovering(id)` (update preempted; staged image discarded) | | `CorruptionDetected(id)` | `Isolable`/`Cascading` | `AssertReset(id)` · `ReportIsolated(id)` | `Handled` (component gated; update continues, staged image kept) | @@ -333,6 +335,8 @@ handler (`handle_supervising`). | `AttestationChallenge` | — | `SignAttestation` | `Handled` (no transition — INV6) | | `CorruptionDetected(id)` | `attrs.failure_policy == Required` | — | `Recovering(id)` (INV5) | | `CorruptionDetected(id)` | `attrs.failure_policy != Required` | `AssertReset(id)` · `ReportIsolated(id)` | `Handled` (component gated; machine stays in current state) | +| `CommitTimeout` | `pending_commit` set | — | `Locked` (the post-activation walk passes through supervised states too; `LatchLockdown` on entry) | +| `CommitTimeout` | `pending_commit` clear | — | `Handled` | | anything else | — | — | `Outcome::Super` (discarded) | --- diff --git a/services/orchestrator/driver/src/tests.rs b/services/orchestrator/driver/src/tests.rs index b733d8f9..aef63c19 100644 --- a/services/orchestrator/driver/src/tests.rs +++ b/services/orchestrator/driver/src/tests.rs @@ -170,6 +170,9 @@ impl core::error::Error for ResetFault {} /// line after the control moves into the driver. struct MockReset { held: std::rc::Rc>, + /// Times the line was asserted, so a test can tell one boot from a + /// second one. + holds: std::rc::Rc>, fail: bool, } @@ -177,6 +180,7 @@ impl MockReset { fn new() -> Self { Self { held: std::rc::Rc::new(core::cell::Cell::new(true)), + holds: std::rc::Rc::new(core::cell::Cell::new(0)), fail: false, } } @@ -189,6 +193,7 @@ impl orchestrator_capabilities::BootControl for MockReset { if self.fail { return Err(ResetFault); } + self.holds.set(self.holds.get() + 1); self.held.set(true); Ok(()) } @@ -1972,30 +1977,43 @@ fn activate_update_ends_the_job() { assert_eq!(driver.pending_update(), None); } -// The path through the SM up to Staged: a request, the pump, staging -// completes, and the pump parks. The full path through activation -// requires the crypto verify-client (emitting UpdateVerified). +// The path through the SM end to end: a request, the pump, staging +// completes, the verdict activates, and the walk that follows resets the +// component into what it just activated. +// +// The verdict is dispatched here rather than read off the pump, which +// parks at Staged until the crypto verify client is wired. #[test] -fn an_update_stages_through_the_sm() { +fn an_update_runs_through_the_sm_and_rewalks() { let mut orch = orchestrator(); let mut driver = update_driver(MockUpdatable::stepping(2)); orch.dispatch(&mut driver, Event::PowerGood(PowerOnResult::Provisioned)); assert_eq!(orch.state(), State::Ready); + let resets_before = driver.board().boot_controls[0].holds.get(); + request_update(&mut orch, &mut driver, C0, CANDIDATE_LEN).unwrap(); assert_eq!(orch.state(), State::Updating(C0)); for tick in 0..16 { - if let Some(event) = driver.pump_update(tick).event { - orch.dispatch(&mut driver, event); + driver.pump_update(tick); + if driver.board().updatables[0].ready { break; } } + assert!(driver.board().updatables[0].ready, "never staged"); - // Pump parked at Staged, no event emitted. SM stays in Updating. - assert_eq!(orch.state(), State::Updating(C0)); - assert!(driver.board().updatables[0].ready); - assert!(driver.pending_update().is_some()); + orch.dispatch(&mut driver, Event::UpdateVerified); + + assert_eq!(orch.state(), State::Ready); + assert!(driver.board().updatables[0].active); + assert_eq!(driver.pending_update(), None); + // The activation only proposed the image; the walk that followed is + // what reset the device into it. + assert!( + driver.board().boot_controls[0].holds.get() > resets_before, + "the updated component was never reset" + ); } // The rejection path through the SM: DiscardStaged runs and the platform diff --git a/services/orchestrator/sm/src/rot.rs b/services/orchestrator/sm/src/rot.rs index b193f818..cc73ce3f 100644 --- a/services/orchestrator/sm/src/rot.rs +++ b/services/orchestrator/sm/src/rot.rs @@ -43,10 +43,21 @@ pub struct Rot { /// [`Event::CommitTimeout`] while it is still set goes to /// [`State::Locked`]. /// - /// Also cleared on entry to [`State::Updating`] (a newer update replaces - /// this one) and [`State::Recovering`] (the running image is now suspect), - /// the two ways to leave `Ready` while still running. Not cleared on - /// `Ready` entry, because activation sets it on the way in. + /// The window spans the walk that follows an activation: the device only + /// boots the candidate once it is reset, so the window cannot close before + /// that walk finishes. Every state it passes through answers + /// [`Event::CommitTimeout`], including the unsupervised + /// [`State::PreSupervision`], or a fire during the walk would be dropped + /// and never come again. In a multi-component chain, the updated device + /// can boot (and confirm) while the walk is still verifying later + /// neighbours; `BootConfirmed` is only handled in `Ready`, so the + /// emitter must re-raise or hold it until the walk finishes. + /// + /// Cleared on the matching [`Event::BootConfirmed`] (the window closes + /// normally) and on entry to the two states that end the window by leaving + /// `Ready` while still running, [`State::Updating`] (a superseding update) + /// and [`State::Recovering`] (the running image is now suspect). Not cleared + /// on `Ready` entry, because activation sets it on the way in. pending_commit: Option, /// Ties the effect-buffer size `E` to this type (zero-sized). _effect_cap: PhantomData<[u8; E]>, @@ -394,6 +405,18 @@ impl Rot { // Cursor walk via Outcome::Handled — a self-transition would reset cursor. State::PreSupervision => match event { + // The commit window can span this walk: an activation enters + // `PreSupervision` with the window open. `PreSupervision` is + // unsupervised, so without this match arm the watchdog fire would be + // dropped and never come again, leaving commit-or-lock + // unenforced for the length of a boot. + Event::CommitTimeout => { + if self.pending_commit.is_some() { + Outcome::Transition(State::Locked) + } else { + Outcome::Handled + } + } Event::VerificationPassed(id) => { // Only the component currently under verification // (`chain[cursor]`, whose `VerifyFirmware` was just emitted) @@ -617,7 +640,15 @@ impl Rot { // driver arms its commit watchdog on `ActivateUpdate`, and // `CommitTimeout` bounds this window (commit-or-lock). self.pending_commit = Some(target); - Outcome::Transition(State::Ready) + // Re-walk rather than returning to `Ready`. Activation only + // proposes the image; the device runs the old one until it + // is reset, and `PreSupervision` entry quiesces every live + // component before verifying at rest. That reset is what + // boots the candidate, and the walk's `VerifyFirmware` is + // what records its SVN, which the floor commit then takes. + // Without it a `BootConfirmed` would commit the previous + // image's SVN and leave the downgrade window open. + Outcome::Transition(State::PreSupervision) } Event::UpdateRejected => { ctx.emit(Effect::DiscardStaged); @@ -769,6 +800,16 @@ impl Rot { ctx.emit(Effect::ReportUpdateDeferred); Outcome::Handled } + // Same window, same reason as `PreSupervision`'s match arm: the walk + // that follows an activation passes through the supervised + // states too, and a dropped watchdog fire never returns. + Event::CommitTimeout => { + if self.pending_commit.is_some() { + Outcome::Transition(State::Locked) + } else { + Outcome::Handled + } + } Event::EffectFailed => Outcome::Transition(State::Locked), _ => Outcome::Super, } diff --git a/services/orchestrator/sm/src/tests.rs b/services/orchestrator/sm/src/tests.rs index d52361e2..a877c1cf 100644 --- a/services/orchestrator/sm/src/tests.rs +++ b/services/orchestrator/sm/src/tests.rs @@ -1979,6 +1979,9 @@ fn an_unrelated_boot_confirmed_leaves_the_commit_window_open() { Event::VerificationPassed(C1), Event::UpdateRequest(C0), Event::UpdateVerified, + // The re-walk has to finish before the confirm can land. + Event::VerificationPassed(C0), + Event::VerificationPassed(C1), Event::BootConfirmed(C1), Event::CommitTimeout, ], @@ -2010,6 +2013,9 @@ fn a_sibling_boot_confirmed_does_not_consume_the_window() { Event::VerificationPassed(C1), Event::UpdateRequest(C0), Event::UpdateVerified, + // The re-walk has to finish before the confirm can land. + Event::VerificationPassed(C0), + Event::VerificationPassed(C1), Event::BootConfirmed(C1), Event::BootConfirmed(C0), ], @@ -2062,12 +2068,98 @@ fn update_verified_activates_update() { Event::UpdateVerified, ], ); - assert_eq!(state, State::Ready); + // Activation only proposes the image. The device runs the old one + // until the walk resets it, so the machine walks rather than + // returning to Ready. + assert_eq!(state, State::PreSupervision); assert!(effects.contains(&Effect::ActivateUpdate)); assert!(!effects.contains(&Effect::DiscardStaged)); assert!(!effects.contains(&Effect::RecoverComponent { id: C0, attempt: 0 })); } +/// The re-walk is what boots the candidate: the activated component is +/// quiesced, verified at rest, and released again. Its `VerifyFirmware` +/// is also what records the new image's SVN for a later floor commit. +#[test] +fn an_activation_resets_and_re_verifies_the_component() { + let (effects, state) = drive( + passive_required(&[C0]), + &[ + BOOT, + Event::VerificationPassed(C0), + Event::UpdateRequest(C0), + Event::UpdateVerified, + Event::VerificationPassed(C0), + ], + ); + + assert_eq!(state, State::Ready); + let activated = effects + .iter() + .position(|e| *e == Effect::ActivateUpdate) + .expect("never activated"); + let after = &effects[activated..]; + assert!(after.contains(&Effect::AssertReset(C0)), "never reset"); + assert!( + after.contains(&Effect::VerifyFirmware(C0)), + "never re-verified" + ); + assert!(after.contains(&Effect::ReleaseReset(C0)), "never released"); +} + +/// The walk that follows an activation passes through the supervised +/// states too, and the watchdog has to be answered there as well. An +/// active component parks the walk in `AwaitingReady` until its iRoT +/// reports, which is where this fire lands. +#[test] +fn a_commit_timeout_while_the_post_update_walk_awaits_readiness_latches_locked() { + // Two components, so the walk moves on to C1 and parks in + // AwaitingReady(C0) instead of finishing in Ready. + let (effects, state) = drive( + chain(&[ + (C0, ComponentAttrs::active_required()), + (C1, ComponentAttrs::passive_required()), + ]), + &[ + BOOT, + Event::VerificationPassed(C0), + Event::ComponentReady(C0), + Event::VerificationPassed(C1), + Event::UpdateRequest(C0), + Event::UpdateVerified, + // The re-walk releases C0 and moves to C1, so the machine is + // in AwaitingReady when the watchdog fires: the supervising + // handler is what has to answer it. + Event::VerificationPassed(C0), + Event::CommitTimeout, + ], + ); + + assert_eq!(state, State::Locked); + assert!(effects.contains(&Effect::LatchLockdown)); + assert!(!effects.contains(&Effect::CommitSvnFloor(C0))); +} + +/// A spurious watchdog fire during an ordinary boot walk, with no update +/// activated, must not brick the boot: there is no window to fail closed +/// on, so the walk carries on. +#[test] +fn a_commit_timeout_during_a_plain_boot_walk_is_ignored() { + let (effects, state) = drive( + passive_required(&[C0]), + &[ + BOOT, + // Mid-walk: C0 is verified but the machine has not left + // PreSupervision yet. + Event::CommitTimeout, + Event::VerificationPassed(C0), + ], + ); + + assert_eq!(state, State::Ready); + assert!(!effects.contains(&Effect::LatchLockdown)); +} + /// The anti-rollback floor is committed only on a proven-healthy boot, never /// at activation. `UpdateVerified` activates the image (authentication) but /// must NOT emit `CommitSvnFloor`; a later `BootConfirmed` (the runtime health @@ -2085,7 +2177,7 @@ fn svn_floor_commits_on_boot_confirmed_not_on_activation() { Event::UpdateVerified, ], ); - assert_eq!(activated_state, State::Ready); + assert_eq!(activated_state, State::PreSupervision); assert!(activated.contains(&Effect::ActivateUpdate)); assert!(!activated.contains(&Effect::CommitSvnFloor(C0))); @@ -2097,6 +2189,9 @@ fn svn_floor_commits_on_boot_confirmed_not_on_activation() { Event::VerificationPassed(C0), Event::UpdateRequest(C0), Event::UpdateVerified, + // The walk the activation started has to finish: the floor + // commit takes the SVN that walk verified. + Event::VerificationPassed(C0), Event::BootConfirmed(C0), ], ); @@ -2117,7 +2212,9 @@ fn commit_timeout_while_pending_latches_locked() { Event::VerificationPassed(C0), Event::UpdateRequest(C0), Event::UpdateVerified, - // Window open: activated, awaiting BootConfirmed. Watchdog fires. + // Window open and the re-walk still running: the watchdog + // fires in PreSupervision, which is unsupervised, so the match arm + // has to be there or the fire is dropped for good. Event::CommitTimeout, ], ); @@ -2140,6 +2237,7 @@ fn commit_timeout_after_confirm_is_stale_noop() { Event::VerificationPassed(C0), Event::UpdateRequest(C0), Event::UpdateVerified, + Event::VerificationPassed(C0), Event::BootConfirmed(C0), // Window already closed by the commit above. Event::CommitTimeout,