Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
dlenski authored and dwmw2 committed Apr 28, 2020
1 parent c3e77c6 commit e49e49e
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 18 deletions.
77 changes: 59 additions & 18 deletions script.c
Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions www/changelog.xml
Expand Up @@ -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>
Expand Down

0 comments on commit e49e49e

Please sign in to comment.