tailcfg,net/dns: add controlknob to disable battery split DNS on iOS (#12346)

Updates corp#15802. Adds the ability for control to disable the recently added change that uses split DNS in more cases on iOS. This will allow us to disable the feature if it leads to regression in production. We plan to remove this knob once we've verified that the feature works properly. Signed-off-by: Andrea Gottardo <andrea@gottardo.me>
2024-11-25 19:15:34 +00:00 · 2024-06-06 15:19:33 -07:00 · 2024-06-06 15:19:33 -07:00 · b65221999c
commit b65221999c
parent e88a5dbc92
3 changed files with 68 additions and 34 deletions
--- a/control/controlknobs/controlknobs.go
+++ b/control/controlknobs/controlknobs.go
@ -81,6 +81,15 @@ type Knobs struct {
 	// how to dial the destination address. When true, it also makes the DNS forwarder
 	// use UserDial instead of SystemDial when dialing resolvers.
 	UserDialUseRoutes atomic.Bool
+
+	// DisableSplitDNSWhenNoCustomResolvers indicates that the node's DNS manager
+	// should not adopt a split DNS configuration even though the Config of the
+	// resolver only contains routes that do not specify custom resolver(s), hence
+	// all DNS queries can be safely sent to the upstream DNS resolver and the
+	// node's DNS forwarder doesn't need to handle all DNS traffic.
+	// 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
 }

 // UpdateFromNodeAttributes updates k (if non-nil) based on the provided self
@ -91,22 +100,23 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) {
 	}
 	has := capMap.Contains
 	var (
-		keepFullWG                    = has(tailcfg.NodeAttrDebugDisableWGTrim)
-		disableDRPO                   = has(tailcfg.NodeAttrDebugDisableDRPO)
-		disableUPnP                   = has(tailcfg.NodeAttrDisableUPnP)
-		randomizeClientPort           = has(tailcfg.NodeAttrRandomizeClientPort)
-		disableDeltaUpdates           = has(tailcfg.NodeAttrDisableDeltaUpdates)
-		oneCGNAT                      opt.Bool
-		forceBackgroundSTUN           = has(tailcfg.NodeAttrDebugForceBackgroundSTUN)
-		peerMTUEnable                 = has(tailcfg.NodeAttrPeerMTUEnable)
-		dnsForwarderDisableTCPRetries = has(tailcfg.NodeAttrDNSForwarderDisableTCPRetries)
-		silentDisco                   = has(tailcfg.NodeAttrSilentDisco)
-		forceIPTables                 = has(tailcfg.NodeAttrLinuxMustUseIPTables)
-		forceNfTables                 = has(tailcfg.NodeAttrLinuxMustUseNfTables)
-		seamlessKeyRenewal            = has(tailcfg.NodeAttrSeamlessKeyRenewal)
-		probeUDPLifetime              = has(tailcfg.NodeAttrProbeUDPLifetime)
-		appCStoreRoutes               = has(tailcfg.NodeAttrStoreAppCRoutes)
-		userDialUseRoutes             = has(tailcfg.NodeAttrUserDialUseRoutes)
+		keepFullWG                           = has(tailcfg.NodeAttrDebugDisableWGTrim)
+		disableDRPO                          = has(tailcfg.NodeAttrDebugDisableDRPO)
+		disableUPnP                          = has(tailcfg.NodeAttrDisableUPnP)
+		randomizeClientPort                  = has(tailcfg.NodeAttrRandomizeClientPort)
+		disableDeltaUpdates                  = has(tailcfg.NodeAttrDisableDeltaUpdates)
+		oneCGNAT                             opt.Bool
+		forceBackgroundSTUN                  = has(tailcfg.NodeAttrDebugForceBackgroundSTUN)
+		peerMTUEnable                        = has(tailcfg.NodeAttrPeerMTUEnable)
+		dnsForwarderDisableTCPRetries        = has(tailcfg.NodeAttrDNSForwarderDisableTCPRetries)
+		silentDisco                          = has(tailcfg.NodeAttrSilentDisco)
+		forceIPTables                        = has(tailcfg.NodeAttrLinuxMustUseIPTables)
+		forceNfTables                        = has(tailcfg.NodeAttrLinuxMustUseNfTables)
+		seamlessKeyRenewal                   = has(tailcfg.NodeAttrSeamlessKeyRenewal)
+		probeUDPLifetime                     = has(tailcfg.NodeAttrProbeUDPLifetime)
+		appCStoreRoutes                      = has(tailcfg.NodeAttrStoreAppCRoutes)
+		userDialUseRoutes                    = has(tailcfg.NodeAttrUserDialUseRoutes)
+		disableSplitDNSWhenNoCustomResolvers = has(tailcfg.NodeAttrDisableSplitDNSWhenNoCustomResolvers)
 	)

 	if has(tailcfg.NodeAttrOneCGNATEnable) {
@ -131,6 +141,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) {
 	k.ProbeUDPLifetime.Store(probeUDPLifetime)
 	k.AppCStoreRoutes.Store(appCStoreRoutes)
 	k.UserDialUseRoutes.Store(userDialUseRoutes)
+	k.DisableSplitDNSWhenNoCustomResolvers.Store(disableSplitDNSWhenNoCustomResolvers)
 }

 // AsDebugJSON returns k as something that can be marshalled with json.Marshal
@ -140,21 +151,22 @@ func (k *Knobs) AsDebugJSON() map[string]any {
 		return nil
 	}
 	return map[string]any{
-		"DisableUPnP":                   k.DisableUPnP.Load(),
-		"DisableDRPO":                   k.DisableDRPO.Load(),
-		"KeepFullWGConfig":              k.KeepFullWGConfig.Load(),
-		"RandomizeClientPort":           k.RandomizeClientPort.Load(),
-		"OneCGNAT":                      k.OneCGNAT.Load(),
-		"ForceBackgroundSTUN":           k.ForceBackgroundSTUN.Load(),
-		"DisableDeltaUpdates":           k.DisableDeltaUpdates.Load(),
-		"PeerMTUEnable":                 k.PeerMTUEnable.Load(),
-		"DisableDNSForwarderTCPRetries": k.DisableDNSForwarderTCPRetries.Load(),
-		"SilentDisco":                   k.SilentDisco.Load(),
-		"LinuxForceIPTables":            k.LinuxForceIPTables.Load(),
-		"LinuxForceNfTables":            k.LinuxForceNfTables.Load(),
-		"SeamlessKeyRenewal":            k.SeamlessKeyRenewal.Load(),
-		"ProbeUDPLifetime":              k.ProbeUDPLifetime.Load(),
-		"AppCStoreRoutes":               k.AppCStoreRoutes.Load(),
-		"UserDialUseRoutes":             k.UserDialUseRoutes.Load(),
+		"DisableUPnP":                          k.DisableUPnP.Load(),
+		"DisableDRPO":                          k.DisableDRPO.Load(),
+		"KeepFullWGConfig":                     k.KeepFullWGConfig.Load(),
+		"RandomizeClientPort":                  k.RandomizeClientPort.Load(),
+		"OneCGNAT":                             k.OneCGNAT.Load(),
+		"ForceBackgroundSTUN":                  k.ForceBackgroundSTUN.Load(),
+		"DisableDeltaUpdates":                  k.DisableDeltaUpdates.Load(),
+		"PeerMTUEnable":                        k.PeerMTUEnable.Load(),
+		"DisableDNSForwarderTCPRetries":        k.DisableDNSForwarderTCPRetries.Load(),
+		"SilentDisco":                          k.SilentDisco.Load(),
+		"LinuxForceIPTables":                   k.LinuxForceIPTables.Load(),
+		"LinuxForceNfTables":                   k.LinuxForceNfTables.Load(),
+		"SeamlessKeyRenewal":                   k.SeamlessKeyRenewal.Load(),
+		"ProbeUDPLifetime":                     k.ProbeUDPLifetime.Load(),
+		"AppCStoreRoutes":                      k.AppCStoreRoutes.Load(),
+		"UserDialUseRoutes":                    k.UserDialUseRoutes.Load(),
+		"DisableSplitDNSWhenNoCustomResolvers": k.DisableSplitDNSWhenNoCustomResolvers.Load(),
 	}
 }
--- a/net/dns/manager.go
+++ b/net/dns/manager.go
@ -52,6 +52,7 @@ type Manager struct {

 	resolver *resolver.Resolver
 	os       OSConfigurator
+	knobs    *controlknobs.Knobs
 	goos     string // if empty, gets set to runtime.GOOS
 }

@ -67,11 +68,13 @@ func NewManager(logf logger.Logf, oscfg OSConfigurator, health *health.Tracker,
 	if goos == "" {
 		goos = runtime.GOOS
 	}
+
 	m := &Manager{
 		logf:     logf,
 		resolver: resolver.New(logf, linkSel, dialer, knobs),
 		os:       oscfg,
 		health:   health,
+		knobs:    knobs,
 		goos:     goos,
 	}
 	m.ctx, m.ctxCancel = context.WithCancel(context.Background())
@ -273,8 +276,12 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig
 		// a query for 'work-laptop' might lead to search domain expansion, resolving
 		// as 'work-laptop.aws.com' for example.
 		if m.goos == "ios" && rcfg.RoutesRequireNoCustomResolvers() {
-			for r := range rcfg.Routes {
-				ocfg.MatchDomains = append(ocfg.MatchDomains, r)
+			if !m.disableSplitDNSOptimization() {
+				for r := range rcfg.Routes {
+					ocfg.MatchDomains = append(ocfg.MatchDomains, r)
+				}
+			} else {
+				m.logf("iOS split DNS is disabled by nodeattr")
 			}
 		}
 		var defaultRoutes []*dnstype.Resolver
@ -288,6 +295,10 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig
 	return rcfg, ocfg, nil
 }

+func (m *Manager) disableSplitDNSOptimization() bool {
+	return m.knobs.DisableSplitDNSWhenNoCustomResolvers.Load()
+}
+
 // 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.
--- a/tailcfg/tailcfg.go
+++ b/tailcfg/tailcfg.go
@ -2286,6 +2286,17 @@ type Oauth2Token struct {
 	// NodeAttrSSHBehaviorV1 forces SSH to use the V1 behavior (no su, run SFTP in-process)
 	// Added 2024-05-29 in Tailscale version 1.68.
 	NodeAttrSSHBehaviorV1 NodeCapability = "ssh-behavior-v1"
+
+	// NodeAttrDisableSplitDNSWhenNoCustomResolvers indicates that the node's
+	// DNS manager should not adopt a split DNS configuration even though the
+	// Config of the resolver only contains routes that do not specify custom
+	// resolver(s), hence all DNS queries can be safely sent to the upstream
+	// DNS resolver and the node's DNS forwarder doesn't need to handle all
+	// DNS traffic.
+	// This is for now (2024-06-06) an iOS-specific battery life optimization,
+	// and this node attribute allows us to disable the optimization remotely
+	// if needed.
+	NodeAttrDisableSplitDNSWhenNoCustomResolvers NodeCapability = "disable-split-dns-when-no-custom-resolvers"
 )

 // SetDNSRequest is a request to add a DNS record.