From e49e49e3314b00bdc2eb42477812312ef99b9d42 Mon Sep 17 00:00:00 2001 From: Daniel Lenski Date: Thu, 23 Apr 2020 10:30:40 -0700 Subject: [PATCH] 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 --- script.c | 77 ++++++++++++++++++++++++++++++++++++----------- www/changelog.xml | 1 + 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/script.c b/script.c index 50407452..9115e567 100644 --- 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 ba0c2b53..3475ac64 100644 --- a/www/changelog.xml +++ b/www/changelog.xml @@ -17,6 +17,7 @@
  • OpenConnect v8.08 -- 2.50.1