Merge pull request 'fix(runner): root-only paths verified on a KVM host' (#3) from fix/igor-root-paths into main
13 files · +806 -85
M README.md +8 -1
| @@ -18,6 +18,7 @@ Both modes run the same executable, the runner's own, attached **read-only** to | |||
| 18 | 18 | - Firecracker is driven through its HTTP API on a unix socket with `net/http`, not through `firecracker-go-sdk` (its last release, v1.0.0 from 2022, predates Firecracker 1.17 and pulls CNI, go-openapi and netlink for a handful of PUT requests). | |
| 19 | 19 | - Disks: the runner executable (read-only boot root, ext4 built with `mkfs.ext4 -d`, cached by the executable's hash), the shared read-only base image, and a per-job sparse scratch disk (`ERMITE_RUNNER_JOB_DISK`). The guest init builds an overlayfs root (lower = base, upper = scratch) and keeps dockerd's data root on the scratch disk. The scratch disk and `eth0` are rate limited (defaults 400 MiB/s and 20000 IOPS, 1 Gbit/s each way; `ERMITE_RUNNER_JOB_DISK_BANDWIDTH` in MiB/s, `_JOB_DISK_IOPS`, `_JOB_NET_BANDWIDTH` in Mbit/s; 0 disables). | |
| 20 | 20 | - The jailer runs each VM as uid/gid `ERMITE_RUNNER_UID_BASE` + slot in its own chroot under the state directory, in a new PID namespace and in a cgroup v2 with `memory.max` (guest memory + 256 MiB for the VMM) and `pids.max`. The kernel, the base image and the state directory must be on the same filesystem (files are hard-linked into the jail) and the kernel and base image must be world-readable. | |
| 21 | + | - **The state directory and every parent of it must be owned by root and not writable by group or others.** The jailer runs as root and builds the chroots under `<state dir>/jail`; the driver checks that `jail` itself is a real directory, root-owned and not group/other-writable, but it does not walk the parents: a user who can rename or replace a parent directory can still redirect root's file operations. `/var/lib/ermite-runner` (the default, created by the unit's `StateDirectory`) is fine; do not point `ERMITE_RUNNER_STATE_DIR` into a home directory or `/tmp`. | |
| 21 | 22 | - One TAP (`ertap<slot>`) per VM with a /30 from `ERMITE_RUNNER_NETWORK` (default `10.200.0.0/16`), vsock for the executor stream, IPv6 disabled in the guest. | |
| 22 | 23 | - Needs root, or `CAP_NET_ADMIN` and the rights the jailer requires (the plan runs the host process as root under systemd). | |
| 23 | 24 | ||
| @@ -301,7 +302,13 @@ ERMITE_RUNNER_IT_E2E=1 ERMITE_RUNNER_TEST_IMAGE=ermite-runner-ubuntu-24.04:dev G | |||
| 301 | 302 | - `TestChainCorpus` runs the whole chain: a fake `RunnerService` (`internal/host/hosttest`, which checks the offsets, limits and tokens of `runner.proto`) hands out 22 jobs of the ermite-conformance corpus, the real host loop starts them in containers (two at a time), and what the fake stored (steps, conclusions, outputs, log lines) is compared with the recordings made on github.com. It reads `ERMITE_CONFORMANCE_DIR` (default: a sibling checkout of ermite-conformance) and never writes to it. | |
| 302 | 303 | - `TestVMRunsAJob` (`internal/sandbox/integration`, run by `scripts/it-vm-userns.sh`) does the first of these in a micro-VM. | |
| 303 | 304 | ||
| 304 | - | Not covered anywhere yet, because they need root on a real host: the jailer (uid drop, chroot, cgroups), a rootful Docker daemon or sysbox with the nested `dockerd`, and the firewall of container mode. | |
| 305 | + | Checked as root on a real KVM host (Arch Linux, kernel 6.18 LTS, Firecracker and jailer 1.17, rootful podman as the Docker API): the jailer (uid drop, chroot, `memory.max` and `pids.max` cgroups, PID namespace), the VM and container integration scenarios, the firewall policy and NAT, the nested `dockerd` on the loop-mounted scratch disk (full disk gives `ENOSPC`), and `internal/e2e` including `TestChainCorpus`. Not checked: sysbox, and a real Docker Engine (podman's compatibility API is not a drop-in, see below). | |
| 306 | + | ||
| 307 | + | **Jailer notes** (found on that host): Firecracker 1.17's jailer needs `--chroot-base-dir` to exist (the driver creates `<state dir>/jail`), starts Firecracker as `/firecracker --id <id> --start-time-us ...` (the `/proc` scan that finds a VMM by id knows that form), and leaves `<state dir>/jail/<exec name>` and the cgroup parent `/sys/fs/cgroup/firecracker` behind (only the per-VM directories are the runner's). The jailed API socket path (`<state dir>/jail/firecracker/<id>/root/run/firecracker.socket`) must fit in 107 bytes; the driver checks it at start. | |
| 308 | + | ||
| 309 | + | **Host firewalls.** The runner's table has its own `input` hook at priority `filter`; another table with a later hook (firewalld at `filter + 10`) still sees a guest's packet to the gateway after the runner accepted it. A firewalld zone that rejects `udp/tcp 53` (the `public` zone with a DNS reject rule, for instance) therefore breaks the guests' DNS (the symptom is `dns silent` in `TestVMNetworkPolicy`) while NAT keeps working. On such a host, allow DNS from `ertap*` and `erbr*` in that firewall, or run the policy test in a private network namespace (as `scripts/it-vm-userns.sh` does). | |
| 310 | + | ||
| 311 | + | **Podman as the Docker API.** `DOCKER_HOST` pointing at rootful podman's compatibility socket gets as far as creating the network, then fails: the compatibility `POST /networks/create` cannot disable netavark's DNS (`dns_enabled` is always true) and `HostConfig.Dns` equal to the network gateway is refused when the container starts (`netavark: error while applying dns entries: DNS server 10.200.0.1 is the gateway of network ...; using it would cause aardvark-dns to forward queries to itself`); the option `com.docker.network.bridge.enable_ip_masquerade` is ignored with a warning, and netavark (`firewall_driver = "nftables"` by default) adds its own rules unless it is configured with `firewall_driver = "none"`. The integration tests passed with a small test-only proxy that rewrites that one request to the libpod endpoint with `dns_enabled=false`, and a private `CONTAINERS_CONF`. Production container mode supports Docker Engine (or sysbox) only. | |
| 305 | 312 | ||
| 306 | 313 | `ERMITE_RUNNER_IT_DOCKER_API=1 ERMITE_RUNNER_TEST_IMAGE=<image> go test -run DockerAPIWiring ./internal/sandbox/container` exercises the real Docker Engine calls (labels, networks, volumes, reaping) against any daemon, including a rootless one; it does not test isolation. | |
| 307 | 314 | ||
| @@ -18,6 +18,7 @@ Both modes run the same executable, the runner's own, attached **read-only** to | |||||
| 18 | - Firecracker is driven through its HTTP API on a unix socket with `net/http`, not through `firecracker-go-sdk` (its last release, v1.0.0 from 2022, predates Firecracker 1.17 and pulls CNI, go-openapi and netlink for a handful of PUT requests). | 18 | - Firecracker is driven through its HTTP API on a unix socket with `net/http`, not through `firecracker-go-sdk` (its last release, v1.0.0 from 2022, predates Firecracker 1.17 and pulls CNI, go-openapi and netlink for a handful of PUT requests). | ||
| 19 | - Disks: the runner executable (read-only boot root, ext4 built with `mkfs.ext4 -d`, cached by the executable's hash), the shared read-only base image, and a per-job sparse scratch disk (`ERMITE_RUNNER_JOB_DISK`). The guest init builds an overlayfs root (lower = base, upper = scratch) and keeps dockerd's data root on the scratch disk. The scratch disk and `eth0` are rate limited (defaults 400 MiB/s and 20000 IOPS, 1 Gbit/s each way; `ERMITE_RUNNER_JOB_DISK_BANDWIDTH` in MiB/s, `_JOB_DISK_IOPS`, `_JOB_NET_BANDWIDTH` in Mbit/s; 0 disables). | 19 | - Disks: the runner executable (read-only boot root, ext4 built with `mkfs.ext4 -d`, cached by the executable's hash), the shared read-only base image, and a per-job sparse scratch disk (`ERMITE_RUNNER_JOB_DISK`). The guest init builds an overlayfs root (lower = base, upper = scratch) and keeps dockerd's data root on the scratch disk. The scratch disk and `eth0` are rate limited (defaults 400 MiB/s and 20000 IOPS, 1 Gbit/s each way; `ERMITE_RUNNER_JOB_DISK_BANDWIDTH` in MiB/s, `_JOB_DISK_IOPS`, `_JOB_NET_BANDWIDTH` in Mbit/s; 0 disables). | ||
| 20 | - The jailer runs each VM as uid/gid `ERMITE_RUNNER_UID_BASE` + slot in its own chroot under the state directory, in a new PID namespace and in a cgroup v2 with `memory.max` (guest memory + 256 MiB for the VMM) and `pids.max`. The kernel, the base image and the state directory must be on the same filesystem (files are hard-linked into the jail) and the kernel and base image must be world-readable. | 20 | - The jailer runs each VM as uid/gid `ERMITE_RUNNER_UID_BASE` + slot in its own chroot under the state directory, in a new PID namespace and in a cgroup v2 with `memory.max` (guest memory + 256 MiB for the VMM) and `pids.max`. The kernel, the base image and the state directory must be on the same filesystem (files are hard-linked into the jail) and the kernel and base image must be world-readable. | ||
| 21 | + | - **The state directory and every parent of it must be owned by root and not writable by group or others.** The jailer runs as root and builds the chroots under `<state dir>/jail`; the driver checks that `jail` itself is a real directory, root-owned and not group/other-writable, but it does not walk the parents: a user who can rename or replace a parent directory can still redirect root's file operations. `/var/lib/ermite-runner` (the default, created by the unit's `StateDirectory`) is fine; do not point `ERMITE_RUNNER_STATE_DIR` into a home directory or `/tmp`. | |||
| 21 | - One TAP (`ertap<slot>`) per VM with a /30 from `ERMITE_RUNNER_NETWORK` (default `10.200.0.0/16`), vsock for the executor stream, IPv6 disabled in the guest. | 22 | - One TAP (`ertap<slot>`) per VM with a /30 from `ERMITE_RUNNER_NETWORK` (default `10.200.0.0/16`), vsock for the executor stream, IPv6 disabled in the guest. | ||
| 22 | - Needs root, or `CAP_NET_ADMIN` and the rights the jailer requires (the plan runs the host process as root under systemd). | 23 | - Needs root, or `CAP_NET_ADMIN` and the rights the jailer requires (the plan runs the host process as root under systemd). | ||
| 23 | 24 | ||||
| @@ -301,7 +302,13 @@ ERMITE_RUNNER_IT_E2E=1 ERMITE_RUNNER_TEST_IMAGE=ermite-runner-ubuntu-24.04:dev G | |||||
| 301 | - `TestChainCorpus` runs the whole chain: a fake `RunnerService` (`internal/host/hosttest`, which checks the offsets, limits and tokens of `runner.proto`) hands out 22 jobs of the ermite-conformance corpus, the real host loop starts them in containers (two at a time), and what the fake stored (steps, conclusions, outputs, log lines) is compared with the recordings made on github.com. It reads `ERMITE_CONFORMANCE_DIR` (default: a sibling checkout of ermite-conformance) and never writes to it. | 302 | - `TestChainCorpus` runs the whole chain: a fake `RunnerService` (`internal/host/hosttest`, which checks the offsets, limits and tokens of `runner.proto`) hands out 22 jobs of the ermite-conformance corpus, the real host loop starts them in containers (two at a time), and what the fake stored (steps, conclusions, outputs, log lines) is compared with the recordings made on github.com. It reads `ERMITE_CONFORMANCE_DIR` (default: a sibling checkout of ermite-conformance) and never writes to it. | ||
| 302 | - `TestVMRunsAJob` (`internal/sandbox/integration`, run by `scripts/it-vm-userns.sh`) does the first of these in a micro-VM. | 303 | - `TestVMRunsAJob` (`internal/sandbox/integration`, run by `scripts/it-vm-userns.sh`) does the first of these in a micro-VM. | ||
| 303 | 304 | ||||
| 304 | - | Not covered anywhere yet, because they need root on a real host: the jailer (uid drop, chroot, cgroups), a rootful Docker daemon or sysbox with the nested `dockerd`, and the firewall of container mode. | 305 | + | Checked as root on a real KVM host (Arch Linux, kernel 6.18 LTS, Firecracker and jailer 1.17, rootful podman as the Docker API): the jailer (uid drop, chroot, `memory.max` and `pids.max` cgroups, PID namespace), the VM and container integration scenarios, the firewall policy and NAT, the nested `dockerd` on the loop-mounted scratch disk (full disk gives `ENOSPC`), and `internal/e2e` including `TestChainCorpus`. Not checked: sysbox, and a real Docker Engine (podman's compatibility API is not a drop-in, see below). |
| 306 | + | ||||
| 307 | + | **Jailer notes** (found on that host): Firecracker 1.17's jailer needs `--chroot-base-dir` to exist (the driver creates `<state dir>/jail`), starts Firecracker as `/firecracker --id <id> --start-time-us ...` (the `/proc` scan that finds a VMM by id knows that form), and leaves `<state dir>/jail/<exec name>` and the cgroup parent `/sys/fs/cgroup/firecracker` behind (only the per-VM directories are the runner's). The jailed API socket path (`<state dir>/jail/firecracker/<id>/root/run/firecracker.socket`) must fit in 107 bytes; the driver checks it at start. | |||
| 308 | + | ||||
| 309 | + | **Host firewalls.** The runner's table has its own `input` hook at priority `filter`; another table with a later hook (firewalld at `filter + 10`) still sees a guest's packet to the gateway after the runner accepted it. A firewalld zone that rejects `udp/tcp 53` (the `public` zone with a DNS reject rule, for instance) therefore breaks the guests' DNS (the symptom is `dns silent` in `TestVMNetworkPolicy`) while NAT keeps working. On such a host, allow DNS from `ertap*` and `erbr*` in that firewall, or run the policy test in a private network namespace (as `scripts/it-vm-userns.sh` does). | |||
| 310 | + | ||||
| 311 | + | **Podman as the Docker API.** `DOCKER_HOST` pointing at rootful podman's compatibility socket gets as far as creating the network, then fails: the compatibility `POST /networks/create` cannot disable netavark's DNS (`dns_enabled` is always true) and `HostConfig.Dns` equal to the network gateway is refused when the container starts (`netavark: error while applying dns entries: DNS server 10.200.0.1 is the gateway of network ...; using it would cause aardvark-dns to forward queries to itself`); the option `com.docker.network.bridge.enable_ip_masquerade` is ignored with a warning, and netavark (`firewall_driver = "nftables"` by default) adds its own rules unless it is configured with `firewall_driver = "none"`. The integration tests passed with a small test-only proxy that rewrites that one request to the libpod endpoint with `dns_enabled=false`, and a private `CONTAINERS_CONF`. Production container mode supports Docker Engine (or sysbox) only. | |||
| 305 | 312 | ||||
| 306 | `ERMITE_RUNNER_IT_DOCKER_API=1 ERMITE_RUNNER_TEST_IMAGE=<image> go test -run DockerAPIWiring ./internal/sandbox/container` exercises the real Docker Engine calls (labels, networks, volumes, reaping) against any daemon, including a rootless one; it does not test isolation. | 313 | `ERMITE_RUNNER_IT_DOCKER_API=1 ERMITE_RUNNER_TEST_IMAGE=<image> go test -run DockerAPIWiring ./internal/sandbox/container` exercises the real Docker Engine calls (labels, networks, volumes, reaping) against any daemon, including a rootless one; it does not test isolation. | ||
| 307 | 314 | ||||
M internal/dnsfwd/dnsfwd.go +18 -0
| @@ -25,9 +25,11 @@ import ( | |||
| 25 | 25 | "strconv" | |
| 26 | 26 | "sync" | |
| 27 | 27 | "sync/atomic" | |
| 28 | + | "syscall" | |
| 28 | 29 | "time" | |
| 29 | 30 | ||
| 30 | 31 | "github.com/bnema/zerowrap" | |
| 32 | + | "golang.org/x/sys/unix" | |
| 31 | 33 | ) | |
| 32 | 34 | ||
| 33 | 35 | const ( | |
| @@ -55,6 +57,11 @@ type Forwarder struct { | |||
| 55 | 57 | port int | |
| 56 | 58 | // Lifetime overrides TCPLifetime; set before Listen (tests). | |
| 57 | 59 | Lifetime time.Duration | |
| 60 | + | // FreeBind binds with IP_FREEBIND, so that the gateway address may be | |
| 61 | + | // configured after the listener. Off by default: with Docker the address | |
| 62 | + | // exists when the network is created and a missing one is an error to | |
| 63 | + | // report at once, not a listener on an address nobody owns. Set before Listen. | |
| 64 | + | FreeBind bool | |
| 58 | 65 | ||
| 59 | 66 | mu sync.Mutex | |
| 60 | 67 | listeners map[netip.Addr]*listener | |
| @@ -94,6 +101,17 @@ func (f *Forwarder) Listen(ctx context.Context, addr netip.Addr) error { | |||
| 94 | 101 | } | |
| 95 | 102 | hp := net.JoinHostPort(addr.String(), strconv.Itoa(f.port)) | |
| 96 | 103 | var lc net.ListenConfig | |
| 104 | + | if f.FreeBind { | |
| 105 | + | // The gateway address may not exist yet (podman's netavark configures the | |
| 106 | + | // bridge address when the first container starts, after this listener). | |
| 107 | + | lc.Control = func(_, _ string, c syscall.RawConn) error { | |
| 108 | + | var serr error | |
| 109 | + | if err := c.Control(func(fd uintptr) { serr = unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, unix.IP_FREEBIND, 1) }); err != nil { | |
| 110 | + | return err | |
| 111 | + | } | |
| 112 | + | return serr | |
| 113 | + | } | |
| 114 | + | } | |
| 97 | 115 | pc, err := lc.ListenPacket(ctx, "udp", hp) | |
| 98 | 116 | if err != nil { | |
| 99 | 117 | return fmt.Errorf("dnsfwd: listen udp %s: %w", hp, err) | |
| @@ -25,9 +25,11 @@ import ( | |||||
| 25 | "strconv" | 25 | "strconv" | ||
| 26 | "sync" | 26 | "sync" | ||
| 27 | "sync/atomic" | 27 | "sync/atomic" | ||
| 28 | + | "syscall" | |||
| 28 | "time" | 29 | "time" | ||
| 29 | 30 | ||||
| 30 | "github.com/bnema/zerowrap" | 31 | "github.com/bnema/zerowrap" | ||
| 32 | + | "golang.org/x/sys/unix" | |||
| 31 | ) | 33 | ) | ||
| 32 | 34 | ||||
| 33 | const ( | 35 | const ( | ||
| @@ -55,6 +57,11 @@ type Forwarder struct { | |||||
| 55 | port int | 57 | port int | ||
| 56 | // Lifetime overrides TCPLifetime; set before Listen (tests). | 58 | // Lifetime overrides TCPLifetime; set before Listen (tests). | ||
| 57 | Lifetime time.Duration | 59 | Lifetime time.Duration | ||
| 60 | + | // FreeBind binds with IP_FREEBIND, so that the gateway address may be | |||
| 61 | + | // configured after the listener. Off by default: with Docker the address | |||
| 62 | + | // exists when the network is created and a missing one is an error to | |||
| 63 | + | // report at once, not a listener on an address nobody owns. Set before Listen. | |||
| 64 | + | FreeBind bool | |||
| 58 | 65 | ||||
| 59 | mu sync.Mutex | 66 | mu sync.Mutex | ||
| 60 | listeners map[netip.Addr]*listener | 67 | listeners map[netip.Addr]*listener | ||
| @@ -94,6 +101,17 @@ func (f *Forwarder) Listen(ctx context.Context, addr netip.Addr) error { | |||||
| 94 | } | 101 | } | ||
| 95 | hp := net.JoinHostPort(addr.String(), strconv.Itoa(f.port)) | 102 | hp := net.JoinHostPort(addr.String(), strconv.Itoa(f.port)) | ||
| 96 | var lc net.ListenConfig | 103 | var lc net.ListenConfig | ||
| 104 | + | if f.FreeBind { | |||
| 105 | + | // The gateway address may not exist yet (podman's netavark configures the | |||
| 106 | + | // bridge address when the first container starts, after this listener). | |||
| 107 | + | lc.Control = func(_, _ string, c syscall.RawConn) error { | |||
| 108 | + | var serr error | |||
| 109 | + | if err := c.Control(func(fd uintptr) { serr = unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, unix.IP_FREEBIND, 1) }); err != nil { | |||
| 110 | + | return err | |||
| 111 | + | } | |||
| 112 | + | return serr | |||
| 113 | + | } | |||
| 114 | + | } | |||
| 97 | pc, err := lc.ListenPacket(ctx, "udp", hp) | 115 | pc, err := lc.ListenPacket(ctx, "udp", hp) | ||
| 98 | if err != nil { | 116 | if err != nil { | ||
| 99 | return fmt.Errorf("dnsfwd: listen udp %s: %w", hp, err) | 117 | return fmt.Errorf("dnsfwd: listen udp %s: %w", hp, err) | ||
M internal/dnsfwd/dnsfwd_test.go +26 -0
| @@ -440,3 +440,29 @@ func TestUDPSaturationDropsAndIsolatesListeners(t *testing.T) { | |||
| 440 | 440 | t.Fatalf("another listener must not be starved by the flood: n=%d err=%v", n, err) | |
| 441 | 441 | } | |
| 442 | 442 | } | |
| 443 | + | ||
| 444 | + | // By default Listen fails fast on an address the host does not own (Docker and | |
| 445 | + | // the VM driver have configured it by then: a missing one is an error). | |
| 446 | + | func TestListenOnAMissingAddressFailsByDefault(t *testing.T) { | |
| 447 | + | f := New(func() []string { return nil }, 0) | |
| 448 | + | t.Cleanup(f.CloseAll) | |
| 449 | + | ip := netip.MustParseAddr("203.0.113.77") // TEST-NET-3: never configured here | |
| 450 | + | if err := f.Listen(context.Background(), ip); err == nil { | |
| 451 | + | t.Fatal("listening on an address the host does not own must fail without FreeBind") | |
| 452 | + | } | |
| 453 | + | } | |
| 454 | + | ||
| 455 | + | // With FreeBind (podman's netavark creates the bridge address only when the | |
| 456 | + | // first container starts) Listen binds an address the host does not own yet. | |
| 457 | + | func TestFreeBindListensOnAnAddressTheHostDoesNotOwnYet(t *testing.T) { | |
| 458 | + | f := New(func() []string { return nil }, 0) | |
| 459 | + | f.FreeBind = true | |
| 460 | + | t.Cleanup(f.CloseAll) | |
| 461 | + | ip := netip.MustParseAddr("203.0.113.77") | |
| 462 | + | if err := f.Listen(context.Background(), ip); err != nil { | |
| 463 | + | t.Fatalf("FreeBind: %v", err) | |
| 464 | + | } | |
| 465 | + | if _, ok := f.Addr(ip); !ok { | |
| 466 | + | t.Fatal("no listener") | |
| 467 | + | } | |
| 468 | + | } | |
| @@ -440,3 +440,29 @@ func TestUDPSaturationDropsAndIsolatesListeners(t *testing.T) { | |||||
| 440 | t.Fatalf("another listener must not be starved by the flood: n=%d err=%v", n, err) | 440 | t.Fatalf("another listener must not be starved by the flood: n=%d err=%v", n, err) | ||
| 441 | } | 441 | } | ||
| 442 | } | 442 | } | ||
| 443 | + | ||||
| 444 | + | // By default Listen fails fast on an address the host does not own (Docker and | |||
| 445 | + | // the VM driver have configured it by then: a missing one is an error). | |||
| 446 | + | func TestListenOnAMissingAddressFailsByDefault(t *testing.T) { | |||
| 447 | + | f := New(func() []string { return nil }, 0) | |||
| 448 | + | t.Cleanup(f.CloseAll) | |||
| 449 | + | ip := netip.MustParseAddr("203.0.113.77") // TEST-NET-3: never configured here | |||
| 450 | + | if err := f.Listen(context.Background(), ip); err == nil { | |||
| 451 | + | t.Fatal("listening on an address the host does not own must fail without FreeBind") | |||
| 452 | + | } | |||
| 453 | + | } | |||
| 454 | + | ||||
| 455 | + | // With FreeBind (podman's netavark creates the bridge address only when the | |||
| 456 | + | // first container starts) Listen binds an address the host does not own yet. | |||
| 457 | + | func TestFreeBindListensOnAnAddressTheHostDoesNotOwnYet(t *testing.T) { | |||
| 458 | + | f := New(func() []string { return nil }, 0) | |||
| 459 | + | f.FreeBind = true | |||
| 460 | + | t.Cleanup(f.CloseAll) | |||
| 461 | + | ip := netip.MustParseAddr("203.0.113.77") | |||
| 462 | + | if err := f.Listen(context.Background(), ip); err != nil { | |||
| 463 | + | t.Fatalf("FreeBind: %v", err) | |||
| 464 | + | } | |||
| 465 | + | if _, ok := f.Addr(ip); !ok { | |||
| 466 | + | t.Fatal("no listener") | |||
| 467 | + | } | |||
| 468 | + | } | |||
M internal/sandbox/container/container_test.go +11 -0
| @@ -1010,3 +1010,14 @@ func TestSocketPathTooLongIsExplained(t *testing.T) { | |||
| 1010 | 1010 | t.Fatalf("leftovers: %v", left) | |
| 1011 | 1011 | } | |
| 1012 | 1012 | } | |
| 1013 | + | ||
| 1014 | + | // GO-002: only a podman daemon gets a DNS listener that may precede the gateway | |
| 1015 | + | // address; Docker keeps a normal bind. | |
| 1016 | + | func TestOnlyPodmanGetsAFreeBindDNSListener(t *testing.T) { | |
| 1017 | + | if newNet(nil, 0, false).DNS.FreeBind { | |
| 1018 | + | t.Fatal("Docker must keep a normal bind") | |
| 1019 | + | } | |
| 1020 | + | if !newNet(nil, 0, true).DNS.FreeBind { | |
| 1021 | + | t.Fatal("podman needs the listener before the bridge address") | |
| 1022 | + | } | |
| 1023 | + | } | |
| @@ -1010,3 +1010,14 @@ func TestSocketPathTooLongIsExplained(t *testing.T) { | |||||
| 1010 | t.Fatalf("leftovers: %v", left) | 1010 | t.Fatalf("leftovers: %v", left) | ||
| 1011 | } | 1011 | } | ||
| 1012 | } | 1012 | } | ||
| 1013 | + | ||||
| 1014 | + | // GO-002: only a podman daemon gets a DNS listener that may precede the gateway | |||
| 1015 | + | // address; Docker keeps a normal bind. | |||
| 1016 | + | func TestOnlyPodmanGetsAFreeBindDNSListener(t *testing.T) { | |||
| 1017 | + | if newNet(nil, 0, false).DNS.FreeBind { | |||
| 1018 | + | t.Fatal("Docker must keep a normal bind") | |||
| 1019 | + | } | |||
| 1020 | + | if !newNet(nil, 0, true).DNS.FreeBind { | |||
| 1021 | + | t.Fatal("podman needs the listener before the bridge address") | |||
| 1022 | + | } | |||
| 1023 | + | } | |||
M internal/sandbox/container/driver.go +10 -1
| @@ -163,7 +163,7 @@ func New(ctx context.Context, cfg sandbox.Config, opt Options) (*Driver, error) | |||
| 163 | 163 | } | |
| 164 | 164 | d.net = opt.Net | |
| 165 | 165 | if d.net == nil { | |
| 166 | - | d.net = netsetup.New(opt.Run, cfg.DNSPort, "") | |
| 166 | + | d.net = newNet(opt.Run, cfg.DNSPort, info.Podman) | |
| 167 | 167 | } | |
| 168 | 168 | if err := d.net.EnableForwarding(); err != nil { | |
| 169 | 169 | return fail(err) | |
| @@ -171,6 +171,15 @@ func New(ctx context.Context, cfg sandbox.Config, opt Options) (*Driver, error) | |||
| 171 | 171 | return d, nil | |
| 172 | 172 | } | |
| 173 | 173 | ||
| 174 | + | // newNet builds the host network plumbing. Only podman creates the bridge | |
| 175 | + | // address late (see daemonInfo.Podman), so only there the DNS listener may come | |
| 176 | + | // before the address; on Docker a missing gateway address stays a startup error. | |
| 177 | + | func newNet(run cmdrun.Runner, dnsPort int, podman bool) *netsetup.Net { | |
| 178 | + | n := netsetup.New(run, dnsPort, "") | |
| 179 | + | n.DNS.FreeBind = podman | |
| 180 | + | return n | |
| 181 | + | } | |
| 182 | + | ||
| 174 | 183 | // maxSockPath is the longest Unix socket path Linux accepts (108 with the NUL). | |
| 175 | 184 | const maxSockPath = 107 | |
| 176 | 185 | ||
| @@ -163,7 +163,7 @@ func New(ctx context.Context, cfg sandbox.Config, opt Options) (*Driver, error) | |||||
| 163 | } | 163 | } | ||
| 164 | d.net = opt.Net | 164 | d.net = opt.Net | ||
| 165 | if d.net == nil { | 165 | if d.net == nil { | ||
| 166 | - | d.net = netsetup.New(opt.Run, cfg.DNSPort, "") | 166 | + | d.net = newNet(opt.Run, cfg.DNSPort, info.Podman) |
| 167 | } | 167 | } | ||
| 168 | if err := d.net.EnableForwarding(); err != nil { | 168 | if err := d.net.EnableForwarding(); err != nil { | ||
| 169 | return fail(err) | 169 | return fail(err) | ||
| @@ -171,6 +171,15 @@ func New(ctx context.Context, cfg sandbox.Config, opt Options) (*Driver, error) | |||||
| 171 | return d, nil | 171 | return d, nil | ||
| 172 | } | 172 | } | ||
| 173 | 173 | ||||
| 174 | + | // newNet builds the host network plumbing. Only podman creates the bridge | |||
| 175 | + | // address late (see daemonInfo.Podman), so only there the DNS listener may come | |||
| 176 | + | // before the address; on Docker a missing gateway address stays a startup error. | |||
| 177 | + | func newNet(run cmdrun.Runner, dnsPort int, podman bool) *netsetup.Net { | |||
| 178 | + | n := netsetup.New(run, dnsPort, "") | |||
| 179 | + | n.DNS.FreeBind = podman | |||
| 180 | + | return n | |||
| 181 | + | } | |||
| 182 | + | ||||
| 174 | // maxSockPath is the longest Unix socket path Linux accepts (108 with the NUL). | 183 | // maxSockPath is the longest Unix socket path Linux accepts (108 with the NUL). | ||
| 175 | const maxSockPath = 107 | 184 | const maxSockPath = 107 | ||
| 176 | 185 | ||||
M internal/sandbox/container/engine.go +20 -0
| @@ -8,11 +8,13 @@ import ( | |||
| 8 | 8 | "fmt" | |
| 9 | 9 | "io" | |
| 10 | 10 | "net/netip" | |
| 11 | + | "strings" | |
| 11 | 12 | ||
| 12 | 13 | cerrdefs "github.com/containerd/errdefs" | |
| 13 | 14 | "github.com/moby/moby/api/types/container" | |
| 14 | 15 | "github.com/moby/moby/api/types/mount" | |
| 15 | 16 | "github.com/moby/moby/api/types/network" | |
| 17 | + | "github.com/moby/moby/api/types/system" | |
| 16 | 18 | "github.com/moby/moby/client" | |
| 17 | 19 | ) | |
| 18 | 20 | ||
| @@ -47,6 +49,9 @@ type daemonInfo struct { | |||
| 47 | 49 | Rootless bool | |
| 48 | 50 | Runtimes []string | |
| 49 | 51 | OSType string | |
| 52 | + | // Podman: the "Docker API" is podman's compatibility service, whose netavark | |
| 53 | + | // configures a bridge address only when the first container starts. | |
| 54 | + | Podman bool | |
| 50 | 55 | } | |
| 51 | 56 | ||
| 52 | 57 | type networkSpec struct { | |
| @@ -108,9 +113,24 @@ func (e *dockerEngine) Info(ctx context.Context) (daemonInfo, error) { | |||
| 108 | 113 | for name := range r.Info.Runtimes { | |
| 109 | 114 | di.Runtimes = append(di.Runtimes, name) | |
| 110 | 115 | } | |
| 116 | + | // Best effort: a daemon that cannot say what it is is treated as Docker. | |
| 117 | + | if v, err := e.cli.ServerVersion(ctx, client.ServerVersionOptions{}); err == nil { | |
| 118 | + | di.Podman = isPodman(v.Components) | |
| 119 | + | } | |
| 111 | 120 | return di, nil | |
| 112 | 121 | } | |
| 113 | 122 | ||
| 123 | + | // isPodman reports whether the daemon's version components name Podman (its | |
| 124 | + | // compatibility service lists "Podman Engine"; Docker lists "Engine"). | |
| 125 | + | func isPodman(components []system.ComponentVersion) bool { | |
| 126 | + | for _, c := range components { | |
| 127 | + | if strings.HasPrefix(c.Name, "Podman") { | |
| 128 | + | return true | |
| 129 | + | } | |
| 130 | + | } | |
| 131 | + | return false | |
| 132 | + | } | |
| 133 | + | ||
| 114 | 134 | func (e *dockerEngine) EnsureImage(ctx context.Context, ref string) error { | |
| 115 | 135 | if _, err := e.cli.ImageInspect(ctx, ref); err == nil { | |
| 116 | 136 | return nil | |
| @@ -8,11 +8,13 @@ import ( | |||||
| 8 | "fmt" | 8 | "fmt" | ||
| 9 | "io" | 9 | "io" | ||
| 10 | "net/netip" | 10 | "net/netip" | ||
| 11 | + | "strings" | |||
| 11 | 12 | ||||
| 12 | cerrdefs "github.com/containerd/errdefs" | 13 | cerrdefs "github.com/containerd/errdefs" | ||
| 13 | "github.com/moby/moby/api/types/container" | 14 | "github.com/moby/moby/api/types/container" | ||
| 14 | "github.com/moby/moby/api/types/mount" | 15 | "github.com/moby/moby/api/types/mount" | ||
| 15 | "github.com/moby/moby/api/types/network" | 16 | "github.com/moby/moby/api/types/network" | ||
| 17 | + | "github.com/moby/moby/api/types/system" | |||
| 16 | "github.com/moby/moby/client" | 18 | "github.com/moby/moby/client" | ||
| 17 | ) | 19 | ) | ||
| 18 | 20 | ||||
| @@ -47,6 +49,9 @@ type daemonInfo struct { | |||||
| 47 | Rootless bool | 49 | Rootless bool | ||
| 48 | Runtimes []string | 50 | Runtimes []string | ||
| 49 | OSType string | 51 | OSType string | ||
| 52 | + | // Podman: the "Docker API" is podman's compatibility service, whose netavark | |||
| 53 | + | // configures a bridge address only when the first container starts. | |||
| 54 | + | Podman bool | |||
| 50 | } | 55 | } | ||
| 51 | 56 | ||||
| 52 | type networkSpec struct { | 57 | type networkSpec struct { | ||
| @@ -108,9 +113,24 @@ func (e *dockerEngine) Info(ctx context.Context) (daemonInfo, error) { | |||||
| 108 | for name := range r.Info.Runtimes { | 113 | for name := range r.Info.Runtimes { | ||
| 109 | di.Runtimes = append(di.Runtimes, name) | 114 | di.Runtimes = append(di.Runtimes, name) | ||
| 110 | } | 115 | } | ||
| 116 | + | // Best effort: a daemon that cannot say what it is is treated as Docker. | |||
| 117 | + | if v, err := e.cli.ServerVersion(ctx, client.ServerVersionOptions{}); err == nil { | |||
| 118 | + | di.Podman = isPodman(v.Components) | |||
| 119 | + | } | |||
| 111 | return di, nil | 120 | return di, nil | ||
| 112 | } | 121 | } | ||
| 113 | 122 | ||||
| 123 | + | // isPodman reports whether the daemon's version components name Podman (its | |||
| 124 | + | // compatibility service lists "Podman Engine"; Docker lists "Engine"). | |||
| 125 | + | func isPodman(components []system.ComponentVersion) bool { | |||
| 126 | + | for _, c := range components { | |||
| 127 | + | if strings.HasPrefix(c.Name, "Podman") { | |||
| 128 | + | return true | |||
| 129 | + | } | |||
| 130 | + | } | |||
| 131 | + | return false | |||
| 132 | + | } | |||
| 133 | + | ||||
| 114 | func (e *dockerEngine) EnsureImage(ctx context.Context, ref string) error { | 134 | func (e *dockerEngine) EnsureImage(ctx context.Context, ref string) error { | ||
| 115 | if _, err := e.cli.ImageInspect(ctx, ref); err == nil { | 135 | if _, err := e.cli.ImageInspect(ctx, ref); err == nil { | ||
| 116 | return nil | 136 | return nil | ||
A internal/sandbox/container/engine_test.go +74 -0
| @@ -0,0 +1,74 @@ | |||
| 1 | + | // SPDX-License-Identifier: MIT | |
| 2 | + | ||
| 3 | + | package container | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "net/http" | |
| 7 | + | "net/http/httptest" | |
| 8 | + | "testing" | |
| 9 | + | ||
| 10 | + | "github.com/moby/moby/client" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // infoDaemon plays the two endpoints Info reads. versionBody "" makes /version | |
| 14 | + | // fail with a 500. | |
| 15 | + | func infoDaemon(t *testing.T, versionBody string) *dockerEngine { | |
| 16 | + | t.Helper() | |
| 17 | + | mux := http.NewServeMux() | |
| 18 | + | mux.HandleFunc("/info", func(w http.ResponseWriter, _ *http.Request) { | |
| 19 | + | w.Header().Set("Content-Type", "application/json") | |
| 20 | + | _, _ = w.Write([]byte(`{"OSType":"linux","SecurityOptions":["name=seccomp,profile=builtin"],"Runtimes":{"runc":{"path":"runc"}}}`)) | |
| 21 | + | }) | |
| 22 | + | mux.HandleFunc("/version", func(w http.ResponseWriter, _ *http.Request) { | |
| 23 | + | if versionBody == "" { | |
| 24 | + | http.Error(w, `{"message":"boom"}`, http.StatusInternalServerError) | |
| 25 | + | return | |
| 26 | + | } | |
| 27 | + | w.Header().Set("Content-Type", "application/json") | |
| 28 | + | _, _ = w.Write([]byte(versionBody)) | |
| 29 | + | }) | |
| 30 | + | srv := httptest.NewServer(http.StripPrefix("/v1.43", mux)) | |
| 31 | + | t.Cleanup(srv.Close) | |
| 32 | + | cli, err := client.New(client.WithHost("tcp://"+srv.Listener.Addr().String()), client.WithAPIVersion("1.43")) | |
| 33 | + | if err != nil { | |
| 34 | + | t.Fatal(err) | |
| 35 | + | } | |
| 36 | + | t.Cleanup(func() { _ = cli.Close() }) | |
| 37 | + | return &dockerEngine{cli: cli} | |
| 38 | + | } | |
| 39 | + | ||
| 40 | + | // GO-004: the Podman flag comes from the version components' names; a daemon | |
| 41 | + | // whose /version fails (or names no Podman) is treated as Docker. | |
| 42 | + | func TestInfoDetectsPodman(t *testing.T) { | |
| 43 | + | for name, tc := range map[string]struct { | |
| 44 | + | version string | |
| 45 | + | podman bool | |
| 46 | + | }{ | |
| 47 | + | "podman": {`{"Version":"6.1.3","Components":[{"Name":"Podman Engine","Version":"6.1.3"},{"Name":"Conmon","Version":"2.2.1"}]}`, true}, | |
| 48 | + | "podman later": {`{"Components":[{"Name":"Conmon","Version":"2"},{"Name":"Podman Engine","Version":"6"}]}`, true}, | |
| 49 | + | "docker": {`{"Version":"29.9.0","Components":[{"Name":"Engine","Version":"29.9.0"},{"Name":"containerd","Version":"2"}]}`, false}, | |
| 50 | + | "no components": {`{"Version":"29.9.0"}`, false}, | |
| 51 | + | "lower case": {`{"Components":[{"Name":"podman engine","Version":"1"}]}`, false}, // the name is "Podman Engine": exact prefix | |
| 52 | + | "version fails": {"", false}, | |
| 53 | + | "malformed json": {`{not json`, false}, | |
| 54 | + | } { | |
| 55 | + | t.Run(name, func(t *testing.T) { | |
| 56 | + | di, err := infoDaemon(t, tc.version).Info(t.Context()) | |
| 57 | + | if err != nil { | |
| 58 | + | t.Fatalf("a /version failure must not fail Info: %v", err) | |
| 59 | + | } | |
| 60 | + | if di.Podman != tc.podman { | |
| 61 | + | t.Fatalf("Podman=%v want %v", di.Podman, tc.podman) | |
| 62 | + | } | |
| 63 | + | if di.OSType != "linux" || len(di.Runtimes) != 1 || di.Rootless { | |
| 64 | + | t.Fatalf("the rest of Info is unchanged: %+v", di) | |
| 65 | + | } | |
| 66 | + | }) | |
| 67 | + | } | |
| 68 | + | } | |
| 69 | + | ||
| 70 | + | func TestIsPodman(t *testing.T) { | |
| 71 | + | if isPodman(nil) { | |
| 72 | + | t.Fatal("no components is not podman") | |
| 73 | + | } | |
| 74 | + | } | |
| @@ -0,0 +1,74 @@ | |||||
| 1 | + | // SPDX-License-Identifier: MIT | |||
| 2 | + | ||||
| 3 | + | package container | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "net/http" | |||
| 7 | + | "net/http/httptest" | |||
| 8 | + | "testing" | |||
| 9 | + | ||||
| 10 | + | "github.com/moby/moby/client" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // infoDaemon plays the two endpoints Info reads. versionBody "" makes /version | |||
| 14 | + | // fail with a 500. | |||
| 15 | + | func infoDaemon(t *testing.T, versionBody string) *dockerEngine { | |||
| 16 | + | t.Helper() | |||
| 17 | + | mux := http.NewServeMux() | |||
| 18 | + | mux.HandleFunc("/info", func(w http.ResponseWriter, _ *http.Request) { | |||
| 19 | + | w.Header().Set("Content-Type", "application/json") | |||
| 20 | + | _, _ = w.Write([]byte(`{"OSType":"linux","SecurityOptions":["name=seccomp,profile=builtin"],"Runtimes":{"runc":{"path":"runc"}}}`)) | |||
| 21 | + | }) | |||
| 22 | + | mux.HandleFunc("/version", func(w http.ResponseWriter, _ *http.Request) { | |||
| 23 | + | if versionBody == "" { | |||
| 24 | + | http.Error(w, `{"message":"boom"}`, http.StatusInternalServerError) | |||
| 25 | + | return | |||
| 26 | + | } | |||
| 27 | + | w.Header().Set("Content-Type", "application/json") | |||
| 28 | + | _, _ = w.Write([]byte(versionBody)) | |||
| 29 | + | }) | |||
| 30 | + | srv := httptest.NewServer(http.StripPrefix("/v1.43", mux)) | |||
| 31 | + | t.Cleanup(srv.Close) | |||
| 32 | + | cli, err := client.New(client.WithHost("tcp://"+srv.Listener.Addr().String()), client.WithAPIVersion("1.43")) | |||
| 33 | + | if err != nil { | |||
| 34 | + | t.Fatal(err) | |||
| 35 | + | } | |||
| 36 | + | t.Cleanup(func() { _ = cli.Close() }) | |||
| 37 | + | return &dockerEngine{cli: cli} | |||
| 38 | + | } | |||
| 39 | + | ||||
| 40 | + | // GO-004: the Podman flag comes from the version components' names; a daemon | |||
| 41 | + | // whose /version fails (or names no Podman) is treated as Docker. | |||
| 42 | + | func TestInfoDetectsPodman(t *testing.T) { | |||
| 43 | + | for name, tc := range map[string]struct { | |||
| 44 | + | version string | |||
| 45 | + | podman bool | |||
| 46 | + | }{ | |||
| 47 | + | "podman": {`{"Version":"6.1.3","Components":[{"Name":"Podman Engine","Version":"6.1.3"},{"Name":"Conmon","Version":"2.2.1"}]}`, true}, | |||
| 48 | + | "podman later": {`{"Components":[{"Name":"Conmon","Version":"2"},{"Name":"Podman Engine","Version":"6"}]}`, true}, | |||
| 49 | + | "docker": {`{"Version":"29.9.0","Components":[{"Name":"Engine","Version":"29.9.0"},{"Name":"containerd","Version":"2"}]}`, false}, | |||
| 50 | + | "no components": {`{"Version":"29.9.0"}`, false}, | |||
| 51 | + | "lower case": {`{"Components":[{"Name":"podman engine","Version":"1"}]}`, false}, // the name is "Podman Engine": exact prefix | |||
| 52 | + | "version fails": {"", false}, | |||
| 53 | + | "malformed json": {`{not json`, false}, | |||
| 54 | + | } { | |||
| 55 | + | t.Run(name, func(t *testing.T) { | |||
| 56 | + | di, err := infoDaemon(t, tc.version).Info(t.Context()) | |||
| 57 | + | if err != nil { | |||
| 58 | + | t.Fatalf("a /version failure must not fail Info: %v", err) | |||
| 59 | + | } | |||
| 60 | + | if di.Podman != tc.podman { | |||
| 61 | + | t.Fatalf("Podman=%v want %v", di.Podman, tc.podman) | |||
| 62 | + | } | |||
| 63 | + | if di.OSType != "linux" || len(di.Runtimes) != 1 || di.Rootless { | |||
| 64 | + | t.Fatalf("the rest of Info is unchanged: %+v", di) | |||
| 65 | + | } | |||
| 66 | + | }) | |||
| 67 | + | } | |||
| 68 | + | } | |||
| 69 | + | ||||
| 70 | + | func TestIsPodman(t *testing.T) { | |||
| 71 | + | if isPodman(nil) { | |||
| 72 | + | t.Fatal("no components is not podman") | |||
| 73 | + | } | |||
| 74 | + | } | |||
M internal/sandbox/integration/container_test.go +6 -1
| @@ -21,12 +21,17 @@ func TestContainerStubExecutor(t *testing.T) { | |||
| 21 | 21 | ctx := ctxWithLog(t) | |
| 22 | 22 | cfg := loadConfig(t, sandbox.Container) | |
| 23 | 23 | // A rootless daemon is refused with a clear error: nothing to test there. | |
| 24 | - | if _, err := drivers.Factories().Container(ctx, cfg); err != nil { | |
| 24 | + | probe, err := drivers.Factories().Container(ctx, cfg) | |
| 25 | + | if err != nil { | |
| 25 | 26 | if strings.Contains(err.Error(), "rootless") { | |
| 26 | 27 | t.Skipf("the Docker daemon is rootless, as designed the driver refuses it: %v", err) | |
| 27 | 28 | } | |
| 28 | 29 | t.Fatal(err) | |
| 29 | 30 | } | |
| 31 | + | // The probe holds the host-wide lock: release it before the scenario takes it. | |
| 32 | + | if err := probe.Close(); err != nil { | |
| 33 | + | t.Fatal(err) | |
| 34 | + | } | |
| 30 | 35 | scenario(t, ctx, cfg, sandbox.Container, func(t *testing.T, sb sandbox.Sandbox) { | |
| 31 | 36 | out, err := exec.Command("docker", "ps", "-q", "--filter", "label="+sandbox.LabelJob).Output() | |
| 32 | 37 | if err != nil { | |
| @@ -21,12 +21,17 @@ func TestContainerStubExecutor(t *testing.T) { | |||||
| 21 | ctx := ctxWithLog(t) | 21 | ctx := ctxWithLog(t) | ||
| 22 | cfg := loadConfig(t, sandbox.Container) | 22 | cfg := loadConfig(t, sandbox.Container) | ||
| 23 | // A rootless daemon is refused with a clear error: nothing to test there. | 23 | // A rootless daemon is refused with a clear error: nothing to test there. | ||
| 24 | - | if _, err := drivers.Factories().Container(ctx, cfg); err != nil { | 24 | + | probe, err := drivers.Factories().Container(ctx, cfg) |
| 25 | + | if err != nil { | |||
| 25 | if strings.Contains(err.Error(), "rootless") { | 26 | if strings.Contains(err.Error(), "rootless") { | ||
| 26 | t.Skipf("the Docker daemon is rootless, as designed the driver refuses it: %v", err) | 27 | t.Skipf("the Docker daemon is rootless, as designed the driver refuses it: %v", err) | ||
| 27 | } | 28 | } | ||
| 28 | t.Fatal(err) | 29 | t.Fatal(err) | ||
| 29 | } | 30 | } | ||
| 31 | + | // The probe holds the host-wide lock: release it before the scenario takes it. | |||
| 32 | + | if err := probe.Close(); err != nil { | |||
| 33 | + | t.Fatal(err) | |||
| 34 | + | } | |||
| 30 | scenario(t, ctx, cfg, sandbox.Container, func(t *testing.T, sb sandbox.Sandbox) { | 35 | scenario(t, ctx, cfg, sandbox.Container, func(t *testing.T, sb sandbox.Sandbox) { | ||
| 31 | out, err := exec.Command("docker", "ps", "-q", "--filter", "label="+sandbox.LabelJob).Output() | 36 | out, err := exec.Command("docker", "ps", "-q", "--filter", "label="+sandbox.LabelJob).Output() | ||
| 32 | if err != nil { | 37 | if err != nil { | ||
M internal/sandbox/integration/integration_test.go +11 -16
| @@ -33,6 +33,7 @@ import ( | |||
| 33 | 33 | "bnema.dev/ermite-runner/internal/netpolicy" | |
| 34 | 34 | "bnema.dev/ermite-runner/internal/sandbox" | |
| 35 | 35 | "bnema.dev/ermite-runner/internal/sandbox/drivers" | |
| 36 | + | "bnema.dev/ermite-runner/internal/sandbox/vm" | |
| 36 | 37 | ) | |
| 37 | 38 | ||
| 38 | 39 | // stubScript is the CI-1 stub job of the plan. | |
| @@ -202,6 +203,11 @@ func leftovers(stateDir string) []string { | |||
| 202 | 203 | _ = filepath.WalkDir(filepath.Join(stateDir, d), func(p string, e os.DirEntry, err error) error { | |
| 203 | 204 | if err == nil && p != filepath.Join(stateDir, d) { | |
| 204 | 205 | rel, _ := filepath.Rel(stateDir, p) | |
| 206 | + | // <state>/jail/<exec file name> is the jailer's static parent of the | |
| 207 | + | // per-job chroots (like jail/ itself): not a leftover when empty. | |
| 208 | + | if e.IsDir() && d == "jail" && filepath.Dir(rel) == "jail" { | |
| 209 | + | return nil | |
| 210 | + | } | |
| 205 | 211 | left = append(left, "file "+rel) | |
| 206 | 212 | } | |
| 207 | 213 | return nil | |
| @@ -214,8 +220,9 @@ func leftovers(stateDir string) []string { | |||
| 214 | 220 | } | |
| 215 | 221 | ||
| 216 | 222 | // vmmProcs returns the processes whose command line carries one of our VM ids | |
| 217 | - | // (and --api-sock or --exec-file), whatever the program is called: the rule the | |
| 218 | - | // driver's Reap uses. | |
| 223 | + | // and a VMM marker (vm.IDFromCmdline: the rule the driver's scan is built on), | |
| 224 | + | // whatever the program is called. Deliberately not narrowed to the state | |
| 225 | + | // directory: the leak check wants to see any process of ours. | |
| 219 | 226 | func vmmProcs() map[int]string { | |
| 220 | 227 | out := map[int]string{} | |
| 221 | 228 | procs, _ := filepath.Glob("/proc/[0-9]*/cmdline") | |
| @@ -224,20 +231,8 @@ func vmmProcs() map[int]string { | |||
| 224 | 231 | if err != nil { | |
| 225 | 232 | continue | |
| 226 | 233 | } | |
| 227 | - | args := strings.Split(strings.TrimRight(string(b), "\x00"), "\x00") | |
| 228 | - | var id string | |
| 229 | - | var marker bool | |
| 230 | - | for i, a := range args { | |
| 231 | - | switch a { | |
| 232 | - | case "--id": | |
| 233 | - | if i+1 < len(args) && sandbox.ValidID(args[i+1]) { | |
| 234 | - | id = args[i+1] | |
| 235 | - | } | |
| 236 | - | case "--api-sock", "--exec-file": | |
| 237 | - | marker = true | |
| 238 | - | } | |
| 239 | - | } | |
| 240 | - | if id == "" || !marker { | |
| 234 | + | id := vm.IDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")) | |
| 235 | + | if id == "" { | |
| 241 | 236 | continue | |
| 242 | 237 | } | |
| 243 | 238 | var pid int | |
| @@ -33,6 +33,7 @@ import ( | |||||
| 33 | "bnema.dev/ermite-runner/internal/netpolicy" | 33 | "bnema.dev/ermite-runner/internal/netpolicy" | ||
| 34 | "bnema.dev/ermite-runner/internal/sandbox" | 34 | "bnema.dev/ermite-runner/internal/sandbox" | ||
| 35 | "bnema.dev/ermite-runner/internal/sandbox/drivers" | 35 | "bnema.dev/ermite-runner/internal/sandbox/drivers" | ||
| 36 | + | "bnema.dev/ermite-runner/internal/sandbox/vm" | |||
| 36 | ) | 37 | ) | ||
| 37 | 38 | ||||
| 38 | // stubScript is the CI-1 stub job of the plan. | 39 | // stubScript is the CI-1 stub job of the plan. | ||
| @@ -202,6 +203,11 @@ func leftovers(stateDir string) []string { | |||||
| 202 | _ = filepath.WalkDir(filepath.Join(stateDir, d), func(p string, e os.DirEntry, err error) error { | 203 | _ = filepath.WalkDir(filepath.Join(stateDir, d), func(p string, e os.DirEntry, err error) error { | ||
| 203 | if err == nil && p != filepath.Join(stateDir, d) { | 204 | if err == nil && p != filepath.Join(stateDir, d) { | ||
| 204 | rel, _ := filepath.Rel(stateDir, p) | 205 | rel, _ := filepath.Rel(stateDir, p) | ||
| 206 | + | // <state>/jail/<exec file name> is the jailer's static parent of the | |||
| 207 | + | // per-job chroots (like jail/ itself): not a leftover when empty. | |||
| 208 | + | if e.IsDir() && d == "jail" && filepath.Dir(rel) == "jail" { | |||
| 209 | + | return nil | |||
| 210 | + | } | |||
| 205 | left = append(left, "file "+rel) | 211 | left = append(left, "file "+rel) | ||
| 206 | } | 212 | } | ||
| 207 | return nil | 213 | return nil | ||
| @@ -214,8 +220,9 @@ func leftovers(stateDir string) []string { | |||||
| 214 | } | 220 | } | ||
| 215 | 221 | ||||
| 216 | // vmmProcs returns the processes whose command line carries one of our VM ids | 222 | // vmmProcs returns the processes whose command line carries one of our VM ids | ||
| 217 | - | // (and --api-sock or --exec-file), whatever the program is called: the rule the | 223 | + | // and a VMM marker (vm.IDFromCmdline: the rule the driver's scan is built on), |
| 218 | - | // driver's Reap uses. | 224 | + | // whatever the program is called. Deliberately not narrowed to the state |
| 225 | + | // directory: the leak check wants to see any process of ours. | |||
| 219 | func vmmProcs() map[int]string { | 226 | func vmmProcs() map[int]string { | ||
| 220 | out := map[int]string{} | 227 | out := map[int]string{} | ||
| 221 | procs, _ := filepath.Glob("/proc/[0-9]*/cmdline") | 228 | procs, _ := filepath.Glob("/proc/[0-9]*/cmdline") | ||
| @@ -224,20 +231,8 @@ func vmmProcs() map[int]string { | |||||
| 224 | if err != nil { | 231 | if err != nil { | ||
| 225 | continue | 232 | continue | ||
| 226 | } | 233 | } | ||
| 227 | - | args := strings.Split(strings.TrimRight(string(b), "\x00"), "\x00") | 234 | + | id := vm.IDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")) |
| 228 | - | var id string | 235 | + | if id == "" { |
| 229 | - | var marker bool | |||
| 230 | - | for i, a := range args { | |||
| 231 | - | switch a { | |||
| 232 | - | case "--id": | |||
| 233 | - | if i+1 < len(args) && sandbox.ValidID(args[i+1]) { | |||
| 234 | - | id = args[i+1] | |||
| 235 | - | } | |||
| 236 | - | case "--api-sock", "--exec-file": | |||
| 237 | - | marker = true | |||
| 238 | - | } | |||
| 239 | - | } | |||
| 240 | - | if id == "" || !marker { | |||
| 241 | continue | 236 | continue | ||
| 242 | } | 237 | } | ||
| 243 | var pid int | 238 | var pid int | ||
M internal/sandbox/vm/driver.go +87 -7
| @@ -15,6 +15,7 @@ import ( | |||
| 15 | 15 | "context" | |
| 16 | 16 | "errors" | |
| 17 | 17 | "fmt" | |
| 18 | + | "io/fs" | |
| 18 | 19 | "net" | |
| 19 | 20 | "net/netip" | |
| 20 | 21 | "os" | |
| @@ -52,7 +53,7 @@ type Options struct { | |||
| 52 | 53 | // FreeSpace overrides statfs for the free-space warning (tests). | |
| 53 | 54 | FreeSpace func(string) (int64, error) | |
| 54 | 55 | // kill overrides killAndWait (tests simulate a VMM that cannot be killed). | |
| 55 | - | kill func(pid int, timeout time.Duration) bool | |
| 56 | + | kill func(pid int, still func() bool, timeout time.Duration) bool | |
| 56 | 57 | } | |
| 57 | 58 | ||
| 58 | 59 | // vmmOverheadMiB is what the Firecracker process itself may use on top of the | |
| @@ -120,7 +121,17 @@ func New(ctx context.Context, cfg sandbox.Config, opt Options) (*Driver, error) | |||
| 120 | 121 | } | |
| 121 | 122 | fail := func(err error) (*Driver, error) { _ = d.Close(); return nil, err } | |
| 122 | 123 | ||
| 124 | + | // A path over sun_path fails later as a 10 s "API not ready" (or a vsock | |
| 125 | + | // that never connects): refuse at start, in both modes. | |
| 126 | + | if err := checkSocketPaths(cfg.StateDir, filepath.Base(d.fcPath), !cfg.JailerOff); err != nil { | |
| 127 | + | return fail(err) | |
| 128 | + | } | |
| 123 | 129 | if !cfg.JailerOff { | |
| 130 | + | // Jailer 1.17 canonicalizes --chroot-base-dir and fails when it does not | |
| 131 | + | // exist ("Failed to canonicalize path"); older releases created it. | |
| 132 | + | if err := ensureJailBase(jailBase(cfg.StateDir)); err != nil { | |
| 133 | + | return fail(err) | |
| 134 | + | } | |
| 124 | 135 | for _, p := range []string{cfg.KernelPath(), cfg.BaseImagePath()} { | |
| 125 | 136 | if err := checkWorldReadable(p); err != nil { | |
| 126 | 137 | return fail(err) | |
| @@ -191,6 +202,9 @@ func (d *Driver) New() (sandbox.Sandbox, error) { | |||
| 191 | 202 | return &vmSandbox{d: d, slot: slot, id: id}, nil | |
| 192 | 203 | } | |
| 193 | 204 | ||
| 205 | + | // scope narrows the /proc scan to this driver's own jail base. | |
| 206 | + | func (d *Driver) scope() scope { return newScope(d.cfg.StateDir) } | |
| 207 | + | ||
| 194 | 208 | func (d *Driver) release(slot int) { | |
| 195 | 209 | d.mu.Lock() | |
| 196 | 210 | d.slots.Release(slot) | |
| @@ -390,6 +404,66 @@ func (s *vmSandbox) bootVsock() (<-chan acceptResult, error) { | |||
| 390 | 404 | ||
| 391 | 405 | const udsName = "v.sock" | |
| 392 | 406 | ||
| 407 | + | // maxSockPath is the longest Unix socket path Linux accepts (108 with the NUL). | |
| 408 | + | const maxSockPath = 107 | |
| 409 | + | ||
| 410 | + | // socketPaths lists the host paths of the unix sockets of a VM: the API socket | |
| 411 | + | // and the vsock listener (the guest's UDS path plus "_<port>"). With the jailer | |
| 412 | + | // they are deep in the chroot; without it they sit in <state>/vm/<id>. | |
| 413 | + | func socketPaths(stateDir, execName string, jailed bool) []string { | |
| 414 | + | lay := newLayout(stateDir, execName, "er00000000", jailed, 0) | |
| 415 | + | api := lay.host("api.sock") | |
| 416 | + | if jailed { | |
| 417 | + | api = lay.host("run/firecracker.socket") | |
| 418 | + | } | |
| 419 | + | return []string{api, lay.host(udsName) + fmt.Sprintf("_%d", sandbox.VsockPort)} | |
| 420 | + | } | |
| 421 | + | ||
| 422 | + | // checkSocketPaths refuses a state directory that makes a VM socket path longer | |
| 423 | + | // than a unix socket address. | |
| 424 | + | func checkSocketPaths(stateDir, execName string, jailed bool) error { | |
| 425 | + | for _, p := range socketPaths(stateDir, execName, jailed) { | |
| 426 | + | if len(p) > maxSockPath { | |
| 427 | + | return fmt.Errorf("socket path %q is %d bytes, over the %d-byte limit of a Unix socket: use a shorter %s", p, len(p), maxSockPath, sandbox.EnvStateDir) | |
| 428 | + | } | |
| 429 | + | } | |
| 430 | + | return nil | |
| 431 | + | } | |
| 432 | + | ||
| 433 | + | // ensureJailBase creates the jailer's chroot base and refuses one that others | |
| 434 | + | // could use to redirect the jailer: anything but a real directory (a symlink is | |
| 435 | + | // not followed), owned by neither root nor the runner's own user, or writable by | |
| 436 | + | // group or others. An existing directory is checked, never chmod-ed. | |
| 437 | + | func ensureJailBase(dir string) error { | |
| 438 | + | if err := os.MkdirAll(dir, 0o700); err != nil { | |
| 439 | + | return fmt.Errorf("jailer chroot base: %w", err) | |
| 440 | + | } | |
| 441 | + | fi, err := os.Lstat(dir) | |
| 442 | + | if err != nil { | |
| 443 | + | return fmt.Errorf("jailer chroot base: %w", err) | |
| 444 | + | } | |
| 445 | + | return checkJailBaseInfo(dir, fi, os.Geteuid()) | |
| 446 | + | } | |
| 447 | + | ||
| 448 | + | func checkJailBaseInfo(dir string, fi fs.FileInfo, euid int) error { | |
| 449 | + | if !fi.IsDir() { | |
| 450 | + | return fmt.Errorf("jailer chroot base %s is not a directory (%v): remove it", dir, fi.Mode().Type()) | |
| 451 | + | } | |
| 452 | + | if fi.Mode().Perm()&0o022 != 0 { | |
| 453 | + | return fmt.Errorf("jailer chroot base %s is writable by group or others (mode %04o): chmod go-w it", dir, fi.Mode().Perm()) | |
| 454 | + | } | |
| 455 | + | st, ok := fi.Sys().(*syscall.Stat_t) | |
| 456 | + | if !ok { | |
| 457 | + | return fmt.Errorf("jailer chroot base %s: cannot read its owner", dir) | |
| 458 | + | } | |
| 459 | + | // The jailer needs root, so the runner is root in production; accepting its own | |
| 460 | + | // user as well keeps the driver testable without root. | |
| 461 | + | if int(st.Uid) != 0 && int(st.Uid) != euid { | |
| 462 | + | return fmt.Errorf("jailer chroot base %s is owned by uid %d, not root: chown it", dir, st.Uid) | |
| 463 | + | } | |
| 464 | + | return nil | |
| 465 | + | } | |
| 466 | + | ||
| 393 | 467 | func (s *vmSandbox) vmConfig(spec sandbox.JobSpec, netw netpolicy.Net, f vmFiles) vmConfig { | |
| 394 | 468 | cfg := s.d.cfg | |
| 395 | 469 | return vmConfig{ | |
| @@ -495,7 +569,7 @@ func (s *vmSandbox) startVMM(spec sandbox.JobSpec, apiSock string) error { | |||
| 495 | 569 | func (s *vmSandbox) awaitVMMGone() { | |
| 496 | 570 | for { | |
| 497 | 571 | alive := false | |
| 498 | - | for pid, id := range vmmProcs(s.d.opt.ProcDir) { | |
| 572 | + | for pid, id := range vmmProcs(s.d.opt.ProcDir, s.d.scope()) { | |
| 499 | 573 | if id == s.id && pidAlive(pid) { | |
| 500 | 574 | alive = true | |
| 501 | 575 | } | |
| @@ -554,7 +628,7 @@ func (s *vmSandbox) vmmPIDs() []int { | |||
| 554 | 628 | if s.cmd != nil && s.cmd.Process != nil { | |
| 555 | 629 | set[s.cmd.Process.Pid] = true | |
| 556 | 630 | } | |
| 557 | - | for pid, id := range vmmProcs(s.d.opt.ProcDir) { | |
| 631 | + | for pid, id := range vmmProcs(s.d.opt.ProcDir, s.d.scope()) { | |
| 558 | 632 | if id == s.id { | |
| 559 | 633 | set[pid] = true | |
| 560 | 634 | } | |
| @@ -566,6 +640,12 @@ func (s *vmSandbox) vmmPIDs() []int { | |||
| 566 | 640 | return out | |
| 567 | 641 | } | |
| 568 | 642 | ||
| 643 | + | // stillMine is the identity check handed to the kill: pid still carries this | |
| 644 | + | // VM's id and passes the scope (command line and root). | |
| 645 | + | func (s *vmSandbox) stillMine(pid int) func() bool { | |
| 646 | + | return func() bool { return cmdlineID(s.d.opt.ProcDir, pid, s.d.scope()) == s.id } | |
| 647 | + | } | |
| 648 | + | ||
| 569 | 649 | // cleanupLocked removes everything the VM created, in dependency order. Steps | |
| 570 | 650 | // that need the VMM to be dead (network device, firewall rules, files, cgroup) | |
| 571 | 651 | // are skipped when it could not be killed: removing the TAP or the policy under | |
| @@ -585,11 +665,11 @@ func (s *vmSandbox) cleanupLocked(ctx context.Context) error { | |||
| 585 | 665 | s.listener = nil | |
| 586 | 666 | } | |
| 587 | 667 | dead := true | |
| 588 | - | if s.cmd != nil && s.cmd.Process != nil { | |
| 589 | - | _ = syscall.Kill(-s.cmd.Process.Pid, syscall.SIGKILL) // the whole group | |
| 590 | - | } | |
| 668 | + | // No kill(-pgid) of the launched command: once it has been waited for, its | |
| 669 | + | // pid (and so its group id) may belong to anyone. Every process of this VM is | |
| 670 | + | // found by the scan (id, markers and scope) and killed through a pidfd. | |
| 591 | 671 | for _, pid := range s.vmmPIDs() { | |
| 592 | - | if !d.opt.kill(pid, 5*time.Second) { | |
| 672 | + | if !d.opt.kill(pid, s.stillMine(pid), 5*time.Second) { | |
| 593 | 673 | dead = false | |
| 594 | 674 | errs = append(errs, fmt.Errorf("vmm process %d did not exit", pid)) | |
| 595 | 675 | } | |
| @@ -15,6 +15,7 @@ import ( | |||||
| 15 | "context" | 15 | "context" | ||
| 16 | "errors" | 16 | "errors" | ||
| 17 | "fmt" | 17 | "fmt" | ||
| 18 | + | "io/fs" | |||
| 18 | "net" | 19 | "net" | ||
| 19 | "net/netip" | 20 | "net/netip" | ||
| 20 | "os" | 21 | "os" | ||
| @@ -52,7 +53,7 @@ type Options struct { | |||||
| 52 | // FreeSpace overrides statfs for the free-space warning (tests). | 53 | // FreeSpace overrides statfs for the free-space warning (tests). | ||
| 53 | FreeSpace func(string) (int64, error) | 54 | FreeSpace func(string) (int64, error) | ||
| 54 | // kill overrides killAndWait (tests simulate a VMM that cannot be killed). | 55 | // kill overrides killAndWait (tests simulate a VMM that cannot be killed). | ||
| 55 | - | kill func(pid int, timeout time.Duration) bool | 56 | + | kill func(pid int, still func() bool, timeout time.Duration) bool |
| 56 | } | 57 | } | ||
| 57 | 58 | ||||
| 58 | // vmmOverheadMiB is what the Firecracker process itself may use on top of the | 59 | // vmmOverheadMiB is what the Firecracker process itself may use on top of the | ||
| @@ -120,7 +121,17 @@ func New(ctx context.Context, cfg sandbox.Config, opt Options) (*Driver, error) | |||||
| 120 | } | 121 | } | ||
| 121 | fail := func(err error) (*Driver, error) { _ = d.Close(); return nil, err } | 122 | fail := func(err error) (*Driver, error) { _ = d.Close(); return nil, err } | ||
| 122 | 123 | ||||
| 124 | + | // A path over sun_path fails later as a 10 s "API not ready" (or a vsock | |||
| 125 | + | // that never connects): refuse at start, in both modes. | |||
| 126 | + | if err := checkSocketPaths(cfg.StateDir, filepath.Base(d.fcPath), !cfg.JailerOff); err != nil { | |||
| 127 | + | return fail(err) | |||
| 128 | + | } | |||
| 123 | if !cfg.JailerOff { | 129 | if !cfg.JailerOff { | ||
| 130 | + | // Jailer 1.17 canonicalizes --chroot-base-dir and fails when it does not | |||
| 131 | + | // exist ("Failed to canonicalize path"); older releases created it. | |||
| 132 | + | if err := ensureJailBase(jailBase(cfg.StateDir)); err != nil { | |||
| 133 | + | return fail(err) | |||
| 134 | + | } | |||
| 124 | for _, p := range []string{cfg.KernelPath(), cfg.BaseImagePath()} { | 135 | for _, p := range []string{cfg.KernelPath(), cfg.BaseImagePath()} { | ||
| 125 | if err := checkWorldReadable(p); err != nil { | 136 | if err := checkWorldReadable(p); err != nil { | ||
| 126 | return fail(err) | 137 | return fail(err) | ||
| @@ -191,6 +202,9 @@ func (d *Driver) New() (sandbox.Sandbox, error) { | |||||
| 191 | return &vmSandbox{d: d, slot: slot, id: id}, nil | 202 | return &vmSandbox{d: d, slot: slot, id: id}, nil | ||
| 192 | } | 203 | } | ||
| 193 | 204 | ||||
| 205 | + | // scope narrows the /proc scan to this driver's own jail base. | |||
| 206 | + | func (d *Driver) scope() scope { return newScope(d.cfg.StateDir) } | |||
| 207 | + | ||||
| 194 | func (d *Driver) release(slot int) { | 208 | func (d *Driver) release(slot int) { | ||
| 195 | d.mu.Lock() | 209 | d.mu.Lock() | ||
| 196 | d.slots.Release(slot) | 210 | d.slots.Release(slot) | ||
| @@ -390,6 +404,66 @@ func (s *vmSandbox) bootVsock() (<-chan acceptResult, error) { | |||||
| 390 | 404 | ||||
| 391 | const udsName = "v.sock" | 405 | const udsName = "v.sock" | ||
| 392 | 406 | ||||
| 407 | + | // maxSockPath is the longest Unix socket path Linux accepts (108 with the NUL). | |||
| 408 | + | const maxSockPath = 107 | |||
| 409 | + | ||||
| 410 | + | // socketPaths lists the host paths of the unix sockets of a VM: the API socket | |||
| 411 | + | // and the vsock listener (the guest's UDS path plus "_<port>"). With the jailer | |||
| 412 | + | // they are deep in the chroot; without it they sit in <state>/vm/<id>. | |||
| 413 | + | func socketPaths(stateDir, execName string, jailed bool) []string { | |||
| 414 | + | lay := newLayout(stateDir, execName, "er00000000", jailed, 0) | |||
| 415 | + | api := lay.host("api.sock") | |||
| 416 | + | if jailed { | |||
| 417 | + | api = lay.host("run/firecracker.socket") | |||
| 418 | + | } | |||
| 419 | + | return []string{api, lay.host(udsName) + fmt.Sprintf("_%d", sandbox.VsockPort)} | |||
| 420 | + | } | |||
| 421 | + | ||||
| 422 | + | // checkSocketPaths refuses a state directory that makes a VM socket path longer | |||
| 423 | + | // than a unix socket address. | |||
| 424 | + | func checkSocketPaths(stateDir, execName string, jailed bool) error { | |||
| 425 | + | for _, p := range socketPaths(stateDir, execName, jailed) { | |||
| 426 | + | if len(p) > maxSockPath { | |||
| 427 | + | return fmt.Errorf("socket path %q is %d bytes, over the %d-byte limit of a Unix socket: use a shorter %s", p, len(p), maxSockPath, sandbox.EnvStateDir) | |||
| 428 | + | } | |||
| 429 | + | } | |||
| 430 | + | return nil | |||
| 431 | + | } | |||
| 432 | + | ||||
| 433 | + | // ensureJailBase creates the jailer's chroot base and refuses one that others | |||
| 434 | + | // could use to redirect the jailer: anything but a real directory (a symlink is | |||
| 435 | + | // not followed), owned by neither root nor the runner's own user, or writable by | |||
| 436 | + | // group or others. An existing directory is checked, never chmod-ed. | |||
| 437 | + | func ensureJailBase(dir string) error { | |||
| 438 | + | if err := os.MkdirAll(dir, 0o700); err != nil { | |||
| 439 | + | return fmt.Errorf("jailer chroot base: %w", err) | |||
| 440 | + | } | |||
| 441 | + | fi, err := os.Lstat(dir) | |||
| 442 | + | if err != nil { | |||
| 443 | + | return fmt.Errorf("jailer chroot base: %w", err) | |||
| 444 | + | } | |||
| 445 | + | return checkJailBaseInfo(dir, fi, os.Geteuid()) | |||
| 446 | + | } | |||
| 447 | + | ||||
| 448 | + | func checkJailBaseInfo(dir string, fi fs.FileInfo, euid int) error { | |||
| 449 | + | if !fi.IsDir() { | |||
| 450 | + | return fmt.Errorf("jailer chroot base %s is not a directory (%v): remove it", dir, fi.Mode().Type()) | |||
| 451 | + | } | |||
| 452 | + | if fi.Mode().Perm()&0o022 != 0 { | |||
| 453 | + | return fmt.Errorf("jailer chroot base %s is writable by group or others (mode %04o): chmod go-w it", dir, fi.Mode().Perm()) | |||
| 454 | + | } | |||
| 455 | + | st, ok := fi.Sys().(*syscall.Stat_t) | |||
| 456 | + | if !ok { | |||
| 457 | + | return fmt.Errorf("jailer chroot base %s: cannot read its owner", dir) | |||
| 458 | + | } | |||
| 459 | + | // The jailer needs root, so the runner is root in production; accepting its own | |||
| 460 | + | // user as well keeps the driver testable without root. | |||
| 461 | + | if int(st.Uid) != 0 && int(st.Uid) != euid { | |||
| 462 | + | return fmt.Errorf("jailer chroot base %s is owned by uid %d, not root: chown it", dir, st.Uid) | |||
| 463 | + | } | |||
| 464 | + | return nil | |||
| 465 | + | } | |||
| 466 | + | ||||
| 393 | func (s *vmSandbox) vmConfig(spec sandbox.JobSpec, netw netpolicy.Net, f vmFiles) vmConfig { | 467 | func (s *vmSandbox) vmConfig(spec sandbox.JobSpec, netw netpolicy.Net, f vmFiles) vmConfig { | ||
| 394 | cfg := s.d.cfg | 468 | cfg := s.d.cfg | ||
| 395 | return vmConfig{ | 469 | return vmConfig{ | ||
| @@ -495,7 +569,7 @@ func (s *vmSandbox) startVMM(spec sandbox.JobSpec, apiSock string) error { | |||||
| 495 | func (s *vmSandbox) awaitVMMGone() { | 569 | func (s *vmSandbox) awaitVMMGone() { | ||
| 496 | for { | 570 | for { | ||
| 497 | alive := false | 571 | alive := false | ||
| 498 | - | for pid, id := range vmmProcs(s.d.opt.ProcDir) { | 572 | + | for pid, id := range vmmProcs(s.d.opt.ProcDir, s.d.scope()) { |
| 499 | if id == s.id && pidAlive(pid) { | 573 | if id == s.id && pidAlive(pid) { | ||
| 500 | alive = true | 574 | alive = true | ||
| 501 | } | 575 | } | ||
| @@ -554,7 +628,7 @@ func (s *vmSandbox) vmmPIDs() []int { | |||||
| 554 | if s.cmd != nil && s.cmd.Process != nil { | 628 | if s.cmd != nil && s.cmd.Process != nil { | ||
| 555 | set[s.cmd.Process.Pid] = true | 629 | set[s.cmd.Process.Pid] = true | ||
| 556 | } | 630 | } | ||
| 557 | - | for pid, id := range vmmProcs(s.d.opt.ProcDir) { | 631 | + | for pid, id := range vmmProcs(s.d.opt.ProcDir, s.d.scope()) { |
| 558 | if id == s.id { | 632 | if id == s.id { | ||
| 559 | set[pid] = true | 633 | set[pid] = true | ||
| 560 | } | 634 | } | ||
| @@ -566,6 +640,12 @@ func (s *vmSandbox) vmmPIDs() []int { | |||||
| 566 | return out | 640 | return out | ||
| 567 | } | 641 | } | ||
| 568 | 642 | ||||
| 643 | + | // stillMine is the identity check handed to the kill: pid still carries this | |||
| 644 | + | // VM's id and passes the scope (command line and root). | |||
| 645 | + | func (s *vmSandbox) stillMine(pid int) func() bool { | |||
| 646 | + | return func() bool { return cmdlineID(s.d.opt.ProcDir, pid, s.d.scope()) == s.id } | |||
| 647 | + | } | |||
| 648 | + | ||||
| 569 | // cleanupLocked removes everything the VM created, in dependency order. Steps | 649 | // cleanupLocked removes everything the VM created, in dependency order. Steps | ||
| 570 | // that need the VMM to be dead (network device, firewall rules, files, cgroup) | 650 | // that need the VMM to be dead (network device, firewall rules, files, cgroup) | ||
| 571 | // are skipped when it could not be killed: removing the TAP or the policy under | 651 | // are skipped when it could not be killed: removing the TAP or the policy under | ||
| @@ -585,11 +665,11 @@ func (s *vmSandbox) cleanupLocked(ctx context.Context) error { | |||||
| 585 | s.listener = nil | 665 | s.listener = nil | ||
| 586 | } | 666 | } | ||
| 587 | dead := true | 667 | dead := true | ||
| 588 | - | if s.cmd != nil && s.cmd.Process != nil { | 668 | + | // No kill(-pgid) of the launched command: once it has been waited for, its |
| 589 | - | _ = syscall.Kill(-s.cmd.Process.Pid, syscall.SIGKILL) // the whole group | 669 | + | // pid (and so its group id) may belong to anyone. Every process of this VM is |
| 590 | - | } | 670 | + | // found by the scan (id, markers and scope) and killed through a pidfd. |
| 591 | for _, pid := range s.vmmPIDs() { | 671 | for _, pid := range s.vmmPIDs() { | ||
| 592 | - | if !d.opt.kill(pid, 5*time.Second) { | 672 | + | if !d.opt.kill(pid, s.stillMine(pid), 5*time.Second) { |
| 593 | dead = false | 673 | dead = false | ||
| 594 | errs = append(errs, fmt.Errorf("vmm process %d did not exit", pid)) | 674 | errs = append(errs, fmt.Errorf("vmm process %d did not exit", pid)) | ||
| 595 | } | 675 | } | ||
M internal/sandbox/vm/proc.go +228 -38
| @@ -4,26 +4,196 @@ package vm | |||
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | 6 | "bytes" | |
| 7 | + | "errors" | |
| 7 | 8 | "os" | |
| 8 | 9 | "path/filepath" | |
| 10 | + | "slices" | |
| 9 | 11 | "strconv" | |
| 10 | 12 | "strings" | |
| 11 | 13 | "sync" | |
| 12 | 14 | "syscall" | |
| 13 | 15 | "time" | |
| 14 | 16 | ||
| 17 | + | "golang.org/x/sys/unix" | |
| 18 | + | ||
| 15 | 19 | "bnema.dev/ermite-runner/internal/sandbox" | |
| 16 | 20 | ) | |
| 17 | 21 | ||
| 22 | + | // Command-line markers of a VMM started for a VM. A process is one when it has | |
| 23 | + | // "--id <id>" (an id of the form sandbox.NewID produces) and at least one of | |
| 24 | + | // them. One set for the driver's scan and for the integration tests' leak check. | |
| 25 | + | const ( | |
| 26 | + | // MarkerAPISock: Firecracker started without the jailer. | |
| 27 | + | MarkerAPISock = "--api-sock" | |
| 28 | + | // MarkerExecFile: the jailer. | |
| 29 | + | MarkerExecFile = "--exec-file" | |
| 30 | + | // MarkerStartTime: the Firecracker the jailer execs into its chroot; it gets | |
| 31 | + | // neither of the first two, only --id and the jailer's --start-time-* flags. | |
| 32 | + | MarkerStartTime = "--start-time-us" | |
| 33 | + | ) | |
| 34 | + | ||
| 35 | + | // VMMMarkers lists every marker. | |
| 36 | + | var VMMMarkers = []string{MarkerAPISock, MarkerExecFile, MarkerStartTime} | |
| 37 | + | ||
| 38 | + | // vmmArgs is what the scan reads from a command line. | |
| 39 | + | type vmmArgs struct { | |
| 40 | + | id string | |
| 41 | + | markers map[string]bool | |
| 42 | + | chrootBase string // the jailer's --chroot-base-dir | |
| 43 | + | apiSock string // Firecracker's --api-sock value | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func parseVMMArgs(args []string) vmmArgs { | |
| 47 | + | a := vmmArgs{markers: map[string]bool{}} | |
| 48 | + | for i := 1; i < len(args); i++ { | |
| 49 | + | switch { | |
| 50 | + | case args[i] == "--id": | |
| 51 | + | if i+1 < len(args) && sandbox.ValidID(args[i+1]) { | |
| 52 | + | a.id = args[i+1] | |
| 53 | + | } | |
| 54 | + | case args[i] == "--chroot-base-dir": | |
| 55 | + | if i+1 < len(args) { | |
| 56 | + | a.chrootBase = args[i+1] | |
| 57 | + | } | |
| 58 | + | case slices.Contains(VMMMarkers, args[i]): | |
| 59 | + | a.markers[args[i]] = true | |
| 60 | + | if args[i] == MarkerAPISock && i+1 < len(args) { | |
| 61 | + | a.apiSock = args[i+1] | |
| 62 | + | } | |
| 63 | + | } | |
| 64 | + | } | |
| 65 | + | return a | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | // IDFromCmdline returns the VM id of a command line that carries one of | |
| 69 | + | // VMMMarkers, or "". It looks at the arguments only; the driver's scan narrows | |
| 70 | + | // it to its own state directory (scope.match). | |
| 71 | + | func IDFromCmdline(args []string) string { | |
| 72 | + | a := parseVMMArgs(args) | |
| 73 | + | if len(a.markers) == 0 { | |
| 74 | + | return "" | |
| 75 | + | } | |
| 76 | + | return a.id | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | // scope says which VMM processes are this driver's: the jailed ones must live in | |
| 80 | + | // (or have been started for) its own jailer chroot base, <state dir>/jail. | |
| 81 | + | type scope struct { | |
| 82 | + | jailBases []string // the base as configured, and with symlinks resolved | |
| 83 | + | vmBases []string // <state dir>/vm (unjailed VMs), likewise | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | func newScope(stateDir string) scope { | |
| 87 | + | sc := scope{} | |
| 88 | + | for _, b := range []struct { | |
| 89 | + | dir string | |
| 90 | + | to *[]string | |
| 91 | + | }{{jailBase(stateDir), &sc.jailBases}, {filepath.Join(stateDir, "vm"), &sc.vmBases}} { | |
| 92 | + | *b.to = []string{b.dir} | |
| 93 | + | if r, err := filepath.EvalSymlinks(b.dir); err == nil && r != b.dir { | |
| 94 | + | *b.to = append(*b.to, r) | |
| 95 | + | } | |
| 96 | + | } | |
| 97 | + | return sc | |
| 98 | + | } | |
| 99 | + | ||
| 100 | + | // apiSockIsOurs: an unjailed Firecracker's --api-sock must be a path below | |
| 101 | + | // <state dir>/vm/<id>/ (compared lexically, after cleaning), the directory the | |
| 102 | + | // driver puts it in. Another Firecracker with an id of our shape and a socket | |
| 103 | + | // elsewhere is someone else's. | |
| 104 | + | func (sc scope) apiSockIsOurs(sock, id string) bool { | |
| 105 | + | if sock == "" || !filepath.IsAbs(sock) { | |
| 106 | + | return false | |
| 107 | + | } | |
| 108 | + | sock = filepath.Clean(sock) | |
| 109 | + | for _, b := range sc.vmBases { | |
| 110 | + | if strings.HasPrefix(sock, filepath.Join(b, id)+"/") { | |
| 111 | + | return true | |
| 112 | + | } | |
| 113 | + | } | |
| 114 | + | return false | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | func (sc scope) isBase(p string) bool { | |
| 118 | + | return slices.Contains(sc.jailBases, filepath.Clean(p)) | |
| 119 | + | } | |
| 120 | + | ||
| 121 | + | func (sc scope) under(p string) bool { | |
| 122 | + | for _, b := range sc.jailBases { | |
| 123 | + | if strings.HasPrefix(filepath.Clean(p), b+"/") { | |
| 124 | + | return true | |
| 125 | + | } | |
| 126 | + | } | |
| 127 | + | return false | |
| 128 | + | } | |
| 129 | + | ||
| 130 | + | // match returns the VM id of the process whose command line is args and whose | |
| 131 | + | // /proc directory is dir, or "". Besides IDFromCmdline: | |
| 132 | + | // - a jailer (--exec-file) must have been given our --chroot-base-dir; | |
| 133 | + | // - a Firecracker that carries only the jailer's --start-time-us (no | |
| 134 | + | // --api-sock) is the jailed VMM: its root must be the chroot of that id | |
| 135 | + | // below our jail base (rootIsOurs). Anything else with that flag and an id | |
| 136 | + | // of our shape belongs to someone else's jailer and is never touched. | |
| 137 | + | // | |
| 138 | + | // A Firecracker started with --api-sock (no jailer) is ours when the socket is | |
| 139 | + | // in <state dir>/vm/<id>/ (apiSockIsOurs). | |
| 140 | + | func (sc scope) match(dir string, args []string) string { | |
| 141 | + | a := parseVMMArgs(args) | |
| 142 | + | if a.id == "" || len(a.markers) == 0 { | |
| 143 | + | return "" | |
| 144 | + | } | |
| 145 | + | switch { | |
| 146 | + | case a.markers[MarkerAPISock]: | |
| 147 | + | if sc.apiSockIsOurs(a.apiSock, a.id) { | |
| 148 | + | return a.id | |
| 149 | + | } | |
| 150 | + | case a.markers[MarkerExecFile]: | |
| 151 | + | if sc.isBase(a.chrootBase) { | |
| 152 | + | return a.id | |
| 153 | + | } | |
| 154 | + | case a.markers[MarkerStartTime]: | |
| 155 | + | if sc.rootIsOurs(dir, a.id) { | |
| 156 | + | return a.id | |
| 157 | + | } | |
| 158 | + | } | |
| 159 | + | return "" | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | // rootIsOurs reports whether the process's root directory is the chroot the | |
| 163 | + | // jailer made for id below our jail base. The jailer pivot_roots into a new | |
| 164 | + | // mount namespace, so readlink(/proc/<pid>/root) says "/" and cannot be used: | |
| 165 | + | // stat follows the magic link into the VM's root, which is the very directory | |
| 166 | + | // <base>/<exec name>/<id>/root of the host (same device and inode). A root | |
| 167 | + | // whose path is lexically below our base is accepted too. | |
| 168 | + | func (sc scope) rootIsOurs(dir, id string) bool { | |
| 169 | + | if root, err := os.Readlink(filepath.Join(dir, "root")); err == nil && sc.under(root) { | |
| 170 | + | return true | |
| 171 | + | } | |
| 172 | + | rootFI, err := os.Stat(filepath.Join(dir, "root")) | |
| 173 | + | if err != nil { | |
| 174 | + | return false | |
| 175 | + | } | |
| 176 | + | for _, b := range sc.jailBases { | |
| 177 | + | execDirs, _ := os.ReadDir(b) | |
| 178 | + | for _, e := range execDirs { | |
| 179 | + | if fi, err := os.Stat(filepath.Join(b, e.Name(), id, "root")); err == nil && os.SameFile(fi, rootFI) { | |
| 180 | + | return true | |
| 181 | + | } | |
| 182 | + | } | |
| 183 | + | } | |
| 184 | + | return false | |
| 185 | + | } | |
| 186 | + | ||
| 18 | 187 | // vmmProcs scans a /proc tree for processes started by this runner for a VM: | |
| 19 | 188 | // the Firecracker process and the jailer. A process is ours when its command | |
| 20 | - | // line has "--id <id>" with an id of the form sandbox.NewID produces and also | |
| 21 | - | // "--api-sock" (Firecracker) or "--exec-file" (jailer). The program name is not | |
| 22 | - | // looked at: the binary may be renamed or versioned (firecracker-v1.17), and a | |
| 23 | - | // jailer in a PID namespace leaves a Firecracker whose argv[0] is whatever the | |
| 24 | - | // jailer chose. Processes of other users of Firecracker (other ids) are never | |
| 25 | - | // matched. It finds orphans even when the state directory was lost. | |
| 26 | - | func vmmProcs(procDir string) map[int]string { | |
| 189 | + | // line has "--id <id>" with an id of the form sandbox.NewID produces and one of | |
| 190 | + | // the VMMMarkers, and it passes scope.match. The program name is not looked at: | |
| 191 | + | // the binary may be renamed or versioned (firecracker-v1.17), and a jailer in a | |
| 192 | + | // PID namespace leaves a Firecracker whose argv[0] is whatever the jailer | |
| 193 | + | // chose. Processes of other users of Firecracker (other ids, other jail bases) | |
| 194 | + | // are never matched. It finds orphans even when the state directory's records | |
| 195 | + | // were lost. | |
| 196 | + | func vmmProcs(procDir string, sc scope) map[int]string { | |
| 27 | 197 | out := map[int]string{} | |
| 28 | 198 | ents, err := os.ReadDir(procDir) | |
| 29 | 199 | if err != nil { | |
| @@ -34,37 +204,18 @@ func vmmProcs(procDir string) map[int]string { | |||
| 34 | 204 | if err != nil || pid <= 1 { | |
| 35 | 205 | continue | |
| 36 | 206 | } | |
| 37 | - | b, err := os.ReadFile(filepath.Join(procDir, e.Name(), "cmdline")) | |
| 207 | + | dir := filepath.Join(procDir, e.Name()) | |
| 208 | + | b, err := os.ReadFile(filepath.Join(dir, "cmdline")) | |
| 38 | 209 | if err != nil || len(b) == 0 { | |
| 39 | 210 | continue | |
| 40 | 211 | } | |
| 41 | - | if id := vmIDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")); id != "" { | |
| 212 | + | if id := sc.match(dir, strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")); id != "" { | |
| 42 | 213 | out[pid] = id | |
| 43 | 214 | } | |
| 44 | 215 | } | |
| 45 | 216 | return out | |
| 46 | 217 | } | |
| 47 | 218 | ||
| 48 | - | // vmIDFromCmdline returns the VM id of a command line, or "". | |
| 49 | - | func vmIDFromCmdline(args []string) string { | |
| 50 | - | var id string | |
| 51 | - | var marker bool | |
| 52 | - | for i := 1; i < len(args); i++ { | |
| 53 | - | switch args[i] { | |
| 54 | - | case "--id": | |
| 55 | - | if i+1 < len(args) && sandbox.ValidID(args[i+1]) { | |
| 56 | - | id = args[i+1] | |
| 57 | - | } | |
| 58 | - | case "--api-sock", "--exec-file": | |
| 59 | - | marker = true | |
| 60 | - | } | |
| 61 | - | } | |
| 62 | - | if marker { | |
| 63 | - | return id | |
| 64 | - | } | |
| 65 | - | return "" | |
| 66 | - | } | |
| 67 | - | ||
| 68 | 219 | // pidAlive reports whether pid exists and is not a zombie. | |
| 69 | 220 | func pidAlive(pid int) bool { | |
| 70 | 221 | if err := syscall.Kill(pid, 0); err != nil { | |
| @@ -105,18 +256,57 @@ func (r *ring) String() string { | |||
| 105 | 256 | return string(bytes.TrimSpace(r.buf)) | |
| 106 | 257 | } | |
| 107 | 258 | ||
| 108 | - | // killAndWait sends SIGKILL to pid and waits until it is gone (or a zombie that | |
| 109 | - | // its parent will reap). It reports whether the process is gone. | |
| 110 | - | func killAndWait(pid int, timeout time.Duration) bool { | |
| 111 | - | _ = syscall.Kill(pid, syscall.SIGKILL) | |
| 259 | + | // killAndWait kills the process that was identified as pid, through a pidfd, and | |
| 260 | + | // waits for it to exit. It reports whether that process is gone. | |
| 261 | + | // | |
| 262 | + | // A pid read from /proc a moment ago may belong to another process by now (the | |
| 263 | + | // VMM exited and the number was recycled). The pidfd pins the process; then | |
| 264 | + | // still() re-runs the identity check (command line and root scope) on the pid, | |
| 265 | + | // and only if the pidfd is still not readable (the process had not exited at | |
| 266 | + | // that point, so the pid could not have been recycled while still() read it) the | |
| 267 | + | // signal goes out through the pidfd, which can only reach that process. A | |
| 268 | + | // mismatch means the process we meant is gone: nothing is signalled and true is | |
| 269 | + | // returned. Where pidfds are not available the process is not touched and false | |
| 270 | + | // is returned (the sandbox is then reported as leaked, never killed blindly). | |
| 271 | + | func killAndWait(pid int, still func() bool, timeout time.Duration) bool { | |
| 272 | + | fd, err := unix.PidfdOpen(pid, 0) | |
| 273 | + | if err != nil { | |
| 274 | + | return errors.Is(err, unix.ESRCH) // no such process: gone | |
| 275 | + | } | |
| 276 | + | defer func() { _ = unix.Close(fd) }() | |
| 277 | + | ours := still() | |
| 278 | + | if exited, _ := pidfdWait(fd, 0); exited { | |
| 279 | + | return true // it exited before or while we checked: nothing to kill | |
| 280 | + | } | |
| 281 | + | if !ours { | |
| 282 | + | return true // alive, but not the process we meant | |
| 283 | + | } | |
| 284 | + | if err := unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0); err != nil && !errors.Is(err, unix.ESRCH) { | |
| 285 | + | return false | |
| 286 | + | } | |
| 287 | + | exited, _ := pidfdWait(fd, timeout) | |
| 288 | + | return exited | |
| 289 | + | } | |
| 290 | + | ||
| 291 | + | // pidfdWait waits up to timeout for the process behind fd to exit (a pidfd is | |
| 292 | + | // readable once it has, zombie included). | |
| 293 | + | func pidfdWait(fd int, timeout time.Duration) (bool, error) { | |
| 112 | 294 | deadline := time.Now().Add(timeout) | |
| 113 | 295 | for { | |
| 114 | - | if !pidAlive(pid) { | |
| 115 | - | return true | |
| 296 | + | left := time.Until(deadline) | |
| 297 | + | if left < 0 { | |
| 298 | + | left = 0 | |
| 116 | 299 | } | |
| 117 | - | if time.Now().After(deadline) { | |
| 118 | - | return false | |
| 300 | + | fds := []unix.PollFd{{Fd: int32(fd), Events: unix.POLLIN}} | |
| 301 | + | n, err := unix.Poll(fds, int(left.Milliseconds())) | |
| 302 | + | switch { | |
| 303 | + | case errors.Is(err, unix.EINTR): | |
| 304 | + | continue | |
| 305 | + | case err != nil: | |
| 306 | + | return false, err | |
| 307 | + | case n > 0: | |
| 308 | + | return true, nil | |
| 119 | 309 | } | |
| 120 | - | time.Sleep(20 * time.Millisecond) | |
| 310 | + | return false, nil | |
| 121 | 311 | } | |
| 122 | 312 | } | |
| @@ -4,26 +4,196 @@ package vm | |||||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | "bytes" | 6 | "bytes" | ||
| 7 | + | "errors" | |||
| 7 | "os" | 8 | "os" | ||
| 8 | "path/filepath" | 9 | "path/filepath" | ||
| 10 | + | "slices" | |||
| 9 | "strconv" | 11 | "strconv" | ||
| 10 | "strings" | 12 | "strings" | ||
| 11 | "sync" | 13 | "sync" | ||
| 12 | "syscall" | 14 | "syscall" | ||
| 13 | "time" | 15 | "time" | ||
| 14 | 16 | ||||
| 17 | + | "golang.org/x/sys/unix" | |||
| 18 | + | ||||
| 15 | "bnema.dev/ermite-runner/internal/sandbox" | 19 | "bnema.dev/ermite-runner/internal/sandbox" | ||
| 16 | ) | 20 | ) | ||
| 17 | 21 | ||||
| 22 | + | // Command-line markers of a VMM started for a VM. A process is one when it has | |||
| 23 | + | // "--id <id>" (an id of the form sandbox.NewID produces) and at least one of | |||
| 24 | + | // them. One set for the driver's scan and for the integration tests' leak check. | |||
| 25 | + | const ( | |||
| 26 | + | // MarkerAPISock: Firecracker started without the jailer. | |||
| 27 | + | MarkerAPISock = "--api-sock" | |||
| 28 | + | // MarkerExecFile: the jailer. | |||
| 29 | + | MarkerExecFile = "--exec-file" | |||
| 30 | + | // MarkerStartTime: the Firecracker the jailer execs into its chroot; it gets | |||
| 31 | + | // neither of the first two, only --id and the jailer's --start-time-* flags. | |||
| 32 | + | MarkerStartTime = "--start-time-us" | |||
| 33 | + | ) | |||
| 34 | + | ||||
| 35 | + | // VMMMarkers lists every marker. | |||
| 36 | + | var VMMMarkers = []string{MarkerAPISock, MarkerExecFile, MarkerStartTime} | |||
| 37 | + | ||||
| 38 | + | // vmmArgs is what the scan reads from a command line. | |||
| 39 | + | type vmmArgs struct { | |||
| 40 | + | id string | |||
| 41 | + | markers map[string]bool | |||
| 42 | + | chrootBase string // the jailer's --chroot-base-dir | |||
| 43 | + | apiSock string // Firecracker's --api-sock value | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func parseVMMArgs(args []string) vmmArgs { | |||
| 47 | + | a := vmmArgs{markers: map[string]bool{}} | |||
| 48 | + | for i := 1; i < len(args); i++ { | |||
| 49 | + | switch { | |||
| 50 | + | case args[i] == "--id": | |||
| 51 | + | if i+1 < len(args) && sandbox.ValidID(args[i+1]) { | |||
| 52 | + | a.id = args[i+1] | |||
| 53 | + | } | |||
| 54 | + | case args[i] == "--chroot-base-dir": | |||
| 55 | + | if i+1 < len(args) { | |||
| 56 | + | a.chrootBase = args[i+1] | |||
| 57 | + | } | |||
| 58 | + | case slices.Contains(VMMMarkers, args[i]): | |||
| 59 | + | a.markers[args[i]] = true | |||
| 60 | + | if args[i] == MarkerAPISock && i+1 < len(args) { | |||
| 61 | + | a.apiSock = args[i+1] | |||
| 62 | + | } | |||
| 63 | + | } | |||
| 64 | + | } | |||
| 65 | + | return a | |||
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | // IDFromCmdline returns the VM id of a command line that carries one of | |||
| 69 | + | // VMMMarkers, or "". It looks at the arguments only; the driver's scan narrows | |||
| 70 | + | // it to its own state directory (scope.match). | |||
| 71 | + | func IDFromCmdline(args []string) string { | |||
| 72 | + | a := parseVMMArgs(args) | |||
| 73 | + | if len(a.markers) == 0 { | |||
| 74 | + | return "" | |||
| 75 | + | } | |||
| 76 | + | return a.id | |||
| 77 | + | } | |||
| 78 | + | ||||
| 79 | + | // scope says which VMM processes are this driver's: the jailed ones must live in | |||
| 80 | + | // (or have been started for) its own jailer chroot base, <state dir>/jail. | |||
| 81 | + | type scope struct { | |||
| 82 | + | jailBases []string // the base as configured, and with symlinks resolved | |||
| 83 | + | vmBases []string // <state dir>/vm (unjailed VMs), likewise | |||
| 84 | + | } | |||
| 85 | + | ||||
| 86 | + | func newScope(stateDir string) scope { | |||
| 87 | + | sc := scope{} | |||
| 88 | + | for _, b := range []struct { | |||
| 89 | + | dir string | |||
| 90 | + | to *[]string | |||
| 91 | + | }{{jailBase(stateDir), &sc.jailBases}, {filepath.Join(stateDir, "vm"), &sc.vmBases}} { | |||
| 92 | + | *b.to = []string{b.dir} | |||
| 93 | + | if r, err := filepath.EvalSymlinks(b.dir); err == nil && r != b.dir { | |||
| 94 | + | *b.to = append(*b.to, r) | |||
| 95 | + | } | |||
| 96 | + | } | |||
| 97 | + | return sc | |||
| 98 | + | } | |||
| 99 | + | ||||
| 100 | + | // apiSockIsOurs: an unjailed Firecracker's --api-sock must be a path below | |||
| 101 | + | // <state dir>/vm/<id>/ (compared lexically, after cleaning), the directory the | |||
| 102 | + | // driver puts it in. Another Firecracker with an id of our shape and a socket | |||
| 103 | + | // elsewhere is someone else's. | |||
| 104 | + | func (sc scope) apiSockIsOurs(sock, id string) bool { | |||
| 105 | + | if sock == "" || !filepath.IsAbs(sock) { | |||
| 106 | + | return false | |||
| 107 | + | } | |||
| 108 | + | sock = filepath.Clean(sock) | |||
| 109 | + | for _, b := range sc.vmBases { | |||
| 110 | + | if strings.HasPrefix(sock, filepath.Join(b, id)+"/") { | |||
| 111 | + | return true | |||
| 112 | + | } | |||
| 113 | + | } | |||
| 114 | + | return false | |||
| 115 | + | } | |||
| 116 | + | ||||
| 117 | + | func (sc scope) isBase(p string) bool { | |||
| 118 | + | return slices.Contains(sc.jailBases, filepath.Clean(p)) | |||
| 119 | + | } | |||
| 120 | + | ||||
| 121 | + | func (sc scope) under(p string) bool { | |||
| 122 | + | for _, b := range sc.jailBases { | |||
| 123 | + | if strings.HasPrefix(filepath.Clean(p), b+"/") { | |||
| 124 | + | return true | |||
| 125 | + | } | |||
| 126 | + | } | |||
| 127 | + | return false | |||
| 128 | + | } | |||
| 129 | + | ||||
| 130 | + | // match returns the VM id of the process whose command line is args and whose | |||
| 131 | + | // /proc directory is dir, or "". Besides IDFromCmdline: | |||
| 132 | + | // - a jailer (--exec-file) must have been given our --chroot-base-dir; | |||
| 133 | + | // - a Firecracker that carries only the jailer's --start-time-us (no | |||
| 134 | + | // --api-sock) is the jailed VMM: its root must be the chroot of that id | |||
| 135 | + | // below our jail base (rootIsOurs). Anything else with that flag and an id | |||
| 136 | + | // of our shape belongs to someone else's jailer and is never touched. | |||
| 137 | + | // | |||
| 138 | + | // A Firecracker started with --api-sock (no jailer) is ours when the socket is | |||
| 139 | + | // in <state dir>/vm/<id>/ (apiSockIsOurs). | |||
| 140 | + | func (sc scope) match(dir string, args []string) string { | |||
| 141 | + | a := parseVMMArgs(args) | |||
| 142 | + | if a.id == "" || len(a.markers) == 0 { | |||
| 143 | + | return "" | |||
| 144 | + | } | |||
| 145 | + | switch { | |||
| 146 | + | case a.markers[MarkerAPISock]: | |||
| 147 | + | if sc.apiSockIsOurs(a.apiSock, a.id) { | |||
| 148 | + | return a.id | |||
| 149 | + | } | |||
| 150 | + | case a.markers[MarkerExecFile]: | |||
| 151 | + | if sc.isBase(a.chrootBase) { | |||
| 152 | + | return a.id | |||
| 153 | + | } | |||
| 154 | + | case a.markers[MarkerStartTime]: | |||
| 155 | + | if sc.rootIsOurs(dir, a.id) { | |||
| 156 | + | return a.id | |||
| 157 | + | } | |||
| 158 | + | } | |||
| 159 | + | return "" | |||
| 160 | + | } | |||
| 161 | + | ||||
| 162 | + | // rootIsOurs reports whether the process's root directory is the chroot the | |||
| 163 | + | // jailer made for id below our jail base. The jailer pivot_roots into a new | |||
| 164 | + | // mount namespace, so readlink(/proc/<pid>/root) says "/" and cannot be used: | |||
| 165 | + | // stat follows the magic link into the VM's root, which is the very directory | |||
| 166 | + | // <base>/<exec name>/<id>/root of the host (same device and inode). A root | |||
| 167 | + | // whose path is lexically below our base is accepted too. | |||
| 168 | + | func (sc scope) rootIsOurs(dir, id string) bool { | |||
| 169 | + | if root, err := os.Readlink(filepath.Join(dir, "root")); err == nil && sc.under(root) { | |||
| 170 | + | return true | |||
| 171 | + | } | |||
| 172 | + | rootFI, err := os.Stat(filepath.Join(dir, "root")) | |||
| 173 | + | if err != nil { | |||
| 174 | + | return false | |||
| 175 | + | } | |||
| 176 | + | for _, b := range sc.jailBases { | |||
| 177 | + | execDirs, _ := os.ReadDir(b) | |||
| 178 | + | for _, e := range execDirs { | |||
| 179 | + | if fi, err := os.Stat(filepath.Join(b, e.Name(), id, "root")); err == nil && os.SameFile(fi, rootFI) { | |||
| 180 | + | return true | |||
| 181 | + | } | |||
| 182 | + | } | |||
| 183 | + | } | |||
| 184 | + | return false | |||
| 185 | + | } | |||
| 186 | + | ||||
| 18 | // vmmProcs scans a /proc tree for processes started by this runner for a VM: | 187 | // vmmProcs scans a /proc tree for processes started by this runner for a VM: | ||
| 19 | // the Firecracker process and the jailer. A process is ours when its command | 188 | // the Firecracker process and the jailer. A process is ours when its command | ||
| 20 | - | // line has "--id <id>" with an id of the form sandbox.NewID produces and also | 189 | + | // line has "--id <id>" with an id of the form sandbox.NewID produces and one of |
| 21 | - | // "--api-sock" (Firecracker) or "--exec-file" (jailer). The program name is not | 190 | + | // the VMMMarkers, and it passes scope.match. The program name is not looked at: |
| 22 | - | // looked at: the binary may be renamed or versioned (firecracker-v1.17), and a | 191 | + | // the binary may be renamed or versioned (firecracker-v1.17), and a jailer in a |
| 23 | - | // jailer in a PID namespace leaves a Firecracker whose argv[0] is whatever the | 192 | + | // PID namespace leaves a Firecracker whose argv[0] is whatever the jailer |
| 24 | - | // jailer chose. Processes of other users of Firecracker (other ids) are never | 193 | + | // chose. Processes of other users of Firecracker (other ids, other jail bases) |
| 25 | - | // matched. It finds orphans even when the state directory was lost. | 194 | + | // are never matched. It finds orphans even when the state directory's records |
| 26 | - | func vmmProcs(procDir string) map[int]string { | 195 | + | // were lost. |
| 196 | + | func vmmProcs(procDir string, sc scope) map[int]string { | |||
| 27 | out := map[int]string{} | 197 | out := map[int]string{} | ||
| 28 | ents, err := os.ReadDir(procDir) | 198 | ents, err := os.ReadDir(procDir) | ||
| 29 | if err != nil { | 199 | if err != nil { | ||
| @@ -34,37 +204,18 @@ func vmmProcs(procDir string) map[int]string { | |||||
| 34 | if err != nil || pid <= 1 { | 204 | if err != nil || pid <= 1 { | ||
| 35 | continue | 205 | continue | ||
| 36 | } | 206 | } | ||
| 37 | - | b, err := os.ReadFile(filepath.Join(procDir, e.Name(), "cmdline")) | 207 | + | dir := filepath.Join(procDir, e.Name()) |
| 208 | + | b, err := os.ReadFile(filepath.Join(dir, "cmdline")) | |||
| 38 | if err != nil || len(b) == 0 { | 209 | if err != nil || len(b) == 0 { | ||
| 39 | continue | 210 | continue | ||
| 40 | } | 211 | } | ||
| 41 | - | if id := vmIDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")); id != "" { | 212 | + | if id := sc.match(dir, strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")); id != "" { |
| 42 | out[pid] = id | 213 | out[pid] = id | ||
| 43 | } | 214 | } | ||
| 44 | } | 215 | } | ||
| 45 | return out | 216 | return out | ||
| 46 | } | 217 | } | ||
| 47 | 218 | ||||
| 48 | - | // vmIDFromCmdline returns the VM id of a command line, or "". | |||
| 49 | - | func vmIDFromCmdline(args []string) string { | |||
| 50 | - | var id string | |||
| 51 | - | var marker bool | |||
| 52 | - | for i := 1; i < len(args); i++ { | |||
| 53 | - | switch args[i] { | |||
| 54 | - | case "--id": | |||
| 55 | - | if i+1 < len(args) && sandbox.ValidID(args[i+1]) { | |||
| 56 | - | id = args[i+1] | |||
| 57 | - | } | |||
| 58 | - | case "--api-sock", "--exec-file": | |||
| 59 | - | marker = true | |||
| 60 | - | } | |||
| 61 | - | } | |||
| 62 | - | if marker { | |||
| 63 | - | return id | |||
| 64 | - | } | |||
| 65 | - | return "" | |||
| 66 | - | } | |||
| 67 | - | ||||
| 68 | // pidAlive reports whether pid exists and is not a zombie. | 219 | // pidAlive reports whether pid exists and is not a zombie. | ||
| 69 | func pidAlive(pid int) bool { | 220 | func pidAlive(pid int) bool { | ||
| 70 | if err := syscall.Kill(pid, 0); err != nil { | 221 | if err := syscall.Kill(pid, 0); err != nil { | ||
| @@ -105,18 +256,57 @@ func (r *ring) String() string { | |||||
| 105 | return string(bytes.TrimSpace(r.buf)) | 256 | return string(bytes.TrimSpace(r.buf)) | ||
| 106 | } | 257 | } | ||
| 107 | 258 | ||||
| 108 | - | // killAndWait sends SIGKILL to pid and waits until it is gone (or a zombie that | 259 | + | // killAndWait kills the process that was identified as pid, through a pidfd, and |
| 109 | - | // its parent will reap). It reports whether the process is gone. | 260 | + | // waits for it to exit. It reports whether that process is gone. |
| 110 | - | func killAndWait(pid int, timeout time.Duration) bool { | 261 | + | // |
| 111 | - | _ = syscall.Kill(pid, syscall.SIGKILL) | 262 | + | // A pid read from /proc a moment ago may belong to another process by now (the |
| 263 | + | // VMM exited and the number was recycled). The pidfd pins the process; then | |||
| 264 | + | // still() re-runs the identity check (command line and root scope) on the pid, | |||
| 265 | + | // and only if the pidfd is still not readable (the process had not exited at | |||
| 266 | + | // that point, so the pid could not have been recycled while still() read it) the | |||
| 267 | + | // signal goes out through the pidfd, which can only reach that process. A | |||
| 268 | + | // mismatch means the process we meant is gone: nothing is signalled and true is | |||
| 269 | + | // returned. Where pidfds are not available the process is not touched and false | |||
| 270 | + | // is returned (the sandbox is then reported as leaked, never killed blindly). | |||
| 271 | + | func killAndWait(pid int, still func() bool, timeout time.Duration) bool { | |||
| 272 | + | fd, err := unix.PidfdOpen(pid, 0) | |||
| 273 | + | if err != nil { | |||
| 274 | + | return errors.Is(err, unix.ESRCH) // no such process: gone | |||
| 275 | + | } | |||
| 276 | + | defer func() { _ = unix.Close(fd) }() | |||
| 277 | + | ours := still() | |||
| 278 | + | if exited, _ := pidfdWait(fd, 0); exited { | |||
| 279 | + | return true // it exited before or while we checked: nothing to kill | |||
| 280 | + | } | |||
| 281 | + | if !ours { | |||
| 282 | + | return true // alive, but not the process we meant | |||
| 283 | + | } | |||
| 284 | + | if err := unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0); err != nil && !errors.Is(err, unix.ESRCH) { | |||
| 285 | + | return false | |||
| 286 | + | } | |||
| 287 | + | exited, _ := pidfdWait(fd, timeout) | |||
| 288 | + | return exited | |||
| 289 | + | } | |||
| 290 | + | ||||
| 291 | + | // pidfdWait waits up to timeout for the process behind fd to exit (a pidfd is | |||
| 292 | + | // readable once it has, zombie included). | |||
| 293 | + | func pidfdWait(fd int, timeout time.Duration) (bool, error) { | |||
| 112 | deadline := time.Now().Add(timeout) | 294 | deadline := time.Now().Add(timeout) | ||
| 113 | for { | 295 | for { | ||
| 114 | - | if !pidAlive(pid) { | 296 | + | left := time.Until(deadline) |
| 115 | - | return true | 297 | + | if left < 0 { |
| 298 | + | left = 0 | |||
| 116 | } | 299 | } | ||
| 117 | - | if time.Now().After(deadline) { | 300 | + | fds := []unix.PollFd{{Fd: int32(fd), Events: unix.POLLIN}} |
| 118 | - | return false | 301 | + | n, err := unix.Poll(fds, int(left.Milliseconds())) |
| 302 | + | switch { | |||
| 303 | + | case errors.Is(err, unix.EINTR): | |||
| 304 | + | continue | |||
| 305 | + | case err != nil: | |||
| 306 | + | return false, err | |||
| 307 | + | case n > 0: | |||
| 308 | + | return true, nil | |||
| 119 | } | 309 | } | ||
| 120 | - | time.Sleep(20 * time.Millisecond) | 310 | + | return false, nil |
| 121 | } | 311 | } | ||
| 122 | } | 312 | } | ||
M internal/sandbox/vm/reap.go +7 -6
| @@ -39,7 +39,7 @@ func (d *Driver) Reap(ctx context.Context) error { | |||
| 39 | 39 | if err != nil { | |
| 40 | 40 | errs = append(errs, err) | |
| 41 | 41 | } | |
| 42 | - | procs := vmmProcs(d.opt.ProcDir) | |
| 42 | + | procs := vmmProcs(d.opt.ProcDir, d.scope()) | |
| 43 | 43 | killed := map[int]bool{} | |
| 44 | 44 | killProc := func(pid int, id string) { | |
| 45 | 45 | if killed[pid] { | |
| @@ -47,7 +47,7 @@ func (d *Driver) Reap(ctx context.Context) error { | |||
| 47 | 47 | } | |
| 48 | 48 | killed[pid] = true | |
| 49 | 49 | log.Warn().Int("pid", pid).Str("vm", id).Msg("reaping leftover VMM process") | |
| 50 | - | if !d.opt.kill(pid, 5*time.Second) { | |
| 50 | + | if !d.opt.kill(pid, func() bool { return cmdlineID(d.opt.ProcDir, pid, d.scope()) == id }, 5*time.Second) { | |
| 51 | 51 | errs = append(errs, fmt.Errorf("vmm process %d (%s) did not exit", pid, id)) | |
| 52 | 52 | } | |
| 53 | 53 | } | |
| @@ -55,7 +55,7 @@ func (d *Driver) Reap(ctx context.Context) error { | |||
| 55 | 55 | // carries this record's id (pids are reused); then every other process of | |
| 56 | 56 | // ours found in /proc, which also covers a lost state directory. | |
| 57 | 57 | for _, r := range recs { | |
| 58 | - | if r.PID > 0 && cmdlineID(d.opt.ProcDir, r.PID) == r.ID { | |
| 58 | + | if r.PID > 0 && cmdlineID(d.opt.ProcDir, r.PID, d.scope()) == r.ID { | |
| 59 | 59 | killProc(r.PID, r.ID) | |
| 60 | 60 | } | |
| 61 | 61 | } | |
| @@ -124,10 +124,11 @@ func (d *Driver) Reap(ctx context.Context) error { | |||
| 124 | 124 | ||
| 125 | 125 | // cmdlineID returns the VM id in the command line of pid, or "" (not running, | |
| 126 | 126 | // not ours, or recycled for another program). | |
| 127 | - | func cmdlineID(procDir string, pid int) string { | |
| 128 | - | b, err := os.ReadFile(filepath.Join(procDir, strconv.Itoa(pid), "cmdline")) | |
| 127 | + | func cmdlineID(procDir string, pid int, sc scope) string { | |
| 128 | + | dir := filepath.Join(procDir, strconv.Itoa(pid)) | |
| 129 | + | b, err := os.ReadFile(filepath.Join(dir, "cmdline")) | |
| 129 | 130 | if err != nil || len(b) == 0 { | |
| 130 | 131 | return "" | |
| 131 | 132 | } | |
| 132 | - | return vmIDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")) | |
| 133 | + | return sc.match(dir, strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")) | |
| 133 | 134 | } | |
| @@ -39,7 +39,7 @@ func (d *Driver) Reap(ctx context.Context) error { | |||||
| 39 | if err != nil { | 39 | if err != nil { | ||
| 40 | errs = append(errs, err) | 40 | errs = append(errs, err) | ||
| 41 | } | 41 | } | ||
| 42 | - | procs := vmmProcs(d.opt.ProcDir) | 42 | + | procs := vmmProcs(d.opt.ProcDir, d.scope()) |
| 43 | killed := map[int]bool{} | 43 | killed := map[int]bool{} | ||
| 44 | killProc := func(pid int, id string) { | 44 | killProc := func(pid int, id string) { | ||
| 45 | if killed[pid] { | 45 | if killed[pid] { | ||
| @@ -47,7 +47,7 @@ func (d *Driver) Reap(ctx context.Context) error { | |||||
| 47 | } | 47 | } | ||
| 48 | killed[pid] = true | 48 | killed[pid] = true | ||
| 49 | log.Warn().Int("pid", pid).Str("vm", id).Msg("reaping leftover VMM process") | 49 | log.Warn().Int("pid", pid).Str("vm", id).Msg("reaping leftover VMM process") | ||
| 50 | - | if !d.opt.kill(pid, 5*time.Second) { | 50 | + | if !d.opt.kill(pid, func() bool { return cmdlineID(d.opt.ProcDir, pid, d.scope()) == id }, 5*time.Second) { |
| 51 | errs = append(errs, fmt.Errorf("vmm process %d (%s) did not exit", pid, id)) | 51 | errs = append(errs, fmt.Errorf("vmm process %d (%s) did not exit", pid, id)) | ||
| 52 | } | 52 | } | ||
| 53 | } | 53 | } | ||
| @@ -55,7 +55,7 @@ func (d *Driver) Reap(ctx context.Context) error { | |||||
| 55 | // carries this record's id (pids are reused); then every other process of | 55 | // carries this record's id (pids are reused); then every other process of | ||
| 56 | // ours found in /proc, which also covers a lost state directory. | 56 | // ours found in /proc, which also covers a lost state directory. | ||
| 57 | for _, r := range recs { | 57 | for _, r := range recs { | ||
| 58 | - | if r.PID > 0 && cmdlineID(d.opt.ProcDir, r.PID) == r.ID { | 58 | + | if r.PID > 0 && cmdlineID(d.opt.ProcDir, r.PID, d.scope()) == r.ID { |
| 59 | killProc(r.PID, r.ID) | 59 | killProc(r.PID, r.ID) | ||
| 60 | } | 60 | } | ||
| 61 | } | 61 | } | ||
| @@ -124,10 +124,11 @@ func (d *Driver) Reap(ctx context.Context) error { | |||||
| 124 | 124 | ||||
| 125 | // cmdlineID returns the VM id in the command line of pid, or "" (not running, | 125 | // cmdlineID returns the VM id in the command line of pid, or "" (not running, | ||
| 126 | // not ours, or recycled for another program). | 126 | // not ours, or recycled for another program). | ||
| 127 | - | func cmdlineID(procDir string, pid int) string { | 127 | + | func cmdlineID(procDir string, pid int, sc scope) string { |
| 128 | - | b, err := os.ReadFile(filepath.Join(procDir, strconv.Itoa(pid), "cmdline")) | 128 | + | dir := filepath.Join(procDir, strconv.Itoa(pid)) |
| 129 | + | b, err := os.ReadFile(filepath.Join(dir, "cmdline")) | |||
| 129 | if err != nil || len(b) == 0 { | 130 | if err != nil || len(b) == 0 { | ||
| 130 | return "" | 131 | return "" | ||
| 131 | } | 132 | } | ||
| 132 | - | return vmIDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")) | 133 | + | return sc.match(dir, strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")) |
| 133 | } | 134 | } | ||
M internal/sandbox/vm/vm_test.go +300 -15
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "errors" | |
| 10 | 10 | "fmt" | |
| 11 | 11 | "io" | |
| 12 | + | "io/fs" | |
| 12 | 13 | "net" | |
| 13 | 14 | "net/http" | |
| 14 | 15 | "net/netip" | |
| @@ -22,6 +23,7 @@ import ( | |||
| 22 | 23 | "time" | |
| 23 | 24 | ||
| 24 | 25 | "github.com/bnema/zerowrap" | |
| 26 | + | "golang.org/x/sys/unix" | |
| 25 | 27 | ||
| 26 | 28 | "bnema.dev/ermite-runner/internal/cmdrun" | |
| 27 | 29 | "bnema.dev/ermite-runner/internal/execstream" | |
| @@ -169,6 +171,8 @@ func newEnvNamed(t *testing.T, mode, argv0 string) *env { | |||
| 169 | 171 | return e | |
| 170 | 172 | } | |
| 171 | 173 | ||
| 174 | + | func (e *env) driverScope() scope { return newScope(e.cfg.StateDir) } | |
| 175 | + | ||
| 172 | 176 | func (e *env) tapCount() int { e.mu.Lock(); defer e.mu.Unlock(); return len(e.taps) } | |
| 173 | 177 | ||
| 174 | 178 | func (e *env) driver(t *testing.T) *Driver { | |
| @@ -649,13 +653,13 @@ func TestFirecrackerProcsScan(t *testing.T) { | |||
| 649 | 653 | _ = os.MkdirAll(filepath.Join(proc, pid), 0o755) | |
| 650 | 654 | _ = os.WriteFile(filepath.Join(proc, pid, "cmdline"), []byte(cmdline), 0o644) | |
| 651 | 655 | } | |
| 652 | - | mk("100", "firecracker\x00--id\x00er00112233\x00--api-sock\x00/x\x00") | |
| 653 | - | mk("101", "/usr/bin/firecracker\x00--api-sock\x00/x\x00--id\x00er44556677\x00") | |
| 656 | + | mk("100", "firecracker\x00--id\x00er00112233\x00--api-sock\x00/var/lib/er/vm/er00112233/api.sock\x00") | |
| 657 | + | mk("101", "/usr/bin/firecracker\x00--api-sock\x00/var/lib/er/vm/er44556677/api.sock\x00--id\x00er44556677\x00") | |
| 654 | 658 | mk("102", "firecracker\x00--id\x00other-tool-vm\x00") // someone else's VM | |
| 655 | 659 | mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker | |
| 656 | 660 | mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched | |
| 657 | 661 | mk("abc", "") | |
| 658 | - | got := vmmProcs(proc) | |
| 662 | + | got := vmmProcs(proc, newScope("/var/lib/er")) | |
| 659 | 663 | if len(got) != 2 || got[100] != "er00112233" || got[101] != "er44556677" { | |
| 660 | 664 | t.Fatalf("%v", got) | |
| 661 | 665 | } | |
| @@ -705,7 +709,7 @@ func TestReapFindsRenamedVMMByID(t *testing.T) { | |||
| 705 | 709 | } | |
| 706 | 710 | runStub(t, conn) | |
| 707 | 711 | pid := sb.(*vmSandbox).rec.PID | |
| 708 | - | if got := cmdlineID("/proc", pid); got != sb.ID() { | |
| 712 | + | if got := cmdlineID("/proc", pid, e.driverScope()); got != sb.ID() { | |
| 709 | 713 | t.Fatalf("cmdlineID=%q want %q (argv[0] is %q)", got, sb.ID(), "fc-renamed-1.17") | |
| 710 | 714 | } | |
| 711 | 715 | _ = d1.lock.Release() | |
| @@ -719,20 +723,22 @@ func TestReapFindsRenamedVMMByID(t *testing.T) { | |||
| 719 | 723 | } | |
| 720 | 724 | } | |
| 721 | 725 | ||
| 722 | - | func TestVMIDFromCmdline(t *testing.T) { | |
| 726 | + | func TestIDFromCmdline(t *testing.T) { | |
| 723 | 727 | for name, tc := range map[string]struct { | |
| 724 | 728 | args []string | |
| 725 | 729 | want string | |
| 726 | 730 | }{ | |
| 727 | - | "firecracker": {[]string{"firecracker", "--id", "er00112233", "--api-sock", "/x"}, "er00112233"}, | |
| 728 | - | "jailer": {[]string{"jailer", "--id", "er00112233", "--exec-file", "/fc", "--uid", "1"}, "er00112233"}, | |
| 729 | - | "renamed": {[]string{"anything", "--api-sock", "/x", "--id", "er44556677"}, "er44556677"}, | |
| 730 | - | "foreign id": {[]string{"firecracker", "--id", "my-vm", "--api-sock", "/x"}, ""}, | |
| 731 | - | "no marker": {[]string{"python", "--id", "er00112233"}, ""}, | |
| 732 | - | "id without value": {[]string{"firecracker", "--api-sock", "/x", "--id"}, ""}, | |
| 733 | - | "path-like id": {[]string{"firecracker", "--id", "er../../../x", "--api-sock", "/x"}, ""}, | |
| 731 | + | "firecracker": {[]string{"firecracker", "--id", "er00112233", "--api-sock", "/x"}, "er00112233"}, | |
| 732 | + | "jailer": {[]string{"jailer", "--id", "er00112233", "--exec-file", "/fc", "--uid", "1"}, "er00112233"}, | |
| 733 | + | "renamed": {[]string{"anything", "--api-sock", "/x", "--id", "er44556677"}, "er44556677"}, | |
| 734 | + | // What a real jailer 1.17 leaves: /firecracker --id <id> --start-time-us ... | |
| 735 | + | "jailed firecracker": {[]string{"/firecracker", "--id", "er00112233", "--start-time-us", "150801025577", "--start-time-cpu-us", "60", "--parent-cpu-time-us", "1332"}, "er00112233"}, | |
| 736 | + | "foreign id": {[]string{"firecracker", "--id", "my-vm", "--api-sock", "/x"}, ""}, | |
| 737 | + | "no marker": {[]string{"python", "--id", "er00112233"}, ""}, | |
| 738 | + | "id without value": {[]string{"firecracker", "--api-sock", "/x", "--id"}, ""}, | |
| 739 | + | "path-like id": {[]string{"firecracker", "--id", "er../../../x", "--api-sock", "/x"}, ""}, | |
| 734 | 740 | } { | |
| 735 | - | if got := vmIDFromCmdline(tc.args); got != tc.want { | |
| 741 | + | if got := IDFromCmdline(tc.args); got != tc.want { | |
| 736 | 742 | t.Errorf("%s: got %q want %q", name, got, tc.want) | |
| 737 | 743 | } | |
| 738 | 744 | } | |
| @@ -760,11 +766,11 @@ func TestStopKeepsNetworkWhenTheVMMSurvives(t *testing.T) { | |||
| 760 | 766 | e := newEnv(t, "") | |
| 761 | 767 | stubborn := true | |
| 762 | 768 | d, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second, | |
| 763 | - | kill: func(pid int, to time.Duration) bool { | |
| 769 | + | kill: func(pid int, still func() bool, to time.Duration) bool { | |
| 764 | 770 | if stubborn { | |
| 765 | 771 | return false | |
| 766 | 772 | } | |
| 767 | - | return killAndWait(pid, to) | |
| 773 | + | return killAndWait(pid, still, to) | |
| 768 | 774 | }}) | |
| 769 | 775 | if err != nil { | |
| 770 | 776 | t.Fatal(err) | |
| @@ -1022,3 +1028,282 @@ func TestRemoveCgroup(t *testing.T) { | |||
| 1022 | 1028 | t.Fatal("a non-empty cgroup cannot be removed and must be reported") | |
| 1023 | 1029 | } | |
| 1024 | 1030 | } | |
| 1031 | + | ||
| 1032 | + | // Jailer 1.17 canonicalizes --chroot-base-dir and exits when it does not exist: | |
| 1033 | + | // the driver must create it (seen on a real host, invisible with the jailer off). | |
| 1034 | + | func TestNewCreatesTheJailerChrootBase(t *testing.T) { | |
| 1035 | + | e := newEnv(t, "ok") | |
| 1036 | + | e.cfg.JailerOff = false | |
| 1037 | + | e.cfg.Jailer = e.cfg.Firecracker // any executable: it is only resolved here | |
| 1038 | + | short, err := os.MkdirTemp("", "er") | |
| 1039 | + | if err != nil { | |
| 1040 | + | t.Fatal(err) | |
| 1041 | + | } | |
| 1042 | + | defer func() { _ = os.RemoveAll(short) }() | |
| 1043 | + | e.cfg.StateDir = filepath.Join(short, "s") // the jailed API socket path must fit in sun_path | |
| 1044 | + | d, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second}) | |
| 1045 | + | if err != nil { | |
| 1046 | + | t.Fatal(err) | |
| 1047 | + | } | |
| 1048 | + | defer func() { _ = d.Close() }() | |
| 1049 | + | st, err := os.Stat(jailBase(e.cfg.StateDir)) | |
| 1050 | + | if err != nil || !st.IsDir() { | |
| 1051 | + | t.Fatalf("jail base not created: %v", err) | |
| 1052 | + | } | |
| 1053 | + | } | |
| 1054 | + | ||
| 1055 | + | // GO-004: the longest socket path of a VM, in both jailer modes, 107 bytes ok / | |
| 1056 | + | // 108 rejected. | |
| 1057 | + | func TestSocketPathBoundaryInBothModes(t *testing.T) { | |
| 1058 | + | for _, jailed := range []bool{true, false} { | |
| 1059 | + | name := map[bool]string{true: "jailed", false: "unjailed"}[jailed] | |
| 1060 | + | // Find the longest socket path with an empty-ish state dir, then pad the | |
| 1061 | + | // state dir so that it is exactly maxSockPath, then one more. | |
| 1062 | + | longest := func(state string) int { | |
| 1063 | + | n := 0 | |
| 1064 | + | for _, p := range socketPaths(state, "firecracker", jailed) { | |
| 1065 | + | n = max(n, len(p)) | |
| 1066 | + | } | |
| 1067 | + | return n | |
| 1068 | + | } | |
| 1069 | + | base := "/s" | |
| 1070 | + | pad := maxSockPath - longest(base) | |
| 1071 | + | if pad < 0 { | |
| 1072 | + | t.Fatalf("%s: base already too long", name) | |
| 1073 | + | } | |
| 1074 | + | ok := base + strings.Repeat("d", pad) | |
| 1075 | + | if longest(ok) != maxSockPath { | |
| 1076 | + | t.Fatalf("%s: longest=%d want %d", name, longest(ok), maxSockPath) | |
| 1077 | + | } | |
| 1078 | + | if err := checkSocketPaths(ok, "firecracker", jailed); err != nil { | |
| 1079 | + | t.Errorf("%s: %d bytes rejected: %v", name, maxSockPath, err) | |
| 1080 | + | } | |
| 1081 | + | err := checkSocketPaths(ok+"d", "firecracker", jailed) | |
| 1082 | + | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |
| 1083 | + | t.Errorf("%s: %d bytes accepted: %v", name, maxSockPath+1, err) | |
| 1084 | + | } | |
| 1085 | + | } | |
| 1086 | + | } | |
| 1087 | + | ||
| 1088 | + | func TestNewRejectsAStateDirThatMakesASocketPathTooLong(t *testing.T) { | |
| 1089 | + | for _, jailerOff := range []bool{false, true} { | |
| 1090 | + | e := newEnv(t, "ok") | |
| 1091 | + | e.cfg.JailerOff = jailerOff | |
| 1092 | + | e.cfg.Jailer = e.cfg.Firecracker | |
| 1093 | + | e.cfg.StateDir = filepath.Join(e.cfg.StateDir, strings.Repeat("d", 60)) | |
| 1094 | + | _, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second}) | |
| 1095 | + | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |
| 1096 | + | t.Fatalf("jailerOff=%v: want a clear socket path error, got %v", jailerOff, err) | |
| 1097 | + | } | |
| 1098 | + | } | |
| 1099 | + | } | |
| 1100 | + | ||
| 1101 | + | // GO-003: the chroot base must be a real directory, owned by root (or the | |
| 1102 | + | // runner's own user), not writable by group or others; an existing one is never | |
| 1103 | + | // chmod-ed into compliance. | |
| 1104 | + | func TestEnsureJailBase(t *testing.T) { | |
| 1105 | + | root := t.TempDir() | |
| 1106 | + | good := filepath.Join(root, "good") | |
| 1107 | + | if err := ensureJailBase(good); err != nil { | |
| 1108 | + | t.Fatal(err) | |
| 1109 | + | } | |
| 1110 | + | if fi, _ := os.Stat(good); fi.Mode().Perm() != 0o700 { | |
| 1111 | + | t.Fatalf("created with %v", fi.Mode().Perm()) | |
| 1112 | + | } | |
| 1113 | + | if err := ensureJailBase(good); err != nil { // idempotent | |
| 1114 | + | t.Fatal(err) | |
| 1115 | + | } | |
| 1116 | + | for name, mode := range map[string]os.FileMode{"group-writable": 0o770, "other-writable": 0o707, "sticky-world": 0o777} { | |
| 1117 | + | d := filepath.Join(root, name) | |
| 1118 | + | _ = os.Mkdir(d, 0o700) | |
| 1119 | + | _ = os.Chmod(d, mode) | |
| 1120 | + | if err := ensureJailBase(d); err == nil || !strings.Contains(err.Error(), "writable by group or others") { | |
| 1121 | + | t.Errorf("%s: %v", name, err) | |
| 1122 | + | } | |
| 1123 | + | if fi, _ := os.Stat(d); fi.Mode().Perm() != mode { | |
| 1124 | + | t.Errorf("%s: mode was changed to %v", name, fi.Mode().Perm()) | |
| 1125 | + | } | |
| 1126 | + | } | |
| 1127 | + | rx := filepath.Join(root, "rx") | |
| 1128 | + | _ = os.Mkdir(rx, 0o755) // readable by others is fine | |
| 1129 | + | _ = os.Chmod(rx, 0o755) | |
| 1130 | + | if err := ensureJailBase(rx); err != nil { | |
| 1131 | + | t.Errorf("0755 refused: %v", err) | |
| 1132 | + | } | |
| 1133 | + | file := filepath.Join(root, "file") | |
| 1134 | + | _ = os.WriteFile(file, nil, 0o600) | |
| 1135 | + | if err := ensureJailBase(file); err == nil { | |
| 1136 | + | t.Error("a file accepted") | |
| 1137 | + | } | |
| 1138 | + | link := filepath.Join(root, "link") | |
| 1139 | + | _ = os.Symlink(good, link) | |
| 1140 | + | if err := ensureJailBase(link); err == nil || !strings.Contains(err.Error(), "not a directory") { | |
| 1141 | + | t.Errorf("a symlink accepted: %v", err) | |
| 1142 | + | } | |
| 1143 | + | } | |
| 1144 | + | ||
| 1145 | + | type fakeInfo struct { | |
| 1146 | + | fs.FileInfo | |
| 1147 | + | uid uint32 | |
| 1148 | + | } | |
| 1149 | + | ||
| 1150 | + | func (f fakeInfo) Sys() any { return &syscall.Stat_t{Uid: f.uid} } | |
| 1151 | + | ||
| 1152 | + | func TestJailBaseOwner(t *testing.T) { | |
| 1153 | + | fi, err := os.Stat(t.TempDir()) | |
| 1154 | + | if err != nil { | |
| 1155 | + | t.Fatal(err) | |
| 1156 | + | } | |
| 1157 | + | d := "/x/jail" | |
| 1158 | + | mine := os.Geteuid() | |
| 1159 | + | if err := checkJailBaseInfo(d, fakeInfo{fi, 0}, mine); err != nil { | |
| 1160 | + | t.Errorf("root-owned refused: %v", err) | |
| 1161 | + | } | |
| 1162 | + | if err := checkJailBaseInfo(d, fakeInfo{fi, uint32(mine)}, mine); err != nil { | |
| 1163 | + | t.Errorf("own user refused: %v", err) | |
| 1164 | + | } | |
| 1165 | + | if err := checkJailBaseInfo(d, fakeInfo{fi, uint32(mine) + 4242}, mine); err == nil || !strings.Contains(err.Error(), "owned by uid") { | |
| 1166 | + | t.Errorf("foreign owner accepted: %v", err) | |
| 1167 | + | } | |
| 1168 | + | } | |
| 1169 | + | ||
| 1170 | + | // GO-001: a Firecracker with only the jailer's --start-time-us is ours only when | |
| 1171 | + | // it lives in our jail base; a jailer only when it was given our chroot base. | |
| 1172 | + | func TestScanIsScopedToOurJailBase(t *testing.T) { | |
| 1173 | + | state := t.TempDir() | |
| 1174 | + | sc := newScope(state) | |
| 1175 | + | foreign := t.TempDir() | |
| 1176 | + | proc := t.TempDir() | |
| 1177 | + | mk := func(pid, root string, args ...string) { | |
| 1178 | + | dir := filepath.Join(proc, pid) | |
| 1179 | + | _ = os.MkdirAll(dir, 0o755) | |
| 1180 | + | _ = os.WriteFile(filepath.Join(dir, "cmdline"), []byte(strings.Join(args, "\x00")+"\x00"), 0o644) | |
| 1181 | + | if root != "" { | |
| 1182 | + | if err := os.Symlink(root, filepath.Join(dir, "root")); err != nil { | |
| 1183 | + | t.Fatal(err) | |
| 1184 | + | } | |
| 1185 | + | } | |
| 1186 | + | } | |
| 1187 | + | ours := filepath.Join(jailBase(state), "firecracker", "er00112233", "root") | |
| 1188 | + | if err := os.MkdirAll(ours, 0o755); err != nil { | |
| 1189 | + | t.Fatal(err) | |
| 1190 | + | } | |
| 1191 | + | // What a pivot_root'ed jailed VMM looks like: its root is our chroot directory | |
| 1192 | + | // but readlink does not say so (it says "/"); a symlinked alias reproduces | |
| 1193 | + | // that: another path, same inode. | |
| 1194 | + | alias := filepath.Join(t.TempDir(), "alias") | |
| 1195 | + | if err := os.Symlink(ours, alias); err != nil { | |
| 1196 | + | t.Fatal(err) | |
| 1197 | + | } | |
| 1198 | + | // Our jail base has a chroot for another id: a foreign VMM must not match it. | |
| 1199 | + | if err := os.MkdirAll(filepath.Join(jailBase(state), "firecracker", "er44556677", "root"), 0o755); err != nil { | |
| 1200 | + | t.Fatal(err) | |
| 1201 | + | } | |
| 1202 | + | foreignRoot := filepath.Join(foreign, "jail", "firecracker", "er44556677", "root") | |
| 1203 | + | _ = os.MkdirAll(foreignRoot, 0o755) | |
| 1204 | + | mk("100", ours, "/firecracker", "--id", "er00112233", "--start-time-us", "1", "--start-time-cpu-us", "2") | |
| 1205 | + | mk("101", foreignRoot, "/firecracker", "--id", "er44556677", "--start-time-us", "1") | |
| 1206 | + | mk("109", alias, "/firecracker", "--id", "er00112233", "--start-time-us", "1") // same inode, other path | |
| 1207 | + | mk("110", alias, "/firecracker", "--id", "er77777777", "--start-time-us", "1") // our dir, but not that id's chroot | |
| 1208 | + | mk("102", "/", "/firecracker", "--id", "er8899aabb", "--start-time-us", "1") // not chrooted at all | |
| 1209 | + | mk("103", "", "/firecracker", "--id", "er0000cccc", "--start-time-us", "1") // root unreadable | |
| 1210 | + | mk("104", "/", "jailer", "--id", "er11111111", "--exec-file", "/fc", "--chroot-base-dir", jailBase(state)) | |
| 1211 | + | mk("105", "/", "jailer", "--id", "er22222222", "--exec-file", "/fc", "--chroot-base-dir", filepath.Join(foreign, "jail")) | |
| 1212 | + | mk("106", "/", "jailer", "--id", "er33333333", "--exec-file", "/fc") | |
| 1213 | + | mk("107", ours+"/../../../../x", "/firecracker", "--id", "er55555555", "--start-time-us", "1") // lexical escape | |
| 1214 | + | vmDir := filepath.Join(state, "vm", "er66666666") | |
| 1215 | + | mk("108", "/", "firecracker", "--id", "er66666666", "--api-sock", filepath.Join(vmDir, "api.sock")) // unjailed, in our dir | |
| 1216 | + | mk("111", "/", "firecracker", "--id", "er99999999", "--api-sock", "/x") // socket elsewhere | |
| 1217 | + | mk("112", "/", "firecracker", "--id", "erabababab", "--api-sock", filepath.Join(vmDir, "api.sock")) // our dir, another id | |
| 1218 | + | mk("113", "/", "firecracker", "--id", "er66666666", "--api-sock", vmDir+"/../../x/api.sock") // escapes by .. | |
| 1219 | + | mk("114", "/", "firecracker", "--id", "er66666666", "--api-sock", "vm/er66666666/api.sock") // relative | |
| 1220 | + | mk("115", "/", "firecracker", "--id", "er66666666", "--api-sock") // no value | |
| 1221 | + | mk("116", "/", "firecracker", "--id", "er66666666", "--api-sock", vmDir+"x/api.sock") // prefix sibling | |
| 1222 | + | got := vmmProcs(proc, sc) | |
| 1223 | + | want := map[int]string{100: "er00112233", 104: "er11111111", 108: "er66666666", 109: "er00112233"} | |
| 1224 | + | if len(got) != len(want) { | |
| 1225 | + | t.Fatalf("got %v want %v", got, want) | |
| 1226 | + | } | |
| 1227 | + | for pid, id := range want { | |
| 1228 | + | if got[pid] != id { | |
| 1229 | + | t.Fatalf("got %v want %v", got, want) | |
| 1230 | + | } | |
| 1231 | + | } | |
| 1232 | + | if cmdlineID(proc, 101, sc) != "" || cmdlineID(proc, 100, sc) != "er00112233" { | |
| 1233 | + | t.Fatal("cmdlineID must apply the same scope") | |
| 1234 | + | } | |
| 1235 | + | } | |
| 1236 | + | ||
| 1237 | + | // GO-001 (pid reuse): the kill goes through a pidfd, after the identity check. | |
| 1238 | + | func startSleeper(t *testing.T) *exec.Cmd { | |
| 1239 | + | t.Helper() | |
| 1240 | + | c := exec.Command("sleep", "60") | |
| 1241 | + | if err := c.Start(); err != nil { | |
| 1242 | + | t.Skipf("cannot start sleep: %v", err) | |
| 1243 | + | } | |
| 1244 | + | t.Cleanup(func() { _ = c.Process.Kill(); _ = c.Wait() }) | |
| 1245 | + | return c | |
| 1246 | + | } | |
| 1247 | + | ||
| 1248 | + | func needPidfd(t *testing.T) { | |
| 1249 | + | t.Helper() | |
| 1250 | + | fd, err := unix.PidfdOpen(os.Getpid(), 0) | |
| 1251 | + | if err != nil { | |
| 1252 | + | t.Skipf("no pidfd on this kernel: %v", err) | |
| 1253 | + | } | |
| 1254 | + | _ = unix.Close(fd) | |
| 1255 | + | } | |
| 1256 | + | ||
| 1257 | + | func TestKillAndWaitKillsThroughThePidfd(t *testing.T) { | |
| 1258 | + | needPidfd(t) | |
| 1259 | + | c := startSleeper(t) | |
| 1260 | + | asked := false | |
| 1261 | + | if !killAndWait(c.Process.Pid, func() bool { asked = true; return true }, 5*time.Second) { | |
| 1262 | + | t.Fatal("the process is still there") | |
| 1263 | + | } | |
| 1264 | + | if !asked { | |
| 1265 | + | t.Fatal("the identity check did not run") | |
| 1266 | + | } | |
| 1267 | + | // The child is a zombie now (we have not waited for it): it must count as gone. | |
| 1268 | + | if _, err := c.Process.Wait(); err != nil { | |
| 1269 | + | t.Fatal(err) | |
| 1270 | + | } | |
| 1271 | + | } | |
| 1272 | + | ||
| 1273 | + | func TestKillAndWaitDoesNotKillAMismatchAfterTheOpen(t *testing.T) { | |
| 1274 | + | needPidfd(t) | |
| 1275 | + | c := startSleeper(t) | |
| 1276 | + | // The pid now belongs to something that is not the VMM we meant (what a | |
| 1277 | + | // recycled pid looks like): no signal, and "gone" (the VMM we meant is). | |
| 1278 | + | if !killAndWait(c.Process.Pid, func() bool { return false }, 2*time.Second) { | |
| 1279 | + | t.Fatal("a mismatch means the intended process is gone: true expected") | |
| 1280 | + | } | |
| 1281 | + | if !pidAlive(c.Process.Pid) { | |
| 1282 | + | t.Fatal("a process that failed the identity check was killed") | |
| 1283 | + | } | |
| 1284 | + | } | |
| 1285 | + | ||
| 1286 | + | func TestKillAndWaitOnAProcessThatIsAlreadyGone(t *testing.T) { | |
| 1287 | + | needPidfd(t) | |
| 1288 | + | c := exec.Command("true") | |
| 1289 | + | if err := c.Run(); err != nil { | |
| 1290 | + | t.Skip(err) | |
| 1291 | + | } | |
| 1292 | + | called := false | |
| 1293 | + | if !killAndWait(c.Process.Pid, func() bool { called = true; return true }, time.Second) { | |
| 1294 | + | t.Fatal("a reaped process must count as gone") | |
| 1295 | + | } | |
| 1296 | + | _ = called | |
| 1297 | + | } | |
| 1298 | + | ||
| 1299 | + | func TestKillAndWaitReportsAProcessThatStays(t *testing.T) { | |
| 1300 | + | needPidfd(t) | |
| 1301 | + | // pid 1 cannot be killed by us in a test (and must never be: the identity | |
| 1302 | + | // check says it is ours here only to show the failure path would not hang). | |
| 1303 | + | if os.Geteuid() == 0 { | |
| 1304 | + | t.Skip("root could signal pid 1") | |
| 1305 | + | } | |
| 1306 | + | if killAndWait(1, func() bool { return true }, 200*time.Millisecond) { | |
| 1307 | + | t.Fatal("an unkillable process was reported gone") | |
| 1308 | + | } | |
| 1309 | + | } | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "errors" | 9 | "errors" | ||
| 10 | "fmt" | 10 | "fmt" | ||
| 11 | "io" | 11 | "io" | ||
| 12 | + | "io/fs" | |||
| 12 | "net" | 13 | "net" | ||
| 13 | "net/http" | 14 | "net/http" | ||
| 14 | "net/netip" | 15 | "net/netip" | ||
| @@ -22,6 +23,7 @@ import ( | |||||
| 22 | "time" | 23 | "time" | ||
| 23 | 24 | ||||
| 24 | "github.com/bnema/zerowrap" | 25 | "github.com/bnema/zerowrap" | ||
| 26 | + | "golang.org/x/sys/unix" | |||
| 25 | 27 | ||||
| 26 | "bnema.dev/ermite-runner/internal/cmdrun" | 28 | "bnema.dev/ermite-runner/internal/cmdrun" | ||
| 27 | "bnema.dev/ermite-runner/internal/execstream" | 29 | "bnema.dev/ermite-runner/internal/execstream" | ||
| @@ -169,6 +171,8 @@ func newEnvNamed(t *testing.T, mode, argv0 string) *env { | |||||
| 169 | return e | 171 | return e | ||
| 170 | } | 172 | } | ||
| 171 | 173 | ||||
| 174 | + | func (e *env) driverScope() scope { return newScope(e.cfg.StateDir) } | |||
| 175 | + | ||||
| 172 | func (e *env) tapCount() int { e.mu.Lock(); defer e.mu.Unlock(); return len(e.taps) } | 176 | func (e *env) tapCount() int { e.mu.Lock(); defer e.mu.Unlock(); return len(e.taps) } | ||
| 173 | 177 | ||||
| 174 | func (e *env) driver(t *testing.T) *Driver { | 178 | func (e *env) driver(t *testing.T) *Driver { | ||
| @@ -649,13 +653,13 @@ func TestFirecrackerProcsScan(t *testing.T) { | |||||
| 649 | _ = os.MkdirAll(filepath.Join(proc, pid), 0o755) | 653 | _ = os.MkdirAll(filepath.Join(proc, pid), 0o755) | ||
| 650 | _ = os.WriteFile(filepath.Join(proc, pid, "cmdline"), []byte(cmdline), 0o644) | 654 | _ = os.WriteFile(filepath.Join(proc, pid, "cmdline"), []byte(cmdline), 0o644) | ||
| 651 | } | 655 | } | ||
| 652 | - | mk("100", "firecracker\x00--id\x00er00112233\x00--api-sock\x00/x\x00") | 656 | + | mk("100", "firecracker\x00--id\x00er00112233\x00--api-sock\x00/var/lib/er/vm/er00112233/api.sock\x00") |
| 653 | - | mk("101", "/usr/bin/firecracker\x00--api-sock\x00/x\x00--id\x00er44556677\x00") | 657 | + | mk("101", "/usr/bin/firecracker\x00--api-sock\x00/var/lib/er/vm/er44556677/api.sock\x00--id\x00er44556677\x00") |
| 654 | mk("102", "firecracker\x00--id\x00other-tool-vm\x00") // someone else's VM | 658 | mk("102", "firecracker\x00--id\x00other-tool-vm\x00") // someone else's VM | ||
| 655 | mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker | 659 | mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker | ||
| 656 | mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched | 660 | mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched | ||
| 657 | mk("abc", "") | 661 | mk("abc", "") | ||
| 658 | - | got := vmmProcs(proc) | 662 | + | got := vmmProcs(proc, newScope("/var/lib/er")) |
| 659 | if len(got) != 2 || got[100] != "er00112233" || got[101] != "er44556677" { | 663 | if len(got) != 2 || got[100] != "er00112233" || got[101] != "er44556677" { | ||
| 660 | t.Fatalf("%v", got) | 664 | t.Fatalf("%v", got) | ||
| 661 | } | 665 | } | ||
| @@ -705,7 +709,7 @@ func TestReapFindsRenamedVMMByID(t *testing.T) { | |||||
| 705 | } | 709 | } | ||
| 706 | runStub(t, conn) | 710 | runStub(t, conn) | ||
| 707 | pid := sb.(*vmSandbox).rec.PID | 711 | pid := sb.(*vmSandbox).rec.PID | ||
| 708 | - | if got := cmdlineID("/proc", pid); got != sb.ID() { | 712 | + | if got := cmdlineID("/proc", pid, e.driverScope()); got != sb.ID() { |
| 709 | t.Fatalf("cmdlineID=%q want %q (argv[0] is %q)", got, sb.ID(), "fc-renamed-1.17") | 713 | t.Fatalf("cmdlineID=%q want %q (argv[0] is %q)", got, sb.ID(), "fc-renamed-1.17") | ||
| 710 | } | 714 | } | ||
| 711 | _ = d1.lock.Release() | 715 | _ = d1.lock.Release() | ||
| @@ -719,20 +723,22 @@ func TestReapFindsRenamedVMMByID(t *testing.T) { | |||||
| 719 | } | 723 | } | ||
| 720 | } | 724 | } | ||
| 721 | 725 | ||||
| 722 | - | func TestVMIDFromCmdline(t *testing.T) { | 726 | + | func TestIDFromCmdline(t *testing.T) { |
| 723 | for name, tc := range map[string]struct { | 727 | for name, tc := range map[string]struct { | ||
| 724 | args []string | 728 | args []string | ||
| 725 | want string | 729 | want string | ||
| 726 | }{ | 730 | }{ | ||
| 727 | - | "firecracker": {[]string{"firecracker", "--id", "er00112233", "--api-sock", "/x"}, "er00112233"}, | 731 | + | "firecracker": {[]string{"firecracker", "--id", "er00112233", "--api-sock", "/x"}, "er00112233"}, |
| 728 | - | "jailer": {[]string{"jailer", "--id", "er00112233", "--exec-file", "/fc", "--uid", "1"}, "er00112233"}, | 732 | + | "jailer": {[]string{"jailer", "--id", "er00112233", "--exec-file", "/fc", "--uid", "1"}, "er00112233"}, |
| 729 | - | "renamed": {[]string{"anything", "--api-sock", "/x", "--id", "er44556677"}, "er44556677"}, | 733 | + | "renamed": {[]string{"anything", "--api-sock", "/x", "--id", "er44556677"}, "er44556677"}, |
| 730 | - | "foreign id": {[]string{"firecracker", "--id", "my-vm", "--api-sock", "/x"}, ""}, | 734 | + | // What a real jailer 1.17 leaves: /firecracker --id <id> --start-time-us ... |
| 731 | - | "no marker": {[]string{"python", "--id", "er00112233"}, ""}, | 735 | + | "jailed firecracker": {[]string{"/firecracker", "--id", "er00112233", "--start-time-us", "150801025577", "--start-time-cpu-us", "60", "--parent-cpu-time-us", "1332"}, "er00112233"}, |
| 732 | - | "id without value": {[]string{"firecracker", "--api-sock", "/x", "--id"}, ""}, | 736 | + | "foreign id": {[]string{"firecracker", "--id", "my-vm", "--api-sock", "/x"}, ""}, |
| 733 | - | "path-like id": {[]string{"firecracker", "--id", "er../../../x", "--api-sock", "/x"}, ""}, | 737 | + | "no marker": {[]string{"python", "--id", "er00112233"}, ""}, |
| 738 | + | "id without value": {[]string{"firecracker", "--api-sock", "/x", "--id"}, ""}, | |||
| 739 | + | "path-like id": {[]string{"firecracker", "--id", "er../../../x", "--api-sock", "/x"}, ""}, | |||
| 734 | } { | 740 | } { | ||
| 735 | - | if got := vmIDFromCmdline(tc.args); got != tc.want { | 741 | + | if got := IDFromCmdline(tc.args); got != tc.want { |
| 736 | t.Errorf("%s: got %q want %q", name, got, tc.want) | 742 | t.Errorf("%s: got %q want %q", name, got, tc.want) | ||
| 737 | } | 743 | } | ||
| 738 | } | 744 | } | ||
| @@ -760,11 +766,11 @@ func TestStopKeepsNetworkWhenTheVMMSurvives(t *testing.T) { | |||||
| 760 | e := newEnv(t, "") | 766 | e := newEnv(t, "") | ||
| 761 | stubborn := true | 767 | stubborn := true | ||
| 762 | d, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second, | 768 | d, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second, | ||
| 763 | - | kill: func(pid int, to time.Duration) bool { | 769 | + | kill: func(pid int, still func() bool, to time.Duration) bool { |
| 764 | if stubborn { | 770 | if stubborn { | ||
| 765 | return false | 771 | return false | ||
| 766 | } | 772 | } | ||
| 767 | - | return killAndWait(pid, to) | 773 | + | return killAndWait(pid, still, to) |
| 768 | }}) | 774 | }}) | ||
| 769 | if err != nil { | 775 | if err != nil { | ||
| 770 | t.Fatal(err) | 776 | t.Fatal(err) | ||
| @@ -1022,3 +1028,282 @@ func TestRemoveCgroup(t *testing.T) { | |||||
| 1022 | t.Fatal("a non-empty cgroup cannot be removed and must be reported") | 1028 | t.Fatal("a non-empty cgroup cannot be removed and must be reported") | ||
| 1023 | } | 1029 | } | ||
| 1024 | } | 1030 | } | ||
| 1031 | + | ||||
| 1032 | + | // Jailer 1.17 canonicalizes --chroot-base-dir and exits when it does not exist: | |||
| 1033 | + | // the driver must create it (seen on a real host, invisible with the jailer off). | |||
| 1034 | + | func TestNewCreatesTheJailerChrootBase(t *testing.T) { | |||
| 1035 | + | e := newEnv(t, "ok") | |||
| 1036 | + | e.cfg.JailerOff = false | |||
| 1037 | + | e.cfg.Jailer = e.cfg.Firecracker // any executable: it is only resolved here | |||
| 1038 | + | short, err := os.MkdirTemp("", "er") | |||
| 1039 | + | if err != nil { | |||
| 1040 | + | t.Fatal(err) | |||
| 1041 | + | } | |||
| 1042 | + | defer func() { _ = os.RemoveAll(short) }() | |||
| 1043 | + | e.cfg.StateDir = filepath.Join(short, "s") // the jailed API socket path must fit in sun_path | |||
| 1044 | + | d, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second}) | |||
| 1045 | + | if err != nil { | |||
| 1046 | + | t.Fatal(err) | |||
| 1047 | + | } | |||
| 1048 | + | defer func() { _ = d.Close() }() | |||
| 1049 | + | st, err := os.Stat(jailBase(e.cfg.StateDir)) | |||
| 1050 | + | if err != nil || !st.IsDir() { | |||
| 1051 | + | t.Fatalf("jail base not created: %v", err) | |||
| 1052 | + | } | |||
| 1053 | + | } | |||
| 1054 | + | ||||
| 1055 | + | // GO-004: the longest socket path of a VM, in both jailer modes, 107 bytes ok / | |||
| 1056 | + | // 108 rejected. | |||
| 1057 | + | func TestSocketPathBoundaryInBothModes(t *testing.T) { | |||
| 1058 | + | for _, jailed := range []bool{true, false} { | |||
| 1059 | + | name := map[bool]string{true: "jailed", false: "unjailed"}[jailed] | |||
| 1060 | + | // Find the longest socket path with an empty-ish state dir, then pad the | |||
| 1061 | + | // state dir so that it is exactly maxSockPath, then one more. | |||
| 1062 | + | longest := func(state string) int { | |||
| 1063 | + | n := 0 | |||
| 1064 | + | for _, p := range socketPaths(state, "firecracker", jailed) { | |||
| 1065 | + | n = max(n, len(p)) | |||
| 1066 | + | } | |||
| 1067 | + | return n | |||
| 1068 | + | } | |||
| 1069 | + | base := "/s" | |||
| 1070 | + | pad := maxSockPath - longest(base) | |||
| 1071 | + | if pad < 0 { | |||
| 1072 | + | t.Fatalf("%s: base already too long", name) | |||
| 1073 | + | } | |||
| 1074 | + | ok := base + strings.Repeat("d", pad) | |||
| 1075 | + | if longest(ok) != maxSockPath { | |||
| 1076 | + | t.Fatalf("%s: longest=%d want %d", name, longest(ok), maxSockPath) | |||
| 1077 | + | } | |||
| 1078 | + | if err := checkSocketPaths(ok, "firecracker", jailed); err != nil { | |||
| 1079 | + | t.Errorf("%s: %d bytes rejected: %v", name, maxSockPath, err) | |||
| 1080 | + | } | |||
| 1081 | + | err := checkSocketPaths(ok+"d", "firecracker", jailed) | |||
| 1082 | + | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |||
| 1083 | + | t.Errorf("%s: %d bytes accepted: %v", name, maxSockPath+1, err) | |||
| 1084 | + | } | |||
| 1085 | + | } | |||
| 1086 | + | } | |||
| 1087 | + | ||||
| 1088 | + | func TestNewRejectsAStateDirThatMakesASocketPathTooLong(t *testing.T) { | |||
| 1089 | + | for _, jailerOff := range []bool{false, true} { | |||
| 1090 | + | e := newEnv(t, "ok") | |||
| 1091 | + | e.cfg.JailerOff = jailerOff | |||
| 1092 | + | e.cfg.Jailer = e.cfg.Firecracker | |||
| 1093 | + | e.cfg.StateDir = filepath.Join(e.cfg.StateDir, strings.Repeat("d", 60)) | |||
| 1094 | + | _, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second}) | |||
| 1095 | + | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |||
| 1096 | + | t.Fatalf("jailerOff=%v: want a clear socket path error, got %v", jailerOff, err) | |||
| 1097 | + | } | |||
| 1098 | + | } | |||
| 1099 | + | } | |||
| 1100 | + | ||||
| 1101 | + | // GO-003: the chroot base must be a real directory, owned by root (or the | |||
| 1102 | + | // runner's own user), not writable by group or others; an existing one is never | |||
| 1103 | + | // chmod-ed into compliance. | |||
| 1104 | + | func TestEnsureJailBase(t *testing.T) { | |||
| 1105 | + | root := t.TempDir() | |||
| 1106 | + | good := filepath.Join(root, "good") | |||
| 1107 | + | if err := ensureJailBase(good); err != nil { | |||
| 1108 | + | t.Fatal(err) | |||
| 1109 | + | } | |||
| 1110 | + | if fi, _ := os.Stat(good); fi.Mode().Perm() != 0o700 { | |||
| 1111 | + | t.Fatalf("created with %v", fi.Mode().Perm()) | |||
| 1112 | + | } | |||
| 1113 | + | if err := ensureJailBase(good); err != nil { // idempotent | |||
| 1114 | + | t.Fatal(err) | |||
| 1115 | + | } | |||
| 1116 | + | for name, mode := range map[string]os.FileMode{"group-writable": 0o770, "other-writable": 0o707, "sticky-world": 0o777} { | |||
| 1117 | + | d := filepath.Join(root, name) | |||
| 1118 | + | _ = os.Mkdir(d, 0o700) | |||
| 1119 | + | _ = os.Chmod(d, mode) | |||
| 1120 | + | if err := ensureJailBase(d); err == nil || !strings.Contains(err.Error(), "writable by group or others") { | |||
| 1121 | + | t.Errorf("%s: %v", name, err) | |||
| 1122 | + | } | |||
| 1123 | + | if fi, _ := os.Stat(d); fi.Mode().Perm() != mode { | |||
| 1124 | + | t.Errorf("%s: mode was changed to %v", name, fi.Mode().Perm()) | |||
| 1125 | + | } | |||
| 1126 | + | } | |||
| 1127 | + | rx := filepath.Join(root, "rx") | |||
| 1128 | + | _ = os.Mkdir(rx, 0o755) // readable by others is fine | |||
| 1129 | + | _ = os.Chmod(rx, 0o755) | |||
| 1130 | + | if err := ensureJailBase(rx); err != nil { | |||
| 1131 | + | t.Errorf("0755 refused: %v", err) | |||
| 1132 | + | } | |||
| 1133 | + | file := filepath.Join(root, "file") | |||
| 1134 | + | _ = os.WriteFile(file, nil, 0o600) | |||
| 1135 | + | if err := ensureJailBase(file); err == nil { | |||
| 1136 | + | t.Error("a file accepted") | |||
| 1137 | + | } | |||
| 1138 | + | link := filepath.Join(root, "link") | |||
| 1139 | + | _ = os.Symlink(good, link) | |||
| 1140 | + | if err := ensureJailBase(link); err == nil || !strings.Contains(err.Error(), "not a directory") { | |||
| 1141 | + | t.Errorf("a symlink accepted: %v", err) | |||
| 1142 | + | } | |||
| 1143 | + | } | |||
| 1144 | + | ||||
| 1145 | + | type fakeInfo struct { | |||
| 1146 | + | fs.FileInfo | |||
| 1147 | + | uid uint32 | |||
| 1148 | + | } | |||
| 1149 | + | ||||
| 1150 | + | func (f fakeInfo) Sys() any { return &syscall.Stat_t{Uid: f.uid} } | |||
| 1151 | + | ||||
| 1152 | + | func TestJailBaseOwner(t *testing.T) { | |||
| 1153 | + | fi, err := os.Stat(t.TempDir()) | |||
| 1154 | + | if err != nil { | |||
| 1155 | + | t.Fatal(err) | |||
| 1156 | + | } | |||
| 1157 | + | d := "/x/jail" | |||
| 1158 | + | mine := os.Geteuid() | |||
| 1159 | + | if err := checkJailBaseInfo(d, fakeInfo{fi, 0}, mine); err != nil { | |||
| 1160 | + | t.Errorf("root-owned refused: %v", err) | |||
| 1161 | + | } | |||
| 1162 | + | if err := checkJailBaseInfo(d, fakeInfo{fi, uint32(mine)}, mine); err != nil { | |||
| 1163 | + | t.Errorf("own user refused: %v", err) | |||
| 1164 | + | } | |||
| 1165 | + | if err := checkJailBaseInfo(d, fakeInfo{fi, uint32(mine) + 4242}, mine); err == nil || !strings.Contains(err.Error(), "owned by uid") { | |||
| 1166 | + | t.Errorf("foreign owner accepted: %v", err) | |||
| 1167 | + | } | |||
| 1168 | + | } | |||
| 1169 | + | ||||
| 1170 | + | // GO-001: a Firecracker with only the jailer's --start-time-us is ours only when | |||
| 1171 | + | // it lives in our jail base; a jailer only when it was given our chroot base. | |||
| 1172 | + | func TestScanIsScopedToOurJailBase(t *testing.T) { | |||
| 1173 | + | state := t.TempDir() | |||
| 1174 | + | sc := newScope(state) | |||
| 1175 | + | foreign := t.TempDir() | |||
| 1176 | + | proc := t.TempDir() | |||
| 1177 | + | mk := func(pid, root string, args ...string) { | |||
| 1178 | + | dir := filepath.Join(proc, pid) | |||
| 1179 | + | _ = os.MkdirAll(dir, 0o755) | |||
| 1180 | + | _ = os.WriteFile(filepath.Join(dir, "cmdline"), []byte(strings.Join(args, "\x00")+"\x00"), 0o644) | |||
| 1181 | + | if root != "" { | |||
| 1182 | + | if err := os.Symlink(root, filepath.Join(dir, "root")); err != nil { | |||
| 1183 | + | t.Fatal(err) | |||
| 1184 | + | } | |||
| 1185 | + | } | |||
| 1186 | + | } | |||
| 1187 | + | ours := filepath.Join(jailBase(state), "firecracker", "er00112233", "root") | |||
| 1188 | + | if err := os.MkdirAll(ours, 0o755); err != nil { | |||
| 1189 | + | t.Fatal(err) | |||
| 1190 | + | } | |||
| 1191 | + | // What a pivot_root'ed jailed VMM looks like: its root is our chroot directory | |||
| 1192 | + | // but readlink does not say so (it says "/"); a symlinked alias reproduces | |||
| 1193 | + | // that: another path, same inode. | |||
| 1194 | + | alias := filepath.Join(t.TempDir(), "alias") | |||
| 1195 | + | if err := os.Symlink(ours, alias); err != nil { | |||
| 1196 | + | t.Fatal(err) | |||
| 1197 | + | } | |||
| 1198 | + | // Our jail base has a chroot for another id: a foreign VMM must not match it. | |||
| 1199 | + | if err := os.MkdirAll(filepath.Join(jailBase(state), "firecracker", "er44556677", "root"), 0o755); err != nil { | |||
| 1200 | + | t.Fatal(err) | |||
| 1201 | + | } | |||
| 1202 | + | foreignRoot := filepath.Join(foreign, "jail", "firecracker", "er44556677", "root") | |||
| 1203 | + | _ = os.MkdirAll(foreignRoot, 0o755) | |||
| 1204 | + | mk("100", ours, "/firecracker", "--id", "er00112233", "--start-time-us", "1", "--start-time-cpu-us", "2") | |||
| 1205 | + | mk("101", foreignRoot, "/firecracker", "--id", "er44556677", "--start-time-us", "1") | |||
| 1206 | + | mk("109", alias, "/firecracker", "--id", "er00112233", "--start-time-us", "1") // same inode, other path | |||
| 1207 | + | mk("110", alias, "/firecracker", "--id", "er77777777", "--start-time-us", "1") // our dir, but not that id's chroot | |||
| 1208 | + | mk("102", "/", "/firecracker", "--id", "er8899aabb", "--start-time-us", "1") // not chrooted at all | |||
| 1209 | + | mk("103", "", "/firecracker", "--id", "er0000cccc", "--start-time-us", "1") // root unreadable | |||
| 1210 | + | mk("104", "/", "jailer", "--id", "er11111111", "--exec-file", "/fc", "--chroot-base-dir", jailBase(state)) | |||
| 1211 | + | mk("105", "/", "jailer", "--id", "er22222222", "--exec-file", "/fc", "--chroot-base-dir", filepath.Join(foreign, "jail")) | |||
| 1212 | + | mk("106", "/", "jailer", "--id", "er33333333", "--exec-file", "/fc") | |||
| 1213 | + | mk("107", ours+"/../../../../x", "/firecracker", "--id", "er55555555", "--start-time-us", "1") // lexical escape | |||
| 1214 | + | vmDir := filepath.Join(state, "vm", "er66666666") | |||
| 1215 | + | mk("108", "/", "firecracker", "--id", "er66666666", "--api-sock", filepath.Join(vmDir, "api.sock")) // unjailed, in our dir | |||
| 1216 | + | mk("111", "/", "firecracker", "--id", "er99999999", "--api-sock", "/x") // socket elsewhere | |||
| 1217 | + | mk("112", "/", "firecracker", "--id", "erabababab", "--api-sock", filepath.Join(vmDir, "api.sock")) // our dir, another id | |||
| 1218 | + | mk("113", "/", "firecracker", "--id", "er66666666", "--api-sock", vmDir+"/../../x/api.sock") // escapes by .. | |||
| 1219 | + | mk("114", "/", "firecracker", "--id", "er66666666", "--api-sock", "vm/er66666666/api.sock") // relative | |||
| 1220 | + | mk("115", "/", "firecracker", "--id", "er66666666", "--api-sock") // no value | |||
| 1221 | + | mk("116", "/", "firecracker", "--id", "er66666666", "--api-sock", vmDir+"x/api.sock") // prefix sibling | |||
| 1222 | + | got := vmmProcs(proc, sc) | |||
| 1223 | + | want := map[int]string{100: "er00112233", 104: "er11111111", 108: "er66666666", 109: "er00112233"} | |||
| 1224 | + | if len(got) != len(want) { | |||
| 1225 | + | t.Fatalf("got %v want %v", got, want) | |||
| 1226 | + | } | |||
| 1227 | + | for pid, id := range want { | |||
| 1228 | + | if got[pid] != id { | |||
| 1229 | + | t.Fatalf("got %v want %v", got, want) | |||
| 1230 | + | } | |||
| 1231 | + | } | |||
| 1232 | + | if cmdlineID(proc, 101, sc) != "" || cmdlineID(proc, 100, sc) != "er00112233" { | |||
| 1233 | + | t.Fatal("cmdlineID must apply the same scope") | |||
| 1234 | + | } | |||
| 1235 | + | } | |||
| 1236 | + | ||||
| 1237 | + | // GO-001 (pid reuse): the kill goes through a pidfd, after the identity check. | |||
| 1238 | + | func startSleeper(t *testing.T) *exec.Cmd { | |||
| 1239 | + | t.Helper() | |||
| 1240 | + | c := exec.Command("sleep", "60") | |||
| 1241 | + | if err := c.Start(); err != nil { | |||
| 1242 | + | t.Skipf("cannot start sleep: %v", err) | |||
| 1243 | + | } | |||
| 1244 | + | t.Cleanup(func() { _ = c.Process.Kill(); _ = c.Wait() }) | |||
| 1245 | + | return c | |||
| 1246 | + | } | |||
| 1247 | + | ||||
| 1248 | + | func needPidfd(t *testing.T) { | |||
| 1249 | + | t.Helper() | |||
| 1250 | + | fd, err := unix.PidfdOpen(os.Getpid(), 0) | |||
| 1251 | + | if err != nil { | |||
| 1252 | + | t.Skipf("no pidfd on this kernel: %v", err) | |||
| 1253 | + | } | |||
| 1254 | + | _ = unix.Close(fd) | |||
| 1255 | + | } | |||
| 1256 | + | ||||
| 1257 | + | func TestKillAndWaitKillsThroughThePidfd(t *testing.T) { | |||
| 1258 | + | needPidfd(t) | |||
| 1259 | + | c := startSleeper(t) | |||
| 1260 | + | asked := false | |||
| 1261 | + | if !killAndWait(c.Process.Pid, func() bool { asked = true; return true }, 5*time.Second) { | |||
| 1262 | + | t.Fatal("the process is still there") | |||
| 1263 | + | } | |||
| 1264 | + | if !asked { | |||
| 1265 | + | t.Fatal("the identity check did not run") | |||
| 1266 | + | } | |||
| 1267 | + | // The child is a zombie now (we have not waited for it): it must count as gone. | |||
| 1268 | + | if _, err := c.Process.Wait(); err != nil { | |||
| 1269 | + | t.Fatal(err) | |||
| 1270 | + | } | |||
| 1271 | + | } | |||
| 1272 | + | ||||
| 1273 | + | func TestKillAndWaitDoesNotKillAMismatchAfterTheOpen(t *testing.T) { | |||
| 1274 | + | needPidfd(t) | |||
| 1275 | + | c := startSleeper(t) | |||
| 1276 | + | // The pid now belongs to something that is not the VMM we meant (what a | |||
| 1277 | + | // recycled pid looks like): no signal, and "gone" (the VMM we meant is). | |||
| 1278 | + | if !killAndWait(c.Process.Pid, func() bool { return false }, 2*time.Second) { | |||
| 1279 | + | t.Fatal("a mismatch means the intended process is gone: true expected") | |||
| 1280 | + | } | |||
| 1281 | + | if !pidAlive(c.Process.Pid) { | |||
| 1282 | + | t.Fatal("a process that failed the identity check was killed") | |||
| 1283 | + | } | |||
| 1284 | + | } | |||
| 1285 | + | ||||
| 1286 | + | func TestKillAndWaitOnAProcessThatIsAlreadyGone(t *testing.T) { | |||
| 1287 | + | needPidfd(t) | |||
| 1288 | + | c := exec.Command("true") | |||
| 1289 | + | if err := c.Run(); err != nil { | |||
| 1290 | + | t.Skip(err) | |||
| 1291 | + | } | |||
| 1292 | + | called := false | |||
| 1293 | + | if !killAndWait(c.Process.Pid, func() bool { called = true; return true }, time.Second) { | |||
| 1294 | + | t.Fatal("a reaped process must count as gone") | |||
| 1295 | + | } | |||
| 1296 | + | _ = called | |||
| 1297 | + | } | |||
| 1298 | + | ||||
| 1299 | + | func TestKillAndWaitReportsAProcessThatStays(t *testing.T) { | |||
| 1300 | + | needPidfd(t) | |||
| 1301 | + | // pid 1 cannot be killed by us in a test (and must never be: the identity | |||
| 1302 | + | // check says it is ours here only to show the failure path would not hang). | |||
| 1303 | + | if os.Geteuid() == 0 { | |||
| 1304 | + | t.Skip("root could signal pid 1") | |||
| 1305 | + | } | |||
| 1306 | + | if killAndWait(1, func() bool { return true }, 200*time.Millisecond) { | |||
| 1307 | + | t.Fatal("an unkillable process was reported gone") | |||
| 1308 | + | } | |||
| 1309 | + | } | |||