Re: [Openvpn-devel] [PATCH v2] Drop recursively routed packets

2016-08-30 Thread Lev Stipakov 
So, following changes are required for V3:

1) No drop_if_recursive() call for P2P
2) Same for TAP
3) Add an option to disable it

Sounds reasonable?


2016-08-24 16:13 GMT+03:00 Gert Doering :

> Hi,
>
> On Wed, Aug 24, 2016 at 10:12:54AM +0200, Jan Just Keijser wrote:
> > may I suggest to make this configurable,
>
> Well...
>
> > i.e. the user can specify
> > whether rec routed packets should be dropped?  I'm afraid that we might
> > end up with code that drops packets that really should not be dropped -
> > people do weird things with routing: in 99% of the cases in error, but
> > in 1% of the cases because they want to do something funky.
>
> There is no way that this might ever work(*).
>
> If your tunnel gateway is 1.2.3.4, and you send packets to 1.2.3.4 into
> the tunnel, how are these going to arrive?
>
> Packet goes to TUN, is ecapsulated, and sent to 1.2.3.4.
>
> 1.2.3.4 route points to TUN, so packet goes to TUN, gets another layer
> of openvpn, and is sent to 1.2.3.4.
>
> 1.2.3.4 route points to TUN, so packet goes to TUN, gets a *third* layer
> of openvpn, and is sent to 1.2.3.4.
>
> repeat, until the packet hits 1500 byte MTU, and gets fragmented into
> *two* packets that loop forever...
>
> gert
>
>
> (*) it will work if and only if there are multiple routing tables involved,
> that is, packets sent by openvpn itself are not subject to the routing
> tables that would move packets *into* the tunnel - so the Android client
> wouldn't need it (VPN API takes care of this), and with the work on
> Github #13 to suport network name spaces on linux and multiple routing
> tables on *BSD, there are indeed scenarios where this makes sense...
>
> ... so you're right, v3 needs to have an option...
>
> (This started out really small and simple :-) )
> --
> USENET is *not* the non-clickable part of WWW!
>//
> www.muc.de/~gert/
> Gert Doering - Munich, Germany
> g...@greenie.muc.de
> fax: +49-89-35655025g...@net.informatik.tu-
> muenchen.de
>



-- 
-Lev
--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] Drop recursively routed packets

2016-08-22 Thread Gert Doering 
Hi,

On Mon, Jan 04, 2016 at 02:43:44PM +0200, Lev Stipakov wrote:
> v2: better method naming
> 
> On certain OSes (Windows, OS X) when network adapter is
> disabled (ethernet cable pulled off, Wi-Fi hardware switch disabled),
> operating system starts to use tun as an external interface.
> Outgoing packets are routed to tun, UDP encapsulated, given to
> routing table and sent to.. tun.
> 
> As a consequence, system starts talking to itself on full power,
> traffic counters skyrocket and user is not happy.
> 
> To prevent that, drop packets which have gateway IP as
> destination address.
> 
> Tested on Win7/10, OS X.
> 
> Trac #642
> 
> Signed-off-by: Lev Stipakov 

ACK.

Thanks for coding this, thanks to Valdikss for testing (David, can
you include the tested-by: in the commit?).

Code still looks good, as in "does what is advertised, and safely does so".


Per discussion with David, we're not totally happy with the performance
implications - but that's not your fault, our is_ipv4() and is_ipv6()
functions are really doing their job in a very non-optimized way (read:
all the work is done twice, once to find out whether it's v4, then 
again to find out whether it's v6).

I volunteer to refactor the two places where is_ipv*() are called today
(thinking along the lines of "ip_version = get_ip_version(...)" and then
compare ip_version to "4" or "6" - plus, making this an inline function)

For now, we stick to "fixes the issue in a easy to read and non-intrusive
way", so it can go into 2.3.12.

thanks again :)

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature
--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] Drop recursively routed packets

2016-08-22 Thread Gert Doering 
Hi,

On Mon, Jan 04, 2016 at 02:43:44PM +0200, Lev Stipakov wrote:
> v2: better method naming
> 
> On certain OSes (Windows, OS X) when network adapter is
> disabled (ethernet cable pulled off, Wi-Fi hardware switch disabled),
> operating system starts to use tun as an external interface.
> Outgoing packets are routed to tun, UDP encapsulated, given to
> routing table and sent to.. tun.
> 
> As a consequence, system starts talking to itself on full power,
> traffic counters skyrocket and user is not happy.
> 
> To prevent that, drop packets which have gateway IP as
> destination address.
> 
> Tested on Win7/10, OS X.
> 
> Trac #642
> 
> Signed-off-by: Lev Stipakov 

ACK.

Thanks for coding this, thanks to Valdikss for testing (David, can
you include the tested-by: in the commit?).

Code still looks good, as in "does what is advertised, and safely does so".


Per discussion with David, we're not totally happy with the performance
implications - but that's not your fault, our is_ipv4() and is_ipv6()
functions are really doing their job in a very non-optimized way (read:
all the work is done twice, once to find out whether it's v4, then 
again to find out whether it's v6).

I volunteer to refactor the two places where is_ipv*() are called today
(thinking along the lines of "ip_version = get_ip_version(...)" and then
compare ip_version to "4" or "6" - plus, making this an inline function)

For now, we stick to "fixes the issue in a easy to read and non-intrusive
way", so it can go into 2.3.12.

thanks again :)

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature
--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] Drop recursively routed packets

2016-08-07 Thread ValdikSS 
Tested on Windows 10 and Linux. Works as intended.
I've connected to the VPN which redirects the gateway and physically unplugged 
Ethernet cable. Git version started to consume 100% CPU while patched version
either doesn't consume CPU and doesn't print anything or doesn't consume CPU 
and print 5-10 messages per second to the log with verb 4.

On 06/11/2016 02:05 PM, Gert Doering wrote:
> Hi,
>
> On Wed, Apr 06, 2016 at 11:44:34PM +0300, ValdikSS wrote:
> Coming back to this patch set - could you share your test results?
>
> I'll then go and see that I can code review & merge during the next week.
>
> gert
>
>
>
>
> --
> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> patterns at an interface-level. Reveals which users, apps, and protocols are 
> consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
> J-Flow, sFlow and other flows. Make informed decisions using capacity 
> planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
>
>
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel



signature.asc
Description: OpenPGP digital signature

Re: [Openvpn-devel] [PATCH v2] Drop recursively routed packets

2016-06-11 Thread Gert Doering 
Hi,

On Wed, Apr 06, 2016 at 11:44:34PM +0300, ValdikSS wrote:
> More like a notification not to forget about it. I'll test it tomorrow and 
> write the results.
> 
> On 06.04.2016 17:51, Gert Doering wrote:
> > Hi,
> >
> > On Wed, Apr 06, 2016 at 02:37:13PM +0300, ValdikSS wrote:
> > Is this a "I have tested it and it works" confirmation, or a "folks, go
> > out and do work!"?

Coming back to this patch set - could you share your test results?

I'll then go and see that I can code review & merge during the next week.

gert


-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

[Openvpn-devel] [PATCH v2] Drop recursively routed packets

2016-01-04 Thread Lev Stipakov 
v2: better method naming

On certain OSes (Windows, OS X) when network adapter is
disabled (ethernet cable pulled off, Wi-Fi hardware switch disabled),
operating system starts to use tun as an external interface.
Outgoing packets are routed to tun, UDP encapsulated, given to
routing table and sent to.. tun.

As a consequence, system starts talking to itself on full power,
traffic counters skyrocket and user is not happy.

To prevent that, drop packets which have gateway IP as
destination address.

Tested on Win7/10, OS X.

Trac #642

Signed-off-by: Lev Stipakov 
---
 src/openvpn/forward.c | 63 +++
 1 file changed, 63 insertions(+)

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 36a99e6..af05bd0 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -973,6 +973,68 @@ read_incoming_tun (struct context *c)
   perf_pop ();
 }

+/**
+ * Drops UDP packets which OS decided to route via tun. 
+ * 
+ * On Windows and OS X when netwotk adapter is disabled or
+ * disconnected, platform starts to use tun as external interface. 
+ * When packet is sent to tun, it comes to openvpn, encapsulated
+ * and sent to routing table, which sends it again to tun.
+ */
+static void
+drop_if_recursive_routing (struct context *c, struct buffer *buf)
+{
+  bool drop = false;
+  struct openvpn_sockaddr tun_sa = c->c2.to_link_addr->dest;
+
+  if (is_ipv4 (TUNNEL_TYPE (c->c1.tuntap), buf))
+{
+  const struct openvpn_iphdr *pip;
+
+  /* make sure we got whole IP header */
+  if (BLEN (buf) < (int) sizeof (struct openvpn_iphdr))
+   return;
+
+  /* skip ipv4 packets for ipv6 tun */
+  if (tun_sa.addr.sa.sa_family != AF_INET)
+   return;
+
+  pip = (struct openvpn_iphdr *) BPTR (buf);
+
+  /* drop packets with same dest addr as gateway */
+  if (tun_sa.addr.in4.sin_addr.s_addr == pip->daddr)
+   drop = true;
+}
+  else if (is_ipv6 (TUNNEL_TYPE (c->c1.tuntap), buf))
+{
+  const struct openvpn_ipv6hdr *pip6;
+
+  /* make sure we got whole IPv6 header */
+  if (BLEN (buf) < (int) sizeof (struct openvpn_ipv6hdr))
+   return;
+
+  /* skip ipv6 packets for ipv4 tun */
+  if (tun_sa.addr.sa.sa_family != AF_INET6)
+   return;
+
+  /* drop packets with same dest addr as gateway */
+  pip6 = (struct openvpn_ipv6hdr *) BPTR(buf);
+  if (IN6_ARE_ADDR_EQUAL(_sa.addr.in6.sin6_addr, >daddr))
+   drop = true;
+}
+
+  if (drop)
+{
+  struct gc_arena gc = gc_new ();
+
+  c->c2.buf.len = 0;
+
+  msg(D_LOW, "Recursive routing detected, drop tun packet to %s",
+   print_link_socket_actual(c->c2.to_link_addr, ));
+  gc_free ();
+}
+}
+
 /*
  * Input:  c->c2.buf
  * Output: c->c2.to_link
@@ -998,6 +1060,7 @@ process_incoming_tun (struct context *c)

   if (c->c2.buf.len > 0)
 {
+  drop_if_recursive_routing (c, >c2.buf);
   /*
* The --passtos and --mssfix options require
* us to examine the IP header (IPv4 or IPv6).
-- 
1.9.1