Orchestrator spec and reference impl - #357
Conversation
|
Do you plan to realize your state machine implementation based on https://github.com/typeconstructor/rot-reducer? |
|
@rusty1968 Should we also add multi-level cascades, concurrent faults, and double-fault recovery tests? Or is this not necessary? |
|
Are we trying to restore the golden image several times or just once? |
There was a problem hiding this comment.
All the other crates have way fewer lines in lib.rs. Would it be better to put it into a separate .rs file?
There was a problem hiding this comment.
The split is model.rs, tests.rs and lib.rs
| rot.awaiting = Some(*id); | ||
| Outcome::Transition(State::AwaitingReady) | ||
| } | ||
| _ => Outcome::Handled, |
There was a problem hiding this comment.
What if the passive device does not manage to boot up? How will we notice that?
There was a problem hiding this comment.
CSA boot-progress checkpointing in 6d5f956
| } | ||
|
|
||
| /// Append one effect. Overflow is silently dropped rather than panicking | ||
| /// (`no_std` safety); overflow means a logic bug. |
There was a problem hiding this comment.
Why does overflow mean a logic bug? Silently dropped does not sound like something I want in my security relevant device.
| /// found. If the rest of the chain is exhausted or entirely held, sets | ||
| /// `cursor` to a past-the-end sentinel (`chain.len()`) and returns | ||
| /// `false` — the caller should treat that as "chain done". | ||
| fn advance_to_next_unheld(&mut self, ctx: &mut Sink, start_idx: usize) -> bool { |
There was a problem hiding this comment.
Device should be in reset when rechecking. A live device is already executing code it loaded before the check, so the recheck says nothing about what's running. It can rewrite its own flash right after the check passes. Either the re-walk re-asserts reset on every non-held component first (recovery = platform re-boot), or re-checking live components becomes an explicitly weaker, differently-named operation instead of the same VerifyFirmware.
| } | ||
| State::Recovering => { | ||
| if let Some(failed) = rot.failed { | ||
| ctx.emit(Effect::RestoreGoldenImage(failed)); |
There was a problem hiding this comment.
Why do we always go back to the golden image? Should we try the last known good before update instead?
There was a problem hiding this comment.
Your intuition is sharp here: the reason we trust golden image is because it is the one trustworthy - it comes from an immutable partition. Previous could perhaps buy us an availability optimization. When/if we have a stringent availability requirement, we can revisit this, but now perhaps is premature optimization.
There was a problem hiding this comment.
Advancing the SVN floor before we have a successful boot feels a bit early. If we only advance the SVN when we have a device that booted up, we could go back to the last version of the firmware if the boot fails. 5 years in, the golden image will be very old, so it could be a good idea to jump back to a more recent firmware version first?
There was a problem hiding this comment.
Advancing the SVN floor before we have a successful boot is a real CSA compliance issue. Addressed in c353501
|
|
||
| | Field | Type | Purpose | | ||
| |---|---|---| | ||
| | `chain` | `Vec<(ComponentId, ComponentAttrs), N>` | Ordered trust chain, supplied by the shell at construction time. Never mutated after build. | |
There was a problem hiding this comment.
What is a shell in this context? It's easy to confuse with bash or debug shell in the context of firmware. Is there a better word? platform layer vs board layer or so?
There was a problem hiding this comment.
I changed it to platform driver.
| }, | ||
|
|
||
| State::Recovering => match event { | ||
| Event::Restored(_) => { |
There was a problem hiding this comment.
Is one failed and one retry_count enough for the whole platform? Should it be per device?
There was a problem hiding this comment.
Yes, the retry budget is per component.
|
Even if we only ever update one device at a time, §5.3.3 background polling can report corruption of device B while device A is mid-update or mid-recovery. What does the machine do then? |
|
Maybe each device should have their own state machine? |
Excellent question - The scenario is covered by tests:
|
Your intuition is directionally correct — but what genuinely matters here is per-device state , and the design already carries it (ComponentStatus, retry_budget, isolation markers,etc). What stays singular is the ordering: we only fix one device at a time, and we only update one device at a time. The machine keeps a single "who am I fixing right now" slot (State::Recovering(id)), so it can't be pointed at two devices at once. That one-at-a-time behavior is on purpose, not a limitation we ran into. |
Correct. typeconstructor is another of my github accounts |
What happens if we get another update firmware request for device B while we are fixing the firmware if device A? |
TL;DRThe That drop is deliberate as a policy (recovery outranks update — "fix first, then update"), but the silence is the part worth a second look: the requester cannot tell "refused, busy recovering" apart from "lost." CSA AlignmentThe CSA mandates that failures be reported and models updates as answered requests, which is exactly the gap I am fixing with 57fedb0 This is a sketch of how we use the report effect. impl Platform for PlatformAst {
fn execute(&mut self, effect: Effect) -> Result<(), EffectError> {
match effect {
// ... device I/O effects (ReadFirmware, ReleaseReset, ...) ...
Effect::ReportIsolated(id) => self.mgmt.report_isolated(id),
Effect::ReportRecoveryFailed(id) => self.mgmt.report_recovery_failed(id),
// Pure protocol reply — no device I/O. Answer the pending PLDM
// FW-update request so the requester gets a definite "busy, retry".
Effect::ReportUpdateDeferred =>
self.pldm.respond_update(PldmCompletion::RetryLater),
// ...
}
}
}The CSA does not mandate update-vs-recovery arbitration. Recovery is described as autonomous and separate from update; the interaction of an in-flight recovery with an incoming update is left to the implementation. Our single-flight, recovery-priority choice is therefore a legitimate implementation-defined policy, neither required nor forbidden by the CSA. |
Add two new documents — a narrative walkthrough of the orchestrator state machine (orchestrator-sm-walkthru.md) and a reference transitions table (orchestrator-sm-transitions.md) — and expand the existing state machine document with superstate handler descriptions. Register both new pages in SUMMARY.md.
PreSupervision isn't linked into SupervisingPlatform, so corruption on an already-released component is dropped during an all-Passive chain walk. CSA defines no mechanism for detecting corruption of a live, executing component, so this isn't a confirmed requirement to fix. Reframes the Superstate doc comment, renames the expected-failing test (...triggers_recovery -> ...is_dropped) to match actual behavior, and softens the unconfirmed CSA-continuity claim in the walkthrough doc.
Replace hardcoded EFFECT_CAP=8 with a per-machine capacity E on Rot/ Orchestrator/Sink, floored at compile time to E >= N+2 (worst case: a full cascade of N AssertResets plus the PreSupervision entry's two effects land in one Sink). emit now panics on overflow instead of silently dropping security-critical effects.
Replace the single global retry_count with a per-component retries map so interleaved recoveries don't share a budget. Clear a component's streak on successful verification and on gating; clear all on Ready.
Extract the unit test module into src/tests.rs via mod tests; and add it to the library srcs. No behavior change; lib.rs drops to ~800 lines.
Introduce Chain<N> with a TryFrom that validates the chain of trust at the boundary (non-empty, unique ids, depends_on exists and is strictly earlier, length fits u8). Orchestrator/Rot::new take Chain<N> instead of a raw heapless::Vec, so the storage representation is no longer part of the public constructor. Add ChainError tests.
Drops the statig 0.4.1 dependency, which was used in exactly one file and never leaked into the public API. The hierarchical state machine is now a hand-written reducer: per-state handlers, one supervising handler, and entry actions as plain inherent methods on Rot, driven by a small engine (Orchestrator::step) that reproduces statig's per-event semantics — dispatch to the leaf, fall through to the supervisor on Super, and run the target's entry action only on Transition. No observable behavior change; all 42 unit tests pass with tests.rs and model.rs unmodified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CSA's degraded-mode clause requires two things when a component is taken out of service: isolate it, and report the failure through the platform management interface. The reducer did only the first — a BMC had no in-band signal that the platform was running degraded. Add `Effect::ReportIsolated` (emitted once per component at the moment it is gated, alongside its `AssertReset`) and `Effect::ReportRecoveryFailed` (emitted for the `Required` component whose exhausted recovery forces the halt, before the machine latches). Both emission points sit in the existing `is_gated` guards, so reporting is idempotent per component and the runtime-corruption and recovery-exhaustion paths report identically. Pairing a report with every `AssertReset` raises the worst-case single-`Sink` effect count from `N + 2` to `2N + 2`; the compile-time floor moves with it, so an under-sized board fails to build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cover each point a component leaves service: `Isolable` exhaustion, `Cascading` exhaustion (root and transitive dependent both reported), runtime corruption under a non-`Required` policy, and `Required` exhaustion — asserting the recovery-failure report precedes the latch. Two negative cases pin the boundaries: a component that recovers within its budget reports nothing, and an already-gated component is not re-reported. `ECAP` and the one hand-sized `Orchestrator` follow the new `2N + 2` effect-buffer floor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Boot-progress watchdog timeouts now enter recovery for the awaited component; stale/spurious timeouts are dropped (INV9). Adds three tests.
…Restored A Required corruption that preempts an in-flight update now emits DiscardStaged so the staged image is not orphaned. Recovering ignores a Restored whose id is not the current recovery target, preventing a displaced episode from mis-crediting the wrong component. Adds two tests.
Replace the gated set and retries map with a single statuses array parallel to chain, each entry a ComponentLifecycle (Nominal/Isolated) plus a retry count. Add status_index/gate_one helpers; rewrite is_gated, bump_retry, clear_retry, gate_by_policy, cascade_hold, and the Ready entry action to index into statuses. State enum and behavior unchanged.
…ress supervision-contract essay
Add BootConfirmed event + CommitSvnFloor effect; Ready emits the commit only on a proven-healthy boot, decoupling the anti-rollback floor advance from ActivateUpdate. Conforms to CSA resiliency: the floor advances after a higher-SVN image successfully boots and is deemed stable.
Track boot-progress per component instead of watching only Active devices. Passive components released from reset are now watched for liveness (new Booted event) under the same watchdog as Active ComponentReady; a released component that misses its window is recovered from any state. Timeout is handled uniformly in the supervisor and PreSupervision; the AwaitingReady-only Timeout arm is removed.
57fedb0 to
50989bd
Compare
|
@rusty1968 Should we add those test?: /// INV9: the iRoT gate must hold even when the eRoT walk finishes first.
/// C0's `ComponentReady` is still outstanding when the last verdict
/// arrives; completing the walk must not bypass the gate.
#[test]
fn chain_exhaustion_does_not_bypass_irot_gate() {
let (_effects, state) = drive(
chain(&[
(C0, ComponentAttrs::active_required()),
(C1, ComponentAttrs::passive_required()),
]),
&[
BOOT,
Event::VerificationPassed(C0), // C0 released; its iRoT still owes ComponentReady
Event::VerificationPassed(C1), // walk done — but C0 never reported
],
);
assert_eq!(state, State::AwaitingReady(Some(C0)));
}/// Trust invariant: a verdict only means something for the component under
/// verification. A replayed `VerificationPassed` for a component that has
/// since been isolated must not re-release it.
#[test]
fn replayed_verdict_does_not_release_isolated_component() {
let (effects, state) = drive(
chain(&[
(C0, ComponentAttrs::passive_isolable()),
(C1, ComponentAttrs::passive_required()),
]),
&[
BOOT,
Event::VerificationPassed(C0), // released; walk moves to C1
Event::CorruptionDetected(C0), // Isolable → AssertReset + Isolated
Event::VerificationPassed(C0), // replayed / spurious verdict
],
);
let releases_c0 = effects
.iter()
.filter(|&&e| e == Effect::ReleaseReset(C0))
.count();
assert_eq!(releases_c0, 1, "isolated component was re-released");
assert_ne!(state, State::Ready, "C1's verdict is still outstanding");
}/// A healthy Active component must not be dragged into recovery by a stale
/// boot-progress watchdog. After a re-walk, C0's `ComponentReady` arrives
/// while the machine is back in `PreSupervision`; the report must clear its
/// watchdog (as `Booted` does there), making the later `Timeout` stale.
#[test]
fn component_ready_during_rewalk_clears_watchdog() {
let (effects, state) = drive(
chain(&[
(C0, ComponentAttrs::active_required()),
(C1, ComponentAttrs::passive_required()),
]),
&[
BOOT,
Event::VerificationPassed(C0), // C0 released, watchdog armed
Event::VerificationFailed(C1), // C1 fails → Recovering(C1)
Event::Restored(C1), // restore ok → re-walk from top
Event::ComponentReady(C0), // C0's iRoT reports in — discarded
Event::Timeout(C0), // stale watchdog fires
],
);
assert!(
!effects.contains(&Effect::RestoreGoldenImage(C0)),
"healthy, ready C0 was sent to recovery by a stale watchdog"
);
assert_ne!(state, State::Recovering(C0));
} |
Add architecture diagram and narrative to the Platform Boundary section describing the Event/Effect boundary and mechanism-neutral effects.
Enforce component-id membership once in step() via Event::component_id(), so no handler acts on an id outside the configured chain. Also: rename RestoreGoldenImage -> RecoverComponent, add the commit-or-lock watchdog (pending_commit/CommitTimeout), and drop #[non_exhaustive].
- model: add §5 (recovery re-boot / at-rest re-verify) and §6 reset-must-hold note - machine: fill the INV8 catalogue gap with verify-before-release (property test) - model/overview: rename the 'platform shell' concept to 'platform' throughout
…rdicts - quiesce_all: assert reset on every live component before a recovery re-walk, so recovery is a genuine platform re-boot and no live component is re-verified while running (closes the TOCTOU on already-released siblings) - VerificationPassed: release only the component under verification (chain[cursor]); drop stale/out-of-turn verdicts, mirroring the INV9 guard on ComponentReady - tests: quiesce coverage (multi-sibling, at-rest re-verify), plus a property test (INV8) asserting verify-before-release across random event sequences
No description provided.