From 01b1fc330dd78c835736ef8896aa17189e391710 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 17 Mar 2025 15:24:43 -0600 Subject: [PATCH 1/2] Don't allow downgrading the system target release Attempting to set the target system release to an earlier version should fail. That constraint is now enforced and tested. --- nexus/src/external_api/http_entrypoints.rs | 20 ++++- .../tests/integration_tests/target_release.rs | 53 ++++++++++- update-common/manifests/fake0.toml | 87 +++++++++++++++++++ 3 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 update-common/manifests/fake0.toml diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 40da27505e7..f59d3cd5283 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6442,14 +6442,30 @@ impl NexusExternalApi for NexusExternalApiImpl { crate::context::op_context_for_external_api(&rqctx).await?; let params = body.into_inner(); let system_version = params.system_version; + + let current_target_release = + nexus.datastore().target_release_get_current(&opctx).await?; + let view = nexus + .datastore() + .target_release_view(&opctx, ¤t_target_release) + .await?; + if let views::TargetReleaseSource::SystemVersion { version } = + view.release_source + { + if version > system_version { + return Err(HttpError::for_bad_request( + None, + "can't downgrade system".into(), + )); + } + } + let tuf_repo_id = nexus .datastore() .update_tuf_repo_get(&opctx, system_version.into()) .await? .repo .id; - let current_target_release = - nexus.datastore().target_release_get_current(&opctx).await?; let next_target_release = nexus_db_model::TargetRelease::new_system_version( ¤t_target_release, diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index abcd63fb9b0..64f110018d6 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -80,7 +80,7 @@ async fn get_set_target_release() -> anyhow::Result<()> { .expect("can't parse tufaceous args") .exec(&logctx.log) .await - .expect("can't assemble TUF repo"); + .expect("can't assemble version 1 TUF repo"); assert_eq!( system_version, @@ -130,6 +130,57 @@ async fn get_set_target_release() -> anyhow::Result<()> { TargetReleaseSource::SystemVersion { version: system_version }, ); + // Attempting to downgrade to an earlier system version (1.0.0 → 0.0.0) should fail. + let system_version = Version::new(0, 0, 0); + tufaceous::Args::try_parse_from([ + "tufaceous", + "assemble", + "../update-common/manifests/fake0.toml", + path.as_str(), + ]) + .expect("can't parse tufaceous args") + .exec(&logctx.log) + .await + .expect("can't assemble version 0 TUF repo"); + + assert_eq!( + system_version, + NexusRequest::new( + RequestBuilder::new( + client, + http::Method::PUT, + "/v1/system/update/repository?file_name=/tmp/foo.zip", + ) + .body_file(Some(&path)) + .expect_status(Some(http::StatusCode::OK)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap() + .recorded + .repo + .system_version + ); + + NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + "/v1/system/update/target-release", + ) + .body(Some(&SetTargetReleaseParams { + system_version: system_version.clone(), + })) + .expect_status(Some(StatusCode::CREATED)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect_err("shouldn't be able to downgrade"); + ctx.teardown().await; logctx.cleanup_successful(); Ok(()) diff --git a/update-common/manifests/fake0.toml b/update-common/manifests/fake0.toml new file mode 100644 index 00000000000..e8e80233cb4 --- /dev/null +++ b/update-common/manifests/fake0.toml @@ -0,0 +1,87 @@ +# This is an artifact manifest that generates fake entries for all components. +# This is completely non-functional and is only useful for testing archive +# extraction in other parts of the repository. + +system_version = "0.0.0" + +[[artifact.gimlet_sp]] +name = "fake-gimlet-sp" +version = "0.0.0" +source = { kind = "fake", size = "1MiB" } + +[[artifact.gimlet_rot]] +name = "fake-gimlet-rot" +version = "0.0.0" +[artifact.gimlet_rot.source] +kind = "composite-rot" +archive_a = { kind = "fake", size = "512KiB" } +archive_b = { kind = "fake", size = "512KiB" } + +[[artifact.host]] +name = "fake-host" +version = "0.0.0" +[artifact.host.source] +kind = "composite-host" +phase_1 = { kind = "fake", size = "512KiB" } +phase_2 = { kind = "fake", size = "1MiB" } + +[[artifact.trampoline]] +name = "fake-trampoline" +version = "0.0.0" +[artifact.trampoline.source] +kind = "composite-host" +phase_1 = { kind = "fake", size = "512KiB" } +phase_2 = { kind = "fake", size = "1MiB" } + +[[artifact.control_plane]] +name = "fake-control-plane" +version = "0.0.0" +[artifact.control_plane.source] +kind = "composite-control-plane" +zones = [ + { kind = "fake", name = "zone1", size = "1MiB" }, + { kind = "fake", name = "zone2", size = "1MiB" }, +] + +[[artifact.psc_sp]] +name = "fake-psc-sp" +version = "0.0.0" +source = { kind = "fake", size = "1MiB" } + +[[artifact.psc_rot]] +name = "fake-psc-rot" +version = "0.0.0" +[artifact.psc_rot.source] +kind = "composite-rot" +archive_a = { kind = "fake", size = "512KiB" } +archive_b = { kind = "fake", size = "512KiB" } + +[[artifact.switch_sp]] +name = "fake-switch-sp" +version = "0.0.0" +source = { kind = "fake", size = "1MiB" } + +[[artifact.switch_rot]] +name = "fake-switch-rot" +version = "0.0.0" +[artifact.switch_rot.source] +kind = "composite-rot" +archive_a = { kind = "fake", size = "512KiB" } +archive_b = { kind = "fake", size = "512KiB" } + +[[artifact.gimlet_rot_bootloader]] +name = "fake-gimlet-rot-bootloader" +version = "0.0.0" +source = { kind = "fake", size = "1MiB" } + +[[artifact.psc_rot_bootloader]] +name = "fake-psc-rot-bootloader" +version = "0.0.0" +source = { kind = "fake", size = "1MiB" } + +[[artifact.switch_rot_bootloader]] +name = "fake-switch-rot-bootloader" +version = "0.0.0" +source = { kind = "fake", size = "1MiB" } + + From c2233b9416fade311f4d1984aa3df2f786549aee Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Sat, 22 Mar 2025 13:03:44 -0600 Subject: [PATCH 2/2] Better comments & error message --- nexus/src/external_api/http_entrypoints.rs | 25 +++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index beee5e6d035..483af422ab9 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6443,23 +6443,37 @@ impl NexusExternalApi for NexusExternalApiImpl { let params = body.into_inner(); let system_version = params.system_version; + // We don't need a transaction for the following queries because + // (1) the generation numbers provide optimistic concurrency control: + // if another request were to successfully update the target release + // between when we fetch it here and when we try to update it below, + // our update would fail because the next generation number would + // would already be taken; and + // (2) we assume that TUF repo depot records are immutable, i.e., + // system version X.Y.Z won't designate different repos over time. let current_target_release = nexus.datastore().target_release_get_current(&opctx).await?; - let view = nexus + + // Disallow downgrades. + if let views::TargetReleaseSource::SystemVersion { version } = nexus .datastore() .target_release_view(&opctx, ¤t_target_release) - .await?; - if let views::TargetReleaseSource::SystemVersion { version } = - view.release_source + .await? + .release_source { if version > system_version { return Err(HttpError::for_bad_request( None, - "can't downgrade system".into(), + format!( + "The requested target system release ({system_version}) \ + is older than the current target system release ({version}). \ + This is not supported." + ), )); } } + // Fetch the TUF repo metadata and update the target release. let tuf_repo_id = nexus .datastore() .update_tuf_repo_get(&opctx, system_version.into()) @@ -6475,6 +6489,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .datastore() .target_release_insert(&opctx, next_target_release) .await?; + Ok(HttpResponseCreated( nexus .datastore()