diff --git a/appc/conn25.go b/appc/conn25.go index 62cb70017..13a41e93f 100644 --- a/appc/conn25.go +++ b/appc/conn25.go @@ -5,13 +5,14 @@ package appc import ( "cmp" + "fmt" "slices" "strings" "tailscale.com/ipn/ipnext" "tailscale.com/tailcfg" "tailscale.com/types/appctype" - "tailscale.com/util/mak" + "tailscale.com/types/dnstype" "tailscale.com/util/set" ) @@ -54,71 +55,32 @@ func PickConnector(nb ipnext.NodeBackend, app appctype.Conn25Attr) []tailcfg.Nod return matches } -// PickSplitDNSPeers looks at the netmap peers capabilities and finds which peers -// want to be connectors for which domains. -func PickSplitDNSPeers(hasCap func(c tailcfg.NodeCapability) bool, self tailcfg.NodeView, peers map[tailcfg.NodeID]tailcfg.NodeView, isSelfEligibleConnector bool) map[string][]tailcfg.NodeView { - var m map[string][]tailcfg.NodeView +// DNSAddrScheme is the custom URI scheme used for conn25-managed split DNS +// entries to determine the destination at query time rather than configuration +// time. +const DNSAddrScheme = "tailscale-app" + +func AppDNSRoutes(hasCap func(c tailcfg.NodeCapability) bool, self tailcfg.NodeView) map[string][]*dnstype.Resolver { if !hasCap(AppConnectorsExperimentalAttrName) { - return m + return nil } apps, err := tailcfg.UnmarshalNodeCapViewJSON[appctype.AppConnectorAttr](self.CapMap(), AppConnectorsExperimentalAttrName) if err != nil { - return m + return nil } - - // We strip the leading *. from any domains because the OS treats all domains - // that we pass to it as wildcard domains, and the OS would treat the * character - // as a literal domain component instead of treating it as a wildcard. - // We also use a Set to deduplicate the domains we pass to the OS in case removing - // the *. prefix resulted in duplicate entries. - tagToDomain := make(map[string]set.Set[string]) - selfTags := set.SetOf(self.Tags().AsSlice()) - selfRoutedDomains := set.Set[string]{} + appNamesByDomain := map[string]string{} for _, app := range apps { - domains := make(set.Set[string]) for _, domain := range app.Domains { - domains.Add(strings.ToLower(strings.TrimPrefix(domain, "*."))) - } - for _, tag := range app.Connectors { - if tagToDomain[tag] == nil { - tagToDomain[tag] = set.Set[string]{} - } - tagToDomain[tag].AddSet(domains) - if isSelfEligibleConnector && selfTags.Contains(tag) { - selfRoutedDomains.AddSet(domains) - } + domain, _ = strings.CutPrefix(domain, "*.") + domain = strings.ToLower(domain) + // in the case of multiple apps specifying the same domain (which is misconfiguration + // that should be validated at point of input) last write wins. + appNamesByDomain[domain] = app.Name } } - // NodeIDs are Comparable, and we have a map of NodeID to NodeView anyway, so - // use a Set of NodeIDs to deduplicate, and populate into a []NodeView later. - var work map[string]set.Set[tailcfg.NodeID] - for _, peer := range peers { - if !isPeerEligibleConnector(peer) { - continue - } - for _, t := range peer.Tags().All() { - domains := tagToDomain[t] - for domain := range domains { - if selfRoutedDomains.Contains(domain) { - continue - } - if work[domain] == nil { - mak.Set(&work, domain, set.Set[tailcfg.NodeID]{}) - } - work[domain].Add(peer.ID()) - } - } - } - - // Populate m. Make a []tailcfg.NodeView from []tailcfg.NodeID using the peers map. - // And sort it to our preference. - for domain, ids := range work { - nodes := make([]tailcfg.NodeView, 0, ids.Len()) - for id := range ids { - nodes = append(nodes, peers[id]) - } - sortByPreference(nodes) - mak.Set(&m, domain, nodes) + m := make(map[string][]*dnstype.Resolver, len(appNamesByDomain)) + for domain, appName := range appNamesByDomain { + m[domain] = []*dnstype.Resolver{{Addr: fmt.Sprintf("%s:%s", DNSAddrScheme, appName)}} } return m } diff --git a/appc/conn25_test.go b/appc/conn25_test.go index dd98312ca..d9b4201a9 100644 --- a/appc/conn25_test.go +++ b/appc/conn25_test.go @@ -5,17 +5,18 @@ package appc import ( "encoding/json" - "reflect" + "fmt" "testing" "github.com/google/go-cmp/cmp" "tailscale.com/ipn/ipnext" "tailscale.com/tailcfg" "tailscale.com/types/appctype" + "tailscale.com/types/dnstype" "tailscale.com/types/opt" ) -func TestPickSplitDNSPeers(t *testing.T) { +func TestAppDNSRoutes(t *testing.T) { getBytesForAttr := func(name string, domains []string, tags []string) []byte { attr := appctype.AppConnectorAttr{ Name: name, @@ -35,206 +36,102 @@ func TestPickSplitDNSPeers(t *testing.T) { appFiveBytes := getBytesForAttr("app5", []string{"*.example.com", "example.com"}, []string{"tag:one"}) appSixBytes := getBytesForAttr("app6", []string{"*.Example.com", "EXAMPLE.com", "EXAMPLE.COM"}, []string{"tag:one"}) - makeNodeView := func(id tailcfg.NodeID, name string, tags []string) tailcfg.NodeView { - return (&tailcfg.Node{ - ID: id, - Name: name, - Tags: tags, - Hostinfo: (&tailcfg.Hostinfo{AppConnector: opt.NewBool(true)}).View(), - }).View() + resolver := func(appName string) []*dnstype.Resolver { + return []*dnstype.Resolver{{Addr: fmt.Sprintf("%s:%s", DNSAddrScheme, appName)}} } - nvp1 := makeNodeView(1, "p1", []string{"tag:one"}) - nvp2 := makeNodeView(2, "p2", []string{"tag:four1", "tag:four2"}) - nvp3 := makeNodeView(3, "p3", []string{"tag:two", "tag:three1"}) - nvp4 := makeNodeView(4, "p4", []string{"tag:two", "tag:three2", "tag:four2"}) for _, tt := range []struct { - name string - peers []tailcfg.NodeView - config []tailcfg.RawMessage - isEligibleConnector bool - selfTags []string - want map[string][]tailcfg.NodeView + name string + hasCap bool + config []tailcfg.RawMessage + want map[string][]*dnstype.Resolver }{ { - name: "empty", + name: "no-capability", // hasCap false should return nil regardless of config. + hasCap: false, }, { - name: "bad-config", // bad config should return a nil map rather than error. + name: "no-apps", // hasCap true but no configured apps returns an empty map. + hasCap: true, + want: map[string][]*dnstype.Resolver{}, + }, + { + name: "bad-config", // bad config should return nil rather than error. + hasCap: true, config: []tailcfg.RawMessage{tailcfg.RawMessage(`hey`)}, }, { - name: "no-peers", + name: "single-app", + hasCap: true, config: []tailcfg.RawMessage{tailcfg.RawMessage(appOneBytes)}, - }, - { - name: "peers-that-are-not-connectors", - config: []tailcfg.RawMessage{tailcfg.RawMessage(appOneBytes)}, - peers: []tailcfg.NodeView{ - (&tailcfg.Node{ - ID: 5, - Name: "p5", - Tags: []string{"tag:one"}, - }).View(), - (&tailcfg.Node{ - ID: 6, - Name: "p6", - Tags: []string{"tag:one"}, - }).View(), + want: map[string][]*dnstype.Resolver{ + "example.com": resolver("app1"), }, }, { - name: "peers-that-dont-match-tags", - config: []tailcfg.RawMessage{tailcfg.RawMessage(appOneBytes)}, - peers: []tailcfg.NodeView{ - makeNodeView(5, "p5", []string{"tag:seven"}), - makeNodeView(6, "p6", nil), + name: "single-app-multi-domain", + hasCap: true, + config: []tailcfg.RawMessage{tailcfg.RawMessage(appThreeBytes)}, + want: map[string][]*dnstype.Resolver{ + "woo.b.example.com": resolver("app3"), + "hoo.b.example.com": resolver("app3"), }, }, { - name: "matching-tagged-connector-peers", + name: "multi-app-no-overlap", + hasCap: true, config: []tailcfg.RawMessage{ tailcfg.RawMessage(appOneBytes), tailcfg.RawMessage(appTwoBytes), - tailcfg.RawMessage(appThreeBytes), - tailcfg.RawMessage(appFourBytes), }, - peers: []tailcfg.NodeView{ - nvp1, - nvp2, - nvp3, - nvp4, - makeNodeView(5, "p5", nil), - }, - want: map[string][]tailcfg.NodeView{ - // p5 has no matching tags and so doesn't appear - "example.com": {nvp1}, - "a.example.com": {nvp3, nvp4}, - "woo.b.example.com": {nvp2, nvp3, nvp4}, - "hoo.b.example.com": {nvp3, nvp4}, - "c.example.com": {nvp2, nvp4}, + want: map[string][]*dnstype.Resolver{ + "example.com": resolver("app1"), + "a.example.com": resolver("app2"), }, }, { - name: "self-connector-exclude-self-domains", + name: "domain-collision-last-write-wins", + hasCap: true, config: []tailcfg.RawMessage{ - tailcfg.RawMessage(appOneBytes), - tailcfg.RawMessage(appTwoBytes), - tailcfg.RawMessage(appThreeBytes), - tailcfg.RawMessage(appFourBytes), + tailcfg.RawMessage(appThreeBytes), // app3: woo.b.example.com, hoo.b.example.com + tailcfg.RawMessage(appFourBytes), // app4: woo.b.example.com, c.example.com }, - peers: []tailcfg.NodeView{ - nvp1, - nvp2, - nvp3, - nvp4, - }, - isEligibleConnector: true, - selfTags: []string{"tag:three1"}, - want: map[string][]tailcfg.NodeView{ - // woo.b.example.com and hoo.b.example.com are covered - // by tag:three1, and so is this self-node. - // So those domains should not be routed to peers. - // woo.b.example.com is also covered by another tag, - // but still not included since this connector can route to it. - "example.com": {nvp1}, - "a.example.com": {nvp3, nvp4}, - "c.example.com": {nvp2, nvp4}, + want: map[string][]*dnstype.Resolver{ + // app4 overwrites app3 for the shared domain + "woo.b.example.com": resolver("app4"), + "hoo.b.example.com": resolver("app3"), + "c.example.com": resolver("app4"), }, }, { - name: "self-eligible-connector-no-matching-tag-include-all-domains", - config: []tailcfg.RawMessage{ - tailcfg.RawMessage(appOneBytes), - tailcfg.RawMessage(appTwoBytes), - tailcfg.RawMessage(appThreeBytes), - tailcfg.RawMessage(appFourBytes), - }, - peers: []tailcfg.NodeView{ - nvp1, - nvp2, - nvp3, - nvp4, - }, - isEligibleConnector: true, - selfTags: []string{"tag:unrelated"}, - want: map[string][]tailcfg.NodeView{ - // Self has prefs set but no tags matching any app, - // so no domains are self-routed and all appear. - "example.com": {nvp1}, - "a.example.com": {nvp3, nvp4}, - "woo.b.example.com": {nvp2, nvp3, nvp4}, - "hoo.b.example.com": {nvp3, nvp4}, - "c.example.com": {nvp2, nvp4}, + name: "wildcards-are-stripped-and-deduped", + hasCap: true, + config: []tailcfg.RawMessage{tailcfg.RawMessage(appFiveBytes)}, + want: map[string][]*dnstype.Resolver{ + // *.example.com and example.com should both normalize to example.com. + "example.com": resolver("app5"), }, }, { - name: "self-not-eligible-connector-but-tagged-include-all-domains", - config: []tailcfg.RawMessage{ - tailcfg.RawMessage(appOneBytes), - tailcfg.RawMessage(appTwoBytes), - tailcfg.RawMessage(appThreeBytes), - tailcfg.RawMessage(appFourBytes), - }, - peers: []tailcfg.NodeView{ - nvp1, - nvp2, - nvp3, - nvp4, - }, - selfTags: []string{"tag:three1"}, - want: map[string][]tailcfg.NodeView{ - // Even though this self node has a tag for an app - // the prefs don't advertise as connector, so - // should still route through other connectors. - "example.com": {nvp1}, - "a.example.com": {nvp3, nvp4}, - "woo.b.example.com": {nvp2, nvp3, nvp4}, - "hoo.b.example.com": {nvp3, nvp4}, - "c.example.com": {nvp2, nvp4}, + name: "domains-are-normalized-and-deduped", + hasCap: true, + config: []tailcfg.RawMessage{tailcfg.RawMessage(appSixBytes)}, + want: map[string][]*dnstype.Resolver{ + // *.Example.com, EXAMPLE.com, EXAMPLE.COM should all normalize to example.com. + "example.com": resolver("app6"), }, }, { - name: "wildcards-are-stripped-and-deduped", - config: []tailcfg.RawMessage{ - tailcfg.RawMessage(appOneBytes), - tailcfg.RawMessage(appFiveBytes), - }, - peers: []tailcfg.NodeView{ - nvp1, - }, - want: map[string][]tailcfg.NodeView{ - // All the domains should be normalized to example.com - "example.com": {nvp1}, - }, - }, - { - name: "domains-are-normalized-and-deduped", - config: []tailcfg.RawMessage{ - tailcfg.RawMessage(appSixBytes), - }, - peers: []tailcfg.NodeView{ - nvp1, - }, - want: map[string][]tailcfg.NodeView{ - // All the domains should be normalized to example.com - "example.com": {nvp1}, - }, - }, - { - name: "sub-domains-and-top-domains-do-not-collide", + name: "sub-domains-and-top-domains-do-not-collide", + hasCap: true, config: []tailcfg.RawMessage{ tailcfg.RawMessage(appTwoBytes), tailcfg.RawMessage(appFiveBytes), }, - peers: []tailcfg.NodeView{ - nvp1, - nvp3, - }, - want: map[string][]tailcfg.NodeView{ - // The sub.example.com should remain distinct from example.com - "example.com": {nvp1}, - "a.example.com": {nvp3}, + want: map[string][]*dnstype.Resolver{ + // *.example.com normalizes to example.com; a.example.com remains distinct. + "a.example.com": resolver("app2"), + "example.com": resolver("app5"), }, }, } { @@ -245,18 +142,12 @@ func TestPickSplitDNSPeers(t *testing.T) { tailcfg.NodeCapability(AppConnectorsExperimentalAttrName): tt.config, } } - selfNode.Tags = append(selfNode.Tags, tt.selfTags...) selfView := selfNode.View() - peers := map[tailcfg.NodeID]tailcfg.NodeView{} - for _, p := range tt.peers { - peers[p.ID()] = p - } - got := PickSplitDNSPeers(func(_ tailcfg.NodeCapability) bool { - return true - }, selfView, peers, tt.isEligibleConnector) - - if !reflect.DeepEqual(got, tt.want) { - t.Fatalf("got %v, want %v", got, tt.want) + got := AppDNSRoutes(func(_ tailcfg.NodeCapability) bool { + return tt.hasCap + }, selfView) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Fatalf("AppDNSRoutes (-want, +got):\n%s", diff) } }) } diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go index 6955b95d9..7d83bb891 100644 --- a/feature/conn25/conn25.go +++ b/feature/conn25/conn25.go @@ -147,6 +147,36 @@ func (e *extension) installHooks(dph *datapathHandler) error { if !ok { return errors.New("could not access system tun") } + resolver := dnsManager.Resolver() + if resolver == nil { + return errors.New("dns manager resolver not ready") + } + + if err := resolver.RegisterCustomScheme(appc.DNSAddrScheme, func(addr string) (string, error) { + scheme, appName, ok := strings.Cut(addr, ":") + if !ok || scheme != appc.DNSAddrScheme { + return "", fmt.Errorf("unexpected conn25 scheme %q", scheme) + } + + if !e.conn25.isConfigured() { + return "", errors.New("conn25 not configured") + } + cfg, ok := e.conn25.getConfig() + if !ok { + return "", errors.New("conn25 no config found") + } + app, ok := cfg.appsByName[appName] + if !ok { + return "", errors.New("no app found for app name") + } + _, urlBase := e.pickConnectorURLBase(app) + if urlBase == "" { + return "", nil + } + return urlBase + "/dns-query", nil + }); err != nil { + return fmt.Errorf("could not register DNS resolver scheme: %w", err) + } // Set up the DNS manager to rewrite responses for app domains // to answer with Magic IPs. @@ -925,17 +955,7 @@ func makePeerAPIReq(ctx context.Context, httpClient *http.Client, urlBase string return nil } -func (e *extension) sendAddressAssignment(ctx context.Context, as addrs) (tailcfg.NodeView, error) { - cfg, ok := e.conn25.getConfig() - if !ok { - return tailcfg.NodeView{}, errors.New("not configured") - } - app, ok := cfg.appsByName[as.app] - if !ok { - e.conn25.logf("App not found for app: %s (domain: %s)", as.app, as.domain) - return tailcfg.NodeView{}, errors.New("app not found") - } - +func (e *extension) pickConnectorURLBase(app appctype.Conn25Attr) (tailcfg.NodeView, string) { nb := e.host.NodeBackend() peers := appc.PickConnector(nb, app) var urlBase string @@ -947,6 +967,20 @@ func (e *extension) sendAddressAssignment(ctx context.Context, as addrs) (tailcf break } } + return conn, urlBase +} + +func (e *extension) sendAddressAssignment(ctx context.Context, as addrs) (tailcfg.NodeView, error) { + cfg, ok := e.conn25.getConfig() + if !ok { + return tailcfg.NodeView{}, errors.New("not configured") + } + app, ok := cfg.appsByName[as.app] + if !ok { + e.conn25.client.logf("App not found for app: %s (domain: %s)", as.app, as.domain) + return tailcfg.NodeView{}, errors.New("app not found") + } + conn, urlBase := e.pickConnectorURLBase(app) if urlBase == "" { return tailcfg.NodeView{}, errors.New("no connector peer found to handle address assignment") } diff --git a/feature/conn25/conn25_test.go b/feature/conn25/conn25_test.go index 3386bdd0e..a1063de74 100644 --- a/feature/conn25/conn25_test.go +++ b/feature/conn25/conn25_test.go @@ -21,6 +21,7 @@ import ( "tailscale.com/ipn" "tailscale.com/ipn/ipnext" "tailscale.com/net/dns" + "tailscale.com/net/netmon" "tailscale.com/net/packet" "tailscale.com/net/tsdial" "tailscale.com/net/tstun" @@ -32,6 +33,7 @@ import ( "tailscale.com/types/logger" "tailscale.com/types/opt" "tailscale.com/util/dnsname" + "tailscale.com/util/eventbus/eventbustest" "tailscale.com/util/must" "tailscale.com/util/set" ) @@ -1393,14 +1395,14 @@ type testSafeBackend struct { sys *tsd.System } -func newTestSafeBackend() *testSafeBackend { - sb := &testSafeBackend{} - sys := &tsd.System{} - sys.Dialer.Set(&tsdial.Dialer{Logf: logger.Discard}) - sys.DNSManager.Set(&dns.Manager{}) +func newTestSafeBackend(t *testing.T) *testSafeBackend { + sys := tsd.NewSystemWithBus(eventbustest.NewBus(t)) + dialer := tsdial.NewDialer(netmon.NewStatic()) + sys.Dialer.Set(dialer) + ht := sys.HealthTracker.Get() + sys.DNSManager.Set(dns.NewManager(logger.Discard, nil, ht, dialer, nil, nil, "", sys.Bus.Get())) sys.Tun.Set(&tstun.Wrapper{}) - sb.sys = sys - return sb + return &testSafeBackend{sys: sys} } func (b *testSafeBackend) Sys() *tsd.System { return b.sys } @@ -1439,7 +1441,7 @@ func TestAddressAssignmentIsHandled(t *testing.T) { ext := &extension{ conn25: newConn25(logger.Discard), - backend: newTestSafeBackend(), + backend: newTestSafeBackend(t), } authReconfigAsyncCalled := make(chan struct{}, 1) if err := ext.Init(&testHost{ @@ -2034,7 +2036,7 @@ func TestHandleAddressAssignmentStoresTransitIPs(t *testing.T) { ext := &extension{ conn25: newConn25(logger.Discard), - backend: newTestSafeBackend(), + backend: newTestSafeBackend(t), } authReconfigAsyncCalled := make(chan struct{}, 1) if err := ext.Init(&testHost{ diff --git a/ipn/ipnlocal/dnsconfig_test.go b/ipn/ipnlocal/dnsconfig_test.go index 9d30029ff..39f03e439 100644 --- a/ipn/ipnlocal/dnsconfig_test.go +++ b/ipn/ipnlocal/dnsconfig_test.go @@ -431,54 +431,11 @@ func TestDNSConfigForNetmap(t *testing.T) { }, Routes: map[dnsname.FQDN][]*dnstype.Resolver{ dnsname.FQDN("example.com."): { - {Addr: "http://100.102.0.1:1234/dns-query"}, + {Addr: "tailscale-app:app1"}, }, }, }, }, - { - name: "conn25-split-dns-no-matching-peers", - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - Name: "a", - Addresses: ipps("100.101.101.101"), - CapMap: tailcfg.NodeCapMap{ - tailcfg.NodeCapability(appc.AppConnectorsExperimentalAttrName): []tailcfg.RawMessage{ - tailcfg.RawMessage(`{"name":"app1","connectors":["tag:woo"],"domains":["example.com"]}`), - }, - }, - }).View(), - AllCaps: set.Of(tailcfg.NodeCapability(appc.AppConnectorsExperimentalAttrName)), - }, - peers: nodeViews([]*tailcfg.Node{ - { - ID: 1, - Name: "p1", - Addresses: ipps("100.102.0.1"), - Tags: []string{"tag:nomatch"}, - Hostinfo: (&tailcfg.Hostinfo{ - Services: []tailcfg.Service{ - { - Proto: tailcfg.PeerAPI4, - Port: 1234, - }, - }, - AppConnector: opt.NewBool(true), - }).View(), - }, - }), - prefs: &ipn.Prefs{ - CorpDNS: true, - }, - want: &dns.Config{ - AcceptDNS: true, - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: map[dnsname.FQDN][]netip.Addr{ - "a.": ips("100.101.101.101"), - "p1.": ips("100.102.0.1"), - }, - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index 0f7bce81e..eb224f71b 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/ipn/ipnlocal/node_backend.go @@ -6,7 +6,6 @@ package ipnlocal import ( "cmp" "context" - "fmt" "maps" "net/netip" "slices" @@ -1031,22 +1030,10 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. // Add split DNS routes, with no regard to exit node configuration. addSplitDNSRoutes(nm.DNS.Routes) - // Add split DNS routes for conn25 - conn25DNSTargets := appc.PickSplitDNSPeers(nm.HasCap, nm.SelfNode, peers, prefs.AppConnector().Advertise) - if conn25DNSTargets != nil { - var m map[string][]*dnstype.Resolver - for domain, candidateSplitDNSPeers := range conn25DNSTargets { - for _, peer := range candidateSplitDNSPeers { - base := peerAPIBase(nm, peer) - if base == "" { - continue - } - mak.Set(&m, domain, []*dnstype.Resolver{{Addr: fmt.Sprintf("%s/dns-query", base)}}) - break // Just make one resolver for the first peer we can get a peerAPIBase for. - } - } - if m != nil { - addSplitDNSRoutes(m) + if buildfeatures.HasConn25 && !prefs.AppConnector().Advertise { + // Add split DNS routes for conn25 + if appRoutes := appc.AppDNSRoutes(nm.HasCap, nm.SelfNode); appRoutes != nil { + addSplitDNSRoutes(appRoutes) } }