Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1541?usp=email
to review the following change.
Change subject: wfp: add flag to allow DNS traffic to loopback
......................................................................
wfp: add flag to allow DNS traffic to loopback
Add a flag 'allow-loopback' to the --block-outside-dns options. When it
is set DNS traffic to loopback (localhost) will not be blocked. This
flag will also allow DNS to localhost when all local traffic is to be
blocked via '--redirect-gateway block-local'.
Fixes #703
Change-Id: I0f7237720b0892f5ba0caf60f941f9cc2241cdb6
Signed-off-by: Heiko Hund <[email protected]>
---
M doc/man-sections/windows-options.rst
M include/openvpn-msg.h
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/wfp_block.c
M src/openvpn/wfp_block.h
M src/openvpn/win32.c
M src/openvpn/win32.h
M src/openvpnserv/interactive.c
10 files changed, 118 insertions(+), 59 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/41/1541/1
diff --git a/doc/man-sections/windows-options.rst
b/doc/man-sections/windows-options.rst
index af2fb45..b31ff2e 100644
--- a/doc/man-sections/windows-options.rst
+++ b/doc/man-sections/windows-options.rst
@@ -10,11 +10,12 @@
after a reboot, or if the driver is unloaded and reloaded. This
directive can only be used by an administrator.
---block-outside-dns
+--block-outside-dns [allow-loopback]
Block DNS servers on other network adapters to prevent DNS leaks. This
option prevents any application from accessing TCP or UDP port 53 except
one inside the tunnel. It uses Windows Filtering Platform (WFP) and
- works on Windows Vista or later.
+ works on Windows Vista or later. If ``allow-loopback`` is set, traffic to
+ localhost port 53 is still allowed.
This option is considered unknown on non-Windows platforms and
unsupported on Windows XP, resulting in fatal error. You may want to use
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index a0cec53..58d4ea9 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -72,9 +72,13 @@
typedef enum
{
- wfp_block_local = 1 << 0,
- wfp_block_dns = 1 << 1
-} wfp_block_flags_t;
+ wfp_block_all,
+ wfp_block_all_except_local_dns,
+ wfp_block_dns,
+ wfp_block_dns_except_local,
+ wfp_block_type_count,
+ wfp_block_none = -1
+} wfp_block_t;
typedef struct
{
@@ -162,7 +166,7 @@
typedef struct
{
message_header_t header;
- wfp_block_flags_t flags;
+ wfp_block_t block_type;
interface_t iface;
} wfp_block_message_t;
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 1391aa85..19ddece 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1771,12 +1771,14 @@
{
#if defined(_WIN32)
/* Fortify 'redirect-gateway block-local' with firewall rules? */
- bool block_local = block_local_needed(c->c1.route_list);
+ const bool block_local = block_local_needed(c->c1.route_list);
+ const bool block_outside_dns = c->options.block_outside_dns;
+ const bool allow_loopback_dns = c->options.allow_loopback_dns;
+ const wfp_block_t block_type = get_wfp_block_type(block_local,
block_outside_dns, allow_loopback_dns);
- if (c->options.block_outside_dns || block_local)
+ if (block_type != wfp_block_none)
{
- BOOL dns_only = !block_local;
- if (!win_wfp_block(c->c1.tuntap->adapter_index,
c->options.msg_channel, dns_only))
+ if (!win_wfp_block(c->c1.tuntap->adapter_index,
c->options.msg_channel, block_type))
{
msg(M_FATAL, "WFP: initialization failed");
}
@@ -1796,7 +1798,11 @@
del_wfp_block(struct context *c, unsigned long adapter_index)
{
#if defined(_WIN32)
- if (c->options.block_outside_dns || block_local_needed(c->c1.route_list))
+ const bool block_local = block_local_needed(c->c1.route_list);
+ const bool block_outside_dns = c->options.block_outside_dns;
+ const wfp_block_t block_type = get_wfp_block_type(block_local,
block_outside_dns, false);
+
+ if (block_type != wfp_block_none)
{
if (!win_wfp_uninit(adapter_index, c->options.msg_channel))
{
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index cdb02e9..4c8a0bd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -848,6 +848,7 @@
o->tuntap_options.dhcp_masq_offset = 0;
o->route_method = ROUTE_METHOD_ADAPTIVE;
o->block_outside_dns = false;
+ o->allow_loopback_dns = false;
o->windows_driver = WINDOWS_DRIVER_UNSPECIFIED;
#endif
o->vlan_accept = VLAN_ALL;
@@ -1955,6 +1956,7 @@
SHOW_BOOL(show_net_up);
SHOW_INT(route_method);
SHOW_BOOL(block_outside_dns);
+ SHOW_BOOL(allow_loopback_dns);
show_tuntap_options(&o->tuntap_options);
#endif
#endif /* ifndef ENABLE_SMALL */
@@ -5172,6 +5174,7 @@
{
VERIFY_PERMISSION(OPT_P_DHCPDNS);
options->block_outside_dns = false;
+ options->allow_loopback_dns = false;
}
#else /* ifdef _WIN32 */
else if (streq(p[0], "dhcp-option") && !p[1])
@@ -8067,10 +8070,14 @@
VERIFY_PERMISSION(OPT_P_DHCPDNS);
options->tuntap_options.register_dns = true;
}
- else if (streq(p[0], "block-outside-dns") && !p[1])
+ else if (streq(p[0], "block-outside-dns") && !p[2])
{
VERIFY_PERMISSION(OPT_P_DHCPDNS);
options->block_outside_dns = true;
+ if (p[1] && streq(p[1], "allow-loopback"))
+ {
+ options->allow_loopback_dns = true;
+ }
}
else if (streq(p[0], "rdns-internal") && !p[1])
/* standalone method for internal use
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index cf9936b..f69a053 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -692,6 +692,7 @@
bool show_net_up;
int route_method;
bool block_outside_dns;
+ bool allow_loopback_dns;
enum tun_driver_type windows_driver;
#endif
diff --git a/src/openvpn/wfp_block.c b/src/openvpn/wfp_block.c
index 212a4b2..6640178 100644
--- a/src/openvpn/wfp_block.c
+++ b/src/openvpn/wfp_block.c
@@ -131,27 +131,10 @@
return err;
}
-/*
- * Block outgoing local traffic, possibly DNS only, except for
- * (i) adapter with the specified index (and loopback, if all is blocked)
- * OR
- * (ii) processes with the specified executable path
- * The firewall filters added here are automatically removed when the process
exits or
- * on calling delete_wfp_block_filters().
- * Arguments:
- * engine_handle : On successful return contains the handle for a newly
opened fwp session
- * in which the filters are added.
- * May be closed by passing to delete_wfp_block_filters to
remove the filters.
- * index : The index of adapter for which traffic is permitted.
- * exe_path : Path of executable for which traffic is permitted.
- * msg_handler : An optional callback function for error reporting.
- * dns_only : Whether the blocking filters should apply for DNS only.
- * Returns 0 on success, a non-zero status code of the last failed action on
failure.
- */
DWORD
add_wfp_block_filters(HANDLE *engine_handle, int index, const WCHAR *exe_path,
- wfp_block_msg_handler_t msg_handler, BOOL dns_only)
+ wfp_block_msg_handler_t msg_handler, const wfp_block_t
block_type)
{
FWPM_SESSION0 session = { 0 };
FWPM_SUBLAYER0 *sublayer_ptr = NULL;
@@ -247,7 +230,7 @@
Filter.layerKey = FWPM_LAYER_ALE_AUTH_CONNECT_V4;
Filter.action.type = FWP_ACTION_PERMIT;
Condition[0] = match_openvpn;
- if (dns_only)
+ if (block_type == wfp_block_dns || block_type ==
wfp_block_dns_except_local)
{
Filter.numFilterConditions = 2;
Condition[1] = match_port_53;
@@ -268,7 +251,7 @@
Filter.weight.type = FWP_EMPTY;
Filter.numFilterConditions = 1;
Condition[0] = match_not_loopback;
- if (dns_only)
+ if (block_type == wfp_block_dns || block_type ==
wfp_block_dns_except_local)
{
Filter.numFilterConditions = 2;
Condition[1] = match_port_53;
@@ -292,7 +275,7 @@
Filter.action.type = FWP_ACTION_PERMIT;
Filter.numFilterConditions = 1;
Condition[0] = match_interface;
- if (dns_only)
+ if (block_type == wfp_block_dns || block_type ==
wfp_block_dns_except_local)
{
Filter.numFilterConditions = 2;
Condition[1] = match_port_53;
@@ -308,20 +291,23 @@
msg_handler(0, "WFP Block: Added permit filters for VPN interface");
- /* Seventh Filter. Block IPv4 DNS requests to loopback from other apps */
- Filter.layerKey = FWPM_LAYER_ALE_AUTH_CONNECT_V4;
- Filter.action.type = FWP_ACTION_BLOCK;
- Filter.weight.type = FWP_EMPTY;
- Filter.numFilterConditions = 2;
- Condition[0] = match_loopback;
- Condition[1] = match_port_53;
- err = FwpmFilterAdd0(*engine_handle, &Filter, NULL, &filterid);
- OUT_ON_ERROR(err, "Add filter to block IPv4 DNS traffic to loopback
failed");
+ if (block_type != wfp_block_all_except_local_dns && block_type !=
wfp_block_dns_except_local)
+ {
+ /* Seventh Filter. Block IPv4 DNS requests to loopback from other apps
*/
+ Filter.layerKey = FWPM_LAYER_ALE_AUTH_CONNECT_V4;
+ Filter.action.type = FWP_ACTION_BLOCK;
+ Filter.weight.type = FWP_EMPTY;
+ Filter.numFilterConditions = 2;
+ Condition[0] = match_loopback;
+ Condition[1] = match_port_53;
+ err = FwpmFilterAdd0(*engine_handle, &Filter, NULL, &filterid);
+ OUT_ON_ERROR(err, "Add filter to block IPv4 DNS traffic to loopback
failed");
- /* Eighth Filter. Block IPv6 DNS requests to loopback from other apps */
- Filter.layerKey = FWPM_LAYER_ALE_AUTH_CONNECT_V6;
- err = FwpmFilterAdd0(*engine_handle, &Filter, NULL, &filterid);
- OUT_ON_ERROR(err, "Add filter to block IPv6 DNS traffic to loopback
failed");
+ /* Eighth Filter. Block IPv6 DNS requests to loopback from other apps
*/
+ Filter.layerKey = FWPM_LAYER_ALE_AUTH_CONNECT_V6;
+ err = FwpmFilterAdd0(*engine_handle, &Filter, NULL, &filterid);
+ OUT_ON_ERROR(err, "Add filter to block IPv6 DNS traffic to loopback
failed");
+ }
msg_handler(0, "WFP Block: Added block filters for DNS traffic to
loopback");
diff --git a/src/openvpn/wfp_block.h b/src/openvpn/wfp_block.h
index 8bc093a..2f28751 100644
--- a/src/openvpn/wfp_block.h
+++ b/src/openvpn/wfp_block.h
@@ -28,18 +28,46 @@
#include <windef.h>
#include <iphlpapi.h>
#include <ws2tcpip.h>
+#include <stdbool.h>
+
+#include "openvpn-msg.h"
/* Any value less than 5 should work fine. 3 is chosen without any real
reason. */
#define WFP_BLOCK_IFACE_METRIC 3
typedef void (*wfp_block_msg_handler_t)(DWORD err, const char *msg);
-DWORD
-delete_wfp_block_filters(HANDLE engine);
-
+/**
+ * Block outgoing local traffic, possibly DNS only, except for
+ * (i) adapter with the specified index (and loopback, if all is blocked)
+ * OR
+ * (ii) processes with the specified executable path
+ * The firewall filters added here are automatically removed when the process
exits or
+ * on calling \ref delete_wfp_block_filters().
+ *
+ * @param engine_handle On successful return contains the handle for a newly
opened fwp
+ * session in which the filters are added. May be closed
by passing
+ * to \ref delete_wfp_block_filters() to remove the
filters.
+ * @param index The index of adapter for which traffic is permitted.
+ * @param exe_path Path of executable for which traffic is permitted.
+ * @param msg_handler An optional callback function for error reporting.
+ * @param block_type Whether the blocking filters should apply for DNS only.
+ *
+ * @return ERROR_SUCCESS, or a non-zero status code of the last failed action
on failure.
+ */
DWORD
add_wfp_block_filters(HANDLE *engine, int iface_index, const WCHAR *exe_path,
- wfp_block_msg_handler_t msg_handler_callback, BOOL
dns_only);
+ wfp_block_msg_handler_t msg_handler_callback, const
wfp_block_t block_type);
+
+/**
+ * Remove the firewall filter rules set with \ref add_wfp_block_filters()
+ *
+ * @param engine The WFP session handle in which the firewall rules
were added.
+ *
+ * @return ERROR_SUCCESS, or a non-zero status code of the last failed action
on failure.
+ */
+DWORD
+delete_wfp_block_filters(HANDLE engine);
/**
* Return interface metric value for the specified interface index.
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index b938d7b..54dab85 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1167,7 +1167,7 @@
}
static bool
-win_wfp_block_service(bool add, bool dns_only, int index, const HANDLE pipe)
+win_wfp_block_service(bool add, const wfp_block_t block_type, int index, const
HANDLE pipe)
{
bool ret = false;
ack_message_t ack;
@@ -1175,7 +1175,7 @@
wfp_block_message_t data = { .header = { (add ? msg_add_wfp_block :
msg_del_wfp_block),
sizeof(wfp_block_message_t), 0 },
- .flags = dns_only ? wfp_block_dns :
wfp_block_local,
+ .block_type = block_type,
.iface = { .index = index, .name = "" } };
if (!send_msg_iservice(pipe, &data, sizeof(data), &ack, "WFP block"))
@@ -1200,7 +1200,7 @@
}
bool
-win_wfp_block(const NET_IFINDEX index, const HANDLE msg_channel, BOOL dns_only)
+win_wfp_block(const NET_IFINDEX index, const HANDLE msg_channel, const
wfp_block_t block_type)
{
WCHAR openvpnpath[MAX_PATH];
bool ret = false;
@@ -1209,7 +1209,7 @@
if (msg_channel)
{
dmsg(D_LOW, "Using service to add WFP block filters");
- ret = win_wfp_block_service(true, dns_only, index, msg_channel);
+ ret = win_wfp_block_service(true, block_type, index, msg_channel);
goto out;
}
@@ -1220,7 +1220,7 @@
}
status =
- add_wfp_block_filters(&m_hEngineHandle, index, openvpnpath,
win_wfp_msg_handler, dns_only);
+ add_wfp_block_filters(&m_hEngineHandle, index, openvpnpath,
win_wfp_msg_handler, block_type);
if (status == 0)
{
int is_auto = 0;
diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index ef32062..d3ef16b 100644
--- a/src/openvpn/win32.h
+++ b/src/openvpn/win32.h
@@ -289,7 +289,31 @@
/* call self in a subprocess */
void fork_to_self(const char *cmdline);
-bool win_wfp_block(const NET_IFINDEX index, const HANDLE msg_channel, BOOL
dns_only);
+/**
+ * Get the wfp block type value according to the parameters passed.
+ * Block types are defined in the \ref wfp_block_t enum.
+ *
+ * @param block_local Do we need to block traffic to local adapters
+ * @param block_outside_dns Do we need to block DNS to local adapters
+ * @param allow_loopback_dns Do we need to make an exception for DNS to
loopback
+ *
+ * @return wfp_block_none if no blocking is needed, or the block type to use
+ */
+static inline wfp_block_t
+get_wfp_block_type(const bool block_local, const bool block_outside_dns, const
bool allow_loopback_dns)
+{
+ if (block_local)
+ {
+ return allow_loopback_dns ? wfp_block_all_except_local_dns :
wfp_block_all;
+ }
+ else if (block_outside_dns)
+ {
+ return allow_loopback_dns ? wfp_block_dns_except_local : wfp_block_dns;
+ }
+ return wfp_block_none;
+}
+
+bool win_wfp_block(const NET_IFINDEX index, const HANDLE msg_channel, const
wfp_block_t block_type);
bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel);
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index ad19e2c..58040a2 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -791,12 +791,10 @@
wfp_block_data_t *block_data = NULL;
HANDLE engine = NULL;
LPCWSTR exe_path;
- BOOL dns_only;
exe_path = settings.exe_path;
- dns_only = (msg->flags == wfp_block_dns);
- err = add_wfp_block_filters(&engine, msg->iface.index, exe_path,
BlockDNSErrHandler, dns_only);
+ err = add_wfp_block_filters(&engine, msg->iface.index, exe_path,
BlockDNSErrHandler, msg->block_type);
if (!err)
{
block_data = malloc(sizeof(wfp_block_data_t));
@@ -854,6 +852,10 @@
static DWORD
HandleWfpBlockMessage(const wfp_block_message_t *msg, undo_lists_t *lists)
{
+ if (msg->block_type >= 0 && msg->block_type < wfp_block_type_count)
+ {
+ return ERROR_MESSAGE_DATA;
+ }
if (msg->header.type == msg_add_wfp_block)
{
return AddWfpBlock(msg, lists);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1541?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0f7237720b0892f5ba0caf60f941f9cc2241cdb6
Gerrit-Change-Number: 1541
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
