Skip to content

Commit

Permalink
[wicketd] Don't check RoT CMPA/CFPA until we've decided to update it
Browse files Browse the repository at this point in the history
Fixes #4420, which is a bit of an edge case: if the TUF repo contains
RoT images with the same version as the target sled, but does not
contain an RoT image signed with a correct key for the target sled, we
would previously fail to mupdate the sled even if the user chose _not_
to update the RoT (since its version number already matched), due to
missing a correctly-signed image.

With this PR, we postpone the key check (implemented as fetching the
CMPA/CFPA pages to verify against each archive) until _after_ we've
decided whether or not to try updating the RoT at all.

An additional check added with this PR that is always true today with
TUF repos built in CI is that for each class of Hubris archive where we
accept multiple options (e.g., "gimlet SP" where we accept multiple
board revisions, "gimlet RoT", "sidecar RoT", etc.), we require all the
archives of that class to have the same version. We will still allow
SP images to have different versions than RoT images, and even allow
"gimlet RoT" images to have different versions of "sidecar RoT" images,
but we do not allow something like "gimlet RoT version 1.0.1 signed with
a dev key and gimlet RoT version 1.0.2 signed with a production key".
  • Loading branch information
jgallagher committed Nov 7, 2023
1 parent 1264772 commit 97d566f
Show file tree
Hide file tree
Showing 3 changed files with 268 additions and 176 deletions.
13 changes: 12 additions & 1 deletion wicketd/src/artifacts/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use camino::Utf8PathBuf;
use display_error_chain::DisplayErrorChain;
use dropshot::HttpError;
use omicron_common::api::external::SemverVersion;
use omicron_common::api::internal::nexus::KnownArtifactKind;
use omicron_common::update::{ArtifactHashId, ArtifactId, ArtifactKind};
use slog::error;
Expand Down Expand Up @@ -105,6 +106,15 @@ pub(super) enum RepositoryError {
#[error("missing artifact of kind `{0:?}`")]
MissingArtifactKind(KnownArtifactKind),

#[error(
"muliple versions present for artifact of kind `{kind:?}`: {v1}, {v2}"
)]
MultipleVersionsPresent {
kind: KnownArtifactKind,
v1: SemverVersion,
v2: SemverVersion,
},

#[error(
"duplicate hash entries found in artifacts.json for kind `{}`, hash `{}`", .0.kind, .0.hash
)]
Expand Down Expand Up @@ -134,7 +144,8 @@ impl RepositoryError {
| RepositoryError::ParsingHubrisArchive { .. }
| RepositoryError::ReadHubrisCaboose { .. }
| RepositoryError::ReadHubrisCabooseBoard { .. }
| RepositoryError::ReadHubrisCabooseBoardUtf8(_) => {
| RepositoryError::ReadHubrisCabooseBoardUtf8(_)
| RepositoryError::MultipleVersionsPresent { .. } => {
HttpError::for_bad_request(None, message)
}

Expand Down
53 changes: 53 additions & 0 deletions wicketd/src/artifacts/update_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,59 @@ impl<'a> UpdatePlanBuilder<'a> {
}
}

// Ensure that all A/B RoT images for each board kind have the same
// version number.
for (kind, mut single_board_rot_artifacts) in [
(
KnownArtifactKind::GimletRot,
self.gimlet_rot_a.iter().chain(&self.gimlet_rot_b),
),
(
KnownArtifactKind::PscRot,
self.psc_rot_a.iter().chain(&self.psc_rot_b),
),
(
KnownArtifactKind::SwitchRot,
self.sidecar_rot_a.iter().chain(&self.sidecar_rot_b),
),
] {
// We know each of these iterators has at least 2 elements (one from
// the A artifacts and one from the B artifacts, checked above) so
// we can safely unwrap the first.
let version =
&single_board_rot_artifacts.next().unwrap().id.version;
for artifact in single_board_rot_artifacts {
if artifact.id.version != *version {
return Err(RepositoryError::MultipleVersionsPresent {
kind,
v1: version.clone(),
v2: artifact.id.version.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 [
(KnownArtifactKind::GimletSp, self.gimlet_sp.values()),
(KnownArtifactKind::PscSp, self.psc_sp.values()),
(KnownArtifactKind::SwitchSp, self.sidecar_sp.values()),
] {
// We know each of these iterators has at least 1 element (checked
// above) so we can safely unwrap the first.
let version = &single_board_sp_artifacts.next().unwrap().id.version;
for artifact in single_board_sp_artifacts {
if artifact.id.version != *version {
return Err(RepositoryError::MultipleVersionsPresent {
kind,
v1: version.clone(),
v2: artifact.id.version.clone(),
});
}
}
}

Ok(UpdatePlan {
system_version: self.system_version,
gimlet_sp: self.gimlet_sp, // checked above
Expand Down
Loading

0 comments on commit 97d566f

Please sign in to comment.