Add lockfile tests; move utility logic out of entrypoints

launcher/sdw-launcher.py

@@ -1,84 +1,25 @@
 #!/usr/bin/env python3
-from logging.handlers import TimedRotatingFileHandler
 from PyQt4 import QtGui
 from sdw_updater_gui.UpdaterApp import UpdaterApp
-import fcntl
+from sdw_updater_gui.Updater import configure_logging, obtain_lock
 import logging
-import os
 import sys
 
-# This script is run as a user, so it does not use /run, which requires root
-# access
-LOCK_FILE = os.path.join("/run/user", str(os.getuid()), "sdw-launcher.lock")
-DEFAULT_HOME = os.path.join(os.path.expanduser("~"), ".securedrop_launcher")
-
-logger = ""  # Global logger object, configured later
-lock_handle = ""  # File handle for lockfile, must be kept open during execution
-
 
 def main():
-    global logger
-    logger = logging.getLogger(__name__)
-    obtain_lock()
-    logger.info("Starting SecureDrop Launcher")
+    sdlog = logging.getLogger(__name__)
     configure_logging()
+    lock_handle = obtain_lock()  # noqa: F841
+    if lock_handle is None:
+        # Preflight updater already running or problems accessing lockfile.
+        # Logged.
+        sys.exit(1)
+    sdlog.info("Starting SecureDrop Launcher")
     app = QtGui.QApplication(sys.argv)
     form = UpdaterApp()
     form.show()
     sys.exit(app.exec_())
 
 
-def obtain_lock():
-    """
-    Obtain an exclusive lock to ensure that only one updater can run at a time,
-    and to inform other processes that it is running.
-    """
-    global lock_handle
-
-    # Attempt to obtain a file handle for the lockfile
-    try:
-        lock_handle = open(LOCK_FILE, 'w')
-    except IOError:
-        logger.error("Error obtaining write access to lock file {}\n"
-                     "User may lack required permissions. Exiting."
-                     .format(LOCK_FILE))
-        sys.exit(1)
-
-    # Attempt to obtain an exlusive, nonblocking lock
-    try:
-        fcntl.lockf(lock_handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
-    except IOError:
-        logger.error("Error obtaining lock on {}\n"
-                     "Launcher may already be running. Exiting."
-                     .format(LOCK_FILE))
-        sys.exit(1)
-
-
-def configure_logging():
-    """
-    All logging related settings are set up by this function.
-    """
-    log_folder = os.path.join(DEFAULT_HOME, "logs")
-    if not os.path.exists(log_folder):
-        os.makedirs(log_folder)
-
-    log_file = os.path.join(DEFAULT_HOME, "logs", "launcher.log")
-
-    # set logging format
-    log_fmt = (
-        "%(asctime)s - %(name)s:%(lineno)d(%(funcName)s) " "%(levelname)s: %(message)s"
-    )
-    formatter = logging.Formatter(log_fmt)
-
-    handler = TimedRotatingFileHandler(log_file)
-    handler.setFormatter(formatter)
-    handler.setLevel(logging.INFO)
-
-    # set up primary log
-    log = logging.getLogger()
-    log.setLevel(logging.INFO)
-    log.addHandler(handler)
-
-
 if __name__ == "__main__":
     main()

launcher/sdw-notify.py

@@ -5,6 +5,8 @@
 not stdout. All settings are in Notify utility module.
 """
 
+import sys
+
 from sdw_notify import Notify
 from PyQt4 import QtGui
 from PyQt4.QtGui import QMessageBox
@@ -18,9 +20,16 @@ def main():
     """
 
     Notify.configure_logging()
-    Notify.obtain_and_release_updater_lock()
+    if(Notify.can_obtain_updater_lock() is False):
+        # Preflight updater is already running. Logged.
+        sys.exit(1)
+
     # Hold on to lock handle during execution
     lock_handle = Notify.obtain_notify_lock()  # noqa: F841
+    if lock_handle is None:
+        # Can't write to lockfile or notifier already running. Logged.
+        sys.exit(1)
+
     if Notify.warning_should_be_shown():
         show_update_warning()
 

launcher/sdw_notify/Notify.py

@@ -46,27 +46,29 @@
 WARNING_THRESHOLD = 432000  # 5 days
 
 
-def obtain_and_release_updater_lock():
+def can_obtain_updater_lock():
     """
     We temporarily obtain a shared, nonblocking lock to the updater's lock
     file to determine whether it is currently running. We do not need to
-    hold on to this lock.
+    hold on to this lock. Returns True if it is safe to continue execution,
+    False if not.
     """
-
     try:
         lh = open(LOCK_FILE_LAUNCHER, 'r')
     except FileNotFoundError:  # noqa: F821
         # Updater may not have run yet during this session
-        return
+        return True
 
     try:
         # Obtain a nonblocking, shared lock
         fcntl.lockf(lh, fcntl.LOCK_SH | fcntl.LOCK_NB)
     except IOError:
         sdlog.error("Error obtaining lock on '{}'. "
-                    "Preflight updater may already be running. Exiting."
+                    "Preflight updater may already be running."
                     .format(LOCK_FILE_LAUNCHER))
-        sys.exit(1)
+        return False
+
+    return True
 
 
 def obtain_notify_lock():
@@ -79,16 +81,17 @@ def obtain_notify_lock():
         sdlog.error("Error writing to lock file '{}'. User may lack the "
                     "required permissions."
                     .format(LOCK_FILE_NOTIFIER))
-        sys.exit(1)
+        return None
 
     try:
         # Obtain an exclusive, nonblocking lock
         fcntl.lockf(lh, fcntl.LOCK_EX | fcntl.LOCK_NB)
     except IOError:
         sdlog.error("Error obtaining lock on '{}'. "
-                    "Notification may already be displaying. Exiting."
+                    "Notification may already be displaying."
                     .format(LOCK_FILE_NOTIFIER))
-        sys.exit(1)
+        return None
+
     return lh
 
 

launcher/sdw_updater_gui/Updater.py

@@ -7,18 +7,22 @@
 from the parent directory.
 """
 
+import fcntl
 import json
 import logging
 import os
 import subprocess
 from datetime import datetime, timedelta
 from enum import Enum
+from logging.handlers import TimedRotatingFileHandler
 
 DATE_FORMAT = "%Y-%m-%d %H:%M:%S"
+DEFAULT_HOME = ".securedrop_launcher"
 FLAG_FILE_STATUS_SD_APP = "/home/user/.securedrop_client/sdw-update-status"
 FLAG_FILE_LAST_UPDATED_SD_APP = "/home/user/.securedrop_client/sdw-last-updated"
-FLAG_FILE_STATUS_DOM0 = ".securedrop_launcher/sdw-update-status"
-FLAG_FILE_LAST_UPDATED_DOM0 = ".securedrop_launcher/sdw-last-updated"
+FLAG_FILE_STATUS_DOM0 = os.path.join(DEFAULT_HOME, "sdw-update-status")
+FLAG_FILE_LAST_UPDATED_DOM0 = os.path.join(DEFAULT_HOME, "sdw-last-updated")
+LOCK_FILE = os.path.join("/run/user", str(os.getuid()), "sdw-launcher.lock")
 
 sdlog = logging.getLogger(__name__)
 
@@ -90,6 +94,58 @@ def apply_updates(vms):
     _shutdown_and_start_vms()
 
 
+def configure_logging():
+    """
+    All logging related settings are set up by this function.
+    """
+    log_folder = os.path.join(get_dom0_path(DEFAULT_HOME), "logs")
+    if not os.path.exists(log_folder):
+        os.makedirs(log_folder)
+
+    log_file = os.path.join(get_dom0_path(DEFAULT_HOME), "logs", "launcher.log")
+
+    # set logging format
+    log_fmt = (
+        "%(asctime)s - %(name)s:%(lineno)d(%(funcName)s) " "%(levelname)s: %(message)s"
+    )
+    formatter = logging.Formatter(log_fmt)
+
+    handler = TimedRotatingFileHandler(log_file)
+    handler.setFormatter(formatter)
+    handler.setLevel(logging.INFO)
+
+    # set up primary log
+    log = logging.getLogger()
+    log.setLevel(logging.INFO)
+    log.addHandler(handler)
+
+
+def obtain_lock():
+    """
+    Obtain an exclusive lock to ensure that only one updater can run at a time,
+    and to inform other processes that it is running.
+    """
+    # Attempt to obtain a file handle for the lockfile
+    try:
+        lock_handle = open(LOCK_FILE, 'w')
+    except IOError:
+        sdlog.error("Error obtaining write access to lock file {}\n"
+                    "User may lack required permissions. Exiting."
+                    .format(LOCK_FILE))
+        return None
+
+    # Attempt to obtain an exlusive, nonblocking lock
+    try:
+        fcntl.lockf(lock_handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
+    except IOError:
+        sdlog.error("Error obtaining lock on {}\n"
+                    "Launcher may already be running. Exiting."
+                    .format(LOCK_FILE))
+        return None
+
+    return lock_handle
+
+
 def _check_updates_dom0():
     """
     Check for dom0 updates

launcher/tests/test_notify.py

@@ -3,14 +3,16 @@
 
 from unittest import mock
 from importlib.machinery import SourceFileLoader
+from multiprocessing import Pool
 from tempfile import TemporaryDirectory
-relpath_notify = "../sdw_notify/Notify.py"
-path_to_script = os.path.join(
-    os.path.dirname(os.path.abspath(__file__)), relpath_notify
-)
 
+relpath_notify = "../sdw_notify/Notify.py"
+path_to_notify = os.path.join(os.path.dirname(os.path.abspath(__file__)), relpath_notify)
+notify = SourceFileLoader("Notify", path_to_notify).load_module()
 
-notify = SourceFileLoader("Notify", path_to_script).load_module()
+relpath_updater = "../sdw_updater_gui/Updater.py"
+path_to_updater = os.path.join(os.path.dirname(os.path.abspath(__file__)), relpath_updater)
+updater = SourceFileLoader("Updater", path_to_updater).load_module()
 
 
 @mock.patch("Notify.sdlog.error")
@@ -39,4 +41,36 @@ def test_notify_lock(mocked_info, mocked_error):
         assert lsof_data[3].find('W') != -1
 
 
-test_notify_lock()
+@mock.patch("Notify.sdlog.error")
+@mock.patch("Notify.sdlog.info")
+def test_updater_lock_prevents_notifier(mocked_info, mocked_error):
+    """
+    Test whether an exlusive lock on the updater lock file prevents the notifier
+    from launching (so it does not come up when the user is in the process of
+    updating).
+    """
+    with TemporaryDirectory() as tmpdir:
+        notify.LOCK_FILE_LAUNCHER = os.path.join(tmpdir, "sdw-launcher.lock")
+        updater.LOCK_FILE = os.path.join(tmpdir, "sdw-launcher.lock")
+        lh = updater.obtain_lock()  # noqa: F841
+
+        # We're in the same process, so obtaining an additional lock would
+        # always succeed. We use the multiprocessing module to run the function
+        # as a separate process.
+        p = Pool(processes=1)
+        lock_result = p.apply(notify.can_obtain_updater_lock)
+        p.close()
+        assert lock_result is False
+
+
+@mock.patch("Notify.sdlog.error")
+@mock.patch("Notify.sdlog.info")
+def test_no_updater_lock_has_no_effect(mocked_info, mocked_error):
+    """
+    Test whether we _can_ run the notifier when we don't have a lock
+    on the updater.
+    """
+    with TemporaryDirectory() as tmpdir:
+        notify.LOCK_FILE_LAUNCHER = os.path.join(tmpdir, "sdw-launcher.lock")
+        lock_result = notify.can_obtain_updater_lock()
+        assert lock_result is True