Attention is currently required from: flichtenheld.

Hello flichtenheld,

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

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

to review the following change.


Change subject: Install host routes with onlink scope iroutes for ifconfig-push 
routes
......................................................................

Install host routes with onlink scope iroutes for ifconfig-push routes

Additional IP addresses for hosts that lie outside the primary network
of the configured device need to be added to the operating system to
ensure that traffic for these IP addresses is also directed to the VPN.

For Linux it is import that these extra routes are routes with scope link
rather than static since otherwise routes via these IP addresses, like
iroute, will not work.

Change-Id: I83295e00d1a756dfa44050b0a4493095fb050fff
Signed-off-by: Arne Schwabe <a...@rfc2549.org>
---
M doc/man-sections/server-options.rst
M src/openvpn/dco.c
M src/openvpn/mroute.h
M src/openvpn/multi.c
M src/openvpn/multi.h
5 files changed, 153 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/92/1192/1

diff --git a/doc/man-sections/server-options.rst 
b/doc/man-sections/server-options.rst
index ccc1374..bb58330 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -314,6 +314,9 @@
   3.  Use ``--ifconfig-pool`` allocation for dynamic IP (last
       choice).

+  When DCO is enabled, OpenVPN will install a /32 route for the local IP
+  address
+
 --ifconfig-ipv6-push args
   for ``--client-config-dir`` per-client static IPv6 interface
   configuration, see ``--client-config-dir`` and ``--ifconfig-push`` for
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 70a8c0a..8280fbb 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -670,8 +670,14 @@
         dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, 
addr->netbits,
                                 c->c2.tls_multi->peer_id);
 #else
+        const struct in6_addr *gateway = 
&mi->context.c2.push_ifconfig_ipv6_local;
+        if (addr->type & MR_ONLINK_ADDR)
+        {
+            gateway = NULL;
+        }
+
         net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, addr->netbits,
-                         &mi->context.c2.push_ifconfig_ipv6_local, 
c->c1.tuntap->actual_name, 0,
+                         gateway, c->c1.tuntap->actual_name, 0,
                          DCO_IROUTE_METRIC);
 #endif
     }
@@ -682,7 +688,13 @@
                                 c->c2.tls_multi->peer_id);
 #else
         in_addr_t dest = htonl(addr->v4.addr);
-        net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, 
&mi->context.c2.push_ifconfig_local,
+        const in_addr_t *gateway = &mi->context.c2.push_ifconfig_local;
+        if (addr->type & MR_ONLINK_ADDR)
+        {
+            gateway = NULL;
+        }
+
+        net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, gateway,
                          c->c1.tuntap->actual_name, 0, DCO_IROUTE_METRIC);
 #endif
     }
@@ -713,6 +725,18 @@
                              DCO_IROUTE_METRIC);
 #endif
         }
+
+        /* Check if we added the local network as /32 route as it was not in
+         * the normal on link scope */
+        if (multi_check_push_ifconfig_extra_route(mi))
+        {
+#if defined(_WIN32)
+            dco_win_del_iroute_ipv4(&c->c1.tuntap->dco, push_ifconfig_local, 
32);
+#else
+            net_route_v4_del(&m->top.net_ctx, 
&mi->context.c2.push_ifconfig_local, 32,
+                             NULL, c->c1.tuntap->actual_name, 0, 
DCO_IROUTE_METRIC);
+#endif
+        }
     }

     if (mi->context.c2.push_ifconfig_ipv6_defined)
@@ -727,6 +751,15 @@
                              DCO_IROUTE_METRIC);
 #endif
         }
+        if (multi_check_push_ifconfig_ipv6_extra_route(mi))
+        {
+#if defined(_WIN32)
+            dco_win_del_iroute_ipv6(&c->c1.tuntap->dco, 
&mi->context.c2.push_ifconfig_ipv6_local, 128);
+#else
+            net_route_v6_del(&m->top.net_ctx, 
&mi->context.c2.push_ifconfig_ipv6_local, 128,
+                             NULL, c->c1.tuntap->actual_name, 0, 
DCO_IROUTE_METRIC);
+#endif
+        }
     }
 #endif /* if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) || 
defined(_WIN32) */
 }
diff --git a/src/openvpn/mroute.h b/src/openvpn/mroute.h
index 5b0c694..f4e54ca 100644
--- a/src/openvpn/mroute.h
+++ b/src/openvpn/mroute.h
@@ -20,6 +20,7 @@
  *  with this program; if not, see <https://www.gnu.org/licenses/>.
  */

+
 #ifndef MROUTE_H
 #define MROUTE_H

@@ -74,6 +75,9 @@
 /* Address type mask indicating that proto # is part of address */
 #define MR_WITH_PROTO 32

+/* MRoute is an on link/scope address  ather than a route */
+#define MR_ONLINK_ADDR 64
+
 struct mroute_addr
 {
     uint8_t len;     /* length of address */
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 6e4cc42..a9c753e 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -42,6 +42,7 @@
 #include "ssl_ncp.h"
 #include "vlan.h"
 #include "auth_token.h"
+#include "route.h"
 #include <inttypes.h>
 #include <string.h>

@@ -1240,11 +1241,26 @@
         management_learn_addr(management, &mi->context.c2.mda_context, &addr, 
primary);
     }
 #endif
+    if (primary && multi_check_push_ifconfig_extra_route(mi))
+    {
+        /* "primary" is the VPN ifconfig address of the peer */
+        /* if it does not fall into the network defined by ifconfig_local
+         * we install this as extra onscope address on the interface  */
+        struct gc_arena gc = gc_new();
+        /* The IP address is not assigned to the interface of the DCO device */
+        msg(D_MULTI_LOW, "MULTI: Adding /32 on-link route for route %s -> %s",
+            print_in_addr_t(remote_si.addr.in4.sin_addr.s_addr, 0, &gc),
+            multi_instance_string(mi, false, &gc));
+
+        addr.netbits = 32;
+        addr.type |= MR_ONLINK_ADDR;
+
+        dco_install_iroute(m, mi, &addr);
+        gc_free(&gc);
+    }
+
     if (!primary)
     {
-        /* "primary" is the VPN ifconfig address of the peer and already
-         * known to DCO, so only install "extra" iroutes (primary = false)
-         */
         ASSERT(netbits >= 0); /* DCO requires populated netbits */
         dco_install_iroute(m, mi, &addr);
     }
@@ -1278,6 +1294,24 @@
         management_learn_addr(management, &mi->context.c2.mda_context, &addr, 
primary);
     }
 #endif
+    if (primary && multi_check_push_ifconfig_ipv6_extra_route(mi))
+    {
+        /* "primary" is the VPN ifconfig address of the peer */
+        /* if it does not fall into the network defined by ifconfig_local
+         * we install this as extra onscope address on the interface  */
+        struct gc_arena gc = gc_new();
+        /* The IP address is not assigned to the interface of the DCO device */
+        msg(D_MULTI_LOW, "MULTI: Adding /129 on-link route for "
+                         "ifconfig-ipv6-push %s -> %s",
+            print_in6_addr(mi->context.options.push_ifconfig_ipv6_local, 0, 
&gc),
+            multi_instance_string(mi, false, &gc));
+
+        addr.netbits = 128;
+        addr.type |= MR_ONLINK_ADDR;
+
+        dco_install_iroute(m, mi, &addr);
+        gc_free(&gc);
+    }
     if (!primary)
     {
         /* "primary" is the VPN ifconfig address of the peer and already
@@ -4309,3 +4343,57 @@
      *  }
      */
 }
+
+/**
+ * Determines if the ifconfig_push_local address falls into the range of the 
local
+ * IP addresses of the VPN interface (ifconfig_local with 
ifconfig_remote_netmask)
+ *
+ * @return Returns true if ifconfig_push is outside that range and requires an 
extra
+ * route to be installed.
+ */
+bool
+multi_check_push_ifconfig_extra_route(struct multi_instance *mi)
+{
+    struct options *o = &mi->context.options;
+    in_addr_t local_addr, local_netmask;
+
+    if (!o->ifconfig_local || !o->ifconfig_remote_netmask)
+    {
+        /* If we do not have a local address, we just return false as
+         * this check doesn't make sense. */
+        return false;
+    }
+
+    /* if it falls into the network defined by ifconfig_local we assume
+     * it is already known to DCO and only install "extra" iroutes  */
+    in_addr_t push_ifconfig_local = htonl(mi->context.c2.push_ifconfig_local);
+    inet_pton(AF_INET, o->ifconfig_local, &local_addr);
+    inet_pton(AF_INET, o->ifconfig_remote_netmask, &local_netmask);
+
+    return (local_addr & local_netmask) != (push_ifconfig_local & 
local_netmask);
+}
+
+
+bool
+multi_check_push_ifconfig_ipv6_extra_route(struct multi_instance *mi)
+{
+    struct options *o = &mi->context.options;
+
+    if (!o->ifconfig_ipv6_local || !o->ifconfig_ipv6_netbits)
+    {
+        /* If we do not have a local address, we just return false as
+         * this check doesn't make sense. */
+        return false;
+    }
+
+    /* if it falls into the network defined by ifconfig_local we assume
+     * it is already known to DCO and only install "extra" iroutes  */
+    struct in6_addr ifconfig_local;
+    if (inet_pton(AF_INET6, o->ifconfig_ipv6_local, &ifconfig_local) != 1)
+    {
+        return false;
+    }
+
+    return (!route_ipv6_match_host(&ifconfig_local, o->ifconfig_ipv6_netbits,
+                                   &o->push_ifconfig_ipv6_local));
+}
\ No newline at end of file
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 087c0e6..5ec3904 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -666,6 +666,26 @@
     return ret;
 }

+/**
+ * Determines if the ifconfig_push_local address falls into the range of the 
local
+ * IP addresses of the VPN interface (ifconfig_local with 
ifconfig_remote_netmask)
+ *
+ * @return Returns true if ifconfig_push is outside that range and requires an 
extra
+ * route to be installed.
+ */
+bool
+multi_check_push_ifconfig_extra_route(struct multi_instance *mi);
+
+/**
+ * Determines if the ifconfig_ipv6_local address falls into the range of the 
local
+ * IP addresses of the VPN interface (ifconfig_local with 
ifconfig_remote_netmask)
+ *
+ * @return Returns true if ifconfig_push is outside that range and requires an 
extra
+ * route to be installed.
+ */
+bool
+multi_check_push_ifconfig_ipv6_extra_route(struct multi_instance *mi);
+
 /*
  * Check for signals.
  */

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1192?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: I83295e00d1a756dfa44050b0a4493095fb050fff
Gerrit-Change-Number: 1192
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to