From faf3559ea7b01f3d55d20ac24f254c94caf9502d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Thu, 23 Apr 2026 21:29:52 +0200 Subject: [PATCH] Revert "[client] Prefer systemd-resolved stub over file mode regardless of resolv.conf header (#5935)" This reverts commit 75e408f51cb54f82c42d4c04d65f3a38e42c433a. --- client/internal/dns/file_parser_unix.go | 1 - client/internal/dns/host_unix.go | 134 ++++++------------------ client/internal/dns/host_unix_test.go | 76 -------------- 3 files changed, 31 insertions(+), 180 deletions(-) delete mode 100644 client/internal/dns/host_unix_test.go diff --git a/client/internal/dns/file_parser_unix.go b/client/internal/dns/file_parser_unix.go index 50ba74c0c..8dacb4e51 100644 --- a/client/internal/dns/file_parser_unix.go +++ b/client/internal/dns/file_parser_unix.go @@ -13,7 +13,6 @@ import ( const ( defaultResolvConfPath = "/etc/resolv.conf" - nsswitchConfPath = "/etc/nsswitch.conf" ) type resolvConf struct { diff --git a/client/internal/dns/host_unix.go b/client/internal/dns/host_unix.go index d7301d725..422fed4e5 100644 --- a/client/internal/dns/host_unix.go +++ b/client/internal/dns/host_unix.go @@ -46,12 +46,12 @@ type restoreHostManager interface { } func newHostManager(wgInterface string) (hostManager, error) { - osManager, reason, err := getOSDNSManagerType() + osManager, err := getOSDNSManagerType() if err != nil { return nil, fmt.Errorf("get os dns manager type: %w", err) } - log.Infof("System DNS manager discovered: %s (%s)", osManager, reason) + log.Infof("System DNS manager discovered: %s", osManager) mgr, err := newHostManagerFromType(wgInterface, osManager) // need to explicitly return nil mgr on error to avoid returning a non-nil interface containing a nil value if err != nil { @@ -74,49 +74,17 @@ func newHostManagerFromType(wgInterface string, osManager osManagerType) (restor } } -func getOSDNSManagerType() (osManagerType, string, error) { - resolved := isSystemdResolvedRunning() - nss := isLibnssResolveUsed() - stub := checkStub() - - // Prefer systemd-resolved whenever it owns libc resolution, regardless of - // who wrote /etc/resolv.conf. File-mode rewrites do not affect lookups - // that go through nss-resolve, and in foreign mode they can loop back - // through resolved as an upstream. - if resolved && (nss || stub) { - return systemdManager, fmt.Sprintf("systemd-resolved active (nss-resolve=%t, stub=%t)", nss, stub), nil - } - - mgr, reason, rejected, err := scanResolvConfHeader() - if err != nil { - return 0, "", err - } - if reason != "" { - return mgr, reason, nil - } - - fallback := fmt.Sprintf("no manager matched (resolved=%t, nss-resolve=%t, stub=%t)", resolved, nss, stub) - if len(rejected) > 0 { - fallback += "; rejected: " + strings.Join(rejected, ", ") - } - return fileManager, fallback, nil -} - -// scanResolvConfHeader walks /etc/resolv.conf header comments and returns the -// matching manager. If reason is empty the caller should pick file mode and -// use rejected for diagnostics. -func scanResolvConfHeader() (osManagerType, string, []string, error) { +func getOSDNSManagerType() (osManagerType, error) { file, err := os.Open(defaultResolvConfPath) if err != nil { - return 0, "", nil, fmt.Errorf("unable to open %s for checking owner, got error: %w", defaultResolvConfPath, err) + return 0, fmt.Errorf("unable to open %s for checking owner, got error: %w", defaultResolvConfPath, err) } defer func() { - if cerr := file.Close(); cerr != nil { - log.Errorf("close file %s: %s", defaultResolvConfPath, cerr) + if err := file.Close(); err != nil { + log.Errorf("close file %s: %s", defaultResolvConfPath, err) } }() - var rejected []string scanner := bufio.NewScanner(file) for scanner.Scan() { text := scanner.Text() @@ -124,48 +92,41 @@ func scanResolvConfHeader() (osManagerType, string, []string, error) { continue } if text[0] != '#' { - break + return fileManager, nil } - if mgr, reason, rej := matchResolvConfHeader(text); reason != "" { - return mgr, reason, nil, nil - } else if rej != "" { - rejected = append(rejected, rej) + if strings.Contains(text, fileGeneratedResolvConfContentHeader) { + return netbirdManager, nil + } + if strings.Contains(text, "NetworkManager") && isDbusListenerRunning(networkManagerDest, networkManagerDbusObjectNode) && isNetworkManagerSupported() { + return networkManager, nil + } + if strings.Contains(text, "systemd-resolved") && isSystemdResolvedRunning() { + if checkStub() { + return systemdManager, nil + } else { + return fileManager, nil + } + } + if strings.Contains(text, "resolvconf") { + if isSystemdResolveConfMode() { + return systemdManager, nil + } + + return resolvConfManager, nil } } if err := scanner.Err(); err != nil && err != io.EOF { - return 0, "", nil, fmt.Errorf("scan: %w", err) + return 0, fmt.Errorf("scan: %w", err) } - return 0, "", rejected, nil + + return fileManager, nil } -// matchResolvConfHeader inspects a single comment line. Returns either a -// definitive (manager, reason) or a non-empty rejected diagnostic. -func matchResolvConfHeader(text string) (osManagerType, string, string) { - if strings.Contains(text, fileGeneratedResolvConfContentHeader) { - return netbirdManager, "netbird-managed resolv.conf header detected", "" - } - if strings.Contains(text, "NetworkManager") { - if isDbusListenerRunning(networkManagerDest, networkManagerDbusObjectNode) && isNetworkManagerSupported() { - return networkManager, "NetworkManager header + supported version on dbus", "" - } - return 0, "", "NetworkManager header (no dbus or unsupported version)" - } - if strings.Contains(text, "resolvconf") { - if isSystemdResolveConfMode() { - return systemdManager, "resolvconf header in systemd-resolved compatibility mode", "" - } - return resolvConfManager, "resolvconf header detected", "" - } - return 0, "", "" -} - -// checkStub reports whether systemd-resolved's stub (127.0.0.53) is listed -// in /etc/resolv.conf. On parse failure we assume it is, to avoid dropping -// into file mode while resolved is active. +// checkStub checks if the stub resolver is disabled in systemd-resolved. If it is disabled, we fall back to file manager. func checkStub() bool { rConf, err := parseDefaultResolvConf() if err != nil { - log.Warnf("failed to parse resolv conf, assuming stub is active: %s", err) + log.Warnf("failed to parse resolv conf: %s", err) return true } @@ -178,36 +139,3 @@ func checkStub() bool { return false } - -// isLibnssResolveUsed reports whether nss-resolve is listed before dns on -// the hosts: line of /etc/nsswitch.conf. When it is, libc lookups are -// delegated to systemd-resolved regardless of /etc/resolv.conf. -func isLibnssResolveUsed() bool { - bs, err := os.ReadFile(nsswitchConfPath) - if err != nil { - log.Debugf("read %s: %v", nsswitchConfPath, err) - return false - } - return parseNsswitchResolveAhead(bs) -} - -func parseNsswitchResolveAhead(data []byte) bool { - for _, line := range strings.Split(string(data), "\n") { - if i := strings.IndexByte(line, '#'); i >= 0 { - line = line[:i] - } - fields := strings.Fields(line) - if len(fields) < 2 || fields[0] != "hosts:" { - continue - } - for _, module := range fields[1:] { - switch module { - case "dns": - return false - case "resolve": - return true - } - } - } - return false -} diff --git a/client/internal/dns/host_unix_test.go b/client/internal/dns/host_unix_test.go deleted file mode 100644 index e936281d3..000000000 --- a/client/internal/dns/host_unix_test.go +++ /dev/null @@ -1,76 +0,0 @@ -//go:build (linux && !android) || freebsd - -package dns - -import "testing" - -func TestParseNsswitchResolveAhead(t *testing.T) { - tests := []struct { - name string - in string - want bool - }{ - { - name: "resolve before dns with action token", - in: "hosts: mymachines resolve [!UNAVAIL=return] files myhostname dns\n", - want: true, - }, - { - name: "dns before resolve", - in: "hosts: files mdns4_minimal [NOTFOUND=return] dns resolve\n", - want: false, - }, - { - name: "debian default with only dns", - in: "hosts: files mdns4_minimal [NOTFOUND=return] dns mymachines\n", - want: false, - }, - { - name: "neither resolve nor dns", - in: "hosts: files myhostname\n", - want: false, - }, - { - name: "no hosts line", - in: "passwd: files systemd\ngroup: files systemd\n", - want: false, - }, - { - name: "empty", - in: "", - want: false, - }, - { - name: "comments and blank lines ignored", - in: "# comment\n\n# another\nhosts: resolve dns\n", - want: true, - }, - { - name: "trailing inline comment", - in: "hosts: resolve [!UNAVAIL=return] dns # fallback\n", - want: true, - }, - { - name: "hosts token must be the first field", - in: " hosts: resolve dns\n", - want: true, - }, - { - name: "other db line mentioning resolve is ignored", - in: "networks: resolve\nhosts: dns\n", - want: false, - }, - { - name: "only resolve, no dns", - in: "hosts: files resolve\n", - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := parseNsswitchResolveAhead([]byte(tt.in)); got != tt.want { - t.Errorf("parseNsswitchResolveAhead() = %v, want %v", got, tt.want) - } - }) - } -}