diff --git a/services/orchestrator/sm/src/lib.rs b/services/orchestrator/sm/src/lib.rs index 956689ba..bb0a8972 100644 --- a/services/orchestrator/sm/src/lib.rs +++ b/services/orchestrator/sm/src/lib.rs @@ -5,7 +5,7 @@ //! //! This is the pure-reducer core ported from `rot_reducer`. It describes side //! effects as [`Effect`] values rather than performing them; the surrounding -//! OpenPRoT shell carries them out via a [`Platform`] impl. No concrete hardware +//! OpenPRoT platform driver carries them out via a [`Platform`] impl. No concrete hardware //! appears here — the machine is generic over an opaque [`ComponentId`]. //! //! See `docs/verification-model.md` and `docs/state-machine.md` in the @@ -30,21 +30,20 @@ pub use model::*; // deployment. The board owns `N` (chain length), `E` (effect-buffer size) and // max_retry. -/// Upper bound on how many events one settle can queue: the triggering outside -/// event, at most one `Emit` follow-up (`RecoveryFailed`, emitted at most once -/// before latching), and at most one injected `EffectFailed` (de-duplicated in -/// `dispatch_with` — it is idempotent and terminal, so a second is never -/// queued). Three total; `PENDING_CAP` keeps headroom above that so the pushes -/// in `dispatch_with` can never overflow. +/// Upper bound on events queued at once inside `dispatch_with`. The queue +/// pops as it settles, so this bounds *in-flight* events, not a run's total +/// length: one batch can queue at most one `Emit` follow-up plus one returned +/// event per external effect. Executors that return an event for many effects +/// of one batch can overflow this; overflow is fail-closed (see +/// `dispatch_with`), never silent loss. const PENDING_CAP: usize = 8; -/// Compile-time floor tying the queue capacity to that worst case, mirroring -/// `Rot::EFFECT_CAP_OK` for the effect buffer. Evaluated at build time (an -/// anonymous `const`), so an under-sized `PENDING_CAP` fails to compile rather -/// than risking a runtime overflow. +/// Compile-time floor: room for an `Emit` follow-up, a returned event, and +/// the injected `EffectFailed`. Evaluated at build time (an anonymous +/// `const`), so an under-sized `PENDING_CAP` fails to compile. const _: () = assert!( PENDING_CAP >= 3, - "PENDING_CAP must hold one outside event + one Emit follow-up + one EffectFailed", + "PENDING_CAP must hold an Emit follow-up + a returned event + EffectFailed", ); /// Result of dispatching one event to a state (or its superstate). @@ -155,7 +154,7 @@ struct ComponentStatus { retry: u8, /// Set while this component has been released from reset but has not yet /// reported its boot-progress signal ([`Event::ComponentReady`] for an - /// `Active` component, [`Event::Booted`] for a `Passive` one). The shell + /// `Active` component, [`Event::Booted`] for a `Passive` one). The platform driver /// arms a per-component watchdog on release; this bit is what a later /// [`Event::Timeout`] consults to tell a real boot failure from a stale or /// spurious timeout. Orthogonal to `lifecycle`: a gated component owes no @@ -319,7 +318,7 @@ impl Rot { } /// Record that `id` has been released and now owes a boot-progress signal. - /// Paired with the `ReleaseReset` emitted at each release site: the shell + /// Paired with the `ReleaseReset` emitted at each release site: the platform driver /// arms its per-component boot watchdog there, and this arms ours. Also /// marks the component *live* (`released`), which a later re-entry to /// [`State::PreSupervision`] uses to quiesce it before re-verifying. @@ -871,13 +870,13 @@ impl Rot { } } -/// Signals that the shell could not carry out an [`Effect`]. The machine does -/// not need the shell's error detail — **every** actuation failure is treated +/// Signals that the platform driver could not carry out an [`Effect`]. The machine does +/// not need the driver's error detail — **every** actuation failure is treated /// the same, fail-closed: the driver injects [`Event::EffectFailed`] and the /// machine latches to [`State::Locked`]. This blanket policy is deliberate and /// is what lets the failure signal stay a payload-less marker; a future design /// that needs per-effect recovery must add a *new*, descriptive event rather -/// than widen this type. The shell logs the specifics on its side. +/// than widen this type. The driver logs the specifics on its side. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct EffectError; @@ -885,10 +884,25 @@ pub struct EffectError; /// [`EffectError`] if it could not be performed. Never called with /// [`Effect::Emit`] — the orchestrator consumes those internally. /// +/// `Ok(Some(event))` feeds back what the effect produced synchronously (e.g. +/// a verification verdict); the driver queues it and settles it in the same +/// dispatch run. At most one event per effect. Synchronous results belong +/// here, not in a driver-side queue — one feedback path keeps ordering honest. +/// Never block in `execute`: results that arrive later (boot progress, timer +/// expiry) are delivered as their own outside events via `dispatch`. +/// +/// Failure stays on the error channel, never in a returned event: `Err` is +/// checked between effects, so a failed actuation aborts the rest of the +/// batch — a feedback event cannot do that. +/// /// Contract the reducer relies on: /// - **Honest, complete feedback.** The reducer's correctness rests entirely on -/// the event stream the shell feeds back; dropping, reordering, or +/// the event stream the driver feeds back; dropping, reordering, or /// synthesizing events silently breaks the state machine's invariants. +/// - **Returned events quiesce.** Every returned event reports a result the +/// reducer consumes (its retry budgets bound re-verification cycles). An +/// executor that manufactures an event for every effect keeps one dispatch +/// run alive indefinitely. /// - **`AssertReset` holds, it does not pulse.** A reset must keep the component /// quiesced and non-executing until its matching `ReleaseReset`. The reducer's /// at-rest verification guarantee depends on this: it re-asserts reset on @@ -898,10 +912,10 @@ pub struct EffectError; /// component resume before verification and void that guarantee. /// - **A failed [`Effect::LatchLockdown`] is a hard fault.** Lockdown is the top /// of the escalation ladder — the reducer has nothing stronger to emit and -/// will *believe* it is `Locked`. The shell must treat that failure as +/// will *believe* it is `Locked`. The driver must treat that failure as /// terminal (halt/reset), not a recoverable error. pub trait Platform { - fn execute(&mut self, effect: Effect) -> Result<(), EffectError>; + fn execute(&mut self, effect: Effect) -> Result, EffectError>; } /// A handle for a caller's own event loop. Owns the machine's storage @@ -983,61 +997,77 @@ impl Orchestrator { } } - /// Handle one event all the way through — including any [`Effect::Emit`] - /// follow-ups — calling `on_effect` for each external effect in order. + /// Handle one event all the way through — every [`Effect::Emit`] + /// follow-up and every event the executors return — calling `on_effect` + /// for each external effect in order. One call runs to quiescence. /// /// If `on_effect` reports an [`EffectError`], the driver injects an - /// [`Event::EffectFailed`] into the same run, so a failed actuation is - /// handled fail-closed (the machine latches to [`State::Locked`]) rather - /// than silently ignored. + /// [`Event::EffectFailed`] at the *front* of the queue, so a failed + /// actuation is handled fail-closed: the latch settles next, and feedback + /// still queued behind it drains into [`State::Locked`] (discarded) + /// instead of actuating hardware after a failure. A pending-queue + /// overflow is handled the same way: losing a returned event would break + /// the honest-feedback contract, so the run latches instead. pub fn dispatch_with( &mut self, event: Event, - mut on_effect: impl FnMut(Effect) -> Result<(), EffectError>, + mut on_effect: impl FnMut(Effect) -> Result, EffectError>, ) { - let mut pending: heapless::Vec = heapless::Vec::new(); + // Fail-closed latch: `EffectFailed` goes to the *front*, so it settles + // next and everything still queued drains into `Locked` (discarded) + // instead of actuating hardware after a failure. Prefer evicting the + // newest queued event over losing the latch itself. + fn latch(pending: &mut heapless::Deque) { + if pending.is_full() { + pending.pop_back(); + } + // Dead Err arm: the eviction above guarantees room. + let _ = pending.push_front(Event::EffectFailed); + } + + let mut pending: heapless::Deque = heapless::Deque::new(); // Dead Err arm: `pending` is empty and `PENDING_CAP >= 3` (asserted at // build time), so the first push always fits. - let _ = pending.push(event); - - let mut i = 0; - while i < pending.len() { - let ev = pending[i]; - i += 1; + let _ = pending.push_back(event); + // `EffectFailed` is injected at most once: it is idempotent and + // terminal (drives to `Locked`, which discards everything after). The + // only external effect executed after it settles is `Locked`'s own + // entry, whose failure must not inject again. + let mut failed = false; + while let Some(ev) = pending.pop_front() { let mut buf = Sink::::new(); self.step(&ev, &mut buf); for &effect in buf.effects() { - match effect { - Effect::Emit(internal) => { - // Dead Err arm: the reducer emits at most one `Emit` - // (`RecoveryFailed`) per settle, well within PENDING_CAP. - let _ = pending.push(internal); - } - external => { - if on_effect(external).is_err() { - // Fail-closed AND fail-fast: abandon the rest of this - // batch so no effect ordered after the failed one hits - // hardware. `step` has already advanced the state as if - // the whole batch applied, so actuating `k+1..` would - // carry out effects for a transition we are about to - // override by latching to `Locked`. - // - // Inject `EffectFailed` once: it is idempotent and - // terminal (drives to `Locked`, which discards - // everything after), so a second injection would be a - // no-op. De-duping against this append-only queue — - // which still holds the first `EffectFailed` as its own - // marker — caps the queue at the worst case - // `PENDING_CAP` is sized for. Dead Err arm: that bound - // is below PENDING_CAP. - if !pending.contains(&Event::EffectFailed) { - let _ = pending.push(Event::EffectFailed); + let follow_up = match effect { + // Internal: handle next, never forwarded to the platform. + Effect::Emit(internal) => Some(internal), + external => match on_effect(external) { + Ok(follow_up) => follow_up, + Err(_) => { + // Fail-closed AND fail-fast: abandon the rest of + // this batch. `step` has already advanced the + // state as if the whole batch applied, and the + // latch overrides that transition, so nothing + // ordered after the failure may hit hardware. + if !failed { + failed = true; + latch(&mut pending); } break; } - } + }, + }; + if let Some(next) = follow_up + && pending.push_back(next).is_err() + && !failed + { + // Queue full: `next` would be lost, breaking the + // honest-feedback contract. Fail closed instead. + failed = true; + latch(&mut pending); + break; } } } diff --git a/services/orchestrator/sm/src/model.rs b/services/orchestrator/sm/src/model.rs index fff7cb6a..ce158cc3 100644 --- a/services/orchestrator/sm/src/model.rs +++ b/services/orchestrator/sm/src/model.rs @@ -67,7 +67,7 @@ pub enum FailurePolicy { /// Opaque recovery-region key supplied by the board at chain-build time. /// Components sharing a `RegionId` are restored together: when any region -/// member enters [`State::Recovering`], the shell resolves and restores the +/// member enters [`State::Recovering`], the platform driver resolves and restores the /// whole region. The core treats this as an equality key only and never /// inspects membership itself. #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -182,7 +182,7 @@ pub enum PowerOnResult { /// Everything the outside world can tell the state machine. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Event { - /// Power-on, carrying the shell's self-verification and provisioning result. + /// Power-on, carrying the platform driver's self-verification and provisioning result. PowerGood(PowerOnResult), /// The eRoT's signature + SVN check on this component passed. VerificationPassed(ComponentId), @@ -215,7 +215,7 @@ pub enum Event { Restored(ComponentId), /// A required component's recovery was exhausted. RecoveryFailed, - /// The shell's boot-progress watchdog fired: `id` did not report its + /// The platform driver's boot-progress watchdog fired: `id` did not report its /// boot-progress signal ([`Event::ComponentReady`] for an `Active` /// component, [`Event::Booted`] for a `Passive` one) within its configured /// boot timeout. Treated as a verification failure — a component still @@ -236,7 +236,7 @@ pub enum Event { /// when it executes [`Effect::ActivateUpdate`] and cancels it on /// [`Effect::CommitSvnFloor`]. CommitTimeout, - /// The shell could not carry out an emitted [`Effect`]; fail-closed, it + /// The platform driver could not carry out an emitted [`Effect`]; fail-closed, it /// latches to [`State::Locked`] from any state. Injected by the driver when /// a [`Platform::execute`](crate::Platform::execute) call fails; never /// produced by a handler. diff --git a/services/orchestrator/sm/src/tests.rs b/services/orchestrator/sm/src/tests.rs index df188ba0..a6919df7 100644 --- a/services/orchestrator/sm/src/tests.rs +++ b/services/orchestrator/sm/src/tests.rs @@ -48,9 +48,9 @@ impl Recorder { } impl Platform for Recorder { - fn execute(&mut self, effect: Effect) -> Result<(), EffectError> { + fn execute(&mut self, effect: Effect) -> Result, EffectError> { self.recorded.push(effect); - Ok(()) + Ok(None) } } @@ -359,7 +359,7 @@ fn retry_count_resets_after_successful_recovery() { ] { orch.dispatch_with(ev, |e| { effects.push(e); - Ok(()) + Ok(None) }); } assert_eq!(orch.state(), State::Ready); @@ -372,7 +372,7 @@ fn retry_count_resets_after_successful_recovery() { ] { orch.dispatch_with(ev, |e| { effects.push(e); - Ok(()) + Ok(None) }); } assert_eq!(orch.state(), State::Ready); @@ -407,7 +407,7 @@ fn retry_budget_is_per_component() { ] { orch.dispatch_with(ev, |e| { effects.push(e); - Ok(()) + Ok(None) }); } @@ -432,7 +432,7 @@ fn custom_retry_cap_latches_sooner() { ] { orch.dispatch_with(ev, |e| { effects.push(e); - Ok(()) + Ok(None) }); } assert_eq!(orch.state(), State::Locked); @@ -457,7 +457,7 @@ fn custom_capacity_walks_full_chain() { ] { orch.dispatch_with(ev, |e| { effects.push(e); - Ok(()) + Ok(None) }); } assert_eq!(orch.state(), State::Ready); @@ -1428,7 +1428,7 @@ fn locked_is_terminal() { for ev in [BOOT, Event::VerificationFailed(C0), Event::Restored(C0)] { orch.dispatch_with(ev, |e| { effects.push(e); - Ok(()) + Ok(None) }); } assert_eq!(orch.state(), State::Locked); @@ -1443,7 +1443,7 @@ fn locked_is_terminal() { ] { orch.dispatch_with(ev, |e| { effects.push(e); - Ok(()) + Ok(None) }); } assert_eq!( @@ -1497,7 +1497,7 @@ fn speculative_read_effects_are_emitted_together() { orch.dispatch_with(BOOT, |e| { effects.push(e); - Ok(()) + Ok(None) }); assert_eq!( effects, @@ -1507,7 +1507,7 @@ fn speculative_read_effects_are_emitted_together() { effects.clear(); orch.dispatch_with(Event::VerificationPassed(C0), |e| { effects.push(e); - Ok(()) + Ok(None) }); // All three effects emitted in the same handler, before ComponentReady. assert_eq!( @@ -1632,13 +1632,13 @@ impl FailOn { } impl Platform for FailOn { - fn execute(&mut self, effect: Effect) -> Result<(), EffectError> { + fn execute(&mut self, effect: Effect) -> Result, EffectError> { self.recorded.push(effect); if effect == self.trigger { self.failed = true; Err(EffectError) } else { - Ok(()) + Ok(None) } } } @@ -1682,7 +1682,7 @@ fn failed_isolation_actuation_latches_lockdown() { assert!(plat.recorded.contains(&Effect::LatchLockdown)); } -/// A failed recovery actuation is fail-closed too: if the shell cannot even +/// A failed recovery actuation is fail-closed too: if the platform driver cannot even /// recover a required component, the platform latches rather /// than continuing with an unrecovered component. #[test] @@ -1966,3 +1966,156 @@ fn property_verify_before_release_holds_under_random_sequences() { } } } + +/// A returned event settles in the same run: a platform that answers every +/// `VerifyFirmware` in place walks a provisioned chain to `Ready` from one +/// dispatch. +#[test] +fn returned_verdicts_settle_in_one_dispatch() { + struct InstantVerify { + recorded: Vec, + } + impl Platform for InstantVerify { + fn execute(&mut self, effect: Effect) -> Result, EffectError> { + self.recorded.push(effect); + Ok(match effect { + Effect::VerifyFirmware(id) => Some(Event::VerificationPassed(id)), + _ => None, + }) + } + } + + let mut orch = Orchestrator::::new( + passive_required(&[C0, C1]).try_into().expect("valid chain"), + MAX_RETRY, + ); + let mut platform = InstantVerify { + recorded: Vec::new(), + }; + + orch.dispatch(&mut platform, BOOT); + + assert_eq!(orch.state(), State::Ready); + assert_eq!( + platform.recorded, + std::vec![ + Effect::ReadFirmware(C0), + Effect::VerifyFirmware(C0), + Effect::ReleaseReset(C0), + Effect::ReadFirmware(C1), + Effect::VerifyFirmware(C1), + Effect::ReleaseReset(C1), + ], + ); +} + +/// A batch whose executors return more events than the pending queue holds +/// fails closed: the run latches `Locked` instead of losing feedback. +#[test] +fn returned_event_overflow_latches_locked() { + struct Chatty; + impl Platform for Chatty { + fn execute(&mut self, effect: Effect) -> Result, EffectError> { + Ok(match effect { + // The re-walk after Restored quiesces every live component + // and re-verifies the first — on a full 8-chain that is more + // returned events than PENDING_CAP in one batch. + Effect::AssertReset(_) | Effect::ReadFirmware(_) => { + Some(Event::AttestationChallenge) + } + Effect::VerifyFirmware(id) => Some(Event::VerificationPassed(id)), + _ => None, + }) + } + } + + let ids: Vec = (0..8).map(ComponentId::new).collect(); + let mut orch = Orchestrator::::new( + passive_required(&ids).try_into().expect("valid chain"), + MAX_RETRY, + ); + let mut platform = Chatty; + + orch.dispatch(&mut platform, BOOT); + assert_eq!(orch.state(), State::Ready); + + orch.dispatch(&mut platform, Event::CorruptionDetected(C0)); + orch.dispatch(&mut platform, Event::Restored(C0)); + + assert_eq!(orch.state(), State::Locked); +} + +/// A failed effect with the pending queue already full still latches: the +/// latch evicts the newest queued event rather than being dropped itself. +/// Regression test — a back-of-queue push would be lost here, and the run +/// would settle to `Ready` as if nothing failed. +#[test] +fn failed_effect_with_full_queue_still_latches() { + struct ChattyThenFail { + rewalking: bool, + } + impl Platform for ChattyThenFail { + fn execute(&mut self, effect: Effect) -> Result, EffectError> { + match effect { + // The re-walk after Restored asserts reset on all eight live + // components first — exactly PENDING_CAP returned events, so + // the queue is full (but never overflowed) when the read + // that follows fails. + Effect::AssertReset(_) if self.rewalking => Ok(Some(Event::AttestationChallenge)), + Effect::ReadFirmware(_) if self.rewalking => Err(EffectError), + Effect::VerifyFirmware(id) => Ok(Some(Event::VerificationPassed(id))), + _ => Ok(None), + } + } + } + + let ids: Vec = (0..8).map(ComponentId::new).collect(); + let mut orch = Orchestrator::::new( + passive_required(&ids).try_into().expect("valid chain"), + MAX_RETRY, + ); + let mut platform = ChattyThenFail { rewalking: false }; + + orch.dispatch(&mut platform, BOOT); + assert_eq!(orch.state(), State::Ready); + + orch.dispatch(&mut platform, Event::CorruptionDetected(C0)); + platform.rewalking = true; + orch.dispatch(&mut platform, Event::Restored(C0)); + + assert_eq!(orch.state(), State::Locked); +} + +/// An event returned by executing `LatchLockdown` itself is queued, settles +/// in `Locked`, and is discarded — the latch stays terminal and nothing is +/// actuated after the lockdown. +#[test] +fn event_returned_during_lockdown_is_discarded() { + struct FailRelease { + recorded: Vec, + } + impl Platform for FailRelease { + fn execute(&mut self, effect: Effect) -> Result, EffectError> { + self.recorded.push(effect); + match effect { + Effect::ReleaseReset(_) => Err(EffectError), + Effect::LatchLockdown => Ok(Some(Event::AttestationChallenge)), + _ => Ok(None), + } + } + } + + let mut orch = Orchestrator::::new( + passive_required(&[C0]).try_into().expect("valid chain"), + MAX_RETRY, + ); + let mut platform = FailRelease { + recorded: Vec::new(), + }; + + orch.dispatch(&mut platform, BOOT); + orch.dispatch(&mut platform, Event::VerificationPassed(C0)); + + assert_eq!(orch.state(), State::Locked); + assert_eq!(platform.recorded.last(), Some(&Effect::LatchLockdown)); +}