From 2269bd152ba7d84bdce5f9726f4475d71ceee333 Mon Sep 17 00:00:00 2001 From: Laura Abbott Date: Thu, 30 May 2024 16:28:16 -0400 Subject: [PATCH] Filter available artifacts by signing hash The artifact repository now allows multiple versions of the same artifact type signed with different keys but this isn't quite sufficent. Wicket still needs to know which version to display for updates. Make the signing information available to wicket for display artifact versions. This also makes it clear if the repository does not contain an image signed with the expected keys. --- gateway/src/http_entrypoints.rs | 14 +++- nexus/inventory/src/builder.rs | 2 + nexus/inventory/src/examples.rs | 1 + openapi/gateway.json | 4 + openapi/wicketd.json | 15 +++- .../tests/output/self-stat-schema.json | 4 +- sp-sim/src/gimlet.rs | 2 + sp-sim/src/sidecar.rs | 2 + .../src/artifacts/artifacts_with_plan.rs | 25 +++++- update-common/src/artifacts/update_plan.rs | 15 +++- wicket/src/events.rs | 2 +- wicket/src/state/inventory.rs | 30 ++++++++ wicket/src/state/mod.rs | 2 +- wicket/src/state/update.rs | 27 +++++-- wicket/src/ui/panes/overview.rs | 11 +++ wicket/src/ui/panes/update.rs | 76 ++++++++++++++----- wicket/src/wicketd.rs | 4 +- wicketd/src/artifacts/store.rs | 2 + wicketd/src/http_entrypoints.rs | 1 + 19 files changed, 202 insertions(+), 37 deletions(-) diff --git a/gateway/src/http_entrypoints.rs b/gateway/src/http_entrypoints.rs index 727ba0950da..2ae46c192e3 100644 --- a/gateway/src/http_entrypoints.rs +++ b/gateway/src/http_entrypoints.rs @@ -530,6 +530,7 @@ pub struct SpComponentCaboose { pub board: String, pub name: String, pub version: String, + pub sign: Option, } /// Identity of a host phase2 recovery image. @@ -773,6 +774,7 @@ async fn sp_component_caboose_get( const CABOOSE_KEY_BOARD: [u8; 4] = *b"BORD"; const CABOOSE_KEY_NAME: [u8; 4] = *b"NAME"; const CABOOSE_KEY_VERSION: [u8; 4] = *b"VERS"; + const CABOOSE_KEY_SIGN: [u8; 4] = *b"SIGN"; let apictx = rqctx.context(); let PathSpComponent { sp, component } = path.into_inner(); @@ -826,13 +828,23 @@ async fn sp_component_caboose_get( sp: sp_id, err, })?; + // Not all images include the SIGN in the caboose, if it's not present + // don't treat it as an error + let sign = match sp + .read_component_caboose(component, firmware_slot, CABOOSE_KEY_SIGN) + .await + .ok() + { + None => None, + Some(v) => Some(from_utf8(&CABOOSE_KEY_SIGN, v)?), + }; let git_commit = from_utf8(&CABOOSE_KEY_GIT_COMMIT, git_commit)?; let board = from_utf8(&CABOOSE_KEY_BOARD, board)?; let name = from_utf8(&CABOOSE_KEY_NAME, name)?; let version = from_utf8(&CABOOSE_KEY_VERSION, version)?; - let caboose = SpComponentCaboose { git_commit, board, name, version }; + let caboose = SpComponentCaboose { git_commit, board, name, version, sign }; Ok(HttpResponseOk(caboose)) } diff --git a/nexus/inventory/src/builder.rs b/nexus/inventory/src/builder.rs index bfa330669ff..dd019de1c63 100644 --- a/nexus/inventory/src/builder.rs +++ b/nexus/inventory/src/builder.rs @@ -1049,6 +1049,7 @@ mod test { git_commit: String::from("git_commit1"), name: String::from("name1"), version: String::from("version1"), + sign: None, }; assert!(!builder .found_caboose_already(&bogus_baseboard, CabooseWhich::SpSlot0)); @@ -1115,6 +1116,7 @@ mod test { git_commit: String::from("git_commit2"), name: String::from("name2"), version: String::from("version2"), + sign: None, }, ) .unwrap_err(); diff --git a/nexus/inventory/src/examples.rs b/nexus/inventory/src/examples.rs index 1a0c70f4566..5b9c29e22f4 100644 --- a/nexus/inventory/src/examples.rs +++ b/nexus/inventory/src/examples.rs @@ -489,6 +489,7 @@ pub fn caboose(unique: &str) -> SpComponentCaboose { git_commit: format!("git_commit_{}", unique), name: format!("name_{}", unique), version: format!("version_{}", unique), + sign: None, } } diff --git a/openapi/gateway.json b/openapi/gateway.json index c5d0eab0b13..232c81afda2 100644 --- a/openapi/gateway.json +++ b/openapi/gateway.json @@ -2428,6 +2428,10 @@ "name": { "type": "string" }, + "sign": { + "nullable": true, + "type": "string" + }, "version": { "type": "string" } diff --git a/openapi/wicketd.json b/openapi/wicketd.json index fd8e49b6e30..c0e4dc108fb 100644 --- a/openapi/wicketd.json +++ b/openapi/wicketd.json @@ -1658,6 +1658,15 @@ "items": { "$ref": "#/components/schemas/ArtifactHashId" } + }, + "sign": { + "nullable": true, + "type": "array", + "items": { + "type": "integer", + "format": "uint8", + "minimum": 0 + } } }, "required": [ @@ -2773,7 +2782,7 @@ ] }, "SpComponentCaboose": { - "description": "SpComponentCaboose\n\n
JSON schema\n\n```json { \"type\": \"object\", \"required\": [ \"board\", \"git_commit\", \"name\", \"version\" ], \"properties\": { \"board\": { \"type\": \"string\" }, \"git_commit\": { \"type\": \"string\" }, \"name\": { \"type\": \"string\" }, \"version\": { \"type\": \"string\" } } } ```
", + "description": "SpComponentCaboose\n\n
JSON schema\n\n```json { \"type\": \"object\", \"required\": [ \"board\", \"git_commit\", \"name\", \"version\" ], \"properties\": { \"board\": { \"type\": \"string\" }, \"git_commit\": { \"type\": \"string\" }, \"name\": { \"type\": \"string\" }, \"sign\": { \"type\": [ \"string\", \"null\" ] }, \"version\": { \"type\": \"string\" } } } ```
", "type": "object", "properties": { "board": { @@ -2785,6 +2794,10 @@ "name": { "type": "string" }, + "sign": { + "nullable": true, + "type": "string" + }, "version": { "type": "string" } diff --git a/oximeter/collector/tests/output/self-stat-schema.json b/oximeter/collector/tests/output/self-stat-schema.json index 286ac63405f..682ffb00932 100644 --- a/oximeter/collector/tests/output/self-stat-schema.json +++ b/oximeter/collector/tests/output/self-stat-schema.json @@ -39,7 +39,7 @@ } ], "datum_type": "cumulative_u64", - "created": "2024-05-21T18:32:24.199619581Z" + "created": "2024-06-03T14:56:43.613214591Z" }, "oximeter_collector:failed_collections": { "timeseries_name": "oximeter_collector:failed_collections", @@ -86,6 +86,6 @@ } ], "datum_type": "cumulative_u64", - "created": "2024-05-21T18:32:24.200514936Z" + "created": "2024-06-03T14:56:43.613836009Z" } } \ No newline at end of file diff --git a/sp-sim/src/gimlet.rs b/sp-sim/src/gimlet.rs index 0c109c1bd78..90fa06ddd41 100644 --- a/sp-sim/src/gimlet.rs +++ b/sp-sim/src/gimlet.rs @@ -1412,6 +1412,8 @@ impl SpHandler for Handler { (SpComponent::ROT, b"BORD") => ROT_BORD, (SpComponent::ROT, b"NAME") => ROT_NAME, (SpComponent::ROT, b"VERS") => ROT_VERS, + // gimlet staging/devel hash + (SpComponent::ROT, b"SIGN") => &"11594bb5548a757e918e6fe056e2ad9e084297c9555417a025d8788eacf55daf".as_bytes(), _ => return Err(SpError::NoSuchCabooseKey(key)), }; diff --git a/sp-sim/src/sidecar.rs b/sp-sim/src/sidecar.rs index 1bd6fe49649..d9cf2876e8c 100644 --- a/sp-sim/src/sidecar.rs +++ b/sp-sim/src/sidecar.rs @@ -1141,6 +1141,8 @@ impl SpHandler for Handler { (SpComponent::ROT, b"BORD") => ROT_BORD, (SpComponent::ROT, b"NAME") => ROT_NAME, (SpComponent::ROT, b"VERS") => ROT_VERS, + // sidecar staging/devel hash + (SpComponent::ROT, b"SIGN") => &"1432cc4cfe5688c51b55546fe37837c753cfbc89e8c3c6aabcf977fdf0c41e27".as_bytes(), _ => return Err(SpError::NoSuchCabooseKey(key)), }; diff --git a/update-common/src/artifacts/artifacts_with_plan.rs b/update-common/src/artifacts/artifacts_with_plan.rs index 950e2c5ab7c..650efccdfb6 100644 --- a/update-common/src/artifacts/artifacts_with_plan.rs +++ b/update-common/src/artifacts/artifacts_with_plan.rs @@ -58,6 +58,10 @@ pub struct ArtifactsWithPlan { // will contain two entries mapping each of the images to their data. by_hash: DebugIgnore>, + // Map from Rot artifact IDs to hash of signing information. This is + // used to select between different artifact versions in the same + // repository + rot_by_sign: DebugIgnore>>, // The plan to use to update a component within the rack. plan: UpdatePlan, } @@ -240,8 +244,13 @@ impl ArtifactsWithPlan { // Ensure we know how to apply updates from this set of artifacts; we'll // remember the plan we create. - let UpdatePlanBuildOutput { plan, by_id, by_hash, artifacts_meta } = - builder.build()?; + let UpdatePlanBuildOutput { + plan, + by_id, + by_hash, + rot_by_sign, + artifacts_meta, + } = builder.build()?; let tuf_repository = repository.repo(); @@ -266,7 +275,13 @@ impl ArtifactsWithPlan { let description = TufRepoDescription { repo: repo_meta, artifacts: artifacts_meta }; - Ok(Self { description, by_id, by_hash: by_hash.into(), plan }) + Ok(Self { + description, + by_id, + by_hash: by_hash.into(), + rot_by_sign: rot_by_sign.into(), + plan, + }) } /// Returns the `ArtifactsDocument` corresponding to this TUF repo. @@ -289,6 +304,10 @@ impl ArtifactsWithPlan { &self.plan } + pub fn rot_by_sign(&self) -> &HashMap> { + &self.rot_by_sign + } + pub fn get_by_hash( &self, id: &ArtifactHashId, diff --git a/update-common/src/artifacts/update_plan.rs b/update-common/src/artifacts/update_plan.rs index 53286ee09ad..278201ba462 100644 --- a/update-common/src/artifacts/update_plan.rs +++ b/update-common/src/artifacts/update_plan.rs @@ -76,7 +76,8 @@ pub struct UpdatePlan { // Used to represent the information extracted from signed RoT images. This // is used when going from `UpdatePlanBuilder` -> `UpdatePlan` to check -// the versions on the RoT images +// the versions on the RoT images and also to generate the map of +// ArtifactId -> Sign hashes for checking artifacts #[derive(Debug, Eq, Hash, PartialEq)] struct RotSignData { kind: KnownArtifactKind, @@ -768,7 +769,7 @@ impl<'a> UpdatePlanBuilder<'a> { // signing key have the same version. (i.e. allow gimlet_rot signed // with a staging key to be a different version from gimlet_rot signed // with a production key) - for (entry, versions) in self.rot_by_sign { + for (entry, versions) in &self.rot_by_sign { let kind = entry.kind; // This unwrap is safe because we check above that each of the types // has at least one entry @@ -784,6 +785,14 @@ impl<'a> UpdatePlanBuilder<'a> { } } } + + let mut rot_by_sign = HashMap::new(); + for (k, v) in self.rot_by_sign { + for id in v { + rot_by_sign.insert(id, k.sign.clone()); + } + } + // Repeat the same version check for all SP images. (This is a separate // loop because the types of the iterators don't match.) for (kind, mut single_board_sp_artifacts) in [ @@ -842,6 +851,7 @@ impl<'a> UpdatePlanBuilder<'a> { plan, by_id: self.by_id, by_hash: self.by_hash, + rot_by_sign, artifacts_meta: self.artifacts_meta, }) } @@ -852,6 +862,7 @@ pub struct UpdatePlanBuildOutput { pub plan: UpdatePlan, pub by_id: BTreeMap>, pub by_hash: HashMap, + pub rot_by_sign: HashMap>, pub artifacts_meta: Vec, } diff --git a/wicket/src/events.rs b/wicket/src/events.rs index fd0ac086adf..77012fab43a 100644 --- a/wicket/src/events.rs +++ b/wicket/src/events.rs @@ -31,7 +31,7 @@ pub enum Event { /// TUF repo artifacts unpacked by wicketd, and event reports ArtifactsAndEventReports { system_version: Option, - artifacts: Vec, + artifacts: Vec<(ArtifactId, Option>)>, event_reports: EventReportMap, }, diff --git a/wicket/src/state/inventory.rs b/wicket/src/state/inventory.rs index 5cfe536dfbe..ac692ccd032 100644 --- a/wicket/src/state/inventory.rs +++ b/wicket/src/state/inventory.rs @@ -139,6 +139,13 @@ fn version_or_unknown(caboose: Option<&SpComponentCaboose>) -> String { caboose.map(|c| c.version.as_str()).unwrap_or("UNKNOWN").to_string() } +fn caboose_sign(caboose: Option<&SpComponentCaboose>) -> Option> { + match caboose { + None => None, + Some(c) => c.sign.as_ref().map(|x| Vec::from(x.as_bytes())), + } +} + impl Component { pub fn sp(&self) -> &Sp { match self { @@ -171,6 +178,29 @@ impl Component { self.sp().rot.as_ref().and_then(|rot| rot.caboose_b.as_ref()), ) } + + // Technically the slots could have different SIGN values in the + // caboose. An active slot implies the RoT is up and valid so + // we should rely on that value for selection + pub fn rot_sign(&self) -> Option> { + match self.rot_active_slot() { + None => return None, + Some(s) => match s { + RotSlot::A => caboose_sign( + self.sp() + .rot + .as_ref() + .and_then(|rot| rot.caboose_a.as_ref()), + ), + RotSlot::B => caboose_sign( + self.sp() + .rot + .as_ref() + .and_then(|rot| rot.caboose_b.as_ref()), + ), + }, + } + } } /// The component type and its slot. diff --git a/wicket/src/state/mod.rs b/wicket/src/state/mod.rs index d287a153a90..866de5e2b64 100644 --- a/wicket/src/state/mod.rs +++ b/wicket/src/state/mod.rs @@ -17,7 +17,7 @@ pub use inventory::{ pub use rack::{KnightRiderMode, RackState}; pub use status::ServiceStatus; pub use update::{ - parse_event_report_map, update_component_title, + parse_event_report_map, update_component_title, ArtifactVersion, CreateClearUpdateStateOptions, CreateStartUpdateOptions, RackUpdateState, UpdateItemState, }; diff --git a/wicket/src/state/update.rs b/wicket/src/state/update.rs index 77bbdd83d2d..d73818e49d4 100644 --- a/wicket/src/state/update.rs +++ b/wicket/src/state/update.rs @@ -22,12 +22,18 @@ use wicketd_client::types::{ ArtifactId, ClearUpdateStateOptions, SemverVersion, StartUpdateOptions, }; +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ArtifactVersion { + pub version: SemverVersion, + pub sign: Option>, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct RackUpdateState { pub items: BTreeMap, pub system_version: Option, - pub artifacts: Vec, - pub artifact_versions: BTreeMap, + pub artifacts: Vec<(ArtifactId, Option>)>, + pub artifact_versions: BTreeMap>, // The update item currently selected is recorded in // state.rack_state.selected. pub status_view_displayed: bool, @@ -94,15 +100,26 @@ impl RackUpdateState { &mut self, logger: &Logger, system_version: Option, - artifacts: Vec, + artifacts: Vec<(ArtifactId, Option>)>, reports: EventReportMap, ) { self.system_version = system_version; self.artifacts = artifacts; self.artifact_versions.clear(); - for id in &mut self.artifacts { + for (id, s) in &mut self.artifacts { if let Ok(known) = id.kind.parse() { - self.artifact_versions.insert(known, id.version.clone()); + self.artifact_versions + .entry(known) + .and_modify(|x| { + x.push(ArtifactVersion { + version: id.version.clone(), + sign: s.clone(), + }) + }) + .or_insert(vec![ArtifactVersion { + version: id.version.clone(), + sign: s.clone(), + }]); } } diff --git a/wicket/src/ui/panes/overview.rs b/wicket/src/ui/panes/overview.rs index f2d4d4a7aba..05f5841484d 100644 --- a/wicket/src/ui/panes/overview.rs +++ b/wicket/src/ui/panes/overview.rs @@ -882,6 +882,7 @@ fn append_caboose( git_commit, // Currently `name` is always the same as `board`, so we'll skip it. name: _, + sign, version, } = caboose; let label_style = style::text_label(); @@ -903,6 +904,16 @@ fn append_caboose( ] .into(), ); + if let Some(s) = sign { + spans.push( + vec![ + prefix.clone(), + Span::styled("Sign Hash: ", label_style), + Span::styled(s.clone(), ok_style), + ] + .into(), + ); + } let mut version_spans = vec![prefix.clone(), Span::styled("Version: ", label_style)]; version_spans.push(Span::styled(version, ok_style)); diff --git a/wicket/src/ui/panes/update.rs b/wicket/src/ui/panes/update.rs index 664c647eaca..de8af491f04 100644 --- a/wicket/src/ui/panes/update.rs +++ b/wicket/src/ui/panes/update.rs @@ -8,8 +8,8 @@ use std::collections::BTreeMap; use super::{align_by, help_text, push_text_lines, Control, PendingScroll}; use crate::keymap::ShowPopupCmd; use crate::state::{ - update_component_title, ComponentId, Inventory, UpdateItemState, - ALL_COMPONENT_IDS, + update_component_title, ArtifactVersion, ComponentId, Inventory, + UpdateItemState, ALL_COMPONENT_IDS, }; use crate::ui::defaults::style; use crate::ui::widgets::{ @@ -37,7 +37,7 @@ use wicket_common::update_events::{ EventBuffer, EventReport, ProgressEvent, StepOutcome, StepStatus, UpdateComponent, }; -use wicketd_client::types::{RotSlot, SemverVersion}; +use wicketd_client::types::RotSlot; const MAX_COLUMN_WIDTH: u16 = 25; @@ -841,8 +841,9 @@ impl UpdatePane { let children: Vec<_> = states .iter() .flat_map(|(component, s)| { - let target_version = - artifact_version(id, component, &versions); + let target_version = artifact_version( + id, component, &versions, inventory, + ); let installed_versions = all_installed_versions(id, component, inventory); installed_versions.into_iter().map(move |v| { @@ -1454,6 +1455,7 @@ impl UpdatePane { &state.rack_state.selected, component, versions, + inventory, ); let installed_versions = all_installed_versions( &state.rack_state.selected, @@ -1733,7 +1735,7 @@ impl From<&'_ State> for ForceUpdateSelectionState { } let artifact_version = - artifact_version(&component_id, component, versions); + artifact_version(&component_id, component, versions, inventory); let installed_version = active_installed_version(&component_id, component, inventory); match component { @@ -2297,29 +2299,32 @@ fn all_installed_versions( fn artifact_version( id: &ComponentId, - component: UpdateComponent, - versions: &BTreeMap, + update_component: UpdateComponent, + versions: &BTreeMap>, + inventory: &Inventory, ) -> String { - let artifact = match (id, component) { + let (artifact, multiple) = match (id, update_component) { (ComponentId::Sled(_), UpdateComponent::Rot) => { - KnownArtifactKind::GimletRot + (KnownArtifactKind::GimletRot, true) } (ComponentId::Sled(_), UpdateComponent::Sp) => { - KnownArtifactKind::GimletSp + (KnownArtifactKind::GimletSp, false) } (ComponentId::Sled(_), UpdateComponent::Host) => { - KnownArtifactKind::Host + (KnownArtifactKind::Host, false) } (ComponentId::Switch(_), UpdateComponent::Rot) => { - KnownArtifactKind::SwitchRot + (KnownArtifactKind::SwitchRot, true) } (ComponentId::Switch(_), UpdateComponent::Sp) => { - KnownArtifactKind::SwitchSp + (KnownArtifactKind::SwitchSp, false) } (ComponentId::Psc(_), UpdateComponent::Rot) => { - KnownArtifactKind::PscRot + (KnownArtifactKind::PscRot, true) + } + (ComponentId::Psc(_), UpdateComponent::Sp) => { + (KnownArtifactKind::PscSp, false) } - (ComponentId::Psc(_), UpdateComponent::Sp) => KnownArtifactKind::PscSp, // Switches and PSCs do not have a host. (ComponentId::Switch(_), UpdateComponent::Host) @@ -2327,10 +2332,41 @@ fn artifact_version( return "N/A".to_string() } }; - versions - .get(&artifact) - .cloned() - .map_or_else(|| "UNKNOWN".to_string(), |v| v.to_string()) + match versions.get(&artifact) { + None => "UNKNOWN".to_string(), + Some(v) => { + let component = match inventory.get_inventory(id) { + Some(c) => c, + None => return "UNKNOWN".to_string(), + }; + let cnt = v.len(); + for ArtifactVersion { version: vers, sign } in v { + match (sign, component.rot_sign()) { + // This matches SP components and old RoT repos + (None, None) => return vers.to_string(), + // if we have a version that's tagged with sign data but + // we can't read from the caboose check if we can fall + // back to just returning the version + (Some(_), None) => { + if multiple && cnt > 1 { + return "UNKNOWN (MISSING SIGN)".to_string(); + } else { + return vers.to_string(); + } + } + // If something isn't tagged with a sign just + // pass on the version + (None, Some(_)) => return vers.to_string(), + (Some(s), Some(c)) => { + if *s == c { + return vers.to_string(); + } + } + } + } + "NO MATCH".to_string() + } + } } impl Control for UpdatePane { diff --git a/wicket/src/wicketd.rs b/wicket/src/wicketd.rs index c0ee3d9b142..ec4c75b5e06 100644 --- a/wicket/src/wicketd.rs +++ b/wicket/src/wicketd.rs @@ -449,7 +449,9 @@ impl WicketdManager { let artifacts = rsp .artifacts .into_iter() - .map(|artifact| artifact.artifact_id) + .map(|artifact| { + (artifact.artifact_id, artifact.sign) + }) .collect(); let system_version = rsp.system_version; let event_reports: EventReportMap = rsp.event_reports; diff --git a/wicketd/src/artifacts/store.rs b/wicketd/src/artifacts/store.rs index 01543432a23..2b47dc068ae 100644 --- a/wicketd/src/artifacts/store.rs +++ b/wicketd/src/artifacts/store.rs @@ -44,12 +44,14 @@ impl WicketdArtifactStore { let artifacts = self.artifacts_with_plan.lock().unwrap(); let artifacts = artifacts.as_ref()?; let system_version = artifacts.plan().system_version.clone(); + let artifact_ids = artifacts .by_id() .iter() .map(|(k, v)| InstallableArtifacts { artifact_id: k.clone(), installable: v.clone(), + sign: artifacts.rot_by_sign().get(&k).cloned(), }) .collect(); Some((system_version, artifact_ids)) diff --git a/wicketd/src/http_entrypoints.rs b/wicketd/src/http_entrypoints.rs index 999428ff06a..a3ef3e8e2f6 100644 --- a/wicketd/src/http_entrypoints.rs +++ b/wicketd/src/http_entrypoints.rs @@ -637,6 +637,7 @@ async fn put_repository( pub struct InstallableArtifacts { pub artifact_id: ArtifactId, pub installable: Vec, + pub sign: Option>, } /// The response to a `get_artifacts` call: the system version, and the list of