~mamarley/openconnect/+git/gitlab-main:IP_and_routing_configuration_simplification

Last commit made on 2021-06-17
Get this branch:
git clone -b IP_and_routing_configuration_simplification https://git.launchpad.net/~mamarley/openconnect/+git/gitlab-main

Branch merges

Branch information

Name:
IP_and_routing_configuration_simplification
Repository:
lp:~mamarley/openconnect/+git/gitlab-main

Recent commits

d7ea375... by Dan Lenski

CSTP: Always set Legacy IP netmask to 0.0.0.0 unless split-includes received

As described in the previous patch, we think that Cisco servers expect us to
set a default Legacy IP route if no split-includes are sent.

However, Cisco servers may also send another "main" Legacy IP route via
X-CSTP-Netmask. As already done for GP, oNCP, and Pulse, we should shunt
that other route to a split-include (unless it's a /32 route, effectively a
no-op).

Once this is merged, we'll be able to remove from vpnc-script the following
Cisco-specific assumption: that a lack of split-includes means we must set
a default Legacy IP route.

Signed-off-by: Daniel Lenski <email address hidden>

3648e2e... by Dan Lenski

Ensure that tunnel configuration script always receives Legacy IP address and netaddr/netmask, or neither

The logic for handling Legacy IP configuration in vpnc-script is very
complex, confusing, and somewhat redundant (see
https://gitlab.com/openconnect/vpnc-scripts/blob/800f671b/vpnc-script):

1. Set a default Legacy IP route on the tunnel interface *unless*
   split-include routes are provided.
2. Set a route to INTERNAL_IP4_NETADDR/INTERNAL_IP4_NETMASK, if provided
   (redundant unless split-include routes provided).
3. Set split-include and split-exclude routes unconditionally, if provided.

This behavior is based on what Cisco servers send; in particular, they
expect us to set a default Legacy IP route if no split-includes are sent.

This patch ensures that if we send a Legacy IP address to the script, we
will *always* also send netaddr/netmask, and vice versa.

Once merged, this will allow us to simplify vpnc-script and make it less
dependent on the historical Cisco-specific behavior (albeit in a way that is
not backwards-compatible with older verisons of OpenConnect).

Signed-off-by: Daniel Lenski <email address hidden>

c6f98c3... by Dan Lenski

Ensure that tunnel configuration script always receives both IPv6 address and netmask, or neither

The logic for handling IPv6 address configuration in vpnc-script is complex
(https://gitlab.com/openconnect/vpnc-scripts/blob/800f671b/vpnc-script#L228-245),
because the script cannot rely on *both* 'INTERNAL_IP6_ADDRESS' *and*
'INTERNAL_IP6_NETMASK' being set. That is because these variables were
historically set based on the behavior of Cisco servers
(https://gitlab.com/openconnect/openconnect/blob/3d845bc9/cstp.c#L560-567),
which are not guaranteed to send both.

This patch ensures that if we send *one* of these variables to the script,
we will always send *both*, continuing the changes begun in
https://gitlab.com/openconnect/openconnect/-/commit/3d845bc9bf62b3e816e0d2fd72970ef33964a191#e1f68b30df0b044d45c9bd36c0856d78f9682f04:

1. If only the IPv6 netmask is set by the server, we remove the /prefixlen
   to make the IPv6 address.
2. If only the IPv6 address is set by the server, we append /128 to make
   the IPv6 netmask.

Once merged, this will allow us to simplify vpnc-script and make it less
dependent on the historical Cisco-specific behavior (albeit in a way that is
not backwards-compatible with older verisons of OpenConnect).

Signed-off-by: Daniel Lenski <email address hidden>

98a0d6d... by Dan Lenski

Update changelog

Signed-off-by: Daniel Lenski <email address hidden>

98043f5... by Dan Lenski

Handle default Legacy IP route for Pulse as for GP and oNCP

If a default Legacy IP route is specified as a "split"-include route, then
we should use that route as the Legacy IP netmask, and add the original
netmask as a split (if it's anything other than 255.255.255.255 or /32).

Signed-off-by: Daniel Lenski <email address hidden>

041c2b7... by Dan Lenski

Handle default Legacy IP route for oNCP as we do for GP

If a default Legacy IP route is specified as a "split"-include route, then
we should use that route as the Legacy IP netmask, and add the original
netmask as a split (if it's anything other than 255.255.255.255 or /32).

See https://gitlab.com/openconnect/openconnect/-/merge_requests/118 for the
rationale.

This patch factors out the function finalize_netmask_fixing_default_route_as_split()
from gpst.c and makes it a global internal function, so that the mechanism
can be shared.

Signed-off-by: Daniel Lenski <email address hidden>

5e9f85a... by Dan Lenski

Do not ignore 0.0.0.0/0 specified as a "split"-{in,ex}clude route for oNCP

This addresses https://gitlab.com/openconnect/openconnect/-/issues/245. In the case
presented there, the oNCP server sends a Legacy IP netmask ("default route") of
255.255.255.255, and a "split"-include route of 0.0.0.0/0.0.0.0:

> Received split include route 0.0.0.0/0.0.0.0
> Received netmask 255.255.255.255

We also should not ignore 0.0.0.0/0 if specified as a "split"-exclude route, though
the purpose of such a route is unclear and we have never seen one in the wild.

Next, we should handle this case in the same way that we do for GlobalProtect,
as of https://gitlab.com/openconnect/openconnect/-/merge_requests/118; namely,
by replacing the 255.255.255.255 netmask with the 0.0.0.0/0 send as a "split"-include,
and removing the latter from the list of split-includes.

Signed-off-by: Daniel Lenski <email address hidden>

0cfdb7b... by Tom Carroll <email address hidden>

Use C99 initializer instead of memset.

Strictly speaking, using memset() here violates strict aliasing rules,
and it would be entirely permissible for an assert() like this example
to *fail*:

 struct gtls_cert_info gci;

 memset(&gci, 0, sizeof gci);
 assert(gci.pkey == NULL);

Signed-off-by: Tom Carroll <email address hidden>

63c984b... by dwmw2

Add more buf_append_base64() tests... and fix it.

Signed-off-by: David Woodhouse <email address hidden>

3b88d27... by dwmw2

Fix first line length in buf_append_base64()

We need to add four (for the characters we're about to append) *after*
checking against line_len and resetting ll to zero. Otherwise the first
line thinks it starts at 4 while the others start at 0.

Signed-off-by: David Woodhouse <email address hidden>