Added additional warnings to flag common gotchas:

* Warn when ethernet bridging that the IP address of the
  bridge adapter is probably not the same address that
  the LAN adapter was set to previously.

* When running as a server, warn if the LAN network address is
  the all-popular 192.168.[0|1].x, since this condition commonly
  leads to subnet conflicts down the road.

* Primarily on the client, check for subnet conflicts between
  the local LAN and the VPN subnet.

Added a 'netmask' parameter to get_default_gateway, to return
the netmask of the adapter containing the default gateway.
Only implemented on Windows so far.  Other platforms will
return 255.255.255.0.  Currently the netmask information is
only used to warn about subnet conflicts.

git-svn-id: http://svn.openvpn.net/projects/openvpn/branches/BETA21/openvpn@3179 e7ae566f-a301-0410-adde-c780ea21d3b5

diff --git a/init.c b/init.c
index 4b84b9e2ed99fa4b548a0673f873d846fd547f81..5bd8cf61585b0b0e1ad35e9b0b8bc3362f1cb72c 100644 (file)
--- a/init.c
+++ b/init.c
@@ -1957,6 +1957,9 @@ do_option_warnings (struct context *c)
     msg (M_WARN, "WARNING: using --pull/--client and --ifconfig together is probably not what you want");
 
 #if P2MP_SERVER
+  if (o->server_bridge_defined | o->server_bridge_proxy_dhcp)
+    msg (M_WARN, "NOTE: when bridging your LAN adapter with the TAP adapter, note that the new bridge adapter will often take on its own IP address that is different from what the LAN adapter was previously set to");
+
   if (o->mode == MODE_SERVER)
     {
       if (o->duplicate_cn && o->client_config_dir)
@@ -1976,6 +1979,8 @@ do_option_warnings (struct context *c)
     msg (M_WARN, "WARNING: You have disabled Crypto IVs (--no-iv) which may make " PACKAGE_NAME " less secure");
 
 #ifdef USE_SSL
+  if (o->tls_server)
+    warn_on_use_of_common_subnets ();
   if (o->tls_client
       && !o->tls_verify
       && !o->tls_remote

diff --git a/route.c b/route.c
index b9684420a39c981df39967f8330ae5363ef92944..d3d438510b42c084aba48fc5c8c6263825d792fd 100644 (file)
--- a/route.c
+++ b/route.c
@@ -39,7 +39,6 @@
 #include "memdbg.h"
 
 static void delete_route (const struct route *r, const struct tuntap *tt, unsigned int flags, const struct env_set *es);
-static bool get_default_gateway (in_addr_t *ret);
 static void get_bypass_addresses (struct route_bypass *rb, const unsigned int flags);
 
 #ifdef ENABLE_DEBUG
@@ -372,11 +371,11 @@ init_route_list (struct route_list *rl,
       rl->spec.default_metric_defined = true;
     }
 
-  rl->spec.net_gateway_defined = get_default_gateway (&rl->spec.net_gateway);
+  rl->spec.net_gateway_defined = get_default_gateway (&rl->spec.net_gateway, NULL);
   if (rl->spec.net_gateway_defined)
     {
       setenv_route_addr (es, "net_gateway", rl->spec.net_gateway, -1);
-      dmsg (D_ROUTE_DEBUG, "ROUTE DEBUG: default_gateway=%s", print_in_addr_t (rl->spec.net_gateway, 0, &gc));
+      dmsg (D_ROUTE, "ROUTE default_gateway=%s", print_in_addr_t (rl->spec.net_gateway, 0, &gc));
     }
   else
     {
@@ -666,9 +665,11 @@ add_routes (struct route_list *rl, const struct tuntap *tt, unsigned int flags,
       
       for (i = 0; i < rl->n; ++i)
        {
+         struct route *r = &rl->routes[i];
+         check_subnet_conflict (r->network, r->netmask, "route");
          if (flags & ROUTE_DELETE_FIRST)
-           delete_route (&rl->routes[i], tt, flags, es);
-         add_route (&rl->routes[i], tt, flags, es);
+           delete_route (r, tt, flags, es);
+         add_route (r, tt, flags, es);
        }
       rl->routes_added = true;
     }
@@ -1142,7 +1143,7 @@ test_route (const IP_ADAPTER_INFO *adapters,
            DWORD *index)
 {
   int count = 0;
-  DWORD i = adapter_index_of_ip (adapters, gateway, &count);
+  DWORD i = adapter_index_of_ip (adapters, gateway, &count, NULL);
   if (index)
     *index = i;
   return count;
@@ -1251,18 +1252,24 @@ get_default_gateway_row (const MIB_IPFORWARDTABLE *routes)
   return ret;
 }
 
-static bool
-get_default_gateway (in_addr_t *ret)
+bool
+get_default_gateway (in_addr_t *gw, in_addr_t *netmask)
 {
   struct gc_arena gc = gc_new ();
   bool ret_bool = false;
 
+  const IP_ADAPTER_INFO *adapters = get_adapter_info_list (&gc);
   const MIB_IPFORWARDTABLE *routes = get_windows_routing_table (&gc);
   const MIB_IPFORWARDROW *row = get_default_gateway_row (routes);
 
   if (row)
     {
-      *ret = ntohl (row->dwForwardNextHop);
+      *gw = ntohl (row->dwForwardNextHop);
+      if (netmask)
+       {
+         if (adapter_index_of_ip (adapters, *gw, NULL, netmask) == ~0)
+           *netmask = ~0;
+       }
       ret_bool = true;
     }
 
@@ -1467,8 +1474,8 @@ show_routes (int msglev)
 
 #elif defined(TARGET_LINUX)
 
-static bool
-get_default_gateway (in_addr_t *gateway)
+bool
+get_default_gateway (in_addr_t *gateway, in_addr_t *netmask)
 {
   struct gc_arena gc = gc_new ();
   bool ret = false;
@@ -1521,6 +1528,10 @@ get_default_gateway (in_addr_t *gateway)
       if (best_gw)
        {
          *gateway = best_gw;
+         if (netmask)
+           {
+             *netmask = 0xFFFFFF00; // FIXME -- get the real netmask of the adapter containing the default gateway
+           }
          ret = true;
        }
 
@@ -1595,8 +1606,8 @@ struct {
 #define ROUNDUP(a) \
         ((a) > 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
 
-static bool
-get_default_gateway (in_addr_t *ret)
+bool
+get_default_gateway (in_addr_t *ret, in_addr_t *netmask)
 {
   struct gc_arena gc = gc_new ();
   int s, seq, l, pid, rtm_addrs, i;
@@ -1676,11 +1687,16 @@ get_default_gateway (in_addr_t *ret)
   if (gate != NULL )
     {
       *ret = ntohl(((struct sockaddr_in *)gate)->sin_addr.s_addr);
-#if 1
+#if 0
       msg (M_INFO, "gw %s",
           print_in_addr_t ((in_addr_t) *ret, 0, &gc));
 #endif
 
+      if (netmask)
+       {
+         *netmask = 0xFFFFFF00; // FIXME -- get the real netmask of the adapter containing the default gateway
+       }
+
       gc_free (&gc);
       return true;
     }
@@ -1752,8 +1768,8 @@ struct {
 #define ROUNDUP(a) \
         ((a) > 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
 
-static bool
-get_default_gateway (in_addr_t *ret)
+bool
+get_default_gateway (in_addr_t *ret, in_addr_t *netmask)
 {
   struct gc_arena gc = gc_new ();
   int s, seq, l, pid, rtm_addrs, i;
@@ -1833,11 +1849,16 @@ get_default_gateway (in_addr_t *ret)
   if (gate != NULL )
     {
       *ret = ntohl(((struct sockaddr_in *)gate)->sin_addr.s_addr);
-#if 1
+#if 0
       msg (M_INFO, "gw %s",
           print_in_addr_t ((in_addr_t) *ret, 0, &gc));
 #endif
 
+      if (netmask)
+       {
+         *netmask = 0xFFFFFF00; // FIXME -- get the real netmask of the adapter containing the default gateway
+       }
+
       gc_free (&gc);
       return true;
     }
@@ -1908,8 +1929,8 @@ struct {
 #define ROUNDUP(a) \
         ((a) > 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
 
-static bool
-get_default_gateway (in_addr_t *ret)
+bool
+get_default_gateway (in_addr_t *ret, in_addr_t *netmask)
 {
   struct gc_arena gc = gc_new ();
   int s, seq, l, rtm_addrs, i;
@@ -1990,11 +2011,16 @@ get_default_gateway (in_addr_t *ret)
   if (gate != NULL )
     {
       *ret = ntohl(((struct sockaddr_in *)gate)->sin_addr.s_addr);
-#if 1
+#if 0
       msg (M_INFO, "gw %s",
           print_in_addr_t ((in_addr_t) *ret, 0, &gc));
 #endif
 
+      if (netmask)
+       {
+         *netmask = 0xFFFFFF00; // FIXME -- get the real netmask of the adapter containing the default gateway
+       }
+
       gc_free (&gc);
       return true;
     }
@@ -2007,8 +2033,8 @@ get_default_gateway (in_addr_t *ret)
 
 #else
 
-static bool
-get_default_gateway (in_addr_t *ret)
+bool
+get_default_gateway (in_addr_t *ret, in_addr_t *netmask)
 {
   return false;
 }
@@ -2126,7 +2152,7 @@ get_default_gateway_mac_addr (unsigned char *macaddr)
   in_addr_t gwip = 0;
   bool ret = false;
 
-  if (!get_default_gateway (&gwip))
+  if (!get_default_gateway (&gwip, NULL))
     {
       msg (M_WARN, "GDGMA: get_default_gateway failed");
       goto err;
@@ -2223,13 +2249,13 @@ get_default_gateway_mac_addr (unsigned char *macaddr)
   DWORD a_index;
   const IP_ADAPTER_INFO *ai;
 
-  if (!get_default_gateway (&gwip))
+  if (!get_default_gateway (&gwip, NULL))
     {
       msg (M_WARN, "GDGMA: get_default_gateway failed");
       goto err;
     }
 
-  a_index = adapter_index_of_ip (adapters, gwip, NULL);
+  a_index = adapter_index_of_ip (adapters, gwip, NULL, NULL);
   ai = get_adapter (adapters, a_index);
 
   if (!ai)

diff --git a/route.h b/route.h
index 1a929bdf0fde94f4cac473e755ce245825a24b79..b5ed18fb5f98b8074b4ff81c5d3a0117f9063d91 100644 (file)
--- a/route.h
+++ b/route.h
@@ -156,6 +156,8 @@ void setenv_routes (struct env_set *es, const struct route_list *rl);
 
 bool is_special_addr (const char *addr_str);
 
+bool get_default_gateway (in_addr_t *ip, in_addr_t *netmask);
+
 #if AUTO_USERID
 bool get_default_gateway_mac_addr (unsigned char *macaddr);
 #endif

diff --git a/tun.c b/tun.c
index 22debbe13bcfb28da1eebff5bd4b42d6be28dc10..3c0e1ea7b3c780260fdbc8d1fc1893c0b5cd2314 100644 (file)
--- a/tun.c
+++ b/tun.c
@@ -277,6 +277,58 @@ check_addr_clash (const char *name,
   gc_free (&gc);
 }
 
+/*
+ * Issue a warning if ip/netmask (on the virtual IP network) conflicts with
+ * the settings on the local LAN.  This is designed to flag issues where
+ * (for example) the OpenVPN server LAN is running on 192.168.1.x, but then
+ * an OpenVPN client tries to connect from a public location that is also running
+ * off of a router set to 192.168.1.x.
+ */
+void
+check_subnet_conflict (const in_addr_t ip,
+                      const in_addr_t netmask,
+                      const char *prefix)
+{
+  struct gc_arena gc = gc_new ();
+  in_addr_t lan_gw = 0;
+  in_addr_t lan_netmask = 0;
+
+  if (get_default_gateway (&lan_gw, &lan_netmask))
+    {
+      const in_addr_t lan_network = lan_gw & lan_netmask; 
+      const in_addr_t network = ip & netmask;
+
+      /* do the two subnets defined by network/netmask and lan_network/lan_netmask intersect? */
+      if ((network & lan_netmask) == lan_network
+         || (lan_network & netmask) == network)
+       {
+         msg (M_WARN, "WARNING: potential %s subnet conflict between local LAN [%s/%s] and remote VPN [%s/%s]",
+              prefix,
+              print_in_addr_t (lan_network, 0, &gc),
+              print_in_addr_t (lan_netmask, 0, &gc),
+              print_in_addr_t (network, 0, &gc),
+              print_in_addr_t (netmask, 0, &gc));
+       }
+    }
+  gc_free (&gc);
+}
+
+void
+warn_on_use_of_common_subnets (void)
+{
+  struct gc_arena gc = gc_new ();
+  in_addr_t lan_gw = 0;
+  in_addr_t lan_netmask = 0;
+
+  if (get_default_gateway (&lan_gw, &lan_netmask))
+    {
+      const in_addr_t lan_network = lan_gw & lan_netmask; 
+      if (lan_network == 0xC0A80000 || lan_network == 0xC0A80100)
+       msg (M_WARN, "NOTE: your local LAN uses the extremely common subnet address 192.168.0.x or 192.168.1.x.  Be aware that this might create routing conflicts if you connect to the VPN server from public locations such as internet cafes that use the same subnet.");
+    }
+  gc_free (&gc);
+}
+
 /*
  * Complain if --dev tap and --ifconfig is used on an OS for which
  * we don't have a custom tap ifconfig template below.
@@ -462,6 +514,11 @@ init_tun (const char *dev,       /* --dev option */
                            remote_public,
                            tt->local,
                            tt->remote_netmask);
+
+         if (tt->type == DEV_TYPE_TAP || (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET))
+           check_subnet_conflict (tt->local, tt->remote_netmask, "TUN/TAP adapter");
+         else if (tt->type == DEV_TYPE_TUN)
+           check_subnet_conflict (tt->local, ~0, "TUN/TAP adapter");
        }
 
       /*
@@ -2856,7 +2913,10 @@ is_ip_in_adapter_subnet (const IP_ADAPTER_INFO *ai, const in_addr_t ip, in_addr_
 }
 
 DWORD
-adapter_index_of_ip (const IP_ADAPTER_INFO *list, const in_addr_t ip, int *count)
+adapter_index_of_ip (const IP_ADAPTER_INFO *list,
+                    const in_addr_t ip,
+                    int *count,
+                    in_addr_t *netmask)
 {
   struct gc_arena gc = gc_new ();
   DWORD ret = ~0;
@@ -2898,6 +2958,9 @@ adapter_index_of_ip (const IP_ADAPTER_INFO *list, const in_addr_t ip, int *count
   if (ret == ~0 && count)
     *count = 0;
 
+  if (netmask)
+    *netmask = highest_netmask;
+
   gc_free (&gc);
   return ret;
 }

diff --git a/tun.h b/tun.h
index b61041ecaef698e72de94d122eb3e82f8795d8ec..072d550a3c50c2e00906a9bc3ed97b039e6bbb91 100644 (file)
--- a/tun.h
+++ b/tun.h
@@ -241,6 +241,12 @@ const char *ifconfig_options_string (const struct tuntap* tt, bool remote, bool
 
 bool is_tun_p2p (const struct tuntap *tt);
 
+void check_subnet_conflict (const in_addr_t ip,
+                           const in_addr_t netmask,
+                           const char *prefix);
+
+void warn_on_use_of_common_subnets (void);
+
 /*
  * Inline functions
  */
@@ -313,7 +319,11 @@ const IP_ADAPTER_INFO *get_adapter (const IP_ADAPTER_INFO *ai, DWORD index);
 
 bool is_adapter_up (const struct tuntap *tt, const IP_ADAPTER_INFO *list);
 bool is_ip_in_adapter_subnet (const IP_ADAPTER_INFO *ai, const in_addr_t ip, in_addr_t *highest_netmask);
-DWORD adapter_index_of_ip (const IP_ADAPTER_INFO *list, const in_addr_t ip, int *count);
+
+DWORD adapter_index_of_ip (const IP_ADAPTER_INFO *list,
+                          const in_addr_t ip,
+                          int *count,
+                          in_addr_t *netmask);
 
 void show_tap_win32_adapters (int msglev, int warnlev);
 void show_adapters (int msglev);