From a903d6c2ed4d068cf2d212d2541cddeeaa1aca43 Mon Sep 17 00:00:00 2001 From: Dmytro Shynkevych Date: Mon, 24 Aug 2020 17:27:21 -0400 Subject: [PATCH] tailcfg, tsdns: derive root domains from list of nodes (#708) Signed-off-by: Dmytro Shynkevych --- ipn/local.go | 2 +- tailcfg/tailcfg.go | 14 ++++++++++--- wgengine/router/dns/config.go | 1 + wgengine/tsdns/map.go | 14 +++++++++---- wgengine/tsdns/map_test.go | 28 +++++++++++++------------- wgengine/tsdns/tsdns.go | 38 +++++++++++++++++++++-------------- wgengine/tsdns/tsdns_test.go | 27 ++++++++++++++----------- wgengine/userspace.go | 8 ++------ 8 files changed, 77 insertions(+), 55 deletions(-) diff --git a/ipn/local.go b/ipn/local.go index a0e2cd49f..85043c946 100644 --- a/ipn/local.go +++ b/ipn/local.go @@ -550,7 +550,7 @@ func (b *LocalBackend) updateDNSMap(netMap *controlclient.NetworkMap) { } set(netMap.Name, netMap.Addresses) - dnsMap := tsdns.NewMap(nameToIP) + dnsMap := tsdns.NewMap(nameToIP, domainsForProxying(netMap)) // map diff will be logged in tsdns.Resolver.SetMap. b.e.SetDNSMap(dnsMap) } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index fc6ce0b0b..8bab4b210 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -496,10 +496,18 @@ var FilterAllowAll = []FilterRule{ // DNSConfig is the DNS configuration. type DNSConfig struct { + // Nameservers are the IP addresses of the nameservers to use. Nameservers []netaddr.IP `json:",omitempty"` - Domains []string `json:",omitempty"` - PerDomain bool - Proxied bool + // Domains are the search domains to use. + Domains []string `json:",omitempty"` + // PerDomain indicates whether it is preferred to use Nameservers + // only for DNS queries for subdomains of Domains. + // Some OSes and OS configurations don't support per-domain DNS configuration, + // in which case Nameservers applies to all DNS requests regardless of PerDomain's value. + PerDomain bool + // Proxied indicates whether DNS requests are proxied through a tsdns.Resolver. + // This enables Magic DNS. It is togglable independently of PerDomain. + Proxied bool } type MapResponse struct { diff --git a/wgengine/router/dns/config.go b/wgengine/router/dns/config.go index fec5d8ffa..64d3a2972 100644 --- a/wgengine/router/dns/config.go +++ b/wgengine/router/dns/config.go @@ -23,6 +23,7 @@ type Config struct { // if the manager does not support per-domain settings. PerDomain bool // Proxied indicates whether DNS requests are proxied through a tsdns.Resolver. + // This enables Magic DNS. Proxied bool } diff --git a/wgengine/tsdns/map.go b/wgengine/tsdns/map.go index 65c77175b..c88eb3cb8 100644 --- a/wgengine/tsdns/map.go +++ b/wgengine/tsdns/map.go @@ -5,7 +5,6 @@ package tsdns import ( - "fmt" "sort" "strings" @@ -21,10 +20,13 @@ type Map struct { ipToName map[netaddr.IP]string // names are the keys of nameToIP in sorted order. names []string + // rootDomains are the domains whose subdomains should always + // be resolved locally to prevent leakage of sensitive names. + rootDomains []string // e.g. "user.provider.beta.tailscale.net." } // NewMap returns a new Map with name to address mapping given by nameToIP. -func NewMap(initNameToIP map[string]netaddr.IP) *Map { +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. names := make([]string, 0, len(initNameToIP)) @@ -49,12 +51,16 @@ func NewMap(initNameToIP map[string]netaddr.IP) *Map { nameToIP: nameToIP, ipToName: ipToName, names: names, + + rootDomains: rootDomains, } } func printSingleNameIP(buf *strings.Builder, name string, ip netaddr.IP) { - // Output width is exactly 80 columns. - fmt.Fprintf(buf, "%s\t%s\n", name, ip) + buf.WriteString(name) + buf.WriteByte('\t') + buf.WriteString(ip.String()) + buf.WriteByte('\n') } func (m *Map) Pretty() string { diff --git a/wgengine/tsdns/map_test.go b/wgengine/tsdns/map_test.go index d1e79b663..5d01f8d77 100644 --- a/wgengine/tsdns/map_test.go +++ b/wgengine/tsdns/map_test.go @@ -16,12 +16,12 @@ func TestPretty(t *testing.T) { dmap *Map want string }{ - {"empty", NewMap(nil), ""}, + {"empty", NewMap(nil, nil), ""}, { "single", NewMap(map[string]netaddr.IP{ "hello.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), - }), + }, nil), "hello.ipn.dev.\t100.101.102.103\n", }, { @@ -29,7 +29,7 @@ func TestPretty(t *testing.T) { NewMap(map[string]netaddr.IP{ "test1.domain.": netaddr.IPv4(100, 101, 102, 103), "test2.sub.domain.": netaddr.IPv4(100, 99, 9, 1), - }), + }, nil), "test1.domain.\t100.101.102.103\ntest2.sub.domain.\t100.99.9.1\n", }, } @@ -57,7 +57,7 @@ func TestPrettyDiffFrom(t *testing.T) { NewMap(map[string]netaddr.IP{ "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), "test2.ipn.dev.": netaddr.IPv4(100, 103, 102, 101), - }), + }, nil), "+test1.ipn.dev.\t100.101.102.103\n+test2.ipn.dev.\t100.103.102.101\n", }, { @@ -65,11 +65,11 @@ func TestPrettyDiffFrom(t *testing.T) { NewMap(map[string]netaddr.IP{ "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), "test2.ipn.dev.": netaddr.IPv4(100, 103, 102, 101), - }), + }, nil), NewMap(map[string]netaddr.IP{ "test2.ipn.dev.": netaddr.IPv4(100, 103, 102, 101), "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), - }), + }, nil), "", }, { @@ -77,11 +77,11 @@ func TestPrettyDiffFrom(t *testing.T) { NewMap(map[string]netaddr.IP{ "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), "test2.ipn.dev.": netaddr.IPv4(100, 103, 102, 101), - }), + }, nil), NewMap(map[string]netaddr.IP{ "test2.ipn.dev.": netaddr.IPv4(100, 104, 102, 101), "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), - }), + }, nil), "-test2.ipn.dev.\t100.103.102.101\n+test2.ipn.dev.\t100.104.102.101\n", }, { @@ -89,12 +89,12 @@ func TestPrettyDiffFrom(t *testing.T) { NewMap(map[string]netaddr.IP{ "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), "test2.ipn.dev.": netaddr.IPv4(100, 103, 102, 101), - }), + }, nil), NewMap(map[string]netaddr.IP{ "test3.ipn.dev.": netaddr.IPv4(100, 105, 106, 107), "test2.ipn.dev.": netaddr.IPv4(100, 103, 102, 101), "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), - }), + }, nil), "+test3.ipn.dev.\t100.105.106.107\n", }, { @@ -102,10 +102,10 @@ func TestPrettyDiffFrom(t *testing.T) { NewMap(map[string]netaddr.IP{ "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), "test2.ipn.dev.": netaddr.IPv4(100, 103, 102, 101), - }), + }, nil), NewMap(map[string]netaddr.IP{ "test1.ipn.dev.": netaddr.IPv4(100, 101, 102, 103), - }), + }, nil), "-test2.ipn.dev.\t100.103.102.101\n", }, { @@ -115,12 +115,12 @@ func TestPrettyDiffFrom(t *testing.T) { "test4.ipn.dev.": netaddr.IPv4(100, 107, 106, 105), "test5.ipn.dev.": netaddr.IPv4(100, 64, 1, 1), "test2.ipn.dev.": netaddr.IPv4(100, 103, 102, 101), - }), + }, nil), NewMap(map[string]netaddr.IP{ "test2.ipn.dev.": netaddr.IPv4(100, 104, 102, 101), "test1.ipn.dev.": netaddr.IPv4(100, 100, 101, 102), "test3.ipn.dev.": netaddr.IPv4(100, 64, 1, 1), - }), + }, nil), "-test1.ipn.dev.\t100.101.102.103\n+test1.ipn.dev.\t100.100.101.102\n" + "-test2.ipn.dev.\t100.103.102.101\n+test2.ipn.dev.\t100.104.102.101\n" + "+test3.ipn.dev.\t100.64.1.1\n-test4.ipn.dev.\t100.107.106.105\n-test5.ipn.dev.\t100.64.1.1\n", diff --git a/wgengine/tsdns/tsdns.go b/wgengine/tsdns/tsdns.go index 0216b8d90..930f0f8ea 100644 --- a/wgengine/tsdns/tsdns.go +++ b/wgengine/tsdns/tsdns.go @@ -58,9 +58,7 @@ type Packet struct { // it delegates to upstream nameservers if any are set. type Resolver struct { logf logger.Logf - // rootDomain is in ... - rootDomain string - // forwarder is + // forwarder forwards requests to upstream nameservers. forwarder *forwarder // queue is a buffered channel holding DNS requests queued for resolution. @@ -95,12 +93,11 @@ type ResolverConfig struct { // The root domain must be in canonical form (with a trailing period). func NewResolver(config ResolverConfig) *Resolver { r := &Resolver{ - logf: logger.WithPrefix(config.Logf, "tsdns: "), - queue: make(chan Packet, pendingQueueSize), - responses: make(chan Packet), - errors: make(chan error), - closed: make(chan struct{}), - rootDomain: config.RootDomain, + logf: logger.WithPrefix(config.Logf, "tsdns: "), + queue: make(chan Packet, pendingQueueSize), + responses: make(chan Packet), + errors: make(chan error), + closed: make(chan struct{}), } if config.Forward { @@ -196,6 +193,17 @@ func (r *Resolver) Resolve(domain string) (netaddr.IP, dns.RCode, error) { return netaddr.IP{}, dns.RCodeServerFailure, errMapNotSet } + anyHasSuffix := false + for _, rootDomain := range dnsMap.rootDomains { + if strings.HasSuffix(domain, rootDomain) { + anyHasSuffix = true + break + } + } + if !anyHasSuffix { + return netaddr.IP{}, dns.RCodeRefused, nil + } + addr, found := dnsMap.nameToIP[domain] if !found { return netaddr.IP{}, dns.RCodeNameError, nil @@ -509,7 +517,8 @@ func (r *Resolver) respondReverse(query []byte, name string, resp *response) ([] return marshalResponse(resp) } -// respond returns a DNS response to query. +// respond returns a DNS response to query if it can be resolved locally. +// Otherwise, it returns errNotOurName. func (r *Resolver) respond(query []byte) ([]byte, error) { resp := new(response) @@ -533,14 +542,13 @@ func (r *Resolver) respond(query []byte) ([]byte, error) { return r.respondReverse(query, name, resp) } - // Delegate forward lookups when not a subdomain of rootDomain. - if !strings.HasSuffix(name, r.rootDomain) { - return nil, errNotOurName - } - switch resp.Question.Type { case dns.TypeA, dns.TypeAAAA, dns.TypeALL: resp.IP, resp.Header.RCode, err = r.Resolve(name) + // This return code is special: it requests forwarding. + if resp.Header.RCode == dns.RCodeRefused { + return nil, errNotOurName + } default: resp.Header.RCode = dns.RCodeNotImplemented err = errNotImplemented diff --git a/wgengine/tsdns/tsdns_test.go b/wgengine/tsdns/tsdns_test.go index 668b846f5..fcdb8ef18 100644 --- a/wgengine/tsdns/tsdns_test.go +++ b/wgengine/tsdns/tsdns_test.go @@ -26,9 +26,10 @@ var testipv6 = netaddr.IPv6Raw([16]byte{ var dnsMap = NewMap( map[string]netaddr.IP{ - "test1.ipn.dev": testipv4, - "test2.ipn.dev": testipv6, + "test1.ipn.dev.": testipv4, + "test2.ipn.dev.": testipv6, }, + []string{"ipn.dev."}, ) func dnspacket(domain string, tp dns.Type) []byte { @@ -178,7 +179,7 @@ func TestRDNSNameToIPv6(t *testing.T) { } func TestResolve(t *testing.T) { - r := NewResolver(ResolverConfig{Logf: t.Logf, RootDomain: "ipn.dev.", Forward: false}) + r := NewResolver(ResolverConfig{Logf: t.Logf, Forward: false}) r.SetMap(dnsMap) if err := r.Start(); err != nil { @@ -195,7 +196,7 @@ func TestResolve(t *testing.T) { {"ipv4", "test1.ipn.dev.", testipv4, dns.RCodeSuccess}, {"ipv6", "test2.ipn.dev.", testipv6, dns.RCodeSuccess}, {"nxdomain", "test3.ipn.dev.", netaddr.IP{}, dns.RCodeNameError}, - {"foreign domain", "google.com.", netaddr.IP{}, dns.RCodeNameError}, + {"foreign domain", "google.com.", netaddr.IP{}, dns.RCodeRefused}, } for _, tt := range tests { @@ -216,7 +217,7 @@ func TestResolve(t *testing.T) { } func TestResolveReverse(t *testing.T) { - r := NewResolver(ResolverConfig{Logf: t.Logf, RootDomain: "ipn.dev.", Forward: false}) + r := NewResolver(ResolverConfig{Logf: t.Logf, Forward: false}) r.SetMap(dnsMap) if err := r.Start(); err != nil { @@ -282,7 +283,8 @@ func TestDelegate(t *testing.T) { return } - r := NewResolver(ResolverConfig{Logf: t.Logf, RootDomain: "ipn.dev.", Forward: true}) + r := NewResolver(ResolverConfig{Logf: t.Logf, Forward: true}) + r.SetMap(dnsMap) r.SetUpstreams([]net.Addr{ v4server.PacketConn.LocalAddr(), v6server.PacketConn.LocalAddr(), @@ -341,7 +343,8 @@ func TestDelegateCollision(t *testing.T) { } defer server.Shutdown() - r := NewResolver(ResolverConfig{Logf: t.Logf, RootDomain: "ipn.dev.", Forward: true}) + r := NewResolver(ResolverConfig{Logf: t.Logf, Forward: true}) + r.SetMap(dnsMap) r.SetUpstreams([]net.Addr{server.PacketConn.LocalAddr()}) if err := r.Start(); err != nil { @@ -406,7 +409,7 @@ func TestDelegateCollision(t *testing.T) { } func TestConcurrentSetMap(t *testing.T) { - r := NewResolver(ResolverConfig{Logf: t.Logf, RootDomain: "ipn.dev.", Forward: false}) + r := NewResolver(ResolverConfig{Logf: t.Logf, Forward: false}) if err := r.Start(); err != nil { t.Fatalf("start: %v", err) @@ -442,7 +445,7 @@ func TestConcurrentSetUpstreams(t *testing.T) { } defer server.Shutdown() - r := NewResolver(ResolverConfig{Logf: t.Logf, RootDomain: "ipn.dev.", Forward: true}) + r := NewResolver(ResolverConfig{Logf: t.Logf, Forward: true}) r.SetMap(dnsMap) if err := r.Start(); err != nil { @@ -549,7 +552,7 @@ var nxdomainResponse = []byte{ } func TestFull(t *testing.T) { - r := NewResolver(ResolverConfig{Logf: t.Logf, RootDomain: "ipn.dev.", Forward: false}) + r := NewResolver(ResolverConfig{Logf: t.Logf, Forward: false}) r.SetMap(dnsMap) if err := r.Start(); err != nil { @@ -584,7 +587,7 @@ func TestFull(t *testing.T) { } func TestAllocs(t *testing.T) { - r := NewResolver(ResolverConfig{Logf: t.Logf, RootDomain: "ipn.dev.", Forward: false}) + r := NewResolver(ResolverConfig{Logf: t.Logf, Forward: false}) r.SetMap(dnsMap) if err := r.Start(); err != nil { @@ -630,7 +633,7 @@ func BenchmarkFull(b *testing.B) { } defer server.Shutdown() - r := NewResolver(ResolverConfig{Logf: b.Logf, RootDomain: "ipn.dev.", Forward: true}) + r := NewResolver(ResolverConfig{Logf: b.Logf, Forward: true}) r.SetMap(dnsMap) r.SetUpstreams([]net.Addr{server.PacketConn.LocalAddr()}) diff --git a/wgengine/userspace.go b/wgengine/userspace.go index c6aa2e915..b5c0790af 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -61,9 +61,6 @@ const ( magicDNSPort = 53 ) -// magicDNSDomain is the parent domain for Tailscale nodes. -const magicDNSDomain = "b.tailscale.net." - // Lazy wireguard-go configuration parameters. const ( // lazyPeerIdleThreshold is the idle duration after @@ -202,9 +199,8 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) { logf := conf.Logf rconf := tsdns.ResolverConfig{ - Logf: conf.Logf, - RootDomain: magicDNSDomain, - Forward: true, + Logf: conf.Logf, + Forward: true, } e := &userspaceEngine{ timeNow: time.Now,