Fix/resolv parser (netbirdio#1520)

fix an issue In case if the original resolv.conf file is empty, then it can cause a nil pointer

client/internal/dns/file_parser_linux.go

@@ -33,9 +33,15 @@ func parseBackupResolvConf() (*resolvConf, error) {
 }
 
 func parseResolvConfFile(resolvConfFile string) (*resolvConf, error) {
+	rconf := &resolvConf{
+		searchDomains: make([]string, 0),
+		nameServers:   make([]string, 0),
+		others:        make([]string, 0),
+	}
+
 	file, err := os.Open(resolvConfFile)
 	if err != nil {
-		return nil, fmt.Errorf("failed to open %s file: %w", resolvConfFile, err)
+		return rconf, fmt.Errorf("failed to open %s file: %w", resolvConfFile, err)
 	}
 	defer func() {
 		if err := file.Close(); err != nil {
@@ -45,17 +51,11 @@ func parseResolvConfFile(resolvConfFile string) (*resolvConf, error) {
 
 	cur, err := os.ReadFile(resolvConfFile)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read %s file: %w", resolvConfFile, err)
+		return rconf, fmt.Errorf("failed to read %s file: %w", resolvConfFile, err)
 	}
 
 	if len(cur) == 0 {
-		return nil, fmt.Errorf("file is empty")
-	}
-
-	rconf := &resolvConf{
-		searchDomains: make([]string, 0),
-		nameServers:   make([]string, 0),
-		others:        make([]string, 0),
+		return rconf, fmt.Errorf("file is empty")
 	}
 
 	for _, line := range strings.Split(string(cur), "\n") {

client/internal/dns/file_parser_linux_test.go

@@ -3,8 +3,8 @@
 package dns
 
 import (
-	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 )
 
@@ -75,40 +75,13 @@ options debug
 			expectedNS:     []string{"192.168.2.1", "100.81.99.197"},
 			expectedOther:  []string{"options debug"},
 		},
-		{
-			input: `# This is /run/systemd/resolve/resolv.conf managed by man:systemd-resolved(8).
-# Do not edit.
-#
-# This file might be symlinked as /etc/resolv.conf. If you're looking at
-# /etc/resolv.conf and seeing this text, you have followed the symlink.
-#
-# This is a dynamic resolv.conf file for connecting local clients directly to
-# all known uplink DNS servers. This file lists all configured search domains.
-#
-# Third party programs should typically not access this file directly, but only
-# through the symlink at /etc/resolv.conf. To manage man:resolv.conf(5) in a
-# different way, replace this symlink by a static file or a different symlink.
-#
-# See man:systemd-resolved.service(8) for details about the supported modes of
-# operation for /etc/resolv.conf.
-
-nameserver 192.168.2.1
-nameserver 100.81.99.197
-search netbird.cloud
-options debug
-options edns0 trust-ad
-`,
-			expectedSearch: []string{"netbird.cloud"},
-			expectedNS:     []string{"192.168.2.1", "100.81.99.197"},
-			expectedOther:  []string{"options debug", "options edns0 trust-ad"},
-		},
 	}
 
 	for _, testCase := range testCases {
 		testCase := testCase
 		t.Run("test", func(t *testing.T) {
 			t.Parallel()
-			tmpResolvConf := fmt.Sprintf("%s/%s", t.TempDir(), "resolv.conf")
+			tmpResolvConf := filepath.Join(t.TempDir(), "resolv.conf")
 			err := os.WriteFile(tmpResolvConf, []byte(testCase.input), 0644)
 			if err != nil {
 				t.Fatal(err)
@@ -147,3 +120,55 @@ func compareLists(search []string, search2 []string) bool {
 	}
 	return true
 }
+
+func Test_emptyFile(t *testing.T) {
+	cfg, err := parseResolvConfFile("/tmp/nothing")
+	if err == nil {
+		t.Errorf("expected error, got nil")
+	}
+	if len(cfg.others) != 0 || len(cfg.searchDomains) != 0 || len(cfg.nameServers) != 0 {
+		t.Errorf("expected empty config, got %v", cfg)
+	}
+}
+
+func Test_symlink(t *testing.T) {
+	input := `# This is /run/systemd/resolve/resolv.conf managed by man:systemd-resolved(8).
+# Do not edit.
+#
+# This file might be symlinked as /etc/resolv.conf. If you're looking at
+# /etc/resolv.conf and seeing this text, you have followed the symlink.
+#
+# This is a dynamic resolv.conf file for connecting local clients directly to
+# all known uplink DNS servers. This file lists all configured search domains.
+#
+# Third party programs should typically not access this file directly, but only
+# through the symlink at /etc/resolv.conf. To manage man:resolv.conf(5) in a
+# different way, replace this symlink by a static file or a different symlink.
+#
+# See man:systemd-resolved.service(8) for details about the supported modes of
+# operation for /etc/resolv.conf.
+
+nameserver 192.168.0.1
+`
+
+	tmpResolvConf := filepath.Join(t.TempDir(), "resolv.conf")
+	err := os.WriteFile(tmpResolvConf, []byte(input), 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tmpLink := filepath.Join(t.TempDir(), "symlink")
+	err = os.Symlink(tmpResolvConf, tmpLink)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cfg, err := parseResolvConfFile(tmpLink)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(cfg.nameServers) != 1 {
+		t.Errorf("unexpected resolv.conf content: %v", cfg)
+	}
+}

client/internal/dns/file_repair_linux.go

@@ -3,8 +3,8 @@
 package dns
 
 import (
-	"fmt"
 	"path"
+	"path/filepath"
 	"sync"
 
 	"github.com/fsnotify/fsnotify"
@@ -32,9 +32,10 @@ type repair struct {
 }
 
 func newRepair(operationFile string, updateFn repairConfFn) *repair {
+	targetFile := targetFile(operationFile)
 	return &repair{
-		operationFile: operationFile,
-		watchDir:      path.Dir(operationFile),
+		operationFile: targetFile,
+		watchDir:      path.Dir(targetFile),
 		updateFn:      updateFn,
 	}
 }
@@ -44,7 +45,7 @@ func (f *repair) watchFileChanges(nbSearchDomains []string, nbNameserverIP strin
 		return
 	}
 
-	log.Infof("start to watch resolv.conf")
+	log.Infof("start to watch resolv.conf: %s", f.operationFile)
 	inotify, err := fsnotify.NewWatcher()
 	if err != nil {
 		log.Errorf("failed to start inotify watcher for resolv.conf: %s", err)
@@ -60,7 +61,7 @@ func (f *repair) watchFileChanges(nbSearchDomains []string, nbNameserverIP strin
 				continue
 			}
 
-			log.Tracef("resolv.conf changed, check if it is broken")
+			log.Tracef("%s changed, check if it is broken", f.operationFile)
 
 			rConf, err := parseResolvConfFile(f.operationFile)
 			if err != nil {
@@ -124,8 +125,7 @@ func (f *repair) isEventRelevant(event fsnotify.Event) bool {
 		return false
 	}
 
-	operationFileSymlink := fmt.Sprintf("%s~", f.operationFile)
-	if event.Name == f.operationFile || event.Name == operationFileSymlink {
+	if event.Name == f.operationFile {
 		return true
 	}
 	return false
@@ -149,3 +149,11 @@ func isNbParamsMissing(nbSearchDomains []string, nbNameserverIP string, rConf *r
 
 	return false
 }
+
+func targetFile(filename string) string {
+	target, err := filepath.EvalSymlinks(filename)
+	if err != nil {
+		log.Errorf("evarl err: %s", err)
+	}
+	return target
+}

client/internal/dns/file_repair_linux_test.go

@@ -5,6 +5,7 @@ package dns
 import (
 	"context"
 	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -126,5 +127,49 @@ nameserver 8.8.8.8`,
 			}
 		})
 	}
+}
+
+func Test_newRepairSymlink(t *testing.T) {
+	resolvConfContent := `
+nameserver 10.0.0.1
+nameserver 8.8.8.8
+searchdomain netbird.cloud something`
+
+	modifyContent := `nameserver 8.8.8.8`
+
+	tmpResolvConf := filepath.Join(t.TempDir(), "resolv.conf")
+	err := os.WriteFile(tmpResolvConf, []byte(resolvConfContent), 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tmpLink := filepath.Join(t.TempDir(), "symlink")
+	err = os.Symlink(tmpResolvConf, tmpLink)
+	if err != nil {
+		t.Fatal(err)
+	}
 
+	var changed bool
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	updateFn := func([]string, string, *resolvConf) error {
+		changed = true
+		cancel()
+		return nil
+	}
+
+	r := newRepair(tmpLink, updateFn)
+	r.watchFileChanges([]string{"netbird.cloud"}, "10.0.0.1")
+
+	err = os.WriteFile(tmpLink, []byte(modifyContent), 0755)
+	if err != nil {
+		t.Fatalf("failed to write out resolv.conf: %s", err)
+	}
+
+	<-ctx.Done()
+
+	r.stopWatchFileChanges()
+
+	if changed != true {
+		t.Errorf("unexpected result: want: %v, got: %v", true, false)
+	}
 }