[refactor] merge tunnel and control modules

3 files changed, 187 insertions, 243 deletions

diff --git a/src/leap/bitmask/vpn/_control.py b/src/leap/bitmask/vpn/_control.py
deleted file mode 100644
index ff39db8..0000000
--- a/src/leap/bitmask/vpn/_control.py
+++ /dev/null
@@ -1,185 +0,0 @@
-import os
-
-from twisted.internet import reactor, defer
-from twisted.logger import Logger
-
-from .process import VPNProcess
-from .constants import IS_LINUX
-
-
-# TODO
-# TODO merge these classes with service.
-# [ ] register change state listener
-# emit_async(catalog.VPN_STATUS_CHANGED)
-# [ ] catch ping-restart
-# 'NETWORK_UNREACHABLE': (
-#    'Network is unreachable (code=101)',),
-# 'PROCESS_RESTART_TLS': (
-#    "SIGTERM[soft,tls-error]",),
-
-
-class VPNControl(object):
-
-    """
-    This is the high-level object that the service knows about.
-    It exposes the start and terminate methods.
-
-    On start, it spawns a VPNProcess instance that will use a vpnlauncher
-    suited for the running platform and connect to the management interface
-    opened by the openvpn process, executing commands over that interface on
-    demand.
-    """
-
-    TERMINATE_MAXTRIES = 10
-    TERMINATE_WAIT = 1  # secs
-    RESTART_WAIT = 2  # secs
-
-    log = Logger()
-
-    def __init__(self, remotes, vpnconfig,
-                 providerconfig, socket_host, socket_port):
-        self._vpnproc = None
-        self._user_stopped = False
-
-        self._remotes = remotes
-        self._vpnconfig = vpnconfig
-        self._providerconfig = providerconfig
-        self._host = socket_host
-        self._port = socket_port
-
-    def start(self):
-        self.log.debug('VPN: start')
-
-        self._user_stopped = False
-
-        args = [self._vpnconfig, self._providerconfig, self._host,
-                self._port]
-        kwargs = {'openvpn_verb': 4, 'remotes': self._remotes,
-                  'restartfun': self.restart}
-
-        vpnproc = VPNProcess(*args, **kwargs)
-
-        # TODO -- restore
-        # if get_openvpn_process():
-        #    self.log.info(
-        #        'Another vpn process is running. Will try to stop it.')
-        #    vpnproc.stop_if_already_running()
-
-        try:
-            vpnproc.preUp()
-        except Exception as e:
-            self.log.error('Error on vpn pre-up {0!r}'.format(e))
-            raise
-        try:
-            cmd = vpnproc.getCommand()
-        except Exception as e:
-            self.log.error(
-                'Error while getting vpn command... {0!r}'.format(e))
-            raise
-
-        env = os.environ
-
-        try:
-            runningproc = reactor.spawnProcess(vpnproc, cmd[0], cmd, env)
-        except Exception as e:
-            self.log.error(
-                'Error while spawning vpn process... {0!r}'.format(e))
-            return False
-
-        # TODO get pid from management instead
-        vpnproc.pid = runningproc.pid
-        self._vpnproc = vpnproc
-
-        return True
-
-    @defer.inlineCallbacks
-    def restart(self):
-        yield self.stop(shutdown=False, restart=True)
-        reactor.callLater(
-            self.RESTART_WAIT, self.start)
-
-    def stop(self, shutdown=False, restart=False):
-        """
-        Stops the openvpn subprocess.
-
-        Attempts to send a SIGTERM first, and after a timeout
-        it sends a SIGKILL.
-
-        :param shutdown: whether this is the final shutdown
-        :type shutdown: bool
-        :param restart: whether this stop is part of a hard restart.
-        :type restart: bool
-        """
-        # We assume that the only valid stops are initiated
-        # by an user action, not hard restarts
-        self._user_stopped = not restart
-        if self._vpnproc is not None:
-            self._vpnproc.restarting = restart
-
-        try:
-            if self._vpnproc is not None:
-                self._vpnproc.preDown()
-        except Exception as e:
-            self.log.error('Error on vpn pre-down {0!r}'.format(e))
-            raise
-
-        d = defer.succeed(True)
-        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:
-                self._sentterm = True
-                self._vpnproc.terminate()
-
-                # we trigger a countdown to be unpolite
-                # if strictly needed.
-                d = defer.Deferred()
-                reactor.callLater(
-                    self.TERMINATE_WAIT, self._kill_if_left_alive, d)
-        return d
-
-    # TODO -- remove indirection
-    @property
-    def status(self):
-        if not self._vpnproc:
-            return {'status': 'off', 'error': None}
-        return self._vpnproc.status
-
-    @property
-    def traffic_status(self):
-        return self._vpnproc.traffic_status
-
-    def _killit(self):
-        if self._vpnproc is None:
-            self.log.debug("There's no vpn process running to kill.")
-        else:
-            self._vpnproc.aborted = True
-            self._vpnproc.kill()
-
-    def _kill_if_left_alive(self, deferred, tries=0):
-        """
-        Check if the process is still alive, and send a
-        SIGKILL after a timeout period.
-
-        :param tries: counter of tries, used in recursion
-        :type tries: int
-        """
-        if tries < self.TERMINATE_MAXTRIES:
-            if self._vpnproc.transport.pid is None:
-                deferred.callback(True)
-                return
-            else:
-                self.log.debug('Process did not die, waiting...')
-
-            tries += 1
-            reactor.callLater(self.TERMINATE_WAIT,
-                              self._kill_if_left_alive, deferred, tries)
-            return
-
-        # after running out of patience, we try a killProcess
-        self.log.debug('Process did not die. Sending a SIGKILL.')
-        try:
-            self._killit()
-        except OSError:
-            self.log.error('Could not kill process!')
-        deferred.callback(True)
diff --git a/src/leap/bitmask/vpn/service.py b/src/leap/bitmask/vpn/service.py
index 2390391..71fb865 100644
--- a/src/leap/bitmask/vpn/service.py
+++ b/src/leap/bitmask/vpn/service.py
@@ -30,10 +30,9 @@ from leap.bitmask.hooks import HookableService
 from leap.bitmask.util import merge_status
 from leap.bitmask.vpn.gateways import GatewaySelector
 from leap.bitmask.vpn.fw.firewall import FirewallManager
-from leap.bitmask.vpn.tunnel import TunnelManager
+from leap.bitmask.vpn.tunnel import ConfiguredTunnel
 from leap.bitmask.vpn._checks import is_service_ready, get_vpn_cert_path
 from leap.bitmask.vpn import privilege, helpers
-from leap.bitmask.vpn.privilege import NoPolkitAuthAgentAvailable
 from leap.common.config import get_path_prefix
 from leap.common.files import check_and_fix_urw_only
 from leap.common.certs import get_cert_time_boundaries
@@ -101,7 +100,7 @@ class VPNService(HookableService):
             raise Exception('Could not start firewall')
 
         try:
-            vpn_ok = self._tunnel.start()
+            self._tunnel.start()
         except Exception as exc:
             self._firewall.stop()
             # TODO get message from exception
@@ -206,7 +205,7 @@ class VPNService(HookableService):
 
     @defer.inlineCallbacks
     def _setup(self, provider):
-        """Set up TunnelManager for a specified provider.
+        """Set up ConfiguredTunnel for a specified provider.
 
         :param provider: the provider to use, e.g. 'demo.bitmask.net'
         :type provider: str"""
@@ -247,7 +246,7 @@ class VPNService(HookableService):
 
         # TODO add remote ports, according to preferred sequence
         remotes = tuple([(ip, '443') for ip in sorted_gateways])
-        self._tunnel = TunnelManager(
+        self._tunnel = ConfiguredTunnel(
             provider, remotes, cert_path, key_path, ca_path, extra_flags)
         self._firewall = FirewallManager(remotes)
 
diff --git a/src/leap/bitmask/vpn/tunnel.py b/src/leap/bitmask/vpn/tunnel.py
index c2b0f58..f56e7bc 100644
--- a/src/leap/bitmask/vpn/tunnel.py
+++ b/src/leap/bitmask/vpn/tunnel.py
@@ -22,28 +22,48 @@ VPN Tunnel.
 import os
 import tempfile
 
-from ._control import VPNControl
+from twisted.internet import reactor, defer
+from twisted.logger import Logger
+
 from ._config import _TempVPNConfig, _TempProviderConfig
-from .constants import IS_WIN
+from .constants import IS_WIN, IS_LINUX
+from .process import VPNProcess
 
 
-# TODO refactor - this class is still a very light proxy around the
-# underlying VPNControl. The main methods here are start/stop, so this
-# looks like it could better use the Service interface.
 # TODO gateway selection should be done in this class.
-# TODO DO NOT pass VPNConfig/ProviderConfig beyond this class.
-# TODO split sync/async vpn control mechanisms.
 
-# TODO ConfiguredVPNConnection ?
+
+# TODO ----------------- refactor --------------------
+# [ ] register change state listener
+# emit_async(catalog.VPN_STATUS_CHANGED)
+# [ ] catch ping-restart
+# 'NETWORK_UNREACHABLE': (
+#    'Network is unreachable (code=101)',),
+# 'PROCESS_RESTART_TLS': (
+#    "SIGTERM[soft,tls-error]",),
 
 
-class TunnelManager(object):
+class ConfiguredTunnel(object):
 
     """
-    A VPN Tunnel holds the configuration for a VPN connection, and allows to
-    control that connection.
+    A ConfiguredTunne holds the configuration for a VPN connection, and allows
+    to control that connection.
+
+    This is the high-level object that the service knows about.
+    It exposes the start and terminate methods for the VPN Tunnel.
+
+    On start, it spawns a VPNProcess instance that will use a vpnlauncher
+    suited for the running platform and connect to the management interface
+    opened by the openvpn process, executing commands over that interface on
+    demand.
     """
 
+    TERMINATE_MAXTRIES = 10
+    TERMINATE_WAIT = 1  # secs
+    RESTART_WAIT = 2  # secs
+
+    log = Logger()
+
     def __init__(self, provider, remotes, cert_path, key_path, ca_path,
                  extra_flags):
         """
@@ -51,66 +71,176 @@ class TunnelManager(object):
             ((ip1, portA), (ip2, portB), ...)
         :type remotes: tuple of tuple(str, int)
         """
-        # TODO we can set all the needed ports, gateways and paths in here
-        # TODO need gateways here
-        # sorting them doesn't belong in here
-        # gateways = ((ip1, portA), (ip2, portB), ...)
-
-        ports = []
-
         self._remotes = remotes
+        ports = []
 
         self._vpnconfig = _TempVPNConfig(extra_flags, cert_path, ports)
         self._providerconfig = _TempProviderConfig(provider, ca_path)
 
-        host, port = self._get_management_location()
+        host, port = _get_management_location()
+        self._host = host
+        self._port = port
 
-        self._vpncontrol = VPNControl(
-            remotes=remotes, vpnconfig=self._vpnconfig,
-            providerconfig=self._providerconfig,
-            socket_host=host, socket_port=port)
+        self._vpnproc = None
+        self._user_stopped = False
 
     def start(self):
-        """
-        Start the VPN process.
-        """
-        result = self._vpncontrol.start()
-        return result
+        return self._start_vpn()
 
     def stop(self):
-        """
-        Bring openvpn down.
+        return self._stop_vpn(shutdown=False, restart=False)
 
-        :returns: True if succeeded, False otherwise.
-        :rtype: bool
-        """
-        # TODO how to return False if this fails
-        result = self._vpncontrol.stop(False, False)  # TODO review params
-        return result
+    #  status
 
     @property
     def status(self):
-        return self._vpncontrol.status
+        if not self._vpnproc:
+            return {'status': 'off', 'error': None}
+        return self._vpnproc.status
 
     @property
     def traffic_status(self):
-        return self._vpncontrol.traffic_status
+        return self._vpnproc.traffic_status
+
+    # VPN Control
+
+    def _start_vpn(self):
+        self.log.debug('VPN: start')
+
+        self._user_stopped = False
+
+        args = [self._vpnconfig, self._providerconfig, self._host,
+                self._port]
+        kwargs = {'openvpn_verb': 4, 'remotes': self._remotes,
+                  'restartfun': self.restart}
+        vpnproc = VPNProcess(*args, **kwargs)
+
+        try:
+            vpnproc.preUp()
+        except Exception as e:
+            self.log.error('Error on vpn pre-up {0!r}'.format(e))
+            raise
+        try:
+            cmd = vpnproc.getCommand()
+        except Exception as e:
+            self.log.error(
+                'Error while getting vpn command... {0!r}'.format(e))
+            raise
+
+        env = os.environ
+        try:
+            runningproc = reactor.spawnProcess(vpnproc, cmd[0], cmd, env)
+        except Exception as e:
+            self.log.error(
+                'Error while spawning vpn process... {0!r}'.format(e))
+            return False
+
+        # TODO get pid from management instead
+        vpnproc.pid = runningproc.pid
+        self._vpnproc = vpnproc
+        return True
+
+    @defer.inlineCallbacks
+    def _restart_vpn(self):
+        yield self.stop(shutdown=False, restart=True)
+        reactor.callLater(
+            self.RESTART_WAIT, self.start)
+
+    def _stop_vpn(self, shutdown=False, restart=False):
+        """
+        Stops the openvpn subprocess.
+
+        Attempts to send a SIGTERM first, and after a timeout
+        it sends a SIGKILL.
 
-    def _get_management_location(self):
+        :param shutdown: whether this is the final shutdown
+        :type shutdown: bool
+        :param restart: whether this stop is part of a hard restart.
+        :type restart: bool
         """
-        Return a tuple with the host (socket) and port to be used for VPN.
+        # TODO how to return False if this fails
+        # XXX maybe return a deferred
+
+        # We assume that the only valid stops are initiated
+        # by an user action, not hard restarts
+        self._user_stopped = not restart
+        if self._vpnproc is not None:
+            self._vpnproc.restarting = restart
+
+        try:
+            if self._vpnproc is not None:
+                self._vpnproc.preDown()
+        except Exception as e:
+            self.log.error('Error on vpn pre-down {0!r}'.format(e))
+            raise
+
+        d = defer.succeed(True)
+        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:
+                self._sentterm = True
+                self._vpnproc.terminate()
+
+                # we trigger a countdown to be unpolite
+                # if strictly needed.
+                d = defer.Deferred()
+                reactor.callLater(
+                    self.TERMINATE_WAIT, self._kill_if_left_alive, d)
+        return d
+
+    def _wait_and_kill(self, deferred, tries=0):
+        """
+        Check if the process is still alive, and send a
+        SIGKILL after a waiting several times during a timeout period.
 
-        :return: (host, port)
-        :rtype: tuple (str, str)
+        :param tries: counter of tries, used in recursion
+        :type tries: int
         """
-        if IS_WIN:
-            host = "localhost"
-            port = "9876"
-        else:
-            # XXX cleanup this on exit too
-            # XXX atexit.register ?
-            host = os.path.join(tempfile.mkdtemp(prefix="leap-tmp"),
-                                'openvpn.socket')
-            port = "unix"
-
-        return host, port
+        if tries < self.TERMINATE_MAXTRIES:
+            if self._vpnproc.transport.pid is None:
+                deferred.callback(True)
+                return
+            else:
+                self.log.debug('Process did not die, waiting...')
+
+            tries += 1
+            reactor.callLater(
+                self.TERMINATE_WAIT,
+                self._wait_and_kill, deferred, tries)
+            return
+
+        # after running out of patience, we try a killProcess
+        self._kill(deferred)
+
+    def _kill(self, d):
+        self.log.debug('Process did not die. Sending a SIGKILL.')
+        try:
+            if self._vpnproc is None:
+                self.log.debug("There's no vpn process running to kill.")
+            else:
+                self._vpnproc.aborted = True
+                self._vpnproc.kill()
+        except OSError:
+            self.log.error('Could not kill process!')
+        d.callback(True)
+
+
+# utils
+
+
+def _get_management_location():
+    """
+    Return a tuple with the host (socket) and port to be used for VPN.
+
+    :return: (host, port)
+    :rtype: tuple (str, str)
+    """
+    if IS_WIN:
+        host = "localhost"
+        port = "9876"
+    else:
+        host = os.path.join(
+            tempfile.mkdtemp(prefix="leap-tmp"), 'openvpn.socket')
+        port = "unix"
+    return host, port