diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 713a8d441..a2ab7271c 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -698,7 +698,7 @@ func tryEngine(logf logger.Logf, sys *tsd.System, name string) (onlyNetstack boo // configuration being unavailable (from the noop // manager). More in Issue 4017. // TODO(bradfitz): add a Synology-specific DNS manager. - conf.DNS, err = dns.NewOSConfigurator(logf, sys.HealthTracker(), "") // empty interface name + conf.DNS, err = dns.NewOSConfigurator(logf, sys.HealthTracker(), sys.ControlKnobs(), "") // empty interface name if err != nil { return false, fmt.Errorf("dns.NewOSConfigurator: %w", err) } @@ -726,7 +726,7 @@ func tryEngine(logf logger.Logf, sys *tsd.System, name string) (onlyNetstack boo return false, fmt.Errorf("creating router: %w", err) } - d, err := dns.NewOSConfigurator(logf, sys.HealthTracker(), devName) + d, err := dns.NewOSConfigurator(logf, sys.HealthTracker(), sys.ControlKnobs(), devName) if err != nil { dev.Close() r.Close() diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index f4cef7b41..9ff12c76f 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -90,6 +90,15 @@ type Knobs struct { // This is for now (2024-06-06) an iOS-specific battery life optimization, // and this knob allows us to disable the optimization remotely if needed. DisableSplitDNSWhenNoCustomResolvers atomic.Bool + + // DisableLocalDNSOverrideViaNRPT indicates that the node's DNS manager should not + // create a default (catch-all) Windows NRPT rule when "Override local DNS" is enabled. + // Without this rule, Windows 8.1 and newer devices issue parallel DNS requests to DNS servers + // associated with all network adapters, even when "Override local DNS" is enabled and/or + // a Mullvad exit node is being used, resulting in DNS leaks. + // We began creating this rule on 2024-06-14, and this knob + // allows us to disable the new behavior remotely if needed. + DisableLocalDNSOverrideViaNRPT atomic.Bool } // UpdateFromNodeAttributes updates k (if non-nil) based on the provided self @@ -117,6 +126,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { appCStoreRoutes = has(tailcfg.NodeAttrStoreAppCRoutes) userDialUseRoutes = has(tailcfg.NodeAttrUserDialUseRoutes) disableSplitDNSWhenNoCustomResolvers = has(tailcfg.NodeAttrDisableSplitDNSWhenNoCustomResolvers) + disableLocalDNSOverrideViaNRPT = has(tailcfg.NodeAttrDisableLocalDNSOverrideViaNRPT) ) if has(tailcfg.NodeAttrOneCGNATEnable) { @@ -142,6 +152,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { k.AppCStoreRoutes.Store(appCStoreRoutes) k.UserDialUseRoutes.Store(userDialUseRoutes) k.DisableSplitDNSWhenNoCustomResolvers.Store(disableSplitDNSWhenNoCustomResolvers) + k.DisableLocalDNSOverrideViaNRPT.Store(disableLocalDNSOverrideViaNRPT) } // AsDebugJSON returns k as something that can be marshalled with json.Marshal @@ -168,5 +179,6 @@ func (k *Knobs) AsDebugJSON() map[string]any { "AppCStoreRoutes": k.AppCStoreRoutes.Load(), "UserDialUseRoutes": k.UserDialUseRoutes.Load(), "DisableSplitDNSWhenNoCustomResolvers": k.DisableSplitDNSWhenNoCustomResolvers.Load(), + "DisableLocalDNSOverrideViaNRPT": k.DisableLocalDNSOverrideViaNRPT.Load(), } } diff --git a/net/dns/manager.go b/net/dns/manager.go index b955e5041..5bffc56b8 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -485,7 +485,7 @@ func (m *Manager) FlushCaches() error { // in case the Tailscale daemon terminated without closing the router. // No other state needs to be instantiated before this runs. func CleanUp(logf logger.Logf, netMon *netmon.Monitor, interfaceName string) { - oscfg, err := NewOSConfigurator(logf, nil, interfaceName) + oscfg, err := NewOSConfigurator(logf, nil, nil, interfaceName) if err != nil { logf("creating dns cleanup: %v", err) return diff --git a/net/dns/manager_darwin.go b/net/dns/manager_darwin.go index 3ccdd6103..7e4d40320 100644 --- a/net/dns/manager_darwin.go +++ b/net/dns/manager_darwin.go @@ -8,12 +8,16 @@ "os" "go4.org/mem" + "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" "tailscale.com/util/mak" ) -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, ifName string) (OSConfigurator, error) { +// NewOSConfigurator creates a new OS configurator. +// +// The health tracker and the knobs may be nil and are ignored on this platform. +func NewOSConfigurator(logf logger.Logf, _ *health.Tracker, _ *controlknobs.Knobs, ifName string) (OSConfigurator, error) { return &darwinConfigurator{logf: logf, ifName: ifName}, nil } diff --git a/net/dns/manager_default.go b/net/dns/manager_default.go index 838d0761c..11dea5ca8 100644 --- a/net/dns/manager_default.go +++ b/net/dns/manager_default.go @@ -6,10 +6,14 @@ package dns import ( + "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" ) -func NewOSConfigurator(logger.Logf, *health.Tracker, string) (OSConfigurator, error) { +// NewOSConfigurator creates a new OS configurator. +// +// The health tracker and the knobs may be nil and are ignored on this platform. +func NewOSConfigurator(logger.Logf, *health.Tracker, *controlknobs.Knobs, string) (OSConfigurator, error) { return NewNoopManager() } diff --git a/net/dns/manager_freebsd.go b/net/dns/manager_freebsd.go index ac4567a6f..1ec9ea841 100644 --- a/net/dns/manager_freebsd.go +++ b/net/dns/manager_freebsd.go @@ -7,11 +7,15 @@ "fmt" "os" + "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" ) -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ string) (OSConfigurator, error) { +// NewOSConfigurator creates a new OS configurator. +// +// The health tracker may be nil; the knobs may be nil and are ignored on this platform. +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ *controlknobs.Knobs, _ string) (OSConfigurator, error) { bs, err := os.ReadFile("/etc/resolv.conf") if os.IsNotExist(err) { return newDirectManager(logf, health), nil diff --git a/net/dns/manager_linux.go b/net/dns/manager_linux.go index 7ce0494ee..3ba3022b6 100644 --- a/net/dns/manager_linux.go +++ b/net/dns/manager_linux.go @@ -14,6 +14,7 @@ "time" "github.com/godbus/dbus/v5" + "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/net/netaddr" "tailscale.com/types/logger" @@ -31,7 +32,10 @@ func (kv kv) String() string { var publishOnce sync.Once -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, interfaceName string) (ret OSConfigurator, err error) { +// NewOSConfigurator created a new OS configurator. +// +// The health tracker may be nil; the knobs may be nil and are ignored on this platform. +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ *controlknobs.Knobs, interfaceName string) (ret OSConfigurator, err error) { env := newOSConfigEnv{ fs: directFS{}, dbusPing: dbusPing, diff --git a/net/dns/manager_openbsd.go b/net/dns/manager_openbsd.go index c55efa509..1a1c4390c 100644 --- a/net/dns/manager_openbsd.go +++ b/net/dns/manager_openbsd.go @@ -8,6 +8,7 @@ "fmt" "os" + "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" ) @@ -20,7 +21,10 @@ func (kv kv) String() string { return fmt.Sprintf("%s=%s", kv.k, kv.v) } -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, interfaceName string) (OSConfigurator, error) { +// NewOSConfigurator created a new OS configurator. +// +// The health tracker may be nil; the knobs may be nil and are ignored on this platform. +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) { return newOSConfigurator(logf, health, interfaceName, newOSConfigEnv{ rcIsResolvd: rcIsResolvd, diff --git a/net/dns/manager_windows.go b/net/dns/manager_windows.go index 56bdbbe06..e4d703257 100644 --- a/net/dns/manager_windows.go +++ b/net/dns/manager_windows.go @@ -22,6 +22,7 @@ "golang.org/x/sys/windows/registry" "golang.zx2c4.com/wireguard/windows/tunnel/winipcfg" "tailscale.com/atomicfile" + "tailscale.com/control/controlknobs" "tailscale.com/envknob" "tailscale.com/health" "tailscale.com/types/logger" @@ -38,6 +39,7 @@ type windowsManager struct { logf logger.Logf guid string + knobs *controlknobs.Knobs // or nil nrptDB *nrptRuleDatabase wslManager *wslManager @@ -45,10 +47,14 @@ type windowsManager struct { closing bool } -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, interfaceName string) (OSConfigurator, error) { +// NewOSConfigurator created a new OS configurator. +// +// The health tracker and the knobs may be nil. +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) { ret := &windowsManager{ logf: logf, guid: interfaceName, + knobs: knobs, wslManager: newWSLManager(logf, health), } @@ -288,6 +294,10 @@ func (m *windowsManager) setPrimaryDNS(resolvers []netip.Addr, domains []dnsname return nil } +func (m *windowsManager) disableLocalDNSOverrideViaNRPT() bool { + return m.knobs != nil && m.knobs.DisableLocalDNSOverrideViaNRPT.Load() +} + func (m *windowsManager) SetDNS(cfg OSConfig) error { // We can configure Windows DNS in one of two ways: // @@ -322,7 +332,17 @@ func (m *windowsManager) SetDNS(cfg OSConfig) error { } if len(cfg.MatchDomains) == 0 { - if err := m.setSplitDNS(nil, nil); err != nil { + var resolvers []netip.Addr + var domains []dnsname.FQDN + if !m.disableLocalDNSOverrideViaNRPT() { + // Create a default catch-all rule to make ourselves the actual primary resolver. + // Without this rule, Windows 8.1 and newer devices issue parallel DNS requests to DNS servers + // associated with all network adapters, even when "Override local DNS" is enabled and/or + // a Mullvad exit node is being used, resulting in DNS leaks. + resolvers = cfg.Nameservers + domains = []dnsname.FQDN{"."} + } + if err := m.setSplitDNS(resolvers, domains); err != nil { return err } if err := m.setHosts(nil); err != nil { @@ -331,8 +351,6 @@ func (m *windowsManager) SetDNS(cfg OSConfig) error { if err := m.setPrimaryDNS(cfg.Nameservers, cfg.SearchDomains); err != nil { return err } - } else if m.nrptDB == nil { - return errors.New("cannot set per-domain resolvers on Windows 7") } else { if err := m.setSplitDNS(cfg.Nameservers, cfg.MatchDomains); err != nil { return err diff --git a/net/dns/manager_windows_test.go b/net/dns/manager_windows_test.go index 917738ec0..183ab1177 100644 --- a/net/dns/manager_windows_test.go +++ b/net/dns/manager_windows_test.go @@ -84,7 +84,7 @@ func TestManagerWindowsGPCopy(t *testing.T) { } defer delIfKey() - cfg, err := NewOSConfigurator(logf, nil, fakeInterface.String()) + cfg, err := NewOSConfigurator(logf, nil, nil, fakeInterface.String()) if err != nil { t.Fatalf("NewOSConfigurator: %v\n", err) } @@ -213,7 +213,7 @@ func runTest(t *testing.T, isLocal bool) { } defer delIfKey() - cfg, err := NewOSConfigurator(logf, nil, fakeInterface.String()) + cfg, err := NewOSConfigurator(logf, nil, nil, fakeInterface.String()) if err != nil { t.Fatalf("NewOSConfigurator: %v\n", err) } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index be3abeb2e..4460eb374 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -139,7 +139,8 @@ // - 96: 2024-05-29: Client understands NodeAttrSSHBehaviorV1 // - 97: 2024-06-06: Client understands NodeAttrDisableSplitDNSWhenNoCustomResolvers // - 98: 2024-06-13: iOS/tvOS clients may provide serial number as part of posture information -const CurrentCapabilityVersion CapabilityVersion = 98 +// - 99: 2024-06-14: Client understands NodeAttrDisableLocalDNSOverrideViaNRPT +const CurrentCapabilityVersion CapabilityVersion = 99 type StableID string @@ -2306,6 +2307,15 @@ type Oauth2Token struct { // and this node attribute allows us to disable the optimization remotely // if needed. NodeAttrDisableSplitDNSWhenNoCustomResolvers NodeCapability = "disable-split-dns-when-no-custom-resolvers" + + // NodeAttrDisableLocalDNSOverrideViaNRPT indicates that the node's DNS manager should not + // create a default (catch-all) Windows NRPT rule when "Override local DNS" is enabled. + // Without this rule, Windows 8.1 and newer devices issue parallel DNS requests to DNS servers + // associated with all network adapters, even when "Override local DNS" is enabled and/or + // a Mullvad exit node is being used, resulting in DNS leaks. + // We began creating this rule on 2024-06-14, and this node attribute + // allows us to disable the new behavior remotely if needed. + NodeAttrDisableLocalDNSOverrideViaNRPT NodeCapability = "disable-local-dns-override-via-nrpt" ) // SetDNSRequest is a request to add a DNS record.