Properly parse dns resolver address (netbirdio#622)

Prevent panic when address is empty. Common with older managers, where resolver is disabled by default as we receive an empty dns config
pulsastrix · Dec 13, 2022 · 71e0a68 · 71e0a68
1 parent 833b681
commit 71e0a68
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 11 deletions.
diff --git a/client/internal/dns/network_manager_linux.go b/client/internal/dns/network_manager_linux.go
@@ -95,7 +95,10 @@ func (n *networkManagerDbusConfigurator) applyDNSConfig(config hostDNSConfig) er
 
 	connSettings.cleanDeprecatedSettings()
 
-	dnsIP := netip.MustParseAddr(config.serverIP)
+	dnsIP, err := netip.ParseAddr(config.serverIP)
+	if err != nil {
+		return fmt.Errorf("unable to parse ip address, error: %s", err)
+	}
 	convDNSIP := binary.LittleEndian.Uint32(dnsIP.AsSlice())
 	connSettings[networkManagerDbusIPv4Key][networkManagerDbusDNSKey] = dbus.MakeVariant([]uint32{convDNSIP})
 	var (

diff --git a/client/internal/dns/server_test.go b/client/internal/dns/server_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"github.com/miekg/dns"
 	nbdns "github.com/netbirdio/netbird/dns"
+	"github.com/netbirdio/netbird/iface"
 	"net"
 	"net/netip"
 	"os"
@@ -185,21 +186,52 @@ func TestUpdateDNSServer(t *testing.T) {
 			expectedUpstreamMap: make(registrationMap),
 			expectedLocalMap:    make(registrationMap),
 		},
+		{
+			name:                "Disabled Service Should clean map",
+			initLocalMap:        registrationMap{"netbird.cloud": struct{}{}},
+			initUpstreamMap:     registrationMap{zoneRecords[0].Name: struct{}{}},
+			initSerial:          0,
+			inputSerial:         1,
+			inputUpdate:         nbdns.Config{ServiceEnable: false},
+			expectedUpstreamMap: make(registrationMap),
+			expectedLocalMap:    make(registrationMap),
+		},
 	}
 
-	for _, testCase := range testCases {
+	for n, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			dnsServer := getDefaultServerWithNoHostManager("127.0.0.1")
-
-			dnsServer.hostManager = newNoopHostMocker()
+			wgIface, err := iface.NewWGIFace(fmt.Sprintf("utun230%d", n), fmt.Sprintf("100.66.100.%d/32", n+1), iface.DefaultMTU)
+			if err != nil {
+				t.Fatal(err)
+			}
+			err = wgIface.Create()
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer func() {
+				err = wgIface.Close()
+				if err != nil {
+					t.Log(err)
+				}
+			}()
+			dnsServer, err := NewDefaultServer(context.Background(), wgIface)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer func() {
+				err = dnsServer.hostManager.restoreHostDNS()
+				if err != nil {
+					t.Log(err)
+				}
+			}()
 
 			dnsServer.dnsMuxMap = testCase.initUpstreamMap
 			dnsServer.localResolver.registeredMap = testCase.initLocalMap
 			dnsServer.updateSerial = testCase.initSerial
 			// pretend we are running
 			dnsServer.listenerIsRunning = true
 
-			err := dnsServer.UpdateDNSServer(testCase.inputSerial, testCase.inputUpdate)
+			err = dnsServer.UpdateDNSServer(testCase.inputSerial, testCase.inputUpdate)
 			if err != nil {
 				if testCase.shouldFail {
 					return
@@ -241,9 +273,12 @@ func TestDNSServerStartStop(t *testing.T) {
 	}
 
 	dnsServer.hostManager = newNoopHostMocker()
-
 	dnsServer.Start()
-
+	time.Sleep(100 * time.Millisecond)
+	if !dnsServer.listenerIsRunning {
+		t.Fatal("dns server listener is not running")
+	}
+	defer dnsServer.Stop()
 	err := dnsServer.localResolver.registerRecord(zoneRecords[0])
 	if err != nil {
 		t.Error(err)

diff --git a/client/internal/dns/systemd_linux.go b/client/internal/dns/systemd_linux.go
@@ -75,12 +75,16 @@ func newSystemdDbusConfigurator(wgInterface *iface.WGIface) (hostManager, error)
 }
 
 func (s *systemdDbusConfigurator) applyDNSConfig(config hostDNSConfig) error {
-	parsedIP := netip.MustParseAddr(config.serverIP).As4()
+	parsedIP, err := netip.ParseAddr(config.serverIP)
+	if err != nil {
+		return fmt.Errorf("unable to parse ip address, error: %s", err)
+	}
+	ipAs4 := parsedIP.As4()
 	defaultLinkInput := systemdDbusDNSInput{
 		Family:  unix.AF_INET,
-		Address: parsedIP[:],
+		Address: ipAs4[:],
 	}
-	err := s.callLinkMethod(systemdDbusSetDNSMethodSuffix, []systemdDbusDNSInput{defaultLinkInput})
+	err = s.callLinkMethod(systemdDbusSetDNSMethodSuffix, []systemdDbusDNSInput{defaultLinkInput})
 	if err != nil {
 		return fmt.Errorf("setting the interface DNS server %s:%d failed with error: %s", config.serverIP, config.serverPort, err)
 	}