Refactor WinFw

9 files changed, 133 insertions, 193 deletions

diff --git a/windows/winfw/src/winfw/fwcontext.cpp b/windows/winfw/src/winfw/fwcontext.cpp
index 931ced34a9..bf7301a464 100644
--- a/windows/winfw/src/winfw/fwcontext.cpp
+++ b/windows/winfw/src/winfw/fwcontext.cpp
@@ -10,11 +10,11 @@
 #include "rules/permitlan.h"
 #include "rules/permitlanservice.h"
 #include "rules/permitloopback.h"
+#include "rules/permittunneldns.h"
 #include "rules/permitvpnrelay.h"
 #include "rules/permitvpntunnel.h"
 #include "rules/permitvpntunnelservice.h"
 #include "rules/permitping.h"
-#include "rules/restrictdns.h"
 #include "libwfp/transaction.h"
 #include "libwfp/filterengine.h"
 #include <functional>
@@ -164,12 +164,10 @@ bool FwContext::applyPolicyConnected
 		tunnelInterfaceAlias
 	));
 
-	ruleset.emplace_back(std::make_unique<rules::RestrictDns>(
+	ruleset.emplace_back(std::make_unique<rules::PermitTunnelDns>(
 		tunnelInterfaceAlias,
-		wfp::IpAddress(std::wstring(v4DnsHost)),
-		nullptr != v6DnsHost ? std::make_optional<wfp::IpAddress>(std::wstring(v6DnsHost)) : std::nullopt,
-		std::wstring(relay.ip),
-		relay.port
+		wfp::IpAddress(v4DnsHost),
+		nullptr != v6DnsHost ? std::optional(wfp::IpAddress(v6DnsHost)) : std::nullopt
 	));
 
 	return applyRuleset(ruleset);
diff --git a/windows/winfw/src/winfw/mullvadguids.cpp b/windows/winfw/src/winfw/mullvadguids.cpp
index e73fac26ed..ef27e4823c 100644
--- a/windows/winfw/src/winfw/mullvadguids.cpp
+++ b/windows/winfw/src/winfw/mullvadguids.cpp
@@ -50,10 +50,8 @@ DetailedWfpObjectRegistry MullvadGuids::BuildDetailedRegistry()
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitVpnRelay()));
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitVpnTunnel_Outbound_Ipv4()));
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitVpnTunnel_Outbound_Ipv6()));
-	registry.insert(std::make_pair(WfpObjectType::Filter, FilterRestrictDns_Outbound_Ipv4()));
-	registry.insert(std::make_pair(WfpObjectType::Filter, FilterRestrictDns_Outbound_Tunnel_Ipv4()));
-	registry.insert(std::make_pair(WfpObjectType::Filter, FilterRestrictDns_Outbound_Ipv6()));
-	registry.insert(std::make_pair(WfpObjectType::Filter, FilterRestrictDns_Outbound_Tunnel_Ipv6()));
+	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitTunnelDns_Ipv4()));
+	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitTunnelDns_Ipv6()));
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitVpnTunnelService_Ipv4()));
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitVpnTunnelService_Ipv6()));
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitNdp_Outbound_Router_Solicitation()));
@@ -445,56 +443,28 @@ const GUID &MullvadGuids::FilterPermitVpnTunnel_Outbound_Ipv6()
 }
 
 //static
-const GUID &MullvadGuids::FilterRestrictDns_Outbound_Ipv4()
+const GUID &MullvadGuids::FilterPermitTunnelDns_Ipv4()
 {
 	static const GUID g =
 	{
-		0xc0792b44,
-		0xfc3c,
-		0x42e8,
-		{ 0xa6, 0x60, 0x25, 0x4b, 0xd0, 0x4, 0xb1, 0x9d }
+		0x60474363,
+		0x42b7,
+		0x44ad,
+		{ 0xa6, 0xdb, 0x9c, 0x4a, 0x4d, 0x3c, 0xde, 0x4a }
 	};
 
 	return g;
 }
 
 //static
-const GUID &MullvadGuids::FilterRestrictDns_Outbound_Tunnel_Ipv4()
+const GUID &MullvadGuids::FilterPermitTunnelDns_Ipv6()
 {
 	static const GUID g =
 	{
-		0x790445dc,
-		0xb23e,
-		0x4ab4,
-		{ 0x8e, 0x2f, 0xc7, 0x6, 0x55, 0x5f, 0x94, 0xff }
-	};
-
-	return g;
-}
-
-//static
-const GUID &MullvadGuids::FilterRestrictDns_Outbound_Ipv6()
-{
-	static const GUID g =
-	{
-		0xcde477eb,
-		0x2d8a,
-		0x45b8,
-		{ 0x9a, 0x3e, 0x9a, 0xa3, 0xbe, 0x4d, 0xe2, 0xb4 }
-	};
-
-	return g;
-}
-
-//static
-const GUID &MullvadGuids::FilterRestrictDns_Outbound_Tunnel_Ipv6()
-{
-	static const GUID g =
-	{
-		0xacc90d87,
-		0xab77,
-		0x4cf4,
-		{ 0x84, 0xee, 0x1d, 0x68, 0x95, 0xf0, 0x66, 0xc2 }
+		0xa832ce1d,
+		0xa250,
+		0x42be,
+		{ 0x8b, 0x97, 0x2, 0xb7, 0x9f, 0x9c, 0x5e, 0x1 }
 	};
 
 	return g;
diff --git a/windows/winfw/src/winfw/mullvadguids.h b/windows/winfw/src/winfw/mullvadguids.h
index 3c3ca9702b..8a32c1c9df 100644
--- a/windows/winfw/src/winfw/mullvadguids.h
+++ b/windows/winfw/src/winfw/mullvadguids.h
@@ -56,10 +56,8 @@ public:
 	static const GUID &FilterPermitVpnTunnel_Outbound_Ipv4();
 	static const GUID &FilterPermitVpnTunnel_Outbound_Ipv6();
 
-	static const GUID &FilterRestrictDns_Outbound_Ipv4();
-	static const GUID &FilterRestrictDns_Outbound_Tunnel_Ipv4();
-	static const GUID &FilterRestrictDns_Outbound_Ipv6();
-	static const GUID &FilterRestrictDns_Outbound_Tunnel_Ipv6();
+	static const GUID &FilterPermitTunnelDns_Ipv4();
+	static const GUID &FilterPermitTunnelDns_Ipv6();
 
 	static const GUID &FilterPermitVpnTunnelService_Ipv4();
 	static const GUID &FilterPermitVpnTunnelService_Ipv6();
diff --git a/windows/winfw/src/winfw/rules/permittunneldns.cpp b/windows/winfw/src/winfw/rules/permittunneldns.cpp
new file mode 100644
index 0000000000..1aafc15f99
--- /dev/null
+++ b/windows/winfw/src/winfw/rules/permittunneldns.cpp
@@ -0,0 +1,93 @@
+#include "stdafx.h"
+#include "permittunneldns.h"
+#include "winfw/mullvadguids.h"
+#include "libwfp/filterbuilder.h"
+#include "libwfp/conditionbuilder.h"
+#include "libwfp/conditions/comparison.h"
+#include "libwfp/conditions/conditioninterface.h"
+#include "libwfp/conditions/conditionip.h"
+#include "libwfp/conditions/conditionport.h"
+
+using namespace wfp::conditions;
+
+namespace
+{
+
+constexpr uint16_t DNS_PORT = 53;
+
+} // anonymous namespace
+
+namespace rules
+{
+
+PermitTunnelDns::PermitTunnelDns(
+	const std::wstring &tunnelInterfaceAlias,
+	const wfp::IpAddress v4DnsHost,
+	const std::optional<wfp::IpAddress> v6DnsHost
+)
+	: m_tunnelInterfaceAlias(tunnelInterfaceAlias)
+	, m_v4DnsHost(v4DnsHost)
+	, m_v6DnsHost(v6DnsHost)
+{
+
+}
+
+bool PermitTunnelDns::apply(IObjectInstaller &objectInstaller)
+{
+	//
+	// Permit outbound DNS traffic to a specific DNS server (IPv4)
+	//
+
+	wfp::FilterBuilder filterBuilder;
+
+	filterBuilder
+		.provider(MullvadGuids::Provider())
+		.key(MullvadGuids::FilterPermitTunnelDns_Ipv4())
+		.name(L"Permit outbound DNS traffic on tunnel interface (IPv4)")
+		.description(L"This filter is part of a rule that permits DNS traffic inside the VPN tunnel")
+		.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V4)
+		.sublayer(MullvadGuids::SublayerWhitelist())
+		.weight(wfp::FilterBuilder::WeightClass::Max)
+		.permit();
+
+	{
+		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V4);
+		conditionBuilder.add_condition(ConditionInterface::Alias(m_tunnelInterfaceAlias));
+		conditionBuilder.add_condition(ConditionIp::Remote(m_v4DnsHost));
+		conditionBuilder.add_condition(ConditionPort::Remote(DNS_PORT));
+
+		if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
+		{
+			return false;
+		}
+	}
+
+	//
+	// Permit outbound DNS traffic to a specific DNS server (IPv6)
+	//
+
+	if (m_v6DnsHost.has_value())
+	{
+		filterBuilder
+			.key(MullvadGuids::FilterPermitTunnelDns_Ipv6())
+			.name(L"Permit outbound DNS traffic on tunnel interface (IPv6)")
+			.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
+
+		{
+			wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
+
+			conditionBuilder.add_condition(ConditionInterface::Alias(m_tunnelInterfaceAlias));
+			conditionBuilder.add_condition(ConditionIp::Remote(m_v6DnsHost.value()));
+			conditionBuilder.add_condition(ConditionPort::Remote(DNS_PORT));
+
+			if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
+			{
+				return false;
+			}
+		}
+	}
+
+	return true;
+}
+
+}
diff --git a/windows/winfw/src/winfw/rules/restrictdns.h b/windows/winfw/src/winfw/rules/permittunneldns.h
index cdfe1f4697..6c3baab25f 100644
--- a/windows/winfw/src/winfw/rules/restrictdns.h
+++ b/windows/winfw/src/winfw/rules/permittunneldns.h
@@ -9,11 +9,11 @@
 namespace rules
 {
 
-class RestrictDns : public IFirewallRule
+class PermitTunnelDns : public IFirewallRule
 {
 public:
 
-	RestrictDns(const std::wstring &tunnelInterfaceAlias, const wfp::IpAddress v4DnsHost, std::optional<wfp::IpAddress> v6DnsHost, wfp::IpAddress relay, uint16_t relayPort);
+	PermitTunnelDns::PermitTunnelDns(const std::wstring &tunnelInterfaceAlias, const wfp::IpAddress v4DnsHost, const std::optional<wfp::IpAddress> v6DnsHost);
 
 	bool apply(IObjectInstaller &objectInstaller) override;
 
@@ -22,8 +22,6 @@ private:
 	const std::wstring m_tunnelInterfaceAlias;
 	const wfp::IpAddress m_v4DnsHost;
 	const std::optional<wfp::IpAddress> m_v6DnsHost;
-	const uint16_t m_relayPort;
-	const wfp::IpAddress m_relayHost;
 
 };
 
diff --git a/windows/winfw/src/winfw/rules/permitvpntunnel.cpp b/windows/winfw/src/winfw/rules/permitvpntunnel.cpp
index e21a99c04d..a757f5e164 100644
--- a/windows/winfw/src/winfw/rules/permitvpntunnel.cpp
+++ b/windows/winfw/src/winfw/rules/permitvpntunnel.cpp
@@ -4,9 +4,17 @@
 #include "libwfp/filterbuilder.h"
 #include "libwfp/conditionbuilder.h"
 #include "libwfp/conditions/conditioninterface.h"
+#include "libwfp/conditions/conditionport.h"
 
 using namespace wfp::conditions;
 
+namespace
+{
+
+constexpr uint16_t DNS_PORT = 53;
+
+} // anonymous namespace
+
 namespace rules
 {
 
@@ -21,6 +29,7 @@ bool PermitVpnTunnel::apply(IObjectInstaller &objectInstaller)
 
 	//
 	// #1 permit locally-initiated traffic on tunnel interface, ipv4
+	//    except DNS requests
 	//
 
 	filterBuilder
@@ -37,6 +46,7 @@ bool PermitVpnTunnel::apply(IObjectInstaller &objectInstaller)
 		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V4);
 
 		conditionBuilder.add_condition(ConditionInterface::Alias(m_tunnelInterfaceAlias));
+		conditionBuilder.add_condition(ConditionPort::Remote(DNS_PORT, CompareNeq()));
 
 		if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
 		{
@@ -46,6 +56,7 @@ bool PermitVpnTunnel::apply(IObjectInstaller &objectInstaller)
 
 	//
 	// #2 permit locally-initiated traffic on tunnel interface, ipv6
+	//    except DNS requests
 	//
 
 	filterBuilder
@@ -56,6 +67,7 @@ bool PermitVpnTunnel::apply(IObjectInstaller &objectInstaller)
 	wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
 
 	conditionBuilder.add_condition(ConditionInterface::Alias(m_tunnelInterfaceAlias));
+	conditionBuilder.add_condition(ConditionPort::Remote(DNS_PORT, CompareNeq()));
 
 	return objectInstaller.addFilter(filterBuilder, conditionBuilder);
 }
diff --git a/windows/winfw/src/winfw/rules/restrictdns.cpp b/windows/winfw/src/winfw/rules/restrictdns.cpp
deleted file mode 100644
index efa4c8421b..0000000000
--- a/windows/winfw/src/winfw/rules/restrictdns.cpp
+++ /dev/null
@@ -1,129 +0,0 @@
-#include "stdafx.h"
-#include "restrictdns.h"
-#include "winfw/mullvadguids.h"
-#include "libwfp/filterbuilder.h"
-#include "libwfp/conditionbuilder.h"
-#include "libwfp/conditions/conditioninterface.h"
-#include "libwfp/conditions/conditionip.h"
-#include "libwfp/conditions/conditionport.h"
-
-using namespace wfp::conditions;
-
-namespace rules
-{
-
-RestrictDns::RestrictDns(const std::wstring &tunnelInterfaceAlias,
-	const wfp::IpAddress v4DnsHost,
-	std::optional<wfp::IpAddress> v6DnsHost,
-	wfp::IpAddress relay,
-	uint16_t relayPort)
-	: m_tunnelInterfaceAlias(tunnelInterfaceAlias)
-	, m_v4DnsHost(v4DnsHost)
-	, m_v6DnsHost(v6DnsHost)
-	, m_relayHost(relay)
-	, m_relayPort(relayPort)
-
-{
-}
-
-bool RestrictDns::apply(IObjectInstaller &objectInstaller)
-{
-	wfp::FilterBuilder filterBuilder;
-
-	//
-	// Requires that the following rules are in effect:
-	//
-	// BlockAll
-	// PermitVpnTunnel
-	//
-	// TODO: Have each rule specify requirements?
-	//
-
-	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();
-
-	{
-		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()));
-
-		if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
-		{
-			return false;
-		}
-	}
-
-	filterBuilder
-		.key(MullvadGuids::FilterRestrictDns_Outbound_Ipv4())
-		.name(L"Block DNS requests outside the VPN tunnel (IPv4)")
-		.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V4)
-		.weight(MAXUINT16 - 1)
-		.block();
-
-	{
-		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V4);
-		conditionBuilder.add_condition(ConditionPort::Remote(53));
-
-		if (53 == m_relayPort)
-		{
-			//
-			// Allow relay traffic over port 53
-			//
-			conditionBuilder.add_condition(ConditionIp::Remote(m_relayHost, CompareNeq()));
-		}
-
-		if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
-		{
-			return false;
-		}
-	}
-
-	//
-	// IPv6 also
-	//
-
-	if (m_v6DnsHost.has_value())
-	{
-		filterBuilder
-			.key(MullvadGuids::FilterRestrictDns_Outbound_Tunnel_Ipv6())
-			.name(L"Restrict DNS requests inside the VPN tunnel (IPv6)")
-			.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V6)
-			.weight(MAXUINT16)
-			.permit();
-
-		{
-			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()));
-
-			if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
-			{
-				return false;
-			}
-		}
-	}
-
-	filterBuilder
-		.key(MullvadGuids::FilterRestrictDns_Outbound_Ipv6())
-		.name(L"Block DNS requests outside the VPN tunnel (IPv6)")
-		.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V6)
-		.weight(MAXUINT16 - 1)
-		.block();
-
-	{
-		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
-		conditionBuilder.add_condition(ConditionPort::Remote(53));
-		return objectInstaller.addFilter(filterBuilder, conditionBuilder);
-	}
-}
-
-}
diff --git a/windows/winfw/src/winfw/winfw.vcxproj b/windows/winfw/src/winfw/winfw.vcxproj
index 15da42ec0f..7b35c8b939 100644
--- a/windows/winfw/src/winfw/winfw.vcxproj
+++ b/windows/winfw/src/winfw/winfw.vcxproj
@@ -23,6 +23,7 @@
     <ClCompile Include="mullvadguids.cpp" />
     <ClCompile Include="mullvadobjects.cpp" />
     <ClCompile Include="objectpurger.cpp" />
+    <ClCompile Include="rules\permittunneldns.cpp" />
     <ClCompile Include="rules\blockall.cpp" />
     <ClCompile Include="rules\permitdhcp.cpp" />
     <ClCompile Include="rules\permitdhcpserver.cpp" />
@@ -34,7 +35,6 @@
     <ClCompile Include="rules\permitvpntunnelservice.cpp" />
     <ClCompile Include="rules\permitvpnrelay.cpp" />
     <ClCompile Include="rules\permitvpntunnel.cpp" />
-    <ClCompile Include="rules\restrictdns.cpp" />
     <ClCompile Include="sessioncontroller.cpp" />
     <ClCompile Include="sessionrecord.cpp" />
     <ClCompile Include="stdafx.cpp">
@@ -52,6 +52,7 @@
     <ClInclude Include="mullvadguids.h" />
     <ClInclude Include="mullvadobjects.h" />
     <ClInclude Include="objectpurger.h" />
+    <ClInclude Include="rules\permittunneldns.h" />
     <ClInclude Include="rules\permitdhcpserver.h" />
     <ClInclude Include="rules\permitndp.h" />
     <ClInclude Include="rules\permitping.h" />
@@ -65,7 +66,6 @@
     <ClInclude Include="rules\permitvpntunnelservice.h" />
     <ClInclude Include="rules\permitvpnrelay.h" />
     <ClInclude Include="rules\permitvpntunnel.h" />
-    <ClInclude Include="rules\restrictdns.h" />
     <ClInclude Include="sessioncontroller.h" />
     <ClInclude Include="sessionrecord.h" />
     <ClInclude Include="stdafx.h" />
diff --git a/windows/winfw/src/winfw/winfw.vcxproj.filters b/windows/winfw/src/winfw/winfw.vcxproj.filters
index a758a1c9ec..c491cb2a8d 100644
--- a/windows/winfw/src/winfw/winfw.vcxproj.filters
+++ b/windows/winfw/src/winfw/winfw.vcxproj.filters
@@ -30,9 +30,6 @@
       <Filter>rules</Filter>
     </ClCompile>
     <ClCompile Include="sessionrecord.cpp" />
-    <ClCompile Include="rules\restrictdns.cpp">
-      <Filter>rules</Filter>
-    </ClCompile>
     <ClCompile Include="rules\permitvpntunnelservice.cpp">
       <Filter>rules</Filter>
     </ClCompile>
@@ -46,6 +43,9 @@
     <ClCompile Include="rules\permitping.cpp">
       <Filter>rules</Filter>
     </ClCompile>
+    <ClCompile Include="rules\permittunneldns.cpp">
+      <Filter>rules</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="stdafx.h" />
@@ -81,9 +81,6 @@
       <Filter>rules</Filter>
     </ClInclude>
     <ClInclude Include="sessionrecord.h" />
-    <ClInclude Include="rules\restrictdns.h">
-      <Filter>rules</Filter>
-    </ClInclude>
     <ClInclude Include="rules\permitvpntunnelservice.h">
       <Filter>rules</Filter>
     </ClInclude>
@@ -99,6 +96,9 @@
     <ClInclude Include="rules\permitping.h">
       <Filter>rules</Filter>
     </ClInclude>
+    <ClInclude Include="rules\permittunneldns.h">
+      <Filter>rules</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <Filter Include="rules">