[bug] proper vpn shutdown in osx bundles

the restarting flag was not being set.

- Resolves: #8959

4 files changed, 26 insertions, 23 deletions

diff --git a/src/leap/bitmask/vpn/_control.py b/src/leap/bitmask/vpn/_control.py
index 7f80f40b..7cf41d1f 100644
--- a/src/leap/bitmask/vpn/_control.py
+++ b/src/leap/bitmask/vpn/_control.py
@@ -90,7 +90,8 @@ class VPNControl(object):
         # this could be extended to a collection of
         # generic watchers
 
-        poll_list = [LoopingCall(vpnproc.pollStatus),
+        poll_list = [
+                     LoopingCall(vpnproc.pollStatus),
                      LoopingCall(vpnproc.pollState),
                      LoopingCall(vpnproc.pollLog)]
         self._pollers.extend(poll_list)
@@ -115,6 +116,11 @@ class VPNControl(object):
         :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
+        self._vpnproc.restarting = restart
+
         self._stop_pollers()
         try:
             self._vpnproc.preDown()
@@ -127,15 +133,10 @@ class VPNControl(object):
             # 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
+                # we trigger a countdown to be unpolite
                 # if strictly needed.
                 d = defer.Deferred()
                 reactor.callLater(
diff --git a/src/leap/bitmask/vpn/_management.py b/src/leap/bitmask/vpn/_management.py
index 14daa48e..fac1b099 100644
--- a/src/leap/bitmask/vpn/_management.py
+++ b/src/leap/bitmask/vpn/_management.py
@@ -121,6 +121,8 @@ class VPNManagement(object):
                 splitted = line.split(',')
                 ts = splitted[0]
                 msg = ','.join(splitted[2:])
+                if msg.startswith('MANAGEMENT'):
+                    continue
                 if ts not in self._logs:
                     self._watcher.watch(msg)
                     self.log.info('VPN: %s' % msg)
@@ -133,7 +135,7 @@ class VPNManagement(object):
         Read as much as available. Position seek pointer to end of stream
         """
         try:
-            self._tn.read_eager()
+            return self._tn.read_eager()
         except EOFError:
             self.log.debug('Could not read from socket. Assuming it died.')
             return
@@ -152,11 +154,10 @@ class VPNManagement(object):
         :return: response read
         :rtype: list
         """
-
         try:
             self._tn.write("%s\n" % (command,))
-            buf = self._tn.read_until(until, 2)
-            self._seek_to_eof()
+            buf = self._tn.read_until(until)
+            seek = self._seek_to_eof()
             blist = buf.split('\r\n')
             if blist[-1].startswith(until):
                 del blist[-1]
@@ -210,7 +211,7 @@ class VPNManagement(object):
                 try:
                     ts, status_step, ok, ip, remote, port, _, _ = parts
                 except ValueError:
-                    self.log.debug('Could not parse %s' % parts)
+                    self.log.debug('Could not parse state line: %s' % line)
 
             return status_step
 
@@ -236,7 +237,7 @@ class VPNManagement(object):
             try:
                 text, value = parts
             except ValueError:
-                self.log.debug('Could not parse %s' % parts)
+                self.log.debug('Could not parse status line %s' % line)
                 return
             # text can be:
             #   "TUN/TAP read bytes"
@@ -253,19 +254,12 @@ class VPNManagement(object):
         return (tun_tap_read, tun_tap_write)
 
     def get_state(self):
-        """
-        Notifies the gui of the output of the state command over
-        the openvpn management interface.
-        """
         if not self.is_connected():
             return ""
-        return self._parse_state(self._send_command("state"))
+        state = self._parse_state(self._send_command("state"))
+        return state
 
     def get_traffic_status(self):
-        """
-        Notifies the gui of the output of the status command over
-        the openvpn management interface.
-        """
         if not self.is_connected():
             return (None, None)
         return self._parse_status(self._send_command("status"))
diff --git a/src/leap/bitmask/vpn/helpers/osx/bitmask-helper b/src/leap/bitmask/vpn/helpers/osx/bitmask-helper
index f8d90b81..4a54295b 100755
--- a/src/leap/bitmask/vpn/helpers/osx/bitmask-helper
+++ b/src/leap/bitmask/vpn/helpers/osx/bitmask-helper
@@ -251,6 +251,10 @@ def openvpn_start(*args):
     syslog.syslog(syslog.LOG_WARNING, 'LAUNCHING VPN: ' + ' '.join(cmd))
 
     global openvpn_proc
+    if openvpn_proc is not None:
+        syslog.syslog(syslog.LOG_WARNING, 'vpn: another process is already started')
+        return
+
     # TODO sanitize options
     openvpn_proc = subprocess.Popen(cmd, shell=False, bufsize=-1)
 
@@ -270,6 +274,8 @@ def openvpn_stop(sig='TERM'):
     :type args: list
     """
     global openvpn_proc
+    syslog.syslog(
+        syslog.LOG_WARNING, "Stopping OpenVPN...")
 
     if openvpn_proc:
         syslog.syslog(
diff --git a/src/leap/bitmask/vpn/service.py b/src/leap/bitmask/vpn/service.py
index 0750a193..7cd595a0 100644
--- a/src/leap/bitmask/vpn/service.py
+++ b/src/leap/bitmask/vpn/service.py
@@ -123,7 +123,9 @@ class VPNService(HookableService):
             if not fw_ok:
                 self.log.error("Firewall: error stopping")
 
-        if not self._tunnel or self._tunnel.status['status'] is not 'on':
+        if not self._tunnel:
+            # XXX this avoids to shutdown anything that was in process.
+            # or self._tunnel.status['status'] is not 'on':
             raise Exception('VPN was not running')
 
         vpn_ok = self._tunnel.stop()