Add ICMP permit rule in firewall during connecting state

10 files changed, 257 insertions, 6 deletions

diff --git a/windows/winfw/src/winfw/fwcontext.cpp b/windows/winfw/src/winfw/fwcontext.cpp
index e5325f0b9c..49f8793572 100644
--- a/windows/winfw/src/winfw/fwcontext.cpp
+++ b/windows/winfw/src/winfw/fwcontext.cpp
@@ -13,10 +13,10 @@
 #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 "libwfp/ipaddress.h"
 #include <functional>
 #include <stdexcept>
 #include <utility>
@@ -99,7 +99,12 @@ FwContext::FwContext(uint32_t timeout, const WinFwSettings &settings)
 	m_baseline = checkpoint;
 }
 
-bool FwContext::applyPolicyConnecting(const WinFwSettings &settings, const WinFwRelay &relay)
+bool FwContext::applyPolicyConnecting
+(
+	const WinFwSettings &settings,
+	const WinFwRelay &relay,
+	const std::optional<PingableHosts> &pingableHosts
+)
 {
 	Ruleset ruleset;
 
@@ -112,6 +117,22 @@ bool FwContext::applyPolicyConnecting(const WinFwSettings &settings, const WinFw
 		TranslateProtocol(relay.protocol)
 	));
 
+	//
+	// Permit pinging the gateway inside the tunnel.
+	//
+	if (pingableHosts.has_value())
+	{
+		const auto &ph = pingableHosts.value();
+
+		for (const auto &host : ph.hosts)
+		{
+			ruleset.emplace_back(std::make_unique<rules::PermitPing>(
+				ph.tunnelInterfaceAlias,
+				host
+			));
+		}
+	}
+
 	return applyRuleset(ruleset);
 }
 
diff --git a/windows/winfw/src/winfw/fwcontext.h b/windows/winfw/src/winfw/fwcontext.h
index 89ef40e1d3..9d5b34c51b 100644
--- a/windows/winfw/src/winfw/fwcontext.h
+++ b/windows/winfw/src/winfw/fwcontext.h
@@ -3,9 +3,11 @@
 #include "winfw.h"
 #include "sessioncontroller.h"
 #include "rules/ifirewallrule.h"
+#include "libwfp/ipaddress.h"
 #include <cstdint>
 #include <memory>
 #include <vector>
+#include <optional>
 
 class FwContext
 {
@@ -16,7 +18,19 @@ public:
 	// This ctor applies the "blocked" policy.
 	FwContext(uint32_t timeout, const WinFwSettings &settings);
 
-	bool applyPolicyConnecting(const WinFwSettings &settings, const WinFwRelay &relay);
+	struct PingableHosts
+	{
+		std::optional<std::wstring> tunnelInterfaceAlias;
+		std::vector<wfp::IpAddress> hosts;
+	};
+
+	bool applyPolicyConnecting
+	(
+		const WinFwSettings &settings,
+		const WinFwRelay &relay,
+		const std::optional<PingableHosts> &pingableHosts
+	);
+
 	bool applyPolicyConnected
 	(
 		const WinFwSettings &settings,
diff --git a/windows/winfw/src/winfw/mullvadguids.cpp b/windows/winfw/src/winfw/mullvadguids.cpp
index 010d41e44a..e73fac26ed 100644
--- a/windows/winfw/src/winfw/mullvadguids.cpp
+++ b/windows/winfw/src/winfw/mullvadguids.cpp
@@ -59,6 +59,8 @@ DetailedWfpObjectRegistry MullvadGuids::BuildDetailedRegistry()
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitNdp_Outbound_Router_Solicitation()));
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitNdp_Inbound_Router_Advertisement()));
 	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitNdp_Inbound_Redirect()));
+	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitPing_Outbound_Icmpv4()));
+	registry.insert(std::make_pair(WfpObjectType::Filter, FilterPermitPing_Outbound_Icmpv6()));
 
 	return registry;
 }
@@ -567,3 +569,31 @@ const GUID &MullvadGuids::FilterPermitNdp_Inbound_Redirect()
 
 	return g;
 }
+
+//static
+const GUID &MullvadGuids::FilterPermitPing_Outbound_Icmpv4()
+{
+	static const GUID g =
+	{
+		0x2ecf7ff7,
+		0xc951,
+		0x4056,
+		{ 0xb0, 0xf7, 0x40, 0xa4, 0x5c, 0x7e, 0xb4, 0xc2 }
+	};
+
+	return g;
+}
+
+//static
+const GUID &MullvadGuids::FilterPermitPing_Outbound_Icmpv6()
+{
+	static const GUID g =
+	{
+		0x3deb8cab,
+		0x1edb,
+		0x4aa1,
+		{ 0xb2, 0x73, 0xec, 0x61, 0x4f, 0x50, 0xdc, 0x13 }
+	};
+
+	return g;
+}
diff --git a/windows/winfw/src/winfw/mullvadguids.h b/windows/winfw/src/winfw/mullvadguids.h
index d4fb470d90..3c3ca9702b 100644
--- a/windows/winfw/src/winfw/mullvadguids.h
+++ b/windows/winfw/src/winfw/mullvadguids.h
@@ -67,4 +67,7 @@ public:
 	static const GUID &FilterPermitNdp_Outbound_Router_Solicitation();
 	static const GUID &FilterPermitNdp_Inbound_Router_Advertisement();
 	static const GUID &FilterPermitNdp_Inbound_Redirect();
+
+	static const GUID &FilterPermitPing_Outbound_Icmpv4();
+	static const GUID &FilterPermitPing_Outbound_Icmpv6();
 };
diff --git a/windows/winfw/src/winfw/rules/permitping.cpp b/windows/winfw/src/winfw/rules/permitping.cpp
new file mode 100644
index 0000000000..f6aed36bf2
--- /dev/null
+++ b/windows/winfw/src/winfw/rules/permitping.cpp
@@ -0,0 +1,98 @@
+#include "stdafx.h"
+#include "permitping.h"
+#include "winfw/mullvadguids.h"
+#include "libwfp/filterbuilder.h"
+#include "libwfp/conditionbuilder.h"
+#include "libwfp/conditions/conditionip.h"
+#include "libwfp/conditions/conditioninterface.h"
+#include "libwfp/conditions/conditionprotocol.h"
+
+
+using namespace wfp::conditions;
+
+namespace rules
+{
+
+PermitPing::PermitPing
+(
+	const std::optional<std::wstring> &interfaceAlias,
+	const wfp::IpAddress &host
+)
+	: m_interfaceAlias(interfaceAlias)
+	, m_host(host)
+{
+}
+
+bool PermitPing::apply(IObjectInstaller &objectInstaller)
+{
+	if (wfp::IpAddress::Type::Ipv4 == m_host.type())
+	{
+		return applyIcmpv4(objectInstaller);
+	}
+
+	return applyIcmpv6(objectInstaller);
+}
+
+bool PermitPing::applyIcmpv4(IObjectInstaller &objectInstaller) const
+{
+	wfp::FilterBuilder filterBuilder;
+
+	//
+	// #1 Permit outbound ICMPv4 to %host% on %interface%
+	//
+
+	filterBuilder
+		.key(MullvadGuids::FilterPermitPing_Outbound_Icmpv4())
+		.name(L"Permit outbound ICMP to specific host (ICMPv4)")
+		.description(L"This filter is part of a rule that permits ping")
+		.provider(MullvadGuids::Provider())
+		.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(ConditionIp::Remote(m_host));
+	conditionBuilder.add_condition(ConditionProtocol::Icmp());
+
+	if (m_interfaceAlias.has_value())
+	{
+		conditionBuilder.add_condition(ConditionInterface::Alias(m_interfaceAlias.value()));
+	}
+
+	return objectInstaller.addFilter(filterBuilder, conditionBuilder);
+}
+
+bool PermitPing::applyIcmpv6(IObjectInstaller &objectInstaller) const
+{
+	wfp::FilterBuilder filterBuilder;
+
+	//
+	// #1 Permit outbound ICMPv6 to %host% on %interface%
+	//
+
+	filterBuilder
+		.key(MullvadGuids::FilterPermitPing_Outbound_Icmpv6())
+		.name(L"Permit outbound ICMP to specific host (ICMPv6)")
+		.description(L"This filter is part of a rule that permits ping")
+		.provider(MullvadGuids::Provider())
+		.layer(FWPM_LAYER_ALE_AUTH_CONNECT_V6)
+		.sublayer(MullvadGuids::SublayerWhitelist())
+		.weight(wfp::FilterBuilder::WeightClass::Max)
+		.permit();
+
+	wfp::ConditionBuilder conditionBuilder(FWPM_LAYER_ALE_AUTH_CONNECT_V6);
+
+	conditionBuilder.add_condition(ConditionIp::Remote(m_host));
+	conditionBuilder.add_condition(ConditionProtocol::IcmpV6());
+
+	if (m_interfaceAlias.has_value())
+	{
+		conditionBuilder.add_condition(ConditionInterface::Alias(m_interfaceAlias.value()));
+	}
+
+	return objectInstaller.addFilter(filterBuilder, conditionBuilder);
+}
+
+}
diff --git a/windows/winfw/src/winfw/rules/permitping.h b/windows/winfw/src/winfw/rules/permitping.h
new file mode 100644
index 0000000000..c8238ceaa8
--- /dev/null
+++ b/windows/winfw/src/winfw/rules/permitping.h
@@ -0,0 +1,28 @@
+#pragma once
+
+#include "ifirewallrule.h"
+#include <libwfp/ipaddress.h>
+#include <string>
+#include <optional>
+
+namespace rules
+{
+
+class PermitPing : public IFirewallRule
+{
+public:
+
+	PermitPing(const std::optional<std::wstring> &interfaceAlias, const wfp::IpAddress &host);
+
+	bool apply(IObjectInstaller &objectInstaller) override;
+
+private:
+
+	const std::optional<std::wstring> m_interfaceAlias;
+	const wfp::IpAddress m_host;
+
+	bool applyIcmpv4(IObjectInstaller &objectInstaller) const;
+	bool applyIcmpv6(IObjectInstaller &objectInstaller) const;
+};
+
+}
diff --git a/windows/winfw/src/winfw/winfw.cpp b/windows/winfw/src/winfw/winfw.cpp
index 7b9ea2dc6b..3065408f3d 100644
--- a/windows/winfw/src/winfw/winfw.cpp
+++ b/windows/winfw/src/winfw/winfw.cpp
@@ -4,6 +4,7 @@
 #include "objectpurger.h"
 #include <windows.h>
 #include <stdexcept>
+#include <optional>
 
 namespace
 {
@@ -15,6 +16,34 @@ void * g_errorContext = nullptr;
 
 FwContext *g_fwContext = nullptr;
 
+std::optional<FwContext::PingableHosts> ConvertPingableHosts(const PingableHosts *pingableHosts)
+{
+	if (nullptr == pingableHosts)
+	{
+		return {};
+	}
+
+	if (nullptr == pingableHosts->hosts
+		|| 0 == pingableHosts->numHosts)
+	{
+		throw std::runtime_error("Invalid PingableHosts structure");
+	}
+
+	FwContext::PingableHosts converted;
+
+	if (nullptr != pingableHosts->tunnelInterfaceAlias)
+	{
+		converted.tunnelInterfaceAlias = pingableHosts->tunnelInterfaceAlias;
+	}
+
+	for (size_t i = 0; i < pingableHosts->numHosts; ++i)
+	{
+		converted.hosts.emplace_back(wfp::IpAddress(pingableHosts->hosts[i]));
+	}
+
+	return converted;
+}
+
 } // anonymous namespace
 
 WINFW_LINKAGE
@@ -130,7 +159,8 @@ bool
 WINFW_API
 WinFw_ApplyPolicyConnecting(
 	const WinFwSettings &settings,
-	const WinFwRelay &relay
+	const WinFwRelay &relay,
+	const PingableHosts *pingableHosts
 )
 {
 	if (nullptr == g_fwContext)
@@ -140,7 +170,7 @@ WinFw_ApplyPolicyConnecting(
 
 	try
 	{
-		return g_fwContext->applyPolicyConnecting(settings, relay);
+		return g_fwContext->applyPolicyConnecting(settings, relay, ConvertPingableHosts(pingableHosts));
 	}
 	catch (std::exception &err)
 	{
diff --git a/windows/winfw/src/winfw/winfw.h b/windows/winfw/src/winfw/winfw.h
index 95e66a608f..6d43b0db4c 100644
--- a/windows/winfw/src/winfw/winfw.h
+++ b/windows/winfw/src/winfw/winfw.h
@@ -105,11 +105,29 @@ WINFW_API
 WinFw_Deinitialize();
 
 //
+// PingableHosts:
+//
+// Specifies a set of IP addresses that should be reachable by ICMP when the connecting
+// policy is effective.
+//
+// The interface alias is optional and can be used to restrict the traffic such
+// that it is only allowed on that specific interface.
+//
+typedef struct tag_PingableHosts
+{
+	const wchar_t *tunnelInterfaceAlias;
+	const wchar_t **hosts;
+	size_t numHosts;
+}
+PingableHosts;
+
+//
 // ApplyPolicyConnecting:
 //
 // Apply restrictions in the firewall that block all traffic, except:
 // - What is specified by settings
 // - Communication with the relay server
+// - ICMP (for ping) to/from tunnel gateway
 //
 extern "C"
 WINFW_LINKAGE
@@ -117,7 +135,8 @@ bool
 WINFW_API
 WinFw_ApplyPolicyConnecting(
 	const WinFwSettings &settings,
-	const WinFwRelay &relay
+	const WinFwRelay &relay,
+	const PingableHosts *pingableHosts
 );
 
 //
diff --git a/windows/winfw/src/winfw/winfw.vcxproj b/windows/winfw/src/winfw/winfw.vcxproj
index 4777503f72..cbabe2f4f7 100644
--- a/windows/winfw/src/winfw/winfw.vcxproj
+++ b/windows/winfw/src/winfw/winfw.vcxproj
@@ -30,6 +30,7 @@
     <ClCompile Include="rules\permitlanservice.cpp" />
     <ClCompile Include="rules\permitloopback.cpp" />
     <ClCompile Include="rules\permitndp.cpp" />
+    <ClCompile Include="rules\permitping.cpp" />
     <ClCompile Include="rules\permitvpntunnelservice.cpp" />
     <ClCompile Include="rules\permitvpnrelay.cpp" />
     <ClCompile Include="rules\permitvpntunnel.cpp" />
@@ -53,6 +54,7 @@
     <ClInclude Include="objectpurger.h" />
     <ClInclude Include="rules\permitdhcpserver.h" />
     <ClInclude Include="rules\permitndp.h" />
+    <ClInclude Include="rules\permitping.h" />
     <ClInclude Include="wfpobjecttype.h" />
     <ClInclude Include="rules\blockall.h" />
     <ClInclude Include="rules\ifirewallrule.h" />
diff --git a/windows/winfw/src/winfw/winfw.vcxproj.filters b/windows/winfw/src/winfw/winfw.vcxproj.filters
index 0319b0214a..a758a1c9ec 100644
--- a/windows/winfw/src/winfw/winfw.vcxproj.filters
+++ b/windows/winfw/src/winfw/winfw.vcxproj.filters
@@ -43,6 +43,9 @@
     <ClCompile Include="rules\permitndp.cpp">
       <Filter>rules</Filter>
     </ClCompile>
+    <ClCompile Include="rules\permitping.cpp">
+      <Filter>rules</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="stdafx.h" />
@@ -93,6 +96,9 @@
     <ClInclude Include="rules\permitndp.h">
       <Filter>rules</Filter>
     </ClInclude>
+    <ClInclude Include="rules\permitping.h">
+      <Filter>rules</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <Filter Include="rules">