Revert "[client] Prefer systemd-resolved stub over file mode regardless of resolv.conf header (#5935)"

This reverts commit 75e408f51c.

client/internal/dns/file_parser_unix.go

@@ -13,7 +13,6 @@ import (
 
 const (
 	defaultResolvConfPath = "/etc/resolv.conf"
-	nsswitchConfPath      = "/etc/nsswitch.conf"
 )
 
 type resolvConf struct {

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) {
+func getOSDNSManagerType() (osManagerType, 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) {
 	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 {
+		if err := file.Close(); err != nil {
-			log.Errorf("close file %s: %s", defaultResolvConfPath, cerr)
+			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 err := scanner.Err(); err != nil && err != io.EOF {
-		return 0, "", nil, fmt.Errorf("scan: %w", err)
-	}
-	return 0, "", rejected, 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", ""
+			return netbirdManager, nil
 		}
-	if strings.Contains(text, "NetworkManager") {
+		if strings.Contains(text, "NetworkManager") && isDbusListenerRunning(networkManagerDest, networkManagerDbusObjectNode) && isNetworkManagerSupported() {
-		if isDbusListenerRunning(networkManagerDest, networkManagerDbusObjectNode) && isNetworkManagerSupported() {
+			return networkManager, nil
-			return networkManager, "NetworkManager header + supported version on dbus", ""
+		}
+		if strings.Contains(text, "systemd-resolved") && isSystemdResolvedRunning() {
+			if checkStub() {
+				return systemdManager, nil
+			} else {
+				return fileManager, nil
 			}
-		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 systemdManager, nil
 			}
-		return resolvConfManager, "resolvconf header detected", ""
+
+			return resolvConfManager, nil
 		}
-	return 0, "", ""
+	}
+	if err := scanner.Err(); err != nil && err != io.EOF {
+		return 0, fmt.Errorf("scan: %w", err)
+	}
+
+	return fileManager, nil
 }
 
-// checkStub reports whether systemd-resolved's stub (127.0.0.53) is listed
+// checkStub checks if the stub resolver is disabled in systemd-resolved. If it is disabled, we fall back to file manager.
-// in /etc/resolv.conf. On parse failure we assume it is, to avoid dropping
-// into file mode while resolved is active.
 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
-}

client/internal/dns/host_unix_test.go

@@ -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)
-			}
-		})
-	}
-}