TUN inbound: Support autoSystemDnsToGateway on FreeBSD as well (#7123)

https://github.com/XTLS/Xray-core/issues/6454#issuecomment-5859521287
https://github.com/XTLS/Xray-core/pull/6691#issuecomment-5445818753
https://github.com/XTLS/Xray-core/pull/6773 & https://github.com/XTLS/Xray-core/pull/6853

infra/conf/tun.go

@@ -59,7 +59,7 @@ func (v *TunConfig) Build() (proto.Message, error) {
 		if slices.Contains(config.AutoSystemWfpBlockLeak, "dns") && len(v.DNS) == 0 {
 			return nil, errors.New(`autoSystemWfpBlockLeak "dns" needs dns to be set`)
 		}
-	case "linux":
+	case "linux", "freebsd":
 		if v.AutoSystemDnsToGateway && len(v.Gateway) == 0 {
 			return nil, errors.New("autoSystemDnsToGateway needs gateway to be set")
 		}

infra/conf/tun_test.go

@@ -3,6 +3,7 @@ package conf_test
 import (
 	"encoding/json"
 	"runtime"
+	"slices"
 	"testing"
 
 	. "github.com/xtls/xray-core/infra/conf"
@@ -43,18 +44,18 @@ func TestTunConfigAutoSystem(t *testing.T) {
 func TestTunConfigAutoSystemNeeds(t *testing.T) {
 	for _, c := range []struct {
 		input string
-		goos  string // where it is rejected
+		goos  []string // where it is rejected
 	}{
-		{`{"name": "xray0", "autoSystemWfpBlockLeak": ["misconfigtun"]}`, "windows"},
+		{`{"name": "xray0", "autoSystemWfpBlockLeak": ["misconfigtun"]}`, []string{"windows"}},
-		{`{"name": "xray0", "autoSystemRoutingTable": ["0.0.0.0/0"], "autoSystemWfpBlockLeak": ["misconfigtun"]}`, ""},
+		{`{"name": "xray0", "autoSystemRoutingTable": ["0.0.0.0/0"], "autoSystemWfpBlockLeak": ["misconfigtun"]}`, nil},
-		{`{"name": "xray0", "autoSystemRoutingTable": ["0.0.0.0/0"], "autoSystemWfpBlockLeak": ["dns"]}`, "windows"},
+		{`{"name": "xray0", "autoSystemRoutingTable": ["0.0.0.0/0"], "autoSystemWfpBlockLeak": ["dns"]}`, []string{"windows"}},
-		{`{"name": "xray0", "autoSystemDnsToGateway": true}`, "linux"},
+		{`{"name": "xray0", "autoSystemDnsToGateway": true}`, []string{"linux", "freebsd"}},
 	} {
 		config := new(TunConfig)
 		if err := json.Unmarshal([]byte(c.input), config); err != nil {
 			t.Fatal(err)
 		}
-		if _, err := config.Build(); (err != nil) != (runtime.GOOS == c.goos) {
+		if _, err := config.Build(); (err != nil) != slices.Contains(c.goos, runtime.GOOS) {
 			t.Errorf("%s on %s: error = %v", c.input, runtime.GOOS, err)
 		}
 	}

proxy/tun/README.md

@@ -15,17 +15,17 @@ Plainly enabling it in the config probably will result nothing, or lock your rou
 ## DETAILS
 
 By default, enabling the feature will only bring the tun interface up. \
-When configured explicitly, Windows and Linux can apply interface addresses from `gateway`, while macOS and FreeBSD use the first IPv4 prefix from `gateway` for the point-to-point address. \
+When configured explicitly, Windows and Linux can apply interface addresses from `gateway`, while macOS uses the first IPv4 prefix from `gateway` for the point-to-point address and FreeBSD for the interface address, the one after the gateway. \
 Without `gateway`, the systems differ: Xray assigns no address on Linux, Windows gives the interface link-local addresses itself (an IPv6 one at once, an IPv4 one from `169.254.0.0/16` after a few seconds), and macOS and FreeBSD use `169.254.10.1/30`. \
-Windows, Linux and macOS can also apply system routes from `autoSystemRoutingTable`.
+Windows, Linux, macOS and FreeBSD can also apply system routes from `autoSystemRoutingTable`.
-macOS does not configure system DNS from the `dns` field, and neither does Linux by default; system DNS remains managed by the OS or distribution-specific network services. \
+macOS does not configure system DNS from the `dns` field, and neither do Linux and FreeBSD by default; system DNS remains managed by the OS or distribution-specific network services. \
 For more advanced routing policies or rules, OS level configuration can still manage the named interface (e.g. xray0) when it appears.
 This keeps complex system level routing and rules in a single place of responsibility - the OS itself. \
 Examples of how to achieve this on a simple Linux system (Ubuntu with systemd-networkd) can be found at the end of this README.
 
-### SYSTEM DNS ON LINUX (`autoSystemDnsToGateway`)
+### SYSTEM DNS ON LINUX AND FREEBSD (`autoSystemDnsToGateway`)
 
-On Linux, setting `autoSystemDnsToGateway` to `true` lets the inbound point the system resolver at the tun interface, so name lookups resolve through Xray instead of going out over the physical link. It is off by default, and it is Linux-only.
+On Linux and FreeBSD, setting `autoSystemDnsToGateway` to `true` lets the inbound point the system resolver at the tun interface, so name lookups resolve through Xray instead of going out over the physical link. It is off by default, and it exists only on those two systems. This section describes Linux; the FreeBSD differences follow in [SYSTEM DNS ON FREEBSD](#system-dns-on-freebsd).
 
 It uses `resolvectl`, which means it only works when all of these hold. Where Xray can tell that one does not, it does not start:
 
@@ -66,6 +66,24 @@ Where it cannot apply, Xray does not start, rather than run with the leak descri
 
 On `Close()` the setting is reverted. It is **not** reverted if the process is killed with `SIGKILL`, since a process cannot handle that signal; run `resolvectl revert <iface>` to clean up by hand. An application that brings its own DNS endpoint is unaffected either way — this only covers the system resolver.
 
+### SYSTEM DNS ON FREEBSD
+
+On FreeBSD the same option goes through `resolvconf(8)`, the openresolv that ships in the base system, instead of `resolvectl`. The checks on Xray's own `dns` and routing configuration above apply unchanged, as does the loop through a `dns` upstream that bootstraps over the system resolver.
+
+The address handed over is the gateway itself, not the next address: on FreeBSD the interface gets the address after the first IPv4 `gateway` (as on macOS), and the gateway, which nothing owns, is routed into the TUN, so a query to it is what Xray answers (e.g. `169.254.10.1/30` -> resolver `169.254.10.1`, local address `169.254.10.2`). Without `gateway` the config is rejected, as on Linux.
+
+The entry is added with `resolvconf -a <iface> -x`, which makes the TUN's nameserver the only one in use, the equivalent of `domain ~.` plus `default-route true` above; without `-x` the nameservers DHCP supplied stay listed alongside it, and the system falls back to them. `resolvconf` exits successfully, and still records the entry, when `resolvconf=NO` is set in `resolvconf.conf`, but then runs none of the subscribers that write the resolver files, so Xray confirms the takeover in those files and does not start when the nameserver reached none of them: `/etc/resolv.conf` first, where it also has to be the only nameserver besides loopback ones (`inclusive_interfaces` in `resolvconf.conf` makes `resolvconf` ignore `-x` and keep the DHCP nameservers listed, which would be the leak again), and then, when `resolvconf.conf` names an `unbound_conf` file, that file. The latter is the `local_unbound` layout, where `local-unbound-setup` turns the `libc` subscriber off and has `resolvconf` write unbound's forwarders instead, with `resolv.conf` left pointed at loopback; the system then resolves through unbound, which forwards into the TUN. That only works with unbound's DNSSEC validation turned off, which `local_unbound` has on by default: the extra queries validation makes get no answer through the `dns` outbound, and resolution times out. A drop-in such as `/var/unbound/conf.d/xray.conf` with `server:` and `module-config: "iterator"`, followed by `service local_unbound restart`, turns it off (verified on FreeBSD 15.1).
+
+| Environment | Behaviour |
+|---|---|
+| default FreeBSD, `resolv.conf` managed by `resolvconf` (DHCP or static) | applies |
+| `local_unbound` enabled | applies, through unbound's forwarders; needs validation off (see above) |
+| `resolvconf=NO` in `resolvconf.conf` | no resolver file changes, does not start |
+| `inclusive_interfaces` covering the TUN in `resolvconf.conf` | DHCP nameservers stay listed, does not start |
+| another resolver in front of `resolv.conf` that `resolvconf` is not configured to feed | nameserver reaches no resolver file, does not start |
+
+On `Close()` the entry is removed with `resolvconf -d <iface>`. It is **not** removed if the process is killed with `SIGKILL`; run `resolvconf -d <iface>` to clean up by hand. The next start with the same interface name replaces the stale entry on its own.
+
 Due to this inbound not actually being a proxy, the configuration ignore required listen and port options, and never listen on any port. \
 Here is simple Xray config snippet to enable the inbound:
 ```
@@ -245,10 +263,10 @@ So everything applicable for ipv4 above also works for ipv6, you only need to gi
 FreeBSD support of the same functionality is implemented through tun(4).
 
 Interface name in the configuration must comply to the scheme "tunN", where N is some number. \
-It's necessary to set an IP address to the interface, ex.:
+FreeBSD requires the tun interface to have an IPv4 address before IPv4 routes can use it. \
-```
+By default Xray uses `169.254.10.1/30` as the gateway and assigns the next address in the prefix to the tun interface; the gateway itself belongs to nothing and is routed into the TUN. \
-ifconfig tun0 inet 169.254.10.1/30
+You can override this by setting `gateway`; FreeBSD uses the first IPv4 prefix in the list, and IPv6 `gateway` entries are not used for addressing.
-```
+
 To attach routing to the interface, route command like following can be executed:
 ```
 route add -net 1.1.1.0/24 -iface tun10
@@ -257,6 +275,10 @@ route add -net 1.1.1.0/24 -iface tun10
 route add -inet6 -host 2606:4700:4700::1111 -iface tun10
 route add -inet6 -host 2606:4700:4700::1001 -iface tun10
 ```
+Alternatively, configure `autoSystemRoutingTable` and Xray will add and remove those system routes while it is running. \
+With `autoSystemRoutingTable`, Xray's own connections are kept out of the TUN through `autoOutboundsInterface` (the default), which on FreeBSD switches them to a second routing table (FIB 1) that mirrors the physical default route; this needs the boot tunable `net.fibs` set to at least 2, and Xray does not start otherwise. \
+`autoSystemDnsToGateway` points the system resolver at the TUN through `resolvconf(8)`, see [SYSTEM DNS ON FREEBSD](#system-dns-on-freebsd).
+
 Important to remember that everything written above about Linux routing concept, also apply to FreeBSD. If you simply route default route through tun interface, that will result network loop and immediate network failure.
 
 ## MAC OS X SUPPORT

proxy/tun/tun_freebsd.go

@@ -5,10 +5,13 @@ package tun
 import (
 	"context"
 	"errors"
+	"io/fs"
 	"net"
 	"net/netip"
 	"os"
+	"os/exec"
 	"slices"
+	"strings"
 	"sync"
 	"unsafe"
 
@@ -38,8 +41,17 @@ func procyield(cycles uint32)
 type FreeBSDTun struct {
 	device        tun.Device
 	options       *Config
+	name          string
 	tunIndex      int
 	autoInterface bool
+	// local is the address the system itself has on the TUN; gateway is the
+	// address before it, which nothing owns, so a packet to it is routed into
+	// the TUN (see selectFreeBSDGateway).
+	gateway netip.Prefix
+	local   netip.Addr
+
+	systemDNSSet   bool
+	systemDNSDirty bool
 
 	systemRoutes     []netip.Prefix
 	escapeMu         sync.Mutex
@@ -102,15 +114,19 @@ func NewTun(options *Config) (Tun, error) {
 	return &FreeBSDTun{
 		device:        tunDev,
 		options:       options,
+		name:          name,
 		tunIndex:      iface.Index,
 		autoInterface: options.AutoOutboundsInterface != "",
+		gateway:       gateway,
+		local:         local,
 	}, nil
 }
 
 // selectFreeBSDGateway picks the first IPv4 prefix from the configured gateway
 // list and the local address derived from it (the darwin semantics: the
-// gateway is the remote side of the point-to-point pair, the local address is
+// interface gets the address right after the gateway, and the gateway, which
-// the next one after it), falling back to the same link-local default.
+// nothing owns, is routed into the TUN), falling back to the same link-local
+// default.
 func selectFreeBSDGateway(configured []string) (netip.Prefix, netip.Addr, error) {
 	gateway := netip.MustParsePrefix(defaultFreeBSDGateway)
 	if len(configured) > 0 {
@@ -199,6 +215,7 @@ func (t *FreeBSDTun) Close() error {
 			_ = t.routeMonitor.Close()
 		}
 	})
+	t.unsetSystemDNS()
 	t.unsetEscapeFib()
 	routeErr := t.unsetSystemRoutes()
 	name, nameErr := t.Name()
@@ -212,6 +229,195 @@ func (t *FreeBSDTun) Close() error {
 	return xerrors.Combine(routeErr, closeErr)
 }
 
+// resolvconfRunner runs resolvconf(8) with the given stdin. Overridable for
+// tests.
+var resolvconfRunner = func(stdin string, args ...string) ([]byte, error) {
+	cmd := exec.Command("resolvconf", args...)
+	if stdin != "" {
+		cmd.Stdin = strings.NewReader(stdin)
+	}
+	return cmd.CombinedOutput()
+}
+
+// readResolverFile reads one of the files the applied check looks at.
+// Overridable for tests.
+var readResolverFile = os.ReadFile
+
+// resolvconfApplied reports whether the nameserver resolvconf was handed
+// reached a resolver. resolvconf exits 0 with resolvconf=NO in
+// resolvconf.conf, and still records the entry for the interface, but runs no
+// subscriber, so the exit status and `resolvconf -l` prove nothing; what the
+// subscribers wrote does. The libc subscriber writes /etc/resolv.conf, where
+// the nameserver also has to be the exclusive one. With
+// local_unbound, local-unbound-setup turns that subscriber off and names
+// unbound's forwarders file as unbound_conf instead, with resolv.conf left
+// pointed at loopback, so that file is checked next.
+func resolvconfApplied(addr netip.Addr) (bool, error) {
+	resolvConf, err := readResolverFile("/etc/resolv.conf")
+	if err != nil {
+		return false, err
+	}
+	if exclusiveNameserver(resolvConf, addr) {
+		return true, nil
+	}
+	resolvconfConf, err := readResolverFile("/etc/resolvconf.conf")
+	if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
+			return false, nil
+		}
+		return false, err
+	}
+	if path := shellAssignment(resolvconfConf, "unbound_conf"); path != "" {
+		forwarders, err := readResolverFile(path)
+		if err != nil {
+			return false, err
+		}
+		if hasDirective(forwarders, "forward-addr:", addr) {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+// exclusiveNameserver reports whether resolv.conf lists addr as a nameserver
+// and no other one except on loopback. With inclusive_interfaces in
+// resolvconf.conf the exclusive marking of -x is ignored and the nameservers
+// DHCP supplied stay listed, and the system falls back to them: that is the
+// leak the takeover exists to close, so it does not count as applied. A
+// loopback nameserver alongside is a local resolver that resolvconf feeds.
+func exclusiveNameserver(resolvConf []byte, addr netip.Addr) bool {
+	found := false
+	for _, line := range strings.Split(string(resolvConf), "\n") {
+		fields := strings.Fields(line)
+		if len(fields) != 2 || fields[0] != "nameserver" {
+			continue
+		}
+		if fields[1] == addr.String() {
+			found = true
+			continue
+		}
+		if other, err := netip.ParseAddr(fields[1]); err != nil || !other.IsLoopback() {
+			return false
+		}
+	}
+	return found
+}
+
+// hasDirective reports whether a line of content is the keyword followed by
+// the address, as in `forward-addr: 10.0.0.1`.
+func hasDirective(content []byte, keyword string, addr netip.Addr) bool {
+	for _, line := range strings.Split(string(content), "\n") {
+		fields := strings.Fields(line)
+		if len(fields) == 2 && fields[0] == keyword && fields[1] == addr.String() {
+			return true
+		}
+	}
+	return false
+}
+
+// shellAssignment returns the value of `name=value` in a sh-style config such
+// as resolvconf.conf, without surrounding quotes, or empty when it is not set.
+func shellAssignment(content []byte, name string) string {
+	for _, line := range strings.Split(string(content), "\n") {
+		line = strings.TrimSpace(line)
+		value, ok := strings.CutPrefix(line, name+"=")
+		if !ok {
+			continue
+		}
+		return strings.Trim(value, `"'`)
+	}
+	return ""
+}
+
+// ConfigureSystemDNS hands the TUN's gateway address to resolvconf(8) as the
+// exclusive nameserver, so name lookups resolve through Xray instead of
+// leaking to the physical link.
+//
+// It acts only when the config opts in, and it verifies the data path first:
+// unless a query to the gateway would actually be handled, host-wide
+// resolution is left to the OS and an error returned. The caller does not
+// start the TUN on an error, as the system DNS would bypass it.
+func (t *FreeBSDTun) ConfigureSystemDNS(ctx context.Context, inboundTag string) error {
+	if !t.options.AutoSystemDnsToGateway {
+		return nil
+	}
+	if t.systemDNSSet {
+		return nil
+	}
+
+	// A previous removal may have failed. Retry before applying anything, so a
+	// stale entry does not silently outlive the attempt to clean it up.
+	if t.systemDNSDirty {
+		if err := t.revertSystemDNS(); err != nil {
+			return xerrors.New("previous system DNS removal still failing").Base(err)
+		}
+	}
+
+	// The resolver is pointed at the gateway: on FreeBSD the system has no
+	// address there, so a query to it is routed into the TUN and answered
+	// inside Xray. A query from this host carries the local address.
+	address := t.gateway.Addr()
+	if err := verifyDNSRouting(ctx, inboundTag, t.local.String(), address.String()); err != nil {
+		return xerrors.New("no DNS path at ", address.String(), ":53").Base(err)
+	}
+
+	// -x makes this interface's nameserver the only one in use, the
+	// equivalent of resolvectl's `domain ~.` plus `default-route true`:
+	// without it openresolv keeps the nameservers dhclient supplied listed
+	// alongside the gateway, and the system falls back to them.
+	if _, err := resolvconfRunner("nameserver "+address.String()+"\n", "-a", t.name, "-x"); err != nil {
+		return xerrors.New("resolvconf -a failed").Base(err)
+	}
+	applied, err := resolvconfApplied(address)
+	if err != nil {
+		return t.rollbackSystemDNS(xerrors.New("cannot confirm the system DNS").Base(err))
+	}
+	if !applied {
+		return t.rollbackSystemDNS(xerrors.New("resolvconf did not apply ", address.String(), " to any resolver (resolvconf=NO or inclusive_interfaces in resolvconf.conf?)"))
+	}
+
+	t.systemDNSSet = true
+	xerrors.LogInfo(ctx, "[tun] system DNS set to ", address.String(), " on ", t.name)
+	return nil
+}
+
+// rollbackSystemDNS undoes a takeover that did not take. A failed removal is
+// recorded so the next attempt retries it, and is reported rather than
+// swallowed.
+func (t *FreeBSDTun) rollbackSystemDNS(cause error) error {
+	if _, err := resolvconfRunner("", "-d", t.name); err != nil {
+		t.systemDNSDirty = true
+		// Combine, because Base overwrites: reporting only the cause would hide
+		// the removal failure, and reporting only the removal failure would
+		// hide why the removal was attempted.
+		return xerrors.New("resolvconf -d failed, the entry may remain").Base(xerrors.Combine(err, cause))
+	}
+	return cause
+}
+
+// revertSystemDNS issues the removal and keeps the dirty flag in step with the
+// outcome.
+func (t *FreeBSDTun) revertSystemDNS() error {
+	_, err := resolvconfRunner("", "-d", t.name)
+	t.systemDNSDirty = err != nil
+	if err != nil {
+		return err
+	}
+	t.systemDNSSet = false
+	return nil
+}
+
+// unsetSystemDNS hands DNS back to the OS. Only meaningful when
+// ConfigureSystemDNS applied something, or a previous removal failed.
+func (t *FreeBSDTun) unsetSystemDNS() {
+	if !t.systemDNSSet && !t.systemDNSDirty {
+		return
+	}
+	if err := t.revertSystemDNS(); err != nil {
+		xerrors.LogInfoInner(context.Background(), err, "[tun] failed to remove the system DNS entry; run `resolvconf -d ", t.name, "` to clean up")
+	}
+}
+
 func destroyInterface(name string) {
 	fd, err := unix.Socket(unix.AF_INET, unix.SOCK_DGRAM, 0)
 	if err != nil {
@@ -286,7 +492,8 @@ const (
 )
 
 // ifAliasReq4 is struct in_aliasreq from netinet/in_var.h in the 64-byte
-// layout unix.SIOCAIFADDR encodes (name + addr/dstaddr/mask sockaddrs).
+// layout unix.SIOCAIFADDR encodes (name + addr/dstaddr/mask sockaddrs; the
+// dstaddr slot is the broadcast address on a broadcast interface).
 type ifAliasReq4 struct {
 	Name    [unix.IFNAMSIZ]byte
 	Addr    unix.RawSockaddrInet4
@@ -321,7 +528,7 @@ type addrLifetime6 struct {
 // header macro does.
 const siocaifaddrIn6 = 0x80000000 | (uintptr(unsafe.Sizeof(ifAliasReq6{})) << 16) | ('i' << 8) | 27
 
-// setIPAddress assigns the local/remote point-to-point IPv4 pair and a
+// setIPAddress assigns the local IPv4 address with the gateway's mask and a
 // link-local IPv6 address to the interface, required for the routing to work
 // (same scheme as the darwin implementation: local address is the one right
 // after the gateway address).
@@ -334,17 +541,20 @@ func setIPAddress(name string, gateway netip.Prefix, local netip.Addr, ifIndex i
 
 	local4 := local.As4()
 
+	// The interface runs in broadcast mode (the wireguard tun library puts it
+	// there), so the second address slot of the request is the broadcast
+	// address, not a point-to-point peer. It stays empty and the kernel
+	// derives the broadcast address from the mask; the gateway is then an
+	// ordinary on-link address, reachable through the interface route, which
+	// is what the system routes and the system DNS takeover need. Filling it
+	// with the gateway made the gateway the broadcast address, and a plain
+	// socket cannot send to that.
 	ifReq4 := ifAliasReq4{
 		Addr: unix.RawSockaddrInet4{
 			Len:    unix.SizeofSockaddrInet4,
 			Family: unix.AF_INET,
 			Addr:   local4,
 		},
-		Dstaddr: unix.RawSockaddrInet4{
-			Len:    unix.SizeofSockaddrInet4,
-			Family: unix.AF_INET,
-			Addr:   gateway.Addr().As4(),
-		},
 		Mask: unix.RawSockaddrInet4{
 			Len:    unix.SizeofSockaddrInet4,
 			Family: unix.AF_INET,
@@ -431,10 +641,9 @@ func (t *FreeBSDTun) setSystemRoutes() error {
 		return err
 	}
 	// Route through the interface, not a gateway: the tun(4) device is a
-	// broadcast interface here, so its point-to-point peer address doubles as
+	// broadcast interface here with no address resolution, so a next hop adds
-	// the subnet broadcast and the kernel refuses to route to it (EACCES).
+	// nothing. Interface routes sidestep the gateway entirely (what wg-quick
-	// Interface routes sidestep the gateway entirely (what wg-quick does on
+	// does on FreeBSD).
-	// FreeBSD).
 	for _, destination := range routes {
 		if err := execRoute(-1, unix.RTM_ADD, t.tunIndex, destination, netip.Addr{}); err != nil {
 			_ = t.unsetSystemRoutes()

proxy/tun/tun_freebsd_dns_test.go

@@ -0,0 +1,390 @@
+//go:build freebsd
+
+package tun
+
+import (
+	"context"
+	"errors"
+	"io/fs"
+	"net/netip"
+	"strings"
+	"testing"
+)
+
+type probeCall struct {
+	inboundTag string
+	source     string
+	address    string
+}
+
+// stubDNSRouting replaces the routing probe for the duration of a test and
+// records how it was called, so tests can assert the probe is representative.
+func stubDNSRouting(t *testing.T, err error) *[]probeCall {
+	t.Helper()
+	original := verifyDNSRouting
+	calls := []probeCall{}
+	verifyDNSRouting = func(_ context.Context, inboundTag, source, address string) error {
+		calls = append(calls, probeCall{inboundTag, source, address})
+		return err
+	}
+	t.Cleanup(func() { verifyDNSRouting = original })
+	return &calls
+}
+
+// recorder installs a resolvconf stub for the duration of a test and returns
+// the captured invocations, each as the arguments followed by the stdin in
+// angle brackets when there was one. An empty failOn succeeds every call;
+// otherwise the named subcommand fails.
+func recorder(t *testing.T, failOn string) *[]string {
+	t.Helper()
+	original := resolvconfRunner
+	calls := []string{}
+	resolvconfRunner = func(stdin string, args ...string) ([]byte, error) {
+		call := strings.Join(args, " ")
+		if stdin != "" {
+			call += " <" + stdin + ">"
+		}
+		calls = append(calls, call)
+		if failOn != "" && len(args) > 0 && args[0] == failOn {
+			return nil, errors.New("boom")
+		}
+		return nil, nil
+	}
+	t.Cleanup(func() { resolvconfRunner = original })
+	return &calls
+}
+
+// files replaces the resolver files read by the applied check for the
+// duration of a test. A path not in the map does not exist.
+func files(t *testing.T, contents map[string]string) {
+	t.Helper()
+	original := readResolverFile
+	readResolverFile = func(path string) ([]byte, error) {
+		if data, ok := contents[path]; ok {
+			return []byte(data), nil
+		}
+		return nil, fs.ErrNotExist
+	}
+	t.Cleanup(func() { readResolverFile = original })
+}
+
+// takenOver is what the libc subscriber writes once the TUN's nameserver is
+// the exclusive one.
+const takenOver = "# Generated by resolvconf\nnameserver 192.168.100.1\n"
+
+// untouched is the DHCP-supplied resolv.conf that resolvconf=NO leaves in
+// place.
+const untouched = "# Generated by resolvconf\nnameserver 10.0.0.1\n"
+
+func optedInTun() *FreeBSDTun {
+	return &FreeBSDTun{
+		options: &Config{
+			Name:                   "tun7",
+			Gateway:                []string{"192.168.100.1/30"},
+			AutoSystemDnsToGateway: true,
+		},
+		name:    "tun7",
+		gateway: netip.MustParsePrefix("192.168.100.1/30"),
+		local:   netip.MustParseAddr("192.168.100.2"),
+	}
+}
+
+func joined(calls []string) string {
+	return strings.Join(calls, " | ")
+}
+
+func TestConfigureSystemDNSDisabledByDefault(t *testing.T) {
+	probes := stubDNSRouting(t, nil)
+	calls := recorder(t, "")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+	t1.options.AutoSystemDnsToGateway = false
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(*calls) != 0 || len(*probes) != 0 {
+		t.Errorf("nothing may run when the option is off, got %q and %d probes", joined(*calls), len(*probes))
+	}
+	if t1.systemDNSSet {
+		t.Error("systemDNSSet must stay false when the option is off")
+	}
+}
+
+func TestConfigureSystemDNSLeavesOSDNSWhenNoRoute(t *testing.T) {
+	stubDNSRouting(t, errors.New("no route"))
+	calls := recorder(t, "")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err == nil {
+		t.Fatal("expected an error when the probe fails")
+	}
+	if len(*calls) != 0 {
+		t.Errorf("resolvconf must not run when the probe fails, got %q", joined(*calls))
+	}
+	if t1.systemDNSSet {
+		t.Error("systemDNSSet must stay false when the probe fails")
+	}
+}
+
+// On FreeBSD the addresses are the other way round from Linux: the local
+// address is the one after the gateway, and the gateway itself has no owner,
+// so a packet to it is routed into the TUN. The probe has to carry that pair,
+// and the resolver has to be pointed at the gateway.
+func TestConfigureSystemDNSProbeCarriesFreeBSDPair(t *testing.T) {
+	probes := stubDNSRouting(t, nil)
+	recorder(t, "")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(*probes) != 1 {
+		t.Fatalf("expected one probe, got %d", len(*probes))
+	}
+	want := probeCall{"tun", "192.168.100.2", "192.168.100.1"}
+	if (*probes)[0] != want {
+		t.Errorf("probe = %+v, want %+v", (*probes)[0], want)
+	}
+}
+
+func TestConfigureSystemDNSAppliesResolvconf(t *testing.T) {
+	stubDNSRouting(t, nil)
+	calls := recorder(t, "")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !t1.systemDNSSet {
+		t.Fatal("systemDNSSet should be true after a successful takeover")
+	}
+
+	want := "-a tun7 -x <nameserver 192.168.100.1\n>"
+	if got := joined(*calls); got != want {
+		t.Errorf("resolvconf calls = %q, want %q", got, want)
+	}
+}
+
+// resolvconf exits 0 with resolvconf=NO in resolvconf.conf, and still records
+// the entry for the interface, but runs no subscriber, so success has to be
+// confirmed in what the subscribers wrote.
+func TestConfigureSystemDNSRefusesWhenResolvconfDidNotApply(t *testing.T) {
+	stubDNSRouting(t, nil)
+	calls := recorder(t, "")
+	files(t, map[string]string{"/etc/resolv.conf": untouched})
+
+	t1 := optedInTun()
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err == nil {
+		t.Fatal("expected an error when the nameserver did not reach a resolver")
+	}
+	if t1.systemDNSSet {
+		t.Error("systemDNSSet must stay false when the takeover did not take")
+	}
+	if !strings.HasSuffix(joined(*calls), "-d tun7") {
+		t.Errorf("expected a removal after the failed check, got %q", joined(*calls))
+	}
+}
+
+func TestConfigureSystemDNSIdempotent(t *testing.T) {
+	stubDNSRouting(t, nil)
+	calls := recorder(t, "")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	before := len(*calls)
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(*calls) != before {
+		t.Errorf("second call must be a no-op, calls went %d -> %d", before, len(*calls))
+	}
+}
+
+func TestConfigureSystemDNSAddFailureIsNotDirty(t *testing.T) {
+	stubDNSRouting(t, nil)
+	calls := recorder(t, "-a")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err == nil {
+		t.Fatal("expected an error when resolvconf -a fails")
+	}
+	if t1.systemDNSSet || t1.systemDNSDirty {
+		t.Error("a failed add leaves nothing to clean up")
+	}
+	if len(*calls) != 1 {
+		t.Errorf("nothing else may run after a failed add, got %q", joined(*calls))
+	}
+}
+
+// If the removal after a failed check fails too, the entry may still be
+// installed, so the state has to be remembered rather than silently dropped.
+func TestConfigureSystemDNSRollbackFailureKeepsDirty(t *testing.T) {
+	stubDNSRouting(t, nil)
+	recorder(t, "-d")
+	files(t, map[string]string{"/etc/resolv.conf": untouched})
+
+	t1 := optedInTun()
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err == nil {
+		t.Fatal("expected an error when the check fails")
+	}
+	if !t1.systemDNSDirty {
+		t.Error("a failed removal must leave the resolver marked dirty")
+	}
+	if t1.systemDNSSet {
+		t.Error("systemDNSSet must stay false when the takeover did not complete")
+	}
+}
+
+// A dirty resolver is retried before anything new is applied.
+func TestConfigureSystemDNSRetriesDirtyBeforeApplying(t *testing.T) {
+	stubDNSRouting(t, nil)
+	calls := recorder(t, "")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+	t1.systemDNSDirty = true
+
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	got := joined(*calls)
+	if !strings.HasPrefix(got, "-d tun7 | -a tun7") {
+		t.Errorf("expected the stale removal first, got %q", got)
+	}
+	if t1.systemDNSDirty {
+		t.Error("a successful retry should clear the dirty flag")
+	}
+	if !t1.systemDNSSet {
+		t.Error("the takeover should proceed once the retry succeeds")
+	}
+}
+
+func TestUnsetSystemDNSRemoves(t *testing.T) {
+	stubDNSRouting(t, nil)
+	calls := recorder(t, "")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+	if err := t1.ConfigureSystemDNS(context.Background(), "tun"); err != nil {
+		t.Fatalf("setup failed: %v", err)
+	}
+	*calls = nil
+
+	t1.unsetSystemDNS()
+	if t1.systemDNSSet {
+		t.Error("systemDNSSet should be false after unset")
+	}
+	if got := joined(*calls); got != "-d tun7" {
+		t.Errorf("unset calls = %q, want %q", got, "-d tun7")
+	}
+
+	t1.unsetSystemDNS()
+	if len(*calls) != 1 {
+		t.Errorf("unsetSystemDNS must be idempotent, got %q", joined(*calls))
+	}
+}
+
+func TestUnsetSystemDNSKeepsDirtyWhenRemoveFails(t *testing.T) {
+	stubDNSRouting(t, nil)
+	calls := recorder(t, "-d")
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+
+	t1 := optedInTun()
+	t1.systemDNSSet = true
+
+	t1.unsetSystemDNS()
+	if !t1.systemDNSDirty {
+		t.Error("a failed removal during unset must be remembered")
+	}
+	if got := joined(*calls); got != "-d tun7" {
+		t.Errorf("expected a removal attempt, got %q", got)
+	}
+}
+
+func TestResolvconfAppliedLibc(t *testing.T) {
+	files(t, map[string]string{"/etc/resolv.conf": takenOver})
+	if ok, err := resolvconfApplied(netip.MustParseAddr("192.168.100.1")); err != nil || !ok {
+		t.Errorf("resolv.conf listing the nameserver must count as applied, got %v, %v", ok, err)
+	}
+}
+
+func TestResolvconfAppliedNotWhenUntouched(t *testing.T) {
+	files(t, map[string]string{"/etc/resolv.conf": untouched})
+	if ok, err := resolvconfApplied(netip.MustParseAddr("192.168.100.1")); err != nil || ok {
+		t.Errorf("an unchanged resolv.conf and no resolvconf.conf must not count as applied, got %v, %v", ok, err)
+	}
+}
+
+// With inclusive_interfaces in resolvconf.conf the exclusive marking is
+// ignored and the DHCP nameservers stay listed, so the system falls back to
+// them; that is the leak the option exists to close, and it must not pass.
+func TestResolvconfAppliedNotWhenOthersRemain(t *testing.T) {
+	files(t, map[string]string{"/etc/resolv.conf": "nameserver 192.168.100.1\nnameserver 10.0.0.1\n"})
+	if ok, _ := resolvconfApplied(netip.MustParseAddr("192.168.100.1")); ok {
+		t.Error("another nameserver alongside the TUN's must not count as applied")
+	}
+}
+
+// A loopback nameserver alongside is a local resolver that resolvconf feeds
+// (local_unbound with the libc subscriber left on), not a leak.
+func TestResolvconfAppliedWithLoopbackAlongside(t *testing.T) {
+	files(t, map[string]string{"/etc/resolv.conf": "nameserver 127.0.0.1\nnameserver 192.168.100.1\n"})
+	if ok, err := resolvconfApplied(netip.MustParseAddr("192.168.100.1")); err != nil || !ok {
+		t.Errorf("a loopback nameserver alongside must still count as applied, got %v, %v", ok, err)
+	}
+}
+
+func TestResolvconfAppliedIgnoresPrefixMatches(t *testing.T) {
+	files(t, map[string]string{"/etc/resolv.conf": "nameserver 192.168.100.10\n"})
+	if ok, _ := resolvconfApplied(netip.MustParseAddr("192.168.100.1")); ok {
+		t.Error("192.168.100.10 must not match 192.168.100.1")
+	}
+}
+
+// local-unbound-setup turns the libc subscriber off and has resolvconf write
+// unbound's forwarders instead; resolv.conf then stays pointed at loopback.
+func TestResolvconfAppliedUnbound(t *testing.T) {
+	files(t, map[string]string{
+		"/etc/resolv.conf":          "# Generated by resolvconf\nnameserver 127.0.0.1\noptions edns0\n",
+		"/etc/resolvconf.conf":      "libc=\"NO\"\nunbound_conf=\"/var/unbound/forward.conf\"\n",
+		"/var/unbound/forward.conf": "forward-zone:\n\tname: .\n\tforward-addr: 192.168.100.1\n",
+	})
+	if ok, err := resolvconfApplied(netip.MustParseAddr("192.168.100.1")); err != nil || !ok {
+		t.Errorf("the nameserver in unbound's forwarders must count as applied, got %v, %v", ok, err)
+	}
+}
+
+func TestResolvconfAppliedNotWhenUnboundUntouched(t *testing.T) {
+	files(t, map[string]string{
+		"/etc/resolv.conf":          "nameserver 127.0.0.1\n",
+		"/etc/resolvconf.conf":      "libc=\"NO\"\nunbound_conf=\"/var/unbound/forward.conf\"\n",
+		"/var/unbound/forward.conf": "forward-zone:\n\tname: .\n\tforward-addr: 10.0.0.1\n",
+	})
+	if ok, _ := resolvconfApplied(netip.MustParseAddr("192.168.100.1")); ok {
+		t.Error("unbound still forwarding to the old server must not count as applied")
+	}
+}
+
+func TestResolvconfAppliedReportsUnreadableResolvConf(t *testing.T) {
+	files(t, map[string]string{})
+	if _, err := resolvconfApplied(netip.MustParseAddr("192.168.100.1")); err == nil {
+		t.Error("a missing resolv.conf is an error, not a verdict")
+	}
+}

proxy/tun/tun_linux.go

@@ -11,19 +11,8 @@ import (
 	"sync"
 
 	"github.com/vishvananda/netlink"
-	appdns "github.com/xtls/xray-core/app/dns"
 	"github.com/xtls/xray-core/common/errors"
-	xnet "github.com/xtls/xray-core/common/net"
 	"github.com/xtls/xray-core/common/platform"
-	"github.com/xtls/xray-core/common/serial"
-	"github.com/xtls/xray-core/common/session"
-	"github.com/xtls/xray-core/core"
-	feature_dns "github.com/xtls/xray-core/features/dns"
-	"github.com/xtls/xray-core/features/dns/localdns"
-	"github.com/xtls/xray-core/features/outbound"
-	"github.com/xtls/xray-core/features/routing"
-	routingsession "github.com/xtls/xray-core/features/routing/session"
-	"github.com/xtls/xray-core/proxy/dns"
 	"golang.org/x/sys/unix"
 )
 
@@ -106,81 +95,6 @@ func (t *LinuxTun) ifaceName() string {
 	return attrs.Name
 }
 
-// probeSourcePort is a representative client port for the routing probe. A real
-// query arrives from an ephemeral port that cannot be known in advance, so this
-// only matters for a rule that matches on a source port.
-const probeSourcePort = 49152
-
-// verifyDNSRouting reports whether a DNS query to address would actually be
-// handled. Redirecting the system resolver at an address nothing answers would
-// break name resolution outright, so the takeover only proceeds when routing
-// hands such a query to a DNS-capable outbound.
-//
-// Overridable for tests.
-var verifyDNSRouting = func(ctx context.Context, inboundTag, source, address string) error {
-	ip, err := netip.ParseAddr(address)
-	if err != nil {
-		return errors.New("invalid DNS address ", address).Base(err)
-	}
-	src, err := netip.ParseAddr(source)
-	if err != nil || src.Is4() != ip.Is4() {
-		return errors.New("invalid source address ", source).Base(err)
-	}
-
-	instance := core.MustFromContext(ctx)
-
-	// Any resolution path that could still reach the system resolver has to be
-	// refused, because pointing the system resolver at the TUN would close a
-	// loop through the DNS outbound. With no `dns` section Core installs such a
-	// client; with a `dns` section that has no name servers app/dns falls back
-	// to one; and a name server pointed at "localhost" is one even when
-	// independent upstreams are configured alongside it, because name servers
-	// are selected per domain.
-	switch dnsFeature := instance.GetFeature(feature_dns.ClientType()).(type) {
-	case *localdns.Client:
-		return errors.New("DNS feature is the system resolver, takeover would loop")
-	case *appdns.DNS:
-		if dnsFeature.MayUseSystemResolver() {
-			return errors.New("DNS configuration may resolve through the system resolver, takeover would loop")
-		}
-	}
-
-	router, ok := instance.GetFeature(routing.RouterType()).(routing.Router)
-	if !ok {
-		return errors.New("router feature unavailable")
-	}
-
-	// A real query from this interface carries a source address, and rules may
-	// match on it, so the probe has to carry one too.
-	queryCtx := session.ContextWithInbound(ctx, &session.Inbound{
-		Name:   "tun",
-		Tag:    inboundTag,
-		Source: xnet.UDPDestination(xnet.IPAddress(src.AsSlice()), probeSourcePort),
-	})
-	queryCtx = session.ContextWithOutbounds(queryCtx, []*session.Outbound{{
-		Target: xnet.UDPDestination(xnet.IPAddress(ip.AsSlice()), 53),
-	}})
-
-	route, err := router.PickRoute(routingsession.AsRoutingContext(queryCtx))
-	if err != nil {
-		return errors.New("no route for ", address, ":53").Base(err)
-	}
-
-	manager, ok := instance.GetFeature(outbound.ManagerType()).(outbound.Manager)
-	if !ok {
-		return errors.New("outbound manager unavailable")
-	}
-
-	handler := manager.GetHandler(route.GetOutboundTag())
-	if handler == nil {
-		return errors.New("outbound ", route.GetOutboundTag(), " does not exist")
-	}
-	if settings := handler.ProxySettings(); settings == nil || settings.Type != serial.GetMessageType(&dns.Config{}) {
-		return errors.New("outbound ", route.GetOutboundTag(), " does not handle DNS")
-	}
-	return nil
-}
-
 // ConfigureSystemDNS points systemd-resolved at this interface so name lookups
 // resolve through Xray instead of leaking to the physical link.
 //

proxy/tun/tun_system_dns.go

@@ -0,0 +1,96 @@
+//go:build (linux && !android) || freebsd
+
+package tun
+
+import (
+	"context"
+	"net/netip"
+
+	appdns "github.com/xtls/xray-core/app/dns"
+	"github.com/xtls/xray-core/common/errors"
+	xnet "github.com/xtls/xray-core/common/net"
+	"github.com/xtls/xray-core/common/serial"
+	"github.com/xtls/xray-core/common/session"
+	"github.com/xtls/xray-core/core"
+	feature_dns "github.com/xtls/xray-core/features/dns"
+	"github.com/xtls/xray-core/features/dns/localdns"
+	"github.com/xtls/xray-core/features/outbound"
+	"github.com/xtls/xray-core/features/routing"
+	routingsession "github.com/xtls/xray-core/features/routing/session"
+	"github.com/xtls/xray-core/proxy/dns"
+)
+
+// probeSourcePort is a representative client port for the routing probe. A real
+// query arrives from an ephemeral port that cannot be known in advance, so this
+// only matters for a rule that matches on a source port.
+const probeSourcePort = 49152
+
+// verifyDNSRouting reports whether a DNS query to address would actually be
+// handled. Redirecting the system resolver at an address nothing answers would
+// break name resolution outright, so the takeover only proceeds when routing
+// hands such a query to a DNS-capable outbound.
+//
+// Overridable for tests.
+var verifyDNSRouting = func(ctx context.Context, inboundTag, source, address string) error {
+	ip, err := netip.ParseAddr(address)
+	if err != nil {
+		return errors.New("invalid DNS address ", address).Base(err)
+	}
+	src, err := netip.ParseAddr(source)
+	if err != nil || src.Is4() != ip.Is4() {
+		return errors.New("invalid source address ", source).Base(err)
+	}
+
+	instance := core.MustFromContext(ctx)
+
+	// Any resolution path that could still reach the system resolver has to be
+	// refused, because pointing the system resolver at the TUN would close a
+	// loop through the DNS outbound. With no `dns` section Core installs such a
+	// client; with a `dns` section that has no name servers app/dns falls back
+	// to one; and a name server pointed at "localhost" is one even when
+	// independent upstreams are configured alongside it, because name servers
+	// are selected per domain.
+	switch dnsFeature := instance.GetFeature(feature_dns.ClientType()).(type) {
+	case *localdns.Client:
+		return errors.New("DNS feature is the system resolver, takeover would loop")
+	case *appdns.DNS:
+		if dnsFeature.MayUseSystemResolver() {
+			return errors.New("DNS configuration may resolve through the system resolver, takeover would loop")
+		}
+	}
+
+	router, ok := instance.GetFeature(routing.RouterType()).(routing.Router)
+	if !ok {
+		return errors.New("router feature unavailable")
+	}
+
+	// A real query from this interface carries a source address, and rules may
+	// match on it, so the probe has to carry one too.
+	queryCtx := session.ContextWithInbound(ctx, &session.Inbound{
+		Name:   "tun",
+		Tag:    inboundTag,
+		Source: xnet.UDPDestination(xnet.IPAddress(src.AsSlice()), probeSourcePort),
+	})
+	queryCtx = session.ContextWithOutbounds(queryCtx, []*session.Outbound{{
+		Target: xnet.UDPDestination(xnet.IPAddress(ip.AsSlice()), 53),
+	}})
+
+	route, err := router.PickRoute(routingsession.AsRoutingContext(queryCtx))
+	if err != nil {
+		return errors.New("no route for ", address, ":53").Base(err)
+	}
+
+	manager, ok := instance.GetFeature(outbound.ManagerType()).(outbound.Manager)
+	if !ok {
+		return errors.New("outbound manager unavailable")
+	}
+
+	handler := manager.GetHandler(route.GetOutboundTag())
+	if handler == nil {
+		return errors.New("outbound ", route.GetOutboundTag(), " does not exist")
+	}
+	if settings := handler.ProxySettings(); settings == nil || settings.Type != serial.GetMessageType(&dns.Config{}) {
+		return errors.New("outbound ", route.GetOutboundTag(), " does not handle DNS")
+	}
+	return nil
+}