diff options
-rw-r--r-- | changes/feature_2916_check-already_running | 1 | ||||
-rw-r--r-- | src/leap/gui/mainwindow.py | 46 | ||||
-rw-r--r-- | src/leap/gui/statuspanel.py | 3 | ||||
-rw-r--r-- | src/leap/services/eip/vpnprocess.py | 138 |
4 files changed, 135 insertions, 53 deletions
diff --git a/changes/feature_2916_check-already_running b/changes/feature_2916_check-already_running new file mode 100644 index 00000000..9cd04443 --- /dev/null +++ b/changes/feature_2916_check-already_running @@ -0,0 +1 @@ + o Try to terminate already running openvpn instances. Closes #2916 diff --git a/src/leap/gui/mainwindow.py b/src/leap/gui/mainwindow.py index 6fe3e72d..3bd7c516 100644 --- a/src/leap/gui/mainwindow.py +++ b/src/leap/gui/mainwindow.py @@ -49,6 +49,8 @@ from leap.platform_init import IS_WIN, IS_MAC from leap.platform_init.initializers import init_platform from leap.services.eip.vpnprocess import VPN +from leap.services.eip.vpnprocess import OpenVPNAlreadyRunning +from leap.services.eip.vpnprocess import AlienOpenVPNAlreadyRunning from leap.services.eip.vpnlaunchers import (VPNLauncherException, OpenVPNNotFoundException, @@ -1047,7 +1049,22 @@ class MainWindow(QtGui.QMainWindow): self.tr("We could not find openvpn binary."), error=True) self._set_eipstatus_off() + except OpenVPNAlreadyRunning as e: + self._status_panel.set_global_status( + self.tr("Another openvpn instance is already running, and " + "could not be stopped."), + error=True) + self._set_eipstatus_off() + except AlienOpenVPNAlreadyRunning as e: + self._status_panel.set_global_status( + self.tr("Another openvpn instance is already running, and " + "could not be stopped because it was not launched by " + "LEAP. Please stop it and try again."), + error=True) + self._set_eipstatus_off() except VPNLauncherException as e: + # XXX We should implement again translatable exceptions so + # we can pass a translatable string to the panel (usermessage attr) self._status_panel.set_global_status("%s" % (e,), error=True) self._set_eipstatus_off() else: @@ -1058,7 +1075,20 @@ class MainWindow(QtGui.QMainWindow): Sets eip status to off """ self._status_panel.set_eip_status(self.tr("OFF"), error=True) + self._status_panel.set_eip_status_icon("error") self._status_panel.set_startstop_enabled(True) + self._status_panel.eip_stopped() + + self._set_action_eipstart_off() + + def _set_action_eipstart_off(self): + """ + Sets eip startstop action to OFF status. + """ + self._action_eip_startstop.setText(self.tr("Turn ON")) + self._action_eip_startstop.disconnect(self) + self._action_eip_startstop.triggered.connect( + self._start_eip) def _stop_eip(self, abnormal=False): """ @@ -1074,24 +1104,20 @@ class MainWindow(QtGui.QMainWindow): :param abnormal: whether this was an abnormal termination. :type abnormal: bool """ + if abnormal: + logger.warning("Abnormal EIP termination.") + self.user_stopped_eip = True self._vpn.terminate() - self._status_panel.set_eip_status(self.tr("OFF")) - self._status_panel.set_eip_status_icon("error") - self._status_panel.eip_stopped() - self._action_eip_startstop.setText(self.tr("Turn ON")) - self._action_eip_startstop.disconnect(self) - self._action_eip_startstop.triggered.connect( - self._start_eip) + self._set_eipstatus_off() + self._already_started_eip = False self._settings.set_defaultprovider(None) if self._logged_user: self._status_panel.set_provider( "%s@%s" % (self._logged_user, self._get_best_provider_config().get_domain())) - if abnormal: - self._status_panel.set_startstop_enabled(True) def _get_best_provider_config(self): """ @@ -1277,7 +1303,7 @@ class MainWindow(QtGui.QMainWindow): "unexpected manner!"), error=True) else: abnormal = False - if exitCode == 0: + if exitCode == 0 and IS_MAC: # XXX remove this warning after I fix cocoasudo. logger.warning("The above exit code MIGHT BE WRONG.") self._stop_eip(abnormal) diff --git a/src/leap/gui/statuspanel.py b/src/leap/gui/statuspanel.py index 04fc6818..ac0f5162 100644 --- a/src/leap/gui/statuspanel.py +++ b/src/leap/gui/statuspanel.py @@ -45,6 +45,9 @@ class RateMovingAverage(object): """ Initializes an empty array of fixed size """ + self.reset() + + def reset(self): self._data = [None for i in xrange(self.SAMPLE_SIZE)] def append(self, x): diff --git a/src/leap/services/eip/vpnprocess.py b/src/leap/services/eip/vpnprocess.py index cbf554da..c4bdb30c 100644 --- a/src/leap/services/eip/vpnprocess.py +++ b/src/leap/services/eip/vpnprocess.py @@ -31,6 +31,7 @@ from leap.config.providerconfig import ProviderConfig from leap.services.eip.vpnlaunchers import get_platform_launcher from leap.services.eip.eipconfig import EIPConfig from leap.services.eip.udstelnet import UDSTelnet +from leap.util import first logger = logging.getLogger(__name__) vpnlog = logging.getLogger('leap.openvpn') @@ -56,6 +57,16 @@ class VPNSignals(QtCore.QObject): QtCore.QObject.__init__(self) +class OpenVPNAlreadyRunning(Exception): + message = ("Another openvpn instance is already running, and could " + "not be stopped.") + + +class AlienOpenVPNAlreadyRunning(Exception): + message = ("Another openvpn instance is already running, and could " + "not be stopped because it was not launched by LEAP.") + + class VPN(object): """ This is the high-level object that the GUI is dealing with. @@ -95,14 +106,15 @@ class VPN(object): :param kwargs: kwargs to be passed to the VPNProcess :type kwargs: dict """ + self._stop_pollers() kwargs['qtsigs'] = self.qtsigs # start the main vpn subprocess vpnproc = VPNProcess(*args, **kwargs) - # XXX Should stop if already running ------- if vpnproc.get_openvpn_process(): - logger.warning("Another vpnprocess is running!") + logger.info("Another vpn process is running. Will try to stop it.") + vpnproc.stop_if_already_running() cmd = vpnproc.getCommand() env = os.environ @@ -169,6 +181,9 @@ class VPN(object): # ...but we also trigger a countdown to be unpolite # if strictly needed. + + # XXX Watch out! This will fail NOW since we are running + # openvpn as root as a workaround for some connection issues. reactor.callLater( self.TERMINATE_WAIT, self._kill_if_left_alive) @@ -287,8 +302,10 @@ class VPNManager(object): except socket.error: # XXX should get a counter and repeat only # after mod X times. - logger.warning('socket error') + logger.warning('socket error (command was: "%s")' % (command,)) self._close_management_socket(announce=False) + logger.debug('trying to connect to management again') + self.try_to_connect_to_management(max_retries=5) return [] # XXX should move this to a errBack! @@ -349,7 +366,9 @@ class VPNManager(object): :param args: not used """ if self._tn: - logger.info('connected to management') + logger.info('Connected to management') + else: + logger.debug('Cannot connect to management...') def _connectErr(self, failure): """ @@ -385,7 +404,7 @@ class VPNManager(object): """ return True if self._tn else False - def try_to_connect_to_management(self, retry=0): + def try_to_connect_to_management(self, retry=0, max_retries=None): """ Attempts to connect to a management interface, and retries after CONNECTION_RETRY_TIME if not successful. @@ -393,8 +412,18 @@ class VPNManager(object): :param retry: number of the retry :type retry: int """ - # TODO decide about putting a max_lim to retries and signaling - # an error. + if max_retries and retry > max_retries: + logger.warning("Max retries reached while attempting to connect " + "to management. Aborting.") + self.aborted = True + return + + # _alive flag is set in the VPNProcess class. + if not self._alive: + logger.debug('Tried to connect to management but process is ' + 'not alive.') + return + logger.debug('trying to connect to management') if not self.aborted and not self.is_connected(): self.connect_to_management(self._socket_host, self._socket_port) self._reactor.callLater( @@ -516,16 +545,13 @@ class VPNManager(object): """ if self._socket_port == "unix": logger.debug('cleaning socket file temp folder') - tempfolder = os.path.split(self._socket_host)[0] # XXX use `first` - if os.path.isdir(tempfolder): + tempfolder = first(os.path.split(self._socket_host)) + if tempfolder and os.path.isdir(tempfolder): try: shutil.rmtree(tempfolder) except OSError: logger.error('could not delete tmpfolder %s' % tempfolder) - # --------------------------------------------------- - # XXX old methods, not adapted to twisted process yet - def get_openvpn_process(self): """ Looks for openvpn instances running. @@ -547,42 +573,68 @@ class VPNManager(object): pass return openvpn_process - def _stop_if_already_running(self): + def stop_if_already_running(self): """ Checks if VPN is already running and tries to stop it. + Might raise OpenVPNAlreadyRunning. + :return: True if stopped, False otherwise + """ - # TODO cleanup this - process = self._get_openvpn_process() - if process: - logger.debug("OpenVPN is already running, trying to stop it...") - cmdline = process.cmdline + process = self.get_openvpn_process() + if not process: + logger.debug('Could not find openvpn process while ' + 'trying to stop it.') + return - manag_flag = "--management" - if isinstance(cmdline, list) and manag_flag in cmdline: - try: - index = cmdline.index(manag_flag) - host = cmdline[index + 1] - port = cmdline[index + 2] - logger.debug("Trying to connect to %s:%s" - % (host, port)) - self._connect_to_management(host, port) - self._send_command("signal SIGTERM") - self._tn.close() - self._tn = None - #self._disconnect_management() - except Exception as e: - logger.warning("Problem trying to terminate OpenVPN: %r" - % (e,)) - - process = self._get_openvpn_process() - if process is None: - logger.warning("Unabled to terminate OpenVPN") - return True - else: - return False - return True + logger.debug("OpenVPN is already running, trying to stop it...") + cmdline = process.cmdline + + manag_flag = "--management" + if isinstance(cmdline, list) and manag_flag in cmdline: + # we know that our invocation has this distinctive fragment, so + # we use this fingerprint to tell other invocations apart. + # this might break if we change the configuration path in the + # launchers + smellslikeleap = lambda s: "leap" in s and "providers" in s + + if not any(map(smellslikeleap, cmdline)): + logger.debug("We cannot stop this instance since we do not " + "recognise it as a leap invocation.") + raise AlienOpenVPNAlreadyRunning + + try: + index = cmdline.index(manag_flag) + host = cmdline[index + 1] + port = cmdline[index + 2] + logger.debug("Trying to connect to %s:%s" + % (host, port)) + self.connect_to_management(host, port) + + # XXX this has a problem with connections to different + # remotes. So the reconnection will only work when we are + # terminating instances left running for the same provider. + # If we are killing an openvpn instance configured for another + # provider, we will get: + # TLS Error: local/remote TLS keys are out of sync + # However, that should be a rare case right now. + self._send_command("signal SIGTERM") + self._close_management_socket(announce=True) + except Exception as e: + logger.warning("Problem trying to terminate OpenVPN: %r" + % (e,)) + else: + logger.debug("Could not find the expected openvpn command line.") + + process = self.get_openvpn_process() + if process is None: + logger.debug("Successfully finished already running " + "openvpn process.") + return True + else: + logger.warning("Unable to terminate OpenVPN") + raise OpenVPNAlreadyRunning class VPNProcess(protocol.ProcessProtocol, VPNManager): @@ -640,7 +692,7 @@ class VPNProcess(protocol.ProcessProtocol, VPNManager): """ self._alive = True self.aborted = False - self.try_to_connect_to_management() + self.try_to_connect_to_management(max_retries=10) def outReceived(self, data): """ |