diff --git a/cmd/controller/main.go b/cmd/controller/main.go index a62553ee..3be74a62 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "flag" + "fmt" "os" "time" @@ -38,6 +39,21 @@ func init() { utilruntime.Must(v1.AddToScheme(scheme)) } +// validatePrepareFlags rejects prepare-time flag combinations that would silently do +// nothing, so an operator asking for a latency optimization is never left with a pod +// shape that cannot deliver it. It runs AFTER resolveRunMode, so enableHuskPods already +// reflects the resolved path (false on --enable-raw-forkd or --mock). +// +// --husk-prepare-egress-link needs BOTH --multi-vm-fork (the stub prepares its tap only +// on the multi-VM Prepare path) AND the husk-pods run path (the raw-forkd path never +// creates a husk pod, so there is no pod to prepare a tap in). +func validatePrepareFlags(prepareEgressLink, multiVMFork, enableHuskPods bool) error { + if prepareEgressLink && (!multiVMFork || !enableHuskPods) { + return fmt.Errorf("--husk-prepare-egress-link requires --multi-vm-fork AND the husk-pods run path (not --enable-raw-forkd or --mock): the tap is prepared only by a multi-VM husk pod, which the raw-forkd path never creates") + } + return nil +} + // resolveRunMode picks the single active controller run path from the flags. // husk pods is the pod-native default; --enable-raw-forkd selects the // fork-per-claim fallback, and --mock forces it too (the dev mock overlay has no @@ -64,6 +80,7 @@ func main() { var huskConnReuse bool var liveCowChildImport bool var prewarmChild bool + var prepareEgressLink bool var huskStubImage string var huskDNSUpstream string var huskControlPort int @@ -96,6 +113,7 @@ func main() { flag.BoolVar(&multiVMFork, "multi-vm-fork", false, "EXPERIMENTAL, DEFAULT OFF: route a hosted fork child into an ADDITIONAL VM spawned INSIDE the source husk pod (the spawn-vm control op) instead of a brand-new child pod, when the source pod is multi-VM capable (its stub runs --multi-vm). OFF is byte-for-byte the current new-pod-per-fork path, so nothing changes unless you opt in AND the source pod is multi-VM capable; a non-capable source silently falls back to a new pod. This wires the routing + status (child status.pod = source pod, status.vmId = the spawned VM); the per-pod and node memory accounting that pends or spills a fork when a pod is full is a follow-up, so co-location is capped conservatively and the remainder spills to new pods. Only used with --enable-husk-pods.") flag.BoolVar(&liveCowFork, "live-cow-fork", false, "EXPERIMENTAL, DEFAULT OFF: start warm husk pods with --live-cow-fork so a CO-LOCATED fork child shares the PARENT's resident guest memory (patched Firecracker memfd + userfaultfd write-protect) instead of restoring from the disk fork snapshot (milestone m4b), driving the hosted fork toward sub-100ms. SEPARATE from --multi-vm-fork so it can be deployed off and canaried independently. OFF is byte-for-byte the current disk co-location. The co-located child still falls back to the disk restore where the child-side memfd import is not yet complete, so turning it on never breaks a fork; it is a no-op off Linux (userfaultfd write-protect is Linux-only). Only meaningful with --enable-husk-pods.") flag.BoolVar(&liveCowChildImport, "live-cow-child-import", false, "EXPERIMENTAL, DEFAULT OFF: with --live-cow-fork on, opt warm husk pods onto the VMSTATE-ONLY fork capture (skip the ~364ms disk mem write) so a co-located child boots its guest RAM from the source shared memfd instead of the disk fork snapshot. REQUIRES a child-side-import patched Firecracker; off keeps the armed source writing the disk mem so every child restores from disk and no fork hangs. Only meaningful with --live-cow-fork and --enable-husk-pods.") + flag.BoolVar(&prepareEgressLink, "husk-prepare-egress-link", false, "EXPERIMENTAL, DEFAULT OFF: warm husk pods bring their default VM's tap up while DORMANT, under a default-deny policy, so a claim pays only the atomic nft transaction that installs the tenant's policy instead of also paying the tap create. Requires --multi-vm-fork and --enable-husk-pods. Off keeps the whole filter on the activate path byte-for-byte.") flag.BoolVar(&prewarmChild, "prewarm-child", false, "EXPERIMENTAL, DEFAULT OFF: keep one dormant generic co-located child Firecracker pre-prepared per multi-vm husk pod so a fork adopts the ready child (fc_boot=0, prepare off the hot path) instead of booting one at fork time. Requires --multi-vm-fork and --enable-husk-pods; a fresh slot re-warms async, a miss falls back to on-demand prepare byte-for-byte.") flag.BoolVar(&huskConnReuse, "husk-conn-reuse", false, "EXPERIMENTAL, DEFAULT OFF: reuse ONE authenticated mTLS husk control connection per husk pod across control-plane RPCs (activate, fork-snapshot, spawn-vm, remove-fork-snapshot) instead of opening a fresh TCP+TLS handshake per RPC. A co-located fork does fork-snapshot then spawn-vm to the SAME source pod, so reuse saves the second full handshake and cuts the per-RPC connection-setup overhead toward zero, driving the hosted fork toward sub-100ms. OFF is byte-for-byte the current one-shot dial-per-RPC path. The husk server always supports both (a one-shot client that closes after one request and a reused connection that sends several), so this flag only changes the CONTROLLER side and can be canaried + rolled back independently. mTLS identity is verified on every dial and one request is in flight per connection, so a reused connection is neither less authenticated nor frame-interleaved (see docs/threat-model.md). Only meaningful with --enable-husk-pods.") flag.StringVar(&huskStubImage, "husk-stub-image", "mitos-husk-stub:latest", "Container image that runs the dormant-VMM stub in a husk pod. Only used with --enable-husk-pods.") @@ -138,6 +156,15 @@ func main() { // (it has no KVM), so mock implies the raw-forkd path the dev overlay uses. // forkd-the-builder runs regardless in both modes (it builds the snapshots). enableHuskPods, rawForkd := resolveRunMode(enableHuskPods, enableRawForkd, mockMode) + // FAIL FAST on a flag combination that would silently do nothing. The husk stub only + // brings its tap up on the multi-VM Prepare path, so --husk-prepare-egress-link + // without --multi-vm-fork would hand the pool builder MultiVM=false while the claim + // reconciler believed the link was prepared. An operator who asked for a latency + // optimization must not be left with a pod shape that cannot deliver it. + if err := validatePrepareFlags(prepareEgressLink, multiVMFork, enableHuskPods); err != nil { + logger.Error(err, "invalid prepare-time flags") + os.Exit(1) + } if rawForkd { logger.Info("run path: raw-forkd (fork per claim); husk pods disabled", "reason-mock", mockMode, "reason-flag", enableRawForkd) } else { @@ -235,6 +262,7 @@ func main() { LiveCowFork: liveCowFork, LiveCowChildImport: liveCowChildImport, PrewarmChild: prewarmChild, + PrepareEgressLink: prepareEgressLink, HuskStubImage: huskStubImage, HuskDNSUpstream: huskDNSUpstream, KVMResourceName: "mitos.run/kvm", @@ -280,6 +308,7 @@ func main() { HuskConns: huskConnPool, LiveCowChildImport: liveCowChildImport, PrewarmChild: prewarmChild, + PrepareEgressLink: prepareEgressLink, HuskControlPort: huskControlPort, HuskStubImage: huskStubImage, HuskDNSUpstream: huskDNSUpstream, diff --git a/cmd/controller/main_test.go b/cmd/controller/main_test.go index c600f71d..50c5bb43 100644 --- a/cmd/controller/main_test.go +++ b/cmd/controller/main_test.go @@ -33,3 +33,24 @@ func TestResolveRunMode(t *testing.T) { }) } } + +func TestValidatePrepareFlags(t *testing.T) { + cases := []struct { + name string + egress, multiVM, huskPods, wantErr bool + }{ + {"off is fine", false, false, false, false}, + {"egress with multivm and husk pods", true, true, true, false}, + {"egress without multivm", true, false, true, true}, + {"egress without husk pods (raw-forkd path)", true, true, false, true}, + {"egress on raw-forkd and single-vm", true, false, false, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validatePrepareFlags(tc.egress, tc.multiVM, tc.huskPods) + if (err != nil) != tc.wantErr { + t.Errorf("validatePrepareFlags(%v,%v,%v) err=%v, wantErr=%v", tc.egress, tc.multiVM, tc.huskPods, err, tc.wantErr) + } + }) + } +} diff --git a/cmd/husk-stub/main.go b/cmd/husk-stub/main.go index 8d9fc7ef..945b0b6f 100644 --- a/cmd/husk-stub/main.go +++ b/cmd/husk-stub/main.go @@ -193,6 +193,9 @@ func run() error { enableEgress = flag.Bool("enable-egress-filter", true, "Program the in-pod nftables egress filter (default-deny + metadata block) for the activated VM. Requires NET_ADMIN in the pod netns. Default true (the husk isolation guarantee).") multiVM = flag.Bool("multi-vm", false, "EXPERIMENTAL, default false: opt into the multi-VM-per-pod execution mode (#764), running many same-tenant CoW forks inside ONE husk pod instead of one pod per VM. Increment 1 wires only a default-off scaffold; false (the default, and the only value the controller emits) keeps the single-VM behavior byte-for-byte. Not a secret.") liveCowFork = flag.Bool("live-cow-fork", false, "EXPERIMENTAL, default false: opt a CO-LOCATED fork child onto the live copy-on-write path (share the PARENT's resident guest memory via the patched Firecracker memfd + userfaultfd write-protect handler) instead of restoring from the disk fork snapshot (milestone m4b). SEPARATE from --multi-vm so it can be deployed off and canaried independently. Off (the default, and the only value the controller emits unless the operator opts in) keeps the co-located fork on the disk snapshot restore byte-for-byte. No-op off Linux (userfaultfd write-protect is Linux-only), failing closed to the disk restore. Not a secret.") + prepareEgressLink = flag.Bool("prepare-egress-link", false, "EXPERIMENTAL, DEFAULT OFF: bring the pod's default-VM tap up while the pod is DORMANT, under a default-deny policy, so a claim pays only the atomic nft transaction that installs the tenant's policy instead of also paying the tap create (measured: the link half is about two thirds of the egress_filter stage). Requires --multi-vm and --in-pod-guest-ip. Off keeps the whole filter on the activate path byte-for-byte. Not a secret.") + inPodGuestIP = flag.String("in-pod-guest-ip", "", "the fixed in-pod guest IP of this pod's default VM (the same value the controller sends in the activate request). Needed BEFORE that request arrives by --prepare-egress-link, because the tap name derives from it. Config, not a secret.") + inPodGatewayIP = flag.String("in-pod-gateway-ip", "", "the fixed in-pod gateway IP paired with --in-pod-guest-ip. Config, not a secret.") prewarmChild = flag.Bool("prewarm-child", false, "EXPERIMENTAL, DEFAULT OFF: keep ONE dormant generic co-located child Firecracker pre-prepared per multi-vm pod so a fork ADOPTS the ready child (fc_boot=0, prepare off the hot path) instead of booting one at fork time. Requires --multi-vm; a fresh slot re-warms async, a miss falls back to the on-demand prepare. Not a secret.") liveCowChildImport = flag.Bool("live-cow-child-import", false, "EXPERIMENTAL, default false: opt the live-cow fork onto the VMSTATE-ONLY capture (skip the ~364ms disk mem write). REQUIRES a shipped child-side memfd-import Firecracker patch: the co-located child boots its guest RAM from the source shared memfd instead of the disk fork snapshot. The currently shipped patched binary patches the SOURCE (restore) side only, so leave this OFF; with it off an armed --live-cow-fork source still writes the disk mem, so every co-located child restores from disk and the fork never hangs. Turn on ONLY once a child-side import binary is shipped. Not a secret.") ) @@ -437,6 +440,9 @@ func run() error { // the fork never hangs. Turn on ONLY once a child-side import binary ships. LiveCowChildImport: *liveCowChildImport, PrewarmChild: *prewarmChild, + PrepareEgressLink: *prepareEgressLink, + InPodGuestIP: *inPodGuestIP, + InPodGatewayIP: *inPodGatewayIP, }) // Publish the constructed stub to the already-serving metering source. publishedStub.Store(stub) diff --git a/docs/superpowers/plans/2026-07-10-prepare-time-restore.md b/docs/superpowers/plans/2026-07-10-prepare-time-restore.md new file mode 100644 index 00000000..2e181ddf --- /dev/null +++ b/docs/superpowers/plans/2026-07-10-prepare-time-restore.md @@ -0,0 +1,139 @@ +# Prepare-time restore: move the snapshot restore off the warm-claim hot path + +Status: slice 1 implemented (default off, not yet canaried on prod); slices 2 and 3 designed. + +## Why + +A warm claim currently pays the whole microVM restore while a tenant waits. Measured on +prod (`mitos-kvm-1`), one matched claim, controller stage log against the husk stub's own: + +| stage | ms | +|---|---| +| controller `activate_rpc` | 129.7 | +| husk-reported activate total | 112.8 | +| ... `guest_ready` | 40.6 | +| ... `egress_filter` | 29.6 | +| ... `vmstate_restore` | 22.9 | +| ... `handshake` | 17.2 | +| ... `resume` | 2.4 | +| RPC + mTLS overhead between the two | ~17 | + +Under sustained load the same stages mean about 62 ms in total, so treat the numbers +above as one cold-ish sample and the ordering, not the absolute values, as the signal. + +Everything except `handshake` is work the pod could have done while it sat dormant with +no tenant attached. `guest_ready` is largely demand fault-in of guest RAM under the lazy +UFFD restore (see `bench/results/2026-07-09-lazy-livecow-restore.md`), and it is also why +the first `run_code` in a fresh sandbox costs ~105 ms against ~35 ms warm: the ipykernel +is already running in the snapshot (`warmKernel: true`), but its pages are not resident. + +Time to Interactive on the hosted API is 307.7 ms P50, of which create is ~196 ms and the +first exec ~118 ms (`bench/results/2026-07-10-tti-hosted.md`). This is the single largest +lever on both halves. + +## What is actually claim-specific + +Reading `activateInstance` (`internal/husk/multivm.go`), only three things depend on the +claim: + +1. the tenant's netfilter policy (`req.Egress`, `req.Allow`, `req.AllowCIDRs`, + `req.Inbound`, `req.BlockNetwork`); +2. the fork-correctness handshake, which carries fresh entropy, the clock step, the + guest's network re-addressing, and the tenant's secrets (`notifyReq`); +3. `onActivated` (serving the sandbox API with the claim's token). + +Everything else is a function of the pod and its pool template: + +- the tap name is `netconf.DeriveTapName(guestIP)`, and the in-pod guest IP is the FIXED + constant `10.200.0.2` (`huskGuestIP`, `internal/controller/sandboxclaim_controller.go`); +- the snapshot dir and its expected digest are already known at Prepare + (`PrepareSnapshotDir` / `PrepareExpectedDigest`), which is why Prepare already + pre-pays the content-addressed verify; +- the rootfs CoW clone is per-pod; +- the live-cow write-protect arm already happens in `prepareInstance`. + +So the restore itself (`setLiveCowMemSource` -> `LoadSnapshot` -> `PatchDrive` -> +`Resume` -> guest-ready) needs nothing from the claim except a tap to bind the baked NIC +to, and the tap name is knowable at Prepare. + +## Target + +Prepare (dormant, no tenant): + +1. ensure the tap exists and install a DEFAULT-DENY netfilter policy on it (a dormant VM + must reach nothing, fail closed); +2. `setLiveCowMemSource`, `LoadSnapshotWithOverrides` (paused), `PatchDrive` to the CoW + clone, `Resume`; +3. wait for the guest agent (this is where the demand fault-in is paid); +4. optionally prefault the run_code kernel's working set by running one inert cell, the + same trick the template build uses (`warmKernelCode`). + +Activate (claim): + +1. replace the netfilter policy with the tenant's, atomically, on the tap that already + exists (one `nft -f -`, not the tap create plus policy); +2. the fork-correctness handshake (entropy, clock, network, secrets); +3. `onActivated`. + +Expected activate: roughly `nft` + `handshake` + `serve_api`, i.e. ~30 ms against 62 to +113 ms today, plus the first `run_code` arriving warm. + +## Guards, in order of how badly they bite + +- **Snapshot identity.** If `req.SnapshotDir` is not the dir Prepare restored, the pod + has already restored the wrong image. FAIL CLOSED with a named error; never silently + reload, because a resumed VM cannot be reloaded. +- **Fork children.** A co-located fork child restores a node-local fork snapshot through + `spawnVM` -> `prepareInstance` -> `activateInstance` with `req.ForkSnapshot` set. It + must keep the current path exactly. Prepare-time restore applies ONLY to + `defaultVMID` with `PrepareSnapshotDir` set and no fork snapshot. +- **Dormant egress.** Between the restore and the claim, a live guest runs behind a tap. + It must be default-deny for that whole window, and the claim must be able to WIDEN it + atomically without a window where no policy is installed. `nft -f -` replaces the + ruleset in one transaction, which is the property we need. +- **Reseed.** The dormant guest holds the snapshot's CRNG until the claim's + `NotifyForked`. It serves no tenant before then, and the handshake still gates Ready + fail-closed. The dormancy window only makes the clock step larger, and the step is + absolute. +- **Teardown.** `Close` must remove a tap created at Prepare, not only one created at + Activate (`inst.activeTap` is already the seam). +- **Memory.** A restored, resumed, prefaulted dormant VM holds its working set resident: + about 72 MiB per sandbox measured on the lazy path, against ~0 for a dormant VMM that + never loaded. At `warm.min: 8` that is roughly 576 MiB per node. The husk pod memory + request is already sized worst-case (`guestRAM * (1 + reserved forks)`), so this fits, + but it must be stated and watched. +- **Rollout.** Default OFF behind a stub flag. Canary ONE dormant pod (delete a single + dormant husk pod, check `restartCount=0`, claim it, compare stages) before recycling + the pool. `git merge-base --is-ancestor HEAD` before building anything + for prod. + +## Slices + +1. **Tap at Prepare.** DONE, default off. `applyEgressFilter` is split into + `ensureEgressLink` (forwarding, link delete, `ip -batch`) and `applyEgressPolicy` (one + atomic `nft -f -`), and the composed call still runs exactly the same commands in the + same order (pinned by `TestApplyEgressFilterIsTheTwoHalves`). Behind + `--husk-prepare-egress-link` the pod ensures the tap with a default-deny policy while + dormant, and the claim installs the tenant policy on it: exactly one command on the hot + path (`TestPrepareBringsTheTapUpDormantAndActivateOnlyInstallsThePolicy`). A rejected + claim policy tears the tap down and clears the prepared marker, so the retry rebuilds + the link rather than installing a policy on a tap that no longer exists + (`TestAFailedClaimPolicyRebuildsTheLinkOnRetry`). Worth roughly 20 ms, and a + prerequisite for slice 2 because Firecracker requires the tap to exist at restore time. + Requires `--multi-vm-fork`; the controller refuses the combination at startup and the pod + builder refuses it again. Still to do: measure it on prod behind a one-pod canary, then a + KVM gate. Until both, the threat model does NOT call this path verified. +2. **Restore at Prepare.** Move `setLiveCowMemSource` / `LoadSnapshot` / `PatchDrive` / + `Resume` / guest-ready into `prepareInstance`, behind the same flag, with the guards + above. Activate becomes policy + handshake + serve_api. +3. **Prefault the kernel.** Run the inert warm cell at Prepare so the first tenant + `run_code` does not fault the ipykernel's pages in. + +Each slice ships with its own KVM gate in `firecracker-test`, because none of this is +observable without `/dev/kvm` and the mitos-patched Firecracker. + +## Not in scope + +`mark_pod_claimed` (16.5 ms) is the optimistic-lock mutual exclusion that stops two +claims taking one VM. Taking it off the hot path needs a reservation protocol, not a +reordering, and is tracked separately. diff --git a/docs/threat-model.md b/docs/threat-model.md index d4dfe3cc..028d9f4a 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -1219,7 +1219,7 @@ VMMs run jailed under throwaway uids. | Husk pod default ServiceAccount token automount | **mitigated (shipped)** | The husk pod spec now sets `AutomountServiceAccountToken: false` (`internal/controller/huskpod.go` `buildHuskPod`), so the kubelet does NOT mount the namespace default ServiceAccount token into a husk pod. The stub speaks vsock + mTLS and never calls the Kubernetes API (verified: no client-go, no `InClusterConfig`, no SA token read in `cmd/husk-stub` or `internal/husk`), so the token was dead weight; a guest that escapes into the stub no longer inherits a free `system:authenticated` token. The opt-out applies to BOTH warm pods and fork-child pods, which share the builder, proven in `TestBuildHuskPodDisablesSATokenAutomount`. | | forkd gRPC fails OPEN without TLS flags | **mitigated (shipped)** | `grpcServerOptions` (`cmd/forkd/main.go`) now FAILS CLOSED: with no TLS flags it returns a fatal error naming the missing `--tls-cert/--tls-key/--tls-ca` flags and the opt-in, so forkd refuses to start unauthenticated. The legacy insecure-with-loud-warning behavior is reachable only behind an explicit `--allow-insecure-grpc` opt-in (default false) for local development. A partial TLS triple is still a configuration error. The shipped DaemonSet always sets TLS, so production is unaffected; this only stops a silent-insecure misconfig. Proven in `TestGRPCServerOptionsFailClosed` (refuses with no TLS and no opt-in; allowed with the opt-in; allowed with a full TLS triple; partial is an error). | | Host-side vsock read has no deadline | **mitigated (shipped)** | The host-side `vsock.Client` now applies a per-request read deadline in `send` (`internal/vsock/client.go`), defaulting to `DefaultRequestTimeout` (60s) and overridable via `SetRequestTimeout`. The CONNECT preamble reads in `Connect`/`DialStream`/`DialStreamUnix` are likewise bounded (then cleared so the long-lived streaming reads stay ctx/`conn.Close`-cancelled). A malicious or wedged guest that connects then stalls (or dribbles a partial line under the `MaxMessageBytes` cap) now causes the one-shot call to return a timeout error rather than hang the host caller goroutine, vsock fd, and (for the husk stub) stream slot indefinitely. Proven in `TestSendReadDeadlineUnblocksOnStall` (a fake agent that connects then never responds makes `Ping` return rather than hang). | -| Husk egress enforcement | **mitigated (IMPLEMENTED and KVM-VERIFIED end to end)** | See section 0 surface 5 and section 4. The husk default path enforces an in-pod nftables default-deny egress filter in the pod's own netns (CNI-independent, the guarantee), an unconditional cloud-metadata block (`169.254.169.254`, `169.254.0.0/16`, IPv6 `fd00:ec2::254`, before any allow), and the threaded per-template allowlist with a per-pod DNS proxy (failover upstream list, recommended `1.1.1.1:53,8.8.8.8:53`, not cluster DNS); a best-effort controller-emitted NetworkPolicy adds defense in depth (CNI-dependent, cannot express name-based allows). Costs one scoped `NET_ADMIN` capability (ADR 0006) plus, on name-egress pools only, a short-lived privileged `enable-ip-forward` init container that sets `net.ipv4.ip_forward=1` in the pod netns and exits (one-shot; no node prerequisite; the long-lived husk-stub container stays unprivileged). VERIFIED: the husk-network KVM cluster e2e (`test/cluster-e2e/husk-network-e2e.sh`, `cluster-husk-network-e2e`) PASSES on the Hetzner Talos KVM cluster; the claim reaches Ready and all three in-VM enforcement assertions are green inside a real restored VM (metadata-blocked exit 1, default-deny exit 1, allowlist-works `example.com` connect exit 0), verified twice (with and without a node sysctl allowance), proving NO node prerequisite. | +| Husk egress enforcement | **mitigated (IMPLEMENTED and KVM-VERIFIED end to end)** | See section 0 surface 5 and section 4. The husk default path enforces an in-pod nftables default-deny egress filter in the pod's own netns (CNI-independent, the guarantee), an unconditional cloud-metadata block (`169.254.169.254`, `169.254.0.0/16`, IPv6 `fd00:ec2::254`, before any allow), and the threaded per-template allowlist with a per-pod DNS proxy (failover upstream list, recommended `1.1.1.1:53,8.8.8.8:53`, not cluster DNS); a best-effort controller-emitted NetworkPolicy adds defense in depth (CNI-dependent, cannot express name-based allows). Costs one scoped `NET_ADMIN` capability (ADR 0006) plus, on name-egress pools only, a short-lived privileged `enable-ip-forward` init container that sets `net.ipv4.ip_forward=1` in the pod netns and exits (one-shot; no node prerequisite; the long-lived husk-stub container stays unprivileged). VERIFIED: the husk-network KVM cluster e2e (`test/cluster-e2e/husk-network-e2e.sh`, `cluster-husk-network-e2e`) PASSES on the Hetzner Talos KVM cluster; the claim reaches Ready and all three in-VM enforcement assertions are green inside a real restored VM (metadata-blocked exit 1, default-deny exit 1, allowlist-works `example.com` connect exit 0), verified twice (with and without a node sysctl allowance), proving NO node prerequisite. PREPARE-TIME LINK (`--husk-prepare-egress-link`, controller; `--prepare-egress-link`, stub; EXPERIMENTAL, DEFAULT OFF, and **NOT covered by the KVM verification above**: that verification exercised the activation-time filter, and the prepare-time path still needs a production canary and its own KVM gate before it may be called verified; it also requires `--multi-vm-fork`, which the controller enforces at startup and the pod builder enforces again): a warm multi-VM husk pod may bring its default VM's tap up while it is DORMANT so a claim pays only the nft transaction, not the tap create. The dormant tap is created together with a DEFAULT-DENY policy in the same Prepare, and Prepare FAILS CLOSED (the pod never reaches dormant, so the pool never offers it) if either half fails, so a tap never exists without a policy. The claim REPLACES that policy with the tenant's in ONE atomic `nft -f -` transaction, so there is no window in which a VM is unfiltered, and a rejected policy tears the tap down and clears the prepared marker so the retry rebuilds the link rather than installing a policy on a tap that no longer exists. Nothing is loaded behind a dormant tap today (the snapshot restore stays on the activate path), and the pod netns is not shared with any tenant. Off, the filter path is byte-for-byte the activate-time one. | | agents.x-k8s.io facade admission surface (v1beta1 conformance, #357) | **mitigated (bounded translation, no host surface)** | The conformance facade (`internal/facade`, opt-in deployment) watches tenant-authored upstream `agents.x-k8s.io/v1beta1` and `extensions.agents.x-k8s.io/v1beta1` custom resources (Sandbox, SandboxTemplate, SandboxWarmPool, SandboxClaim) and TRANSLATES each into our `mitos.run/v1` SandboxPool/Sandbox objects in the SAME namespace, owner-referenced to the upstream CR for GC. It creates only namespace-scoped objects and grants no cross-namespace or host access; the run-path security boundary is unchanged (the bridged Sandbox is governed by the husk-pod and sandbox-API controls above, not by the facade). Tenant-controlled upstream fields are BOUNDED: `spec.operatingMode` (Running/Suspended) drives only release/re-activate of the bridged claim; `SandboxClaim.spec.warmPoolRef.name` is a name lookup of OUR pool of that name (no path or host input); `spec.podTemplate.spec.containers[0].env` (including `env.valueFrom` references) is mirrored through by reference and never logged; all other podTemplate fields and `volumeClaimTemplates` are UNMAPPED (no tenant-driven volume provisioning or host-shaped field reaches our engine via the facade, see docs/facade-conformance.md exceptions 1, 3, 4, 5). The v0.5.0 migration to v1beta1 REDUCED the input surface: it removed the v1alpha1 `warmpool` policy resolution branch (none/default/named), so a claim now references a warm pool by name with no policy logic to abuse. Test-infra note: v1beta1 is the storage version with a `conversion.strategy: Webhook` we do not run; the kind conformance job pins the live CRD conversion to None at install time and exercises only the storage version (the vendored CRD on disk is unchanged). Residual: the facade trusts cluster RBAC that admits the upstream CRs, so whoever may create an `agents.x-k8s.io` Sandbox in a namespace can drive a pool binding there; the same-namespace tenancy boundary (section 7a) applies, and the facade adds no privilege beyond it. | | Control-plane egress lockdown (chart profile, issue #704) | **mitigated (opt-in)** | The chart ships an opt-in default-deny EGRESS profile for the control plane (`networkPolicy.controlPlane.enabled`, `templates/control-plane-netpol.yaml`): a CiliumNetworkPolicy selecting the controller, console, and gateway that enumerates every legitimate flow (DNS via kube-dns with L7 visibility, the kube-apiserver/host/remote-node entities, intra-namespace) and denies the rest, bounding what a COMPROMISED control-plane component can reach (no lateral movement into other namespaces, no arbitrary internet egress for exfiltration). When `console.onboarding.smtp.host` is set, a SECOND policy scoped to the console ALONE additionally allows that SMTP host and port; it is DERIVED from the same values that configure the sender, so the network allow cannot drift from the sender config (the drift class that silently broke hosted signups: SMTP env present, egress absent, every verification send timed out). The SMTP policy never renders without the baseline profile (a lone egress policy would itself flip the console to default-deny), and the controller and gateway gain no mail-relay egress (a compromised controller cannot exfiltrate over SMTP). `extraEgress` passes deployment-specific rules through verbatim (e.g. an in-cluster OIDC issuer namespace); rules added there widen the boundary and are the operator's responsibility. Requires the Cilium CNI (CiliumNetworkPolicy CRD), consistent with the NetworkPolicy-enforcing-CNI requirement stated for the tenancy boundary. Default OFF: a plain install is unchanged. Rendering is asserted in `deploy/charts/charttest/netpol_test.go` (opt-in absent by default, baseline flows present, SMTP allow derived and console-scoped, all-or-nothing coupling). Residual: this is an egress control only (ingress lockdown for the control plane is separate work), and on a non-Cilium CNI the profile cannot be expressed (values accepted but objects unusable; the CRD requirement is documented). | diff --git a/internal/controller/huskpod.go b/internal/controller/huskpod.go index 7c9a8dd5..48466d71 100644 --- a/internal/controller/huskpod.go +++ b/internal/controller/huskpod.go @@ -200,6 +200,11 @@ type HuskPodOptions struct { // one dormant generic co-located child Firecracker pre-prepared and a fork adopts // it (fc_boot off the hot path). DEFAULT OFF; requires MultiVM. PrewarmChild bool + // PrepareEgressLink starts the husk stub with --prepare-egress-link (plus the + // in-pod link addresses it needs before an activate request arrives), so a warm + // pod brings its tap up while dormant and a claim pays only the nft transaction + // that installs the tenant's policy. Requires MultiVM. Default off. + PrepareEgressLink bool // MultiVMForkVMs is the number of ADDITIONAL fork-child VMs a multi-VM pod // reserves node memory for up front (beyond the source VM), so the co-location // routing has room to co-locate that many children before a fork spills to a new @@ -563,6 +568,20 @@ func (r *SandboxPoolReconciler) buildHuskPod(pool *v1.SandboxPool, template *v1. if opts.PrewarmChild { args = append(args, "--prewarm-child") } + // Gated on MultiVM as documented: the stub only brings the tap up on the multi-VM + // Prepare path, so emitting these to a single-VM stub would produce an unsupported + // pod shape whose flags do nothing. main rejects the flag combination outright; this + // is the second gate, at the boundary that actually builds the pod. + if opts.PrepareEgressLink && opts.MultiVM { + // The stub needs the in-pod link BEFORE a claim arrives, because the tap name + // derives from the guest IP. These are the same fixed values huskNotifyNetwork + // sends in the activate request; they are config, not secrets. + args = append(args, + "--prepare-egress-link", + "--in-pod-guest-ip", huskGuestIP, + "--in-pod-gateway-ip", huskGatewayIP, + ) + } // Live-cow write-protect needs a KERNEL-MODE userfaultfd over the guest RAM: the // source Firecracker registers UFFD_WP so the KVM guest's own writes fault to the @@ -1396,6 +1415,7 @@ func (r *SandboxPoolReconciler) reconcileHuskPods(ctx context.Context, pool *v1. LiveCowFork: r.LiveCowFork, LiveCowChildImport: r.LiveCowChildImport, PrewarmChild: r.PrewarmChild, + PrepareEgressLink: r.PrepareEgressLink, MultiVMForkVMs: r.MultiVMForkVMs, StubImage: r.HuskStubImage, DNSUpstream: r.HuskDNSUpstream, diff --git a/internal/controller/huskpod_test.go b/internal/controller/huskpod_test.go index 0f2b39fa..1d884b4d 100644 --- a/internal/controller/huskpod_test.go +++ b/internal/controller/huskpod_test.go @@ -1506,3 +1506,53 @@ func TestReconcileHuskPodsThreadsMultiVM(t *testing.T) { } } } + +// TestBuildHuskPodThreadsPrepareEgressLink proves the prepare-time link flag reaches the +// stub together with the in-pod addresses it cannot work without: the tap name derives +// from the guest IP, and the stub needs it BEFORE an activate request arrives. Off (the +// default) the pod is byte-for-byte the current activate-time filter. +func TestBuildHuskPodThreadsPrepareEgressLink(t *testing.T) { + r := &controller.SandboxPoolReconciler{} + pool := &v1.SandboxPool{ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "ns"}} + tmpl := &v1.PoolTemplateSpec{} + + on := r.BuildHuskPodForTest(pool, tmpl, controller.HuskPodOptions{StubImage: "img", MultiVM: true, PrepareEgressLink: true}) + args := on.Spec.Containers[0].Args + for _, want := range []string{"--prepare-egress-link", "--in-pod-guest-ip", "--in-pod-gateway-ip"} { + if !argsContain(args, want) { + t.Errorf("husk args missing %s when prepare-egress-link is enabled: %v", want, args) + } + } + // The addresses must be the ones the activate request carries, or the stub would + // bring up a tap the claim never resolves to and the skip would silently not apply. + if !argsContain(args, "10.200.0.2") || !argsContain(args, "10.200.0.1") { + t.Errorf("husk args do not carry the fixed in-pod /30: %v", args) + } + + off := r.BuildHuskPodForTest(pool, tmpl, controller.HuskPodOptions{StubImage: "img", MultiVM: true}) + for _, unwanted := range []string{"--prepare-egress-link", "--in-pod-guest-ip", "--in-pod-gateway-ip"} { + if argsContain(off.Spec.Containers[0].Args, unwanted) { + t.Errorf("husk args must omit %s by default: %v", unwanted, off.Spec.Containers[0].Args) + } + } +} + +// TestBuildHuskPodOmitsPrepareEgressLinkWithoutMultiVM: the option is documented as +// requiring MultiVM, and the stub only brings its tap up on the multi-VM Prepare path. +// Emitting the flags to a single-VM stub would produce an unsupported pod shape whose +// arguments do nothing. cmd/controller rejects the combination outright; this pins the +// second gate, at the boundary that actually builds the pod. +func TestBuildHuskPodOmitsPrepareEgressLinkWithoutMultiVM(t *testing.T) { + r := &controller.SandboxPoolReconciler{} + pool := &v1.SandboxPool{ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "ns"}} + tmpl := &v1.PoolTemplateSpec{} + + pod := r.BuildHuskPodForTest(pool, tmpl, controller.HuskPodOptions{ + StubImage: "img", MultiVM: false, PrepareEgressLink: true, + }) + for _, unwanted := range []string{"--prepare-egress-link", "--in-pod-guest-ip", "--in-pod-gateway-ip"} { + if argsContain(pod.Spec.Containers[0].Args, unwanted) { + t.Errorf("husk args carry %s without --multi-vm: %v", unwanted, pod.Spec.Containers[0].Args) + } + } +} diff --git a/internal/controller/sandboxclaim_controller.go b/internal/controller/sandboxclaim_controller.go index 04b600e3..9202470c 100644 --- a/internal/controller/sandboxclaim_controller.go +++ b/internal/controller/sandboxclaim_controller.go @@ -155,6 +155,10 @@ type SandboxReconciler struct { // PrewarmChild passes --prewarm-child to warm husk pods so a fork adopts a // pre-warmed dormant child. DEFAULT OFF; requires MultiVMFork. PrewarmChild bool + // PrepareEgressLink passes --prepare-egress-link to warm husk pods so a dormant + // pod brings its default VM's tap up before any claim arrives, leaving only the + // atomic nft policy transaction on the warm-claim hot path. Requires MultiVMFork. + PrepareEgressLink bool // spawnVM is the controller->husk spawn-vm seam used by the MultiVMFork routing. // Nil defaults to SpawnVMOnHusk; tests inject a fake. diff --git a/internal/controller/sandboxpool_controller.go b/internal/controller/sandboxpool_controller.go index 546ddbfb..960805f8 100644 --- a/internal/controller/sandboxpool_controller.go +++ b/internal/controller/sandboxpool_controller.go @@ -58,6 +58,10 @@ type SandboxPoolReconciler struct { // PrewarmChild passes --prewarm-child to warm husk pods so a fork adopts a // pre-warmed dormant child. DEFAULT OFF; requires MultiVM. PrewarmChild bool + // PrepareEgressLink passes --prepare-egress-link to warm husk pods so a dormant + // pod brings its default VM's tap up before any claim arrives, leaving only the + // atomic nft policy transaction on the warm-claim hot path. Requires MultiVMFork. + PrepareEgressLink bool // MultiVMForkVMs is how many co-located fork VMs a multi-VM warm pod reserves // node memory for up front (beyond the source VM), so the co-location routing // has room before a fork spills to a new pod. Only consulted when MultiVM is set; diff --git a/internal/husk/multivm.go b/internal/husk/multivm.go index 42407a36..d0e46941 100644 --- a/internal/husk/multivm.go +++ b/internal/husk/multivm.go @@ -13,6 +13,7 @@ import ( "strings" "time" + v1 "mitos.run/mitos/api/v1" "mitos.run/mitos/internal/cas" "mitos.run/mitos/internal/firecracker" "mitos.run/mitos/internal/fork" @@ -371,10 +372,56 @@ func (s *Stub) prepareInstanceOpt(ctx context.Context, id vmID, opts prepareOpts } } + // Bring THIS pod's default-VM tap up while it is dormant, under a DEFAULT-DENY + // policy, so the claim pays only the atomic nft transaction that installs the + // tenant's policy. Opt-in (Options.PrepareEgressLink), default VM only, and only + // when the in-pod link is configured; otherwise activate does the whole thing as + // before. FAIL CLOSED: ensureEgressLink tears the tap down on any error and we + // refuse to go dormant, so the pool never offers a pod whose datapath is + // half-built. A dormant tap is never a hazard: nothing is loaded behind it, and + // the policy on it denies everything. + if err := s.prepareEgressLinkFor(ctx, id, inst); err != nil { + if inst.vm != nil { + _ = inst.vm.Close() + inst.vm = nil + } + return err + } + inst.state = StateDormant return nil } +// prepareEgressLinkFor brings up the default VM's tap with a default-deny policy while +// the pod is dormant, when the stub opted in. A no-op otherwise, and never for a +// secondary (co-located fork child) VM, whose tap is derived per fork at activate. +func (s *Stub) prepareEgressLinkFor(ctx context.Context, id vmID, inst *vmInstance) error { + // !s.multiVM is implied today (only the multi-VM Prepare reaches prepareInstance), + // but it is stated rather than relied upon: the single-VM Activate path does not + // consult preparedLinkTap, so a dormant tap it never reuses would be dead weight. + if !s.prepareEgressLink || !s.multiVM || id != defaultVMID || s.netRunner == nil || s.inPodGuestIP == "" { + return nil + } + tap := netconf.DeriveTapName(s.inPodGuestIP) + cfg := NetfilterConfig{ + Tap: tap, + GuestIP: net.ParseIP(s.inPodGuestIP), + HostIP: net.ParseIP(s.inPodGatewayIP), + Egress: v1.EgressDeny, + } + if err := ensureEgressLink(ctx, s.netRunner, s.enableForwarding, cfg); err != nil { + return fmt.Errorf("husk: prepare in-pod egress link for vm %q: %w", id, err) + } + if err := applyEgressPolicy(ctx, s.netRunner, cfg); err != nil { + return fmt.Errorf("husk: prepare default-deny egress policy for vm %q: %w", id, err) + } + // Record it on BOTH fields: activeTap so Close tears it down even if this pod is + // never claimed, preparedLinkTap so activate knows it can skip the link setup. + inst.activeTap = tap + inst.preparedLinkTap = tap + return nil +} + // cloneRootfsForInstance makes THIS VM's per-activation rootfs CoW clone and // records it on the instance so activate rebinds the drive to it and Close removes // it. The clone SOURCE is the pool template rootfs by default; a co-located fork @@ -508,8 +555,24 @@ func (s *Stub) activateInstance(ctx context.Context, id vmID, req ActivateReques cfg.Tap = tap cfg.GuestIP = net.ParseIP(perNet.GuestIP) cfg.HostIP = net.ParseIP(perNet.GatewayIP) - if err := applyEgressFilter(ctx, s.netRunner, s.enableForwarding, cfg); err != nil { - werr := fmt.Errorf("husk: apply in-pod egress filter for vm %q: %w", id, err) + // When this instance brought the SAME tap up while dormant, the claim pays only + // the nft transaction that REPLACES the default-deny policy with the tenant's. + // nft applies it atomically, so there is no window in which the VM is + // unfiltered. Any other tap (a fork child's, or a pod that did not opt in) takes + // the full path. Both halves fail closed by tearing the tap down, so clear the + // prepared marker first: after a failure the link no longer exists and the next + // attempt must rebuild it. + prepared := inst.preparedLinkTap != "" && inst.preparedLinkTap == tap + inst.preparedLinkTap = "" + var ferr error + if prepared { + ferr = applyEgressPolicy(ctx, s.netRunner, cfg) + } else { + ferr = applyEgressFilter(ctx, s.netRunner, s.enableForwarding, cfg) + } + if ferr != nil { + inst.activeTap = "" + werr := fmt.Errorf("husk: apply in-pod egress filter for vm %q: %w", id, ferr) return ActivateResult{OK: false, Error: werr.Error()}, werr } // Pin the baked NIC to THIS VM's tap so the restored VM's NIC is backed by diff --git a/internal/husk/multivm_network_test.go b/internal/husk/multivm_network_test.go index 3f560179..d598b7f8 100644 --- a/internal/husk/multivm_network_test.go +++ b/internal/husk/multivm_network_test.go @@ -208,3 +208,141 @@ func TestMultiVMActivateProgramsDistinctTapPerVMID(t *testing.T) { t.Errorf("the two VMs must be handed distinct guest IPs, got %v", seen) } } + +// --- prepare-time egress link ------------------------------------------------------ + +// countIPBatch counts the ip -batch (tap create) invocations. +func countIPBatch(calls []recordedCall) int { + n := 0 + for _, c := range calls { + if len(c.argv) > 0 && strings.Contains(c.argv[0], "ip") && strings.Contains(strings.Join(c.argv, " "), "-batch") { + n++ + } + } + return n +} + +func countNft(calls []recordedCall) int { + n := 0 + for _, c := range calls { + if len(c.argv) > 0 && strings.Contains(c.argv[0], "nft") { + n++ + } + } + return n +} + +// newPreparedLinkStub builds a multi-VM stub that brings its default VM's tap up while +// dormant, over a recording net runner. +func newPreparedLinkStub(t *testing.T, rr *recordingRunner) *Stub { + t.Helper() + start := func(_ firecracker.VMConfig) (vmm, error) { return &fakeVMM{}, nil } + s := New(firecracker.VMConfig{ID: "husk-test"}, Options{ + Start: start, + Ready: readyOK, + Notify: (&fakeNotifier{}).notify, + Verify: verifyOK, + MultiVM: true, + PrepareEgressLink: true, + InPodGuestIP: "10.200.0.2", + InPodGatewayIP: "10.200.0.1", + }) + s.SetNetRunner(rr.run) + return s +} + +// TestPrepareBringsTheTapUpDormantAndActivateOnlyInstallsThePolicy is the point of the +// split. A pod that opted in pays the tap create while it is DORMANT, so the claim's +// egress_filter stage is one atomic nft transaction and nothing else. The dormant tap +// carries a default-deny policy, so the VM behind it is never unfiltered. +func TestPrepareBringsTheTapUpDormantAndActivateOnlyInstallsThePolicy(t *testing.T) { + var rr recordingRunner + s := newPreparedLinkStub(t, &rr) + ctx := context.Background() + + if err := s.Prepare(ctx); err != nil { + t.Fatalf("Prepare: %v", err) + } + if got := countIPBatch(rr.calls); got != 1 { + t.Errorf("Prepare ran %d tap creates, want exactly 1", got) + } + if got := countNft(rr.calls); got != 1 { + t.Errorf("Prepare installed %d policies, want exactly 1 (the dormant default-deny)", got) + } + tap := netconf.DeriveTapName("10.200.0.2") + if !strings.Contains(strings.Join(rr.calls[0].argv, " "), tap) { + t.Errorf("Prepare did not touch the pod's tap %q: %v", tap, rr.calls[0].argv) + } + + prepared := len(rr.calls) + if _, err := s.Activate(ctx, ActivateRequest{SnapshotDir: "/snap", Network: baseNet()}); err != nil { + t.Fatalf("Activate: %v", err) + } + claim := rr.calls[prepared:] + if got := countIPBatch(claim); got != 0 { + t.Errorf("the claim re-created the tap %d times; the dormant pod already brought it up", got) + } + if got := countNft(claim); got != 1 { + t.Errorf("the claim ran %d nft transactions, want exactly 1 (replace the policy): %v", got, claim) + } + if len(claim) != 1 { + t.Errorf("the claim ran %d commands, want exactly 1: %v", len(claim), claim) + } +} + +// TestPrepareTimeLinkIsOptIn: a stub that did not opt in must behave byte-for-byte as +// before, doing all of it at activate. +func TestPrepareTimeLinkIsOptIn(t *testing.T) { + var rr recordingRunner + start := func(_ firecracker.VMConfig) (vmm, error) { return &fakeVMM{}, nil } + s := New(firecracker.VMConfig{ID: "husk-test"}, Options{ + Start: start, Ready: readyOK, Notify: (&fakeNotifier{}).notify, Verify: verifyOK, + MultiVM: true, + }) + s.SetNetRunner(rr.run) + ctx := context.Background() + + if err := s.Prepare(ctx); err != nil { + t.Fatalf("Prepare: %v", err) + } + if len(rr.calls) != 0 { + t.Fatalf("Prepare touched the network without opting in: %v", rr.calls) + } + if _, err := s.Activate(ctx, ActivateRequest{SnapshotDir: "/snap", Network: baseNet()}); err != nil { + t.Fatalf("Activate: %v", err) + } + if got := countIPBatch(rr.calls); got != 1 { + t.Errorf("activate ran %d tap creates, want 1", got) + } + if got := countNft(rr.calls); got != 1 { + t.Errorf("activate ran %d nft transactions, want 1", got) + } +} + +// TestAFailedClaimPolicyRebuildsTheLinkOnRetry: applyEgressPolicy fails closed by +// tearing the tap down, so the prepared marker must be cleared. Otherwise the retry +// would install a policy on a tap that no longer exists and the pod would be poisoned. +func TestAFailedClaimPolicyRebuildsTheLinkOnRetry(t *testing.T) { + var rr recordingRunner + s := newPreparedLinkStub(t, &rr) + ctx := context.Background() + if err := s.Prepare(ctx); err != nil { + t.Fatalf("Prepare: %v", err) + } + + inst := s.instanceFor(defaultVMID, false) + if inst == nil || inst.preparedLinkTap == "" { + t.Fatal("Prepare did not record the prepared tap") + } + + // A claim whose policy is rejected: the CIDR list cannot be parsed. + if _, err := s.Activate(ctx, ActivateRequest{SnapshotDir: "/snap", Network: baseNet(), AllowCIDRs: []string{"not-a-cidr"}}); err == nil { + t.Fatal("Activate accepted a malformed CIDR allowlist") + } + if inst.preparedLinkTap != "" { + t.Error("a failed claim left the prepared-link marker set; the retry would skip rebuilding a torn-down tap") + } + if inst.activeTap != "" { + t.Error("a failed claim left activeTap set for a tap that was torn down") + } +} diff --git a/internal/husk/netfilter.go b/internal/husk/netfilter.go index 4ee8b533..54837231 100644 --- a/internal/husk/netfilter.go +++ b/internal/husk/netfilter.go @@ -81,15 +81,37 @@ type netfilterRunner func(ctx context.Context, argv []string, stdin string) erro // `nft -f` that applies the shared table, this VM's per-tap chain, the SNAT, the // shared input table, and this tap's input chain. A malformed allowlist fails // the whole call (fail-closed: a VM never comes up with a half-applied filter). +// applyEgressFilter brings up this VM's tap and installs its policy, fail-closed. +// +// It is the composition of the two halves below, and it stays the ONLY entry point for +// callers that have no prepared link: the two are split so a warm husk pod can pay the +// link half while it is dormant (no tenant attached) and leave only the policy half on +// the claim's hot path. Measured on prod, the link half is roughly two thirds of the +// ~30 ms this call costs at activate. func applyEgressFilter(ctx context.Context, run netfilterRunner, enableForwarding func() error, cfg NetfilterConfig) (err error) { - enforceable, _, err := netconf.SplitAllowList(cfg.Allow) - if err != nil { + // Reject a malformed allowlist BEFORE creating anything, so a bad policy never + // costs a tap create and teardown, and the caller sees the parse error rather than + // a later one. applyEgressPolicy re-parses; parsing is pure and cheap. + if _, _, err := netconf.SplitAllowList(cfg.Allow); err != nil { return fmt.Errorf("husk netfilter: parse allowlist: %w", err) } - cidrV4, cidrV6, err := netconf.ParseCIDRList(cfg.AllowCIDRs) - if err != nil { + if _, _, err := netconf.ParseCIDRList(cfg.AllowCIDRs); err != nil { return fmt.Errorf("husk netfilter: parse CIDR allowlist: %w", err) } + if err := ensureEgressLink(ctx, run, enableForwarding, cfg); err != nil { + return err + } + return applyEgressPolicy(ctx, run, cfg) +} + +// ensureEgressLink enables forwarding and (re)creates this VM's tap with its host /30. +// It installs NO policy: a tap that exists without a policy carries no traffic, because +// nothing is loaded behind it yet and the shared forward table's default is a drop. +// +// FAIL CLOSED and idempotent: any error tears the tap back down before returning, and a +// pre-existing tap of the same name is removed first (the name is deterministic per +// template, so a leaked tap would otherwise poison the pod with EBUSY forever, #428). +func ensureEgressLink(ctx context.Context, run netfilterRunner, enableForwarding func() error, cfg NetfilterConfig) (err error) { // IPv4 forwarding in the pod netns: the kernel will not route the guest /30 // between the tap and the pod uplink without it, so the SNAT below would have // nothing to NAT. Nil seam (tests) skips it. Done first so a failure aborts @@ -111,11 +133,11 @@ func applyEgressFilter(ctx context.Context, run netfilterRunner, enableForwardin // ip -batch invocation may create the tap and then fail on a LATER line in the // SAME process, which would leak the tap: its name is deterministic per // template, so a leak makes the next activation fail right at tap creation - // with EBUSY (Device or resource busy), masking the real first-attempt error - // and permanently poisoning the warm pod (issue #428). Tearing down on every - // error path removes a partially created tap (link del of an absent tap is an - // ignored no-op), and the original error is preserved as the named return so - // the true root-cause step error is surfaced, not the EBUSY of a later retry. + // with EBUSY, masking the real first-attempt error and permanently poisoning + // the warm pod (issue #428). Tearing down on every error path removes a + // partially created tap (link del of an absent tap is an ignored no-op), and + // the original error is preserved as the named return so the true root-cause + // step error is surfaced, not the EBUSY of a later retry. defer func() { if err != nil { // Detached from ctx so cleanup still runs when the failure was a @@ -133,6 +155,30 @@ func applyEgressFilter(ctx context.Context, run netfilterRunner, enableForwardin if err := run(ctx, netconf.IPBatchArgs(), netconf.RenderIPBatch(cfg.Tap, cfg.HostIP, cfg.ResolverIP)); err != nil { return fmt.Errorf("husk netfilter: set up tap %s: %w", cfg.Tap, err) } + return nil +} + +// applyEgressPolicy installs (or REPLACES) this VM's netfilter policy on a tap that +// already exists. It is one nft process applying one atomic transaction, so a claim can +// widen a dormant VM's default-deny policy to its tenant posture with no window in +// which the VM is unfiltered, and a malformed allowlist installs NOTHING. +// +// FAIL CLOSED: on any error the tap is torn down, exactly as a full applyEgressFilter +// would, so a VM never comes up half-filtered. +func applyEgressPolicy(ctx context.Context, run netfilterRunner, cfg NetfilterConfig) (err error) { + enforceable, _, err := netconf.SplitAllowList(cfg.Allow) + if err != nil { + return fmt.Errorf("husk netfilter: parse allowlist: %w", err) + } + cidrV4, cidrV6, err := netconf.ParseCIDRList(cfg.AllowCIDRs) + if err != nil { + return fmt.Errorf("husk netfilter: parse CIDR allowlist: %w", err) + } + defer func() { + if err != nil { + _ = teardownEgressFilter(context.WithoutCancel(ctx), run, cfg.Tap) + } + }() // One nft process instead of ~5, applied as a SINGLE atomic transaction: // - the pod-global shared forward table (idempotent skeleton), // - this VM's per-tap egress chain (metadata drop + allowlist + counter), diff --git a/internal/husk/netfilter_test.go b/internal/husk/netfilter_test.go index 2436fcc2..4b851bec 100644 --- a/internal/husk/netfilter_test.go +++ b/internal/husk/netfilter_test.go @@ -341,3 +341,118 @@ func TestApplyEgressFilterBatchesNftIntoOneTransaction(t *testing.T) { t.Errorf("shared table must precede the per-sandbox chain in the batched payload (shared=%d chain=%d)", sharedIdx, chainIdx) } } + +// --- prepare-time link, claim-time policy ----------------------------------------- + +// isNftApply reports whether argv invokes nft. +func isNftApply(argv []string) bool { + return len(argv) > 0 && strings.Contains(argv[0], "nft") +} + +// isIPCommand reports whether argv invokes ip (tap create / link delete). +func isIPCommand(argv []string) bool { + return len(argv) > 0 && strings.Contains(argv[0], "ip") +} + +// TestApplyEgressFilterIsTheTwoHalves pins that splitting the call did not change what +// it runs: the composed call still issues exactly the link commands and then the single +// atomic nft transaction. +func TestApplyEgressFilterIsTheTwoHalves(t *testing.T) { + cfg := NetfilterConfig{Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), Egress: v1.EgressDeny} + + var whole recordingRunner + if err := applyEgressFilter(context.Background(), whole.run, func() error { return nil }, cfg); err != nil { + t.Fatalf("applyEgressFilter: %v", err) + } + + var halves recordingRunner + if err := ensureEgressLink(context.Background(), halves.run, func() error { return nil }, cfg); err != nil { + t.Fatalf("ensureEgressLink: %v", err) + } + if err := applyEgressPolicy(context.Background(), halves.run, cfg); err != nil { + t.Fatalf("applyEgressPolicy: %v", err) + } + + if len(whole.calls) != len(halves.calls) { + t.Fatalf("composed call ran %d commands, the two halves ran %d", len(whole.calls), len(halves.calls)) + } + for i := range whole.calls { + if strings.Join(whole.calls[i].argv, " ") != strings.Join(halves.calls[i].argv, " ") { + t.Errorf("command %d differs: composed %v, halves %v", i, whole.calls[i].argv, halves.calls[i].argv) + } + if whole.calls[i].stdin != halves.calls[i].stdin { + t.Errorf("command %d stdin differs", i) + } + } +} + +// TestApplyEgressPolicyTouchesOnlyNft is the point of the split: a claim that lands on a +// pod whose tap was already brought up while it was dormant must pay ONE nft transaction +// and no ip commands. The ip half is roughly two thirds of the ~30 ms this costs today. +func TestApplyEgressPolicyTouchesOnlyNft(t *testing.T) { + cfg := NetfilterConfig{Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), Egress: v1.EgressDeny} + + var rr recordingRunner + if err := applyEgressPolicy(context.Background(), rr.run, cfg); err != nil { + t.Fatalf("applyEgressPolicy: %v", err) + } + if len(rr.calls) != 1 { + t.Fatalf("applyEgressPolicy ran %d commands, want exactly 1 (the nft transaction): %v", len(rr.calls), rr.calls) + } + if !isNftApply(rr.calls[0].argv) { + t.Errorf("the one command is not nft: %v", rr.calls[0].argv) + } + for _, c := range rr.calls { + if isIPCommand(c.argv) { + t.Errorf("applyEgressPolicy must not touch the link: %v", c.argv) + } + } +} + +// TestApplyEgressPolicyFailsClosed: a rejected policy must leave no tap behind, exactly +// as the composed call does, because a VM must never run half-filtered. +func TestApplyEgressPolicyFailsClosed(t *testing.T) { + cfg := NetfilterConfig{Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), Egress: v1.EgressDeny} + rr := &failingRunner{failAt: 0, err: errors.New("nft: syntax error")} + + if err := applyEgressPolicy(context.Background(), rr.run, cfg); err == nil { + t.Fatal("applyEgressPolicy accepted a rejected nft transaction") + } + if got := countTapDelete(rr.calls, "tap0"); got == 0 { + t.Error("a rejected policy left the tap behind; it must be torn down") + } +} + +// TestEnsureEgressLinkInstallsNoPolicy: the dormant half must not install a ruleset. A +// tap with no policy carries no traffic (nothing is loaded behind it and the shared +// forward table defaults to a drop); a tap with the WRONG policy would. +func TestEnsureEgressLinkInstallsNoPolicy(t *testing.T) { + cfg := NetfilterConfig{Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), Egress: v1.EgressDeny} + + var rr recordingRunner + if err := ensureEgressLink(context.Background(), rr.run, func() error { return nil }, cfg); err != nil { + t.Fatalf("ensureEgressLink: %v", err) + } + for _, c := range rr.calls { + if isNftApply(c.argv) { + t.Errorf("ensureEgressLink applied a policy: %v", c.argv) + } + } +} + +// TestApplyEgressFilterRejectsABadAllowlistBeforeTouchingTheLink: a malformed policy +// must cost nothing. It used to be rejected before any command ran; splitting the call +// must not start creating taps for a policy that can never be installed. +func TestApplyEgressFilterRejectsABadAllowlistBeforeTouchingTheLink(t *testing.T) { + cfg := NetfilterConfig{ + Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), + Egress: v1.EgressDeny, AllowCIDRs: []string{"not-a-cidr"}, + } + var rr recordingRunner + if err := applyEgressFilter(context.Background(), rr.run, func() error { return nil }, cfg); err == nil { + t.Fatal("applyEgressFilter accepted a malformed CIDR allowlist") + } + if len(rr.calls) != 0 { + t.Errorf("a malformed allowlist ran %d commands, want none: %v", len(rr.calls), rr.calls) + } +} diff --git a/internal/husk/stub.go b/internal/husk/stub.go index 20ff203c..5589e63f 100644 --- a/internal/husk/stub.go +++ b/internal/husk/stub.go @@ -99,6 +99,11 @@ type vmInstance struct { prepareVerified bool rootfsClonePath string activeTap string + // preparedLinkTap names the tap this instance brought up while DORMANT + // (Options.PrepareEgressLink). When it matches the tap the claim resolves to, + // activate installs the tenant policy on it and skips the link setup. Cleared + // whenever the tap is torn down, so a retry re-ensures it. + preparedLinkTap string dnsProxy *dnsproxy.Server // childUFFDPlan carries a co-located live-cow fork child's LAZY UFFD import @@ -605,6 +610,23 @@ type Options struct { // Independent of LiveCowFork so the source can be armed (freezer live) without // yet skipping the disk mem. LiveCowChildImport bool + // PrepareEgressLink opts a multi-VM pod into bringing up its default VM's tap + // while the pod is DORMANT (no tenant attached), so a claim pays only the atomic + // nft transaction that installs the tenant's policy instead of also paying the + // tap create. Measured on prod, the link half is roughly two thirds of the ~30 ms + // the egress_filter stage costs on the warm-claim hot path. + // + // DEFAULTS false. Requires MultiVM and InPodGuestIP; a no-op otherwise, so a pod + // that does not opt in behaves byte-for-byte as before. The dormant tap carries a + // DEFAULT-DENY policy, and the claim REPLACES it in one nft transaction, so there + // is never a window in which a VM is unfiltered. + PrepareEgressLink bool + // InPodGuestIP / InPodGatewayIP are the fixed in-pod /30 the pod's default VM + // uses. They are the same values the controller sends in the activate request; + // PrepareEgressLink needs them BEFORE that request arrives, because the tap name + // derives from the guest IP. Config, never secrets. + InPodGuestIP string + InPodGatewayIP string // PrewarmChild opts into keeping ONE dormant, generic co-located child // Firecracker pre-prepared (booted, and snapshot-verified when a template // snapshot is configured) in a multi-VM pod, so a co-located fork that does @@ -728,6 +750,11 @@ type Stub struct { // restores from the disk fork snapshot until a child-side memfd-import // Firecracker patch ships) is always restorable and the fork never hangs. liveCowChildImport bool + + // prepareEgressLink / inPodGuestIP / inPodGatewayIP back Options.PrepareEgressLink. + prepareEgressLink bool + inPodGuestIP string + inPodGatewayIP string // prewarmChild keeps ONE dormant, generic co-located child Firecracker // pre-prepared (Options.PrewarmChild) so a co-located fork that needs no // fork-specific launch env activates it instead of paying the process boot on @@ -805,6 +832,9 @@ func New(cfg firecracker.VMConfig, opts Options) *Stub { multiVM: opts.MultiVM, liveCowFork: opts.LiveCowFork, liveCowChildImport: opts.LiveCowChildImport, + prepareEgressLink: opts.PrepareEgressLink, + inPodGuestIP: opts.InPodGuestIP, + inPodGatewayIP: opts.InPodGatewayIP, prewarmChild: opts.PrewarmChild, } // Multi-VM scaffold (#764), default off: allocate the per-fork instance map