Release branch v1.5.3

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 {

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)
+			}
+		})
+	}
+}

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,13 +264,19 @@ func (p *prog) registerControlServerHandler() {
 		switch req.Pin {
 		case cdDeactivationPin.Load():
 			code = http.StatusOK
+			deactivationFailedAttempts.Store(0)
 			select {
 			case p.pinCodeValidCh <- struct{}{}:
 			default:
 			}
 		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)
 	}))

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:<listener> -> 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")

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
+}

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)
+	}
+}

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())
 }