Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

[ValdikSS]

Indeed.
Pushed fixed version to github repo.
https://github.com/ValdikSS/openvpn-with-patches/commit/287ceb11abfa33ee331ba2651572908cbad008d1

If there is no other remarks, I'll send PATCH v8.

On 04.12.2015 08:50, Selva Nair wrote:
> Hi,
>
> On Fri, Dec 4, 2015 at 12:14 AM, ValdikSS  > wrote:
>
> I think you missed the NET_LUID one. I had thought this was fixed earlier, 
> but v7 still has this issue
>
>
> The relevant code is
>
> + if (ConvertInterfaceIndexToLuid(index, ) == NO_ERROR)
> +   dmsg (D_LOW, "Tap Luid: %I64d", tapluid.Value);
>
> which leaves tapluid undefined if there was an error.
>
> Selva
>



signature.asc
Description: OpenPGP digital signature

Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

[Selva Nair]

Hi,

On Fri, Dec 4, 2015 at 12:14 AM, ValdikSS  wrote:

> These issues should be fixed. Please check PATCH v7.
>

I think you missed the NET_LUID one. I had thought this was fixed earlier,
but v7 still has this issue

On 04.12.2015 04:19, James Yonan wrote:
> > These may have been fixed by now, but noticed some issues in the
> original patch that was discussed in the OpenVPN-devel IRC meeting several
> weeks ago.
> >
> > * win_adapter_index_to_luid is declared to return a
> >   NET_LUID but not all code paths return a value.
>

The relevant code is

+ if (ConvertInterfaceIndexToLuid(index, ) == NO_ERROR)
+   dmsg (D_LOW, "Tap Luid: %I64d", tapluid.Value);

which leaves tapluid undefined if there was an error.

Selva

Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

[ValdikSS]

These issues should be fixed. Please check PATCH v7.

On 04.12.2015 04:19, James Yonan wrote:
> These may have been fixed by now, but noticed some issues in the original 
> patch that was discussed in the OpenVPN-devel IRC meeting several weeks ago.
>
> * win_adapter_index_to_luid is declared to return a
>   NET_LUID but not all code paths return a value.
>
> * wcscat(svchostpath, L"\\svchost.exe") isn't checking
>   for buffer overflow.
>
> * FwpmGetAppIdFromFileName0 must be paired with a
>   corresponding FwpmFreeMemory0
>
> James




signature.asc
Description: OpenPGP digital signature

Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

[James Yonan]

These may have been fixed by now, but noticed some issues in the 
original patch that was discussed in the OpenVPN-devel IRC meeting 
several weeks ago.


* win_adapter_index_to_luid is declared to return a
  NET_LUID but not all code paths return a value.

* wcscat(svchostpath, L"\\svchost.exe") isn't checking
  for buffer overflow.

* FwpmGetAppIdFromFileName0 must be paired with a
  corresponding FwpmFreeMemory0

James

Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

[Selva Nair]

Hi,

On Sun, Nov 29, 2015 at 5:10 PM, ValdikSS  wrote:

> This option blocks all out-of-tunnel communication on TCP/UDP port 53
> (except
> for OpenVPN itself), preventing DNS Leaks on Windows 8.1 and 10.


v7 looks good and behaves as promised..

Tested v7-master and v7-2.3 (the latter built with and without wfp) on
windows 7 and windows 10 (64 bit only). For windows 7, also see the note
below.

Selva

Note: On windows7 I would suggest to use this option with either a script
that does "ipconfig /registerdns" or with the config option --register-dns
(the latter is slow as it restarts the dnsclient service etc., but may be
pushed). Without registerdns, at least on my win7 machine, any public dns
servers on other interfaces get queries through the tunnel for a long while
after the connection is up. As if those dns servers have been specified on
the tunnel interface! It doesn't appear to have anything to do with this
patch.

[Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

[ValdikSS]

This option blocks all out-of-tunnel communication on TCP/UDP port 53 (except
for OpenVPN itself), preventing DNS Leaks on Windows 8.1 and 10.
---
 doc/openvpn.8   |  12 ++-
 src/openvpn/Makefile.am |   2 +-
 src/openvpn/init.c  |  17 
 src/openvpn/openvpn.vcxproj |   4 +-
 src/openvpn/options.c   |  10 +++
 src/openvpn/options.h   |   1 +
 src/openvpn/win32.c | 212 
 src/openvpn/win32.h |   3 +
 8 files changed, 256 insertions(+), 5 deletions(-)
 mode change 100755 => 100644 src/openvpn/openvpn.vcxproj

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 9889540..7e73073 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -1129,8 +1129,8 @@ When used with
 .B \-\-client
 or
 .B \-\-pull,
-accept options pushed by server EXCEPT for routes and dhcp options
-like DNS servers.
+accept options pushed by server EXCEPT for routes, block-outside-dns and dhcp
+options like DNS servers.

 When used on the client, this option effectively bars the
 server from adding routes to the client's routing table,
@@ -5574,6 +5574,14 @@ adapter list to the syslog or log file after the TUN/TAP 
adapter
 has been brought up and any routes have been added.
 .\"*
 .TP
+.B \-\-block\-outside\-dns
+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.
+.\"*
+.TP
 .B \-\-dhcp\-renew
 Ask Windows to renew the TAP adapter lease on startup.
 This option is normally unnecessary, as Windows automatically
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index c840f16..c55a520 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -127,5 +127,5 @@ openvpn_LDADD = \
$(OPTIONAL_DL_LIBS)
 if WIN32
 openvpn_SOURCES += openvpn_win32_resources.rc
-openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm
+openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm 
-lfwpuclnt -lrpcrt4
 endif
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c5c0ab6..9f3da60 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1468,6 +1468,15 @@ do_open_tun (struct context *c)
   "up",
   c->c2.es);

+#if defined(WIN32)
+  if (c->options.block_outside_dns)
+  {
+dmsg (D_LOW, "Blocking outside DNS");
+if (!win_wfp_block_dns(c->c1.tuntap->adapter_index))
+msg (M_FATAL, "Blocking DNS failed!");
+  }
+#endif
+
   /* possibly add routes */
   if ((route_order() == ROUTE_AFTER_TUN) && 
(!c->options.route_delay_defined))
do_route (>options, c->c1.route_list, c->c1.route_ipv6_list,
@@ -1596,6 +1605,14 @@ do_close_tun (struct context *c, bool force)
   "down",
   c->c2.es);

+#if defined(WIN32)
+if (c->options.block_outside_dns)
+{
+if (!win_wfp_uninit())
+msg (M_FATAL, "Uninitialising WFP failed!");
+}
+#endif
+
  /* actually close tun/tap device based on --down-pre flag */
  if (c->options.down_pre)
do_close_tun_simple (c);
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
old mode 100755
new mode 100644
index b117b0b..821c46c
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -64,7 +64,7 @@
   
$(SOURCEBASE);%(AdditionalIncludeDirectories)
 
 
-  
libeay32.lib;ssleay32.lib;lzo2.lib;pkcs11-helper.dll.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;%(AdditionalDependencies)
+  
libeay32.lib;ssleay32.lib;lzo2.lib;pkcs11-helper.dll.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;%(AdditionalDependencies)
   
$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)
   true
   Console
@@ -89,7 +89,7 @@
   
$(SOURCEBASE);%(AdditionalIncludeDirectories)
 
 
-  
libeay32.lib;ssleay32.lib;lzo2.lib;pkcs11-helper.dll.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;%(AdditionalDependencies)
+  
libeay32.lib;ssleay32.lib;lzo2.lib;pkcs11-helper.dll.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;%(AdditionalDependencies)
   
$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)
   true
   Console
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 36290a0..4b98275 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -704,6 +704,9 @@ static const char usage_message[] =
   "   optional parameter controls the initial state of 
ex.\n"