several fixes for riseupvpn snap

- move snap folder to root folder, to allow automated builds
- install only polkit file outside of snap
- change path of bitmask-root accordingly in bitmask
- fix check for polkit file inside snap
- change and document the algorithm for picking bitmask-root in linux
- add LD_LIBRARY_PATH as an environment entry for bitmask-systray

6 files changed, 77 insertions, 18 deletions

diff --git a/src/leap/bitmask/vpn/constants.py b/src/leap/bitmask/vpn/constants.py
index dce5e81c..a380bc3c 100644
--- a/src/leap/bitmask/vpn/constants.py
+++ b/src/leap/bitmask/vpn/constants.py
@@ -23,10 +23,15 @@ from leap.bitmask.system import IS_LINUX
 if IS_LINUX:
     BITMASK_ROOT_SYSTEM = '/usr/sbin/bitmask-root'
     BITMASK_ROOT_LOCAL = '/usr/local/sbin/bitmask-root'
+    # this should change when bitmask is also a snap. for now,
+    # snap is only RiseupVPN
+    BITMASK_ROOT_SNAP = '/snap/bin/riseup-vpn.bitmask-root'
+
     OPENVPN_SYSTEM = '/usr/sbin/openvpn'
     OPENVPN_LOCAL = '/usr/local/sbin/leap-openvpn'
     # this should change when bitmask is also a snap. for now,
-    # snap means RiseupVPN
+    # snap is only RiseupVPN
     OPENVPN_SNAP = '/snap/bin/riseup-vpn.openvpn'
     POLKIT_LOCAL = '/usr/share/polkit-1/actions/se.leap.bitmask.bundle.policy'
     POLKIT_SYSTEM = '/usr/share/polkit-1/actions/se.leap.bitmask.policy'
+    POLKIT_SNAP = '/usr/share/polkit-1/actions/se.leap.bitmask.riseupvpn.policy'
diff --git a/src/leap/bitmask/vpn/fw/firewall.py b/src/leap/bitmask/vpn/fw/firewall.py
index 98b317b0..cc5d76d0 100644
--- a/src/leap/bitmask/vpn/fw/firewall.py
+++ b/src/leap/bitmask/vpn/fw/firewall.py
@@ -25,7 +25,10 @@ import subprocess
 
 from twisted.logger import Logger
 
-from leap.bitmask.system import IS_MAC, IS_LINUX
+from leap.bitmask.system import IS_MAC, IS_LINUX, IS_SNAP
+from leap.bitmask.vpn.constants import BITMASK_ROOT_SYSTEM
+from leap.bitmask.vpn.constants import BITMASK_ROOT_LOCAL
+from leap.bitmask.vpn.constants import BITMASK_ROOT_SNAP
 from leap.common.events import catalog, emit_async
 
 from leap.bitmask.vpn.launchers import darwin
@@ -87,11 +90,23 @@ class _LinuxFirewallManager(object):
     This allows us to achieve fail close on a vpn connection.
     """
 
-    _SYSTEM_BITMASK_ROOT = '/usr/sbin/bitmask-root'
-    if os.path.isfile(_SYSTEM_BITMASK_ROOT):
-        BITMASK_ROOT = _SYSTEM_BITMASK_ROOT
+    # TODO factor out choosing a version of bitmask-root.
+    # together with linux vpnlauncher.
+
+    if IS_SNAP:
+        # snap has its own version under /snap
+        BITMASK_ROOT = BITMASK_ROOT_SNAP
+    elif IS_STANDALONE and os.path.isfile(BITMASK_ROOT_LOCAL):
+        # if this is a bundle, we pick local. bundles ask to install it there.
+        BITMASK_ROOT = BITMASK_ROOT_LOCAL
     else:
-        BITMASK_ROOT = "/usr/local/sbin/bitmask-root"
+        if os.path.isfile(BITMASK_ROOT_SYSTEM):
+            # we can be running from the debian package,
+            # or some other distro. it's the maintainer responsibility to put bitmask-root there.
+            BITMASK_ROOT = BITMASK_ROOT_SYSTEM
+        else:
+            # as a last case, we fall back to installing into the /usr/local/sbin version.
+            BITMASK_ROOT = BITMASK_ROOT_LOCAL
 
     def __init__(self, remotes):
         """
diff --git a/src/leap/bitmask/vpn/helpers/__init__.py b/src/leap/bitmask/vpn/helpers/__init__.py
index e932422d..96ec4f2e 100644
--- a/src/leap/bitmask/vpn/helpers/__init__.py
+++ b/src/leap/bitmask/vpn/helpers/__init__.py
@@ -18,13 +18,15 @@ if IS_LINUX:
 
     from leap.bitmask.vpn.constants import BITMASK_ROOT_SYSTEM
     from leap.bitmask.vpn.constants import BITMASK_ROOT_LOCAL
+    from leap.bitmask.vpn.constants import BITMASK_ROOT_SNAP
     from leap.bitmask.vpn.constants import OPENVPN_SYSTEM, OPENVPN_LOCAL
     from leap.bitmask.vpn.constants import OPENVPN_SNAP
-    from leap.bitmask.vpn.constants import POLKIT_SYSTEM, POLKIT_LOCAL
+    from leap.bitmask.vpn.constants import POLKIT_SYSTEM, POLKIT_LOCAL, POLKIT_SNAP
     from leap.bitmask.vpn.privilege import is_pkexec_in_system
     from leap.bitmask.vpn.privilege import LinuxPolicyChecker
 
     def install():
+        print('installing bitmask helpers...')
         helper_from = _config.get_bitmask_helper_path()
         polkit_from = _config.get_bitmask_polkit_policy_path()
         openvpn_from = _config.get_bitmask_openvpn_path()
@@ -86,7 +88,7 @@ if IS_LINUX:
             return True
 
         if IS_SNAP:
-            if os.path.isfile(BITMASK_ROOT_LOCAL):
+            if os.path.isfile(BITMASK_ROOT_SNAP):
                 return True
             log.error('Cannot find bitmask-root in snap')
             return False
@@ -131,12 +133,17 @@ if IS_LINUX:
         return False
 
     def _check_polkit_file_exist():
-        # XXX: we are just checking if there is any policy file installed not
-        # if it's valid or if it's the correct one that will be used.
-        # (if LOCAL is used if /usr/local/sbin/bitmask-root is used and SYSTEM
-        # if /usr/sbin/bitmask-root)
+        """
+        We are just checking if there is any policy file installed not
+        if it's valid or if it's the correct one that will be used.
+
+        If LOCAL: we use /usr/local/sbin/bitmask-root
+        If SYSTEM: we use /usr/sbin/bitmask-root, and
+        if SNAP:   we use /snap/bin/riseup-vpn.bitmask-root
+        """
         return (os.path.exists(POLKIT_LOCAL) or
-                os.path.exists(POLKIT_SYSTEM))
+                os.path.exists(POLKIT_SYSTEM) or
+                os.path.exists(POLKIT_SNAP))
 
     def _exists_and_can_read(file_path):
         return access(file_path, R_OK)
diff --git a/src/leap/bitmask/vpn/helpers/linux/se.leap.bitmask.riseupvpn.policy b/src/leap/bitmask/vpn/helpers/linux/se.leap.bitmask.riseupvpn.policy
new file mode 100644
index 00000000..3bdf06a6
--- /dev/null
+++ b/src/leap/bitmask/vpn/helpers/linux/se.leap.bitmask.riseupvpn.policy
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE policyconfig PUBLIC
+ "-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/PolicyKit/1/policyconfig.dtd">
+<policyconfig>
+
+  <vendor>LEAP Project</vendor>
+  <vendor_url>http://leap.se/</vendor_url>
+
+  <action id="se.leap.bitmask.riseupvpn.policy">
+    <description>Runs bitmask helper to launch firewall and openvpn (RiseupVPN)</description>
+    <description xml:lang="es">Ejecuta el asistente de bitmask para lanzar el firewall y openvpn (RiseupVPN)</description>
+    <message>RiseupVPN needs that you authenticate to start</message>
+    <message xml:lang="es">RiseupVPN necesita autorizacion para comenzar</message>
+    <icon_name>package-x-generic</icon_name> 
+    <defaults>
+      <allow_any>yes</allow_any>
+      <allow_inactive>yes</allow_inactive>
+      <allow_active>yes</allow_active>
+    </defaults>
+    <annotate key="org.freedesktop.policykit.exec.path">/snap/bin/riseup-vpn.bitmask-root</annotate>
+  </action>
+</policyconfig>
diff --git a/src/leap/bitmask/vpn/launchers/linux.py b/src/leap/bitmask/vpn/launchers/linux.py
index 748fe858..db8ffce7 100644
--- a/src/leap/bitmask/vpn/launchers/linux.py
+++ b/src/leap/bitmask/vpn/launchers/linux.py
@@ -86,15 +86,16 @@ class LinuxVPNLauncher(VPNLauncher):
 
     class BITMASK_ROOT(object):
         def __call__(self):
-
             current_version = self._version(_config.get_bitmask_helper_path())
             _sys = constants.BITMASK_ROOT_SYSTEM
             _sys_version = 0
             _local = constants.BITMASK_ROOT_LOCAL
             _local_version = 0
+            _snap = constants.BITMASK_ROOT_SNAP
+            _snap_version = 0
 
             if IS_SNAP:
-                return _local
+                return _snap
 
             if os.path.isfile(_sys):
                 _sys_version = self._version(_sys)
@@ -182,13 +183,22 @@ class LinuxVPNLauncher(VPNLauncher):
             openvpn_verb)
 
         if IS_SNAP:
-            return ["pkexec", "/usr/local/sbin/bitmask-root",
+            # cannot reference bitmask_root because 'local variable command
+            # referenced before assignment' XXX bug!
+            # this should change when bitmask is also a snap. for now,
+            # snap means RiseupVPN
+            return ["pkexec", constants.BITMASK_ROOT_SNAP,
                     "openvpn", "start"] + command
 
-        command.insert(0, force_eval(kls.BITMASK_ROOT))
+        bitmask_root = force_eval(kls.BITMASK_ROOT)
+        command.insert(0, bitmask_root)
         command.insert(1, "openvpn")
         command.insert(2, "start")
 
+        # this is a workaround for integration tests, since it's not
+        # trivial to run polkit inside docker containers.
+        # however, you might want to run bitmask as root under certain
+        # environments, like embedded devices.
         if os.getuid() != 0:
             policyChecker = LinuxPolicyChecker()
             pkexec = policyChecker.get_usable_pkexec()
diff --git a/src/leap/bitmask/vpn/process.py b/src/leap/bitmask/vpn/process.py
index 17d8fddc..d1d929d7 100644
--- a/src/leap/bitmask/vpn/process.py
+++ b/src/leap/bitmask/vpn/process.py
@@ -293,7 +293,6 @@ class _VPNProcess(protocol.ProcessProtocol):
 
         :rtype: list of str
         """
-        print("GETTING COMMAND", self._launcher)
         try:
             command = self._launcher.get_vpn_command(
                 vpnconfig=self._vpnconfig,