From 12a6b40487ccd1d52b99e4bb27f7e2fbb028d1a8 Mon Sep 17 00:00:00 2001 From: nhubao Date: Sun, 11 Oct 2026 22:42:42 +0700 Subject: [PATCH 1/6] hosttun: fix Linux TUN route verification The Linux TUN reconciliation added in 74e5e8f reads route state back from the kernel and verifies it against the plan. Two kernel behaviors made that verification reject every IPv6 route, so tailmixd could not configure the aggregate TUN at all: - The kernel records IP6_RT_PRIO_USER (1024) as the metric of an IPv6 route that is added without an explicit metric, so every route tailmix installed read back as an unexpected route and was reinstalled on each Configure. - The kernel adds a route of its own for each address on the device. For IPv6 that route lands in the main table, so verification reported it as an unexpected route and the next reconciliation deleted it as stale. Install routes with an explicit family metric, expect that metric when classifying kernel routes, and ignore kernel routes that target one of the device's own addresses. IPv4 routes keep metric 0 and are unaffected. Change-Id: Ibb491e59c8eced53f85bb1a5cdbfb3adfc581656 --- hosttun/hosttun_linux.go | 48 ++++++++++++++++++++++++++++----- hosttun/hosttun_linux_test.go | 51 +++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 7 deletions(-) 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) + } + } +} From ebe6474108d2c839321e1f206fb5c86111e8b6f9 Mon Sep 17 00:00:00 2001 From: nhubao Date: Mon, 12 Oct 2026 09:25:01 +0700 Subject: [PATCH 2/6] dns,cmd/tailmixd: answer SERVFAIL when a DNS forwarder fails A profile DNS forwarder that could not get an answer from its upstream dropped the query. The aggregate Tailscale resolver then waited out its own ten second timeout, and the host resolver behind it waited again, so one unanswerable name stalled host DNS. The reported "waiting for response or error from [127.0.0.1:50690]: context deadline exceeded" lines came from that path. Bound each exchange to three seconds, answer SERVFAIL when it fails or expires, and abandon a profile DNS query at the caller's deadline. QueryDNS takes no context, so the query now runs in a goroutine that the deadline can leave behind. A SERVFAIL response lets the OS resolver fail over instead of waiting, and the Tailscale resolver maps it to a SERVFAIL answer of its own. A reconfiguration also replaced every forwarder and closed the old ones immediately. Queries the aggregate resolver issued just before the switch were still in flight against the closed loopback address and never got an answer, which is how the reported timeouts arose during startup. Keep the replaced forwarders open until the next reconfiguration instead. RELNOTE: Fix host DNS stalls when a profile DNS resolver does not answer. Change-Id: I7b0d0f4a0e2b3c6d9f8a1e5c4b7d2a3f6c9e0b18 --- cmd/tailmixd/supervisor.go | 28 ++++-- cmd/tailmixd/supervisor_test.go | 149 ++++++++++++++++++++++++++++++++ dns/forwarder.go | 86 ++++++++++++++++-- dns/forwarder_test.go | 106 +++++++++++++++++++++++ docs/architecture.md | 7 +- 5 files changed, 357 insertions(+), 19 deletions(-) diff --git a/cmd/tailmixd/supervisor.go b/cmd/tailmixd/supervisor.go index fb9eb4e53..049a252c8 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 } @@ -713,9 +717,13 @@ func (s *supervisor) configureDNSLocked(cfg tailmixdns.LiveConfig) error { closeForwarders(nextForwarders) return err } - old := s.dnsForwarders + // Keep the replaced forwarders open until the next reconfiguration: the + // aggregate resolver can still have queries in flight against them, and a + // closed endpoint leaves those clients waiting out their own timeout. + 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/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 From 3e7a5ac1074f03e5ddc711336e218e0c35eabf89 Mon Sep 17 00:00:00 2001 From: nhubao Date: Mon, 12 Oct 2026 09:28:25 +0700 Subject: [PATCH 3/6] dns: cover the SERVFAIL a host resolver receives The aggregate resolver turns a failed profile forwarder into a SERVFAIL response for the host, so a name the tailnet cannot answer no longer leaves the host resolver waiting out its own timeout. Cover that path end to end: a packet service built from a route whose forwarder cannot reach its profile DNS must answer SERVFAIL instead of nothing. The test fails without the forwarder change, where the query produced no response at all. Factor the packet service test harness so a test can supply its own resolver configuration. Change-Id: I3c5e1d9a7b2f4c8e0a6d1f3b5c7e9a2d4f6b8c01 --- dns/packet_service_test.go | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) 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") + } +} From 3fb0b00432f875b0de648d1a8961fa1ba330427f Mon Sep 17 00:00:00 2001 From: nhubao Date: Mon, 12 Oct 2026 09:32:50 +0700 Subject: [PATCH 4/6] cmd/tailmixd: wrap the exit route append call Put the append call and its arguments on separate lines, matching the surrounding subnet route entries. No behavior change. Change-Id: I8d2f4b6a1c3e5f7092b4d6a8c0e2f4b6d8a0c2e4 --- cmd/tailmixd/supervisor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/tailmixd/supervisor.go b/cmd/tailmixd/supervisor.go index 049a252c8..3be38780e 100644 --- a/cmd/tailmixd/supervisor.go +++ b/cmd/tailmixd/supervisor.go @@ -526,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, From 3daee7819f70669f7fa350d2ad773c7051c2c303 Mon Sep 17 00:00:00 2001 From: nhubao Date: Mon, 12 Oct 2026 09:44:34 +0700 Subject: [PATCH 5/6] cmd/tailmixd: shorten the retired forwarder comment The comment above the retire swap repeated, almost verbatim, the reason recorded at the retiredDNSForwarders field. Keep the invariant in one place and describe the local rotation order at the call site instead. Change-Id: I2a7c4e6b8d0f1a3c5e7b9d1f3a5c7e9b0d2f4a68 --- cmd/tailmixd/supervisor.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/tailmixd/supervisor.go b/cmd/tailmixd/supervisor.go index 3be38780e..1aaa6f1c4 100644 --- a/cmd/tailmixd/supervisor.go +++ b/cmd/tailmixd/supervisor.go @@ -718,9 +718,8 @@ func (s *supervisor) configureDNSLocked(cfg tailmixdns.LiveConfig) error { closeForwarders(nextForwarders) return err } - // Keep the replaced forwarders open until the next reconfiguration: the - // aggregate resolver can still have queries in flight against them, and a - // closed endpoint leaves those clients waiting out their own timeout. + // 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 From 3c726dc2b6c15ab9e7b0bddf0c122fcce50be7af Mon Sep 17 00:00:00 2001 From: nhubao Date: Mon, 12 Oct 2026 11:30:56 +0700 Subject: [PATCH 6/6] profile,routingpolicy: skip exit-node DNS for profiles without one An exit node is the DNS authority for queries that no other route covers. Selecting a raw WireGuard profile as an exit node therefore planned a profile DNS route for it, and every reconcile failed with "profile %q does not expose its effective DNS resolver". That error is fatal during startup, so a daemon that had the selection persisted could not start, which leaves the exit node unusable. Report the capability through Status instead: the tailscale engine resolves queries through the profile's own DNS service, while a raw WireGuard profile carries no resolver. The DNS policy only makes the selected exit node the DNS authority when its profile has that capability, so the host keeps resolving with its own resolvers, whose traffic already leaves through the exit node. --- cmd/tailmixd/main_test.go | 45 ++++++++++++++++++++++++++++++++++ cmd/tailmixd/policyapi_test.go | 4 ++- docs/profile-management.md | 6 +++++ profile/profile.go | 11 ++++++--- profile/tsnetengine.go | 1 + routingpolicy/policy.go | 4 +++ routingpolicy/policy_test.go | 22 +++++++++++++++-- 7 files changed, 86 insertions(+), 7 deletions(-) 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/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/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")