Merge PermitEndpoint and PermitVpnRelay and remove hardcoded UUIDs

9 files changed, 26 insertions, 181 deletions

diff --git a/windows/winfw/src/winfw/fwcontext.cpp b/windows/winfw/src/winfw/fwcontext.cpp
index 7747d7c822..1393949223 100644
--- a/windows/winfw/src/winfw/fwcontext.cpp
+++ b/windows/winfw/src/winfw/fwcontext.cpp
@@ -14,12 +14,11 @@
 #include "rules/baseline/permitvpntunnel.h"
 #include "rules/baseline/permitvpntunnelservice.h"
 #include "rules/baseline/permitdns.h"
-#include "rules/baseline/permitendpoint.h"
 #include "rules/dns/blockall.h"
 #include "rules/dns/permitloopback.h"
 #include "rules/dns/permittunnel.h"
 #include "rules/dns/permitnontunnel.h"
-#include "rules/multi/permitvpnrelay.h"
+#include "rules/multi/permitendpoint.h"
 #include <libwfp/transaction.h>
 #include <libwfp/filterengine.h>
 #include <libcommon/error.h>
@@ -40,11 +39,11 @@ namespace
 // it in the DNS sublayer instead. The PermitDNS rule in the baseline sublayer accomplishes this.
 //
 // This has implications for the way the relay access is configured. In the regular case there
-// is no issue: The PermitVpnRelay rule can be installed in the baseline sublayer.
+// is no issue: The PermitEndpoint rule can be installed in the baseline sublayer.
 //
 // However, if the relay is running on the DNS port (53), it would be blocked unless the DNS
 // sublayer permits this traffic. For this reason, whenever the relay is on port 53, the
-// PermitVpnRelay rule has to be installed to the DNS sublayer instead of the baseline sublayer.
+// PermitEndpoint rule has to be installed to the DNS sublayer instead of the baseline sublayer.
 //
 void AppendSettingsRules
 (
@@ -87,11 +86,11 @@ void AppendRelayRules
 	auto sublayer =
 	(
 		DNS_SERVER_PORT == relay.port
-		? rules::multi::PermitVpnRelay::Sublayer::Dns
-		: rules::multi::PermitVpnRelay::Sublayer::Baseline
+		? rules::multi::PermitEndpoint::Sublayer::Dns
+		: rules::multi::PermitEndpoint::Sublayer::Baseline
 	);
 
-	ruleset.emplace_back(std::make_unique<multi::PermitVpnRelay>(
+	ruleset.emplace_back(std::make_unique<multi::PermitEndpoint>(
 		wfp::IpAddress(relay.ip),
 		relay.port,
 		relay.protocol,
@@ -115,11 +114,13 @@ void AppendAllowedEndpointRules
 		clients.push_back(endpoint.clients[i]);
 	}
 
-	ruleset.emplace_back(std::make_unique<baseline::PermitEndpoint>(
+	ruleset.emplace_back(std::make_unique<multi::PermitEndpoint>(
 		wfp::IpAddress(endpoint.endpoint.ip),
-		clients,
 		endpoint.endpoint.port,
-		endpoint.endpoint.protocol
+		endpoint.endpoint.protocol,
+		clients,
+		// TODO: DNS sublayer if port 53
+		multi::PermitEndpoint::Sublayer::Baseline
 	));
 }
 
diff --git a/windows/winfw/src/winfw/mullvadguids.cpp b/windows/winfw/src/winfw/mullvadguids.cpp
index 44fc5866f2..e072b59eab 100644
--- a/windows/winfw/src/winfw/mullvadguids.cpp
+++ b/windows/winfw/src/winfw/mullvadguids.cpp
@@ -465,34 +465,6 @@ const GUID &MullvadGuids::Filter_Baseline_PermitDhcpServer_Outbound_Response_Ipv
 }
 
 //static
-const GUID &MullvadGuids::Filter_Baseline_PermitVpnRelay()
-{
-	static const GUID g =
-	{
-		0x160c205d,
-		0xdb40,
-		0x4f79,
-		{ 0x90, 0x6d, 0xfd, 0xa1, 0xe1, 0xc1, 0x8a, 0x70 }
-	};
-
-	return g;
-}
-
-//static
-const GUID &MullvadGuids::Filter_Baseline_PermitEndpoint()
-{
-	static const GUID g =
-	{
-		0x99dc8dac,
-		0x8520,
-		0x41be,
-		{ 0xbf, 0xab, 0x0c, 0x9, 0xbf, 0x12, 0xeb, 0 }
-	};
-
-	return g;
-}
-
-//static
 const GUID &MullvadGuids::Filter_Baseline_PermitVpnTunnel_Outbound_Ipv4_1()
 {
 	static const GUID g =
diff --git a/windows/winfw/src/winfw/mullvadguids.h b/windows/winfw/src/winfw/mullvadguids.h
index 9b94760191..a086155a77 100644
--- a/windows/winfw/src/winfw/mullvadguids.h
+++ b/windows/winfw/src/winfw/mullvadguids.h
@@ -46,10 +46,6 @@ public:
 	static const GUID &Filter_Baseline_PermitDhcpServer_Inbound_Request_Ipv4();
 	static const GUID &Filter_Baseline_PermitDhcpServer_Outbound_Response_Ipv4();
 
-	static const GUID &Filter_Baseline_PermitVpnRelay();
-
-	static const GUID &Filter_Baseline_PermitEndpoint();
-
 	static const GUID &Filter_Baseline_PermitVpnTunnel_Outbound_Ipv4_1();
 	static const GUID &Filter_Baseline_PermitVpnTunnel_Outbound_Ipv6_1();
 	static const GUID &Filter_Baseline_PermitVpnTunnel_Outbound_Ipv4_2();
diff --git a/windows/winfw/src/winfw/rules/baseline/permitendpoint.cpp b/windows/winfw/src/winfw/rules/baseline/permitendpoint.cpp
deleted file mode 100644
index c1c74ba6ba..0000000000
--- a/windows/winfw/src/winfw/rules/baseline/permitendpoint.cpp
+++ /dev/null
@@ -1,81 +0,0 @@
-#include "stdafx.h"
-#include "permitendpoint.h"
-#include <winfw/mullvadguids.h>
-#include <winfw/rules/shared.h>
-#include <libwfp/filterbuilder.h>
-#include <libwfp/conditionbuilder.h>
-#include <libwfp/conditions/conditionprotocol.h>
-#include <libwfp/conditions/conditionip.h>
-#include <libwfp/conditions/conditionport.h>
-#include <libwfp/conditions/conditionapplication.h>
-#include <libcommon/error.h>
-
-using namespace wfp::conditions;
-
-namespace rules::baseline
-{
-
-namespace
-{
-
-const GUID &OutboundLayerFromIp(const wfp::IpAddress &ip)
-{
-	switch (ip.type())
-	{
-		case wfp::IpAddress::Type::Ipv4: return FWPM_LAYER_ALE_AUTH_CONNECT_V4;
-		case wfp::IpAddress::Type::Ipv6: return FWPM_LAYER_ALE_AUTH_CONNECT_V6;
-		default:
-		{
-			THROW_ERROR("Missing case handler in switch clause");
-		}
-	};
-}
-
-} // anonymous namespace
-
-PermitEndpoint::PermitEndpoint
-(
-	const wfp::IpAddress &address,
-	const std::vector<std::wstring> &clients,
-	uint16_t port,
-	WinFwProtocol protocol
-)
-	: m_address(address)
-	, m_clients(clients)
-	, m_port(port)
-	, m_protocol(protocol)
-{
-}
-
-bool PermitEndpoint::apply(IObjectInstaller &objectInstaller)
-{
-	wfp::FilterBuilder filterBuilder;
-
-	//
-	// Permit outbound connections to endpoint.
-	//
-
-	filterBuilder
-		.key(MullvadGuids::Filter_Baseline_PermitEndpoint())
-		.name(L"Permit outbound connections to a given endpoint")
-		.description(L"This filter is part of a rule that permits traffic to a specific endpoint")
-		.provider(MullvadGuids::Provider())
-		.layer(OutboundLayerFromIp(m_address))
-		.sublayer(MullvadGuids::SublayerBaseline())
-		.weight(wfp::FilterBuilder::WeightClass::Max)
-		.permit();
-
-	wfp::ConditionBuilder conditionBuilder(OutboundLayerFromIp(m_address));
-
-	conditionBuilder.add_condition(ConditionIp::Remote(m_address));
-	conditionBuilder.add_condition(ConditionPort::Remote(m_port));
-	conditionBuilder.add_condition(CreateProtocolCondition(m_protocol));
-
-	for (const auto client : m_clients) {
-		conditionBuilder.add_condition(std::make_unique<ConditionApplication>(client));
-	}
-
-	return objectInstaller.addFilter(filterBuilder, conditionBuilder);
-}
-
-}
diff --git a/windows/winfw/src/winfw/rules/baseline/permitendpoint.h b/windows/winfw/src/winfw/rules/baseline/permitendpoint.h
deleted file mode 100644
index 9e5e2fc923..0000000000
--- a/windows/winfw/src/winfw/rules/baseline/permitendpoint.h
+++ /dev/null
@@ -1,34 +0,0 @@
-#pragma once
-
-#include <winfw/rules/ifirewallrule.h>
-#include <winfw/winfw.h>
-#include <libwfp/ipaddress.h>
-#include <vector>
-#include <string>
-
-namespace rules::baseline
-{
-
-class PermitEndpoint : public IFirewallRule
-{
-public:
-
-	PermitEndpoint
-	(
-		const wfp::IpAddress &address,
-		const std::vector<std::wstring> &clients,
-		uint16_t port,
-		WinFwProtocol protocol
-	);
-	
-	bool apply(IObjectInstaller &objectInstaller) override;
-
-private:
-
-	const wfp::IpAddress m_address;
-	const std::vector<std::wstring> m_clients;
-	const uint16_t m_port;
-	const WinFwProtocol m_protocol;
-};
-
-}
diff --git a/windows/winfw/src/winfw/rules/multi/permitvpnrelay.cpp b/windows/winfw/src/winfw/rules/multi/permitendpoint.cpp
index 19ce09571b..224f7ecfc5 100644
--- a/windows/winfw/src/winfw/rules/multi/permitvpnrelay.cpp
+++ b/windows/winfw/src/winfw/rules/multi/permitendpoint.cpp
@@ -1,5 +1,5 @@
 #include "stdafx.h"
-#include "permitvpnrelay.h"
+#include "permitendpoint.h"
 #include <winfw/mullvadguids.h>
 #include <winfw/winfw.h>
 #include <winfw/rules/shared.h>
@@ -32,12 +32,12 @@ const GUID &LayerFromIp(const wfp::IpAddress &ip)
 	};
 }
 
-const GUID &TranslateSublayer(PermitVpnRelay::Sublayer sublayer)
+const GUID &TranslateSublayer(PermitEndpoint::Sublayer sublayer)
 {
 	switch (sublayer)
 	{
-		case PermitVpnRelay::Sublayer::Baseline: return MullvadGuids::SublayerBaseline();
-		case PermitVpnRelay::Sublayer::Dns: return MullvadGuids::SublayerDns();
+		case PermitEndpoint::Sublayer::Baseline: return MullvadGuids::SublayerBaseline();
+		case PermitEndpoint::Sublayer::Dns: return MullvadGuids::SublayerDns();
 		default:
 		{
 			THROW_ERROR("Missing case handler in switch clause");
@@ -47,7 +47,7 @@ const GUID &TranslateSublayer(PermitVpnRelay::Sublayer sublayer)
 
 } // anonymous namespace
 
-PermitVpnRelay::PermitVpnRelay
+PermitEndpoint::PermitEndpoint
 (
 	const wfp::IpAddress &relay,
 	uint16_t relayPort,
@@ -63,18 +63,17 @@ PermitVpnRelay::PermitVpnRelay
 {
 }
 
-bool PermitVpnRelay::apply(IObjectInstaller &objectInstaller)
+bool PermitEndpoint::apply(IObjectInstaller &objectInstaller)
 {
-	wfp::FilterBuilder filterBuilder;
+	wfp::FilterBuilder filterBuilder(wfp::BuilderValidation::OnlyCritical);
 
 	//
 	// #1 Permit outbound connections to relay.
 	//
 
 	filterBuilder
-		.key(MullvadGuids::Filter_Baseline_PermitVpnRelay())
-		.name(L"Permit outbound connections to VPN relay")
-		.description(L"This filter is part of a rule that permits communication with a VPN relay")
+		.name(L"Permit outbound connections to an endpoint")
+		.description(L"This filter is part of a rule that permits communication with an endpoint")
 		.provider(MullvadGuids::Provider())
 		.layer(LayerFromIp(m_relay))
 		.sublayer(TranslateSublayer(m_sublayer))
diff --git a/windows/winfw/src/winfw/rules/multi/permitvpnrelay.h b/windows/winfw/src/winfw/rules/multi/permitendpoint.h
index a2bfc16384..025c3f781c 100644
--- a/windows/winfw/src/winfw/rules/multi/permitvpnrelay.h
+++ b/windows/winfw/src/winfw/rules/multi/permitendpoint.h
@@ -8,7 +8,7 @@
 namespace rules::multi
 {
 
-class PermitVpnRelay : public IFirewallRule
+class PermitEndpoint : public IFirewallRule
 {
 public:
 
@@ -18,7 +18,7 @@ public:
 		Dns
 	};
 
-	PermitVpnRelay
+	PermitEndpoint
 	(
 		const wfp::IpAddress &relay,
 		uint16_t relayPort,
diff --git a/windows/winfw/src/winfw/winfw.vcxproj b/windows/winfw/src/winfw/winfw.vcxproj
index eb14e0332d..c5031efb49 100644
--- a/windows/winfw/src/winfw/winfw.vcxproj
+++ b/windows/winfw/src/winfw/winfw.vcxproj
@@ -35,7 +35,6 @@
     <ClCompile Include="rules\baseline\permitdhcp.cpp" />
     <ClCompile Include="rules\baseline\permitdhcpserver.cpp" />
     <ClCompile Include="rules\baseline\permitdns.cpp" />
-    <ClCompile Include="rules\baseline\permitendpoint.cpp" />
     <ClCompile Include="rules\baseline\permitlan.cpp" />
     <ClCompile Include="rules\baseline\permitlanservice.cpp" />
     <ClCompile Include="rules\baseline\permitloopback.cpp" />
@@ -46,7 +45,7 @@
     <ClCompile Include="rules\dns\permitloopback.cpp" />
     <ClCompile Include="rules\dns\permitnontunnel.cpp" />
     <ClCompile Include="rules\dns\permittunnel.cpp" />
-    <ClCompile Include="rules\multi\permitvpnrelay.cpp" />
+    <ClCompile Include="rules\multi\permitendpoint.cpp" />
     <ClCompile Include="rules\persistent\blockall.cpp" />
     <ClCompile Include="rules\shared.cpp" />
     <ClCompile Include="sessioncontroller.cpp" />
@@ -72,7 +71,6 @@
     <ClInclude Include="rules\baseline\permitdhcp.h" />
     <ClInclude Include="rules\baseline\permitdhcpserver.h" />
     <ClInclude Include="rules\baseline\permitdns.h" />
-    <ClInclude Include="rules\baseline\permitendpoint.h" />
     <ClInclude Include="rules\baseline\permitlan.h" />
     <ClInclude Include="rules\baseline\permitlanservice.h" />
     <ClInclude Include="rules\baseline\permitloopback.h" />
@@ -83,7 +81,7 @@
     <ClInclude Include="rules\dns\permitloopback.h" />
     <ClInclude Include="rules\dns\permitnontunnel.h" />
     <ClInclude Include="rules\dns\permittunnel.h" />
-    <ClInclude Include="rules\multi\permitvpnrelay.h" />
+    <ClInclude Include="rules\multi\permitendpoint.h" />
     <ClInclude Include="rules\persistent\blockall.h" />
     <ClInclude Include="rules\ports.h" />
     <ClInclude Include="rules\shared.h" />
diff --git a/windows/winfw/src/winfw/winfw.vcxproj.filters b/windows/winfw/src/winfw/winfw.vcxproj.filters
index daecbb03fb..89805fb4c8 100644
--- a/windows/winfw/src/winfw/winfw.vcxproj.filters
+++ b/windows/winfw/src/winfw/winfw.vcxproj.filters
@@ -55,10 +55,7 @@
     <ClCompile Include="rules\persistent\blockall.cpp">
       <Filter>rules\persistent</Filter>
     </ClCompile>
-    <ClCompile Include="rules\baseline\permitendpoint.cpp">
-      <Filter>rules\baseline</Filter>
-    </ClCompile>
-    <ClCompile Include="rules\multi\permitvpnrelay.cpp">
+    <ClCompile Include="rules\multi\permitendpoint.cpp">
       <Filter>rules\multi</Filter>
     </ClCompile>
     <ClCompile Include="rules\dns\permitloopback.cpp">
@@ -129,10 +126,7 @@
     <ClInclude Include="rules\persistent\blockall.h">
       <Filter>rules\persistent</Filter>
     </ClInclude>
-    <ClInclude Include="rules\baseline\permitendpoint.h">
-      <Filter>rules\baseline</Filter>
-    </ClInclude>
-    <ClInclude Include="rules\multi\permitvpnrelay.h">
+    <ClInclude Include="rules\multi\permitendpoint.h">
       <Filter>rules\multi</Filter>
     </ClInclude>
     <ClInclude Include="rules\dns\permitloopback.h">