From 3ed1a5a2d73ce3f40edc7bac95b37173422f1985 Mon Sep 17 00:00:00 2001 From: Marko Vejnovic Date: Mon, 12 Oct 2026 20:59:01 -0700 Subject: [PATCH] refactor(networking): tear out VM networking for ground-up rewrite Remove all VM networking implementation so the subsystem can be rebuilt from a blank state. Interfaces are kept but marked unimplemented: - suidhelper: network prepare/teardown/teardown-orphan/host-init keep their CLI shape but exit with an unimplemented error and touch nothing; netns/veth/NAT/host-init code, jailer --netns and /dev/net/tun in the chroot jail are gone. - Elixir: Hyper.SuidHelper.Network, Hyper.Node.FireVMM.Net and Hyper.Cfg.Network are unimplemented stubs with no callers. VMs boot without a NIC or ip=/hyper.resolver= cmdline; no network_ready preflight; Reaper no longer sweeps netns (Plan.orphans/3). - guest-agent: no longer writes /etc/resolv.conf. - A legacy [network] config table is accepted and ignored. - Docs, setup.sh and provision-kvm-host.sh drop networking setup. net_bw accounting (Sys.Mon.NetBw, NetDev, budgets) is unchanged. --- .github/scripts/provision-kvm-host.sh | 47 +- CHANGELOG.md | 9 + README.md | 3 + docs/cookbook/config.md | 27 +- docs/cookbook/install.md | 69 +-- lib/hyper/cfg/network.ex | 71 +-- lib/hyper/node.ex | 18 +- lib/hyper/node/fire_vmm/boot_spec.ex | 21 +- lib/hyper/node/fire_vmm/daemon.ex | 25 +- lib/hyper/node/fire_vmm/jailer.ex | 42 +- lib/hyper/node/fire_vmm/net.ex | 77 ++-- lib/hyper/node/fire_vmm/state.ex | 23 +- lib/hyper/node/reaper.ex | 36 +- lib/hyper/node/reaper/plan.ex | 12 +- lib/hyper/suid_helper/network.ex | 76 ++- lib/hyper/vm.ex | 4 +- lib/hyper/vm/instance.ex | 5 +- native/guest-agent/Cargo.toml | 6 +- native/guest-agent/src/init.rs | 28 -- native/guest-agent/tests/init.rs | 65 --- native/suidhelper/Cargo.toml | 12 - native/suidhelper/src/config.rs | 76 +-- native/suidhelper/src/tools/jailer.rs | 14 +- native/suidhelper/src/tools/mod.rs | 2 +- native/suidhelper/src/tools/network/addr.rs | 77 ---- native/suidhelper/src/tools/network/args.rs | 405 ---------------- native/suidhelper/src/tools/network/exec.rs | 79 ---- .../suidhelper/src/tools/network/host_init.rs | 106 ----- native/suidhelper/src/tools/network/mod.rs | 96 ++-- .../suidhelper/src/tools/network/prepare.rs | 144 ------ .../suidhelper/src/tools/network/teardown.rs | 70 --- .../src/tools/network/teardown_orphan.rs | 124 ----- native/suidhelper/src/util/chroot_jail.rs | 31 +- native/suidhelper/src/util/jailer.rs | 10 - native/suidhelper/src/util/safe_dir.rs | 57 +-- native/suidhelper/tests/config_network.rs | 47 +- native/suidhelper/tests/e2e/chroot_jail.rs | 82 +--- native/suidhelper/tests/e2e/jailer.rs | 31 +- native/suidhelper/tests/tools/host_init.rs | 44 -- native/suidhelper/tests/tools/network_addr.rs | 79 ---- native/suidhelper/tests/tools/network_args.rs | 435 ------------------ setup.sh | 16 +- test/e2e/network_test.exs | 61 --- test/hyper/cfg/network_test.exs | 45 -- test/hyper/node/fire_vmm/boot_spec_test.exs | 14 +- test/hyper/node/fire_vmm/daemon_test.exs | 47 +- test/hyper/node/fire_vmm/jailer_test.exs | 18 - test/hyper/node/fire_vmm/net_test.exs | 40 -- test/hyper/node/reaper/liveness_test.exs | 12 +- .../node/reaper/plan_properties_test.exs | 16 +- test/hyper/node/reaper/plan_test.exs | 20 +- test/hyper/suid_helper/network_test.exs | 19 - 52 files changed, 273 insertions(+), 2720 deletions(-) delete mode 100644 native/guest-agent/tests/init.rs delete mode 100644 native/suidhelper/src/tools/network/addr.rs delete mode 100644 native/suidhelper/src/tools/network/args.rs delete mode 100644 native/suidhelper/src/tools/network/exec.rs delete mode 100644 native/suidhelper/src/tools/network/host_init.rs delete mode 100644 native/suidhelper/src/tools/network/prepare.rs delete mode 100644 native/suidhelper/src/tools/network/teardown.rs delete mode 100644 native/suidhelper/src/tools/network/teardown_orphan.rs delete mode 100644 native/suidhelper/tests/tools/host_init.rs delete mode 100644 native/suidhelper/tests/tools/network_addr.rs delete mode 100644 native/suidhelper/tests/tools/network_args.rs delete mode 100644 test/e2e/network_test.exs delete mode 100644 test/hyper/cfg/network_test.exs delete mode 100644 test/hyper/node/fire_vmm/net_test.exs delete mode 100644 test/hyper/suid_helper/network_test.exs diff --git a/.github/scripts/provision-kvm-host.sh b/.github/scripts/provision-kvm-host.sh index 00da31d8..07a8e876 100755 --- a/.github/scripts/provision-kvm-host.sh +++ b/.github/scripts/provision-kvm-host.sh @@ -18,9 +18,6 @@ # the node) # - DEBIAN_FRONTEND=noninteractive (install.md is written for an interactive # operator session; CI has no tty to prompt on) -# - [network] is always written here (install.md presents it as an optional -# section) since the runner's default route interface is detected fresh -# each run and integration coverage wants egress networking exercised set -euo pipefail export DEBIAN_FRONTEND=noninteractive @@ -34,20 +31,12 @@ sudo udevadm trigger --name-match=kvm sudo apt-get update sudo apt-get install -y \ - coreutils e2fsprogs iproute2 libc-bin lvm2 nftables skopeo thin-provisioning-tools util-linux \ + coreutils e2fsprogs libc-bin lvm2 skopeo thin-provisioning-tools util-linux \ "linux-modules-extra-$(uname -r)" # -a is load-bearing: without it modprobe reads the 2nd+ names as module # PARAMETERS of the first, and the load fails. -# -# nf_tables + nf_nat back host-init's `hyper` nftables table and its -# masquerade/SNAT/DNAT rules. A fresh GitHub-hosted runner does not have them -# loaded, and the netfilter family does not always autoload on first `nft` use, -# so `nft add table ip hyper` fails (empty stderr, non-zero exit) and the node -# refuses to start. Load the base explicitly, as we do for the dm modules; the -# nat expression sub-modules (nft_chain_nat, nft_masq) autoload once the base is -# present and a rule is issued. -sudo modprobe -av dm_snapshot dm_thin_pool loop nf_tables nf_nat +sudo modprobe -av dm_snapshot dm_thin_pool loop targets="$(sudo dmsetup targets)" echo "dmsetup targets: ${targets}" grep -q thin-pool <<<"${targets}" || { echo "ERROR: thin-pool dm target missing" >&2; exit 1; } @@ -59,33 +48,6 @@ command -v thin_dump >/dev/null || { echo "ERROR: thin_dump missing (thin-provis sudo install -o root -g root -m 0755 "$(readlink -f "$(command -v thin_dump)")" /usr/local/sbin/thin_dump [ ! -L /usr/local/sbin/thin_dump ] || { echo "ERROR: /usr/local/sbin/thin_dump is still a symlink" >&2; exit 1; } -# host-init (run by the node at start) asserts ip_forward=1 rather than -# setting it, so provisioning must turn it on and persist it across reboots. -sudo sysctl -w net.ipv4.ip_forward=1 -echo 'net.ipv4.ip_forward=1' \ - | sudo tee /etc/sysctl.d/99-hyper-ip-forward.conf >/dev/null - -# GitHub-hosted runners ship Docker, which sets the iptables `filter` FORWARD -# policy to DROP and only admits its own bridge traffic. Hyper's guest egress is -# *forwarded* (per-VM netns veth -> uplink), so Docker's drop silently eats -# every packet the guest sends — the guest configures its NIC fine but nothing -# ever returns. Admit the clone pool (172.31.0.0/16, the helper's default) both -# directions via DOCKER-USER, which Docker evaluates before its own drops; the -# `hyper` nftables forward chain still enforces guest isolation. Fall back to an -# accepting FORWARD policy on a host without Docker's chain. -clone_pool="172.31.0.0/16" -if sudo iptables -L DOCKER-USER >/dev/null 2>&1; then - sudo iptables -I DOCKER-USER -s "${clone_pool}" -j ACCEPT - sudo iptables -I DOCKER-USER -d "${clone_pool}" -j ACCEPT -else - sudo iptables -P FORWARD ACCEPT -fi - -# The default-route interface is the uplink guests NAT egress through. -uplink="$(ip route show default | awk '{print $5; exit}')" -[ -n "${uplink}" ] || { echo "ERROR: could not detect default-route uplink interface" >&2; exit 1; } -echo "detected uplink: ${uplink}" - sudo mkdir -p /etc/hyper sudo tee /etc/hyper/config.toml >/dev/null < **VM networking** is being rewritten from the ground up and is currently +> unavailable: VMs boot with no NIC and have no network egress. + ## Docs Full docs on getting started, as well as useful diagrams are available on diff --git a/docs/cookbook/config.md b/docs/cookbook/config.md index 1ede588f..71209120 100644 --- a/docs/cookbook/config.md +++ b/docs/cookbook/config.md @@ -110,29 +110,10 @@ uid_gid_range = [900000, 999999] ## VM Networking Configuration -**Required.** Every VM gets NAT'd egress out through a physical host interface, -and a node refuses to start without a `[network]` table (see the -[install guide](install.md#vm-networking-required) for the host prerequisites: -`iproute2`/`nftables` and `net.ipv4.ip_forward=1`). `config.toml`-only — the -setuid helper reads the same `uplink` and `clone_pool` to build each VM's netns, -veth pair, and NAT rules, so it cannot live in `config.exs`. - -### `config.toml` - -```toml -[network] -# **required**. The physical uplink interface guest egress is NAT'd out through. -# On a single-uplink host, the default-route NIC is the usual choice: -# ip route show default | awk '{print $5; exit}' -uplink = "eth0" - -# optional -- defaults shown. The IPv4 CIDR the per-VM /30s are carved from. -clone_pool = "172.31.0.0/16" - -# optional -- defaults shown. DNS resolver handed to guests via the kernel -# cmdline (the guest agent writes it to /etc/resolv.conf). -resolver = "1.1.1.1" -``` +VM networking is being rewritten from the ground up and is currently +unavailable: VMs boot with no NIC, and a node no longer requires or reads a +`[network]` table. An existing `[network]` table in `config.toml` is accepted +but ignored (see the [install guide](install.md#vm-networking)). ## gRPC Configuration diff --git a/docs/cookbook/install.md b/docs/cookbook/install.md index fe555d98..73cd097b 100644 --- a/docs/cookbook/install.md +++ b/docs/cookbook/install.md @@ -193,71 +193,12 @@ cgroup = "hyper" For more details on configuring and tuning Hyper, we suggest you see the [configuration guide](config.md). -### VM Networking (required) +### VM networking -VM networking is **mandatory**: a node refuses to start without a `[network]` -table in `/etc/hyper/config.toml`, and every VM gets NAT'd egress out through a -physical interface on the host. There is no opt-out — a node that fails this -preflight will not boot (`{:error, :network_not_configured}`, a missing uplink, -or IPv4 forwarding off). The `hyper` nft table itself is created (and owned) by -`host-init`, which runs automatically at node start — you never create nft rules -by hand. You must prepare the host first: - -Install `iproute2` and `nftables`: - -```sh -sudo apt install -y iproute2 nftables # dnf install -y iproute nftables -``` - -Enable IPv4 forwarding. `host-init` only *asserts* this is on — it refuses to -proceed rather than setting it for you: - -```sh -sudo sysctl -w net.ipv4.ip_forward=1 -``` - -> #### Persistent Config {: .warning} -> -> Loading this via `sysctl -w` is ephemeral and resets on reboot. Persist it: -> -> ```sh -> echo 'net.ipv4.ip_forward=1' \ -> | sudo tee /etc/sysctl.d/99-hyper-ip-forward.conf -> ``` - -> #### Conflicting FORWARD firewall {: .warning} -> -> Guest egress is *forwarded* (per-VM netns veth → uplink). If the host already -> runs a firewall that defaults the `filter` FORWARD chain to DROP — Docker does -> this, and so do some hardened base images — those drops silently eat every -> guest packet: the guest configures its NIC correctly but nothing ever returns. -> Admit the clone pool through the firewall's user hook (Docker evaluates -> `DOCKER-USER` before its own drops); the `hyper` nft forward chain still -> enforces guest isolation: -> -> ```sh -> sudo iptables -I DOCKER-USER -s 172.31.0.0/16 -j ACCEPT -> sudo iptables -I DOCKER-USER -d 172.31.0.0/16 -j ACCEPT -> ``` -> -> On a host without Docker, ensure nothing else sets the FORWARD policy to DROP. - -Then set `uplink` to the physical NIC guests should NAT out through — the -default-route interface is a reasonable choice on a single-uplink host: - -```sh -ip route show default | awk '{print $5; exit}' -``` - -```toml -[network] -# **required**. The physical uplink interface guests NAT egress through. -uplink = "eth0" - -# optional, defaults shown -clone_pool = "172.31.0.0/16" -resolver = "1.1.1.1" -``` +VM networking is being rewritten from the ground up and is currently +unavailable: VMs boot with no NIC, so guests have no egress. No `[network]` +table, `nftables`, `iproute2` or IPv4 forwarding is needed on the host. An +existing `[network]` table in `/etc/hyper/config.toml` is accepted but ignored. ### Cgroups diff --git a/lib/hyper/cfg/network.ex b/lib/hyper/cfg/network.ex index a7228973..660ba4d2 100644 --- a/lib/hyper/cfg/network.ex +++ b/lib/hyper/cfg/network.ex @@ -1,53 +1,30 @@ defmodule Hyper.Cfg.Network do @moduledoc """ - VM egress networking settings from the `[network]` table — `config.toml`-only - because the setuid helper reads the same `uplink` and `clone_pool` to build - each VM's netns, veth, and NAT rules. + Unimplemented placeholder pending the VM-networking rewrite. - Networking is **mandatory**: a node refuses to start unless `[network]` is - configured (see `Hyper.Node.FireVMM.Jailer.Checks.network_ready/0`). Every VM - gets a NIC; there is no opt-out. `configured?/0` exists only so the startup - preflight can fail fast with a clear message rather than crash later. + A `[network]` table in `/etc/hyper/config.toml` is tolerated but ignored + (`Hyper.Cfg.Toml` is a loose dotted-path lookup), and a node no longer + requires it to start. Every function raises; nothing in Hyper calls them. """ - import Hyper.Cfg, only: [get_cfg: 1] - - @default_clone_pool "172.31.0.0/16" - @default_resolver "1.1.1.1" - - @doc """ - Whether the required `[network] uplink` is present. Used only by the startup - preflight to fail fast; once a node is running, networking is guaranteed - configured, so runtime code calls `uplink/0` directly. - """ - @spec configured?() :: boolean() - def configured?, do: not is_nil(get_cfg(toml: "network.uplink", default: nil)) - - @doc "Physical uplink interface guest egress is NAT'd out of. Raises if unset (a running node has already passed the preflight)." - @spec uplink() :: String.t() - def uplink do - case get_cfg(toml: "network.uplink", default: nil) do - nil -> - raise Hyper.Cfg.MissingError, "network.uplink is required — VM networking is mandatory" - - v when is_binary(v) -> - v - - other -> - raise ArgumentError, "network.uplink must be a string, got: #{inspect(other)}" - end - end - - @doc "IPv4 CIDR the per-VM clone /30s are carved from. `[network] clone_pool`." - @spec clone_pool() :: String.t() - def clone_pool, do: get_cfg(toml: "network.clone_pool", default: @default_clone_pool) - - @doc """ - DNS resolver IP handed to the guest via the `hyper.resolver=` kernel cmdline - param, since the kernel's `ip=` autoconfig sets the guest's address/route but - never DNS. The PID-1 guest agent reads this back out of `/proc/cmdline` and - writes it to `/etc/resolv.conf`. `[network] resolver`. - """ - @spec resolver() :: String.t() - def resolver, do: get_cfg(toml: "network.resolver", default: @default_resolver) + @doc "Unimplemented: whether VM networking is configured." + @spec configured?() :: no_return() + def configured?, + do: + raise("Hyper.Cfg.Network.configured?/0 is unimplemented: VM networking is being rewritten") + + @doc "Unimplemented: the uplink interface guest egress leaves through." + @spec uplink() :: no_return() + def uplink, + do: raise("Hyper.Cfg.Network.uplink/0 is unimplemented: VM networking is being rewritten") + + @doc "Unimplemented: the address pool per-VM links are carved from." + @spec clone_pool() :: no_return() + def clone_pool, + do: raise("Hyper.Cfg.Network.clone_pool/0 is unimplemented: VM networking is being rewritten") + + @doc "Unimplemented: the DNS resolver handed to guests." + @spec resolver() :: no_return() + def resolver, + do: raise("Hyper.Cfg.Network.resolver/0 is unimplemented: VM networking is being rewritten") end diff --git a/lib/hyper/node.ex b/lib/hyper/node.ex index 01c7d13f..a287b183 100644 --- a/lib/hyper/node.ex +++ b/lib/hyper/node.ex @@ -14,7 +14,7 @@ defmodule Hyper.Node do * `Hyper.Node.VMSupervisor` - a **local** `DynamicSupervisor` that starts one `Hyper.Node.FireVMM` per VM. Local on purpose: a firecracker VM is pinned to - this machine's kernel/rootfs/cgroup/tap devices and cannot migrate, so we + this machine's kernel/rootfs/cgroup and cannot migrate, so we deliberately avoid `Horde.DynamicSupervisor` (which would try to restart VMs on a surviving node - cold-booting a ghost). @@ -310,25 +310,11 @@ defmodule Hyper.Node do :ok <- Hyper.Node.Layer.Repo.test_system(), :ok <- Hyper.SuidHelper.test_system(), {:ok, base} <- Hyper.SuidHelper.sys_test(), - :ok <- check_helper_base(base), - :ok <- init_network() do + :ok <- check_helper_base(base) do Hyper.Node.FireVMM.test_system() end end - # VM networking is mandatory: refuse to start unless `[network]` is - # configured, then idempotently reconcile the host-wide `hyper` nftables - # table. Runs after `Hyper.SuidHelper.test_system/0` so the helper is - # confirmed present first. - @spec init_network :: :ok | {:error, term()} - defp init_network do - if Hyper.Cfg.Network.configured?() do - Hyper.SuidHelper.Network.host_init() - else - {:error, :network_not_configured} - end - end - @spec check_firecracker_bins :: :ok | {:error, {:firecracker_bin_missing | :jailer_bin_missing, Path.t()}} diff --git a/lib/hyper/node/fire_vmm/boot_spec.ex b/lib/hyper/node/fire_vmm/boot_spec.ex index dc663854..189e1547 100644 --- a/lib/hyper/node/fire_vmm/boot_spec.ex +++ b/lib/hyper/node/fire_vmm/boot_spec.ex @@ -20,34 +20,22 @@ defmodule Hyper.Node.FireVMM.BootSpec do defmodule Cold do @moduledoc "A resolved cold boot." @enforce_keys [:machine_config, :boot_source, :drives] - defstruct [:machine_config, :boot_source, drives: [], network_interfaces: []] + defstruct [:machine_config, :boot_source, drives: []] @type t :: %__MODULE__{ machine_config: MachineConfiguration.t(), boot_source: BootSource.t(), - drives: [Drive.t()], - network_interfaces: [Hyper.Firecracker.Api.NetworkInterface.t()] + drives: [Drive.t()] } end @spec resolve(Hyper.Vm.source(), Instance.t()) :: Cold.t() def resolve(source, type) when is_map(source) do - # Networking is mandatory: every VM gets a NIC on the uniform inner-world - # address plus the kernel `ip=`/`hyper.resolver=` cmdline. A node that - # reached here has already passed the startup network preflight. - vm_id = Map.fetch!(source, :vm_id) - - boot_args = - Map.get(source, :boot_args, @default_boot_args) <> - " " <> - Hyper.Node.FireVMM.Net.ip_cmdline() <> - " " <> Hyper.Node.FireVMM.Net.resolver_cmdline(Hyper.Cfg.Network.resolver()) - %Cold{ machine_config: Instance.Spec.machine_config(Instance.spec(type)), boot_source: %BootSource{ kernel_image_path: Map.fetch!(source, :kernel_image_path), - boot_args: boot_args + boot_args: Map.get(source, :boot_args, @default_boot_args) }, drives: [ %Drive{ @@ -56,8 +44,7 @@ defmodule Hyper.Node.FireVMM.BootSpec do is_read_only: false, path_on_host: Map.fetch!(source, :root_drive_path) } - ], - network_interfaces: [Hyper.Node.FireVMM.Net.interface(vm_id)] + ] } end end diff --git a/lib/hyper/node/fire_vmm/daemon.ex b/lib/hyper/node/fire_vmm/daemon.ex index 56713f40..cab99fe4 100644 --- a/lib/hyper/node/fire_vmm/daemon.ex +++ b/lib/hyper/node/fire_vmm/daemon.ex @@ -57,7 +57,7 @@ defmodule Hyper.Node.FireVMM.Daemon do # link kill) and so `terminate/2` runs on supervisor shutdown. Process.flag(:trap_exit, true) - with :ok <- prelaunch(opts), + with :ok <- clear_jail(opts), {:ok, muontrap} <- launch(opts) do {:ok, %__MODULE__{opts: opts, muontrap: muontrap}} else @@ -91,25 +91,12 @@ defmodule Hyper.Node.FireVMM.Daemon do end end - # Clear any stale jail + netns left by a prior incarnation, then prepare the - # fresh netns the jailer's `--netns` will enter. Must run before `launch/1`: - # the jailer refuses to start if the netns it is told to enter does not yet - # exist. Networking is mandatory, so this is unconditional. - @spec prelaunch(Opts.t()) :: :ok | {:error, term()} - defp prelaunch(%Opts{vm_id: id, uid: uid} = opts) do - with :ok <- clear_jail(opts) do - Hyper.SuidHelper.Network.prepare(id, uid) - end - end - - # Best-effort: both the chroot/cgroup removal and the network teardown are - # attempted regardless of the other's outcome (a `Reaper` backstops either - # failure); the first error is surfaced to the caller. + # Remove the VM's chroot and kill its cgroup leaf. On (re)start this clears any + # stale jail left by a prior incarnation, which must happen before `launch/1` + # because the jailer refuses to reuse an existing chroot. @spec clear_jail(Opts.t()) :: :ok | {:error, term()} - defp clear_jail(%Opts{vm_id: id, uid: uid}) do - net = Hyper.SuidHelper.Network.teardown(id, uid) - jail = SuidHelper.ChrootJail.remove(Jailer.chroot_dir(id), Jailer.cgroup_dir(id)) - with :ok <- net, do: jail + defp clear_jail(%Opts{vm_id: id}) do + SuidHelper.ChrootJail.remove(Jailer.chroot_dir(id), Jailer.cgroup_dir(id)) end @spec launch(Opts.t()) :: {:ok, pid()} | {:error, term()} diff --git a/lib/hyper/node/fire_vmm/jailer.ex b/lib/hyper/node/fire_vmm/jailer.ex index eb15b986..cc86040b 100644 --- a/lib/hyper/node/fire_vmm/jailer.ex +++ b/lib/hyper/node/fire_vmm/jailer.ex @@ -38,9 +38,7 @@ defmodule Hyper.Node.FireVMM.Jailer do first failure. """ - alias Hyper.Cfg.{Dirs, Jails, Network} - - @ip_forward_path "/proc/sys/net/ipv4/ip_forward" + alias Hyper.Cfg.{Dirs, Jails} @doc "Run every pre-requisite check, halting at the first failure." @spec run() :: :ok | {:error, term()} @@ -58,46 +56,10 @@ defmodule Hyper.Node.FireVMM.Jailer do &kvm_present/0, &cgroup_v2_available/0, &parent_cgroup_present/0, - &chroot_writable/0, - &network_ready/0 + &chroot_writable/0 ] end - @doc """ - Host preflight for VM egress networking, which is mandatory: refuses to - start unless `[network]` is configured, the uplink interface exists, and - the kernel is forwarding IPv4 — all prerequisites - `Hyper.SuidHelper.Network.host_init/0` and per-VM `prepare/2` depend on but - cannot themselves provision. This is the gate that makes networking - non-optional: a node missing any of these will not boot. - """ - @spec network_ready() :: :ok | {:error, term()} - def network_ready do - if Network.configured?() do - with :ok <- uplink_present(Network.uplink()) do - ip_forward_enabled() - end - else - {:error, :network_not_configured} - end - end - - defp uplink_present(uplink) do - if File.dir?("/sys/class/net/#{uplink}"), - do: :ok, - else: {:error, {:missing_uplink, uplink}} - end - - defp ip_forward_enabled do - case File.read(@ip_forward_path) do - {:ok, contents} -> - if String.trim(contents) == "1", do: :ok, else: {:error, :ip_forward_disabled} - - {:error, reason} -> - {:error, {:ip_forward_unreadable, reason}} - end - end - defp kvm_present do if File.exists?("/dev/kvm"), do: :ok, else: {:error, :kvm_unavailable} end diff --git a/lib/hyper/node/fire_vmm/net.ex b/lib/hyper/node/fire_vmm/net.ex index a323f0ab..844d9edd 100644 --- a/lib/hyper/node/fire_vmm/net.ex +++ b/lib/hyper/node/fire_vmm/net.ex @@ -1,53 +1,40 @@ defmodule Hyper.Node.FireVMM.Net do @moduledoc """ - The per-VM *inner-world* networking contract. Every VM's netns is identical — - `tap0` at 172.30.0.1/30, guest at 172.30.0.2/30 — so a snapshot clone restores - a correct network with zero renumbering; host-side uniqueness is the helper's - job (see `Hyper.SuidHelper.Network`). This module owns the three values that - must agree between the guest kernel cmdline, the Firecracker NIC config, and - the helper: the guest MAC, the `ip=` autoconfig string, and the NIC spec. - """ - alias Hyper.Firecracker.Api.NetworkInterface - - @guest_ip "172.30.0.2" - @tap_ip "172.30.0.1" - @netmask "255.255.255.252" - @iface "eth0" - @tap "tap0" - - @doc "Kernel `ip=` autoconfig fragment appended to every networked VM's cmdline." - @spec ip_cmdline() :: String.t() - def ip_cmdline, do: "ip=#{@guest_ip}::#{@tap_ip}:#{@netmask}::#{@iface}:off" + Unimplemented placeholder pending the VM-networking rewrite. - @doc """ - Kernel `hyper.resolver=` cmdline fragment carrying the DNS resolver IP the - PID-1 guest agent writes to `/etc/resolv.conf` — kernel `ip=` autoconfig sets - the guest's address/route but never DNS, so this closes that gap. Pure: - callers (`BootSpec.resolve/2`) read the configured resolver themselves. + VMs currently boot with no NIC and no `ip=` / `hyper.resolver=` kernel + cmdline fragments. Every function raises; nothing in Hyper calls them. """ - @spec resolver_cmdline(String.t()) :: String.t() - def resolver_cmdline(resolver), do: "hyper.resolver=#{resolver}" - @doc """ - Stable locally-administered unicast MAC for `vm_id`. Derived from a SHA-256 of - the id so it is deterministic on any node (aids debugging); duplicate MACs are - harmless since no two guests ever share an L2 segment (each is alone in its netns). - """ - @spec guest_mac(Hyper.Vm.Id.t()) :: String.t() - def guest_mac(vm_id) do - # Six octets: the fixed locally-administered-unicast leading byte plus five - # from the id hash. Firecracker rejects anything but a full XX:XX:XX:XX:XX:XX - # MAC (a five-octet address 400s the NIC config and the VM never boots). - <<_::8, b1, b2, b3, b4, b5, _::binary>> = :crypto.hash(:sha256, vm_id) + @doc "Unimplemented: the guest kernel `ip=` autoconfig fragment." + @spec ip_cmdline() :: no_return() + def ip_cmdline, + do: + raise( + "Hyper.Node.FireVMM.Net.ip_cmdline/0 is unimplemented: VM networking is being rewritten" + ) + + @doc "Unimplemented: the guest kernel `hyper.resolver=` fragment." + @spec resolver_cmdline(String.t()) :: no_return() + def resolver_cmdline(_resolver), + do: + raise( + "Hyper.Node.FireVMM.Net.resolver_cmdline/1 is unimplemented: VM networking is being rewritten" + ) - [0x02, b1, b2, b3, b4, b5] - |> Enum.map_join(":", &(&1 |> Integer.to_string(16) |> String.pad_leading(2, "0"))) - |> String.downcase() - end + @doc "Unimplemented: the guest MAC for `vm_id`." + @spec guest_mac(Hyper.Vm.Id.t()) :: no_return() + def guest_mac(_vm_id), + do: + raise( + "Hyper.Node.FireVMM.Net.guest_mac/1 is unimplemented: VM networking is being rewritten" + ) - @doc "The Firecracker NIC spec for `vm_id` (guest `eth0` bound to in-netns `tap0`)." - @spec interface(Hyper.Vm.Id.t()) :: NetworkInterface.t() - def interface(vm_id) do - %NetworkInterface{iface_id: @iface, host_dev_name: @tap, guest_mac: guest_mac(vm_id)} - end + @doc "Unimplemented: the Firecracker NIC spec for `vm_id`." + @spec interface(Hyper.Vm.Id.t()) :: no_return() + def interface(_vm_id), + do: + raise( + "Hyper.Node.FireVMM.Net.interface/1 is unimplemented: VM networking is being rewritten" + ) end diff --git a/lib/hyper/node/fire_vmm/state.ex b/lib/hyper/node/fire_vmm/state.ex index 4e1b7bbe..55535e2a 100644 --- a/lib/hyper/node/fire_vmm/state.ex +++ b/lib/hyper/node/fire_vmm/state.ex @@ -14,7 +14,7 @@ defmodule Hyper.Node.FireVMM.State do answers, or fail the boot if the readiness deadline lapses. * `:configuring` - stage the kernel + rootfs device into the jail chroot (rewriting the spec to in-jail paths), then push - machine-config, boot-source, drives, NICs, and + machine-config, boot-source, drives, and `InstanceStart`. * `:running` - guest is live; handles `stop`. * `:stopping` - teardown requested in-band; reject further calls. @@ -92,7 +92,7 @@ defmodule Hyper.Node.FireVMM.State do ) do case Hyper.Cluster.Routing.register_self({id, :state}) do :ok -> - source = boot_source(id, kernel, Mutable.blk_path(mutable), boot_args) + source = boot_source(kernel, Mutable.blk_path(mutable), boot_args) spec = BootSpec.resolve(source, type) deadline = System.monotonic_time(:millisecond) + Time.as_ms(@ready_timeout) data = %State{opts: opts, spec: spec, boot_deadline: deadline} @@ -105,15 +105,13 @@ defmodule Hyper.Node.FireVMM.State do end # Assemble the `Hyper.Vm.source()` BootSpec expects from the resolved kernel + - # device. `boot_args` is omitted when nil so BootSpec applies its default; - # `vm_id` is always included (networking is mandatory — BootSpec derives the - # guest NIC from it). - @spec boot_source(Hyper.Vm.Id.t(), Path.t(), Path.t(), String.t() | nil) :: Hyper.Vm.source() - defp boot_source(vm_id, kernel, dev, nil), - do: %{vm_id: vm_id, kernel_image_path: kernel, root_drive_path: dev} + # device. `boot_args` is omitted when nil so BootSpec applies its default. + @spec boot_source(Path.t(), Path.t(), String.t() | nil) :: Hyper.Vm.source() + defp boot_source(kernel, dev, nil), + do: %{kernel_image_path: kernel, root_drive_path: dev} - defp boot_source(vm_id, kernel, dev, boot_args), - do: %{vm_id: vm_id, kernel_image_path: kernel, root_drive_path: dev, boot_args: boot_args} + defp boot_source(kernel, dev, boot_args), + do: %{kernel_image_path: kernel, root_drive_path: dev, boot_args: boot_args} @impl :gen_statem # Answered in every lifecycle state: the opts are static data, and a fork must @@ -271,7 +269,7 @@ defmodule Hyper.Node.FireVMM.State do {:keep_state_and_data, [{:reply, from, {:error, :not_running}}]} end - # Device PUT sequence: machine-config -> boot-source -> drives -> NICs -> vsock. + # Device PUT sequence: machine-config -> boot-source -> drives -> vsock. # InstanceStart is deferred until the vsock UDS grant succeeds. @spec configure_devices(Hyper.Vm.Id.t(), BootSpec.Cold.t()) :: :ok | {:error, term()} @decorate with_span("Hyper.Node.FireVMM.State.Configuring.configure_devices", include: [:id]) @@ -286,9 +284,6 @@ defmodule Hyper.Node.FireVMM.State do Enum.map(cold.drives, fn drive -> fn opts -> Operations.put_guest_drive_by_id(drive.drive_id, drive, opts) end end) ++ - Enum.map(cold.network_interfaces, fn nic -> - fn opts -> Operations.put_guest_network_interface_by_id(nic.iface_id, nic, opts) end - end) ++ [ fn opts -> Operations.put_guest_vsock( diff --git a/lib/hyper/node/reaper.ex b/lib/hyper/node/reaper.ex index 5541db41..09cea79a 100644 --- a/lib/hyper/node/reaper.ex +++ b/lib/hyper/node/reaper.ex @@ -9,15 +9,12 @@ defmodule Hyper.Node.Reaper do Liveness is the whole point. The reaper consults three independent sources of truth for "this vm is alive" — the local VM supervisor's children, the cluster routing table, and the node's live mutable layers (`Img.Mutable.active_vm_ids/0`) - — (`Plan.orphans/4` removes their union from the candidate set) and only ever - touches `hyper-rw-*` dm names, per-VM cgroup leaves, and (when networking is - enabled) per-VM netns names under `/var/run/netns` - never `hyper-thinpool`, - `hyper-img-*`, or a live VM's resources. A - candidate must also survive two consecutive ticks (`Plan.confirm/2`) before it - is reaped, so a VM caught mid-boot (resources present, not yet registered) is - given a grace tick rather than destroyed — this is what protects a netns that - `Hyper.Node.FireVMM.Daemon.prelaunch/1` just created for a VM that has not yet - appeared in `gather_live/0`: it takes two orphan sightings to reap, and a + — (`Plan.orphans/3` removes their union from the candidate set) and only ever + touches `hyper-rw-*` dm names and per-VM cgroup leaves - never + `hyper-thinpool`, `hyper-img-*`, or a live VM's resources. A candidate must + also survive two consecutive ticks (`Plan.confirm/2`) before it is reaped, so + a VM caught mid-boot (resources present, not yet registered) is given a grace + tick rather than destroyed: it takes two orphan sightings to reap, and a normally-booting VM never produces a second one. The decision logic lives in the pure `Hyper.Node.Reaper.Plan`; this module is a @@ -88,9 +85,8 @@ defmodule Hyper.Node.Reaper do live = gather_live() leaves = list_cgroup_leaves() rw = Plan.rw_ids(list_rw_dm()) - netns = list_netns() - current = Plan.orphans(live, leaves, rw, netns) + current = Plan.orphans(live, leaves, rw) {to_reap, next} = Plan.confirm(current, state.last_orphans) Enum.each(to_reap, &reap_one/1) @@ -166,23 +162,6 @@ defmodule Hyper.Node.Reaper do end end - # Networking is mandatory, so `/var/run/netns` is expected present; an :enoent - # (nothing booted yet) is still normal and yields no orphans. - @spec list_netns() :: [String.t()] - defp list_netns do - case File.ls("/var/run/netns") do - {:ok, names} -> - names - - {:error, :enoent} -> - [] - - {:error, reason} -> - Logger.warning("reaper: could not list netns: #{inspect(reason)}") - [] - end - end - @spec reap_one(String.t()) :: :ok @decorate with_span("Hyper.Node.Reaper.reap_one", include: [:id]) defp reap_one(id) do @@ -195,7 +174,6 @@ defmodule Hyper.Node.Reaper do ) log_result("dm volume", id, Dmsetup.remove(Img.Mutable.dm_name(id))) - log_result("netns", id, Hyper.SuidHelper.Network.teardown_orphan(id)) :ok end diff --git a/lib/hyper/node/reaper/plan.ex b/lib/hyper/node/reaper/plan.ex index 24ccf955..21277c6c 100644 --- a/lib/hyper/node/reaper/plan.ex +++ b/lib/hyper/node/reaper/plan.ex @@ -17,19 +17,13 @@ defmodule Hyper.Node.Reaper.Plan do end @doc """ - Candidate orphans this tick: (cgroup leaves ∪ rw vm_ids ∪ netns names) minus - the live set. `netns_names` are the leaf names of `/var/run/netns/*` — a VM's - netns is named exactly its vm_id (see `Hyper.SuidHelper.Network`), so it folds - into the same union/difference as the other two sources and is subject to the - same two-strike `confirm/2` grace before `Hyper.Node.Reaper` ever reaps it. + Candidate orphans this tick: (cgroup leaves ∪ rw vm_ids) minus the live set. """ - @spec orphans(MapSet.t(String.t()), [String.t()], [String.t()], [String.t()]) :: - MapSet.t(String.t()) - def orphans(live, cgroup_leaves, rw_ids, netns_names) do + @spec orphans(MapSet.t(String.t()), [String.t()], [String.t()]) :: MapSet.t(String.t()) + def orphans(live, cgroup_leaves, rw_ids) do cgroup_leaves |> MapSet.new() |> MapSet.union(MapSet.new(rw_ids)) - |> MapSet.union(MapSet.new(netns_names)) |> MapSet.difference(live) end diff --git a/lib/hyper/suid_helper/network.ex b/lib/hyper/suid_helper/network.ex index e4eea2f7..5467eb73 100644 --- a/lib/hyper/suid_helper/network.ex +++ b/lib/hyper/suid_helper/network.ex @@ -1,54 +1,40 @@ defmodule Hyper.SuidHelper.Network do @moduledoc """ - Privileged per-VM egress networking via the setuid helper's `network` - subcommands. The helper derives every address, interface name, and netns path - from `vm_id` + the validated `uid` + the root-owned `[network]` config — the - node passes only those two untrusted values. + Unimplemented placeholder pending the VM-networking rewrite. + + Mirrors the setuid helper's `network` subcommands (`prepare --vm-id --uid`, + `teardown --vm-id --uid`, `teardown-orphan --vm-id`, `host-init`), which + themselves return an unimplemented error without touching the system. Nothing + in Hyper calls this module; VMs currently boot with no NIC. """ - use OpenTelemetryDecorator - alias Hyper.SuidHelper - @type err :: SuidHelper.err() + @type err :: Hyper.SuidHelper.err() @doc false - @spec argv(:prepare | :teardown, Hyper.Vm.Id.t(), non_neg_integer()) :: [String.t()] - def argv(op, vm_id, uid), - do: ["network", Atom.to_string(op), "--vm-id", vm_id, "--uid", to_string(uid)] + @spec argv(:prepare | :teardown, Hyper.Vm.Id.t(), non_neg_integer()) :: no_return() + def argv(_op, _vm_id, _uid), + do: + raise("Hyper.SuidHelper.Network.argv/3 is unimplemented: VM networking is being rewritten") @doc false - @spec argv(:teardown_orphan, Hyper.Vm.Id.t()) :: [String.t()] - def argv(:teardown_orphan, vm_id), do: ["network", "teardown-orphan", "--vm-id", vm_id] - - @doc "Create `vm_id`'s netns, veth pair, TAP, and NAT rules. Idempotent-safe on relaunch." - @spec prepare(Hyper.Vm.Id.t(), non_neg_integer()) :: :ok | {:error, err()} - @decorate with_span("Hyper.SuidHelper.Network.prepare", include: [:vm_id, :uid]) - def prepare(vm_id, uid), do: run(argv(:prepare, vm_id, uid)) - - @doc "Remove `vm_id`'s netns and host veth. Idempotent (a never-prepared VM is a no-op)." - @spec teardown(Hyper.Vm.Id.t(), non_neg_integer()) :: :ok | {:error, err()} - @decorate with_span("Hyper.SuidHelper.Network.teardown", include: [:vm_id, :uid]) - def teardown(vm_id, uid), do: run(argv(:teardown, vm_id, uid)) - - @doc """ - Remove `vm_id`'s netns by id alone, with no `uid` — for the `Reaper`'s - crashed-node backstop, where the owning node never persisted a uid to derive - the clone-pool slot from. Idempotent: a netns already gone (raced with a - normal `teardown/2`, or never fully created) is not an error. - """ - @spec teardown_orphan(Hyper.Vm.Id.t()) :: :ok | {:error, err()} - @decorate with_span("Hyper.SuidHelper.Network.teardown_orphan", include: [:vm_id]) - def teardown_orphan(vm_id), do: run(argv(:teardown_orphan, vm_id)) - - @doc "Ensure the host-wide `hyper` nft table (masquerade + forward policy) exists. Idempotent." - @spec host_init() :: :ok | {:error, err()} - @decorate with_span("Hyper.SuidHelper.Network.host_init", include: []) - def host_init, do: run(["network", "host-init"]) - - @spec run([String.t()]) :: :ok | {:error, err()} - defp run(args) do - case SuidHelper.exec(args) do - {:ok, _} -> :ok - {:error, _} = err -> err - end - end + @spec argv(:teardown_orphan, Hyper.Vm.Id.t()) :: no_return() + def argv(_op, _vm_id), + do: + raise("Hyper.SuidHelper.Network.argv/2 is unimplemented: VM networking is being rewritten") + + @doc "Unimplemented: will prepare `vm_id`'s networking once the rewrite lands." + @spec prepare(Hyper.Vm.Id.t(), non_neg_integer()) :: {:error, :unimplemented} + def prepare(_vm_id, _uid), do: {:error, :unimplemented} + + @doc "Unimplemented: will tear down `vm_id`'s networking once the rewrite lands." + @spec teardown(Hyper.Vm.Id.t(), non_neg_integer()) :: {:error, :unimplemented} + def teardown(_vm_id, _uid), do: {:error, :unimplemented} + + @doc "Unimplemented: will tear down an orphaned VM's networking by id alone." + @spec teardown_orphan(Hyper.Vm.Id.t()) :: {:error, :unimplemented} + def teardown_orphan(_vm_id), do: {:error, :unimplemented} + + @doc "Unimplemented: will perform host-wide networking setup." + @spec host_init() :: {:error, :unimplemented} + def host_init, do: {:error, :unimplemented} end diff --git a/lib/hyper/vm.ex b/lib/hyper/vm.ex index 219a9c3a..a6064a3b 100644 --- a/lib/hyper/vm.ex +++ b/lib/hyper/vm.ex @@ -8,13 +8,11 @@ defmodule Hyper.Vm do @typedoc """ What a VM boots from: explicit, already-jail-visible artifact paths for a cold boot (kernel + root drive). `boot_args` defaults to a standard serial-console - cmdline when omitted. `vm_id` is required — networking is mandatory and - `BootSpec.resolve/2` derives the guest NIC (`ip=` cmdline + MAC) from it. + cmdline when omitted. """ @type source :: %{ required(:kernel_image_path) => Path.t(), required(:root_drive_path) => Path.t(), - required(:vm_id) => Hyper.Vm.Id.t(), optional(:boot_args) => String.t() } diff --git a/lib/hyper/vm/instance.ex b/lib/hyper/vm/instance.ex index 0ce66514..46380ebe 100644 --- a/lib/hyper/vm/instance.ex +++ b/lib/hyper/vm/instance.ex @@ -123,8 +123,9 @@ defmodule Hyper.Vm.Instance do @doc """ The full resource bundle for a `type`. `vcpus` and `mem` feed both the cgroup - caps and firecracker's machine-config; `disk` sizes the rootfs image; the - `disk_bw`/`net_bw` throughputs feed firecracker's drive/NIC rate limiters. + caps and firecracker's machine-config; `disk` sizes the rootfs image; + `disk_bw`/`net_bw` are the per-VM throughput budgets the node's soft + admission checks account against. """ @spec spec(t()) :: Spec.t() def spec(type) when is_map_key(@specs, type), do: @specs[type] diff --git a/native/guest-agent/Cargo.toml b/native/guest-agent/Cargo.toml index a1d38c78..895943e8 100644 --- a/native/guest-agent/Cargo.toml +++ b/native/guest-agent/Cargo.toml @@ -3,7 +3,7 @@ name = "hyper-guest-agent" version = "0.1.0" edition = "2021" license = "MIT" -description = "PID-1 guest agent for Hyper microVMs: mounts, resolv.conf, and a vsock exec server." +description = "PID-1 guest agent for Hyper microVMs: mounts and a vsock exec server." repository = "https://github.com/harmont-dev/hyper" [lib] @@ -34,10 +34,6 @@ tokio-stream = "0.1" name = "exec" path = "tests/exec.rs" -[[test]] -name = "init" -path = "tests/init.rs" - [profile.release] opt-level = "z" lto = true diff --git a/native/guest-agent/src/init.rs b/native/guest-agent/src/init.rs index 7de75a1f..ae3b524b 100644 --- a/native/guest-agent/src/init.rs +++ b/native/guest-agent/src/init.rs @@ -1,10 +1,7 @@ use std::io; -use std::net::Ipv4Addr; use nix::mount::{mount, MsFlags}; -const RESOLVER_PARAM: &str = "hyper.resolver="; - // As PID 1 nothing else mounts these; exec'd programs need /proc and /dev. pub fn setup() -> io::Result<()> { let none: Option<&str> = None; @@ -19,30 +16,5 @@ pub fn setup() -> io::Result<()> { // must not abort the agent; a missing /proc only degrades exec'd tools. let _ = mount(Some(src), target, Some(fstype), MsFlags::empty(), none); } - // Best-effort, run after /proc is mounted: the kernel's `ip=` autoconfig - // sets the guest's address/route but never DNS, so BootSpec appends - // `hyper.resolver=` to the cmdline for us to relay into resolv.conf. A - // missing param or unwritable /etc only degrades DNS, never boot. - let _ = write_resolv_conf(); Ok(()) } - -/// Extracts the value of the `hyper.resolver=` token from a `/proc/cmdline`-style -/// space-separated string. `None` if the param is absent, has an empty value, -/// or the value isn't a valid IPv4 address. Where the same param repeats, the -/// first occurrence wins (matches kernel cmdline parsing conventions). -pub fn resolver_from_cmdline(cmdline: &str) -> Option { - cmdline - .split_whitespace() - .find_map(|token| token.strip_prefix(RESOLVER_PARAM)) - .filter(|value| !value.is_empty()) - .and_then(|value| value.parse().ok()) -} - -fn write_resolv_conf() -> io::Result<()> { - let cmdline = std::fs::read_to_string("/proc/cmdline")?; - let Some(resolver) = resolver_from_cmdline(&cmdline) else { - return Ok(()); - }; - std::fs::write("/etc/resolv.conf", format!("nameserver {resolver}\n")) -} diff --git a/native/guest-agent/tests/init.rs b/native/guest-agent/tests/init.rs deleted file mode 100644 index 98014376..00000000 --- a/native/guest-agent/tests/init.rs +++ /dev/null @@ -1,65 +0,0 @@ -use hyper_guest_agent::init::resolver_from_cmdline; -use std::net::Ipv4Addr; - -#[test] -fn present_among_other_params() { - let cmdline = "reboot=k panic=1 hyper.resolver=1.1.1.1 ip=172.30.0.2::172.30.0.1:255.255.255.252::eth0:off"; - assert_eq!( - resolver_from_cmdline(cmdline), - Some(Ipv4Addr::new(1, 1, 1, 1)) - ); -} - -#[test] -fn absent_is_none() { - let cmdline = "reboot=k panic=1 ip=172.30.0.2::172.30.0.1:255.255.255.252::eth0:off"; - assert_eq!(resolver_from_cmdline(cmdline), None); -} - -#[test] -fn empty_cmdline_is_none() { - assert_eq!(resolver_from_cmdline(""), None); -} - -#[test] -fn empty_value_is_none() { - assert_eq!(resolver_from_cmdline("hyper.resolver= ip=foo"), None); -} - -#[test] -fn non_ipv4_value_is_none() { - assert_eq!(resolver_from_cmdline("hyper.resolver=not-an-ip"), None); -} - -#[test] -fn first_of_multiple_tokens_wins() { - let cmdline = "hyper.resolver=1.1.1.1 hyper.resolver=8.8.8.8"; - assert_eq!( - resolver_from_cmdline(cmdline), - Some(Ipv4Addr::new(1, 1, 1, 1)) - ); -} - -#[test] -fn first_of_multiple_tokens_wins_among_other_params() { - let cmdline = "a hyper.resolver=1.1.1.1 b hyper.resolver=8.8.8.8 c"; - assert_eq!( - resolver_from_cmdline(cmdline), - Some(Ipv4Addr::new(1, 1, 1, 1)) - ); -} - -// Regression: split_whitespace() + strip_prefix() requires the param to be its -// own whitespace-delimited token, not merely a substring anywhere on the -// cmdline. A future refactor to `cmdline.contains("hyper.resolver=")` would -// match this look-alike token and could point the guest at an attacker -// resolver embedded in an unrelated kernel param — that must never happen. -#[test] -fn prefixed_look_alike_token_is_not_matched() { - assert_eq!(resolver_from_cmdline("xhyper.resolver=9.9.9.9"), None); -} - -#[test] -fn suffixed_look_alike_key_is_not_matched() { - assert_eq!(resolver_from_cmdline("hyper.resolverX=1.1.1.1"), None); -} diff --git a/native/suidhelper/Cargo.toml b/native/suidhelper/Cargo.toml index 51748e3b..4da34c4c 100644 --- a/native/suidhelper/Cargo.toml +++ b/native/suidhelper/Cargo.toml @@ -56,18 +56,6 @@ path = "tests/tools/blockcopy.rs" name = "tools_jailer" path = "tests/tools/jailer.rs" -[[test]] -name = "tools_network_addr" -path = "tests/tools/network_addr.rs" - -[[test]] -name = "tools_network_args" -path = "tests/tools/network_args.rs" - -[[test]] -name = "tools_host_init" -path = "tests/tools/host_init.rs" - [[test]] name = "util_confinement" path = "tests/util/confinement.rs" diff --git a/native/suidhelper/src/config.rs b/native/suidhelper/src/config.rs index 39dd1762..10c6c829 100644 --- a/native/suidhelper/src/config.rs +++ b/native/suidhelper/src/config.rs @@ -94,6 +94,10 @@ fn config_path() -> PathBuf { } /// Hyper's /etc/hyper/config.toml file format. +/// +/// A legacy `[network]` table and `[tools] ip`/`nft`/`sysctl` keys are accepted +/// and ignored (serde drops unknown keys), so operator configs written before +/// the networking rewrite keep working. #[derive(Debug, Clone, Deserialize)] pub struct Config { work_dir: PathBuf, @@ -101,8 +105,6 @@ pub struct Config { tools: Tools, #[serde(default)] jails: Jails, - #[serde(default)] - network: Option, } /// The `[jails]` table: how the helper places and confines each VM jail. @@ -142,9 +144,6 @@ pub struct Tools { dmsetup: PathBuf, losetup: PathBuf, blockdev: PathBuf, - ip: PathBuf, - nft: PathBuf, - sysctl: PathBuf, thin_dump: PathBuf, firecracker: Option, jailer: Option, @@ -156,12 +155,6 @@ impl Default for Tools { dmsetup: "/usr/sbin/dmsetup".into(), losetup: "/usr/sbin/losetup".into(), blockdev: "/usr/sbin/blockdev".into(), - // /usr/bin/ip, not /usr/sbin/ip: on merged-usr distros the latter - // is a symlink to the former, and SafeBin rejects symlinked tool - // paths. iproute2's real binary lives at /usr/bin/ip. - ip: "/usr/bin/ip".into(), - nft: "/usr/sbin/nft".into(), - sysctl: "/usr/sbin/sysctl".into(), thin_dump: "/usr/sbin/thin_dump".into(), firecracker: None, jailer: None, @@ -187,34 +180,10 @@ impl Default for Config { work_dir: default_work_dir(), tools: Tools::default(), jails: Jails::default(), - network: None, } } } -/// The `[network]` table: VM egress networking. Absent, or present without -/// `uplink`, ⇒ networking disabled — see [`Config::network`]. `uplink` is the -/// physical interface guest traffic is masqueraded out of; `clone_pool` is the -/// IPv4 CIDR per-VM /30 clone addresses are carved from (default -/// `172.31.0.0/16`). Both are read identically by the Elixir node -/// (`Hyper.Cfg.Network`) — keep the defaults in sync. -/// -/// `uplink` is `Option` rather than required so that a `[network]` table -/// present with only `clone_pool` set (`uplink` omitted) parses as -/// networking-disabled rather than failing to deserialize the whole -/// [`Config`] — see [`Config::network`]'s doc for why that asymmetry would be -/// dangerous. -#[derive(Debug, Clone, Deserialize)] -pub struct Network { - pub uplink: Option, - #[serde(default = "default_clone_pool")] - pub clone_pool: String, -} - -fn default_clone_pool() -> String { - "172.31.0.0/16".into() -} - impl Config { /// The process-wide config, loaded once (and forced unprivileged by /// [`Config::init`]). An absent file yields the built-in defaults; a @@ -258,43 +227,6 @@ impl Config { SafeBin::from_path(&self.tools.blockdev) } - /// The validated `ip` (iproute2) binary the network tool runs. - pub fn ip(&self) -> Result, safe_bin::Error> { - SafeBin::from_path(&self.tools.ip) - } - - /// The validated `nft` (nftables) binary the network tool runs. - pub fn nft(&self) -> Result, safe_bin::Error> { - SafeBin::from_path(&self.tools.nft) - } - - /// The `sysctl` binary, used to enable IPv4 forwarding inside a VM's netns - /// (a fresh netns defaults `net.ipv4.ip_forward` to 0, independent of the - /// host, so the netns would not route the guest's egress without it). - pub fn sysctl(&self) -> Result, safe_bin::Error> { - SafeBin::from_path(&self.tools.sysctl) - } - - /// The `[network]` table, or `None` when VM networking is disabled — either - /// because `[network]` is absent entirely, or because it is present but - /// `uplink` is not set. - /// - /// The latter case matters on its own: `Config` is a process-wide - /// [`LazyLock`] that exits the whole helper on any parse failure (every - /// subcommand — dmsetup, losetup, chroot-jail, jailer, not just - /// networking — calls [`Config::get`] first), so if `uplink` were a - /// required field, an operator who sets `clone_pool` before `uplink` (or - /// simply omits `uplink` to disable networking, mirroring the Elixir - /// node's `Hyper.Cfg.Network.enabled?/0`, which treats absent `uplink` as - /// disabled rather than an error) would brick every tool the helper - /// offers, not just networking. Filtering here — rather than making - /// `uplink` required — keeps this side symmetric with Elixir's. - pub fn network(&self) -> Option<&Network> { - self.network - .as_ref() - .filter(|network| network.uplink.is_some()) - } - /// The validated `thin_dump` binary (thin-provisioning-tools) the helper /// will run. pub fn thin_dump(&self) -> Result, safe_bin::Error> { diff --git a/native/suidhelper/src/tools/jailer.rs b/native/suidhelper/src/tools/jailer.rs index db3fcedc..b58d0817 100644 --- a/native/suidhelper/src/tools/jailer.rs +++ b/native/suidhelper/src/tools/jailer.rs @@ -64,15 +64,7 @@ pub fn run(args: JailerArgs) -> Result { let uid = validate_id_number(args.uid, range).map_err(Error::Jail)?; let gid = validate_id_number(args.gid, range).map_err(Error::Jail)?; - // The netns path is derived entirely from trusted config (whether VM - // networking is enabled) and the already-validated `--id` — the caller - // cannot name it. Must match the `ip netns add ` path the network tool - // creates. Networking is mandatory, so in practice this is always `Some`. - let netns: Option = config - .network() - .map(|_| PathBuf::from(format!("/var/run/netns/{}", args.id))); - - Ok(Jailer::builder() + Jailer::builder() .jailer(&jailer) .firecracker(&firecracker) .jail_base(&jail_base) @@ -82,7 +74,7 @@ pub fn run(args: JailerArgs) -> Result { .gid(gid) .cgroups(&args.cgroup) .api_sock(&args.api_sock) - .maybe_netns(netns.as_deref()) .build() - .invoke()?) + .invoke() + .map_err(Error::Jail) } diff --git a/native/suidhelper/src/tools/mod.rs b/native/suidhelper/src/tools/mod.rs index 80acae07..80a7d0a1 100644 --- a/native/suidhelper/src/tools/mod.rs +++ b/native/suidhelper/src/tools/mod.rs @@ -103,7 +103,7 @@ pub enum Tool { #[command(subcommand)] op: ChrootJailOp, }, - /// Per-VM egress networking (netns + veth + TAP + NAT). + /// Per-VM networking (unimplemented placeholder pending the networking rewrite). Network { #[command(subcommand)] op: NetworkOp, diff --git a/native/suidhelper/src/tools/network/addr.rs b/native/suidhelper/src/tools/network/addr.rs deleted file mode 100644 index 2b2ef9b5..00000000 --- a/native/suidhelper/src/tools/network/addr.rs +++ /dev/null @@ -1,77 +0,0 @@ -//! Pure address & interface-name math for per-VM egress networking. -//! -//! Laws (see `tests/tools/network_addr.rs`): -//! - the derived clone `/30` always lands inside the configured pool, never in -//! the inner `172.30.0.0/30` constant space; -//! - `veth` names always satisfy `IFNAMSIZ` (<= 15); -//! - a `uid` below the accepted range, or a `slot` past the pool's capacity, is -//! *always* refused — never wrapped into another VM's address. -use crate::tools::jailer::VmId; -use std::net::Ipv4Addr; -use thiserror::Error; - -pub const INNER_TAP_IP: Ipv4Addr = Ipv4Addr::new(172, 30, 0, 1); -pub const INNER_GUEST_IP: Ipv4Addr = Ipv4Addr::new(172, 30, 0, 2); -pub const INNER_PREFIX: u8 = 30; -pub const TAP_NAME: &str = "tap0"; -pub const GUEST_IFACE: &str = "eth0"; - -#[derive(Debug, Error)] -pub enum AddrError { - #[error("uid {uid} is below the accepted range floor {floor}")] - UidBelowRange { uid: u32, floor: u32 }, - #[error("slot {slot} does not fit the clone pool")] - SlotOutOfPool { slot: u32 }, -} - -#[derive(Debug, Clone)] -pub struct Plan { - pub slot: u32, - pub netns: String, - pub veth_host: String, - pub veth_ns: String, - pub veth_host_ip: Ipv4Addr, - pub veth_ns_ip: Ipv4Addr, -} - -impl Plan { - pub fn derive( - uid: u32, - uid_range: (u32, u32), - clone_pool: Ipv4Addr, - vm_id: &VmId, - ) -> Result { - let floor = uid_range.0; - if uid < floor { - return Err(AddrError::UidBelowRange { uid, floor }); - } - let slot = uid - floor; - // Each VM consumes a /30 (4 addresses). The block's last address must - // stay within the 32-bit space above the pool base. - let base = u32::from(clone_pool); - let block = base - .checked_add( - slot.checked_mul(4) - .ok_or(AddrError::SlotOutOfPool { slot })?, - ) - .ok_or(AddrError::SlotOutOfPool { slot })?; - // Guard against overflowing the pool's own /16 (65_536 addresses). - if slot >= (1u32 << 16) / 4 { - return Err(AddrError::SlotOutOfPool { slot }); - } - let veth_host_ip = block - .checked_add(1) - .ok_or(AddrError::SlotOutOfPool { slot })?; - let veth_ns_ip = block - .checked_add(2) - .ok_or(AddrError::SlotOutOfPool { slot })?; - Ok(Self { - slot, - netns: vm_id.to_string(), - veth_host: format!("hv{slot}"), - veth_ns: format!("hp{slot}"), - veth_host_ip: Ipv4Addr::from(veth_host_ip), - veth_ns_ip: Ipv4Addr::from(veth_ns_ip), - }) - } -} diff --git a/native/suidhelper/src/tools/network/args.rs b/native/suidhelper/src/tools/network/args.rs deleted file mode 100644 index 4f25a1ba..00000000 --- a/native/suidhelper/src/tools/network/args.rs +++ /dev/null @@ -1,405 +0,0 @@ -//! Pure `ip`/`nft` argv builders for per-VM egress networking. -//! -//! Each step is data — a [`Command`] naming which privileged binary runs it and -//! its exact argv — so the sequences below can be asserted in -//! `tests/tools/network_args.rs` without root or a real network device. -//! Execution (actually invoking the binaries) lands in Task 4; this module only -//! builds the argv. -//! -//! Laws (tested): -//! - `prepare_commands` creates the netns first, and only requests the in-netns -//! default route after the host veth (`hv`) is brought up — the guest's -//! route target must already be reachable when the route is added. -//! - `teardown_commands` always deletes the netns, which reclaims the ns-side -//! veth peer, the TAP device, and any in-netns nftables state. -//! - `host_init_commands` masquerades the clone pool out the configured uplink, -//! drops guest traffic addressed to the cloud metadata IP -//! (`169.254.169.254`), and drops all clone-pool-sourced traffic addressed to -//! the host itself on the INPUT hook (distinct from FORWARD — see that -//! function's doc), so no VM can ever reach it or a host-local service. -use super::addr::{self, Plan}; - -/// Which privileged binary a [`Command`] invokes. Kept to exactly two — `ip` -/// and `nft` — so the setuid helper's whitelist never grows past what egress -/// networking needs. In-netns `nft` operations still run as `ip netns exec -/// nft ...`, so they are represented as [`Which::Ip`] commands. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum Which { - Ip, - Nft, -} - -/// One step of a prepare/teardown/host-init sequence: the binary to run and its -/// exact argv (argv[0] is the binary itself, named by [`Which`], not repeated -/// here). `allow_failure` marks a step whose non-zero exit must not stop the -/// sequence — used only by the leading `nft delete table ip hyper` in -/// [`host_init_commands`], since that command's whole purpose is to clear -/// state that may not exist yet. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Command { - pub bin: Which, - pub argv: Vec, - pub allow_failure: bool, -} - -macro_rules! argv { - ($($x:expr),* $(,)?) => { - vec![$($x.to_string()),*] - }; -} - -impl Command { - fn ip(argv: Vec) -> Self { - Self { - bin: Which::Ip, - argv, - allow_failure: false, - } - } - - /// `ip -n ...` — runs an `ip` operation inside the VM's netns. - fn ip_ns(netns: &str, argv: Vec) -> Self { - let mut full = argv!["-n", netns]; - full.extend(argv); - Self::ip(full) - } - - fn nft(argv: Vec) -> Self { - Self { - bin: Which::Nft, - argv, - allow_failure: false, - } - } - - /// An `nft` step whose non-zero exit is tolerated — see the `allow_failure` - /// doc on [`Command`]. - fn nft_allow_failure(argv: Vec) -> Self { - Self { - allow_failure: true, - ..Self::nft(argv) - } - } - - /// An `ip` step whose non-zero exit is tolerated — for idempotent deletes - /// (`ip netns del`, `ip link del`) issued speculatively where the target may - /// not exist. See the `allow_failure` doc on [`Command`]. - fn ip_allow_failure(argv: Vec) -> Self { - Self { - allow_failure: true, - ..Self::ip(argv) - } - } - - /// `ip netns exec ...` — runs an `nft` operation inside the - /// VM's netns, kept as a [`Which::Ip`] command so only `ip` and `nft` are - /// ever the privileged binary named directly. `nft` is the *absolute* path - /// to the nft binary, not the bare name: the helper spawns every command - /// with a cleared environment (no `PATH`), so `ip netns exec` could not - /// resolve a bare `nft`, and the config names the exact binary anyway. - fn nft_ns(netns: &str, nft: &str, argv: Vec) -> Self { - let mut full = argv!["netns", "exec", netns, nft]; - full.extend(argv); - Self::ip(full) - } - - /// `ip netns exec ...` — runs a `sysctl` write inside the - /// VM's netns. Like [`nft_ns`], it stays a [`Which::Ip`] command (only `ip` - /// and `nft` are ever the directly-named privileged binary) and takes the - /// *absolute* sysctl path, since the helper clears `PATH` before spawning. - fn sysctl_ns(netns: &str, sysctl: &str, argv: Vec) -> Self { - let mut full = argv!["netns", "exec", netns, sysctl]; - full.extend(argv); - Self::ip(full) - } -} - -/// Bring up the VM's netns, host/ns veth pair, TAP device, and in-netns NAT so -/// the guest's inner address (`addr::INNER_GUEST_IP`) can reach the host's -/// uplink. The default route is requested last, after the host veth end -/// (`hv`) is up, so the route's target is already reachable. -pub fn prepare_commands(plan: &Plan, nft: &str, sysctl: &str) -> Vec { - let netns = plan.netns.as_str(); - let veth_host = plan.veth_host.as_str(); - let veth_ns = plan.veth_ns.as_str(); - let veth_host_ip = plan.veth_host_ip; - let veth_ns_ip = plan.veth_ns_ip; - - let mut commands = vec![ - Command::ip(argv!["netns", "add", netns]), - Command::ip(argv![ - "link", "add", veth_host, "type", "veth", "peer", "name", veth_ns - ]), - Command::ip(argv!["link", "set", veth_ns, "netns", netns]), - Command::ip(argv![ - "addr", - "add", - format!("{veth_host_ip}/{}", addr::INNER_PREFIX), - "dev", - veth_host - ]), - Command::ip(argv!["link", "set", veth_host, "up"]), - Command::ip_ns( - netns, - argv![ - "addr", - "add", - format!("{veth_ns_ip}/{}", addr::INNER_PREFIX), - "dev", - veth_ns - ], - ), - Command::ip_ns(netns, argv!["link", "set", veth_ns, "up"]), - Command::ip_ns(netns, argv!["tuntap", "add", addr::TAP_NAME, "mode", "tap"]), - Command::ip_ns( - netns, - argv![ - "addr", - "add", - format!("{}/{}", addr::INNER_TAP_IP, addr::INNER_PREFIX), - "dev", - addr::TAP_NAME - ], - ), - Command::ip_ns(netns, argv!["link", "set", addr::TAP_NAME, "up"]), - Command::ip_ns(netns, argv!["link", "set", "lo", "up"]), - Command::ip_ns(netns, argv!["route", "add", "default", "via", veth_host_ip]), - // A fresh netns defaults net.ipv4.ip_forward to 0 (independent of the - // host's setting), so without this the netns silently drops every - // packet it would route between the guest's tap and the veth uplink — - // i.e. all guest egress. Enable it once the netns exists. - Command::sysctl_ns(netns, sysctl, argv!["-w", "net.ipv4.ip_forward=1"]), - ]; - commands.extend(nft_prepare_commands(netns, veth_ns, veth_ns_ip, nft)); - commands -} - -/// The in-netns NAT: SNAT the guest's inner source address to the netns's own -/// veth IP on the way out (so the host sees a source unique to this VM), and -/// DNAT return traffic addressed to that veth IP back to the guest. -fn nft_prepare_commands( - netns: &str, - veth_ns: &str, - veth_ns_ip: std::net::Ipv4Addr, - nft: &str, -) -> Vec { - vec![ - Command::nft_ns(netns, nft, argv!["add", "table", "ip", "nat"]), - Command::nft_ns( - netns, - nft, - argv![ - "add", - "chain", - "ip", - "nat", - "post", - "{", - "type", - "nat", - "hook", - "postrouting", - "priority", - "100", - ";", - "}" - ], - ), - Command::nft_ns( - netns, - nft, - argv![ - "add", - "rule", - "ip", - "nat", - "post", - "ip", - "saddr", - addr::INNER_GUEST_IP, - "oifname", - veth_ns, - "snat", - "to", - veth_ns_ip - ], - ), - Command::nft_ns( - netns, - nft, - argv![ - "add", - "chain", - "ip", - "nat", - "pre", - "{", - "type", - "nat", - "hook", - "prerouting", - "priority", - "-100", - ";", - "}" - ], - ), - Command::nft_ns( - netns, - nft, - argv![ - "add", - "rule", - "ip", - "nat", - "pre", - "ip", - "daddr", - veth_ns_ip, - "dnat", - "to", - addr::INNER_GUEST_IP - ], - ), - ] -} - -/// Tear down a VM's netns. Deleting the netns reclaims the ns-side veth peer, -/// the TAP device, and any in-netns nftables state with it; the host-side veth -/// end usually goes with it too, but `ip link del` is issued explicitly in -/// case it lingers (a no-op if already gone). -/// -/// Both deletes tolerate a missing target: `Daemon` runs teardown speculatively -/// in `clear_jail` *before* launching a fresh VM (to clear any stale prior -/// incarnation), so on a first boot the netns and veth do not exist yet — -/// `ip netns del` on a missing namespace exits non-zero, which must not fail -/// the launch. Like the chroot removal it sits beside, teardown is idempotent -/// cleanup; a genuine leak is backstopped by `Hyper.Node.Reaper`. -pub fn teardown_commands(plan: &Plan) -> Vec { - vec![ - Command::ip_allow_failure(argv!["netns", "del", plan.netns.as_str()]), - Command::ip_allow_failure(argv!["link", "del", plan.veth_host.as_str()]), - ] -} - -/// Tear down an *orphan* VM's netns by vm_id alone — no `uid`, so no [`Plan`] -/// and no host-veth `link del` (that command needs the derived `hv` -/// name, which needs the uid). Deleting the netns is sufficient on its own: -/// it reclaims the ns-side veth peer, the TAP device, and any in-netns -/// nftables state, and the kernel removes the host-side veth end along with -/// its peer's netns. See `teardown_orphan`'s module doc for why the caller -/// (`Hyper.Node.Reaper`) has no uid to give here. -pub fn teardown_orphan_commands(netns: &str) -> Vec { - vec![Command::ip(argv!["netns", "del", netns])] -} - -/// One-time host setup: a `hyper` nftables table that masquerades the clone -/// pool out `uplink`, a forward-chain default-drop policy that only admits -/// the pool's own traffic — explicitly dropping anything addressed to the -/// cloud metadata IP (`169.254.169.254`), so no VM can ever reach it — and an -/// input-chain rule that drops all clone-pool-sourced traffic addressed to -/// the host itself. -/// -/// The input-chain drop closes a distinct hook from the forward-chain policy: -/// a guest packet addressed to a *host-owned* IP (the guest's own gateway -/// address, or any other address the host answers for) is locally delivered -/// and hits the kernel's INPUT hook, never FORWARD — so without this rule a -/// guest could reach host-local services (epmd, distribution, Postgres, the -/// gRPC listener) regardless of the forward-chain policy above. The input -/// chain's own policy stays `accept` so unrelated host traffic (SSH, other -/// subnets) is untouched; only clone-pool-sourced packets are dropped. This -/// does not affect guest egress (which is FORWARDed, never delivered to -/// INPUT) or ARP (an L2 protocol the `ip` family's INPUT hook never sees), and -/// the guest's DNS queries are forwarded to an external resolver, not -/// terminated on the host, so they are unaffected too. -/// -/// Reconciles to desired state rather than diffing: the first command -/// unconditionally deletes the `hyper` table (tolerating "no such file" when -/// it doesn't exist yet), and every command after it rebuilds the table from -/// scratch. This makes host-init idempotent by construction — the table is -/// always either absent or complete, never partially applied — so a -/// crash/interruption mid-sequence on one run can never strand a later run -/// with, say, NAT but no forward-chain drop policy (or no input-chain -/// isolation). -pub fn host_init_commands(uplink: &str, clone_pool: &str) -> Vec { - vec![ - Command::nft_allow_failure(argv!["delete", "table", "ip", "hyper"]), - Command::nft(argv!["add", "table", "ip", "hyper"]), - Command::nft(argv![ - "add", - "chain", - "ip", - "hyper", - "postrouting", - "{", - "type", - "nat", - "hook", - "postrouting", - "priority", - "100", - ";", - "}" - ]), - Command::nft(argv![ - "add", - "rule", - "ip", - "hyper", - "postrouting", - "ip", - "saddr", - clone_pool, - "oifname", - uplink, - "masquerade" - ]), - Command::nft(argv![ - "add", "chain", "ip", "hyper", "forward", "{", "type", "filter", "hook", "forward", - "priority", "0", ";", "policy", "drop", ";", "}" - ]), - // Must precede the broad egress accept below: `accept` is a - // terminating verdict in nftables, so a rule reachable before this - // one would let a guest packet to the metadata IP exit via the - // uplink before this drop is ever evaluated. - Command::nft(argv![ - "add", - "rule", - "ip", - "hyper", - "forward", - "ip", - "saddr", - clone_pool, - "ip", - "daddr", - "169.254.169.254", - "drop" - ]), - Command::nft(argv![ - "add", "rule", "ip", "hyper", "forward", "ip", "saddr", clone_pool, "oifname", uplink, - "accept" - ]), - Command::nft(argv![ - "add", - "rule", - "ip", - "hyper", - "forward", - "ip", - "daddr", - clone_pool, - "ct", - "state", - "established,related", - "accept" - ]), - Command::nft(argv![ - "add", "chain", "ip", "hyper", "input", "{", "type", "filter", "hook", "input", - "priority", "0", ";", "policy", "accept", ";", "}" - ]), - Command::nft(argv![ - "add", "rule", "ip", "hyper", "input", "ip", "saddr", clone_pool, "drop" - ]), - ] -} diff --git a/native/suidhelper/src/tools/network/exec.rs b/native/suidhelper/src/tools/network/exec.rs deleted file mode 100644 index 2eb199aa..00000000 --- a/native/suidhelper/src/tools/network/exec.rs +++ /dev/null @@ -1,79 +0,0 @@ -//! Shared execution of an [`args::Command`] sequence: resolve each command's -//! binary via `resolve`, spawn it with a cleared environment, and stop at the -//! first non-zero exit — unless the command is marked `allow_failure`, in -//! which case its exit status is ignored and the sequence continues. This is -//! the whole privileged window for every network op — `prepare`, `teardown`, -//! and `host_init` differ only in which commands they hand to [`run_all`]. -use super::args::{Command, Which}; -use crate::util::setuid_privileged::become_root_permanently; -use std::io; -use std::os::unix::process::CommandExt; -use std::path::Path; -use std::process::Command as Proc; -use thiserror::Error as ThisError; - -#[derive(Debug, ThisError)] -pub enum Error { - #[error("running {bin:?} {argv:?}: {source}")] - Spawn { - bin: Which, - argv: Vec, - #[source] - source: std::io::Error, - }, - #[error("{bin:?} {argv:?} exited {code}: stderr={stderr:?} stdout={stdout:?}")] - Failed { - bin: Which, - argv: Vec, - code: String, - stderr: String, - stdout: String, - }, -} - -pub(super) fn run_all<'a>( - commands: &[Command], - resolve: impl Fn(Which) -> &'a Path, -) -> Result<(), Error> { - for command in commands { - let mut proc = Proc::new(resolve(command.bin)); - proc.args(&command.argv).env_clear(); - - // `ip` and `nft` refuse to run under `AT_SECURE` — the setuid-like state - // the kernel flags when a child is exec'd with `ruid != euid`. Our - // privilege guard (`Privileged::acquire`) raises only euid to 0, leaving - // ruid at the caller, so without this the child inherits that mismatch - // and nft exits 111 with no output at all. Promote the child to genuine - // root (all of real/effective/saved uid == 0) just before exec — the - // same reason the jailer handoff uses `become_root_permanently`. This - // runs in the forked child, so it never affects the parent's own - // privilege window. - // - // SAFETY: `become_root_permanently` calls only async-signal-safe - // setresuid/setresgid/setgroups syscalls and allocates nothing, which is - // the contract `pre_exec` requires of the post-fork, pre-exec child. - unsafe { - proc.pre_exec(|| become_root_permanently().map_err(io::Error::other)); - } - - let output = proc.output().map_err(|source| Error::Spawn { - bin: command.bin, - argv: command.argv.clone(), - source, - })?; - - if !output.status.success() && !command.allow_failure { - return Err(Error::Failed { - bin: command.bin, - argv: command.argv.clone(), - code: output - .status - .code() - .map_or_else(|| "signal".to_string(), |c| c.to_string()), - stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(), - stdout: String::from_utf8_lossy(&output.stdout).trim().to_string(), - }); - } - } - Ok(()) -} diff --git a/native/suidhelper/src/tools/network/host_init.rs b/native/suidhelper/src/tools/network/host_init.rs deleted file mode 100644 index 85cc79dc..00000000 --- a/native/suidhelper/src/tools/network/host_init.rs +++ /dev/null @@ -1,106 +0,0 @@ -//! `network host-init`: one-time host setup of the `hyper` nftables table, -//! its masquerade rule, and the forward-chain default-drop policy. -//! -//! Idempotency: host-init reconciles to desired state rather than diffing. -//! [`args::host_init_commands`] always leads with a (failure-tolerant) delete -//! of the `hyper` table, then rebuilds it from scratch. A presence check -//! (e.g. "does `nft list table ip hyper` already mention masquerade?") looks -//! idempotent but isn't: masquerade is added partway through the sequence, so -//! a crash between that step and the later forward-chain drop policy would -//! leave the table permanently short a security rule — every subsequent -//! host-init would see masquerade present and skip, never adding the missing -//! policy. Delete-then-recreate has no such window: the table is always -//! either fully absent or fully complete. -use super::args; -use super::exec; -use super::NetworkOut; -use crate::config::Config; -use crate::tools::IsTool; -use crate::util::safe_bin; -use clap::Args; -use std::path::PathBuf; -use thiserror::Error as ThisError; - -#[derive(Args)] -pub struct HostInitArgs {} - -/// `/proc` file the kernel exposes IPv4 forwarding through: `1` when enabled. -const IP_FORWARD_PATH: &str = "/proc/sys/net/ipv4/ip_forward"; - -#[derive(Debug, ThisError)] -pub enum Error { - #[error(transparent)] - NetworkingDisabled(#[from] super::NetworkingDisabled), - #[error(transparent)] - Bin(#[from] safe_bin::Error), - #[error(transparent)] - Exec(#[from] exec::Error), - #[error("failed to read {IP_FORWARD_PATH}: {0}")] - IpForwardRead(#[source] std::io::Error), - #[error( - "net.ipv4.ip_forward is disabled ({IP_FORWARD_PATH} != 1); provision the host with \ - `sysctl -w net.ipv4.ip_forward=1` (see docs/cookbook/install.md) before VM networking \ - can be enabled" - )] - IpForwardDisabled, -} - -/// Whether `contents` (the raw text of `/proc/sys/net/ipv4/ip_forward`) says -/// forwarding is on. Pure so the parsing rule is testable without root or a -/// real `/proc`; the privileged read itself lives in `run_privileged`. -pub fn ip_forward_enabled(contents: &str) -> bool { - contents.trim() == "1" -} - -pub fn run(args: HostInitArgs) -> Result { - HostInit::new(args) - .map_err(|e| crate::tools::Error::Tool(Box::new(e)))? - .run() -} - -struct HostInit { - uplink: String, - clone_pool: String, - nft: PathBuf, -} - -impl HostInit { - fn new(_args: HostInitArgs) -> Result { - let config = Config::get(); - let network = super::resolve_network(config.network())?; - Ok(Self { - // `Config::network()` only ever returns `Some` when `uplink` is - // set (see its doc), so `resolve_network`'s `Ok` carries that same - // guarantee. - uplink: network - .uplink - .clone() - .expect("Config::network() guarantees uplink is set"), - clone_pool: network.clone_pool.clone(), - nft: config.nft()?.into(), - }) - } -} - -impl IsTool for HostInit { - type Args = HostInitArgs; - type Output = NetworkOut; - type RunT = Result<(), Error>; - - fn run_privileged(&self) -> Self::RunT { - let ip_forward = std::fs::read_to_string(IP_FORWARD_PATH).map_err(Error::IpForwardRead)?; - if !ip_forward_enabled(&ip_forward) { - return Err(Error::IpForwardDisabled); - } - - let commands = args::host_init_commands(&self.uplink, &self.clone_pool); - // host_init_commands only ever issues `nft` commands (see args.rs). - exec::run_all(&commands, |_which| self.nft.as_path())?; - Ok(()) - } - - fn parse(&self, res: Self::RunT) -> Result> { - res?; - Ok(NetworkOut::HostReady) - } -} diff --git a/native/suidhelper/src/tools/network/mod.rs b/native/suidhelper/src/tools/network/mod.rs index e1cb94e6..e50288c9 100644 --- a/native/suidhelper/src/tools/network/mod.rs +++ b/native/suidhelper/src/tools/network/mod.rs @@ -1,76 +1,70 @@ // SPDX-License-Identifier: MIT -//! `network`: per-VM egress networking (netns + veth + TAP + NAT). +//! `network`: per-VM networking — an **unimplemented placeholder** pending a +//! ground-up rewrite. //! -//! Each op ([`prepare`], [`teardown`], [`host_init`]) derives its `ip`/`nft` -//! argv (built in [`args`]) from the validated uid and [`crate::config::Config::network`], -//! then runs it through [`exec::run_all`], which stops at the first non-zero -//! exit. Every refusal — `uid` outside the configured range, a malformed -//! `clone_pool` CIDR, or `[network]` absent from config — happens while -//! resolving the op (see `prepare::resolve`), strictly before `run_privileged` -//! is ever reached, so no privileged command runs on invalid input. -pub mod addr; -pub mod args; -mod exec; -pub mod host_init; -pub mod prepare; -pub mod teardown; -pub mod teardown_orphan; +//! The subcommand tree and its arguments are kept as the CLI interface, but +//! every op fails with [`Unimplemented`] without acquiring privilege or +//! touching the system. -use crate::config::Network; -use clap::Subcommand; -use serde::Serialize; +use crate::util::jailer::VmId; +use clap::{Args, Subcommand}; use thiserror::Error as ThisError; -pub use host_init::HostInitArgs; -pub use prepare::PrepareArgs; -pub use teardown::TeardownArgs; -pub use teardown_orphan::TeardownOrphanArgs; - -/// `[network]` is absent from config, i.e. VM networking is disabled. +/// Returned by every `network` op until VM networking is rewritten. #[derive(Debug, ThisError, PartialEq, Eq)] -#[error("VM networking is not configured ([network] absent from config)")] -pub struct NetworkingDisabled; +#[error("VM networking is unimplemented (pending the networking rewrite)")] +pub struct Unimplemented; + +#[derive(Args)] +pub struct PrepareArgs { + /// Microvm id. + #[arg(long)] + pub vm_id: VmId, + /// Unprivileged uid the VM runs as. + #[arg(long)] + pub uid: u32, +} -/// Refuse before any privileged command is ever spawned when `[network]` is -/// absent from config. Pure — takes the already-loaded `Option<&Network>` -/// rather than reaching into [`crate::config::Config`] itself — so this exact -/// refusal path is exercisable from tests without root or a live config file, -/// and `prepare`/`teardown`/`host_init` all resolve through this one function -/// rather than duplicating the check. -pub fn resolve_network(network: Option<&Network>) -> Result<&Network, NetworkingDisabled> { - network.ok_or(NetworkingDisabled) +#[derive(Args)] +pub struct TeardownArgs { + /// Microvm id. + #[arg(long)] + pub vm_id: VmId, + /// Unprivileged uid the VM ran as. + #[arg(long)] + pub uid: u32, } -/// The result shape shared by all three networking ops, tagged by which ran. -#[derive(Serialize)] -#[serde(tag = "result", rename_all = "snake_case")] -pub enum NetworkOut { - Prepared, - ToreDown, - HostReady, +#[derive(Args)] +pub struct TeardownOrphanArgs { + /// Microvm id. + #[arg(long)] + pub vm_id: VmId, } +#[derive(Args)] +pub struct HostInitArgs {} + #[derive(Subcommand)] pub enum NetworkOp { - /// Create a VM's netns, veth pair, TAP device, and in-netns NAT. + /// Set up a VM's networking (unimplemented). Prepare(PrepareArgs), - /// Remove a VM's netns and any lingering host-side veth end. + /// Tear down a VM's networking (unimplemented). Teardown(TeardownArgs), - /// Idempotently remove an orphan VM's netns by vm_id alone (no uid). + /// Reclaim an orphan VM's networking by vm_id alone (unimplemented). TeardownOrphan(TeardownOrphanArgs), - /// One-time host setup: the `hyper` nftables table and forward policy. + /// One-time host networking setup (unimplemented). HostInit(HostInitArgs), } impl NetworkOp { - /// Route to the selected op; each runs in its own privileged scope and - /// returns its own serialized `Value`. + /// Every op refuses with [`Unimplemented`]; nothing is run. pub fn run(self) -> Result { match self { - NetworkOp::Prepare(args) => prepare::run(args), - NetworkOp::Teardown(args) => teardown::run(args), - NetworkOp::TeardownOrphan(args) => teardown_orphan::run(args), - NetworkOp::HostInit(args) => host_init::run(args), + NetworkOp::Prepare(_) + | NetworkOp::Teardown(_) + | NetworkOp::TeardownOrphan(_) + | NetworkOp::HostInit(_) => Err(crate::tools::Error::Tool(Box::new(Unimplemented))), } } } diff --git a/native/suidhelper/src/tools/network/prepare.rs b/native/suidhelper/src/tools/network/prepare.rs deleted file mode 100644 index 3011cea6..00000000 --- a/native/suidhelper/src/tools/network/prepare.rs +++ /dev/null @@ -1,144 +0,0 @@ -//! `network prepare`: create a VM's netns, host/ns veth pair, TAP device, and -//! in-netns NAT so the guest's inner address can reach the host's uplink. -use super::addr::{AddrError, Plan}; -use super::args; -use super::exec; -use super::NetworkOut; -use crate::config::Config; -use crate::tools::jailer::{self, VmId}; -use crate::tools::IsTool; -use crate::util::safe_bin; -use clap::Args; -use std::net::Ipv4Addr; -use std::path::PathBuf; -use thiserror::Error as ThisError; - -#[derive(Args)] -pub struct PrepareArgs { - /// Microvm id; becomes the netns name. - #[arg(long)] - vm_id: VmId, - /// Unprivileged uid the VM runs as; derives its clone-pool `/30` slot. - #[arg(long)] - uid: u32, -} - -#[derive(Debug, ThisError)] -pub enum Error { - #[error(transparent)] - NetworkingDisabled(#[from] super::NetworkingDisabled), - #[error("clone_pool {0:?} is not a valid IPv4 CIDR")] - InvalidClonePool(String), - #[error(transparent)] - Uid(#[from] crate::util::jailer::Error), - #[error(transparent)] - Addr(#[from] AddrError), - #[error(transparent)] - Bin(#[from] safe_bin::Error), - #[error(transparent)] - Exec(#[from] exec::Error), -} - -/// Derive the [`Plan`] for `uid`, refusing before any command is ever run. -/// `uid` must satisfy [`jailer::validate_id_number`] against `range` — the -/// same check the jailer itself applies to `--uid`, so a uid the jailer would -/// refuse is refused here too — and `clone_pool_str` must be a parseable IPv4 -/// CIDR. -pub fn plan_from( - uid: u32, - range: (u32, u32), - clone_pool_str: &str, - vm_id: &VmId, -) -> Result { - jailer::validate_id_number(uid, range)?; - let clone_pool = parse_cidr_base(clone_pool_str)?; - Ok(Plan::derive(uid, range, clone_pool, vm_id)?) -} - -/// Parse an IPv4 CIDR's base address, e.g. `"172.31.0.0/16"` -> `172.31.0.0`. -/// The prefix length is validated (`0..=32`) but otherwise unused: `Plan::derive` -/// only needs the pool's base address, not its width. -fn parse_cidr_base(s: &str) -> Result { - let reject = || Error::InvalidClonePool(s.to_string()); - let mut parts = s.splitn(2, '/'); - let addr: Ipv4Addr = parts - .next() - .ok_or_else(reject)? - .parse() - .map_err(|_| reject())?; - let prefix: u8 = parts - .next() - .ok_or_else(reject)? - .parse() - .map_err(|_| reject())?; - if prefix > 32 { - return Err(reject()); - } - Ok(addr) -} - -/// Resolve the [`Plan`] and the config's `ip`/`nft` binaries for a `prepare` or -/// `teardown` op. Shared by both since they derive the identical `Plan` from -/// the same `uid` + config; only the argv sequence they run differs. Every -/// refusal here (`[network]` absent, uid out of range, malformed CIDR, a -/// misconfigured binary) happens before any privileged command is spawned. -pub(super) fn resolve(uid: u32, vm_id: &VmId) -> Result<(Plan, PathBuf, PathBuf, PathBuf), Error> { - let config = Config::get(); - let network = super::resolve_network(config.network())?; - let plan = plan_from(uid, config.uid_gid_range(), &network.clone_pool, vm_id)?; - Ok(( - plan, - config.ip()?.into(), - config.nft()?.into(), - config.sysctl()?.into(), - )) -} - -pub fn run(args: PrepareArgs) -> Result { - Prepare::new(args) - .map_err(|e| crate::tools::Error::Tool(Box::new(e)))? - .run() -} - -struct Prepare { - plan: Plan, - ip: PathBuf, - nft: PathBuf, - sysctl: PathBuf, -} - -impl Prepare { - fn new(args: PrepareArgs) -> Result { - let (plan, ip, nft, sysctl) = resolve(args.uid, &args.vm_id)?; - Ok(Self { - plan, - ip, - nft, - sysctl, - }) - } -} - -impl IsTool for Prepare { - type Args = PrepareArgs; - type Output = NetworkOut; - type RunT = Result<(), Error>; - - fn run_privileged(&self) -> Self::RunT { - let commands = args::prepare_commands( - &self.plan, - &self.nft.to_string_lossy(), - &self.sysctl.to_string_lossy(), - ); - exec::run_all(&commands, |which| match which { - args::Which::Ip => self.ip.as_path(), - args::Which::Nft => self.nft.as_path(), - })?; - Ok(()) - } - - fn parse(&self, res: Self::RunT) -> Result> { - res?; - Ok(NetworkOut::Prepared) - } -} diff --git a/native/suidhelper/src/tools/network/teardown.rs b/native/suidhelper/src/tools/network/teardown.rs deleted file mode 100644 index 146b8401..00000000 --- a/native/suidhelper/src/tools/network/teardown.rs +++ /dev/null @@ -1,70 +0,0 @@ -//! `network teardown`: remove a VM's netns, reclaiming its veth peer, TAP -//! device, and in-netns nftables state, plus any lingering host-side veth end. -use super::args; -use super::exec; -use super::prepare; -use super::NetworkOut; -use crate::tools::jailer::VmId; -use crate::tools::IsTool; -use clap::Args; -use std::path::PathBuf; -use thiserror::Error as ThisError; - -#[derive(Args)] -pub struct TeardownArgs { - /// Microvm id; names the netns to remove. - #[arg(long)] - vm_id: VmId, - /// Unprivileged uid the VM ran as; derives its clone-pool `/30` slot. - #[arg(long)] - uid: u32, -} - -#[derive(Debug, ThisError)] -pub enum Error { - #[error(transparent)] - Plan(#[from] prepare::Error), - #[error(transparent)] - Exec(#[from] exec::Error), -} - -pub fn run(args: TeardownArgs) -> Result { - Teardown::new(args) - .map_err(|e| crate::tools::Error::Tool(Box::new(e)))? - .run() -} - -struct Teardown { - plan: super::addr::Plan, - ip: PathBuf, - nft: PathBuf, -} - -impl Teardown { - fn new(args: TeardownArgs) -> Result { - // Teardown only deletes the netns/veth (no in-netns sysctl), so the - // resolved sysctl path is unused here. - let (plan, ip, nft, _sysctl) = prepare::resolve(args.uid, &args.vm_id)?; - Ok(Self { plan, ip, nft }) - } -} - -impl IsTool for Teardown { - type Args = TeardownArgs; - type Output = NetworkOut; - type RunT = Result<(), Error>; - - fn run_privileged(&self) -> Self::RunT { - let commands = args::teardown_commands(&self.plan); - exec::run_all(&commands, |which| match which { - args::Which::Ip => self.ip.as_path(), - args::Which::Nft => self.nft.as_path(), - })?; - Ok(()) - } - - fn parse(&self, res: Self::RunT) -> Result> { - res?; - Ok(NetworkOut::ToreDown) - } -} diff --git a/native/suidhelper/src/tools/network/teardown_orphan.rs b/native/suidhelper/src/tools/network/teardown_orphan.rs deleted file mode 100644 index 72f64da6..00000000 --- a/native/suidhelper/src/tools/network/teardown_orphan.rs +++ /dev/null @@ -1,124 +0,0 @@ -//! `network teardown-orphan`: reclaim a crashed node's leaked VM netns by -//! vm_id alone. Unlike [`super::teardown`], this takes no `uid` — a crashed -//! node never persisted one for `Hyper.Node.Reaper` to pass back in — so it -//! cannot derive a full [`super::addr::Plan`] and instead runs the one -//! command that needs no derived address or interface name: `ip netns del -//! `, which reclaims the ns-side veth peer, TAP device, and in-netns -//! nftables state with it (see `args::teardown_orphan_commands`). -//! -//! Idempotent by design, not by accident: the reaper races a normal -//! `teardown/2` that may already have deleted the netns (or one that never -//! finished being created), so `ip netns del` failing with "No such file or -//! directory" is treated as success rather than propagated. Any other -//! failure (wrong permissions, `ip` itself unusable) still errors. The real -//! ENOENT path only exists once a genuine `/var/run/netns` entry is deleted -//! out from under a concurrent call, which needs root and a real netns to -//! exercise — covered by the root e2e / reaper e2e suites, not this crate's -//! unit tests. -use super::args::{self, Command}; -use super::NetworkOut; -use crate::config::Config; -use crate::tools::jailer::VmId; -use crate::tools::IsTool; -use crate::util::safe_bin; -use clap::Args; -use std::path::{Path, PathBuf}; -use std::process::Command as Proc; -use thiserror::Error as ThisError; - -#[derive(Args)] -pub struct TeardownOrphanArgs { - /// Microvm id; names the netns to remove. No `uid` — see the module doc. - #[arg(long)] - vm_id: VmId, -} - -#[derive(Debug, ThisError)] -pub enum Error { - #[error(transparent)] - NetworkingDisabled(#[from] super::NetworkingDisabled), - #[error(transparent)] - Bin(#[from] safe_bin::Error), - #[error("running ip {argv:?}: {source}")] - Spawn { - argv: Vec, - #[source] - source: std::io::Error, - }, - #[error("ip {argv:?} failed: {stderr}")] - Failed { argv: Vec, stderr: String }, -} - -pub fn run(args: TeardownOrphanArgs) -> Result { - TeardownOrphan::new(args) - .map_err(|e| crate::tools::Error::Tool(Box::new(e)))? - .run() -} - -struct TeardownOrphan { - netns: String, - ip: PathBuf, -} - -impl TeardownOrphan { - fn new(args: TeardownOrphanArgs) -> Result { - let config = Config::get(); - super::resolve_network(config.network())?; - Ok(Self { - netns: args.vm_id.to_string(), - ip: config.ip()?.into(), - }) - } -} - -impl IsTool for TeardownOrphan { - type Args = TeardownOrphanArgs; - type Output = NetworkOut; - type RunT = Result<(), Error>; - - fn run_privileged(&self) -> Self::RunT { - run_idempotent(&args::teardown_orphan_commands(&self.netns), &self.ip) - } - - fn parse(&self, res: Self::RunT) -> Result> { - res?; - Ok(NetworkOut::ToreDown) - } -} - -/// Run each command against `ip`, tolerating an ENOENT-shaped failure (the -/// namespace is already gone) as success. Kept separate from -/// `super::exec::run_all`, whose contract for `prepare`/`teardown` is that -/// any non-`allow_failure` non-zero exit is a real error — this op's whole -/// point is that a missing netns is *not* an error. -fn run_idempotent(commands: &[Command], ip: &Path) -> Result<(), Error> { - for command in commands { - let output = Proc::new(ip) - .args(&command.argv) - .env_clear() - .output() - .map_err(|source| Error::Spawn { - argv: command.argv.clone(), - source, - })?; - - if output.status.success() { - continue; - } - - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - if is_enoent(&stderr) { - continue; - } - - return Err(Error::Failed { - argv: command.argv.clone(), - stderr, - }); - } - Ok(()) -} - -fn is_enoent(stderr: &str) -> bool { - stderr.to_lowercase().contains("no such file or directory") -} diff --git a/native/suidhelper/src/util/chroot_jail.rs b/native/suidhelper/src/util/chroot_jail.rs index d72e6396..c8c8b41b 100644 --- a/native/suidhelper/src/util/chroot_jail.rs +++ b/native/suidhelper/src/util/chroot_jail.rs @@ -8,7 +8,7 @@ use crate::util::safe_file::{self, Any, IsBlockDevice, SafeFile}; use crate::util::safe_path::{self, IsAbsolute, SafePath, StrictComponents}; use nix::errno::Errno; use nix::fcntl::{open as nix_open, OFlag}; -use nix::sys::stat::{makedev, Mode}; +use nix::sys::stat::Mode; use std::io; use std::os::unix::io::FromRawFd; use std::path::{Path, PathBuf}; @@ -19,10 +19,6 @@ use thiserror::Error as ThisError; const KERNEL_NAME: &str = "vmlinux"; const ROOTFS_NAME: &str = "rootfs"; -/// `/dev/net/tun`'s well-known major:minor, per Linux's `Documentation/admin-guide/devices.txt`. -const TUN_MAJOR: u64 = 10; -const TUN_MINOR: u64 = 200; - #[derive(Debug, ThisError)] pub enum Error { #[error(transparent)] @@ -125,11 +121,7 @@ impl ChrootJail { pub fn build_under(self, jail_base: &Path, hyper_base: &Path) -> Result<(), Error> { let chroot = open_chroot_under(jail_base, &self.chroot)?; stage_kernel_under(&chroot, &self.kernel.0, hyper_base, self.uid, self.gid)?; - make_rootfs(&chroot, &self.rootfs.0, self.uid, self.gid)?; - if Config::get().network().is_some() { - make_tun_node(&chroot, self.uid, self.gid)?; - } - Ok(()) + make_rootfs(&chroot, &self.rootfs.0, self.uid, self.gid) } } @@ -220,22 +212,3 @@ fn make_rootfs(chroot: &SafeDir, device: &BlockDev, uid: u32, gid: u32) -> Resul chroot.mknod_block(Path::new(ROOTFS_NAME), rdev, uid, gid)?; Ok(()) } - -/// Create `dev/net/tun` (char `10:200`, mode `0666`) inside the jail, owned -/// `uid:gid`, so the chrooted firecracker can `open("/dev/net/tun")` and -/// `TUNSETIFF` onto the tap device waiting in its netns. Only called when VM -/// networking is enabled; a jail built without it has no `dev` tree at all -/// today (see [`ChrootJail::build_under`]). -fn make_tun_node(chroot: &SafeDir, uid: u32, gid: u32) -> Result<(), Error> { - let dev = Path::new("dev"); - chroot.mkdir(dev, 0o755)?; - let dev_dir = chroot.openat_dir(dev)?; - - let net = Path::new("net"); - dev_dir.mkdir(net, 0o755)?; - let net_dir = dev_dir.openat_dir(net)?; - - let rdev = makedev(TUN_MAJOR, TUN_MINOR); - net_dir.mknod_char(Path::new("tun"), rdev, uid, gid)?; - Ok(()) -} diff --git a/native/suidhelper/src/util/jailer.rs b/native/suidhelper/src/util/jailer.rs index df104593..1d216f0b 100644 --- a/native/suidhelper/src/util/jailer.rs +++ b/native/suidhelper/src/util/jailer.rs @@ -214,11 +214,6 @@ pub struct Jailer<'a> { gid: u32, cgroups: &'a [CgroupSetting], api_sock: &'a JailSock, - /// The netns the jailer must enter (`--netns`), `Some` when VM networking is - /// enabled. Derived by the caller from trusted config + the validated `vm` - /// id — never from caller input — and must match the `ip netns add ` - /// path the `network` tool creates. - netns: Option<&'a Path>, } impl Jailer<'_> { @@ -249,11 +244,6 @@ impl Jailer<'_> { argv.push(cg.to_string().to_cstring()?); } - if let Some(netns) = self.netns { - argv.push("--netns".to_cstring()?); - argv.push(netns.to_cstring()?); - } - argv.push("--".to_cstring()?); argv.push("--api-sock".to_cstring()?); argv.push(self.api_sock.to_string().to_cstring()?); diff --git a/native/suidhelper/src/util/safe_dir.rs b/native/suidhelper/src/util/safe_dir.rs index 515b1e81..3a7f969d 100644 --- a/native/suidhelper/src/util/safe_dir.rs +++ b/native/suidhelper/src/util/safe_dir.rs @@ -15,12 +15,9 @@ use super::safe_file::{Any, SafeFile}; use super::safe_path::SafePath; use nix::dir::{Dir, Type}; -use nix::errno::Errno; use nix::fcntl::{openat, AtFlags, OFlag}; use nix::libc::dev_t; -use nix::sys::stat::{ - fchmod, fchmodat, fstatat, mkdirat, mknodat, FchmodatFlags, FileStat, Mode, SFlag, -}; +use nix::sys::stat::{fchmod, fchmodat, fstatat, mknodat, FchmodatFlags, FileStat, Mode, SFlag}; use nix::unistd::{dup, fchown, fchownat, linkat, unlinkat, write, Gid, Uid, UnlinkatFlags}; use std::ffi::OsStr; use std::os::unix::ffi::OsStrExt; @@ -38,8 +35,6 @@ pub enum Error { Unlink { name: PathBuf, source: nix::Error }, #[error("write {name:?}: {source}")] Write { name: PathBuf, source: nix::Error }, - #[error("mkdirat {name:?}: {source}")] - Mkdir { name: PathBuf, source: nix::Error }, #[error("mknodat {name:?}: {source}")] Mknod { name: PathBuf, source: nix::Error }, #[error("fchownat {name:?}: {source}")] @@ -62,7 +57,6 @@ impl Error { Error::Open { source, .. } | Error::Unlink { source, .. } | Error::Write { source, .. } - | Error::Mkdir { source, .. } | Error::Mknod { source, .. } | Error::Chown { source, .. } | Error::Chmod { source, .. } @@ -198,55 +192,6 @@ impl SafeDir { self.chown(name, uid, gid) } - /// Create a character device node `name` in this directory (mode `0666`, - /// matching `/dev/net/tun`'s conventional permissions) with the given - /// `rdev`, then chown it to `uid:gid`. - /// - /// `mknodat` masks its mode argument with the process umask (typically 022, - /// which would yield `0644`), so the exact `0666` is set with an explicit - /// `chmod` afterwards — `fchmodat` ignores the umask. - pub fn mknod_char(&self, name: &Path, rdev: dev_t, uid: u32, gid: u32) -> Result<(), Error> { - // Tolerate EEXIST: jail staging is re-run when a VM cold-boots after a - // failed attempt, so the node may survive from the prior attempt. The - // chown+chmod below re-assert the exact ownership and mode regardless, - // so a pre-existing node ends up identical to a freshly-created one. - match mknodat( - Some(self.0.as_raw_fd()), - name, - SFlag::S_IFCHR, - Mode::from_bits_truncate(0o666), - rdev, - ) { - Ok(()) | Err(Errno::EEXIST) => {} - Err(source) => { - return Err(Error::Mknod { - name: name.to_path_buf(), - source, - }) - } - } - self.chown(name, uid, gid)?; - self.chmod(name, 0o666) - } - - /// Create directory `name` in this directory with `mode`, tolerating - /// `EEXIST` (`mkdir -p` semantics): the jailer may have already created - /// intermediate directories (e.g. `dev`, to bind-mount `/dev/kvm`) before - /// this helper runs. - pub fn mkdir(&self, name: &Path, mode: u32) -> Result<(), Error> { - match mkdirat( - Some(self.0.as_raw_fd()), - name, - Mode::from_bits_truncate(mode), - ) { - Ok(()) | Err(Errno::EEXIST) => Ok(()), - Err(source) => Err(Error::Mkdir { - name: name.to_path_buf(), - source, - }), - } - } - /// Hard-link the file at host path `src` into this directory as `name`. pub fn link_from(&self, src: &Path, name: &Path) -> Result<(), Error> { linkat( diff --git a/native/suidhelper/tests/config_network.rs b/native/suidhelper/tests/config_network.rs index 9e2cf7e5..1da3b72b 100644 --- a/native/suidhelper/tests/config_network.rs +++ b/native/suidhelper/tests/config_network.rs @@ -1,30 +1,23 @@ -use hyper_suidhelper::config::{Config, Network}; +//! Contract: VM networking is unimplemented pending a rewrite, but operator +//! configs written before it must keep loading. A legacy `[network]` table and +//! `[tools] ip`/`nft`/`sysctl` keys are accepted and ignored. `Config` is a +//! process-wide `LazyLock` that exits the helper on any parse failure, so +//! rejecting these keys would brick every tool, not just networking. +use hyper_suidhelper::config::Config; #[test] -fn absent_network_table_means_disabled() { - // Default config (no file) has no [network] table. - let cfg = Config::default(); - assert!(cfg.network().is_none()); -} - -#[test] -fn present_table_with_only_uplink_defaults_clone_pool() { - // clone_pool is safety-critical: it must match Elixir's - // Hyper.Cfg.Network.clone_pool/0 default. This is the present-path test — - // it fails if this default ever drifts from the Elixir literal. - let network: Network = toml::from_str("uplink = \"eth0\"\n").expect("valid TOML"); - assert_eq!(network.clone_pool, "172.31.0.0/16"); -} - -#[test] -fn partial_network_table_without_uplink_disables_not_bricks() { - // A `[network]` table with `clone_pool` set but `uplink` omitted must - // parse the WHOLE config successfully (not fail the process-wide, - // exit-on-error `Config` load that every subcommand — dmsetup, losetup, - // chroot-jail, jailer, not just networking — depends on) and must resolve - // to networking-disabled, mirroring Elixir's Hyper.Cfg.Network.enabled?/0 - // (uplink absent ⇒ disabled, not an error). - let toml_str = "work_dir = \"/srv/hyper\"\n[network]\nclone_pool = \"10.0.0.0/8\"\n"; - let config: Config = toml::from_str(toml_str).expect("a partial [network] table must parse"); - assert!(config.network().is_none()); +fn legacy_network_keys_are_accepted_and_ignored() { + let toml_str = "\ +work_dir = \"/srv/hyper\" +[tools] +ip = \"/usr/bin/ip\" +nft = \"/usr/sbin/nft\" +sysctl = \"/usr/sbin/sysctl\" +[network] +uplink = \"eth0\" +clone_pool = \"172.31.0.0/16\" +resolver = \"1.1.1.1\" +"; + let config: Config = toml::from_str(toml_str).expect("legacy networking keys must parse"); + assert_eq!(config.hyper_base(), std::path::Path::new("/srv/hyper")); } diff --git a/native/suidhelper/tests/e2e/chroot_jail.rs b/native/suidhelper/tests/e2e/chroot_jail.rs index fcea9aed..37524a79 100644 --- a/native/suidhelper/tests/e2e/chroot_jail.rs +++ b/native/suidhelper/tests/e2e/chroot_jail.rs @@ -3,15 +3,10 @@ //! never touch /etc/hyper or /srv. Root-gated: the helper acquires privileges for //! mknod/chown, so these self-skip without root. The cgroup half is exercised //! only with a MISSING leaf (idempotent, mutates nothing on the real host). -//! -//! The `dev/net/tun` node `prepare` creates when `[network]` is configured is -//! likewise privileged `mknod` — no unit-level substitute exists; it is proven -//! only here, root-gated, with the insecure config seam supplying a `[network]` -//! table. #![cfg(feature = "insecure_test_seams")] use std::fs; -use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt}; +use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::path::{Path, PathBuf}; use std::process::Command; @@ -27,22 +22,6 @@ fn write_config(dir: &Path, work_dir: &Path) -> PathBuf { p } -/// Same as [`write_config`], plus a `[network]` table — the gate that makes -/// `chroot-jail prepare` also mknod `dev/net/tun` into the jail. -fn write_config_with_network(dir: &Path, work_dir: &Path) -> PathBuf { - let p = dir.join("config.toml"); - fs::write( - &p, - format!( - "work_dir = \"{}\"\n[network]\nuplink = \"eth0\"\n", - work_dir.display() - ), - ) - .unwrap(); - fs::set_permissions(&p, fs::Permissions::from_mode(0o644)).unwrap(); - p -} - fn setup_loop(tmp: &Path) -> Option { let backing = tmp.join("backing.img"); let f = fs::File::create(&backing).ok()?; @@ -123,65 +102,6 @@ fn prepare_succeeds_and_builds_jail_as_root() { ); } -// `chroot-jail prepare` against a config with `[network]` set also mknods -// `dev/net/tun` into the jail: char device 10:200, mode 0666, owned uid:gid. -#[test] -fn prepare_creates_tun_node_when_networking_enabled_as_root() { - if !is_root() { - eprintln!("SKIP prepare_creates_tun_node_when_networking_enabled: needs root"); - return; - } - let tmp = tempfile::tempdir().unwrap(); - let work = tmp.path().join("srv"); - let chroot = work.join("jails").join("exec").join("id"); - fs::create_dir_all(&chroot).unwrap(); - let kernel = work.join("vmlinux-src"); - fs::write(&kernel, b"kernel image").unwrap(); - let cfg = write_config_with_network(tmp.path(), &work); - - let Some(dev) = setup_loop(tmp.path()) else { - eprintln!("SKIP prepare_creates_tun_node: losetup unavailable"); - return; - }; - - let uid = 1234; - let gid = 5678; - let out = run( - &cfg, - &[ - "chroot-jail", - "prepare", - "--chroot", - chroot.to_str().unwrap(), - "--kernel", - kernel.to_str().unwrap(), - "--device", - dev.to_str().unwrap(), - "--uid", - &uid.to_string(), - "--gid", - &gid.to_string(), - ], - ); - teardown_loop(&dev); - - assert_eq!( - out.status.code(), - Some(0), - "stderr: {}", - String::from_utf8_lossy(&out.stderr) - ); - - let tun = chroot.join("dev").join("net").join("tun"); - let meta = fs::metadata(&tun).expect("dev/net/tun must exist"); - assert!(meta.file_type().is_char_device(), "must be a char device"); - assert_eq!(nix::sys::stat::major(meta.rdev()), 10); - assert_eq!(nix::sys::stat::minor(meta.rdev()), 200); - assert_eq!(meta.uid(), uid); - assert_eq!(meta.gid(), gid); - assert_eq!(meta.permissions().mode() & 0o777, 0o666); -} - // `chroot-jail prepare` with a system device (not a loop / hyper-* dm) is // rejected by BlockDev parsing at clap parse time — exit 2, nothing built. This // case needs no root (clap rejects before the privilege boundary) but also no diff --git a/native/suidhelper/tests/e2e/jailer.rs b/native/suidhelper/tests/e2e/jailer.rs index 8ed0dc4c..26cbd9a2 100644 --- a/native/suidhelper/tests/e2e/jailer.rs +++ b/native/suidhelper/tests/e2e/jailer.rs @@ -64,12 +64,13 @@ fn write_root_config(dir: &Path, jailer: &Path, firecracker: &Path) -> PathBuf { p } -/// Same as [`write_root_config`] but with a `[network]` table present, so -/// `Config::network()` is `Some` and `run()` derives a `--netns` flag. -fn write_root_config_with_network(dir: &Path, jailer: &Path, firecracker: &Path) -> PathBuf { +/// Same as [`write_root_config`] plus a legacy `[network]` table and +/// `[tools] ip`/`nft`/`sysctl` keys, as an operator config written before the +/// networking rewrite would carry. +fn write_root_config_with_legacy_network(dir: &Path, jailer: &Path, firecracker: &Path) -> PathBuf { let p = dir.join("config.toml"); let body = format!( - "work_dir = \"/srv/hyper\"\n[tools]\njailer = \"{}\"\nfirecracker = \"{}\"\n[network]\nuplink = \"eth0\"\n", + "work_dir = \"/srv/hyper\"\n[tools]\njailer = \"{}\"\nfirecracker = \"{}\"\nip = \"/usr/bin/ip\"\nnft = \"/usr/sbin/nft\"\nsysctl = \"/usr/sbin/sysctl\"\n[network]\nuplink = \"eth0\"\nclone_pool = \"172.31.0.0/16\"\n", jailer.display(), firecracker.display(), ); @@ -174,9 +175,11 @@ fn execs_jailer_with_canonical_argv_and_empty_env_as_root() { } #[test] -fn execs_jailer_with_netns_flag_when_networking_enabled_as_root() { +fn execs_jailer_without_netns_with_legacy_network_table_as_root() { if !is_root() { - eprintln!("SKIP jailer netns: needs root to become_root_permanently + own the fakes"); + eprintln!( + "SKIP jailer legacy [network]: needs root to become_root_permanently + own the fakes" + ); return; } let tmp = tempfile::tempdir().unwrap(); @@ -184,7 +187,7 @@ fn execs_jailer_with_netns_flag_when_networking_enabled_as_root() { let env_rec = tmp.path().join("environ.bin"); let jailer = install_recorder(tmp.path(), &argv_rec, &env_rec); let firecracker = install_firecracker(tmp.path()); - let cfg = write_root_config_with_network(tmp.path(), &jailer, &firecracker); + let cfg = write_root_config_with_legacy_network(tmp.path(), &jailer, &firecracker); let out = run( &cfg, @@ -210,19 +213,9 @@ fn execs_jailer_with_netns_flag_when_networking_enabled_as_root() { let argv: Vec = serde_json::from_str(&fs::read_to_string(&argv_rec).expect("recorded argv")).unwrap(); - let i = argv - .iter() - .position(|a| a == "--netns") - .expect("--netns present when [network] is configured"); - assert_eq!( - argv[i + 1], - "/var/run/netns/vnet1", - "netns path must be derived from --id, not caller-supplied" - ); - let sep = argv.iter().position(|a| a == "--").unwrap(); assert!( - i < sep, - "--netns must precede the `--` firecracker separator" + !argv.iter().any(|a| a == "--netns"), + "the jailer must never be told to enter a netns: {argv:?}" ); } diff --git a/native/suidhelper/tests/tools/host_init.rs b/native/suidhelper/tests/tools/host_init.rs deleted file mode 100644 index 9e195adf..00000000 --- a/native/suidhelper/tests/tools/host_init.rs +++ /dev/null @@ -1,44 +0,0 @@ -//! `ip_forward_enabled` is the pure predicate `host_init` uses to decide -//! whether `/proc/sys/net/ipv4/ip_forward` says forwarding is on. Law under -//! test: it is `true` exactly for `"1"` modulo surrounding whitespace, and -//! `false` for everything else (kernel default `"0"`, empty, garbage). -use hyper_suidhelper::tools::network::host_init::ip_forward_enabled; -use proptest::prelude::*; - -#[test] -fn enabled_for_bare_one() { - assert!(ip_forward_enabled("1")); -} - -#[test] -fn enabled_for_one_with_trailing_newline() { - assert!(ip_forward_enabled("1\n")); -} - -#[test] -fn disabled_for_zero() { - assert!(!ip_forward_enabled("0")); - assert!(!ip_forward_enabled("0\n")); -} - -#[test] -fn disabled_for_empty_contents() { - assert!(!ip_forward_enabled("")); -} - -proptest! { - #[test] - fn whitespace_padding_around_one_is_still_enabled( - leading in "[ \t\n]{0,4}", - trailing in "[ \t\n]{0,4}", - ) { - let contents = format!("{leading}1{trailing}"); - prop_assert!(ip_forward_enabled(&contents)); - } - - #[test] - fn anything_but_one_after_trim_is_disabled(digits in "[0-9]{2,4}") { - // Multi-digit strings never equal the literal "1" after trimming. - prop_assert!(!ip_forward_enabled(&digits)); - } -} diff --git a/native/suidhelper/tests/tools/network_addr.rs b/native/suidhelper/tests/tools/network_addr.rs deleted file mode 100644 index 96ec7d93..00000000 --- a/native/suidhelper/tests/tools/network_addr.rs +++ /dev/null @@ -1,79 +0,0 @@ -use hyper_suidhelper::tools::jailer::VmId; -use hyper_suidhelper::tools::network::addr::{self, AddrError, Plan}; -use proptest::prelude::*; -use std::net::Ipv4Addr; -use std::str::FromStr; - -const POOL: Ipv4Addr = Ipv4Addr::new(172, 31, 0, 0); -const RANGE: (u32, u32) = (900_000, 999_999); - -fn vm_id() -> VmId { - VmId::from_str("vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap() -} - -#[test] -fn slot_zero_is_first_block() { - let p = Plan::derive(900_000, RANGE, POOL, &vm_id()).unwrap(); - assert_eq!(p.slot, 0); - assert_eq!(p.veth_host_ip, Ipv4Addr::new(172, 31, 0, 1)); - assert_eq!(p.veth_ns_ip, Ipv4Addr::new(172, 31, 0, 2)); - assert_eq!(p.veth_host, "hv0"); - assert_eq!(p.veth_ns, "hp0"); -} - -#[test] -fn uid_below_range_is_refused() { - assert!(matches!( - Plan::derive(42, RANGE, POOL, &vm_id()), - Err(AddrError::UidBelowRange { .. }) - )); -} - -proptest! { - // Inner and clone address spaces never overlap, and veth names always fit - // IFNAMSIZ, for every uid the helper would ever accept. - #[test] - fn derived_plan_is_well_formed(offset in 0u32..16_384) { - let uid = RANGE.0 + offset; - let p = Plan::derive(uid, RANGE, POOL, &vm_id()).unwrap(); - prop_assert!(p.veth_host.len() <= 15 && p.veth_ns.len() <= 15); - // clone addresses live in 172.31/16, never in the inner 172.30/30 - prop_assert_eq!(p.veth_host_ip.octets()[1], 31); - prop_assert_ne!(p.veth_host_ip, addr::INNER_GUEST_IP); - // host and ns ends are distinct and adjacent - prop_assert_eq!(u32::from(p.veth_ns_ip), u32::from(p.veth_host_ip) + 1); - } - - // A slot past the /16's capacity is always refused, never wrapped. - #[test] - fn slot_past_pool_is_refused(offset in 16_384u32..40_000) { - let uid = RANGE.0 + offset; - let result = Plan::derive(uid, RANGE, POOL, &vm_id()); - let is_slot_out_of_pool = matches!(result, Err(AddrError::SlotOutOfPool { .. })); - prop_assert!(is_slot_out_of_pool); - } - - // Two distinct uids always derive non-overlapping /30 blocks: the gap - // between their veth_host_ip addresses is exactly 4x the slot distance, - // never wrapping one VM's address into another's. - #[test] - fn distinct_slots_never_share_a_block( - offset_a in 0u32..16_384, - offset_b in 0u32..16_384, - ) { - let uid_a = RANGE.0 + offset_a; - let uid_b = RANGE.0 + offset_b; - let plan_a = Plan::derive(uid_a, RANGE, POOL, &vm_id()).unwrap(); - let plan_b = Plan::derive(uid_b, RANGE, POOL, &vm_id()).unwrap(); - - let ip_a = u32::from(plan_a.veth_host_ip); - let ip_b = u32::from(plan_b.veth_host_ip); - let slot_gap = i64::from(plan_b.slot) - i64::from(plan_a.slot); - let ip_gap = i64::from(ip_b) - i64::from(ip_a); - - prop_assert_eq!(ip_gap, slot_gap * 4); - if plan_a.slot != plan_b.slot { - prop_assert_ne!(ip_a, ip_b); - } - } -} diff --git a/native/suidhelper/tests/tools/network_args.rs b/native/suidhelper/tests/tools/network_args.rs deleted file mode 100644 index 98e6226f..00000000 --- a/native/suidhelper/tests/tools/network_args.rs +++ /dev/null @@ -1,435 +0,0 @@ -use hyper_suidhelper::config::Network; -use hyper_suidhelper::tools::jailer::VmId; -use hyper_suidhelper::tools::network::addr::Plan; -use hyper_suidhelper::tools::network::args::{self, Which}; -use hyper_suidhelper::tools::network::{prepare, resolve_network, NetworkingDisabled}; -use std::net::Ipv4Addr; -use std::str::FromStr; - -fn plan0() -> Plan { - Plan::derive( - 900_000, - (900_000, 999_999), - Ipv4Addr::new(172, 31, 0, 0), - &VmId::from_str("vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap(), - ) - .unwrap() -} - -#[test] -fn prepare_creates_netns_first_and_default_route_last() { - let cmds = args::prepare_commands(&plan0(), "/usr/sbin/nft", "/usr/sbin/sysctl"); - let first = &cmds[0]; - assert!(matches!(first.bin, Which::Ip)); - assert_eq!( - first.argv, - vec!["netns", "add", "vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] - ); - // The default route (guest reachability) must come after the veth is up. - let route_idx = cmds - .iter() - .position(|c| c.argv.contains(&"default".to_string())) - .unwrap(); - let veth_up_idx = cmds - .iter() - .position(|c| c.argv == vec!["link", "set", "hv0", "up"]) - .unwrap(); - assert!(route_idx > veth_up_idx); -} - -#[test] -fn teardown_deletes_netns() { - let cmds = args::teardown_commands(&plan0()); - assert!(cmds - .iter() - .any(|c| c.argv == vec!["netns", "del", "vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"])); -} - -#[test] -fn teardown_orphan_is_exactly_ip_netns_del_by_id_alone() { - // No uid, no veth cleanup: an orphan's teardown has only the vm_id to work - // from (see `teardown_orphan`'s module doc), and `ip netns del` alone is - // sufficient to reclaim the veth peer, TAP, and in-netns nftables state. - let cmds = args::teardown_orphan_commands("vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); - assert_eq!( - cmds, - vec![cmd( - Which::Ip, - &["netns", "del", "vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] - )] - ); -} - -#[test] -fn host_init_masquerades_pool_out_uplink() { - let cmds = args::host_init_commands("eth0", "172.31.0.0/16"); - assert!(cmds.iter().any( - |c| c.argv.contains(&"masquerade".to_string()) && c.argv.contains(&"eth0".to_string()) - )); - // metadata IP is dropped - assert!(cmds - .iter() - .any(|c| c.argv.contains(&"169.254.169.254".to_string()))); -} - -#[test] -fn host_init_drops_guest_traffic_to_host() { - // The INPUT hook is a distinct chain from FORWARD: a guest packet - // addressed to a host-owned IP (its own gateway, or any other host IP) is - // locally delivered, never forwarded, so without an explicit input-chain - // drop a guest could reach host-local services (epmd, distribution, - // Postgres, gRPC) regardless of the forward-chain policy. Pinned as its - // own assertion so a future edit to the forward chain can't silently drop - // this isolation. - let cmds = args::host_init_commands("eth0", "172.31.0.0/16"); - assert!(cmds.iter().any(|c| c.bin == Which::Nft - && c.argv.contains(&"input".to_string()) - && c.argv.contains(&"saddr".to_string()) - && c.argv.contains(&"172.31.0.0/16".to_string()) - && c.argv.contains(&"drop".to_string()))); -} - -fn cmd(bin: Which, argv: &[&str]) -> args::Command { - args::Command { - bin, - argv: argv.iter().map(|s| s.to_string()).collect(), - allow_failure: false, - } -} - -fn cmd_allow_failure(bin: Which, argv: &[&str]) -> args::Command { - args::Command { - allow_failure: true, - ..cmd(bin, argv) - } -} - -/// Full-sequence golden test: pins every bin/argv, in order, so a wrong flag, -/// a wrong address, or a reordered/reversed SNAT/DNAT target fails here even -/// though it would slip past the substring checks above. -#[test] -fn prepare_commands_golden_sequence() { - let netns = "vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; - // In-netns nft runs via `ip netns exec ` — the absolute - // path, since the helper clears PATH before spawning (see nft_ns). - let nft = "/usr/sbin/nft"; - let sysctl = "/usr/sbin/sysctl"; - let cmds = args::prepare_commands(&plan0(), nft, sysctl); - let expected = vec![ - cmd(Which::Ip, &["netns", "add", netns]), - cmd( - Which::Ip, - &["link", "add", "hv0", "type", "veth", "peer", "name", "hp0"], - ), - cmd(Which::Ip, &["link", "set", "hp0", "netns", netns]), - cmd(Which::Ip, &["addr", "add", "172.31.0.1/30", "dev", "hv0"]), - cmd(Which::Ip, &["link", "set", "hv0", "up"]), - cmd( - Which::Ip, - &["-n", netns, "addr", "add", "172.31.0.2/30", "dev", "hp0"], - ), - cmd(Which::Ip, &["-n", netns, "link", "set", "hp0", "up"]), - cmd( - Which::Ip, - &["-n", netns, "tuntap", "add", "tap0", "mode", "tap"], - ), - cmd( - Which::Ip, - &["-n", netns, "addr", "add", "172.30.0.1/30", "dev", "tap0"], - ), - cmd(Which::Ip, &["-n", netns, "link", "set", "tap0", "up"]), - cmd(Which::Ip, &["-n", netns, "link", "set", "lo", "up"]), - cmd( - Which::Ip, - &["-n", netns, "route", "add", "default", "via", "172.31.0.1"], - ), - cmd( - Which::Ip, - &[ - "netns", - "exec", - netns, - sysctl, - "-w", - "net.ipv4.ip_forward=1", - ], - ), - cmd( - Which::Ip, - &["netns", "exec", netns, nft, "add", "table", "ip", "nat"], - ), - cmd( - Which::Ip, - &[ - "netns", - "exec", - netns, - nft, - "add", - "chain", - "ip", - "nat", - "post", - "{", - "type", - "nat", - "hook", - "postrouting", - "priority", - "100", - ";", - "}", - ], - ), - cmd( - Which::Ip, - &[ - "netns", - "exec", - netns, - nft, - "add", - "rule", - "ip", - "nat", - "post", - "ip", - "saddr", - "172.30.0.2", - "oifname", - "hp0", - "snat", - "to", - "172.31.0.2", - ], - ), - cmd( - Which::Ip, - &[ - "netns", - "exec", - netns, - nft, - "add", - "chain", - "ip", - "nat", - "pre", - "{", - "type", - "nat", - "hook", - "prerouting", - "priority", - "-100", - ";", - "}", - ], - ), - cmd( - Which::Ip, - &[ - "netns", - "exec", - netns, - nft, - "add", - "rule", - "ip", - "nat", - "pre", - "ip", - "daddr", - "172.31.0.2", - "dnat", - "to", - "172.30.0.2", - ], - ), - ]; - assert_eq!(cmds, expected); -} - -/// Full-sequence golden test for the host-init nftables program: pins the -/// leading, failure-tolerant `delete table` (the reconcile-to-desired-state -/// step — see `host_init.rs`'s module doc), the metadata-IP drop rule -/// immediately after the forward chain is created and before the broad -/// egress accept (since `accept` is a terminating verdict in nftables and a -/// drop reachable only after it would never fire), the trailing input-chain -/// isolation (drops all clone-pool-sourced traffic addressed to the host -/// itself, on the INPUT hook FORWARD never sees — see `host_init_commands`'s -/// doc), and that every command after the delete is *not* failure-tolerant. -#[test] -fn host_init_commands_golden_sequence() { - let cmds = args::host_init_commands("eth0", "172.31.0.0/16"); - let expected = vec![ - cmd_allow_failure(Which::Nft, &["delete", "table", "ip", "hyper"]), - cmd(Which::Nft, &["add", "table", "ip", "hyper"]), - cmd( - Which::Nft, - &[ - "add", - "chain", - "ip", - "hyper", - "postrouting", - "{", - "type", - "nat", - "hook", - "postrouting", - "priority", - "100", - ";", - "}", - ], - ), - cmd( - Which::Nft, - &[ - "add", - "rule", - "ip", - "hyper", - "postrouting", - "ip", - "saddr", - "172.31.0.0/16", - "oifname", - "eth0", - "masquerade", - ], - ), - cmd( - Which::Nft, - &[ - "add", "chain", "ip", "hyper", "forward", "{", "type", "filter", "hook", "forward", - "priority", "0", ";", "policy", "drop", ";", "}", - ], - ), - cmd( - Which::Nft, - &[ - "add", - "rule", - "ip", - "hyper", - "forward", - "ip", - "saddr", - "172.31.0.0/16", - "ip", - "daddr", - "169.254.169.254", - "drop", - ], - ), - cmd( - Which::Nft, - &[ - "add", - "rule", - "ip", - "hyper", - "forward", - "ip", - "saddr", - "172.31.0.0/16", - "oifname", - "eth0", - "accept", - ], - ), - cmd( - Which::Nft, - &[ - "add", - "rule", - "ip", - "hyper", - "forward", - "ip", - "daddr", - "172.31.0.0/16", - "ct", - "state", - "established,related", - "accept", - ], - ), - cmd( - Which::Nft, - &[ - "add", "chain", "ip", "hyper", "input", "{", "type", "filter", "hook", "input", - "priority", "0", ";", "policy", "accept", ";", "}", - ], - ), - cmd( - Which::Nft, - &[ - "add", - "rule", - "ip", - "hyper", - "input", - "ip", - "saddr", - "172.31.0.0/16", - "drop", - ], - ), - ]; - assert_eq!(cmds, expected); -} - -#[test] -fn prepare_refuses_uid_outside_range() { - // Derivation from an out-of-range uid must error before any command runs. - // Proving `Err` here is sufficient, not just necessary, to prove no - // privileged command spawns: `plan_from` only ever returns a `Plan`, and - // every op derives its commands from that returned `Plan` in a separate - // step downstream, one that is structurally unreachable when derivation - // itself fails. - let err = prepare::plan_from( - 42, - (900_000, 999_999), - "172.31.0.0/16", - &VmId::from_str("vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap(), - ); - assert!(err.is_err()); -} - -#[test] -fn prepare_refuses_malformed_clone_pool_cidr() { - // A malformed CIDR must error before any command runs, distinctly from a - // uid-range refusal. - let err = prepare::plan_from( - 900_000, - (900_000, 999_999), - "not-a-cidr", - &VmId::from_str("vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap(), - ); - assert!(err.is_err()); -} - -#[test] -fn resolve_network_refuses_when_networking_disabled() { - // `[network]` absent from config is the security-critical refusal: every - // op (`prepare`, `teardown`, `host_init`) resolves through this same pure - // function before spawning anything privileged, so this exercises the - // real refusal path, not a copy of it — no root and no config file - // needed, since it takes the already-resolved `Option<&Network>` rather - // than reaching into `Config` itself. - assert!(matches!(resolve_network(None), Err(NetworkingDisabled))); -} - -#[test] -fn resolve_network_returns_the_configured_network_when_present() { - let network = Network { - uplink: Some("eth0".to_string()), - clone_pool: "172.31.0.0/16".to_string(), - }; - let resolved = resolve_network(Some(&network)).unwrap(); - assert_eq!(resolved.uplink.as_deref(), Some("eth0")); - assert_eq!(resolved.clone_pool, "172.31.0.0/16"); -} diff --git a/setup.sh b/setup.sh index 868f7b91..001af367 100755 --- a/setup.sh +++ b/setup.sh @@ -41,7 +41,7 @@ sudo -v info "OS packages" sudo apt-get update sudo apt-get install -y \ - coreutils e2fsprogs libc-bin lvm2 nftables iproute2 skopeo \ + coreutils e2fsprogs libc-bin lvm2 skopeo \ thin-provisioning-tools util-linux "linux-modules-extra-$(uname -r)" info "device-mapper modules" @@ -53,11 +53,6 @@ printf 'dm_snapshot\ndm_thin_pool\nloop\n' \ sudo dmsetup targets | grep -q thin-pool \ || fail "thin-pool dm target missing after modprobe" -info "IPv4 forwarding (required — host-init only asserts it, never sets it)" -sudo sysctl -w net.ipv4.ip_forward=1 >/dev/null -echo 'net.ipv4.ip_forward=1' \ - | sudo tee /etc/sysctl.d/99-hyper-ip-forward.conf >/dev/null - # Ubuntu ships thin_dump as a symlink into pdata_tools; the helper's SafeBin # rejects symlinks, so install a dereferenced hard copy under the name the # multi-call binary dispatches on. Skip when a prior run already placed it -- @@ -77,10 +72,6 @@ if [ -f /etc/hyper/config.toml ]; then info "/etc/hyper/config.toml exists -- leaving it alone" else info "writing /etc/hyper/config.toml" - # A single-uplink host NATs guest egress out of its default-route NIC. - uplink="$(ip route show default | awk '{print $5; exit}')" - [ -n "$uplink" ] \ - || fail "could not detect a default-route interface -- set [network] uplink in /etc/hyper/config.toml by hand (see docs/cookbook/install.md)" sudo mkdir -p /etc/hyper sudo tee /etc/hyper/config.toml >/dev/null < Hyper.Node.stop_image_vm(vm) end) - - # One exec gambles on guest-agent readiness once (not per-check), with a - # generous deadline for a cold boot on a shared runner. Hyper.exec runs argv - # directly with no PATH (see its @doc) — a bare name returns exit 127 — so - # route through an absolute `/bin/sh -c` and let busybox's own default PATH - # resolve the `ip`/`wget` applets inside the guest. - # - # `ip addr show eth0` proves the uniform inner-world address is configured; - # the `wget` then proves real egress with an explicit HTTP 200 (not just a - # zero exit): `-S` writes response headers to stderr, merged via `2>&1` so we - # can match the `... 200 ...` status line. Alpine ships busybox wget, not - # curl; `-O /dev/null` discards the body but still drives the full - # netns → SNAT → veth → MASQUERADE → uplink path and DNS resolution. - script = - "echo '--ip--'; ip addr show eth0; " <> - "echo '--http--'; wget -S -O /dev/null http://example.com 2>&1" - - assert {:ok, %{stdout: out, exit_code: 0}} = - await_exec(vm, ["/bin/sh", "-c", script], :timer.seconds(120)) - - assert out =~ "172.30.0.2", "guest eth0 lacks the inner-world address:\n#{out}" - - assert out =~ ~r"HTTP/[\d.]+ 200\b", - "expected an HTTP 200 status line from the guest's egress fetch:\n#{out}" - end -end diff --git a/test/hyper/cfg/network_test.exs b/test/hyper/cfg/network_test.exs deleted file mode 100644 index 95fabdc4..00000000 --- a/test/hyper/cfg/network_test.exs +++ /dev/null @@ -1,45 +0,0 @@ -defmodule Hyper.Cfg.NetworkTest do - use ExUnit.Case, async: false - alias Hyper.Cfg.Network - alias Hyper.Cfg.Toml - - describe "clone_pool/0" do - test "defaults when unset" do - # Mirrors default_clone_pool() in native/suidhelper/src/config.rs — the - # two literals are safety-critical and must move together. - assert Network.clone_pool() == "172.31.0.0/16" - end - end - - describe "configured?/0" do - test "false when no uplink configured" do - # Base test config sets no [network] table. This is the predicate the - # startup preflight uses to refuse booting a node without networking. - refute Network.configured?() - end - end - - describe "uplink/0" do - test "raises Hyper.Cfg.MissingError when network.uplink is unset" do - # Pins the refusal contract: a [network] table present without `uplink` - # must raise, not silently disable networking or crash uninformatively. - on_exit(fn -> Toml.reload() end) - Toml.put_cache(%{"network" => %{}}) - - assert_raise Hyper.Cfg.MissingError, ~r/network\.uplink/, fn -> - Network.uplink() - end - end - - test "raises ArgumentError when network.uplink is not a string" do - # A non-string uplink must raise ArgumentError, not be silently coerced - # or passed to the setuid helper which would then build a broken netns. - on_exit(fn -> Toml.reload() end) - Toml.put_cache(%{"network" => %{"uplink" => 123}}) - - assert_raise ArgumentError, ~r/network\.uplink/, fn -> - Network.uplink() - end - end - end -end diff --git a/test/hyper/node/fire_vmm/boot_spec_test.exs b/test/hyper/node/fire_vmm/boot_spec_test.exs index b7aa8ea4..ce766ff0 100644 --- a/test/hyper/node/fire_vmm/boot_spec_test.exs +++ b/test/hyper/node/fire_vmm/boot_spec_test.exs @@ -3,12 +3,7 @@ defmodule Hyper.Node.FireVMM.BootSpecTest do alias Hyper.Node.FireVMM.BootSpec - # Networking is mandatory: every VM gets a NIC plus the kernel - # `ip=`/`hyper.resolver=` cmdline, derived entirely from the vm_id and the - # uniform inner world (no `[network]` config needed — the base test config - # has none). The pure formatting of those fragments is pinned in net_test.exs; - # this module pins that resolve/2 always emits them. - @source %{vm_id: "vabc", kernel_image_path: "/vmlinux", root_drive_path: "/rootfs"} + @source %{kernel_image_path: "/vmlinux", root_drive_path: "/rootfs"} test "default cmdline boots our agent as init with no serial console" do cold = BootSpec.resolve(@source, :micro) @@ -18,10 +13,9 @@ defmodule Hyper.Node.FireVMM.BootSpecTest do refute cold.boot_source.boot_args =~ "console=" end - test "every VM gets a NIC and the ip=/hyper.resolver= cmdline" do + test "a VM boots with no NIC-related kernel cmdline" do cold = BootSpec.resolve(@source, :micro) - assert length(cold.network_interfaces) == 1 - assert cold.boot_source.boot_args =~ "ip=" - assert cold.boot_source.boot_args =~ "hyper.resolver=" + refute cold.boot_source.boot_args =~ "ip=" + refute cold.boot_source.boot_args =~ "hyper.resolver=" end end diff --git a/test/hyper/node/fire_vmm/daemon_test.exs b/test/hyper/node/fire_vmm/daemon_test.exs index e564d7c9..342dc176 100644 --- a/test/hyper/node/fire_vmm/daemon_test.exs +++ b/test/hyper/node/fire_vmm/daemon_test.exs @@ -1,19 +1,17 @@ defmodule Hyper.Node.FireVMM.DaemonTest do @moduledoc """ - `Daemon` shells every privileged step (netns prepare/teardown, chroot - removal, jailer launch) through the setuid helper binary named by - `Hyper.Cfg.Tools.suidhelper/0`. These tests point that at a small fake - script instead of the real setuid helper — the same seam `JailerTest` uses - for `tools.firecracker`/`tools.jailer` — so they can observe real call - order and gating without root, a real netns, or a real firecracker process. - - Invariants under test (networking is mandatory — these fire unconditionally, - even with no `[network]` table in config): - - * the jailer enters the netns via `--netns` (Task 8), so it must already - exist: `network prepare` must run before the `jailer` launch. - * jail clearing (`chroot-jail remove`) and network teardown both fire - wherever the jail is cleared (stale-reset on relaunch, and terminate). + `Daemon` shells every privileged step (chroot removal, jailer launch) + through the setuid helper binary named by `Hyper.Cfg.Tools.suidhelper/0`. + These tests point that at a small fake script instead of the real setuid + helper — the same seam `JailerTest` uses for `tools.firecracker`/`tools.jailer` + — so they can observe the real calls without root or a real firecracker + process. + + Invariants under test: + + * the jailer is launched, and no `network` subcommand is ever invoked + (VM networking is unimplemented pending its rewrite). + * `terminate/2` clears the chroot jail. """ use ExUnit.Case, async: false @@ -83,10 +81,8 @@ defmodule Hyper.Node.FireVMM.DaemonTest do Hyper.Cfg.Toml.put_cache(%{"tools" => tools}) end - describe "netns lifecycle (mandatory)" do - test "start_link prepares the netns before launching the jailer", %{log: log, fake: fake} do - # No [network] table on purpose: the netns prepare must fire regardless, - # since networking is mandatory rather than config-gated. + describe "jail lifecycle" do + test "start_link launches the jailer without any network setup", %{log: log, fake: fake} do put_toml(fake) Process.flag(:trap_exit, true) @@ -94,15 +90,14 @@ defmodule Hyper.Node.FireVMM.DaemonTest do assert_receive {:EXIT, ^pid, _reason}, 2_000 ops = calls(log) - prepare_at = Enum.find_index(ops, &(&1 == "network")) - jailer_at = Enum.find_index(ops, &(&1 == "jailer")) - - assert prepare_at, "expected a `network` (prepare) call before the jailer launch" - assert jailer_at, "expected the jailer to have been launched" - assert prepare_at < jailer_at, "netns must be prepared before the jailer enters it" + assert "jailer" in ops + refute "network" in ops end - test "terminate/2 tears down both the network and the chroot jail", %{log: log, fake: fake} do + test "terminate/2 removes the chroot jail without any network teardown", %{ + log: log, + fake: fake + } do put_toml(fake) assert :ok = @@ -112,8 +107,8 @@ defmodule Hyper.Node.FireVMM.DaemonTest do }) ops = calls(log) - assert "network" in ops assert "chroot-jail" in ops + refute "network" in ops end end end diff --git a/test/hyper/node/fire_vmm/jailer_test.exs b/test/hyper/node/fire_vmm/jailer_test.exs index 3b1d9f14..1f36dfd8 100644 --- a/test/hyper/node/fire_vmm/jailer_test.exs +++ b/test/hyper/node/fire_vmm/jailer_test.exs @@ -85,22 +85,4 @@ defmodule Hyper.Node.FireVMM.JailerTest do assert Jailer.host_vsock(@vm_id) == "/srv/hyper/jails/firecracker/#{@vm_id}/root/vsock.sock" end - - describe "Checks.network_ready/0" do - test "refuses to start when no [network] table is configured" do - # Networking is mandatory: absence is a hard startup failure, not a - # silent opt-out. - Hyper.Cfg.Toml.put_cache(%{}) - on_exit(fn -> Hyper.Cfg.Toml.reload() end) - - assert Jailer.Checks.network_ready() == {:error, :network_not_configured} - end - - test "fails when uplink iface is absent" do - Hyper.Cfg.Toml.put_cache(%{"network" => %{"uplink" => "hyper-nonexistent-nic"}}) - on_exit(fn -> Hyper.Cfg.Toml.reload() end) - - assert {:error, {:missing_uplink, "hyper-nonexistent-nic"}} = Jailer.Checks.network_ready() - end - end end diff --git a/test/hyper/node/fire_vmm/net_test.exs b/test/hyper/node/fire_vmm/net_test.exs deleted file mode 100644 index e38f086a..00000000 --- a/test/hyper/node/fire_vmm/net_test.exs +++ /dev/null @@ -1,40 +0,0 @@ -defmodule Hyper.Node.FireVMM.NetTest do - use ExUnit.Case, async: true - alias Hyper.Node.FireVMM.Net - - test "guest_mac is a stable locally-administered unicast MAC" do - mac = Net.guest_mac("vabcdef") - assert mac == Net.guest_mac("vabcdef"), "must be deterministic in vm_id" - - octets = String.split(mac, ":") - # Firecracker rejects anything but a full six-octet MAC — a short address - # 400s the NIC config and the VM never boots. - assert length(octets) == 6, "MAC must have six octets, got #{mac}" - assert Enum.all?(octets, &(byte_size(&1) == 2 and match?({_, ""}, Integer.parse(&1, 16)))) - - <> = Base.decode16!(hd(octets), case: :mixed) - # locally administered (bit 1 set), unicast (bit 0 clear) - assert Bitwise.band(byte, 0x02) == 0x02 - assert Bitwise.band(byte, 0x01) == 0x00 - end - - test "distinct vm_ids get distinct MACs" do - refute Net.guest_mac("vaaa") == Net.guest_mac("vbbb") - end - - test "ip_cmdline pins the inner-world contract" do - assert Net.ip_cmdline() == "ip=172.30.0.2::172.30.0.1:255.255.255.252::eth0:off" - end - - test "resolver_cmdline formats the hyper.resolver= fragment for the given resolver" do - assert Net.resolver_cmdline("1.1.1.1") == "hyper.resolver=1.1.1.1" - assert Net.resolver_cmdline("10.0.0.53") == "hyper.resolver=10.0.0.53" - end - - test "interface targets tap0/eth0 with the derived MAC" do - nic = Net.interface("vabcdef") - assert nic.iface_id == "eth0" - assert nic.host_dev_name == "tap0" - assert nic.guest_mac == Net.guest_mac("vabcdef") - end -end diff --git a/test/hyper/node/reaper/liveness_test.exs b/test/hyper/node/reaper/liveness_test.exs index 13c250b9..e9068dc6 100644 --- a/test/hyper/node/reaper/liveness_test.exs +++ b/test/hyper/node/reaper/liveness_test.exs @@ -1,5 +1,5 @@ defmodule Hyper.Node.Reaper.LivenessTest do - # Guards the Img.Mutable.active_vm_ids/0 + Plan.orphans/4 contract: a live mutable + # Guards the Img.Mutable.active_vm_ids/0 + Plan.orphans/3 contract: a live mutable # layer protects its vm_id from reaping. The gather_live/0 union of this source into # the full live set is verified by manual live-node testing (Task 4), not CI. use ExUnit.Case, async: false @@ -38,14 +38,6 @@ defmodule Hyper.Node.Reaper.LivenessTest do # The reaper would see hyper-rw-vm-live in `dmsetup ls` (rw candidate) and no # cgroup leaf, yet the live mutable owner must protect it from reaping. - assert Plan.orphans(live, [], ["vm-live"], []) == MapSet.new([]) - end - - test "a vm with a live mutable layer is never an orphan, even with a leftover netns" do - register_live("vm-live-netns") - - live = MapSet.new(Mutable.active_vm_ids()) - - assert Plan.orphans(live, [], [], ["vm-live-netns"]) == MapSet.new([]) + assert Plan.orphans(live, [], ["vm-live"]) == MapSet.new([]) end end diff --git a/test/hyper/node/reaper/plan_properties_test.exs b/test/hyper/node/reaper/plan_properties_test.exs index 35514b2b..6720a608 100644 --- a/test/hyper/node/reaper/plan_properties_test.exs +++ b/test/hyper/node/reaper/plan_properties_test.exs @@ -29,25 +29,13 @@ defmodule Hyper.Node.Reaper.PlanPropertiesTest do check all( live <- id_set(), leaves <- id_list(), - rw <- id_list(), - netns <- id_list() + rw <- id_list() ) do - orphans = Plan.orphans(live, leaves, rw, netns) + orphans = Plan.orphans(live, leaves, rw) assert MapSet.disjoint?(orphans, live) end end - property "every orphan netns name not in live is a candidate, and none from live are" do - check all( - live <- id_set(), - netns <- id_list() - ) do - orphans = Plan.orphans(live, [], [], netns) - expected = MapSet.difference(MapSet.new(netns), live) - assert orphans == expected - end - end - property "only twice-seen orphans are reaped; current is carried forward" do check all( current <- id_set(), diff --git a/test/hyper/node/reaper/plan_test.exs b/test/hyper/node/reaper/plan_test.exs index b1e57ac9..0021db98 100644 --- a/test/hyper/node/reaper/plan_test.exs +++ b/test/hyper/node/reaper/plan_test.exs @@ -5,35 +5,27 @@ defmodule Hyper.Node.Reaper.PlanTest do defp set(ids), do: MapSet.new(ids) - describe "orphans/4" do + describe "orphans/3" do test "a cgroup-leaf-only orphan is a candidate" do - assert Plan.orphans(set([]), ["dead"], [], []) == set(["dead"]) + assert Plan.orphans(set([]), ["dead"], []) == set(["dead"]) end test "a dm-only orphan is a candidate" do - assert Plan.orphans(set([]), [], ["dead"], []) == set(["dead"]) + assert Plan.orphans(set([]), [], ["dead"]) == set(["dead"]) end test "an id seen in both sources is a single candidate" do - assert Plan.orphans(set([]), ["dead"], ["dead"], []) == set(["dead"]) + assert Plan.orphans(set([]), ["dead"], ["dead"]) == set(["dead"]) end test "an id present in live is never a candidate, even if it also has resources" do - assert Plan.orphans(set(["alive"]), ["alive"], ["alive"], ["alive"]) == set([]) + assert Plan.orphans(set(["alive"]), ["alive"], ["alive"]) == set([]) end test "only the non-live ids survive as candidates" do - assert Plan.orphans(set(["alive"]), ["alive", "dead"], ["alive", "gone"], []) == + assert Plan.orphans(set(["alive"]), ["alive", "dead"], ["alive", "gone"]) == set(["dead", "gone"]) end - - test "a netns-only orphan is a candidate" do - assert Plan.orphans(set([]), [], [], ["dead-netns"]) == set(["dead-netns"]) - end - - test "a netns name whose vm_id is live is never a candidate" do - assert Plan.orphans(set(["alive"]), [], [], ["alive"]) == set([]) - end end describe "confirm/2 two-strike grace" do diff --git a/test/hyper/suid_helper/network_test.exs b/test/hyper/suid_helper/network_test.exs deleted file mode 100644 index 0b406814..00000000 --- a/test/hyper/suid_helper/network_test.exs +++ /dev/null @@ -1,19 +0,0 @@ -defmodule Hyper.SuidHelper.NetworkTest do - use ExUnit.Case, async: true - alias Hyper.SuidHelper.Network - - test "prepare/2 builds the network prepare argv" do - assert Network.argv(:prepare, "vabc", 900_100) == - ["network", "prepare", "--vm-id", "vabc", "--uid", "900100"] - end - - test "teardown/2 builds the network teardown argv" do - assert Network.argv(:teardown, "vabc", 900_100) == - ["network", "teardown", "--vm-id", "vabc", "--uid", "900100"] - end - - test "teardown_orphan/1 builds the network teardown-orphan argv, with no uid" do - assert Network.argv(:teardown_orphan, "vabc") == - ["network", "teardown-orphan", "--vm-id", "vabc"] - end -end