From db85c0b01f40e1ba1902556d27ed3b6647c10ed5 Mon Sep 17 00:00:00 2001 From: Ben Stoltz Date: Sat, 29 Jun 2024 19:06:35 -0700 Subject: [PATCH] Implement epoch-based rollback protection in LPC55 update_server --- Cargo.lock | 3 +- Cargo.toml | 12 +- app/gimlet/base.toml | 1 + app/lpc55xpresso/app.toml | 2 +- app/oxide-rot-1/app-dev.toml | 2 +- build/xtask/src/dist.rs | 9 +- drv/lpc55-update-api/src/lib.rs | 10 ++ drv/lpc55-update-server/src/images.rs | 155 +++++++++++++++++++++++--- drv/lpc55-update-server/src/main.rs | 27 +++-- drv/update-api/src/lib.rs | 2 + 10 files changed, 192 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4eb6d6b39d..f22e9531aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4613,6 +4613,7 @@ dependencies = [ "drv-auxflash-api", "drv-caboose", "drv-caboose-pos", + "drv-gimlet-hf-api", "drv-gimlet-seq-api", "drv-hf-api", "drv-ignition-api", @@ -4754,8 +4755,8 @@ dependencies = [ "byteorder", "cfg-if", "cortex-m", + "drv-gimlet-hf-api", "drv-hash-api", - "drv-hf-api", "drv-i2c-api", "drv-lpc55-gpio-api", "drv-sp-ctrl-api", diff --git a/Cargo.toml b/Cargo.toml index 2f648718df..8073dacba4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -120,14 +120,20 @@ zip = { version = "0.6", default-features = false, features = ["bzip2"] } # Oxide forks and repos attest-data = { git = "https://github.com/oxidecomputer/dice-util", default-features = false, version = "0.2.0" } dice-mfg-msgs = { git = "https://github.com/oxidecomputer/dice-util", default-features = false, version = "0.2.1" } -gateway-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", default-features = false, features = ["smoltcp"] } +#gateway-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", default-features = false, features = ["smoltcp"] } +# XXX fix before push +gateway-messages = { path = "/home/stoltz/Oxide/src/mgs/epoch/gateway-messages", default-features = false, features = ["smoltcp"] } gimlet-inspector-protocol = { git = "https://github.com/oxidecomputer/gimlet-inspector-protocol", version = "0.1.0" } hif = { git = "https://github.com/oxidecomputer/hif", default-features = false } humpty = { git = "https://github.com/oxidecomputer/humpty", default-features = false, version = "0.1.3" } -hubtools = { git = "https://github.com/oxidecomputer/hubtools", default-features = false, version = "0.4.1" } +#hubtools = { git = "https://github.com/oxidecomputer/hubtools", default-features = false, version = "0.4.1" } +# XXX fix before push +# hubtools = { git = "https://github.com/oxidecomputer/hubtools", default-features = false, branch = "epoch", version = "0.4.7" } +hubtools = { path = "/home/stoltz/Oxide/src/hubtools/epoch/hubtools" } idol = { git = "https://github.com/oxidecomputer/idolatry.git", default-features = false } idol-runtime = { git = "https://github.com/oxidecomputer/idolatry.git", default-features = false } -lpc55_sign = { git = "https://github.com/oxidecomputer/lpc55_support", default-features = false } +#lpc55_sign = { git = "https://github.com/oxidecomputer/lpc55_support", default-features = false } +lpc55_sign = { path = "/home/stoltz/Oxide/src/lpc55_support/lpc55_sign", default-features = false } ordered-toml = { git = "https://github.com/oxidecomputer/ordered-toml", default-features = false } pmbus = { git = "https://github.com/oxidecomputer/pmbus", default-features = false } salty = { version = "0.3", default-features = false } diff --git a/app/gimlet/base.toml b/app/gimlet/base.toml index 6a271cf4e5..d9c5bcac6f 100644 --- a/app/gimlet/base.toml +++ b/app/gimlet/base.toml @@ -3,6 +3,7 @@ chip = "../../chips/stm32h7" memory = "memory-large.toml" stacksize = 896 fwid = true +epoch = 0 [kernel] name = "gimlet" diff --git a/app/lpc55xpresso/app.toml b/app/lpc55xpresso/app.toml index a50895d4af..12ac78ceeb 100644 --- a/app/lpc55xpresso/app.toml +++ b/app/lpc55xpresso/app.toml @@ -48,7 +48,7 @@ start = true [tasks.update_server] name = "lpc55-update-server" priority = 3 -max-sizes = {flash = 27008, ram = 16704} +max-sizes = {flash = 30368, ram = 16704} stacksize = 8192 start = true sections = {bootstate = "usbsram"} diff --git a/app/oxide-rot-1/app-dev.toml b/app/oxide-rot-1/app-dev.toml index 328e61760d..c970d6d7f0 100644 --- a/app/oxide-rot-1/app-dev.toml +++ b/app/oxide-rot-1/app-dev.toml @@ -49,7 +49,7 @@ start = true [tasks.update_server] name = "lpc55-update-server" priority = 3 -max-sizes = {flash = 27904, ram = 17344, usbsram = 4096} +max-sizes = {flash = 30368, ram = 17344, usbsram = 4096} # TODO: Size this appropriately stacksize = 8192 start = true diff --git a/build/xtask/src/dist.rs b/build/xtask/src/dist.rs index dc35f5d1de..da2d657af4 100644 --- a/build/xtask/src/dist.rs +++ b/build/xtask/src/dist.rs @@ -611,7 +611,7 @@ pub fn package( // The Git hash is included in the default caboose under the key // `GITC`, so we don't include it in the pseudo-version. archive - .write_default_caboose(None) + .write_default_caboose(None, None) .context("writing caboose into archive")?; archive.overwrite().context("overwriting archive")?; } @@ -621,8 +621,13 @@ pub fn package( if let Some(signing) = &cfg.toml.signing { let mut archive = hubtools::RawHubrisArchive::load(&archive_name) .context("loading archive with hubtools")?; + let priv_key_rel_path = signing + .certs + .private_key + .clone() + .context("missing private key path")?; let private_key = lpc55_sign::cert::read_rsa_private_key( - &cfg.app_src_dir.join(&signing.certs.private_key), + &cfg.app_src_dir.join(priv_key_rel_path), ) .with_context(|| { format!( diff --git a/drv/lpc55-update-api/src/lib.rs b/drv/lpc55-update-api/src/lib.rs index c853669986..62da0b2f27 100644 --- a/drv/lpc55-update-api/src/lib.rs +++ b/drv/lpc55-update-api/src/lib.rs @@ -242,6 +242,16 @@ impl From for SlotId { } } +impl SlotId { + pub fn other(&self) -> SlotId { + if *self == SlotId::A { + SlotId::B + } else { + SlotId::A + } + } +} + impl TryFrom for SlotId { type Error = (); fn try_from(i: u16) -> Result { diff --git a/drv/lpc55-update-server/src/images.rs b/drv/lpc55-update-server/src/images.rs index 49d66210e7..741a77322f 100644 --- a/drv/lpc55-update-server/src/images.rs +++ b/drv/lpc55-update-server/src/images.rs @@ -20,7 +20,10 @@ use crate::{ use abi::{ImageHeader, CABOOSE_MAGIC, HEADER_MAGIC}; use core::ops::Range; use core::ptr::addr_of; -use drv_lpc55_update_api::{RawCabooseError, RotComponent, SlotId}; +use drv_caboose::CabooseReader; +use drv_lpc55_update_api::{ + RawCabooseError, RotComponent, SlotId, BLOCK_SIZE_BYTES, +}; use drv_update_api::UpdateError; use zerocopy::{AsBytes, FromBytes}; @@ -47,6 +50,7 @@ pub const HEADER_BLOCK: usize = 0; // An image may have an ImageHeader located after the // LPC55's mixed header/vector table. pub const IMAGE_HEADER_OFFSET: u32 = 0x130; +pub const CABOOSE_TAG_EPOC: [u8; 4] = [b'E', b'P', b'O', b'C']; /// Address ranges that may contain an image during storage and active use. /// `stored` and `at_runtime` ranges are the same except for `stage0next`. @@ -181,7 +185,7 @@ impl TryFrom<&[u8]> for ImageVectorsLpc55 { /// the end of optional caboose and the beginning of the signature block. pub fn validate_header_block( header_access: &ImageAccess<'_>, -) -> Result { +) -> Result<(Option, u32), UpdateError> { let mut vectors = ImageVectorsLpc55::new_zeroed(); let mut header = ImageHeader::new_zeroed(); @@ -208,15 +212,17 @@ pub fn validate_header_block( // Note that `ImageHeader.epoch` is used by rollback protection for early // rejection of invalid images. // TODO: Improve estimate of where the first executable instruction can be. - let code_offset = if header.magic == HEADER_MAGIC { + let (code_offset, epoch) = if header.magic == HEADER_MAGIC { if header.total_image_len != vectors.nxp_offset_to_specific_header { // ImageHeader disagrees with LPC55 vectors. return Err(UpdateError::InvalidHeaderBlock); } - // Adding constants should be resolved at compile time: no call to panic. - IMAGE_HEADER_OFFSET + (core::mem::size_of::() as u32) + ( + IMAGE_HEADER_OFFSET + (core::mem::size_of::() as u32), + Some(Epoch::from(header.epoch)), + ) } else { - IMAGE_HEADER_OFFSET + (IMAGE_HEADER_OFFSET, None) }; if vectors.nxp_image_length as usize > header_access.at_runtime().len() { @@ -243,7 +249,7 @@ pub fn validate_header_block( return Err(UpdateError::InvalidHeaderBlock); } - Ok(vectors.nxp_offset_to_specific_header) + Ok((epoch, vectors.nxp_offset_to_specific_header)) } /// Get the range of the caboose contained within an image if it exists. @@ -260,7 +266,7 @@ pub fn caboose_slice( // // In this context, NoImageHeader actually means that the image // is not well formed. - let image_end_offset = validate_header_block(image) + let (_epoch, image_end_offset) = validate_header_block(image) .map_err(|_| RawCabooseError::NoImageHeader)?; // By construction, the last word of the caboose is its size as a `u32` @@ -318,7 +324,7 @@ enum Accessor<'a> { }, // Hybrid is used for later implementation of rollback protection. // The buffer is used in place of the beginning of the flash range. - _Hybrid { + Hybrid { buffer: &'a [u8], flash: &'a drv_lpc55_flash::Flash<'a>, span: FlashRange, @@ -330,7 +336,7 @@ impl Accessor<'_> { match self { Accessor::Flash { span, .. } | Accessor::Ram { span, .. } - | Accessor::_Hybrid { span, .. } => &span.at_runtime, + | Accessor::Hybrid { span, .. } => &span.at_runtime, } } } @@ -375,7 +381,7 @@ impl ImageAccess<'_> { } } - pub fn _new_hybrid<'a>( + pub fn new_hybrid<'a>( flash: &'a drv_lpc55_flash::Flash<'a>, buffer: &'a [u8], component: RotComponent, @@ -383,7 +389,7 @@ impl ImageAccess<'_> { ) -> ImageAccess<'a> { let span = flash_range(component, slot); ImageAccess { - accessor: Accessor::_Hybrid { + accessor: Accessor::Hybrid { flash, buffer, span, @@ -430,7 +436,7 @@ impl ImageAccess<'_> { .and_then(u32::read_from) .ok_or(UpdateError::OutOfBounds)?) } - Accessor::_Hybrid { + Accessor::Hybrid { buffer, flash, span, @@ -491,7 +497,7 @@ impl ImageAccess<'_> { Err(UpdateError::OutOfBounds) } } - Accessor::_Hybrid { + Accessor::Hybrid { buffer: ram, flash, span, @@ -547,7 +553,7 @@ impl ImageAccess<'_> { ImageVectorsLpc55::read_from_prefix(&buffer[..]) .ok_or(UpdateError::OutOfBounds) } - Accessor::Ram { buffer, .. } | Accessor::_Hybrid { buffer, .. } => { + Accessor::Ram { buffer, .. } | Accessor::Hybrid { buffer, .. } => { ImageVectorsLpc55::read_from_prefix(buffer) .ok_or(UpdateError::OutOfBounds) } @@ -556,3 +562,122 @@ impl ImageAccess<'_> { round_up_to_flash_page(len).ok_or(UpdateError::BadLength) } } + +#[derive(Clone, PartialEq)] +pub struct Epoch { + value: u32, +} + +/// Convert from the ImageHeader.epoch format +impl From for Epoch { + fn from(number: u32) -> Self { + Epoch { value: number } + } +} + +/// Convert from the caboose EPOC value format. +// +// Invalid EPOC values converted to Epoch{value:0} include: +// - empty slice +// - any non-ASCII-digits in slice +// - any non-UTF8 in slice +// - leading '+' normally allowed by parse() +// - values greater than u32::MAX +// +// Hand coding reduces size by about 950 bytes. +impl From<&[u8]> for Epoch { + fn from(chars: &[u8]) -> Self { + let epoch = + if chars.first().map(|c| c.is_ascii_digit()).unwrap_or(false) { + if let Ok(chars) = core::str::from_utf8(chars) { + chars.parse::().unwrap_or(0) + } else { + 0 + } + } else { + 0 + }; + Epoch::from(epoch) + } +} + +impl Epoch { + pub fn can_update_to(&self, next: Epoch) -> bool { + self.value >= next.value + } +} + +/// Check a next image against an active image to determine +/// if rollback policy allows the next image. +/// If ImageHeader and Caboose are absent or Caboose does +/// not have an `EPOC` tag, then the Epoch is defaulted to zero. +/// This test is also used when the header block first arrives +/// so that images can be rejected early, i.e. before any flash has +/// been altered. +pub fn check_rollback_policy( + next_image: ImageAccess<'_>, + active_image: ImageAccess<'_>, + complete: bool, +) -> Result<(), UpdateError> { + let next_epoch = get_image_epoch(&next_image)?; + let active_epoch = get_image_epoch(&active_image)?; + match (active_epoch, next_epoch) { + // No active_epoch is treated as zero; update can proceed. + (None, _) => Ok(()), + (Some(active_epoch), None) => { + // If next_image is partial and HEADER_BLOCK has no ImageHeader, + // then there is no early rejection, proceed. + if !complete || active_epoch.can_update_to(Epoch::from(0u32)) { + Ok(()) + } else { + Err(UpdateError::RollbackProtection) + } + } + (Some(active_epoch), Some(next_epoch)) => { + if active_epoch.can_update_to(next_epoch) { + Ok(()) + } else { + Err(UpdateError::RollbackProtection) + } + } + } +} + +/// Get ImageHeader epoch and/or caboose EPOC from an Image if it exists. +/// Return default of zero epoch if neither is present. +/// This function is called at points where the image's signature has not been +/// checked or the image is incomplete. Sanity checks are required before using +/// any data. +fn get_image_epoch( + image: &ImageAccess<'_>, +) -> Result, UpdateError> { + let (header_epoch, _caboose_offset) = validate_header_block(image)?; + + if let Ok(span) = caboose_slice(image) { + let mut block = [0u8; BLOCK_SIZE_BYTES]; + let caboose = block[0..span.len()].as_bytes_mut(); + image.read_bytes(span.start, caboose)?; + let reader = CabooseReader::new(caboose); + let caboose_epoch = if let Ok(epoc) = reader.get(CABOOSE_TAG_EPOC) { + Some(Epoch::from(epoc)) + } else { + None + }; + match (header_epoch, caboose_epoch) { + (None, None) => Ok(None), + (Some(header_epoch), None) => Ok(Some(header_epoch)), + (None, Some(caboose_epoch)) => Ok(Some(caboose_epoch)), + (Some(header_epoch), Some(caboose_epoch)) => { + if caboose_epoch == header_epoch { + Ok(Some(caboose_epoch)) + } else { + // Epochs present in both and not matching is invalid. + // The image will be rejected after epoch 0. + Ok(Some(Epoch::from(0u32))) + } + } + } + } else { + Ok(header_epoch) + } +} diff --git a/drv/lpc55-update-server/src/main.rs b/drv/lpc55-update-server/src/main.rs index 5a0041c3ff..5e63dc28e3 100644 --- a/drv/lpc55-update-server/src/main.rs +++ b/drv/lpc55-update-server/src/main.rs @@ -9,7 +9,9 @@ #![no_std] #![no_main] -use crate::images::{validate_header_block, ImageVectorsLpc55}; +use crate::images::{ + check_rollback_policy, validate_header_block, ImageVectorsLpc55, +}; use core::convert::Infallible; use core::mem::{size_of, MaybeUninit}; use core::ops::Range; @@ -200,6 +202,14 @@ impl idl::InOrderUpdateImpl for ServerImpl<'_> { self.header_block = None; return Err(e.into()); } + let active_image = + ImageAccess::new_flash(&self.flash, component, slot.other()); + if let Err(e) = + check_rollback_policy(active_image, next_image, false) + { + self.header_block = None; + return Err(e.into()); + } } else { // Block order is enforced above. If we're here then we have // seen block zero already. @@ -249,13 +259,14 @@ impl idl::InOrderUpdateImpl for ServerImpl<'_> { } let (component, slot) = self.image.unwrap_lite(); - do_block_write( - &mut self.flash, - component, - slot, - HEADER_BLOCK, - self.header_block.as_ref().unwrap_lite(), - )?; + let buffer = self.header_block.as_ref().unwrap_lite(); + let next_image = + ImageAccess::new_hybrid(&self.flash, buffer, component, slot); + let active_image = + ImageAccess::new_flash(&self.flash, component, slot.other()); + check_rollback_policy(active_image, next_image, true)?; + + do_block_write(&mut self.flash, component, slot, HEADER_BLOCK, buffer)?; // Now erase the unused portion of the flash slot so that // flash slot has predictable contents and the FWID for it diff --git a/drv/update-api/src/lib.rs b/drv/update-api/src/lib.rs index 4f77a7333d..ecebe76ee9 100644 --- a/drv/update-api/src/lib.rs +++ b/drv/update-api/src/lib.rs @@ -65,6 +65,7 @@ pub enum UpdateError { ImageMismatch, SignatureNotValidated, VersionNotSupported, + RollbackProtection, } impl From for GwUpdateError { @@ -103,6 +104,7 @@ impl From for GwUpdateError { UpdateError::ImageMismatch => Self::ImageMismatch, UpdateError::SignatureNotValidated => Self::SignatureNotValidated, UpdateError::VersionNotSupported => Self::VersionNotSupported, + UpdateError::RollbackProtection => Self::RollbackProtection, } } }