From 91b001b65f6974897fb0d7fd13991facd8227c47 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 15 Jun 2017 09:24:03 -0700 Subject: [feat] fix OpenVPN start/stop in OSX using a process canary - correctly start the openvpn process canary - use helper to fix tearing down of the vpn --- src/leap/bitmask/vpn/_control.py | 52 +++++++++++++++++--------------- src/leap/bitmask/vpn/_management.py | 12 +++++--- src/leap/bitmask/vpn/fw/firewall.py | 8 +++-- src/leap/bitmask/vpn/launchers/darwin.py | 1 - src/leap/bitmask/vpn/process.py | 27 +++++++++++------ src/leap/bitmask/vpn/tunnel.py | 18 ++++++----- src/leap/bitmask/vpn/tunnelmanager.py | 2 +- 7 files changed, 69 insertions(+), 51 deletions(-) (limited to 'src') diff --git a/src/leap/bitmask/vpn/_control.py b/src/leap/bitmask/vpn/_control.py index 53d4d31..f20184f 100644 --- a/src/leap/bitmask/vpn/_control.py +++ b/src/leap/bitmask/vpn/_control.py @@ -10,10 +10,7 @@ from .constants import IS_MAC log = Logger() -# NOTE: We need to set a bigger poll time in OSX because it seems -# openvpn malfunctions when you ask it a lot of things in a short -# amount of time. Check this! -POLL_TIME = 2.5 if IS_MAC else 1.0 +POLL_TIME = 1 class VPNControl(object): @@ -25,6 +22,9 @@ class VPNControl(object): suited for the running platform and connect to the management interface opened by the openvpn process, executing commands over that interface on demand. + + This class also has knowledge of the reactor, since it controlls the + pollers that write and read to the management interface. """ TERMINATE_MAXTRIES = 10 TERMINATE_WAIT = 1 # secs @@ -84,7 +84,8 @@ class VPNControl(object): # generic watchers poll_list = [LoopingCall(vpnproc.pollStatus), - LoopingCall(vpnproc.pollState)] + LoopingCall(vpnproc.pollState), + LoopingCall(vpnproc.pollLog)] self._pollers.extend(poll_list) self._start_pollers() return True @@ -107,26 +108,29 @@ class VPNControl(object): :type restart: bool """ self._stop_pollers() + try: + self._vpnproc.preDown() + except Exception as e: + log.error('Error on vpn pre-down {0!r}'.format(e)) + raise - # First we try to be polite and send a SIGTERM... - if self._vpnproc is not None: - # We assume that the only valid stops are initiated - # by an user action, not hard restarts - self._user_stopped = not restart - self._vpnproc.restarting = restart - - self._sentterm = True - self._vpnproc.terminate_openvpn(shutdown=shutdown) - - # ...but we also trigger a countdown to be unpolite - # if strictly needed. - reactor.callLater( - self.TERMINATE_WAIT, self._kill_if_left_alive) - else: - log.debug('VPN is not running.') - - self._vpnproc.traffic_status = (0, 0) - + if IS_LINUX: + # TODO factor this out to a linux-only launcher mechanism. + # First we try to be polite and send a SIGTERM... + if self._vpnproc is not None: + # We assume that the only valid stops are initiated + # by an user action, not hard restarts + self._user_stopped = not restart + self._vpnproc.restarting = restart + + self._sentterm = True + self._vpnproc.terminate(shutdown=shutdown) + + # ...but we also trigger a countdown to be unpolite + # if strictly needed. + reactor.callLater( + self.TERMINATE_WAIT, self._kill_if_left_alive) + self._vpnproc.traffic_status = (0, 0) return True @property diff --git a/src/leap/bitmask/vpn/_management.py b/src/leap/bitmask/vpn/_management.py index 26050be..80a82c9 100644 --- a/src/leap/bitmask/vpn/_management.py +++ b/src/leap/bitmask/vpn/_management.py @@ -79,8 +79,6 @@ class VPNManagement(object): if not self._host or not self._port: raise ImproperlyConfigured('Connection is not configured') - if self.is_connected(): - self._close_management_socket() try: self._tn = UDSTelnet(self._host, self._port) self._tn.read_eager() @@ -117,7 +115,7 @@ class VPNManagement(object): self.connect_retry, retry + 1) def process_log(self): - if not self._watcher: + if not self._watcher or not self._tn: return lines = self._send_command('log 20') @@ -203,6 +201,7 @@ class VPNManagement(object): :type output: list """ for line in output: + status_step = '' stripped = line.strip() if stripped == "END": continue @@ -212,7 +211,10 @@ class VPNManagement(object): try: ts, status_step, ok, ip, remote, port, _, _, _ = parts except ValueError: - ts, status_step, ok, ip, remote, port, _, _ = parts + try: + ts, status_step, ok, ip, remote, port, _, _ = parts + except ValueError: + self.log.debug('Could not parse %s' % parts) state = status_step if state != self._last_state: @@ -280,7 +282,7 @@ class VPNManagement(object): if self.is_connected(): return self._parse_status_and_notify(self._send_command("status")) - def terminate_openvpn(self, shutdown=False): + def terminate(self, shutdown=False): """ Attempts to terminate openvpn by sending a SIGTERM. """ diff --git a/src/leap/bitmask/vpn/fw/firewall.py b/src/leap/bitmask/vpn/fw/firewall.py index dcd956d..7147971 100644 --- a/src/leap/bitmask/vpn/fw/firewall.py +++ b/src/leap/bitmask/vpn/fw/firewall.py @@ -48,27 +48,31 @@ class _OSXFirewallManager(object): def __init__(self, remotes): self._remotes = list(remotes) self._helper = darwin.HelperCommand() + self._started = False def start(self, restart=False): gateways = [gateway for gateway, port in self._remotes] cmd = 'firewall_start %s' % (' '.join(gateways),) self._helper.send(cmd) + self._started = True # TODO parse OK from result return True def stop(self): cmd = 'firewall_stop' self._helper.send(cmd) + self._started = False return True def is_up(self): # TODO implement!!! - return True + return self._started @property def status(self): # TODO implement!!! -- factor out, too - return {'status': 'on', 'error': None} + status = 'on' if self._started else 'off' + return {'status': status, 'error': None} class _LinuxFirewallManager(object): diff --git a/src/leap/bitmask/vpn/launchers/darwin.py b/src/leap/bitmask/vpn/launchers/darwin.py index d83ede2..440ed59 100644 --- a/src/leap/bitmask/vpn/launchers/darwin.py +++ b/src/leap/bitmask/vpn/launchers/darwin.py @@ -49,7 +49,6 @@ class HelperCommand(object): raise RuntimeError(msg) def send(self, cmd, args=''): - # TODO check cmd is in allowed list self._connect() sock = self._sock data = "" diff --git a/src/leap/bitmask/vpn/process.py b/src/leap/bitmask/vpn/process.py index 3de652f..170df44 100644 --- a/src/leap/bitmask/vpn/process.py +++ b/src/leap/bitmask/vpn/process.py @@ -137,7 +137,6 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): # TODO -- internalize this into _status!!! so that it can be shared if 'SIGTERM[soft,ping-restart]' in line: self.restarting = True - self.log.info(line) def processExited(self, failure): """ @@ -151,10 +150,14 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): elif err == internet_error.ProcessTerminated: status, errmsg = 'failure', failure.value.exitCode if errmsg: - self.log.debug('processExited, status %d' % (errmsg,)) + self.log.debug('Process Exited, status %d' % (errmsg,)) else: self.log.warn('%r' % failure.value) + if IS_MAC: + # TODO: need to exit properly! + status, errmsg = 'off', None + self._status.set_status(status, errmsg) self._alive = False @@ -195,6 +198,9 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): def preUp(self): pass + def preDown(self): + pass + def getCommand(self): """ Gets the vpn command from the aproppriate launcher. @@ -255,8 +261,8 @@ elif IS_MAC: class _VPNCanary(_VPNProcess): """ - Special form of _VPNProcess, for Darwin Launcher (windows might use - same strategy). + Special form of _VPNProcess, for Darwin Launcher (windows might end up + using the same strategy). This is a Canary Process that does not run openvpn itself, but it's notified by the privileged process when the process dies. @@ -274,10 +280,13 @@ elif IS_MAC: cmd = self.getVPNCommand() self.helper.send('openvpn_start %s' % ' '.join(cmd)) + def preDown(self): + self.helper.send('openvpn_stop') + def connectionMade(self): - super(_VPNProcess, self).connectionMade() self.setupHelper() reactor.callLater(2, self.registerPID) + _VPNProcess.connectionMade(self) def registerPID(self): cmd = 'openvpn_set_watcher %s' % self.pid @@ -293,11 +302,9 @@ elif IS_MAC: def getCommand(self): canary = '''import sys, signal, time - def receive_signal(signum, stack): - sys.exit() - signal.signal(signal.SIGTERM, receive_signal) - while True: - time.sleep(60)''' +def receive_signal(signum, stack): sys.exit() +signal.signal(signal.SIGTERM, receive_signal) +while True: time.sleep(90)''' return ['python', '-c', '%s' % canary] VPNProcess = _VPNCanary diff --git a/src/leap/bitmask/vpn/tunnel.py b/src/leap/bitmask/vpn/tunnel.py index 4236edf..70b4be0 100644 --- a/src/leap/bitmask/vpn/tunnel.py +++ b/src/leap/bitmask/vpn/tunnel.py @@ -34,6 +34,8 @@ from .constants import IS_WIN # TODO DO NOT pass VPNConfig/ProviderConfig beyond this class. # TODO split sync/async vpn control mechanisms. +# TODO ConfiguredVPNConnection ? + class VPNTunnel(object): @@ -63,16 +65,16 @@ class VPNTunnel(object): host, port = self._get_management_location() - self._vpn = VPNControl(remotes=remotes, - vpnconfig=self._vpnconfig, - providerconfig=self._providerconfig, - socket_host=host, socket_port=port) + self._vpncontrol = VPNControl( + remotes=remotes, vpnconfig=self._vpnconfig, + providerconfig=self._providerconfig, + socket_host=host, socket_port=port) def start(self): """ Start the VPN process. """ - result = self._vpn.start() + result = self._vpncontrol.start() return result def stop(self): @@ -83,16 +85,16 @@ class VPNTunnel(object): :rtype: bool """ # TODO how to return False if this fails - result = self._vpn.stop(False, False) # TODO review params + result = self._vpncontrol.stop(False, False) # TODO review params return result @property def status(self): - return self._vpn.status + return self._vpncontrol.status @property def traffic_status(self): - return self._vpn.traffic_status + return self._vpncontrol.traffic_status def _get_management_location(self): """ diff --git a/src/leap/bitmask/vpn/tunnelmanager.py b/src/leap/bitmask/vpn/tunnelmanager.py index 5faac66..4fb5004 100644 --- a/src/leap/bitmask/vpn/tunnelmanager.py +++ b/src/leap/bitmask/vpn/tunnelmanager.py @@ -30,7 +30,7 @@ from leap.bitmask.vpn.tunnel import VPNTunnel class TunnelManager(object): """ - A TunnelManager controls VPN and Firewall + A TunnelManager controls a VPNTunnel and the Firewall """ def __init__(self, provider, remotes, cert, key, ca, flags): -- cgit v1.2.3