From 45802a1de905ae6eb0c1fdbaa16c7a5a9554ff3a Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Tue, 18 Aug 2026 22:25:35 +0000 Subject: [PATCH 1/8] feat(system-tests): support API boundary node playnets on the local backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `with_api_boundary_nodes_playnet` gives API boundary nodes a domain name and a certificate for it that the nodes trust. Both halves came from Farm, so every test using it — and therefore every test with a cloud engine subnet, which requires it — was pinned to `backend = "farm"`. Stand up an equivalent locally: * Turn the group's `dnsmasq` into its DNS server. It already ran on the group bridge as an RA/DHCPv4 daemon with `--port=0`; dropping that and adding `--no-resolv --no-hosts` makes it a hermetic resolver that answers from an `--addn-hosts` file (new `LocalBackend::add_dns_record`) and from a `--synth-domain` mirroring the public `nip.io` wildcard service. GuestOS has no name-server knob and boots with `IPv6AcceptRA=no`, so rather than reconfiguring the guests, `create_group` assigns the four addresses GuestOS is hard-coded to query to the bridge. Inside the backend's own network namespace those addresses are free and no query can escape, so every node gets a working resolver without touching IC-OS. * Add `InternetComputer::setup_api_bn_local_playnet`, which issues an ephemeral CA plus a leaf covering the API boundary nodes' domains and registers those domains with the group's `dnsmasq`. `bootstrap` serves the leaf from `ic-boundary` through the existing `ic_boundary_tls_cert` mechanism. * Let the replica trust that CA. `nns_delegation_manager` built its root store from the compiled-in public roots only, which no test-issued certificate can satisfy. It now also honours `extra_api_boundary_node_trust_anchors_pem`, a new dev-only `GuestOSDevSettings` field that is unset in production, leaving the public roots as the only anchors there. Drops `backend = "farm"` from `canister_http_socks_test`, `cloud_engine_canister_sig_test`, `xnet_cloud_engine_isolation_test`, `nns_delegation_branch_nns_version_test` and `delete_subnet_test`. The `cpus` of `canister_http_socks_test` was understated: its comment omitted the four cloud engine nodes. Two tests stay on Farm for unrelated reasons, now recorded accurately: `nns_delegation_mainnet_nns_version_test` runs the mainnet GuestOS, whose replica predates the new field; and `firewall_correctness_test` asserts that port 8080 is closed between certain nodes, which the local backend cannot satisfy because `ic-prep` always adds 8080 to the `fd00::/8` rule the driver needs to reach the nodes. That test did assume the global firewall rule set starts out empty, which it does not on the local backend, so it now reads the current rules through the new `TopologySnapshot::firewall_rules`. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 1 + rs/config/src/http_handler.rs | 11 ++ .../src/nns_delegation_manager.rs | 39 +++- .../tool/src/guestos/generate_ic_config.rs | 15 ++ .../config/tool/templates/ic.json5.template | 1 + .../fixtures/guestos_v1.16.0.json | 53 ++++++ .../fixtures/hostos_v1.16.0.json | 53 ++++++ rs/ic_os/config/types/src/lib.rs | 12 +- rs/ic_os/networking/network/BUILD.bazel | 4 + rs/ic_os/networking/network/src/systemd.rs | 36 ++++ rs/orchestrator/src/firewall.rs | 1 + rs/tests/crypto/BUILD.bazel | 1 - rs/tests/driver/BUILD.bazel | 1 + rs/tests/driver/Cargo.toml | 1 + rs/tests/driver/src/driver/bootstrap.rs | 50 +++-- rs/tests/driver/src/driver/ic.rs | 151 +++++++++++++-- rs/tests/driver/src/driver/local_backend.rs | 173 +++++++++++++++--- rs/tests/driver/src/driver/test_env_api.rs | 22 ++- rs/tests/driver/src/util.rs | 1 + rs/tests/message_routing/xnet/BUILD.bazel | 1 - rs/tests/networking/BUILD.bazel | 22 +-- .../networking/canister_http_socks_test.rs | 26 ++- rs/tests/networking/firewall/BUILD.bazel | 9 +- .../firewall/firewall_correctness_test.rs | 15 +- rs/tests/nns/BUILD.bazel | 1 - 25 files changed, 613 insertions(+), 87 deletions(-) create mode 100644 rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.16.0.json create mode 100644 rs/ic_os/config/types/compatibility_tests/fixtures/hostos_v1.16.0.json diff --git a/Cargo.lock b/Cargo.lock index b3aab50c5c70..cfb3f9bcdf9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14486,6 +14486,7 @@ dependencies = [ "lazy_static", "libc", "macaddr", + "network", "nix 0.31.3", "num_cpus", "on_wire", diff --git a/rs/config/src/http_handler.rs b/rs/config/src/http_handler.rs index cd6ee368ce1d..16c27a391daf 100644 --- a/rs/config/src/http_handler.rs +++ b/rs/config/src/http_handler.rs @@ -66,6 +66,16 @@ pub struct Config { /// Serving at most `max_tracing_flamegraph_concurrent_requests` requests concurrently for all endpoints under `/_/tracing/flamegraph`. pub max_tracing_flamegraph_concurrent_requests: usize, + + /// PEM-encoded certificates to trust, in addition to the public roots + /// compiled into the replica, when connecting to an API boundary node. + /// + /// A cloud engine subnet fetches its NNS delegation from an API boundary + /// node over TLS. In a testnet that node's certificate comes from a + /// throw-away CA rather than a public one, so that CA has to be handed to + /// the replica. Left unset in production, where the public roots are the + /// only trust anchors. To be used in system tests only. + pub extra_api_boundary_node_trust_anchors_pem: Option, } impl Default for Config { @@ -87,6 +97,7 @@ impl Default for Config { max_pprof_concurrent_requests: 5, ingress_message_certificate_timeout_seconds: 10, max_tracing_flamegraph_concurrent_requests: 5, + extra_api_boundary_node_trust_anchors_pem: None, } } } diff --git a/rs/http_endpoints/nns_delegation_manager/src/nns_delegation_manager.rs b/rs/http_endpoints/nns_delegation_manager/src/nns_delegation_manager.rs index c538063af0a7..591ad89b92f7 100644 --- a/rs/http_endpoints/nns_delegation_manager/src/nns_delegation_manager.rs +++ b/rs/http_endpoints/nns_delegation_manager/src/nns_delegation_manager.rs @@ -38,7 +38,10 @@ use ic_types::{ time::expiry_time_from_now, }; use rand::{Rng, seq::SliceRandom}; -use rustls::{ClientConfig, pki_types::ServerName}; +use rustls::{ + ClientConfig, + pki_types::{CertificateDer, ServerName, pem::PemObject}, +}; use tokio::{ net::TcpStream, select, @@ -389,6 +392,7 @@ async fn try_fetch_delegation_from_nns( CONNECTION_TIMEOUT, connect( log.clone(), + config, rt_handle, subnet_type, nns_subnet_id, @@ -537,8 +541,36 @@ fn observe_delegation_sizes(builder: &NNSDelegationBuilder, metrics: &Delegation .observe(builder.flat_certificate_size_bytes() as f64); } +/// The trust anchors used to authenticate an API boundary node: the public roots +/// compiled into the replica, plus anything in `extra_anchors_pem`. +/// +/// `extra_anchors_pem` is unset in production. It exists for testnets, where the +/// API boundary node's certificate is issued by a throw-away CA rather than by a +/// public one; see [`Config::extra_api_boundary_node_trust_anchors_pem`]. +fn api_boundary_node_root_store( + extra_anchors_pem: Option<&str>, +) -> Result { + let mut root_store = + rustls::RootCertStore::from_iter(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); + + let Some(extra_anchors_pem) = extra_anchors_pem else { + return Ok(root_store); + }; + + for certificate in CertificateDer::pem_slice_iter(extra_anchors_pem.as_bytes()) { + let certificate = certificate + .map_err(|err| format!("Could not parse an extra API BN trust anchor: {err}"))?; + root_store + .add(certificate) + .map_err(|err| format!("Could not add an extra API BN trust anchor: {err}"))?; + } + + Ok(root_store) +} + async fn connect( log: ReplicaLogger, + config: &Config, rt_handle: &tokio::runtime::Handle, subnet_type: SubnetType, nns_subnet_id: SubnetId, @@ -604,8 +636,9 @@ async fn connect( let addr = SocketAddr::new(ip_addr, 443); - let root_store = - rustls::RootCertStore::from_iter(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); + let root_store = api_boundary_node_root_store( + config.extra_api_boundary_node_trust_anchors_pem.as_deref(), + )?; let tls_client_config = rustls::ClientConfig::builder() .with_root_certificates(root_store) .with_no_client_auth(); diff --git a/rs/ic_os/config/tool/src/guestos/generate_ic_config.rs b/rs/ic_os/config/tool/src/guestos/generate_ic_config.rs index 320d0b844fbb..403d6cc1c4b7 100644 --- a/rs/ic_os/config/tool/src/guestos/generate_ic_config.rs +++ b/rs/ic_os/config/tool/src/guestos/generate_ic_config.rs @@ -29,6 +29,8 @@ pub struct IcConfigTemplate { pub domain_name: String, pub node_reward_type: String, pub malicious_behavior: String, + /// Already JSON-encoded: either `null` or a quoted string. + pub extra_api_boundary_node_trust_anchors_pem: String, } /// Generate IC configuration from template and guestos config @@ -186,6 +188,18 @@ fn get_config_vars(guestos_config: &GuestOSConfig) -> Result { .map(|mb| serde_json::to_string(mb).unwrap_or_default()) .unwrap_or_default(); + let extra_api_boundary_node_trust_anchors_pem = match &guestos_config + .guestos_settings + .guestos_dev_settings + .extra_api_boundary_node_trust_anchors_pem + { + // A PEM spans several lines, so it has to be JSON-encoded rather than + // interpolated verbatim. + Some(pem) => serde_json::to_string(pem) + .context("Failed to encode the extra API boundary node trust anchors")?, + None => "null".to_string(), + }; + Ok(IcConfigTemplate { // TODO https://dfinity.atlassian.net/browse/NODE-1909 ipv6_prefix, @@ -199,6 +213,7 @@ fn get_config_vars(guestos_config: &GuestOSConfig) -> Result { domain_name, node_reward_type, malicious_behavior: with_default(malicious_behavior, "null"), + extra_api_boundary_node_trust_anchors_pem, }) } diff --git a/rs/ic_os/config/tool/templates/ic.json5.template b/rs/ic_os/config/tool/templates/ic.json5.template index 2beab67ca6e0..ede3e2e7dbc1 100644 --- a/rs/ic_os/config/tool/templates/ic.json5.template +++ b/rs/ic_os/config/tool/templates/ic.json5.template @@ -119,6 +119,7 @@ // ==================================== http_handler: { listen_addr: "[::]:8080", + extra_api_boundary_node_trust_anchors_pem: {{ extra_api_boundary_node_trust_anchors_pem }}, }, // ==================================== diff --git a/rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.16.0.json b/rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.16.0.json new file mode 100644 index 000000000000..f3f64e18c8cd --- /dev/null +++ b/rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.16.0.json @@ -0,0 +1,53 @@ +{ + "config_version": "1.16.0", + "network_settings": { + "ipv6_config": { + "Fixed": { + "address": "2a00:fb01:400:200::1/64", + "gateway": "2a00:fb01:400:200::1" + } + }, + "ipv4_config": { + "address": "192.168.1.1", + "gateway": "192.168.1.254", + "prefix_length": 24 + }, + "domain_name": "ic.test" + }, + "icos_settings": { + "node_reward_type": "type3.1", + "mgmt_mac": "00:00:00:00:00:01", + "deployment_environment": "mainnet", + "nns_urls": [ + "https://icp-api.io,https//icp0.io,https://ic0.app" + ], + "node_operator_private_key": null, + "enable_trusted_execution_environment": false, + "use_ssh_authorized_keys": false, + "icos_dev_settings": {} + }, + "guestos_settings": { + "guestos_dev_settings": { + "backup_spool": null, + "malicious_behavior": null, + "query_stats_epoch_length": null, + "bitcoind_addr": null, + "dogecoind_addr": null, + "jaeger_addr": null, + "socks_proxy": null, + "hostname": null, + "generate_ic_boundary_tls_cert": null, + "ic_boundary_tls_cert": null, + "extra_api_boundary_node_trust_anchors_pem": null, + "nns_pub_key_override": null + } + }, + "guest_vm_type": "default", + "upgrade_config": { + "peer_guest_vm_address": "2a00:fb01:400:200:6801:95ff:fed7:d475" + }, + "trusted_execution_environment_config": { + "sev_cert_chain_pem": "-----BEGIN CERTIFICATE----------END CERTIFICATE-----" + }, + "recovery_config": null +} \ No newline at end of file diff --git a/rs/ic_os/config/types/compatibility_tests/fixtures/hostos_v1.16.0.json b/rs/ic_os/config/types/compatibility_tests/fixtures/hostos_v1.16.0.json new file mode 100644 index 000000000000..2e73eb892e85 --- /dev/null +++ b/rs/ic_os/config/types/compatibility_tests/fixtures/hostos_v1.16.0.json @@ -0,0 +1,53 @@ +{ + "config_version": "1.16.0", + "network_settings": { + "ipv6_config": { + "Fixed": { + "address": "2a00:fb01:400:200::1/64", + "gateway": "2a00:fb01:400:200::1" + } + }, + "ipv4_config": { + "address": "192.168.1.1", + "gateway": "192.168.1.254", + "prefix_length": 24 + }, + "domain_name": "ic.test" + }, + "icos_settings": { + "node_reward_type": "type3.1", + "mgmt_mac": "00:00:00:00:00:01", + "deployment_environment": "mainnet", + "nns_urls": [ + "https://icp-api.io,https//icp0.io,https://ic0.app" + ], + "node_operator_private_key": null, + "enable_trusted_execution_environment": false, + "use_ssh_authorized_keys": false, + "icos_dev_settings": {} + }, + "hostos_settings": { + "hostos_dev_settings": { + "vm_memory": 16, + "vm_cpu": "kvm", + "vm_nr_of_vcpus": 64 + }, + "verbose": false + }, + "guestos_settings": { + "guestos_dev_settings": { + "backup_spool": null, + "malicious_behavior": null, + "query_stats_epoch_length": null, + "bitcoind_addr": null, + "dogecoind_addr": null, + "jaeger_addr": null, + "socks_proxy": null, + "hostname": null, + "generate_ic_boundary_tls_cert": null, + "ic_boundary_tls_cert": null, + "extra_api_boundary_node_trust_anchors_pem": null, + "nns_pub_key_override": null + } + } +} \ No newline at end of file diff --git a/rs/ic_os/config/types/src/lib.rs b/rs/ic_os/config/types/src/lib.rs index 4b68735016c4..8e141b544321 100644 --- a/rs/ic_os/config/types/src/lib.rs +++ b/rs/ic_os/config/types/src/lib.rs @@ -41,7 +41,7 @@ use std::str::FromStr; use strum::{Display, EnumString}; use url::Url; -pub const CONFIG_VERSION: &str = "1.15.0"; +pub const CONFIG_VERSION: &str = "1.16.0"; /// List of field paths that have been removed and should not be reused. pub static RESERVED_FIELD_PATHS: &[&str] = &[ @@ -249,6 +249,16 @@ pub struct GuestOSDevSettings { /// Pre-generated TLS certificate and key for ic-boundary. #[serde(default)] pub ic_boundary_tls_cert: Option, + /// PEM-encoded certificates to trust, in addition to the public roots + /// compiled into the replica, when connecting to an API boundary node. + /// + /// A cloud engine subnet fetches its NNS delegation from an API boundary + /// node over TLS. In a testnet that node's certificate comes from a + /// throw-away CA rather than a public one, so that CA has to be handed to + /// the replica. Left unset in production, where the public roots are the + /// only trust anchors. To be used in system tests only. + #[serde(default)] + pub extra_api_boundary_node_trust_anchors_pem: Option, /// PEM-encoded NNS public key. /// Overrides the hardcoded NNS public key on the rootfs. pub nns_pub_key_override: Option, diff --git a/rs/ic_os/networking/network/BUILD.bazel b/rs/ic_os/networking/network/BUILD.bazel index e71f7186da9c..3ab56f39f787 100644 --- a/rs/ic_os/networking/network/BUILD.bazel +++ b/rs/ic_os/networking/network/BUILD.bazel @@ -10,6 +10,10 @@ rust_library( aliases = {}, crate_name = "network", proc_macro_deps = [], + visibility = [ + "//rs:ic-os-pkg", + "//rs:system-tests-pkg", + ], deps = [ # Keep sorted. "//rs/ic_os/config/types:config_types", diff --git a/rs/ic_os/networking/network/src/systemd.rs b/rs/ic_os/networking/network/src/systemd.rs index d86484e4f5ab..8640fb11b914 100644 --- a/rs/ic_os/networking/network/src/systemd.rs +++ b/rs/ic_os/networking/network/src/systemd.rs @@ -12,6 +12,27 @@ use macaddr::MacAddr6; pub static DEFAULT_SYSTEMD_NETWORK_DIR: &str = "/run/systemd/network"; +/// The IPv6 name servers GuestOS is configured with (Cloudflare and Google). +/// +/// Kept in sync with [`IPV6_NAME_SERVER_NETWORKD_CONTENTS`] by a unit test. +/// +/// The system-test local backend depends on this being *the* set of addresses a +/// GuestOS node sends its DNS queries to: it assigns them to the test group's +/// bridge inside its isolated network namespace and answers on them, which is +/// how nodes get a resolver without any node-side configuration. See +/// `LocalBackend::create_group` in +/// `rs/tests/driver/src/driver/local_backend.rs`. +pub const IPV6_NAME_SERVERS: [Ipv6Addr; 4] = [ + // 2606:4700:4700::1111 + Ipv6Addr::new(0x2606, 0x4700, 0x4700, 0, 0, 0, 0, 0x1111), + // 2606:4700:4700::1001 + Ipv6Addr::new(0x2606, 0x4700, 0x4700, 0, 0, 0, 0, 0x1001), + // 2001:4860:4860::8888 + Ipv6Addr::new(0x2001, 0x4860, 0x4860, 0, 0, 0, 0, 0x8888), + // 2001:4860:4860::8844 + Ipv6Addr::new(0x2001, 0x4860, 0x4860, 0, 0, 0, 0, 0x8844), +]; + pub const IPV6_NAME_SERVER_NETWORKD_CONTENTS: &str = r#" DNS=2606:4700:4700::1111 DNS=2606:4700:4700::1001 @@ -157,3 +178,18 @@ fn generate_and_write_systemd_files( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_ipv6_name_servers_match_networkd_contents() { + let rendered: String = IPV6_NAME_SERVERS + .iter() + .map(|name_server| format!("DNS={name_server}\n")) + .collect(); + + assert_eq!(IPV6_NAME_SERVER_NETWORKD_CONTENTS, format!("\n{rendered}")); + } +} diff --git a/rs/orchestrator/src/firewall.rs b/rs/orchestrator/src/firewall.rs index 81c6f1d6011d..9e9be4810e48 100644 --- a/rs/orchestrator/src/firewall.rs +++ b/rs/orchestrator/src/firewall.rs @@ -1404,6 +1404,7 @@ mod tests { domain_name: "".to_string(), node_reward_type: "".to_string(), malicious_behavior: "null".to_string(), + extra_api_boundary_node_trust_anchors_pem: "null".to_string(), }; let ic_json = generate_ic_config::render_ic_config(template) diff --git a/rs/tests/crypto/BUILD.bazel b/rs/tests/crypto/BUILD.bazel index 47b86406824f..2d4fb156ea27 100644 --- a/rs/tests/crypto/BUILD.bazel +++ b/rs/tests/crypto/BUILD.bazel @@ -392,7 +392,6 @@ system_test( system_test( name = "cloud_engine_canister_sig_test", - backend = "farm", # Requires API boundary node playnet (with_api_boundary_nodes_playnet), unsupported on the local backend. cpus = MIN_LOCAL_CPUS + 4 * DEFAULT_VCPUS_PER_VM, # 3 IC Node VMs + 1 API boundary node VM * 6 vCPUs. runtime_deps = UNIVERSAL_CANISTER_RUNTIME_DEPS | { "II_BACKEND_WASM_PATH": "@mainnet_canisters//:internet_identity_backend.wasm.gz", diff --git a/rs/tests/driver/BUILD.bazel b/rs/tests/driver/BUILD.bazel index c3b38d348871..c227f3b21a54 100644 --- a/rs/tests/driver/BUILD.bazel +++ b/rs/tests/driver/BUILD.bazel @@ -38,6 +38,7 @@ rust_library( "//rs/ic_os/dev_test_tools/bare_metal_deployment", "//rs/ic_os/dev_test_tools/setupos-image-config", "//rs/ic_os/networking/deterministic_ips", + "//rs/ic_os/networking/network", "//rs/interfaces/registry", "//rs/ledger_suite/common/ledger_core", "//rs/ledger_suite/icp:icp_ledger", diff --git a/rs/tests/driver/Cargo.toml b/rs/tests/driver/Cargo.toml index 8bb4c04ceaab..f8876a56e3bf 100644 --- a/rs/tests/driver/Cargo.toml +++ b/rs/tests/driver/Cargo.toml @@ -85,6 +85,7 @@ itertools = { workspace = true } lazy_static = { workspace = true } libc = { workspace = true } macaddr = { workspace = true } +network = { path = "../../ic_os/networking/network" } nix = { workspace = true } num_cpus = { workspace = true } on_wire = { path = "../../rust_canisters/on_wire" } diff --git a/rs/tests/driver/src/driver/bootstrap.rs b/rs/tests/driver/src/driver/bootstrap.rs index 86c41b1e2545..97d7e02e648c 100644 --- a/rs/tests/driver/src/driver/bootstrap.rs +++ b/rs/tests/driver/src/driver/bootstrap.rs @@ -11,7 +11,7 @@ use crate::driver::{ constants::SSH_USERNAME, driver_setup::{SSH_AUTHORIZED_PRIV_KEYS_DIR, SSH_AUTHORIZED_PUB_KEYS_DIR}, farm::{AttachImageSpec, Farm, FarmResult, FileId}, - ic::{InternetComputer, Node}, + ic::{InternetComputer, LocalApiBoundaryNodesPlaynet, Node}, nested::{HasNestedVms, NESTED_CONFIG_IMAGE_PATH, UnassignedRecordConfig}, node_software_version::NodeSoftwareVersion, port_allocator::AddrType, @@ -290,16 +290,35 @@ pub fn setup_and_start_vms( for node in initialized_ic.api_boundary_nodes.values() { nodes.push(node.clone()); } - let api_bn_tls_cert: Option = if ic.api_bn_use_playnet { - let playnet = Playnet::read_attribute(env); - let cert = &playnet.playnet_cert.cert; - Some(IcBoundaryTlsCert { - cert_pem: format!("{}{}", cert.cert_pem, cert.chain_pem), - key_pem: cert.priv_key_pem.clone(), - }) - } else { - None - }; + // The API boundary nodes' TLS certificate, and — when it was issued by a CA + // the nodes do not already trust — that CA, which every node in the group + // then gets as an extra trust anchor. Farm's playnet certificate is publicly + // trusted, so only the local backend needs the second half. See + // `InternetComputer::setup_api_bn_local_playnet`. + let (api_bn_tls_cert, api_bn_trust_anchors_pem): (Option, Option) = + match ( + ic.api_bn_use_playnet, + SystemTestBackend::read_attribute(env), + ) { + (false, _) => (None, None), + (true, SystemTestBackend::Farm) => { + let playnet = Playnet::read_attribute(env); + let cert = &playnet.playnet_cert.cert; + let tls_cert = IcBoundaryTlsCert { + cert_pem: format!("{}{}", cert.cert_pem, cert.chain_pem), + key_pem: cert.priv_key_pem.clone(), + }; + (Some(tls_cert), None) + } + (true, SystemTestBackend::Local) => { + let playnet = LocalApiBoundaryNodesPlaynet::read_attribute(env); + let tls_cert = IcBoundaryTlsCert { + cert_pem: format!("{}{}", playnet.cert_pem, playnet.ca_pem), + key_pem: playnet.key_pem.clone(), + }; + (Some(tls_cert), Some(playnet.ca_pem)) + } + }; let api_bn_node_ids: Vec = initialized_ic .api_boundary_nodes .values() @@ -324,6 +343,10 @@ pub fn setup_and_start_vms( } else { None }; + // Given to every node, not just the API boundary nodes: it is the + // *clients* of an API boundary node — the cloud engine replicas fetching + // their NNS delegation — that need to trust its certificate. + let api_bn_trust_anchors_pem = api_bn_trust_anchors_pem.clone(); nodes_info.insert(node.node_id, malicious_behavior.clone()); join_handles.push(thread::spawn(move || { create_config_disk_image( @@ -335,6 +358,7 @@ pub fn setup_and_start_vms( domain, recovery_hash, ic_boundary_tls_cert, + api_bn_trust_anchors_pem, &t_env, )?; @@ -516,6 +540,7 @@ fn create_config_disk_image( domain_name: Option, recovery_hash: Option, ic_boundary_tls_cert: Option, + api_bn_trust_anchors_pem: Option, test_env: &TestEnv, ) -> anyhow::Result<()> { let mut bootstrap_options = BootstrapOptions { @@ -538,6 +563,7 @@ fn create_config_disk_image( domain_name, recovery_hash, ic_boundary_tls_cert, + api_bn_trust_anchors_pem, test_env, ic_name, )?; @@ -582,6 +608,7 @@ fn create_guestos_config_for_node( domain_name: Option, recovery_hash: Option, ic_boundary_tls_cert: Option, + api_bn_trust_anchors_pem: Option, test_env: &TestEnv, ic_name: &str, ) -> anyhow::Result { @@ -676,6 +703,7 @@ fn create_guestos_config_for_node( hostname: Some(node.node_id.to_string()), generate_ic_boundary_tls_cert: node.node_config.domain.clone(), ic_boundary_tls_cert, + extra_api_boundary_node_trust_anchors_pem: api_bn_trust_anchors_pem, nns_pub_key_override, }; diff --git a/rs/tests/driver/src/driver/ic.rs b/rs/tests/driver/src/driver/ic.rs index 9d0960468e5f..c9d72ddbe3db 100644 --- a/rs/tests/driver/src/driver/ic.rs +++ b/rs/tests/driver/src/driver/ic.rs @@ -3,6 +3,7 @@ use crate::driver::{ bootstrap::{init_ic, setup_and_start_vms}, farm::{DnsRecord, DnsRecordType, Farm, HostFeature}, ic_gateway_vm::Playnet, + local_backend::LocalBackend, nested::UnassignedRecordConfig, node_software_version::NodeSoftwareVersion, resource::{AllocatedVm, ResourceGroup, allocate_resources, get_resource_request}, @@ -13,7 +14,7 @@ use crate::driver::{ }, test_setup::{GroupSetup, SystemTestBackend}, }; -use anyhow::Result; +use anyhow::{Context, Result}; use ic_prep_lib::prep_state_directory::IcPrepStateDir; use ic_prep_lib::{node::NodeSecretKeyStore, subnet_configuration::SubnetRunningState}; use ic_protobuf::registry::{dc::v1::DataCenterRecord, node::v1::NodeRewardType}; @@ -26,15 +27,55 @@ use ic_types::malicious_behavior::MaliciousBehavior; use ic_types::{Height, NodeId, PrincipalId}; use ic_types_cycles::CanisterCyclesCostSchedule; use phantom_newtype::AmountOf; +use rcgen::{BasicConstraints, CertificateParams, DnType, IsCa, KeyPair, KeyUsagePurpose}; use serde::{Deserialize, Serialize}; use slog::info; use std::collections::BTreeMap; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; -use std::net::{Ipv6Addr, SocketAddr}; +use std::net::{IpAddr, Ipv6Addr, SocketAddr}; use std::path::Path; use std::time::Duration; +/// Domain suffix of an API boundary node that is not part of a Farm playnet. +/// +/// Only ever resolved from inside a test group: on the local backend by the +/// group's own `dnsmasq` (see +/// [`InternetComputer::setup_api_bn_local_playnet`]), and on Farm not at all — +/// [`InternetComputer::with_api_boundary_nodes`] hands out these names without +/// creating DNS records for them. +const API_BOUNDARY_NODE_DOMAIN_SUFFIX: &str = "ic.net"; + +/// The domain of the `idx`-th API boundary node outside a Farm playnet. +fn api_boundary_node_domain(idx: usize) -> String { + format!("apibn-{idx}.{API_BOUNDARY_NODE_DOMAIN_SUFFIX}") +} + +/// The TLS material behind the local backend's replacement for a Farm playnet, +/// written by [`InternetComputer::setup_api_bn_local_playnet`] and read by +/// `bootstrap` when it builds each API boundary node's config image. +/// +/// Deliberately a separate attribute from [`Playnet`], which the IC gateway VM +/// also reads and writes — sharing it would let whichever ran last silently +/// replace the other's certificate. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct LocalApiBoundaryNodesPlaynet { + /// PEM-encoded certificate covering every API boundary node's domain, served + /// by their `ic-boundary`. + pub cert_pem: String, + /// PEM-encoded private key belonging to `cert_pem`. + pub key_pem: String, + /// PEM-encoded certificate of the ephemeral CA that issued `cert_pem`. The + /// nodes are given this as an extra trust anchor. + pub ca_pem: String, +} + +impl TestEnvAttribute for LocalApiBoundaryNodesPlaynet { + fn attribute_name() -> String { + "local_api_boundary_nodes_playnet".to_string() + } +} + /// Builder object to declare a topology of an InternetComputer. /// Used as input to the IC Manager. #[derive(Clone, Debug, Default)] @@ -168,16 +209,26 @@ impl InternetComputer { Node::new() .with_boot_image(BootImage::GroupDefault) .with_required_host_features(self.required_host_features.clone()) - .with_domain(format!("apibn-{idx}.ic.net")), + .with_domain(api_boundary_node_domain(idx)), ); } self } - /// Add the given number of API boundary nodes with playnet support. + /// Add the given number of API boundary nodes with playnet support, i.e. + /// with a domain name *and* a certificate for it that the nodes trust. + /// /// Unlike `with_api_boundary_nodes`, nodes are created without domains here — - /// real domains (e.g. `apibn-0.ic50.farm.dfinity.systems`) are assigned during - /// `setup_and_start` after a playnet certificate is acquired from Farm. + /// they are assigned during `setup_and_start`, together with a matching + /// certificate: on the Farm backend a real domain + /// (e.g. `apibn-0.ic50.farm.dfinity.systems`) with a Farm-issued publicly + /// trusted certificate, and on the local backend `apibn-0.ic.net` with a + /// certificate from an ephemeral per-group CA. See + /// [`setup_api_bn_playnet`](Self::setup_api_bn_playnet) and + /// [`setup_api_bn_local_playnet`](Self::setup_api_bn_local_playnet). + /// + /// This is required for cloud engine subnets, whose replicas reach an API + /// boundary node by domain name over TLS to fetch their NNS delegation. pub fn with_api_boundary_nodes_playnet(mut self, no_of_nodes: usize) -> Self { self.api_bn_use_playnet = true; for _ in 0..no_of_nodes { @@ -309,12 +360,7 @@ impl InternetComputer { if self.api_bn_use_playnet { match SystemTestBackend::read_attribute(env) { SystemTestBackend::Farm => self.setup_api_bn_playnet(env), - SystemTestBackend::Local => { - slog::warn!( - env.logger(), - "LocalBackend: skipping API BN playnet setup (no playnet DNS/TLS)" - ); - } + SystemTestBackend::Local => self.setup_api_bn_local_playnet(env, &group_name)?, } } @@ -420,6 +466,87 @@ impl InternetComputer { playnet.write_attribute(env); } + /// The local backend's replacement for + /// [`setup_api_bn_playnet`](Self::setup_api_bn_playnet). + /// + /// A Farm playnet gives the API boundary nodes a publicly resolvable domain + /// and a publicly trusted certificate for it. Neither service exists locally, + /// so both halves are produced here instead: + /// + /// * an ephemeral CA plus one leaf certificate covering every API boundary + /// node's domain, stored in the [`LocalApiBoundaryNodesPlaynet`] attribute. + /// `bootstrap` serves the leaf from `ic-boundary` through the existing + /// `ic_boundary_tls_cert` mechanism and hands the CA to the nodes as an + /// extra trust anchor; + /// * a DNS record per node in the group's `dnsmasq`, which is the resolver + /// every VM in the group queries (see [`LocalBackend::add_dns_record`]). + /// + /// Both halves are needed by cloud engine subnets, whose + /// `nns_delegation_manager` resolves an API boundary node's domain and then + /// validates its certificate. + fn setup_api_bn_local_playnet(&mut self, env: &TestEnv, group_name: &str) -> Result<()> { + let logger = env.logger(); + + for (idx, node) in self.api_boundary_nodes.iter_mut().enumerate() { + node.domain = Some(api_boundary_node_domain(idx)); + } + let domains: Vec = self + .api_boundary_nodes + .iter() + .map(|node| { + node.domain + .clone() + .expect("API BN domain was just assigned above") + }) + .collect(); + + let ca_key = KeyPair::generate().context("generating the API BN CA key")?; + let mut ca_params = CertificateParams::new(Vec::new()) + .context("building the API BN CA certificate parameters")?; + ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::CrlSign]; + ca_params + .distinguished_name + .push(DnType::CommonName, "IC system-test local API BN CA"); + let ca_cert = ca_params + .self_signed(&ca_key) + .context("self-signing the API BN CA certificate")?; + + let leaf_key = KeyPair::generate().context("generating the API BN certificate key")?; + let mut leaf_params = CertificateParams::new(domains.clone()) + .context("building the API BN certificate parameters")?; + leaf_params + .distinguished_name + .push(DnType::CommonName, domains[0].clone()); + let leaf_cert = leaf_params + .signed_by(&leaf_key, &ca_cert, &ca_key) + .context("signing the API BN certificate")?; + + LocalApiBoundaryNodesPlaynet { + cert_pem: leaf_cert.pem(), + key_pem: leaf_key.serialize_pem(), + ca_pem: ca_cert.pem(), + } + .write_attribute(env); + + info!( + logger, + "Issued a local API BN certificate for {domains:?} from an ephemeral CA" + ); + + let local_backend = LocalBackend::from_test_env(env)?; + for node in self.api_boundary_nodes.iter() { + let domain = node + .domain + .as_ref() + .expect("API BN domain was just assigned above"); + let ipv6 = node.ipv6.expect("API BN missing IPv6"); + local_backend.add_dns_record(group_name, domain, IpAddr::V6(ipv6))?; + } + + Ok(()) + } + pub fn has_malicious_behaviors(&self) -> bool { let has_malicious_nodes: bool = self .subnets diff --git a/rs/tests/driver/src/driver/local_backend.rs b/rs/tests/driver/src/driver/local_backend.rs index 7c895beb9a86..444896e2bc9b 100644 --- a/rs/tests/driver/src/driver/local_backend.rs +++ b/rs/tests/driver/src/driver/local_backend.rs @@ -6,8 +6,8 @@ //! Boots each VM as a per-VM daemonized `qemu-system-x86_64` process, controlled //! afterwards through its pid-file (destroy) and a per-VM QMP unix socket //! (reboot). Networking (per-group Linux bridge + per-VM TAPs, `dnsmasq` -//! RA/DHCPv4) and disk images (qcow2 overlays over a shared base) are managed -//! directly by this backend. +//! RA/DHCPv4/DNS) and disk images (qcow2 overlays over a shared base) are +//! managed directly by this backend. //! //! Many Farm features have no local equivalent (managed playnet DNS, TLS //! issuance, HTTP file upload, multi-tenant scheduling); those operations warn @@ -25,11 +25,12 @@ use crate::driver::test_env_api::get_dependency_path_from_env; use anyhow::{Context, Result, anyhow, bail}; use deterministic_ips::MacAddr6Ext; use macaddr::MacAddr6; +use network::systemd::IPV6_NAME_SERVERS; use serde::{Deserialize, Serialize}; use slog::{Logger, info, warn}; use std::collections::HashMap; use std::io::{BufRead, BufReader, Write}; -use std::net::Ipv6Addr; +use std::net::{IpAddr, Ipv6Addr}; use std::os::unix::fs::PermissionsExt; use std::os::unix::net::UnixStream; use std::path::{Path, PathBuf}; @@ -37,6 +38,17 @@ use std::process::{Command, Stdio}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; +/// The domain under which the group's `dnsmasq` synthesises a DNS name for every +/// address in the group's `/64`, by writing the address with `:` replaced by `-` +/// (e.g. `fd00-1-2--3.ipv6.nip.io`). +/// +/// This mirrors the public `nip.io` wildcard DNS service, which is what system +/// tests use when they need to reach a VM by a *name* rather than by an address +/// literal — see `rs/tests/networking/canister_http_socks_test.rs`. Answering +/// for it locally is what makes those tests work without external DNS; on the +/// Farm backend the real service resolves the same names to the same addresses. +const NIP_IO_DOMAIN: &str = "ipv6.nip.io"; + /// Environment variables holding the runfiles paths of the split OVMF (UEFI) /// firmware images, provided by the `@ovmf` Bazel repo (extracted from the /// Ubuntu `ovmf-generic-hwe` package; see `bazel/ovmf.bzl`). The code image is @@ -245,7 +257,7 @@ impl LocalBackend { // a *non-zero* inner uid/gid instead of `0`. // // The reason: the backend relies on `dnsmasq` staying unprivileged so it - // skips its privilege-drop path (see `start_ra_daemon`). That path is + // skips its privilege-drop path (see `start_dnsmasq`). That path is // gated purely on `getuid() == 0`, and when taken it fails in this // namespace — `setgroups` is denied (below) and the default `dip` gid is // unmapped. This normally holds because the action runs as an ordinary @@ -550,6 +562,11 @@ impl LocalBackend { /// ([`group_mgmt_ipv6`](Self::group_mgmt_ipv6)). No IP forwarding is /// involved — the management address is on `lo`, so traffic to it terminates /// on the host. + /// + /// The bridge additionally carries the name-server addresses GuestOS is + /// hard-coded to query ([`IPV6_NAME_SERVERS`]), so that the group's + /// `dnsmasq` can answer DNS on them; see + /// [`start_dnsmasq`](Self::start_dnsmasq). pub fn create_group(&self, group_name: &str) -> Result<()> { let bridge = Self::bridge_name(group_name); let prefix = Self::group_ipv6_prefix(group_name); @@ -570,6 +587,19 @@ impl LocalBackend { "Creating local bridge {bridge} for group {group_name} ({prefix}/64, {ipv4_prefix}.0/24)" ); + // The name-server addresses GuestOS sends its DNS queries to. They are + // globally routable addresses owned by Cloudflare and Google, but the + // backend runs in its own network namespace with no external + // connectivity (see `ensure_administrable_netns`), so nothing else can + // claim them and no query can escape. Assigning them here — rather than + // reconfiguring the guests, which have no name-server knob and boot with + // `IPv6AcceptRA=no` — is what gives every node a working resolver + // without touching IC-OS. + let name_server_addrs: String = IPV6_NAME_SERVERS + .iter() + .map(|name_server| format!("ip -6 addr add {name_server}/128 dev {bridge} nodad && ")) + .collect(); + // (Re)create the bridge, assign the gateway, and bring it up. Deleting // first makes this idempotent across an interrupted run that leaked the // bridge. The IPv4 `/24` gateway is always assigned (harmless if no VM @@ -590,6 +620,7 @@ impl LocalBackend { ip link set dev {bridge} up && \ ip -6 addr add {gateway}/64 dev {bridge} nodad && \ ip addr add {ipv4_gateway}/24 dev {bridge} && \ + {name_server_addrs}\ ip -6 addr replace {mgmt}/128 dev lo && \ ip -6 addr replace {logs}/128 dev lo && \ ip -6 addr replace {files}/128 dev lo && \ @@ -600,13 +631,14 @@ impl LocalBackend { // Start the RA daemon. Non-IC-node VMs (e.g. universal VMs) SLAAC their // global address from it; IC GuestOS nodes use a static config instead. // The same `dnsmasq` also serves DHCPv4 on the group's IPv4 `/24` for - // VMs that requested a second NIC. - self.start_ra_daemon(group_name, &bridge, &prefix, &ipv4_prefix)?; + // VMs that requested a second NIC, and DNS on the name-server addresses + // assigned above. + self.start_dnsmasq(group_name, &bridge, &prefix, &ipv4_prefix)?; Ok(()) } - /// Path of the pid-file for the group's `dnsmasq` RA daemon. + /// Path of the pid-file for the group's `dnsmasq`. fn dnsmasq_pid_path(&self, bridge: &str) -> PathBuf { self.active_local_backend .working_dir @@ -614,13 +646,28 @@ impl LocalBackend { .join(format!("{bridge}.pid")) } - /// Spawn a minimal `dnsmasq` as an IPv6 Router Advertisement daemon on - /// `bridge`, advertising the group's `/64` for SLAAC with a non-zero router - /// lifetime (installing the host as the default router for VMs that use the - /// RA; IC GuestOS nodes use a static config instead). The same daemon serves - /// DHCPv4 on the group's IPv4 `/24` for VMs with a second NIC. See - /// [`create_group`](Self::create_group) for the rationale. - fn start_ra_daemon( + /// Path of the extra hosts-file the group's `dnsmasq` serves DNS records + /// from (`--addn-hosts`), written by + /// [`add_dns_record`](Self::add_dns_record). + fn dnsmasq_hosts_path(&self, bridge: &str) -> PathBuf { + self.active_local_backend + .working_dir + .join("dnsmasq") + .join(format!("{bridge}.hosts")) + } + + /// Spawn a minimal `dnsmasq` on `bridge` serving three roles: + /// + /// * an IPv6 Router Advertisement daemon advertising the group's `/64` for + /// SLAAC with a non-zero router lifetime (installing the host as the + /// default router for VMs that use the RA; IC GuestOS nodes use a static + /// config instead), + /// * a DHCPv4 server on the group's IPv4 `/24` for VMs with a second NIC, + /// * the group's DNS server, answering on the name-server addresses + /// [`create_group`](Self::create_group) put on the bridge. + /// + /// See [`create_group`](Self::create_group) for the rationale. + fn start_dnsmasq( &self, group_name: &str, bridge: &str, @@ -632,26 +679,49 @@ impl LocalBackend { format!("creating dnsmasq working dir at {}", dnsmasq_dir.display()) })?; let pid_path = self.dnsmasq_pid_path(bridge); + let hosts_path = self.dnsmasq_hosts_path(bridge); let lease_path = dnsmasq_dir.join(format!("{bridge}.leases")); let log_path = dnsmasq_dir.join(format!("{bridge}.log")); // Remove a stale pid-file from a previous interrupted run. let _ = std::fs::remove_file(&pid_path); + // Truncate the hosts-file, both to drop any records such a run left and + // so it exists before `dnsmasq` starts: a missing `--addn-hosts` file is + // tolerated, but relying on it being picked up later is needless risk. + std::fs::write(&hosts_path, "") + .with_context(|| format!("creating {}", hosts_path.display()))?; info!( self.logger, - "Starting RA daemon (dnsmasq) for group {group_name} on bridge {bridge}" + "Starting dnsmasq for group {group_name} on bridge {bridge}" ); // `dnsmasq` needs `CAP_NET_RAW`/`CAP_NET_ADMIN` to open the ICMPv6 raw // socket and send RAs, and `CAP_NET_BIND_SERVICE` to bind UDP port 67 for - // DHCPv4; it inherits them from the ambient capability set the driver set - // up (see `ensure_administrable_netns`). + // DHCPv4 and port 53 for DNS; it inherits them from the ambient + // capability set the driver set up (see `ensure_administrable_netns`). // `--ra-param=,10,1800` sends an RA every 10s with a 1800s router // lifetime; `--dhcp-range=,ra-only` advertises the autonomous // prefix for SLAAC without stateful leases. The second `--dhcp-range` // enables stateful DHCPv4 on the IPv4 `/24` for the guest's second NIC - // (`enp2s0`). `--port=0` disables DNS. `dnsmasq` daemonizes (writing its - // pid-file) and is signalled via it in teardown. + // (`enp2s0`). `dnsmasq` daemonizes (writing its pid-file) and is + // signalled via it in teardown. + // + // DNS: `--no-resolv --no-hosts` keeps the resolver hermetic — it neither + // reads the driver host's `/etc/resolv.conf` nor its `/etc/hosts`. With + // no upstream server left to forward to, anything it cannot answer is + // REFUSED rather than leaked. It answers from two sources: + // + // * `--addn-hosts` — records tests register through + // [`add_dns_record`](Self::add_dns_record). + // * `--synth-domain` — synthesises `
.ipv6.nip.io` for the + // group's `/64`, with `:` written as `-`, mirroring the public + // `nip.io` wildcard service that tests use to name a VM by its + // address. `dnsmasq` parses the label with `inet_pton`, so it accepts + // exactly the form Rust's `Ipv6Addr` Display produces. + // + // `--bind-interfaces` binds the bridge's addresses as they are at + // startup, which is why `create_group` assigns the name-server addresses + // before calling this. // // `dnsmasq` runs unprivileged: `ensure_administrable_netns` guarantees a // non-zero uid inside the driver's user namespace (identity-mapped, or @@ -667,27 +737,76 @@ impl LocalBackend { --pid-file={pid} \ --dhcp-leasefile={lease} \ --log-facility={log} \ - --port=0 \ --bind-interfaces \ --interface={bridge} \ --except-interface=lo \ --enable-ra \ --dhcp-range={prefix},ra-only \ --dhcp-range={ipv4_prefix}.2,{ipv4_prefix}.254,255.255.255.0,1h \ - --ra-param={bridge},10,1800", + --ra-param={bridge},10,1800 \ + --no-resolv \ + --no-hosts \ + --addn-hosts={hosts} \ + --synth-domain={NIP_IO_DOMAIN},{prefix}/64", pid = pid_path.display(), lease = lease_path.display(), log = log_path.display(), + hosts = hosts_path.display(), ); - Self::run_shell(&dnsmasq_script, "start dnsmasq RA daemon")?; + Self::run_shell(&dnsmasq_script, "start dnsmasq")?; Ok(()) } - /// Stop the group's `dnsmasq` RA daemon, if running. It runs as the current - /// user, so it is signalled directly via its pid-file. Best-effort and - /// idempotent. - fn stop_ra_daemon(&self, bridge: &str) { + /// Register a DNS record with the group's `dnsmasq`, so that `name` resolves + /// to `addr` on every VM in the group. + /// + /// Appends to the `--addn-hosts` file and signals `dnsmasq` with `SIGHUP`, + /// which makes it flush its cache and re-read that file. Records therefore + /// accumulate across calls. + /// + /// This is how the local backend replaces Farm's playnet DNS: see + /// `InternetComputer::setup_api_bn_local_playnet`. + pub fn add_dns_record(&self, group_name: &str, name: &str, addr: IpAddr) -> Result<()> { + let bridge = Self::bridge_name(group_name); + let hosts_path = self.dnsmasq_hosts_path(&bridge); + + info!( + self.logger, + "Registering DNS record {name} -> {addr} with the dnsmasq of group {group_name}" + ); + + let mut hosts_file = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&hosts_path) + .with_context(|| format!("opening {}", hosts_path.display()))?; + writeln!(hosts_file, "{addr} {name}") + .with_context(|| format!("appending to {}", hosts_path.display()))?; + drop(hosts_file); + + // `dnsmasq` runs as the current user, so it can be signalled directly via + // its pid-file. + let pid_path = self.dnsmasq_pid_path(&bridge); + let pid = std::fs::read_to_string(&pid_path) + .with_context(|| format!("reading {}", pid_path.display()))? + .trim() + .parse::() + .with_context(|| format!("parsing the pid in {}", pid_path.display()))?; + let status = Command::new("kill") + .args(["-HUP", &pid.to_string()]) + .status() + .context("signalling dnsmasq with SIGHUP")?; + if !status.success() { + bail!("failed to SIGHUP dnsmasq (pid {pid}): {status}"); + } + + Ok(()) + } + + /// Stop the group's `dnsmasq`, if running. It runs as the current user, so + /// it is signalled directly via its pid-file. Best-effort and idempotent. + fn stop_dnsmasq(&self, bridge: &str) { let pid_path = self.dnsmasq_pid_path(bridge); if let Ok(contents) = std::fs::read_to_string(&pid_path) && let Ok(pid) = contents.trim().parse::() @@ -784,7 +903,7 @@ impl LocalBackend { ); // Stop the RA daemon before removing the bridge it listens on. - self.stop_ra_daemon(&bridge); + self.stop_dnsmasq(&bridge); // Best effort: stop every VM QEMU process started for this group. Each // VM records its pid under `working_dir/vms//qemu.pid`; killing it diff --git a/rs/tests/driver/src/driver/test_env_api.rs b/rs/tests/driver/src/driver/test_env_api.rs index fbfc7db87707..d7b257b6251a 100644 --- a/rs/tests/driver/src/driver/test_env_api.rs +++ b/rs/tests/driver/src/driver/test_env_api.rs @@ -167,17 +167,19 @@ use ic_nns_test_utils::{ }; use ic_prep_lib::prep_state_directory::IcPrepStateDir; use ic_protobuf::registry::{ - node::v1 as pb_node, replica_version::v1::ReplicaVersionRecord, subnet::v1 as pb_subnet, - unassigned_nodes_config::v1::UnassignedNodesConfigRecord, + firewall::v1::FirewallRule, node::v1 as pb_node, replica_version::v1::ReplicaVersionRecord, + subnet::v1 as pb_subnet, unassigned_nodes_config::v1::UnassignedNodesConfigRecord, }; use ic_registry_client_helpers::{ api_boundary_node::ApiBoundaryNodeRegistry, + firewall::FirewallRegistry, node::NodeRegistry, replica_version::ReplicaVersionRegistry, routing_table::RoutingTableRegistry, subnet::{SubnetListRegistry, SubnetRegistry}, unassigned_nodes::UnassignedNodeRegistry, }; +use ic_registry_keys::FirewallRulesScope; use ic_registry_local_registry::LocalRegistry; use ic_registry_routing_table::CanisterIdRange; use ic_registry_subnet_type::SubnetType; @@ -526,6 +528,22 @@ impl TopologySnapshot { .context("get_all_replica_version_records always returns Some (and it did not)") } + /// The firewall rules currently registered for `scope`, in their registered + /// order, or an empty vector when the scope has no rules. + /// + /// A test that proposes a change to a rule set needs these: the registry + /// canister rejects the proposal unless it carries a hash of the rule set + /// the change is meant to apply to. The set is not necessarily empty to + /// begin with — the local backend, for instance, seeds a global rule that + /// lets the test driver reach the nodes from outside their `/64`. + pub fn firewall_rules(&self, scope: &FirewallRulesScope) -> Result> { + Ok(self + .local_registry + .get_firewall_rules(scope, self.registry_version)? + .map(|rule_set| rule_set.entries) + .unwrap_or_default()) + } + /// The subnet id of the root subnet. /// /// This method panics if in the underlying registry, the root subnet id is diff --git a/rs/tests/driver/src/util.rs b/rs/tests/driver/src/util.rs index e014aea8140a..87a88c314645 100644 --- a/rs/tests/driver/src/util.rs +++ b/rs/tests/driver/src/util.rs @@ -1574,6 +1574,7 @@ pub fn get_config() -> ConfigOptional { domain_name: "".to_string(), node_reward_type: "".to_string(), malicious_behavior: "null".to_string(), + extra_api_boundary_node_trust_anchors_pem: "null".to_string(), }; let ic_json = diff --git a/rs/tests/message_routing/xnet/BUILD.bazel b/rs/tests/message_routing/xnet/BUILD.bazel index 212f55359429..745f66d73ea1 100644 --- a/rs/tests/message_routing/xnet/BUILD.bazel +++ b/rs/tests/message_routing/xnet/BUILD.bazel @@ -124,7 +124,6 @@ system_test_nns( system_test( name = "xnet_cloud_engine_isolation_test", - backend = "farm", # Requires API boundary node playnet (with_api_boundary_nodes_playnet), unsupported on the local backend. cpus = MIN_LOCAL_CPUS + 5 * DEFAULT_VCPUS_PER_VM, # 4 subnets * 1 node + 1 API boundary node, 6 vCPUs each. runtime_deps = UNIVERSAL_CANISTER_RUNTIME_DEPS, deps = [ diff --git a/rs/tests/networking/BUILD.bazel b/rs/tests/networking/BUILD.bazel index e1b590b35519..7ef1f12510eb 100644 --- a/rs/tests/networking/BUILD.bazel +++ b/rs/tests/networking/BUILD.bazel @@ -131,22 +131,9 @@ system_test_nns( ], ) -# NOTE: This test is currently non-functional because API boundary nodes running GuestOS on Farm VMs do not support IPv4. system_test_nns( name = "canister_http_socks_test", - # TODO: support this test on the local backend (drop `backend = "farm"`). - # The test makes a canister HTTP outcall to a `nip.io` hostname (which encodes - # the httpbin UVM's IPv6 address), but the IC nodes' canister-http adapter - # cannot resolve `nip.io` on the local backend because the nodes have no - # external network/DNS access: - # - # Http request failed response: Err((SysTransient, "Connecting to - # .ipv6.nip.io failed: ... dns error ...")) - # - # Supporting this locally needs the nip.io-style hostname to be resolvable by - # the nodes (e.g. via a local DNS resolver). - backend = "farm", - cpus = MIN_LOCAL_CPUS + 7 * DEFAULT_VCPUS_PER_VM + 1 * DEFAULT_VCPUS_PER_VM, # 7 IC Node VMs (1 system + 4 app + 2 API BN) + 1 UVM (httpbin), 6 vCPUs each. + cpus = MIN_LOCAL_CPUS + 11 * DEFAULT_VCPUS_PER_VM + 1 * DEFAULT_VCPUS_PER_VM, # 11 IC Node VMs (1 system + 4 app + 4 cloud engine + 2 API BN) + 1 UVM (httpbin), 6 vCPUs each. enable_uvm = True, tags = [ "dynamic_testnet", @@ -299,7 +286,11 @@ rust_binary( system_test_nns( name = "nns_delegation_mainnet_nns_version_test", - backend = "farm", # Requires API boundary node playnet (with_api_boundary_nodes_playnet), unsupported on the local backend. + # The nodes run the mainnet GuestOS, whose replica predates + # `extra_api_boundary_node_trust_anchors_pem`. It therefore only trusts + # publicly issued API boundary node certificates, which rules out the local + # backend's own CA. Drop this once the field has reached mainnet. + backend = "farm", cpus = MIN_LOCAL_CPUS + 4 * DEFAULT_VCPUS_PER_VM + 1 * DEFAULT_VCPUS_PER_VM, # 4 single-node subnets + 1 API BN, 6 vCPUs each. guestos = "mainnet_nns", guestos_update = True, @@ -314,7 +305,6 @@ system_test_nns( system_test_nns( name = "nns_delegation_branch_nns_version_test", - backend = "farm", # Requires API boundary node playnet (with_api_boundary_nodes_playnet), unsupported on the local backend. cpus = MIN_LOCAL_CPUS + 4 * DEFAULT_VCPUS_PER_VM + 1 * DEFAULT_VCPUS_PER_VM, # 4 single-node subnets + 1 API BN, 6 vCPUs each. guestos_update = True, tags = [ diff --git a/rs/tests/networking/canister_http_socks_test.rs b/rs/tests/networking/canister_http_socks_test.rs index 2245b317c6ab..a96c879a7679 100644 --- a/rs/tests/networking/canister_http_socks_test.rs +++ b/rs/tests/networking/canister_http_socks_test.rs @@ -1,17 +1,17 @@ /* tag::catalog[] -Title:: HTTP requests from canisters to remote IPv4 service through socks proxy on API boundary node. +Title:: HTTP requests from canisters to a remote service through the socks proxy on an API boundary node. -Goal:: Ensure that HTTP requests from canisters to a remote IPv4 service are routed through the -correct API boundary node (system API BN for system subnet, application API BN for application and -cloud engine subnets). +Goal:: Ensure that HTTP requests from canisters that cannot reach the remote service directly are +routed through the correct API boundary node (system API BN for system subnet, application API BN +for application and cloud engine subnets). Runbook:: 1. Instantiate an IC with one application, one cloud engine and one system subnet with the HTTP feature enabled. 2. Install NNS canisters 3. Install the proxy canister on all subnets. -4. Make a http outcall request to the IPv4 interface of the http server from the system subnet. -5. Make a http outcall request to the IPv4 interface of the http server from the application subnet. -6. Make a http outcall request to the IPv4 interface of the http server from the cloud engine. +4. For each of those subnets: block the outcalls adapter's direct route to the http server, make an + outcall, and check the server saw the request coming from an API boundary node; then unblock the + direct route, make the same outcall, and check the server saw it coming from a subnet node. Success:: 1. Received http response with status 200 that is routed through the correct API boundary node. @@ -150,8 +150,16 @@ fn setup_and_run_subnet_test( let webserver_ipv6 = get_universal_vm_address(&env).to_string(); - // The dante server running on the API boundary node expects a domain name. - // Construct nip.io hostname for the IPv6 address. + // The outcall has to target a *host name*, not an address literal: the + // adapter's socks connector always encodes the SOCKS5 target as a domain + // name, and for an IPv6 URL that string keeps its brackets (`[fd00::1]`), + // which the dante server on the API boundary node cannot resolve. + // + // So name the webserver after its own address, the way the public `nip.io` + // wildcard service does: colons written as dashes. The webserver's + // certificate covers that name (see `start_httpbin_on_uvm`), and on the + // local backend the group's dnsmasq synthesises it (see + // `LocalBackend::start_ra_daemon`), so no external DNS is involved there. let nip_io_hostname = webserver_ipv6.replace(':', "-") + ".ipv6.nip.io"; let webserver_url = format!("https://{}/ip", nip_io_hostname); diff --git a/rs/tests/networking/firewall/BUILD.bazel b/rs/tests/networking/firewall/BUILD.bazel index 1bb12f0cdd00..045bcaa7faa1 100644 --- a/rs/tests/networking/firewall/BUILD.bazel +++ b/rs/tests/networking/firewall/BUILD.bazel @@ -40,7 +40,14 @@ system_test_nns( system_test_nns( name = "firewall_correctness_test", - backend = "farm", # Requires API boundary node playnet (with_api_boundary_nodes_playnet), unsupported on the local backend. + # The test asserts that port 8080 is closed between particular pairs of + # nodes, but on the local backend `bootstrap.rs` has to whitelist the + # driver's ULA range `fd00::/8` on the replica's ports so the driver can + # reach the nodes at all — and `ic-prep` always adds 8080 to that rule. Since + # the nodes themselves live in `fd00::/8`, that opens 8080 between all of + # them. Supporting this locally means narrowing that rule to the driver's own + # addresses. + backend = "farm", cpus = MIN_LOCAL_CPUS + 6 * DEFAULT_VCPUS_PER_VM, # 5 IC Node VMs (System 1 + Application 2 + CloudEngine 2) + 1 API boundary node VM, all 6 vCPUs. deps = [ ":ic_firewall_system_test_utils", diff --git a/rs/tests/networking/firewall/firewall_correctness_test.rs b/rs/tests/networking/firewall/firewall_correctness_test.rs index 9f7923997aa1..6882067fab47 100644 --- a/rs/tests/networking/firewall/firewall_correctness_test.rs +++ b/rs/tests/networking/firewall/firewall_correctness_test.rs @@ -117,6 +117,7 @@ pub fn firewall_correctness_test(env: TestEnv) { // The rule allows necessary ports (SSH and the metrics ports) to be open to everyone, such // that the test driver can still connect to the nodes and perform the test add_necessary_ports_registry_rule( + &topology_snapshot, &topology_snapshot.root_subnet().nodes().next().unwrap(), &logger, ) @@ -167,7 +168,11 @@ pub fn firewall_correctness_test(env: TestEnv) { }); } -async fn add_necessary_ports_registry_rule(nns_node: &IcNodeSnapshot, log: &Logger) { +async fn add_necessary_ports_registry_rule( + topology_snapshot: &TopologySnapshot, + nns_node: &IcNodeSnapshot, + log: &Logger, +) { let ipv6_prefixes = get_config().firewall.unwrap().default_rules[0] .ipv6_prefixes .clone(); @@ -180,13 +185,19 @@ async fn add_necessary_ports_registry_rule(nns_node: &IcNodeSnapshot, log: &Logg user: None, direction: Some(FirewallRuleDirection::Inbound as i32), }; + // The global scope does not necessarily start out empty — the local backend + // seeds a rule that lets the test driver reach the nodes — and the proposal + // has to carry a hash of the rule set it applies to. + let previous_rules = topology_snapshot + .firewall_rules(&FirewallRulesScope::Global) + .expect("Could not read the global firewall rules"); execute_add_firewall_rules_proposal( log, nns_node, FirewallRulesScope::Global, vec![rule], vec![0], - vec![], + previous_rules, ) .await; } diff --git a/rs/tests/nns/BUILD.bazel b/rs/tests/nns/BUILD.bazel index 22c2df2dafd5..d6a154590168 100644 --- a/rs/tests/nns/BUILD.bazel +++ b/rs/tests/nns/BUILD.bazel @@ -61,7 +61,6 @@ system_test_nns( system_test( name = "delete_subnet_test", - backend = "farm", # Requires API boundary node playnet (with_api_boundary_nodes_playnet), unsupported on the local backend. cpus = MIN_LOCAL_CPUS + 8 * DEFAULT_VCPUS_PER_VM, # 1 API boundary node + 1 System + 1 App + 1 VerifiedApp + 4 CloudEngine IC Node VMs * 6 vCPUs. tags = [ "long_test", From 9817af49e010782c42a0f994d2a1e5ce1e4dfb74 Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Tue, 18 Aug 2026 22:36:53 +0000 Subject: [PATCH 2/8] fix(config-types): give the fixture generator one Url per NNS URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Url::parse` was handed the whole comma-separated list, which yields a single `Url` whose host is `icp-api.io,https` and whose path is the remainder. That re-serialises to `https://icp-api.io,https//icp0.io,https://ic0.app` — not a valid URL, and not the three the fixture meant to carry. Split it into three, and regenerate the v1.16.0 fixtures, which this branch introduces. The older fixtures keep the malformed value: they are historical records that must stay byte-for-byte as generated. Co-Authored-By: Claude Opus 5 (1M context) --- .../compatibility_tests/fixtures/guestos_v1.16.0.json | 4 +++- .../compatibility_tests/fixtures/hostos_v1.16.0.json | 4 +++- rs/ic_os/config/types/compatibility_tests/src/fixture.rs | 8 +++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.16.0.json b/rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.16.0.json index f3f64e18c8cd..dd69d5107b24 100644 --- a/rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.16.0.json +++ b/rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.16.0.json @@ -19,7 +19,9 @@ "mgmt_mac": "00:00:00:00:00:01", "deployment_environment": "mainnet", "nns_urls": [ - "https://icp-api.io,https//icp0.io,https://ic0.app" + "https://icp-api.io/", + "https://icp0.io/", + "https://ic0.app/" ], "node_operator_private_key": null, "enable_trusted_execution_environment": false, diff --git a/rs/ic_os/config/types/compatibility_tests/fixtures/hostos_v1.16.0.json b/rs/ic_os/config/types/compatibility_tests/fixtures/hostos_v1.16.0.json index 2e73eb892e85..ebf14a912a55 100644 --- a/rs/ic_os/config/types/compatibility_tests/fixtures/hostos_v1.16.0.json +++ b/rs/ic_os/config/types/compatibility_tests/fixtures/hostos_v1.16.0.json @@ -19,7 +19,9 @@ "mgmt_mac": "00:00:00:00:00:01", "deployment_environment": "mainnet", "nns_urls": [ - "https://icp-api.io,https//icp0.io,https://ic0.app" + "https://icp-api.io/", + "https://icp0.io/", + "https://ic0.app/" ], "node_operator_private_key": null, "enable_trusted_execution_environment": false, diff --git a/rs/ic_os/config/types/compatibility_tests/src/fixture.rs b/rs/ic_os/config/types/compatibility_tests/src/fixture.rs index 6179d4c4cc0e..4ed7e3ffca01 100644 --- a/rs/ic_os/config/types/compatibility_tests/src/fixture.rs +++ b/rs/ic_os/config/types/compatibility_tests/src/fixture.rs @@ -66,8 +66,14 @@ fn generate_hostos_config() -> HostOSConfig { mgmt_mac: "00:00:00:00:00:01".parse().unwrap(), deployment_environment: DeploymentEnvironment::Mainnet, enable_trusted_execution_environment: false, + // One `Url` per entry. Passing the whole comma-separated list to + // `Url::parse` yields a single `Url` whose host is `icp-api.io,https` + // and whose path is the rest, which re-serialises to the malformed + // `https://icp-api.io,https//icp0.io,https://ic0.app`. nns_urls: vec![ - url::Url::parse("https://icp-api.io,https://icp0.io,https://ic0.app").unwrap(), + url::Url::parse("https://icp-api.io").unwrap(), + url::Url::parse("https://icp0.io").unwrap(), + url::Url::parse("https://ic0.app").unwrap(), ], node_operator_private_key: None, use_ssh_authorized_keys: false, From 9cb6889da5bad639a1bb61dcb480c3e069d145cf Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Tue, 18 Aug 2026 22:47:59 +0000 Subject: [PATCH 3/8] docs(system-tests): drop stale "RA daemon" wording from the local backend `start_ra_daemon`/`stop_ra_daemon` became `start_dnsmasq`/`stop_dnsmasq` when the daemon took on DNS, but a cross-reference in `canister_http_socks_test` still pointed at the old name, and three comments still called it the RA daemon even though it now serves RA, DHCPv4 and DNS. Co-Authored-By: Claude Opus 5 (1M context) --- rs/tests/driver/src/driver/local_backend.rs | 16 ++++++++-------- rs/tests/networking/canister_http_socks_test.rs | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rs/tests/driver/src/driver/local_backend.rs b/rs/tests/driver/src/driver/local_backend.rs index 444896e2bc9b..2268d2de4451 100644 --- a/rs/tests/driver/src/driver/local_backend.rs +++ b/rs/tests/driver/src/driver/local_backend.rs @@ -551,11 +551,11 @@ impl LocalBackend { /// /// IC GuestOS nodes statically configure their global IPv6: the test driver /// hands each node a fixed address plus the `::1` gateway (which - /// lives on the bridge), so they need neither RA nor SLAAC. We still run a - /// minimal `dnsmasq` as an RA daemon on the bridge for non-IC-node VMs (e.g. - /// universal VMs), which bring up only a link-local address and derive their - /// global one via SLAAC from the RA; the RA's non-zero router lifetime also - /// installs the bridge (the host) as their default router. + /// lives on the bridge), so they need neither RA nor SLAAC. The group's + /// `dnsmasq` still advertises the prefix on the bridge for non-IC-node VMs + /// (e.g. universal VMs), which bring up only a link-local address and derive + /// their global one via SLAAC from the RA; the RA's non-zero router lifetime + /// also installs the bridge (the host) as their default router. /// /// Either way the host is each guest's default router, which lets a guest /// reply to the driver's off-`/64` management address @@ -628,8 +628,8 @@ impl LocalBackend { ); Self::run_shell(&create_script, "create group bridge")?; - // Start the RA daemon. Non-IC-node VMs (e.g. universal VMs) SLAAC their - // global address from it; IC GuestOS nodes use a static config instead. + // Start `dnsmasq`. Non-IC-node VMs (e.g. universal VMs) SLAAC their + // global address from its RA; IC GuestOS nodes use a static config instead. // The same `dnsmasq` also serves DHCPv4 on the group's IPv4 `/24` for // VMs that requested a second NIC, and DNS on the name-server addresses // assigned above. @@ -902,7 +902,7 @@ impl LocalBackend { "Deleting local group {group_name} (bridge {bridge})" ); - // Stop the RA daemon before removing the bridge it listens on. + // Stop `dnsmasq` before removing the bridge it listens on. self.stop_dnsmasq(&bridge); // Best effort: stop every VM QEMU process started for this group. Each diff --git a/rs/tests/networking/canister_http_socks_test.rs b/rs/tests/networking/canister_http_socks_test.rs index a96c879a7679..fd3b97446649 100644 --- a/rs/tests/networking/canister_http_socks_test.rs +++ b/rs/tests/networking/canister_http_socks_test.rs @@ -159,7 +159,7 @@ fn setup_and_run_subnet_test( // wildcard service does: colons written as dashes. The webserver's // certificate covers that name (see `start_httpbin_on_uvm`), and on the // local backend the group's dnsmasq synthesises it (see - // `LocalBackend::start_ra_daemon`), so no external DNS is involved there. + // `LocalBackend::start_dnsmasq`), so no external DNS is involved there. let nip_io_hostname = webserver_ipv6.replace(':', "-") + ".ipv6.nip.io"; let webserver_url = format!("https://{}/ip", nip_io_hostname); From b6b3a80106efe459f7f3ba1968e5da7e168b5bb6 Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Wed, 19 Aug 2026 11:33:15 +0000 Subject: [PATCH 4/8] revert(system-tests): drop the firewall_correctness_test previous-rules plumbing `firewall_correctness_test` stays `backend = "farm"`, and on Farm the global firewall scope starts out empty, so reading the current rules and passing them as the proposal's `previous_rules` buys nothing there. It was a remnant of trying to also enable the test on the local backend, where the backend seeds a global rule so the driver can reach the nodes. Move it, and the `TopologySnapshot::firewall_rules` accessor it needed, to the follow-up that actually enables the test locally. Both files are byte-identical to master again. Co-Authored-By: Claude Opus 5 (1M context) --- rs/tests/driver/src/driver/test_env_api.rs | 22 ++----------------- .../firewall/firewall_correctness_test.rs | 15 ++----------- 2 files changed, 4 insertions(+), 33 deletions(-) diff --git a/rs/tests/driver/src/driver/test_env_api.rs b/rs/tests/driver/src/driver/test_env_api.rs index d7b257b6251a..fbfc7db87707 100644 --- a/rs/tests/driver/src/driver/test_env_api.rs +++ b/rs/tests/driver/src/driver/test_env_api.rs @@ -167,19 +167,17 @@ use ic_nns_test_utils::{ }; use ic_prep_lib::prep_state_directory::IcPrepStateDir; use ic_protobuf::registry::{ - firewall::v1::FirewallRule, node::v1 as pb_node, replica_version::v1::ReplicaVersionRecord, - subnet::v1 as pb_subnet, unassigned_nodes_config::v1::UnassignedNodesConfigRecord, + node::v1 as pb_node, replica_version::v1::ReplicaVersionRecord, subnet::v1 as pb_subnet, + unassigned_nodes_config::v1::UnassignedNodesConfigRecord, }; use ic_registry_client_helpers::{ api_boundary_node::ApiBoundaryNodeRegistry, - firewall::FirewallRegistry, node::NodeRegistry, replica_version::ReplicaVersionRegistry, routing_table::RoutingTableRegistry, subnet::{SubnetListRegistry, SubnetRegistry}, unassigned_nodes::UnassignedNodeRegistry, }; -use ic_registry_keys::FirewallRulesScope; use ic_registry_local_registry::LocalRegistry; use ic_registry_routing_table::CanisterIdRange; use ic_registry_subnet_type::SubnetType; @@ -528,22 +526,6 @@ impl TopologySnapshot { .context("get_all_replica_version_records always returns Some (and it did not)") } - /// The firewall rules currently registered for `scope`, in their registered - /// order, or an empty vector when the scope has no rules. - /// - /// A test that proposes a change to a rule set needs these: the registry - /// canister rejects the proposal unless it carries a hash of the rule set - /// the change is meant to apply to. The set is not necessarily empty to - /// begin with — the local backend, for instance, seeds a global rule that - /// lets the test driver reach the nodes from outside their `/64`. - pub fn firewall_rules(&self, scope: &FirewallRulesScope) -> Result> { - Ok(self - .local_registry - .get_firewall_rules(scope, self.registry_version)? - .map(|rule_set| rule_set.entries) - .unwrap_or_default()) - } - /// The subnet id of the root subnet. /// /// This method panics if in the underlying registry, the root subnet id is diff --git a/rs/tests/networking/firewall/firewall_correctness_test.rs b/rs/tests/networking/firewall/firewall_correctness_test.rs index 6882067fab47..9f7923997aa1 100644 --- a/rs/tests/networking/firewall/firewall_correctness_test.rs +++ b/rs/tests/networking/firewall/firewall_correctness_test.rs @@ -117,7 +117,6 @@ pub fn firewall_correctness_test(env: TestEnv) { // The rule allows necessary ports (SSH and the metrics ports) to be open to everyone, such // that the test driver can still connect to the nodes and perform the test add_necessary_ports_registry_rule( - &topology_snapshot, &topology_snapshot.root_subnet().nodes().next().unwrap(), &logger, ) @@ -168,11 +167,7 @@ pub fn firewall_correctness_test(env: TestEnv) { }); } -async fn add_necessary_ports_registry_rule( - topology_snapshot: &TopologySnapshot, - nns_node: &IcNodeSnapshot, - log: &Logger, -) { +async fn add_necessary_ports_registry_rule(nns_node: &IcNodeSnapshot, log: &Logger) { let ipv6_prefixes = get_config().firewall.unwrap().default_rules[0] .ipv6_prefixes .clone(); @@ -185,19 +180,13 @@ async fn add_necessary_ports_registry_rule( user: None, direction: Some(FirewallRuleDirection::Inbound as i32), }; - // The global scope does not necessarily start out empty — the local backend - // seeds a rule that lets the test driver reach the nodes — and the proposal - // has to carry a hash of the rule set it applies to. - let previous_rules = topology_snapshot - .firewall_rules(&FirewallRulesScope::Global) - .expect("Could not read the global firewall rules"); execute_add_firewall_rules_proposal( log, nns_node, FirewallRulesScope::Global, vec![rule], vec![0], - previous_rules, + vec![], ) .await; } From d70ee7ac5c466217bc26496dc74104eb491d3a30 Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Fri, 21 Aug 2026 10:06:26 +0000 Subject: [PATCH 5/8] support standard_engine_replica_version_test on the local backend --- rs/tests/consensus/orchestrator/BUILD.bazel | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/rs/tests/consensus/orchestrator/BUILD.bazel b/rs/tests/consensus/orchestrator/BUILD.bazel index 93de4109524e..fecf5c0745d2 100644 --- a/rs/tests/consensus/orchestrator/BUILD.bazel +++ b/rs/tests/consensus/orchestrator/BUILD.bazel @@ -151,21 +151,6 @@ system_test( system_test_nns( name = "standard_engine_replica_version_test", - # TODO: Remove this once PR 11208 is in master. - # Why: This test needs an API Boundary Node, and those do not work - # when you do `bazel test //rs/tests:widget_test_local`. This test - # only works, when run in Farm. This blocks `_local`. - # - # Ok, but why is API BN needed: Cloud Engines. - # - # Ok, but why do Cloud Engines need API BNs? They are not allowed - # to talk NATIVELY to canisters that live in other subnets, but - # they need to talk to the Registry canister, which lives in the NNS - # subnet. - # - # To satisfy these conflicting requirements, Cloud Engines talk to - # Registry via API BNs. - backend = "farm", # 1 API BN + 1 System + 2 * 4 Cloud Engine = 10 IC Node VMs * 6 vCPUs. cpus = MIN_LOCAL_CPUS + 10 * DEFAULT_VCPUS_PER_VM, # TODO: Re-enable this once BOTH of these pins in From 37395b0440439d79e2607192f1798f25041dcb78 Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Fri, 21 Aug 2026 12:32:01 +0000 Subject: [PATCH 6/8] fix(system-tests): give standard_engine_replica_version_test room for local upgrades `assert_assigned_replica_version` allows a node 600s to come up on a new replica version. On Farm that covers a whole GuestOS upgrade cycle; on the local backend it does not. This test's 10 VMs ask for 60 vCPUs and 40 GiB of guest RAM from a single host, and one cycle was measured at ~9 min there: ~90s to download the ~580 MiB update image, ~240s for `manageboot.sh` to `tar`-unpack it into the guest's tmpfs `/tmp`, ~35s to `dd` it onto the inactive slot and ~150s to reboot. Step 5's deadline expired 79s after the orchestrator came back up on the new version, before any replica had bound :8080, so the test panicked with "Replica did reboot, but never came back online!" -- which is only what `assert_assigned_replica_version_with_time` prints when its last poll errored. Wait 20 min per node instead, and raise the per-test timeout from 30 to 50 min: `ImageUpgrader::execute_upgrade` deletes the update image after installing and never checks whether the target version already sits on the inactive slot, so the roll back in Step 7 and the roll forward in Step 9 each pay for the full cycle again. A local run now passes in 2300s, still well inside the `test_timeout = "eternal"` hour the BUILD file gives the action. Co-Authored-By: Claude Opus 5 (1M context) --- .../standard_engine_replica_version_test.rs | 47 +++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/rs/tests/consensus/orchestrator/standard_engine_replica_version_test.rs b/rs/tests/consensus/orchestrator/standard_engine_replica_version_test.rs index e9c84e5a8087..e2712ca0a97d 100644 --- a/rs/tests/consensus/orchestrator/standard_engine_replica_version_test.rs +++ b/rs/tests/consensus/orchestrator/standard_engine_replica_version_test.rs @@ -97,7 +97,7 @@ use ic_canister_client::Sender; use ic_consensus_system_test_upgrade_common::elect_target_version; use ic_consensus_system_test_utils::rw_message::install_nns_with_customizations_and_check_progress; use ic_consensus_system_test_utils::upgrade::{ - assert_assigned_replica_version, get_assigned_replica_version, + assert_assigned_replica_version_with_time, get_assigned_replica_version, }; use ic_engine_controller::{CreateEngineArgs, NewSubnet}; use ic_nervous_system_common_test_keys::{TEST_NEURON_1_ID, TEST_NEURON_1_OWNER_KEYPAIR}; @@ -135,6 +135,41 @@ const ENGINE_NODE_COUNT: usize = 4; // upgrade priorities can upgrade one, but not the other. const NUM_ENGINES: usize = 2; +// How long to wait for one Cloud Engine node to come up on another replica +// version, and how often to poll it in the meantime. +// +// Every such wait is a full GuestOS upgrade cycle: the orchestrator downloads +// the ~580 MiB update image, `tar`-unpacks it into ~11 GiB of boot.img + +// root.img, `dd`s those onto the inactive slot, reboots, and only then starts +// the replica. There is no shortcut for a version that already sits on the +// inactive slot (see `ImageUpgrader::execute_upgrade`), so the roll back in +// [Step 7] and the roll forward in [Step 9] each pay for the cycle again. +// +// On Farm one cycle fits in the 600 s that `assert_assigned_replica_version` +// defaults to. On the `local` backend it does not: all 8 engine nodes run the +// cycle simultaneously on a single host that the 10 VMs of this test +// oversubscribe (60 vCPUs and 40 GiB of guest RAM), and a measured cycle took +// ~9 min there -- ~90 s to download, ~240 s to unpack, ~35 s to `dd`, ~150 s to +// reboot -- leaving no room for the replica to start before the deadline. +const REPLICA_VERSION_TIMEOUT_SECS: u64 = 20 * 60; +const REPLICA_VERSION_BACKOFF_SECS: u64 = 10; + +/// Waits until `node` is healthy and running `expected_version`, panicking if +/// that does not happen within [`REPLICA_VERSION_TIMEOUT_SECS`]. +fn assert_assigned_replica_version( + node: &IcNodeSnapshot, + expected_version: &ReplicaVersion, + logger: Logger, +) { + assert_assigned_replica_version_with_time( + node, + expected_version, + logger, + REPLICA_VERSION_TIMEOUT_SECS, + REPLICA_VERSION_BACKOFF_SECS, + ) +} + fn setup(env: TestEnv) { let logger = env.logger(); @@ -427,9 +462,13 @@ fn main() -> Result<()> { SystemTestGroup::new() .with_setup(setup) .add_test(systest!(test)) - // Give this test more time, because one successful run was observed - // to take about 20 minutes. - .with_timeout_per_test(Duration::from_secs(30 * 60)) + // Give this test more time. One successful Farm run was observed to + // take about 20 minutes; on the `local` backend each of the three + // upgrade waves costs ~10 minutes on its own (see + // `REPLICA_VERSION_TIMEOUT_SECS`), so budget enough for that while + // staying under the `test_timeout = "eternal"` (1 hour) that the BUILD + // file gives the whole action. + .with_timeout_per_test(Duration::from_secs(50 * 60)) .execute_from_args()?; Ok(()) } From 19aeb5047b7f42301b4e7eb1f1476949a33b933e Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Wed, 19 Aug 2026 13:17:44 +0000 Subject: [PATCH 7/8] feat(system-tests): run firewall_correctness_test on the local backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `firewall_correctness_test` asserts that port 8080 is closed from cloud engine nodes to non-cloud-engine nodes. `init_ic` seeds a global registry firewall rule so the test driver can reach the nodes once the orchestrator's nftables ruleset is live — the driver's source addresses lie outside the nodes' `/64`, so nothing whitelists them by default. That rule whitelisted the group's whole ULA range `fd00::/8` and `ic-prep` always adds 8080 to it, so since the nodes are addressed out of that same range it opened 8080 between all of them. Whitelist the driver's own three addresses as `/128`s instead of the range they live in. Node<->node traffic is then governed by the registry's node-whitelisting rules alone, exactly as on Farm. Nothing in the IC needs the wider range: non-cloud-engine nodes reach the NNS on `:8080` through those same rules, cloud engine nodes reach it through an API boundary node's `:443`, and XNet `:2497` comes from the all-nodes rule. A VM in the group other than the driver can need it, though. Such a VM shares the nodes' `/64`, which the GuestOS firewall already accepts on 7070/9090/9091/9100/19100/19522/19531 (plus 9314 on cloud engines and 9324 on API boundary nodes), so only 22, 2497, 4100, 8080 and 19523 become unreachable. `InternetComputer::with_extra_firewall_whitelist` widens the whitelist for those cases, adding to the driver's prefixes and ports rather than replacing them. Of the local tests that put a VM in the group, only the rosetta ones need it: they run `ic-rosetta-api --ic-url http://[node]:8080` on a universal VM. The driver's three addresses now come from a single `group_driver_ipv6s`, shared by `create_group`, `delete_group` and the whitelist, so a fourth one cannot fall out of the firewall rule unnoticed. The test itself reads the current global rules through the new `TopologySnapshot::firewall_rules` rather than assuming that scope starts out empty, which it does not on this backend. Co-Authored-By: Claude Opus 5 (1M context) --- rs/tests/driver/src/driver/bootstrap.rs | 60 ++++++++++++++----- rs/tests/driver/src/driver/ic.rs | 35 +++++++++++ rs/tests/driver/src/driver/local_backend.rs | 52 +++++++++++++--- rs/tests/driver/src/driver/test_env_api.rs | 22 ++++++- .../rosetta/rosetta_test_lib/setup.rs | 5 ++ rs/tests/networking/firewall/BUILD.bazel | 8 --- .../firewall/firewall_correctness_test.rs | 15 ++++- 7 files changed, 161 insertions(+), 36 deletions(-) diff --git a/rs/tests/driver/src/driver/bootstrap.rs b/rs/tests/driver/src/driver/bootstrap.rs index 97d7e02e648c..0f5c6a2b761f 100644 --- a/rs/tests/driver/src/driver/bootstrap.rs +++ b/rs/tests/driver/src/driver/bootstrap.rs @@ -1,5 +1,6 @@ use crate::driver::ic_gateway_vm::{HasIcGatewayVm, IC_GATEWAY_VM_NAME, Playnet}; use crate::driver::ic_images::try_get_setupos_img_version; +use crate::driver::local_backend::LocalBackend; use crate::driver::nested::NestedVm; use crate::driver::resource::{BootImage, DiskImage}; use crate::driver::test_env_api::{ @@ -23,7 +24,7 @@ use crate::driver::{ get_guestos_initial_update_img_sha256, get_guestos_initial_update_img_url, get_setupos_img_sha256, get_setupos_img_url, try_get_guestos_img_version, }, - test_setup::SystemTestBackend, + test_setup::{GroupSetup, SystemTestBackend}, }; use anyhow::{Context, Result, bail}; use bare_metal_deployment::SshAuthMethod; @@ -75,6 +76,13 @@ const DOGECOIND_ADDR_PATH: &str = "dogecoind_addr"; const JAEGER_ADDR_PATH: &str = "jaeger_addr"; const SOCKS_PROXY_PATH: &str = "socks_proxy"; +/// The ports the Local backend whitelists for the test driver on every node, +/// mirroring the ports the firewall template's `default_rules` open to Farm's +/// management prefixes. `ic-prep` always adds 8080 on top of these. +const LOCAL_WHITELISTED_PORTS: &[u32] = &[ + 22, 2497, 4100, 7070, 9090, 9091, 9100, 9324, 19100, 19523, 19531, +]; + fn mk_compressed_img_path() -> std::string::String { format!("{CONF_IMG_FNAME}.zst") } @@ -220,16 +228,30 @@ pub fn init_ic( ic_config.set_use_specified_ids_allocation_range(specific_ids); - // On the Local backend the test driver reaches the nodes over the group's - // IPv6 ULA bridge (`fd00::/8`). Unlike Farm — whose management prefixes are - // covered by the firewall template's built-in `default_rules` — these ULA - // source addresses are not whitelisted by default. Once the orchestrator - // applies its nftables ruleset (whose presence several tests assert on), the - // driver would otherwise be locked out of the replica endpoints it needs - // (`:8080`, SSH, metrics, ...). Whitelist the ULA range on the same ports - // the Farm `default_rules` cover so the firewall can be fully active while - // keeping the nodes reachable from the driver. Port 8080 is always included - // by `ic-prep`. + // On the Local backend the test driver reaches the nodes from addresses + // that lie outside their `/64` (see `LocalBackend::group_driver_ipv6s`). + // Unlike Farm — whose management prefixes are covered by the firewall + // template's built-in `default_rules` — those source addresses are not + // whitelisted by default. Once the orchestrator applies its nftables ruleset + // (whose presence several tests assert on), the driver would otherwise be + // locked out of the replica endpoints it needs (`:8080`, SSH, metrics, ...). + // Whitelist the driver's own addresses on the same ports the Farm + // `default_rules` cover, so the firewall can be fully active while keeping + // the nodes reachable from the driver. Port 8080 is always added by + // `ic-prep`. + // + // Only the driver is whitelisted, not the group's whole ULA range `fd00::/8` + // that every VM — the nodes included — is addressed out of. Whitelisting + // that range would open these ports, 8080 among them, between all nodes, so + // node↔node traffic would no longer be governed by the registry's + // node-whitelisting rules alone as it is on Farm — which is exactly what + // `firewall_correctness_test` asserts. Nothing in the IC needs the wider + // range: non-cloud-engine nodes reach the NNS on `:8080` through those same + // whitelisting rules, and cloud engine nodes reach it through an API + // boundary node's `:443` (see `get_node_api_urls` in + // `rs/orchestrator/registry_replicator/src/internal_state.rs`). A test that + // has one of its *other* VMs talk to a node widens the whitelist explicitly + // with `InternetComputer::with_extra_firewall_whitelist`. // // Note: injecting this global registry rule makes the orchestrator use the // registry firewall rules *instead of* the config-file `default_rules` (see @@ -242,10 +264,18 @@ pub fn init_ic( SystemTestBackend::read_attribute(test_env), SystemTestBackend::Local ) { - ic_config.set_whitelisted_prefixes(Some("fd00::/8".to_string())); - ic_config.set_whitelisted_ports(Some( - "22,2497,4100,7070,9090,9091,9100,9324,19100,19523,19531".to_string(), - )); + let group_name = GroupSetup::read_attribute(test_env).infra_group_name; + let prefixes: Vec = LocalBackend::group_driver_ipv6_prefixes(&group_name) + .into_iter() + .chain(ic.extra_firewall_whitelist_prefixes.iter().cloned()) + .collect(); + let ports: Vec = LOCAL_WHITELISTED_PORTS + .iter() + .chain(ic.extra_firewall_whitelist_ports.iter()) + .map(|port| port.to_string()) + .collect(); + ic_config.set_whitelisted_prefixes(Some(prefixes.join(","))); + ic_config.set_whitelisted_ports(Some(ports.join(","))); } for dc_record in &ic.data_centers { diff --git a/rs/tests/driver/src/driver/ic.rs b/rs/tests/driver/src/driver/ic.rs index c9d72ddbe3db..af2c01807f5d 100644 --- a/rs/tests/driver/src/driver/ic.rs +++ b/rs/tests/driver/src/driver/ic.rs @@ -99,6 +99,8 @@ pub struct InternetComputer { pub api_bn_use_playnet: bool, pub data_centers: Vec, pub node_operators: Vec, + pub extra_firewall_whitelist_prefixes: Vec, + pub extra_firewall_whitelist_ports: Vec, } /// Configuration for a node operator to be added to the initial registry. @@ -247,6 +249,39 @@ impl InternetComputer { self } + /// Whitelist additional sources on the nodes' firewall, *on top of* the test + /// driver's own addresses, which the local backend always whitelists (see + /// `init_ic` in `rs/tests/driver/src/driver/bootstrap.rs`). + /// + /// Needed by tests that have a VM other than the driver talk to a node. Such + /// a VM shares the nodes' `/64`, which the GuestOS firewall already accepts + /// on 7070, 9090, 9091, 9100, 19100, 19522 and 19531 (plus 9314 on cloud + /// engines and 9324 on API boundary nodes) — so this is only required to + /// reach a node on one of the *other* whitelisted ports: 22, 2497, 4100, + /// 8080 and 19523. + /// + /// The prefixes and ports are added to the driver's, never replace them, so + /// a caller cannot lock the driver out. Ignored on the Farm backend, whose + /// management prefixes the firewall template's `default_rules` already + /// cover. + pub fn with_extra_firewall_whitelist(mut self, prefixes: Vec, ports: Vec) -> Self { + self.extra_firewall_whitelist_prefixes.extend(prefixes); + self.extra_firewall_whitelist_ports.extend(ports); + self + } + + /// Whitelist every VM in the test group on the nodes' firewall, by adding + /// the local backend's ULA range ([`LocalBackend::GROUP_ULA_PREFIX`]) to the + /// whitelist. + /// + /// A shorthand for the common case of + /// [`with_extra_firewall_whitelist`](Self::with_extra_firewall_whitelist), + /// and what the local backend did unconditionally before the whitelist was + /// narrowed to the driver's own addresses. + pub fn with_group_wide_firewall_whitelist(self) -> Self { + self.with_extra_firewall_whitelist(vec![LocalBackend::GROUP_ULA_PREFIX.to_string()], vec![]) + } + /// Add a single unassigned node with the given IPv4 configuration pub fn with_ipv4_enabled_unassigned_node(mut self, ipv4_config: IPv4Config) -> Self { self.unassigned_nodes.push( diff --git a/rs/tests/driver/src/driver/local_backend.rs b/rs/tests/driver/src/driver/local_backend.rs index 2268d2de4451..1cd322369580 100644 --- a/rs/tests/driver/src/driver/local_backend.rs +++ b/rs/tests/driver/src/driver/local_backend.rs @@ -431,8 +431,9 @@ impl LocalBackend { /// (vs the nodes' `0`), so it lies *outside* every node `/64` — meaning the /// GuestOS firewall's hard-coded accept for a node's own prefix does not /// match the driver, letting registry-derived deny rules actually be - /// exercised — while staying in the ULA range `fd00::/8` the backend - /// whitelists at bootstrap. + /// exercised. `init_ic` whitelists it (see + /// [`group_driver_ipv6_prefixes`](Self::group_driver_ipv6_prefixes)) so the + /// driver can still reach the nodes once the firewall is active. /// /// It is reserved for the driver's *own* host→node traffic; journald /// streaming ([`group_logs_ipv6`](Self::group_logs_ipv6)) and the file @@ -497,6 +498,43 @@ impl LocalBackend { ) } + /// The IPv6 ULA range every address the local backend hands out lives in: + /// the nodes' `/64`, the driver's own addresses and any other VM in the + /// group. Offered to tests that have to whitelist the *whole* group on the + /// nodes' firewall; see + /// [`InternetComputer::with_group_wide_firewall_whitelist`](crate::driver::ic::InternetComputer::with_group_wide_firewall_whitelist). + pub const GROUP_ULA_PREFIX: &'static str = "fd00::/8"; + + /// The three addresses the test driver reaches the group's VMs from: the + /// management source ([`group_mgmt_ipv6`](Self::group_mgmt_ipv6)), the + /// journald-streaming source ([`group_logs_ipv6`](Self::group_logs_ipv6)) + /// and the file server's listen address + /// ([`group_files_ipv6`](Self::group_files_ipv6)). + /// + /// Kept as one list so that the places which have to know the full set — + /// assigning them to `lo` in [`create_group`](Self::create_group), removing + /// them again in [`delete_group`](Self::delete_group), and whitelisting them + /// on the nodes' firewall via + /// [`group_driver_ipv6_prefixes`](Self::group_driver_ipv6_prefixes) — cannot + /// drift apart when a fourth one is added. + fn group_driver_ipv6s(group_name: &str) -> [String; 3] { + [ + Self::group_mgmt_ipv6(group_name), + Self::group_logs_ipv6(group_name), + Self::group_files_ipv6(group_name), + ] + } + + /// [`group_driver_ipv6s`](Self::group_driver_ipv6s) as `/128` prefixes, for + /// the firewall whitelist `init_ic` seeds into the initial registry (see + /// `rs/tests/driver/src/driver/bootstrap.rs`). + pub fn group_driver_ipv6_prefixes(group_name: &str) -> Vec { + Self::group_driver_ipv6s(group_name) + .into_iter() + .map(|addr| format!("{addr}/128")) + .collect() + } + /// Returns the per-group private IPv4 `/24` (a deterministic subnet in /// `10.0.0.0/8`). Hashed from the group name so concurrent groups get /// distinct subnets, with the `.0` network and `.1` gateway reserved. @@ -574,10 +612,8 @@ impl LocalBackend { let gateway = Self::group_gateway_ipv6(group_name); // Driver addresses, all assigned to `lo`: the management source for // host→node traffic, the dedicated journald-streaming source, and the - // file server's listen address. See the respective `group_*_ipv6`. - let mgmt = Self::group_mgmt_ipv6(group_name); - let logs = Self::group_logs_ipv6(group_name); - let files = Self::group_files_ipv6(group_name); + // file server's listen address. See `group_driver_ipv6s`. + let [mgmt, logs, files] = Self::group_driver_ipv6s(group_name); // The IPv4 gateway (`.1`) also lives on the bridge so // `dnsmasq` can serve DHCPv4 to VMs that requested a second NIC. let ipv4_prefix = Self::group_ipv4_prefix(group_name); @@ -894,9 +930,7 @@ impl LocalBackend { /// journald-streaming and file-server) from `lo`. pub fn delete_group(&self, group_name: &str) -> Result<()> { let bridge = Self::bridge_name(group_name); - let mgmt = Self::group_mgmt_ipv6(group_name); - let logs = Self::group_logs_ipv6(group_name); - let files = Self::group_files_ipv6(group_name); + let [mgmt, logs, files] = Self::group_driver_ipv6s(group_name); info!( self.logger, "Deleting local group {group_name} (bridge {bridge})" diff --git a/rs/tests/driver/src/driver/test_env_api.rs b/rs/tests/driver/src/driver/test_env_api.rs index fbfc7db87707..d7b257b6251a 100644 --- a/rs/tests/driver/src/driver/test_env_api.rs +++ b/rs/tests/driver/src/driver/test_env_api.rs @@ -167,17 +167,19 @@ use ic_nns_test_utils::{ }; use ic_prep_lib::prep_state_directory::IcPrepStateDir; use ic_protobuf::registry::{ - node::v1 as pb_node, replica_version::v1::ReplicaVersionRecord, subnet::v1 as pb_subnet, - unassigned_nodes_config::v1::UnassignedNodesConfigRecord, + firewall::v1::FirewallRule, node::v1 as pb_node, replica_version::v1::ReplicaVersionRecord, + subnet::v1 as pb_subnet, unassigned_nodes_config::v1::UnassignedNodesConfigRecord, }; use ic_registry_client_helpers::{ api_boundary_node::ApiBoundaryNodeRegistry, + firewall::FirewallRegistry, node::NodeRegistry, replica_version::ReplicaVersionRegistry, routing_table::RoutingTableRegistry, subnet::{SubnetListRegistry, SubnetRegistry}, unassigned_nodes::UnassignedNodeRegistry, }; +use ic_registry_keys::FirewallRulesScope; use ic_registry_local_registry::LocalRegistry; use ic_registry_routing_table::CanisterIdRange; use ic_registry_subnet_type::SubnetType; @@ -526,6 +528,22 @@ impl TopologySnapshot { .context("get_all_replica_version_records always returns Some (and it did not)") } + /// The firewall rules currently registered for `scope`, in their registered + /// order, or an empty vector when the scope has no rules. + /// + /// A test that proposes a change to a rule set needs these: the registry + /// canister rejects the proposal unless it carries a hash of the rule set + /// the change is meant to apply to. The set is not necessarily empty to + /// begin with — the local backend, for instance, seeds a global rule that + /// lets the test driver reach the nodes from outside their `/64`. + pub fn firewall_rules(&self, scope: &FirewallRulesScope) -> Result> { + Ok(self + .local_registry + .get_firewall_rules(scope, self.registry_version)? + .map(|rule_set| rule_set.entries) + .unwrap_or_default()) + } + /// The subnet id of the root subnet. /// /// This method panics if in the underlying registry, the root subnet id is diff --git a/rs/tests/financial_integrations/rosetta/rosetta_test_lib/setup.rs b/rs/tests/financial_integrations/rosetta/rosetta_test_lib/setup.rs index f8a6453d667d..b0d14a4fb5d7 100644 --- a/rs/tests/financial_integrations/rosetta/rosetta_test_lib/setup.rs +++ b/rs/tests/financial_integrations/rosetta/rosetta_test_lib/setup.rs @@ -71,6 +71,11 @@ pub fn setup( fn create_ic(env: &TestEnv) { InternetComputer::new() .add_fast_single_node_subnet(SubnetType::System) + // `install_rosetta` runs `ic-rosetta-api --ic-url http://[node]:8080` on + // a universal VM. On the local backend the nodes' firewall only opens + // `:8080` to the test driver and to the other nodes in the registry, so + // the VM needs the group's addresses whitelisted to reach the replica. + .with_group_wide_firewall_whitelist() .setup_and_start(env) .expect("Failed to setup IC under test"); check_nodes_health(env); diff --git a/rs/tests/networking/firewall/BUILD.bazel b/rs/tests/networking/firewall/BUILD.bazel index 045bcaa7faa1..cee7a7c72019 100644 --- a/rs/tests/networking/firewall/BUILD.bazel +++ b/rs/tests/networking/firewall/BUILD.bazel @@ -40,14 +40,6 @@ system_test_nns( system_test_nns( name = "firewall_correctness_test", - # The test asserts that port 8080 is closed between particular pairs of - # nodes, but on the local backend `bootstrap.rs` has to whitelist the - # driver's ULA range `fd00::/8` on the replica's ports so the driver can - # reach the nodes at all — and `ic-prep` always adds 8080 to that rule. Since - # the nodes themselves live in `fd00::/8`, that opens 8080 between all of - # them. Supporting this locally means narrowing that rule to the driver's own - # addresses. - backend = "farm", cpus = MIN_LOCAL_CPUS + 6 * DEFAULT_VCPUS_PER_VM, # 5 IC Node VMs (System 1 + Application 2 + CloudEngine 2) + 1 API boundary node VM, all 6 vCPUs. deps = [ ":ic_firewall_system_test_utils", diff --git a/rs/tests/networking/firewall/firewall_correctness_test.rs b/rs/tests/networking/firewall/firewall_correctness_test.rs index 9f7923997aa1..6882067fab47 100644 --- a/rs/tests/networking/firewall/firewall_correctness_test.rs +++ b/rs/tests/networking/firewall/firewall_correctness_test.rs @@ -117,6 +117,7 @@ pub fn firewall_correctness_test(env: TestEnv) { // The rule allows necessary ports (SSH and the metrics ports) to be open to everyone, such // that the test driver can still connect to the nodes and perform the test add_necessary_ports_registry_rule( + &topology_snapshot, &topology_snapshot.root_subnet().nodes().next().unwrap(), &logger, ) @@ -167,7 +168,11 @@ pub fn firewall_correctness_test(env: TestEnv) { }); } -async fn add_necessary_ports_registry_rule(nns_node: &IcNodeSnapshot, log: &Logger) { +async fn add_necessary_ports_registry_rule( + topology_snapshot: &TopologySnapshot, + nns_node: &IcNodeSnapshot, + log: &Logger, +) { let ipv6_prefixes = get_config().firewall.unwrap().default_rules[0] .ipv6_prefixes .clone(); @@ -180,13 +185,19 @@ async fn add_necessary_ports_registry_rule(nns_node: &IcNodeSnapshot, log: &Logg user: None, direction: Some(FirewallRuleDirection::Inbound as i32), }; + // The global scope does not necessarily start out empty — the local backend + // seeds a rule that lets the test driver reach the nodes — and the proposal + // has to carry a hash of the rule set it applies to. + let previous_rules = topology_snapshot + .firewall_rules(&FirewallRulesScope::Global) + .expect("Could not read the global firewall rules"); execute_add_firewall_rules_proposal( log, nns_node, FirewallRulesScope::Global, vec![rule], vec![0], - vec![], + previous_rules, ) .await; } From 5a2afab46e3a2255365cf18c511199944fa304fb Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Wed, 19 Aug 2026 15:08:58 +0000 Subject: [PATCH 8/8] fix(system-tests): dedupe the local firewall whitelist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the local backend's firewall whitelist. The prefixes and ports render into anonymous nftables sets (`ip6 saddr { ... }`), and `nft` rejects a set with a repeated element, taking the whole ruleset down with it. Since `with_extra_firewall_whitelist` adds to the driver's own entries, a test could introduce a duplicate easily enough — by calling it twice, or by passing a port that is already whitelisted. Deduplicate both lists, which also lets `Itertools::join` replace the map/collect/join dance. Also record why `firewall_correctness_test` still inserts its rule at position 0 now that the scope is not necessarily empty: both that rule and the seeded one are `Allow`, so their relative order cannot change the outcome, and 0 is the only valid position when the scope does start out empty, as on Farm. Co-Authored-By: Claude Opus 5 (1M context) --- rs/tests/driver/src/driver/bootstrap.rs | 21 ++++++++++++------- .../firewall/firewall_correctness_test.rs | 6 ++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/rs/tests/driver/src/driver/bootstrap.rs b/rs/tests/driver/src/driver/bootstrap.rs index 0f5c6a2b761f..e5e08aac5457 100644 --- a/rs/tests/driver/src/driver/bootstrap.rs +++ b/rs/tests/driver/src/driver/bootstrap.rs @@ -51,6 +51,7 @@ use ic_registry_canister_api::IPv4Config; use ic_registry_provisional_whitelist::ProvisionalWhitelist; use ic_registry_subnet_type::SubnetType; use ic_types::malicious_behavior::MaliciousBehavior; +use itertools::Itertools; use slog::{Logger, debug, info, warn}; use std::{ collections::BTreeMap, @@ -265,17 +266,23 @@ pub fn init_ic( SystemTestBackend::Local ) { let group_name = GroupSetup::read_attribute(test_env).infra_group_name; - let prefixes: Vec = LocalBackend::group_driver_ipv6_prefixes(&group_name) + // Deduplicated: these become anonymous nftables sets + // (`ip6 saddr { ... }`), and `nft` rejects a set with a repeated + // element, taking the whole ruleset down with it. A test can introduce + // one easily enough — calling `with_extra_firewall_whitelist` twice, or + // passing a port that is already in `LOCAL_WHITELISTED_PORTS`. + let prefixes = LocalBackend::group_driver_ipv6_prefixes(&group_name) .into_iter() .chain(ic.extra_firewall_whitelist_prefixes.iter().cloned()) - .collect(); - let ports: Vec = LOCAL_WHITELISTED_PORTS + .unique() + .join(","); + let ports = LOCAL_WHITELISTED_PORTS .iter() .chain(ic.extra_firewall_whitelist_ports.iter()) - .map(|port| port.to_string()) - .collect(); - ic_config.set_whitelisted_prefixes(Some(prefixes.join(","))); - ic_config.set_whitelisted_ports(Some(ports.join(","))); + .unique() + .join(","); + ic_config.set_whitelisted_prefixes(Some(prefixes)); + ic_config.set_whitelisted_ports(Some(ports)); } for dc_record in &ic.data_centers { diff --git a/rs/tests/networking/firewall/firewall_correctness_test.rs b/rs/tests/networking/firewall/firewall_correctness_test.rs index 6882067fab47..48bbf1a27ca3 100644 --- a/rs/tests/networking/firewall/firewall_correctness_test.rs +++ b/rs/tests/networking/firewall/firewall_correctness_test.rs @@ -188,6 +188,12 @@ async fn add_necessary_ports_registry_rule( // The global scope does not necessarily start out empty — the local backend // seeds a rule that lets the test driver reach the nodes — and the proposal // has to carry a hash of the rule set it applies to. + // + // The rule goes to position 0, which is the only valid position when the + // scope does start out empty, as it does on Farm. Where it does not, the + // position is immaterial: both this rule and the seeded one are `Allow`, and + // a packet is accepted by whichever it matches first, so their relative + // order cannot change the outcome. let previous_rules = topology_snapshot .firewall_rules(&FirewallRulesScope::Global) .expect("Could not read the global firewall rules");