From d18de54f1fe00c0f7cfa3be64faed9863e248231 Mon Sep 17 00:00:00 2001 From: Ruben Pollan Date: Thu, 29 Jun 2017 12:39:59 +0200 Subject: [refactor] move TunnelManager duties to VPNService VPNService has knowledge of the tunnel and the firewall and controls them separately. Also exceptions from VPNControl are handled locally instead of being propagated. - Resolves: #8976 - Related: #8942 --- src/leap/bitmask/vpn/_control.py | 12 +++- src/leap/bitmask/vpn/service.py | 65 +++++++++++-------- src/leap/bitmask/vpn/tunnelmanager.py | 115 ---------------------------------- 3 files changed, 47 insertions(+), 145 deletions(-) delete mode 100644 src/leap/bitmask/vpn/tunnelmanager.py (limited to 'src/leap') diff --git a/src/leap/bitmask/vpn/_control.py b/src/leap/bitmask/vpn/_control.py index a57c6e47..7f80f40b 100644 --- a/src/leap/bitmask/vpn/_control.py +++ b/src/leap/bitmask/vpn/_control.py @@ -66,17 +66,23 @@ class VPNControl(object): vpnproc.preUp() except Exception as e: self.log.error('Error on vpn pre-up {0!r}'.format(e)) - raise + return False try: cmd = vpnproc.getCommand() except Exception as e: self.log.error( 'Error while getting vpn command... {0!r}'.format(e)) - raise + return False env = os.environ - runningproc = reactor.spawnProcess(vpnproc, cmd[0], cmd, env) + try: + runningproc = reactor.spawnProcess(vpnproc, cmd[0], cmd, env) + except Exception as e: + self.log.error( + 'Error while spwanning vpn process... {0!r}'.format(e)) + return False + vpnproc.pid = runningproc.pid self._vpnproc = vpnproc diff --git a/src/leap/bitmask/vpn/service.py b/src/leap/bitmask/vpn/service.py index 70c26248..075295cf 100644 --- a/src/leap/bitmask/vpn/service.py +++ b/src/leap/bitmask/vpn/service.py @@ -24,9 +24,12 @@ import os from time import strftime from twisted.internet import defer +from twisted.logger import Logger from leap.bitmask.hooks import HookableService -from leap.bitmask.vpn.tunnelmanager import TunnelManager +from leap.bitmask.util import merge_status +from leap.bitmask.vpn.fw.firewall import FirewallManager +from leap.bitmask.vpn.tunnel import VPNTunnel 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 @@ -45,6 +48,7 @@ class VPNService(HookableService): name = 'vpn' _last_vpn_path = os.path.join('leap', 'last_vpn') + log = Logger() def __init__(self, basepath=None): """ @@ -52,8 +56,8 @@ class VPNService(HookableService): """ super(VPNService, self).__init__() - self._started = False - self._tunnelmanager = None + self._tunnel = None + self._firewall = None self._domain = '' if basepath is None: @@ -75,7 +79,7 @@ class VPNService(HookableService): @defer.inlineCallbacks def start_vpn(self, domain=None): - if self._started: + if self.do_status()['status'] == 'on': exc = Exception('VPN already started') exc.expected = True raise exc @@ -89,7 +93,14 @@ class VPNService(HookableService): yield self._setup(domain) try: - started = self._tunnelmanager.start() + fw_ok = self._firewall.start() + if not fw_ok: + raise Exception('Could not start firewall') + + vpn_ok = self._tunnel.start() + if not vpn_ok: + self._firewall.stop() + raise Exception('Could not start VPN') # XXX capture it inside start method # here I'd like to get (status, message) @@ -98,37 +109,36 @@ class VPNService(HookableService): raise e # -------------------------------------- - self._started = started self._domain = domain self._write_last(domain) - if started: - defer.returnValue({'result': 'started'}) - else: - raise Exception('Could not start VPN, check logs') + defer.returnValue({'result': 'started'}) def stop_vpn(self): - if not self._tunnelmanager: - raise Exception('VPN was not running') + if self._firewall and self._firewall.is_up(): + fw_ok = self._firewall.stop() + if not fw_ok: + self.log.error("Firewall: error stopping") - if self._started: - self._tunnelmanager.stop() - self._started = False - return {'result': 'vpn stopped'} - elif self._tunnelmanager.is_firewall_up(): - self._tunnelmanager.stop_firewall() - return {'result': 'firewall stopped'} - else: + if not self._tunnel or self._tunnel.status['status'] is not 'on': raise Exception('VPN was not running') + vpn_ok = self._tunnel.stop() + if not vpn_ok: + self.log.error("VPN: error stopping") + + return {'result': 'vpn stopped'} + def do_status(self): - status = { - 'status': 'off', - 'error': None, - 'childrenStatus': {} + childrenStatus = { + 'vpn': {'status': 'off', 'error': None}, + 'firewall': {'status': 'off', 'error': None}, } - if self._tunnelmanager: - status = self._tunnelmanager.get_status() + if self._tunnel: + childrenStatus['vpn'] = self._tunnel.status + if self._firewall: + childrenStatus['firewall'] = self._firewall.status + status = merge_status(childrenStatus) if self._domain: status['domain'] = self._domain @@ -202,8 +212,9 @@ class VPNService(HookableService): 'Cannot find provider certificate. ' 'Please configure provider.') - self._tunnelmanager = TunnelManager( + self._tunnel = VPNTunnel( provider, remotes, cert_path, key_path, ca_path, extra_flags) + self._firewall = FirewallManager(remotes) def _cert_expires(self, provider): path = os.path.join(self._basepath, "leap", "providers", provider, diff --git a/src/leap/bitmask/vpn/tunnelmanager.py b/src/leap/bitmask/vpn/tunnelmanager.py deleted file mode 100644 index 4fb50047..00000000 --- a/src/leap/bitmask/vpn/tunnelmanager.py +++ /dev/null @@ -1,115 +0,0 @@ -#!/usr/bin/env python -# -*- coding: utf-8 -*- -# cli.py -# Copyright (C) 2015 LEAP -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -from colorama import Fore - -from leap.bitmask.util import merge_status - -from leap.bitmask.vpn.fw.firewall import FirewallManager -from leap.bitmask.vpn.tunnel import VPNTunnel - - -# TODO further refactor pending: merge with VPNService? - - -class TunnelManager(object): - - """ - A TunnelManager controls a VPNTunnel and the Firewall - """ - - def __init__(self, provider, remotes, cert, key, ca, flags): - - self._vpntunnel = VPNTunnel( - provider, remotes, cert, key, ca, flags) - self._firewall = FirewallManager(remotes) - self.starting = False - - def start(self): - # TODO we should have some way of switching this flag to False - # other than parsing the result of the status command. - self.starting = True - print(Fore.BLUE + "Firewall: starting..." + Fore.RESET) - fw_ok = self._firewall.start() - if not fw_ok: - print(Fore.RED + "Firewall: problem!") - self.starting = False - return False - print(Fore.GREEN + "Firewall: started" + Fore.RESET) - - try: - vpn_ok = self._vpntunnel.start() - except Exception: - self.starting = False - return False - - if not vpn_ok: - print (Fore.RED + "VPN: Error starting." + Fore.RESET) - self._firewall.stop() - print(Fore.GREEN + "Firewall: stopped." + Fore.RESET) - self.starting = False - return False - print(Fore.GREEN + "VPN: started" + Fore.RESET) - return True - - def stop(self): - self.starting = False - print(Fore.BLUE + "Firewall: stopping..." + Fore.RESET) - fw_ok = self._firewall.stop() - - if not fw_ok: - print (Fore.RED + "Firewall: Error stopping." + Fore.RESET) - return False - - print(Fore.GREEN + "Firewall: stopped." + Fore.RESET) - print(Fore.BLUE + "VPN: stopping..." + Fore.RESET) - - vpn_ok = self._vpntunnel.stop() - if not vpn_ok: - print (Fore.RED + "VPN: Error stopping." + Fore.RESET) - return False - - print(Fore.GREEN + "VPN: stopped." + Fore.RESET) - return True - - def stop_firewall(self): - self._firewall.stop() - - def is_firewall_up(self): - return self._firewall.is_up() - - def get_status(self): - childrenStatus = { - "vpn": self._vpntunnel.status, - "firewall": self._firewall.status - } - if self.starting: - # XXX small correction to the merge: if we are starting fw+vpn, - # we report vpn as starting so that is consistent with the ui or - # cli action. this state propagates from the parent - # object to the vpn child, and we revert it when we reach - # the 'on' state. this needs to be revisited in the formal state - # machine, and mainly needs a way of setting that state directly - # and resetting the 'starting' flag without resorting to hijack - # this command. - vpnstatus = childrenStatus['vpn']['status'] - if vpnstatus == 'off': - childrenStatus['vpn']['status'] = 'starting' - if vpnstatus == 'on': - self.starting = False - return merge_status(childrenStatus) -- cgit v1.2.3