Repository navigation
docs(security): say which isolation boundary you actually get - #332
Merged
Merged
Conversation
docs/SECURITY.md told operators that every child ran in a KVM-backed microVM, so escaping needed a KVM or Firecracker vulnerability -- "the same boundary AWS Lambda relies on". forkd-serve runs gVisor, which shares the host kernel, so that sentence was false for every v0.6 deployment. Anyone who read it and sized their risk by it sized it against the wrong kernel, so the old claim is quoted and withdrawn in place rather than quietly edited away. The threat model now gives the boundary per backend, and records why gVisor is the default: Firecracker needs /dev/kvm and most cloud instances do not expose nested virtualisation. Verified on all three machines forkd is developed and measured on -- no /dev/kvm, no vmx or svm in /proc/cpuinfo, no kvm modules. A backend that cannot run on the machines operators have cannot be the default. Firecracker stays supported and is the stronger choice on a host with KVM, which is also why forkd-vmm, issue #317 and PR #321 remain live work rather than work about to be made moot. The same pass fixed a posture table that still described v0.5: --token-file, --tls-cert, TLS off by default, a shared host bridge, per_child_netns as an option. And added Known gaps, for the things that read as covered and are not -- chiefly that every sandbox is issued a 128-bit token that nothing verifies, because there is no gateway in front of a sandbox for it to gate yet. tag::is_safe is now one function used by create_sandbox, branch_sandbox and the node. create_sandbox had no check at all. That is not a traversal hole -- the node validates before it joins the path, which is the layer that stops it -- but the tag reached the database first and failed later on a node with a message about a missing directory. A validator inlined in some handlers and missing from the one that joins a path is the exact shape of the 0.1.4 advisory, so it has a test. DESIGN-v0.6.md is re-exported at rev 51: the backend decision and its reason are recorded in section 2, section 3 no longer says a sandbox is a jailer plus Firecracker process tree, and the verdict table calls Firecracker an optional backend instead of the backend. The Past advisories section is removed on request. The 0.1.4 fix it described is in CHANGELOG.md under its release with the affected versions; there are no published GitHub security advisories, so Supported versions points at the CHANGELOG rather than claiming otherwise. 136 tests, clippy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #330, which GitHub closed automatically when its base branch
feat/v0.6-mtlswas deleted on #329's merge; a closed PR's base cannot be retargeted, so this is the same branch reopened againstdev. Same commit, rebased onto the merged #329 — the two mTLS commits dropped as already applied.The claim that was false
docs/SECURITY.mdtold operators:forkd-serveruns gVisor. The host kernel is shared, so escaping needs a Sentry bug, not a KVM bug. The sentence was false for every v0.6 deployment, and anyone who read it and sized their risk by it sized it against the wrong kernel — so it is quoted and withdrawn in place rather than quietly edited away.What replaces it
The threat model now gives the boundary per backend:
forkd-serve(v0.6)forkd-vmm(via the CLI)runsc/dev/kvmAnd records why gVisor is the default rather than leaving it to be inferred: Firecracker needs
/dev/kvm, and most cloud instances do not expose nested virtualisation. Checked on all three machines forkd is developed and measured on — no/dev/kvm, novmx/svmin/proc/cpuinfo, no kvm modules. A backend that cannot run on the machines operators have cannot be the default.Firecracker stays supported and is the stronger choice on a KVM host. That is also why
forkd-vmm, #317 and #321 are live work rather than work about to be made moot.The posture table still described v0.5
--token-file,--tls-cert, TLS off by default, "per-child netns: shared (same host bridge)",packaging/k8s/forkd-controller.yaml, and a Kubernetes section claiming the pod needs/dev/kvm. Every row is now checked against the code. Several defaults are stronger than the document claimed, which is its own kind of wrong — a v0.5 runbook describes a weaker posture than you actually have.Known gaps, added
For the things that read as covered and are not:
FORWARD/INPUTdrop siblings, off-box traffic and the host's own services — so today it is reachable only from its own node. The moment a port is exposed, that token has to become mandatory on the path that exposes it. RFC 16.5 calls this layer two; it is not built.sb-plus 12 hex characters, not the 128 bits 16.5 asks for.ownercolumn exists and nothing reads it.tag::is_safeOne function, used by
create_sandbox,branch_sandboxand the node.create_sandboxhad no check at all.Not a traversal hole: the node validates before it joins the path, which is the layer that actually stops it. But the tag reached the database first and failed later on a node with a message about a missing directory — a 500-shaped failure for what is a 400. A validator inlined in some handlers and missing from the one that joins a path is the exact shape of the 0.1.4 advisory, so it has a test using that advisory's own payload.
The Past advisories section is removed
On request. Before deleting it I checked whether v0.6 had reintroduced what it described — that is how the
create_sandboxgap above was found. The 0.1.4 fix is inCHANGELOG.mdunder its release with the affected versions; there are no published GitHub security advisories, so Supported versions points at the CHANGELOG rather than claiming otherwise.136 tests, clippy clean.
🤖 Generated with Claude Code