Prevent DNS leaks in connecting and error states on Windows with LAN sharing enabled

3 files changed, 52 insertions, 42 deletions

diff --git a/windows/winfw/src/winfw/fwcontext.cpp b/windows/winfw/src/winfw/fwcontext.cpp
index 610936de0b..609f097336 100644
--- a/windows/winfw/src/winfw/fwcontext.cpp
+++ b/windows/winfw/src/winfw/fwcontext.cpp
@@ -53,10 +53,11 @@ void AppendSettingsRules(FwContext::Ruleset &ruleset, const WinFwSettings &setti
 	}
 }
 
-void AppendNetBlockedRules(FwContext::Ruleset &ruleset)
+void AppendNetBlockedRules(FwContext::Ruleset &ruleset, const std::optional<WinFwRelay> &relay, const std::optional<rules::RestrictDns::DnsHosts> &dnsHosts)
 {
 	ruleset.emplace_back(std::make_unique<rules::BlockAll>());
 	ruleset.emplace_back(std::make_unique<rules::PermitLoopback>());
+	ruleset.emplace_back(std::make_unique<rules::RestrictDns>(relay, dnsHosts));
 }
 
 } // anonymous namespace
@@ -108,7 +109,7 @@ bool FwContext::applyPolicyConnecting
 {
 	Ruleset ruleset;
 
-	AppendNetBlockedRules(ruleset);
+	AppendNetBlockedRules(ruleset, relay, std::nullopt);
 	AppendSettingsRules(ruleset, settings);
 
 	ruleset.emplace_back(std::make_unique<rules::PermitVpnRelay>(
@@ -147,7 +148,13 @@ bool FwContext::applyPolicyConnected
 {
 	Ruleset ruleset;
 
-	AppendNetBlockedRules(ruleset);
+	rules::RestrictDns::DnsHosts dnsHosts =
+	{
+		tunnelInterfaceAlias,
+		v4DnsHost,
+		v6DnsHost
+	};
+	AppendNetBlockedRules(ruleset, relay, dnsHosts);
 	AppendSettingsRules(ruleset, settings);
 
 	ruleset.emplace_back(std::make_unique<rules::PermitVpnRelay>(
@@ -164,13 +171,6 @@ bool FwContext::applyPolicyConnected
 		tunnelInterfaceAlias
 	));
 
-	ruleset.emplace_back(std::make_unique<rules::RestrictDns>(
-		tunnelInterfaceAlias,
-		v4DnsHost,
-		v6DnsHost,
-		53 == relay.port ? std::make_optional(wfp::IpAddress(relay.ip)) : std::nullopt
-	));
-
 	return applyRuleset(ruleset);
 }
 
@@ -191,7 +191,7 @@ FwContext::Ruleset FwContext::composePolicyBlocked(const WinFwSettings &settings
 {
 	Ruleset ruleset;
 
-	AppendNetBlockedRules(ruleset);
+	AppendNetBlockedRules(ruleset, std::nullopt, std::nullopt);
 	AppendSettingsRules(ruleset, settings);
 
 	return ruleset;
diff --git a/windows/winfw/src/winfw/rules/restrictdns.cpp b/windows/winfw/src/winfw/rules/restrictdns.cpp
index e1c81bfd53..751e278233 100644
--- a/windows/winfw/src/winfw/rules/restrictdns.cpp
+++ b/windows/winfw/src/winfw/rules/restrictdns.cpp
@@ -13,16 +13,15 @@ namespace rules
 {
 
 RestrictDns::RestrictDns(
-	const std::wstring &tunnelInterfaceAlias,
-	const wfp::IpAddress v4DnsHost,
-	std::optional<wfp::IpAddress> v6DnsHost,
-	std::optional<wfp::IpAddress> allowHost
+	const std::optional<WinFwRelay> &relay,
+	const std::optional<DnsHosts> &dnsHosts
 )
-	: m_tunnelInterfaceAlias(tunnelInterfaceAlias)
-	, m_v4DnsHost(v4DnsHost)
-	, m_v6DnsHost(v6DnsHost)
-	, m_allowHost(allowHost)
+	: m_dnsHosts(dnsHosts)
 {
+	if (relay.has_value() && 53 == relay->port)
+	{
+		m_allowHost = std::make_optional(wfp::IpAddress(relay->ip));
+	}
 }
 
 bool RestrictDns::apply(IObjectInstaller &objectInstaller)
@@ -41,22 +40,27 @@ bool RestrictDns::apply(IObjectInstaller &objectInstaller)
 	filterBuilder
 		.provider(MullvadGuids::Provider())
 		.description(L"This filter is part of a rule that restricts DNS traffic")
-		.sublayer(MullvadGuids::SublayerBlacklist())
-		.key(MullvadGuids::FilterRestrictDns_Outbound_Tunnel_Ipv4())
-		.name(L"Restrict DNS requests inside the VPN tunnel (IPv4)")
-		.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V4)
-		.weight(MAXUINT16)
-		.permit();
+		.sublayer(MullvadGuids::SublayerBlacklist());
 
+	if (m_dnsHosts.has_value())
 	{
-		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V4);
-
-		conditionBuilder.add_condition(ConditionInterface::Alias(m_tunnelInterfaceAlias, CompareEq()));
-		conditionBuilder.add_condition(ConditionIp::Remote(m_v4DnsHost, CompareEq()));
+		filterBuilder
+			.key(MullvadGuids::FilterRestrictDns_Outbound_Tunnel_Ipv4())
+			.name(L"Restrict DNS requests inside the VPN tunnel (IPv4)")
+			.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V4)
+			.weight(MAXUINT16)
+			.permit();
 
-		if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
 		{
-			return false;
+			wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V4);
+
+			conditionBuilder.add_condition(ConditionInterface::Alias(m_dnsHosts->tunnelInterfaceAlias, CompareEq()));
+			conditionBuilder.add_condition(ConditionIp::Remote(m_dnsHosts->v4DnsHost, CompareEq()));
+
+			if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
+			{
+				return false;
+			}
 		}
 	}
 
@@ -71,11 +75,11 @@ bool RestrictDns::apply(IObjectInstaller &objectInstaller)
 		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V4);
 		conditionBuilder.add_condition(ConditionPort::Remote(53));
 
+		//
+		// Allow DNS traffic over select host
+		//
 		if (m_allowHost.has_value())
 		{
-			//
-			// Allow DNS traffic over select host
-			//
 			conditionBuilder.add_condition(ConditionIp::Remote(*m_allowHost, CompareNeq()));
 		}
 
@@ -89,7 +93,7 @@ bool RestrictDns::apply(IObjectInstaller &objectInstaller)
 	// IPv6 also
 	//
 
-	if (m_v6DnsHost.has_value())
+	if (m_dnsHosts.has_value() && m_dnsHosts->v6DnsHost.has_value())
 	{
 		filterBuilder
 			.key(MullvadGuids::FilterRestrictDns_Outbound_Tunnel_Ipv6())
@@ -101,8 +105,8 @@ bool RestrictDns::apply(IObjectInstaller &objectInstaller)
 		{
 			wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
 
-			conditionBuilder.add_condition(ConditionInterface::Alias(m_tunnelInterfaceAlias, CompareEq()));
-			conditionBuilder.add_condition(ConditionIp::Remote(m_v6DnsHost.value(), CompareEq()));
+			conditionBuilder.add_condition(ConditionInterface::Alias(m_dnsHosts->tunnelInterfaceAlias, CompareEq()));
+			conditionBuilder.add_condition(ConditionIp::Remote(*m_dnsHosts->v6DnsHost, CompareEq()));
 
 			if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
 			{
diff --git a/windows/winfw/src/winfw/rules/restrictdns.h b/windows/winfw/src/winfw/rules/restrictdns.h
index 0d730e4a32..9cf0ad00a9 100644
--- a/windows/winfw/src/winfw/rules/restrictdns.h
+++ b/windows/winfw/src/winfw/rules/restrictdns.h
@@ -2,6 +2,7 @@
 
 #include "ifirewallrule.h"
 #include "libwfp/ipaddress.h"
+#include "winfw/winfw.h"
 #include <optional>
 #include <string>
 #include <cstdint>
@@ -13,16 +14,21 @@ class RestrictDns : public IFirewallRule
 {
 public:
 
-	RestrictDns(const std::wstring &tunnelInterfaceAlias, const wfp::IpAddress v4DnsHost, std::optional<wfp::IpAddress> v6DnsHost, std::optional<wfp::IpAddress> allowHost);
+	struct DnsHosts
+	{
+		std::wstring tunnelInterfaceAlias;
+		wfp::IpAddress v4DnsHost;
+		std::optional<wfp::IpAddress> v6DnsHost;
+	};
+
+	RestrictDns(const std::optional<WinFwRelay> &relay, const std::optional<DnsHosts> &dnsHosts);
 
 	bool apply(IObjectInstaller &objectInstaller) override;
 
 private:
 
-	const std::wstring m_tunnelInterfaceAlias;
-	const wfp::IpAddress m_v4DnsHost;
-	const std::optional<wfp::IpAddress> m_v6DnsHost;
-	const std::optional<wfp::IpAddress> m_allowHost;
+	std::optional<wfp::IpAddress> m_allowHost;
+	const std::optional<DnsHosts> m_dnsHosts;
 
 };