diff mbox series

[Openvpn-devel] Fix combination of --dev tap and --topology subnet across multiple platforms.

Message ID 20200914070843.51678-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel] Fix combination of --dev tap and --topology subnet across multiple platforms. | expand

Commit Message

Gert Doering Sept. 13, 2020, 9:08 p.m. UTC 
--topology should have no effect in tap mode (tap is always "subnet"),
but due to the way options are checked, setting "topology subnet" caught
an improper branch on all non-linux and non-win32 platforms.

Easily tested by adding "--topology subnet" to a "--dev tap" t_client
test.

Tested, verified, and fixed on FreeBSD 13.3, NetBSD 8.1, OpenBSD 6.5,
OpenIndiana 2019 (Solaris) and MacOS X Mojave.

This is a forward-port of commit 6c13e24e5709 - the original intent
for "master" was to restructure tun.c in a larger way and clean up
these if() blocks more nicely... which has not happened yet, so this
patch is basically applying exactly the same changes to context that
has changed too much for git to be able to do this automatically.

Trac: #1085

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/tun.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Antonio Quartulli Sept. 17, 2020, 9:28 p.m. UTC | #1 
Hi,

On 14/09/2020 09:08, Gert Doering wrote:
> --topology should have no effect in tap mode (tap is always "subnet"),
> but due to the way options are checked, setting "topology subnet" caught
> an improper branch on all non-linux and non-win32 platforms.
> 
> Easily tested by adding "--topology subnet" to a "--dev tap" t_client
> test.
> 
> Tested, verified, and fixed on FreeBSD 13.3, NetBSD 8.1, OpenBSD 6.5,
> OpenIndiana 2019 (Solaris) and MacOS X Mojave.
> 
> This is a forward-port of commit 6c13e24e5709 - the original intent
> for "master" was to restructure tun.c in a larger way and clean up
> these if() blocks more nicely... which has not happened yet, so this
> patch is basically applying exactly the same changes to context that
> has changed too much for git to be able to do this automatically.
> 
> Trac: #1085
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>

This change is very well contained and straightforward.
It just does what it says and makes the if-branches more clear.

Acked-by: Antonio Quartulli <a@unstable.cc>
Gert Doering Sept. 17, 2020, 9:45 p.m. UTC | #2 
Patch has been applied to the master and release/2.5 branch.

(I leave the ticket open with "milestone 2.6" to prod me into
the long-planned refactoring of these if() variants into something
with an enum and "speaking" identifiers)

commit 860a7bc77ef515f1d042a2860f7e2bd9980e19be (master)
commit a5964e34057bd3a2c0cb232f1abc6feeefdf146e (release/2.5)
Author: Gert Doering
Date:   Mon Sep 14 09:08:43 2020 +0200

     Fix combination of --dev tap and --topology subnet across multiple platforms.

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Antonio Quartulli <a@unstable.cc>
     Message-Id: <20200914070843.51678-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20987.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 651cb871..fde94294 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1224,7 +1224,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
         argv_printf(&argv, "%s %s netmask 255.255.255.255", IFCONFIG_PATH,
                     ifname);
     }
-    else if (tt->topology == TOP_SUBNET)
+    else if (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         argv_printf(&argv, "%s %s %s %s netmask %s mtu %d up", IFCONFIG_PATH,
                     ifname, ifconfig_local, ifconfig_local,
@@ -1243,7 +1243,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
         solaris_error_close(tt, es, ifname, false);
     }
 
-    if (!tun && tt->topology == TOP_SUBNET)
+    if (!tun && tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         /* Add a network route for the local tun interface */
         struct route_ipv4 r;
@@ -1274,7 +1274,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
                     IFCONFIG_PATH, ifname, ifconfig_local,
                     ifconfig_remote_netmask, tun_mtu);
     }
-    else if (tt->topology == TOP_SUBNET)
+    else if (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         remote_end = create_arbitrary_remote( tt );
         argv_printf(&argv, "%s %s %s %s mtu %d netmask %s up -link0",
@@ -1292,7 +1292,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
     openvpn_execve_check(&argv, es, S_FATAL, "OpenBSD ifconfig failed");
 
     /* Add a network route for the local tun interface */
-    if (!tun && tt->topology == TOP_SUBNET)
+    if (!tun && tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         struct route_ipv4 r;
         CLEAR(r);
@@ -1312,7 +1312,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
                     IFCONFIG_PATH, ifname, ifconfig_local,
                     ifconfig_remote_netmask, tun_mtu);
     }
-    else if (tt->topology == TOP_SUBNET)
+    else if (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         remote_end = create_arbitrary_remote(tt);
         argv_printf(&argv, "%s %s %s %s mtu %d netmask %s up", IFCONFIG_PATH,
@@ -1334,7 +1334,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
     openvpn_execve_check(&argv, es, S_FATAL, "NetBSD ifconfig failed");
 
     /* Add a network route for the local tun interface */
-    if (!tun && tt->topology == TOP_SUBNET)
+    if (!tun && tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         struct route_ipv4 r;
         CLEAR(r);
@@ -1366,7 +1366,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
     }
     else
     {
-        if (tt->topology == TOP_SUBNET)
+        if (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
         {
             argv_printf(&argv, "%s %s %s %s netmask %s mtu %d up",
                         IFCONFIG_PATH, ifname, ifconfig_local, ifconfig_local,
@@ -1384,7 +1384,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
     openvpn_execve_check(&argv, es, S_FATAL, "Mac OS X ifconfig failed");
 
     /* Add a network route for the local tun interface */
-    if (!tun && tt->topology == TOP_SUBNET)
+    if (!tun && tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         struct route_ipv4 r;
         CLEAR(r);
@@ -1406,7 +1406,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
                     IFCONFIG_PATH, ifname, ifconfig_local,
                     ifconfig_remote_netmask, tun_mtu);
     }
-    else if (tt->topology == TOP_SUBNET)
+    else if (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         remote_end = create_arbitrary_remote( tt );
         argv_printf(&argv, "%s %s %s %s mtu %d netmask %s up", IFCONFIG_PATH,
@@ -1423,7 +1423,7 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
     openvpn_execve_check(&argv, es, S_FATAL, "FreeBSD ifconfig failed");
 
     /* Add a network route for the local tun interface */
-    if (!tun && tt->topology == TOP_SUBNET)
+    if (!tun && tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
     {
         struct route_ipv4 r;
         CLEAR(r);