Skip to content

Commit 635463b

Browse files
committed
Handle reason='attempt-reconnect' on Linux, and stub for it on macOS/*BSD
The case where the "real" network device disappears or disconnects, while the VPN/tunnel device stays up is a surprisingly complex one to handle correctly. The main issue is that the "explicit route" to the VPN gateway on the underlying network device may have disappeared, and the OS routing utilities may erroneously suggest a looped-back route (over the VPN/tunnel device) as the optimal route to the gateway. This issue arises particularly — but NOT exclusively — if the *default* route for the relevant network family is now assigned to the VPN/tunnel device. See: - https://gitlab.com/openconnect/openconnect/issues/17 for the initial report of this problem, - https://gitlab.com/openconnect/openconnect/-/commit/c2755eefb4e00e915c330495b33d3f5db926615b for where the vpnc-script call with reason='attempt-reconnect' was added to OpenConnect (merged in v8.02) - https://gitlab.com/openconnect/vpnc-scripts/-/commit/1000e0f6dd7d6bff163169a46359211c1fc3a6d2 for where an initial placeholder was first added to vpnc-script - https://gitlab.com/openconnect/vpnc-scripts/-/merge_requests/14 for the first actually-working support in vpnc-script (for Linux)m - and numerous subsequent changes to handle macOS/*BSD, IPv6, and corner cases We need to handle attempt-reconnect in vpn-slice. This mostly borrows from what vpnc-script does. Still TODO: - Flesh out the macOS/*BSD implementation. Instead of 'route -n get', we should use 'netstat -r -n' to ensure that we don't get a looped-back route, as vpnc-script does since https://gitlab.com/openconnect/vpnc-scripts/-/blob/412a1faffa72fcda54e8c42d22e0057e56240ff1/vpnc-script#L395-402 - Linux: preserve the 'onlink' route flag. This requires replacing 'ip route get' with 'ip route show' in all cases. See https://gitlab.com/openconnect/vpnc-scripts/-/merge_requests/27 Here is an example of OpenConnect + vpn-slice correctly re-establishing the route to the VPN gateway even after it's removed by a network outage, leaving the default route looped-back through the VPN/tunnel interface. $ openconnect --script 'vpn-slice --dump -vvv 0.0.0.0/0' ... ... <authenticate successfully> ... Called by /usr/sbin/openconnect (PID 123456) with environment variables for vpnc-script: reason => reason=<reasons.connect: 2> VPNGATEWAY => gateway=IPv4Address('1.2.3.4') TUNDEV => tundev='tun0' .... Complete set of subnets to include in VPN routes: 0.0.0.0/0 Set explicit route to VPN gateway 1.2.3.4 (via 10.224.0.1, dev wlan0, src 10.224.0.123) Blocked incoming traffic from VPN interface with iptables. Adding route to nameserver 8.8.8.8 through tun0. Adding route to nameserver 1.1.1.1 through tun0. Adding route to subnet 0.0.0.0/0 through tun0. # <---- Added routes for 2 nameservers, 1 subnets, 0 aliases. ... ... <we have successfully connected> ... ... <disconnect and reconnect from WiFi, so that explicit route to gateway is lost> ... ... <OpenConnect dead peer detection kicks in> ... Failed to reconnect to host vpn.company.com: Interrupted system call sleep 10s, remaining timeout 300s ... Called by /usr/sbin/openconnect (PID 551671) with environment variables for vpnc-script: reason => reason=<reasons.attempt_reconnect: 5> VPNGATEWAY => gateway=IPv4Address('1.2.3.4') TUNDEV => tundev='tun0' ... Complete set of subnets to include in VPN routes: 0.0.0.0/0 Reset explicit route to VPN gateway 1.2.3.4 (via 10.224.0.1, dev wlan0, metric 600) # <--- SSL negotiation with vpn.company.com Connected to HTTPS on vpn.company.com with ciphersuite (TLS1.2)-(ECDHE-SECP256R1)-(RSA-SHA512)-(AES-256-GCM)
1 parent ba76f82 commit 635463b

5 files changed

Lines changed: 106 additions & 18 deletions

File tree

vpn_slice/__main__.py

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,32 @@ def do_disconnect(env, args):
154154
print("WARNING: failed to deconfigure domains vpn dns", file=stderr)
155155

156156

157+
def do_attempt_reconnect(env, args):
158+
# set explicit route to gateway, avoiding loopback through tunnel
159+
if not env.gateway:
160+
print("WARNING: No gateway address.", file=stderr)
161+
elif env.gateway.is_loopback:
162+
print("WARNING: Gateway address is loopback (%s); probably a local proxy.", file=stderr)
163+
else:
164+
# avoid loopback routes through tunnel
165+
gwrs = [gwr for gwr in providers.route.get_all_routes(env.gateway) if gwr.get('dev') != env.tundev]
166+
if not gwrs:
167+
print("WARNING: no routes to VPN gateway found %s; cannot set explicit route to it." % env.gateway)
168+
return
169+
170+
for gwr in gwrs:
171+
try:
172+
# We do not want to use 'replace', since a route to the gateway that already
173+
# exists is mostly likely the correct one (e.g. the case of a reconnect attempt
174+
# after dead-peer detection, but no change in the underlying network devices).
175+
providers.route.add_route(env.gateway, **gwr)
176+
except CalledProcessError:
177+
pass
178+
else:
179+
if args.verbose > 1:
180+
print("Reset explicit route to VPN gateway %s (%s)" % (env.gateway, ', '.join('%s %s' % kv for kv in gwr.items())), file=stderr)
181+
182+
157183
def do_connect(env, args):
158184
global providers
159185
if args.banner and env.banner:
@@ -594,8 +620,10 @@ def main(args=None, environ=os.environ):
594620
do_pre_init(env, args)
595621
elif env.reason == reasons.disconnect:
596622
do_disconnect(env, args)
597-
elif env.reason in (reasons.reconnect, reasons.attempt_reconnect):
598-
# FIXME: is there anything that reconnect or attempt_reconnect /should/ do
623+
elif env.reason == reasons.attempt_reconnect:
624+
do_attempt_reconnect(env, args)
625+
elif env.reason == reasons.reconnect:
626+
# FIXME: is there anything that reconnect /should/ do
599627
# on a modern system (Linux) which automatically removes routes to
600628
# a tunnel adapter that has been removed? I am not clear on whether
601629
# any other behavior is potentially useful.

vpn_slice/linux.py

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from .posix import PosixProcessProvider
66
from .provider import FirewallProvider, RouteProvider, TunnelPrepProvider
7-
from .util import get_executable
7+
from .util import MAX_UINT32, get_executable
88

99

1010
class ProcfsProvider(PosixProcessProvider):
@@ -33,27 +33,50 @@ def _iproute(self, *args, **kwargs):
3333
for k, v in kwargs.items():
3434
if v is not None:
3535
cl.extend((k, str(v)))
36+
skip = next((ii for ii, v in enumerate(args) if not v.startswith('-')), len(args))
37+
args = args[skip:]
3638

39+
output_start = multi = route_junk = None
3740
if args[:2] == ('route', 'get'):
38-
output_start, keys = 1, ('via', 'dev', 'src', 'mtu')
41+
output_start, keys = 1, ('via', 'dev', 'src', 'mtu', 'metric')
42+
route_junk = True
43+
elif args[:2] == ('route', 'show'):
44+
output_start, keys = 1, ('via', 'dev', 'src', 'mtu', 'metric')
45+
multi = route_junk = True
3946
elif args[:2] == ('link', 'show'):
4047
output_start, keys = 3, ('state', 'mtu')
41-
else:
42-
output_start = None
48+
49+
# FIXME/rant: parsing the output of 'ip route {get,show}' is a bit of a nightmare.
50+
# Per https://linux.die.net/man/8/ip, the output ...
51+
# 1) Can include 'mtu' followed by either 1 parameter ('mtu X') or two ('mtu lock X')
52+
# 2) Can include 'onlink' followed by 0 parameters, but only in 'ip show' output
53+
# (https://gitlab.com/openconnect/vpnc-scripts/-/issues/20#note_542783676)
54+
#
55+
# We get both of the above cases wrong currently.
56+
# Possibly it'd be saner/stabler to parse /proc/net/route.
4357

4458
if output_start is not None:
45-
words = subprocess.check_output(cl, universal_newlines=True).split()
46-
if args[:2] == ('route', 'get') and words[0] in ('broadcast', 'multicast', 'local', 'unreachable'):
47-
output_start += 1
48-
return {words[i]: words[i + 1] for i in range(output_start, len(words), 2) if words[i] in keys}
59+
if not multi:
60+
words = subprocess.check_output(cl, universal_newlines=True).split()
61+
if route_junk and words[0] in ('broadcast', 'multicast', 'local', 'unreachable'):
62+
output_start += 1
63+
results = {words[i]: words[i + 1] for i in range(output_start, len(words), 2) if words[i] in keys}
64+
else:
65+
results = []
66+
for line in subprocess.check_output(cl, universal_newlines=True).splitlines():
67+
words = line.split()
68+
if route_junk and words[0] in ('broadcast', 'multicast', 'local', 'unreachable'):
69+
output_start += 1
70+
results.append({words[i]: words[i + 1] for i in range(output_start, len(words), 2) if words[i] in keys})
71+
return results
4972
else:
5073
subprocess.check_call(cl)
5174

52-
def add_route(self, destination, *, via=None, dev=None, src=None, mtu=None):
53-
self._iproute('route', 'add', destination, via=via, dev=dev, src=src, mtu=mtu)
75+
def add_route(self, destination, *, via=None, dev=None, src=None, mtu=None, metric=None):
76+
self._iproute('route', 'add', destination, via=via, dev=dev, src=src, mtu=mtu, metric=metric)
5477

55-
def replace_route(self, destination, *, via=None, dev=None, src=None, mtu=None):
56-
self._iproute('route', 'replace', destination, via=via, dev=dev, src=src, mtu=mtu)
78+
def replace_route(self, destination, *, via=None, dev=None, src=None, mtu=None, metric=None):
79+
self._iproute('route', 'replace', destination, via=via, dev=dev, src=src, mtu=mtu, metric=metric)
5780

5881
def remove_route(self, destination):
5982
self._iproute('route', 'del', destination)
@@ -66,7 +89,17 @@ def get_route(self, destination):
6689
if 'dev' in r or 'via' in r:
6790
return r
6891

92+
def get_all_routes(self, destination):
93+
if destination.version == 4:
94+
flag, root = '-4', '0/0'
95+
else:
96+
flag, root = '-6', '::/0'
97+
# Ignore localhost or incomplete routes
98+
rs = [r for r in self._iproute(flag, 'route', 'show', 'to', destination, 'root', root) if r.get('dev') != 'lo' and ('dev' in r or 'via' in r)]
99+
return sorted(rs, key=lambda r: int(r.get('metric', MAX_UINT32)))
100+
69101
def flush_cache(self):
102+
# IPv4 route cache is obsolete as of Linux 3.6 (https://gitlab.com/openconnect/vpnc-scripts/-/merge_requests/30)
70103
self._iproute('route', 'flush', 'cache')
71104
self._iproute('-6', 'route', 'flush', 'cache')
72105

vpn_slice/mac.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def _ifconfig(self, *args):
4343
def _family_option(self, destination):
4444
return '-inet6' if destination.version == 6 else '-inet'
4545

46-
def add_route(self, destination, *, via=None, dev=None, src=None, mtu=None):
46+
def add_route(self, destination, *, via=None, dev=None, src=None, mtu=None, metric=None):
4747
args = ['add', self._family_option(destination)]
4848
if mtu is not None:
4949
args.extend(('-mtu', str(mtu)))
@@ -78,6 +78,13 @@ def get_route(self, destination):
7878
'mtu': info_d.get('mtu', None),
7979
}
8080

81+
def get_all_routes(self, destination):
82+
# FIXME: Use netstat to ensure that we don't get a loopback route via the VPN interface itself,
83+
# like vpnc-script now does.
84+
# https://gitlab.com/openconnect/vpnc-scripts/-/blob/412a1faffa72fcda54e8c42d22e0057e56240ff1/vpnc-script#L395-402
85+
route = self.get_route(destination)
86+
return [route] if route else []
87+
8188
def flush_cache(self):
8289
pass
8390

vpn_slice/provider.py

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def kill(self, pid):
2323

2424
class RouteProvider(metaclass=ABCMeta):
2525
@abstractmethod
26-
def add_route(self, destination, *, via=None, dev=None, src=None, mtu=None):
26+
def add_route(self, destination, *, via=None, dev=None, src=None, mtu=None, metric=None):
2727
"""Add a route to a destination.
2828
2929
You must specify a device or gateway saying where to route to.
@@ -35,7 +35,7 @@ def add_route(self, destination, *, via=None, dev=None, src=None, mtu=None):
3535
"""
3636

3737
@abstractmethod
38-
def replace_route(self, destination, *, via=None, dev=None, src=None, mtu=None):
38+
def replace_route(self, destination, *, via=None, dev=None, src=None, mtu=None, metric=None):
3939
"""Add or replace a route to a destination.
4040
4141
You must specify a device or gateway saying where to route to.
@@ -52,7 +52,8 @@ def remove_route(self, destination):
5252

5353
@abstractmethod
5454
def get_route(self, destination):
55-
"""Return the gateway to a destination.
55+
"""Return one route to a destination (what the OS considers the
56+
optimal or most specific route).
5657
5758
Return a dict with these keys containing the information,
5859
or None if it is unavailable:
@@ -61,6 +62,23 @@ def get_route(self, destination):
6162
* dev
6263
* src
6364
* mtu
65+
* metric
66+
67+
"""
68+
69+
@abstractmethod
70+
def get_all_routes(self, destination):
71+
"""Return all routes to a destination, sorted in order of decreasing
72+
optimality or specificity.
73+
74+
Return a list of dict with these keys containing the
75+
information:
76+
77+
* via
78+
* dev
79+
* src
80+
* mtu
81+
* metric
6482
6583
"""
6684

vpn_slice/util.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
import os.path
33
from shutil import which
44

5+
MAX_UINT32 = 4294967295
6+
57

68
def get_executable(path):
79
path = which(os.path.basename(path)) or path

0 commit comments

Comments
 (0)