From 2a060ad48840df7b43cede18b306bc89441156b3 Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Fri, 10 Jul 2026 21:30:00 -0700 Subject: [PATCH] sec(hardening): sandbox denies host secrets, firewall in-VM control plane, dind privileged off by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additive, low-regression security hardening from an audit. Three fixes, all secure-defaults / additive denies — no behaviour flips beyond the dind default. NATIVE-1 (Critical): the macOS native sandbox denied ~/.ssh anchored on the daemon's HOME, but the daemon runs as root with no HOME, so the deny targeted "/.ssh" and left every operator secret under /Users readable under (allow default) — including the GitHub App PEM. Added a host-wide `(deny file-read-data (subpath "/Users"))` (contents blocked, metadata allowed so path traversal/getcwd still work, mirroring the native/ subtree technique) plus a re-allow of the /Users node, and threaded the configured private_key_path down to the profile generator for an explicit belt-and-suspenders literal + parent-dir deny. LINUX-1/2 (Critical/High): the in-VM firewall only hooked FORWARD, so a container could reach the gateway's unauthenticated dispatch gRPC server (bind 0.0.0.0) and containerd. Added targeted INPUT DROP rules (subnet -> gateway, specific TCP dport only: containerd 10000, dispatch 10001, debug-exec 10002) so NAT and DNS stay intact. Added ip6tables mirrors for the FORWARD private-range denies and the control-plane INPUT drops — IPv6 was entirely unfiltered (v6 cloud metadata reachable). DIND-1 (High): ResolvedAllowPrivileged defaulted to true on macOS/Windows. Changed the default to false on all platforms — privileged docker is now opt-in everywhere. Deferred (out of scope, pending live-host testing): deny-by-default sandbox, RLIMIT limits, removing the /private/tmp write allow, the JIT-config-in-argv change (NATIVE-3), and a gRPC auth interceptor (the firewall is this PR's mitigation for the unauthenticated dispatch server). --- cmd/ephemerd/main.go | 18 ++- pkg/config/config.go | 20 ++-- pkg/config/config_test.go | 16 +-- pkg/native/native_darwin.go | 58 +++++++++- pkg/native/native_darwin_test.go | 103 ++++++++++++++++- pkg/native/native_other.go | 2 +- pkg/networking/firewall_linux.go | 154 ++++++++++++++++++++++++++ pkg/networking/firewall_linux_test.go | 142 ++++++++++++++++++++++++ pkg/networking/networking.go | 23 +++- pkg/scheduler/scheduler.go | 80 ++++++------- 10 files changed, 537 insertions(+), 79 deletions(-) create mode 100644 pkg/networking/firewall_linux_test.go diff --git a/cmd/ephemerd/main.go b/cmd/ephemerd/main.go index 3a2df77f..6e1a02e3 100644 --- a/cmd/ephemerd/main.go +++ b/cmd/ephemerd/main.go @@ -236,12 +236,19 @@ func serve(ctx context.Context, configFile, imagesDirFlag string, containerdTCPP // with the existing bridge's IP. networking.CleanStaleBridge(log) + // Control ports the in-VM daemon exposes on the bridge gateway that + // containers must never reach: containerd (TCP), the unauthenticated + // dispatch gRPC server (containerd+1), and the debug exec server + // (containerd+2). See startWorkerDebugExec / StartDispatchServer above. + controlPorts := []int{int(containerdTCPPort), int(containerdTCPPort) + 1, int(containerdTCPPort) + 2} + net, err := networking.New(networking.Config{ - DataDir: configDir, - Subnet: cfg.Network.Subnet, - MTU: cfg.Network.MTU, - CNIBinDir: cm.Dir(), - Log: log, + DataDir: configDir, + Subnet: cfg.Network.Subnet, + MTU: cfg.Network.MTU, + CNIBinDir: cm.Dir(), + ControlPorts: controlPorts, + Log: log, }) if err != nil { return fmt.Errorf("initializing networking: %w", err) @@ -638,6 +645,7 @@ func serve(ctx context.Context, configFile, imagesDirFlag string, containerdTCPP MacOSModeForRepo: cfg.Runner.MacOS.ModeForRepo, NativeMacUser: cfg.Runner.MacOS.User, RunnerDir: rm.Dir(), + PrivateKeyPath: cfg.GitHub.PrivateKeyPath, Retry: retryCfg, OrphanSweep: orphanCfg, Log: log, diff --git a/pkg/config/config.go b/pkg/config/config.go index 9fff5b5c..c157225d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -206,22 +206,20 @@ type DindConfig struct { } // ResolvedAllowPrivileged returns whether privileged dind sibling -// containers are allowed, applying the platform-aware default when the -// operator hasn't set the key explicitly. +// containers are allowed, applying the secure default when the operator +// hasn't set the key explicitly. // -// Default policy: -// - Windows / macOS host → true. The dind containerd backing store -// runs inside a VM that ephemerd manages (WSL2/Hyper-V on Windows, -// Vz on macOS), so the worst-case escape stays inside that VM. -// - Linux host → false. ephemerd runs directly on the host with no -// VM fence, so a privileged escape is bare-metal-host compromise. -// Operators that trust their workloads (e.g. internal CI for KIND -// tests) can opt in via `allow_privileged = true`. +// Default policy: false on ALL platforms. Privileged is opt-in everywhere. +// A privileged sibling container is effectively root on whatever host runs +// the backing containerd, so shipping it on-by-default is unsafe even where +// a VM fence limits the blast radius (Windows/macOS) — an operator that +// needs it (KIND clusters, nested containerd, /dev/fuse) opts in explicitly +// with `allow_privileged = true`. func (d *DindConfig) ResolvedAllowPrivileged() bool { if d.AllowPrivileged != nil { return *d.AllowPrivileged } - return goruntime.GOOS != "linux" + return false } // DindCachePruneInterval returns the prune interval with the default diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index dcde0148..de262433 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1931,12 +1931,12 @@ owner = "testorg" } } -func TestResolvedAllowPrivileged_DefaultByOS(t *testing.T) { +func TestResolvedAllowPrivileged_DefaultOffEveryOS(t *testing.T) { + // Secure default (DIND-1): privileged is opt-in on ALL platforms, so the + // default must resolve false regardless of GOOS. d := DindConfig{} - got := d.ResolvedAllowPrivileged() - wantTrue := goruntime.GOOS != "linux" - if got != wantTrue { - t.Errorf("default ResolvedAllowPrivileged on GOOS=%s = %v, want %v", goruntime.GOOS, got, wantTrue) + if d.ResolvedAllowPrivileged() { + t.Errorf("default ResolvedAllowPrivileged on GOOS=%s = true, want false (privileged must be opt-in)", goruntime.GOOS) } } @@ -1976,9 +1976,9 @@ enabled = true if cfg.Dind.AllowPrivileged != nil { t.Errorf("AllowPrivileged ptr = %v, want nil (key not set in TOML)", *cfg.Dind.AllowPrivileged) } - want := goruntime.GOOS != "linux" - if got := cfg.Dind.ResolvedAllowPrivileged(); got != want { - t.Errorf("ResolvedAllowPrivileged on GOOS=%s = %v, want %v", goruntime.GOOS, got, want) + // Omitted key → secure default false on every platform. + if got := cfg.Dind.ResolvedAllowPrivileged(); got { + t.Errorf("ResolvedAllowPrivileged on GOOS=%s = true, want false (default off)", goruntime.GOOS) } } diff --git a/pkg/native/native_darwin.go b/pkg/native/native_darwin.go index ead4b63f..d232d115 100644 --- a/pkg/native/native_darwin.go +++ b/pkg/native/native_darwin.go @@ -194,6 +194,7 @@ type Runner struct { log *slog.Logger jobDir string // /native// + pemPath string // configured GitHub App private_key_path (empty if unset / PAT auth) keychainPath string // per-job keychain runAsUser string // existing user to run as (empty = _ephemerd service user) jobUID uint32 // uid the runner executes as @@ -211,7 +212,12 @@ func (r *Runner) SetRunAsUser(username string) { // New creates a native macOS runner for a single job. It prepares the // workspace directory structure but does not start the runner process. -func New(dataDir, jobID, jitConfig, runnerSrc string, log *slog.Logger) (*Runner, error) { +// +// pemPath is the configured GitHub App private_key_path (empty for PAT +// auth). It is threaded into the sandbox profile so the runner is denied +// read access to the operator's App private key even though the daemon +// runs as root with no HOME — see GenerateSandboxProfile. +func New(dataDir, jobID, jitConfig, runnerSrc, pemPath string, log *slog.Logger) (*Runner, error) { jobDir := filepath.Join(dataDir, "native", jobID) // Create workspace directories @@ -233,6 +239,7 @@ func New(dataDir, jobID, jitConfig, runnerSrc string, log *slog.Logger) (*Runner jobID: jobID, jitConfig: jitConfig, runnerSrc: runnerSrc, + pemPath: pemPath, log: log, jobDir: jobDir, }, nil @@ -250,7 +257,7 @@ func (r *Runner) Start(ctx context.Context) error { // Generate and write sandbox profile profilePath := filepath.Join(r.jobDir, "sandbox.sb") - profile := GenerateSandboxProfile(r.jobDir, r.dataDir) + profile := GenerateSandboxProfile(r.jobDir, r.dataDir, r.pemPath) if err := os.WriteFile(profilePath, []byte(profile), 0o644); err != nil { return fmt.Errorf("writing sandbox profile: %w", err) } @@ -444,7 +451,11 @@ func (r *Runner) deleteKeychain() { // GenerateSandboxProfile returns a macOS sandbox profile that restricts // the runner process. Paths are templated with the job-specific directories. -func GenerateSandboxProfile(jobDir, dataDir string) string { +// +// pemPath is the configured GitHub App private_key_path (may be empty). +// When set it is denied explicitly as a belt-and-suspenders measure on top +// of the broad /Users content deny below. +func GenerateSandboxProfile(jobDir, dataDir, pemPath string) string { // Resolve to absolute, symlink-free paths. The sandbox matches against // kernel (resolved) paths: /var and /tmp are symlinks to /private/var // and /private/tmp on macOS, so rules written with the unresolved @@ -463,6 +474,22 @@ func GenerateSandboxProfile(jobDir, dataDir string) string { absDataDir := resolve(dataDir) homeDir := resolve(os.Getenv("HOME")) + // Belt-and-suspenders deny for the configured GitHub App private key. + // The broad /Users content deny below already covers a key under any + // user's home, but the PEM can live outside /Users (e.g. /etc, + // /var/lib/ephemerd) so name it — and its parent dir — explicitly. + // Resolved through the same helper so symlinked config paths match the + // kernel's view. + var pemDenies string + if pemPath != "" { + absPem := resolve(pemPath) + pemDenies = fmt.Sprintf(` +;; === GitHub App private key (belt and suspenders) === +(deny file-read* (literal "%[1]s")) +(deny file-read* (subpath "%[2]s")) +`, absPem, filepath.Dir(absPem)) + } + // NOTE: this profile is allow-by-default with an explicit deny list. // For native (no-VM) execution the stronger posture is deny-by-default // with an allow list, but that requires enumerating every path the GHA @@ -513,6 +540,27 @@ func GenerateSandboxProfile(jobDir, dataDir string) string { ;; contents) — job ids are not secret. (allow file-read-data (literal "%[2]s/native")) +;; Block reading the CONTENTS of every user's home under /Users. The +;; daemon runs as root with no HOME, so a deny anchored on $HOME (below) +;; targets "/.ssh" and leaves the operator's real ~/.ssh — including the +;; GitHub App PEM and every other dotfile secret — readable under +;; (allow default). This closes that hole host-wide without depending on +;; the daemon's HOME. +;; +;; Deny file-read-DATA (not file-read*), mirroring the native/ subtree +;; technique above: on a file it blocks reading contents; on a directory +;; it blocks readdir. file-read-METADATA stays allowed so lstat/realpath +;; path resolution can traverse THROUGH /Users (denying metadata breaks +;; getcwd and the .NET host's executable-path resolution). +(deny file-read-data (subpath "/Users")) +(deny file-write* (subpath "/Users")) + +;; Re-allow reading the /Users directory NODE itself (not its children), +;; same reason as the native/ node re-allow: getcwd()/bash walk UP through +;; /Users and must readdir it to resolve the current path. This leaks the +;; list of usernames (not their file contents) — usernames are not secret. +(allow file-read-data (literal "/Users")) + ;; Block sensitive host paths entirely — read and write. .ssh was ;; previously read-only-denied, leaving a writable authorized_keys hole ;; on any host where the runner uid can reach the operator's home. @@ -524,7 +572,7 @@ func GenerateSandboxProfile(jobDir, dataDir string) string { (deny file-write* (literal "%[2]s/ephemerd.sock")) (deny file-read* (subpath "%[2]s/vm")) (deny file-write* (subpath "%[2]s/vm")) - +%[4]s ;; Block writes to shared tools (read-only access only) (deny file-write* (subpath "/opt/homebrew")) (deny file-write* (subpath "/Applications")) @@ -540,7 +588,7 @@ func GenerateSandboxProfile(jobDir, dataDir string) string { (allow file-read-data (subpath "%[3]s")) (allow file-write* (subpath "%[3]s")) (allow file-write* (subpath "/private/tmp")) -`, homeDir, absDataDir, absJobDir) +`, homeDir, absDataDir, absJobDir, pemDenies) } // copyRunnerFiles copies the runner directory to the per-job location. diff --git a/pkg/native/native_darwin_test.go b/pkg/native/native_darwin_test.go index b20407b6..0e73694e 100644 --- a/pkg/native/native_darwin_test.go +++ b/pkg/native/native_darwin_test.go @@ -29,12 +29,15 @@ func TestGenerateSandboxProfile(t *testing.T) { t.Fatal(err) } - profile := GenerateSandboxProfile(jobDir, dataDir) + profile := GenerateSandboxProfile(jobDir, dataDir, "") checks := []struct { desc string substr string }{ + {"denies /Users content reads", `(deny file-read-data (subpath "/Users"))`}, + {"denies /Users writes", `(deny file-write* (subpath "/Users"))`}, + {"re-allows /Users node read (getcwd)", `(allow file-read-data (literal "/Users"))`}, {"allows DNS UDP", `(allow network-outbound (remote udp "localhost:53"))`}, {"allows DNS TCP", `(allow network-outbound (remote tcp "localhost:53"))`}, {"blocks localhost", `(deny network-outbound (remote ip "localhost:*"))`}, @@ -87,7 +90,7 @@ func TestGenerateSandboxProfile_ResolvesSymlinks(t *testing.T) { // Generate using the SYMLINK path — the profile must contain the // resolved target, and not rules pointing at the symlink. - profile := GenerateSandboxProfile(filepath.Join(linkData, "native", "j1"), linkData) + profile := GenerateSandboxProfile(filepath.Join(linkData, "native", "j1"), linkData, "") if !strings.Contains(profile, `(deny file-read-data (subpath "`+resolvedData+`/native"))`) { t.Errorf("profile should deny the RESOLVED native path %q, got:\n%s", resolvedData, profile) @@ -97,6 +100,100 @@ func TestGenerateSandboxProfile_ResolvesSymlinks(t *testing.T) { } } +// TestGenerateSandboxProfile_PEMDenies pins NATIVE-1: when the GitHub App +// private_key_path is configured, the profile must deny reading it (and its +// parent dir) explicitly, on top of the broad /Users content deny. The deny +// must not depend on the daemon's HOME. +func TestGenerateSandboxProfile_PEMDenies(t *testing.T) { + base := t.TempDir() + dataDir := filepath.Join(base, "data") + jobDir := filepath.Join(dataDir, "native", "job123") + if err := os.MkdirAll(jobDir, 0o755); err != nil { + t.Fatal(err) + } + + // A PEM that lives OUTSIDE /Users (e.g. an operator's config dir) — the + // /Users deny wouldn't cover it, so the explicit deny must. + pemDir := filepath.Join(base, "secrets") + if err := os.MkdirAll(pemDir, 0o755); err != nil { + t.Fatal(err) + } + pemPath := filepath.Join(pemDir, "app.pem") + if err := os.WriteFile(pemPath, []byte("-----BEGIN PRIVATE KEY-----"), 0o600); err != nil { + t.Fatal(err) + } + resolvedPem, err := filepath.EvalSymlinks(pemPath) + if err != nil { + t.Fatal(err) + } + resolvedPemDir := filepath.Dir(resolvedPem) + + profile := GenerateSandboxProfile(jobDir, dataDir, pemPath) + + wantLiteral := `(deny file-read* (literal "` + resolvedPem + `"))` + if !strings.Contains(profile, wantLiteral) { + t.Errorf("profile missing explicit PEM literal deny %q, got:\n%s", wantLiteral, profile) + } + wantDir := `(deny file-read* (subpath "` + resolvedPemDir + `"))` + if !strings.Contains(profile, wantDir) { + t.Errorf("profile missing PEM parent-dir deny %q, got:\n%s", wantDir, profile) + } +} + +// TestGenerateSandboxProfile_NoPEM confirms the PEM deny block is omitted +// entirely when no private_key_path is configured (PAT auth), so an empty +// literal deny never lands in the profile. +func TestGenerateSandboxProfile_NoPEM(t *testing.T) { + base := t.TempDir() + dataDir := filepath.Join(base, "data") + jobDir := filepath.Join(dataDir, "native", "job123") + if err := os.MkdirAll(jobDir, 0o755); err != nil { + t.Fatal(err) + } + + profile := GenerateSandboxProfile(jobDir, dataDir, "") + if strings.Contains(profile, `(literal ""))`) { + t.Errorf("profile must not emit an empty literal deny when no PEM is set, got:\n%s", profile) + } + if strings.Contains(profile, "GitHub App private key") { + t.Errorf("PEM deny block should be absent when no PEM is set, got:\n%s", profile) + } +} + +// TestGenerateSandboxProfile_JobHomeUnaffected confirms the new /Users +// content deny does not touch a job whose HOME lives under the data dir +// (e.g. /var/lib/ephemerd/native//home): that path is re-allowed for +// read AND write, so the runner's own home is fully usable. +func TestGenerateSandboxProfile_JobHomeUnaffected(t *testing.T) { + // Use a data dir under a non-/Users root to mirror /var/lib/ephemerd. + base := t.TempDir() // t.TempDir() on macOS resolves under /private/var + dataDir := filepath.Join(base, "ephemerd") + jobDir := filepath.Join(dataDir, "native", "job123") + if err := os.MkdirAll(jobDir, 0o755); err != nil { + t.Fatal(err) + } + resolvedJob, err := filepath.EvalSymlinks(jobDir) + if err != nil { + t.Fatal(err) + } + + profile := GenerateSandboxProfile(jobDir, dataDir, "") + + // The job dir (which contains home/) must be re-allowed for read+write, + // and must not sit under /Users so the /Users deny can't reach it. + if strings.HasPrefix(resolvedJob, "/Users") { + t.Fatalf("test setup error: job dir unexpectedly under /Users: %s", resolvedJob) + } + for _, want := range []string{ + `(allow file-read* (subpath "` + resolvedJob + `"))`, + `(allow file-write* (subpath "` + resolvedJob + `"))`, + } { + if !strings.Contains(profile, want) { + t.Errorf("profile missing job-home re-allow %q, got:\n%s", want, profile) + } + } +} + func TestNewCreatesWorkspace(t *testing.T) { tmpDir := t.TempDir() dataDir := filepath.Join(tmpDir, "data") @@ -107,7 +204,7 @@ func TestNewCreatesWorkspace(t *testing.T) { t.Fatal(err) } - r, err := New(dataDir, "test-job-42", "fake-jit-config", runnerSrc, nil) + r, err := New(dataDir, "test-job-42", "fake-jit-config", runnerSrc, "", nil) if err != nil { t.Fatalf("New() error: %v", err) } diff --git a/pkg/native/native_other.go b/pkg/native/native_other.go index eb64d8c8..1d902a54 100644 --- a/pkg/native/native_other.go +++ b/pkg/native/native_other.go @@ -12,7 +12,7 @@ import ( type Runner struct{} // New returns an error on non-darwin platforms. -func New(_, _, _, _ string, _ *slog.Logger) (*Runner, error) { +func New(_, _, _, _, _ string, _ *slog.Logger) (*Runner, error) { return nil, fmt.Errorf("native macOS runner is only supported on darwin") } diff --git a/pkg/networking/firewall_linux.go b/pkg/networking/firewall_linux.go index 4a96049b..877c76a7 100644 --- a/pkg/networking/firewall_linux.go +++ b/pkg/networking/firewall_linux.go @@ -5,6 +5,7 @@ package networking import ( "fmt" "os/exec" + "strings" ) const chainName = "EPHEMERD-FORWARD" @@ -16,6 +17,16 @@ var deniedRanges = []string{ "169.254.0.0/16", } +// deniedRanges6 mirrors deniedRanges for IPv6. Without these the FORWARD +// filter is IPv4-only and a container with a global IPv6 address could reach +// IPv6 private/link-local space — including the IPv6 cloud metadata endpoint +// (fd00:ec2::254 on AWS). fc00::/7 covers ULA (the v6 analogue of RFC1918), +// fe80::/10 covers link-local. +var deniedRanges6 = []string{ + "fc00::/7", + "fe80::/10", +} + func iptables(args ...string) error { out, err := exec.Command("iptables", args...).CombinedOutput() if err != nil { @@ -24,6 +35,42 @@ func iptables(args ...string) error { return nil } +func ip6tables(args ...string) error { + out, err := exec.Command("ip6tables", args...).CombinedOutput() + if err != nil { + return fmt.Errorf("ip6tables: %w: %s", err, out) + } + return nil +} + +// controlPlaneInputRules returns the argv for each targeted INPUT DROP rule +// that blocks the container subnet from reaching the ephemerd control plane +// bound on the gateway (containerd, dispatch gRPC, debug exec). The rules are +// intentionally narrow — source = container subnet, destination = gateway IP, +// specific TCP dport — so container→gateway NAT and DNS (dport 53) are +// unaffected. Returns nil when no control ports are configured (bare-metal +// Linux with no in-VM dispatch server on the bridge). +// +// Exposed as a pure function (no side effects) so tests can assert the exact +// rule set without invoking iptables. +func controlPlaneInputRules(subnet, gateway string, ports []int) [][]string { + if subnet == "" || gateway == "" || len(ports) == 0 { + return nil + } + rules := make([][]string, 0, len(ports)) + for _, port := range ports { + rules = append(rules, []string{ + "INPUT", + "-s", subnet, + "-d", gateway, + "-p", "tcp", + "--dport", fmt.Sprintf("%d", port), + "-j", "DROP", + }) + } + return rules +} + func (l *linuxNetworking) installFirewallRules() error { // Create our own chain to avoid interference from CNI/Netavark _ = iptables("-N", chainName) @@ -91,10 +138,98 @@ func (l *linuxNetworking) installFirewallRules() error { return fmt.Errorf("adding outbound rule: %w", err) } + // 6. Block the container subnet from reaching the ephemerd control plane + // (containerd / dispatch gRPC / debug exec) bound on the gateway. The + // FORWARD chain above only governs container→off-host traffic; traffic + // addressed to the gateway IP itself is delivered locally and hits the + // INPUT chain, so it must be dropped there. These rules are narrow + // (subnet→gateway, specific TCP dport) so NAT and DNS stay intact. + for _, rule := range controlPlaneInputRules(l.cfg.subnet(), gateway, l.cfg.ControlPorts) { + port := rule[len(rule)-3] // "--dport -j DROP" → port is 3 from end + l.cfg.Log.Info("adding firewall rule", "rule", "drop container→gateway control port "+port) + // Idempotent: check-before-insert. Insert (not append) so the DROP + // precedes any permissive INPUT rule an earlier subsystem installed. + if err := iptables(append([]string{"-C"}, rule...)...); err != nil { + insert := append([]string{"-I", rule[0], "1"}, rule[1:]...) + if err := iptables(insert...); err != nil { + return fmt.Errorf("adding control-plane INPUT drop (tcp/%s): %w", port, err) + } + } + } + + // 7. IPv6: mirror the private/link-local FORWARD denies and the + // control-plane INPUT drops. IPv6 was previously entirely unfiltered, + // so a container that inherited host v6 forwarding could reach v6 + // ULA/link-local space and the v6 cloud metadata endpoint. Best-effort: + // on hosts with IPv6 disabled these fail harmlessly and are logged, + // not fatal — IPv4 firewalling is already in place at this point. + for _, r := range ipv6FirewallRules(l.cfg.ControlPorts) { + // Idempotent check-before-insert. The -C check uses the chain + match + // args (no insert position); the apply uses -A (append) or -I + // 1 (insert first) per the rule's Insert flag. + if err := ip6tables(append([]string{"-C", r.chain}, r.match...)...); err == nil { + continue // already present + } + var apply []string + if r.insert { + apply = append([]string{"-I", r.chain, "1"}, r.match...) + } else { + apply = append([]string{"-A", r.chain}, r.match...) + } + if err := ip6tables(apply...); err != nil { + l.cfg.Log.Warn("ip6tables rule not installed (IPv6 may be disabled)", + "chain", r.chain, "match", strings.Join(r.match, " "), "error", err) + } + } + l.cfg.Log.Info("firewall rules installed") return nil } +// ip6Rule is a single ip6tables rule split into chain, match args, and +// whether it should be inserted first (INPUT drops) or appended (FORWARD +// denies). Kept structured so both the -C idempotency check and the apply +// step derive from the same match args. +type ip6Rule struct { + chain string + match []string + insert bool +} + +// ipv6FirewallRules returns the IPv6 firewall rules that mirror the IPv4 +// posture. There is no configured IPv6 container subnet (the CNI bridge is +// IPv4-only), so the FORWARD denies match by DESTINATION only — safe because +// the bridge only forwards our containers' traffic — and the control-plane +// INPUT drops match by destination (a v6 gateway would live in link-local/ULA +// space) plus TCP dport. Exposed as a pure function so tests can assert the +// emitted set without invoking ip6tables. +func ipv6FirewallRules(controlPorts []int) []ip6Rule { + var rules []ip6Rule + + // Deny v6 private/link-local destinations in FORWARD (cloud metadata, + // ULA, link-local). + for _, cidr := range deniedRanges6 { + rules = append(rules, ip6Rule{ + chain: "FORWARD", + match: []string{"-d", cidr, "-j", "REJECT", "--reject-with", "icmp6-adm-prohibited"}, + }) + } + + // Drop v6 traffic to the control ports destined to link-local/ULA (where + // a v6 gateway address would sit). Inserted first in INPUT. + for _, port := range controlPorts { + for _, cidr := range deniedRanges6 { + rules = append(rules, ip6Rule{ + chain: "INPUT", + match: []string{"-d", cidr, "-p", "tcp", "--dport", fmt.Sprintf("%d", port), "-j", "DROP"}, + insert: true, + }) + } + } + + return rules +} + func (l *linuxNetworking) removeFirewallRules() { // Remove jump rules from FORWARD if err := iptables("-D", "FORWARD", "-s", l.cfg.subnet(), "-j", chainName); err != nil { @@ -112,6 +247,25 @@ func (l *linuxNetworking) removeFirewallRules() { l.cfg.Log.Debug("failed to delete chain", "error", err) } + // Remove the IPv4 control-plane INPUT drops (they live in the built-in + // INPUT chain, not our own chain, so -F/-X above doesn't touch them). + gateway := deriveGateway(l.cfg.subnet()) + for _, rule := range controlPlaneInputRules(l.cfg.subnet(), gateway, l.cfg.ControlPorts) { + del := append([]string{"-D", rule[0]}, rule[1:]...) + if err := iptables(del...); err != nil { + l.cfg.Log.Debug("failed to remove control-plane INPUT drop", "error", err) + } + } + + // Remove the IPv6 rules (FORWARD denies + INPUT control-plane drops). + // Best-effort; on hosts without IPv6 these were never installed. + for _, r := range ipv6FirewallRules(l.cfg.ControlPorts) { + del := append([]string{"-D", r.chain}, r.match...) + if err := ip6tables(del...); err != nil { + l.cfg.Log.Debug("failed to remove ip6tables rule", "chain", r.chain, "error", err) + } + } + // Delete the bridge interface so it doesn't conflict on next startup // (the subnet may change between runs) out, err := exec.Command("ip", "link", "delete", defaultBridgeName).CombinedOutput() diff --git a/pkg/networking/firewall_linux_test.go b/pkg/networking/firewall_linux_test.go new file mode 100644 index 00000000..9a3341bf --- /dev/null +++ b/pkg/networking/firewall_linux_test.go @@ -0,0 +1,142 @@ +//go:build linux + +package networking + +import ( + "strings" + "testing" +) + +// joinRule renders an argv slice back to a space-joined string for +// substring assertions. +func joinRule(r []string) string { return strings.Join(r, " ") } + +func TestControlPlaneInputRules_EmitsDropPerControlPort(t *testing.T) { + subnet := "10.88.0.0/16" + gateway := "10.88.0.1" + ports := []int{10000, 10001, 10002} // containerd, dispatch, debug exec + + rules := controlPlaneInputRules(subnet, gateway, ports) + if len(rules) != len(ports) { + t.Fatalf("got %d rules, want %d (one DROP per control port)", len(rules), len(ports)) + } + + for i, port := range []string{"10000", "10001", "10002"} { + got := joinRule(rules[i]) + want := "INPUT -s " + subnet + " -d " + gateway + " -p tcp --dport " + port + " -j DROP" + if got != want { + t.Errorf("rule[%d] = %q, want %q", i, got, want) + } + } +} + +// TestControlPlaneInputRules_NarrowScope pins the safety property: every +// control-plane DROP is scoped to source=subnet, dest=gateway, a specific +// TCP dport — never a blanket gateway block and never touching port 53. That +// is what keeps container→gateway NAT and DNS working. +func TestControlPlaneInputRules_NarrowScope(t *testing.T) { + rules := controlPlaneInputRules("10.88.0.0/16", "10.88.0.1", []int{10000, 10001, 10002}) + for _, r := range rules { + s := joinRule(r) + if !strings.Contains(s, "-s 10.88.0.0/16") { + t.Errorf("rule %q missing source-subnet scope", s) + } + if !strings.Contains(s, "-d 10.88.0.1") { + t.Errorf("rule %q missing gateway-dest scope", s) + } + if !strings.Contains(s, "-p tcp") || !strings.Contains(s, "--dport") { + t.Errorf("rule %q is not TCP-dport-scoped (would over-block)", s) + } + if strings.Contains(s, "--dport 53") { + t.Errorf("rule %q blocks DNS (dport 53) — must not", s) + } + } +} + +func TestControlPlaneInputRules_EmptyWhenNoPorts(t *testing.T) { + if rules := controlPlaneInputRules("10.88.0.0/16", "10.88.0.1", nil); rules != nil { + t.Errorf("expected nil rules with no control ports, got %v", rules) + } + if rules := controlPlaneInputRules("", "10.88.0.1", []int{10000}); rules != nil { + t.Errorf("expected nil rules with empty subnet, got %v", rules) + } + if rules := controlPlaneInputRules("10.88.0.0/16", "", []int{10000}); rules != nil { + t.Errorf("expected nil rules with empty gateway, got %v", rules) + } +} + +func TestIPv6FirewallRules_ForwardDenies(t *testing.T) { + rules := ipv6FirewallRules(nil) + // With no control ports, only the FORWARD private-range denies are emitted. + if len(rules) != len(deniedRanges6) { + t.Fatalf("got %d rules, want %d FORWARD denies", len(rules), len(deniedRanges6)) + } + wantCIDRs := map[string]bool{"fc00::/7": false, "fe80::/10": false} + for _, r := range rules { + if r.chain != "FORWARD" { + t.Errorf("rule chain = %q, want FORWARD", r.chain) + } + if r.insert { + t.Errorf("FORWARD deny should append, not insert: %v", r) + } + s := joinRule(r.match) + if !strings.Contains(s, "-j REJECT") { + t.Errorf("FORWARD deny %q should REJECT", s) + } + for cidr := range wantCIDRs { + if strings.Contains(s, "-d "+cidr) { + wantCIDRs[cidr] = true + } + } + } + for cidr, seen := range wantCIDRs { + if !seen { + t.Errorf("missing IPv6 FORWARD deny for %s", cidr) + } + } +} + +// TestIPv6FirewallRules_ControlPlaneInputDrops confirms the v6 control-plane +// INPUT drops are emitted for each control port × denied v6 range, inserted +// first, and TCP-dport-scoped (mirroring the IPv4 posture). +func TestIPv6FirewallRules_ControlPlaneInputDrops(t *testing.T) { + ports := []int{10000, 10001, 10002} + rules := ipv6FirewallRules(ports) + + var inputDrops int + for _, r := range rules { + if r.chain != "INPUT" { + continue + } + inputDrops++ + if !r.insert { + t.Errorf("INPUT drop should insert-first: %v", r) + } + s := joinRule(r.match) + if !strings.Contains(s, "-p tcp") || !strings.Contains(s, "--dport") || !strings.Contains(s, "-j DROP") { + t.Errorf("v6 INPUT rule %q not a TCP-dport DROP", s) + } + } + want := len(ports) * len(deniedRanges6) + if inputDrops != want { + t.Errorf("got %d v6 INPUT drops, want %d (ports × ranges)", inputDrops, want) + } +} + +// TestIPv6FirewallRules_PortsPresent asserts each control port shows up in +// the emitted v6 INPUT rule set. +func TestIPv6FirewallRules_PortsPresent(t *testing.T) { + rules := ipv6FirewallRules([]int{10000, 10001, 10002}) + for _, port := range []string{"10000", "10001", "10002"} { + found := false + for _, r := range rules { + if r.chain == "INPUT" && strings.Contains(joinRule(r.match), "--dport "+port+" ") { + found = true + break + } + } + if !found { + t.Errorf("no v6 INPUT drop found for control port %s", port) + } + } +} diff --git a/pkg/networking/networking.go b/pkg/networking/networking.go index e4758337..1874cc12 100644 --- a/pkg/networking/networking.go +++ b/pkg/networking/networking.go @@ -13,12 +13,23 @@ const DefaultSubnet = "10.88.0.0/16" // Config for container networking. type Config struct { - DataDir string - Subnet string // container subnet (auto-selected if empty) - MTU int // bridge MTU (auto-detected from host if 0) - CNIBinDir string // path to CNI plugin binaries (Linux only, ignored elsewhere) - GatewayPorts []int // extra TCP ports to allow from containers to the gateway (e.g., module proxy) - Log *slog.Logger + DataDir string + Subnet string // container subnet (auto-selected if empty) + MTU int // bridge MTU (auto-detected from host if 0) + CNIBinDir string // path to CNI plugin binaries (Linux only, ignored elsewhere) + GatewayPorts []int // extra TCP ports to allow from containers to the gateway (e.g., module proxy) + + // ControlPorts are TCP ports the ephemerd control plane binds on the + // gateway (bridge) address that MUST NOT be reachable from inside + // containers: the in-VM containerd (default 10000), the unauthenticated + // dispatch gRPC server (containerd+1), and the debug exec server + // (containerd+2). The firewall adds targeted INPUT DROP rules from the + // container subnet to the gateway on these ports. Empty = no INPUT + // control-plane rules (e.g. a bare-metal Linux host with no in-VM + // dispatch server listening on the bridge). + ControlPorts []int + + Log *slog.Logger } // pickSubnet tries the default subnet first. If it conflicts with an existing diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index e7950014..8b860c54 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -27,25 +27,25 @@ import ( // Config for the scheduler. type Config struct { - Runtime *runtime.Runtime - Providers []providers.Provider - Artifacts *artifacts.Extractor // OCI image layer extractor for macOS VM jobs (nil if not available) - LinuxDispatcher *DispatchClient // if non-nil, Linux jobs are dispatched to a Linux VM worker via gRPC - MacOSVMConfig *vm.MacOSVMConfig // if non-nil, macOS-native jobs are enabled (darwin only) - DataDir string // ephemerd data directory (used for artifact extraction paths) - MaxConcurrent int - MaxMacOSVMs int // max concurrent macOS VMs (Vz limit; default auto-detected) - Labels []string - PollInterval time.Duration // if >0, use polling mode (default) - WebhookPort int // listen port for health/webhook server - WebhookSecret string // webhook signature secret - TLSCert string // TLS certificate path - TLSKey string // TLS private key path - Tunnel tunnel.Provider // if non-nil, creates a public tunnel for webhooks - TunnelMaxRetries int // max consecutive reconnect failures before fallback to polling (0 = default 5) - JobTimeout time.Duration - ShutdownTimeout time.Duration - LogRetention time.Duration // max age for job log files (default 7d) + Runtime *runtime.Runtime + Providers []providers.Provider + Artifacts *artifacts.Extractor // OCI image layer extractor for macOS VM jobs (nil if not available) + LinuxDispatcher *DispatchClient // if non-nil, Linux jobs are dispatched to a Linux VM worker via gRPC + MacOSVMConfig *vm.MacOSVMConfig // if non-nil, macOS-native jobs are enabled (darwin only) + DataDir string // ephemerd data directory (used for artifact extraction paths) + MaxConcurrent int + MaxMacOSVMs int // max concurrent macOS VMs (Vz limit; default auto-detected) + Labels []string + PollInterval time.Duration // if >0, use polling mode (default) + WebhookPort int // listen port for health/webhook server + WebhookSecret string // webhook signature secret + TLSCert string // TLS certificate path + TLSKey string // TLS private key path + Tunnel tunnel.Provider // if non-nil, creates a public tunnel for webhooks + TunnelMaxRetries int // max consecutive reconnect failures before fallback to polling (0 = default 5) + JobTimeout time.Duration + ShutdownTimeout time.Duration + LogRetention time.Duration // max age for job log files (default 7d) // Retry configures the claim/provision retry queue. When the initial // attempt to claim a queued job fails with a retryable error @@ -77,6 +77,7 @@ type Config struct { MacOSModeForRepo func(repo string) string // returns "native" or "vm" per repo (nil = always VM) NativeMacUser string // non-root user for native macOS runner processes RunnerDir string // path to extracted GHA runner binary dir (runner.Manager.Dir()) + PrivateKeyPath string // GitHub App private_key_path, denied read access in the native sandbox (empty for PAT auth) Log *slog.Logger } @@ -85,10 +86,10 @@ type Config struct { // // Resolution order: // -// 1. Image declared in the workflow YAML (FetchJobImage) -// 2. Per-repo override from [runner.images.]. -// 3. Provider per-OS default (DefaultImageFor) -// 4. Empty — runtime.Create picks its host-aware fallback +// 1. Image declared in the workflow YAML (FetchJobImage) +// 2. Per-repo override from [runner.images.]. +// 3. Provider per-OS default (DefaultImageFor) +// 4. Empty — runtime.Create picks its host-aware fallback func (s *Scheduler) resolveImage(ctx context.Context, event *providers.JobEvent, os string) string { if event == nil || event.Provider == nil { return "" @@ -140,20 +141,20 @@ func keyFor(event providers.JobEvent) jobKey { // When a job is queued, it provisions a runner environment. // When the job completes, it destroys the environment. type Scheduler struct { - cfg Config - running map[jobKey]*runningJob - seen map[jobKey]time.Time // recently handled jobs for dedup - pending map[jobKey]struct{} // jobs dispatched to a handler but not yet holding sem - attempts map[jobKey]int // provisioning passes per job, for zombie detection - runners map[string]*runnerBinding // dispatched runners by name; tracks observed job assignment - webhookMode bool // true when job events arrive via webhooks (in_progress observable) - mu sync.Mutex + cfg Config + running map[jobKey]*runningJob + seen map[jobKey]time.Time // recently handled jobs for dedup + pending map[jobKey]struct{} // jobs dispatched to a handler but not yet holding sem + attempts map[jobKey]int // provisioning passes per job, for zombie detection + runners map[string]*runnerBinding // dispatched runners by name; tracks observed job assignment + webhookMode bool // true when job events arrive via webhooks (in_progress observable) + mu sync.Mutex sem chan struct{} // local/native job concurrency limiter linuxSem chan struct{} // Linux dispatch (VM) concurrency limiter macSem chan struct{} // macOS VM concurrency limiter (Vz has a hard cap) nativeMacSem chan struct{} // native macOS job concurrency limiter (separate from VM limit) - draining bool // true when shutting down, rejects new jobs - startTime time.Time + draining bool // true when shutting down, rejects new jobs + startTime time.Time // retry holds pending re-attempts for jobs whose initial claim // failed with a retryable error. Nil when Config.Retry.Enabled=false. @@ -220,11 +221,11 @@ type runningJob struct { repo string image string cancel context.CancelFunc - artifactsDir string // non-empty if OCI artifacts were extracted for this job - dispatched string // non-empty if dispatched to Linux VM worker (stores container name) - macosVM vm.MacOSVM // non-nil if running as a macOS VM job - nativeRunner interface{ Stop() } // non-nil if running as a native macOS job - startedAt time.Time + artifactsDir string // non-empty if OCI artifacts were extracted for this job + dispatched string // non-empty if dispatched to Linux VM worker (stores container name) + macosVM vm.MacOSVM // non-nil if running as a macOS VM job + nativeRunner interface{ Stop() } // non-nil if running as a native macOS job + startedAt time.Time } // runnerName returns the name the runner was registered under with the @@ -1103,7 +1104,7 @@ func (s *Scheduler) handleNativeMacOSJob(ctx context.Context, event providers.Jo } // Create the native runner - nr, err := native.New(s.cfg.DataDir, fmt.Sprintf("%d", jobID), claim.RunnerConfig, s.cfg.RunnerDir, log) + nr, err := native.New(s.cfg.DataDir, fmt.Sprintf("%d", jobID), claim.RunnerConfig, s.cfg.RunnerDir, s.cfg.PrivateKeyPath, log) if err != nil { log.Error("failed to create native runner", "error", err) if rmErr := event.Provider.ReleaseJob(ctx, claim); rmErr != nil { @@ -1614,7 +1615,6 @@ func (s *Scheduler) handleHealthz(w http.ResponseWriter, r *http.Request) { } } - func (s *Scheduler) cleanSeen() { s.mu.Lock() defer s.mu.Unlock()