From 09efc86b4d176bb18f0a06bc6592b405dda9a0db Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Thu, 6 Aug 2026 19:05:59 -0700 Subject: [PATCH] sec(networking): host-firewall egress backstop for Windows job containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows jobs could reach the fleet's management planes (Incus, Grafana) because installFirewallRules was a no-op on Windows and the per-endpoint HNS/VFP ACLs alone did not contain a Hyper-V-isolated job on the real runner (found by the containment suite, #134). Implement the Windows counterpart of firewall_linux.go: Windows Defender Firewall rules installed on the host, where WinNAT routes every container flow regardless of vSwitch policy. Outbound blocks cover RFC1918 + link-local with the container subnet (gateway, DNS, NAT, container-to- container) subtracted via range arithmetic — Windows Firewall has no rule ordering and Block beats Allow, so the gateway can never appear inside a blocked range. Inbound rules block container→gateway traffic on the ephemerd control ports, mirroring controlPlaneInputRules. Rules are named ephemerd-egress-* and installed delete-before-add, so reinstallation is idempotent and Cleanup removes the exact set. Install failure surfaces as an error that callers already treat as a warning, so a host without firewall privileges degrades to the endpoint ACLs instead of refusing to start. Closes #135 --- pkg/networking/firewall_windows.go | 241 ++++++++++++++++++++++++ pkg/networking/firewall_windows_test.go | 229 ++++++++++++++++++++++ pkg/networking/network_windows.go | 12 +- 3 files changed, 473 insertions(+), 9 deletions(-) create mode 100644 pkg/networking/firewall_windows.go create mode 100644 pkg/networking/firewall_windows_test.go diff --git a/pkg/networking/firewall_windows.go b/pkg/networking/firewall_windows.go new file mode 100644 index 0000000..d497265 --- /dev/null +++ b/pkg/networking/firewall_windows.go @@ -0,0 +1,241 @@ +//go:build windows + +package networking + +import ( + "encoding/binary" + "fmt" + "net" + "os/exec" + "strconv" + "strings" +) + +// Host-side egress firewall for Windows job containers. +// +// Two layers restrict what a Windows job can reach: +// +// 1. Per-endpoint HNS ACL policies (applyACLPolicies in network_windows.go) — +// VFP rules on the container's vSwitch port, applied in setup(). +// 2. The host-global Windows Defender Firewall rules installed here. +// +// Layer 2 exists because layer 1 alone left the fleet reachable in practice: +// the containment suite (.github/workflows/containment.yml, "Fleet management +// planes must be unreachable") reached the Incus daemon and Grafana from a +// Hyper-V-isolated job (#135), so per-endpoint vSwitch ACLs cannot be the only +// line of defense. Every container flow is routed and NATed by the host +// network stack (WinNAT), so host firewall rules sit in that path regardless +// of what the vSwitch port enforces — and, like the Linux FORWARD chain, they +// are host-global: one rule set covers every endpoint, including stale ones +// leaked by a crashed run. HNS network-level ACLs were considered instead but +// rejected: they use the same VFP enforcement point as the endpoint ACLs that +// just failed, and they cannot express "this range minus the gateway". +// +// Windows Firewall has no rule ordering and a Block rule always overrides an +// Allow rule, so the Linux pattern "allow the gateway above the RFC1918 deny" +// cannot be ported literally. Instead the container subnet — which contains +// the NAT gateway (DNS, the default route, module-proxy GatewayPorts) and the +// other containers — is subtracted from each blocked range up front +// (subtractCIDR), so it never appears in any block rule. Blocking the gateway +// would brick all container networking: DNS and outbound NAT both go through +// it. +// +// The outbound blocks are scoped localip=: forwarded +// container traffic is evaluated pre-NAT with its container source address, +// so the host's own traffic (sourced from the host LAN address) can never +// match — a mis-scoped rule here must degrade to a no-op, never to cutting +// the fleet host off its own management LAN. +// +// IPv4 only, deliberately: the HCN NAT network is IPv4-only (no v6 IPAM), so +// containers have no IPv6 path, and a host-wide v6 link-local block without a +// container-source scope would break the HOST's neighbor discovery. + +// firewallRulePrefix names every host-firewall rule ephemerd installs so the +// set is findable (netsh advfirewall firewall show rule name=all | findstr +// ephemerd-egress) and removable on Cleanup. +const firewallRulePrefix = "ephemerd-egress" + +func netsh(args ...string) error { + out, err := exec.Command("netsh", args...).CombinedOutput() + if err != nil { + return fmt.Errorf("netsh: %w: %s", err, out) + } + return nil +} + +// winFirewallRule is one host-firewall rule: its unique name (used for the +// idempotent delete-before-add and for removal on Cleanup) and the key=value +// spec that creates it. +type winFirewallRule struct { + name string + spec []string +} + +// addArgs returns the netsh argv that creates the rule. +func (r winFirewallRule) addArgs() []string { + return append([]string{"advfirewall", "firewall", "add", "rule", "name=" + r.name}, r.spec...) +} + +// deleteArgs returns the netsh argv that deletes every rule with this name. +func (r winFirewallRule) deleteArgs() []string { + return []string{"advfirewall", "firewall", "delete", "rule", "name=" + r.name} +} + +// hostFirewallRules returns the full host-firewall rule set for the given +// container subnet, gateway, and control-plane ports: outbound blocks for +// every denied range (with the container subnet carved out) plus inbound +// blocks for container→gateway traffic on the control ports. +// +// Exposed as a pure function (no side effects) so tests can assert the exact +// rule set without invoking netsh. +func hostFirewallRules(subnet, gateway string, controlPorts []int) ([]winFirewallRule, error) { + var rules []winFirewallRule + + // Outbound RFC1918 + link-local blocks. The container subnet is subtracted + // from any overlapping range (see the ordering note above: an allow rule + // cannot outrank a block rule, so the gateway and the container-to-container + // range must never appear inside a blocked range in the first place). + // Everything outside these ranges — the internet — is untouched. + for _, cidr := range egressBlockedCIDRs { + remote, err := subtractCIDR(cidr, subnet) + if err != nil { + return nil, fmt.Errorf("computing blocked ranges for %s: %w", cidr, err) + } + if len(remote) == 0 { + continue // fully covered by the container subnet + } + rules = append(rules, winFirewallRule{ + name: firewallRulePrefix + "-block-" + strings.ReplaceAll(cidr, "/", "_"), + spec: []string{ + "dir=out", + "action=block", + "protocol=any", + "localip=" + subnet, + "remoteip=" + strings.Join(remote, ","), + "profile=any", + "enable=yes", + }, + }) + } + + // Inbound control-plane blocks: container subnet → gateway on the ephemerd + // control ports (containerd, dispatch gRPC, debug exec). Intentionally + // narrow — source = container subnet, destination = gateway, one TCP port + // each — so DNS (53) and NAT stay intact. Mirrors controlPlaneInputRules + // on Linux; traffic addressed to the gateway IP terminates at the host, so + // the outbound blocks above never see it. + for _, port := range controlPorts { + rules = append(rules, winFirewallRule{ + name: fmt.Sprintf("%s-control-%d", firewallRulePrefix, port), + spec: []string{ + "dir=in", + "action=block", + "protocol=TCP", + "localip=" + gateway, + "localport=" + strconv.Itoa(port), + "remoteip=" + subnet, + "profile=any", + "enable=yes", + }, + }) + } + + return rules, nil +} + +// subtractCIDR removes exclude from cidr and renders the remainder in netsh +// remoteip syntax: the original CIDR when the two do not overlap, otherwise up +// to two "start-end" ranges. Returns an empty slice when exclude covers cidr +// entirely. IPv4 only — the HCN NAT network has no IPv6 IPAM. +func subtractCIDR(cidr, exclude string) ([]string, error) { + clo, chi, err := v4Range(cidr) + if err != nil { + return nil, err + } + xlo, xhi, err := v4Range(exclude) + if err != nil { + return nil, err + } + + if xhi < clo || xlo > chi { + return []string{cidr}, nil // no overlap — keep the CIDR as-is + } + + var out []string + if xlo > clo { + out = append(out, u32ToIP(clo)+"-"+u32ToIP(xlo-1)) + } + if xhi < chi { + out = append(out, u32ToIP(xhi+1)+"-"+u32ToIP(chi)) + } + return out, nil +} + +// v4Range returns the first and last address of an IPv4 CIDR as uint32. +func v4Range(cidr string) (lo, hi uint32, err error) { + _, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + return 0, 0, fmt.Errorf("parsing %s: %w", cidr, err) + } + ip4 := ipnet.IP.To4() + if ip4 == nil { + return 0, 0, fmt.Errorf("parsing %s: not an IPv4 CIDR", cidr) + } + ones, bits := ipnet.Mask.Size() + if bits != 32 { + return 0, 0, fmt.Errorf("parsing %s: not an IPv4 mask", cidr) + } + lo = binary.BigEndian.Uint32(ip4) + hi = lo | (1<<(32-ones) - 1) + return lo, hi, nil +} + +// u32ToIP renders a uint32 back to dotted-quad form. +func u32ToIP(v uint32) string { + return net.IPv4(byte(v>>24), byte(v>>16), byte(v>>8), byte(v)).String() +} + +func (w *windowsNetworking) installFirewallRules() error { + // init() always creates the HCN network on DefaultSubnet with + // defaultGateway (cfg.Subnet is not consulted on Windows), so the firewall + // must match those, not cfg.Subnet. + rules, err := hostFirewallRules(DefaultSubnet, defaultGateway, w.cfg.ControlPorts) + if err != nil { + return fmt.Errorf("building host firewall rules: %w", err) + } + + for _, r := range rules { + // Idempotent: delete any rule carrying this name from a previous run + // before adding, so re-running install never accumulates duplicates. + // netsh delete removes every rule matching the name; "no rules match" + // on a fresh host is expected and ignored. + _ = netsh(r.deleteArgs()...) + + w.cfg.Log.Info("adding firewall rule", "rule", r.name) + if err := netsh(r.addArgs()...); err != nil { + // Callers treat this as a warning, not fatal (see + // cmd/ephemerd/main.go): a host where the daemon lacks the + // privilege to program the firewall degrades to the per-endpoint + // ACLs instead of refusing to start. + return fmt.Errorf("adding firewall rule %s: %w", r.name, err) + } + } + + w.cfg.Log.Info("host firewall rules installed", "rules", len(rules)) + return nil +} + +func (w *windowsNetworking) removeFirewallRules() { + // Recompute the same deterministic rule set install built and delete each + // rule by name. Best-effort, like the Linux removal path. + rules, err := hostFirewallRules(DefaultSubnet, defaultGateway, w.cfg.ControlPorts) + if err != nil { + w.cfg.Log.Debug("failed to rebuild firewall rule set for removal", "error", err) + return + } + for _, r := range rules { + if err := netsh(r.deleteArgs()...); err != nil { + w.cfg.Log.Debug("failed to remove firewall rule", "rule", r.name, "error", err) + } + } +} diff --git a/pkg/networking/firewall_windows_test.go b/pkg/networking/firewall_windows_test.go new file mode 100644 index 0000000..4a86170 --- /dev/null +++ b/pkg/networking/firewall_windows_test.go @@ -0,0 +1,229 @@ +//go:build windows + +package networking + +import ( + "slices" + "strings" + "testing" +) + +// specValue extracts the value of a key=value pair from a rule spec, or "" +// when the key is absent. +func specValue(r winFirewallRule, key string) string { + for _, s := range r.spec { + if v, ok := strings.CutPrefix(s, key+"="); ok { + return v + } + } + return "" +} + +func TestSubtractCIDR(t *testing.T) { + tests := []struct { + name string + cidr string + exclude string + want []string + wantErr bool + }{ + { + name: "subnet inside range splits it", + cidr: "10.0.0.0/8", + exclude: "10.88.0.0/16", + want: []string{"10.0.0.0-10.87.255.255", "10.89.0.0-10.255.255.255"}, + }, + { + name: "no overlap keeps the CIDR", + cidr: "172.16.0.0/12", + exclude: "10.88.0.0/16", + want: []string{"172.16.0.0/12"}, + }, + { + name: "exclude equals the range", + cidr: "10.88.0.0/16", + exclude: "10.88.0.0/16", + want: nil, + }, + { + name: "exclude covers the range", + cidr: "10.88.0.0/16", + exclude: "10.0.0.0/8", + want: nil, + }, + { + name: "exclude at the start leaves one range", + cidr: "10.0.0.0/8", + exclude: "10.0.0.0/16", + want: []string{"10.1.0.0-10.255.255.255"}, + }, + { + name: "exclude at the end leaves one range", + cidr: "10.0.0.0/8", + exclude: "10.255.0.0/16", + want: []string{"10.0.0.0-10.254.255.255"}, + }, + { + name: "malformed CIDR errors", + cidr: "not-a-cidr", + exclude: "10.88.0.0/16", + wantErr: true, + }, + { + name: "IPv6 CIDR errors", + cidr: "fc00::/7", + exclude: "10.88.0.0/16", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := subtractCIDR(tt.cidr, tt.exclude) + if tt.wantErr { + if err == nil { + t.Fatalf("subtractCIDR(%q, %q) = %v, want error", tt.cidr, tt.exclude, got) + } + return + } + if err != nil { + t.Fatalf("subtractCIDR(%q, %q): %v", tt.cidr, tt.exclude, err) + } + if !slices.Equal(got, tt.want) { + t.Errorf("subtractCIDR(%q, %q) = %v, want %v", tt.cidr, tt.exclude, got, tt.want) + } + }) + } +} + +// TestHostFirewallRules_BlocksEveryDeniedRange verifies each RFC1918 + +// link-local range produces an outbound block rule; a missing range would let +// a job reach that slice of the LAN. +func TestHostFirewallRules_BlocksEveryDeniedRange(t *testing.T) { + rules, err := hostFirewallRules(DefaultSubnet, defaultGateway, nil) + if err != nil { + t.Fatalf("hostFirewallRules: %v", err) + } + if len(rules) != len(egressBlockedCIDRs) { + t.Fatalf("got %d rules, want %d (one outbound block per denied range)", len(rules), len(egressBlockedCIDRs)) + } + for i, cidr := range egressBlockedCIDRs { + r := rules[i] + wantName := firewallRulePrefix + "-block-" + strings.ReplaceAll(cidr, "/", "_") + if r.name != wantName { + t.Errorf("rule[%d].name = %q, want %q", i, r.name, wantName) + } + if specValue(r, "dir") != "out" || specValue(r, "action") != "block" { + t.Errorf("rule %s is not an outbound block: %v", r.name, r.spec) + } + if specValue(r, "remoteip") == "" { + t.Errorf("rule %s has no remoteip scope", r.name) + } + } +} + +// TestHostFirewallRules_GatewayAndSubnetNeverBlocked pins the safety property: +// the container subnet — which contains the NAT gateway (DNS, default route, +// GatewayPorts) and the other containers — must never appear inside a blocked +// range. Windows Firewall has no rule ordering and Block beats Allow, so a +// blocked gateway could not be rescued by an allow rule: it would brick all +// container networking. +func TestHostFirewallRules_GatewayAndSubnetNeverBlocked(t *testing.T) { + rules, err := hostFirewallRules("10.88.0.0/16", "10.88.0.1", nil) + if err != nil { + t.Fatalf("hostFirewallRules: %v", err) + } + + for _, r := range rules { + remote := specValue(r, "remoteip") + if strings.Contains(remote, "10.88.") { + t.Errorf("rule %s blocks the container subnet: remoteip=%s", r.name, remote) + } + // Outbound blocks must be scoped to container-sourced traffic so the + // host's own LAN access can never match. + if specValue(r, "localip") != "10.88.0.0/16" { + t.Errorf("rule %s not scoped to the container subnet: localip=%s", r.name, specValue(r, "localip")) + } + } + + // The 10.0.0.0/8 block must be split exactly around the subnet. + want := "10.0.0.0-10.87.255.255,10.89.0.0-10.255.255.255" + for _, r := range rules { + if r.name == firewallRulePrefix+"-block-10.0.0.0_8" { + if got := specValue(r, "remoteip"); got != want { + t.Errorf("10/8 block remoteip = %q, want %q", got, want) + } + return + } + } + t.Error("no block rule found for 10.0.0.0/8") +} + +// TestHostFirewallRules_ControlPortRules confirms the container→gateway +// control-plane blocks mirror the Linux INPUT drops: inbound, TCP, one +// specific port each, source = container subnet, destination = gateway — +// never a blanket gateway block and never port 53. +func TestHostFirewallRules_ControlPortRules(t *testing.T) { + ports := []int{10000, 10001, 10002} // containerd, dispatch, debug exec + rules, err := hostFirewallRules(DefaultSubnet, defaultGateway, ports) + if err != nil { + t.Fatalf("hostFirewallRules: %v", err) + } + + var control []winFirewallRule + for _, r := range rules { + if specValue(r, "dir") == "in" { + control = append(control, r) + } + } + if len(control) != len(ports) { + t.Fatalf("got %d inbound rules, want %d (one per control port)", len(control), len(ports)) + } + + for i, port := range []string{"10000", "10001", "10002"} { + r := control[i] + if specValue(r, "action") != "block" || specValue(r, "protocol") != "TCP" { + t.Errorf("rule %s is not a TCP block: %v", r.name, r.spec) + } + if specValue(r, "localport") != port { + t.Errorf("rule %s localport = %s, want %s", r.name, specValue(r, "localport"), port) + } + if specValue(r, "remoteip") != DefaultSubnet { + t.Errorf("rule %s missing source-subnet scope: %v", r.name, r.spec) + } + if specValue(r, "localip") != defaultGateway { + t.Errorf("rule %s missing gateway-dest scope: %v", r.name, r.spec) + } + if specValue(r, "localport") == "53" { + t.Errorf("rule %s blocks DNS (port 53) — must not", r.name) + } + } +} + +// TestHostFirewallRules_NamesAndArgs pins the naming and argv contract: every +// rule carries the ephemerd prefix (so the set is findable and removable), +// and delete targets exactly the name add created (that is what makes +// delete-before-add idempotent). +func TestHostFirewallRules_NamesAndArgs(t *testing.T) { + rules, err := hostFirewallRules(DefaultSubnet, defaultGateway, []int{10000}) + if err != nil { + t.Fatalf("hostFirewallRules: %v", err) + } + for _, r := range rules { + if !strings.HasPrefix(r.name, firewallRulePrefix) { + t.Errorf("rule name %q missing %q prefix", r.name, firewallRulePrefix) + } + add := r.addArgs() + wantAdd := []string{"advfirewall", "firewall", "add", "rule", "name=" + r.name} + if !slices.Equal(add[:5], wantAdd) { + t.Errorf("addArgs()[:5] = %v, want %v", add[:5], wantAdd) + } + if !slices.Equal(add[5:], r.spec) { + t.Errorf("addArgs() spec = %v, want %v", add[5:], r.spec) + } + wantDel := []string{"advfirewall", "firewall", "delete", "rule", "name=" + r.name} + if !slices.Equal(r.deleteArgs(), wantDel) { + t.Errorf("deleteArgs() = %v, want %v", r.deleteArgs(), wantDel) + } + } +} diff --git a/pkg/networking/network_windows.go b/pkg/networking/network_windows.go index 3b1677b..e963637 100644 --- a/pkg/networking/network_windows.go +++ b/pkg/networking/network_windows.go @@ -230,15 +230,9 @@ func (w *windowsNetworking) applyACLPolicies(endpoint *hcn.HostComputeEndpoint) }) } -func (w *windowsNetworking) installFirewallRules() error { - // ACL policies are applied per-endpoint in setup(), not globally - w.cfg.Log.Info("Windows ACL firewall policies configured per-endpoint") - return nil -} - -func (w *windowsNetworking) removeFirewallRules() { - // ACL policies are removed when endpoints are deleted -} +// installFirewallRules and removeFirewallRules live in firewall_windows.go +// (mirroring firewall_linux.go): the host-global Windows Firewall backstop +// that complements the per-endpoint ACLs applied above. func (w *windowsNetworking) cleanup() {}