Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Windows support #102

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
setproctitle
dnspython
portalocker
79 changes: 65 additions & 14 deletions vpn_slice/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ def get_default_providers():
dns = DNSPythonProvider or DigProvider,
hosts = PosixHostsFileProvider,
)
elif platform.startswith('win32'):
from .win import WinProcessProvider, WinHostsFileProvider, WinRouteProvider, WinNrptSplitDNSProvider, WinNrptProvider
from .dnspython import DNSPythonProvider
return dict(
process = WinProcessProvider,
route = WinRouteProvider,
dns = DNSPythonProvider,
hosts = WinHostsFileProvider,
domain_vpn_dns=WinNrptSplitDNSProvider,
nrpt = WinNrptProvider,
)
else:
return dict(
platform = OSError('Your platform, {}, is unsupported'.format(platform))
Expand Down Expand Up @@ -134,7 +145,13 @@ def do_disconnect(env, args):

# delete explicit route to gateway
try:
providers.route.remove_route(env.gateway)
# the following commented out code can be used to narrow-down the route removal, if needed
#gwr = providers.route.get_route(env.gateway)
providers.route.remove_route( \
env.gateway, \
#dev=(gwr["dev"] if platform.startswith("win32") else None), \
#via=(gwr["via"] if platform.startswith("win32") else None), \
)
except CalledProcessError:
print("WARNING: could not delete route to VPN gateway (%s)" % env.gateway, file=stderr)

Expand All @@ -151,9 +168,13 @@ def do_disconnect(env, args):
except OSError:
print("WARNING: failed to deconfigure domains vpn dns", file=stderr)

# unset NRPT DNS rules (Windows only)
for (domain, servers) in args.nrpt.items():
providers.nrpt.remove_nrtp(domain, servers)

def do_connect(env, args):
global providers
global platform
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this. platform is already a global, imported with from sys import platform

if args.banner and env.banner:
print("Connect Banner:")
for l in env.banner.splitlines(): print("| "+l)
Expand Down Expand Up @@ -201,6 +222,10 @@ def do_connect(env, args):
print("WARNING: guessing default MTU of %d (couldn't determine MTU of %s)" % (mtu, dev), file=stderr)
providers.route.set_link_info(env.tundev, state='up', mtu=mtu)

# erase all addresses (needed on Windows to clean-up the adapter)
# TODO: also cleanup DNS?
# TODO: also cleanup routes?
providers.route.remove_address(env.tundev)
Comment on lines +225 to +228
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of removing the preexisting addresses from the VPN adapter when connecting?

From OpenConnect development, we already know that if another (down) adapter is using the same address… we have to clean it up before we can assign it to the VPN adapter. In v9.0, we'll do this automatically: https://gitlab.com/openconnect/openconnect/-/blob/master/tun-win32.c#L228

But I don't know what issue is being solved by removing the previously assigned addresses when connecting. 🤷‍♂️

# set IPv4, IPv6 addresses for tunnel device
if env.myaddr:
providers.route.add_address(env.tundev, env.myaddr)
Expand All @@ -221,7 +246,9 @@ def do_connect(env, args):
for dest, tag in chain(tagged(ns, "nameserver"), tagged(args.subnets, "subnet"), tagged(args.aliases, "alias")):
if args.verbose > 1:
print("Adding route to %s %s through %s." % (tag, dest, env.tundev), file=stderr)
providers.route.replace_route(dest, dev=env.tundev)
providers.route.replace_route(dest, dev=env.tundev, \
# via=(env.via if platform.startswith("win32") else None) \
)
else:
providers.route.flush_cache()
if args.verbose:
Expand All @@ -245,8 +272,13 @@ def do_connect(env, args):
providers.domain_vpn_dns.configure_domain_vpn_dns(args.vpn_domains, env.dns)


# set NRPT DNS rules (Windows only)
for (domain, servers) in args.nrpt.items():
providers.nrpt.add_nrtp(domain, servers)

def do_post_connect(env, args):
global providers
global platform
# lookup named hosts for which we need routes and/or host_map entries
# (the DNS/NBNS servers already have their routes)
ip_routes = set()
Expand Down Expand Up @@ -295,7 +327,7 @@ def do_post_connect(env, args):
for ip in ip_routes:
if args.verbose > 1:
print("Adding route to %s (for named hosts) through %s." % (ip, env.tundev), file=stderr)
providers.route.replace_route(ip, dev=env.tundev)
providers.route.replace_route(ip, dev=env.tundev, via=(env.via if platform.startswith("win32") else None))
else:
providers.route.flush_cache()
if args.verbose:
Expand Down Expand Up @@ -387,6 +419,8 @@ def parse_env(environ=os.environ):
raise AssertionError("IPv4 network (INTERNAL_IP4_{{NETADDR,NETMASK}}) {ad}/{nm} does not match INTERNAL_IP4_NETMASKLEN={nml} (implies /{nmi})".format(
ad=orig_netaddr, nm=env.netmask, nml=env.netmasklen, nmi=env.network.netmask))
assert env.network.netmask == env.netmask
# first/lowest IP-addr in the range should be internal GW (seen in Windows vpnc-script.js)
env.via = next(env.network.hosts())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://gitlab.com/openconnect/vpnc-scripts/-/commit/b3dec790951444d5e5f5c6659ca8b51569207b03 🤔

  1. This won't work if env.network is /32. n=ipaddress.ip_network('192.168.12.25/32'); next(n.hosts())StopIteration.
  2. If env.network isn't /32, then in order to match vpnc-script-win.js, it should be the second host IP address, not the first.

How about this?

from itertools import islice

lowest_hosts = islice(env.network.hosts(), 0, 2)
env.via = lowest_hosts[1] if len(lowest_hosts) == 2 else env.network.network_address


# Need to match behavior of original vpnc-script here
# Examples:
Expand Down Expand Up @@ -452,6 +486,8 @@ def parse_args_and_env(args=None, environ=os.environ):
g.add_argument('--no-ns-hosts', action='store_false', dest='ns_hosts', default=True, help='Do not add nameserver aliases to /etc/hosts (default is to name them dns0.tun0, etc.)')
g.add_argument('--nbns', action='store_true', dest='nbns', help='Include NBNS (Windows/NetBIOS nameservers) as well as DNS nameservers')
g.add_argument('--domains-vpn-dns', dest='vpn_domains', default=None, help="comma separated domains to query with vpn dns")
if platform.startswith('win32'):
g.add_argument('--nrpt', default=[], action='append', help='NRPT DNS mapping in form .sub.domain.org=8.8.8.8,4.4.4.4')
g = p.add_argument_group('Debugging options')
g.add_argument('--self-test', action='store_true', help='Stop after verifying that environment variables and providers are configured properly.')
g.add_argument('-v', '--verbose', default=0, action='count', help="Explain what %(prog)s is doing. Specify repeatedly to increase the level of detail.")
Expand Down Expand Up @@ -511,21 +547,36 @@ def finalize_args_and_env(args, env):
args.ppid = providers.process.ppid_of(args.ppid)


def main(args=None, environ=os.environ):
if args.nrpt:
args_nrpt = args.nrpt
args.nrpt = {}
for nrpt in args_nrpt:
dom, dns = str.split(nrpt,"=", 1);
args.nrpt[dom] = str.split(dns, ",");

def initGlobalProviders():
global providers

if "provider" in globals():
return

# Set platform-specific providers
providers = slurpy()
for pn, pv in get_default_providers().items():
try:
if isinstance(pv, Exception):
raise pv
providers[pn] = pv()
except Exception as e:
print("WARNING: Couldn't configure {} provider: {}".format(pn, e), file=stderr)

def main(args=None, environ=os.environ):
global providers

try:
p, args, env = parse_args_and_env(args, environ)

# Set platform-specific providers
providers = slurpy()
for pn, pv in get_default_providers().items():
try:
if isinstance(pv, Exception):
raise pv
providers[pn] = pv()
except Exception as e:
print("WARNING: Couldn't configure {} provider: {}".format(pn, e), file=stderr)

Comment on lines +557 to +578
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this is being changed.

initGlobalProviders()

# Fail if necessary providers are missing
required = {'route', 'process'}
Expand Down
4 changes: 2 additions & 2 deletions vpn_slice/freebsd.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import os

from .posix import PosixProcessProvider
from .portable import PythonOsProcessProvider


class ProcfsProvider(PosixProcessProvider):
class ProcfsProvider(PythonOsProcessProvider):
def pid2exe(self, pid):
try:
return os.readlink('/proc/%d/file' % pid)
Expand Down
4 changes: 2 additions & 2 deletions vpn_slice/linux.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
import stat
import subprocess

from .posix import PosixProcessProvider
from .portable import PythonOsProcessProvider
from .provider import FirewallProvider, RouteProvider, TunnelPrepProvider
from .util import get_executable


class ProcfsProvider(PosixProcessProvider):
class ProcfsProvider(PythonOsProcessProvider):
def pid2exe(self, pid):
try:
return os.readlink('/proc/%d/exe' % pid)
Expand Down
4 changes: 2 additions & 2 deletions vpn_slice/mac.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
import subprocess
from ipaddress import ip_interface

from .posix import PosixProcessProvider
from .portable import PythonOsProcessProvider
from .provider import FirewallProvider, RouteProvider, SplitDNSProvider
from .util import get_executable


class PsProvider(PosixProcessProvider):
class PsProvider(PythonOsProcessProvider):
def __init__(self):
self.lsof = get_executable('/usr/sbin/lsof')
self.ps = get_executable('/bin/ps')
Expand Down
19 changes: 19 additions & 0 deletions vpn_slice/portable.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import os
from signal import SIGTERM

from .provider import ProcessProvider


class PythonOsProcessProvider(ProcessProvider):
def kill(self, pid, signal=SIGTERM):
os.kill(pid, signal)

def pid(self):
return os.getpid()

def is_alive(self, pid):
try:
os.kill(pid, 0)
return True
except ProcessLookupError:
return False
Comment on lines +7 to +19
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about put this in generic.py, rather than creating a new portable.py? (And let's just call it OsProcessProvider rather than add the seemingly-redundant Python to the name.)

27 changes: 10 additions & 17 deletions vpn_slice/posix.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fcntl
import os
import subprocess
from abc import abstractmethod
from ipaddress import ip_address
from signal import SIGTERM

Expand Down Expand Up @@ -72,10 +72,14 @@ def __init__(self, path):
if not os.access(path, os.R_OK | os.W_OK):
raise OSError('Cannot read/write {}'.format(path))

@abstractmethod
def lock_hosts_file(self, hostf):
"""Lock the hosts file."""
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this method abstract, rather than using the POSIX implementation (fcntl.flock(hostf, fctnl.LOCK_EX)?


def write_hosts(self, host_map, name):
tag = 'vpn-slice-{} AUTOCREATED'.format(name)
with open(self.path, 'r+') as hostf:
fcntl.flock(hostf, fcntl.LOCK_EX) # POSIX only, obviously
self.lock_hosts_file(hostf)
lines = hostf.readlines()
keeplines = [l for l in lines if not l.endswith('# %s\n' % tag)]
hostf.seek(0, 0)
Expand All @@ -89,18 +93,7 @@ def write_hosts(self, host_map, name):
class PosixHostsFileProvider(HostsFileProvider):
def __init__(self):
super().__init__('/etc/hosts')


class PosixProcessProvider(ProcessProvider):
def kill(self, pid, signal=SIGTERM):
os.kill(pid, signal)

def pid(self):
return os.getpid()

def is_alive(self, pid):
try:
os.kill(pid, 0)
return True
except ProcessLookupError:
return False

def lock_hosts_file(self, hostf):
import fcntl
fcntl.flock(hostf, fcntl.LOCK_EX) # POSIX only, obviously
Loading