From fdda95c92d22b4ede61bfb7587320ddb36da5cd7 Mon Sep 17 00:00:00 2001 From: kali Date: Sat, 15 Jun 2013 01:17:35 +0900 Subject: working openvpn termination: SIGTERM, then SIGKILL. Closes: #2753 --- src/leap/app.py | 9 ++- src/leap/gui/mainwindow.py | 19 +++-- src/leap/services/eip/vpnprocess.py | 141 +++++++++++++++++++++++++++++------- 3 files changed, 130 insertions(+), 39 deletions(-) (limited to 'src/leap') diff --git a/src/leap/app.py b/src/leap/app.py index 05b54d1f..cb9951c1 100644 --- a/src/leap/app.py +++ b/src/leap/app.py @@ -138,11 +138,14 @@ def main(): app.setApplicationName("leap") app.setOrganizationDomain("leap.se") + # XXX --------------------------------------------------------- + # In quarantine, looks like we don't need it anymore. # This dummy timer ensures that control is given to the outside # loop, so we can hook our sigint handler. - timer = QtCore.QTimer() - timer.start(500) - timer.timeout.connect(lambda: None) + #timer = QtCore.QTimer() + #timer.start(500) + #timer.timeout.connect(lambda: None) + # XXX --------------------------------------------------------- window = MainWindow( lambda: twisted_main.quit(app), diff --git a/src/leap/gui/mainwindow.py b/src/leap/gui/mainwindow.py index 42148836..dd9fb148 100644 --- a/src/leap/gui/mainwindow.py +++ b/src/leap/gui/mainwindow.py @@ -994,19 +994,21 @@ class MainWindow(QtGui.QMainWindow): self._status_panel.set_startstop_enabled(True) def _stop_eip(self): + """ + Stops vpn process and makes gui adjustments to reflect + the change of state. + """ 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._already_started_eip = False self._settings.set_defaultprovider(None) - if self._logged_user: self._status_panel.set_provider( "%s@%s" % (self._logged_user, @@ -1204,12 +1206,12 @@ class MainWindow(QtGui.QMainWindow): """ logger.debug('About to quit, doing cleanup...') - logger.debug('Killing vpn') - self._vpn.terminate() - logger.debug('Cleaning pidfiles') self._cleanup_pidfiles() + logger.debug('Terminating vpn') + self._vpn.terminate() + def quit(self): """ Cleanup and tidely close the main window before quitting. @@ -1223,13 +1225,14 @@ class MainWindow(QtGui.QMainWindow): if self._logger_window: self._logger_window.close() - self.close() - if self._login_defer: self._login_defer.cancel() + self.close() + if self._quit_callback: self._quit_callback() + logger.debug('Bye.') diff --git a/src/leap/services/eip/vpnprocess.py b/src/leap/services/eip/vpnprocess.py index eae8aadd..162dc7f0 100644 --- a/src/leap/services/eip/vpnprocess.py +++ b/src/leap/services/eip/vpnprocess.py @@ -20,6 +20,8 @@ VPN Manager, spawned in a custom processProtocol. import logging import os import psutil +import socket +import time from PySide import QtCore @@ -63,6 +65,9 @@ class VPN(object): opened by the openvpn process, executing commands over that interface on demand. """ + TERMINATE_MAXTRIES = 10 + TERMINATE_WAIT = 1 # secs + def __init__(self): """ Instantiate empty attributes and get a copy @@ -94,6 +99,10 @@ class VPN(object): # 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!") + cmd = vpnproc.getCommand() env = os.environ for key, val in vpnproc.vpn_env.items(): @@ -103,7 +112,7 @@ class VPN(object): self._vpnproc = vpnproc # add pollers for status and state - # XXX this could be extended to a collection of + # this could be extended to a collection of # generic watchers poll_list = [LoopingCall(vpnproc.pollStatus), @@ -111,15 +120,50 @@ class VPN(object): self._pollers.extend(poll_list) self._start_pollers() + def _kill_if_left_alive(self, tries=0): + """ + Check if the process is still alive, and sends a + SIGKILL after a timeout period. + + :param tries: counter of tries, used in recursion + :type tries: int + """ + from twisted.internet import reactor + while tries < self.TERMINATE_MAXTRIES: + if self._vpnproc.transport.pid is None: + logger.debug("Process has been happily terminated.") + return + else: + logger.debug("Process did not die, waiting...") + tries += 1 + reactor.callLater(self.TERMINATE_WAIT, + self._kill_if_left_alive, tries) + + # after running out of patience, we try a killProcess + logger.debug("Process did not died. Sending a SIGKILL.") + self._vpnproc.killProcess() + def terminate(self): """ Stops the openvpn subprocess. + + Attempts to send a SIGTERM first, and after a timeout + it sends a SIGKILL. """ + from twisted.internet import reactor self._stop_pollers() - # XXX we should leave a KILL as a last resort. - # First we should try to send a SIGTERM + + # First we try to be polite and send a SIGTERM... if self._vpnproc: - self._vpnproc.killProcess() + self._sentterm = True + self._vpnproc.terminate_openvpn() + + # ...but we also trigger a countdown to be unpolite + # if strictly needed. + reactor.callLater( + self.TERMINATE_WAIT, self._kill_if_left_alive) + + # TODO: should also cleanup tempfiles!!! def _start_pollers(self): """ @@ -148,6 +192,10 @@ class VPNManager(object): A copy of a QObject containing signals as attributes is passed along upon initialization, and we use that object to emit signals to qt-land. + + For more info about management methods:: + + zcat `dpkg -L openvpn | grep management` """ # Timers, in secs @@ -183,15 +231,15 @@ class VPNManager(object): def qtsigs(self): return self._qtsigs - def _disconnect(self): + def _seek_to_eof(self): """ - Disconnects the telnet connection to the openvpn process. + Read as much as available. Position seek pointer to end of stream """ - logger.debug('Closing socket') - self._tn.write("quit\n") - self._tn.read_all() - self._tn.close() - self._tn = None + try: + self._tn.read_eager() + except EOFError: + logger.debug("Could not read from socket. Assuming it died.") + return def _send_command(self, command, until=b"END"): """ @@ -208,12 +256,24 @@ class VPNManager(object): :rtype: list """ leap_assert(self._tn, "We need a tn connection!") + try: self._tn.write("%s\n" % (command,)) buf = self._tn.read_until(until, 2) - self._tn.read_eager() - lines = buf.split("\n") - return lines + self._seek_to_eof() + blist = buf.split('\r\n') + if blist[-1].startswith(until): + del blist[-1] + return blist + else: + return [] + + except socket.error: + # XXX should get a counter and repeat only + # after mod X times. + logger.warning('socket error') + self._close_management_socket(announce=False) + return [] # XXX should move this to a errBack! except Exception as e: @@ -221,9 +281,21 @@ class VPNManager(object): (command, e)) return [] - def _connect(self, socket_host, socket_port): + def _close_management_socket(self, announce=True): + """ + Close connection to openvpn management interface. + """ + logger.debug('closing socket') + if announce: + self._tn.write("quit\n") + self._tn.read_all() + self._tn.get_socket().close() + del self._tn + + def _connect_management(self, socket_host, socket_port): """ - Connects to the specified socket_host socket_port. + Connects to the management interface on the specified + socket_host socket_port. :param socket_host: either socket path (unix) or socket IP :type socket_host: str @@ -232,6 +304,9 @@ class VPNManager(object): socket, or port otherwise :type socket_port: str """ + if self.is_connected(): + self._close_management_socket() + try: self._tn = UDSTelnet(socket_host, socket_port) @@ -268,7 +343,7 @@ class VPNManager(object): """ logger.warning(failure) - def connect(self, host, port): + def connect_to_management(self, host, port): """ Connect to a management interface. @@ -280,7 +355,8 @@ class VPNManager(object): :returns: a deferred """ - self.connectd = defer.maybeDeferred(self._connect, host, port) + self.connectd = defer.maybeDeferred( + self._connect_management, host, port) self.connectd.addCallbacks(self._connectCb, self._connectErr) return self.connectd @@ -293,7 +369,7 @@ class VPNManager(object): """ return True if self._tn else False - def try_to_connect(self, retry=0): + def try_to_connect_to_management(self, retry=0): """ Attempts to connect to a management interface, and retries after CONNECTION_RETRY_TIME if not successful. @@ -304,9 +380,10 @@ class VPNManager(object): # TODO decide about putting a max_lim to retries and signaling # an error. if not self.is_connected(): - self.connect(self._socket_host, self._socket_port) + self.connect_to_management(self._socket_host, self._socket_port) self._reactor.callLater( - self.CONNECTION_RETRY_TIME, self.try_to_connect, retry + 1) + self.CONNECTION_RETRY_TIME, + self.try_to_connect_to_management, retry + 1) def _parse_state_and_notify(self, output): """ @@ -405,9 +482,17 @@ class VPNManager(object): """ return self._launcher.get_vpn_env(self._providerconfig) + def terminate_openvpn(self): + """ + Attempts to terminate openvpn by sending a SIGTERM. + """ + if self.is_connected(): + self._send_command("signal SIGTERM") + + # --------------------------------------------------- # XXX old methods, not adapted to twisted process yet - def _get_openvpn_process(self): + def get_openvpn_process(self): """ Looks for openvpn instances running. @@ -421,7 +506,7 @@ class VPNManager(object): # we should check that cmdline BEGINS # with openvpn or with our wrapper # (pkexec / osascript / whatever) - if self._launcher.OPENVPN_BIN in ' '.join(p.cmdline): + if "openvpn" in ' '.join(p.cmdline): openvpn_process = p break except psutil.error.AccessDenied: @@ -434,10 +519,10 @@ class VPNManager(object): :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") + logger.debug("OpenVPN is already running, trying to stop it...") cmdline = process.cmdline manag_flag = "--management" @@ -448,11 +533,11 @@ class VPNManager(object): port = cmdline[index + 2] logger.debug("Trying to connect to %s:%s" % (host, port)) - self._connect(host, port) + self._connect_to_management(host, port) self._send_command("signal SIGTERM") self._tn.close() self._tn = None - #self._disconnect() + #self._disconnect_management() except Exception as e: logger.warning("Problem trying to terminate OpenVPN: %r" % (e,)) @@ -518,7 +603,7 @@ class VPNProcess(protocol.ProcessProtocol, VPNManager): .. seeAlso: `http://twistedmatrix.com/documents/13.0.0/api/twisted.internet.protocol.ProcessProtocol.html` # noqa """ - self.try_to_connect() + self.try_to_connect_to_management() def outReceived(self, data): """ -- cgit v1.2.3