diff --git a/cmd/cli/firewall.go b/cmd/cli/firewall.go index 8dcb0e9..dc06bc0 100644 --- a/cmd/cli/firewall.go +++ b/cmd/cli/firewall.go @@ -11,6 +11,7 @@ import ( "github.com/kardianos/service" "github.com/miekg/dns" + "github.com/Control-D-Inc/ctrld/internal/controld" "github.com/Control-D-Inc/ctrld/internal/firewall" ) @@ -52,7 +53,7 @@ func (p *prog) setFirewallAllowList(al *firewall.AllowList) { // - CGNAT range (100.64.0.0/10) — used by Tailscale, carrier NAT // - ctrld listener IPs // - DoH/DoT/DoQ upstream resolver IPs -// - ControlD API endpoint IPs +// - ControlD API and upgrade download server IPs func (p *prog) initFirewallAllowList(ctx context.Context, al *firewall.AllowList) { // Loopback. al.AddPermanentPrefix(netip.MustParsePrefix("127.0.0.0/8")) @@ -84,6 +85,9 @@ func (p *prog) initFirewallAllowList(ctx context.Context, al *firewall.AllowList // Upstream resolver IPs — ctrld needs to reach its upstreams. p.addUpstreamIPsToPermanent(al) + // ControlD API and download IPs — ctrld needs to reach its own control plane. + p.addControlDEndpointIPsToPermanent(al) + // Platform-specific enforcement (pf on macOS, WFP on Windows) is initialized // from postRun() after startDNSIntercept() has prepared dnsInterceptState. @@ -130,6 +134,7 @@ func (p *prog) syncFirewallMode(ctx context.Context) { } } else { p.addUpstreamIPsToPermanent(al) + p.addControlDEndpointIPsToPermanent(al) } // Open this run's firewall generation before any work is scheduled against it, @@ -150,7 +155,14 @@ func (p *prog) syncFirewallMode(ctx context.Context) { // On reload, postRun() is not called, so initialize platform enforcement here // if intercept state already exists. Initial startup still defers to postRun() // because DNS intercept state is prepared there. - if p.dnsInterceptState != nil && p.platformFirewallState == nil { + // + // Called whether or not enforcement is already up, because the permanent adds + // above reach memory only: AddPermanent fires no change callback, so a reload + // that resolves a new API address would log it as permitted while the platform + // never hears about it. Each platform's re-entry is a refresh - Windows + // reinstalls the permanent filters it is missing, macOS returns early - so + // calling it when enforcement is already up costs nothing and closes that gap. + if p.dnsInterceptState != nil { p.initPlatformFirewall() } } @@ -555,6 +567,57 @@ func prefixStrings(prefixes []netip.Prefix) []string { return out } +// addControlDEndpointIPsToPermanent permits the ControlD endpoints ctrld dials on +// its own behalf. Called at startup and on config reload, like the upstream IPs. +// +// Firewall Mode permits what ctrld's listener resolved, and each of these has a +// hardcoded address it dials when DNS is unusable - which is exactly the state a +// ctrld blocked by its own filters is in. Nothing teaches the allowlist about +// those addresses, so the block-all filters deny ctrld's own sockets. See +// controld.APIEndpointIPs for the incident this comes from. +func (p *prog) addControlDEndpointIPsToPermanent(al *firewall.AllowList) { + // The API. Its transport resolves with ctrld.LookupIP, which queries the OS + // nameservers directly rather than through the listener, so neither what it + // resolves nor what it falls back to is ever learned - both are permitted here. + p.addPermanentIPs(al, "ControlD API", controld.APIEndpointIPs(cdDev)) + p.addPermanentResolvedIPs(al, "ControlD API", controld.APIDomain(cdDev)) + + // The upgrade download server. performUpgrade spawns a detached child process, + // which WFP's block-all filters deny exactly like this one: they carry no + // process condition. Its hostname lookup does go through the listener and is + // learned, so only the direct IP it falls back to needs permitting - and that + // fallback is the one an upgrade on a blocked host depends on. + p.addPermanentIPs(al, "ControlD download server", []string{downloadServerIp}) +} + +// addPermanentIPs permits literal addresses, ignoring any that do not parse. +func (p *prog) addPermanentIPs(al *firewall.AllowList, what string, ips []string) { + for _, ipStr := range ips { + if ip, err := netip.ParseAddr(ipStr); err == nil { + al.AddPermanent(ip) + p.Debug().Msgf("Firewall: added %s IP %s to permanent allowlist", what, ip) + } + } +} + +// addPermanentResolvedIPs permits whatever domain resolves to right now. +func (p *prog) addPermanentResolvedIPs(al *firewall.AllowList, what, domain string) { + ips, err := net.LookupHost(domain) + if err != nil { + // Neither fatal nor surprising during early startup, and not a Warn: the + // direct addresses are permitted regardless, and they are what the + // transport itself falls back to in this same situation. + p.Debug().Err(err).Msgf("Firewall: could not resolve %s for the permanent allowlist; its direct IPs are permitted", domain) + return + } + for _, ipStr := range ips { + if ip, err := netip.ParseAddr(ipStr); err == nil { + al.AddPermanent(ip) + p.Debug().Msgf("Firewall: added %s IP %s (%s) to permanent allowlist", what, ip, domain) + } + } +} + // extractHostFromEndpoint extracts the hostname or IP from a DoH/DoT/DoQ endpoint URL. // Handles formats like: // - "https://dns.controld.com/abcdef" diff --git a/cmd/cli/firewall_test.go b/cmd/cli/firewall_test.go index f11c87d..5b74b79 100644 --- a/cmd/cli/firewall_test.go +++ b/cmd/cli/firewall_test.go @@ -619,3 +619,50 @@ func TestAllowedDestinationLogsKeepAddressesOutOfWarnings(t *testing.T) { t.Errorf("the wide entry %q appears in no Debug line", wide) } } + +// TestFirewallPermanentAllowListPermitsControlDEndpoints is the regression guard +// for the Windows lockout described on controld.APIEndpointIPs. +// +// Firewall Mode learns destinations from queries ctrld's own listener answered. +// The addresses asserted here are the ones each endpoint falls back to when DNS +// does not work at all - which is the state a ctrld blocked by its own filters is +// in - so they are exactly the ones no lookup can ever teach it. +func TestFirewallPermanentAllowListPermitsControlDEndpoints(t *testing.T) { + for _, dev := range []bool{false, true} { + t.Run(map[bool]string{false: "prod", true: "dev"}[dev], func(t *testing.T) { + origDev := cdDev + cdDev = dev + t.Cleanup(func() { cdDev = origDev }) + + al := firewall.New() + p := &prog{cfg: &ctrld.Config{}} + p.logger.Store(mainLog.Load()) + p.initFirewallAllowList(context.Background(), al) + + apiIPs := controld.APIEndpointIPs(dev) + if len(apiIPs) == 0 { + t.Fatal("no ControlD API addresses to permit") + } + + endpoints := map[string][]string{ + // The API transport's direct addresses. + "API": apiIPs, + // The upgrade download server's fallback. performUpgrade runs the + // download in a detached child process, and WFP's block-all filters + // carry no process condition, so the service blocks its own upgrade. + "download server": {downloadServerIp}, + } + for what, ips := range endpoints { + for _, ipStr := range ips { + ip, err := netip.ParseAddr(ipStr) + if err != nil { + t.Fatalf("the %s address %q does not parse: %v", what, ipStr, err) + } + if !al.Contains(ip) { + t.Errorf("the ControlD %s address %s is not permitted; Firewall Mode would block ctrld's own socket to it", what, ip) + } + } + } + }) + } +} diff --git a/cmd/cli/firewall_windows.go b/cmd/cli/firewall_windows.go index fb64ec4..34abb27 100644 --- a/cmd/cli/firewall_windows.go +++ b/cmd/cli/firewall_windows.go @@ -87,14 +87,18 @@ func (p *prog) shutdownPlatformFirewall() { // initPlatformFirewall initializes Windows-specific firewall enforcement (WFP filters). func (p *prog) initPlatformFirewall() { if fwState, ok := p.platformFirewallState.(*wfpFirewallState); ok && fwState != nil { + // A reload re-enters here with enforcement already up, to install permits + // for permanent entries added since - a newly resolved API address, say - + // which AddPermanent records in memory without any callback that would + // reach WFP. Both populate calls skip what they already hold, so this is a + // refresh rather than a reinstall. fwState.populatePermanentFilters(p) - // Both callers gate on platformFirewallState being nil, so nothing reaches - // this today. Should something re-initialize enforcement over existing - // state, the filter IDs this state holds describe whatever engine session - // installed them, which is not necessarily the live one - so ask for a full - // replace instead of a delta against a snapshot that may describe filters - // that no longer exist. markDestinationsForResync is idempotent and cheap, - // and an unnecessary replace is a no-op the mirrors already tolerate. + // The filter IDs this state holds describe whatever engine session + // installed them, which after a rebuildDNSIntercept is not the live one - + // so ask for a full replace instead of a delta against a snapshot that may + // describe filters that no longer exist. markDestinationsForResync is + // idempotent and cheap, and an unnecessary replace is a no-op the mirrors + // already tolerate. p.markDestinationsForResync() p.reconcileAllowedDestinations() fwState.populateFilters(p) diff --git a/internal/controld/api_dial_test.go b/internal/controld/api_dial_test.go new file mode 100644 index 0000000..be24bab --- /dev/null +++ b/internal/controld/api_dial_test.go @@ -0,0 +1,178 @@ +package controld + +import ( + "errors" + "net" + "slices" + "strings" + "syscall" + "testing" +) + +// TestJoinAttemptErrorsKeepsEveryAttempt pins the diagnosis the incident lost. +// +// The transport dials several address families in turn. The IPv4 attempt is the +// one that says "the host is blocking ctrld"; the last attempt is usually an IPv6 +// address that is simply unroutable and reports "no route to host". Returning only +// the last error is what turned a self-inflicted block into a phantom routing +// problem in the logs, and sent the investigation after a network fault that did +// not exist. +func TestJoinAttemptErrorsKeepsEveryAttempt(t *testing.T) { + blocked := &net.OpError{Op: "dial", Net: "tcp4", Err: wsaEACCES} + unroutable := &net.OpError{Op: "dial", Net: "tcp6", Err: syscall.EHOSTUNREACH} + + err := joinAttemptErrors([]error{ + wrapAttempt("resolved ipv4", blocked), + wrapAttempt("direct ipv6", unroutable), + }) + if err == nil { + t.Fatal("joinAttemptErrors() = nil for two failed attempts") + } + + msg := err.Error() + for _, want := range []string{"resolved ipv4", "direct ipv6"} { + if !strings.Contains(msg, want) { + t.Errorf("error text does not name the %q attempt: %s", want, msg) + } + } + if !errors.Is(err, wsaEACCES) { + t.Errorf("the IPv4 socket denial did not survive; a caller can no longer tell a local block from a routing failure: %s", msg) + } + if !errors.Is(err, syscall.EHOSTUNREACH) { + t.Errorf("the last attempt's error did not survive: %s", msg) + } + if strings.Contains(msg, "\n") { + t.Errorf("the joined error spans lines, which breaks one-record-per-failure logging: %q", msg) + } +} + +// TestJoinAttemptErrorsSingleAndEmpty covers the degenerate inputs: one attempt is +// returned untouched, and no attempt at all still has to be an error rather than a +// nil the dialer would hand back as a successful connection. +func TestJoinAttemptErrorsSingleAndEmpty(t *testing.T) { + only := errors.New("only attempt") + if got := joinAttemptErrors([]error{only}); !errors.Is(got, only) { + t.Errorf("joinAttemptErrors() = %v, want the single attempt unwrapped", got) + } + if got := joinAttemptErrors(nil); got == nil { + t.Error("joinAttemptErrors(nil) = nil; the dialer would report success with no connection") + } +} + +// TestAPIDialStagesAlwaysDialTheDirectIPs is the guarantee the direct addresses +// exist for: when DNS is unusable, ctrld must still reach the API. +// +// Whatever resolution returns - nothing, stale addresses, one family only - every +// direct address is dialed. The only thing the duplicate trim removes is a second +// dial of an address an earlier stage already covers. +func TestAPIDialStagesAlwaysDialTheDirectIPs(t *testing.T) { + const ( + directV4 = apiDomainComIPv4 + directV6 = apiDomainComIPv6 + ) + v4, v6 := []string{directV4}, []string{directV6} + + tests := []struct { + name string + resolved []string + }{ + {"resolution returned nothing", nil}, + {"resolution returned the direct ips", []string{directV4, directV6}}, + {"resolution returned stale ips", []string{"203.0.113.10", "2001:db8::1"}}, + {"resolution returned ipv4 only", []string{"203.0.113.10"}}, + {"resolution returned ipv6 only", []string{"2001:db8::1"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stages := apiDialStages(tt.resolved, v4, v6) + + var dialed []string + for _, stage := range stages { + if len(stage.ips) == 0 { + t.Errorf("stage %q has no address; it would dial nothing", stage.what) + } + dialed = append(dialed, stage.ips...) + } + for _, direct := range []string{directV4, directV6} { + if !slices.Contains(dialed, direct) { + t.Errorf("the direct address %s is never dialed; the API is unreachable without DNS", direct) + } + if n := count(dialed, direct); n != 1 { + t.Errorf("the direct address %s is dialed %d times, want exactly 1", direct, n) + } + } + for _, ip := range tt.resolved { + if !slices.Contains(dialed, ip) { + t.Errorf("the resolved address %s is never dialed", ip) + } + } + }) + } +} + +// TestAPIDialStagesTryIPv4First pins the order: the IPv4 stages come before the +// IPv6 ones. IPv6 at these hosts is commonly unroutable, and its "no route to +// host" is what used to be the only error a failure reported. +func TestAPIDialStagesTryIPv4First(t *testing.T) { + stages := apiDialStages([]string{"203.0.113.10", "2001:db8::1"}, + []string{apiDomainComIPv4}, []string{apiDomainComIPv6}) + + var order []string + for _, stage := range stages { + order = append(order, stage.network) + } + want := []string{"tcp4", "tcp4", "tcp6", "tcp6"} + if len(order) != len(want) { + t.Fatalf("stage networks = %v, want %v", order, want) + } + for i := range want { + if order[i] != want[i] { + t.Fatalf("stage networks = %v, want %v", order, want) + } + } +} + +func count(haystack []string, needle string) int { + var n int + for _, s := range haystack { + if s == needle { + n++ + } + } + return n +} + +// TestNotInSkipsAlreadyDialedAddresses covers the duplicate-dial trim: LookupIP +// normally answers with the direct addresses, so dialing both lists doubles every +// failure for no added chance of success. +func TestNotInSkipsAlreadyDialedAddresses(t *testing.T) { + if got := notIn([]string{apiDomainComIPv4}, []string{apiDomainComIPv4}); len(got) != 0 { + t.Errorf("notIn() = %v, want empty: the address was already dialed", got) + } + if got := notIn([]string{apiDomainComIPv4}, []string{"203.0.113.10"}); len(got) != 1 { + t.Errorf("notIn() = %v, want the direct address kept when it was not dialed", got) + } + if got := notIn([]string{apiDomainComIPv4}, nil); len(got) != 1 { + t.Errorf("notIn() = %v, want the direct address kept when nothing resolved", got) + } +} + +// TestAPIEndpointIPsCoverEveryDialedAddress ties the Firewall Mode allowlist to the +// transport. Firewall Mode permits APIEndpointIPs; the transport dials +// apiDirectIPs. If one grows an address the other does not, ctrld starts blocking +// its own control plane again, which is precisely the 38-hour outage. +func TestAPIEndpointIPsCoverEveryDialedAddress(t *testing.T) { + for _, dev := range []bool{false, true} { + permitted := APIEndpointIPs(dev) + v4, v6 := apiDirectIPs(dev) + for _, ip := range append(append([]string{}, v4...), v6...) { + if !slices.Contains(permitted, ip) { + t.Errorf("cdDev=%v: the transport dials %s but APIEndpointIPs does not report it, so Firewall Mode will not permit it", dev, ip) + } + } + if len(permitted) != len(v4)+len(v6) { + t.Errorf("cdDev=%v: APIEndpointIPs = %v, but the split halves are %v/%v", dev, permitted, v4, v6) + } + } +} diff --git a/internal/controld/config.go b/internal/controld/config.go index 895828c..2e0cd14 100644 --- a/internal/controld/config.go +++ b/internal/controld/config.go @@ -11,6 +11,7 @@ import ( "net" "net/http" "runtime" + "slices" "strings" "time" @@ -322,20 +323,49 @@ func ParseRawUID(rawUID string) (string, string) { return uid, clientID } +// APIDomain returns the ControlD API hostname for the environment. +func APIDomain(cdDev bool) string { + if cdDev { + return apiDomainDev + } + return apiDomainCom +} + +// APIEndpointIPs returns the addresses the API transport dials directly when the +// hostname cannot be resolved. +// +// Exported because Firewall Mode has to permit them: it blocks every destination +// ctrld did not resolve through its own listener, and the API is resolved through +// the OS resolver by LookupIP instead, so nothing ever teaches the allowlist about +// it. Left unpermitted, ctrld's own block-all filters deny its API socket - which +// is what stranded the 2026-08-16 Windows run with 920 WSAEACCES denials and not +// one successful configuration refresh in 38 hours. +func APIEndpointIPs(cdDev bool) []string { + if cdDev { + return []string{apiDomainDevIPv4} + } + return []string{apiDomainComIPv4, apiDomainComIPv6} +} + +// apiDirectIPs splits APIEndpointIPs into its IPv4 and IPv6 halves. +func apiDirectIPs(cdDev bool) (v4, v6 []string) { + for _, ip := range APIEndpointIPs(cdDev) { + if strings.Contains(ip, ":") { + v6 = append(v6, ip) + } else { + v4 = append(v4, ip) + } + } + return v4, v6 +} + // apiTransport returns an HTTP transport for connecting to ControlD API endpoint. func apiTransport(loggerCtx context.Context, cdDev bool) *http.Transport { transport := http.DefaultTransport.(*http.Transport).Clone() transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { - apiDomain := apiDomainCom - apiIpsV4 := []string{apiDomainComIPv4} - apiIpsV6 := []string{apiDomainComIPv6} - apiIPs := []string{apiDomainComIPv4, apiDomainComIPv6} - if cdDev { - apiDomain = apiDomainDev - apiIpsV4 = []string{apiDomainDevIPv4} - apiIpsV6 = []string{} - apiIPs = []string{apiDomainDevIPv4} - } + apiDomain := APIDomain(cdDev) + apiIpsV4, apiIpsV6 := apiDirectIPs(cdDev) + apiIPs := APIEndpointIPs(cdDev) ips := ctrld.LookupIP(loggerCtx, apiDomain) if len(ips) == 0 { @@ -344,18 +374,6 @@ func apiTransport(loggerCtx context.Context, cdDev bool) *http.Transport { ips = apiIPs } - // Separate IPv4 and IPv6 addresses - // This separation is needed because different network stacks may have different - // connectivity to IPv4 vs IPv6, so we try them separately for better reliability - var ipv4s, ipv6s []string - for _, ip := range ips { - if strings.Contains(ip, ":") { - ipv6s = append(ipv6s, ip) - } else { - ipv4s = append(ipv4s, ip) - } - } - dial := func(ctx context.Context, network string, addrs []string) (net.Conn, error) { d := &ctrldnet.ParallelDialer{} logger := ctrld.LoggerFromCtx(loggerCtx) @@ -363,25 +381,21 @@ func apiTransport(loggerCtx context.Context, cdDev bool) *http.Transport { } _, port, _ := net.SplitHostPort(addr) - // Try IPv4 first - if len(ipv4s) > 0 { - if conn, err := dial(ctx, "tcp4", addrsFromPort(ipv4s, port)); err == nil { + var attempts []error + for _, stage := range apiDialStages(ips, apiIpsV4, apiIpsV6) { + conn, err := dial(ctx, stage.network, addrsFromPort(stage.ips, port)) + if err == nil { return conn, nil } + attempts = append(attempts, wrapAttempt(stage.what, err)) } - // Fallback to direct IPv4 - if conn, err := dial(ctx, "tcp4", addrsFromPort(apiIpsV4, port)); err == nil { - return conn, nil - } - - // Fallback to IPv6 if available - if len(ipv6s) > 0 { - if conn, err := dial(ctx, "tcp6", addrsFromPort(ipv6s, port)); err == nil { - return conn, nil - } - } - // Fallback to direct IPv6 - return dial(ctx, "tcp6", addrsFromPort(apiIpsV6, port)) + // Every attempt is reported, not just the last one. The stage that + // diagnoses a local block is the IPv4 one - on Windows a firewall denying + // ctrld's own socket surfaces there as WSAEACCES - while the last stage is + // an IPv6 address that is commonly unroutable and fails with a bare "no + // route to host". Returning only that turned a self-inflicted block into a + // phantom routing problem and sent an incident investigation the wrong way. + return nil, joinAttemptErrors(attempts) } if runtime.GOOS == "android" { transport.TLSClientConfig = &tls.Config{RootCAs: certs.CACertPool(), MinVersion: tls.VersionTLS12} @@ -389,6 +403,105 @@ func apiTransport(loggerCtx context.Context, cdDev bool) *http.Transport { return transport } +// apiDialStage is one attempt in the API transport's fallback order. +type apiDialStage struct { + what string + network string + ips []string +} + +// apiDialStages plans the dial order for one API connection: resolved IPv4, the +// direct IPv4, then the same for IPv6. The families are attempted separately +// because a host can have working connectivity to one and not the other. +// +// Every direct address is always dialed. It is the address that has to work when +// DNS does not, so it is dropped from its own stage only when it is already in +// the resolved list and the earlier stage therefore dials it anyway - dialing it +// twice doubles the failures without adding a chance of success. If resolution +// returns nothing, or returns addresses that are stale or wrong, the direct +// stages still carry the full direct list. +func apiDialStages(resolved, directV4, directV6 []string) []apiDialStage { + // Different network stacks may have different connectivity to IPv4 vs IPv6. + var ipv4s, ipv6s []string + for _, ip := range resolved { + if strings.Contains(ip, ":") { + ipv6s = append(ipv6s, ip) + } else { + ipv4s = append(ipv4s, ip) + } + } + stages := []apiDialStage{ + {"resolved ipv4", "tcp4", ipv4s}, + {"direct ipv4", "tcp4", notIn(directV4, ipv4s)}, + {"resolved ipv6", "tcp6", ipv6s}, + {"direct ipv6", "tcp6", notIn(directV6, ipv6s)}, + } + out := make([]apiDialStage, 0, len(stages)) + for _, stage := range stages { + if len(stage.ips) > 0 { + out = append(out, stage) + } + } + return out +} + +// notIn returns the members of ips that are absent from seen. +// +// The direct-IP stages exist for when the hostname does not resolve, and LookupIP +// usually answers with those very addresses, so dialing both lists doubles the +// failures for no added chance of success. +func notIn(ips, seen []string) []string { + if len(seen) == 0 { + return ips + } + var out []string + for _, ip := range ips { + if !slices.Contains(seen, ip) { + out = append(out, ip) + } + } + return out +} + +// wrapAttempt labels one dial attempt's failure with the stage that produced it, +// so a joined error says which family and which address list failed how. +func wrapAttempt(what string, err error) error { + return fmt.Errorf("%s: %w", what, err) +} + +// joinAttemptErrors combines the dial attempts into one error. +func joinAttemptErrors(attempts []error) error { + switch len(attempts) { + case 0: + return errors.New("no api address to dial") + case 1: + return attempts[0] + } + return &dialAttemptsError{attempts: attempts} +} + +// dialAttemptsError carries every attempt the API dialer made. +// +// errors.Join would do the same for errors.Is, but renders one attempt per line, +// and these end up in a single log record; this keeps them on one line. Unwrap +// returns all of them, so a caller testing for a specific errno - a local socket +// denial rather than an unroutable address - finds it wherever in the sequence it +// happened, not only if it happened last. +type dialAttemptsError struct { + attempts []error +} + +func (e *dialAttemptsError) Error() string { + msgs := make([]string, 0, len(e.attempts)) + for _, err := range e.attempts { + msgs = append(msgs, err.Error()) + } + return strings.Join(msgs, "; ") +} + +// Unwrap exposes every attempt to errors.Is and errors.As. +func (e *dialAttemptsError) Unwrap() []error { return e.attempts } + func addrsFromPort(ips []string, port string) []string { addrs := make([]string, len(ips)) for i, ip := range ips {