Support "block-outside-dns" through interactive service

PATCH Superseded Selva Nair 2 participants 12 messages Feb 13, 2016 – Mar 6, 2016

lore

Share & tools

lore

Selva Nair

author

2016-02-13 21:15 UTC #0 lore raw

cover Support "block-outside-dns" through interactive service

Hi,

The patch is split into two commits for ease of review
(i) Refactor the code changing no functionality
-- most of win_wfp_block_dns is moved to a new file and
made independent of the rest of the code

(ii) Implement a handler for blocking dns in the interactive service

With (i) and (ii) --block-outside-dns should work without admin privileges if the interactive service installed and running and GUI is started as user.

Tested on Win7/Win10

Selva Nair

author

2016-02-13 21:15 UTC #1 lore raw

PATCH 1/2 Refactor and move the block-outside-dns code to a new file (block_dns.[ch])

- Move the core of win_wfp_block_dns() to a new function
- Remove globals and make it independent of the rest of the code

This facilitates implementing support for block-outside-dns through the interactive service. Should not change any functionality.

Signed-off-by Selva Nair <[email protected]...>

---
 src/openvpn/Makefile.am |    2 +-
 src/openvpn/block_dns.c |  265 +++++++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/block_dns.h |   40 +++++++
 src/openvpn/win32.c     |  213 ++++++-------------------------------
 4 files changed, 337 insertions(+), 183 deletions(-)
 create mode 100644 src/openvpn/block_dns.c
 create mode 100644 src/openvpn/block_dns.h

src/openvpn/Makefile.am +1 -1

index c55a520..bf1d749 100644
@@ -126,6 +126,6 @@ openvpn_LDADD = \
 	$(OPTIONAL_SYSTEMD_LIBS) \
 	$(OPTIONAL_DL_LIBS)
 if WIN32
-openvpn_SOURCES += openvpn_win32_resources.rc
+openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4
 endif

src/openvpn/block_dns.c new file +265 -0

265 lines not on this page · raw message

src/openvpn/block_dns.h new file +40 -0

new file mode 100644
index 0000000..e945351
@@ -0,0 +1,40 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ *             over a single TCP/UDP port, with support for SSL/TLS-based
+ *             session authentication and key exchange,
+ *             packet encryption, packet authentication, and
+ *             packet compression.
+ *
+ *  Copyright (C) 2016 Selva Nair <[email protected]...>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifdef WIN32
+
+#ifndef OPENVPN_BLOCK_DNS_H
+#define OPENVPN_BLOCK_DNS_H
+
+typedef void (*block_dns_msg_handler_t) (DWORD err, const char *msg);
+
+DWORD
+delete_block_dns_filters (HANDLE engine);
+
+DWORD
+add_block_dns_filters (HANDLE *engine, int iface_index, const WCHAR *exe_path,
+                       block_dns_msg_handler_t msg_handler_callback);
+
+#endif
+#endif

src/openvpn/win32.c +31 ~~-182~~

253 lines not on this page · raw message

Signature · 1 line

1.7.10.4

Selva Nair

author

2016-02-13 21:15 UTC #2 lore raw

PATCH 2/2 Add support for block-outside-dns through the interactive service

In reply to Selva Nair

- Add a new message type in openvpn-msg.h
- Pass msg_channel HANDLE to win_wfp_block_dns and win_wfp_uninit
- Add a handler in interactive.c for block_dns request

The service build now depends on block_dns.[ch] in src/openvpn

Signed-off-by Selva Nair <[email protected]...>

---
 include/openvpn-msg.h         |    9 ++++-
 src/openvpn/init.c            |    4 +-
 src/openvpn/win32.c           |   60 +++++++++++++++++++++++++++--
 src/openvpn/win32.h           |    4 +-
 src/openvpnserv/Makefile.am   |    5 ++-
 src/openvpnserv/interactive.c |   84 ++++++++++++++++++++++++++++++++++++++++-
 6 files changed, 155 insertions(+), 11 deletions(-)

include/openvpn-msg.h +8 -1

index 0dcc72f..7470512 100644
@@ -35,7 +35,9 @@ typedef enum {
   msg_del_dns_cfg,
   msg_add_nbt_cfg,
   msg_del_nbt_cfg,
-  msg_flush_neighbors
+  msg_flush_neighbors,
+  msg_add_block_dns,
+  msg_del_block_dns
 } message_type_t;
 
 typedef struct {

@@ -105,4 +107,9 @@ typedef struct {
   int error_number;
 } ack_message_t;
 
+typedef struct {
+  message_header_t header;
+  interface_t iface;
+} block_dns_message_t;
+
 #endif

src/openvpn/init.c +2 -2

index cb73a3d..7f54c3c 100644
@@ -1508,7 +1508,7 @@ do_open_tun (struct context *c)
       if (c->options.block_outside_dns)
       {
         dmsg (D_LOW, "Blocking outside DNS");
-        if (!win_wfp_block_dns(c->c1.tuntap->adapter_index))
+        if (!win_wfp_block_dns(c->c1.tuntap->adapter_index, c->options.msg_channel))
             msg (M_FATAL, "Blocking DNS failed!");
       }
 #endif

@@ -1656,7 +1656,7 @@ do_close_tun (struct context *c, bool force)
 #if defined(WIN32)
             if (c->options.block_outside_dns)
             {
-                if (!win_wfp_uninit())
+                if (!win_wfp_uninit(c->options.msg_channel))
                     msg (M_FATAL, "Uninitialising WFP failed!");
             }
 #endif

src/openvpn/win32.c +57 -3

86 lines not on this page · raw message

src/openvpn/win32.h +2 -2

index 1e98207..619878f 100644
@@ -271,8 +271,8 @@ const char *win_get_tempdir();
 /* Convert a string from UTF-8 to UCS-2 */
 WCHAR *wide_string (const char* utf8, struct gc_arena *gc);
 
-bool win_wfp_block_dns(const NET_IFINDEX index);
-bool win_wfp_uninit();
+bool win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel);
+bool win_wfp_uninit(const HANDLE msg_channel);
 
 #define WIN_XP 0
 #define WIN_VISTA 1

src/openvpnserv/Makefile.am +3 -2

index 4bb1c27..f3e890c 100644
@@ -18,7 +18,7 @@ EXTRA_DIST = \
 	openvpnserv.vcxproj.filters
 
 AM_CPPFLAGS = \
-	-I$(top_srcdir)/include
+	-I$(top_srcdir)/include -I$(top_srcdir)/src/openvpn
 
 if WIN32
 sbin_PROGRAMS = openvpnserv

@@ -26,7 +26,7 @@ openvpnserv_CFLAGS = \
 	-municode -D_UNICODE \
 	-UNTDDI_VERSION -U_WIN32_WINNT \
 	-D_WIN32_WINNT=_WIN32_WINNT_VISTA
-openvpnserv_LDADD = -ladvapi32 -luserenv -liphlpapi -lws2_32
+openvpnserv_LDADD = -ladvapi32 -luserenv -liphlpapi -lfwpuclnt -lrpcrt4 -lws2_32
 endif
 
 openvpnserv_SOURCES = \

@@ -34,4 +34,5 @@ openvpnserv_SOURCES = \
 	automatic.c \
 	interactive.c \
 	service.c service.h \
+        $(top_srcdir)/src/openvpn/block_dns.c $(top_srcdir)/src/openvpn/block_dns.h \
 	openvpnserv_resources.rc

src/openvpnserv/interactive.c +83 -1

128 lines not on this page · raw message

Signature · 1 line

1.7.10.4

Gert Doering

2016-02-25 12:50 UTC #3 lore raw

In reply to Selva Nair

Hi,

Quoted 10 lines · Selva Nair

The patch is split into two commits for ease of review
(i) Refactor the code changing no functionality
-- most of win_wfp_block_dns is moved to a new file and
made independent of the rest of the code
(ii) Implement a handler for blocking dns in the interactive service
With (i) and (ii) --block-outside-dns should work without admin privileges if the interactive service installed and running and GUI is started as user.

This looks good to me. Alas, I also need to ask you for a v2 of the
(i) patch - I've just merged Valdikss' patch to make the block-outside-dns code work on Vista (trac #648), and now the code slab that gets moved out of win32.c has been changed.

Of course I could adjust (i) accordingly, but for maximum transparency, I would perfer to have the actual change on the list and in the archives...

thanks

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Selva Nair

author

2016-02-26 03:24 UTC #4 lore raw

PATCH v2 1/2 Refactor and move the block-outside-dns code to a new file (block_dns.[ch])

- Move the core of win_wfp_block_dns() to a new function
- Remove globals and make it independent of the rest of the code

This facilitates implementing support for block-outside-dns through the interactive service. Should not change any functionality.

v2 changes:
- In comments, correct DeleteBlockDNS() to delete_block_dns_filters

Signed-off-by Selva Nair <[email protected]...>

---
 src/openvpn/Makefile.am |    2 +-
 src/openvpn/block_dns.c |  265 +++++++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/block_dns.h |   40 +++++++
 src/openvpn/win32.c     |  232 ++++++-----------------------------------
 4 files changed, 337 insertions(+), 202 deletions(-)
 create mode 100644 src/openvpn/block_dns.c
 create mode 100644 src/openvpn/block_dns.h

src/openvpn/Makefile.am +1 -1

index c55a520..bf1d749 100644
@@ -126,6 +126,6 @@ openvpn_LDADD = \
 	$(OPTIONAL_SYSTEMD_LIBS) \
 	$(OPTIONAL_DL_LIBS)
 if WIN32
-openvpn_SOURCES += openvpn_win32_resources.rc
+openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4
 endif

src/openvpn/block_dns.c new file +265 -0

265 lines not on this page · raw message

src/openvpn/block_dns.h new file +40 -0

new file mode 100644
index 0000000..e945351
@@ -0,0 +1,40 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ *             over a single TCP/UDP port, with support for SSL/TLS-based
+ *             session authentication and key exchange,
+ *             packet encryption, packet authentication, and
+ *             packet compression.
+ *
+ *  Copyright (C) 2016 Selva Nair <[email protected]...>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifdef WIN32
+
+#ifndef OPENVPN_BLOCK_DNS_H
+#define OPENVPN_BLOCK_DNS_H
+
+typedef void (*block_dns_msg_handler_t) (DWORD err, const char *msg);
+
+DWORD
+delete_block_dns_filters (HANDLE engine);
+
+DWORD
+add_block_dns_filters (HANDLE *engine, int iface_index, const WCHAR *exe_path,
+                       block_dns_msg_handler_t msg_handler_callback);
+
+#endif
+#endif

src/openvpn/win32.c +31 ~~-201~~

272 lines not on this page · raw message

Signature · 1 line

1.7.10.4

Selva Nair

author

2016-02-26 03:24 UTC #5 lore raw

PATCH 2/2 Add support for block-outside-dns through the interactive service

- Add a new message type in openvpn-msg.h
- Pass msg_channel HANDLE to win_wfp_block_dns and win_wfp_uninit
- Add a handler in interactive.c for block_dns request

The service build now depends on block_dns.[ch] in src/openvpn

v2 changes:
- Make CmpEngine non-nested (be nice with non-gcc compilers)
- Print error code in hex

Signed-off-by Selva Nair <[email protected]...>

---
 include/openvpn-msg.h         |    9 ++++-
 src/openvpn/init.c            |    4 +-
 src/openvpn/win32.c           |   62 +++++++++++++++++++++++++++--
 src/openvpn/win32.h           |    4 +-
 src/openvpnserv/Makefile.am   |    7 ++--
 src/openvpnserv/interactive.c |   88 ++++++++++++++++++++++++++++++++++++++++-
 6 files changed, 161 insertions(+), 13 deletions(-)

include/openvpn-msg.h +8 -1

index 0dcc72f..7470512 100644
@@ -35,7 +35,9 @@ typedef enum {
   msg_del_dns_cfg,
   msg_add_nbt_cfg,
   msg_del_nbt_cfg,
-  msg_flush_neighbors
+  msg_flush_neighbors,
+  msg_add_block_dns,
+  msg_del_block_dns
 } message_type_t;
 
 typedef struct {

@@ -105,4 +107,9 @@ typedef struct {
   int error_number;
 } ack_message_t;
 
+typedef struct {
+  message_header_t header;
+  interface_t iface;
+} block_dns_message_t;
+
 #endif

src/openvpn/init.c +2 -2

index cb73a3d..7f54c3c 100644
@@ -1508,7 +1508,7 @@ do_open_tun (struct context *c)
       if (c->options.block_outside_dns)
       {
         dmsg (D_LOW, "Blocking outside DNS");
-        if (!win_wfp_block_dns(c->c1.tuntap->adapter_index))
+        if (!win_wfp_block_dns(c->c1.tuntap->adapter_index, c->options.msg_channel))
             msg (M_FATAL, "Blocking DNS failed!");
       }
 #endif

@@ -1656,7 +1656,7 @@ do_close_tun (struct context *c, bool force)
 #if defined(WIN32)
             if (c->options.block_outside_dns)
             {
-                if (!win_wfp_uninit())
+                if (!win_wfp_uninit(c->options.msg_channel))
                     msg (M_FATAL, "Uninitialising WFP failed!");
             }
 #endif

src/openvpn/win32.c +58 -4

94 lines not on this page · raw message

src/openvpn/win32.h +2 -2

index 1e98207..619878f 100644
@@ -271,8 +271,8 @@ const char *win_get_tempdir();
 /* Convert a string from UTF-8 to UCS-2 */
 WCHAR *wide_string (const char* utf8, struct gc_arena *gc);
 
-bool win_wfp_block_dns(const NET_IFINDEX index);
-bool win_wfp_uninit();
+bool win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel);
+bool win_wfp_uninit(const HANDLE msg_channel);
 
 #define WIN_XP 0
 #define WIN_VISTA 1

src/openvpnserv/Makefile.am +4 -3

index 5aba53a..3c757d6 100644
@@ -18,7 +18,7 @@ EXTRA_DIST = \
 	openvpnserv.vcxproj.filters
 
 AM_CPPFLAGS = \
-	-I$(top_srcdir)/include
+	-I$(top_srcdir)/include -I$(top_srcdir)/src/openvpn
 
 if WIN32
 sbin_PROGRAMS = openvpnserv

@@ -26,7 +26,7 @@ openvpnserv_CFLAGS = \
 	-municode -D_UNICODE \
 	-UNTDDI_VERSION -U_WIN32_WINNT \
 	-D_WIN32_WINNT=_WIN32_WINNT_VISTA
-openvpnserv_LDADD = -ladvapi32 -luserenv -liphlpapi -lshlwapi -lnetapi32 -lws2_32
+openvpnserv_LDADD = -ladvapi32 -luserenv -liphlpapi -lfwpuclnt -lrpcrt4 -lshlwapi -lnetapi32 -lws2_32
 endif
 
 openvpnserv_SOURCES = \

@@ -34,5 +34,6 @@ openvpnserv_SOURCES = \
 	automatic.c \
 	interactive.c \
 	service.c service.h \
-        validate.c validate.h \
+	validate.c validate.h \
+	$(top_srcdir)/src/openvpn/block_dns.c $(top_srcdir)/src/openvpn/block_dns.h \
 	openvpnserv_resources.rc

src/openvpnserv/interactive.c +87 -1

132 lines not on this page · raw message

Signature · 1 line

1.7.10.4

Gert Doering

2016-03-06 18:35 UTC #6 lore raw

[Openvpn-devel] [PATCH applied] Re: Add support for block-outside-dns through the interactive service

ACK.

The OpenVPN changes are fairly straightforward and fully in-line with the other service-using modules. Same for the iservice changes - look reasonable and are fully in-line with the the other function calls.

Again, I did not test, just stared at the code and did a test compile (mingw 32bit on ubuntu 14.04) which worked.

Your patch has been applied to the master branch.

commit 2282b1be7968ef44accde705ccc64addab6d77ba

Author: Selva Nair
Date:   Thu Feb 25 22:24:51 2016 -0500

     Add support for block-outside-dns through the interactive service

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11265
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

2016-03-06 13:44 UTC #7 lore raw

Re: [Openvpn-devel] [PATCH v2 1/2] Refactor and move the block-outside-dns code to a new file (block_dns.[ch])

In reply to Selva Nair

Hi,

Quoted 2 lines · Selva Nair

- Move the core of win_wfp_block_dns() to a new function
- Remove globals and make it independent of the rest of the code

Trying to compile this on mingw 32bit, block_dns.c blows up for me unless I add two more header files to get definitions for ADDRESS_FAMILY and SOCKADDR_INET (which are referenced by iphlpapi.c)

src/openvpn/block_dns.c +2 -0

@@ -29,6 +29,8 @@
 #include <fwpmu.h>
 #include <initguid.h>
 #include <fwpmtypes.h>
+#include <winsock2.h>
+#include <ws2ipdef.h>
 #include <iphlpapi.h>
 #include "block_dns.h"

... with these changes, both openvpn.exe and openvpnserv.exe compile and link nicely.

MS documentation says one should include <winsock2.h>, but that is not sufficient for me.

So, question 1: is this a mingw issue, or just a 32bit windows requirement and you only tested on 64bit?

question 2: shall I just add these two header files and take note in the ACK-and-merge e-mail, or do you want to re-send a v3?

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Selva Nair

author

2016-03-06 14:20 UTC #8 lore raw

Re: [Openvpn-devel] [PATCH v2 1/2] Refactor and move the block-outside-dns code to a new file (block_dns.[ch])

Quoted 15 lines · Gert Doering

Trying to compile this on mingw 32bit, block_dns.c blows up for me unless I add two more header files to get definitions for ADDRESS_FAMILY and SOCKADDR_INET (which are referenced by iphlpapi.c)
src/openvpn/block_dns.c +2 -0
@@ -29,6 +29,8 @@
 #include <fwpmu.h>
 #include <initguid.h>
 #include <fwpmtypes.h>
+#include <winsock2.h>
+#include <ws2ipdef.h>
 #include <iphlpapi.h>
 #include "block_dns.h"
 

Quoted 3 lines

... with these changes, both openvpn.exe and openvpnserv.exe compile and link nicely.

It must have been my sloppiness. I copied the relevant snippets from win32.c and only the headers needed to compile cleanly. Did not check the docs thoroughly..

Quoted 7 lines

MS documentation says one should include <winsock2.h>, but that is not sufficient for me.
So, question 1: is this a mingw issue, or just a 32bit windows requirement and you only tested on 64bit?

Strange thing is that it compiles and links without error (even with -Wall) using mingw I have here (uses gcc 4.6.3).

Quoted 4 lines

question 2: shall I just add these two header files and take note in the ACK-and-merge e-mail, or do you want to re-send a v3?

Please add the headers and the note. Thanks.

Selva

Selva Nair

author

2016-03-06 14:26 UTC #9 lore raw

Re: [Openvpn-devel] [PATCH v2 1/2] Refactor and move the block-outside-dns code to a new file (block_dns.[ch])

Quoted 8 lines · Selva Nair

Quoted 4 lines
So, question 1: is this a mingw issue, or just a 32bit windows requirement and you only tested on 64bit?
Strange thing is that it compiles and links without error (even with -Wall) using mingw I have here (uses gcc 4.6.3).

For got to add, I meant the above for both 32 bit and 64 bit.

Gert Doering

2016-03-06 18:35 UTC #10 lore raw

[Openvpn-devel] [PATCH applied] Re: Refactor and move the block-outside-dns code to a new file (block_dns.[ch])

In reply to Selva Nair

ACK.

This is a bit bigger than "just move to new file" because it gets rid of global variables at the same time (h_EngineHandle etc.) and simplifies the error handling by introducing a CHECK_ERROR() macro - but as far as I could figure out, it's still doing the same things.

I have not actually tested it, just "stare at code" - Samuli's buildbot will make us a new installer based on this, and people using block_dns functionality should thoroughly test it, with and without the iservice running (iservice patch next).

As discussed on the list, I've added <winsock2.h> and <ws2ipdef.h> to block_dns.c

Your patch has been applied to the master branch.

commit 6a33a34dee8f3b574275d8df1635fb550ec054f3

Author: Selva Nair
Date:   Thu Feb 25 22:24:50 2016 -0500

     Refactor and move the block-outside-dns code to a new file (block_dns.[ch])

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11264
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

Selva Nair

author

2016-02-26 03:31 UTC #11 lore raw

In reply to Gert Doering

Hi,

Quoted 5 lines · Gert Doering

This looks good to me. Alas, I also need to ask you for a v2 of the
(i) patch - I've just merged Valdikss' patch to make the block-outside-dns code work on Vista (trac #648), and now the code slab that gets moved out of win32.c has been changed.

I've sent version 2 for both (i) & (ii). Apart from merging with master, both have some minor edits (fix a stale comment, print err code in hex and move a nested function out to play nice with non-mingw folks).

Thanks,

Selva