diff --git a/.github/workflows/run-nvx-microvm-tests.yml b/.github/workflows/run-nvx-microvm-tests.yml index 3b7757e8a..0e83643c2 100644 --- a/.github/workflows/run-nvx-microvm-tests.yml +++ b/.github/workflows/run-nvx-microvm-tests.yml @@ -547,7 +547,7 @@ jobs: with open(sys.argv[1], encoding="utf-8") as stream: config = json.load(stream) - if config["format"] != 9 or config["export"]["owner"] != "caller": + if config["format"] != 10 or config["export"]["owner"] != "caller": sys.exit(f"managed sandbox did not persist caller ownership: {config}") EOF errors=$(mktemp) @@ -690,7 +690,7 @@ jobs: ("/opt/hostedtoolcache", "", "ro", ["nvx-denied"], "directory"), ("/results/output.txt", "", "rw", [], "file"), ] - if config["format"] != 9 or "mounts" in config or shares != expected: + if config["format"] != 10 or "mounts" in config or shares != expected: sys.exit(f"managed sandbox did not persist every share: {config}") EOF nvx-rs/target/release/nvx-rs sandbox start --state-dir "${state}" @@ -819,7 +819,7 @@ jobs: ["nvx-writable"], ), ] - if config["format"] != 9 or "mounts" in config or shares != expected: + if config["format"] != 10 or "mounts" in config or shares != expected: sys.exit(f"managed sandbox did not persist the access policy: {config}") EOF nvx-rs/target/release/nvx-rs sandbox start --state-dir "${state}" @@ -1125,7 +1125,7 @@ jobs: $denied = ($child.denied | ForEach-Object { $_.Substring($child.root.Length + 1).Replace("\", "/") }) -join ";" "$($bind.target),$($bind.source),$mode,$denied,$kind" }) -join " " - if ($config.format -ne 9 -or $shares -ne "/workspace,,rw,nvx-denied,directory /config/settings.json,,ro,,file /opt/hostedtoolcache,,ro,nvx-denied,directory /results/output.txt,,rw,,file") { + if ($config.format -ne 10 -or $shares -ne "/workspace,,rw,nvx-denied,directory /config/settings.json,,ro,,file /opt/hostedtoolcache,,ro,nvx-denied,directory /results/output.txt,,rw,,file") { throw "managed sandbox did not persist every share: $($config | ConvertTo-Json -Compress -Depth 5)" } try { @@ -1223,7 +1223,7 @@ jobs: function Get-Relative($paths) { ($paths | ForEach-Object { $_.Substring($child.root.Length + 1).Replace("\", "/") }) -join "," } - if ($config.format -ne 9 -or @($config.export.children).Count -ne 1 -or @($config.export.binds).Count -ne 1 -or $bind.target -ne "/workspace" -or $bind.readOnly -or (Get-Relative $child.denied) -ne "nvx-denied" -or (Get-Relative $child.allowed) -ne "nvx-denied/nvx-allowed" -or (Get-Relative $child.write) -ne "nvx-writable") { + if ($config.format -ne 10 -or @($config.export.children).Count -ne 1 -or @($config.export.binds).Count -ne 1 -or $bind.target -ne "/workspace" -or $bind.readOnly -or (Get-Relative $child.denied) -ne "nvx-denied" -or (Get-Relative $child.allowed) -ne "nvx-denied/nvx-allowed" -or (Get-Relative $child.write) -ne "nvx-writable") { throw "managed sandbox did not persist the access policy: $($config | ConvertTo-Json -Compress -Depth 5)" } try { diff --git a/doc/acceptance-case-matrix.md b/doc/acceptance-case-matrix.md index 2307d756e..ac9303d55 100644 --- a/doc/acceptance-case-matrix.md +++ b/doc/acceptance-case-matrix.md @@ -340,7 +340,7 @@ suite in its shards, and the public sandbox scenarios in its guest and sandbox j | `sandbox-gpt-layer` | kept | The [cold cells](#cold-boots) of a GPT disk, over a scratch upper and over a RAM upper, through `run`, and `provision` and `start` | | `sandbox-lifecycle` | kept | A [cold cell](#cold-boots): the EROFS layer over a scratch upper, through `provision` and `start`, which also reattach to the VM from a new process for each command | | `sandbox-proxy-only` | kept | Its proxy-only network, over the EROFS layer and a scratch upper; it is no [cold cell](#cold-boots) of its own | -| `sandbox-ram-upper` | kept | A [cold cell](#cold-boots): the EROFS layer over a RAM upper, through `run`, and `provision` and `start` | +| `sandbox-ram-upper` | kept | A [cold cell](#cold-boots): the EROFS layer over a RAM upper, through `run`, and `provision` and `start`. Since SL3.10c, it checks that `provision` records the configuration's format 10 | | `scratch-snapshot` | kept | | | `smp` | kept | | | `smp-lapic` | kept | Runs only when a run names it | @@ -447,7 +447,7 @@ their own. The tests that a step runs have rows of their own where they concern | `.github/workflows/run-nvx-microvm-tests.yml` | Run public managed sandbox acceptance on Linux and on Windows | kept | Runs `sandbox-gpt-layer` and `vm-lifecycle` too, since SL5.1b: see [Cold boots](#cold-boots); and `snapshot-core` and `snapshot-tiers`, since SL5.1c: see [Snapshots](#snapshots) | | `.github/workflows/run-nvx-microvm-tests.yml` | Run Ubuntu and Azure Linux guest smoke tests on Linux and on Windows | kept | | | `.github/workflows/run-nvx-microvm-tests.yml` | Run extended Ubuntu guest tests on Linux/KVM | kept | No longer runs `snapshot-core`, which needs the Alpine control guest since SL5.1c rewrote it | -| `.github/workflows/run-nvx-microvm-tests.yml` | Run the Ubuntu sandbox layer, live-share, managed live-share, caller-owned live-share, concurrent shares and share access policy tests on Linux and on Windows | kept | The layer smoke test is the `run` of a [cold cell](#cold-boots) | +| `.github/workflows/run-nvx-microvm-tests.yml` | Run the Ubuntu sandbox layer, live-share, managed live-share, caller-owned live-share, concurrent shares and share access policy tests on Linux and on Windows | kept | The layer smoke test is the `run` of a [cold cell](#cold-boots). Since SL3.10c, the share tests check that `provision` records the configuration's format 10 | | `.github/workflows/run-nvx-microvm-tests.yml` | Build the runtime adapter's guest agent on Linux; Download the runtime adapter's guest agent on Windows | retired | The adapter's guest agent | | `.github/workflows/run-nvx-microvm-tests.yml` | Run runtime adapter scenarios on Linux and on Windows | retired | The adapter's scenarios, whose fates are above | | `.github/workflows/run-platform.yml` | Run benchmark: the acceptance test, the performance suite, the multi-vCPU shell restore and the device operation rates | kept | | diff --git a/doc/run.md b/doc/run.md index 0f9ae62ad..45aa02e29 100644 --- a/doc/run.md +++ b/doc/run.md @@ -1293,7 +1293,7 @@ inside a writable part of a share. On Windows, it refuses a path that only a verbatim `\\?\` path names, such as that of a directory whose name ends in a period or a space, which the path without the prefix would not name. The shares' children follow their resolved host paths. A managed sandbox stores -the plan in its typed configuration, in format 9. Each `start` checks it again, +the plan in its typed configuration, in format 10. Each `start` checks it again, as `run` does once it has verified the distro layer: it refuses a host path that no longer holds a directory or regular file as recorded, a recorded path whose file identity changed, and shares whose host paths came to overlap, such diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 3608f6df5..87ddc0642 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -2879,15 +2879,16 @@ The state directory is the directory of the sandbox's sandbox shares, and `config.json` is the CLI's own record beside them. It also records the [distro binding](#distro-bindings) and the [scratch binding](#checked-handoff) of the sandbox, and its network as the -typed `SandboxNetwork`, in the serde form of the plan's network. The typed -configuration names one EROFS or GPT image by path and identity, and a scratch -upper by path or a RAM upper with its cap from 1 through 90 percent, 50 by -default. `nvx-rs` refuses earlier configuration formats rather than migrating -them: +network options from which each `start` builds the policy again, as `provision` +built it. The typed configuration names one EROFS or GPT image by path and +identity, and a scratch upper by path or a RAM upper with its cap from 1 through +90 percent, 50 by default. `nvx-rs` refuses earlier configuration formats, such +as format 9, rather than migrating them, and says to deprovision the sandbox and +provision it again: | File | Contents | | --- | --- | -| `config.json` | The provisioned sandbox, in format 9 only: `image` holds its format, path and identity; `upper` is tagged as scratch with its path or RAM with `max_memory_percent`. `distro_binding` binds the image, and `scratch_binding` binds scratch or is `null` for RAM. The export and network are typed, `null` when absent. A sandbox that `snapshot restore` provisioned also records its snapshot's name under `source_snapshot`, the absolute path of the snapshot's store under `source_store`, and the program and arguments of the workload that each restore runs under `workload`; one that boots cold records none of them. An atomic pretty-JSON write uses mode 0600, and start reads the same strict typed fields without a legacy reader. | +| `config.json` | The provisioned sandbox, in format 10 only: `image` holds its format, path and identity; `upper` is tagged as scratch with its path or RAM with `max_memory_percent`. `distro_binding` binds the image, and `scratch_binding` binds scratch or is `null` for RAM. The export is typed, `null` when absent. `network`, `null` without `--net`, holds `--net`, `--network-egress`, `--network-ingress`, `--host-loopback`, `--network-proxy`, each `null` when not given, and the `--host-loopback-forward` options, as given, with the egress rules in OpenVMM's form, from a policy file's compile if one was given. A sandbox that `snapshot restore` provisioned also records its snapshot's name under `source_snapshot`, the absolute path of the snapshot's store under `source_store`, and the program and arguments of the workload that each restore runs under `workload`; one that boots cold records none of them. An atomic pretty-JSON write uses mode 0600, and start reads the same strict typed fields without a legacy reader. | | `sandbox.lock` | The store's lock, which serializes the lifecycle transitions | | `exec.lock` | The exec lock, which an `exec` holds from before it finds the running VM until its workload ends, so that one `exec` at a time runs in the sandbox; it stays from the first `exec` until `deprovision` | | `launch.json` | The store's journal of a start in progress, or of one that was interrupted | diff --git a/nvx-rs/src/host/cli/run.rs b/nvx-rs/src/host/cli/run.rs index f6d47e6e1..2d82f1252 100644 --- a/nvx-rs/src/host/cli/run.rs +++ b/nvx-rs/src/host/cli/run.rs @@ -63,9 +63,7 @@ use super::{Exit, Result, check_interrupt, output}; use crate::ErrorCode; use crate::activation::{MachineIdMode, View}; use crate::host::artifacts::{ArtifactDir, Guest, require_artifact}; -use crate::host::config::{ - Boot, ConsoleAttachment, NetworkConfig, NicConfig, RootConfig, VmConfig, -}; +use crate::host::config::{Boot, ConsoleAttachment, NicConfig, RootConfig, VmConfig}; use crate::host::filesystem::{ self, ExportPlan, ExportRequest, Mapping, Owner, PathList, ReservedTargets, Target, }; @@ -643,17 +641,7 @@ fn cold_vm_config( rules: &NativeRules, filesystem: Option, ) -> Result { - let network = sandbox_network(&options.network, rules)? - .map(|network| -> Result { - Ok(NetworkConfig { - guest_network: Some( - crate::net::Ipv4Cidr::parse(network.guest_network()) - .map_err(|error| Exit::error(ErrorCode::Unsupported, error.to_string()))?, - ), - policy: network.policy().clone(), - }) - }) - .transpose()?; + let network = sandbox_network(&options.network, rules)?; let config = VmConfig { processors: options.processors, memory_mib: memory_mib(options), diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs index ed1b61918..589471f2b 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs @@ -31,7 +31,8 @@ use crate::host::cli::artifacts::{Located, locate}; use crate::host::cli::deadline::RETRY_INTERVAL; use crate::host::cli::sandbox::lock::LifecycleLock; use crate::host::cli::sandbox::state::{ - Config, RecordedDistro, RecordedScratch, Source, read_config, serialize_launch, write_json, + Config, NetworkRecord, RecordedDistro, RecordedScratch, Source, read_config, serialize_launch, + write_json, }; use crate::host::cli::sandbox::{Artifacts, error_with_cause}; use crate::host::cli::session::Session; @@ -68,6 +69,8 @@ pub(crate) struct ProvisionConfig<'a> { pub(crate) cmdline: &'a str, /// The RAM upper's cap, or the default when omitted. pub(crate) ram_upper_percent: Option, + /// The network options, or `None` without `--net`. + pub(crate) network: Option<&'a NetworkRecord>, } /// The control features that the layered sandbox root must provide for the requests of diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs index 4dae1ee6d..011ab2541 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs @@ -24,7 +24,7 @@ use crate::host::cli::download::Hold; use crate::host::cli::exec_lock::ExecLock; use crate::host::cli::sandbox::lock::LifecycleLock; use crate::host::cli::sandbox::state::{ - Config, RecordedDistro, RecordedScratch, Source, read_config, serialize_launch, + Config, NetworkRecord, RecordedDistro, RecordedScratch, Source, read_config, serialize_launch, }; use crate::host::cli::sandbox::{ Artifacts, Options, error_with_cause, hypervisor, path_string, refuse_own_files, @@ -538,9 +538,10 @@ pub(crate) fn exec_workload( /// the workload ended, or nothing for a dry run, which launches nothing. /// /// The VM is the one that `start` launches for the configuration that `provision` records for -/// `launch`, whose distro layer `distro` is at the path that `provision` records, with the options -/// of `options`: the run serializes that configuration and reads it back as `start` does, then -/// launches it as `start` does, but in a temporary VM directory rather than a state directory. +/// `launch`, whose distro layer `distro` is at the path that `provision` records, and `network`, +/// the record of its network options, if any, with the options of `options`: the run serializes +/// that configuration and reads it back as `start` does, then launches it as `start` does, but in +/// a temporary VM directory rather than a state directory. /// Before it downloads or launches anything, it refuses what `start` refuses of the configuration /// and `exec` of `request`, after a missing artifact; it then refuses the shares that `start` /// refuses, with its VM directory in place of a state directory, and those of the OpenVMM that it @@ -555,6 +556,7 @@ pub(crate) fn exec_workload( pub(crate) fn run( launch: &SandboxLaunch, distro: &RecordedDistro, + network: Option<&NetworkRecord>, artifacts: &Artifacts<'_>, options: &Options, request: &ExecRequest, @@ -566,6 +568,7 @@ pub(crate) fn run( memory_mib: options.memory_mib, cmdline: &options.cmdline, ram_upper_percent: options.ram_upper_percent, + network, }; let recorded = record(launch, distro, &provision).and_then(|(config, launch)| { // As `start` does, refuse the user's own nvx_* tokens, and the command line that the diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs index 0bddb5b4d..3e35b3a9e 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs @@ -56,6 +56,7 @@ fn run_checks_again_that_its_shares_do_not_overlap() { memory_mib: 256, cmdline: "", ram_upper_percent: None, + network: None, }; record(&launch, &distro, &config).unwrap(); diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs index 0536ef688..1257c9523 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs @@ -19,7 +19,6 @@ use crate::host::cli::sandbox::sandbox_network; use crate::host::cli::sandbox::state::{RecordedDistro, serialize_launch, write_json}; use crate::host::cli::snapshots::{self, Name}; use crate::host::cli::store::StateDir; -use crate::host::config::NetworkConfig; use crate::host::filesystem::{Bind, ExportChild, ExportPlan, Owner}; use crate::host::network::{Access, NetworkPolicy}; use crate::host::paths::resolve; @@ -31,7 +30,6 @@ use crate::host::sandbox::options::sandbox_mounts; use crate::host::sandbox::{SandboxLaunch, SandboxSettings, SandboxUpper}; use crate::host::snapshot::tests::RestoreFixture; use crate::host::snapshot::{DESCRIPTION_FILE, KEY_FILE, OPENVMM_DIR}; -use crate::net::Ipv4Cidr; use crate::protocol::root::RootImage; use crate::protocol::{ APP_ACTIVATE, APP_EXEC, APP_EXIT, APP_HEADER_LEN, APP_MAPS, APP_READY, APP_STDOUT, OUTER_DATA, @@ -385,12 +383,8 @@ fn a_one_off_restore_installs_the_network_options_of_run_as_its_own_policy() { .nic(); let network = run::restored_network(options.network, nic).unwrap(); let rules = network.egress_rules().unwrap(); - let network = sandbox_network(&network, &rules).unwrap().unwrap(); let mut config = fixture.restore.config.clone(); - config.network = Some(NetworkConfig { - guest_network: Some(Ipv4Cidr::parse(network.guest_network()).unwrap()), - policy: network.policy().clone(), - }); + config.network = sandbox_network(&network, &rules).unwrap(); fixture.start("state", config, "ready", 30.0).unwrap(); // The guest learns the DNS server that the policy lets it reach, and announces its listeners, // which the policy publishes on host loopback. @@ -691,6 +685,7 @@ fn deprovision_removes_a_restored_sandboxs_view_only_under_its_snapshots_lock() memory_mib: 512, cmdline: "", ram_upper_percent: None, + network: None, }; let distro = RecordedDistro::new(&launch, binding).unwrap(); let text = serialize_launch(&launch, &distro, None, &options, Some(&source)).unwrap(); @@ -742,6 +737,7 @@ fn a_restored_start_refuses_a_share_of_its_snapshot_store() { memory_mib: 512, cmdline: "", ram_upper_percent: None, + network: None, }; let distro = RecordedDistro::new(&launch, binding).unwrap(); let source = fixture.source(); diff --git a/nvx-rs/src/host/cli/sandbox/mod.rs b/nvx-rs/src/host/cli/sandbox/mod.rs index e5def05a8..ff3edcb3f 100644 --- a/nvx-rs/src/host/cli/sandbox/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/mod.rs @@ -45,8 +45,8 @@ pub(super) use self::options::{ HYPERVISORS, MAX_MEMORY_MIB, MOUNT_OWNERS, OPTIONS, hypervisor, parse, path_text, }; pub(crate) use self::options::{Options, path_string}; -use self::state::RecordedDistro; pub(crate) use self::state::Source; +use self::state::{NetworkRecord, RecordedDistro}; /// The artifacts of a sandbox, found when an operation first needs them. pub(crate) struct Artifacts<'a> { @@ -293,8 +293,9 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re // `provision` records its binding, which `start` checks without reading the layer again. let binding = validate_sandbox_systemd_policy(&launch)?; // The network's policy is checked before anything launches or is recorded. - let launch = launch.with_network(sandbox_network(&options.network, &rules)?); - Some((launch, binding)) + sandbox_network(&options.network, &rules)?; + let network = NetworkRecord::new(&options.network, &rules); + Some((launch, binding, network)) } else { None }; @@ -317,7 +318,7 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re inherit_default_environment: options.inherit_default_environment, }; match (operation, launch) { - (SandboxOperation::Provision, Some((launch, binding))) => { + (SandboxOperation::Provision, Some((launch, binding, network))) => { // `provision` records the layer at its resolved path, where `start` checks the // binding, with the manifest beside that path. let distro = RecordedDistro::new(&launch, binding)?; @@ -339,6 +340,7 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re memory_mib: options.memory_mib, cmdline: &options.cmdline, ram_upper_percent: options.ram_upper_percent, + network: network.as_ref(), }, source, options.timeout, @@ -377,7 +379,7 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re (SandboxOperation::Deprovision, _) => { lifecycle::deprovision(state_dir("deprovision")?, &artifacts, options.timeout) } - (SandboxOperation::Run, Some((launch, binding))) => { + (SandboxOperation::Run, Some((launch, binding, network))) => { if options.state_dir.is_some() { return Err(Exit::error( ErrorCode::Unsupported, @@ -387,8 +389,15 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re // A run launches the layer at the path that a provision would record. let distro = RecordedDistro::new(&launch, binding)?; let held = early.transpose()?.unwrap_or_else(Hold::none); - let Some(result) = - lifecycle::run(&launch, &distro, &artifacts, &options, &request, held)? + let Some(result) = lifecycle::run( + &launch, + &distro, + network.as_ref(), + &artifacts, + &options, + &request, + held, + )? else { return Ok(()); }; diff --git a/nvx-rs/src/host/cli/sandbox/options.rs b/nvx-rs/src/host/cli/sandbox/options.rs index bf82e4c95..c48a1cd5e 100644 --- a/nvx-rs/src/host/cli/sandbox/options.rs +++ b/nvx-rs/src/host/cli/sandbox/options.rs @@ -7,15 +7,17 @@ use crate::ErrorCode; use crate::host::cli::args::{Parser, Receiver, Spec, Syntax, choice, integer}; use crate::host::cli::network::{NativeRules, NetworkOptions}; use crate::host::cli::{Exit, Result}; +use crate::host::config::NetworkConfig; use crate::host::network::{ Access, EgressRule, EgressRules, HostLoopback, LoopbackForward, NetworkPolicy, NetworkProxy, + openvmm_arguments, }; use crate::host::paths; use crate::host::sandbox::distro::DistroFormat; use crate::host::sandbox::mount::{MountOwner, PolicyKind}; use crate::host::sandbox::options::{PolicyPathOption, SandboxOperation}; use crate::host::sandbox::{ - DEFAULT_ENTRYPOINT, DEFAULT_HOSTNAME, DEFAULT_WORKLOAD_IDENTITY, SandboxLayer, SandboxNetwork, + DEFAULT_ENTRYPOINT, DEFAULT_HOSTNAME, DEFAULT_WORKLOAD_IDENTITY, SandboxLayer, parse_workload_identity, }; use crate::net::{Ipv4Cidr, guest_gateway}; @@ -402,8 +404,8 @@ pub(crate) fn path_string(path: &Path) -> Result { /// Returns the network that `network` describes, with `rules`, the egress /// rules that [`NetworkOptions::egress_rules`] resolved: the typed policy at the guest network of -/// `--net`, which [`SandboxNetwork::new`] checks before any launch, or `None` without `--net`, which -/// no other network option may then accompany. +/// `--net`, which it checks before any launch, as OpenVMM lowers it, or `None` without `--net`, +/// which no other network option may then accompany. /// /// Omitted options take OpenVMM's defaults: egress allowed, ingress denied, and host-loopback /// access from the guest alone, [`HostLoopback::Outbound`]. As OpenVMM does, the policy takes @@ -413,7 +415,7 @@ pub(crate) fn path_string(path: &Path) -> Result { pub(in crate::host::cli) fn sandbox_network( network: &NetworkOptions, rules: &NativeRules, -) -> Result> { +) -> Result> { let Some(net) = &network.net else { let network_options = network.network_egress.is_some() || network.network_ingress.is_some() @@ -433,7 +435,8 @@ pub(in crate::host::cli) fn sandbox_network( }; // `Ipv4Cidr` gives a guest network's refusal, and one that it accepts has a gateway: its // network's first address. - Ipv4Cidr::parse(net).map_err(|message| Exit::error(ErrorCode::Unsupported, message))?; + let guest_network = + Ipv4Cidr::parse(net).map_err(|message| Exit::error(ErrorCode::Unsupported, message))?; let gateway = guest_gateway(net).expect("a guest network that parses has a gateway"); if network.network_egress.is_none() && (!rules.allow.is_empty() || !rules.deny.is_empty()) { return Err(Exit::error( @@ -478,5 +481,10 @@ pub(in crate::host::cli) fn sandbox_network( .map_err(|error| refused("--network-proxy", error))?; policy.proxy = Some(proxy); } - Ok(Some(SandboxNetwork::new(net, policy)?)) + // A policy that OpenVMM's options cannot express is refused before anything launches. + openvmm_arguments(Some(&policy), net)?; + Ok(Some(NetworkConfig { + guest_network: Some(guest_network), + policy, + })) } diff --git a/nvx-rs/src/host/cli/sandbox/state.rs b/nvx-rs/src/host/cli/sandbox/state.rs index c2dbcaf92..66e034f1b 100644 --- a/nvx-rs/src/host/cli/sandbox/state.rs +++ b/nvx-rs/src/host/cli/sandbox/state.rs @@ -1,10 +1,14 @@ -//! The CLI's typed `config.json`, in format 9 only: one EROFS or GPT image by path, +//! The CLI's typed `config.json`, in format 10 only: one EROFS or GPT image by path, //! a scratch upper by path or a RAM upper with its explicit cap, the image bindings, //! and the workload's view, network and export. //! -//! Provision checks the same [`VmConfig`] that start reconstructs. Older records are -//! refused, not migrated. Bindings still check the recorded caller-owned paths before -//! launch, and the caller keeps those paths stable while the VM uses them. +//! Provision checks the same [`VmConfig`] that start reconstructs. Older records, those of format +//! 9 among them, are refused, not migrated, with the advice to deprovision the sandbox and +//! provision it again. Bindings still check the recorded caller-owned paths before launch, and the +//! caller keeps those paths stable while the VM uses them. +//! +//! The network is the [`NetworkRecord`] of the sandbox's network options, from which each start +//! builds the network's policy again, as provision built it. //! //! The configuration of a restored sandbox also records its [`Source`]: the name of its snapshot //! under `source_snapshot`, the absolute path of the snapshot's store under `source_store`, and @@ -20,10 +24,11 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use super::lifecycle::ProvisionConfig; -use super::{MAX_MEMORY_MIB, path_string}; +use super::{MAX_MEMORY_MIB, path_string, sandbox_network}; use crate::ErrorCode; use crate::activation::{Journal, MachineIdMode, OverlayMode, View}; use crate::host::cli::json::{Object, pretty_serde, read_object}; +use crate::host::cli::network::{NativeRules, NetworkOptions}; use crate::host::cli::output::LINE_SEPARATOR; use crate::host::cli::snapshots::Name as SnapshotName; use crate::host::cli::{Exit, Result, io_error, io_message}; @@ -35,8 +40,7 @@ use crate::host::paths; use crate::host::sandbox::distro::{DISTRO_POLICY_REVISION, DistroBinding, DistroFormat}; use crate::host::sandbox::layer::{LayerRole, SandboxLayer}; use crate::host::sandbox::scratch::ScratchBinding; -use crate::host::sandbox::{SandboxLaunch, SandboxNetwork, SandboxSettings, SandboxUpper}; -use crate::net::Ipv4Cidr; +use crate::host::sandbox::{SandboxLaunch, SandboxSettings, SandboxUpper}; use crate::protocol::root::{RAM_UPPER_CAPS, RootImage}; use crate::uuid::Uuid; @@ -46,7 +50,7 @@ pub(crate) use crate::host::cli::{hex, random_bytes, temporary_path}; mod tests; /// The typed image and upper configuration that provision writes and start reads. -pub(crate) const CONFIG_FORMAT: u64 = 9; +pub(crate) const CONFIG_FORMAT: u64 = 10; /// Every configuration format that `start` reads. pub(crate) const CONFIG_FORMATS: [u64; 1] = [CONFIG_FORMAT]; /// What messages call a configuration. @@ -210,6 +214,87 @@ pub(crate) struct Source { pub(crate) workload: Vec, } +/// The network options of a sandbox that `provision` records, from which each `start` builds the +/// network again with [`sandbox_network`], as `provision` built it: `--net`, `--network-egress`, +/// `--network-ingress`, `--host-loopback`, `--network-proxy`, and the `--host-loopback-forward` +/// options, as given, and the egress rules in OpenVMM's form, as +/// [`NetworkOptions::egress_rules`] resolved them, from a policy file's compile if one was given, +/// which no start reads again. The record requires every field, an option that was not given as +/// `null`. +#[derive(Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct NetworkRecord { + /// `--net`: the guest's address and prefix. + net: String, + /// `--network-egress`, if given. + #[serde(deserialize_with = "Option::deserialize")] + egress: Option, + /// `--network-ingress`, if given. + #[serde(deserialize_with = "Option::deserialize")] + ingress: Option, + /// The rules that permit destinations. + egress_allow: Vec, + /// The rules that block destinations. + egress_deny: Vec, + /// `--host-loopback`, if given. + #[serde(deserialize_with = "Option::deserialize")] + host_loopback: Option, + /// The `--host-loopback-forward` options. + host_loopback_forwards: Vec, + /// `--network-proxy`, if given. + #[serde(deserialize_with = "Option::deserialize")] + proxy: Option, +} + +impl NetworkRecord { + /// Returns the record of the network that `network` describes with `rules`, the egress rules + /// that [`NetworkOptions::egress_rules`] resolved, or `None` without `--net`. The caller + /// checks the network with [`sandbox_network`] first. + pub(crate) fn new(network: &NetworkOptions, rules: &NativeRules) -> Option { + Some(Self { + net: network.net.clone()?, + egress: network.network_egress.map(str::to_owned), + ingress: network.network_ingress.map(str::to_owned), + egress_allow: rules.allow.clone(), + egress_deny: rules.deny.clone(), + host_loopback: network.host_loopback.map(str::to_owned), + host_loopback_forwards: network.host_loopback_forward.clone(), + proxy: network.network_proxy.clone(), + }) + } + + /// Returns the network that the record describes, as [`sandbox_network`] builds it from the + /// options that it holds. A record whose options their parser or [`sandbox_network`] refuses + /// fails as `sandbox configuration is malformed`, as the old plan's network did when read. + fn config(&self) -> Result { + let mut network = NetworkOptions::default(); + let options = [ + ("--net", Some(&self.net)), + ("--network-egress", self.egress.as_ref()), + ("--network-ingress", self.ingress.as_ref()), + ("--host-loopback", self.host_loopback.as_ref()), + ("--network-proxy", self.proxy.as_ref()), + ]; + let forwards = self + .host_loopback_forwards + .iter() + .map(|forward| ("--host-loopback-forward", Some(forward))); + for (name, value) in options.into_iter().chain(forwards) { + if let Some(value) = value { + network.take(name, value).map_err(|_| malformed())?; + } + } + let rules = NativeRules { + allow: self.egress_allow.clone(), + deny: self.egress_deny.clone(), + }; + sandbox_network(&network, &rules) + .ok() + .flatten() + .ok_or_else(malformed) + } +} + /// Returns the JSON text of the configuration that `provision` records for `launch` and the other /// options, with `distro`, the launch's distro layer at the path that `provision` verified and /// records, and its binding, and `scratch`, the launch's scratch image at the path that @@ -243,7 +328,7 @@ pub(crate) fn serialize_launch( workload_gid, hypervisor: config.hypervisor.to_owned(), memory_mib: config.memory_mib, - network: launch.network().cloned(), + network: config.network.cloned(), cmdline: config.cmdline.to_owned(), distro_binding: distro.binding.clone(), scratch_binding: scratch.map(|scratch| scratch.binding.clone()), @@ -262,9 +347,19 @@ pub(crate) fn serialize_launch( } /// Reads the configuration at `path`, which must be an object of one of the -/// [`CONFIG_FORMATS`], as [`read_object`] reads it, and fails as it does. +/// [`CONFIG_FORMATS`], as [`read_object`] reads it, and fails as it does; a configuration of +/// another format, such as an older one, also says to deprovision the sandbox and provision it +/// again. pub(crate) fn read_config(path: &Path) -> Result { - read_object(path, DESCRIPTION, "format", &CONFIG_FORMATS) + read_object(path, DESCRIPTION, "format", &CONFIG_FORMATS).map_err(|error| { + match (error.code(), error.message()) { + (Some(ErrorCode::FormatUnsupported), Some(message)) => Exit::error( + ErrorCode::FormatUnsupported, + format!("{message}; deprovision the sandbox and provision it again"), + ), + _ => error, + } + }) } /// The sole configuration form, shared by provision's writer and start's reader. @@ -293,7 +388,7 @@ pub(crate) struct Config { #[serde(deserialize_with = "Option::deserialize")] scratch_binding: Option, #[serde(deserialize_with = "Option::deserialize")] - network: Option, + network: Option, /// The immutable named snapshot that supplies a restored sandbox's view, absent for a cold /// launch. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -328,8 +423,8 @@ impl Config { } /// Returns the fields of `object`, a configuration that [`read_config`] read, which fails as - /// `sandbox configuration is malformed` without a field that it requires, or with a value of - /// another type. + /// `sandbox configuration is malformed` without a field that it requires, with a value of + /// another type, or with a network that its record cannot build again. /// /// Guest RAM that is a number, but not an integer from 1 to [`MAX_MEMORY_MIB`], the range of /// `--memory-mib`, fails as `sandbox configuration records N MiB of guest RAM`, which names @@ -369,7 +464,13 @@ impl Config { )); } - serde_json::from_value(Value::Object(object)).map_err(|_| malformed()) + let config: Self = + serde_json::from_value(Value::Object(object)).map_err(|_| malformed())?; + // The network is checked as it is read, as the old plan's network was. + if let Some(network) = &config.network { + network.config()?; + } + Ok(config) } /// Returns the hypervisor that OpenVMM uses. @@ -400,9 +501,7 @@ impl Config { ..SandboxSettings::default() }, )?; - let launch = launch - .with_export(self.export.clone())? - .with_network(self.network.clone()); + let launch = launch.with_export(self.export.clone())?; launch.validated()?; Ok(launch) } @@ -450,18 +549,11 @@ impl Config { command_line.push(' '); command_line.push_str(&self.cmdline); } - let network = - launch - .network() - .map(|network| -> Result { - Ok(NetworkConfig { - guest_network: Some(Ipv4Cidr::parse(network.guest_network()).map_err( - |error| Exit::error(ErrorCode::Unsupported, error.to_string()), - )?), - policy: network.policy().clone(), - }) - }) - .transpose()?; + let network = self + .network + .as_ref() + .map(NetworkRecord::config) + .transpose()?; Ok(VmConfig { processors: 1, memory_mib: self.memory_mib, diff --git a/nvx-rs/src/host/cli/sandbox/state/tests.rs b/nvx-rs/src/host/cli/sandbox/state/tests.rs index 89f952492..d4bf9e8fd 100644 --- a/nvx-rs/src/host/cli/sandbox/state/tests.rs +++ b/nvx-rs/src/host/cli/sandbox/state/tests.rs @@ -22,6 +22,7 @@ fn restored_sandbox_records_round_trip_only_their_own_source() { memory_mib: 128, cmdline: "", ram_upper_percent: None, + network: None, }; for name in [None, Some("My.snapshot-1"), Some("other")] { let source = name.map(|name| Source { diff --git a/nvx-rs/tests/cli.rs b/nvx-rs/tests/cli.rs index 8888c6d9f..3fc99510c 100644 --- a/nvx-rs/tests/cli.rs +++ b/nvx-rs/tests/cli.rs @@ -1640,25 +1640,25 @@ fn network_policy_files_compile_into_the_typed_policy() { ); } -/// The serde form of a valid proxy-only network, as a configuration records it. +/// The record of a valid proxy-only network, as a configuration records its network options. fn recorded_proxy_only_network() -> Value { json!({ - "guestNetwork": "10.0.0.2/24", + "net": "10.0.0.2/24", "egress": "deny", - "allow": {"name": "--network-egress-allow", "rules": []}, - "deny": {"name": "--network-egress-deny", "rules": []}, "ingress": "deny", - "hostLoopback": "deny", - "hostLoopbackForwards": [], - "proxy": {"port": 3128, "scheme": null}, + "egress_allow": [], + "egress_deny": [], + "host_loopback": "deny", + "host_loopback_forwards": [], + "proxy": "10.0.0.1:3128", }) } #[test] fn start_refuses_a_configuration_without_a_valid_network_policy() { - // A configuration records the typed network that provision checked, which start reads and - // checks again; no reader reads the network options that nvx.py and earlier releases - // recorded. + // A configuration records the network options that provision checked, from which start builds + // the network and checks it again; no reader reads the network options that nvx.py and + // earlier releases recorded. let root = Root::new(); let malformed = "sandbox configuration is malformed".to_owned(); let cases: [(&str, ConfigEdit, &str, String); 5] = [ diff --git a/nvx-rs/tests/cli/formats.rs b/nvx-rs/tests/cli/formats.rs index 4f44826f1..b0e3256c8 100644 --- a/nvx-rs/tests/cli/formats.rs +++ b/nvx-rs/tests/cli/formats.rs @@ -27,7 +27,7 @@ fn configurations_of_every_format_start_and_exec() { root.nvx_rs(&sandbox("provision", "{root/state}", &options)) .assert_silent_success(); let record = json(&root.below("state/config.json")); - assert_eq!(record["format"], 9, "{context}"); + assert_eq!(record["format"], 10, "{context}"); assert_eq!( record["image"], root.expand_json(&json!({ @@ -110,6 +110,8 @@ fn old_configuration_formats_are_refused_without_migration() { include_str!("../fixtures/state/format-7.json"), include_str!("../fixtures/state/format-7-without-shares.json"), r#"{"format":8}"#, + // The format before the CLI recorded its own network options. + r#"{"format":9}"#, ] { let root = Root::new(); root.nvx_rs(&provision("{root/state}", &[])) @@ -117,7 +119,13 @@ fn old_configuration_formats_are_refused_without_migration() { root.write("state/config.json", text); let before = root.snapshot(); root.nvx_rs(&sandbox::<&str>("start", "{root/state}", &[])) - .assert_error("format-unsupported", &["unsupported format"]); + .assert_error( + "format-unsupported", + &[ + "unsupported format", + "deprovision the sandbox and provision it again", + ], + ); assert_eq!(root.snapshot(), before); assert_eq!(entries(&root.below("state")), PROVISIONED); root.nvx_rs(&sandbox::<&str>("deprovision", "{root/state}", &[])) diff --git a/nvx-rs/tests/cli/lifecycle.rs b/nvx-rs/tests/cli/lifecycle.rs index e615256f3..caf2a601e 100644 --- a/nvx-rs/tests/cli/lifecycle.rs +++ b/nvx-rs/tests/cli/lifecycle.rs @@ -71,13 +71,13 @@ pub(crate) fn provision(state: &str, options: &[&str]) -> Vec { sandbox("provision", state, &all) } -/// Returns the configuration that provision records for the minimal sandbox, in format 9, with +/// Returns the configuration that provision records for the minimal sandbox, in format 10, with /// placeholders, and without the bindings of its images, which [`assert_bindings`] checks. pub(crate) fn minimal_config() -> Value { json!({ "cmdline": "", "export": null, - "format": 9, + "format": 10, "hostname": "nvx-sandbox", "hypervisor": HYPERVISOR, "image": {"path": "{root/distro.erofs}", "format": "erofs", "identity": UUID}, @@ -373,7 +373,8 @@ fn configurations() -> Vec { file.maps = Some(json!([mapped("0", "/etc/single.txt", true, false)])); // A managed sandbox runs the guest agent, whatever entrypoint and arguments provision gets, - // and records the policy file's rules in its typed network. + // and records its network options, with the policy file's rules in OpenVMM's form, from which + // each start builds the policy again. let mut network = configuration( "network and launch options", vec![ @@ -418,28 +419,13 @@ fn configurations() -> Vec { ( "network", json!({ - "allow": { - "name": "--network-egress-allow", - "rules": [{ - "ports": [{"endPort": null, "port": 443, "protocol": "tcp"}], - "to": [{"cidr": "192.0.2.0/24", "except": []}], - }], - }, - "deny": { - "name": "--network-egress-deny", - "rules": [{ - "ports": [], - "to": [{"cidr": "192.0.2.128/25", "except": []}], - }], - }, "egress": "allow", - "guestNetwork": "10.0.0.2/24", - "hostLoopback": "allow", - "hostLoopbackForwards": [ - {"guestPort": 8080, "hostPort": 18080, "protocol": "tcp"}, - {"guestPort": 9090, "hostPort": 19090, "protocol": "udp"}, - ], + "egress_allow": ["192.0.2.0/24:tcp:443"], + "egress_deny": ["192.0.2.128/25"], + "host_loopback": "allow", + "host_loopback_forwards": ["tcp:18080:8080", "udp:19090:9090"], "ingress": "deny", + "net": "10.0.0.2/24", "proxy": null, }), ), @@ -491,14 +477,14 @@ fn configurations() -> Vec { { let mut config = minimal_config(); config["network"] = json!({ - "allow": {"name": "--network-egress-allow", "rules": []}, - "deny": {"name": "--network-egress-deny", "rules": []}, "egress": "deny", - "guestNetwork": "10.0.0.2/24", - "hostLoopback": "deny", - "hostLoopbackForwards": [], - "ingress": "deny", - "proxy": {"port": 3128, "scheme": null}, + "egress_allow": [], + "egress_deny": [], + "host_loopback": "deny", + "host_loopback_forwards": [], + "ingress": null, + "net": "10.0.0.2/24", + "proxy": "10.0.0.1:3128", }); config }, @@ -1250,20 +1236,44 @@ fn malformed_configuration(root: &Root) { ram_with_binding["upper"] = json!({"kind": "ram", "max_memory_percent": 50}); let mut format_1_with_export = without_upper.clone(); format_1_with_export["format"] = json!(1); + // A network record requires each of its fields, and one whose options their parser or the + // network's own checks refuse is malformed, as the old plan's network was. + let mut network = config.clone(); + network["network"] = json!({ + "egress": "deny", + "egress_allow": [], + "egress_deny": [], + "host_loopback": null, + "host_loopback_forwards": [], + "ingress": null, + "net": "10.0.0.2/24", + "proxy": null, + }); + let mut network_without_egress = network.clone(); + network_without_egress["network"] + .as_object_mut() + .unwrap() + .remove("egress"); + let mut network_of_a_gateway = network.clone(); + network_of_a_gateway["network"]["net"] = json!("10.0.0.1/24"); + let mut network_with_another_egress = network.clone(); + network_with_another_egress["network"]["egress"] = json!("sometimes"); + let mut network_with_a_broken_rule = network.clone(); + network_with_a_broken_rule["network"]["egress_allow"] = json!(["192.0.2.0/33"]); let unsupported = &["unsupported format", "{root/state/config.json}"][..]; let missing_export = &["sandbox configuration has no export plan"][..]; let malformed = &["sandbox configuration is malformed"][..]; for (text, code, fragments) in [ ( - r#"{"format": 10}"#.to_owned(), + r#"{"format": 11}"#.to_owned(), "format-unsupported", unsupported, ), ("[]".to_owned(), "format-unsupported", unsupported), - // Only the integer 9 is a format, and the configuration is strict JSON, in + // Only the integer 10 is a format, and the configuration is strict JSON, in // which no object repeats a field. ( - r#"{"format": 9.0}"#.to_owned(), + r#"{"format": 10.0}"#.to_owned(), "format-unsupported", unsupported, ), @@ -1316,6 +1326,10 @@ fn malformed_configuration(root: &Root) { "format-unsupported", unsupported, ), + (network_without_egress.to_string(), "io", malformed), + (network_of_a_gateway.to_string(), "io", malformed), + (network_with_another_egress.to_string(), "io", malformed), + (network_with_a_broken_rule.to_string(), "io", malformed), ] { root.write("state/config.json", &text); let before = root.snapshot(); diff --git a/scripts/nvx_tools/sandbox_ram_upper_tests.py b/scripts/nvx_tools/sandbox_ram_upper_tests.py index 52fa0de9e..493b8c877 100644 --- a/scripts/nvx_tools/sandbox_ram_upper_tests.py +++ b/scripts/nvx_tools/sandbox_ram_upper_tests.py @@ -220,7 +220,7 @@ def check_managed(self) -> None: name: config[name] for name in ("format", "upper", "scratch_binding") } if recorded != { - "format": 9, + "format": 10, "upper": {"kind": "ram", "max_memory_percent": 50}, "scratch_binding": None, } or any(name in config for name in ("layers", "scratch")): diff --git a/scripts/test_microvm_tests.py b/scripts/test_microvm_tests.py index 9c2cb664c..6ad80c10f 100644 --- a/scripts/test_microvm_tests.py +++ b/scripts/test_microvm_tests.py @@ -2759,7 +2759,7 @@ def _provision( config = json.loads(path.read_text(encoding="utf-8")) _role, image, identity = values["--layer"].split(",") config.update( - format=9, + format=10, image={"format": "erofs", "path": image, "identity": identity}, upper={"kind": "ram", "max_memory_percent": 50}, scratch_binding=None, @@ -2767,7 +2767,7 @@ def _provision( if "records-scratch" in self.faults: config["upper"] = {"kind": "scratch", "path": "scratch.ext4"} if "records-old-format" in self.faults: - config["format"] = 8 + config["format"] = 9 if "records-other-cap" in self.faults: config["upper"]["max_memory_percent"] = 51 path.write_text(json.dumps(config), encoding="utf-8") @@ -3054,7 +3054,7 @@ def _provision( upper = "ram" if upper == "scratch" else "scratch" image = "erofs" if "records-erofs" in self.faults else "gpt" config.update( - format=9, + format=10, image={"format": image, "path": disk, "identity": guid}, upper=( {"kind": "ram", "max_memory_percent": 50}