diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index d7fad76dd..cab5002a1 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3339,9 +3339,7 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. } addDefault := func(resolvers []*dnstype.Resolver) { - for _, r := range resolvers { - dcfg.DefaultResolvers = append(dcfg.DefaultResolvers, r) - } + dcfg.DefaultResolvers = append(dcfg.DefaultResolvers, resolvers...) } // If we're using an exit node and that exit node is new enough (1.19.x+) @@ -3351,7 +3349,17 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. return dcfg } - addDefault(nm.DNS.Resolvers) + // 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) + } + } + for suffix, resolvers := range nm.DNS.Routes { fqdn, err := dnsname.ToFQDN(suffix) if err != nil { @@ -3366,11 +3374,10 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. // // While we're already populating it, might as well size the // slice appropriately. + // Per #9498 the exact requirements of nil vs empty slice remain + // unclear, this is a haunted graveyard to be resolved. dcfg.Routes[fqdn] = make([]*dnstype.Resolver, 0, len(resolvers)) - - for _, r := range resolvers { - dcfg.Routes[fqdn] = append(dcfg.Routes[fqdn], r) - } + dcfg.Routes[fqdn] = append(dcfg.Routes[fqdn], resolvers...) } // Set FallbackResolvers as the default resolvers in the @@ -4768,6 +4775,32 @@ func exitNodeCanProxyDNS(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg return "", false } +// wireguardExitNodeDNSResolvers returns the DNS resolvers to use for a +// WireGuard-only exit node, if it has resolver addresses. +func wireguardExitNodeDNSResolvers(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg.NodeView, exitNodeID tailcfg.StableNodeID) ([]*dnstype.Resolver, bool) { + if exitNodeID.IsZero() { + return nil, false + } + + for _, p := range peers { + if p.StableID() == exitNodeID { + if p.IsWireGuardOnly() { + resolvers := p.ExitNodeDNSResolvers() + if !resolvers.IsNil() && resolvers.Len() > 0 { + copies := make([]*dnstype.Resolver, resolvers.Len()) + for i := range resolvers.LenIter() { + copies[i] = resolvers.At(i).AsStruct() + } + return copies, true + } + } + return nil, false + } + } + + return nil, false +} + func peerCanProxyDNS(p tailcfg.NodeView) bool { if p.Cap() >= 26 { // Actually added at 25 diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 757f2254b..2bb037a30 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -22,11 +22,13 @@ import ( "tailscale.com/tailcfg" "tailscale.com/tsd" "tailscale.com/tstest" + "tailscale.com/types/dnstype" "tailscale.com/types/key" "tailscale.com/types/logger" "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" @@ -899,3 +901,259 @@ func TestWhoIs(t *testing.T) { }) } } + +func TestWireguardExitNodeDNSResolvers(t *testing.T) { + type tc struct { + name string + id tailcfg.StableNodeID + peers []*tailcfg.Node + wantOK bool + wantResolvers []*dnstype.Resolver + } + + tests := []tc{ + { + name: "no peers", + id: "1", + wantOK: false, + wantResolvers: nil, + }, + { + name: "non wireguard peer", + id: "1", + peers: []*tailcfg.Node{ + { + ID: 1, + StableID: "1", + IsWireGuardOnly: false, + ExitNodeDNSResolvers: []*dnstype.Resolver{{Addr: "dns.example.com"}}, + }, + }, + wantOK: false, + wantResolvers: nil, + }, + { + name: "no matching IDs", + id: "2", + peers: []*tailcfg.Node{ + { + ID: 1, + StableID: "1", + IsWireGuardOnly: true, + ExitNodeDNSResolvers: []*dnstype.Resolver{{Addr: "dns.example.com"}}, + }, + }, + wantOK: false, + wantResolvers: nil, + }, + { + name: "wireguard peer", + id: "1", + peers: []*tailcfg.Node{ + { + ID: 1, + StableID: "1", + IsWireGuardOnly: true, + ExitNodeDNSResolvers: []*dnstype.Resolver{{Addr: "dns.example.com"}}, + }, + }, + wantOK: true, + wantResolvers: []*dnstype.Resolver{{Addr: "dns.example.com"}}, + }, + } + + for _, tc := range tests { + peers := peersMap(nodeViews(tc.peers)) + nm := &netmap.NetworkMap{} + gotResolvers, gotOK := wireguardExitNodeDNSResolvers(nm, peers, tc.id) + + if gotOK != tc.wantOK || !resolversEqual(t, 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{ + ID: 1, + StableID: "wg", + IsWireGuardOnly: true, + ExitNodeDNSResolvers: wgResolvers, + 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 + } + + 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) + } + }) + } +} + +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 + } + 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 + } + } + return true +} diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 9ead3525b..b3754f2b5 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -115,7 +115,8 @@ type CapabilityVersion int // - 73: 2023-09-01: Non-Windows clients expect to receive ClientVersion // - 74: 2023-09-18: Client understands NodeCapMap // - 75: 2023-09-12: Client understands NodeAttrDNSForwarderDisableTCPRetries -const CurrentCapabilityVersion CapabilityVersion = 75 +// - 76: 2023-09-20: Client understands ExitNodeDNSResolvers for IsWireGuardOnly nodes +const CurrentCapabilityVersion CapabilityVersion = 76 type StableID string