Add parameter "dev <device>" to add_route and delete_route for IPv4, as it is done for IPv6.

When openvpn tries to add (or delete) an IPv4 route, doesn't use the parameter "dev <device>" in the command. If there's a local subnet that matches the VPN Server network, the route would use a wrong interface. Ex:
local subnet 10.100.0.0/24 on iface eth2
openvpn pool 10.100.0.0/16 on iface tap2 (ifconfig_pool_remote_ip is 10.100.0.1)

      push "route 10.10.0.1 255.255.255.255"

Without the patch, the route will be added with eth2, instead of tap2

---
 route.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

index cb685c3..b9b478f 100644
@@ -1090,6 +1090,7 @@ add_route (struct route *r, const struct tuntap *tt, unsigned int flags, const s
   const char *netmask;
   const char *gateway;
   bool status = false;
+  const char *device = tt->actual_name;
 
   if (!r->defined)
     return;

@@ -1113,23 +1114,26 @@ add_route (struct route *r, const struct tuntap *tt, unsigned int flags, const s
 
 #if defined(TARGET_LINUX)
 #ifdef CONFIG_FEATURE_IPROUTE
-  argv_printf (&argv, "%s route add %s/%d via %s",
+  argv_printf (&argv, "%s route add %s/%d via %s dev %s",
   	      iproute_path,
 	      network,
 	      count_netmask_bits(netmask),
-	      gateway);
+	      gateway,
+	      device);
   if (r->metric_defined)
     argv_printf_cat (&argv, "metric %d", r->metric);
 
 #else
-  argv_printf (&argv, "%s add -net %s netmask %s gw %s",
+  argv_printf (&argv, "%s add -net %s netmask %s gw %s dev %s",
 		ROUTE_PATH,
 	      network,
 	      netmask,
-	      gateway);
+	      gateway,
+	      device);
   if (r->metric_defined)
     argv_printf_cat (&argv, "metric %d", r->metric);
 #endif  /*CONFIG_FEATURE_IPROUTE*/
+
   argv_msg (D_ROUTE, &argv);
   status = openvpn_execve_check (&argv, es, 0, "ERROR: Linux route add command failed");
 

@@ -1452,6 +1456,7 @@ delete_route (const struct route *r, const struct tuntap *tt, unsigned int flags
   const char *network;
   const char *netmask;
   const char *gateway;
+  const char *device = tt->actual_name;
 
   if (!r->defined)
     return;

@@ -1465,16 +1470,18 @@ delete_route (const struct route *r, const struct tuntap *tt, unsigned int flags
 
 #if defined(TARGET_LINUX)
 #ifdef CONFIG_FEATURE_IPROUTE
-  argv_printf (&argv, "%s route del %s/%d",
+  argv_printf (&argv, "%s route del %s/%d dev %s",
   	      iproute_path,
 	      network,
-	      count_netmask_bits(netmask));
+	      count_netmask_bits(netmask),
+	      device);
 #else
 
-  argv_printf (&argv, "%s del -net %s netmask %s",
+  argv_printf (&argv, "%s del -net %s netmask %s dev %s",
 		ROUTE_PATH,
 	      network,
-	      netmask);
+	      netmask,
+	      device);
 #endif /*CONFIG_FEATURE_IPROUTE*/
   if (r->metric_defined)
     argv_printf_cat (&argv, "metric %d", r->metric);

1.7.1

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Thanks a lot for a better explanation and resending the patch. At first glance this makes sense. But it will break some features in OpenVPN.

- From the man page for --route:

--route network/IP [netmask] [gateway] [metric]
[...snip...]

gateway default -- taken from --route-gateway or the second parameter to --ifconfig when --dev tun is spec‐ ified.

metric default -- taken from --route-metric otherwise 0.

The default can be specified by leaving an option blank or setting it to "default".

The network and gateway parameters can also be speci‐ fied as a DNS or /etc/hosts file resolvable name, or as one of three special keywords:

vpn_gateway -- The remote VPN endpoint address
(derived either from --route-gateway or the second parameter to --ifconfig when --dev tun is specified).

net_gateway -- The pre-existing IP default gateway, read from the routing table (not supported on all OSes).

remote_host -- The --remote address if OpenVPN is being run in client mode, and is undefined in server mode.

As you can see, the [gateway] part of the --route statement can also have the keywords 'vpn_gateway', 'net_gateway' or 'remote_host'. Your patch in the current shape will break this feature.

Gert and I have discussed your patch, and we believe a more appropriate patch would consider the 'vpn_gateway' keyword. So if the gateway the new route will use matches with what 'vpn_gateway' expands to, then adding a 'dev' argument to route makes sense.

What do you think?

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk3WNN0ACgkQDC186MBRfrqbPACfbnziB9jPepFiVfYlRI7hIsFi cAwAn0ClniBDn6HY1oV1JIms5TWIBmtO
=Oy1c
-----END PGP SIGNATURE-----