diff --git a/rs/registry/canister/src/invariants/hostos_version.rs b/rs/registry/canister/src/invariants/hostos_version.rs index 72a6faa645a1..31040a2407ba 100644 --- a/rs/registry/canister/src/invariants/hostos_version.rs +++ b/rs/registry/canister/src/invariants/hostos_version.rs @@ -5,6 +5,7 @@ use crate::invariants::common::{ use ic_protobuf::registry::hostos_version::v1::HostosVersionRecord; use ic_registry_keys::make_hostos_version_key; +use ic_types::hostos_version::HostosVersion; /// A predicate on the HostOS version records contained in a registry /// snapshot. @@ -13,6 +14,7 @@ use ic_registry_keys::make_hostos_version_key; /// or that is contained in a HostosVersionRecord, the following is checked: /// /// * The corresponding HostosVersionRecord exists. +/// * The version ID is well-formed, i.e. it can be parsed as a HostosVersion. /// * Each set URL is well-formed. /// * Each set hash is a well-formed hex-encoded SHA256 value. pub(crate) fn check_hostos_version_invariants( @@ -31,6 +33,12 @@ pub(crate) fn check_hostos_version_invariants( all_versions.dedup(); for version in all_versions { + // Enforce that the version ID is well-formed, so that consumers reading + // it back out of the Registry can turn it into a HostosVersion. + if let Err(err) = HostosVersion::try_from(version.as_str()) { + panic!("Registered an invalid HostOS version ID: {err}"); + } + // Check that every referenced version exists, i.e. we can only set a // Node's version to one that has already been added to the registry. let r = get_hostos_version_record(snapshot, version); @@ -60,3 +68,47 @@ fn get_all_hostos_versions_of_nodes(snapshot: &RegistrySnapshot) -> Vec .filter_map(|node_record| node_record.hostos_version_id) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + + use crate::common::test_helpers::invariant_compliant_registry; + use ic_registry_transport::{insert, pb::v1::RegistryMutation}; + use prost::Message; + + // HostOS version IDs are git commit IDs (pointing to the source code used + // to build HostOS). + const HOSTOS_VERSION_ID: &str = "eb3ab997954f2a91db8a42f84132cf37078d481c"; + + fn register_version_mutation(hostos_version_id: &str) -> Vec { + vec![insert( + make_hostos_version_key(hostos_version_id).as_bytes(), + HostosVersionRecord { + release_package_urls: vec![], + release_package_sha256_hex: "".to_string(), + hostos_version_id: hostos_version_id.to_string(), + } + .encode_to_vec(), + )] + } + + #[test] + fn no_panic_when_registering_valid_version() { + let registry = invariant_compliant_registry(0); + + let mutations = register_version_mutation(HOSTOS_VERSION_ID); + + registry.check_global_state_invariants(&mutations); + } + + #[test] + #[should_panic(expected = "Registered an invalid HostOS version ID")] + fn panic_when_registering_version_with_illegal_characters() { + let registry = invariant_compliant_registry(0); + + let mutations = register_version_mutation("G@RBAGE"); + + registry.check_global_state_invariants(&mutations); + } +} diff --git a/rs/registry/canister/src/invariants/replica_version.rs b/rs/registry/canister/src/invariants/replica_version.rs index 70bfb41dcd81..b4e0c082b9f8 100644 --- a/rs/registry/canister/src/invariants/replica_version.rs +++ b/rs/registry/canister/src/invariants/replica_version.rs @@ -20,6 +20,7 @@ use ic_registry_keys::{ make_replica_version_key, make_standard_engine_replica_version_record_key, make_subnet_record_key, make_unassigned_nodes_config_record_key, }; +use ic_types::ReplicaVersion; use prost::Message; /// A predicate on the replica version records contained in a registry @@ -30,6 +31,7 @@ use prost::Message; /// or that is used by the unassigned nodes, the following is checked: /// /// * The corresponding ReplicaVersionRecord exists. +/// * The version ID is well-formed, i.e. it can be parsed as a ReplicaVersion. /// * Each URL is well-formed. /// * Release package hash is a well-formed hex-encoded SHA256 value. /// @@ -66,6 +68,12 @@ pub(crate) fn check_replica_version_invariants( ); for version in elected_set { + // Enforce that the version ID is well-formed, so that consumers reading + // it back out of the Registry can turn it into a ReplicaVersion. + if let Err(err) = ReplicaVersion::try_from(version.as_str()) { + panic!("Elected an invalid version ID: {err}"); + } + let r = get_replica_version_record(snapshot, &version); // Check whether release package URLs (update image) and corresponding hash are well-formed. @@ -195,7 +203,6 @@ mod tests { GuestLaunchMeasurement, GuestLaunchMeasurementMetadata, GuestLaunchMeasurements, }; use ic_registry_transport::{delete, insert, pb::v1::RegistryMutation, upsert}; - use ic_types::ReplicaVersion; use prost::Message; const MOCK_HASH: &str = "C0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEED00D"; @@ -244,6 +251,27 @@ mod tests { registry.check_global_state_invariants(&mutations); } + #[test] + #[should_panic(expected = "Elected an invalid version ID")] + fn panic_when_electing_version_with_illegal_characters() { + let registry = invariant_compliant_registry(0); + + let mutations = elect_version_mutations(vec!["G@RBAGE".into()]); + + registry.check_global_state_invariants(&mutations); + } + + #[test] + fn no_panic_when_electing_version_with_test_suffix() { + let registry = invariant_compliant_registry(0); + + // Version IDs like this are used by system-tests, so they must remain + // acceptable. + let mutations = elect_version_mutations(vec![format!("{REPLICA_VERSION_ID_1}-test")]); + + registry.check_global_state_invariants(&mutations); + } + #[test] #[should_panic(expected = "Using a version that isn't elected.")] fn panic_when_using_unelected_version() { diff --git a/rs/registry/canister/unreleased_changelog.md b/rs/registry/canister/unreleased_changelog.md index ce83d6e800f1..e723acaf78fc 100644 --- a/rs/registry/canister/unreleased_changelog.md +++ b/rs/registry/canister/unreleased_changelog.md @@ -12,6 +12,11 @@ on the process that this file is part of, see * Invariant requiring that SEV-enabled subnets may only run a GuestOS version that has `guest_launch_measurements`. +* Invariant requiring that every elected GuestOS and HostOS version ID is well-formed, + i.e. that it consists only of alphanumeric characters, dots, dashes and underscores. + Such IDs are what `ReplicaVersion` and `HostosVersion` accept, so until now, it was + possible to elect a version that consumers could not read back out of the Registry. + ## Changed ## Deprecated