Fix combination of --dev tap and --topology subnet across multiple platforms.
PATCH 1/1 Fix combination of --dev tap and --topology subnet across multiple platforms.
--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 10.4, NetBSD 7.0.1, OpenBSD 6.0, and OpenSolaris 10. Compile-tested on MacOS X.
Trac: #1085
--- src/openvpn/tun.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
src/openvpn/tun.c +10 -10
@@ -1091,7 +1091,7 @@ do_ifconfig(struct tuntap *tt,actual ); } -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",
@@ -1173,7 +1173,7 @@ do_ifconfig(struct tuntap *tt, } } -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;
@@ -1210,7 +1210,7 @@ do_ifconfig(struct tuntap *tt,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,
@@ -1239,7 +1239,7 @@ do_ifconfig(struct tuntap *tt,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);
@@ -1282,7 +1282,7 @@ do_ifconfig(struct tuntap *tt,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,
@@ -1316,7 +1316,7 @@ do_ifconfig(struct tuntap *tt,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);
@@ -1372,7 +1372,7 @@ do_ifconfig(struct tuntap *tt, }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",
@@ -1402,7 +1402,7 @@ do_ifconfig(struct tuntap *tt, tt->did_ifconfig = true;/* 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);
@@ -1445,7 +1445,7 @@ do_ifconfig(struct tuntap *tt,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,
@@ -1475,7 +1475,7 @@ do_ifconfig(struct tuntap *tt, tt->did_ifconfig = true;/* 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);
Signature · 1 line
2.18.0
Hi,
Quoted 11 lines · Gert Doering
--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_clienttest.Tested, verified, and fixed on FreeBSD 10.4, NetBSD 7.0.1, OpenBSD 6.0, and OpenSolaris 10. Compile-tested on MacOS X.
Trac: #1085
Oh, because the comments omits that notice (it is in the ticket) - this is 2.4 only. Master looks sufficiently different that this won't apply, and Antonio and I have already conspired and decided that this needs to be fixed for good in master - like:
- introduce a new enum "interface_style" or so, which can be
"TUN_MODE_P2P", "TUN_MODE_SUBNET" and "TAP_MODE" (or so)
- drop "tun", which tries to do the above in boolean form and fails
- in all platform code, have a switch/case statement instead of a chained or nested if/else, making more explicit what the variants are - or, if multiple if/else, at least have unambiguous conditions
(Before you ask: "net30" is not handled as subnet internally - except on windows - but as point-to-point tun)
gert
Signature · 6 lines
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany [email protected]...Hi,
Quoted 7 lines · Gert Doering
--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_clienttest.
Quoted 6 lines
Tested, verified, and fixed on FreeBSD 10.4, NetBSD 7.0.1, OpenBSD 6.0, and OpenSolaris 10. Compile-tested on MacOS X.
Trac: #1085
Signed-off-by Gert Doering <[email protected]...>
After staring at the code I couldn't identify any issue and the logic followed by the code is not more clear (for what it can be ..).
I had a discussion with Gert on IRC and he clarified some doubts I had in a clear manner, therefore I am happy with this patch.
Signature · 1 line
Antonio Quartulli
Thanks for the review.
Patch has been applied to the release/2.4 branch.
For reference: this is still broken in master, but warrants a proper cleanup/refactor approach there.
commit 6c13e24e5709f404231632f14758ea8f6bd9ec83
Author: Gert Doering Date: Sun Aug 19 22:07:03 2018 +0200
Fix combination of --dev tap and --topology subnet across multiple platforms.
Signed-off-by: Gert Doering <[email protected]...>
Acked-by: Antonio Quartulli <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg17414.html
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering