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,