diff --git a/cmd/tailmixd/main_test.go b/cmd/tailmixd/main_test.go index 3616edb84..88036879e 100644 --- a/cmd/tailmixd/main_test.go +++ b/cmd/tailmixd/main_test.go @@ -827,6 +827,7 @@ func TestTUNPlanInstallsSelectedExitNodeDefaults(t *testing.T) { ProfileID: "work", BackendState: "Running", ExitNodeID: "exit-node", + ProfileDNS: true, SelfNodeID: "self", SelfIPs: []netip.Addr{ netip.MustParseAddr("100.64.0.10"), @@ -892,6 +893,50 @@ func TestTUNPlanInstallsSelectedExitNodeDefaults(t *testing.T) { } } +func TestTUNPlanInstallsWireGuardExitNodeWithoutProfileDNS(t *testing.T) { + st := state.State{ + SyntheticPool: "10.250.0.0/16", + SyntheticPoolV6: "fd6d:6e65:7400::/120", + Profiles: []state.Profile{{ID: "wg", Name: "wg"}}, + ExitNode: &state.ExitNode{ + ProfileID: "wg", + NodeID: "gateway", + PeerIP: netip.MustParseAddr("10.80.0.1"), + }, + } + status := tailmixprofile.Status{ + ProfileID: "wg", + Kind: "wireguard", + BackendState: "Running", + ExitNodeID: "gateway", + SelfIPs: []netip.Addr{netip.MustParseAddr("10.80.0.2")}, + } + plan, err := buildTUNPlan(st, []tailmixprofile.Status{status}) + if err != nil { + t.Fatal(err) + } + if route, ok := plan.Table.ExitRoutes.Lookup(netip.MustParseAddr("203.0.113.10")); !ok || !route.Active || route.ProfileID != "wg" { + t.Fatalf("exit route for a WireGuard exit node = %+v, %v", route, ok) + } + found := false + for _, route := range plan.HostConfig.Routes { + if route.Destination == netip.MustParsePrefix("0.0.0.0/1") { + if !route.Exit { + t.Fatalf("host route %+v is not marked as an exit route", route) + } + found = true + } + } + if !found { + t.Fatalf("host routes %+v do not contain the exit route", plan.HostConfig.Routes) + } + for _, route := range plan.DNSConfig.Routes { + if route.ProfileDNS { + t.Fatalf("profile DNS route for a WireGuard exit node: %+v", plan.DNSConfig.Routes) + } + } +} + func hasDNSRecord(records []tailmixdns.Record, name string, ip netip.Addr) bool { for _, record := range records { if record.Name == name && record.EffectiveIP == ip { diff --git a/cmd/tailmixd/policyapi_test.go b/cmd/tailmixd/policyapi_test.go index 4d8c0dc9c..49401ba3d 100644 --- a/cmd/tailmixd/policyapi_test.go +++ b/cmd/tailmixd/policyapi_test.go @@ -23,6 +23,7 @@ func (e *exitPreferenceEngine) Close() error { return nil } func (e *exitPreferenceEngine) Dial(context.Context, string, string) (net.Conn, error) { return nil, net.ErrClosed } + func (e *exitPreferenceEngine) Status(context.Context) (tailmixprofile.Status, error) { return e.status, nil } @@ -38,6 +39,7 @@ func (e *exitPreferenceEngine) SetExitNodeIP(_ context.Context, peerIP netip.Add } return nil } + func (e *exitPreferenceEngine) SetRouteAll(_ context.Context, enabled bool) error { e.routeChanges = append(e.routeChanges, enabled) e.status.RouteAll = enabled @@ -179,7 +181,7 @@ func TestStatusReportsSelectedPolicyWithoutAvailableChoices(t *testing.T) { prefix := netip.MustParsePrefix("10.20.0.0/16") engine := &exitPreferenceEngine{status: tailmixprofile.Status{ ProfileID: "p_work", Alias: "work", BackendState: "Running", - MagicDNSSuffix: "work.example", ExitNodeID: "exit-node", + MagicDNSSuffix: "work.example", ProfileDNS: true, ExitNodeID: "exit-node", AvailableRoutes: []tailmixprofile.RouteStatus{{ Prefix: prefix, PrimaryRouter: "subnet-router", }}, diff --git a/cmd/tailmixd/supervisor.go b/cmd/tailmixd/supervisor.go index fb9eb4e53..1aaa6f1c4 100644 --- a/cmd/tailmixd/supervisor.go +++ b/cmd/tailmixd/supervisor.go @@ -81,13 +81,17 @@ type supervisor struct { mux *tunmux.Mux dnsService tailmixdns.Service dnsForwarders []*tailmixdns.Forwarder - socksListener net.Listener - socksRouter *socksproxy.DynamicRouter - networkErr chan error - reconcileErr string - updateMu sync.Mutex - updateWake chan struct{} - updateRestart chan updateRestart + // retiredDNSForwarders holds the forwarders replaced by the last DNS + // reconfiguration. They stay open until the next reconfiguration because + // the aggregate resolver can still have queries in flight against them. + retiredDNSForwarders []*tailmixdns.Forwarder + socksListener net.Listener + socksRouter *socksproxy.DynamicRouter + networkErr chan error + reconcileErr string + updateMu sync.Mutex + updateWake chan struct{} + updateRestart chan updateRestart leaseActivity effectiveip.ActivityTracker } @@ -522,7 +526,8 @@ func (s *supervisor) reconcileLocked() (err error) { }) } if exitProfileID := activeExitProfile(s.st, statuses); exitProfileID != "" { - subnetRoutes = append(subnetRoutes, + subnetRoutes = append( + subnetRoutes, socksproxy.SubnetRoute{ Prefix: netip.PrefixFrom(netip.IPv4Unspecified(), 0), ProfileID: exitProfileID, Active: true, @@ -713,9 +718,12 @@ func (s *supervisor) configureDNSLocked(cfg tailmixdns.LiveConfig) error { closeForwarders(nextForwarders) return err } - old := s.dnsForwarders + // Rotate the retired set before closing it: the generation that was live + // before this one now takes its place. + retired := s.retiredDNSForwarders + s.retiredDNSForwarders = s.dnsForwarders s.dnsForwarders = nextForwarders - closeForwarders(old) + closeForwarders(retired) return nil } @@ -741,6 +749,8 @@ func (s *supervisor) close() error { } closeForwarders(s.dnsForwarders) s.dnsForwarders = nil + closeForwarders(s.retiredDNSForwarders) + s.retiredDNSForwarders = nil if s.dnsService != nil { _ = s.dnsService.Close() s.dnsService = nil diff --git a/cmd/tailmixd/supervisor_test.go b/cmd/tailmixd/supervisor_test.go index 313d8e7a2..7fafc4ab6 100644 --- a/cmd/tailmixd/supervisor_test.go +++ b/cmd/tailmixd/supervisor_test.go @@ -6,6 +6,7 @@ import ( "errors" "io" "net" + "net/netip" "os" "path/filepath" "strings" @@ -13,6 +14,7 @@ import ( "time" "github.com/maisem/tailmix/controlapi" + tailmixdns "github.com/maisem/tailmix/dns" "github.com/maisem/tailmix/hosttun" tailmixprofile "github.com/maisem/tailmix/profile" "github.com/maisem/tailmix/profilesocket" @@ -20,6 +22,7 @@ import ( "github.com/maisem/tailmix/state" "github.com/maisem/tailmix/wireguardcfg" "github.com/tailscale/wireguard-go/tun" + "golang.org/x/net/dns/dnsmessage" ) type lifecycleEngine struct { @@ -33,9 +36,11 @@ func (e *lifecycleEngine) Close() error { e.closeCalls++ return e.closeErr } + func (e *lifecycleEngine) Dial(context.Context, string, string) (net.Conn, error) { return nil, net.ErrClosed } + func (e *lifecycleEngine) Status(context.Context) (tailmixprofile.Status, error) { return e.status, nil } @@ -314,3 +319,147 @@ func TestSupervisorCloseReportsHostCleanupError(t *testing.T) { t.Fatalf("stderr = %q, want host cleanup failure", stderr.String()) } } + +// profileDNSEngine is a lifecycleEngine whose profile exposes an effective DNS +// resolver, so the supervisor can build a profile DNS forwarder for it. +type profileDNSEngine struct { + lifecycleEngine + response []byte +} + +func (e *profileDNSEngine) QueryDNS(string, dnsmessage.Type) ([]byte, error) { + return e.response, nil +} + +type recordingDNSService struct { + configs []tailmixdns.LiveConfig +} + +func (s *recordingDNSService) Addr() netip.AddrPort { return netip.AddrPort{} } + +func (s *recordingDNSService) Configure([]tailmixdns.Domain, []tailmixdns.Record) error { + return nil +} + +func (s *recordingDNSService) ConfigureFull(cfg tailmixdns.LiveConfig) error { + s.configs = append(s.configs, cfg) + return nil +} + +func (s *recordingDNSService) HandlePacket([]byte) bool { return false } +func (s *recordingDNSService) Outbound() <-chan []byte { return nil } +func (s *recordingDNSService) Err() error { return nil } +func (s *recordingDNSService) Close() error { return nil } + +func dnsTestQuery(t *testing.T, name string) []byte { + t.Helper() + builder := dnsmessage.NewBuilder(nil, dnsmessage.Header{ID: 0x1234, RecursionDesired: true}) + if err := builder.StartQuestions(); err != nil { + t.Fatal(err) + } + if err := builder.Question(dnsmessage.Question{ + Name: dnsmessage.MustNewName(name), Type: dnsmessage.TypeA, Class: dnsmessage.ClassINET, + }); err != nil { + t.Fatal(err) + } + query, err := builder.Finish() + if err != nil { + t.Fatal(err) + } + return query +} + +func dnsTestResponse(t *testing.T, query []byte, addr netip.Addr) []byte { + t.Helper() + var parser dnsmessage.Parser + header, err := parser.Start(query) + if err != nil { + t.Fatal(err) + } + question, err := parser.Question() + if err != nil { + t.Fatal(err) + } + builder := dnsmessage.NewBuilder(nil, dnsmessage.Header{ + ID: header.ID, Response: true, + RecursionDesired: header.RecursionDesired, RecursionAvailable: true, + }) + if err := builder.StartQuestions(); err != nil { + t.Fatal(err) + } + if err := builder.Question(question); err != nil { + t.Fatal(err) + } + if err := builder.StartAnswers(); err != nil { + t.Fatal(err) + } + resourceHeader := dnsmessage.ResourceHeader{ + Name: question.Name, Type: dnsmessage.TypeA, Class: dnsmessage.ClassINET, TTL: 60, + } + if err := builder.AResource(resourceHeader, dnsmessage.AResource{A: addr.As4()}); err != nil { + t.Fatal(err) + } + response, err := builder.Finish() + if err != nil { + t.Fatal(err) + } + return response +} + +func TestConfigureDNSKeepsReplacedForwardersServing(t *testing.T) { + s := newLifecycleTestSupervisor(t, lifecycleTestState()) + query := dnsTestQuery(t, "db.work.ts.net.") + response := dnsTestResponse(t, query, netip.MustParseAddr("100.127.0.7")) + s.runtimes["p_work"] = &managedProfile{ + runtime: runtimeProfile{Engine: &profileDNSEngine{response: response}}, + } + s.dnsService = &recordingDNSService{} + + first := tailmixdns.LiveConfig{Routes: []tailmixdns.Route{{ + Suffix: "work.ts.net.", ProfileID: "p_work", ProfileDNS: true, + }}} + if err := s.configureDNSLocked(first); err != nil { + t.Fatal(err) + } + if len(s.dnsForwarders) != 1 { + t.Fatalf("forwarders = %d, want 1", len(s.dnsForwarders)) + } + replaced := s.dnsForwarders[0] + + second := tailmixdns.LiveConfig{Routes: []tailmixdns.Route{{ + Suffix: "other.ts.net.", ProfileID: "p_work", ProfileDNS: true, + }}} + if err := s.configureDNSLocked(second); err != nil { + t.Fatal(err) + } + if len(s.dnsForwarders) != 1 || s.dnsForwarders[0] == replaced { + t.Fatalf("forwarders = %v, want a replacement", s.dnsForwarders) + } + + // The aggregate resolver can have queries in flight against the previous + // forwarder, so it must keep answering until the next reconfiguration. + conn, err := net.Dial("udp", replaced.Resolver().Addr) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if err := conn.SetDeadline(time.Now().Add(2 * time.Second)); err != nil { + t.Fatal(err) + } + if _, err := conn.Write(query); err != nil { + t.Fatal(err) + } + buffer := make([]byte, 512) + n, err := conn.Read(buffer) + if err != nil { + t.Fatalf("replaced forwarder did not answer: %v", err) + } + var parser dnsmessage.Parser + header, err := parser.Start(buffer[:n]) + if err != nil { + t.Fatal(err) + } + if !header.Response || header.RCode != dnsmessage.RCodeSuccess || header.ID != 0x1234 { + t.Fatalf("response header = %+v, want a successful answer", header) + } +} diff --git a/dns/forwarder.go b/dns/forwarder.go index 3beca950d..f58fd28f8 100644 --- a/dns/forwarder.go +++ b/dns/forwarder.go @@ -19,6 +19,12 @@ import ( "tailscale.com/types/dnstype" ) +// forwarderQueryTimeout bounds one upstream exchange. Two clients wait on this +// endpoint: the Tailscale DNS manager gives up after ten seconds, and the host +// resolver (systemd-resolved) retries after five. Answering SERVFAIL within +// three seconds keeps both of them from seeing a timeout. +const forwarderQueryTimeout = 3 * time.Second + type ProfileDialer interface { Dial(context.Context, string, string) (net.Conn, error) } @@ -103,6 +109,36 @@ func (f *Forwarder) Close() error { return result } +// servfailResponse builds a SERVFAIL response for query. It returns nil when +// query is not a DNS query that can be answered. +func servfailResponse(query []byte) []byte { + var parser dnsmessage.Parser + header, err := parser.Start(query) + if err != nil { + return nil + } + questions, err := parser.AllQuestions() + if err != nil { + return nil + } + message := dnsmessage.Message{ + Header: dnsmessage.Header{ + ID: header.ID, + Response: true, + OpCode: header.OpCode, + RecursionDesired: header.RecursionDesired, + RecursionAvailable: true, + RCode: dnsmessage.RCodeServerFailure, + }, + Questions: questions, + } + response, err := message.Pack() + if err != nil { + return nil + } + return response +} + func (f *Forwarder) serveUDP() { defer f.wg.Done() buffer := make([]byte, 64<<10) @@ -115,12 +151,16 @@ func (f *Forwarder) serveUDP() { f.wg.Add(1) go func() { defer f.wg.Done() - ctx, cancel := context.WithTimeout(f.ctx, 10*time.Second) + ctx, cancel := context.WithTimeout(f.ctx, forwarderQueryTimeout) defer cancel() response, err := f.exchange(ctx, "udp", query) - if err == nil { - _, _ = f.udp.WriteToUDP(response, peer) + if err != nil { + response = servfailResponse(query) + if response == nil { + return + } } + _, _ = f.udp.WriteToUDP(response, peer) }() } } @@ -146,10 +186,16 @@ func (f *Forwarder) serveTCP() { if _, err := io.ReadFull(conn, query); err != nil { return } - ctx, cancel := context.WithTimeout(f.ctx, 10*time.Second) + ctx, cancel := context.WithTimeout(f.ctx, forwarderQueryTimeout) response, err := f.exchange(ctx, "tcp", query) cancel() - if err != nil || len(response) > 65535 { + if err != nil { + response = servfailResponse(query) + if response == nil { + return + } + } + if len(response) > 65535 { return } if err := binary.Write(conn, binary.BigEndian, uint16(len(response))); err != nil { @@ -212,10 +258,28 @@ func (f *Forwarder) exchangeProfileDNS(ctx context.Context, query []byte) ([]byt if questions[0].Class != dnsmessage.ClassINET { return nil, fmt.Errorf("profile DNS query uses unsupported class %v", questions[0].Class) } - response, err := f.queryer.QueryDNS(questions[0].Name.String(), questions[0].Type) - if err != nil { - return nil, err + // QueryDNS has no context parameter and the profile's own DNS manager can + // hold a query for ten seconds. Abandon it on the caller's deadline so the + // client receives a SERVFAIL instead of waiting out its own timeout. + type profileResult struct { + response []byte + err error } + results := make(chan profileResult, 1) + go func() { + response, err := f.queryer.QueryDNS(questions[0].Name.String(), questions[0].Type) + results <- profileResult{response: response, err: err} + }() + var result profileResult + select { + case <-ctx.Done(): + return nil, ctx.Err() + case result = <-results: + } + if result.err != nil { + return nil, result.err + } + response := result.response if len(response) < 2 { return nil, errors.New("profile DNS resolver returned a truncated header") } @@ -234,7 +298,11 @@ func (f *Forwarder) exchangeClassic(ctx context.Context, network string, resolve return nil, err } defer conn.Close() - _ = conn.SetDeadline(time.Now().Add(10 * time.Second)) + if deadline, ok := ctx.Deadline(); ok { + _ = conn.SetDeadline(deadline) + } else { + _ = conn.SetDeadline(time.Now().Add(forwarderQueryTimeout)) + } if network == "tcp" { if len(query) > 65535 { return nil, errors.New("DNS query is too large") diff --git a/dns/forwarder_test.go b/dns/forwarder_test.go index 49e5418cb..b450c27d1 100644 --- a/dns/forwarder_test.go +++ b/dns/forwarder_test.go @@ -3,6 +3,7 @@ package dns import ( "context" "encoding/binary" + "errors" "io" "net" "net/http" @@ -10,6 +11,7 @@ import ( "strings" "sync" "testing" + "time" "golang.org/x/net/dns/dnsmessage" "tailscale.com/types/dnstype" @@ -112,3 +114,107 @@ func TestForwarderDoHUsesProfileDialerAndBootstrapResolution(t *testing.T) { t.Fatalf("profile dial addresses = %q", dialer.addrs) } } + +type failingDNSQueryer struct{} + +func (failingDNSQueryer) QueryDNS(string, dnsmessage.Type) ([]byte, error) { + return nil, errors.New("profile DNS unavailable") +} + +func buildForwarderTestQuery(t *testing.T, name dnsmessage.Name, queryType dnsmessage.Type) []byte { + t.Helper() + builder := dnsmessage.NewBuilder(nil, dnsmessage.Header{ID: 0xbeef, RecursionDesired: true}) + if err := builder.StartQuestions(); err != nil { + t.Fatal(err) + } + if err := builder.Question(dnsmessage.Question{ + Name: name, Type: queryType, Class: dnsmessage.ClassINET, + }); err != nil { + t.Fatal(err) + } + query, err := builder.Finish() + if err != nil { + t.Fatal(err) + } + return query +} + +func queryForwarder(t *testing.T, addr netip.AddrPort, query []byte) dnsmessage.Header { + t.Helper() + conn, err := net.Dial("udp", addr.String()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if err := conn.SetDeadline(time.Now().Add(2 * time.Second)); err != nil { + t.Fatal(err) + } + if _, err := conn.Write(query); err != nil { + t.Fatal(err) + } + buffer := make([]byte, 512) + n, err := conn.Read(buffer) + if err != nil { + t.Fatalf("no DNS response: %v", err) + } + var parser dnsmessage.Parser + header, err := parser.Start(buffer[:n]) + if err != nil { + t.Fatal(err) + } + return header +} + +func TestForwarderAnswersSERVFAILWhenProfileDNSFails(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + forwarder, err := StartProfileDNSForwarder(ctx, failingDNSQueryer{}) + if err != nil { + t.Fatal(err) + } + defer forwarder.Close() + + header := queryForwarder(t, forwarder.addr, buildForwarderTestQuery(t, dnsmessage.MustNewName("example.com."), dnsmessage.TypeA)) + if header.ID != 0xbeef { + t.Fatalf("response ID = %#x, want %#x", header.ID, 0xbeef) + } + if !header.Response || header.RCode != dnsmessage.RCodeServerFailure { + t.Fatalf("response header = %+v, want SERVFAIL", header) + } +} + +func TestForwarderAnswersSERVFAILWhenResolverIsUnreachable(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + forwarder, err := StartForwarder(ctx, &networkDialer{}, []*dnstype.Resolver{{Addr: "127.0.0.1:1"}}) + if err != nil { + t.Fatal(err) + } + defer forwarder.Close() + + header := queryForwarder(t, forwarder.addr, buildForwarderTestQuery(t, dnsmessage.MustNewName("example.com."), dnsmessage.TypeA)) + if !header.Response || header.RCode != dnsmessage.RCodeServerFailure { + t.Fatalf("response header = %+v, want SERVFAIL", header) + } +} + +// blockingDNSQueryer models a profile whose DNS manager never answers. +type blockingDNSQueryer struct{} + +func (blockingDNSQueryer) QueryDNS(string, dnsmessage.Type) ([]byte, error) { + select {} +} + +func TestForwarderAbandonsProfileDNSQueryAtDeadline(t *testing.T) { + forwarder := &Forwarder{queryer: blockingDNSQueryer{}} + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + query := buildForwarderTestQuery(t, dnsmessage.MustNewName("example.com."), dnsmessage.TypeA) + start := time.Now() + if _, err := forwarder.exchangeProfileDNS(ctx, query); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("exchangeProfileDNS error = %v, want deadline exceeded", err) + } + if elapsed := time.Since(start); elapsed > time.Second { + t.Fatalf("exchangeProfileDNS returned after %v", elapsed) + } +} diff --git a/dns/packet_service_test.go b/dns/packet_service_test.go index fbf5552dc..79831fcad 100644 --- a/dns/packet_service_test.go +++ b/dns/packet_service_test.go @@ -24,6 +24,7 @@ import ( "tailscale.com/net/netmon" "tailscale.com/net/packet" "tailscale.com/net/tsdial" + "tailscale.com/types/dnstype" "tailscale.com/types/logger" "tailscale.com/util/eventbus" ) @@ -59,6 +60,11 @@ func newTestPacketService(t *testing.T, name string, wantIP netip.Addr) *packetS if err != nil { t.Fatal(err) } + return newPacketServiceWithConfig(t, dnsCfg) +} + +func newPacketServiceWithConfig(t *testing.T, dnsCfg tailscaledns.Config) *packetService { + t.Helper() bus := eventbus.NewWithOptions(eventbus.BusOptions{Logf: logger.Discard}) netMon := netmon.NewStatic() dialer := tsdial.NewDialer(netMon) @@ -317,3 +323,55 @@ func TestPacketServiceAnswersTCPFromBothResolverAddresses(t *testing.T) { }) } } + +// TestPacketServiceAnswersSERVFAILWhenForwarderFails covers the case a host +// resolver hits when a profile cannot answer: the query must produce a +// response, so the host resolver can fail over instead of waiting out its own +// timeout. +func TestPacketServiceAnswersSERVFAILWhenForwarderFails(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + forwarder, err := StartProfileDNSForwarder(ctx, failingDNSQueryer{}) + if err != nil { + t.Fatal(err) + } + defer forwarder.Close() + + dnsCfg, err := configForService(ServiceConfig{ + Routes: []Route{{ + Suffix: "corp.internal.", + ProfileID: "work", + ProfileDNS: true, + Resolvers: []*dnstype.Resolver{forwarder.Resolver()}, + }}, + }) + if err != nil { + t.Fatal(err) + } + service := newPacketServiceWithConfig(t, dnsCfg) + + clientIP := netip.MustParseAddr("100.127.0.10") + query := packet.Generate(packet.UDP4Header{ + IP4Header: packet.IP4Header{Src: clientIP, Dst: testResolverIP}, + SrcPort: 54321, + DstPort: 53, + }, dnsAQuery(t, "db.corp.internal")) + if !service.HandlePacket(query) { + t.Fatalf("UDP packet to %v was not consumed", testResolverIP) + } + select { + case raw := <-service.Outbound(): + var parsed packet.Parsed + parsed.Decode(raw) + var parser dnsmessage.Parser + header, err := parser.Start(parsed.Payload()) + if err != nil { + t.Fatal(err) + } + if header.ID != 1 || !header.Response || header.RCode != dnsmessage.RCodeServerFailure { + t.Fatalf("response header = %+v, want SERVFAIL", header) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for a response from an unavailable profile resolver") + } +} diff --git a/docs/architecture.md b/docs/architecture.md index f017ee2d0..05902660a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -295,7 +295,12 @@ exact-name route without claiming its entire source suffix. For advertised split-DNS and default resolvers, tailmix creates a loopback forwarder tied to the chosen profile. Classic DNS and DNS-over-HTTPS connections -therefore travel through that profile rather than the host's default network. +therefore travel through that profile rather than the host's default network. A +forwarder whose upstream does not answer returns SERVFAIL within a few seconds +instead of dropping the query, so the OS resolver can fail over rather than wait +out its own timeout. A forwarder replaced by a reconfiguration keeps answering +until the next one, because the aggregate resolver can still have queries in +flight against it. On macOS, the Tailscale DNS manager installs native split-DNS resolver entries. When a root DNS route is active, it installs a synthetic System Configuration diff --git a/docs/profile-management.md b/docs/profile-management.md index 2e86c9593..64c2bda2f 100644 --- a/docs/profile-management.md +++ b/docs/profile-management.md @@ -304,6 +304,12 @@ In SOCKS mode, the selected exit node is the fallback for otherwise-unmatched IP literals and DNS names. Explicit IP and DNS policy still wins. Clearing the selection removes the fallback in both modes. +The exit node is also the DNS authority for queries that no other route +covers, so tailmix resolves those queries through the selected profile's own +DNS service. A profile that carries no DNS service of its own, such as a raw +WireGuard profile, does not become that authority: the host keeps resolving +with its own resolvers, and their traffic already leaves through the exit node. + Disabling the selected profile withdraws its default route but retains the desired selection, which resumes when the same profile is enabled. Removing the selected profile clears the selection. There is no automatic failover to diff --git a/hosttun/hosttun_linux.go b/hosttun/hosttun_linux.go index f0b880e92..ef3cdba86 100644 --- a/hosttun/hosttun_linux.go +++ b/hosttun/hosttun_linux.go @@ -23,8 +23,30 @@ import ( const ( tailmixExitRouteTable = 527 tailmixExitRulePref = 5300 + + // linuxIPv6RouteMetric is the metric the Linux kernel records for an IPv6 + // route added without an explicit metric (IP6_RT_PRIO_USER). Tailmix sets it + // explicitly, so a route read back from the kernel has the metric tailmix + // expects. IPv4 routes use metric 0. + linuxIPv6RouteMetric = 1024 ) +// linuxRouteMetric returns the metric tailmix assigns to its routes of the given +// address family. +func linuxRouteMetric(family int) int { + if family == netlink.FAMILY_V6 { + return linuxIPv6RouteMetric + } + return 0 +} + +func linuxRouteFamily(destination netip.Prefix) int { + if destination.Addr().Is6() { + return netlink.FAMILY_V6 + } + return netlink.FAMILY_V4 +} + type linuxNetlinkOps struct { addrReplace func(netlink.Link, *netlink.Addr) error addrDel func(netlink.Link, *netlink.Addr) error @@ -193,6 +215,13 @@ func (h *linuxHost) readInstalledState(wantRoutes []Route) (linuxInstalledState, state.localAddrs = append(state.localAddrs, prefix) } } + // The kernel installs a route of its own for each address on the device. + // Tailmix manages only the routes it plans, and a plan never targets a local + // address, so these kernel routes must not count as stale or unexpected. + localAddrs := make(map[netip.Addr]bool, len(state.localAddrs)) + for _, prefix := range state.localAddrs { + localAddrs[prefix.Addr()] = true + } wantByDestination := make(map[netip.Prefix]Route, len(wantRoutes)) for _, route := range wantRoutes { wantByDestination[route.Destination] = route @@ -202,7 +231,7 @@ func (h *linuxHost) readInstalledState(wantRoutes []Route) (linuxInstalledState, if err != nil { return linuxInstalledState{}, fmt.Errorf("list Linux TUN %s routes: %w", h.name, err) } - state.mainRoutes = appendLinuxRoutes(state.mainRoutes, mainRoutes, family, wantByDestination) + state.mainRoutes = appendLinuxRoutes(state.mainRoutes, mainRoutes, family, wantByDestination, localAddrs) tableRoutes, err := netlink.RouteListFiltered(family, &netlink.Route{ LinkIndex: h.link.Attrs().Index, @@ -211,7 +240,7 @@ func (h *linuxHost) readInstalledState(wantRoutes []Route) (linuxInstalledState, if err != nil { return linuxInstalledState{}, fmt.Errorf("list Linux TUN %s routes in table %d: %w", h.name, tailmixExitRouteTable, err) } - state.tableRoutes = appendLinuxRoutes(state.tableRoutes, tableRoutes, family, wantByDestination) + state.tableRoutes = appendLinuxRoutes(state.tableRoutes, tableRoutes, family, wantByDestination, localAddrs) rules, err := netlink.RuleList(family) if err != nil { @@ -229,16 +258,16 @@ func (h *linuxHost) readInstalledState(wantRoutes []Route) (linuxInstalledState, return state, nil } -func appendLinuxRoutes(dst []linuxInstalledRoute, routes []netlink.Route, family int, want map[netip.Prefix]Route) []linuxInstalledRoute { +func appendLinuxRoutes(dst []linuxInstalledRoute, routes []netlink.Route, family int, want map[netip.Prefix]Route, localAddrs map[netip.Addr]bool) []linuxInstalledRoute { for _, route := range routes { - if installed, ok := linuxRouteFromNetlink(route, family, want); ok { + if installed, ok := linuxRouteFromNetlink(route, family, want, localAddrs); ok { dst = append(dst, installed) } } return dst } -func linuxRouteFromNetlink(route netlink.Route, family int, want map[netip.Prefix]Route) (linuxInstalledRoute, bool) { +func linuxRouteFromNetlink(route netlink.Route, family int, want map[netip.Prefix]Route, localAddrs map[netip.Addr]bool) (linuxInstalledRoute, bool) { var destination netip.Prefix if route.Dst == nil { if family == netlink.FAMILY_V6 { @@ -253,13 +282,17 @@ func linuxRouteFromNetlink(route netlink.Route, family int, want map[netip.Prefi return linuxInstalledRoute{}, false } } + destination = destination.Masked() + if localAddrs[destination.Addr()] { + return linuxInstalledRoute{}, false + } source, _ := netip.AddrFromSlice(route.Src) source = source.Unmap() installed := linuxInstalledRoute{ - Route: Route{Destination: destination.Masked(), Source: source}, + Route: Route{Destination: destination, Source: source}, controlled: len(route.Gw) == 0 && len(route.MultiPath) == 0 && route.Via == nil && route.Encap == nil && (route.Type == 0 || route.Type == unix.RTN_UNICAST) && route.Scope == netlink.SCOPE_UNIVERSE && - route.Priority == 0 && route.Tos == 0, + route.Priority == linuxRouteMetric(family) && route.Tos == 0, } if desired, ok := want[installed.Destination]; ok { installed.Exit = desired.Exit @@ -483,6 +516,7 @@ func (h *linuxHost) replaceRouteInTable(route Route, table int) error { Dst: netipx.PrefixIPNet(route.Destination), Src: net.IP(route.Source.AsSlice()), Table: table, + Priority: linuxRouteMetric(linuxRouteFamily(route.Destination)), }); err != nil { return fmt.Errorf("route %v through Linux TUN %s: %w", route.Destination, h.name, err) } diff --git a/hosttun/hosttun_linux_test.go b/hosttun/hosttun_linux_test.go index 24c92f33b..07bcf80ea 100644 --- a/hosttun/hosttun_linux_test.go +++ b/hosttun/hosttun_linux_test.go @@ -4,11 +4,14 @@ package hosttun import ( "fmt" + "net" "net/netip" "strings" "testing" "github.com/tailscale/netlink" + "go4.org/netipx" + "golang.org/x/sys/unix" ) func noOpLinuxOps() linuxNetlinkOps { @@ -136,3 +139,51 @@ func TestLinuxConfigureRemovesObservedStaleRoute(t *testing.T) { t.Fatalf("deleted routes = %v, want %v", deleted, stale.Destination) } } + +// TestLinuxConfigureAcceptsKernelInstalledState builds the state that the Linux +// kernel reports for a device that tailmix already configured: IPv6 routes carry +// the kernel's default IPv6 metric, and the kernel adds a route of its own for +// each address on the device. Configure must accept that state and leave the +// kernel's own route alone. +func TestLinuxConfigureAcceptsKernelInstalledState(t *testing.T) { + source := netip.MustParseAddr("fd6d:6e65:7400::1") + local := netip.PrefixFrom(source, 128) + route := Route{Destination: netip.MustParsePrefix("fd6d:6e65:7400::2/128"), Source: source} + wantByDestination := map[netip.Prefix]Route{route.Destination: route} + localAddrs := map[netip.Addr]bool{source: true} + mainRoutes := appendLinuxRoutes(nil, []netlink.Route{ + { + LinkIndex: 42, + Dst: netipx.PrefixIPNet(local), + Protocol: unix.RTPROT_KERNEL, + Type: unix.RTN_UNICAST, + Scope: netlink.SCOPE_UNIVERSE, + Priority: 256, + }, + { + LinkIndex: 42, + Dst: netipx.PrefixIPNet(route.Destination), + Src: net.IP(source.AsSlice()), + Protocol: unix.RTPROT_BOOT, + Type: unix.RTN_UNICAST, + Scope: netlink.SCOPE_UNIVERSE, + Priority: linuxIPv6RouteMetric, + }, + }, netlink.FAMILY_V6, wantByDestination, localAddrs) + installed := linuxInstalledState{localAddrs: []netip.Prefix{local}, mainRoutes: mainRoutes} + h := testLinuxHost(installed, installed) + var deleted []string + h.ops.routeDel = func(route *netlink.Route) error { + deleted = append(deleted, route.Dst.String()) + return nil + } + cfg := Config{LocalAddrs: []netip.Prefix{local}, Routes: []Route{route}} + if err := h.Configure(cfg); err != nil { + t.Fatalf("Configure error = %v, want nil", err) + } + for _, deletedRoute := range deleted { + if deletedRoute == local.String() { + t.Fatalf("Configure deleted the kernel's own route for %v", local) + } + } +} diff --git a/profile/profile.go b/profile/profile.go index bb0e6e484..c82c55741 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -28,10 +28,13 @@ type Update struct { } type Status struct { - ProfileID string - Alias string - Kind string - MagicDNSSuffix string + ProfileID string + Alias string + Kind string + MagicDNSSuffix string + // ProfileDNS reports whether the engine resolves queries through the + // profile's own DNS service. + ProfileDNS bool BackendState string AuthURL string SelfNodeID string diff --git a/profile/tsnetengine.go b/profile/tsnetengine.go index dca60fdfe..df3a76276 100644 --- a/profile/tsnetengine.go +++ b/profile/tsnetengine.go @@ -277,6 +277,7 @@ func (e *TSNetEngine) Status(ctx context.Context) (Status, error) { Alias: e.cfg.Alias, Kind: "tailscale", MagicDNSSuffix: suffix, + ProfileDNS: true, BackendState: st.BackendState, AuthURL: st.AuthURL, SelfNodeID: selfNode, diff --git a/routingpolicy/policy.go b/routingpolicy/policy.go index 40fda3bdc..a964dc97c 100644 --- a/routingpolicy/policy.go +++ b/routingpolicy/policy.go @@ -398,8 +398,12 @@ func activeDNSExitProfile(st state.State, statuses []profile.Status) string { return "" } for _, status := range statuses { + // A profile without a DNS service of its own, such as a raw WireGuard + // profile, cannot answer for the exit node. The host keeps resolving + // with its own resolvers, which the exit node already routes. if status.ProfileID == st.ExitNode.ProfileID && profileAvailable(status) && + status.ProfileDNS && status.ExitNodeID == st.ExitNode.NodeID { return status.ProfileID } diff --git a/routingpolicy/policy_test.go b/routingpolicy/policy_test.go index 9851fc19f..f8d762848 100644 --- a/routingpolicy/policy_test.go +++ b/routingpolicy/policy_test.go @@ -117,7 +117,7 @@ func TestExplicitDNSDefaultOverridesAndSurvivesExitNodeDefault(t *testing.T) { }, } statuses := []profile.Status{ - {ProfileID: "work-id", BackendState: "Running", ExitNodeID: "exit-node"}, + {ProfileID: "work-id", BackendState: "Running", ExitNodeID: "exit-node", ProfileDNS: true}, {ProfileID: "dns-id", BackendState: "Running", DNSRoutes: []profile.DNSRouteStatus{{ Domain: ".", Source: "default", Resolvers: []*dnstype.Resolver{{Addr: "9.9.9.9"}}, }}}, @@ -194,7 +194,7 @@ func TestExitNodeAddsAutomaticEffectiveProfileDNSDefault(t *testing.T) { }, } status := profile.Status{ - ProfileID: "work-id", BackendState: "Running", ExitNodeID: "exit-node", + ProfileID: "work-id", BackendState: "Running", ExitNodeID: "exit-node", ProfileDNS: true, } plan := BuildDNS(st, []profile.Status{status}) resolved, ok := plan.Resolve("example.com") @@ -213,6 +213,24 @@ func TestExitNodeAddsAutomaticEffectiveProfileDNSDefault(t *testing.T) { } } +func TestExitNodeWithoutProfileDNSAddsNoAutomaticDNSRoute(t *testing.T) { + st := state.State{ + Profiles: []state.Profile{{ID: "wg-id", Name: "wg"}}, + ExitNode: &state.ExitNode{ + ProfileID: "wg-id", + NodeID: "gateway", + PeerIP: netip.MustParseAddr("10.80.0.1"), + }, + } + status := profile.Status{ + ProfileID: "wg-id", Kind: "wireguard", BackendState: "Running", ExitNodeID: "gateway", + } + plan := BuildDNS(st, []profile.Status{status}) + if _, ok := plan.Resolve("example.com"); ok { + t.Fatalf("exit-node DNS route for a profile without its own DNS service: %+v", plan.Automatic) + } +} + func TestDNSContainsUsesTailscaleDNSNameRules(t *testing.T) { if !DNSContains("_service._tcp.example.com.", "Node._Service._TCP.Example.COM") { t.Fatal("expected dnsname-valid service labels to match case-insensitively")