Clean up 'PermitDhcp' rule

2 files changed, 54 insertions, 35 deletions

diff --git a/windows/winfw/src/winfw/rules/permitdhcp.cpp b/windows/winfw/src/winfw/rules/permitdhcp.cpp
index 4650a3586f..1a52865cf1 100644
--- a/windows/winfw/src/winfw/rules/permitdhcp.cpp
+++ b/windows/winfw/src/winfw/rules/permitdhcp.cpp
@@ -7,15 +7,29 @@
 #include "libwfp/conditions/conditionprotocol.h"
 #include "libwfp/conditions/conditionport.h"
 #include "libwfp/conditions/conditionip.h"
-#include "libwfp/conditions/conditionport.h"
 
 using namespace wfp::conditions;
 
 namespace rules
 {
 
+namespace
+{
+
+static const uint32_t DHCPV4_CLIENT_PORT = 68;
+static const uint32_t DHCPV4_SERVER_PORT = 67;
+static const uint32_t DHCPV6_CLIENT_PORT = 546;
+static const uint32_t DHCPV6_SERVER_PORT = 547;
+
+} // anonymous namespace
+
 bool PermitDhcp::apply(IObjectInstaller &objectInstaller)
 {
+	return applyIpv4(objectInstaller) && applyIpv6(objectInstaller);
+}
+
+bool PermitDhcp::applyIpv4(IObjectInstaller &objectInstaller) const
+{
 	//
 	// First UDP packet for a unique [remote address, port] tuple is mapped into:
 	//
@@ -25,8 +39,6 @@ bool PermitDhcp::apply(IObjectInstaller &objectInstaller)
 
 	wfp::FilterBuilder filterBuilder;
 
-	const wfp::IpAddress::Literal6 fe80{ 0xFE80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 };
-
 	//
 	// #1 permit outbound DHCPv4 request
 	//
@@ -45,9 +57,9 @@ bool PermitDhcp::apply(IObjectInstaller &objectInstaller)
 		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V4);
 
 		conditionBuilder.add_condition(ConditionProtocol::Udp());
-		conditionBuilder.add_condition(ConditionPort::Local(68));
+		conditionBuilder.add_condition(ConditionPort::Local(DHCPV4_CLIENT_PORT));
 		conditionBuilder.add_condition(ConditionIp::Remote(wfp::IpAddress::Literal({ 255, 255, 255, 255 })));
-		conditionBuilder.add_condition(ConditionPort::Remote(67));
+		conditionBuilder.add_condition(ConditionPort::Remote(DHCPV4_SERVER_PORT));
 
 		if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
 		{
@@ -56,48 +68,50 @@ bool PermitDhcp::apply(IObjectInstaller &objectInstaller)
 	}
 
 	//
-	// #2 permit outbound DHCPv6 request
+	// #2 permit inbound DHCPv4 response
 	//
 
 	filterBuilder
-		.key(MullvadGuids::FilterPermitDhcpV6_Outbound_Request())
-		.name(L"Permit outbound DHCPv6 request")
-		.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
+		.key(MullvadGuids::FilterPermitDhcpV4_Inbound_Response())
+		.name(L"Permit inbound DHCPv4 response")
+		.layer(FWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V4);
 
-	{
-		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
+	wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V4);
 
-		const wfp::IpAddress::Literal6 linkLocal{ 0xFF02, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x2 };
-		const wfp::IpAddress::Literal6 siteLocal{ 0xFF05, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x3 };
+	conditionBuilder.add_condition(ConditionProtocol::Udp());
+	conditionBuilder.add_condition(ConditionPort::Local(DHCPV4_CLIENT_PORT));
+	conditionBuilder.add_condition(ConditionPort::Remote(DHCPV4_SERVER_PORT));
 
-		conditionBuilder.add_condition(ConditionProtocol::Udp());
-		conditionBuilder.add_condition(ConditionIp::Remote(linkLocal));
-		conditionBuilder.add_condition(ConditionIp::Remote(siteLocal));
-		conditionBuilder.add_condition(ConditionPort::Remote(547));
-		conditionBuilder.add_condition(ConditionIp::Local(fe80, uint8_t(10)));
-		conditionBuilder.add_condition(ConditionPort::Local(546));
+	return objectInstaller.addFilter(filterBuilder, conditionBuilder);
+}
 
-		if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
-		{
-			return false;
-		}
-	}
+bool PermitDhcp::applyIpv6(IObjectInstaller &objectInstaller) const
+{
+	const wfp::IpAddress::Literal6 fe80{ 0xFE80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 };
+
+	wfp::FilterBuilder filterBuilder;
 
 	//
-	// #3 permit inbound DHCPv4 response
+	// #1 permit outbound DHCPv6 request
 	//
 
 	filterBuilder
-		.key(MullvadGuids::FilterPermitDhcpV4_Inbound_Response())
-		.name(L"Permit inbound DHCPv4 response")
-		.layer(FWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V4);
+		.key(MullvadGuids::FilterPermitDhcpV6_Outbound_Request())
+		.name(L"Permit outbound DHCPv6 request")
+		.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
 
 	{
-		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V4);
+		wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
+
+		const wfp::IpAddress::Literal6 linkLocalDhcpMulticast{ 0xFF02, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x2 };
+		const wfp::IpAddress::Literal6 siteLocalDhcpMulticast{ 0xFF05, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x3 };
 
 		conditionBuilder.add_condition(ConditionProtocol::Udp());
-		conditionBuilder.add_condition(ConditionPort::Remote(67));
-		conditionBuilder.add_condition(ConditionPort::Local(68));
+		conditionBuilder.add_condition(ConditionIp::Local(fe80, uint8_t(10)));
+		conditionBuilder.add_condition(ConditionPort::Local(DHCPV6_CLIENT_PORT));
+		conditionBuilder.add_condition(ConditionIp::Remote(linkLocalDhcpMulticast));
+		conditionBuilder.add_condition(ConditionIp::Remote(siteLocalDhcpMulticast));
+		conditionBuilder.add_condition(ConditionPort::Remote(DHCPV6_SERVER_PORT));
 
 		if (!objectInstaller.addFilter(filterBuilder, conditionBuilder))
 		{
@@ -106,7 +120,7 @@ bool PermitDhcp::apply(IObjectInstaller &objectInstaller)
 	}
 
 	//
-	// #4 permit inbound DHCPv6 response
+	// #2 permit inbound DHCPv6 response
 	//
 
 	filterBuilder
@@ -117,10 +131,10 @@ bool PermitDhcp::apply(IObjectInstaller &objectInstaller)
 	wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_RECV_ACCEPT_V6);
 
 	conditionBuilder.add_condition(ConditionProtocol::Udp());
-	conditionBuilder.add_condition(ConditionIp::Remote(fe80, uint8_t(10)));
-	conditionBuilder.add_condition(ConditionPort::Remote(547));
 	conditionBuilder.add_condition(ConditionIp::Local(fe80, uint8_t(10)));
-	conditionBuilder.add_condition(ConditionPort::Local(546));
+	conditionBuilder.add_condition(ConditionPort::Local(DHCPV6_CLIENT_PORT));
+	conditionBuilder.add_condition(ConditionIp::Remote(fe80, uint8_t(10)));
+	conditionBuilder.add_condition(ConditionPort::Remote(DHCPV6_SERVER_PORT));
 
 	return objectInstaller.addFilter(filterBuilder, conditionBuilder);
 }
diff --git a/windows/winfw/src/winfw/rules/permitdhcp.h b/windows/winfw/src/winfw/rules/permitdhcp.h
index 58bd90bfa5..5500829c0c 100644
--- a/windows/winfw/src/winfw/rules/permitdhcp.h
+++ b/windows/winfw/src/winfw/rules/permitdhcp.h
@@ -13,6 +13,11 @@ public:
 	~PermitDhcp() = default;
 	
 	bool apply(IObjectInstaller &objectInstaller) override;
+
+private:
+
+	bool applyIpv4(IObjectInstaller &objectInstaller) const;
+	bool applyIpv6(IObjectInstaller &objectInstaller) const;
 };
 
 }