diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index 691d308c..b75856fb 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -638,6 +638,19 @@ const defaultDeactivationPin = -1 // cdDeactivationPin is used in cd mode to decide whether stop and uninstall commands can be run. var cdDeactivationPin atomic.Int64 +// Brute-force protection for the deactivation PIN endpoint on the control socket. +// After deactivationMaxFailedAttempts consecutive wrong PINs, further attempts are +// rejected for deactivationLockoutSeconds. Counter resets on a correct PIN. +const ( + deactivationMaxFailedAttempts = 5 + deactivationLockoutSeconds = 60 +) + +var ( + deactivationFailedAttempts atomic.Int64 + deactivationLockedUntil atomic.Int64 +) + func init() { cdDeactivationPin.Store(defaultDeactivationPin) } @@ -1245,7 +1258,7 @@ func tryUpdateListenerConfigIntercept(cfg *ctrld.Config, notifyFunc func(), fata return false, true } - hasExplicitConfig := lc.IP != "" && lc.IP != "0.0.0.0" && lc.Port != 0 + hasExplicitConfig := isExplicitInterceptListener(lc.IP, lc.Port) if !hasExplicitConfig { // Set defaults for intercept mode if lc.IP == "" || lc.IP == "0.0.0.0" { @@ -1303,6 +1316,16 @@ func tryUpdateListenerConfigIntercept(cfg *ctrld.Config, notifyFunc func(), fata return updated, false } +func isExplicitInterceptListener(ip string, port int) bool { + if ip == "" || ip == "0.0.0.0" || port == 0 { + return false + } + // 127.0.0.1:53 is the default macOS DNS-intercept listener. It can appear + // in generated/custom Control D configs, but it should still be allowed to + // fall back to 127.0.0.1:5354 when mDNSResponder already owns port 53. + return !(ip == "127.0.0.1" && port == 53) +} + // tryUpdateListenerConfig tries updating listener config with a working one. // If fatal is true, and there's listen address conflicted, the function do // fatal error. @@ -1809,6 +1832,9 @@ var errInvalidDeactivationPin = errors.New("deactivation pin is invalid") // errRequiredDeactivationPin indicates that the deactivation pin is required but not provided by users. var errRequiredDeactivationPin = errors.New("deactivation pin is required to stop or uninstall the service") +// errTooManyDeactivationPin represents an error indicating excessive deactivation PIN request attempts. +var errTooManyDeactivationPin = errors.New("too many request attempts") + // checkDeactivationPin validates if the deactivation pin matches one in ControlD config. func checkDeactivationPin(s service.Service, stopCh chan struct{}) error { mainLog.Load().Debug().Msg("Checking deactivation pin") @@ -1837,6 +1863,9 @@ func checkDeactivationPin(s service.Service, stopCh chan struct{}) error { case http.StatusBadRequest: mainLog.Load().Error().Msg(errRequiredDeactivationPin.Error()) return errRequiredDeactivationPin // pin is required + case http.StatusTooManyRequests: + mainLog.Load().Error().Msg(errTooManyDeactivationPin.Error()) + return errTooManyDeactivationPin case http.StatusOK: return nil // valid pin case http.StatusNotFound: @@ -1849,7 +1878,9 @@ func checkDeactivationPin(s service.Service, stopCh chan struct{}) error { // isCheckDeactivationPinErr reports whether there is an error during check deactivation pin process. func isCheckDeactivationPinErr(err error) bool { - return errors.Is(err, errInvalidDeactivationPin) || errors.Is(err, errRequiredDeactivationPin) + return errors.Is(err, errInvalidDeactivationPin) || + errors.Is(err, errRequiredDeactivationPin) || + errors.Is(err, errTooManyDeactivationPin) } // ensureUninstall ensures that s.Uninstall will remove ctrld service from system completely. @@ -2002,17 +2033,25 @@ func doValidateCdRemoteConfig(cdUID string, fatal bool) error { } else { if errors.As(cfgErr, &viper.ConfigParseError{}) { if configStr, _ := base64.StdEncoding.DecodeString(rc.Ctrld.CustomConfig); len(configStr) > 0 { - tmpDir := os.TempDir() - tmpConfFile := filepath.Join(tmpDir, "ctrld.toml") errorLogged := false - // Write remote config to a temporary file to get details error. - if we := os.WriteFile(tmpConfFile, configStr, 0600); we == nil { + // Write remote config to a uniquely named temporary file to get detailed error. + if tmpFile, tmpErr := os.CreateTemp("", "ctrld-*.toml"); tmpErr == nil { + tmpConfFile := tmpFile.Name() + if _, err := tmpFile.Write(configStr); err != nil { + mainLog.Load().Error().Err(err).Msg("failed to write temporary config file") + } + if err := tmpFile.Close(); err != nil { + mainLog.Load().Error().Err(err).Msg("failed to save temporary config file") + + } if de := decoderErrorFromTomlFile(tmpConfFile); de != nil { row, col := de.Position() mainLog.Load().Error().Msgf("failed to parse custom config at line: %d, column: %d, error: %s", row, col, de.Error()) errorLogged = true } - _ = os.Remove(tmpConfFile) + if err := os.Remove(tmpConfFile); err != nil { + mainLog.Load().Error().Err(err).Msg("failed to remove temporary config file") + } } // If we could not log details error, emit what we have already got. if !errorLogged { diff --git a/cmd/cli/cli_intercept_listener_test.go b/cmd/cli/cli_intercept_listener_test.go new file mode 100644 index 00000000..489b7acd --- /dev/null +++ b/cmd/cli/cli_intercept_listener_test.go @@ -0,0 +1,28 @@ +package cli + +import "testing" + +func TestIsExplicitInterceptListener(t *testing.T) { + tests := []struct { + name string + ip string + port int + want bool + }{ + {name: "empty", ip: "", port: 0, want: false}, + {name: "wildcard", ip: "0.0.0.0", port: 53, want: false}, + {name: "zero port", ip: "127.0.0.1", port: 0, want: false}, + {name: "default intercept listener", ip: "127.0.0.1", port: 53, want: false}, + {name: "fallback port explicit", ip: "127.0.0.1", port: 5354, want: true}, + {name: "custom loopback explicit", ip: "127.0.0.2", port: 53, want: true}, + {name: "custom address explicit", ip: "192.0.2.10", port: 53, want: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isExplicitInterceptListener(tt.ip, tt.port); got != tt.want { + t.Fatalf("isExplicitInterceptListener(%q, %d) = %v, want %v", tt.ip, tt.port, got, tt.want) + } + }) + } +} diff --git a/cmd/cli/control_server.go b/cmd/cli/control_server.go index 6f4388a1..976569b6 100644 --- a/cmd/cli/control_server.go +++ b/cmd/cli/control_server.go @@ -59,12 +59,18 @@ func newControlServer(addr string) (*controlServer, error) { func (s *controlServer) start() error { _ = os.Remove(s.addr) unixListener, err := net.Listen("unix", s.addr) - if l, ok := unixListener.(*net.UnixListener); ok { - l.SetUnlinkOnClose(true) - } if err != nil { return err } + // Restrict socket permissions to owner-only (0600) so that only the + // process owner (typically root) can connect. Defense-in-depth since + // the control server endpoints carry no authentication of their own. + if err := os.Chmod(s.addr, 0600); err != nil { + return err + } + if l, ok := unixListener.(*net.UnixListener); ok { + l.SetUnlinkOnClose(true) + } go s.server.Serve(unixListener) return nil } @@ -219,6 +225,12 @@ func (p *prog) registerControlServerHandler() { return } + // Reject further attempts while locked out due to repeated wrong PINs. + if now := time.Now().Unix(); now < deactivationLockedUntil.Load() { + w.WriteHeader(http.StatusTooManyRequests) + return + } + // Re-fetch pin code from API. rcReq := &controld.ResolverConfigRequest{ RawUID: cdUID, @@ -252,6 +264,7 @@ func (p *prog) registerControlServerHandler() { switch req.Pin { case cdDeactivationPin.Load(): code = http.StatusOK + deactivationFailedAttempts.Store(0) select { case p.pinCodeValidCh <- struct{}{}: default: @@ -259,6 +272,11 @@ func (p *prog) registerControlServerHandler() { case defaultDeactivationPin: // If the pin code was set, but users do not provide --pin, return proper code to client. code = http.StatusBadRequest + default: + if deactivationFailedAttempts.Add(1) >= deactivationMaxFailedAttempts { + deactivationLockedUntil.Store(time.Now().Unix() + deactivationLockoutSeconds) + deactivationFailedAttempts.Store(0) + } } w.WriteHeader(code) })) diff --git a/cmd/cli/dns_intercept_darwin.go b/cmd/cli/dns_intercept_darwin.go index 88d7310d..7b72df9c 100644 --- a/cmd/cli/dns_intercept_darwin.go +++ b/cmd/cli/dns_intercept_darwin.go @@ -1207,7 +1207,6 @@ func stringSlicesEqual(a, b []string) bool { return true } - // pfStartStabilization enters stabilization mode, suppressing all pf restores // until the VPN's ruleset stops changing. This prevents a death spiral where // ctrld and the VPN repeatedly overwrite each other's pf rules. @@ -1284,6 +1283,10 @@ func (p *prog) pfStabilizationLoop(ctx context.Context, stableRequired time.Dura p.pfStabilizing.Store(false) mainLog.Load().Info().Msgf("DNS intercept: pf stable for %s — restoring anchor rules", stableRequired) p.ensurePFAnchorActive() + routes, domainlessServers, exemptions := p.refreshDNSAfterVPNSettle("pf_stabilized") + if routes == 0 && domainlessServers == 0 && exemptions == 0 { + p.scheduleDNSAfterVPNSettleRefresh("pf_stabilized_followup", pfAnchorRecheckDelayLong) + } p.pfLastRestoreTime.Store(time.Now().UnixMilli()) return } @@ -1446,6 +1449,15 @@ func (p *prog) ensurePFAnchorActive() bool { return true } +func (p *prog) scheduleDNSAfterVPNSettleRefresh(reason string, delay time.Duration) { + time.AfterFunc(delay, func() { + if p.dnsInterceptState == nil { + return + } + p.refreshDNSAfterVPNSettle(reason) + }) +} + // pfWatchdog periodically checks that our pf anchor is still active. // Other programs (e.g., Windscribe desktop app, macOS configd) can replace // scheduleDelayedRechecks schedules delayed re-checks after a network change event. @@ -1811,7 +1823,14 @@ func (p *prog) forceReloadPFMainRuleset() { mainLog.Load().Error().Err(err).Msgf("DNS intercept: force reload — failed to load anchor (output: %s)", strings.TrimSpace(string(out))) } - // Reset upstream transports — pf reload flushes state table, killing DoH connections. + // Flush stale rdr/reply states after the forced ruleset + anchor reload. + // Without this, macOS can keep using pre-reload state and try to send + // redirected DNS replies directly from loopback to tunnel client addresses + // (for example, 127.0.0.1: -> 100.64.0.0/10), which fails with + // "sendmsg: can't assign requested address". + flushPFStates() + + // Reset upstream transports — pf reload/state flush kills existing DoH connections. p.resetUpstreamTransports() mainLog.Load().Info().Msg("DNS intercept: force reload — pf ruleset and anchor reloaded successfully") diff --git a/cmd/cli/dns_intercept_settle.go b/cmd/cli/dns_intercept_settle.go new file mode 100644 index 00000000..bc8a0d78 --- /dev/null +++ b/cmd/cli/dns_intercept_settle.go @@ -0,0 +1,50 @@ +package cli + +import "github.com/Control-D-Inc/ctrld" + +var initializeOsResolver = ctrld.InitializeOsResolver + +func (p *prog) refreshDNSAfterVPNSettle(reason string) (routes, domainlessServers, exemptions int) { + mainLog.Load().Info().Msgf("DNS intercept: refreshing OS/VPN DNS route state after VPN settle (%s)", reason) + ns := initializeOsResolver(true) + mainLog.Load().Debug().Msgf("DNS intercept: post-settle OS resolver nameservers: %v", ns) + + if p.vpnDNS == nil { + mainLog.Load().Debug().Msg("DNS intercept: post-settle VPN DNS route refresh skipped — manager unavailable") + return 0, 0, 0 + } + + beforeExemptions := p.vpnDNS.CurrentExemptions() + routes, domainlessServers, exemptions = p.vpnDNS.RefreshRoutesOnly() + afterExemptions := p.vpnDNS.CurrentExemptions() + + if vpnDNSExemptionsEqual(beforeExemptions, afterExemptions) { + mainLog.Load().Info().Msgf("DNS intercept: post-settle VPN DNS route refresh completed — %d routes, %d domainless servers, %d exemptions (pf unchanged)", + routes, domainlessServers, exemptions) + return routes, domainlessServers, exemptions + } + + if err := p.exemptVPNDNSServers(afterExemptions); err != nil { + mainLog.Load().Warn().Err(err).Msg("DNS intercept: post-settle VPN DNS exemption update failed") + } else { + mainLog.Load().Info().Msgf("DNS intercept: post-settle VPN DNS exemptions changed — updated pf/WFP with %d exemptions", len(afterExemptions)) + } + return routes, domainlessServers, exemptions +} + +func vpnDNSExemptionsEqual(a, b []vpnDNSExemption) bool { + if len(a) != len(b) { + return false + } + seen := make(map[vpnDNSExemption]int, len(a)) + for _, ex := range a { + seen[ex]++ + } + for _, ex := range b { + if seen[ex] == 0 { + return false + } + seen[ex]-- + } + return true +} diff --git a/cmd/cli/dns_intercept_settle_test.go b/cmd/cli/dns_intercept_settle_test.go new file mode 100644 index 00000000..925806ce --- /dev/null +++ b/cmd/cli/dns_intercept_settle_test.go @@ -0,0 +1,49 @@ +package cli + +import ( + "context" + "testing" + + "github.com/Control-D-Inc/ctrld" +) + +func TestRefreshDNSAfterVPNSettleRefreshesOSResolverAndVPNRoutes(t *testing.T) { + oldInitialize := initializeOsResolver + defer func() { initializeOsResolver = oldInitialize }() + + var initialized []bool + initializeOsResolver = func(force bool) []string { + initialized = append(initialized, force) + return []string{"10.102.26.10:53"} + } + + var exemptionUpdates [][]vpnDNSExemption + p := &prog{} + p.vpnDNS = newVPNDNSManager(func(exemptions []vpnDNSExemption) error { + exemptionUpdates = append(exemptionUpdates, append([]vpnDNSExemption{}, exemptions...)) + return nil + }) + p.vpnDNS.discoverVPNDNS = func(context.Context) []ctrld.VPNDNSConfig { + return []ctrld.VPNDNSConfig{{ + InterfaceName: "utun4", + Servers: []string{"10.102.26.10"}, + Domains: []string{"bmwgroup.net"}, + }} + } + + routes, domainlessServers, exemptions := p.refreshDNSAfterVPNSettle("test") + + if routes != 1 || domainlessServers != 0 || exemptions != 1 { + t.Fatalf("expected 1 route, 0 domainless servers, 1 exemption, got routes=%d domainless=%d exemptions=%d", + routes, domainlessServers, exemptions) + } + if len(initialized) != 1 || !initialized[0] { + t.Fatalf("expected forced OS resolver refresh once, got %v", initialized) + } + if got := p.vpnDNS.UpstreamForDomain("jira.cc.bmwgroup.net."); len(got) != 1 || got[0] != "10.102.26.10" { + t.Fatalf("expected refreshed VPN DNS route, got %v", got) + } + if len(exemptionUpdates) != 0 { + t.Fatalf("expected route-only refresh to avoid pf exemption updates, got %+v", exemptionUpdates) + } +} diff --git a/cmd/cli/main_test.go b/cmd/cli/main_test.go index 6ed26c73..3e9cd725 100644 --- a/cmd/cli/main_test.go +++ b/cmd/cli/main_test.go @@ -2,6 +2,7 @@ package cli import ( "os" + "os/exec" "strings" "testing" @@ -13,5 +14,20 @@ var logOutput strings.Builder func TestMain(m *testing.M) { l := zerolog.New(&logOutput) mainLog.Store(&l) + + // Stub the self-upgrade command builder for the whole test binary. The real + // builder execs os.Executable() — which under `go test` IS this test binary + // — with positional args ("upgrade", ...). `go test` stops flag parsing at + // the first positional arg and ignores the rest, so the child just re-runs + // the entire suite, hits the upgrade tests again, and spawns more children: + // a fork bomb of detached processes that stalls the host and (on Windows) + // holds the test binary's image locked, breaking CI artifact cleanup. + // Point it at the test binary with a no-match -test.run so any test that + // reaches performUpgrade still exercises the cmd.Start() success path while + // the child exits immediately without recursing. + newUpgradeCmd = func(exe string) *exec.Cmd { + return exec.Command(exe, "-test.run=^$") + } + os.Exit(m.Run()) } diff --git a/cmd/cli/prog.go b/cmd/cli/prog.go index 96fd2feb..5c5b2759 100644 --- a/cmd/cli/prog.go +++ b/cmd/cli/prog.go @@ -1651,6 +1651,19 @@ func shouldUpgrade(vt string, cv *semver.Version, logger *zerolog.Logger) bool { return true } +// newUpgradeCmd builds the detached command used to self-upgrade. It is a +// package-level variable so tests can stub it. With the real implementation a +// *test* binary would re-exec itself — os.Executable() is the test binary, and +// because `go test` stops flag parsing at the first positional arg ("upgrade") +// it ignores the args and re-runs the entire suite. That child hits the same +// upgrade test and spawns another child, recursively: a fork bomb of detached +// processes that pins the host and locks the test binary's image file. +var newUpgradeCmd = func(exe string) *exec.Cmd { + cmd := exec.Command(exe, "upgrade", "prod", "-vv") + cmd.SysProcAttr = sysProcAttrForDetachedChildProcess() + return cmd +} + // performUpgrade executes the self-upgrade command. // Returns true if upgrade was initiated successfully, false otherwise. func performUpgrade(vt string) bool { @@ -1659,8 +1672,7 @@ func performUpgrade(vt string) bool { mainLog.Load().Error().Err(err).Msg("failed to get executable path, skipped self-upgrade") return false } - cmd := exec.Command(exe, "upgrade", "prod", "-vv") - cmd.SysProcAttr = sysProcAttrForDetachedChildProcess() + cmd := newUpgradeCmd(exe) if err := cmd.Start(); err != nil { mainLog.Load().Error().Err(err).Msg("failed to start self-upgrade") return false diff --git a/cmd/cli/prog_test.go b/cmd/cli/prog_test.go index c4ef5c3b..6831f5ae 100644 --- a/cmd/cli/prog_test.go +++ b/cmd/cli/prog_test.go @@ -262,6 +262,8 @@ func Test_performUpgrade(t *testing.T) { }, } + // newUpgradeCmd is stubbed in TestMain so performUpgrade does not re-exec + // (and fork-bomb) the test binary; see the comment there. for _, tc := range tests { tc := tc t.Run(tc.name, func(t *testing.T) { diff --git a/cmd/cli/vpn_dns.go b/cmd/cli/vpn_dns.go index f671d2d1..f6f8c20c 100644 --- a/cmd/cli/vpn_dns.go +++ b/cmd/cli/vpn_dns.go @@ -7,6 +7,7 @@ import ( "strings" "sync" + "github.com/rs/zerolog" "tailscale.com/net/netmon" "github.com/Control-D-Inc/ctrld" @@ -94,6 +95,8 @@ func (m *vpnDNSManager) Refresh(guardAgainstNoNameservers bool) { m.mu.Lock() defer m.mu.Unlock() + previousExemptions := m.currentExemptionsLocked() + if vpnDNSSettlingEnabled && len(configs) == 0 && guardAgainstNoNameservers && m.hasVPNDNSStateLocked() { if !m.retainedAfterEmptyDiscovery { exemptions := m.currentExemptionsLocked() @@ -180,14 +183,85 @@ func (m *vpnDNSManager) Refresh(guardAgainstNoNameservers bool) { logger.Debug().Msgf("VPN DNS refresh completed: %d configs, %d routes, %d domainless servers, %d unique exemptions", len(m.configs), len(m.routes), len(m.domainlessServers), len(exemptions)) - // Update intercept rules to permit VPN DNS traffic. - // Always call onServersChanged — including when exemptions is empty — so that - // stale exemptions from a previous VPN session get cleared on disconnect. - if m.onServersChanged != nil { - if err := m.onServersChanged(exemptions); err != nil { - logger.Error().Err(err).Msg("Failed to update intercept exemptions for VPN DNS servers") + // Update intercept rules to permit VPN DNS traffic only when the exemption set + // actually changes. Network-change events can fire repeatedly while macOS/VPN + // state is otherwise identical; rewriting pf for identical exemptions can feed + // a self-triggering network-change loop. Empty exemptions are still applied + // when they differ from the previous set, so stale VPN exemptions are cleared + // on disconnect. + m.updateInterceptExemptionsIfChanged(logger, previousExemptions, exemptions, "VPN DNS") +} + +func (m *vpnDNSManager) updateInterceptExemptionsIfChanged(logger *zerolog.Logger, before, after []vpnDNSExemption, reason string) { + if m.onServersChanged == nil { + return + } + if vpnDNSExemptionsEqual(before, after) { + logger.Debug().Msgf("VPN DNS exemptions unchanged after %s refresh; skipping intercept rule update", reason) + return + } + if err := m.onServersChanged(after); err != nil { + logger.Error().Err(err).Msg("Failed to update intercept exemptions for VPN DNS servers") + } +} + +// RefreshRoutesOnly re-discovers VPN DNS configs and updates only ctrld's +// in-memory split-DNS routes. It intentionally does not call onServersChanged, +// so it does not rewrite/reload pf/WFP rules. Use this for post-settle discovery +// checks where we only need to learn late-published VPN search domains. +func (m *vpnDNSManager) RefreshRoutesOnly() (routes, domainlessServers, exemptions int) { + logger := mainLog.Load() + + logger.Debug().Msg("Refreshing VPN DNS route state only") + discoverVPNDNS := m.discoverVPNDNS + if discoverVPNDNS == nil { + discoverVPNDNS = ctrld.DiscoverVPNDNS + } + configs := discoverVPNDNS(context.Background()) + + if dri, err := netmon.DefaultRouteInterface(); err == nil && dri != "" { + for i := range configs { + if configs[i].InterfaceName == dri { + configs[i].IsExitMode = true + } } } + + m.mu.Lock() + defer m.mu.Unlock() + + m.retainedAfterEmptyDiscovery = false + m.configs = configs + m.routes = make(map[string][]string) + + for _, config := range configs { + for _, domain := range config.Domains { + domain = strings.TrimPrefix(domain, "~") + domain = strings.TrimPrefix(domain, ".") + domain = strings.ToLower(domain) + if domain != "" { + m.routes[domain] = append([]string{}, config.Servers...) + } + } + } + + var domainless []string + seenDomainless := make(map[string]bool) + for _, config := range configs { + if len(config.Domains) == 0 && len(config.Servers) > 0 { + for _, server := range config.Servers { + if !seenDomainless[server] { + seenDomainless[server] = true + domainless = append(domainless, server) + } + } + } + } + m.domainlessServers = domainless + + logger.Debug().Msgf("VPN DNS route-only refresh completed: %d configs, %d routes, %d domainless servers, %d exemptions", + len(m.configs), len(m.routes), len(m.domainlessServers), len(m.currentExemptionsLocked())) + return len(m.routes), len(m.domainlessServers), len(m.currentExemptionsLocked()) } func (m *vpnDNSManager) hasVPNDNSStateLocked() bool { diff --git a/cmd/cli/vpn_dns_test.go b/cmd/cli/vpn_dns_test.go index e8c03e67..6a6d520d 100644 --- a/cmd/cli/vpn_dns_test.go +++ b/cmd/cli/vpn_dns_test.go @@ -69,6 +69,31 @@ func TestVPNDNSRefreshClearsOnSecondGuardedEmptyDiscovery(t *testing.T) { } } +func TestVPNDNSRefreshSkipsUnchangedInterceptExemptions(t *testing.T) { + var updates [][]vpnDNSExemption + m := newVPNDNSManager(func(exemptions []vpnDNSExemption) error { + updates = append(updates, append([]vpnDNSExemption{}, exemptions...)) + return nil + }) + m.discoverVPNDNS = func(context.Context) []ctrld.VPNDNSConfig { + return []ctrld.VPNDNSConfig{{ + InterfaceName: "utun-test", + Servers: []string{"10.102.26.10"}, + Domains: []string{"example.internal"}, + }} + } + + m.Refresh(true) + m.Refresh(true) + + if len(updates) != 1 { + t.Fatalf("expected exactly one intercept exemption update for unchanged VPN DNS state, got %d", len(updates)) + } + if len(updates[0]) != 1 || updates[0][0].Server != "10.102.26.10" || updates[0][0].Interface != "utun-test" { + t.Fatalf("unexpected exemption update: %+v", updates[0]) + } +} + func TestVPNDNSTransportFailureSuppressesFallbackOnlyWhileRetainingState(t *testing.T) { withVPNDNSSettlingEnabled(t) m := newVPNDNSManager(nil) diff --git a/config.go b/config.go index edba183d..1fc82e2f 100644 --- a/config.go +++ b/config.go @@ -640,6 +640,7 @@ func (uc *UpstreamConfig) newDOHTransport(addrs []string) *http.Transport { transport.TLSClientConfig = &tls.Config{ RootCAs: uc.certPool, ClientSessionCache: tls.NewLRUClientSessionCache(0), + MinVersion: tls.VersionTLS12, } // Prevent bad tcp connection hanging the requests for too long. diff --git a/config_quic.go b/config_quic.go index 237bb82d..4c35fa8f 100644 --- a/config_quic.go +++ b/config_quic.go @@ -18,7 +18,7 @@ func (uc *UpstreamConfig) newDOH3Transport(addrs []string) http.RoundTripper { return nil } rt := &http3.Transport{} - rt.TLSClientConfig = &tls.Config{RootCAs: uc.certPool} + rt.TLSClientConfig = &tls.Config{RootCAs: uc.certPool, MinVersion: tls.VersionTLS12} rt.Dial = func(ctx context.Context, addr string, tlsCfg *tls.Config, cfg *quic.Config) (*quic.Conn, error) { _, port, _ := net.SplitHostPort(addr) // if we have a bootstrap ip set, use it to avoid DNS lookup @@ -77,7 +77,17 @@ type parallelDialerResult struct { err error } -type quicParallelDialer struct{} +// quicParallelDialer races DialEarly across a list of remote addresses and +// returns the first successful connection. When transport is non-nil, all +// dials share that transport's UDP socket, which removes both the per-dial +// socket allocation and the winner-path socket leak that an owner-of-the-conn +// receiver cannot clean up. When transport is nil, the dialer falls back to a +// fresh UDP socket per attempt (compat path used where no shared transport is +// available yet); the loser paths close their sockets, and the winner path's +// socket is owned by quic.DialEarly's internal transport. +type quicParallelDialer struct { + transport *quic.Transport +} // Dial performs parallel dialing to the given address list. func (d *quicParallelDialer) Dial(ctx context.Context, addrs []string, tlsCfg *tls.Config, cfg *quic.Config) (*quic.Conn, error) { @@ -105,12 +115,24 @@ func (d *quicParallelDialer) Dial(ctx context.Context, addrs []string, tlsCfg *t ch <- ¶llelDialerResult{conn: nil, err: err} return } - udpConn, err := net.ListenUDP("udp", nil) - if err != nil { - ch <- ¶llelDialerResult{conn: nil, err: err} - return + var ( + conn *quic.Conn + udpConn *net.UDPConn + ) + if d.transport != nil { + conn, err = d.transport.DialEarly(ctx, remoteAddr, tlsCfg, cfg) + } else { + udpConn, err = net.ListenUDP("udp", nil) + if err != nil { + ch <- ¶llelDialerResult{conn: nil, err: err} + return + } + conn, err = quic.DialEarly(ctx, udpConn, remoteAddr, tlsCfg, cfg) + if err != nil { + udpConn.Close() + udpConn = nil + } } - conn, err := quic.DialEarly(ctx, udpConn, remoteAddr, tlsCfg, cfg) select { case ch <- ¶llelDialerResult{conn: conn, err: err}: case <-done: diff --git a/docker/Dockerfile b/docker/Dockerfile index 68d4d6dd..dd25aeba 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,4 +1,4 @@ -# Using Debian bullseye for building regular image. +# Using Debian bookworm for building regular image. # Using scratch image for minimal image size. # The final image has: # @@ -8,11 +8,12 @@ # - Non-cgo ctrld binary. # # CI_COMMIT_TAG is used to set the version of ctrld binary. -FROM golang:1.20-bullseye as base +FROM golang:1.25-bookworm AS base WORKDIR /app -RUN apt-get update && apt-get install -y upx-ucl +RUN echo "deb http://deb.debian.org/debian bookworm-backports main" | tee /etc/apt/sources.list.d/backports.list +RUN apt update && apt install -t bookworm-backports upx-ucl COPY . . diff --git a/docker/Dockerfile.debug b/docker/Dockerfile.debug index 2ba36028..d5f053eb 100644 --- a/docker/Dockerfile.debug +++ b/docker/Dockerfile.debug @@ -1,4 +1,4 @@ -# Using Debian bullseye for building regular image. +# Using Debian bookworm for building regular image. # Using scratch image for minimal image size. # The final image has: # @@ -8,11 +8,12 @@ # - Non-cgo ctrld binary. # # CI_COMMIT_TAG is used to set the version of ctrld binary. -FROM golang:bullseye as base +FROM golang:1.25-bookworm AS base WORKDIR /app -RUN apt-get update && apt-get install -y upx-ucl +RUN echo "deb http://deb.debian.org/debian bookworm-backports main" | tee /etc/apt/sources.list.d/backports.list +RUN apt update && apt install -t bookworm-backports upx-ucl COPY . . diff --git a/doh.go b/doh.go index 6b41c116..9f2b87d1 100644 --- a/doh.go +++ b/doh.go @@ -25,6 +25,16 @@ const ( dohOsHeader = "x-cd-os" dohClientIDPrefHeader = "x-cd-cpref" headerApplicationDNS = "application/dns-message" + + // dohMaxResponseSize caps the response body read from a DoH/DoH3 + // upstream. A DNS message is bounded by the protocol's 16-bit length + // field; anything larger cannot be a valid response. The cap stops a + // malicious or compromised upstream from driving ctrld into unbounded + // memory growth via io.ReadAll on attacker-controlled bytes. + dohMaxResponseSize = dns.MaxMsgSize + // dohMaxErrorBodySize bounds how much of a non-200 response body is + // read for inclusion in the returned error. + dohMaxErrorBodySize = 1024 ) // EncodeOsNameMap provides mapping from OS name to a shorter string, used for encoding x-cd-os value. @@ -130,13 +140,17 @@ func (r *dohResolver) Resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, erro } defer resp.Body.Close() - buf, err := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, dohMaxErrorBodySize)) + return nil, fmt.Errorf("wrong response from DOH server, got: %s, status: %d", string(body), resp.StatusCode) + } + + buf, err := io.ReadAll(io.LimitReader(resp.Body, dohMaxResponseSize+1)) if err != nil { return nil, fmt.Errorf("could not read message from response: %w", err) } - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("wrong response from DOH server, got: %s, status: %d", string(buf), resp.StatusCode) + if len(buf) > dohMaxResponseSize { + return nil, fmt.Errorf("DoH response exceeds %d-byte maximum DNS message size", dohMaxResponseSize) } answer := new(dns.Msg) diff --git a/doh_test.go b/doh_test.go index 92fa79f8..6d1e7267 100644 --- a/doh_test.go +++ b/doh_test.go @@ -12,6 +12,7 @@ import ( "net/url" "runtime" "strings" + "sync/atomic" "testing" "time" @@ -196,6 +197,7 @@ func testTLSServer(t *testing.T, handler http.Handler) (*httptest.Server, *x509. server := httptest.NewUnstartedServer(handler) server.TLS = &tls.Config{ Certificates: []tls.Certificate{testCert.tlsCert}, + MinVersion: tls.VersionTLS12, } server.StartTLS() @@ -232,6 +234,7 @@ func newTestHTTP3Server(t *testing.T, handler http.Handler) *testHTTP3Server { tlsConfig := &tls.Config{ Certificates: []tls.Certificate{testCert.tlsCert}, NextProtos: []string{"h3"}, // HTTP/3 protocol identifier + MinVersion: tls.VersionTLS12, } // Create HTTP/3 server @@ -264,3 +267,221 @@ func newTestHTTP3Server(t *testing.T, handler http.Handler) *testHTTP3Server { return h3Server } + +// oversizedDoHHandler streams `bodyBytes` bytes of zeros with the given +// HTTP status. The atomic counter records bytes the handler actually +// wrote, so tests can confirm the client tore down the stream before +// consuming the whole attacker-controlled body. +func oversizedDoHHandler(status int, bodyBytes int64, written *atomic.Int64) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", headerApplicationDNS) + w.WriteHeader(status) + chunk := make([]byte, 64*1024) + var sent int64 + for sent < bodyBytes { + n := int64(len(chunk)) + if remaining := bodyBytes - sent; remaining < n { + n = remaining + } + m, err := w.Write(chunk[:n]) + if err != nil { + return + } + sent += int64(m) + if written != nil { + written.Add(int64(m)) + } + if f, ok := w.(http.Flusher); ok { + f.Flush() + } + } + } +} + +// dohUpstreamForTLSServer wires an UpstreamConfig at a local httptest TLS +// server, trusting its self-signed certificate. BootstrapIP is set so no +// real DNS lookup runs. +func dohUpstreamForTLSServer(t *testing.T, srv *httptest.Server) *UpstreamConfig { + t.Helper() + pool := x509.NewCertPool() + pool.AddCert(srv.Certificate()) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parse server URL: %v", err) + } + uc := &UpstreamConfig{ + Name: "doh-oversize", + Type: ResolverTypeDOH, + Endpoint: srv.URL + "/dns-query", + BootstrapIP: u.Hostname(), + Timeout: 2000, + } + uc.SetCertPool(pool) + uc.Init() + return uc +} + +// doh3UpstreamForAddr wires an UpstreamConfig at a local HTTP/3 server, +// trusting its self-signed certificate. +func doh3UpstreamForAddr(t *testing.T, addr string, cert *x509.Certificate) *UpstreamConfig { + t.Helper() + pool := x509.NewCertPool() + pool.AddCert(cert) + host, _, err := net.SplitHostPort(addr) + if err != nil { + t.Fatalf("split host/port %q: %v", addr, err) + } + uc := &UpstreamConfig{ + Name: "doh3-oversize", + Type: ResolverTypeDOH3, + Endpoint: "h3://" + addr + "/dns-query", + BootstrapIP: host, + Timeout: 5000, + } + uc.SetCertPool(pool) + uc.Init() + return uc +} + +// TestDoHResolve_OversizedBody_Rejected locks in the fix for +// github.com/Control-D-Inc/ctrld/issues/312: a malicious DoH upstream +// returning a body larger than the DNS protocol allows must be rejected +// with an explicit size error rather than buffered into ctrld memory. +func TestDoHResolve_OversizedBody_Rejected(t *testing.T) { + const oversized = 2 * 1024 * 1024 // far past dohMaxResponseSize (~64 KiB) + var written atomic.Int64 + srv := httptest.NewUnstartedServer(oversizedDoHHandler(http.StatusOK, oversized, &written)) + testCert := generateTestCertificate(t) + srv.TLS = &tls.Config{ + Certificates: []tls.Certificate{testCert.tlsCert}, + NextProtos: []string{"h2", "http/1.1"}, + MinVersion: tls.VersionTLS12, + } + srv.StartTLS() + t.Cleanup(srv.Close) + + uc := dohUpstreamForTLSServer(t, srv) + r, err := NewResolver(uc) + if err != nil { + t.Fatalf("NewResolver: %v", err) + } + + msg := new(dns.Msg) + msg.SetQuestion("example.com.", dns.TypeA) + msg.RecursionDesired = true + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + answer, err := r.Resolve(ctx, msg) + if err == nil { + t.Fatalf("Resolve unexpectedly succeeded; answer=%v", answer) + } + if !strings.Contains(err.Error(), "maximum DNS message size") { + t.Fatalf("error %q does not mention the size cap", err) + } + if answer != nil { + t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer) + } + if got := written.Load(); got >= int64(oversized) { + t.Fatalf("server wrote the entire %d-byte body before client tore down (wrote=%d) — cap not effective", oversized, got) + } +} + +// TestDoHResolve_NonOKStatus_BoundedErrorBody locks in that a non-200 +// response with a huge body does not pull the body fully into ctrld +// memory just to format an error string. +func TestDoHResolve_NonOKStatus_BoundedErrorBody(t *testing.T) { + const huge = 8 * 1024 * 1024 + var written atomic.Int64 + srv := httptest.NewUnstartedServer(oversizedDoHHandler(http.StatusBadGateway, huge, &written)) + testCert := generateTestCertificate(t) + srv.TLS = &tls.Config{ + Certificates: []tls.Certificate{testCert.tlsCert}, + NextProtos: []string{"h2", "http/1.1"}, + MinVersion: tls.VersionTLS12, + } + srv.StartTLS() + t.Cleanup(srv.Close) + + uc := dohUpstreamForTLSServer(t, srv) + r, err := NewResolver(uc) + if err != nil { + t.Fatalf("NewResolver: %v", err) + } + + msg := new(dns.Msg) + msg.SetQuestion("example.com.", dns.TypeA) + msg.RecursionDesired = true + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + answer, err := r.Resolve(ctx, msg) + if err == nil { + t.Fatalf("Resolve unexpectedly succeeded; answer=%v", answer) + } + if !strings.Contains(err.Error(), "status: 502") { + t.Fatalf("error %q does not surface the upstream status", err) + } + if answer != nil { + t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer) + } + if got := written.Load(); got > 1024*1024 { + t.Fatalf("server wrote %d bytes before client tore down — error path is reading too much body", got) + } +} + +// TestDoHResolve_OversizedBody_DoH3 mirrors the DoH oversized-body check +// on the HTTP/3 transport, since github-312 specifically reproduced the +// OOM via DoH3. +func TestDoHResolve_OversizedBody_DoH3(t *testing.T) { + const oversized = 2 * 1024 * 1024 + testCert := generateTestCertificate(t) + udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0}) + if err != nil { + t.Fatalf("udp listen: %v", err) + } + h3 := &http3.Server{ + Handler: oversizedDoHHandler(http.StatusOK, oversized, nil), + TLSConfig: &tls.Config{ + Certificates: []tls.Certificate{testCert.tlsCert}, + NextProtos: []string{"h3"}, + MinVersion: tls.VersionTLS12, + }, + } + go func() { + if err := h3.Serve(udpConn); err != nil && !errors.Is(err, http.ErrServerClosed) { + t.Logf("h3 server: %v", err) + } + }() + t.Cleanup(func() { + _ = h3.Close() + _ = udpConn.Close() + }) + time.Sleep(100 * time.Millisecond) + + uc := doh3UpstreamForAddr(t, udpConn.LocalAddr().String(), testCert.cert) + r, err := NewResolver(uc) + if err != nil { + t.Fatalf("NewResolver: %v", err) + } + + msg := new(dns.Msg) + msg.SetQuestion("example.com.", dns.TypeA) + msg.RecursionDesired = true + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + answer, err := r.Resolve(ctx, msg) + if err == nil { + t.Fatalf("Resolve unexpectedly succeeded; answer=%v", answer) + } + if !strings.Contains(err.Error(), "maximum DNS message size") { + t.Fatalf("error %q does not mention the size cap", err) + } + if answer != nil { + t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer) + } +} diff --git a/doq.go b/doq.go index 9729607d..a79cba9f 100644 --- a/doq.go +++ b/doq.go @@ -6,15 +6,23 @@ import ( "context" "crypto/tls" "errors" + "fmt" "io" "net" "runtime" + "sync" "time" "github.com/miekg/dns" "github.com/quic-go/quic-go" ) +// doqMaxResponseSize caps the bytes read from a DoQ stream: a 2-byte +// length prefix plus a DNS message bounded by dns.MaxMsgSize. Anything +// larger cannot be a valid response and is rejected before buffering more +// data from the upstream. +const doqMaxResponseSize = 2 + dns.MaxMsgSize + type doqResolver struct { uc *UpstreamConfig } @@ -41,6 +49,10 @@ func (r *doqResolver) Resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, erro const doqPoolSize = 16 // doqConnPool manages a pool of QUIC connections for DoQ queries using a buffered channel. +// A single quic.Transport (and its UDP socket) is shared by every connection in the pool, +// so the OS socket lifecycle is tied to the pool rather than to each dial. Without this +// ownership model, a strict DoQ upstream that triggers reconnect churn would leak one +// caller-owned UDP socket per dial — see github.com/Control-D-Inc/ctrld/issues/309. type doqConnPool struct { uc *UpstreamConfig addrs []string @@ -48,6 +60,13 @@ type doqConnPool struct { tlsConfig *tls.Config quicConfig *quic.Config conns chan *doqConn + + transportMu sync.Mutex + transport *quic.Transport + transportConn *net.UDPConn + transportErr error + transportInit bool + closed bool } type doqConn struct { @@ -64,6 +83,7 @@ func newDOQConnPool(uc *UpstreamConfig, addrs []string) *doqConnPool { NextProtos: []string{"doq"}, RootCAs: uc.certPool, ServerName: uc.Domain, + MinVersion: tls.VersionTLS12, } quicConfig := &quic.Config{ @@ -167,26 +187,59 @@ func (p *doqConnPool) doResolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, er return nil, err } - // Read response - buf, err := io.ReadAll(stream) - stream.Close() - - // Return connection to pool (mark as potentially bad if error occurred) - isGood := err == nil && len(buf) > 0 - p.putConn(conn, isGood) + // RFC 9250 section 4.2 requires the client to indicate end-of-request by + // closing the send side of the stream (STREAM FIN). Servers may defer + // processing until FIN arrives, so the close must happen before reading. + // Stream.Close closes only the send direction; the receive direction + // remains open for the response. + if err := stream.Close(); err != nil { + p.putConn(conn, false) + return nil, err + } + // A DoQ response is a 2-byte length prefix followed by a DNS message. + // The DNS message is bounded by the protocol at dns.MaxMsgSize, so a + // well-formed response is at most doqMaxResponseSize bytes. Read one + // byte past that cap to distinguish "at limit" from "over limit" and + // reject oversized responses before they can drive memory growth from + // a malicious or compromised upstream. + buf, err := io.ReadAll(io.LimitReader(stream, doqMaxResponseSize+1)) if err != nil { + p.putConn(conn, false) return nil, err } - // io.ReadAll hides io.EOF error, so check for empty buffer + // io.ReadAll hides io.EOF error, so check for empty buffer. if len(buf) == 0 { + p.putConn(conn, false) return nil, io.EOF } - // Unpack DNS response (skip 2-byte length prefix) + if len(buf) > doqMaxResponseSize { + p.putConn(conn, false) + return nil, fmt.Errorf("DoQ response exceeds %d-byte maximum", doqMaxResponseSize) + } + + // RFC 9250: each DoQ DNS message is encoded as a 2-octet length field + // followed by the DNS message. Reject responses that are shorter than + // the prefix or whose prefix declares more bytes than were received, + // and retire the misbehaving connection. Without this guard, buf[2:] + // would panic when len(buf) < 2. + if len(buf) < 2 { + p.putConn(conn, false) + return nil, fmt.Errorf("malformed DoQ response: %d byte(s), need >= 2 for length prefix", len(buf)) + } + respLen := int(buf[0])<<8 | int(buf[1]) + if 2+respLen > len(buf) { + p.putConn(conn, false) + return nil, fmt.Errorf("malformed DoQ response: length prefix %d exceeds payload %d", respLen, len(buf)-2) + } + + p.putConn(conn, true) + + // Unpack DNS response (skip 2-byte length prefix). answer := new(dns.Msg) - if err := answer.Unpack(buf[2:]); err != nil { + if err := answer.Unpack(buf[2 : 2+respLen]); err != nil { return nil, err } answer.SetReply(msg) @@ -233,25 +286,26 @@ func (p *doqConnPool) putConn(conn *quic.Conn, isGood bool) { } // dialConn creates a new QUIC connection using parallel dialing like DoH3. +// All connections from the pool multiplex on a single pool-owned UDP socket, +// so reconnect churn cannot grow the host's FD count. func (p *doqConnPool) dialConn(ctx context.Context) (string, *quic.Conn, error) { logger := ProxyLogger.Load() + tr, err := p.getOrInitTransport() + if err != nil { + return "", nil, err + } + // If we have a bootstrap IP, use it directly if p.uc.BootstrapIP != "" { addr := net.JoinHostPort(p.uc.BootstrapIP, p.port) Log(ctx, logger.Debug(), "Sending DoQ request to: %s", addr) - udpConn, err := net.ListenUDP("udp", nil) - if err != nil { - return "", nil, err - } remoteAddr, err := net.ResolveUDPAddr("udp", addr) if err != nil { - udpConn.Close() return "", nil, err } - conn, err := quic.DialEarly(ctx, udpConn, remoteAddr, p.tlsConfig, p.quicConfig) + conn, err := tr.DialEarly(ctx, remoteAddr, p.tlsConfig, p.quicConfig) if err != nil { - udpConn.Close() return "", nil, err } return addr, conn, nil @@ -263,7 +317,7 @@ func (p *doqConnPool) dialConn(ctx context.Context) (string, *quic.Conn, error) dialAddrs[i] = net.JoinHostPort(p.addrs[i], p.port) } - pd := &quicParallelDialer{} + pd := &quicParallelDialer{transport: tr} conn, err := pd.Dial(ctx, dialAddrs, p.tlsConfig, p.quicConfig) if err != nil { return "", nil, err @@ -274,9 +328,35 @@ func (p *doqConnPool) dialConn(ctx context.Context) (string, *quic.Conn, error) return addr, conn, nil } -// CloseIdleConnections closes all connections in the pool. -// Connections currently checked out (in use) are not closed. +// getOrInitTransport returns the pool's shared quic.Transport, initialising it +// on first call. Once the pool has been closed it permanently returns an error +// so that callers cannot resurrect a dead pool. +func (p *doqConnPool) getOrInitTransport() (*quic.Transport, error) { + p.transportMu.Lock() + defer p.transportMu.Unlock() + if p.closed { + return nil, errors.New("doq pool closed") + } + if p.transportInit { + return p.transport, p.transportErr + } + p.transportInit = true + udpConn, err := net.ListenUDP("udp", nil) + if err != nil { + p.transportErr = err + return nil, err + } + p.transportConn = udpConn + p.transport = &quic.Transport{Conn: udpConn} + return p.transport, nil +} + +// CloseIdleConnections closes all idle connections, the shared quic.Transport, +// and the pool's UDP socket. Connections currently checked out (in use) get +// terminated by the transport close as well — without that, the OS socket +// would remain bound to a goroutine that the caller cannot reach to clean up. func (p *doqConnPool) CloseIdleConnections() { +drain: for { select { case dc := <-p.conns: @@ -284,7 +364,22 @@ func (p *doqConnPool) CloseIdleConnections() { dc.conn.CloseWithError(quic.ApplicationErrorCode(quic.NoError), "") } default: - return + break drain } } + p.transportMu.Lock() + if p.closed { + p.transportMu.Unlock() + return + } + p.closed = true + tr := p.transport + udpConn := p.transportConn + p.transportMu.Unlock() + if tr != nil { + _ = tr.Close() + } + if udpConn != nil { + _ = udpConn.Close() + } } diff --git a/doq_test.go b/doq_test.go index 14055dd0..405b05bf 100644 --- a/doq_test.go +++ b/doq_test.go @@ -1,4 +1,3 @@ -// test_helpers.go package ctrld import ( @@ -8,8 +7,11 @@ import ( "crypto/tls" "crypto/x509" "crypto/x509/pkix" + "io" "math/big" "net" + "os" + "runtime" "strings" "testing" "time" @@ -99,6 +101,7 @@ func newTestQUICServer(t *testing.T) *testQUICServer { tlsConfig := &tls.Config{ Certificates: []tls.Certificate{testCert.tlsCert}, NextProtos: []string{"doq"}, + MinVersion: tls.VersionTLS12, } // Create QUIC listener @@ -221,3 +224,493 @@ func (s *testQUICServer) handleStream(t *testing.T, stream *quic.Stream) { return } } + +// malformedDoQServer is a test QUIC server that drains the client's DoQ +// request and writes caller-supplied raw bytes back. The bytes are not +// required to be a well-framed DoQ response, which is what lets the +// regression tests exercise malformed-response handling. +type malformedDoQServer struct { + listener *quic.Listener + cert *x509.Certificate + addr string + response []byte +} + +func newMalformedDoQServer(t *testing.T, response []byte) *malformedDoQServer { + t.Helper() + + testCert := generateTestCertificate(t) + tlsConfig := &tls.Config{ + Certificates: []tls.Certificate{testCert.tlsCert}, + NextProtos: []string{"doq"}, + } + + listener, err := quic.ListenAddr("127.0.0.1:0", tlsConfig, nil) + if err != nil { + t.Fatalf("failed to create QUIC listener: %v", err) + } + + s := &malformedDoQServer{ + listener: listener, + cert: testCert.cert, + addr: listener.Addr().String(), + response: response, + } + + go s.serve() + t.Cleanup(func() { _ = listener.Close() }) + return s +} + +func (s *malformedDoQServer) serve() { + for { + conn, err := s.listener.Accept(context.Background()) + if err != nil { + return + } + go s.handleConn(conn) + } +} + +func (s *malformedDoQServer) handleConn(conn *quic.Conn) { + for { + stream, err := conn.AcceptStream(context.Background()) + if err != nil { + return + } + go s.handleStream(stream) + } +} + +func (s *malformedDoQServer) handleStream(stream *quic.Stream) { + defer stream.Close() + + // Drain the client's DoQ-framed request so the client's writes complete + // cleanly before we reply with our attacker-controlled bytes. Using + // io.ReadFull because a single Read on a QUIC stream may return short. + lenBuf := make([]byte, 2) + if _, err := io.ReadFull(stream, lenBuf); err != nil { + return + } + msgLen := uint16(lenBuf[0])<<8 | uint16(lenBuf[1]) + if msgLen > 0 { + discard := make([]byte, msgLen) + if _, err := io.ReadFull(stream, discard); err != nil { + return + } + } + + if len(s.response) > 0 { + _, _ = stream.Write(s.response) + } +} + +// newMalformedDoQUpstream builds an UpstreamConfig wired to a local +// malformed test server with the test certificate trusted via a custom +// cert pool. We bypass SetupBootstrapIP by setting BootstrapIP directly, +// so the pool dials 127.0.0.1 without any DNS lookup. +func newMalformedDoQUpstream(t *testing.T, cert *x509.Certificate, addr string) *UpstreamConfig { + t.Helper() + + pool := x509.NewCertPool() + pool.AddCert(cert) + + host, _, err := net.SplitHostPort(addr) + if err != nil { + t.Fatalf("split host/port %q: %v", addr, err) + } + + uc := &UpstreamConfig{ + Name: "doq-malformed", + Type: ResolverTypeDOQ, + Endpoint: addr, + Domain: host, + BootstrapIP: host, + Timeout: 2000, + } + uc.SetCertPool(pool) + return uc +} + +// TestDoQResolve_MalformedResponse verifies that DoQ upstream +// responses violating RFC 9250 framing — fewer than 2 bytes, or a +// length prefix declaring more payload than was received — return a +// handled error instead of panicking on the length-prefix slice. +func TestDoQResolve_MalformedResponse(t *testing.T) { + tests := []struct { + name string + response []byte + }{ + // Empty stream is already handled via io.EOF; locked in so a + // future change that drops that branch is caught. + {"empty response", nil}, + + // One byte: too short to hold the 2-octet length prefix. + {"single byte response", []byte{0x00}}, + + // Length prefix declares 16 bytes; payload is absent. + {"length prefix only", []byte{0x00, 0x10}}, + + // Length prefix declares 65535 bytes; only 1 byte of payload + // arrived. + {"length prefix larger than payload", []byte{0xFF, 0xFF, 0x00}}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + server := newMalformedDoQServer(t, tt.response) + uc := newMalformedDoQUpstream(t, server.cert, server.addr) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + pool := newDOQConnPool(uc, []string{"127.0.0.1"}) + t.Cleanup(pool.CloseIdleConnections) + + msg := new(dns.Msg) + msg.SetQuestion("example.com.", dns.TypeA) + msg.RecursionDesired = true + + answer, err := pool.Resolve(ctx, msg) + if err == nil { + t.Fatalf("Resolve unexpectedly succeeded for malformed response %v; answer=%v", tt.response, answer) + } + if answer != nil { + t.Fatalf("Resolve returned non-nil answer alongside error: answer=%v err=%v", answer, err) + } + }) + } +} + +// strictDoQServer accepts DoQ queries but defers the response until the +// client signals end-of-request with STREAM FIN, as required by RFC 9250 +// section 4.2. It exists to lock in the fix for +// github.com/Control-D-Inc/ctrld/issues/309 where a client +// that never closes its send side caused the server to wait forever and the +// client to churn through reconnects. +type strictDoQServer struct { + listener *quic.Listener + cert *x509.Certificate + addr string +} + +func newStrictDoQServer(t *testing.T) *strictDoQServer { + t.Helper() + + testCert := generateTestCertificate(t) + tlsConfig := &tls.Config{ + Certificates: []tls.Certificate{testCert.tlsCert}, + NextProtos: []string{"doq"}, + MinVersion: tls.VersionTLS12, + } + + listener, err := quic.ListenAddr("127.0.0.1:0", tlsConfig, nil) + if err != nil { + t.Fatalf("failed to create QUIC listener: %v", err) + } + + s := &strictDoQServer{ + listener: listener, + cert: testCert.cert, + addr: listener.Addr().String(), + } + go s.serve() + t.Cleanup(func() { _ = listener.Close() }) + return s +} + +func (s *strictDoQServer) serve() { + for { + conn, err := s.listener.Accept(context.Background()) + if err != nil { + return + } + go s.handleConn(conn) + } +} + +func (s *strictDoQServer) handleConn(conn *quic.Conn) { + for { + stream, err := conn.AcceptStream(context.Background()) + if err != nil { + return + } + go s.handleStream(stream) + } +} + +func (s *strictDoQServer) handleStream(stream *quic.Stream) { + defer stream.Close() + + // Drain until the client closes the send side. This is the behaviour + // that triggered the bug: if the client never sends STREAM FIN, this + // read blocks until the stream's deadline fires. + body, err := io.ReadAll(stream) + if err != nil { + return + } + if len(body) < 2 { + return + } + msgLen := uint16(body[0])<<8 | uint16(body[1]) + if int(msgLen) != len(body)-2 { + return + } + + msg := new(dns.Msg) + if err := msg.Unpack(body[2:]); err != nil { + return + } + + response := new(dns.Msg) + response.SetReply(msg) + response.Authoritative = true + if len(msg.Question) > 0 && msg.Question[0].Qtype == dns.TypeA { + response.Answer = append(response.Answer, &dns.A{ + Hdr: dns.RR_Header{ + Name: msg.Question[0].Name, + Rrtype: dns.TypeA, + Class: dns.ClassINET, + Ttl: 300, + }, + A: net.ParseIP("192.0.2.1"), + }) + } + + respBytes, err := response.Pack() + if err != nil { + return + } + respLen := uint16(len(respBytes)) + if _, err := stream.Write([]byte{byte(respLen >> 8), byte(respLen & 0xFF)}); err != nil { + return + } + if _, err := stream.Write(respBytes); err != nil { + return + } +} + +func newStrictDoQUpstream(t *testing.T, cert *x509.Certificate, addr string, useBootstrap bool) *UpstreamConfig { + t.Helper() + + pool := x509.NewCertPool() + pool.AddCert(cert) + + host, _, err := net.SplitHostPort(addr) + if err != nil { + t.Fatalf("split host/port %q: %v", addr, err) + } + + uc := &UpstreamConfig{ + Name: "doq-strict", + Type: ResolverTypeDOQ, + Endpoint: addr, + Domain: host, + Timeout: 3000, + } + if useBootstrap { + uc.BootstrapIP = host + } + uc.SetCertPool(pool) + return uc +} + +// TestDoQResolve_StrictServerWaitsForFIN exercises the RFC 9250 client-FIN +// requirement. With the bug present, the server's io.ReadAll blocks until +// the stream deadline expires and the client sees a timeout, so a successful +// resolve here proves that the client now sends STREAM FIN before reading. +func TestDoQResolve_StrictServerWaitsForFIN(t *testing.T) { + t.Parallel() + + server := newStrictDoQServer(t) + uc := newStrictDoQUpstream(t, server.cert, server.addr, true) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + host, _, _ := net.SplitHostPort(server.addr) + pool := newDOQConnPool(uc, []string{host}) + t.Cleanup(pool.CloseIdleConnections) + + msg := new(dns.Msg) + msg.SetQuestion("example.com.", dns.TypeA) + msg.RecursionDesired = true + + answer, err := pool.Resolve(ctx, msg) + if err != nil { + t.Fatalf("Resolve failed against strict DoQ server: %v", err) + } + if answer == nil || len(answer.Answer) == 0 { + t.Fatalf("Resolve returned no answer records: %+v", answer) + } + a, ok := answer.Answer[0].(*dns.A) + if !ok || !a.A.Equal(net.ParseIP("192.0.2.1")) { + t.Fatalf("unexpected answer: %+v", answer.Answer[0]) + } +} + +// TestDoQResolve_ParallelDialPathStrictFIN exercises the parallel-dial path +// (no BootstrapIP) against the same FIN-strict server, so that both the +// single-dial branch and the parallel-dial branch are covered. +func TestDoQResolve_ParallelDialPathStrictFIN(t *testing.T) { + t.Parallel() + + server := newStrictDoQServer(t) + uc := newStrictDoQUpstream(t, server.cert, server.addr, false) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + host, _, _ := net.SplitHostPort(server.addr) + pool := newDOQConnPool(uc, []string{host}) + t.Cleanup(pool.CloseIdleConnections) + + msg := new(dns.Msg) + msg.SetQuestion("example.com.", dns.TypeA) + msg.RecursionDesired = true + + answer, err := pool.Resolve(ctx, msg) + if err != nil { + t.Fatalf("Resolve (parallel-dial path) failed against strict DoQ server: %v", err) + } + if answer == nil || len(answer.Answer) == 0 { + t.Fatalf("Resolve (parallel-dial path) returned no answer records: %+v", answer) + } +} + +// TestDoQPool_ChurnDoesNotGrowFDs exercises the reconnect-churn scenario +// described in github.com/Control-D-Inc/ctrld/issues/309: repeated dials +// against a server that closes existing connections must not grow the process +// FD count, because the pool now shares one UDP socket via quic.Transport instead +// of allocating one per dial. Linux-only because /proc/self/fd is the cheapest +// portable proxy for "what's still open." +func TestDoQPool_ChurnDoesNotGrowFDs(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("FD accounting via /proc/self/fd is linux-only") + } + t.Parallel() + + server := newStrictDoQServer(t) + uc := newStrictDoQUpstream(t, server.cert, server.addr, true) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + host, _, _ := net.SplitHostPort(server.addr) + pool := newDOQConnPool(uc, []string{host}) + t.Cleanup(pool.CloseIdleConnections) + + makeQuery := func(i int) *dns.Msg { + msg := new(dns.Msg) + // Vary the question so any caching layer cannot short-circuit. + msg.SetQuestion(dns.Fqdn(strings.Repeat("a", 1+i%8)+".example.com"), dns.TypeA) + msg.RecursionDesired = true + return msg + } + + // Warm the pool so the steady-state transport and at least one + // connection are open. Without this, the first resolve in the measured + // loop would inflate the baseline. + if _, err := pool.Resolve(ctx, makeQuery(0)); err != nil { + t.Fatalf("warm-up Resolve failed: %v", err) + } + + baseline := countOpenFDs(t) + + // Force reconnect churn by closing the connection between each query. + // Without the fix this would leak one UDP socket per round; with the + // fix the pool's shared transport keeps a single socket open. + const rounds = 20 + for i := 1; i <= rounds; i++ { + // Drain any pooled connection so the next Resolve has to redial. + drainPooledConns(pool) + + if _, err := pool.Resolve(ctx, makeQuery(i)); err != nil { + t.Fatalf("Resolve in churn loop iteration %d failed: %v", i, err) + } + } + + // Give quic-go a moment to drop any background goroutines that hold + // references to closed sockets. + time.Sleep(200 * time.Millisecond) + + after := countOpenFDs(t) + + // Allow a small slack for transient FDs (goroutine wake-ups, qlog, + // etc.) but reject anything that scales with the number of rounds. + const slack = 5 + if after > baseline+slack { + t.Fatalf("FD count grew under DoQ churn: baseline=%d after=%d rounds=%d (slack=%d)", baseline, after, rounds, slack) + } +} + +// drainPooledConns removes any idle pooled connections so the next Resolve +// is forced to dial a fresh one. It does not close the pool's transport. +func drainPooledConns(p *doqConnPool) { + for { + select { + case dc := <-p.conns: + if dc.conn != nil { + dc.conn.CloseWithError(quic.ApplicationErrorCode(quic.NoError), "") + } + default: + return + } + } +} + +func countOpenFDs(t *testing.T) int { + t.Helper() + entries, err := os.ReadDir("/proc/self/fd") + if err != nil { + t.Fatalf("read /proc/self/fd: %v", err) + } + return len(entries) +} + +// TestDoQResolve_OversizedResponse_Rejected locks in the fix for +// github.com/Control-D-Inc/ctrld/issues/312 on the DoQ transport: a +// malicious upstream that writes a response larger than the DNS protocol +// allows must be rejected with an explicit size error, not buffered +// without bound into ctrld memory. +func TestDoQResolve_OversizedResponse_Rejected(t *testing.T) { + t.Parallel() + + // doqMaxResponseSize is 2 + dns.MaxMsgSize. Send something well past + // that. 256 KiB is enough to exceed the cap while keeping the test + // fast on loopback. + response := make([]byte, 256*1024) + // A well-formed length prefix isn't required: the size cap should + // fire before any framing check runs. Use a non-zero prefix so the + // test also documents that the order of validation is "size first, + // framing later." + response[0] = 0xFF + response[1] = 0xFF + + server := newMalformedDoQServer(t, response) + uc := newMalformedDoQUpstream(t, server.cert, server.addr) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + pool := newDOQConnPool(uc, []string{"127.0.0.1"}) + t.Cleanup(pool.CloseIdleConnections) + + msg := new(dns.Msg) + msg.SetQuestion("example.com.", dns.TypeA) + msg.RecursionDesired = true + + answer, err := pool.Resolve(ctx, msg) + if err == nil { + t.Fatalf("Resolve unexpectedly succeeded for oversized response; answer=%v", answer) + } + if !strings.Contains(err.Error(), "exceeds") { + t.Fatalf("error %q does not surface the size cap", err) + } + if answer != nil { + t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer) + } +} diff --git a/dot.go b/dot.go index 654fa865..ff0aac27 100644 --- a/dot.go +++ b/dot.go @@ -64,7 +64,8 @@ func newDOTClientPool(uc *UpstreamConfig, addrs []string) *dotConnPool { dialer := newDialer(net.JoinHostPort(controldPublicDns, "53")) tlsConfig := &tls.Config{ - RootCAs: uc.certPool, + RootCAs: uc.certPool, + MinVersion: tls.VersionTLS12, } if uc.BootstrapIP != "" { diff --git a/go.mod b/go.mod index b01897e8..ebbde36e 100644 --- a/go.mod +++ b/go.mod @@ -29,16 +29,16 @@ require ( github.com/prometheus/client_golang v1.19.1 github.com/prometheus/client_model v0.5.0 github.com/prometheus/prom2json v1.3.3 - github.com/quic-go/quic-go v0.57.1 + github.com/quic-go/quic-go v0.59.1 github.com/rs/zerolog v1.28.0 github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.6 github.com/spf13/viper v1.16.0 github.com/stretchr/testify v1.11.1 github.com/vishvananda/netlink v1.3.1 - golang.org/x/net v0.55.0 - golang.org/x/sync v0.20.0 - golang.org/x/sys v0.45.0 + golang.org/x/net v0.56.0 + golang.org/x/sync v0.21.0 + golang.org/x/sys v0.46.0 golang.zx2c4.com/wireguard/windows v0.5.3 tailscale.com v1.74.0 ) @@ -92,11 +92,11 @@ require ( github.com/yusufpapurcu/wmi v1.2.4 // indirect go4.org/mem v0.0.0-20220726221520-4f986261bf13 // indirect go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect - golang.org/x/crypto v0.51.0 // indirect + golang.org/x/crypto v0.53.0 // indirect golang.org/x/exp v0.0.0-20240119083558-1b970713d09a // indirect - golang.org/x/mod v0.35.0 // indirect - golang.org/x/text v0.37.0 // indirect - golang.org/x/tools v0.44.0 // indirect + golang.org/x/mod v0.36.0 // indirect + golang.org/x/text v0.38.0 // indirect + golang.org/x/tools v0.45.0 // indirect google.golang.org/protobuf v1.33.0 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index 439caa78..f9a9f110 100644 --- a/go.sum +++ b/go.sum @@ -271,8 +271,8 @@ github.com/prometheus/prom2json v1.3.3 h1:IYfSMiZ7sSOfliBoo89PcufjWO4eAR0gznGcET github.com/prometheus/prom2json v1.3.3/go.mod h1:Pv4yIPktEkK7btWsrUTWDDDrnpUrAELaOCj+oFwlgmc= github.com/quic-go/qpack v0.6.0 h1:g7W+BMYynC1LbYLSqRt8PBg5Tgwxn214ZZR34VIOjz8= github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII= -github.com/quic-go/quic-go v0.57.1 h1:25KAAR9QR8KZrCZRThWMKVAwGoiHIrNbT72ULHTuI10= -github.com/quic-go/quic-go v0.57.1/go.mod h1:ly4QBAjHA2VhdnxhojRsCUOeJwKYg+taDlos92xb1+s= +github.com/quic-go/quic-go v0.59.1 h1:0Gmua0HW1Tv7ANR7hUYwRyD0MG5OJfgvYSZasGZzBic= +github.com/quic-go/quic-go v0.59.1/go.mod h1:upnsH4Ju1YkqpLXC305eW3yDZ4NfnNbmQRCMWS58IKU= github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.4.4 h1:8TfxU8dW6PdqD27gjM8MVNuicgxIjxpm4K7x4jp8sis= github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= @@ -349,8 +349,8 @@ golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm golang.org/x/crypto v0.0.0-20211209193657-4570a0811e8b/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= -golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= +golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= +golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -386,8 +386,8 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= -golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= +golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4= +golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -420,8 +420,8 @@ golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= -golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= +golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= +golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -441,8 +441,8 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= -golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= +golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -492,8 +492,8 @@ golang.org/x/sys v0.4.1-0.20230131160137-e7d7f63158de/go.mod h1:oPkhp1MJrh7nUepC golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= -golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= +golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -504,13 +504,11 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= -golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE= +golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= -golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= -golang.org/x/time v0.12.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= @@ -558,8 +556,8 @@ golang.org/x/tools v0.0.0-20201208233053-a543418bbed2/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= -golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= -golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= +golang.org/x/tools v0.45.0 h1:18qN3FAooORvApf5XjCXgsuayZOEtXf6JK18I3+ONa8= +golang.org/x/tools v0.45.0/go.mod h1:LuUGqqaXcXMEFEruIVJVm5mgDD8vww/z/SR1gQ4uE/0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/certs/root_ca_test.go b/internal/certs/root_ca_test.go index fcfd16e5..295be56e 100644 --- a/internal/certs/root_ca_test.go +++ b/internal/certs/root_ca_test.go @@ -11,7 +11,8 @@ func TestCACertPool(t *testing.T) { c := &http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ - RootCAs: CACertPool(), + RootCAs: CACertPool(), + MinVersion: tls.VersionTLS12, }, }, Timeout: 2 * time.Second, diff --git a/internal/controld/config.go b/internal/controld/config.go index 436d22db..181358c3 100644 --- a/internal/controld/config.go +++ b/internal/controld/config.go @@ -293,7 +293,7 @@ func apiTransport(cdDev bool) *http.Transport { return dial(ctx, "tcp6", addrsFromPort(apiIpsV6, port)) } if router.Name() == ddwrt.Name || runtime.GOOS == "android" { - transport.TLSClientConfig = &tls.Config{RootCAs: certs.CACertPool()} + transport.TLSClientConfig = &tls.Config{RootCAs: certs.CACertPool(), MinVersion: tls.VersionTLS12} } return transport } diff --git a/internal/router/dnsmasq/dnsmasq.go b/internal/router/dnsmasq/dnsmasq.go index 058b0b59..61b321d6 100644 --- a/internal/router/dnsmasq/dnsmasq.go +++ b/internal/router/dnsmasq/dnsmasq.go @@ -2,11 +2,11 @@ package dnsmasq import ( "errors" - "html/template" "net" "os" "path/filepath" "strings" + "text/template" "github.com/Control-D-Inc/ctrld" )