diff --git a/docs/src/design/orchestrator/orchestrator-platform.md b/docs/src/design/orchestrator/orchestrator-platform.md index 3d6edc464..ddc9e3b5c 100644 --- a/docs/src/design/orchestrator/orchestrator-platform.md +++ b/docs/src/design/orchestrator/orchestrator-platform.md @@ -23,7 +23,7 @@ inside the orchestrator process: - **Device capabilities** (`services/orchestrator/capabilities`) — the narrow contracts the state machine's effects are executed against, e.g. `BootControl` (hold a device in reset / release it). HAL bindings live in - `services/orchestrator/hal-adapters`. + `services/orchestrator/adapters/hal`. - **Board device table** (`services/orchestrator/config`, schema; values in `target//devices.rs`) — declares the managed devices: reset signal, boot checkpoints and windows, commit policy. Validated at compile time, so diff --git a/services/orchestrator/hal-adapters/BUILD.bazel b/services/orchestrator/adapters/hal/BUILD.bazel similarity index 100% rename from services/orchestrator/hal-adapters/BUILD.bazel rename to services/orchestrator/adapters/hal/BUILD.bazel diff --git a/services/orchestrator/hal-adapters/README.md b/services/orchestrator/adapters/hal/README.md similarity index 100% rename from services/orchestrator/hal-adapters/README.md rename to services/orchestrator/adapters/hal/README.md diff --git a/services/orchestrator/hal-adapters/src/gpio_boot_monitor.rs b/services/orchestrator/adapters/hal/src/gpio_boot_monitor.rs similarity index 100% rename from services/orchestrator/hal-adapters/src/gpio_boot_monitor.rs rename to services/orchestrator/adapters/hal/src/gpio_boot_monitor.rs diff --git a/services/orchestrator/hal-adapters/src/hal_boot_control.rs b/services/orchestrator/adapters/hal/src/hal_boot_control.rs similarity index 100% rename from services/orchestrator/hal-adapters/src/hal_boot_control.rs rename to services/orchestrator/adapters/hal/src/hal_boot_control.rs diff --git a/services/orchestrator/hal-adapters/src/lib.rs b/services/orchestrator/adapters/hal/src/lib.rs similarity index 100% rename from services/orchestrator/hal-adapters/src/lib.rs rename to services/orchestrator/adapters/hal/src/lib.rs diff --git a/services/orchestrator/adapters/pldm/BUILD.bazel b/services/orchestrator/adapters/pldm/BUILD.bazel new file mode 100644 index 000000000..ce7c90b94 --- /dev/null +++ b/services/orchestrator/adapters/pldm/BUILD.bazel @@ -0,0 +1,24 @@ +# Licensed under the Apache-2.0 license +# SPDX-License-Identifier: Apache-2.0 + +load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") + +rust_library( + name = "orchestrator_pldm_adapter", + srcs = [ + "src/lib.rs", + ], + crate_name = "openprot_orchestrator_pldm_adapter", + edition = "2024", + visibility = ["//visibility:public"], + deps = [ + "//services/orchestrator/sm:orchestrator_sm", + "//services/pldm:pldm_service", + ], +) + +# Host tests: build on the host platform, no kernel/QEMU. +rust_test( + name = "orchestrator_pldm_adapter_test", + crate = ":orchestrator_pldm_adapter", +) diff --git a/services/orchestrator/adapters/pldm/src/lib.rs b/services/orchestrator/adapters/pldm/src/lib.rs new file mode 100644 index 000000000..3b8304514 --- /dev/null +++ b/services/orchestrator/adapters/pldm/src/lib.rs @@ -0,0 +1,88 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! PLDM-backed adapter for the Boot Orchestrator's update-request input. +//! +//! [`UpdateRequestLatch`] binds the PLDM firmware-device service's +//! [`FdEventSink`] seam to the orchestrator's +//! [`Event::UpdateRequest`]: the PLDM run loop notifies the latch when the +//! Update Agent's `RequestUpdate` is accepted, and the orchestrator run loop +//! drains it with [`take`](UpdateRequestLatch::take). This crate depends on +//! both stacks by design — the PLDM service stays orchestrator-free and the +//! orchestrator stays transport-free, the same rule that keeps HAL adapters +//! out of `orchestrator-capabilities`. + +#![cfg_attr(not(test), no_std)] +#![forbid(unsafe_code)] +#![warn(missing_docs)] + +use openprot_orchestrator_sm::Event; +use openprot_pldm_service::firmware_device::{FdEvent, FdEventSink}; + +/// Latches an accepted PLDM `RequestUpdate` until the orchestrator run loop +/// drains it as [`Event::UpdateRequest`]. +/// +/// A `bool` latch, not a counter: the FD rejects a second `RequestUpdate` +/// while an update is in progress (`ALREADY_IN_UPDATE_MODE`), so at most one +/// accepted request can be outstanding per update cycle. Should a completed +/// or cancelled cycle admit a new `RequestUpdate` before the previous latch +/// is drained, the two coalesce into one [`Event::UpdateRequest`] — which is +/// what the state machine would do anyway (an update already being handled +/// defers further requests). +#[derive(Default)] +pub struct UpdateRequestLatch { + pending: bool, +} + +impl UpdateRequestLatch { + /// A latch with nothing pending. + pub const fn new() -> Self { + Self { pending: false } + } + + /// Drain the latch: [`Event::UpdateRequest`] if a `RequestUpdate` was + /// accepted since the last call, else `None`. + pub fn take(&mut self) -> Option { + self.pending.then(|| { + self.pending = false; + Event::UpdateRequest + }) + } +} + +/// Latches [`FdEvent::UpdateRequested`]; other FD lifecycle events have no +/// orchestrator mapping yet and are dropped here by design. +impl FdEventSink for UpdateRequestLatch { + fn notify(&mut self, event: FdEvent) { + if matches!(event, FdEvent::UpdateRequested) { + self.pending = true; + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn empty_latch_yields_nothing() { + assert_eq!(UpdateRequestLatch::new().take(), None); + } + + #[test] + fn accepted_request_yields_one_event() { + let mut latch = UpdateRequestLatch::new(); + latch.notify(FdEvent::UpdateRequested); + assert_eq!(latch.take(), Some(Event::UpdateRequest)); + assert_eq!(latch.take(), None, "a drained latch must not re-fire"); + } + + #[test] + fn undrained_notifications_coalesce() { + let mut latch = UpdateRequestLatch::new(); + latch.notify(FdEvent::UpdateRequested); + latch.notify(FdEvent::UpdateRequested); + assert_eq!(latch.take(), Some(Event::UpdateRequest)); + assert_eq!(latch.take(), None); + } +} diff --git a/services/orchestrator/capabilities/BUILD.bazel b/services/orchestrator/capabilities/BUILD.bazel index 29f69d1f0..4c413c822 100644 --- a/services/orchestrator/capabilities/BUILD.bazel +++ b/services/orchestrator/capabilities/BUILD.bazel @@ -9,6 +9,7 @@ rust_library( "src/boot_control.rs", "src/boot_watch.rs", "src/evidence.rs", + "src/incremental_verifier.rs", "src/lib.rs", "src/lockdown_latch.rs", "src/svn_floor.rs", diff --git a/services/orchestrator/capabilities/src/incremental_verifier.rs b/services/orchestrator/capabilities/src/incremental_verifier.rs new file mode 100644 index 000000000..842859368 --- /dev/null +++ b/services/orchestrator/capabilities/src/incremental_verifier.rs @@ -0,0 +1,287 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! The [`IncrementalVerifier`] update-verification capability contract. + +use crate::PayloadSource; + +/// Incremental firmware verification: judge a candidate image one bounded +/// step at a time, so a single-threaded runtime stays live while hashing +/// megabytes of payload. +/// +/// The caller (the platform driver's update pump) calls [`start`] once to +/// begin a session, then calls [`poll`] repeatedly on its own clock. Each +/// poll does at most one read from the payload and one hash update, then +/// returns. The chunk size is implementor-chosen, sized so each poll fits +/// the caller's per-poll time budget. The caller watches `hashed` +/// progress and abandons a session that stalls, on its own budget; the +/// verifier never judges liveness. +/// +/// A session produces exactly one terminal verdict: [`Authenticated`] or +/// [`Rejected`]. A fault (unreadable payload, crypto engine error) is +/// `Err`, not a verdict, because a check that could not run must not forge +/// a judgment. After an `Err`, the session is abandoned; only [`start`] +/// returns the verifier to a defined state. +/// +/// [`start`] discards any in-progress session, so the caller can abandon +/// and restart without a separate reset method. Unlike `Updatable`, which +/// starts implicitly from idle, the explicit `start` ensures that one +/// extra poll after a verdict is caught rather than silently re-hashing +/// from zero. The boot-time synchronous `Verifier` (in the driver crate) +/// is unaffected: it stays one-shot for the chain walk, where the image is +/// small and local. +/// +/// [`start`]: IncrementalVerifier::start +/// [`poll`]: IncrementalVerifier::poll +/// [`Authenticated`]: VerifyStep::Authenticated +/// [`Rejected`]: VerifyStep::Rejected +pub trait IncrementalVerifier { + /// The error reported when the check itself cannot run: crypto fault, + /// unreadable payload, or misuse (poll outside a session). A bad image + /// is [`Rejected`](VerifyStep::Rejected), not an error. + type Error: core::error::Error; + + /// Discards any in-progress session and prepares to verify from the + /// start of the image. The next [`poll`](Self::poll) begins hashing + /// at offset zero. + fn start(&mut self); + + /// Processes one bounded step: at most one read from `payload` and one + /// hash update, then returns. Never waits on device progress, never + /// sleeps. Returns the session's current state: still processing + /// (with byte-level progress), or a terminal verdict. + /// + /// Calling `poll` after a terminal verdict (or before [`start`](Self::start)) + /// is an error. + fn poll(&mut self, payload: &dyn PayloadSource) -> Result; +} + +/// One step of an incremental verification session. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum VerifyStep { + /// One chunk hashed. `hashed` bytes processed so far out of `total`. + /// The caller watches progress and abandons a session whose `hashed` + /// stops advancing, on its own stall budget. + Processing { hashed: u64, total: u64 }, + /// The complete image authenticated (signature and policy checks passed). + Authenticated, + /// The complete image was checked and found invalid. + Rejected, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{PayloadReadError, PayloadSource}; + + // A PayloadSource over a plain byte slice. + struct SlicePayload(&'static [u8]); + + impl PayloadSource for SlicePayload { + fn len(&self) -> u64 { + self.0.len() as u64 + } + + fn read_at(&self, offset: u64, buf: &mut [u8]) -> Result<(), PayloadReadError> { + let start = usize::try_from(offset).map_err(|_| PayloadReadError::OutOfRange)?; + let end = start + .checked_add(buf.len()) + .ok_or(PayloadReadError::OutOfRange)?; + buf.copy_from_slice(self.0.get(start..end).ok_or(PayloadReadError::OutOfRange)?); + Ok(()) + } + } + + // A verifier that hashes 4 bytes per poll and accepts any image whose + // first byte is nonzero. + struct ChunkedVerifier { + offset: u64, + total: u64, + active: bool, + } + + #[derive(Debug, PartialEq)] + struct VerifierFault; + + impl core::fmt::Display for VerifierFault { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str("verifier fault") + } + } + + impl core::error::Error for VerifierFault {} + + impl ChunkedVerifier { + fn new() -> Self { + Self { + offset: 0, + total: 0, + active: false, + } + } + } + + impl IncrementalVerifier for ChunkedVerifier { + type Error = VerifierFault; + + fn start(&mut self) { + self.offset = 0; + self.total = 0; + self.active = true; + } + + fn poll(&mut self, payload: &dyn PayloadSource) -> Result { + if !self.active { + return Err(VerifierFault); + } + if self.total == 0 { + self.total = payload.len(); + } + if self.offset >= self.total { + self.active = false; + let mut first = [0u8; 1]; + payload.read_at(0, &mut first).map_err(|_| VerifierFault)?; + return Ok(if first[0] != 0 { + VerifyStep::Authenticated + } else { + VerifyStep::Rejected + }); + } + let chunk = core::cmp::min(4, (self.total - self.offset) as usize); + let mut buf = [0u8; 4]; + payload + .read_at(self.offset, &mut buf[..chunk]) + .map_err(|_| VerifierFault)?; + self.offset += chunk as u64; + Ok(VerifyStep::Processing { + hashed: self.offset, + total: self.total, + }) + } + } + + #[test] + fn multi_poll_until_authenticated() { + let payload = SlicePayload(&[0xAA; 10]); + let mut v = ChunkedVerifier::new(); + v.start(); + + // 4 bytes, 4 bytes, 2 bytes = 3 Processing steps, then verdict. + assert_eq!( + v.poll(&payload), + Ok(VerifyStep::Processing { + hashed: 4, + total: 10 + }) + ); + assert_eq!( + v.poll(&payload), + Ok(VerifyStep::Processing { + hashed: 8, + total: 10 + }) + ); + assert_eq!( + v.poll(&payload), + Ok(VerifyStep::Processing { + hashed: 10, + total: 10 + }) + ); + assert_eq!(v.poll(&payload), Ok(VerifyStep::Authenticated)); + } + + #[test] + fn rejected_image() { + let payload = SlicePayload(&[0x00; 8]); + let mut v = ChunkedVerifier::new(); + v.start(); + + // Drain processing steps. + assert_eq!( + v.poll(&payload), + Ok(VerifyStep::Processing { + hashed: 4, + total: 8 + }) + ); + assert_eq!( + v.poll(&payload), + Ok(VerifyStep::Processing { + hashed: 8, + total: 8 + }) + ); + assert_eq!(v.poll(&payload), Ok(VerifyStep::Rejected)); + } + + #[test] + fn start_discards_in_progress_session() { + let payload = SlicePayload(&[0xFF; 12]); + let mut v = ChunkedVerifier::new(); + v.start(); + + // Partial progress. + assert_eq!( + v.poll(&payload), + Ok(VerifyStep::Processing { + hashed: 4, + total: 12 + }) + ); + + // Restart: offset resets, next poll begins from zero. + v.start(); + assert_eq!( + v.poll(&payload), + Ok(VerifyStep::Processing { + hashed: 4, + total: 12 + }) + ); + } + + #[test] + fn poll_before_start_is_an_error() { + let payload = SlicePayload(&[0xFF; 4]); + let mut v = ChunkedVerifier::new(); + assert!(v.poll(&payload).is_err()); + } + + // A PayloadSource whose read_at always fails. + struct Lying; + + impl PayloadSource for Lying { + fn len(&self) -> u64 { + 64 + } + + fn read_at(&self, _offset: u64, _buf: &mut [u8]) -> Result<(), PayloadReadError> { + Err(PayloadReadError::Storage) + } + } + + #[test] + fn read_fault_is_err_not_verdict() { + let mut v = ChunkedVerifier::new(); + v.start(); + + // The verifier tries to read, fails, and returns Err (not Rejected). + let result = v.poll(&Lying); + assert!(result.is_err(), "payload fault must be Err, not a verdict"); + } + + #[test] + fn poll_after_verdict_is_an_error() { + let payload = SlicePayload(&[0xFF; 4]); + let mut v = ChunkedVerifier::new(); + v.start(); + + // Drain to verdict. + let _ = v.poll(&payload); + let _ = v.poll(&payload); + + // Post-verdict poll is a fault. + assert!(v.poll(&payload).is_err()); + } +} diff --git a/services/orchestrator/capabilities/src/lib.rs b/services/orchestrator/capabilities/src/lib.rs index a7a479f1e..f738968d9 100644 --- a/services/orchestrator/capabilities/src/lib.rs +++ b/services/orchestrator/capabilities/src/lib.rs @@ -22,6 +22,10 @@ //! the chunked read seam staging pulls from — transports and slot //! bookkeeping stay behind the adapter. //! +//! `IncrementalVerifier` is the polled verification seam: judge a candidate +//! image one bounded step at a time, so the update pump never blocks on +//! hashing megabytes of payload. Same poll-not-block contract as `Updatable`. +//! //! `BootWatch` is the seam the orchestrator polls: one device's boot walk, //! erased of every device-specific type, answering with a `WalkVerdict`. //! @@ -41,6 +45,7 @@ mod boot_control; mod boot_watch; mod evidence; +mod incremental_verifier; mod lockdown_latch; mod svn_floor; mod updatable; @@ -48,6 +53,7 @@ mod updatable; pub use boot_control::BootControl; pub use boot_watch::{BootWatch, FailureCause, WalkVerdict}; pub use evidence::{BootStatus, EvidenceReader}; +pub use incremental_verifier::{IncrementalVerifier, VerifyStep}; pub use lockdown_latch::LockdownLatch; pub use svn_floor::{Svn, SvnFloor}; pub use updatable::{PayloadReadError, PayloadSource, StageProgress, Updatable, UpdateError}; diff --git a/services/orchestrator/capabilities/src/updatable.rs b/services/orchestrator/capabilities/src/updatable.rs index 7d89f645f..b68693e6c 100644 --- a/services/orchestrator/capabilities/src/updatable.rs +++ b/services/orchestrator/capabilities/src/updatable.rs @@ -24,12 +24,13 @@ /// What this trait deliberately does not claim: /// /// - **Verification** runs on the candidate before staging as -/// orchestrator policy; post-write read-back is the optional -/// `ReadBack` capability. +/// orchestrator policy. Post-write readback is no capability either: +/// it is the implementor's staging discipline (see `Ready` below). /// - **Commit.** Activation is always tentative: it proposes the staged /// image as the preferred boot target, never commits it. The commit -/// gate is the optional `TrialBoot` capability, which resolves what -/// `activate` proposed; there is no second slot-selection owner. +/// gate is the orchestrator's confirmed-boot flow (`BootConfirmed` +/// gating `SvnFloor::advance`, or the device committing internally); +/// there is no second slot-selection owner. /// - **Booting.** Resetting the device into the candidate is /// [`BootControl`](crate::BootControl). When activation takes effect /// (next reset, or a device-internal restart on self-activating @@ -52,9 +53,16 @@ /// [`abandon`](Self::abandon) instead of waiting out a blocked call. /// Liveness policy stays with the caller: it watches `written` and /// abandons a transfer that stalls too long, on its own clock. -/// - **`Ready` means ready.** The device holds the complete payload and -/// `activate` may be called. `activate` in any other staging state is -/// an error. +/// - **`Ready` means ready.** The device holds the complete payload, +/// verified to its archetype's discipline: a direct-flash adapter +/// reads written data back and re-verifies before reporting `Ready`, +/// a PLDM device runs its own verify step after the transfer: +/// `FdOps::verify` in pldm-lib, reported to the update agent as +/// `VerifyComplete`. A mismatch fails the step as +/// [`ReadbackMismatch`](UpdateError::ReadbackMismatch), kept apart +/// from [`Device`](UpdateError::Device) so a caller can retire a slot +/// that keeps mismatching. `activate` may be called; in any other +/// staging state it is an error. pub trait Updatable { /// Advances staging by one step, pulling from `payload`. /// @@ -120,7 +128,8 @@ pub enum StageProgress { /// Total payload bytes. total: u64, }, - /// The device holds the complete payload; `activate` may be called. + /// The device holds the complete, verified payload; `activate` may + /// be called. Ready, } @@ -139,6 +148,12 @@ pub enum UpdateError { Payload(PayloadReadError), /// The device failed or refused the step; staging anew may succeed. Device, + /// Written data did not read back as written. Distinct from + /// [`Device`](Self::Device) because the cause is narrower — the write + /// path reported success and the storage still disagrees — so a + /// caller can count it separately and retire a slot that keeps + /// mismatching instead of retrying forever. + ReadbackMismatch, /// `poll_stage` with an empty payload, a caller bug: an empty image /// must never stage to [`Ready`](StageProgress::Ready). EmptyPayload, @@ -152,6 +167,7 @@ impl core::fmt::Display for UpdateError { match self { UpdateError::Payload(_) => f.write_str("staging pull failed"), UpdateError::Device => f.write_str("device failed the update step"), + UpdateError::ReadbackMismatch => f.write_str("readback mismatch"), UpdateError::EmptyPayload => f.write_str("empty payload"), UpdateError::NothingStaged => f.write_str("nothing staged"), } @@ -162,7 +178,10 @@ impl core::error::Error for UpdateError { fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { match self { UpdateError::Payload(fault) => Some(fault), - UpdateError::Device | UpdateError::EmptyPayload | UpdateError::NothingStaged => None, + UpdateError::Device + | UpdateError::ReadbackMismatch + | UpdateError::EmptyPayload + | UpdateError::NothingStaged => None, } } } @@ -482,12 +501,17 @@ mod tests { // shape: one fixed-size chunk request per step, at offsets the device // picks, including a retransmit. Out-of-range reads are a fault, so // the adapter clamps the device's last request to the payload end. + // A finished transfer does not mean `Ready`: the device runs its own + // verify first, one further step, the way a PLDM firmware device + // enters VERIFY and reports the outcome in `VerifyComplete`. const PLDM_CHUNK: usize = 3; struct MockPldmDevice { staged: Vec, offset: usize, retransmitted: bool, + // The whole image is transferred and its verify step is due. + verifying: bool, ready: bool, active: bool, } @@ -498,6 +522,7 @@ mod tests { staged: Vec::new(), offset: 0, retransmitted: false, + verifying: false, ready: false, active: false, } @@ -519,6 +544,13 @@ mod tests { if self.staged.len() != total { self.staged = vec![0; total]; } + // The device's own verify, the step that earns `Ready`. It + // transfers nothing, so no chunk is requested. + if self.verifying { + self.verifying = false; + self.ready = true; + return Ok(StageProgress::Ready); + } // The device re-requests the previous chunk once mid-transfer. let request = if self.offset == 2 * PLDM_CHUNK && !self.retransmitted { self.retransmitted = true; @@ -534,20 +566,19 @@ mod tests { self.offset += len; } if self.offset == total { - self.ready = true; - Ok(StageProgress::Ready) - } else { - Ok(StageProgress::Transferring { - written: self.offset as u64, - total: total as u64, - }) + self.verifying = true; } + Ok(StageProgress::Transferring { + written: self.offset as u64, + total: total as u64, + }) } fn abandon(&mut self) { self.staged = Vec::new(); self.offset = 0; self.retransmitted = false; + self.verifying = false; self.ready = false; } @@ -590,6 +621,16 @@ mod tests { total, }) ); + // The short last chunk completes the transfer. Still not `Ready`: + // the device's verify has not run. + assert_eq!( + dev.poll_stage(&payload), + Ok(StageProgress::Transferring { + written: total, + total + }) + ); + // The verify step earns `Ready`. assert_eq!(dev.poll_stage(&payload), Ok(StageProgress::Ready)); dev.activate().expect("activate failed"); @@ -597,18 +638,30 @@ mod tests { } // A direct-flash adapter, the erase-before-write shape: one step is - // one flash operation, either erasing the next sector or programming - // the next page. Erase steps pull nothing and hold `written` still. + // one flash operation, erasing the next sector, programming the next + // page, or reading a written page back. Erase and write steps hold + // `written` still; only a page that passed readback advances it, so + // `written` counts verified bytes. This is the readback discipline the + // `Ready` contract prescribes for direct-flash devices. const FLASH_PAGE: usize = 2; // bytes programmed per write step const FLASH_SECTOR: usize = 4; // bytes erased per erase step const FLASH_SECTORS: usize = 2; // sectors in the slot + // What a page written last step must read back as. + struct ExpectedReadback { + start: usize, + expected: Vec, + } + struct MockFlashDevice { slot: [u8; FLASH_SECTORS * FLASH_SECTOR], erased: [bool; FLASH_SECTORS], + pending: Option, count: usize, ready: bool, active: bool, + // Test knob: corrupt the next written page so its readback fails. + corrupt_next_write: bool, } impl MockFlashDevice { @@ -616,9 +669,11 @@ mod tests { MockFlashDevice { slot: [0; FLASH_SECTORS * FLASH_SECTOR], erased: [false; FLASH_SECTORS], + pending: None, count: 0, ready: false, active: false, + corrupt_next_write: false, } } } @@ -640,6 +695,24 @@ mod tests { if total > self.slot.len() { return Err(UpdateError::Device); } + // Readback: re-verify the page written last step. Only now + // does the page count as staged. + if let Some(ExpectedReadback { start, expected }) = self.pending.take() { + if self.slot[start..start + expected.len()] != expected[..] { + self.abandon(); + return Err(UpdateError::ReadbackMismatch); + } + self.count = start + expected.len(); + return if self.count == total { + self.ready = true; + Ok(StageProgress::Ready) + } else { + Ok(StageProgress::Transferring { + written: self.count as u64, + total: total as u64, + }) + }; + } let sector = self.count / FLASH_SECTOR; if !self.erased[sector] { self.slot[sector * FLASH_SECTOR..(sector + 1) * FLASH_SECTOR].fill(0xff); @@ -650,23 +723,28 @@ mod tests { }); } let end = (self.count + FLASH_PAGE).min(total); + let mut page = vec![0; end - self.count]; payload - .read_at(self.count as u64, &mut self.slot[self.count..end]) + .read_at(self.count as u64, &mut page) .map_err(UpdateError::Payload)?; - self.count = end; - if self.count == total { - self.ready = true; - Ok(StageProgress::Ready) - } else { - Ok(StageProgress::Transferring { - written: self.count as u64, - total: total as u64, - }) + self.slot[self.count..end].copy_from_slice(&page); + if self.corrupt_next_write { + self.corrupt_next_write = false; + self.slot[self.count] ^= 0xff; } + self.pending = Some(ExpectedReadback { + start: self.count, + expected: page, + }); + Ok(StageProgress::Transferring { + written: self.count as u64, + total: total as u64, + }) } fn abandon(&mut self) { self.erased = [false; FLASH_SECTORS]; + self.pending = None; self.count = 0; self.ready = false; } @@ -690,40 +768,25 @@ mod tests { let sector = FLASH_SECTOR as u64; assert_eq!(total as usize, FLASH_SECTORS * FLASH_SECTOR); - // Erase sector 0: a step with no pull, `written` holds still. - assert_eq!( - dev.poll_stage(&payload), - Ok(StageProgress::Transferring { written: 0, total }) - ); - assert_eq!( - dev.poll_stage(&payload), - Ok(StageProgress::Transferring { - written: page, - total - }) - ); - assert_eq!( - dev.poll_stage(&payload), - Ok(StageProgress::Transferring { - written: sector, - total - }) - ); - // Erase sector 1. - assert_eq!( - dev.poll_stage(&payload), - Ok(StageProgress::Transferring { - written: sector, - total - }) - ); - assert_eq!( - dev.poll_stage(&payload), - Ok(StageProgress::Transferring { - written: sector + page, - total, - }) - ); + // `written` after each step; it advances only on a passed readback. + let expected = [ + 0, // erase sector 0 + 0, // program page 0 + page, // page 0 readback passed + page, // program page 1 + sector, // page 1 readback passed + sector, // erase sector 1 + sector, // program page 2 + sector + page, // page 2 readback passed + sector + page, // program page 3 + ]; + for written in expected { + assert_eq!( + dev.poll_stage(&payload), + Ok(StageProgress::Transferring { written, total }) + ); + } + // The final readback completes staging. assert_eq!(dev.poll_stage(&payload), Ok(StageProgress::Ready)); dev.activate().expect("activate failed"); @@ -751,6 +814,21 @@ mod tests { assert_eq!(pldm.staged, b"chunked"); } + #[test] + fn a_readback_mismatch_is_a_staging_error() { + let mut dev = MockFlashDevice::idle(); + dev.corrupt_next_write = true; + let payload = SliceSource(b"8 bytes!"); + + let err = stage_all(&mut dev, &payload).expect_err("expected the readback mismatch"); + assert_eq!(err, UpdateError::ReadbackMismatch); + + // Staging anew after the fault is allowed and verifies clean. + stage_all(&mut dev, &payload).expect("re-staging failed"); + assert_eq!(&dev.slot, b"8 bytes!"); + dev.activate().expect("activate after re-staging failed"); + } + #[test] fn a_payload_beyond_the_slot_is_rejected_before_any_flash_op() { let mut dev = MockFlashDevice::idle(); diff --git a/services/orchestrator/driver/src/board.rs b/services/orchestrator/driver/src/board.rs index 78ca53d33..f471a289a 100644 --- a/services/orchestrator/driver/src/board.rs +++ b/services/orchestrator/driver/src/board.rs @@ -7,6 +7,7 @@ use openprot_orchestrator_sm::{ComponentId, ComponentKind}; pub use orchestrator_capabilities::{BootControl, BootWatch}; +use orchestrator_capabilities::{Svn, SvnFloor}; /// Access to one component's active firmware image, however it is reached — /// interposed flash, a PLDM/MCTP transfer, a RAM copy in tests. @@ -46,8 +47,8 @@ impl ImageSource for &mut S { } } -/// Judges a component's firmware image; board wiring decides what -/// "authentic" means. +/// Judges a component's firmware image; board wiring decides what counts +/// as authenticated. pub trait Verifier { /// The error type reported by this verifier. type Error: core::error::Error; @@ -83,12 +84,72 @@ impl Verifier for &mut V { /// A [`Verifier`]'s judgment of one image. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Verdict { - /// Reported as `Event::VerificationPassed`. - Authenticated, + /// Reported as `Event::VerificationPassed`. Carries the image's + /// manifest SVN: verification is the only authenticated reading of the + /// manifest, so this is the one value the anti-rollback commit + /// ([`Effect::CommitSvnFloor`](openprot_orchestrator_sm::Effect)) may + /// trust. + Authenticated { + /// The verified image's security version number. + svn: Svn, + }, /// Reported as `Event::VerificationFailed`. Rejected, } +/// One fact about the platform running degraded, carried outward to +/// management software. `#[non_exhaustive]`: a sink routes what it +/// recognises and ignores the rest, so a new report is not a trait break. +/// +/// Payloads stay `Copy` and lifetime-free, like the effects these mirror. A +/// report names a component only where the effect it mirrors does. +#[non_exhaustive] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Report { + /// Held in reset and out of the trust chain, so the platform runs + /// degraded. Reported once per component, as it is gated. + Isolated(ComponentId), + /// Recovery attempts exhausted and the platform halts. Reported before + /// the lockdown latch, while there is still a platform to report from. + RecoveryFailed(ComponentId), + /// An update request declined because the platform was busy. Nothing + /// staged, and the requester may ask again. Platform-wide, not + /// per-component: the machine supervises one update at a time and + /// `Event::UpdateRequest` names no component. + UpdateDeferred, + /// An update in flight superseded by recovery. Its staged image is + /// discarded and no verdict for that request follows. Platform-wide for + /// the same reason as [`Report::UpdateDeferred`]. + UpdateAborted, +} + +/// Where the driver hands its [`Report`]s. What a report becomes, a log +/// entry, a management transport message, a fault line, is board wiring. +/// +/// Infallible by design: a report names something that already happened, so +/// an undeliverable one costs information, not containment. An error channel +/// would put reports on the fail-closed path, letting the act of reporting a +/// contained failure escalate it. +pub trait ReportSink { + /// Receives one report. A sink that cannot deliver immediately queues on + /// its own side rather than stalling the effect batch. + fn report(&mut self, report: Report); +} + +/// Drops every report, for a board with no management side to tell. Losing +/// reports costs visibility only, so this is a wiring choice, not a stub. +impl ReportSink for () { + #[inline(always)] + fn report(&mut self, _report: Report) {} +} + +impl ReportSink for &mut S { + #[inline(always)] + fn report(&mut self, report: Report) { + (**self).report(report) + } +} + /// The set of platform capabilities one board composes into the /// `PlatformDriver`, named by a marker type. A new seam adds an associated /// type here and a field on [`Board`] — never another parameter. @@ -101,9 +162,25 @@ pub trait BoardCapabilities { type BootControl: BootControl; /// Boot-checkpoint supervision for the managed components. type BootWatch: BootWatch; + /// The anti-rollback floor of the managed components. The SVN number + /// survives reset and power loss, otherwise a power cycle would + /// re-admit images below the floor. + type SvnFloor: SvnFloor; // Later seams: Recovery, Staging. } +/// Who keeps one component's anti-rollback floor. Spelled as its own type +/// so a board must state the choice; an eRoT floor and a device tracking +/// its own SVN are different wirings, not a present or absent value. +pub enum SvnFloorBinding { + /// The eRoT holds the floor and `CommitSvnFloor` advances it. + Erot(F), + /// The component tracks its own SVN (iRoT, or a PLDM device + /// committing internally). The eRoT keeps no second floor and its + /// `CommitSvnFloor` is a no-op. + SelfManaged, +} + /// Everything the board supplies, built once at bring-up and handed to /// `PlatformDriver::new`. Fields are public: executors may need two parts at once /// (disjoint borrows). @@ -115,6 +192,7 @@ pub trait BoardCapabilities { /// type Verifier = ManifestVerifier; // signature + SVN via the crypto engine /// type BootControl = ExtrstGpio; // per-component reset line /// type BootWatch = CheckpointWalk; // GPIO checkpoint walk over the boot window +/// type SvnFloor = OtpSvnFloor; // fuse-backed anti-rollback floor /// } /// let board = Board:: { /// images: [bmc_image, cpld_image], @@ -122,6 +200,7 @@ pub trait BoardCapabilities { /// boot_controls: [bmc_reset, cpld_reset], /// boot_watches: [bmc_walk, cpld_walk], /// component_kinds: [ComponentKind::Active, ComponentKind::Passive], +/// svn_floors: [SvnFloorBinding::Erot(bmc_floor), SvnFloorBinding::SelfManaged], /// }; /// ``` pub struct Board { @@ -140,5 +219,8 @@ pub struct Board { /// `ComponentReady` for `Active`, `Booted` for `Passive`. Comes from /// the same board table as the SM's chain, so both sides agree. pub component_kinds: [ComponentKind; N], + /// `svn_floors[i]` says who keeps `ComponentId(i)`'s anti-rollback + /// floor, same indexing as `images`. + pub svn_floors: [SvnFloorBinding; N], // Later seams add fields, e.g. recovery: [B::Recovery; N]. } diff --git a/services/orchestrator/driver/src/driver.rs b/services/orchestrator/driver/src/driver.rs index 5f3d9cc23..1a14d41f2 100644 --- a/services/orchestrator/driver/src/driver.rs +++ b/services/orchestrator/driver/src/driver.rs @@ -6,8 +6,8 @@ use openprot_orchestrator_sm::{ComponentId, ComponentKind, Effect, EffectError, Event, Platform}; -use crate::board::{Board, BoardCapabilities, ImageSource, Verdict, Verifier}; -use orchestrator_capabilities::{BootControl, BootWatch, WalkVerdict}; +use crate::board::{Board, BoardCapabilities, ImageSource, SvnFloorBinding, Verdict, Verifier}; +use orchestrator_capabilities::{BootControl, BootWatch, Svn, SvnFloor, WalkVerdict}; /// Why the driver could not carry out an effect. #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -23,6 +23,11 @@ pub enum DriverError { VerifierFault, /// The component's boot control could not actuate the reset line. BootControlFault, + /// A floor commit was asked for a component with no verified image — + /// the SVN to advance to is unknown; fail closed. + NoVerifiedImage, + /// The component's SVN floor could not be advanced. + SvnFloorFault, } impl core::fmt::Display for DriverError { @@ -33,6 +38,8 @@ impl core::fmt::Display for DriverError { DriverError::NotStaged => "no image staged for this component", DriverError::VerifierFault => "verifier could not perform the check", DriverError::BootControlFault => "boot control could not actuate the reset", + DriverError::NoVerifiedImage => "no verified image to commit the floor to", + DriverError::SvnFloorFault => "svn floor could not be advanced", }) } } @@ -50,6 +57,10 @@ pub struct PlatformDriver { /// terminal verdict. Only watched walks are polled, so a finished or /// quiesced walk emits no stale event. watching: [bool; N], + /// `verified_svn[i]` is the manifest SVN of `ComponentId(i)`'s last + /// authenticated image — the only value a floor commit may trust. + /// `None` until a verification passes; cleared again on rejection. + verified_svn: [Option; N], } impl PlatformDriver { @@ -60,9 +71,19 @@ impl PlatformDriver { board, staged: None, watching: [false; N], + verified_svn: [None; N], } } + /// The board wiring, read-only, for the tests: they observe a + /// capability after it moved into the driver, instead of every mock + /// smuggling out a shared handle. Real consumers get targeted queries + /// when they exist — not this. + #[cfg(test)] + pub(crate) fn board(&self) -> &Board { + &self.board + } + /// `id`'s image source. Takes the array rather than `&mut self` so the /// caller can borrow `board.verifier` alongside the returned image. fn source(images: &mut [B::Image; N], id: ComponentId) -> Result<&mut B::Image, DriverError> { @@ -95,12 +116,37 @@ impl PlatformDriver { .verifier .verify(id, source) .map_err(|_| DriverError::VerifierFault)?; + let idx = id.get() as usize; Ok(match verdict { - Verdict::Authenticated => Event::VerificationPassed(id), - Verdict::Rejected => Event::VerificationFailed(id), + Verdict::Authenticated { svn } => { + self.verified_svn[idx] = Some(svn); + Event::VerificationPassed(id) + } + Verdict::Rejected => { + self.verified_svn[idx] = None; + Event::VerificationFailed(id) + } }) } + /// Advance `id`'s anti-rollback floor to its verified image's SVN. + /// A self-managed component keeps its own floor; the commit is a + /// no-op. A target at or below the current floor is the capability's + /// documented no-op, so a replayed commit is harmless. + pub fn commit_svn_floor(&mut self, id: ComponentId) -> Result<(), DriverError> { + let idx = id.get() as usize; + let SvnFloorBinding::Erot(floor) = self + .board + .svn_floors + .get_mut(idx) + .ok_or(DriverError::UnknownComponent)? + else { + return Ok(()); + }; + let svn = self.verified_svn[idx].ok_or(DriverError::NoVerifiedImage)?; + floor.advance(svn).map_err(|_| DriverError::SvnFloorFault) + } + /// `id`'s reset actuator. fn boot_control(&mut self, id: ComponentId) -> Result<&mut B::BootControl, DriverError> { self.board @@ -212,21 +258,20 @@ impl Platform for PlatformDriver { Effect::VerifyFirmware(id) => self.verify_firmware(id).map(Some), Effect::ReleaseReset(id) => self.release_reset(id).map(|_| None), Effect::AssertReset(id) => self.assert_reset(id).map(|_| None), + Effect::CommitSvnFloor(id) => self.commit_svn_floor(id).map(|_| None), // No board capability is composed for these seams yet, so they // fail closed here instead of behind stub methods. Each group // gains an executor when its capability joins // [`BoardCapabilities`], as BootControl did above: recovery // sourcing for RecoverComponent; update staging, authentication - // and trial activation for the update quartet; anti-rollback - // commit for CommitSvnFloor; evidence signing for - // SignAttestation; the management reporting path for the Report - // effects; the terminal latch for LatchLockdown. + // and trial activation for the update quartet; evidence signing + // for SignAttestation; the management reporting path for the + // Report effects; the terminal latch for LatchLockdown. Effect::RecoverComponent { .. } | Effect::AuthenticateUpdate | Effect::StageUpdate | Effect::ActivateUpdate | Effect::DiscardStaged - | Effect::CommitSvnFloor(_) | Effect::SignAttestation | Effect::ReportIsolated(_) | Effect::ReportRecoveryFailed(_) diff --git a/services/orchestrator/driver/src/lib.rs b/services/orchestrator/driver/src/lib.rs index d251b463d..b2644addc 100644 --- a/services/orchestrator/driver/src/lib.rs +++ b/services/orchestrator/driver/src/lib.rs @@ -16,9 +16,10 @@ //! //! Everything device-specific arrives through the seams in [`board`]: //! image access ([`ImageSource`]), image judgment ([`Verifier`]), reset -//! actuation ([`orchestrator_capabilities::BootControl`]) and boot -//! supervision ([`orchestrator_capabilities::BootWatch`]), bundled in one -//! [`Board`] built by the board's composition crate. +//! actuation ([`orchestrator_capabilities::BootControl`]), boot +//! supervision ([`orchestrator_capabilities::BootWatch`]) and the +//! anti-rollback floor ([`orchestrator_capabilities::SvnFloor`]), bundled +//! in one [`Board`] built by the board's composition crate. //! //! Boot-walk verdicts are the one asynchronous read: the run loop calls //! [`PlatformDriver::poll_boot_walks`] and dispatches the returned events @@ -34,5 +35,7 @@ mod driver; #[cfg(test)] mod tests; -pub use board::{Board, BoardCapabilities, ImageSource, Verdict, Verifier}; +pub use board::{ + Board, BoardCapabilities, ImageSource, Report, ReportSink, SvnFloorBinding, Verdict, Verifier, +}; pub use driver::{BootWalkPoll, DriverError, PlatformDriver}; diff --git a/services/orchestrator/driver/src/tests.rs b/services/orchestrator/driver/src/tests.rs index f98851af7..c18b82f41 100644 --- a/services/orchestrator/driver/src/tests.rs +++ b/services/orchestrator/driver/src/tests.rs @@ -5,9 +5,10 @@ extern crate std; use crate::*; use openprot_orchestrator_sm::{ - ComponentAttrs, ComponentId, ComponentKind, Event, Orchestrator, PowerOnResult, State, + ComponentAttrs, ComponentId, ComponentKind, Effect, Event, Orchestrator, Platform, + PowerOnResult, State, }; -use orchestrator_capabilities::{BootWatch, FailureCause, WalkVerdict}; +use orchestrator_capabilities::{BootWatch, FailureCause, Svn, SvnFloor, WalkVerdict}; const C0: ComponentId = ComponentId::new(0); @@ -36,9 +37,14 @@ impl core::fmt::Display for MemFault { impl core::error::Error for MemFault {} -/// RAM-backed image source — the seam satisfied without a HAL. +/// RAM-backed image source — the seam satisfied without a HAL. A re-flash +/// can be queued: it lands on the first re-open (a re-stage), the way real +/// flash changes underneath a source between stagings — never on the +/// initial staging. struct MemImage { data: std::vec::Vec, + reflash: Option>, + opened: bool, fail_open: bool, fail_read: bool, } @@ -47,10 +53,18 @@ impl MemImage { fn holding(data: std::vec::Vec) -> Self { Self { data, + reflash: None, + opened: false, fail_open: false, fail_read: false, } } + + /// Queue a re-flash: the first re-open stages `data` instead. + fn reflash_on_reopen(mut self, data: std::vec::Vec) -> Self { + self.reflash = Some(data); + self + } } impl ImageSource for MemImage { @@ -60,6 +74,10 @@ impl ImageSource for MemImage { if self.fail_open { return Err(MemFault); } + if let Some(data) = self.reflash.take_if(|_| self.opened) { + self.data = data; + } + self.opened = true; Ok(()) } @@ -91,6 +109,10 @@ impl core::error::Error for VerifierError {} /// chunks. struct XorVerifier { fault: bool, + /// The manifest SVN this verifier reports for an authenticated image. + /// A real verifier reads it from the signed manifest of the image it + /// just checked; the test image format has no manifest, so tests pin it. + svn: u32, } impl Verifier for XorVerifier { @@ -122,7 +144,7 @@ impl Verifier for XorVerifier { } let ok = len > IMAGE_MAGIC.len() && magic == IMAGE_MAGIC && xor == 0; Ok(if ok { - Verdict::Authenticated + Verdict::Authenticated { svn: Svn(self.svn) } } else { Verdict::Rejected }) @@ -224,6 +246,52 @@ impl BootWatch for MockWalk { } } +#[derive(Debug, PartialEq, Eq)] +struct FloorFaultInjected; + +impl core::fmt::Display for FloorFaultInjected { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str("floor fault injected") + } +} + +impl core::error::Error for FloorFaultInjected {} + +/// An SVN floor without storage; tests read it back through the +/// capability's own `floor()` via `PlatformDriver::board`. +struct MockFloor { + floor: u32, + fail: bool, +} + +impl MockFloor { + fn new() -> Self { + Self { + floor: 0, + fail: false, + } + } +} + +impl orchestrator_capabilities::SvnFloor for MockFloor { + type Error = FloorFaultInjected; + + fn floor(&self) -> Result { + if self.fail { + return Err(FloorFaultInjected); + } + Ok(Svn(self.floor)) + } + + fn advance(&mut self, to: Svn) -> Result<(), FloorFaultInjected> { + if self.fail { + return Err(FloorFaultInjected); + } + self.floor = self.floor.max(to.0); + Ok(()) + } +} + /// The test board's type choices. struct MockBoard; @@ -232,15 +300,20 @@ impl BoardCapabilities for MockBoard { type Verifier = XorVerifier; type BootControl = MockReset; type BootWatch = MockWalk; + type SvnFloor = MockFloor; } fn driver(images: [MemImage; 1]) -> PlatformDriver { PlatformDriver::new(Board { images, - verifier: XorVerifier { fault: false }, + verifier: XorVerifier { + fault: false, + svn: 5, + }, boot_controls: [MockReset::new()], boot_watches: [MockWalk::idle()], component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], }) } @@ -320,10 +393,14 @@ fn verifier_fault_fails_closed() { let mut orch = orchestrator(); let mut driver = PlatformDriver::::new(Board { images: [MemImage::holding(valid_image())], - verifier: XorVerifier { fault: true }, + verifier: XorVerifier { + fault: true, + svn: 5, + }, boot_controls: [MockReset::new()], boot_watches: [MockWalk::idle()], component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], }); orch.dispatch(&mut driver, Event::PowerGood(PowerOnResult::Provisioned)); @@ -340,10 +417,17 @@ fn verify_for_a_different_component_is_refused() { MemImage::holding(valid_image()), MemImage::holding(valid_image()), ], - verifier: XorVerifier { fault: false }, + verifier: XorVerifier { + fault: false, + svn: 5, + }, boot_controls: [MockReset::new(), MockReset::new()], boot_watches: [MockWalk::idle(), MockWalk::idle()], component_kinds: [ComponentKind::Passive, ComponentKind::Passive], + svn_floors: [ + SvnFloorBinding::Erot(MockFloor::new()), + SvnFloorBinding::Erot(MockFloor::new()), + ], }); driver.stage_firmware(C0).unwrap(); @@ -374,21 +458,23 @@ fn verify_of_unknown_component_is_refused() { #[test] fn reset_release_and_assert_reach_the_boot_control() { - let control = MockReset::new(); - let held = control.held.clone(); let mut driver = PlatformDriver::::new(Board { images: [MemImage::holding(valid_image())], - verifier: XorVerifier { fault: false }, - boot_controls: [control], + verifier: XorVerifier { + fault: false, + svn: 5, + }, + boot_controls: [MockReset::new()], boot_watches: [MockWalk::idle()], component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], }); driver.release_reset(C0).unwrap(); - assert!(!held.get()); + assert!(!driver.board().boot_controls[0].held.get()); driver.assert_reset(C0).unwrap(); - assert!(held.get()); + assert!(driver.board().boot_controls[0].held.get()); } #[test] @@ -411,10 +497,14 @@ fn reset_line_fault_is_reported() { control.fail = true; let mut driver = PlatformDriver::::new(Board { images: [MemImage::holding(valid_image())], - verifier: XorVerifier { fault: false }, + verifier: XorVerifier { + fault: false, + svn: 5, + }, boot_controls: [control], boot_watches: [MockWalk::idle()], component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], }); assert_eq!(driver.release_reset(C0), Err(DriverError::BootControlFault)); @@ -477,6 +567,7 @@ impl BoardCapabilities for WatchBoard { type Verifier = LineWatchingVerifier; type BootControl = MockReset; type BootWatch = MockWalk; + type SvnFloor = MockFloor; } // The at-rest guarantee end to end: the component is still held while its @@ -489,13 +580,17 @@ fn release_follows_verification() { let mut driver = PlatformDriver::::new(Board { images: [MemImage::holding(valid_image())], verifier: LineWatchingVerifier { - inner: XorVerifier { fault: false }, + inner: XorVerifier { + fault: false, + svn: 5, + }, line: held.clone(), held_during_verify: held_during_verify.clone(), }, boot_controls: [control], boot_watches: [MockWalk::idle()], component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], }); let mut orch = orchestrator(); @@ -518,10 +613,14 @@ fn failed_release_fails_closed() { let held = control.held.clone(); let mut driver = PlatformDriver::::new(Board { images: [MemImage::holding(valid_image())], - verifier: XorVerifier { fault: false }, + verifier: XorVerifier { + fault: false, + svn: 5, + }, boot_controls: [control], boot_watches: [MockWalk::idle()], component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], }); let mut orch = orchestrator(); @@ -546,10 +645,17 @@ fn walk_driver( MemImage::holding(valid_image()), MemImage::holding(valid_image()), ], - verifier: XorVerifier { fault: false }, + verifier: XorVerifier { + fault: false, + svn: 5, + }, boot_controls: [MockReset::new(), MockReset::new()], boot_watches: walks, component_kinds, + svn_floors: [ + SvnFloorBinding::Erot(MockFloor::new()), + SvnFloorBinding::Erot(MockFloor::new()), + ], }) } @@ -729,10 +835,14 @@ fn booted_walk_settles_in_ready() { let mut orch = orchestrator(); let mut driver = PlatformDriver::::new(Board { images: [MemImage::holding(valid_image())], - verifier: XorVerifier { fault: false }, + verifier: XorVerifier { + fault: false, + svn: 5, + }, boot_controls: [MockReset::new()], boot_watches: [MockWalk::scripted(std::vec![WalkVerdict::Complete])], component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], }); orch.dispatch(&mut driver, Event::PowerGood(PowerOnResult::Provisioned)); @@ -754,13 +864,17 @@ fn boot_timeout_fails_closed_without_recovery() { let mut orch = orchestrator(); let mut driver = PlatformDriver::::new(Board { images: [MemImage::holding(valid_image())], - verifier: XorVerifier { fault: false }, + verifier: XorVerifier { + fault: false, + svn: 5, + }, boot_controls: [MockReset::new()], boot_watches: [MockWalk::scripted(std::vec![WalkVerdict::Failed { checkpoint: "heartbeat", cause: FailureCause::TimedOut, }])], component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], }); orch.dispatch(&mut driver, Event::PowerGood(PowerOnResult::Provisioned)); @@ -772,3 +886,183 @@ fn boot_timeout_fails_closed_without_recovery() { assert_eq!(orch.state(), State::Locked); } + +// --------------------------------------------------------------------------- +// Report sink. +// --------------------------------------------------------------------------- + +/// Records what it is handed: the seam satisfied without a management +/// transport. +struct RecordingSink { + seen: std::vec::Vec, +} + +impl RecordingSink { + fn new() -> Self { + Self { + seen: std::vec::Vec::new(), + } + } +} + +impl ReportSink for RecordingSink { + fn report(&mut self, report: Report) { + self.seen.push(report); + } +} + +/// One of each report, so a test covers the whole enum. +fn every_report() -> [Report; 4] { + [ + Report::Isolated(C0), + Report::RecoveryFailed(C0), + Report::UpdateDeferred, + Report::UpdateAborted, + ] +} + +// Every report is deliverable through the seam alone, in the order handed +// over; the unit sink is a wiring choice and satisfies the same caller. +#[test] +fn every_report_reaches_a_sink() { + fn tell(sink: &mut S, reports: [Report; 4]) { + for report in reports { + sink.report(report); + } + } + + let mut recording = RecordingSink::new(); + tell(&mut recording, every_report()); + assert_eq!(recording.seen, every_report()); + + tell(&mut (), every_report()); +} + +// --------------------------------------------------------------------------- +// Anti-rollback floor commits. +// --------------------------------------------------------------------------- + +// The floor may only move to the SVN the verifier vouched for, and only +// after a verification has passed — the two halves of the commit contract. +#[test] +fn commit_advances_the_floor_to_the_verified_svn() { + let mut driver = PlatformDriver::::new(Board { + images: [MemImage::holding(valid_image())], + verifier: XorVerifier { + fault: false, + svn: 5, + }, + boot_controls: [MockReset::new()], + boot_watches: [MockWalk::idle()], + component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], + }); + + driver + .execute(Effect::ReadFirmware(C0)) + .expect("stage failed"); + driver + .execute(Effect::VerifyFirmware(C0)) + .expect("verify failed"); + + assert_eq!(driver.execute(Effect::CommitSvnFloor(C0)), Ok(None)); + // Read back through the capability's own seam. + let SvnFloorBinding::Erot(floor) = &driver.board().svn_floors[0] else { + panic!("C0 is wired with an eRoT floor"); + }; + assert_eq!( + floor.floor(), + Ok(Svn(5)), + "floor advanced to the verifier's SVN" + ); +} + +// A component wired without an eRoT floor tracks its own SVN. The commit +// succeeds even with no verified image cached: there is no floor to +// mis-advance. +#[test] +fn commit_without_an_erot_floor_is_a_no_op() { + let mut driver = PlatformDriver::::new(Board { + images: [MemImage::holding(valid_image())], + verifier: XorVerifier { + fault: false, + svn: 5, + }, + boot_controls: [MockReset::new()], + boot_watches: [MockWalk::idle()], + component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::SelfManaged], + }); + + assert_eq!(driver.execute(Effect::CommitSvnFloor(C0)), Ok(None)); +} + +#[test] +fn commit_without_a_verified_image_fails_closed() { + let mut driver = driver([MemImage::holding(valid_image())]); + + assert_eq!( + driver.commit_svn_floor(C0), + Err(DriverError::NoVerifiedImage) + ); +} + +// A rejection must clear the cached SVN, or the floor could commit against +// an image that is no longer the authenticated one. +#[test] +fn rejected_image_clears_the_verified_svn() { + let mut corrupt = valid_image(); + corrupt[7] ^= 0x01; + let mut driver = PlatformDriver::::new(Board { + images: [MemImage::holding(valid_image()).reflash_on_reopen(corrupt)], + verifier: XorVerifier { + fault: false, + svn: 5, + }, + boot_controls: [MockReset::new()], + boot_watches: [MockWalk::idle()], + component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(MockFloor::new())], + }); + + driver.stage_firmware(C0).expect("stage failed"); + assert_eq!( + driver.verify_firmware(C0), + Ok(Event::VerificationPassed(C0)) + ); + + // The queued re-flash lands on the re-stage; the rejection must take + // the cached SVN with it. + driver.stage_firmware(C0).expect("re-stage failed"); + assert_eq!( + driver.verify_firmware(C0), + Ok(Event::VerificationFailed(C0)) + ); + + assert_eq!( + driver.commit_svn_floor(C0), + Err(DriverError::NoVerifiedImage) + ); +} + +#[test] +fn floor_fault_is_reported() { + let mut mock = MockFloor::new(); + mock.fail = true; + let mut driver = PlatformDriver::::new(Board { + images: [MemImage::holding(valid_image())], + verifier: XorVerifier { + fault: false, + svn: 5, + }, + boot_controls: [MockReset::new()], + boot_watches: [MockWalk::idle()], + component_kinds: [ComponentKind::Passive], + svn_floors: [SvnFloorBinding::Erot(mock)], + }); + + driver.stage_firmware(C0).expect("stage failed"); + driver.verify_firmware(C0).expect("verify failed"); + + assert_eq!(driver.commit_svn_floor(C0), Err(DriverError::SvnFloorFault)); +} diff --git a/services/pldm/BUILD.bazel b/services/pldm/BUILD.bazel index 6f9ddbc4d..d420d0b92 100644 --- a/services/pldm/BUILD.bazel +++ b/services/pldm/BUILD.bazel @@ -55,6 +55,8 @@ rust_test( ":pldm_service", "//services/mctp/api:mctp_api", "//services/mctp/server:mctp_server_lib", + "//services/orchestrator/adapters/pldm:orchestrator_pldm_adapter", + "//services/orchestrator/sm:orchestrator_sm", "@rust_crates//:mctp", "@rust_crates//:mctp-lib", "@rust_crates//:pldm-common", diff --git a/services/pldm/src/firmware_device.rs b/services/pldm/src/firmware_device.rs index 4246b3c30..2580b29ce 100644 --- a/services/pldm/src/firmware_device.rs +++ b/services/pldm/src/firmware_device.rs @@ -62,6 +62,46 @@ pub const FD_MAX_MSG: usize = 1024; /// within this window is expected and is not treated as an error. const RESPONDER_POLL_TIMEOUT_MILLIS: u32 = 1; +/// Update-lifecycle notification out of the PLDM FD state machine, one per +/// state-machine edge. +/// +/// Every variant payload is `Copy` and lifetime-free by design. Anything +/// buffer-shaped (image chunks, package data, version strings) lands in +/// flash or an `FdOps`-owned buffer; an event carries at most the small +/// fixed-size values that name or qualify the edge. +#[non_exhaustive] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FdEvent { + /// The Update Agent's `RequestUpdate` was accepted: the FD moved out of + /// `Idle` (into `LearnComponents`) and the success response has already + /// been sent. Emitted exactly once per accepted `RequestUpdate`; + /// rejected ones (`ALREADY_IN_UPDATE_MODE`, bad transfer size) never + /// reach here because they leave the FD state unchanged. + UpdateRequested, +} + +/// Receiver for [`FdEvent`] notifications out of the PLDM FD state machine. +/// +/// [`FirmwareDevice::run_terminus`] owns the PLDM state machine but has no +/// knowledge of the platform's update orchestration; this trait is the seam +/// between the two. It is deliberately PLDM-flavored (no orchestrator +/// types) so that depending on this crate never pulls in the orchestrator +/// stack; the mapping to an orchestrator event lives in an adapter crate, +/// following the same rule as the orchestrator's HAL adapters. +/// +/// [`FdEvent`] is `#[non_exhaustive]`: implementors match the variants they +/// care about and ignore the rest, so new FD lifecycle events do not break +/// existing sinks. +pub trait FdEventSink { + /// Receive one FD lifecycle event. + fn notify(&mut self, event: FdEvent); +} + +/// Drop update notifications, for callers with no orchestration to notify. +impl FdEventSink for () { + fn notify(&mut self, _event: FdEvent) {} +} + /// Outcome of [`FirmwareDevice::run_terminus`]. pub enum RunTerminusResult { /// The loop exited normally (currently unreachable: `run_terminus` only @@ -151,6 +191,11 @@ impl<'a, O: FdOps, Cr: MctpClient, Cq: MctpClient> FirmwareDevice<'a, O, Cr, Cq> /// responder path to stay live even during a stalled FD-initiated /// request should pass a bounded value instead. /// + /// `sink` receives [`FdEvent::UpdateRequested`] once per + /// accepted `RequestUpdate` (the FD's only `Idle` → non-`Idle` + /// transition), after the success response has been sent. Callers with + /// nothing to notify pass `&mut ()`. + /// /// [`should_start_initiator_mode`]: pldm_interface::firmware_device::fd_context::FirmwareDeviceContext pub fn run_terminus( &mut self, @@ -158,8 +203,15 @@ impl<'a, O: FdOps, Cr: MctpClient, Cq: MctpClient> FirmwareDevice<'a, O, Cr, Cq> buf: &mut [u8], timeout_millis: u32, requester_timeout_millis: u32, + sink: &mut impl FdEventSink, ) -> RunTerminusResult { - match self.run_terminus_inner(remote_eid, buf, timeout_millis, requester_timeout_millis) { + match self.run_terminus_inner( + remote_eid, + buf, + timeout_millis, + requester_timeout_millis, + sink, + ) { Ok(()) => RunTerminusResult::Completed, Err(e) => RunTerminusResult::StoppedByError(e), } @@ -171,6 +223,7 @@ impl<'a, O: FdOps, Cr: MctpClient, Cq: MctpClient> FirmwareDevice<'a, O, Cr, Cq> buf: &mut [u8], timeout_millis: u32, requester_timeout_millis: u32, + sink: &mut impl FdEventSink, ) -> Result<(), PldmServiceError> { let mut responder_listener = self .responder_transport @@ -220,6 +273,11 @@ impl<'a, O: FdOps, Cr: MctpClient, Cq: MctpClient> FirmwareDevice<'a, O, Cr, Cq> timeout_millis }; responder_listener.set_timeout(poll_timeout); + // Sampled around the responder poll: `RequestUpdate` is the only + // command that takes the FD out of `Idle`, so the false→true edge + // of `is_update_mode()` identifies exactly one accepted + // `RequestUpdate` (the initiator phase above never leaves `Idle`). + let was_update_mode = self.cmd_interface.fd_ctx.is_update_mode(); match self.responder_transport.respond_once( &mut responder_listener, buf, @@ -234,7 +292,11 @@ impl<'a, O: FdOps, Cr: MctpClient, Cq: MctpClient> FirmwareDevice<'a, O, Cr, Cq> .map_err(PldmServiceError::MsgHandler) }, ) { - Ok(()) => {} + Ok(()) => { + if !was_update_mode && self.cmd_interface.fd_ctx.is_update_mode() { + sink.notify(FdEvent::UpdateRequested); + } + } // A short poll timeout while an initiator request is active // just means no UA command arrived in that window; keep // looping so the transfer can continue. diff --git a/services/pldm/src/lib.rs b/services/pldm/src/lib.rs index bbc5fe417..7c7ddabce 100644 --- a/services/pldm/src/lib.rs +++ b/services/pldm/src/lib.rs @@ -60,8 +60,10 @@ //! // `run_terminus` loops forever, interleaving inbound UA commands with any //! // FD-initiated requests (e.g. RequestFirmwareData) once an update begins. //! // It returns only on error; a `timeout_millis`/`requester_timeout_millis` -//! // of `0` blocks indefinitely while idle. -//! if let Err(e) = fd.run_terminus(UA_EID, &mut buf, 0, 0) { +//! // of `0` blocks indefinitely while idle. The final argument is an +//! // `FdEventSink` notified once per accepted RequestUpdate; `&mut ()` +//! // drops the notifications. +//! if let Err(e) = fd.run_terminus(UA_EID, &mut buf, 0, 0, &mut ()) { //! // handle or log error //! } //! ``` diff --git a/services/pldm/tests/base_host.rs b/services/pldm/tests/base_host.rs index e71c8b2b7..29e1fc936 100644 --- a/services/pldm/tests/base_host.rs +++ b/services/pldm/tests/base_host.rs @@ -207,7 +207,7 @@ fn base_full_chain_via_firmware_device() { // which point it returns Mctp(TimedOut); that terminating timeout means // "done", not a failure. let mut run_fd_once = - || match fd.run_terminus(UA_EID, &mut fd_buf, TIMEOUT_MILLIS, TIMEOUT_MILLIS) { + || match fd.run_terminus(UA_EID, &mut fd_buf, TIMEOUT_MILLIS, TIMEOUT_MILLIS, &mut ()) { RunTerminusResult::Completed => {} RunTerminusResult::StoppedByError(PldmServiceError::Mctp(e)) if e.is_timeout() => {} RunTerminusResult::StoppedByError(e) => panic!("firmware device failed: {e:?}"), diff --git a/services/pldm/tests/firmware_update_host.rs b/services/pldm/tests/firmware_update_host.rs index 0ebef4fa1..1e8d8752c 100644 --- a/services/pldm/tests/firmware_update_host.rs +++ b/services/pldm/tests/firmware_update_host.rs @@ -26,6 +26,8 @@ use mctp::Eid; use mctp_lib::Sender; use openprot_mctp_api::Handle; use openprot_mctp_server::Server; +use openprot_orchestrator_pldm_adapter::UpdateRequestLatch; +use openprot_orchestrator_sm::Event; use openprot_pldm_service::firmware_device::{FirmwareDevice, RunTerminusResult}; use openprot_pldm_service::{MctpPldmTransport, PldmServiceError}; use pldm_common::codec::{PldmCodec, PldmCodecWithLifetime}; @@ -51,7 +53,8 @@ use pldm_common::protocol::base::{ }; use pldm_common::protocol::firmware_update::{ ComponentClassification, ComponentResponseCode, Descriptor, FirmwareDeviceState, FwUpdateCmd, - PldmFirmwareString, UpdateOptionFlags, VersionStringType, PLDM_FWUP_IMAGE_SET_VER_STR_MAX_LEN, + FwUpdateCompletionCode, PldmFirmwareString, UpdateOptionFlags, VersionStringType, + PLDM_FWUP_IMAGE_SET_VER_STR_MAX_LEN, }; use pldm_common::util::fw_component::FirmwareComponent; use pldm_interface::firmware_device::fd_ops::{ComponentOperation, FdOps, FdOpsError}; @@ -312,6 +315,10 @@ fn firmware_update_full_flow_via_requester() { )); let fd_buf = RefCell::new([0u8; 1024]); + // Orchestrator-facing latch: `run_terminus` marks it on each accepted + // RequestUpdate; the assertions below drain it as `Event::UpdateRequest`. + let update_events = RefCell::new(UpdateRequestLatch::new()); + // Run one full UA->FD->UA command round-trip and return the PLDM response // payload (without the MCTP framing byte). let ua_transact = |req_pldm: &[u8]| -> Vec { @@ -332,6 +339,7 @@ fn firmware_update_full_flow_via_requester() { &mut fd_buf.borrow_mut()[..], TIMEOUT_MILLIS, TIMEOUT_MILLIS, + &mut *update_events.borrow_mut(), ) { RunTerminusResult::Completed => {} RunTerminusResult::StoppedByError(PldmServiceError::Mctp(e)) if e.is_timeout() => {} @@ -371,6 +379,42 @@ fn firmware_update_full_flow_via_requester() { resp[3], 0, "RequestUpdate completion code should be success" ); + assert_eq!( + update_events.borrow_mut().take(), + Some(Event::UpdateRequest), + "accepted RequestUpdate should latch exactly one orchestrator event" + ); + assert_eq!( + update_events.borrow_mut().take(), + None, + "the latch must not re-fire once drained" + ); + + // ---- Duplicate RequestUpdate: rejected, must not latch an event ---- + instance_id += 1; + let dup_update = RequestUpdateRequest::new( + instance_id, + PldmMsgType::Request, + IMAGE_SIZE, + 1, + 1, + 0, + &comp_ver, + ); + let len = dup_update + .encode(&mut buf) + .expect("encode duplicate RequestUpdate"); + let resp = ua_transact(&buf[..len]); + assert_eq!( + resp[3], + FwUpdateCompletionCode::AlreadyInUpdateMode as u8, + "second RequestUpdate should be rejected while in update mode" + ); + assert_eq!( + update_events.borrow_mut().take(), + None, + "a rejected RequestUpdate must not latch an orchestrator event" + ); // ---- PassComponentTable (Start+End): move to ReadyXfer ---- instance_id += 1; @@ -451,6 +495,11 @@ fn firmware_update_full_flow_via_requester() { ); assert!(fd_ops.verified.get(), "firmware should have been verified"); assert!(fd_ops.applied.get(), "firmware should have been applied"); + assert_eq!( + update_events.borrow_mut().take(), + None, + "no command after the accepted RequestUpdate should latch an event" + ); println!( "Firmware update host test completed: downloaded {} bytes, verified={}, applied={}", diff --git a/services/pldm/tests/unexpected_eid_fw_host.rs b/services/pldm/tests/unexpected_eid_fw_host.rs index fef06823f..055b974be 100644 --- a/services/pldm/tests/unexpected_eid_fw_host.rs +++ b/services/pldm/tests/unexpected_eid_fw_host.rs @@ -228,7 +228,7 @@ fn responder_ignores_fw_commands_from_unexpected_eid() { // "done", not a failure. `UA_EID` is the only EID `run_terminus` is told // to serve, so commands from `ATTACKER_EID` must be ignored below. let mut run_fd_once = - || match fd.run_terminus(UA_EID, &mut fd_buf, TIMEOUT_MILLIS, TIMEOUT_MILLIS) { + || match fd.run_terminus(UA_EID, &mut fd_buf, TIMEOUT_MILLIS, TIMEOUT_MILLIS, &mut ()) { RunTerminusResult::Completed => {} RunTerminusResult::StoppedByError(PldmServiceError::Mctp(e)) if e.is_timeout() => {} RunTerminusResult::StoppedByError(e) => panic!("firmware device failed: {e:?}"), diff --git a/services/pldm/tests/unexpected_eid_host.rs b/services/pldm/tests/unexpected_eid_host.rs index b3d0302fa..c074f2571 100644 --- a/services/pldm/tests/unexpected_eid_host.rs +++ b/services/pldm/tests/unexpected_eid_host.rs @@ -202,7 +202,7 @@ fn responder_ignores_commands_from_unexpected_eid() { // "done", not a failure. `UA_EID` is the only EID `run_terminus` is told // to serve, so commands from `ATTACKER_EID` must be ignored below. let mut run_fd_once = - || match fd.run_terminus(UA_EID, &mut fd_buf, TIMEOUT_MILLIS, TIMEOUT_MILLIS) { + || match fd.run_terminus(UA_EID, &mut fd_buf, TIMEOUT_MILLIS, TIMEOUT_MILLIS, &mut ()) { RunTerminusResult::Completed => {} RunTerminusResult::StoppedByError(PldmServiceError::Mctp(e)) if e.is_timeout() => {} RunTerminusResult::StoppedByError(e) => panic!("firmware device failed: {e:?}"),