From b65221999ca2644e917efce394dc0c9603cfacb7 Mon Sep 17 00:00:00 2001 From: Andrea Gottardo Date: Thu, 6 Jun 2024 15:19:33 -0700 Subject: [PATCH] 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 --- control/controlknobs/controlknobs.go | 76 ++++++++++++++++------------ net/dns/manager.go | 15 +++++- tailcfg/tailcfg.go | 11 ++++ 3 files changed, 68 insertions(+), 34 deletions(-) diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index 0b86c2d3b..f4cef7b41 100644 --- 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(), } } diff --git a/net/dns/manager.go b/net/dns/manager.go index cdbc676eb..61c6dfecc 100644 --- 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. diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index f9cf9f8fc..c5a517cba 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -2286,6 +2286,17 @@ const ( // 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.