Improve --ifconfig and --ifconfig-push sanity check
cover Improve --ifconfig and --ifconfig-push sanity check
This patch set is combining two separate mail threads [1] [2] as they are related.
These patches have also been rearragned, where the first patch adds the generic improvements and prepares for the push.c update which is in the second patch.
These patches combined will resolve the issue reported in Trac ticket #755
[1] <https://www.mail-archive.com/[email protected]/msg13445.html>,
Message-Id: <[email protected]...> [2] <https://www.mail-archive.com/[email protected]/msg13334.html>, Message-Id: <[email protected]...>
David Sommerseth (2):
Improve ifconfig_sanity_check()
push: Provide a warning if --ifconfig-push have --topology mismatch
src/openvpn/push.c | 8 ++++++++ src/openvpn/tun.c | 46 ++++++++++++++++++++++++++++++++++------------ src/openvpn/tun.h | 2 ++ 3 files changed, 44 insertions(+), 12 deletions(-)
Signature · 1 line
1.8.3.1
PATCH v3 1/2 Improve ifconfig_sanity_check()
- Extend ifconfig_sanity_check() to know which context it is called from, if it is used to check --ifconfig or --ifconfig-push
- Improve error messages to also report errornous IP address usage when being in TOP_SUBNET
- Improve the TAP check too, providing the IP address used instead of the subnet mask
v2 - Revert the subnet mask check to the initial version and extend
ifconfig_sanity_check() with a context flag.
v3 - Rearranged some of the code so this can be applied first. Before,
what is now the second patch was to be applied first.
--- src/openvpn/tun.c | 46 ++++++++++++++++++++++++++++++++++------------ src/openvpn/tun.h | 2 ++ 2 files changed, 36 insertions(+), 12 deletions(-)
src/openvpn/tun.c +34 -12
@@ -285,29 +285,51 @@ guess_tuntap_dev (const char *dev,/* --ifconfig-nowarn disables some options sanity checking */ static const char ifconfig_warn_how_to_silence[] ="(silence this warning with --ifconfig-nowarn)" ; -/* -* If !tun, make sure ifconfig_remote_netmask looks -* like a netmask. +/** +* If not a tun device, make sure ifconfig_remote_netmask looks +* like a netmask. * -* If tun, make sure ifconfig_remote_netmask looks -* like an IPv4 address. +* If a tun device, make sure ifconfig_remote_netmask looks +* like an IPv4 address if topology is also TOP_NET30 or TOP_P2P. +* +* The result of this check is only reported to the log file as a warning +* when issues are found. +* +* @param tun Boolean; if true device is a tun device, otherwise tap +* @param addr Address to do sanity check on +* @param topology Expected to be TOP_NET30, TOP_P2P, TOP_SUBNET +* @param pushctx Is this called from a push context or not? This decides if +* the warning should point at --ifconfig or --ifconfig-push */ -static void -ifconfig_sanity_check (bool tun,in_addr_t addr,int topology) +void +ifconfig_sanity_check (bool tun,in_addr_t addr,int topology,bool pushctx) {struct gc_arena gc =gc_new ();const bool looks_like_netmask = ((addr &0xFF000000 ) ==0xFF000000 ); +if (tun) {if (looks_like_netmask && (topology ==TOP_NET30 || topology ==TOP_P2P )) -msg (M_WARN ,"WARNING: Since you are using --dev tun with a point-to-point topology, the second argument to --ifconfig must be an IP address. You are using something (%s) that looks more like a netmask. %s" , -print_in_addr_t (addr,0 , &gc), - ifconfig_warn_how_to_silence); + { +msg (M_WARN ,"WARNING: Since you are using --dev tun with a point-to-point topology, the second argument to %s must be an IP address. You are using something (%s) that looks more like a netmask. %s" , + (pushctx ?"--ifconfig-push" :"--ifconfig" ), +print_in_addr_t (addr,0 , &gc), + ifconfig_warn_how_to_silence); + } +else if (!looks_like_netmask && topology ==TOP_SUBNET ) + { +msg (M_WARN ,"WARNING: Since you are using --dev tun with subnet topology, the second argument to %s must be a netmask, for example something like 255.255.255.0. You are using something (%s) that looks more like an IP address. %s" , + (pushctx ?"--ifconfig-push" :"--ifconfig" ), +print_in_addr_t (addr,0 , &gc), + ifconfig_warn_how_to_silence); + } }else /* tap */ {if (!looks_like_netmask) -msg (M_WARN ,"WARNING: Since you are using --dev tap, the second argument to --ifconfig must be a netmask, for example something like 255.255.255.0. %s" , +msg (M_WARN ,"WARNING: Since you are using --dev tap, the second argument to %s must be a netmask, for example something like 255.255.255.0. You are using something (%s) that looks more like an IP address. %s" , + (pushctx ?"--ifconfig-push" :"--ifconfig" ), +print_in_addr_t (addr,0 , &gc), ifconfig_warn_how_to_silence); }gc_free (&gc);
@@ -643,7 +665,7 @@ init_tun (const char *dev, /* --dev option */if (strict_warn) {struct addrinfo *curele; -ifconfig_sanity_check (tt->type ==DEV_TYPE_TUN , tt->remote_netmask, tt->topology); +ifconfig_sanity_check (tt->type ==DEV_TYPE_TUN , tt->remote_netmask, tt->topology, false);/* * If local_public or remote_public addresses are defined,
src/openvpn/tun.h +2 -0
@@ -229,6 +229,8 @@ const char *guess_tuntap_dev (const char *dev,const char *dev_node, struct gc_arena *gc); +void ifconfig_sanity_check (bool tun,in_addr_t addr,int topology,bool pushctx); + struct tuntap *init_tun (const char *dev, /* --dev option */ const char *dev_type, /* --dev-type option */ int topology, /* one of the TOP_x values */
Signature · 1 line
1.8.3.1
PATCH v3 2/2 push: Provide a warning if --ifconfig-push have --topology mismatch
In reply to David SommersethThis adds a warning to the log file if --topology is configured to use subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option is not an subnet mask.
v2 - Make use of ifconfig_sanity_check() in tun.c instead of doing the exact
same check and warning in prepare_push_reply(). Also improve documentation of ifconfig_sanity_check() while at it.
v3 - Adopted to the new ifconfig_sanity_check() API
Trac: #755
--- src/openvpn/push.c | 8 ++++++++ 1 file changed, 8 insertions(+)
src/openvpn/push.c +8 -0
@@ -333,6 +333,14 @@ prepare_push_reply (struct context *c, struct gc_arena *gc,print_in_addr_t (ifconfig_local,0 , gc),print_in_addr_t (c->c2.push_ifconfig_remote_netmask, 0, gc)); + +/* Warn if ifconfig_remote_netmask contains an unexpected value +* when checking configuration up against TUN/TAP device and +* network topology +*/ +ifconfig_sanity_check (c->c1.tuntap->type ==DEV_TYPE_TUN , + c->c2.push_ifconfig_remote_netmask, + c->options.topology, true); }/* Send peer-id if client supports it */
Signature · 1 line
1.8.3.1