diff --git a/.github/scripts/provision-kvm-host.sh b/.github/scripts/provision-kvm-host.sh index 0d514c38..86eb231d 100755 --- a/.github/scripts/provision-kvm-host.sh +++ b/.github/scripts/provision-kvm-host.sh @@ -17,6 +17,9 @@ # 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 @@ -30,12 +33,20 @@ sudo udevadm trigger --name-match=kvm sudo apt-get update sudo apt-get install -y \ - coreutils e2fsprogs libc-bin lvm2 skopeo thin-provisioning-tools util-linux \ + coreutils e2fsprogs iproute2 libc-bin lvm2 nftables 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. -sudo modprobe -av dm_snapshot dm_thin_pool loop +# +# 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 targets="$(sudo dmsetup targets)" echo "dmsetup targets: ${targets}" grep -q thin-pool <<<"${targets}" || { echo "ERROR: thin-pool dm target missing" >&2; exit 1; } @@ -47,8 +58,35 @@ 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 <<'EOF' +sudo tee /etc/hyper/config.toml >/dev/null < #### 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" +``` + ### Cgroups Hyper uses cgroups to impose limits on each VM. Each VM has its own cgroup, diff --git a/lib/hyper/cfg/network.ex b/lib/hyper/cfg/network.ex new file mode 100644 index 00000000..a7228973 --- /dev/null +++ b/lib/hyper/cfg/network.ex @@ -0,0 +1,53 @@ +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. + + 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. + """ + + 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) +end diff --git a/lib/hyper/node.ex b/lib/hyper/node.ex index 22a3f818..01c7d13f 100644 --- a/lib/hyper/node.ex +++ b/lib/hyper/node.ex @@ -310,11 +310,25 @@ 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) do + :ok <- check_helper_base(base), + :ok <- init_network() 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 700780f0..dc663854 100644 --- a/lib/hyper/node/fire_vmm/boot_spec.ex +++ b/lib/hyper/node/fire_vmm/boot_spec.ex @@ -32,11 +32,22 @@ defmodule Hyper.Node.FireVMM.BootSpec do @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: Map.get(source, :boot_args, @default_boot_args) + boot_args: boot_args }, drives: [ %Drive{ @@ -45,7 +56,8 @@ 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 882a6cca..56713f40 100644 --- a/lib/hyper/node/fire_vmm/daemon.ex +++ b/lib/hyper/node/fire_vmm/daemon.ex @@ -57,11 +57,13 @@ defmodule Hyper.Node.FireVMM.Daemon do # link kill) and so `terminate/2` runs on supervisor shutdown. Process.flag(:trap_exit, true) - with :ok <- reset_stale_jail(id), + with :ok <- prelaunch(opts), {:ok, muontrap} <- launch(opts) do {:ok, %__MODULE__{opts: opts, muontrap: muontrap}} else - {:error, reason} -> {:stop, reason} + {:error, reason} -> + Logger.error("vm #{id}: init failed: #{inspect(reason)}") + {:stop, reason} end end @@ -79,8 +81,8 @@ defmodule Hyper.Node.FireVMM.Daemon do # here is logged, and the `Reaper` will retry, but it must not crash teardown. @impl true @decorate with_span("Hyper.Node.FireVMM.Daemon.terminate", include: [:id]) - def terminate(_reason, %__MODULE__{opts: %Opts{vm_id: id}}) do - case clear_jail(id) do + def terminate(_reason, %__MODULE__{opts: %Opts{vm_id: id} = opts}) do + case clear_jail(opts) do :ok -> :ok @@ -89,12 +91,25 @@ defmodule Hyper.Node.FireVMM.Daemon do end end - @spec reset_stale_jail(Hyper.Vm.Id.t()) :: :ok | {:error, term()} - defp reset_stale_jail(id), do: clear_jail(id) + # 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 - @spec clear_jail(Hyper.Vm.Id.t()) :: :ok | {:error, term()} - defp clear_jail(id) do - SuidHelper.ChrootJail.remove(Jailer.chroot_dir(id), Jailer.cgroup_dir(id)) + # 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. + @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 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 cc86040b..eb15b986 100644 --- a/lib/hyper/node/fire_vmm/jailer.ex +++ b/lib/hyper/node/fire_vmm/jailer.ex @@ -38,7 +38,9 @@ defmodule Hyper.Node.FireVMM.Jailer do first failure. """ - alias Hyper.Cfg.{Dirs, Jails} + alias Hyper.Cfg.{Dirs, Jails, Network} + + @ip_forward_path "/proc/sys/net/ipv4/ip_forward" @doc "Run every pre-requisite check, halting at the first failure." @spec run() :: :ok | {:error, term()} @@ -56,10 +58,46 @@ defmodule Hyper.Node.FireVMM.Jailer do &kvm_present/0, &cgroup_v2_available/0, &parent_cgroup_present/0, - &chroot_writable/0 + &chroot_writable/0, + &network_ready/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 new file mode 100644 index 00000000..a323f0ab --- /dev/null +++ b/lib/hyper/node/fire_vmm/net.ex @@ -0,0 +1,53 @@ +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" + + @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. + """ + @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) + + [0x02, b1, b2, b3, b4, b5] + |> Enum.map_join(":", &(&1 |> Integer.to_string(16) |> String.pad_leading(2, "0"))) + |> String.downcase() + end + + @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 +end diff --git a/lib/hyper/node/fire_vmm/state.ex b/lib/hyper/node/fire_vmm/state.ex index f7ccaabe..4e1b7bbe 100644 --- a/lib/hyper/node/fire_vmm/state.ex +++ b/lib/hyper/node/fire_vmm/state.ex @@ -92,7 +92,8 @@ defmodule Hyper.Node.FireVMM.State do ) do case Hyper.Cluster.Routing.register_self({id, :state}) do :ok -> - spec = BootSpec.resolve(boot_source(kernel, Mutable.blk_path(mutable), boot_args), type) + source = boot_source(id, 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} @@ -104,13 +105,15 @@ 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. - @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(kernel, dev, boot_args), - do: %{kernel_image_path: kernel, root_drive_path: dev, boot_args: boot_args} + # 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} + + 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} @impl :gen_statem # Answered in every lifecycle state: the opts are static data, and a fork must diff --git a/lib/hyper/node/img/server.ex b/lib/hyper/node/img/server.ex index 8fd7f0b8..bcdd5f83 100644 --- a/lib/hyper/node/img/server.ex +++ b/lib/hyper/node/img/server.ex @@ -186,6 +186,12 @@ defmodule Hyper.Node.Img.Server do # on top. Rolls back any devices it created if a later one fails. @spec build_chain(Hyper.Img.id(), [Path.t()]) :: {:ok, %{blk_path: Path.t(), dm_names: [String.t()]}} | {:error, term()} + # An unknown image id resolves to no layers; return a clean domain error so + # `init/1` stops with `{:stop, :unknown_image}` (which the scheduler maps to + # `:no_capacity`) rather than crashing with a FunctionClauseError + SASL + # report. This is the placement-refusal path for a never-loaded image. + defp build_chain(_img_id, []), do: {:error, :unknown_image} + defp build_chain(img_id, [base | deltas]) do with {:ok, sectors} <- SuidHelper.Blockdev.device_sectors(base) do deltas diff --git a/lib/hyper/node/reaper.ex b/lib/hyper/node/reaper.ex index c6effa84..5541db41 100644 --- a/lib/hyper/node/reaper.ex +++ b/lib/hyper/node/reaper.ex @@ -9,12 +9,16 @@ 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/3` removes their union from the candidate set) and only ever - touches `hyper-rw-*` dm names and per-VM cgroup leaves - never `hyper-thinpool`, + — (`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. + 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 + normally-booting VM never produces a second one. The decision logic lives in the pure `Hyper.Node.Reaper.Plan`; this module is a thin I/O adapter that gathers the inputs, calls the plan, and executes the @@ -84,8 +88,9 @@ 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) + current = Plan.orphans(live, leaves, rw, netns) {to_reap, next} = Plan.confirm(current, state.last_orphans) Enum.each(to_reap, &reap_one/1) @@ -161,6 +166,23 @@ 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 @@ -173,6 +195,8 @@ 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 17b6dde9..24ccf955 100644 --- a/lib/hyper/node/reaper/plan.ex +++ b/lib/hyper/node/reaper/plan.ex @@ -16,12 +16,20 @@ defmodule Hyper.Node.Reaper.Plan do do: String.replace_prefix(name, @rw_prefix, "") end - @doc "Candidate orphans this tick: (cgroup leaves ∪ rw vm_ids) minus the live set." - @spec orphans(MapSet.t(String.t()), [String.t()], [String.t()]) :: MapSet.t(String.t()) - def orphans(live, cgroup_leaves, rw_ids) do + @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. + """ + @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 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 new file mode 100644 index 00000000..e4eea2f7 --- /dev/null +++ b/lib/hyper/suid_helper/network.ex @@ -0,0 +1,54 @@ +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. + """ + use OpenTelemetryDecorator + alias Hyper.SuidHelper + + @type err :: 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)] + + @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 +end diff --git a/lib/hyper/vm.ex b/lib/hyper/vm.ex index a6064a3b..219a9c3a 100644 --- a/lib/hyper/vm.ex +++ b/lib/hyper/vm.ex @@ -8,11 +8,13 @@ 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. + cmdline when omitted. `vm_id` is required — networking is mandatory and + `BootSpec.resolve/2` derives the guest NIC (`ip=` cmdline + MAC) from it. """ @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/native/guest-agent/Cargo.toml b/native/guest-agent/Cargo.toml index 353b6907..5392bb3b 100644 --- a/native/guest-agent/Cargo.toml +++ b/native/guest-agent/Cargo.toml @@ -31,6 +31,10 @@ 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 ae3b524b..7de75a1f 100644 --- a/native/guest-agent/src/init.rs +++ b/native/guest-agent/src/init.rs @@ -1,7 +1,10 @@ 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; @@ -16,5 +19,30 @@ 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 new file mode 100644 index 00000000..98014376 --- /dev/null +++ b/native/guest-agent/tests/init.rs @@ -0,0 +1,65 @@ +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 46992a85..5d272603 100644 --- a/native/suidhelper/Cargo.toml +++ b/native/suidhelper/Cargo.toml @@ -56,6 +56,18 @@ 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" @@ -108,6 +120,10 @@ path = "tests/e2e/thin_dump.rs" name = "config_uid_gid_range" path = "tests/config_uid_gid_range.rs" +[[test]] +name = "config_network" +path = "tests/config_network.rs" + [[test]] name = "util_linux_thin_dump" path = "tests/util/linux/thin_dump.rs" diff --git a/native/suidhelper/src/config.rs b/native/suidhelper/src/config.rs index 00d04e25..09e43c5c 100644 --- a/native/suidhelper/src/config.rs +++ b/native/suidhelper/src/config.rs @@ -101,6 +101,8 @@ 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. @@ -140,6 +142,9 @@ pub struct Tools { dmsetup: PathBuf, losetup: PathBuf, blockdev: PathBuf, + ip: PathBuf, + nft: PathBuf, + sysctl: PathBuf, thin_dump: PathBuf, firecracker: Option, jailer: Option, @@ -148,10 +153,16 @@ pub struct Tools { impl Default for Tools { fn default() -> Self { Self { - dmsetup: default_dmsetup(), - losetup: default_losetup(), - blockdev: default_blockdev(), - thin_dump: default_thin_dump(), + 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, } @@ -165,22 +176,6 @@ fn default_work_dir() -> PathBuf { PathBuf::from("/srv/hyper") } -fn default_dmsetup() -> PathBuf { - PathBuf::from("/usr/sbin/dmsetup") -} - -fn default_losetup() -> PathBuf { - PathBuf::from("/usr/sbin/losetup") -} - -fn default_blockdev() -> PathBuf { - PathBuf::from("/usr/sbin/blockdev") -} - -fn default_thin_dump() -> PathBuf { - PathBuf::from("/usr/sbin/thin_dump") -} - fn default_parent_cgroup() -> String { // Must match Elixir node's `@parent_cgroup`; operators need to keep them in sync. "hyper".into() @@ -192,10 +187,34 @@ 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 @@ -239,6 +258,43 @@ 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 6c0ff8f2..78c12b78 100644 --- a/native/suidhelper/src/tools/jailer.rs +++ b/native/suidhelper/src/tools/jailer.rs @@ -64,6 +64,14 @@ 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(&jailer) .firecracker(&firecracker) @@ -74,6 +82,7 @@ pub fn run(args: JailerArgs) -> Result { .gid(gid) .cgroups(&args.cgroup) .api_sock(&args.api_sock) + .maybe_netns(netns.as_deref()) .build() .invoke()?) } diff --git a/native/suidhelper/src/tools/mod.rs b/native/suidhelper/src/tools/mod.rs index dd421565..80acae07 100644 --- a/native/suidhelper/src/tools/mod.rs +++ b/native/suidhelper/src/tools/mod.rs @@ -10,6 +10,7 @@ pub mod chroot_jail; mod dmsetup; pub mod jailer; pub mod losetup; +pub mod network; pub mod thin_dump; pub use blockcopy::{Blockcopy, BlockcopyArgs}; @@ -17,6 +18,7 @@ pub use blockdev::{Blockdev, BlockdevArgs}; pub use chroot_jail::ChrootJailOp; pub use dmsetup::{DmTable, Dmsetup, DmsetupArgs, ThinMessage}; pub use losetup::{Losetup, LosetupArgs}; +pub use network::NetworkOp; pub use thin_dump::{ThinDump, ThinDumpArgs}; use crate::config::Config; @@ -101,6 +103,11 @@ pub enum Tool { #[command(subcommand)] op: ChrootJailOp, }, + /// Per-VM egress networking (netns + veth + TAP + NAT). + Network { + #[command(subcommand)] + op: NetworkOp, + }, /// Dump a thin device's provisioned ranges from the pool metadata. ThinDump { #[command(flatten)] @@ -130,6 +137,7 @@ impl Tool { } Tool::Blockcopy { args } => Blockcopy::new(args).run(), Tool::ChrootJail { op } => op.run(), + Tool::Network { op } => op.run(), Tool::ThinDump { args } => { let bin = config.thin_dump().map_err(|e| Error::Tool(Box::new(e)))?; ThinDump::new(bin.into(), args).run() diff --git a/native/suidhelper/src/tools/network/addr.rs b/native/suidhelper/src/tools/network/addr.rs new file mode 100644 index 00000000..2b2ef9b5 --- /dev/null +++ b/native/suidhelper/src/tools/network/addr.rs @@ -0,0 +1,77 @@ +//! 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 new file mode 100644 index 00000000..4f25a1ba --- /dev/null +++ b/native/suidhelper/src/tools/network/args.rs @@ -0,0 +1,405 @@ +//! 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 new file mode 100644 index 00000000..2eb199aa --- /dev/null +++ b/native/suidhelper/src/tools/network/exec.rs @@ -0,0 +1,79 @@ +//! 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 new file mode 100644 index 00000000..85cc79dc --- /dev/null +++ b/native/suidhelper/src/tools/network/host_init.rs @@ -0,0 +1,106 @@ +//! `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 new file mode 100644 index 00000000..ca2a9007 --- /dev/null +++ b/native/suidhelper/src/tools/network/mod.rs @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: AGPL-3.0-only +//! `network`: per-VM egress networking (netns + veth + TAP + NAT). +//! +//! 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; + +use crate::config::Network; +use clap::Subcommand; +use serde::Serialize; +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. +#[derive(Debug, ThisError, PartialEq, Eq)] +#[error("VM networking is not configured ([network] absent from config)")] +pub struct NetworkingDisabled; + +/// 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) +} + +/// 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(Subcommand)] +pub enum NetworkOp { + /// Create a VM's netns, veth pair, TAP device, and in-netns NAT. + Prepare(PrepareArgs), + /// Remove a VM's netns and any lingering host-side veth end. + Teardown(TeardownArgs), + /// Idempotently remove an orphan VM's netns by vm_id alone (no uid). + TeardownOrphan(TeardownOrphanArgs), + /// One-time host setup: the `hyper` nftables table and forward policy. + HostInit(HostInitArgs), +} + +impl NetworkOp { + /// Route to the selected op; each runs in its own privileged scope and + /// returns its own serialized `Value`. + 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), + } + } +} diff --git a/native/suidhelper/src/tools/network/prepare.rs b/native/suidhelper/src/tools/network/prepare.rs new file mode 100644 index 00000000..3011cea6 --- /dev/null +++ b/native/suidhelper/src/tools/network/prepare.rs @@ -0,0 +1,144 @@ +//! `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 new file mode 100644 index 00000000..146b8401 --- /dev/null +++ b/native/suidhelper/src/tools/network/teardown.rs @@ -0,0 +1,70 @@ +//! `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 new file mode 100644 index 00000000..72f64da6 --- /dev/null +++ b/native/suidhelper/src/tools/network/teardown_orphan.rs @@ -0,0 +1,124 @@ +//! `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 6a618187..22c6523a 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::Mode; +use nix::sys::stat::{makedev, Mode}; use std::io; use std::os::unix::io::FromRawFd; use std::path::{Path, PathBuf}; @@ -19,6 +19,10 @@ 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)] @@ -122,6 +126,9 @@ impl ChrootJail { 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(()) } } @@ -213,3 +220,22 @@ 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 ae6e092e..3743f451 100644 --- a/native/suidhelper/src/util/jailer.rs +++ b/native/suidhelper/src/util/jailer.rs @@ -214,6 +214,11 @@ 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<'_> { @@ -244,6 +249,11 @@ 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 61808d8a..ccf886d9 100644 --- a/native/suidhelper/src/util/safe_dir.rs +++ b/native/suidhelper/src/util/safe_dir.rs @@ -15,9 +15,12 @@ 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, mknodat, FchmodatFlags, FileStat, Mode, SFlag}; +use nix::sys::stat::{ + fchmod, fchmodat, fstatat, mkdirat, 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; @@ -35,6 +38,8 @@ 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}")] @@ -57,6 +62,7 @@ impl Error { Error::Open { source, .. } | Error::Unlink { source, .. } | Error::Write { source, .. } + | Error::Mkdir { source, .. } | Error::Mknod { source, .. } | Error::Chown { source, .. } | Error::Chmod { source, .. } @@ -192,6 +198,55 @@ 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 new file mode 100644 index 00000000..9e2cf7e5 --- /dev/null +++ b/native/suidhelper/tests/config_network.rs @@ -0,0 +1,30 @@ +use hyper_suidhelper::config::{Config, Network}; + +#[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()); +} diff --git a/native/suidhelper/tests/e2e/chroot_jail.rs b/native/suidhelper/tests/e2e/chroot_jail.rs index 37524a79..fcea9aed 100644 --- a/native/suidhelper/tests/e2e/chroot_jail.rs +++ b/native/suidhelper/tests/e2e/chroot_jail.rs @@ -3,10 +3,15 @@ //! 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::{MetadataExt, PermissionsExt}; +use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt}; use std::path::{Path, PathBuf}; use std::process::Command; @@ -22,6 +27,22 @@ 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()?; @@ -102,6 +123,65 @@ 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 3b44341c..8ed0dc4c 100644 --- a/native/suidhelper/tests/e2e/jailer.rs +++ b/native/suidhelper/tests/e2e/jailer.rs @@ -64,6 +64,20 @@ 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 { + let p = dir.join("config.toml"); + let body = format!( + "work_dir = \"/srv/hyper\"\n[tools]\njailer = \"{}\"\nfirecracker = \"{}\"\n[network]\nuplink = \"eth0\"\n", + jailer.display(), + firecracker.display(), + ); + fs::write(&p, body).unwrap(); + fs::set_permissions(&p, fs::Permissions::from_mode(0o644)).unwrap(); + p +} + #[test] fn execs_jailer_with_canonical_argv_and_empty_env_as_root() { if !is_root() { @@ -159,6 +173,59 @@ fn execs_jailer_with_canonical_argv_and_empty_env_as_root() { ); } +#[test] +fn execs_jailer_with_netns_flag_when_networking_enabled_as_root() { + if !is_root() { + eprintln!("SKIP jailer netns: needs root to become_root_permanently + own the fakes"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let argv_rec = tmp.path().join("argv.json"); + 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 out = run( + &cfg, + &[ + "jailer", + "--id", + "vnet1", + "--uid", + "900001", + "--gid", + "900002", + "--api-sock", + "/api.sock", + ], + ); + + assert_eq!( + out.status.code(), + Some(RECORDER_EXIT), + "exit status did not propagate; stderr: {}", + String::from_utf8_lossy(&out.stderr), + ); + + 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" + ); +} + #[test] fn refuses_uid_zero_without_exec_as_root() { if !is_root() { diff --git a/native/suidhelper/tests/tools/host_init.rs b/native/suidhelper/tests/tools/host_init.rs new file mode 100644 index 00000000..9e195adf --- /dev/null +++ b/native/suidhelper/tests/tools/host_init.rs @@ -0,0 +1,44 @@ +//! `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 new file mode 100644 index 00000000..96ec7d93 --- /dev/null +++ b/native/suidhelper/tests/tools/network_addr.rs @@ -0,0 +1,79 @@ +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 new file mode 100644 index 00000000..98e6226f --- /dev/null +++ b/native/suidhelper/tests/tools/network_args.rs @@ -0,0 +1,435 @@ +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/test/e2e/network_test.exs b/test/e2e/network_test.exs new file mode 100644 index 00000000..4a7e4eae --- /dev/null +++ b/test/e2e/network_test.exs @@ -0,0 +1,61 @@ +defmodule Hyper.E2e.NetworkTest do + @moduledoc """ + Live end-to-end contract of VM egress networking on a provisioned host: a + booted guest's `eth0` carries the uniform inner-world address + (`172.30.0.2`), and the netns+veth+tap+NAT path it rides on actually + reaches the internet — DNS resolves and an HTTP fetch completes. + + VM networking is mandatory (`Hyper.Node.FireVMM.Jailer.Checks.network_ready/0` + refuses to start a node without `[network]`), so there is nothing to skip: a + host that reached the point of running this suite already booted the node, + which means networking is provisioned. CI's `integration` job provisions it + (see `.github/scripts/provision-kvm-host.sh`). + + Runs only under `--only integration` / `--include integration` on a host + provisioned per docs/cookbook/install.md. + """ + use ExUnit.Case, async: false + + import Hyper.E2e + + @moduletag :integration + @moduletag timeout: :timer.minutes(10) + + # public.ecr.aws mirrors library images without Docker Hub's per-IP pull + # limits, which shared GHA egress IPs routinely exhaust. + @image System.get_env("HYPER_E2E_IMAGE", "public.ecr.aws/docker/library/alpine:3.19") + + test "a booted guest can reach the internet over its NIC" do + assert {:ok, img_id} = Hyper.Img.OciLoader.load(@image) + + # :micro, not the :base default — :base asks for 32 GiB of disk budget, + # which the default node budget (4 GiB) refuses with :no_capacity on a + # small CI runner. + assert {:ok, vm} = Hyper.create_vm(%Hyper.Vm.Spec{img_id: img_id, type: :micro}) + on_exit(fn -> 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 new file mode 100644 index 00000000..50fddcb9 --- /dev/null +++ b/test/hyper/cfg/network_test.exs @@ -0,0 +1,34 @@ +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 + 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 b79185f8..b7aa8ea4 100644 --- a/test/hyper/node/fire_vmm/boot_spec_test.exs +++ b/test/hyper/node/fire_vmm/boot_spec_test.exs @@ -3,12 +3,25 @@ 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"} + test "default cmdline boots our agent as init with no serial console" do - source = %{kernel_image_path: "/vmlinux", root_drive_path: "/rootfs"} - cold = BootSpec.resolve(source, :micro) + cold = BootSpec.resolve(@source, :micro) assert cold.boot_source.boot_args =~ "init=/hyper-init" # No console by default: serial printk blocks the boot vCPU. A debug boot # re-adds console=ttyS0 through the per-VM boot_args override. refute cold.boot_source.boot_args =~ "console=" end + + test "every VM gets a NIC and the ip=/hyper.resolver= 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=" + end end diff --git a/test/hyper/node/fire_vmm/daemon_test.exs b/test/hyper/node/fire_vmm/daemon_test.exs new file mode 100644 index 00000000..e564d7c9 --- /dev/null +++ b/test/hyper/node/fire_vmm/daemon_test.exs @@ -0,0 +1,119 @@ +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). + """ + + use ExUnit.Case, async: false + + alias Hyper.Node.FireVMM + alias Hyper.Node.FireVMM.Daemon + + @vm_id "vdaemontest01" + + setup do + log = Path.join(System.tmp_dir!(), "daemon_test_#{System.unique_integer([:positive])}.log") + fake = write_fake_suidhelper!(log) + + on_exit(fn -> + Hyper.Cfg.Toml.reload() + File.rm(fake) + File.rm(log) + end) + + {:ok, log: log, fake: fake} + end + + defp opts do + %FireVMM.Opts{ + vm_id: @vm_id, + uid: 900_500, + gid: 900_500, + type: :micro, + arch: :x86_64, + mutable: nil, + kernel: "/nonexistent/vmlinux", + boot_args: nil + } + end + + # A stand-in for hyper-suidhelper: appends "" as one line per + # invocation (so call order is observable across the test) and prints an + # empty JSON object so `SuidHelper.exec/1`'s `Jason.decode!/1` succeeds. + # Exits immediately, so a `jailer` invocation looks like an instant crash + # rather than a live firecracker — fine here, only the call log matters. + defp write_fake_suidhelper!(log) do + path = + Path.join(System.tmp_dir!(), "fake_suidhelper_#{System.unique_integer([:positive])}.sh") + + File.write!(path, """ + #!/usr/bin/env bash + echo "$*" >> #{log} + echo '{}' + """) + + File.chmod!(path, 0o755) + path + end + + defp calls(log) do + log + |> File.read!() + |> String.split("\n", trim: true) + |> Enum.map(&hd(String.split(&1, " "))) + end + + # `Jailer.chroot_dir/1` derives the in-jail exec name from `tools.firecracker` + # (see `JailerTest`), so every scenario stubs it alongside `tools.suidhelper` + # to keep it off the real filesystem. + defp put_toml(fake) do + tools = %{"suidhelper" => fake, "firecracker" => "/usr/local/bin/firecracker"} + 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. + put_toml(fake) + + Process.flag(:trap_exit, true) + {:ok, pid} = Daemon.start_link(opts()) + 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" + end + + test "terminate/2 tears down both the network and the chroot jail", %{log: log, fake: fake} do + put_toml(fake) + + assert :ok = + Daemon.terminate(:shutdown, %Daemon{ + opts: opts(), + muontrap: nil + }) + + ops = calls(log) + assert "network" in ops + assert "chroot-jail" 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 1f36dfd8..3b1d9f14 100644 --- a/test/hyper/node/fire_vmm/jailer_test.exs +++ b/test/hyper/node/fire_vmm/jailer_test.exs @@ -85,4 +85,22 @@ 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 new file mode 100644 index 00000000..e38f086a --- /dev/null +++ b/test/hyper/node/fire_vmm/net_test.exs @@ -0,0 +1,40 @@ +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 e9068dc6..13c250b9 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/3 contract: a live mutable + # Guards the Img.Mutable.active_vm_ids/0 + Plan.orphans/4 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,6 +38,14 @@ 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([]) + 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([]) end end diff --git a/test/hyper/node/reaper/plan_properties_test.exs b/test/hyper/node/reaper/plan_properties_test.exs index 6720a608..35514b2b 100644 --- a/test/hyper/node/reaper/plan_properties_test.exs +++ b/test/hyper/node/reaper/plan_properties_test.exs @@ -29,13 +29,25 @@ defmodule Hyper.Node.Reaper.PlanPropertiesTest do check all( live <- id_set(), leaves <- id_list(), - rw <- id_list() + rw <- id_list(), + netns <- id_list() ) do - orphans = Plan.orphans(live, leaves, rw) + orphans = Plan.orphans(live, leaves, rw, netns) 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 0021db98..b1e57ac9 100644 --- a/test/hyper/node/reaper/plan_test.exs +++ b/test/hyper/node/reaper/plan_test.exs @@ -5,27 +5,35 @@ defmodule Hyper.Node.Reaper.PlanTest do defp set(ids), do: MapSet.new(ids) - describe "orphans/3" do + describe "orphans/4" 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"]) == set([]) + assert Plan.orphans(set(["alive"]), ["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 new file mode 100644 index 00000000..0b406814 --- /dev/null +++ b/test/hyper/suid_helper/network_test.exs @@ -0,0 +1,19 @@ +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