fix IPv4 split-{in,ex}clude routes with misspecified host bits

Some VPN platforms (GlobalProtect, apparently) allow administrators to input
such non-canonical IPv4 routes, and some routing configuration utilities
(apparently *not* iproute2) simply do not accept such non-canonical IPv4
routes.

An example of the confusion this can cause:
    https://lists.infradead.org/pipermail/openconnect-devel/2020-April/005665.html

The robustness principle suggests that the best thing to do here is to fix
these routes, but complain about them while we're at it.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

diff --git a/script.c b/script.c
index 5040745248a2da9033efd1933d89758ed778c92e..9115e567bff6c711ee21b303e16a6229a4384d37 100644 (file)
--- a/script.c
+++ b/script.c
@@ -91,15 +91,15 @@ static int process_split_xxclude(struct openconnect_info *vpninfo,
                                 int include, const char *route, int *v4_incs,
                                 int *v6_incs)
 {
-       struct in_addr addr;
+       struct in_addr net_addr, mask_addr;
        const char *in_ex = include ? "IN" : "EX";
-       char envname[80];
+       char envname[80], uptoslash[20];
        const char *slash;
        char *endp;
        int masklen;
 
        slash = strchr(route, '/');
-       envname[79] = 0;
+       envname[79] = uptoslash[19] = 0;
 
        if (strchr(route, ':')) {
                snprintf(envname, 79, "CISCO_IPV6_SPLIT_%sC_%d_ADDR", in_ex,
@@ -114,33 +114,63 @@ static int process_split_xxclude(struct openconnect_info *vpninfo,
                return 0;
        }
 
+       if (!slash)
+               strncpy(uptoslash, route, sizeof(uptoslash)-1);
+       else {
+               int l = MIN(slash - route, sizeof(uptoslash)-1);
+               strncpy(uptoslash, route, l);
+               uptoslash[l] = 0;
+       }
+
+       /* Network address must be parseable */
+       if (!inet_aton(uptoslash, &net_addr)) {
+       bad:
+               if (include)
+                       vpn_progress(vpninfo, PRG_ERR,
+                                        _("Discard bad split include: \"%s\"\n"),
+                                        route);
+               else
+                       vpn_progress(vpninfo, PRG_ERR,
+                                        _("Discard bad split exclude: \"%s\"\n"),
+                                        route);
+               return -EINVAL;
+       }
+
+       /* Accept netmask in several forms */
        if (!slash) {
                /* no mask (same as /32) */
                masklen = 32;
-               addr.s_addr = netmaskbits(32);
+               mask_addr.s_addr = netmaskbits(32);
        } else if ((masklen = strtol(slash+1, &endp, 10))<=32 && *endp!='.') {
                /* mask is /N */
-               addr.s_addr = netmaskbits(masklen);
-       } else if (inet_aton(slash+1, &addr)) {
+               mask_addr.s_addr = netmaskbits(masklen);
+       } else if (inet_aton(slash+1, &mask_addr)) {
                /* mask is /A.B.C.D */
-               masklen = netmasklen(addr);
-       } else {
+               masklen = netmasklen(mask_addr);
+               /* something invalid like /255.0.0.1 */
+               if (netmaskbits(masklen) != mask_addr.s_addr)
+                       goto bad;
+       } else
+               goto bad;
+
+       /* Fix incorrectly-set host bits */
+       if (net_addr.s_addr & ~mask_addr.s_addr) {
+               net_addr.s_addr &= mask_addr.s_addr;
                if (include)
                        vpn_progress(vpninfo, PRG_ERR,
-                                        _("Discard bad split include: \"%s\"\n"),
-                                        route);
+                                    _("WARNING: Split include \"%s\" has host bits set, replacing with \"%s/%d\".\n"),
+                                    route, inet_ntoa(net_addr), masklen);
                else
                        vpn_progress(vpninfo, PRG_ERR,
-                                        _("Discard bad split exclude: \"%s\"\n"),
-                                        route);
-               return -EINVAL;
+                                    _("WARNING: Split exclude \"%s\" has host bits set, replacing with \"%s/%d\".\n"),
+                                    route, inet_ntoa(net_addr), masklen);
        }
 
        snprintf(envname, 79, "CISCO_SPLIT_%sC_%d_ADDR", in_ex, *v4_incs);
-       script_setenv(vpninfo, envname, route, slash ? slash - route : 0, 0);
+       script_setenv(vpninfo, envname, inet_ntoa(net_addr), 0, 0);
 
        snprintf(envname, 79, "CISCO_SPLIT_%sC_%d_MASK", in_ex, *v4_incs);
-       script_setenv(vpninfo, envname, inet_ntoa(addr), 0, 0);
+       script_setenv(vpninfo, envname, inet_ntoa(mask_addr), 0, 0);
 
        snprintf(envname, 79, "CISCO_SPLIT_%sC_%d_MASKLEN", in_ex, *v4_incs);
        script_setenv_int(vpninfo, envname, masklen);
@@ -237,16 +267,27 @@ void prepare_script_env(struct openconnect_info *vpninfo)
                        struct in_addr addr;
                        struct in_addr mask;
 
-                       if (inet_aton(vpninfo->ip_info.addr, &addr) &&
-                           inet_aton(vpninfo->ip_info.netmask, &mask)) {
+                       if (!inet_aton(vpninfo->ip_info.addr, &addr))
+                               vpn_progress(vpninfo, PRG_ERR,
+                                            _("Ignoring legacy network because address \"%s\" is invalid.\n"),
+                                            vpninfo->ip_info.netmask);
+                       else if (!inet_aton(vpninfo->ip_info.netmask, &mask))
+                       bad_netmask:
+                               vpn_progress(vpninfo, PRG_ERR,
+                                            _("Ignoring legacy network because netmask \"%s\" is invalid.\n"),
+                                            vpninfo->ip_info.netmask);
+                       else {
                                char *netaddr;
+                               int masklen = netmasklen(mask);
 
+                               if (netmaskbits(masklen) != mask.s_addr)
+                                       goto bad_netmask;
                                addr.s_addr &= mask.s_addr;
                                netaddr = inet_ntoa(addr);
 
                                script_setenv(vpninfo, "INTERNAL_IP4_NETADDR", netaddr, 0, 0);
                                script_setenv(vpninfo, "INTERNAL_IP4_NETMASK", vpninfo->ip_info.netmask, 0, 0);
-                               script_setenv_int(vpninfo, "INTERNAL_IP4_NETMASKLEN", netmasklen(mask));
+                               script_setenv_int(vpninfo, "INTERNAL_IP4_NETMASKLEN", masklen);
                        }
                }
        }

diff --git a/www/changelog.xml b/www/changelog.xml
index ba0c2b53f4a58df719913dc991fc5dcf98bdcf6b..3475ac64a46d06f6297f9184560941296e16d7c6 100644 (file)
--- a/www/changelog.xml
+++ b/www/changelog.xml
@@ -17,6 +17,7 @@
      <ul>
        <li>Add bash completion support.</li>
        <li>Give more helpful error in case of Pulse servers asking for TNCC.</li>
+       <li>Sanitize non-canonical Legacy IP network addresses (<a href="https://gitlab.com/openconnect/openconnect/merge_requests/97">!97</a>)</li>
      </ul><br/>
   </li>
   <li><b><a href="ftp://ftp.infradead.org/pub/openconnect/openconnect-8.08.tar.gz">OpenConnect v8.08</a></b>