Improve dns handling on reconnect (#483)

daemon/jobs.go

@@ -95,9 +95,6 @@ func (r *RPC) StartKillSwitch() {
 
 	if cfg.KillSwitch {
 		allowlist := cfg.AutoConnectData.Allowlist
-		if cfg.LanDiscovery {
-			allowlist = addLANPermissions(allowlist)
-		}
 		if err := r.netw.SetKillSwitch(allowlist); err != nil {
 			log.Println(internal.ErrorPrefix, "starting killswitch:", err)
 			return

daemon/rpc_connect.go

@@ -139,9 +139,6 @@ func (r *RPC) Connect(in *pb.ConnectRequest, srv pb.Daemon_ConnectServer) (retEr
 	}
 
 	allowlist := cfg.AutoConnectData.Allowlist
-	if cfg.LanDiscovery {
-		allowlist = addLANPermissions(allowlist)
-	}
 
 	event.ServerFromAPI = remote
 	event.TargetServerCity = country.City.Name

daemon/rpc_set_allowlist.go

@@ -46,9 +46,6 @@ func (r *RPC) SetAllowlist(ctx context.Context, in *pb.SetAllowlistRequest) (*pb
 	// If LAN discovery is enabled, we want to append LANs to the new allowlist and modify the
 	// firewall. We do not want to add LANs to the configuration, so we have to create a copy.
 	firewallAllowlist := allowlist
-	if cfg.LanDiscovery {
-		firewallAllowlist = addLANPermissions(firewallAllowlist)
-	}
 
 	if err := r.netw.SetAllowlist(firewallAllowlist); err != nil {
 		log.Println(internal.ErrorPrefix, err)

daemon/rpc_set_killswitch.go

@@ -32,10 +32,6 @@ func (r *RPC) SetKillSwitch(ctx context.Context, in *pb.SetKillSwitchRequest) (*
 			in.GetAllowlist().GetSubnets(),
 		)
 
-		if cfg.LanDiscovery {
-			allowlist = addLANPermissions(allowlist)
-		}
-
 		if err := r.netw.SetKillSwitch(allowlist); err != nil {
 			log.Println(internal.ErrorPrefix, "enabling killswitch:", err)
 			return &pb.Payload{

daemon/rpc_set_lan_discovery.go

@@ -48,7 +48,7 @@ func (r *RPC) SetLANDiscovery(ctx context.Context, in *pb.SetLANDiscoveryRequest
 		}
 
 		cfg.AutoConnectData.Allowlist.Subnets = subnets
-		allowlist = addLANPermissions(cfg.AutoConnectData.Allowlist)
+		allowlist = cfg.AutoConnectData.Allowlist
 	}
 
 	if err := r.netw.SetAllowlist(allowlist); err != nil {

daemon/allowlist_lan.go → networker/allowlist_lan.go

@@ -1,4 +1,4 @@
-package daemon
+package networker
 
 import (
 	"github.com/NordSecurity/nordvpn-linux/config"

networker/networker.go

@@ -431,13 +431,7 @@ func (netw *Combined) restart(
 		return err
 	}
 
-	dnsGetter := &dns.NameServers{}
-	if netw.isMeshnetSet && defaultMeshSubnet.Contains(serverData.IP) {
-		err = netw.setDNS(dnsGetter.Get(false, false))
-	} else {
-		err = netw.setDNS(nameservers)
-	}
-	if err != nil {
+	if err := netw.configureDNS(serverData, nameservers); err != nil {
 		return err
 	}
 
@@ -900,6 +894,12 @@ func (netw *Combined) setAllowlist(allowlist config.Allowlist) error {
 		return err
 	}
 
+	// allow traffic to LAN - only when user enabled lan-discovery
+	if netw.lanDiscovery {
+		allowlist = addLANPermissions(allowlist)
+	}
+
+	// start adding set of rules
 	rules := []firewall.Rule{}
 	var subnets []netip.Prefix
 
@@ -925,12 +925,6 @@ func (netw *Combined) setAllowlist(allowlist config.Allowlist) error {
 			Direction:      firewall.TwoWay,
 			Allow:          true,
 		})
-		rules = append(rules, firewall.Rule{
-			Name:             "allowlist_forward_related",
-			Direction:        firewall.Forward,
-			Allow:            true,
-			ConnectionStates: firewall.ConnectionStates{States: []firewall.ConnectionState{firewall.Established, firewall.Related}},
-		})
 		rules = append(rules, firewall.Rule{
 			Name:           "allowlist_subnets_forward",
 			Interfaces:     ifaces,
@@ -968,10 +962,19 @@ func (netw *Combined) setAllowlist(allowlist config.Allowlist) error {
 			}
 		}
 	}
-
 	if err := netw.fw.Add(rules); err != nil {
 		return err
 	}
+
+	// if port 53 is whitelisted - do not add drop-dns rules
+	if !allowlist.Ports.TCP[53] && !allowlist.Ports.UDP[53] {
+		// disable DNS traffic to private LAN ranges - to prevent DNS leaks
+		// when /etc/resolv.conf has nameserver default gateway
+		if err := netw.denyDNS(); err != nil {
+			return err
+		}
+	}
+
 	netw.allowlist = allowlist
 
 	// adjust allow subnet routing rules
@@ -999,20 +1002,25 @@ func (netw *Combined) unsetAllowlist() error {
 	for _, rule := range []string{
 		"allowlist_subnets",
 		"allowlist_subnets_forward",
-		"allowlist_forward_related",
 		"allowlist_ports_tcp",
 		"allowlist_ports_udp",
 	} {
 		err := netw.fw.Delete([]string{rule})
 		if err != nil && !errors.Is(err, firewall.ErrRuleNotFound) {
-			return err
+			return fmt.Errorf("disabling allowlist firewall rules: %w", err)
 		}
 	}
 
 	if err := netw.allowlistRouting.Disable(); err != nil {
 		return fmt.Errorf("disabling allowlist routing: %w", err)
 	}
 
+	if !netw.allowlist.Ports.TCP[53] && !netw.allowlist.Ports.UDP[53] {
+		if err := netw.undenyDNS(); err != nil {
+			return fmt.Errorf("unsetting deny dns: %w", err)
+		}
+	}
+
 	return nil
 }
 
@@ -1505,6 +1513,54 @@ func (netw *Combined) allowFileshare(publicKey string, address netip.Addr) error
 	return nil
 }
 
+func (netw *Combined) undenyDNS() error {
+	ruleName := "deny-private-dns"
+
+	ruleIndex := slices.Index(netw.rules, ruleName)
+
+	if ruleIndex == -1 {
+		return nil
+	}
+
+	if err := netw.fw.Delete([]string{ruleName}); err != nil {
+		return err
+	}
+	netw.rules = slices.Delete(netw.rules, ruleIndex, ruleIndex+1)
+
+	return nil
+}
+
+func (netw *Combined) denyDNS() error {
+	ruleName := "deny-private-dns"
+	rules := []firewall.Rule{{
+		Name:           ruleName,
+		Direction:      firewall.Outbound,
+		Protocols:      []string{"udp", "tcp"},
+		Ports:          []int{53},
+		PortsDirection: firewall.Destination,
+		RemoteNetworks: []netip.Prefix{
+			netip.MustParsePrefix("10.0.0.0/8"),
+			netip.MustParsePrefix("172.16.0.0/12"),
+			netip.MustParsePrefix("192.168.0.0/16"),
+			netip.MustParsePrefix("169.254.0.0/16"),
+		},
+		Allow: false,
+	}}
+
+	ruleIndex := slices.Index(netw.rules, ruleName)
+
+	if ruleIndex != -1 {
+		return nil
+	}
+
+	if err := netw.fw.Add(rules); err != nil {
+		return fmt.Errorf("adding deny-private-dns rule to firewall: %w", err)
+	}
+
+	netw.rules = append(netw.rules, ruleName)
+	return nil
+}
+
 // Unblock address.
 func (netw *Combined) BlockIncoming(uniqueAddress meshnet.UniqueAddress) error {
 	netw.mu.Lock()

test/qa/lib/firewall.py

@@ -103,7 +103,6 @@
     "-A FORWARD -d 192.168.0.0/16 -o eth0 -m comment --comment nordvpn -j ACCEPT",
     "-A FORWARD -d 172.16.0.0/12 -o eth0 -m comment --comment nordvpn -j ACCEPT",
     "-A FORWARD -d 10.0.0.0/8 -o eth0 -m comment --comment nordvpn -j ACCEPT",
-    "-A FORWARD -m conntrack --ctstate RELATED,ESTABLISHED -m comment --comment nordvpn -j ACCEPT",
 ]
 
 OUTPUT_LAN_DISCOVERY_RULES = [
@@ -132,6 +131,14 @@ def __rules_connmark_chain_forward(interface: str):
 def __rules_connmark_chain_output(interface: str):
     return \
         [
+            "-A OUTPUT -d 169.254.0.0/16 -p tcp -m tcp --dport 53 -m comment --comment nordvpn -j DROP",
+            "-A OUTPUT -d 169.254.0.0/16 -p udp -m udp --dport 53 -m comment --comment nordvpn -j DROP",
+            "-A OUTPUT -d 192.168.0.0/16 -p tcp -m tcp --dport 53 -m comment --comment nordvpn -j DROP",
+            "-A OUTPUT -d 192.168.0.0/16 -p udp -m udp --dport 53 -m comment --comment nordvpn -j DROP",
+            "-A OUTPUT -d 172.16.0.0/12 -p tcp -m tcp --dport 53 -m comment --comment nordvpn -j DROP",
+            "-A OUTPUT -d 172.16.0.0/12 -p udp -m udp --dport 53 -m comment --comment nordvpn -j DROP",
+            "-A OUTPUT -d 10.0.0.0/8 -p tcp -m tcp --dport 53 -m comment --comment nordvpn -j DROP",
+            "-A OUTPUT -d 10.0.0.0/8 -p udp -m udp --dport 53 -m comment --comment nordvpn -j DROP",
             f"-A OUTPUT -o {interface} -m mark --mark 0xe1f1 -m comment --comment nordvpn -j CONNMARK --save-mark --nfmask 0xffffffff --ctmask 0xffffffff",
             f"-A OUTPUT -o {interface} -m connmark --mark 0xe1f1 -m comment --comment nordvpn -j ACCEPT",
             f"-A OUTPUT -o {interface} -m comment --comment nordvpn -j DROP"
@@ -164,15 +171,17 @@ def __rules_allowlist_subnet_chain_forward(interface: str, subnets: list[str]):
     for subnet in subnets:
         result += (f"-A FORWARD -d {subnet} -o {interface} -m comment --comment nordvpn -j ACCEPT", )
 
+    result += (f"-A FORWARD -o {interface} -m comment --comment nordvpn -j DROP", )
+
     current_subnet_rules_forward_chain = []
 
     fw_lines = os.popen("sudo iptables -S").read()
 
     for line in fw_lines.splitlines():
-        if "FORWARD" in line and "-d" in line:
+        if "FORWARD" in line and ("-d" in line or "DROP" in line):
             current_subnet_rules_forward_chain.append(line)
 
-    if current_subnet_rules_forward_chain:
+    if len(current_subnet_rules_forward_chain) > len(result):
         return sort_list_by_other_list(result, current_subnet_rules_forward_chain)
     else:
         return result
@@ -184,6 +193,15 @@ def __rules_allowlist_subnet_chain_output(interface: str, subnets: list[str]):
     for subnet in subnets:
         result += (f"-A OUTPUT -d {subnet} -o {interface} -m comment --comment nordvpn -j ACCEPT", )
 
+    result += ("-A OUTPUT -d 169.254.0.0/16 -p tcp -m tcp --dport 53 -m comment --comment nordvpn -j DROP", )
+    result += ("-A OUTPUT -d 169.254.0.0/16 -p udp -m udp --dport 53 -m comment --comment nordvpn -j DROP", )
+    result += ("-A OUTPUT -d 192.168.0.0/16 -p tcp -m tcp --dport 53 -m comment --comment nordvpn -j DROP", )
+    result += ("-A OUTPUT -d 192.168.0.0/16 -p udp -m udp --dport 53 -m comment --comment nordvpn -j DROP", )
+    result += ("-A OUTPUT -d 172.16.0.0/12 -p tcp -m tcp --dport 53 -m comment --comment nordvpn -j DROP", )
+    result += ("-A OUTPUT -d 172.16.0.0/12 -p udp -m udp --dport 53 -m comment --comment nordvpn -j DROP", )
+    result += ("-A OUTPUT -d 10.0.0.0/8 -p tcp -m tcp --dport 53 -m comment --comment nordvpn -j DROP", )
+    result += ("-A OUTPUT -d 10.0.0.0/8 -p udp -m udp --dport 53 -m comment --comment nordvpn -j DROP", )
+
     current_subnet_rules_input_chain = []
 
     fw_lines = os.popen("sudo iptables -S").read()
@@ -192,7 +210,7 @@ def __rules_allowlist_subnet_chain_output(interface: str, subnets: list[str]):
         if "OUTPUT" in line and "-d" in line:
             current_subnet_rules_input_chain.append(line)
 
-    if current_subnet_rules_input_chain:
+    if len(current_subnet_rules_input_chain) > len(result):
         return sort_list_by_other_list(result, current_subnet_rules_input_chain)
     else:
         return result

test/qa/test_allowlist_subnet.py

@@ -3,7 +3,6 @@
 
 import pytest
 import sh
-import timeout_decorator
 
 import lib
 from lib import (
@@ -58,8 +57,6 @@ def test_allowlist_does_not_create_new_routes_when_adding_deleting_subnets_disco
 
 
 @pytest.mark.parametrize(("tech", "proto", "obfuscated"), lib.TECHNOLOGIES)
-@pytest.mark.flaky(reruns=2, reruns_delay=90)
-@timeout_decorator.timeout(40)
 def test_connect_allowlist_subnet(tech, proto, obfuscated):
     lib.set_technology_and_protocol(tech, proto, obfuscated)
 
@@ -81,8 +78,6 @@ def test_connect_allowlist_subnet(tech, proto, obfuscated):
 
 
 @pytest.mark.parametrize(("tech", "proto", "obfuscated"), lib.TECHNOLOGIES)
-@pytest.mark.flaky(reruns=2, reruns_delay=90)
-@timeout_decorator.timeout(40)
 def test_allowlist_subnet_connect(tech, proto, obfuscated):
     lib.set_technology_and_protocol(tech, proto, obfuscated)
 
@@ -120,8 +115,6 @@ def test_allowlist_subnet_twice_disconnected(tech, proto, obfuscated, subnet):
 
 @pytest.mark.parametrize("subnet", lib.SUBNETS)
 @pytest.mark.parametrize(("tech", "proto", "obfuscated"), lib.TECHNOLOGIES)
-@pytest.mark.flaky(reruns=2, reruns_delay=90)
-@timeout_decorator.timeout(40)
 def test_allowlist_subnet_twice_connected(tech, proto, obfuscated, subnet):
     lib.set_technology_and_protocol(tech, proto, obfuscated)
 
@@ -156,8 +149,6 @@ def test_allowlist_subnet_and_remove_disconnected(tech, proto, obfuscated):
 
 
 @pytest.mark.parametrize(("tech", "proto", "obfuscated"), lib.TECHNOLOGIES)
-@pytest.mark.flaky(reruns=2, reruns_delay=90)
-@timeout_decorator.timeout(40)
 def test_allowlist_subnet_and_remove_connected(tech, proto, obfuscated):
     lib.set_technology_and_protocol(tech, proto, obfuscated)
 
@@ -192,8 +183,6 @@ def test_allowlist_subnet_remove_nonexistent_disconnected(tech, proto, obfuscate
 
 @pytest.mark.parametrize(("tech", "proto", "obfuscated"), lib.TECHNOLOGIES)
 @pytest.mark.parametrize("subnet", lib.SUBNETS)
-@pytest.mark.flaky(reruns=2, reruns_delay=90)
-@timeout_decorator.timeout(40)
 def test_allowlist_subnet_remove_nonexistent_connected(tech, proto, obfuscated, subnet):
     lib.set_technology_and_protocol(tech, proto, obfuscated)