diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 4ab7ea03f..c985037ae 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -547,7 +547,7 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM } request := tailcfg.MapRequest{ - Version: 8, + Version: tailcfg.CurrentMapRequestVersion, KeepAlive: c.keepAlive, NodeKey: tailcfg.NodeKey(persist.PrivateNodeKey.Public()), DiscoKey: c.discoPubKey, diff --git a/ipn/local.go b/ipn/local.go index 5b781542c..60274ba11 100644 --- a/ipn/local.go +++ b/ipn/local.go @@ -639,7 +639,7 @@ func (b *LocalBackend) updateDNSMap(netMap *controlclient.NetworkMap) { } set(netMap.Name, netMap.Addresses) - dnsMap := tsdns.NewMap(nameToIP, domainsForProxying(netMap)) + dnsMap := tsdns.NewMap(nameToIP, magicDNSRootDomains(netMap)) // map diff will be logged in tsdns.Resolver.SetMap. b.e.SetDNSMap(dnsMap) } @@ -1209,20 +1209,14 @@ func (b *LocalBackend) authReconfig() { // If CorpDNS is false, rcfg.DNS remains the zero value. if uc.CorpDNS { - domains := nm.DNS.Domains proxied := nm.DNS.Proxied - if proxied { - if len(nm.DNS.Nameservers) == 0 { - b.logf("[unexpected] dns proxied but no nameservers") - proxied = false - } else { - // Domains for proxying should come first to avoid leaking queries. - domains = append(domainsForProxying(nm), domains...) - } + if proxied && len(nm.DNS.Nameservers) == 0 { + b.logf("[unexpected] dns proxied but no nameservers") + proxied = false } rcfg.DNS = dns.Config{ Nameservers: nm.DNS.Nameservers, - Domains: domains, + Domains: nm.DNS.Domains, PerDomain: nm.DNS.PerDomain, Proxied: proxied, } @@ -1235,32 +1229,31 @@ func (b *LocalBackend) authReconfig() { b.logf("[v1] authReconfig: ra=%v dns=%v 0x%02x: %v", uc.RouteAll, uc.CorpDNS, flags, err) } -// domainsForProxying produces a list of search domains for proxied DNS. -func domainsForProxying(nm *controlclient.NetworkMap) []string { - var domains []string - if idx := strings.IndexByte(nm.Name, '.'); idx != -1 { - domains = append(domains, nm.Name[idx+1:]) - } - for _, peer := range nm.Peers { - idx := strings.IndexByte(peer.Name, '.') - if idx == -1 { - continue +// magicDNSRootDomains returns the subset of nm.DNS.Domains that are the search domains for MagicDNS. +// Each entry has a trailing period. +func magicDNSRootDomains(nm *controlclient.NetworkMap) []string { + searchPathUsedAsDNSSuffix := func(suffix string) bool { + if tsdns.NameHasSuffix(nm.Name, suffix) { + return true } - domain := peer.Name[idx+1:] - seen := false - // In theory this makes the function O(n^2) worst case, - // but in practice we expect domains to contain very few elements - // (only one until invitations are introduced). - for _, seenDomain := range domains { - if domain == seenDomain { - seen = true + for _, p := range nm.Peers { + if tsdns.NameHasSuffix(p.Name, suffix) { + return true } } - if !seen { - domains = append(domains, domain) + return false + } + + var ret []string + for _, d := range nm.DNS.Domains { + if searchPathUsedAsDNSSuffix(d) { + if !strings.HasSuffix(d, ".") { + d += "." + } + ret = append(ret, d) } } - return domains + return ret } // routerConfig produces a router.Config from a wireguard config and IPN prefs. diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 1a198c667..f751b1519 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -22,6 +22,18 @@ "tailscale.com/types/structs" ) +// CurrentMapRequestVersion is the current MapRequest.Version value. +// +// History of versions: +// 3: implicit compression, keep-alives +// 4: opt-in keep-alives via KeepAlive field, opt-in compression via Compress +// 5: 2020-10-19, implies IncludeIPv6, delta Peers/UserProfiles, supports MagicDNS +// 6: 2020-12-07: means MapResponse.PacketFilter nil means unchanged +// 7: 2020-12-15: FilterRule.SrcIPs accepts CIDRs+ranges, doesn't warn about 0.0.0.0/:: +// 8: 2020-12-19: client can receive IPv6 addresses and routes if beta enabled server-side +// 9: 2020-12-30: client doesn't auto-add implicit search domains from peers; only DNSConfig.Domains +const CurrentMapRequestVersion = 9 + type ID int64 type UserID ID @@ -471,14 +483,9 @@ type MapRequest struct { // we want to signal to the control server that we're capable of something // different. // - // History of versions: - // 3: implicit compression, keep-alives - // 4: opt-in keep-alives via KeepAlive field, opt-in compression via Compress - // 5: 2020-10-19, implies IncludeIPv6, delta Peers/UserProfiles, supports MagicDNS - // 6: 2020-12-07: means MapResponse.PacketFilter nil means unchanged - // 7: 2020-12-15: FilterRule.SrcIPs accepts CIDRs+ranges, doesn't warn about 0.0.0.0/:: - // 8: 2020-12-19: client can receive IPv6 addresses and routes if beta enabled server-side - Version int + // For current values and history, see CurrentMapRequestVersion above. + Version int + Compress string // "zstd" or "" (no compression) KeepAlive bool // whether server should send keep-alives back to us NodeKey NodeKey @@ -627,11 +634,20 @@ type MapResponse struct { // // TODO(dmytro): should be sent in DNSConfig.Nameservers once clients have updated. DNS []netaddr.IP `json:",omitempty"` - // SearchPaths are the same as DNSConfig.Domains. + + // SearchPaths is the old way to specify DNS search + // domains. Clients should use these values if set, but the + // server will omit this field for clients with + // MapRequest.Version >= 9. Clients should prefer to use + // DNSConfig.Domains instead. + SearchPaths []string `json:",omitempty"` + + // DNSConfig contains the DNS settings for the client to use. // - // TODO(dmytro): should be sent in DNSConfig.Domains once clients have updated. - SearchPaths []string `json:",omitempty"` - DNSConfig DNSConfig `json:",omitempty"` + // TODO(bradfitz): make this a pointer and conditionally sent + // only if changed, like DERPMap, PacketFilter, etc. It's + // small, though. + DNSConfig DNSConfig `json:",omitempty"` // Domain is the name of the network that this node is // in. It's either of the form "example.com" (for user diff --git a/wgengine/tsdns/map.go b/wgengine/tsdns/map.go index c88eb3cb8..5cc774555 100644 --- a/wgengine/tsdns/map.go +++ b/wgengine/tsdns/map.go @@ -26,6 +26,10 @@ type Map struct { } // NewMap returns a new Map with name to address mapping given by nameToIP. +// +// rootDomains are the domains whose subdomains should always be +// resolved locally to prevent leakage of sensitive names. They should +// end in a period ("user-foo.tailscale.net."). func NewMap(initNameToIP map[string]netaddr.IP, rootDomains []string) *Map { // TODO(dmytro): we have to allocate names and ipToName, but nameToIP can be avoided. // It is here because control sends us names not in canonical form. Change this. diff --git a/wgengine/tsdns/tsdns.go b/wgengine/tsdns/tsdns.go index 9ca03f39e..b1ea86ee3 100644 --- a/wgengine/tsdns/tsdns.go +++ b/wgengine/tsdns/tsdns.go @@ -194,8 +194,8 @@ func (r *Resolver) Resolve(domain string, tp dns.Type) (netaddr.IP, dns.RCode, e } anyHasSuffix := false - for _, rootDomain := range dnsMap.rootDomains { - if strings.HasSuffix(domain, rootDomain) { + for _, suffix := range dnsMap.rootDomains { + if NameHasSuffix(domain, suffix) { anyHasSuffix = true break } @@ -611,3 +611,12 @@ func (r *Resolver) respond(query []byte) ([]byte, error) { return marshalResponse(resp) } + +// NameHasSuffix reports whether the provided DNS name ends with the +// component(s) in suffix, ignoring any trailing dots. +func NameHasSuffix(name, suffix string) bool { + name = strings.TrimSuffix(name, ".") + suffix = strings.TrimSuffix(suffix, ".") + nameBase := strings.TrimSuffix(name, suffix) + return len(nameBase) < len(name) && strings.HasSuffix(nameBase, ".") +} diff --git a/wgengine/tsdns/tsdns_test.go b/wgengine/tsdns/tsdns_test.go index 2df2e35d7..5b3f83c07 100644 --- a/wgengine/tsdns/tsdns_test.go +++ b/wgengine/tsdns/tsdns_test.go @@ -758,3 +758,24 @@ func TestMarshalResponseFormatError(t *testing.T) { } t.Logf("response: %q", v) } + +func TestNameHasSuffix(t *testing.T) { + tests := []struct { + name, suffix string + want bool + }{ + {"foo.com", "com", true}, + {"foo.com.", "com", true}, + {"foo.com.", "com.", true}, + + {"", "", false}, + {"foo.com.", "", false}, + {"foo.com.", "o.com", false}, + } + for _, tt := range tests { + got := NameHasSuffix(tt.name, tt.suffix) + if got != tt.want { + t.Errorf("NameHasSuffix(%q, %q) = %v; want %v", tt.name, tt.suffix, got, tt.want) + } + } +}