diff --git a/net/dns/manager.go b/net/dns/manager.go index 6c574659b..08e671753 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -33,6 +33,7 @@ import ( "tailscale.com/util/eventbus" "tailscale.com/util/slicesx" "tailscale.com/util/syspolicy/policyclient" + "tailscale.com/version" ) var ( @@ -376,8 +377,11 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig // This bool is used in a couple of places below to implement this // workaround. isWindows := m.goos == "windows" - isApple := (m.goos == "darwin" || m.goos == "ios") - if m.os.SupportsSplitDNS() && !isWindows && !isApple { + isIOS := m.goos == "ios" + // Sandboxed macOS builds use NetworkExtension DNS settings, not + // tailscaled's /etc/resolver configurator, so keep the Apple workaround. + appleSplitDNSWorkaround := isIOS || (m.goos == "darwin" && isSandboxedMacOS()) + if m.os.SupportsSplitDNS() && !isWindows && !appleSplitDNSWorkaround { if srs := toIPsOnly(cfg.singleResolverSet()); len(srs) > 0 { // Split DNS configuration requested, where all split domains // go to the same resolvers. We can let the OS do it. @@ -395,11 +399,11 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig var baseCfg *OSConfig // base config; non-nil if/when known - // Even though Apple devices can do split DNS, they don't provide a way to + // Even though iOS devices can do split DNS, they don't provide a way to // selectively answer ExtraRecords, and ignore other DNS traffic. As a // workaround, we read the existing default resolver configuration and use // that as the forwarder for all DNS traffic that quad-100 doesn't handle. - if isApple || !m.os.SupportsSplitDNS() { + if appleSplitDNSWorkaround || !m.os.SupportsSplitDNS() { // If the OS can't do native split-dns, read out the underlying // resolver config and blend it into our config. On apple platforms, [OSConfigurator.GetBaseConfig] // has a tendency to temporarily fail if called immediately following @@ -408,9 +412,9 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig cfg, err := m.os.GetBaseConfig() if err == nil { baseCfg = &cfg - } else if (isApple || isNoopManager(m.os)) && err == ErrGetBaseConfigNotSupported { + } else if (isIOS || isNoopManager(m.os)) && err == ErrGetBaseConfigNotSupported { // Expected when using noopManager (userspace networking) or on - // certain iOS/macOS builds. Continue without base config. + // certain iOS builds. Continue without base config. } else { m.health.SetUnhealthy(osConfigurationReadWarnable, health.Args{health.ArgError: err.Error()}) return resolver.Config{}, OSConfig{}, err @@ -460,6 +464,8 @@ func (m *Manager) disableSplitDNSOptimization() bool { return m.knobs != nil && m.knobs.DisableSplitDNSWhenNoCustomResolvers.Load() } +var isSandboxedMacOS = version.IsSandboxedMacOS + // toIPsOnly returns only the IP portion of dnstype.Resolver. // Only safe to use if the resolvers slice has been cleared of // DoH or custom-port entries with something like hasDefaultIPResolversOnly. diff --git a/net/dns/manager_darwin.go b/net/dns/manager_darwin.go index 90686b246..a93a131f6 100644 --- a/net/dns/manager_darwin.go +++ b/net/dns/manager_darwin.go @@ -8,6 +8,8 @@ import ( "fmt" "io/fs" "os" + "os/exec" + "strconv" "strings" "go4.org/mem" @@ -30,6 +32,7 @@ func NewOSConfigurator(logf logger.Logf, _ *health.Tracker, _ *eventbus.Bus, _ p ifName: ifName, resolverDir: "/etc/resolver", resolvConfPath: "/etc/resolv.conf", + runScutil: runScutil, }, nil } @@ -41,11 +44,14 @@ type darwinConfigurator struct { ifName string resolverDir string // default "/etc/resolver" resolvConfPath string // default "/etc/resolv.conf" + runScutil func(script string) (output string, err error) } func (c *darwinConfigurator) Close() error { - c.removeResolverFiles(func(domain string) bool { return true }) - return nil + if err := c.removeGlobalDNS(); err != nil { + return err + } + return c.removeResolverFiles(func(domain string) bool { return true }) } func (c *darwinConfigurator) SupportsSplitDNS() bool { @@ -53,6 +59,16 @@ func (c *darwinConfigurator) SupportsSplitDNS() bool { } func (c *darwinConfigurator) SetDNS(cfg OSConfig) error { + if len(cfg.Nameservers) > 0 && len(cfg.MatchDomains) == 0 { + if err := c.setGlobalDNS(cfg); err != nil { + return err + } + return c.removeResolverFiles(func(domain string) bool { return true }) + } + if err := c.removeGlobalDNS(); err != nil { + return err + } + var buf bytes.Buffer buf.WriteString(macResolverFileHeader) for _, ip := range cfg.Nameservers { @@ -107,6 +123,89 @@ func (c *darwinConfigurator) SetDNS(cfg OSConfig) error { return c.removeResolverFiles(func(domain string) bool { return !keep[domain] }) } +// macOSGlobalDNSKey is a stable synthetic SystemConfiguration service key for +// tailscaled's global DNS resolver. The UUID does not identify a real network +// service; it just gives tailscaled one well-known dynamic-store location to +// set and later remove. +const macOSGlobalDNSKey = "State:/Network/Service/FF457792-79C0-4A25-8392-D875BBEACCA6/DNS" + +// setGlobalDNS installs cfg.Nameservers as a default resolver using +// SystemConfiguration's dynamic store. /etc/resolver is only a split-DNS +// mechanism; it cannot express a primary resolver. +func (c *darwinConfigurator) setGlobalDNS(cfg OSConfig) error { + var script strings.Builder + script.WriteString("d.init\n") + script.WriteString("d.add SearchOrder # 100000\n") + script.WriteString("d.add ServerAddresses *") + for _, ip := range cfg.Nameservers { + script.WriteByte(' ') + script.WriteString(ip.String()) + } + script.WriteByte('\n') + // An empty SupplementalMatchDomains entry makes this a default resolver. + script.WriteString("d.add SupplementalMatchDomains * \"\"\n") + if len(cfg.SearchDomains) > 0 { + script.WriteString("d.add SearchDomains *") + for _, fqdn := range cfg.SearchDomains { + script.WriteByte(' ') + writeScutilString(&script, fqdn.WithoutTrailingDot()) + } + script.WriteByte('\n') + } + script.WriteString("set ") + script.WriteString(macOSGlobalDNSKey) + script.WriteString("\nquit\n") + + out, err := c.runScutilScript(script.String()) + if err != nil { + return err + } + out = strings.TrimSpace(out) + if out != "" { + return fmt.Errorf("scutil set %s: %s", macOSGlobalDNSKey, out) + } + return nil +} + +func (c *darwinConfigurator) removeGlobalDNS() error { + script := "remove " + macOSGlobalDNSKey + "\nquit\n" + out, err := c.runScutilScript(script) + if err != nil { + return err + } + out = strings.TrimSpace(out) + if out == "" || out == "No such key" { + return nil + } + return fmt.Errorf("scutil remove %s: %s", macOSGlobalDNSKey, out) +} + +func (c *darwinConfigurator) runScutilScript(script string) (string, error) { + run := c.runScutil + if run == nil { + run = runScutil + } + return run(script) +} + +func runScutil(script string) (string, error) { + cmd := exec.Command("/usr/sbin/scutil") + cmd.Stdin = strings.NewReader(script) + out, err := cmd.CombinedOutput() + if err != nil { + return "", fmt.Errorf("scutil: %w: %s", err, strings.TrimSpace(string(out))) + } + return string(out), nil +} + +func writeScutilString(b *strings.Builder, v string) { + if v == "" || strings.ContainsAny(v, " \t\n\"") { + b.WriteString(strconv.Quote(v)) + return + } + b.WriteString(v) +} + func isValidResolverFileName(name string) bool { // Verify that the filename doesn't contain any characters that // might cause issues when used as a filename; os.Root is a diff --git a/net/dns/manager_darwin_test.go b/net/dns/manager_darwin_test.go index 8596f9575..98af98002 100644 --- a/net/dns/manager_darwin_test.go +++ b/net/dns/manager_darwin_test.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "slices" + "strings" "testing" "tailscale.com/types/logger" @@ -35,6 +36,7 @@ func newTestConfigurator(t *testing.T) *darwinConfigurator { ifName: "utun99", resolverDir: resolverDir, resolvConfPath: resolvConf, + runScutil: func(string) (string, error) { return "No such key\n", nil }, } } @@ -111,6 +113,110 @@ func TestSetDNS(t *testing.T) { } } +func TestSetDNSGlobal(t *testing.T) { + c := newTestConfigurator(t) + + // Start with split DNS files present, so this test verifies that switching + // to a global resolver cleans up stale /etc/resolver state. + if err := c.SetDNS(OSConfig{ + Nameservers: []netip.Addr{netip.MustParseAddr("100.100.100.100")}, + SearchDomains: []dnsname.FQDN{"tail1234.ts.net."}, + MatchDomains: []dnsname.FQDN{"ts.net."}, + }); err != nil { + t.Fatalf("setting initial split DNS config failed: %v", err) + } + unmanaged := filepath.Join(c.resolverDir, "other.conf") + if err := os.WriteFile(unmanaged, []byte("# not ours\nnameserver 8.8.8.8\n"), 0644); err != nil { + t.Fatal(err) + } + + var gotScripts []string + c.runScutil = func(script string) (string, error) { + gotScripts = append(gotScripts, script) + return "", nil + } + + cfg := OSConfig{ + Nameservers: []netip.Addr{ + netip.MustParseAddr("100.100.100.100"), + netip.MustParseAddr("fd7a:115c:a1e0::53"), + }, + SearchDomains: []dnsname.FQDN{"tail1234.ts.net."}, + } + if err := c.SetDNS(cfg); err != nil { + t.Fatalf("SetDNS failed: %v", err) + } + + wantScript := strings.Join([]string{ + "d.init", + "d.add SearchOrder # 100000", + "d.add ServerAddresses * 100.100.100.100 fd7a:115c:a1e0::53", + `d.add SupplementalMatchDomains * ""`, + "d.add SearchDomains * tail1234.ts.net", + "set " + macOSGlobalDNSKey, + "quit", + "", + }, "\n") + if !slices.Equal(gotScripts, []string{wantScript}) { + t.Errorf("scutil scripts mismatch:\ngot:\n%s\nwant:\n%s", strings.Join(gotScripts, "\n---\n"), wantScript) + } + + files, err := os.ReadDir(c.resolverDir) + if err != nil { + t.Fatalf("reading resolver directory: %v", err) + } + var fileNames []string + for _, f := range files { + fileNames = append(fileNames, f.Name()) + } + if !slices.Equal(fileNames, []string{"other.conf"}) { + t.Fatalf("expected only unmanaged resolver file after global DNS config, got %v", fileNames) + } +} + +func TestSetDNSSplitRemovesGlobal(t *testing.T) { + c := newTestConfigurator(t) + + // Start with global DNS configured, so this test verifies the stale global + // dynamic-store key is removed when switching back to split DNS. + c.runScutil = func(script string) (string, error) { + return "", nil + } + if err := c.SetDNS(OSConfig{ + Nameservers: []netip.Addr{netip.MustParseAddr("100.100.100.100")}, + }); err != nil { + t.Fatalf("setting initial global DNS config failed: %v", err) + } + + var gotScripts []string + c.runScutil = func(script string) (string, error) { + gotScripts = append(gotScripts, script) + return "", nil + } + + cfg := OSConfig{ + Nameservers: []netip.Addr{netip.MustParseAddr("100.100.100.100")}, + MatchDomains: []dnsname.FQDN{"ts.net."}, + } + if err := c.SetDNS(cfg); err != nil { + t.Fatalf("SetDNS failed: %v", err) + } + + wantScript := "remove " + macOSGlobalDNSKey + "\nquit\n" + if !slices.Equal(gotScripts, []string{wantScript}) { + t.Errorf("scutil scripts mismatch:\ngot:\n%s\nwant:\n%s", strings.Join(gotScripts, "\n---\n"), wantScript) + } + + const wantFile = macResolverFileHeader + "nameserver 100.100.100.100\n" + gotFile, err := os.ReadFile(filepath.Join(c.resolverDir, "ts.net")) + if err != nil { + t.Fatalf("reading split resolver file: %v", err) + } + if string(gotFile) != wantFile { + t.Errorf("split resolver file contents mismatch:\ngot: %q\nwant: %q", string(gotFile), wantFile) + } +} + func TestSetDNS_PathTraversal(t *testing.T) { c := newTestConfigurator(t) diff --git a/net/dns/manager_test.go b/net/dns/manager_test.go index 8797fb68f..b10c89056 100644 --- a/net/dns/manager_test.go +++ b/net/dns/manager_test.go @@ -187,14 +187,15 @@ func TestManager(t *testing.T) { // reasonable to make this unsupported as well, in which case // these tests will need tweaking. tests := []struct { - name string - in Config - split bool - bs OSConfig - os OSConfig - knobs *controlknobs.Knobs - rs resolver.Config - goos string // empty means "linux" + name string + in Config + split bool + bs OSConfig + os OSConfig + knobs *controlknobs.Knobs + rs resolver.Config + goos string // empty means "linux" + sandboxedMacOS bool }{ { name: "empty", @@ -450,6 +451,32 @@ func TestManager(t *testing.T) { MatchDomains: fqdns("corp.com"), }, }, + { + // Sandboxed macOS app builds use NetworkExtension DNS settings, not + // tailscaled's /etc/resolver configurator, so they keep the older + // Apple base-config behavior. + name: "routes-split-sandboxed-darwin", + in: Config{ + Routes: upstreams("corp.com", "2.2.2.2"), + SearchDomains: fqdns("tailscale.com", "universe.tf"), + }, + split: true, + bs: OSConfig{ + Nameservers: mustIPs("8.8.8.8"), + SearchDomains: fqdns("coffee.shop"), + }, + os: OSConfig{ + Nameservers: serviceAddr46, + SearchDomains: fqdns("tailscale.com", "universe.tf", "coffee.shop"), + }, + rs: resolver.Config{ + Routes: upstreams( + ".", "8.8.8.8", + "corp.com.", "2.2.2.2"), + }, + goos: "darwin", + sandboxedMacOS: true, + }, { name: "routes-multi", in: Config{ @@ -495,25 +522,23 @@ func TestManager(t *testing.T) { goos: "linux", }, { - // The `routes-multi-split-linux` test case above on Darwin should NOT result in a split - // DNS configuration. - // Check that MatchDomains is empty. Due to Apple limitations, we cannot set MatchDomains - // without those domains also being SearchDomains. - name: "routes-multi-does-not-split-on-darwin", + // The `routes-multi-split-linux` test case above should match on + // macOS, where tailscaled configures split DNS via /etc/resolver. + name: "routes-multi-split-darwin", in: Config{ Routes: upstreams( "corp.com", "2.2.2.2", "bigco.net", "3.3.3.3"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, - split: false, + split: true, os: OSConfig{ Nameservers: serviceAddr46, SearchDomains: fqdns("tailscale.com", "universe.tf"), + MatchDomains: fqdns("bigco.net", "corp.com"), }, rs: resolver.Config{ Routes: upstreams( - ".", "", "corp.com.", "2.2.2.2", "bigco.net.", "3.3.3.3"), }, @@ -593,10 +618,9 @@ func TestManager(t *testing.T) { goos: "linux", }, { - // The `magic-split` test case above on Darwin should NOT result in a split DNS configuration. - // Check that MatchDomains is empty. Due to Apple limitations, we cannot set MatchDomains - // without those domains also being SearchDomains. - name: "magic-split-does-not-split-on-darwin", + // The `magic-split` test case above should match on macOS, where + // tailscaled configures split DNS via /etc/resolver. + name: "magic-split-darwin", in: Config{ Hosts: hosts( "dave.ts.com.", "1.2.3.4", @@ -604,13 +628,13 @@ func TestManager(t *testing.T) { Routes: upstreams("ts.com", ""), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, - split: false, + split: true, os: OSConfig{ Nameservers: serviceAddr46, SearchDomains: fqdns("tailscale.com", "universe.tf"), + MatchDomains: fqdns("ts.com"), }, rs: resolver.Config{ - Routes: upstreams(".", ""), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), @@ -698,11 +722,9 @@ func TestManager(t *testing.T) { goos: "linux", }, { - // The `routes-magic-split-linux` test case above on Darwin should NOT result in a - // split DNS configuration. - // Check that MatchDomains is empty. Due to Apple limitations, we cannot set MatchDomains - // without those domains also being SearchDomains. - name: "routes-magic-does-not-split-on-darwin", + // The `routes-magic-split-linux` test case above should match on + // macOS, where tailscaled configures split DNS via /etc/resolver. + name: "routes-magic-split-darwin", in: Config{ Routes: upstreams( "corp.com", "2.2.2.2", @@ -716,12 +738,10 @@ func TestManager(t *testing.T) { os: OSConfig{ Nameservers: serviceAddr46, SearchDomains: fqdns("tailscale.com", "universe.tf"), + MatchDomains: fqdns("corp.com", "ts.com"), }, rs: resolver.Config{ - Routes: upstreams( - ".", "", - "corp.com.", "2.2.2.2", - ), + Routes: upstreams("corp.com.", "2.2.2.2"), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), @@ -866,9 +886,9 @@ func TestManager(t *testing.T) { goos: "ios", }, { - // on darwin, verify that with the same config as in ios-use-split-dns-when-no-custom-resolvers, - // MatchDomains are NOT set. - name: "darwin-dont-use-split-dns-when-no-custom-resolvers", + // macOS should match Linux here. iOS remains special-cased above + // for battery-life behavior. + name: "darwin-use-split-dns-when-no-custom-resolvers", in: Config{ Routes: upstreams("ts.net", "199.247.155.52", "optimistic-display.ts.net", ""), SearchDomains: fqdns("optimistic-display.ts.net"), @@ -877,12 +897,10 @@ func TestManager(t *testing.T) { os: OSConfig{ Nameservers: serviceAddr46, SearchDomains: fqdns("optimistic-display.ts.net"), + MatchDomains: fqdns("optimistic-display.ts.net", "ts.net"), }, rs: resolver.Config{ - Routes: upstreams( - ".", "", - "ts.net", "199.247.155.52", - ), + Routes: upstreams("ts.net", "199.247.155.52"), LocalDomains: fqdns("optimistic-display.ts.net."), }, goos: "darwin", @@ -986,6 +1004,7 @@ func TestManager(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + tstest.Replace(t, &isSandboxedMacOS, func() bool { return test.sandboxedMacOS }) f := fakeOSConfigurator{ SplitDNS: test.split, BaseConfig: test.bs,