Skip to content

Commit

Permalink
NM: Make DNS settings the maximum to avoid DNS leak fallback
Browse files Browse the repository at this point in the history
NM has atrocious defaults. In the current situation, if you're
connected to OpenVPN/WireGuard (full tunnel, default gateway), DNS may
go outside of the VPN if the hostname only resolves in e.g. a DHCP
provided dns server. This is determined with the ipv4/ipv6
dns-priority setting and the dns-search setting.

We have to ensure that the dns-priority gets a negative value, from
nm-settings:

DNS servers priority. The relative priority for DNS servers specified
by this setting. A lower numerical value is better (higher
priority). Negative values have the special effect of excluding other
configurations with a greater numerical priority value; so in presence
of at least one negative priority, only DNS servers from connections
with the lowest priority value will be used. To avoid all DNS leaks,
set the priority of the profile that should be used to the most
negative value of all active connections profiles

We thus make the following change if full tunnel/default gateway:

- Set the DNS priority:
nmcli con modify eduVPN ipv4.dns-priority -2147483648 (int32 min)
nmcli con modify eduVPN ipv6.dns-priority -2147483648 (int32 min)

- Include the ~. DNS search domain:
nmcli con modify eduVPN ipv4.dns-search "~."
nmcli con modify eduVPN ipv6.dns-search "~."

Modifying the DNS search domain to ~. is needed according to
https://systemd.io/RESOLVED-VPNS/. This doc also states it's good to
set never-default to no (even if it doesn't do much), which we already
did for OpenVPN. Let's do the same for WireGuard.

Note that this only seems to happen with systemd-resolved which is the
default on Fedora. E.g. Openresolv resolvconf is not affected
  • Loading branch information
jwijenbergh committed Feb 2, 2024
1 parent 52b7d08 commit 602c435
Showing 1 changed file with 35 additions and 30 deletions.
65 changes: 35 additions & 30 deletions eduvpn/nm.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from shutil import rmtree
from socket import AF_INET, AF_INET6
from tempfile import mkdtemp
from typing import Any, Callable, List, Optional, TextIO, Tuple
from typing import Any, Callable, Optional, TextIO, Tuple

from eduvpn.ovpn import Ovpn
from eduvpn.storage import get_uuid, set_uuid, write_ovpn
Expand Down Expand Up @@ -394,12 +394,7 @@ def set_connection(
self,
new_connection: "NM.SimpleConnection",
callback: Callable,
default_gateway: Optional[bool],
dns_search_domains: List[str] = [],
):
new_connection = self.set_setting_ip_config(
new_connection, default_gateway, dns_search_domains
)
new_connection = self.set_setting_ensure_permissions(new_connection)
if self.uuid:
old_con = self.client.get_connection_by_uuid(self.uuid)
Expand All @@ -408,28 +403,6 @@ def set_connection(
return
self.add_connection(new_connection, callback)

def set_setting_ip_config(
self,
con: "NM.SimpleConnection",
default_gateway: Optional[bool],
dns_search_domains: List[str] = [],
) -> "NM.SimpleConnection":
"Set IP config settings like default gateway and search domains."
_logger.debug(
f"setting ip config, default gateway: {default_gateway}, dns_search_domains: {dns_search_domains}"
)
ipv4_setting = con.get_setting_ip4_config()
ipv6_setting = con.get_setting_ip6_config()
if default_gateway is not None:
ipv4_setting.set_property("never-default", not default_gateway)
ipv6_setting.set_property("never-default", not default_gateway)
if dns_search_domains:
ipv4_setting.set_property("dns-search", dns_search_domains)
ipv6_setting.set_property("dns-search", dns_search_domains)
con.add_setting(ipv4_setting)
con.add_setting(ipv6_setting)
return con

def set_setting_ensure_permissions(
self, con: "NM.SimpleConnection"
) -> "NM.SimpleConnection":
Expand All @@ -443,11 +416,31 @@ def start_openvpn_connection(
) -> None:
_logger.debug("writing ovpn configuration to Network Manager")
new_con = self.import_ovpn(ovpn)
self.set_connection(new_con, callback, default_gateway, dns_search_domains) # type: ignore
s_ip4 = new_con.get_setting_ip4_config()
s_ip6 = new_con.get_setting_ip6_config()

# avoid DNS leaks in default gateway
# see man nm-settings for dns-priority
# and https://systemd.io/RESOLVED-VPNS/
if default_gateway:
s_ip4.set_property(NM.SETTING_IP_CONFIG_DNS_PRIORITY, -2147483648)
s_ip6.set_property(NM.SETTING_IP_CONFIG_DNS_PRIORITY, -2147483648)
s_ip4.add_dns_search("~.")
s_ip6.add_dns_search("~.")
for i in dns_search_domains:
s_ip4.add_dns_search(i)
s_ip6.add_dns_search(i)
s_ip4.set_property("never-default", not default_gateway)
s_ip6.set_property("never-default", not default_gateway)
new_con.add_setting(s_ip4)
new_con.add_setting(s_ip6)

self.set_connection(new_con, callback) # type: ignore

def start_wireguard_connection( # noqa: C901
self,
config: ConfigParser,
default_gateway,
*,
allow_wg_lan=False,
callback=None,
Expand Down Expand Up @@ -517,10 +510,22 @@ def start_wireguard_connection( # noqa: C901
s_ip4.add_dns(i)
for i in dns6:
s_ip6.add_dns(i)

# avoid DNS leaks in default gateway
# see man nm-settings for dns-priority
# and https://systemd.io/RESOLVED-VPNS/
if default_gateway:
s_ip4.set_property(NM.SETTING_IP_CONFIG_DNS_PRIORITY, -2147483648)
s_ip6.set_property(NM.SETTING_IP_CONFIG_DNS_PRIORITY, -2147483648)
s_ip4.add_dns_search("~.")
s_ip6.add_dns_search("~.")
for i in dns_hostnames:
s_ip4.add_dns_search(i)
s_ip6.add_dns_search(i)

s_ip4.set_property("never-default", not default_gateway)
s_ip6.set_property("never-default", not default_gateway)

s_ip4.set_property(NM.SETTING_IP_CONFIG_METHOD, "manual")
s_ip6.set_property(NM.SETTING_IP_CONFIG_METHOD, "manual")

Expand Down Expand Up @@ -586,7 +591,7 @@ def start_wireguard_connection( # noqa: C901
profile.add_setting(s_con)
profile.add_setting(w_con)

self.set_connection(profile, callback, None) # type: ignore
self.set_connection(profile, callback) # type: ignore

@run_in_glib_thread
def activate_connection(self, callback: Optional[Callable] = None) -> None:
Expand Down

0 comments on commit 602c435

Please sign in to comment.