diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 77ca3dce1..345799752 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3308,7 +3308,9 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. } addDefault := func(resolvers []*dnstype.Resolver) { - dcfg.DefaultResolvers = append(dcfg.DefaultResolvers, resolvers...) + for _, r := range resolvers { + dcfg.DefaultResolvers = append(dcfg.DefaultResolvers, r) + } } // If we're using an exit node and that exit node is new enough (1.19.x+) @@ -3318,17 +3320,14 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. return dcfg } - // If the user has set default resolvers ("override local DNS"), prefer to - // use those resolvers as the default, otherwise if there are WireGuard exit - // node resolvers, use those as the default. - if len(nm.DNS.Resolvers) > 0 { - addDefault(nm.DNS.Resolvers) - } else { - if resolvers, ok := wireguardExitNodeDNSResolvers(nm, peers, prefs.ExitNodeID()); ok { - addDefault(resolvers) - } + // If we're using an exit node and that exit node is IsWireGuardOnly with + // ExitNodeDNSResolver set, then add that as the default. + if resolvers, ok := wireguardExitNodeDNSResolvers(nm, peers, prefs.ExitNodeID()); ok { + addDefault(resolvers) + return dcfg } + addDefault(nm.DNS.Resolvers) for suffix, resolvers := range nm.DNS.Routes { fqdn, err := dnsname.ToFQDN(suffix) if err != nil { @@ -3343,7 +3342,11 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. // // While we're already populating it, might as well size the // slice appropriately. - dcfg.Routes[fqdn] = append(dcfg.Routes[fqdn], resolvers...) + dcfg.Routes[fqdn] = make([]*dnstype.Resolver, 0, len(resolvers)) + + for _, r := range resolvers { + dcfg.Routes[fqdn] = append(dcfg.Routes[fqdn], r) + } } // Set FallbackResolvers as the default resolvers in the diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 0957d0863..a55b6f638 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -28,7 +28,6 @@ import ( "tailscale.com/types/logid" "tailscale.com/types/netmap" "tailscale.com/types/ptr" - "tailscale.com/util/dnsname" "tailscale.com/util/set" "tailscale.com/wgengine" "tailscale.com/wgengine/filter" @@ -923,191 +922,41 @@ func TestWireguardExitNodeDNSResolvers(t *testing.T) { nm := &netmap.NetworkMap{} gotResolvers, gotOK := wireguardExitNodeDNSResolvers(nm, peers, tc.id) - if gotOK != tc.wantOK || !resolversEqual(t, gotResolvers, tc.wantResolvers) { + if gotOK != tc.wantOK || !resolversEqual(gotResolvers, tc.wantResolvers) { t.Errorf("case: %s: got %v, %v, want %v, %v", tc.name, gotOK, gotResolvers, tc.wantOK, tc.wantResolvers) } } } -func TestDNSConfigForNetmapForExitNodeConfigs(t *testing.T) { - type tc struct { - name string - exitNode tailcfg.StableNodeID - peers []tailcfg.NodeView - dnsConfig *tailcfg.DNSConfig - wantDefaultResolvers []*dnstype.Resolver - wantRoutes map[dnsname.FQDN][]*dnstype.Resolver - } - - defaultResolvers := []*dnstype.Resolver{{Addr: "default.example.com"}} - wgResolvers := []*dnstype.Resolver{{Addr: "wg.example.com"}} - peers := []tailcfg.NodeView{ - (&tailcfg.Node{ +func TestDNSConfigForNetmapForWireguardExitNode(t *testing.T) { + resolvers := []*dnstype.Resolver{{Addr: "dns.example.com"}} + nm := &netmap.NetworkMap{} + peers := map[tailcfg.NodeID]tailcfg.NodeView{ + 1: (&tailcfg.Node{ ID: 1, - StableID: "wg", + StableID: "1", IsWireGuardOnly: true, - ExitNodeDNSResolvers: wgResolvers, + ExitNodeDNSResolvers: resolvers, Hostinfo: (&tailcfg.Hostinfo{}).View(), }).View(), - // regular tailscale exit node with DNS capabilities - (&tailcfg.Node{ - Cap: 26, - ID: 2, - StableID: "ts", - Hostinfo: (&tailcfg.Hostinfo{}).View(), - }).View(), } - exitDOH := peerAPIBase(&netmap.NetworkMap{Peers: peers}, peers[0]) + "/dns-query" - routes := map[dnsname.FQDN][]*dnstype.Resolver{ - "route.example.com.": {{Addr: "route.example.com"}}, - } - stringifyRoutes := func(routes map[dnsname.FQDN][]*dnstype.Resolver) map[string][]*dnstype.Resolver { - if routes == nil { - return nil - } - m := make(map[string][]*dnstype.Resolver) - for k, v := range routes { - m[string(k)] = v - } - return m + prefs := &ipn.Prefs{ + ExitNodeID: "1", + CorpDNS: true, } - tests := []tc{ - { - name: "noExit/noRoutes/noResolver", - exitNode: "", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{}, - wantDefaultResolvers: nil, - wantRoutes: nil, - }, - { - name: "tsExit/noRoutes/noResolver", - exitNode: "ts", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{}, - wantDefaultResolvers: []*dnstype.Resolver{{Addr: exitDOH}}, - wantRoutes: nil, - }, - { - name: "tsExit/noRoutes/defaultResolver", - exitNode: "ts", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{Resolvers: defaultResolvers}, - wantDefaultResolvers: []*dnstype.Resolver{{Addr: exitDOH}}, - wantRoutes: nil, - }, - - // The following two cases may need to be revisited. For a shared-in - // exit node split-DNS may effectively break, furthermore in the future - // if different nodes observe different DNS configurations, even a - // tailnet local exit node may present a different DNS configuration, - // which may not meet expectations in some use cases. - // In the case where a default resolver is set, the default resolver - // should also perhaps take precedence also. - { - name: "tsExit/routes/noResolver", - exitNode: "ts", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{Routes: stringifyRoutes(routes)}, - wantDefaultResolvers: []*dnstype.Resolver{{Addr: exitDOH}}, - wantRoutes: nil, - }, - { - name: "tsExit/routes/defaultResolver", - exitNode: "ts", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{Routes: stringifyRoutes(routes), Resolvers: defaultResolvers}, - wantDefaultResolvers: []*dnstype.Resolver{{Addr: exitDOH}}, - wantRoutes: nil, - }, - - // WireGuard exit nodes with DNS capabilities provide a "fallback" type - // behavior, they have a lower precedence than a default resolver, but - // otherwise allow split-DNS to operate as normal, and are used when - // there is no default resolver. - { - name: "wgExit/noRoutes/noResolver", - exitNode: "wg", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{}, - wantDefaultResolvers: wgResolvers, - wantRoutes: nil, - }, - { - name: "wgExit/noRoutes/defaultResolver", - exitNode: "wg", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{Resolvers: defaultResolvers}, - wantDefaultResolvers: defaultResolvers, - wantRoutes: nil, - }, - { - name: "wgExit/routes/defaultResolver", - exitNode: "wg", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{Routes: stringifyRoutes(routes), Resolvers: defaultResolvers}, - wantDefaultResolvers: defaultResolvers, - wantRoutes: routes, - }, - { - name: "wgExit/routes/noResolver", - exitNode: "wg", - peers: peers, - dnsConfig: &tailcfg.DNSConfig{Routes: stringifyRoutes(routes)}, - wantDefaultResolvers: wgResolvers, - wantRoutes: routes, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - nm := &netmap.NetworkMap{ - Peers: tc.peers, - DNS: *tc.dnsConfig, - } - - prefs := &ipn.Prefs{ExitNodeID: tc.exitNode, CorpDNS: true} - got := dnsConfigForNetmap(nm, peersMap(tc.peers), prefs.View(), t.Logf, "") - if !resolversEqual(t, got.DefaultResolvers, tc.wantDefaultResolvers) { - t.Errorf("DefaultResolvers: got %#v, want %#v", got.DefaultResolvers, tc.wantDefaultResolvers) - } - if !routesEqual(t, got.Routes, tc.wantRoutes) { - t.Errorf("Routes: got %#v, want %#v", got.Routes, tc.wantRoutes) - } - }) + got := dnsConfigForNetmap(nm, peers, prefs.View(), t.Logf, "") + if !resolversEqual(got.DefaultResolvers, resolvers) { + t.Errorf("got %v, want %v", got.DefaultResolvers, resolvers) } } -func resolversEqual(t *testing.T, a, b []*dnstype.Resolver) bool { - if a == nil && b == nil { - return true - } - if a == nil || b == nil { - t.Errorf("resolversEqual: a == nil || b == nil : %#v != %#v", a, b) - return false - } +func resolversEqual(a, b []*dnstype.Resolver) bool { if len(a) != len(b) { - t.Errorf("resolversEqual: len(a) != len(b) : %#v != %#v", a, b) return false } for i := range a { if !a[i].Equal(b[i]) { - t.Errorf("resolversEqual: a != b [%d]: %v != %v", i, *a[i], *b[i]) - return false - } - } - return true -} - -func routesEqual(t *testing.T, a, b map[dnsname.FQDN][]*dnstype.Resolver) bool { - if len(a) != len(b) { - t.Logf("routes: len(a) != len(b): %d != %d", len(a), len(b)) - return false - } - for name := range a { - if !resolversEqual(t, a[name], b[name]) { - t.Logf("routes: a != b [%s]: %v != %v", name, a[name], b[name]) return false } }