diff --git a/cmd/cli/dns_intercept_darwin.go b/cmd/cli/dns_intercept_darwin.go index 36836cc..708c87f 100644 --- a/cmd/cli/dns_intercept_darwin.go +++ b/cmd/cli/dns_intercept_darwin.go @@ -12,6 +12,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "sync/atomic" "syscall" "time" @@ -83,6 +84,51 @@ const ( pfAnchorFile = "/etc/pf.anchors/com.controld.ctrld" ) +// pfAnchorWriteMu serializes writers of the anchor file. Seven code paths rebuild it +// (startup, tunnel change, watchdog restore, VPN DNS exemptions, forced reload, +// forwarded-source reconcile, firewall shutdown) from timers and network-change +// callbacks in their own goroutines, with no lock between them. +var pfAnchorWriteMu sync.Mutex + +// writePFAnchorFile replaces the anchor file atomically: write a temp file in the +// same directory, then rename over the target. +// +// os.WriteFile truncates and then writes, so a pfctl -f racing that window can read a +// partial ruleset and reject the anchor - taking DNS interception down until the next +// watchdog restore. A rename is atomic, so a concurrent reader sees either the whole +// previous ruleset or the whole new one. +// +// Two writers can still be followed by two loads in either order, but every load now +// sees a complete ruleset, and each writer's load re-applies a full anchor, so the +// worst case is redundant work rather than a broken anchor. +func writePFAnchorFile(rules string) error { + pfAnchorWriteMu.Lock() + defer pfAnchorWriteMu.Unlock() + + tmp, err := os.CreateTemp(filepath.Dir(pfAnchorFile), ".ctrld-anchor-*") + if err != nil { + return err + } + tmpName := tmp.Name() + defer func() { + // No-op once the rename succeeded; removes the temp file on any failure path. + _ = os.Remove(tmpName) + }() + + if _, err := tmp.WriteString(rules); err != nil { + tmp.Close() + return err + } + if err := tmp.Chmod(0644); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + return os.Rename(tmpName, pfAnchorFile) +} + // pfState holds the state of the pf DNS interception on macOS. type pfState struct { anchorFile string @@ -240,12 +286,13 @@ func (p *prog) startDNSIntercept() error { } } - rules := p.buildPFAnchorRules(initialExemptions) + forwarded := p.currentForwardedSources() + rules := p.buildPFAnchorRulesWith(initialExemptions, forwarded) if err := os.MkdirAll(pfAnchorDir, 0755); err != nil { return fmt.Errorf("dns intercept: failed to create pf anchor directory %s: %w", pfAnchorDir, err) } - if err := os.WriteFile(pfAnchorFile, []byte(rules), 0644); err != nil { + if err := writePFAnchorFile(rules); err != nil { return fmt.Errorf("dns intercept: failed to write pf anchor file %s: %w", pfAnchorFile, err) } mainLog.Load().Debug().Msgf("DNS intercept: wrote pf anchor file: %s", pfAnchorFile) @@ -255,6 +302,9 @@ func (p *prog) startDNSIntercept() error { os.Remove(pfAnchorFile) return fmt.Errorf("dns intercept: failed to load pf anchor: %w (output: %s)", err, strings.TrimSpace(string(out))) } + // Baseline the reconcile snapshot: the anchor just loaded already contains this + // set, so the first watchdog tick must not treat it as a change. + p.recordAppliedForwardedSources(forwarded) mainLog.Load().Debug().Msgf("DNS intercept: loaded pf anchor %q from %s", pfAnchorName, pfAnchorFile) if err := p.ensurePFAnchorReference(); err != nil { @@ -747,7 +797,22 @@ func (p *prog) validateDNSIntercept() error { // from the redirect to prevent ctrld from querying itself in a loop. // // pf requires strict rule ordering: translation (rdr) BEFORE filtering (pass). +// buildPFAnchorRules detects the current forwarded-source set itself. Callers that +// need to know which set the anchor was built from - so they can record it as applied +// - should use buildPFAnchorRulesWith instead. func (p *prog) buildPFAnchorRules(vpnExemptions []vpnDNSExemption) string { + return p.buildPFAnchorRulesWith(vpnExemptions, p.currentForwardedSources()) +} + +// buildPFAnchorRulesWith is buildPFAnchorRules over an explicit forwarded-source set. +// +// Every rebuild path used to re-detect internally, which meant the caller could not +// tell what it had just installed. The reconcile snapshot therefore stayed at the +// older set, and the next reconcile "discovered" the same change again: another +// rebuild, another round of killed states, and a transition logged for something that +// had already taken effect. Passing the set in lets each path record exactly what pf +// accepted. +func (p *prog) buildPFAnchorRulesWith(vpnExemptions []vpnDNSExemption, forwardedSources []forwardedSource) string { // Read the actual listener address from config. In intercept mode, ctrld may // be on a non-standard port (e.g., 127.0.0.1:5354) if mDNSResponder holds *:53. // The pf rdr rules must redirect to wherever ctrld is actually listening. @@ -969,6 +1034,9 @@ func (p *prog) buildPFAnchorRules(vpnExemptions []vpnDNSExemption) string { // DNS intercept rules must evaluate first so that DNS queries work (they're how // IPs get into the allowlist in the first place). if p.firewallModeEnabled() { + // Make declared VM/container source subnets first-class firewall + // clients (DNS forced through ctrld) before the blanket allowlist block. + rules.WriteString(buildPFForwardedSourceRulesFor(forwardedSources, listenerIP)) rules.WriteString(buildPFFirewallRules()) } @@ -1128,8 +1196,9 @@ func (p *prog) checkTunnelInterfaceChanges() bool { if p.vpnDNS != nil { vpnExemptions = p.vpnDNS.CurrentExemptions() } - rulesStr := p.buildPFAnchorRules(vpnExemptions) - if err := os.WriteFile(pfAnchorFile, []byte(rulesStr), 0644); err != nil { + forwarded := p.currentForwardedSources() + rulesStr := p.buildPFAnchorRulesWith(vpnExemptions, forwarded) + if err := writePFAnchorFile(rulesStr); err != nil { mainLog.Load().Error().Err(err).Msg("DNS intercept: failed to write rebuilt anchor file") return true } @@ -1138,6 +1207,7 @@ func (p *prog) checkTunnelInterfaceChanges() bool { mainLog.Load().Error().Err(err).Msgf("DNS intercept: failed to reload rebuilt anchor (output: %s)", strings.TrimSpace(string(out))) return true } + p.recordAppliedForwardedSources(forwarded) flushPFStates() mainLog.Load().Info().Msgf("DNS intercept: rebuilt pf anchor with %d tunnel interfaces", len(current)) @@ -1388,13 +1458,15 @@ func (p *prog) ensurePFAnchorActive() bool { if p.vpnDNS != nil { vpnExemptions = p.vpnDNS.CurrentExemptions() } - rulesStr := p.buildPFAnchorRules(vpnExemptions) - if err := os.WriteFile(pfAnchorFile, []byte(rulesStr), 0644); err != nil { + forwarded := p.currentForwardedSources() + rulesStr := p.buildPFAnchorRulesWith(vpnExemptions, forwarded) + if err := writePFAnchorFile(rulesStr); err != nil { mainLog.Load().Error().Err(err).Msg("DNS intercept watchdog: failed to write anchor file") } else if out, err := exec.Command("pfctl", "-a", pfAnchorName, "-f", pfAnchorFile).CombinedOutput(); err != nil { p.pfBackoffResourceExhaustion(err, out, "load rebuilt anchor") mainLog.Load().Error().Err(err).Msgf("DNS intercept watchdog: failed to load rebuilt anchor (output: %s)", strings.TrimSpace(string(out))) } else { + p.recordAppliedForwardedSources(forwarded) flushPFStates() mainLog.Load().Info().Msg("DNS intercept watchdog: rebuilt and loaded anchor rules") } @@ -1510,6 +1582,10 @@ func (p *prog) scheduleDelayedRechecks() { if p.vpnDNS != nil { p.vpnDNS.Refresh(ctx, true) } + // A VM/container network often gets its address slightly after the + // interface appears, so the immediate handler can see it without a subnet. + // Re-check here (no-op when the forwarded-source set is unchanged). + p.reconcileForwardedSources() }) p.pfDelayedRecheckTimers = append(p.pfDelayedRecheckTimers, timer) } @@ -1567,6 +1643,13 @@ func (p *prog) pfWatchdog() { mainLog.Load().Info().Msgf("DNS intercept watchdog: pf anchor stable again after %d consecutive restores", old) } } + + // VM/container networks appear and disappear without always + // producing a network-change event, and an intact anchor passes every + // check above. Reconciling here bounds how long a started guest can go + // untrusted (or a stopped one stay trusted) to one watchdog interval. + // No-op when the effective forwarded-source set is unchanged. + p.reconcileForwardedSources() } } } @@ -1581,9 +1664,10 @@ func (p *prog) exemptVPNDNSServers(exemptions []vpnDNSExemption) error { return fmt.Errorf("pf state not available") } - rulesStr := p.buildPFAnchorRules(exemptions) + forwarded := p.currentForwardedSources() + rulesStr := p.buildPFAnchorRulesWith(exemptions, forwarded) - if err := os.WriteFile(pfAnchorFile, []byte(rulesStr), 0644); err != nil { + if err := writePFAnchorFile(rulesStr); err != nil { return fmt.Errorf("dns intercept: failed to rewrite pf anchor: %w", err) } @@ -1591,6 +1675,7 @@ func (p *prog) exemptVPNDNSServers(exemptions []vpnDNSExemption) error { if err != nil { return fmt.Errorf("dns intercept: failed to reload pf anchor: %w (output: %s)", err, strings.TrimSpace(string(out))) } + p.recordAppliedForwardedSources(forwarded) // Flush stale pf states so packets are re-evaluated against new rules. flushPFStates() @@ -1832,11 +1917,14 @@ func (p *prog) forceReloadPFMainRuleset() { if p.vpnDNS != nil { vpnExemptions = p.vpnDNS.CurrentExemptions() } - rulesStr := p.buildPFAnchorRules(vpnExemptions) - if err := os.WriteFile(pfAnchorFile, []byte(rulesStr), 0644); err != nil { + forwarded := p.currentForwardedSources() + rulesStr := p.buildPFAnchorRulesWith(vpnExemptions, forwarded) + if err := writePFAnchorFile(rulesStr); err != nil { mainLog.Load().Error().Err(err).Msg("DNS intercept: force reload — failed to write anchor file") } else if out, err := exec.Command("pfctl", "-a", pfAnchorName, "-f", pfAnchorFile).CombinedOutput(); err != nil { mainLog.Load().Error().Err(err).Msgf("DNS intercept: force reload — failed to load anchor (output: %s)", strings.TrimSpace(string(out))) + } else { + p.recordAppliedForwardedSources(forwarded) } // Flush stale rdr/reply states after the forced ruleset + anchor reload. diff --git a/cmd/cli/dns_intercept_others.go b/cmd/cli/dns_intercept_others.go index 50c7fd0..9106d5b 100644 --- a/cmd/cli/dns_intercept_others.go +++ b/cmd/cli/dns_intercept_others.go @@ -38,6 +38,9 @@ func (p *prog) scheduleDelayedRechecks() {} // pfInterceptMonitor is a no-op on unsupported platforms. func (p *prog) pfInterceptMonitor() {} +// reconcileForwardedSources is a no-op on unsupported platforms (macOS-only). +func (p *prog) reconcileForwardedSources() {} + // osHealthcheckSuppressed always returns false on non-Windows platforms — // WFP loopback protect (the trigger for suppression) is Windows-only. func (p *prog) osHealthcheckSuppressed() bool { return false } diff --git a/cmd/cli/dns_intercept_windows.go b/cmd/cli/dns_intercept_windows.go index b35d3fd..3471ef4 100644 --- a/cmd/cli/dns_intercept_windows.go +++ b/cmd/cli/dns_intercept_windows.go @@ -1742,6 +1742,10 @@ func (p *prog) nrptHealthMonitor(state *wfpState) { // and don't suffer from the pf translation state corruption that macOS has. func (p *prog) pfInterceptMonitor() {} +// reconcileForwardedSources is a no-op on Windows — forwarded-workload DNS +// interception is macOS-pf-only; Windows Firewall Mode is tracked separately. +func (p *prog) reconcileForwardedSources() {} + const ( // nrptProbeDomain is the suffix used for NRPT verification probe queries. // Probes use "_nrpt-probe-." — ctrld recognizes the diff --git a/cmd/cli/dns_proxy.go b/cmd/cli/dns_proxy.go index 45c00e3..cccf039 100644 --- a/cmd/cli/dns_proxy.go +++ b/cmd/cli/dns_proxy.go @@ -1801,6 +1801,10 @@ func (p *prog) monitorNetworkChanges(ctx context.Context) error { p.Info().Str("interface", changedIface). Msg("DNS intercept: interface appeared/disappeared — starting interception probe monitor") go p.pfInterceptMonitor() + // A VM/container bridge appearing/disappearing changes the + // effective forwarded-source set; rebuild the anchor if so, since + // the probe monitor alone won't (an intact anchor passes its probe). + p.reconcileForwardedSources() } } } @@ -1902,6 +1906,9 @@ func (p *prog) monitorNetworkChanges(ctx context.Context) error { if p.vpnDNS != nil { p.vpnDNS.Refresh(ctrld.LoggerCtx(ctx, p.logger.Load()), true) } + // Rebuild the anchor if a VM/container bridge appeared/disappeared + // on this network change (no-op when the forwarded-source set is unchanged). + p.reconcileForwardedSources() // Schedule delayed re-checks to catch async VPN teardown changes. p.scheduleDelayedRechecks() } diff --git a/cmd/cli/firewall_darwin.go b/cmd/cli/firewall_darwin.go index 26bc84a..7206208 100644 --- a/cmd/cli/firewall_darwin.go +++ b/cmd/cli/firewall_darwin.go @@ -4,9 +4,10 @@ package cli import ( "fmt" + "net" "net/netip" - "os" "os/exec" + "sort" "strings" "sync" "time" @@ -25,6 +26,34 @@ type pfFirewallState struct { // batchTimer fires after the accumulation window to flush pending changes. batchTimer *time.Timer + + // lastForwardedSources is the forwarded-source set (auto-detected VM networks + + // configured) that pf has actually accepted, and lastForwardedKey its + // order-independent signature. reconcileForwardedSources compares against these + // to rebuild the anchor when guests appear/disappear, and to know which subnets + // gained or lost trust so their stale pf states can be dropped. Both advance only + // after a successful anchor load, so a failed reload is retried, never latched. + lastForwardedSources []forwardedSource + lastForwardedKey string + + // applyForwardedMu serializes the compare-reload-record sequence in + // applyForwardedSourceChange, so a watchdog tick and a network change cannot both + // rebuild the anchor for the same transition or interleave their snapshot updates. + // Separate from mu, which must not be held across pfctl execution. + applyForwardedMu sync.Mutex +} + +// forwardedSourceWarnTracker dedupes the "unusable configured entry" warning by +// signature. Config is re-parsed on every anchor build and every watchdog tick, so +// warning unconditionally would repeat the same line for the life of the process; +// comparing signatures still reports an entry a config reload has just introduced. +// +// Package-level rather than a pfFirewallState field because it has to work before +// that state exists: initPlatformFirewall defers pf enforcement until intercept mode +// starts, so Firewall Mode can be on with no pfFirewallState yet. +var forwardedSourceWarnTracker struct { + mu sync.Mutex + key string } const ( @@ -36,6 +65,93 @@ const ( pfFirewallBatchInterval = 200 * time.Millisecond ) +// hypervisorVMNetPrefixes are interface-name prefixes that are specific to a +// virtualization vendor's private VM/NAT network, and therefore reliable proof of +// VM ownership by name alone. +// +// Two vendors still put the subnet directly on such an interface: Parallels (vnic0) +// and VirtualBox host-only (vboxnet0). vmnet* covers legacy kext-based VMware Fusion +// on Intel. They are also what proves ownership of a bridge they are a member of - +// see bridgeHasVMMember. +// +// On current macOS this is not sufficient on its own. vmnet.framework - which backs +// Virtualization.framework guests (UTM, Docker Desktop, Multipass, Fusion 12.1+ NAT) +// - puts the RFC1918 gateway address on a bridge10x interface and attaches the +// vendor-named vmenet* interface as an address-less member of it. Matching on name +// alone therefore never fires for those stacks, which is why bridge membership is +// checked too. +var hypervisorVMNetPrefixes = []string{ + "vmnet", // VMware Fusion (legacy kext-based, Intel) + "vmenet", // vmnet.framework member interface (UTM, Docker, Multipass, Fusion 12.1+) + "vnic", // Parallels Desktop + "vboxnet", // VirtualBox host-only +} + +// isHypervisorVMNetIface reports whether an interface name is a known +// vendor-specific VM/NAT network (see hypervisorVMNetPrefixes). +func isHypervisorVMNetIface(name string) bool { + for _, prefix := range hypervisorVMNetPrefixes { + if strings.HasPrefix(name, prefix) { + return true + } + } + return false +} + +// isBridgeIface reports whether name is a macOS bridge interface. +// +// A bridge name is deliberately NOT treated as proof of anything: macOS uses this +// namespace for Thunderbolt/aggregated links too (this repo's own tunnel-change code +// lists bridge0 among physical interfaces), and auto-trusting those could force-route +// unrelated same-subnet traffic. Ownership comes from the member list instead. +func isBridgeIface(name string) bool { + return strings.HasPrefix(name, "bridge") +} + +// bridgeMembersFn returns a bridge's member interfaces. Indirected so detection can +// be tested without a hypervisor or ifconfig. +var bridgeMembersFn = bridgeMembers + +// bridgeMembers returns the member interfaces of a macOS bridge, via ifconfig. +// There is no address-family-independent syscall for this that does not mean +// hand-rolling SIOCGDRVSPEC/BRDGGIFS structs, and this runs at most once per +// candidate bridge per anchor build. +func bridgeMembers(name string) []string { + out, err := exec.Command("ifconfig", name).CombinedOutput() + if err != nil { + return nil + } + return parseBridgeMembers(string(out)) +} + +// parseBridgeMembers extracts member interface names from ifconfig output, which +// lists each as a line of the form "\tmember: vmenet0 flags=3". +func parseBridgeMembers(ifconfigOut string) []string { + var members []string + for _, line := range strings.Split(ifconfigOut, "\n") { + fields := strings.Fields(line) + if len(fields) < 2 || fields[0] != "member:" { + continue + } + members = append(members, fields[1]) + } + return members +} + +// bridgeHasVMMember reports whether a bridge's member list proves VM/NAT ownership, +// i.e. whether a vendor-specific VM interface is bridged into it. +// +// This is the ownership evidence a bridge name lacks. A vmnet.framework bridge has a +// vmenet* member; a Thunderbolt bridge has en* members and so is never trusted. +func bridgeHasVMMember(members []string) bool { + for _, m := range members { + if isHypervisorVMNetIface(m) { + return true + } + } + return false +} + // firewallFlushPlatform flushes the pf table on macOS. func (p *prog) firewallFlushPlatform() { p.pfFirewallFlushTable() @@ -56,7 +172,7 @@ func (p *prog) shutdownPlatformFirewall() { vpnExemptions = p.vpnDNS.CurrentExemptions() } rulesStr := p.buildPFAnchorRules(vpnExemptions) - if err := os.WriteFile(pfAnchorFile, []byte(rulesStr), 0644); err != nil { + if err := writePFAnchorFile(rulesStr); err != nil { p.Warn().Err(err).Msg("Firewall: failed to write pf anchor during shutdown") return } @@ -71,7 +187,7 @@ func (p *prog) initPlatformFirewall() { return } - // pf enforcement is only meaningful when intercept mode is active — + // pf enforcement is only meaningful when intercept mode is active - // without it, we have no pf anchor to add rules to. if dnsIntercept && p.dnsInterceptState != nil { p.initPFFirewall() @@ -95,7 +211,7 @@ func (p *prog) initPFFirewall() { state := &pfFirewallState{} p.platformFirewallState = state - // Register batch callback — AllowList reaper and FlushDomain use this. + // Register batch callback - AllowList reaper and FlushDomain use this. p.allowList.SetOnBatchChange(func(added []netip.Addr, removed []netip.Addr) { state.mu.Lock() defer state.mu.Unlock() @@ -109,7 +225,7 @@ func (p *prog) initPFFirewall() { state.scheduleBatchFlush(p) }) - // Register individual change callback — Add() and Remove() use this. + // Register individual change callback - Add() and Remove() use this. p.allowList.SetOnChange(func(ip netip.Addr, added bool) { state.mu.Lock() defer state.mu.Unlock() @@ -127,6 +243,17 @@ func (p *prog) initPFFirewall() { // as the in-memory allowlist. p.pfFirewallPopulateTable() + // Seed the forwarded-source snapshot with what the anchor was just built with, + // so the first reconcile only fires on a real subsequent change. + sources := p.forwardedSources() + state.lastForwardedSources = sources + state.lastForwardedKey = forwardedSourceSetKey(sources) + + // Report the effective trust set, including the configured entries. Without this + // an admin who sets firewall_forwarded_sources has no way to confirm it took + // effect short of reading pf rules. + p.logForwardedSources(sources) + p.Info().Msg("Firewall: pf table enforcement initialized") } @@ -190,7 +317,7 @@ func (s *pfFirewallState) flushBatch(p *prog) { if len(tableRemoves) > 0 { args := append([]string{"-a", pfAnchorName, "-t", pfFirewallTable, "-T", "delete"}, tableRemoves...) if out, err := exec.Command("pfctl", args...).CombinedOutput(); err != nil { - // Not a hard error — the IP may have already been removed (e.g., by a Flush). + // Not a hard error - the IP may have already been removed (e.g., by a Flush). p.Debug().Err(err).Str("output", string(out)). Msgf("Firewall: failed to remove %d IPs from pf table (may already be gone)", len(tableRemoves)) } else { @@ -275,17 +402,17 @@ func buildPFFirewallRules() string { fmt.Fprintf(&rules, "pass out quick inet proto { tcp, udp } from any to <%s>\n", pfFirewallTable) fmt.Fprintf(&rules, "pass out quick inet6 proto { tcp, udp } from any to <%s>\n\n", pfFirewallTable) - // Allow ICMP/ICMPv6 — needed for path MTU discovery, ping, etc. + // Allow ICMP/ICMPv6 - needed for path MTU discovery, ping, etc. rules.WriteString("# Allow ICMP (path MTU discovery, ping, etc.)\n") rules.WriteString("pass out quick inet proto icmp\n") rules.WriteString("pass out quick inet6 proto icmp6\n\n") - // Allow all loopback traffic (safety net — permanent allowlist covers this too). + // Allow all loopback traffic (safety net - permanent allowlist covers this too). rules.WriteString("# Allow all loopback traffic.\n") rules.WriteString("pass out quick on lo0\n") rules.WriteString("pass in quick on lo0\n\n") - // Allow RFC1918 and link-local — these are in the permanent allowlist but + // Allow RFC1918 and link-local - these are in the permanent allowlist but // explicit pf rules prevent the block rule below from catching them. rules.WriteString("# Allow private/link-local ranges (LAN, printers, NAS, mDNS, DHCP).\n") rules.WriteString("pass out quick inet proto { tcp, udp } from any to 10.0.0.0/8\n") @@ -300,7 +427,7 @@ func buildPFFirewallRules() string { rules.WriteString("pass out quick inet proto { tcp, udp } from any to 224.0.0.0/4\n") rules.WriteString("pass out quick inet6 proto { tcp, udp } from any to ff00::/8\n\n") - // Allow DHCP (UDP 67/68) — needed for network configuration. + // Allow DHCP (UDP 67/68) - needed for network configuration. rules.WriteString("# Allow DHCP.\n") rules.WriteString("pass out quick inet proto udp from any port 68 to any port 67\n\n") @@ -312,3 +439,568 @@ func buildPFFirewallRules() string { return rules.String() } + +// forwardedSource is a trusted VM/container source subnet. iface is the ingress +// interface for an auto-detected source, used to scope its pf rules to that +// interface so unrelated same-subnet traffic on other interfaces is unaffected. +// iface is empty for an operator-configured source, which matches on the source +// CIDR alone — an explicit opt-in the admin is responsible for. +type forwardedSource struct { + prefix netip.Prefix + iface string +} + +// firewallForwardedSources parses the operator-configured forwarded-workload +// source subnets (service.firewall_forwarded_sources), dropping - with a warning - +// any entry that is not a valid CIDR, or is not IPv4, so one bad line never voids the +// whole set. These augment auto-detection (see forwardedSources) and are the supported +// way to trust generic bridge* stacks (Multipass, Docker Desktop). +// +// The IPv4 restriction is not cosmetic: forwarded DNS can only be redirected to +// ctrld's IPv4 intercept listener, and an IPv6 source would otherwise produce pf rules +// whose address family contradicts their source literal, which makes pfctl reject the +// entire anchor and take DNS interception down with it. +func (p *prog) firewallForwardedSources() []forwardedSource { + sources, rejected := parseForwardedSourceConfig(p.cfg.Service.FirewallForwardedSources) + p.warnRejectedForwardedSources(rejected) + return sources +} + +// rejectedForwardedSource is a configured entry that could not be used, with the +// reason to report to the operator. +type rejectedForwardedSource struct { + value string + reason string +} + +// parseForwardedSourceConfig parses configured entries, returning the usable sources +// and the rejected ones. It is pure and silent: callers decide when a rejection is +// worth logging, because this runs on every anchor build and every watchdog tick. +func parseForwardedSourceConfig(raw []string) ([]forwardedSource, []rejectedForwardedSource) { + if len(raw) == 0 { + return nil, nil + } + out := make([]forwardedSource, 0, len(raw)) + var rejected []rejectedForwardedSource + for _, s := range raw { + pfx, err := netip.ParsePrefix(strings.TrimSpace(s)) + if err != nil { + rejected = append(rejected, rejectedForwardedSource{ + value: s, + reason: "not a valid CIDR (want e.g. 192.168.64.0/24): " + err.Error(), + }) + continue + } + if !pfx.Addr().Is4() { + rejected = append(rejected, rejectedForwardedSource{ + value: s, + reason: "not IPv4 - forwarded-workload DNS interception is IPv4-only (ctrld's intercept listener is IPv4)", + }) + continue + } + out = append(out, forwardedSource{prefix: pfx.Masked()}) + } + return out, rejected +} + +// warnRejectedForwardedSources reports unusable configured entries, but only when the +// set of rejections changes (see forwardedSourceWarnTracker). +func (p *prog) warnRejectedForwardedSources(rejected []rejectedForwardedSource) { + key := rejectedForwardedSourcesKey(rejected) + + forwardedSourceWarnTracker.mu.Lock() + unchanged := forwardedSourceWarnTracker.key == key + forwardedSourceWarnTracker.key = key + forwardedSourceWarnTracker.mu.Unlock() + + if unchanged { + return + } + for _, r := range rejected { + p.Warn().Str("value", r.value). + Msgf("Firewall: ignoring firewall_forwarded_sources entry - %s", r.reason) + } +} + +// rejectedForwardedSourcesKey returns an order-independent signature of a rejection +// set, so re-parsing an unchanged config is recognised as nothing new to report. +func rejectedForwardedSourcesKey(rejected []rejectedForwardedSource) string { + if len(rejected) == 0 { + return "" + } + parts := make([]string, 0, len(rejected)) + for _, r := range rejected { + parts = append(parts, r.value+"|"+r.reason) + } + sort.Strings(parts) + return strings.Join(parts, ",") +} + +// detectForwardedSources auto-discovers VM/NAT networks on the host so the common +// case works with no configuration. +// +// An interface qualifies only if it is up, carries an RFC1918 IPv4 network, AND its +// VM ownership can be proven one of two ways: +// +// - its own name is vendor-specific (hypervisorVMNetPrefixes): Parallels, legacy +// Fusion, VirtualBox host-only; +// - it is a bridge whose member list contains a vendor-specific interface +// (bridgeHasVMMember): every vmnet.framework stack, where the address lives on +// bridge10x and the vendor-named vmenet* member has none. +// +// Ownership proof is the security boundary: a bridge is trusted for what is bridged +// into it, never for its name. The RFC1918 filter is the second boundary - a stack +// presenting a public range is never auto-trusted. Each detected source keeps its +// ingress interface so its pf rules stay scoped to it. +// +// Addresses are checked before membership so the ifconfig call is only made for a +// bridge that could actually qualify; a Thunderbolt bridge with no RFC1918 address +// costs nothing. +func (p *prog) detectForwardedSources() []forwardedSource { + ifaces, err := net.Interfaces() + if err != nil { + p.Warn().Err(err).Msg("Firewall: could not enumerate interfaces for forwarded-source detection") + return nil + } + var out []forwardedSource + for _, ifi := range ifaces { + if ifi.Flags&net.FlagUp == 0 { + continue + } + addrs, err := ifi.Addrs() + if err != nil { + continue + } + sources, reason := forwardedSourcesForIface(ifi.Name, addrs, bridgeMembersFn) + for _, src := range sources { + out = append(out, src) + // Debug, not Info: detection re-runs on every anchor build. The effective + // set is reported once by logForwardedSources at init and on every change. + p.Debug().Str("iface", src.iface).Str("subnet", src.prefix.String()).Str("reason", reason). + Msg("Firewall: auto-detected VM/container network for forwarded DNS") + } + } + return out +} + +// forwardedSourcesForIface decides whether one up interface is an auto-trusted +// forwarded-workload source, and returns its subnets plus why it qualified. +// +// Split out from interface enumeration so the trust decision - which is a security +// boundary - is testable against synthetic interfaces, including the cases that must +// NOT qualify: a Thunderbolt bridge, a public-range VM network, an address-less +// vendor interface. +// +// members is only consulted for a bridge that already has an RFC1918 IPv4 address, so +// no subprocess runs for the address-less or public bridges on a typical host. +func forwardedSourcesForIface(name string, addrs []net.Addr, members func(string) []string) ([]forwardedSource, string) { + vendorNamed := isHypervisorVMNetIface(name) + if !vendorNamed && !isBridgeIface(name) { + return nil, "" + } + prefixes := privateIPv4Prefixes(addrs) + if len(prefixes) == 0 { + return nil, "" + } + reason := "vendor VM interface" + if !vendorNamed { + mem := members(name) + if !bridgeHasVMMember(mem) { + return nil, "" + } + reason = "bridge with VM member " + strings.Join(vmMembers(mem), ",") + } + out := make([]forwardedSource, 0, len(prefixes)) + for _, pfx := range prefixes { + out = append(out, forwardedSource{prefix: pfx, iface: name}) + } + return out, reason +} + +// privateIPv4Prefixes returns the masked RFC1918 IPv4 networks among addrs, skipping +// public and IPv6 addresses. +func privateIPv4Prefixes(addrs []net.Addr) []netip.Prefix { + var out []netip.Prefix + for _, a := range addrs { + ipnet, ok := a.(*net.IPNet) + if !ok { + continue + } + pfx, err := netip.ParsePrefix(ipnet.String()) + if err != nil { + continue + } + pfx = pfx.Masked() + if !pfx.Addr().Is4() || !pfx.Addr().IsPrivate() { + continue + } + out = append(out, pfx) + } + return out +} + +// vmMembers returns the vendor VM interfaces among a bridge's members, for logging +// which member made the bridge trusted. +func vmMembers(members []string) []string { + var out []string + for _, m := range members { + if isHypervisorVMNetIface(m) { + out = append(out, m) + } + } + return out +} + +// forwardedSources returns the effective trusted set: auto-detected vendor VM +// networks UNION operator-configured subnets, de-duplicated by prefix (a subnet +// that is both auto-detected and configured keeps the interface-scoped +// auto-detected form). Config augments auto-detection; it never disables it. +func (p *prog) forwardedSources() []forwardedSource { + var out []forwardedSource + seen := make(map[netip.Prefix]struct{}) + for _, group := range [][]forwardedSource{ + p.detectForwardedSources(), + p.firewallForwardedSources(), + } { + for _, src := range group { + if _, dup := seen[src.prefix]; dup { + continue + } + seen[src.prefix] = struct{}{} + out = append(out, src) + } + } + return out +} + +// forwardedSourceSetKey returns a deterministic, order-independent signature of a +// forwarded-source set. reconcileForwardedSources compares it across time to detect +// when VM/container interfaces appear or disappear. +func forwardedSourceSetKey(sources []forwardedSource) string { + parts := make([]string, 0, len(sources)) + for _, s := range sources { + parts = append(parts, s.iface+"|"+s.prefix.String()) + } + sort.Strings(parts) + return strings.Join(parts, ",") +} + +// currentForwardedSources returns the effective forwarded-source set, or nil when +// Firewall Mode is off. +// +// The gate matters because detection is not free: it enumerates interfaces, execs +// ifconfig for each candidate bridge, and re-parses config. Those rules are only ever +// emitted in firewall mode, so doing any of it with firewall mode off is pure waste - +// and it would report unusable config entries on a path where nothing wants them. +// Anchor rebuilds happen on every tunnel change, watchdog restore and VPN DNS update +// regardless of firewall mode, so this is the difference between zero work and work on +// every one of them. +func (p *prog) currentForwardedSources() []forwardedSource { + if !p.firewallModeEnabled() { + return nil + } + return p.forwardedSources() +} + +// buildPFForwardedSourceRulesFor generates the pf rules that make the given +// VM/container (forwarded/NATed) source subnets first-class Firewall Mode clients +// WITHOUT an interface-wide bypass. Returns "" for an empty input so anchor +// behavior is unchanged when there are no sources. +// +// For each source subnet: +// +// - Plaintext DNS (port 53) is force-routed through ctrld's loopback listener +// (route-to lo0, which then hits the existing rdr-on-lo0 redirect). ctrld +// therefore observes and policy-enforces every guest resolution, and the +// resolved IP lands in . The guest's subsequent egress to that +// IP is permitted by the existing "pass out ... to " rule: NAT +// rewrites the guest source to the host, which that rule's "from any" covers. +// - DoT (port 853) to any resolver is blocked so the guest cannot swap in an +// alternate encrypted resolver to escape policy. DoH over 443 is +// indistinguishable from ordinary HTTPS and is a documented limitation. +// +// Auto-detected sources carry their ingress interface and are scoped with +// "on ", so an unrelated interface on the same private range is never +// affected. Configured sources (admin opt-in) match on the source CIDR alone. +// All matches are on the pre-NAT guest source (inbound): after NAT the source is +// the host and could no longer be told apart. This is an explicit, per-subnet +// trust boundary - a direct public IP the guest never resolved through ctrld stays +// blocked, so the guest cannot bypass Control D policy. +// +// IPv4 only, and strictly per address family: ctrld's intercept listener is IPv4, so +// IPv6 guest DNS cannot be redirected here - the anchor's existing +// "block out ... inet6 ... port 53" rule forces guests to fall back to interceptable +// IPv4 DNS, and guest IPv6 egress to an alternate resolver is covered by the blanket +// IPv6 block plus the fact that such a resolver never enters . +// Consequently only IPv4 sources produce rules, and each rule's address family +// matches its source literal: pf rejects a whole anchor over a single +// "inet6 ... from 192.168.x.0/24" mismatch, which would take DNS interception down +// with it. Non-IPv4 sources are skipped here as a backstop; firewallForwardedSources +// already drops them at parse time with a warning. +func buildPFForwardedSourceRulesFor(sources []forwardedSource, listenerIP string) string { + sources = ipv4ForwardedSources(sources) + if len(sources) == 0 { + return "" + } + + var rules strings.Builder + rules.WriteString("\n# --- Firewall Mode: forwarded workload (VM/container) DNS interception ---\n") + rules.WriteString("# VM/container source subnets (auto-detected vendor VM networks + configured).\n") + rules.WriteString("# Their DNS is forced through ctrld so guest resolutions are policy-enforced and\n") + fmt.Fprintf(&rules, "# populate <%s>; egress to allowed IPs is then permitted by the allowlist rule\n", pfFirewallTable) + rules.WriteString("# below. Auto-detected sources are scoped to their ingress interface; this is an\n") + rules.WriteString("# explicit, per-subnet trust boundary - NOT an interface-wide permit.\n\n") + + for _, src := range sources { + cidr := src.prefix.String() + on := "" + label := "configured (source-CIDR scope)" + if src.iface != "" { + on = "on " + src.iface + " " + label = "auto-detected on " + src.iface + } + fmt.Fprintf(&rules, "# %s - %s\n", cidr, label) + // Force guest plaintext DNS (port 53) onto loopback, where the existing + // rdr-on-lo0 rule redirects it to ctrld. Matched inbound (pre-NAT); "quick" + // so it wins over the blanket block that buildPFFirewallRules appends after. + fmt.Fprintf(&rules, "pass in quick %sroute-to lo0 inet proto udp from %s to ! %s port 53\n", on, cidr, listenerIP) + fmt.Fprintf(&rules, "pass in quick %sroute-to lo0 inet proto tcp from %s to ! %s port 53\n", on, cidr, listenerIP) + // Block DoT so the guest cannot escape ctrld via an alternate encrypted + // resolver. DoH over 443 is indistinguishable from HTTPS - documented limitation. + // inet only: the source literal is IPv4, and pf refuses to load an anchor + // containing an inet6 rule with an IPv4 source. + fmt.Fprintf(&rules, "block return in quick %sinet proto { tcp, udp } from %s to any port 853\n\n", on, cidr) + } + + return rules.String() +} + +// ipv4ForwardedSources returns the IPv4 subset of sources. Forwarded-workload DNS +// interception is IPv4-only (the intercept listener is IPv4), and every emitted rule +// must match its source's address family or pfctl rejects the entire anchor. +func ipv4ForwardedSources(sources []forwardedSource) []forwardedSource { + out := make([]forwardedSource, 0, len(sources)) + for _, src := range sources { + if src.prefix.Addr().Is4() { + out = append(out, src) + } + } + return out +} + +// reconcileForwardedSources rebuilds the pf anchor when the effective forwarded- +// source set (auto-detected VM networks + configured) has changed since the last +// build, and drops the pf states of every subnet whose trust changed. +// +// VM/container interfaces appear and disappear at runtime (guest start/stop) and no +// existing path rebuilds an otherwise-intact anchor for that: ensurePFAnchorActive() +// returns early while the rules still exist, checkTunnelInterfaceChanges() tracks +// only tunnel interfaces, and pfInterceptMonitor() rebuilds only after the host +// interception probe fails. Without this, a guest started after ctrld would never be +// trusted, and a stopped guest's subnet would stay trusted until an unrelated +// rebuild. Invoked from the network-change paths and, as a time bound when no event +// fires, from the pf watchdog tick. +// +// A reload failure is not latched: the applied snapshot only advances once pf has +// actually accepted the new anchor, so the next reconcile (at the latest the next +// watchdog tick) retries the same change instead of treating it as done. +func (p *prog) reconcileForwardedSources() { + if !p.firewallModeEnabled() || p.dnsInterceptState == nil { + return + } + state, ok := p.platformFirewallState.(*pfFirewallState) + if !ok || state == nil { + return + } + sources := p.forwardedSources() + reload := func() error { return p.reloadForwardedSourceAnchor(sources) } + gained, lost, changed, err := state.applyForwardedSourceChange(sources, reload) + if !changed { + return + } + if err != nil { + p.Warn().Err(err).Strs("gained_trust", prefixStrings(gained)).Strs("lost_trust", prefixStrings(lost)). + Msg("Firewall: forwarded-source set changed but the pf anchor reload failed — keeping the previously applied set so the next reconcile retries") + return + } + p.Info().Strs("gained_trust", prefixStrings(gained)).Strs("lost_trust", prefixStrings(lost)). + Msg("Firewall: forwarded-source set changed (VM/container start/stop), pf anchor rebuilt") + // Restate the whole effective set, so one log line always answers "what is trusted + // right now" without replaying every earlier transition. + p.logForwardedSources(sources) + // Rules only govern new states, so kill the states of the affected subnets: + // a subnet that lost trust must stop using states created while it was trusted, + // and one that just gained it must have its pre-existing (un-intercepted) DNS + // states re-evaluated instead of running until they expire. The guest simply + // re-establishes the connections under the new rules. Only reached after a + // successful load - killing states against the old anchor would just have them + // recreated under the very rules the change was meant to replace. + p.killForwardedSourceStates(append(gained, lost...)) +} + +// applyForwardedSourceChange performs one reconcile step: compare cur against the +// last applied forwarded-source set and, when it differs, install it via reload. +// It reports which subnets gained and lost trust (trust identity is (subnet, +// interface scope), so a subnet that stays but changes scope appears in both lists), +// whether there was anything to do at all, and reload's error. +// +// The applied snapshot advances ONLY after reload returns nil. A failed write or +// pfctl load therefore leaves the previous set recorded, so the running anchor and +// the snapshot cannot diverge and the change is retried on the next reconcile rather +// than silently dropped. The whole check-reload-record sequence is serialized so +// concurrent callers (watchdog tick vs. network change) cannot both rebuild or +// interleave their snapshot updates. +// +// Split out from reconcileForwardedSources so the guest start/stop lifecycle and the +// failure-then-retry path are deterministically testable without pf or a hypervisor. +func (s *pfFirewallState) applyForwardedSourceChange(cur []forwardedSource, reload func() error) (gained, lost []netip.Prefix, changed bool, err error) { + s.applyForwardedMu.Lock() + defer s.applyForwardedMu.Unlock() + + key := forwardedSourceSetKey(cur) + s.mu.Lock() + applied, appliedKey := s.lastForwardedSources, s.lastForwardedKey + s.mu.Unlock() + + if key == appliedKey { + return nil, nil, false, nil + } + gained = forwardedSubnetsNotIn(cur, applied) + lost = forwardedSubnetsNotIn(applied, cur) + + if err := reload(); err != nil { + return gained, lost, true, err + } + + s.mu.Lock() + s.lastForwardedSources = cur + s.lastForwardedKey = key + s.mu.Unlock() + return gained, lost, true, nil +} + +// forwardedSubnetsNotIn returns the subnets of a whose exact trust entry (subnet + +// interface scope) is absent from b, de-duplicated. +func forwardedSubnetsNotIn(a, b []forwardedSource) []netip.Prefix { + inB := make(map[forwardedSource]struct{}, len(b)) + for _, src := range b { + inB[src] = struct{}{} + } + var out []netip.Prefix + seen := make(map[netip.Prefix]struct{}, len(a)) + for _, src := range a { + if _, ok := inB[src]; ok { + continue + } + if _, dup := seen[src.prefix]; dup { + continue + } + seen[src.prefix] = struct{}{} + out = append(out, src.prefix) + } + return out +} + +// prefixStrings renders prefixes for logging. +func prefixStrings(prefixes []netip.Prefix) []string { + out := make([]string, 0, len(prefixes)) + for _, pfx := range prefixes { + out = append(out, pfx.String()) + } + return out +} + +// forwardedSourceDescriptions renders the effective trust set for logging, naming each +// subnet's origin so an admin can tell an auto-detected VM network (and the interface +// its rules are scoped to) from an entry they configured. +func forwardedSourceDescriptions(sources []forwardedSource) []string { + out := make([]string, 0, len(sources)) + for _, src := range sources { + if src.iface != "" { + out = append(out, src.prefix.String()+" (auto-detected on "+src.iface+")") + continue + } + out = append(out, src.prefix.String()+" (configured)") + } + return out +} + +// logForwardedSources reports the effective forwarded-workload trust set. Configured +// entries were previously invisible in the log - only auto-detection said anything - +// so an admin had no way to confirm firewall_forwarded_sources took effect. The empty +// case is logged too, with what to do about it, because "no guest DNS interception" +// looks identical to "feature silently did nothing". +func (p *prog) logForwardedSources(sources []forwardedSource) { + if len(sources) == 0 { + p.Info().Msg("Firewall: no forwarded-workload (VM/container) sources — guest DNS is not intercepted. " + + "Auto-detection needs an up interface with an RFC1918 IPv4 address that is either vendor-named " + + "(vnic*, vboxnet*, vmnet*) or a bridge with a VM member (vmenet*); anything else must be listed " + + "in service.firewall_forwarded_sources") + return + } + p.Info().Int("count", len(sources)).Strs("sources", forwardedSourceDescriptions(sources)). + Msg("Firewall: forwarded-workload (VM/container) DNS interception active for these source subnets") +} + +// killForwardedSourceStates drops the pf state entries sourced from the given +// subnets. Targeted (pfctl -k ) rather than a global state flush, so a +// guest starting or stopping never resets unrelated host connections. +func (p *prog) killForwardedSourceStates(prefixes []netip.Prefix) { + for _, pfx := range prefixes { + out, err := exec.Command("pfctl", "-k", pfx.String()).CombinedOutput() + if err != nil { + // Not a hard error - most often there simply are no matching states. + p.Debug().Err(err).Str("subnet", pfx.String()).Str("output", strings.TrimSpace(string(out))). + Msg("Firewall: could not kill pf states for changed forwarded source") + continue + } + p.Info().Str("subnet", pfx.String()). + Msg("Firewall: killed pf states for changed forwarded source") + } +} + +// recordAppliedForwardedSources records sources as the set pf is now enforcing. +// +// Called by the rebuild paths that are not the forwarded-source reconcile itself +// (tunnel change, watchdog restore, VPN DNS exemptions, forced reload, startup): each +// of those installs a full anchor that already contains the current set, so without +// this the next reconcile would compare against a stale snapshot and redo the work. +// +// Takes applyForwardedMu so a record cannot land in the middle of a reconcile's +// compare-reload-record sequence and be overwritten by it, or overwrite it. +func (p *prog) recordAppliedForwardedSources(sources []forwardedSource) { + state, ok := p.platformFirewallState.(*pfFirewallState) + if !ok || state == nil { + return + } + state.applyForwardedMu.Lock() + defer state.applyForwardedMu.Unlock() + + state.mu.Lock() + state.lastForwardedSources = sources + state.lastForwardedKey = forwardedSourceSetKey(sources) + state.mu.Unlock() +} + +// reloadForwardedSourceAnchor rebuilds and reloads the ctrld pf anchor so the given +// forwarded-source rules take effect. Mirrors the anchor reload used by the intercept +// watchdog, but reports failure to the caller: whether pf actually accepted the anchor +// decides whether the new source set may be recorded as applied. +// +// The set is passed in rather than re-detected, so what gets loaded is exactly what +// the caller compared and will record. Re-detecting here could install a set that +// differs from the recorded snapshot if an interface appeared in between. +func (p *prog) reloadForwardedSourceAnchor(sources []forwardedSource) error { + var vpnExemptions []vpnDNSExemption + if p.vpnDNS != nil { + vpnExemptions = p.vpnDNS.CurrentExemptions() + } + rulesStr := p.buildPFAnchorRulesWith(vpnExemptions, sources) + if err := writePFAnchorFile(rulesStr); err != nil { + return fmt.Errorf("write pf anchor %s: %w", pfAnchorFile, err) + } + if out, err := exec.Command("pfctl", "-a", pfAnchorName, "-f", pfAnchorFile).CombinedOutput(); err != nil { + return fmt.Errorf("load pf anchor %s: %w (output: %s)", pfAnchorName, err, strings.TrimSpace(string(out))) + } + return nil +} diff --git a/cmd/cli/firewall_darwin_bridge_test.go b/cmd/cli/firewall_darwin_bridge_test.go new file mode 100644 index 0000000..3c78436 --- /dev/null +++ b/cmd/cli/firewall_darwin_bridge_test.go @@ -0,0 +1,416 @@ +//go:build darwin + +package cli + +import ( + "net" + "net/netip" + "testing" + + "github.com/Control-D-Inc/ctrld/internal/firewall" +) + +// vmnetBridgeIfconfig is ifconfig output for a vmnet.framework bridge: the RFC1918 +// gateway lives here, and the vendor-named vmenet0 is an address-less member. This is +// the shape that name-only detection could never see. +const vmnetBridgeIfconfig = `bridge100: flags=8a63 mtu 1500 + options=3 + ether 5e:cf:7f:9a:1b:64 + inet 192.168.64.1 netmask 0xffffff00 broadcast 192.168.64.255 + Configuration: + id 0:0:0:0:0:0 priority 0 hellotime 0 fwddelay 0 + maxage 0 holdcnt 0 proto stp maxaddr 100 timeout 1200 + root id 0:0:0:0:0:0 priority 0 ifcost 0 port 0 + ipfilter disabled flags 0x0 + member: vmenet0 flags=3 + ifmaxaddr 0 port 22 priority 0 path cost 0 + nd6 options=201 + media: + status: active +` + +// thunderboltBridgeIfconfig is ifconfig output for the Thunderbolt bridge macOS +// creates by default. It can carry an RFC1918 address, and its members are physical +// interfaces - trusting it would force-route unrelated same-subnet traffic. +const thunderboltBridgeIfconfig = `bridge0: flags=8863 mtu 1500 + options=63 + ether 36:12:8a:1f:2b:00 + inet 192.168.10.5 netmask 0xffffff00 broadcast 192.168.10.255 + Configuration: + id 0:0:0:0:0:0 priority 0 hellotime 0 fwddelay 0 + member: en1 flags=3 + ifmaxaddr 0 port 9 priority 0 path cost 0 + member: en2 flags=3 + ifmaxaddr 0 port 10 priority 0 path cost 0 + nd6 options=201 + media: + status: inactive +` + +func TestParseBridgeMembers(t *testing.T) { + tests := []struct { + name string + out string + want []string + }{ + {"vmnet.framework bridge", vmnetBridgeIfconfig, []string{"vmenet0"}}, + {"thunderbolt bridge", thunderboltBridgeIfconfig, []string{"en1", "en2"}}, + {"no members", "bridge2: flags=8822 mtu 1500\n\tether 1a:2b:3c\n", nil}, + {"empty output", "", nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := parseBridgeMembers(tc.out) + if len(got) != len(tc.want) { + t.Fatalf("parseBridgeMembers() = %v, want %v", got, tc.want) + } + for i := range got { + if got[i] != tc.want[i] { + t.Errorf("member[%d] = %q, want %q", i, got[i], tc.want[i]) + } + } + }) + } +} + +func TestBridgeHasVMMember(t *testing.T) { + tests := []struct { + name string + members []string + want bool + }{ + {"vmnet.framework member", []string{"vmenet0"}, true}, + {"parallels member", []string{"vnic0"}, true}, + {"mixed with vm member", []string{"en1", "vmenet2"}, true}, + {"physical members only", []string{"en1", "en2"}, false}, + {"no members", nil, false}, + // A name that merely looks bridge-ish proves nothing. + {"bridge member", []string{"bridge1"}, false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := bridgeHasVMMember(tc.members); got != tc.want { + t.Errorf("bridgeHasVMMember(%v) = %v, want %v", tc.members, got, tc.want) + } + }) + } +} + +func mustAddrs(t *testing.T, cidrs ...string) []net.Addr { + t.Helper() + var out []net.Addr + for _, c := range cidrs { + ip, ipnet, err := net.ParseCIDR(c) + if err != nil { + t.Fatalf("bad test CIDR %q: %v", c, err) + } + out = append(out, &net.IPNet{IP: ip, Mask: ipnet.Mask}) + } + return out +} + +// TestForwardedSourcesForIface covers the trust decision, including every case that +// must NOT be auto-trusted. This is a security boundary: anything that qualifies here +// gets its guest traffic passed to allowed public destinations. +func TestForwardedSourcesForIface(t *testing.T) { + tests := []struct { + name string + iface string + addrs []string + members []string + wantCIDR []string + }{ + { + // The case name-only detection missed: address on the bridge, vendor + // interface bridged into it. This is UTM/Docker/Multipass/Fusion 12.1+. + name: "bridge with vmenet member is trusted", + iface: "bridge100", + addrs: []string{"192.168.64.1/24"}, + members: []string{"vmenet0"}, + wantCIDR: []string{"192.168.64.0/24"}, + }, + { + // The reason membership is required rather than the bridge name. + name: "thunderbolt bridge is not trusted", + iface: "bridge0", + addrs: []string{"192.168.10.5/24"}, + members: []string{"en1", "en2"}, + }, + { + name: "vendor-named interface with its own address is trusted", + iface: "vnic0", + addrs: []string{"10.211.55.2/24"}, + wantCIDR: []string{"10.211.55.0/24"}, + }, + { + // vmenet* under vmnet.framework: up, but no address of its own. + name: "address-less vendor interface yields nothing", + iface: "vmenet0", + addrs: nil, + }, + { + // The RFC1918 boundary: a VM network on a public range is never trusted. + name: "public range on a VM bridge is not trusted", + iface: "bridge100", + addrs: []string{"93.184.216.34/24"}, + members: []string{"vmenet0"}, + }, + { + // Interception is IPv4-only; an IPv6-only VM bridge must not qualify. + name: "ipv6 only is not trusted", + iface: "bridge100", + addrs: []string{"fd00::1/64"}, + members: []string{"vmenet0"}, + }, + { + name: "physical uplink is not trusted", + iface: "en0", + addrs: []string{"192.168.1.20/24"}, + }, + { + name: "vpn tunnel is not trusted", + iface: "utun4", + addrs: []string{"10.2.0.2/24"}, + }, + { + name: "mixed addresses keep only the private ipv4 one", + iface: "bridge101", + addrs: []string{"fd00::1/64", "192.168.105.1/24"}, + members: []string{"vmenet1"}, + wantCIDR: []string{"192.168.105.0/24"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + members := func(string) []string { return tc.members } + got, reason := forwardedSourcesForIface(tc.iface, mustAddrs(t, tc.addrs...), members) + if len(got) != len(tc.wantCIDR) { + t.Fatalf("got %d sources %v, want %d %v", len(got), got, len(tc.wantCIDR), tc.wantCIDR) + } + for i, want := range tc.wantCIDR { + if got[i].prefix != netip.MustParsePrefix(want) { + t.Errorf("prefix[%d] = %s, want %s", i, got[i].prefix, want) + } + // Rules must be scoped to the interface traffic actually arrives on. + if got[i].iface != tc.iface { + t.Errorf("source %s scoped to %q, want %q", got[i].prefix, got[i].iface, tc.iface) + } + } + if len(got) > 0 && reason == "" { + t.Error("a trusted source must report why it qualified") + } + }) + } +} + +// TestForwardedSourcesForIface_NoMemberLookupWithoutAddress verifies the ifconfig call +// is skipped for a bridge that cannot qualify anyway. Detection runs on every anchor +// build and every watchdog tick, so this keeps a typical host at zero subprocesses. +func TestForwardedSourcesForIface_NoMemberLookupWithoutAddress(t *testing.T) { + called := false + members := func(string) []string { + called = true + return []string{"vmenet0"} + } + + if got, _ := forwardedSourcesForIface("bridge100", nil, members); got != nil { + t.Errorf("address-less bridge must yield nothing, got %v", got) + } + if called { + t.Error("member list must not be queried for a bridge with no RFC1918 address") + } + + // A public-range bridge is equally hopeless, and equally must not exec. + called = false + if got, _ := forwardedSourcesForIface("bridge100", mustAddrs(t, "93.184.216.34/24"), members); got != nil { + t.Errorf("public-range bridge must yield nothing, got %v", got) + } + if called { + t.Error("member list must not be queried for a bridge with no RFC1918 address") + } +} + +// TestParseForwardedSourceConfig covers the parse/reject split: usable entries survive +// alongside bad ones, and each rejection carries a reason to report. +func TestParseForwardedSourceConfig(t *testing.T) { + sources, rejected := parseForwardedSourceConfig([]string{ + "192.168.64.7/24", // host bits get normalized + "not-a-cidr", // malformed + "fd00::/64", // not IPv4 + " 10.0.0.0/8 ", // surrounding space tolerated + }) + + wantSources := map[string]bool{"192.168.64.0/24": true, "10.0.0.0/8": true} + if len(sources) != len(wantSources) { + t.Fatalf("got %d usable sources %v, want %d", len(sources), sources, len(wantSources)) + } + for _, src := range sources { + if !wantSources[src.prefix.String()] { + t.Errorf("unexpected usable prefix %s", src.prefix) + } + if src.iface != "" { + t.Errorf("configured source %s must have no interface scope, got %q", src.prefix, src.iface) + } + } + + if len(rejected) != 2 { + t.Fatalf("got %d rejections %v, want 2", len(rejected), rejected) + } + for _, r := range rejected { + if r.value != "not-a-cidr" && r.value != "fd00::/64" { + t.Errorf("unexpected rejected value %q", r.value) + } + if r.reason == "" { + t.Errorf("rejection of %q carries no reason", r.value) + } + } +} + +func TestParseForwardedSourceConfig_Empty(t *testing.T) { + sources, rejected := parseForwardedSourceConfig(nil) + if sources != nil || rejected != nil { + t.Errorf("empty config must yield nothing, got %v / %v", sources, rejected) + } +} + +// TestRejectedForwardedSourcesKey verifies the signature ignores order, so re-parsing +// an unchanged config is recognised as nothing new, while a changed set is not. +func TestRejectedForwardedSourcesKey(t *testing.T) { + a := []rejectedForwardedSource{{value: "x", reason: "r1"}, {value: "y", reason: "r2"}} + b := []rejectedForwardedSource{{value: "y", reason: "r2"}, {value: "x", reason: "r1"}} + if rejectedForwardedSourcesKey(a) != rejectedForwardedSourcesKey(b) { + t.Error("key must be order-independent") + } + if rejectedForwardedSourcesKey(nil) != "" { + t.Error("no rejections must produce an empty key") + } + c := []rejectedForwardedSource{{value: "x", reason: "r1"}} + if rejectedForwardedSourcesKey(a) == rejectedForwardedSourcesKey(c) { + t.Error("different rejection sets must produce different keys") + } +} + +// resetForwardedSourceWarnTracker clears the process-wide warning dedupe so each test +// starts from "nothing reported yet". +func resetForwardedSourceWarnTracker(t *testing.T) { + t.Helper() + forwardedSourceWarnTracker.mu.Lock() + forwardedSourceWarnTracker.key = "" + forwardedSourceWarnTracker.mu.Unlock() +} + +func trackedRejectionKey() string { + forwardedSourceWarnTracker.mu.Lock() + defer forwardedSourceWarnTracker.mu.Unlock() + return forwardedSourceWarnTracker.key +} + +// TestWarnRejectedForwardedSources_OnlyOnChange verifies a standing bad entry is +// reported once rather than on every watchdog tick, and that a newly-introduced one is +// still reported after a config reload. +// +// The dedupe must not depend on pfFirewallState: no state is installed here, matching +// the window where Firewall Mode is on but pf enforcement is still deferred until +// intercept mode starts. +func TestWarnRejectedForwardedSources_OnlyOnChange(t *testing.T) { + resetForwardedSourceWarnTracker(t) + p := progWithForwardedSources("not-a-cidr", "192.168.64.0/24") + + // First parse reports; the signature is now recorded. + p.firewallForwardedSources() + first := trackedRejectionKey() + if first == "" { + t.Fatal("a rejected entry must be recorded as reported") + } + + // Re-parsing the same config (every anchor build, every 30s tick) must not change + // what is recorded - that is what stops the repeated warning. + for i := 0; i < 5; i++ { + p.firewallForwardedSources() + } + if got := trackedRejectionKey(); got != first { + t.Errorf("recorded rejection key changed on re-parse: %q -> %q", first, got) + } + + // A config reload that introduces a different bad entry must be reported. + p.cfg.Service.FirewallForwardedSources = []string{"also-not-a-cidr"} + p.firewallForwardedSources() + if trackedRejectionKey() == first { + t.Error("a newly-introduced bad entry must be reported, not suppressed") + } + + // Fixing the config clears the recorded set, so a later regression reports again. + p.cfg.Service.FirewallForwardedSources = []string{"192.168.64.0/24"} + p.firewallForwardedSources() + if got := trackedRejectionKey(); got != "" { + t.Errorf("a clean config must clear the recorded rejections, got %q", got) + } +} + +// TestCurrentForwardedSources_GatedOnFirewallMode verifies no detection or config +// parsing happens with Firewall Mode off. Anchor rebuilds run on tunnel changes, +// watchdog restores and VPN DNS updates regardless of firewall mode, so an ungated +// call would enumerate interfaces, exec ifconfig and re-report bad config entries on +// every one of them. +func TestCurrentForwardedSources_GatedOnFirewallMode(t *testing.T) { + resetForwardedSourceWarnTracker(t) + p := progWithForwardedSources("not-a-cidr", "192.168.64.0/24") + + // Firewall mode off: nothing detected, and the bad entry is not even looked at. + if got := p.currentForwardedSources(); got != nil { + t.Errorf("firewall mode off must yield no sources, got %v", got) + } + if got := trackedRejectionKey(); got != "" { + t.Errorf("config must not be parsed with firewall mode off, but a rejection was recorded: %q", got) + } + + // With firewall mode on (allowList present), the configured entry is honoured and + // the bad one reported. + p.allowList = firewall.New() + got := p.currentForwardedSources() + if len(got) != 1 || got[0].prefix != netip.MustParsePrefix("192.168.64.0/24") { + t.Errorf("firewall mode on must yield the configured source, got %v", got) + } + if trackedRejectionKey() == "" { + t.Error("the unusable entry must be reported once firewall mode is on") + } +} + +// TestRecordAppliedForwardedSources verifies a full-anchor rebuild can baseline the +// reconcile snapshot, so the next reconcile does not redo the same change. +func TestRecordAppliedForwardedSources(t *testing.T) { + p := progWithForwardedSources() + state := &pfFirewallState{} + p.platformFirewallState = state + + sources := []forwardedSource{{prefix: netip.MustParsePrefix("192.168.64.0/24"), iface: "bridge100"}} + p.recordAppliedForwardedSources(sources) + + if state.lastForwardedKey != forwardedSourceSetKey(sources) { + t.Errorf("snapshot key = %q, want %q", state.lastForwardedKey, forwardedSourceSetKey(sources)) + } + if len(state.lastForwardedSources) != 1 || state.lastForwardedSources[0] != sources[0] { + t.Errorf("snapshot sources = %v, want %v", state.lastForwardedSources, sources) + } + + // A reconcile against the same set must now find nothing to do: no second rebuild, + // no killed states, no transition logged for something already in effect. + reloads := 0 + _, _, changed, err := state.applyForwardedSourceChange(sources, func() error { reloads++; return nil }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if changed { + t.Error("reconcile treated an already-applied set as a change") + } + if reloads != 0 { + t.Errorf("anchor was rebuilt %d times for an unchanged set, want 0", reloads) + } +} + +// TestRecordAppliedForwardedSources_NoState verifies recording is a no-op when +// firewall mode is off, since the rebuild paths call it unconditionally. +func TestRecordAppliedForwardedSources_NoState(t *testing.T) { + p := progWithForwardedSources() + p.recordAppliedForwardedSources([]forwardedSource{{prefix: netip.MustParsePrefix("10.0.0.0/8")}}) +} diff --git a/cmd/cli/firewall_darwin_test.go b/cmd/cli/firewall_darwin_test.go new file mode 100644 index 0000000..d31289b --- /dev/null +++ b/cmd/cli/firewall_darwin_test.go @@ -0,0 +1,571 @@ +//go:build darwin + +package cli + +import ( + "errors" + "net/netip" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/Control-D-Inc/ctrld" + "github.com/Control-D-Inc/ctrld/internal/firewall" +) + +// progWithForwardedSources builds a prog whose config declares the given +// forwarded-workload source subnets. A logger is attached so the invalid-entry +// warning path is safe to exercise. +func progWithForwardedSources(sources ...string) *prog { + p := &prog{cfg: &ctrld.Config{ + Listener: map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 53}}, + }} + p.cfg.Service.FirewallForwardedSources = sources + p.logger.Store(mainLog.Load()) + return p +} + +// TestIsHypervisorVMNetIface verifies only vendor-specific VM/NAT interfaces +// qualify for auto-detection - never generic bridges (bridge*, which macOS also +// uses for Thunderbolt/aggregated links), physical uplinks, loopback, or VPN tunnels. +func TestIsHypervisorVMNetIface(t *testing.T) { + tests := []struct { + name string + want bool + }{ + {"vmnet8", true}, // VMware Fusion + {"vmenet0", true}, // Apple Virtualization.framework / UTM + {"vnic0", true}, // Parallels + {"vboxnet0", true}, // VirtualBox + {"bridge0", false}, // generic bridge (Thunderbolt/aggregated) - NOT auto-trusted + {"bridge100", false}, // Multipass/Docker generic bridge - opt-in only + {"en0", false}, // physical uplink + {"lo0", false}, // loopback + {"utun3", false}, // VPN tunnel + {"awdl0", false}, // Apple Wireless Direct Link + } + for _, tt := range tests { + if got := isHypervisorVMNetIface(tt.name); got != tt.want { + t.Errorf("isHypervisorVMNetIface(%q) = %v, want %v", tt.name, got, tt.want) + } + } +} + +// TestBuildPFForwardedSourceRulesFor_Basic verifies the rules emit, for each source: +// a route-to-lo0 redirect of the guest's plaintext DNS (udp+tcp) and a DoT block; +// that auto-detected sources are scoped "on " while configured sources match +// on CIDR alone; and crucially NOT an interface-wide or destination-wide permit that +// would let the guest bypass policy. +func TestBuildPFForwardedSourceRulesFor_Basic(t *testing.T) { + sources := []forwardedSource{ + {prefix: netip.MustParsePrefix("192.168.105.0/24"), iface: "vmnet8"}, // auto-detected + {prefix: netip.MustParsePrefix("10.211.55.0/24")}, // configured (no iface) + } + rules := buildPFForwardedSourceRulesFor(sources, "127.0.0.1") + + wants := []string{ + // Auto-detected: scoped to its ingress interface. + "pass in quick on vmnet8 route-to lo0 inet proto udp from 192.168.105.0/24 to ! 127.0.0.1 port 53", + "pass in quick on vmnet8 route-to lo0 inet proto tcp from 192.168.105.0/24 to ! 127.0.0.1 port 53", + "block return in quick on vmnet8 inet proto { tcp, udp } from 192.168.105.0/24 to any port 853", + // Configured: CIDR-only (admin opt-in), no "on ". + "pass in quick route-to lo0 inet proto udp from 10.211.55.0/24 to ! 127.0.0.1 port 53", + "block return in quick inet proto { tcp, udp } from 10.211.55.0/24 to any port 853", + } + for _, w := range wants { + if !strings.Contains(rules, w) { + t.Errorf("missing rule:\n %s\nin:\n%s", w, rules) + } + } + + // Address family must match the source literal. An "inet6 ... from " + // rule makes pfctl reject the whole anchor, which would take DNS interception + // down with it, so no inet6 rule may name an IPv4 source. + for _, line := range strings.Split(rules, "\n") { + if strings.Contains(line, "inet6") && (strings.Contains(line, "192.168.105.0/24") || strings.Contains(line, "10.211.55.0/24")) { + t.Errorf("inet6 rule with an IPv4 source - pf address-family mismatch:\n %s", line) + } + } + + // Security boundary: every "pass" for a source must be the port-53 redirect; + // no rule may grant a source an unrestricted destination. + for _, line := range strings.Split(rules, "\n") { + if strings.HasPrefix(line, "pass") && !strings.Contains(line, "port 53") { + t.Errorf("forwarded-source pass rule is not scoped to DNS - possible policy bypass:\n %s", line) + } + if strings.HasPrefix(line, "pass") && strings.HasSuffix(strings.TrimSpace(line), "to any") { + t.Errorf("forwarded-source rules must not contain a blanket 'to any' permit:\n %s", line) + } + } +} + +// TestBuildPFForwardedSourceRulesFor_Empty verifies that with no sources the +// builder returns "", leaving anchor behavior unchanged. +func TestBuildPFForwardedSourceRulesFor_Empty(t *testing.T) { + if got := buildPFForwardedSourceRulesFor(nil, "127.0.0.1"); got != "" { + t.Errorf("expected empty output with no sources, got:\n%s", got) + } +} + +// TestFirewallForwardedSources_InvalidDropped verifies a malformed CIDR and a +// non-IPv4 CIDR are dropped (with a warning) without voiding the valid config +// entries, that host bits are normalized to the network address, and that configured +// sources carry no interface. Rejecting IPv6 at parse time is what keeps the emitted +// rules single-family: a mixed-family rule makes pfctl reject the whole anchor. +func TestFirewallForwardedSources_InvalidDropped(t *testing.T) { + p := progWithForwardedSources("192.168.64.7/24", "not-a-cidr", "fd00::/64", "10.0.0.0/8") + got := p.firewallForwardedSources() + + want := map[string]bool{"192.168.64.0/24": true, "10.0.0.0/8": true} + if len(got) != len(want) { + t.Fatalf("got %d sources, want %d: %v", len(got), len(want), got) + } + for _, src := range got { + if !want[src.prefix.String()] { + t.Errorf("unexpected prefix %s (invalid entries should be dropped)", src.prefix) + } + if src.iface != "" { + t.Errorf("configured source %s must have no interface, got %q", src.prefix, src.iface) + } + if !src.prefix.Addr().Is4() { + t.Errorf("non-IPv4 source %s must be dropped (interception is IPv4-only)", src.prefix) + } + } +} + +// TestBuildPFForwardedSourceRulesFor_SkipsNonIPv4 verifies an IPv6 source produces no +// rules even if one reaches the builder, so a stray entry can never introduce a +// mixed-family rule that pfctl would reject the whole anchor over. +func TestBuildPFForwardedSourceRulesFor_SkipsNonIPv4(t *testing.T) { + v6 := forwardedSource{prefix: netip.MustParsePrefix("fd00::/64")} + if got := buildPFForwardedSourceRulesFor([]forwardedSource{v6}, "127.0.0.1"); got != "" { + t.Errorf("IPv6-only source must produce no rules, got:\n%s", got) + } + + v4 := forwardedSource{prefix: netip.MustParsePrefix("192.168.64.0/24"), iface: "vmnet8"} + rules := buildPFForwardedSourceRulesFor([]forwardedSource{v6, v4}, "127.0.0.1") + if strings.Contains(rules, "fd00::") { + t.Errorf("IPv6 source must be skipped in a mixed set:\n%s", rules) + } + if !strings.Contains(rules, "from 192.168.64.0/24 to ! 127.0.0.1 port 53") { + t.Errorf("IPv4 source must still produce its rules:\n%s", rules) + } + for _, line := range strings.Split(rules, "\n") { + if strings.HasPrefix(line, "pass") || strings.HasPrefix(line, "block") { + if strings.Contains(line, "inet6") { + t.Errorf("no inet6 rule may be emitted for IPv4-only sources:\n %s", line) + } + } + } +} + +// TestPFForwardedSourceRules_Syntax runs the real pf parser over the generated rules. +// This is the check string assertions cannot make: pfctl rejects an ENTIRE ruleset +// over one malformed or mixed-address-family rule, so a bad forwarded-source rule +// would take DNS interception down with it rather than just failing to trust a guest. +// +// Two rulesets are parsed: the forwarded-source rules alone (self-contained, so this +// arm is environment-independent) and the full anchor ctrld would load. +func TestPFForwardedSourceRules_Syntax(t *testing.T) { + // lo0 as the auto-detected ingress interface: any interface name parses, and lo0 + // is the one guaranteed to exist on every runner. + sources := []forwardedSource{ + {prefix: netip.MustParsePrefix("192.168.105.0/24"), iface: "lo0"}, // auto-detected, scoped + {prefix: netip.MustParsePrefix("10.211.55.0/24")}, // configured, CIDR-only + } + + t.Run("forwarded rules alone", func(t *testing.T) { + rules := buildPFForwardedSourceRulesFor(sources, "127.0.0.1") + if rules == "" { + t.Fatal("no forwarded-source rules generated") + } + pfctlParseCheck(t, rules) + }) + + t.Run("full anchor", func(t *testing.T) { + p := progWithForwardedSources("192.168.64.0/24", "10.211.55.0/24") + p.allowList = firewall.New() + rules := p.buildPFAnchorRules(nil) + if !strings.Contains(rules, "from 192.168.64.0/24 to ! ") { + t.Fatalf("forwarded-source rules missing from anchor under test:\n%s", rules) + } + pfctlParseCheck(t, stripPFGroupRules(rules)) + }) +} + +// pfctlParseCheck validates a ruleset with the real pf parser in ctrld's anchor +// context, failing the test on any parse error. pfctl needs /dev/pf, so the check +// skips (rather than fails) where the runner cannot open it. +func pfctlParseCheck(t *testing.T, ruleset string) { + t.Helper() + + pfctl, err := exec.LookPath("pfctl") + if err != nil { + t.Skip("pfctl not available:", err) + } + file := filepath.Join(t.TempDir(), "ctrld-rules-test.conf") + if err := os.WriteFile(file, []byte(ruleset), 0600); err != nil { + t.Fatalf("write ruleset under test: %v", err) + } + + // -n parses and validates without loading anything. + out, err := exec.Command(pfctl, "-a", pfAnchorName, "-n", "-f", file).CombinedOutput() + if err == nil { + return + } + msg := strings.TrimSpace(string(out)) + if strings.Contains(msg, "Permission denied") || strings.Contains(msg, "Operation not permitted") || + strings.Contains(msg, "/dev/pf") { + t.Skipf("pfctl cannot open /dev/pf on this runner (%v): %s", err, msg) + } + t.Errorf("pfctl rejected the generated ruleset (%v):\n%s\n--- ruleset ---\n%s", err, msg, ruleset) +} + +// stripPFGroupRules drops rules scoped to ctrld's runtime group. That group is created +// by the installed service (dscl), so on a dev box or CI runner pfctl reports "unknown +// group _ctrld" for them - an environment fact, not a defect in the generated rules. +// Only those lines are removed, so pf's ordering requirement (translation rules before +// filtering rules) still holds for what remains. +func stripPFGroupRules(ruleset string) string { + lines := strings.Split(ruleset, "\n") + kept := make([]string, 0, len(lines)) + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if !strings.HasPrefix(trimmed, "#") && strings.Contains(trimmed, "group "+pfGroupName) { + continue + } + kept = append(kept, line) + } + return strings.Join(kept, "\n") +} + +// TestForwardedSources_UnionDedup verifies the effective set unions auto-detected +// and configured subnets, de-duplicated by prefix. Auto-detection is +// environment-dependent, so this asserts config entries are always included and +// that duplicate config entries collapse to one - deterministic regardless of host. +func TestForwardedSources_UnionDedup(t *testing.T) { + p := progWithForwardedSources("192.168.199.0/24", "192.168.199.0/24") + got := p.forwardedSources() + + count := 0 + for _, src := range got { + if src.prefix.String() == "192.168.199.0/24" { + count++ + } + } + if count != 1 { + t.Errorf("configured subnet appears %d times, want exactly 1 (union must dedup):\n%v", count, got) + } +} + +// TestForwardedSourceDescriptions verifies the log rendering names each subnet's +// origin, including the interface an auto-detected source is scoped to. Configured +// entries used to be invisible in the log, which left admins unable to confirm +// firewall_forwarded_sources took effect. +func TestForwardedSourceDescriptions(t *testing.T) { + got := forwardedSourceDescriptions([]forwardedSource{ + {prefix: netip.MustParsePrefix("192.168.105.0/24"), iface: "vmenet0"}, + {prefix: netip.MustParsePrefix("192.168.252.0/24")}, + }) + want := []string{ + "192.168.105.0/24 (auto-detected on vmenet0)", + "192.168.252.0/24 (configured)", + } + if !equalStringSets(got, want) { + t.Errorf("descriptions = %v, want %v", got, want) + } + + if got := forwardedSourceDescriptions(nil); len(got) != 0 { + t.Errorf("empty set must render no descriptions, got %v", got) + } +} + +// TestForwardedSourceSetKey verifies the signature is order-independent, distinguishes +// interface scope, and changes when the set changes - the basis for detecting VM +// start/stop at runtime. +func TestForwardedSourceSetKey(t *testing.T) { + a := netip.MustParsePrefix("192.168.64.0/24") + b := netip.MustParsePrefix("10.211.55.0/24") + + // Order-independent. + k1 := forwardedSourceSetKey([]forwardedSource{{prefix: a, iface: "vmnet8"}, {prefix: b}}) + k2 := forwardedSourceSetKey([]forwardedSource{{prefix: b}, {prefix: a, iface: "vmnet8"}}) + if k1 != k2 { + t.Errorf("key must be order-independent: %q vs %q", k1, k2) + } + + // A guest appearing changes the key (empty -> one source). + if forwardedSourceSetKey(nil) == k1 { + t.Error("adding a source must change the key") + } + + // A guest stopping changes the key (two sources -> one). + k3 := forwardedSourceSetKey([]forwardedSource{{prefix: b}}) + if k3 == k1 { + t.Error("removing a source must change the key") + } + + // Same prefix on a different interface is a distinct trust and must differ. + kIface := forwardedSourceSetKey([]forwardedSource{{prefix: a, iface: "vmnet8"}}) + kNoIface := forwardedSourceSetKey([]forwardedSource{{prefix: a}}) + if kIface == kNoIface { + t.Error("interface scope must affect the key") + } +} + +// TestApplyForwardedSourceChange_Lifecycle walks the guest start/stop lifecycle +// deterministically (no pf, no hypervisor): initial build, guest start, no-change +// re-check, second guest start, guest stop, scope change, and back to none. Each +// anchor reload succeeds here. It asserts both halves of the contract - whether the +// anchor needs rebuilding, and which subnets' pf states must be dropped because +// their trust changed. +func TestApplyForwardedSourceChange_Lifecycle(t *testing.T) { + vmA := forwardedSource{prefix: netip.MustParsePrefix("192.168.105.0/24"), iface: "vmnet8"} + vmB := forwardedSource{prefix: netip.MustParsePrefix("10.211.55.0/24"), iface: "vnic0"} + cfgB := forwardedSource{prefix: vmB.prefix} // same subnet, configured (no iface scope) + + steps := []struct { + name string + cur []forwardedSource + wantChanged bool + wantGained []string + wantLost []string + }{ + {name: "initial state, no guests", cur: nil, wantChanged: false}, + { + name: "first guest starts", cur: []forwardedSource{vmA}, + wantChanged: true, wantGained: []string{"192.168.105.0/24"}, + }, + {name: "network change, nothing moved", cur: []forwardedSource{vmA}, wantChanged: false}, + { + // Reordered plus a new guest: order must not register as a change. + name: "second guest starts", cur: []forwardedSource{vmB, vmA}, + wantChanged: true, wantGained: []string{"10.211.55.0/24"}, + }, + { + name: "first guest stops", cur: []forwardedSource{vmB}, + wantChanged: true, wantLost: []string{"192.168.105.0/24"}, + }, + { + // Same subnet, different scope: a distinct trust, so its states must be + // dropped even though the subnet itself neither appeared nor vanished. + name: "guest subnet loses its interface scope", cur: []forwardedSource{cfgB}, + wantChanged: true, + wantGained: []string{"10.211.55.0/24"}, + wantLost: []string{"10.211.55.0/24"}, + }, + { + name: "last guest stops", cur: nil, + wantChanged: true, wantLost: []string{"10.211.55.0/24"}, + }, + {name: "still no guests", cur: nil, wantChanged: false}, + } + + state := &pfFirewallState{} + reloads := 0 + okReload := func() error { reloads++; return nil } + wantReloads := 0 + for _, step := range steps { + gained, lost, changed, err := state.applyForwardedSourceChange(step.cur, okReload) + if err != nil { + t.Fatalf("%s: unexpected reload error: %v", step.name, err) + } + if changed != step.wantChanged { + t.Errorf("%s: changed = %v, want %v", step.name, changed, step.wantChanged) + } + if got := prefixStrings(gained); !equalStringSets(got, step.wantGained) { + t.Errorf("%s: gained trust = %v, want %v", step.name, got, step.wantGained) + } + if got := prefixStrings(lost); !equalStringSets(got, step.wantLost) { + t.Errorf("%s: lost trust = %v, want %v", step.name, got, step.wantLost) + } + // The anchor must be rebuilt exactly on the transitions, never on a re-check. + if step.wantChanged { + wantReloads++ + } + if reloads != wantReloads { + t.Errorf("%s: anchor reloads = %d, want %d", step.name, reloads, wantReloads) + } + } +} + +// TestApplyForwardedSourceChange_FailureThenRetry is the convergence guarantee: a +// failed anchor write/load must NOT advance the applied snapshot, so the very next +// reconcile (at the latest the next watchdog tick) retries the same transition +// instead of seeing the new key and going quiet with the old anchor still installed. +func TestApplyForwardedSourceChange_FailureThenRetry(t *testing.T) { + guest := netip.MustParsePrefix("192.168.105.0/24") + cur := []forwardedSource{{prefix: guest, iface: "vmnet8"}} + + state := &pfFirewallState{} + loadErr := errors.New("pfctl: syntax error") + attempts := 0 + failing := func() error { attempts++; return loadErr } + succeeding := func() error { attempts++; return nil } + + // Attempt 1: guest starts, reload fails. The caller is told what changed (so it + // can log it) but must not treat it as applied. + gained, lost, changed, err := state.applyForwardedSourceChange(cur, failing) + if !changed || !errors.Is(err, loadErr) { + t.Fatalf("failed reload: changed = %v, err = %v, want true / the load error", changed, err) + } + if got := prefixStrings(gained); !equalStringSets(got, []string{guest.String()}) { + t.Errorf("failed reload: gained trust = %v, want %v", got, []string{guest.String()}) + } + if len(lost) != 0 { + t.Errorf("failed reload: lost trust = %v, want none", prefixStrings(lost)) + } + if state.lastForwardedKey != "" || state.lastForwardedSources != nil { + t.Fatalf("failed reload must not advance the applied snapshot, got key %q sources %v", + state.lastForwardedKey, state.lastForwardedSources) + } + + // Attempt 2: nothing else moved, but the change is still pending - it must be + // retried and reported identically, not swallowed. + gained, _, changed, err = state.applyForwardedSourceChange(cur, failing) + if !changed || err == nil { + t.Fatalf("retry after failure: changed = %v, err = %v, want true / an error", changed, err) + } + if got := prefixStrings(gained); !equalStringSets(got, []string{guest.String()}) { + t.Errorf("retry after failure: gained trust = %v, want %v", got, []string{guest.String()}) + } + + // Attempt 3: pf accepts the anchor - now the snapshot advances and the affected + // subnet's states are reported for killing. + gained, _, changed, err = state.applyForwardedSourceChange(cur, succeeding) + if !changed || err != nil { + t.Fatalf("successful reload: changed = %v, err = %v, want true / nil", changed, err) + } + if got := prefixStrings(gained); !equalStringSets(got, []string{guest.String()}) { + t.Errorf("successful reload: gained trust = %v, want %v", got, []string{guest.String()}) + } + if state.lastForwardedKey == "" { + t.Fatal("successful reload must record the applied source set") + } + + // Attempt 4: converged - no further rebuild, and no reload call at all. + before := attempts + if _, _, changed, err := state.applyForwardedSourceChange(cur, succeeding); changed || err != nil { + t.Errorf("after convergence: changed = %v, err = %v, want false / nil", changed, err) + } + if attempts != before { + t.Errorf("after convergence: reload was called %d extra time(s), want 0", attempts-before) + } + + // A failure while *removing* trust must likewise not be latched: the subnet stays + // recorded as applied until pf accepts the anchor without it. + if _, lost, changed, err := state.applyForwardedSourceChange(nil, failing); !changed || err == nil { + t.Errorf("guest stop with failing reload: changed = %v, err = %v, want true / an error", changed, err) + } else if got := prefixStrings(lost); !equalStringSets(got, []string{guest.String()}) { + t.Errorf("guest stop with failing reload: lost trust = %v, want %v", got, []string{guest.String()}) + } + if state.lastForwardedKey == "" { + t.Error("failed removal must keep the previously applied set recorded") + } + if _, lost, _, err := state.applyForwardedSourceChange(nil, succeeding); err != nil { + t.Errorf("guest stop retry: unexpected error %v", err) + } else if got := prefixStrings(lost); !equalStringSets(got, []string{guest.String()}) { + t.Errorf("guest stop retry: lost trust = %v, want %v", got, []string{guest.String()}) + } + if state.lastForwardedKey != "" || state.lastForwardedSources != nil { + t.Errorf("after successful removal the applied set must be empty, got key %q sources %v", + state.lastForwardedKey, state.lastForwardedSources) + } +} + +// TestReconcileForwardedSources_GatedOff verifies the reconcile entry point is inert +// when firewall mode is off or pf state was never initialized, so the network-change +// and watchdog call sites never touch pf outside firewall mode. +func TestReconcileForwardedSources_GatedOff(t *testing.T) { + // Firewall mode off (no allowList) - must return before touching pf state. + off := progWithForwardedSources("192.168.64.0/24") + off.platformFirewallState = &pfFirewallState{} + off.dnsInterceptState = &pfState{anchorFile: pfAnchorFile, anchorName: pfAnchorName} + off.reconcileForwardedSources() + if state := off.platformFirewallState.(*pfFirewallState); state.lastForwardedKey != "" { + t.Errorf("reconcile must not record a source set when firewall mode is off, got %q", state.lastForwardedKey) + } + + // Firewall mode on but pf firewall state not initialized - must not panic. + noState := progWithForwardedSources("192.168.64.0/24") + noState.allowList = firewall.New() + noState.dnsInterceptState = &pfState{anchorFile: pfAnchorFile, anchorName: pfAnchorName} + noState.reconcileForwardedSources() + + // Firewall mode on but intercept inactive - no anchor to rebuild. + noIntercept := progWithForwardedSources("192.168.64.0/24") + noIntercept.allowList = firewall.New() + noIntercept.platformFirewallState = &pfFirewallState{} + noIntercept.reconcileForwardedSources() + if state := noIntercept.platformFirewallState.(*pfFirewallState); state.lastForwardedKey != "" { + t.Errorf("reconcile must not record a source set without intercept, got %q", state.lastForwardedKey) + } +} + +// equalStringSets compares two string slices ignoring order and nil-vs-empty. +func equalStringSets(got, want []string) bool { + if len(got) != len(want) { + return false + } + seen := make(map[string]int, len(got)) + for _, s := range got { + seen[s]++ + } + for _, s := range want { + seen[s]-- + if seen[s] < 0 { + return false + } + } + return true +} + +// TestDetectForwardedSources_OnlyPrivate verifies detection returns only private +// IPv4 vendor-VM subnets, each tagged with a vendor interface. Environment-dependent, +// so it asserts a property rather than an exact set. +func TestDetectForwardedSources_OnlyPrivate(t *testing.T) { + p := &prog{cfg: &ctrld.Config{}} + p.logger.Store(mainLog.Load()) + for _, src := range p.detectForwardedSources() { + if !src.prefix.Addr().Is4() { + t.Errorf("detected non-IPv4 forwarded source: %s", src.prefix) + } + if !src.prefix.Addr().IsPrivate() { + t.Errorf("detected non-private forwarded source (must never auto-trust public): %s", src.prefix) + } + if !isHypervisorVMNetIface(src.iface) { + t.Errorf("detected source on non-vendor interface %q", src.iface) + } + } +} + +// TestPFBuildAnchorRules_ForwardedSourcesGating verifies the forwarded-source rules +// appear in the full anchor only when firewall mode is active, and when a configured +// source is present it appears before the blanket allowlist block so the redirect +// wins. A configured source makes the "on" case deterministic regardless of host. +func TestPFBuildAnchorRules_ForwardedSourcesGating(t *testing.T) { + // Firewall OFF (no allowList): a configured source must NOT appear. + off := progWithForwardedSources("192.168.64.0/24") + if rules := off.buildPFAnchorRules(nil); strings.Contains(rules, "192.168.64.0/24 to ! ") { + t.Errorf("forwarded-source rules must not be emitted when firewall mode is off:\n%s", rules) + } + + // Firewall ON: allowList present → rules appear, before the blanket block. + on := progWithForwardedSources("192.168.64.0/24") + on.allowList = firewall.New() + rules := on.buildPFAnchorRules(nil) + + fwdIdx := strings.Index(rules, "from 192.168.64.0/24 to ! 127.0.0.1 port 53") + blockIdx := strings.Index(rules, "block return out quick inet proto { tcp, udp } from any to any") + if fwdIdx < 0 { + t.Fatalf("configured forwarded-source redirect missing when firewall mode is on:\n%s", rules) + } + if blockIdx < 0 { + t.Fatalf("blanket firewall block missing:\n%s", rules) + } + if fwdIdx >= blockIdx { + t.Errorf("forwarded-source redirect (%d) must come before the blanket block (%d)", fwdIdx, blockIdx) + } +} diff --git a/config.go b/config.go index a6a5ac9..a44fdc9 100644 --- a/config.go +++ b/config.go @@ -255,8 +255,24 @@ type ServiceConfig struct { // Requires intercept mode to be active for enforcement on desktop platforms. // On mobile, the netstack layer uses the allowlist directly. FirewallMode string `mapstructure:"firewall_mode" toml:"firewall_mode,omitempty" validate:"omitempty,oneof=off on"` - Daemon bool `mapstructure:"-" toml:"-"` - AllocateIP bool `mapstructure:"-" toml:"-"` + // FirewallForwardedSources lists VM/container source subnets (CIDR) whose + // forwarded/NATed DNS is redirected through ctrld under Firewall Mode, so + // guest resolutions are policy-enforced and populate the allowlist, and guest + // egress to allowed destinations is permitted without an interface-wide + // bypass. These augment auto-detection of VM networks and are the supported way + // to trust a stack that auto-detection cannot prove ownership of. Empty (the + // default) preserves prior behavior. macOS only; see + // buildPFForwardedSourceRules. + // + // Deliberately not validated with `cidr`: entries are checked at use time and a + // bad one is dropped with a warning while the rest of the set still applies + // (see firewallForwardedSources). A hard validator here would make one typo in + // an MDM-pushed subnet fatal at startup - validateConfig exits the process - + // taking down DNS service for the whole host over a line that only ever + // widened a firewall allowance. + FirewallForwardedSources []string `mapstructure:"firewall_forwarded_sources" toml:"firewall_forwarded_sources,omitempty"` + Daemon bool `mapstructure:"-" toml:"-"` + AllocateIP bool `mapstructure:"-" toml:"-"` } // NetworkConfig specifies configuration for networks where ctrld will handle requests. diff --git a/config_forwarded_sources_test.go b/config_forwarded_sources_test.go new file mode 100644 index 0000000..436cc77 --- /dev/null +++ b/config_forwarded_sources_test.go @@ -0,0 +1,41 @@ +package ctrld_test + +import ( + "testing" + + "github.com/go-playground/validator/v10" + "github.com/stretchr/testify/require" + + "github.com/Control-D-Inc/ctrld" + "github.com/Control-D-Inc/ctrld/testhelper" +) + +// TestValidateConfig_FirewallForwardedSourcesLenient verifies a bad +// firewall_forwarded_sources entry does not fail config validation. +// +// This field is deliberately not validated with `cidr`: validateConfig failure exits +// the process, so a hard validator would turn one typo in an MDM-pushed subnet into a +// host-wide DNS outage. Bad entries are dropped with a warning at use time instead +// (see firewallForwardedSources on darwin), which is what the docs promise. +func TestValidateConfig_FirewallForwardedSourcesLenient(t *testing.T) { + tests := []struct { + name string + sources []string + }{ + {"malformed entry", []string{"not-a-cidr"}}, + {"missing prefix length", []string{"192.168.64.0"}}, + {"bad entry alongside good ones", []string{"192.168.64.0/24", "oops", "10.0.0.0/8"}}, + {"non-IPv4 entry", []string{"fd00::/64"}}, + {"empty string", []string{""}}, + {"valid entries", []string{"192.168.64.0/24"}}, + {"unset", nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := testhelper.SampleConfig(t) + cfg.Service.FirewallForwardedSources = tc.sources + require.NoError(t, ctrld.ValidateConfig(validator.New(), cfg), + "a bad forwarded-source entry must not stop ctrld from starting") + }) + } +} diff --git a/docs/firewall-mode.md b/docs/firewall-mode.md index e608243..500653e 100644 --- a/docs/firewall-mode.md +++ b/docs/firewall-mode.md @@ -1,7 +1,7 @@ # Firewall Mode Firewall mode makes DNS policy unbypassable by blocking outbound connections to any -IP that wasn't resolved by ctrld. This closes the "DNS gap" — where apps use hardcoded +IP that wasn't resolved by ctrld. This closes the "DNS gap" - where apps use hardcoded IPs, direct-IP fallbacks, or alternative DNS resolvers to bypass DNS-based filtering. ## How It Works @@ -73,14 +73,14 @@ These IPs are always allowed regardless of DNS resolution: | Range | Reason | |-------|--------| -| `127.0.0.0/8`, `::1` | Loopback — local services | -| `10.0.0.0/8` | RFC1918 — LAN, printers, NAS | -| `172.16.0.0/12` | RFC1918 — LAN | -| `192.168.0.0/16` | RFC1918 — LAN | -| `169.254.0.0/16`, `fe80::/10` | Link-local — DHCP, mDNS | -| `100.64.0.0/10` | CGNAT — Tailscale, carrier NAT | -| `224.0.0.0/4`, `ff00::/8` | Multicast — mDNS, SSDP | -| ctrld listener IPs | Self — DNS proxy must be reachable | +| `127.0.0.0/8`, `::1` | Loopback - local services | +| `10.0.0.0/8` | RFC1918 - LAN, printers, NAS | +| `172.16.0.0/12` | RFC1918 - LAN | +| `192.168.0.0/16` | RFC1918 - LAN | +| `169.254.0.0/16`, `fe80::/10` | Link-local - DHCP, mDNS | +| `100.64.0.0/10` | CGNAT - Tailscale, carrier NAT | +| `224.0.0.0/4`, `ff00::/8` | Multicast - mDNS, SSDP | +| ctrld listener IPs | Self - DNS proxy must be reachable | | Upstream resolver IPs | DoH/DoT/DoQ endpoints | ## Live Profile Updates @@ -92,7 +92,7 @@ When a ControlD profile changes (domain goes from allowed → blocked or vice ve 3. Subsequent DNS queries repopulate the allowlist under the new policy 4. Brief connectivity interruption (~seconds) while DNS cache repopulates -This is the "flush and repopulate" strategy — simple and correct, with a small +This is the "flush and repopulate" strategy - simple and correct, with a small tradeoff of a brief connectivity blip on config changes. ## Network State Changes @@ -134,6 +134,162 @@ remain blocked until the app performs DNS resolution again. This is an accepted v1 tradeoff and should be called out in release notes and compatibility testing for common apps. +## VM / Container Workloads (macOS) + +By default a VM or container resolves DNS through a path the host ctrld does not +observe (the hypervisor's own resolver on the guest bridge, or a resolver the +guest is configured to use). The guest-resolved public IP therefore never enters +``, and the guest's forwarded/NATed egress to that IP is dropped by +the blanket block - DNS "works" inside the guest but TCP/443 fails. (Tracked as +issue #569.) + +Exempting the whole bridge interface would turn the guest into a policy bypass, +so it is intentionally **not** done. Instead ctrld makes those guests first-class +Firewall Mode clients by forcing their DNS through itself. The trusted source +subnets are the **union** of: + +1. **Auto-detected VM networks (default, no config).** At pf-anchor build time an + interface is trusted only when it is up, carries an **RFC1918 IPv4** network, and + its VM ownership can be proven one of two ways: + + - **its own name is vendor-specific** - `vnic` (Parallels), `vboxnet` + (VirtualBox host-only), `vmnet` (legacy kext-based VMware Fusion on Intel); + - **it is a `bridge*` whose member list contains a vendor VM interface** + (typically `vmenet*`). This is the case for every `vmnet.framework` stack - + UTM and other Virtualization.framework guests, Docker Desktop, Multipass, and + Fusion 12.1+ NAT - where the RFC1918 gateway address sits on `bridge10x` and + the vendor-named `vmenet*` interface is an address-less member of it. Matching + on interface name alone never sees those stacks. + + Physical uplinks (`en*`), loopback, VPN tunnels (`utun*`), public ranges, and + IPv6 never qualify. Each auto-trusted subnet is logged at debug level + (`Firewall: auto-detected VM/container network for forwarded DNS`, with the + `reason` field naming the proof), and its pf rules are scoped to the interface it + was detected on (`on `), so an unrelated interface carrying the same + private range is never affected. + + A `bridge*` **name** is still not proof of anything: macOS uses that namespace + for Thunderbolt and aggregated links too (ctrld's own tunnel-change code treats + `bridge0` as physical). Membership is what distinguishes them - a Thunderbolt + bridge has `en*` members and is never trusted, however private its address. + +2. **Configured subnets (opt-in).** Needed for any stack whose ownership + auto-detection cannot prove - a VM network on a plain interface with no vendor + name, a bridge with no vendor member, or a deliberately non-RFC1918 range: + + ```toml + [service] + firewall_mode = "on" + intercept_mode = "hard" + # Only needed when auto-detection cannot prove the network is a VM network. + firewall_forwarded_sources = ["192.168.64.0/24"] + ``` + + Find the subnet with `ifconfig` on the host - for a `vmnet.framework` stack it is + the `bridge1xx` interface serving the VM. Use the network address in CIDR form; + host bits are normalized away. A configured entry matches on the source CIDR + alone - it is an admin opt-in, so it is not tied to one interface. Entries must be + **IPv4**; interception targets ctrld's IPv4 listener, so an IPv6 entry is ignored + with a warning. Config only **adds** to auto-detection; it never disables it. + + A malformed or non-IPv4 entry is dropped with a warning and the rest of the set + still applies - this field is deliberately **not** hard-validated at startup, so a + typo in an MDM-pushed subnet cannot stop ctrld from serving DNS. The warning is + logged when the set of bad entries changes, not on every internal rebuild, so a + standing typo does not fill the log. + +### Guest start/stop and network changes + +VM/container interfaces come and go while ctrld runs, and the pf watchdog does not +rebuild an anchor whose rules are still intact. ctrld therefore tracks the effective +forwarded-source set (auto-detected ∪ configured) and, whenever it changes, +rebuilds the anchor and drops the pf states of the affected subnets (targeted +`pfctl -k `, not a global state flush) so the new policy applies +immediately instead of when old states expire. A guest's in-flight connections are +re-established under the new rules. + +Reconciliation runs on interface appear/disappear, on network changes, on the +delayed post-change re-checks (a new VM network often gets its address slightly +after its interface appears), and on the pf watchdog tick - which bounds how long a +started guest can go untrusted, or a stopped guest stay trusted, to one watchdog +interval even if no network event fires. Each transition is logged with the subnets +that gained and lost trust. + +The set ctrld considers applied only advances once pf has actually accepted the new +anchor. If the write or `pfctl -f` fails, the previous set stays recorded, nothing is +flushed, a warning is logged, and the next reconciliation (at the latest the next +watchdog tick) retries the same transition - so a transient failure cannot leave the +old anchor installed while ctrld believes the change is done. + +### Supported behavior and trust boundary + +For each source subnet (auto-detected or configured), when Firewall Mode + +intercept are active, ctrld: + +- **Forces guest plaintext DNS (port 53) through ctrld** (pf `route-to lo0` onto + the existing loopback redirect). Every guest resolution is policy-enforced and + populates ``, so the guest's egress to allowed destinations is + then permitted by the same allowlist rule as the host. +- **Blocks guest IPv4 DoT (port 853)** so a guest cannot swap in an alternate + encrypted resolver to escape policy. Rules are emitted for the source's own + address family only (all sources are IPv4) - pf refuses to load an entire anchor + containing an `inet6` rule with an IPv4 source, which would take DNS interception + down with it. Guest traffic to an IPv6 DoT resolver is instead covered by the + blanket IPv6 outbound block, since such a resolver never enters ``. + +The boundary is explicit and per-subnet - a guest still **cannot** bypass Control +D policy via a direct public IP (never resolved through ctrld ⇒ never allowlisted) +or an alternate plaintext/DoT resolver. It is not an interface-wide permit. + +### Confirming what is trusted + +At startup (and on every change) ctrld logs the effective set, naming each subnet's +origin, so `firewall_forwarded_sources` can be verified without reading pf rules: + +``` +Firewall: forwarded-workload (VM/container) DNS interception active for these source subnets count=2 sources=["192.168.64.0/24 (auto-detected on bridge100)","192.168.252.0/24 (configured)"] +``` + +The interface named is where the address lives, which for a `vmnet.framework` stack +is the bridge (`bridge100`), not its `vmenet*` member. + +When the set is empty the log says so explicitly, rather than staying silent: + +``` +Firewall: no forwarded-workload (VM/container) sources — guest DNS is not intercepted. Auto-detection needs an up interface with an RFC1918 IPv4 address that is either vendor-named (vnic*, vboxnet*, vmnet*) or a bridge with a VM member (vmenet*); anything else must be listed in service.firewall_forwarded_sources +``` + +Note that `firewall_forwarded_sources` is a **local** config setting. If it is not in +`/etc/controld/ctrld.toml` on the device, ctrld has nothing to act on - check the file +itself, not only the dashboard. + +### Limitations + +- **A resolver running inside the guest is not supported** while Firewall Mode is on. + The design depends on the guest sending plaintext DNS (port 53) that host ctrld can + observe. A guest-side resolver (ctrld, systemd-resolved with DoT, dnscrypt, ...) + sends its upstream queries encrypted instead, so the host learns no addresses and + the guest's egress is blocked - and its DoT is blocked outright by the port-853 rule. + That is the trust boundary working as intended, not a regression: a guest that + resolves privately could otherwise reach any destination it liked. Point the guest + at the host bridge address (its default DHCP resolver) and let host ctrld enforce + policy for it. +- **DoH over 443** inside the guest is indistinguishable from ordinary HTTPS and + is not intercepted. To keep enforcement strict, disable DoH in the guest OS/ + browser, or restrict the guest to the host resolver. +- **IPv6 guest DNS** is not redirected (ctrld's intercept listener is IPv4); the + anchor's existing IPv6 DNS block forces guests to fall back to interceptable + IPv4 DNS. IPv6 forwarded sources are therefore unsupported: an IPv6 + `firewall_forwarded_sources` entry is ignored with a warning rather than emitted + as a rule. +- Auto-detection needs ownership proof: a vendor interface name, or a bridge with a + vendor VM member. A VM network on a plain unrecognized interface, or a bridge whose + hypervisor attaches no vendor-named member, needs an explicit + `firewall_forwarded_sources` entry. Bridge membership is read with `ifconfig`, and + only for a bridge that already carries an RFC1918 IPv4 address. +- macOS only. Windows (WFP) Firewall Mode VM behavior is tracked separately + (#568). + ## Metrics Allowlist stats are logged every 5 minutes: @@ -151,7 +307,7 @@ Firewall allowlist stats allowed_ips=142 permanent_ips=18 tracked_domains=89 tot - Check allowlist stats for hit/miss ratio ### Certain apps don't work -- The app may be using hardcoded IPs (this is the intended behavior — those IPs aren't DNS-resolved) +- The app may be using hardcoded IPs (this is the intended behavior - those IPs aren't DNS-resolved) - Check if the app uses a custom DNS resolver that bypasses ctrld - RFC1918 traffic is always allowed, so LAN-only apps should work