[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-05-18 Thread cron2 (Code Review)
Attention is currently required from: d12fk, plaisthos.

cron2 has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 5: Code-Review-1

(2 comments)

Patchset:

PS5:
Overall the patch does what it says, but it is not -Werror clean.

Please fix, rebase (copyright dates have changed for the files renamed and 
commit 989b22cb6e0 moved around "pipe_message_t" + changed indentation).  
Thanks.


File src/openvpn/init.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/a60e11c6_446166ef :
PS5, Line 2058: unsigned long adapter_index;
This is not `-Werror` clean.  GH builds complain

```
init.c:2115:26: error: variable 'adapter_index' is uninitialized when used here 
[-Werror,-Wuninitialized]
68
del_wfp_block(c, adapter_index);
69
 ^
70
init.c:2047:32: note: initialize the variable 'adapter_index' to silence this 
warning
71
unsigned long adapter_index;
72
   ^
73
= 0
74
1 error generated.
```

This is trivially adjusted (`=0`) but according to project rules I can't do 
that.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 5
Gerrit-Owner: d12fk 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: stipa 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: d12fk 
Gerrit-Comment-Date: Sat, 18 May 2024 16:08:30 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-17 Thread stipa (Code Review)
Attention is currently required from: cron2, d12fk, plaisthos.

stipa has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 5:

(1 comment)

Patchset:

PS5:
Looks good to me. I have compiled it with MSVC and tested following scenarios 
(with server pushing block-local)

 - access to LAN is blocked (got "general failure" when pinging local gateway)

 - traffic to VPN gateway is blocked expect from VPN process (got "general 
failure" when pinging VPN gateway via its public IP)



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 5
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: stipa 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: d12fk 
Gerrit-Comment-Date: Wed, 17 Jan 2024 09:05:02 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-17 Thread stipa (Code Review)
Attention is currently required from: cron2, d12fk, plaisthos.

stipa has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 5: Code-Review+2

(2 comments)

File include/openvpn-msg.h:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/d1fb891c_bb0bba56 :
PS4, Line 27: #include 
> They define some types used in the header. […]
Acknowledged


File src/openvpn/win32.h:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/e3203291_cfd5bef9 :
PS4, Line 28: #include 
> They define some types used in the header. […]
Acknowledged



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 5
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: stipa 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: d12fk 
Gerrit-Comment-Date: Wed, 17 Jan 2024 08:56:03 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: d12fk 
Comment-In-Reply-To: stipa 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-12 Thread flichtenheld (Code Review)
Attention is currently required from: cron2, d12fk, plaisthos, stipa.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 5: Code-Review+1

(2 comments)

Patchset:

PS5:
Generally looks fine to me


File src/openvpn/win32.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/4f22baef_1cbaf8d1 :
PS1, Line 1225: if (ret == false)
> Yes, but no. The function has a single point of return, let's keep it that 
> way. […]
Acknowledged



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 5
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-CC: stipa 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: d12fk 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Fri, 12 Jan 2024 16:25:11 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: d12fk 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-10 Thread d12fk (Code Review)
Attention is currently required from: cron2, d12fk, flichtenheld, plaisthos.

Hello flichtenheld, plaisthos,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/489?usp=email

to look at the new patch set (#5).


Change subject: Windows: enforce 'block-local' with WFP filters
..

Windows: enforce 'block-local' with WFP filters

In an attempt to better defend against the TunnelCrack attacks, enforce
that no traffic can pass to anything else than the VPN interface when
the 'block-local' flags is given with either --redirect-gateway or
--redirect-private.

Reuse much of the existing --block-outside-dns code, but make it more
general, so that it can also block any traffic, not just port 53.

Uses the Windows Filtering Platform for enforcement in addition to the
routes redirecting the networks into the tunnel.

Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Signed-off-by: Heiko Hund 
---
M CMakeLists.txt
M doc/man-sections/vpn-network-options.rst
M include/openvpn-msg.h
M src/openvpn/Makefile.am
M src/openvpn/init.c
M src/openvpn/route.c
M src/openvpn/route.h
M src/openvpn/tun.c
R src/openvpn/wfp_block.c
R src/openvpn/wfp_block.h
M src/openvpn/win32.c
M src/openvpn/win32.h
M src/openvpnserv/CMakeLists.txt
M src/openvpnserv/Makefile.am
M src/openvpnserv/interactive.c
15 files changed, 337 insertions(+), 243 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/89/489/5

diff --git a/CMakeLists.txt b/CMakeLists.txt
index bc46c27..e560ef3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -352,8 +352,6 @@
 src/openvpn/base64.c
 src/openvpn/base64.h
 src/openvpn/basic.h
-src/openvpn/block_dns.h
-src/openvpn/block_dns.c
 src/openvpn/buffer.c
 src/openvpn/buffer.h
 src/openvpn/circ_list.h
@@ -533,6 +531,8 @@
 src/openvpn/ssl_util.h
 src/openvpn/vlan.c
 src/openvpn/vlan.h
+src/openvpn/wfp_block.c
+src/openvpn/wfp_block.h
 src/openvpn/win32.c
 src/openvpn/win32-util.c
 src/openvpn/win32.h
diff --git a/doc/man-sections/vpn-network-options.rst 
b/doc/man-sections/vpn-network-options.rst
index 41d367b..7160a17 100644
--- a/doc/man-sections/vpn-network-options.rst
+++ b/doc/man-sections/vpn-network-options.rst
@@ -352,6 +352,10 @@
   Block access to local LAN when the tunnel is active, except for
   the LAN gateway itself. This is accomplished by routing the local
   LAN (except for the LAN gateway address) into the tunnel.
+  On Windows WFP filters are added in addition to the routes which
+  block access to resources not routed through the VPN adapter.
+  Push this flag to protect against TunnelCrack type of attacks
+  (see: https://tunnelcrack.mathyvanhoef.com/).

   :code:`ipv6`
   Redirect IPv6 routing into the tunnel. This works similar to
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index a1464cd..0fa0cb5 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -24,6 +24,9 @@
 #ifndef OPENVPN_MSG_H_
 #define OPENVPN_MSG_H_

+#include 
+#include 
+
 typedef enum {
 msg_acknowledgement,
 msg_add_address,
@@ -35,8 +38,8 @@
 msg_add_nbt_cfg,
 msg_del_nbt_cfg,
 msg_flush_neighbors,
-msg_add_block_dns,
-msg_del_block_dns,
+msg_add_wfp_block,
+msg_del_wfp_block,
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
@@ -61,6 +64,11 @@
 char name[256];
 } interface_t;

+typedef enum {
+wfp_block_local = 1<<0,
+wfp_block_dns = 1<<1
+} wfp_block_flags_t;
+
 typedef struct {
 message_header_t header;
 short family;
@@ -120,8 +128,9 @@

 typedef struct {
 message_header_t header;
+wfp_block_flags_t flags;
 interface_t iface;
-} block_dns_message_t;
+} wfp_block_message_t;

 typedef struct {
 message_header_t header;
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index b953961..6bf3862 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -156,6 +156,6 @@
$(OPTIONAL_DL_LIBS) \
$(OPTIONAL_INOTIFY_LIBS)
 if WIN32
-openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h 
ring_buffer.h
+openvpn_SOURCES += openvpn_win32_resources.rc wfp_block.c wfp_block.h 
ring_buffer.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm 
-lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi -lbcrypt
 endif
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..396d5ce 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1793,6 +1793,54 @@
 #endif
 }

+/**
+ * Add WFP filters to block traffic to local networks.
+ * Depending on the configuration all or just DNS is filtered.
+ * This functionality is only available on Windows on all other
+ * systems this function is a noop.
+ *
+ * @param c pointer to the connection context
+ */
+static void
+add_wfp_block(struct context *c)
+{
+#if defined(_WIN32)
+/* 

[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-10 Thread d12fk (Code Review)
Attention is currently required from: cron2, flichtenheld, plaisthos, stipa.

d12fk has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 4:

(5 comments)

File include/openvpn-msg.h:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/8bc08e9f_7c28e168 :
PS4, Line 27: #include 
> why those are needed here?
They define some types used in the header. Not that all wouldn't compile 
without them, but when looking at the single file (like clangd does) these 
undefinedness.


File src/openvpn/route.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/25c045e0_c6d575fc :
PS4, Line 621: do_block_local(const struct route_list *rl)
> since this function doesn't really do any blocking and only check options, 
> maybe rename to "is_block […]
Done


File src/openvpn/wfp_block.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/50237516_00bec708 :
PS4, Line 310: /* Fifth filter. Permit IPv4 for the VPN interface.
> Permit DNS or all IPv4 traffic for the VPN interface?
Done


http://gerrit.openvpn.net/c/openvpn/+/489/comment/7e9bc610_f12f2c41 :
PS4, Line 327: /* Sixth filter. Permit IPv6 for the VPN interface.
> Same as above - Permit DNS or all IPv6 traffic for the VPN interface?
Done


File src/openvpn/win32.h:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/fff56b45_305383c8 :
PS4, Line 28: #include 
> why those changes?
They define some types used in the header. Not that all wouldn't compile 
without them, but when looking at the single file (like clangd does) these 
undefinedness.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 4
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-CC: stipa 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Wed, 10 Jan 2024 15:47:32 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: stipa 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-10 Thread stipa (Code Review)
Attention is currently required from: cron2, d12fk, flichtenheld, plaisthos.

stipa has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 4:

(5 comments)

File include/openvpn-msg.h:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/b79b7779_628d912d :
PS4, Line 27: #include 
why those are needed here?


File src/openvpn/route.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/e376b80f_a07c08de :
PS4, Line 621: do_block_local(const struct route_list *rl)
since this function doesn't really do any blocking and only check options, 
maybe rename to "is_block_local_needed()" ?


File src/openvpn/wfp_block.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/98a45da8_a4f2c958 :
PS4, Line 310: /* Fifth filter. Permit IPv4 for the VPN interface.
Permit DNS or all IPv4 traffic for the VPN interface?


http://gerrit.openvpn.net/c/openvpn/+/489/comment/d9207c40_fa180f47 :
PS4, Line 327: /* Sixth filter. Permit IPv6 for the VPN interface.
Same as above - Permit DNS or all IPv6 traffic for the VPN interface?


File src/openvpn/win32.h:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/55c79df7_1e0f8209 :
PS4, Line 28: #include 
why those changes?



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 4
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-CC: stipa 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: d12fk 
Gerrit-Comment-Date: Wed, 10 Jan 2024 10:16:58 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-09 Thread d12fk (Code Review)
Attention is currently required from: cron2, flichtenheld, plaisthos.

Hello flichtenheld, plaisthos,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/489?usp=email

to look at the new patch set (#4).


Change subject: Windows: enforce 'block-local' with WFP filters
..

Windows: enforce 'block-local' with WFP filters

In an attempt to better defend against the TunnelCrack attacks, enforce
that no traffic can pass to anything else than the VPN interface when
the 'block-local' flags is given with either --redirect-gateway or
--redirect-private.

Reuse much of the existing --block-outside-dns code, but make it more
general, so that it can also block any traffic, not just port 53.

Uses the Windows Filtering Platform for enforcement in addition to the
routes redirecting the networks into the tunnel.

Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Signed-off-by: Heiko Hund 
---
M CMakeLists.txt
M doc/man-sections/vpn-network-options.rst
M include/openvpn-msg.h
M src/openvpn/Makefile.am
M src/openvpn/init.c
M src/openvpn/route.c
M src/openvpn/route.h
M src/openvpn/tun.c
R src/openvpn/wfp_block.c
R src/openvpn/wfp_block.h
M src/openvpn/win32.c
M src/openvpn/win32.h
M src/openvpnserv/CMakeLists.txt
M src/openvpnserv/Makefile.am
M src/openvpnserv/interactive.c
15 files changed, 337 insertions(+), 243 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/89/489/4

diff --git a/CMakeLists.txt b/CMakeLists.txt
index bc46c27..e560ef3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -352,8 +352,6 @@
 src/openvpn/base64.c
 src/openvpn/base64.h
 src/openvpn/basic.h
-src/openvpn/block_dns.h
-src/openvpn/block_dns.c
 src/openvpn/buffer.c
 src/openvpn/buffer.h
 src/openvpn/circ_list.h
@@ -533,6 +531,8 @@
 src/openvpn/ssl_util.h
 src/openvpn/vlan.c
 src/openvpn/vlan.h
+src/openvpn/wfp_block.c
+src/openvpn/wfp_block.h
 src/openvpn/win32.c
 src/openvpn/win32-util.c
 src/openvpn/win32.h
diff --git a/doc/man-sections/vpn-network-options.rst 
b/doc/man-sections/vpn-network-options.rst
index 41d367b..7160a17 100644
--- a/doc/man-sections/vpn-network-options.rst
+++ b/doc/man-sections/vpn-network-options.rst
@@ -352,6 +352,10 @@
   Block access to local LAN when the tunnel is active, except for
   the LAN gateway itself. This is accomplished by routing the local
   LAN (except for the LAN gateway address) into the tunnel.
+  On Windows WFP filters are added in addition to the routes which
+  block access to resources not routed through the VPN adapter.
+  Push this flag to protect against TunnelCrack type of attacks
+  (see: https://tunnelcrack.mathyvanhoef.com/).

   :code:`ipv6`
   Redirect IPv6 routing into the tunnel. This works similar to
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index a1464cd..0fa0cb5 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -24,6 +24,9 @@
 #ifndef OPENVPN_MSG_H_
 #define OPENVPN_MSG_H_

+#include 
+#include 
+
 typedef enum {
 msg_acknowledgement,
 msg_add_address,
@@ -35,8 +38,8 @@
 msg_add_nbt_cfg,
 msg_del_nbt_cfg,
 msg_flush_neighbors,
-msg_add_block_dns,
-msg_del_block_dns,
+msg_add_wfp_block,
+msg_del_wfp_block,
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
@@ -61,6 +64,11 @@
 char name[256];
 } interface_t;

+typedef enum {
+wfp_block_local = 1<<0,
+wfp_block_dns = 1<<1
+} wfp_block_flags_t;
+
 typedef struct {
 message_header_t header;
 short family;
@@ -120,8 +128,9 @@

 typedef struct {
 message_header_t header;
+wfp_block_flags_t flags;
 interface_t iface;
-} block_dns_message_t;
+} wfp_block_message_t;

 typedef struct {
 message_header_t header;
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index b953961..6bf3862 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -156,6 +156,6 @@
$(OPTIONAL_DL_LIBS) \
$(OPTIONAL_INOTIFY_LIBS)
 if WIN32
-openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h 
ring_buffer.h
+openvpn_SOURCES += openvpn_win32_resources.rc wfp_block.c wfp_block.h 
ring_buffer.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm 
-lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi -lbcrypt
 endif
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..a2cd041 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1793,6 +1793,54 @@
 #endif
 }

+/**
+ * Add WFP filters to block traffic to local networks.
+ * Depending on the configuration all or just DNS is filtered.
+ * This functionality is only available on Windows on all other
+ * systems this function is a noop.
+ *
+ * @param c pointer to the connection context
+ */
+static void
+add_wfp_block(struct context *c)
+{
+#if defined(_WIN32)
+/* Fortify 

[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-09 Thread d12fk (Code Review)
Attention is currently required from: cron2, flichtenheld, plaisthos.

d12fk has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 3:

(5 comments)

File doc/man-sections/vpn-network-options.rst:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/b1bfa846_8997b555 :
PS1, Line 357:   Push this flag to defend against the TunnelCrack attacks.
> Most terminals handle links sensibly these days. Also we publish this as HTML 
> documentation. […]
Done


File src/openvpn/init.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/2586766b_9e2183fd :
PS1, Line 1971: /* Fortify 'redirect-gateway block-local' with firewall 
rules? */
> Okay there's not much to gain with respect to breaking out run_up_down, as 
> that one requires too muc […]
Done


File src/openvpn/route.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/2b4628b0_c3b07ee0 :
PS1, Line 78: static bool add_route(struct route_ipv4 *r, const struct tuntap 
*tt, unsigned int flags,
> Right, failed to spot this. The build results speak for themselves. […]
Done


File src/openvpn/wfp_block.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/ed619643_4cbcafe1 :
PS1, Line 197: FWPM_FILTER_CONDITION0 Condition[2];
> because the filters are zeroed right below, and than copied into [0] and/or 
> [1] below as needed, so  […]
Done


File src/openvpn/win32.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/a0fa7528_684d91ad :
PS1, Line 1225: if (ret == false)
> Right, but you could just replace the "got out" with "return false" anyway.
Yes, but no. The function has a single point of return, let's keep it that way. 
There's not enough to gain by changing this.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 3
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 09 Jan 2024 14:53:24 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: d12fk 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-09 Thread d12fk (Code Review)
Attention is currently required from: cron2, flichtenheld, plaisthos.

Hello flichtenheld, plaisthos,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/489?usp=email

to look at the new patch set (#3).


Change subject: Windows: enforce 'block-local' with WFP filters
..

Windows: enforce 'block-local' with WFP filters

In an attempt to better defend against the TunnelCrack attacks, enforce
that no traffic can pass to anything else than the VPN interface when
the 'block-local' flags is given with either --redirect-gateway or
--redirect-private.

Reuse much of the existing --block-outside-dns code, but make it more
general, so that it can also block any traffic, not just port 53.

Uses the Windows Filtering Platform for enforcement in addition to the
routes redirecting the networks into the tunnel.

Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Signed-off-by: Heiko Hund 
---
M CMakeLists.txt
M doc/man-sections/vpn-network-options.rst
M include/openvpn-msg.h
M src/openvpn/Makefile.am
M src/openvpn/init.c
M src/openvpn/route.c
M src/openvpn/route.h
M src/openvpn/tun.c
R src/openvpn/wfp_block.c
R src/openvpn/wfp_block.h
M src/openvpn/win32.c
M src/openvpn/win32.h
M src/openvpnserv/CMakeLists.txt
M src/openvpnserv/Makefile.am
M src/openvpnserv/interactive.c
15 files changed, 333 insertions(+), 240 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/89/489/3

diff --git a/CMakeLists.txt b/CMakeLists.txt
index bc46c27..e560ef3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -352,8 +352,6 @@
 src/openvpn/base64.c
 src/openvpn/base64.h
 src/openvpn/basic.h
-src/openvpn/block_dns.h
-src/openvpn/block_dns.c
 src/openvpn/buffer.c
 src/openvpn/buffer.h
 src/openvpn/circ_list.h
@@ -533,6 +531,8 @@
 src/openvpn/ssl_util.h
 src/openvpn/vlan.c
 src/openvpn/vlan.h
+src/openvpn/wfp_block.c
+src/openvpn/wfp_block.h
 src/openvpn/win32.c
 src/openvpn/win32-util.c
 src/openvpn/win32.h
diff --git a/doc/man-sections/vpn-network-options.rst 
b/doc/man-sections/vpn-network-options.rst
index 41d367b..7160a17 100644
--- a/doc/man-sections/vpn-network-options.rst
+++ b/doc/man-sections/vpn-network-options.rst
@@ -352,6 +352,10 @@
   Block access to local LAN when the tunnel is active, except for
   the LAN gateway itself. This is accomplished by routing the local
   LAN (except for the LAN gateway address) into the tunnel.
+  On Windows WFP filters are added in addition to the routes which
+  block access to resources not routed through the VPN adapter.
+  Push this flag to protect against TunnelCrack type of attacks
+  (see: https://tunnelcrack.mathyvanhoef.com/).

   :code:`ipv6`
   Redirect IPv6 routing into the tunnel. This works similar to
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index a1464cd..0fa0cb5 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -24,6 +24,9 @@
 #ifndef OPENVPN_MSG_H_
 #define OPENVPN_MSG_H_

+#include 
+#include 
+
 typedef enum {
 msg_acknowledgement,
 msg_add_address,
@@ -35,8 +38,8 @@
 msg_add_nbt_cfg,
 msg_del_nbt_cfg,
 msg_flush_neighbors,
-msg_add_block_dns,
-msg_del_block_dns,
+msg_add_wfp_block,
+msg_del_wfp_block,
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
@@ -61,6 +64,11 @@
 char name[256];
 } interface_t;

+typedef enum {
+wfp_block_local = 1<<0,
+wfp_block_dns = 1<<1
+} wfp_block_flags_t;
+
 typedef struct {
 message_header_t header;
 short family;
@@ -120,8 +128,9 @@

 typedef struct {
 message_header_t header;
+wfp_block_flags_t flags;
 interface_t iface;
-} block_dns_message_t;
+} wfp_block_message_t;

 typedef struct {
 message_header_t header;
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index b953961..6bf3862 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -156,6 +156,6 @@
$(OPTIONAL_DL_LIBS) \
$(OPTIONAL_INOTIFY_LIBS)
 if WIN32
-openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h 
ring_buffer.h
+openvpn_SOURCES += openvpn_win32_resources.rc wfp_block.c wfp_block.h 
ring_buffer.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm 
-lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi -lbcrypt
 endif
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..efc5f14 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1793,6 +1793,54 @@
 #endif
 }

+/**
+ * Add WFP filters to block traffic to local networks.
+ * Depending on the configuration all or just DNS is filtered.
+ * This functionality is only available on Windows on all other
+ * systems this function is a noop.
+ *
+ * @param c pointer to the connection context
+ */
+static void
+add_wfp_block(struct context *c)
+{
+#if defined(_WIN32)
+/* Fortify 

[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-09 Thread d12fk (Code Review)
Attention is currently required from: cron2, flichtenheld, plaisthos.

d12fk has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 2:

(1 comment)

File src/openvpn/init.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/536482e0_0434a02b :
PS1, Line 1971: /* Fortify 'redirect-gateway block-local' with firewall 
rules? */
> I think there's even more duplicate code before. […]
Okay there's not much to gain with respect to breaking out run_up_down, as that 
one requires too much special handling between the up and down cases.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 2
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 09 Jan 2024 14:39:14 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: d12fk 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-09 Thread d12fk (Code Review)
Attention is currently required from: cron2, d12fk, flichtenheld, plaisthos.

Hello flichtenheld, plaisthos,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/489?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld


Change subject: Windows: enforce 'block-local' with WFP filters
..

Windows: enforce 'block-local' with WFP filters

In an attempt to better defend against the TunnelCrack attacks, enforce
that no traffic can pass to anything else than the VPN interface when
the 'block-local' flags is given with either --redirect-gateway or
--redirect-private.

Reuse much of the existing --block-outside-dns code, but make it more
general, so that it can also block any traffic, not just port 53.

Uses the Windows Filtering Platform for enforcement in addition to the
routes redirecting the networks into the tunnel.

Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Signed-off-by: Heiko Hund 
---
M CMakeLists.txt
M doc/man-sections/vpn-network-options.rst
M include/openvpn-msg.h
M src/openvpn/Makefile.am
M src/openvpn/init.c
M src/openvpn/route.c
M src/openvpn/route.h
M src/openvpn/tun.c
R src/openvpn/wfp_block.c
R src/openvpn/wfp_block.h
M src/openvpn/win32.c
M src/openvpn/win32.h
M src/openvpnserv/CMakeLists.txt
M src/openvpnserv/Makefile.am
M src/openvpnserv/interactive.c
15 files changed, 317 insertions(+), 240 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/89/489/2

diff --git a/CMakeLists.txt b/CMakeLists.txt
index bc46c27..e560ef3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -352,8 +352,6 @@
 src/openvpn/base64.c
 src/openvpn/base64.h
 src/openvpn/basic.h
-src/openvpn/block_dns.h
-src/openvpn/block_dns.c
 src/openvpn/buffer.c
 src/openvpn/buffer.h
 src/openvpn/circ_list.h
@@ -533,6 +531,8 @@
 src/openvpn/ssl_util.h
 src/openvpn/vlan.c
 src/openvpn/vlan.h
+src/openvpn/wfp_block.c
+src/openvpn/wfp_block.h
 src/openvpn/win32.c
 src/openvpn/win32-util.c
 src/openvpn/win32.h
diff --git a/doc/man-sections/vpn-network-options.rst 
b/doc/man-sections/vpn-network-options.rst
index 41d367b..7160a17 100644
--- a/doc/man-sections/vpn-network-options.rst
+++ b/doc/man-sections/vpn-network-options.rst
@@ -352,6 +352,10 @@
   Block access to local LAN when the tunnel is active, except for
   the LAN gateway itself. This is accomplished by routing the local
   LAN (except for the LAN gateway address) into the tunnel.
+  On Windows WFP filters are added in addition to the routes which
+  block access to resources not routed through the VPN adapter.
+  Push this flag to protect against TunnelCrack type of attacks
+  (see: https://tunnelcrack.mathyvanhoef.com/).

   :code:`ipv6`
   Redirect IPv6 routing into the tunnel. This works similar to
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index a1464cd..0fa0cb5 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -24,6 +24,9 @@
 #ifndef OPENVPN_MSG_H_
 #define OPENVPN_MSG_H_

+#include 
+#include 
+
 typedef enum {
 msg_acknowledgement,
 msg_add_address,
@@ -35,8 +38,8 @@
 msg_add_nbt_cfg,
 msg_del_nbt_cfg,
 msg_flush_neighbors,
-msg_add_block_dns,
-msg_del_block_dns,
+msg_add_wfp_block,
+msg_del_wfp_block,
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
@@ -61,6 +64,11 @@
 char name[256];
 } interface_t;

+typedef enum {
+wfp_block_local = 1<<0,
+wfp_block_dns = 1<<1
+} wfp_block_flags_t;
+
 typedef struct {
 message_header_t header;
 short family;
@@ -120,8 +128,9 @@

 typedef struct {
 message_header_t header;
+wfp_block_flags_t flags;
 interface_t iface;
-} block_dns_message_t;
+} wfp_block_message_t;

 typedef struct {
 message_header_t header;
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index b953961..6bf3862 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -156,6 +156,6 @@
$(OPTIONAL_DL_LIBS) \
$(OPTIONAL_INOTIFY_LIBS)
 if WIN32
-openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h 
ring_buffer.h
+openvpn_SOURCES += openvpn_win32_resources.rc wfp_block.c wfp_block.h 
ring_buffer.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm 
-lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi -lbcrypt
 endif
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..72ee52a 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1793,6 +1793,38 @@
 #endif
 }

+static void
+add_wfp_block(struct context *c)
+{
+#if defined(_WIN32)
+/* Fortify 'redirect-gateway block-local' with firewall rules? */
+bool block_local = do_block_local(c->c1.route_list);
+
+if (c->options.block_outside_dns || block_local)
+{
+BOOL 

[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-09 Thread flichtenheld (Code Review)
Attention is currently required from: cron2, d12fk, plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 1:

(2 comments)

File doc/man-sections/vpn-network-options.rst:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/9ab5407d_5fbd4418 :
PS1, Line 357:   Push this flag to defend against the TunnelCrack attacks.
> Agree not to explain TunnelCrack in the openvpn man page, but then references 
> are not much of a thin […]
Most terminals handle links sensibly these days. Also we publish this as HTML 
documentation. We already have existing http links in the document. So adding a 
link would probably not be a bad idea. How about:

"Push this flag to protect against attacks where the attacker tries to tricks 
the client into accessing services through a public network instead of the VPN 
(See e.g. https://tunnelcrack.mathyvanhoef.com/)."


File src/openvpn/win32.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/70f36809_aae74546 :
PS1, Line 1225: if (ret == false)
> We need to set ret as return value anyway, so doing it before the if is more 
> readable IMHO compared  […]
Right, but you could just replace the "got out" with "return false" anyway.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: d12fk 
Gerrit-Comment-Date: Tue, 09 Jan 2024 13:42:47 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: d12fk 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-09 Thread d12fk (Code Review)
Attention is currently required from: cron2, flichtenheld, plaisthos.

d12fk has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 1:

(10 comments)

File doc/man-sections/vpn-network-options.rst:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/616966e3_6506d21f :
PS1, Line 357:   Push this flag to defend against the TunnelCrack attacks.
> Should explain a bit more what this protects against. […]
Agree not to explain TunnelCrack in the openvpn man page, but then references 
are not much of a thing in roff either. Could you elaborate what you have in 
mind? The technicalities are explained before the concluding sentence, so if 
you have a clue about TunnnelCrack things should be clear enough.


File src/openvpn/init.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/b47799d4_dca0b2c0 :
PS1, Line 1971: /* Fortify 'redirect-gateway block-local' with firewall 
rules? */
> Since this hunk and the previous are completely identical I would move them 
> to a separate function. […]
I think there's even more duplicate code before. I'll take a look and submit a 
separate commit if it is not tightly related to wfp only.


File src/openvpn/route.h:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/0c835951_e7364760 :
PS1, Line 248:  * is connected. This definatly returns false when not 
redirecting the gateway
> Typo "definatly"
Done


File src/openvpn/route.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/eb7787ef_be0c1c5c :
PS1, Line 78: static bool add_route(struct route_ipv4 *r, const struct tuntap 
*tt, unsigned int flags,
> this breaks compilation on a zillion of platforms that want to call 
> add_route() from tun. […]
Right, failed to spot this. The build results speak for themselves. =/


http://gerrit.openvpn.net/c/openvpn/+/489/comment/0e347637_c3fa5a5a :
PS1, Line 612: size_t i;
> no reason to leave that on its own line
Done


File src/openvpn/wfp_block.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/97c5a316_ea67f28e :
PS1, Line 167:  * Block outgoing port 53 traffic except for
> "port 53" needs to be changed to reflect the new functionality
Done


http://gerrit.openvpn.net/c/openvpn/+/489/comment/ff62cf3d_89d511ff :
PS1, Line 197: FWPM_FILTER_CONDITION0 Condition[2];
> Why remove the "= {0}" here?
because the filters are zeroed right below, and than copied into [0] and/or [1] 
below as needed, so there's no uninitialized memory.


http://gerrit.openvpn.net/c/openvpn/+/489/comment/662c3688_6dc87eea :
PS1, Line 294: /* Third filter. Block IPv4 to port 53 or all besided 
loopback. */
> "besides"? Or maybe "except"?
Done


http://gerrit.openvpn.net/c/openvpn/+/489/comment/af60e388_d7cb7c20 :
PS1, Line 303: /* Forth filter. Block IPv6 to port 53 or all besides 
loopback */
> "Fourth"
Done


File src/openvpn/win32.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/bdfe473f_2369c40d :
PS1, Line 1225: if (ret == false)
> Simplify to "!win_get_exe_path(openvpnpath, _countof(openvpnpath))"
We need to set ret as return value anyway, so doing it before the if is more 
readable IMHO compared to cramming the function call between the parentheses.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 09 Jan 2024 13:16:20 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-09 Thread flichtenheld (Code Review)
Attention is currently required from: d12fk, plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 1: Code-Review-1

(9 comments)

File doc/man-sections/vpn-network-options.rst:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/fae460bf_083f3462 :
PS1, Line 357:   Push this flag to defend against the TunnelCrack attacks.
Should explain a bit more what this protects against. A reference to 
TunnelCrack is fine but the documentation should be understandable without 
looking it up.


File src/openvpn/init.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/c60f4d43_2e7f9790 :
PS1, Line 1971: /* Fortify 'redirect-gateway block-local' with firewall 
rules? */
Since this hunk and the previous are completely identical I would move them to 
a separate function. 12 lines are definitely worth it IMHO.


File src/openvpn/route.h:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/703fcdb5_6ee704b9 :
PS1, Line 248:  * is connected. This definatly returns false when not 
redirecting the gateway
Typo "definatly"


File src/openvpn/route.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/39e95055_e284408d :
PS1, Line 612: size_t i;
no reason to leave that on its own line


File src/openvpn/wfp_block.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/d821e3e7_d0986f52 :
PS1, Line 167:  * Block outgoing port 53 traffic except for
"port 53" needs to be changed to reflect the new functionality


http://gerrit.openvpn.net/c/openvpn/+/489/comment/81797d0e_701d2e70 :
PS1, Line 197: FWPM_FILTER_CONDITION0 Condition[2];
Why remove the "= {0}" here?


http://gerrit.openvpn.net/c/openvpn/+/489/comment/d1a0883e_d9fb0c63 :
PS1, Line 294: /* Third filter. Block IPv4 to port 53 or all besided 
loopback. */
"besides"? Or maybe "except"?


http://gerrit.openvpn.net/c/openvpn/+/489/comment/919ec205_02180481 :
PS1, Line 303: /* Forth filter. Block IPv6 to port 53 or all besides 
loopback */
"Fourth"


File src/openvpn/win32.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/8c406c41_df81902d :
PS1, Line 1225: if (ret == false)
Simplify to "!win_get_exe_path(openvpnpath, _countof(openvpnpath))"



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: d12fk 
Gerrit-Comment-Date: Tue, 09 Jan 2024 09:15:29 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-08 Thread cron2 (Code Review)
Attention is currently required from: d12fk, flichtenheld, plaisthos.

cron2 has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/489?usp=email )

Change subject: Windows: enforce 'block-local' with WFP filters
..


Patch Set 1:

(1 comment)

File src/openvpn/route.c:

http://gerrit.openvpn.net/c/openvpn/+/489/comment/5b55e13a_2a09a8aa :
PS1, Line 78: static bool add_route(struct route_ipv4 *r, const struct tuntap 
*tt, unsigned int flags,
this breaks compilation on a zillion of platforms that want to call add_route() 
from tun.c - please do not mix "cleanup of unrelated code" and "new features"



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/489?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Gerrit-Change-Number: 489
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: cron2 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: d12fk 
Gerrit-Comment-Date: Tue, 09 Jan 2024 07:03:33 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Windows: enforce 'block-local' with WFP filters

2024-01-08 Thread d12fk (Code Review)
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

http://gerrit.openvpn.net/c/openvpn/+/489?usp=email

to review the following change.


Change subject: Windows: enforce 'block-local' with WFP filters
..

Windows: enforce 'block-local' with WFP filters

In an attempt to better defend against the TunnelCrack attacks, enforce
that no traffic can pass to anything else than the VPN interface when
the 'block-local' flags is given with either --redirect-gateway or
--redirect-private.

Reuse much of the existing --block-outside-dns code, but make it more
general, so that it can also block any traffic, not just port 53.

Uses the Windows Filtering Platform for enforcement in addition to the
routes redirecting the networks into the tunnel.

Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba
Signed-off-by: Heiko Hund 
---
M CMakeLists.txt
M doc/man-sections/vpn-network-options.rst
M include/openvpn-msg.h
M src/openvpn/Makefile.am
M src/openvpn/init.c
M src/openvpn/route.c
M src/openvpn/route.h
M src/openvpn/tun.c
R src/openvpn/wfp_block.c
R src/openvpn/wfp_block.h
M src/openvpn/win32.c
M src/openvpn/win32.h
M src/openvpnserv/CMakeLists.txt
M src/openvpnserv/Makefile.am
M src/openvpnserv/interactive.c
15 files changed, 299 insertions(+), 213 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/89/489/1

diff --git a/CMakeLists.txt b/CMakeLists.txt
index bc46c27..e560ef3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -352,8 +352,6 @@
 src/openvpn/base64.c
 src/openvpn/base64.h
 src/openvpn/basic.h
-src/openvpn/block_dns.h
-src/openvpn/block_dns.c
 src/openvpn/buffer.c
 src/openvpn/buffer.h
 src/openvpn/circ_list.h
@@ -533,6 +531,8 @@
 src/openvpn/ssl_util.h
 src/openvpn/vlan.c
 src/openvpn/vlan.h
+src/openvpn/wfp_block.c
+src/openvpn/wfp_block.h
 src/openvpn/win32.c
 src/openvpn/win32-util.c
 src/openvpn/win32.h
diff --git a/doc/man-sections/vpn-network-options.rst 
b/doc/man-sections/vpn-network-options.rst
index 41d367b..630e8f5 100644
--- a/doc/man-sections/vpn-network-options.rst
+++ b/doc/man-sections/vpn-network-options.rst
@@ -352,6 +352,9 @@
   Block access to local LAN when the tunnel is active, except for
   the LAN gateway itself. This is accomplished by routing the local
   LAN (except for the LAN gateway address) into the tunnel.
+  On Windows WFP filters are added in addition to the routes which
+  block access to resources not routed through the VPN adapter.
+  Push this flag to defend against the TunnelCrack attacks.

   :code:`ipv6`
   Redirect IPv6 routing into the tunnel. This works similar to
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index a1464cd..0fa0cb5 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -24,6 +24,9 @@
 #ifndef OPENVPN_MSG_H_
 #define OPENVPN_MSG_H_

+#include 
+#include 
+
 typedef enum {
 msg_acknowledgement,
 msg_add_address,
@@ -35,8 +38,8 @@
 msg_add_nbt_cfg,
 msg_del_nbt_cfg,
 msg_flush_neighbors,
-msg_add_block_dns,
-msg_del_block_dns,
+msg_add_wfp_block,
+msg_del_wfp_block,
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
@@ -61,6 +64,11 @@
 char name[256];
 } interface_t;

+typedef enum {
+wfp_block_local = 1<<0,
+wfp_block_dns = 1<<1
+} wfp_block_flags_t;
+
 typedef struct {
 message_header_t header;
 short family;
@@ -120,8 +128,9 @@

 typedef struct {
 message_header_t header;
+wfp_block_flags_t flags;
 interface_t iface;
-} block_dns_message_t;
+} wfp_block_message_t;

 typedef struct {
 message_header_t header;
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index b953961..6bf3862 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -156,6 +156,6 @@
$(OPTIONAL_DL_LIBS) \
$(OPTIONAL_INOTIFY_LIBS)
 if WIN32
-openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h 
ring_buffer.h
+openvpn_SOURCES += openvpn_win32_resources.rc wfp_block.c wfp_block.h 
ring_buffer.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm 
-lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi -lbcrypt
 endif
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..4108e8c 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1916,12 +1916,15 @@
 c->c2.es);

 #if defined(_WIN32)
-if (c->options.block_outside_dns)
+/* Fortify 'redirect-gateway block-local' with firewall rules? */
+bool block_local = do_block_local(c->c1.route_list);
+
+if (c->options.block_outside_dns || block_local)
 {
-dmsg(D_LOW, "Blocking outside DNS");
-if (!win_wfp_block_dns(c->c1.tuntap->adapter_index, 
c->options.msg_channel))
+