From 7a2538979549971431a25b811ebe0e3b0e73e762 Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Fri, 7 Feb 2020 10:51:35 -0800 Subject: [PATCH 01/11] adding argument to launcher desktop file to specify time delta for skipping updates --- config.json.example | 3 ++- dom0/sd-dom0-files.sls | 19 +++++++++++-------- ...desktop => securedrop-launcher.desktop.j2} | 2 +- 3 files changed, 14 insertions(+), 10 deletions(-) rename dom0/{securedrop-launcher.desktop => securedrop-launcher.desktop.j2} (59%) diff --git a/config.json.example b/config.json.example index 577f0c4c..f2845294 100644 --- a/config.json.example +++ b/config.json.example @@ -4,7 +4,8 @@ "hostname": "avgfxawdn6c3coe3.onion", "key": "Il8Xas7uf6rjtc0LxYwhrx" }, - "environment": "prod", + "environment": "prod", + "update_skip_delta": 28800, "vmsizes": { "sd_app": 10, "sd_log": 5 diff --git a/dom0/sd-dom0-files.sls b/dom0/sd-dom0-files.sls index 7af55fb9..b6873034 100644 --- a/dom0/sd-dom0-files.sls +++ b/dom0/sd-dom0-files.sls @@ -190,14 +190,6 @@ dom0-securedrop-launcher-executables: - mode: 755 - replace: false -dom0-securedrop-launcher-desktop-shortcut: - file.managed: - - name: /home/{{ gui_user }}/Desktop/securedrop-launcher.desktop - - source: "salt://securedrop-launcher.desktop" - - user: {{ gui_user }} - - group: {{ gui_user }} - - mode: 755 - {% import_json "sd/config.json" as d %} {% if d.environment == "dev" %} dom0-remove-securedrop-workstation-dom0-config: @@ -215,3 +207,14 @@ dom0-install-securedrop-workstation-dom0-config: - file: dom0-workstation-rpm-repo {% endif %} + +dom0-securedrop-launcher-desktop-shortcut: + file.managed: + - name: /home/{{ gui_user }}/Desktop/securedrop-launcher.desktop + - source: "salt://securedrop-launcher.desktop.j2" + - template: jinja + - context: + update_skip_delta: {{ d.update_skip_delta }} + - user: {{ gui_user }} + - group: {{ gui_user }} + - mode: 755 diff --git a/dom0/securedrop-launcher.desktop b/dom0/securedrop-launcher.desktop.j2 similarity index 59% rename from dom0/securedrop-launcher.desktop rename to dom0/securedrop-launcher.desktop.j2 index 3df2e49f..89d2dad8 100644 --- a/dom0/securedrop-launcher.desktop +++ b/dom0/securedrop-launcher.desktop.j2 @@ -4,4 +4,4 @@ Type=Application Terminal=false Icon=/usr/share/securedrop/icons/sd-logo.png Name=SecureDrop -Exec=/opt/securedrop/launcher/sdw-launcher.py +Exec=/opt/securedrop/launcher/sdw-launcher.py --skip-delta={{ update_skip_delta }} From d90890e393d378542869c2c0fd3337ea4fa3f26d Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Fri, 7 Feb 2020 12:09:42 -0800 Subject: [PATCH 02/11] Added check for last update in launcher script --- launcher/sdw-launcher.py | 42 +++++++++++++++---- launcher/sdw_updater_gui/Updater.py | 57 ++++++++++++++++++++++++++ launcher/sdw_updater_gui/UpdaterApp.py | 14 ++++++- 3 files changed, 105 insertions(+), 8 deletions(-) diff --git a/launcher/sdw-launcher.py b/launcher/sdw-launcher.py index 5d67ee3f..7c691da8 100644 --- a/launcher/sdw-launcher.py +++ b/launcher/sdw-launcher.py @@ -3,12 +3,32 @@ from sdw_updater_gui.UpdaterApp import UpdaterApp from sdw_util import Util from sdw_updater_gui import Updater - +from sdw_updater_gui.UpdaterApp import launch_securedrop_client +from sdw_updater_gui.Updater import should_launch_updater import logging import sys +import argparse + + + +def parse_argv(argv): + parser = argparse.ArgumentParser() + parser.add_argument("--skip-delta", type=int, default=0) + return parser.parse_args(argv) + + +def launch_updater(): + """ + Start the updater GUI + """ + + app = QtGui.QApplication(sys.argv) + form = UpdaterApp() + form.show() + sys.exit(app.exec_()) -def main(): +def main(argv): sdlog = logging.getLogger(__name__) Util.configure_logging(Updater.LOG_FILE) lock_handle = Util.obtain_lock(Updater.LOCK_FILE) @@ -17,11 +37,19 @@ def main(): # Logged. sys.exit(1) sdlog.info("Starting SecureDrop Launcher") - app = QtGui.QApplication(sys.argv) - form = UpdaterApp() - form.show() - sys.exit(app.exec_()) + sdlog = logging.getLogger(__name__) + configure_logging() + sdlog.info("Starting SecureDrop Launcher") + + interval = DEFAULT_INTERVAL + args = parse_argv(argv) + if args.skip_delta: + interval = int(args.skip_delta) + if should_launch_updater(interval): + launch_updater() + else: + launch_securedrop_client() if __name__ == "__main__": - main() + main(sys.argv[1:]) diff --git a/launcher/sdw_updater_gui/Updater.py b/launcher/sdw_updater_gui/Updater.py index 40f4f9fd..21f1a780 100644 --- a/launcher/sdw_updater_gui/Updater.py +++ b/launcher/sdw_updater_gui/Updater.py @@ -430,6 +430,63 @@ def _safely_start_vm(vm): sdlog.error(str(e)) +def should_launch_updater(interval): + sdlog.info("Starting SecureDrop Launcher") + + status = read_dom0_update_flag_from_disk(with_timestamp=True) + + if _valid_status(status): + if _interval_expired(interval, status): + sdlog.info("Update interval expired: launching updater.") + return True + else: + if _status_ok_or_rebooted(status): + sdlog.info("Update interval not expired, launching client.") + return False + else: + sdlog.info("Updates or reboot required, launching updater.") + return True + else: + sdlog.info("Update status not available, launching updater.") + return True + + +def _valid_status(status): + """ + status should contain 2 items, the update flag and a timestamp. + """ + if isinstance(status, dict) and len(status) == 2: + return True + return False + + +def _interval_expired(interval, status): + """ + Check if specified update interval has expired. + """ + + try: + update_time = datetime.strptime(status['last_status_update'], DATE_FORMAT) + except ValueError: + # Broken timestamp? run the updater. + return True + if ((datetime.now() - update_time) < timedelta(seconds=interval)): + return False + return True + + +def _status_ok_or_rebooted(status): + """ + Check if update status is OK or post-reboot. + """ + + if (status['status'] == UpdateStatus.UPDATES_OK.value or # noqa W504 + (status['status'] == UpdateStatus.REBOOT_REQUIRED.value and # noqa W504 + last_required_reboot_performed())): + return True + return False + + class UpdateStatus(Enum): """ Standardizes return codes for update/upgrade methods diff --git a/launcher/sdw_updater_gui/UpdaterApp.py b/launcher/sdw_updater_gui/UpdaterApp.py index 07f7a7d3..ad685b43 100644 --- a/launcher/sdw_updater_gui/UpdaterApp.py +++ b/launcher/sdw_updater_gui/UpdaterApp.py @@ -10,6 +10,18 @@ logger = logging.getLogger(__name__) +def launch_securedrop_client(): + """ + Helper function to launch the SecureDrop Client + """ + try: + logger.info("Launching SecureDrop client") + subprocess.Popen(["qvm-run", "sd-app", "gtk-launch securedrop-client"]) + except subprocess.CalledProcessError as e: + self.proposedActionDescription.setText(strings.descri) + logger.error("Error while launching SecureDrop client") + logger.error(str(e)) + sys.exit(0) class UpdaterApp(QtGui.QMainWindow, Ui_UpdaterDialog): def __init__(self, parent=None): @@ -19,7 +31,7 @@ def __init__(self, parent=None): self.setupUi(self) self.clientOpenButton.setEnabled(False) self.clientOpenButton.hide() - self.clientOpenButton.clicked.connect(self.launch_securedrop_client) + self.clientOpenButton.clicked.connect(launch_securedrop_client) self.rebootButton.setEnabled(False) self.rebootButton.hide() self.rebootButton.clicked.connect(self.reboot_workstation) From 591ca58431bfca0574d59001bff55243dcb702b5 Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Tue, 11 Feb 2020 10:25:49 -0800 Subject: [PATCH 03/11] Added test coverage for updater-vs.-client logic --- launcher/tests/test_updater.py | 76 ++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/launcher/tests/test_updater.py b/launcher/tests/test_updater.py index d33324f6..bf736197 100644 --- a/launcher/tests/test_updater.py +++ b/launcher/tests/test_updater.py @@ -4,6 +4,7 @@ import subprocess from datetime import datetime from importlib.machinery import SourceFileLoader +from datetime import datetime, timedelta from tempfile import TemporaryDirectory from unittest import mock from unittest.mock import call @@ -840,3 +841,78 @@ def test_last_required_reboot_performed_not_required( result = updater.last_required_reboot_performed() assert result is True assert not mocked_error.called + +@pytest.mark.parametrize("status, expected, rebooted", [ + (UpdateStatus.UPDATES_OK, True, True), + (UpdateStatus.UPDATES_REQUIRED, True, True), + (UpdateStatus.REBOOT_REQUIRED, True, True), + (UpdateStatus.UPDATES_FAILED, True, True), + (UpdateStatus.UPDATES_OK, True, False), + (UpdateStatus.UPDATES_REQUIRED, True, False), + (UpdateStatus.REBOOT_REQUIRED, True, False), + (UpdateStatus.UPDATES_FAILED, True, False) +]) + +# @mock.patch("Updater.last_required_reboot_performed", return_value=True) +def test_should_run_updater_status_interval_expired( + status, expected, rebooted +): + TEST_INTERVAL=3600 + with mock.patch("Updater.last_required_reboot_performed") as mocked_last: + mocked_last.return_value = rebooted + with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: + mocked_read.return_value={ + "last_status_update": str(( + datetime.now() - timedelta(seconds=(TEST_INTERVAL+10) + )).strftime(updater.DATE_FORMAT)), + "status": status.value, + } + # assuming that the tests won't take an hour to run! + assert expected == updater.should_launch_updater(TEST_INTERVAL) + + +@pytest.mark.parametrize("status, expected, rebooted", [ + (UpdateStatus.UPDATES_OK, False, True), + (UpdateStatus.UPDATES_REQUIRED, True, True), + (UpdateStatus.REBOOT_REQUIRED, False, True), + (UpdateStatus.UPDATES_FAILED, True, True), + (UpdateStatus.UPDATES_OK, False, False), + (UpdateStatus.UPDATES_REQUIRED, True, False), + (UpdateStatus.REBOOT_REQUIRED, True, False), + (UpdateStatus.UPDATES_FAILED, True, False) +]) + +def test_should_run_updater_status_interval_not_expired( + status, expected, rebooted +): + TEST_INTERVAL=3600 + with mock.patch("Updater.last_required_reboot_performed") as mocked_last: + mocked_last.return_value = rebooted + with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: + mocked_read.return_value={ + "last_status_update": str(datetime.now().strftime(updater.DATE_FORMAT)), + "status": status.value, + } + # assuming that the tests won't take an hour to run! + assert expected == updater.should_launch_updater(TEST_INTERVAL) + +def test_should_run_updater_invalid_status(): + TEST_INTERVAL=3600 + with mock.patch("Updater.last_required_reboot_performed") as mocked_last: + mocked_last.return_value = True + with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: + mocked_read.return_value={} + # assuming that the tests won't take an hour to run! + assert updater.should_launch_updater(TEST_INTERVAL) == True + +def test_should_run_updater_invalid_timestamp(): + TEST_INTERVAL=3600 + with mock.patch("Updater.last_required_reboot_performed") as mocked_last: + mocked_last.return_value = True + with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: + mocked_read.return_value={ + "last_status_update": "time to die", + "status": UpdateStatus.UPDATES_OK.value, + } + # assuming that the tests won't take an hour to run! + assert updater.should_launch_updater(TEST_INTERVAL) == True From 09b7cb115d9a2829d09696288896c1ddf7837b00 Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Tue, 11 Feb 2020 11:52:33 -0800 Subject: [PATCH 04/11] corrected lint errors --- launcher/sdw-launcher.py | 4 +++ launcher/sdw_updater_gui/UpdaterApp.py | 3 ++- launcher/tests/test_updater.py | 37 +++++++++++++------------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/launcher/sdw-launcher.py b/launcher/sdw-launcher.py index 7c691da8..9318035f 100644 --- a/launcher/sdw-launcher.py +++ b/launcher/sdw-launcher.py @@ -9,6 +9,9 @@ import sys import argparse +DEFAULT_INTERVAL = 0 + + def parse_argv(argv): @@ -51,5 +54,6 @@ def main(argv): else: launch_securedrop_client() + if __name__ == "__main__": main(sys.argv[1:]) diff --git a/launcher/sdw_updater_gui/UpdaterApp.py b/launcher/sdw_updater_gui/UpdaterApp.py index ad685b43..4653bf2e 100644 --- a/launcher/sdw_updater_gui/UpdaterApp.py +++ b/launcher/sdw_updater_gui/UpdaterApp.py @@ -10,6 +10,7 @@ logger = logging.getLogger(__name__) + def launch_securedrop_client(): """ Helper function to launch the SecureDrop Client @@ -18,11 +19,11 @@ def launch_securedrop_client(): logger.info("Launching SecureDrop client") subprocess.Popen(["qvm-run", "sd-app", "gtk-launch securedrop-client"]) except subprocess.CalledProcessError as e: - self.proposedActionDescription.setText(strings.descri) logger.error("Error while launching SecureDrop client") logger.error(str(e)) sys.exit(0) + class UpdaterApp(QtGui.QMainWindow, Ui_UpdaterDialog): def __init__(self, parent=None): super(UpdaterApp, self).__init__(parent) diff --git a/launcher/tests/test_updater.py b/launcher/tests/test_updater.py index bf736197..9fd0c4a9 100644 --- a/launcher/tests/test_updater.py +++ b/launcher/tests/test_updater.py @@ -842,6 +842,7 @@ def test_last_required_reboot_performed_not_required( assert result is True assert not mocked_error.called + @pytest.mark.parametrize("status, expected, rebooted", [ (UpdateStatus.UPDATES_OK, True, True), (UpdateStatus.UPDATES_REQUIRED, True, True), @@ -852,19 +853,18 @@ def test_last_required_reboot_performed_not_required( (UpdateStatus.REBOOT_REQUIRED, True, False), (UpdateStatus.UPDATES_FAILED, True, False) ]) - # @mock.patch("Updater.last_required_reboot_performed", return_value=True) def test_should_run_updater_status_interval_expired( - status, expected, rebooted + status, expected, rebooted ): - TEST_INTERVAL=3600 + TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = rebooted with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: - mocked_read.return_value={ - "last_status_update": str(( - datetime.now() - timedelta(seconds=(TEST_INTERVAL+10) - )).strftime(updater.DATE_FORMAT)), + mocked_read.return_value = { + "last_status_update": str( + (datetime.now() - timedelta(seconds=(TEST_INTERVAL + 10))) + .strftime(updater.DATE_FORMAT)), "status": status.value, } # assuming that the tests won't take an hour to run! @@ -881,38 +881,39 @@ def test_should_run_updater_status_interval_expired( (UpdateStatus.REBOOT_REQUIRED, True, False), (UpdateStatus.UPDATES_FAILED, True, False) ]) - def test_should_run_updater_status_interval_not_expired( - status, expected, rebooted + status, expected, rebooted ): - TEST_INTERVAL=3600 + TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = rebooted with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: - mocked_read.return_value={ + mocked_read.return_value = { "last_status_update": str(datetime.now().strftime(updater.DATE_FORMAT)), "status": status.value, } # assuming that the tests won't take an hour to run! assert expected == updater.should_launch_updater(TEST_INTERVAL) + def test_should_run_updater_invalid_status(): - TEST_INTERVAL=3600 + TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = True with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: - mocked_read.return_value={} + mocked_read.return_value = {} # assuming that the tests won't take an hour to run! - assert updater.should_launch_updater(TEST_INTERVAL) == True + assert updater.should_launch_updater(TEST_INTERVAL) is True + def test_should_run_updater_invalid_timestamp(): - TEST_INTERVAL=3600 + TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = True with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: - mocked_read.return_value={ + mocked_read.return_value = { "last_status_update": "time to die", "status": UpdateStatus.UPDATES_OK.value, - } + } # assuming that the tests won't take an hour to run! - assert updater.should_launch_updater(TEST_INTERVAL) == True + assert updater.should_launch_updater(TEST_INTERVAL) is True From 7005388dbe0668328552d4f8f286e3ddbc5defc1 Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Tue, 11 Feb 2020 12:27:54 -0800 Subject: [PATCH 05/11] updated RPM build config with revised shortcut filename --- MANIFEST.in | 1 - rpm-build/SPECS/securedrop-workstation-dom0-config.spec | 1 - 2 files changed, 2 deletions(-) diff --git a/MANIFEST.in b/MANIFEST.in index 5b2a5b35..5ee78a09 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -5,7 +5,6 @@ include dom0/*.yml include dom0/securedrop-admin include dom0/securedrop-update include dom0/securedrop-login -include dom0/securedrop-launcher.desktop include dom0/securedrop-handle-upgrade include config.json.example include README.md diff --git a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec index fc28a458..cd1bc721 100644 --- a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec +++ b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec @@ -55,7 +55,6 @@ install -m 644 dom0/*.j2 %{buildroot}/srv/salt/ install -m 644 dom0/*.yml %{buildroot}/srv/salt/ install -m 644 dom0/securedrop-update %{buildroot}/srv/salt/ install -m 644 dom0/securedrop-login %{buildroot}/srv/salt/ -install -m 644 dom0/securedrop-launcher.desktop %{buildroot}/srv/salt/ install -m 655 dom0/securedrop-handle-upgrade %{buildroot}/srv/salt/ # The next file should get installed via RPM not via salt install -m 755 dom0/securedrop-update %{buildroot}/srv/salt/securedrop-update From 83df22c40e09f53e987519e2b0be2a1d78e9ad51 Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Tue, 11 Feb 2020 13:58:35 -0800 Subject: [PATCH 06/11] removed update_skip_delta from config.json, cleaned up --- MANIFEST.in | 1 + config.json.example | 3 +-- dom0/sd-dom0-files.sls | 19 ++++++++----------- ...desktop.j2 => securedrop-launcher.desktop} | 2 +- launcher/sdw-launcher.py | 2 +- .../securedrop-workstation-dom0-config.spec | 1 + 6 files changed, 13 insertions(+), 15 deletions(-) rename dom0/{securedrop-launcher.desktop.j2 => securedrop-launcher.desktop} (59%) diff --git a/MANIFEST.in b/MANIFEST.in index 5ee78a09..5b2a5b35 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -5,6 +5,7 @@ include dom0/*.yml include dom0/securedrop-admin include dom0/securedrop-update include dom0/securedrop-login +include dom0/securedrop-launcher.desktop include dom0/securedrop-handle-upgrade include config.json.example include README.md diff --git a/config.json.example b/config.json.example index f2845294..577f0c4c 100644 --- a/config.json.example +++ b/config.json.example @@ -4,8 +4,7 @@ "hostname": "avgfxawdn6c3coe3.onion", "key": "Il8Xas7uf6rjtc0LxYwhrx" }, - "environment": "prod", - "update_skip_delta": 28800, + "environment": "prod", "vmsizes": { "sd_app": 10, "sd_log": 5 diff --git a/dom0/sd-dom0-files.sls b/dom0/sd-dom0-files.sls index b6873034..7af55fb9 100644 --- a/dom0/sd-dom0-files.sls +++ b/dom0/sd-dom0-files.sls @@ -190,6 +190,14 @@ dom0-securedrop-launcher-executables: - mode: 755 - replace: false +dom0-securedrop-launcher-desktop-shortcut: + file.managed: + - name: /home/{{ gui_user }}/Desktop/securedrop-launcher.desktop + - source: "salt://securedrop-launcher.desktop" + - user: {{ gui_user }} + - group: {{ gui_user }} + - mode: 755 + {% import_json "sd/config.json" as d %} {% if d.environment == "dev" %} dom0-remove-securedrop-workstation-dom0-config: @@ -207,14 +215,3 @@ dom0-install-securedrop-workstation-dom0-config: - file: dom0-workstation-rpm-repo {% endif %} - -dom0-securedrop-launcher-desktop-shortcut: - file.managed: - - name: /home/{{ gui_user }}/Desktop/securedrop-launcher.desktop - - source: "salt://securedrop-launcher.desktop.j2" - - template: jinja - - context: - update_skip_delta: {{ d.update_skip_delta }} - - user: {{ gui_user }} - - group: {{ gui_user }} - - mode: 755 diff --git a/dom0/securedrop-launcher.desktop.j2 b/dom0/securedrop-launcher.desktop similarity index 59% rename from dom0/securedrop-launcher.desktop.j2 rename to dom0/securedrop-launcher.desktop index 89d2dad8..2905c4e8 100644 --- a/dom0/securedrop-launcher.desktop.j2 +++ b/dom0/securedrop-launcher.desktop @@ -4,4 +4,4 @@ Type=Application Terminal=false Icon=/usr/share/securedrop/icons/sd-logo.png Name=SecureDrop -Exec=/opt/securedrop/launcher/sdw-launcher.py --skip-delta={{ update_skip_delta }} +Exec=/opt/securedrop/launcher/sdw-launcher.py --skip-delta=28800 diff --git a/launcher/sdw-launcher.py b/launcher/sdw-launcher.py index 9318035f..1e7f7431 100644 --- a/launcher/sdw-launcher.py +++ b/launcher/sdw-launcher.py @@ -9,7 +9,7 @@ import sys import argparse -DEFAULT_INTERVAL = 0 +DEFAULT_INTERVAL = 28800 diff --git a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec index cd1bc721..fc28a458 100644 --- a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec +++ b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec @@ -55,6 +55,7 @@ install -m 644 dom0/*.j2 %{buildroot}/srv/salt/ install -m 644 dom0/*.yml %{buildroot}/srv/salt/ install -m 644 dom0/securedrop-update %{buildroot}/srv/salt/ install -m 644 dom0/securedrop-login %{buildroot}/srv/salt/ +install -m 644 dom0/securedrop-launcher.desktop %{buildroot}/srv/salt/ install -m 655 dom0/securedrop-handle-upgrade %{buildroot}/srv/salt/ # The next file should get installed via RPM not via salt install -m 755 dom0/securedrop-update %{buildroot}/srv/salt/securedrop-update From ea7a9af1e71502e2c043d685ced4aca721007889 Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Wed, 12 Feb 2020 08:33:07 -0800 Subject: [PATCH 07/11] fixed default launcher behaviour --- dom0/securedrop-launcher.desktop | 2 +- launcher/sdw-launcher.py | 15 ++-- launcher/sdw_updater_gui/Updater.py | 17 +++-- launcher/sdw_updater_gui/UpdaterApp.py | 13 ---- launcher/tests/test_updater.py | 95 +++++++++++--------------- 5 files changed, 64 insertions(+), 78 deletions(-) diff --git a/dom0/securedrop-launcher.desktop b/dom0/securedrop-launcher.desktop index 2905c4e8..3df2e49f 100644 --- a/dom0/securedrop-launcher.desktop +++ b/dom0/securedrop-launcher.desktop @@ -4,4 +4,4 @@ Type=Application Terminal=false Icon=/usr/share/securedrop/icons/sd-logo.png Name=SecureDrop -Exec=/opt/securedrop/launcher/sdw-launcher.py --skip-delta=28800 +Exec=/opt/securedrop/launcher/sdw-launcher.py diff --git a/launcher/sdw-launcher.py b/launcher/sdw-launcher.py index 1e7f7431..c7c18992 100644 --- a/launcher/sdw-launcher.py +++ b/launcher/sdw-launcher.py @@ -16,7 +16,7 @@ def parse_argv(argv): parser = argparse.ArgumentParser() - parser.add_argument("--skip-delta", type=int, default=0) + parser.add_argument("--skip-delta", type=int) return parser.parse_args(argv) @@ -44,10 +44,17 @@ def main(argv): configure_logging() sdlog.info("Starting SecureDrop Launcher") - interval = DEFAULT_INTERVAL args = parse_argv(argv) - if args.skip_delta: - interval = int(args.skip_delta) + + try: + args.skip_delta + except NameError: + args.skip_delta = DEFAULT_INTERVAL + + if args.skip_delta is None: + args.skip_delta = DEFAULT_INTERVAL + + interval = int(args.skip_delta) if should_launch_updater(interval): launch_updater() diff --git a/launcher/sdw_updater_gui/Updater.py b/launcher/sdw_updater_gui/Updater.py index 21f1a780..b0d27615 100644 --- a/launcher/sdw_updater_gui/Updater.py +++ b/launcher/sdw_updater_gui/Updater.py @@ -444,7 +444,11 @@ def should_launch_updater(interval): sdlog.info("Update interval not expired, launching client.") return False else: - sdlog.info("Updates or reboot required, launching updater.") + sdlog.info( + "Update status is {}, launching updater.".format( + str(status["status"]) + ) + ) return True else: sdlog.info("Update status not available, launching updater.") @@ -466,11 +470,11 @@ def _interval_expired(interval, status): """ try: - update_time = datetime.strptime(status['last_status_update'], DATE_FORMAT) + update_time = datetime.strptime(status["last_status_update"], DATE_FORMAT) except ValueError: # Broken timestamp? run the updater. return True - if ((datetime.now() - update_time) < timedelta(seconds=interval)): + if (datetime.now() - update_time) < timedelta(seconds=interval): return False return True @@ -480,9 +484,10 @@ def _status_ok_or_rebooted(status): Check if update status is OK or post-reboot. """ - if (status['status'] == UpdateStatus.UPDATES_OK.value or # noqa W504 - (status['status'] == UpdateStatus.REBOOT_REQUIRED.value and # noqa W504 - last_required_reboot_performed())): + if status["status"] == UpdateStatus.UPDATES_OK.value or ( # noqa W504 + status["status"] == UpdateStatus.REBOOT_REQUIRED.value + and last_required_reboot_performed() # noqa W504 + ): return True return False diff --git a/launcher/sdw_updater_gui/UpdaterApp.py b/launcher/sdw_updater_gui/UpdaterApp.py index 4653bf2e..3a12ffac 100644 --- a/launcher/sdw_updater_gui/UpdaterApp.py +++ b/launcher/sdw_updater_gui/UpdaterApp.py @@ -186,19 +186,6 @@ def get_vms_that_need_upgrades(self, results): vms_to_upgrade.append(vm) return vms_to_upgrade - def launch_securedrop_client(self): - """ - Helper method to launch the SecureDrop Client - """ - try: - logger.info("Launching SecureDrop client") - subprocess.Popen(["qvm-run", "sd-app", "gtk-launch securedrop-client"]) - except subprocess.CalledProcessError as e: - self.proposedActionDescription.setText(strings.descri) - logger.error("Error while launching SecureDrop client") - logger.error(str(e)) - sys.exit(0) - def apply_all_updates(self): """ Method used by the applyUpdatesButton that will create and start an diff --git a/launcher/tests/test_updater.py b/launcher/tests/test_updater.py index 9fd0c4a9..0f757979 100644 --- a/launcher/tests/test_updater.py +++ b/launcher/tests/test_updater.py @@ -192,7 +192,7 @@ def test_check_updates_debian_updates_required( call("Command 'check_call' returned non-zero exit status 1."), ] info_log = [ - call("Checking for updates {}:{}".format("sd-app", "sd-app-buster-template")), + call("Checking for updates {}:{}".format("sd-app", "sd-app-buster-template")) ] mocked_error.assert_has_calls(error_log) mocked_info.assert_has_calls(info_log) @@ -217,7 +217,7 @@ def test_check_debian_updates_failed(mocked_info, mocked_error, mocked_call, cap call("Command 'check_call' returned non-zero exit status 1."), ] info_log = [ - call("Checking for updates {}:{}".format("sd-app", "sd-app-buster-template")), + call("Checking for updates {}:{}".format("sd-app", "sd-app-buster-template")) ] mocked_error.assert_has_calls(error_log) mocked_info.assert_has_calls(info_log) @@ -239,7 +239,7 @@ def test_check_debian_has_updates(mocked_info, mocked_error, mocked_call, capsys call("Command 'check_call' returned non-zero exit status 1."), ] info_log = [ - call("Checking for updates {}:{}".format("sd-log", "sd-log-buster-template")), + call("Checking for updates {}:{}".format("sd-log", "sd-log-buster-template")) ] status = updater._check_updates_debian("sd-log") @@ -290,9 +290,7 @@ def test_check_updates_calls_correct_commands( call(["qvm-shutdown", "--wait", current_templates[vm]]), ] elif vm == "dom0": - subprocess_call_list = [ - call(["sudo", "qubes-dom0-update", "--check-only"]), - ] + subprocess_call_list = [call(["sudo", "qubes-dom0-update", "--check-only"])] else: pytest.fail("Unupported VM: {}".format(vm)) mocked_call.assert_has_calls(subprocess_call_list) @@ -453,10 +451,7 @@ def test_write_updates_status_flag_to_disk_failure_dom0( mocked_info, mocked_error, mocked_call, mocked_expand, mocked_open, status ): - error_calls = [ - call("Error writing update status flag to dom0"), - call("os_error"), - ] + error_calls = [call("Error writing update status flag to dom0"), call("os_error")] updater._write_updates_status_flag_to_disk(status) mocked_error.assert_has_calls(error_calls) @@ -669,9 +664,7 @@ def test_overall_update_status_reboot_not_done_previously( @mock.patch("Updater.sdlog.error") @mock.patch("Updater.sdlog.info") def test_safely_shutdown(mocked_info, mocked_error, mocked_call, vm): - call_list = [ - call(["qvm-shutdown", "--wait", "{}".format(vm)]), - ] + call_list = [call(["qvm-shutdown", "--wait", "{}".format(vm)])] updater._safely_shutdown_vm(vm) mocked_call.assert_has_calls(call_list) @@ -683,9 +676,7 @@ def test_safely_shutdown(mocked_info, mocked_error, mocked_call, vm): @mock.patch("Updater.sdlog.error") @mock.patch("Updater.sdlog.info") def test_safely_start(mocked_info, mocked_error, mocked_call, vm): - call_list = [ - call(["qvm-start", "--skip-if-running", "{}".format(vm)]), - ] + call_list = [call(["qvm-start", "--skip-if-running", "{}".format(vm)])] updater._safely_start_vm(vm) mocked_call.assert_has_calls(call_list) @@ -731,12 +722,7 @@ def test_safely_shutdown_fails(mocked_info, mocked_error, mocked_call, vm): def test_shutdown_and_start_vms( mocked_info, mocked_error, mocked_shutdown, mocked_start ): - call_list = [ - call("sd-proxy"), - call("sd-whonix"), - call("sd-app"), - call("sd-gpg"), - ] + call_list = [call("sd-proxy"), call("sd-whonix"), call("sd-app"), call("sd-gpg")] updater._shutdown_and_start_vms() mocked_shutdown.assert_has_calls(call_list) mocked_start.assert_has_calls(call_list) @@ -785,9 +771,7 @@ def test_read_dom0_update_flag_from_disk_fails( except Exception: pytest.fail("Error writing file") - info_calls = [ - call("Cannot read dom0 status flag, assuming first run"), - ] + info_calls = [call("Cannot read dom0 status flag, assuming first run")] assert updater.read_dom0_update_flag_from_disk() is None assert not mocked_error.called @@ -843,47 +827,50 @@ def test_last_required_reboot_performed_not_required( assert not mocked_error.called -@pytest.mark.parametrize("status, expected, rebooted", [ - (UpdateStatus.UPDATES_OK, True, True), - (UpdateStatus.UPDATES_REQUIRED, True, True), - (UpdateStatus.REBOOT_REQUIRED, True, True), - (UpdateStatus.UPDATES_FAILED, True, True), - (UpdateStatus.UPDATES_OK, True, False), - (UpdateStatus.UPDATES_REQUIRED, True, False), - (UpdateStatus.REBOOT_REQUIRED, True, False), - (UpdateStatus.UPDATES_FAILED, True, False) -]) -# @mock.patch("Updater.last_required_reboot_performed", return_value=True) -def test_should_run_updater_status_interval_expired( - status, expected, rebooted -): +@pytest.mark.parametrize( + "status, expected, rebooted", + [ + (UpdateStatus.UPDATES_OK, True, True), + (UpdateStatus.UPDATES_REQUIRED, True, True), + (UpdateStatus.REBOOT_REQUIRED, True, True), + (UpdateStatus.UPDATES_FAILED, True, True), + (UpdateStatus.UPDATES_OK, True, False), + (UpdateStatus.UPDATES_REQUIRED, True, False), + (UpdateStatus.REBOOT_REQUIRED, True, False), + (UpdateStatus.UPDATES_FAILED, True, False), + ], +) +def test_should_run_updater_status_interval_expired(status, expected, rebooted): TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = rebooted with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: mocked_read.return_value = { "last_status_update": str( - (datetime.now() - timedelta(seconds=(TEST_INTERVAL + 10))) - .strftime(updater.DATE_FORMAT)), + (datetime.now() - timedelta(seconds=(TEST_INTERVAL + 10))).strftime( + updater.DATE_FORMAT + ) + ), "status": status.value, } # assuming that the tests won't take an hour to run! assert expected == updater.should_launch_updater(TEST_INTERVAL) -@pytest.mark.parametrize("status, expected, rebooted", [ - (UpdateStatus.UPDATES_OK, False, True), - (UpdateStatus.UPDATES_REQUIRED, True, True), - (UpdateStatus.REBOOT_REQUIRED, False, True), - (UpdateStatus.UPDATES_FAILED, True, True), - (UpdateStatus.UPDATES_OK, False, False), - (UpdateStatus.UPDATES_REQUIRED, True, False), - (UpdateStatus.REBOOT_REQUIRED, True, False), - (UpdateStatus.UPDATES_FAILED, True, False) -]) -def test_should_run_updater_status_interval_not_expired( - status, expected, rebooted -): +@pytest.mark.parametrize( + "status, expected, rebooted", + [ + (UpdateStatus.UPDATES_OK, False, True), + (UpdateStatus.UPDATES_REQUIRED, True, True), + (UpdateStatus.REBOOT_REQUIRED, False, True), + (UpdateStatus.UPDATES_FAILED, True, True), + (UpdateStatus.UPDATES_OK, False, False), + (UpdateStatus.UPDATES_REQUIRED, True, False), + (UpdateStatus.REBOOT_REQUIRED, True, False), + (UpdateStatus.UPDATES_FAILED, True, False), + ], +) +def test_should_run_updater_status_interval_not_expired(status, expected, rebooted): TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = rebooted From ed92761727d27da113732e35e0cefd045aa8bf0d Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Wed, 12 Feb 2020 15:12:52 -0800 Subject: [PATCH 08/11] updated flag file behaviour to prevent stale status file when updater skipped. --- launcher/sdw_updater_gui/Updater.py | 27 ++++++++++++-------------- launcher/sdw_updater_gui/UpdaterApp.py | 2 +- launcher/tests/test_updater.py | 16 +++++++++++---- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/launcher/sdw_updater_gui/Updater.py b/launcher/sdw_updater_gui/Updater.py index b0d27615..a07b1f60 100644 --- a/launcher/sdw_updater_gui/Updater.py +++ b/launcher/sdw_updater_gui/Updater.py @@ -440,9 +440,19 @@ def should_launch_updater(interval): sdlog.info("Update interval expired: launching updater.") return True else: - if _status_ok_or_rebooted(status): - sdlog.info("Update interval not expired, launching client.") + if status["status"] == UpdateStatus.UPDATES_OK.value: + sdlog.info("Updates OK and interval not expired, launching client.") return False + elif status["status"] == UpdateStatus.REBOOT_REQUIRED.value: + if last_required_reboot_performed(): + sdlog.info( + "Required reboot performed, updating status and launching client." + ) + _write_updates_status_flag_to_disk(UpdateStatus.UPDATES_OK) + return False + else: + sdlog.info("Required reboot pending, launching updater") + return True else: sdlog.info( "Update status is {}, launching updater.".format( @@ -479,19 +489,6 @@ def _interval_expired(interval, status): return True -def _status_ok_or_rebooted(status): - """ - Check if update status is OK or post-reboot. - """ - - if status["status"] == UpdateStatus.UPDATES_OK.value or ( # noqa W504 - status["status"] == UpdateStatus.REBOOT_REQUIRED.value - and last_required_reboot_performed() # noqa W504 - ): - return True - return False - - class UpdateStatus(Enum): """ Standardizes return codes for update/upgrade methods diff --git a/launcher/sdw_updater_gui/UpdaterApp.py b/launcher/sdw_updater_gui/UpdaterApp.py index 3a12ffac..5c87ba39 100644 --- a/launcher/sdw_updater_gui/UpdaterApp.py +++ b/launcher/sdw_updater_gui/UpdaterApp.py @@ -249,7 +249,7 @@ def run(self): # write the flags to disk run_results = Updater.overall_update_status(results) Updater._write_updates_status_flag_to_disk(run_results) - if run_results == UpdateStatus.UPDATES_OK: + if run_results in {UpdateStatus.UPDATES_OK, UpdateStatus.REBOOT_REQUIRED}: Updater._write_last_updated_flags_to_disk() # populate signal contents message = results # copy all the information from results diff --git a/launcher/tests/test_updater.py b/launcher/tests/test_updater.py index 0f757979..d3ed076b 100644 --- a/launcher/tests/test_updater.py +++ b/launcher/tests/test_updater.py @@ -840,7 +840,10 @@ def test_last_required_reboot_performed_not_required( (UpdateStatus.UPDATES_FAILED, True, False), ], ) -def test_should_run_updater_status_interval_expired(status, expected, rebooted): +@mock.patch("Updater._write_updates_status_flag_to_disk") +def test_should_run_updater_status_interval_expired( + mocked_write, status, expected, rebooted +): TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = rebooted @@ -870,7 +873,10 @@ def test_should_run_updater_status_interval_expired(status, expected, rebooted): (UpdateStatus.UPDATES_FAILED, True, False), ], ) -def test_should_run_updater_status_interval_not_expired(status, expected, rebooted): +@mock.patch("Updater._write_updates_status_flag_to_disk") +def test_should_run_updater_status_interval_not_expired( + mocked_write, status, expected, rebooted +): TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = rebooted @@ -883,7 +889,8 @@ def test_should_run_updater_status_interval_not_expired(status, expected, reboot assert expected == updater.should_launch_updater(TEST_INTERVAL) -def test_should_run_updater_invalid_status(): +@mock.patch("Updater._write_updates_status_flag_to_disk") +def test_should_run_updater_invalid_status(mocked_write): TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = True @@ -893,7 +900,8 @@ def test_should_run_updater_invalid_status(): assert updater.should_launch_updater(TEST_INTERVAL) is True -def test_should_run_updater_invalid_timestamp(): +@mock.patch("Updater._write_updates_status_flag_to_disk") +def test_should_run_updater_invalid_timestamp(mocked_write): TEST_INTERVAL = 3600 with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = True From 25df3de1bffc0d9dbeefb59119a30e6b35e48edc Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Fri, 14 Feb 2020 08:04:22 -0800 Subject: [PATCH 09/11] rebased and added a test to get 100% coverage back --- launcher/sdw-launcher.py | 5 ----- launcher/tests/test_updater.py | 10 +++++++++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/launcher/sdw-launcher.py b/launcher/sdw-launcher.py index c7c18992..5fe6bec7 100644 --- a/launcher/sdw-launcher.py +++ b/launcher/sdw-launcher.py @@ -12,8 +12,6 @@ DEFAULT_INTERVAL = 28800 - - def parse_argv(argv): parser = argparse.ArgumentParser() parser.add_argument("--skip-delta", type=int) @@ -40,9 +38,6 @@ def main(argv): # Logged. sys.exit(1) sdlog.info("Starting SecureDrop Launcher") - sdlog = logging.getLogger(__name__) - configure_logging() - sdlog.info("Starting SecureDrop Launcher") args = parse_argv(argv) diff --git a/launcher/tests/test_updater.py b/launcher/tests/test_updater.py index d3ed076b..91538511 100644 --- a/launcher/tests/test_updater.py +++ b/launcher/tests/test_updater.py @@ -2,7 +2,6 @@ import os import pytest import subprocess -from datetime import datetime from importlib.machinery import SourceFileLoader from datetime import datetime, timedelta from tempfile import TemporaryDirectory @@ -810,6 +809,15 @@ def test_last_required_reboot_performed_failed(mocked_info, mocked_error, mocked assert not mocked_error.called +@mock.patch("Updater.read_dom0_update_flag_from_disk", return_value=None) +@mock.patch("Updater.sdlog.error") +@mock.patch("Updater.sdlog.info") +def test_last_required_reboot_performed_no_file(mocked_info, mocked_error, mocked_read): + result = updater.last_required_reboot_performed() + assert result is True + assert not mocked_error.called + + @mock.patch( "Updater.read_dom0_update_flag_from_disk", return_value={ From 115a54156ddef682d69062aa63a2a8e1be72e2cd Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Fri, 14 Feb 2020 08:20:35 -0800 Subject: [PATCH 10/11] Updated version to 0.1.4 --- VERSION | 2 +- rpm-build/SPECS/securedrop-workstation-dom0-config.spec | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/VERSION b/VERSION index b1e80bb2..845639ee 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.3 +0.1.4 diff --git a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec index fc28a458..2c2f25a9 100644 --- a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec +++ b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec @@ -1,12 +1,12 @@ Name: securedrop-workstation-dom0-config -Version: 0.1.3 +Version: 0.1.4 Release: 1%{?dist} Summary: SecureDrop Workstation Group: Library License: GPLv3+ URL: https://github.com/freedomofpress/securedrop-workstation -Source0: securedrop-workstation-dom0-config-0.1.3.tar.gz +Source0: securedrop-workstation-dom0-config-0.1.4.tar.gz BuildArch: noarch BuildRequires: python3-setuptools @@ -94,6 +94,9 @@ find /srv/salt -maxdepth 1 -type f -iname '*.top' \ | xargs qubesctl top.enable > /dev/null %changelog +* Fri Feb 14 2020 Kevin O Gorman - 0.1.4 +- Modifies updater to allow for a configurable interval between checks + * Tue Feb 11 2020 SecureDrop Team - 0.1.3 - Adds sdw-notify script - Sets executable bits within package specification From c719c3459b53a2ff10bace253550d04d53990fd6 Mon Sep 17 00:00:00 2001 From: Kevin O'Gorman Date: Fri, 14 Feb 2020 18:46:54 -0800 Subject: [PATCH 11/11] Updates based on PR review --- launcher/sdw-launcher.py | 2 +- launcher/sdw_updater_gui/Updater.py | 18 ++++- launcher/sdw_updater_gui/UpdaterApp.py | 3 +- launcher/tests/test_updater.py | 65 ++++++++++++------- .../securedrop-workstation-dom0-config.spec | 2 +- 5 files changed, 62 insertions(+), 28 deletions(-) diff --git a/launcher/sdw-launcher.py b/launcher/sdw-launcher.py index 5fe6bec7..2b93d5c6 100644 --- a/launcher/sdw-launcher.py +++ b/launcher/sdw-launcher.py @@ -9,7 +9,7 @@ import sys import argparse -DEFAULT_INTERVAL = 28800 +DEFAULT_INTERVAL = 28800 # 8hr default for update interval def parse_argv(argv): diff --git a/launcher/sdw_updater_gui/Updater.py b/launcher/sdw_updater_gui/Updater.py index a07b1f60..30917730 100644 --- a/launcher/sdw_updater_gui/Updater.py +++ b/launcher/sdw_updater_gui/Updater.py @@ -431,8 +431,6 @@ def _safely_start_vm(vm): def should_launch_updater(interval): - sdlog.info("Starting SecureDrop Launcher") - status = read_dom0_update_flag_from_disk(with_timestamp=True) if _valid_status(status): @@ -453,9 +451,23 @@ def should_launch_updater(interval): else: sdlog.info("Required reboot pending, launching updater") return True + elif status["status"] == UpdateStatus.UPDATES_REQUIRED.value: + sdlog.info( + "Updates are required, launching updater.".format( + str(status["status"]) + ) + ) + return True + elif status["status"] == UpdateStatus.UPDATES_FAILED.value: + sdlog.info( + "Preceding update failed, launching updater.".format( + str(status["status"]) + ) + ) + return True else: sdlog.info( - "Update status is {}, launching updater.".format( + "Update status is unknown, launching updater.".format( str(status["status"]) ) ) diff --git a/launcher/sdw_updater_gui/UpdaterApp.py b/launcher/sdw_updater_gui/UpdaterApp.py index 5c87ba39..ec1411bf 100644 --- a/launcher/sdw_updater_gui/UpdaterApp.py +++ b/launcher/sdw_updater_gui/UpdaterApp.py @@ -246,7 +246,8 @@ def run(self): results[vm] = result self.progress_signal.emit(progress) - # write the flags to disk + # write the flags to disk after successful updates, including updates + # that require a reboot. run_results = Updater.overall_update_status(results) Updater._write_updates_status_flag_to_disk(run_results) if run_results in {UpdateStatus.UPDATES_OK, UpdateStatus.REBOOT_REQUIRED}: diff --git a/launcher/tests/test_updater.py b/launcher/tests/test_updater.py index 91538511..fbb8f8e8 100644 --- a/launcher/tests/test_updater.py +++ b/launcher/tests/test_updater.py @@ -836,23 +836,25 @@ def test_last_required_reboot_performed_not_required( @pytest.mark.parametrize( - "status, expected, rebooted", + "status, rebooted, expect_status_change, expect_updater", [ - (UpdateStatus.UPDATES_OK, True, True), - (UpdateStatus.UPDATES_REQUIRED, True, True), - (UpdateStatus.REBOOT_REQUIRED, True, True), - (UpdateStatus.UPDATES_FAILED, True, True), - (UpdateStatus.UPDATES_OK, True, False), - (UpdateStatus.UPDATES_REQUIRED, True, False), - (UpdateStatus.REBOOT_REQUIRED, True, False), - (UpdateStatus.UPDATES_FAILED, True, False), + (UpdateStatus.UPDATES_OK, True, False, True), + (UpdateStatus.UPDATES_REQUIRED, True, False, True), + (UpdateStatus.REBOOT_REQUIRED, True, False, True), + (UpdateStatus.UPDATES_FAILED, True, False, True), + (UpdateStatus.UPDATES_OK, False, False, True), + (UpdateStatus.UPDATES_REQUIRED, False, False, True), + (UpdateStatus.REBOOT_REQUIRED, False, False, True), + (UpdateStatus.UPDATES_FAILED, False, False, True), ], ) @mock.patch("Updater._write_updates_status_flag_to_disk") def test_should_run_updater_status_interval_expired( - mocked_write, status, expected, rebooted + mocked_write, status, rebooted, expect_status_change, expect_updater ): TEST_INTERVAL = 3600 + # the updater should always run when checking interval has expired, + # regardless of update or reboot status with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = rebooted with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: @@ -865,27 +867,31 @@ def test_should_run_updater_status_interval_expired( "status": status.value, } # assuming that the tests won't take an hour to run! - assert expected == updater.should_launch_updater(TEST_INTERVAL) + assert expect_updater == updater.should_launch_updater(TEST_INTERVAL) + assert expect_status_change == mocked_write.called @pytest.mark.parametrize( - "status, expected, rebooted", + "status, rebooted, expect_status_change, expect_updater", [ - (UpdateStatus.UPDATES_OK, False, True), - (UpdateStatus.UPDATES_REQUIRED, True, True), - (UpdateStatus.REBOOT_REQUIRED, False, True), - (UpdateStatus.UPDATES_FAILED, True, True), - (UpdateStatus.UPDATES_OK, False, False), - (UpdateStatus.UPDATES_REQUIRED, True, False), - (UpdateStatus.REBOOT_REQUIRED, True, False), - (UpdateStatus.UPDATES_FAILED, True, False), + (UpdateStatus.UPDATES_OK, True, False, False), + (UpdateStatus.UPDATES_REQUIRED, True, False, True), + (UpdateStatus.REBOOT_REQUIRED, True, True, False), + (UpdateStatus.UPDATES_FAILED, True, False, True), + (UpdateStatus.UPDATES_OK, False, False, False), + (UpdateStatus.UPDATES_REQUIRED, False, False, True), + (UpdateStatus.REBOOT_REQUIRED, False, False, True), + (UpdateStatus.UPDATES_FAILED, False, False, True), ], ) @mock.patch("Updater._write_updates_status_flag_to_disk") def test_should_run_updater_status_interval_not_expired( - mocked_write, status, expected, rebooted + mocked_write, status, rebooted, expect_status_change, expect_updater ): TEST_INTERVAL = 3600 + # Even if the interval hasn't expired, the updater should only be skipped when: + # - the updater status is UPDATESr_OK, or + # - the updater status is REBOOT_REQUIRED and the reboot has been performed. with mock.patch("Updater.last_required_reboot_performed") as mocked_last: mocked_last.return_value = rebooted with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: @@ -894,7 +900,8 @@ def test_should_run_updater_status_interval_not_expired( "status": status.value, } # assuming that the tests won't take an hour to run! - assert expected == updater.should_launch_updater(TEST_INTERVAL) + assert expect_updater == updater.should_launch_updater(TEST_INTERVAL) + assert expect_status_change == mocked_write.called @mock.patch("Updater._write_updates_status_flag_to_disk") @@ -920,3 +927,17 @@ def test_should_run_updater_invalid_timestamp(mocked_write): } # assuming that the tests won't take an hour to run! assert updater.should_launch_updater(TEST_INTERVAL) is True + + +@mock.patch("Updater._write_updates_status_flag_to_disk") +def test_should_run_updater_invalid_status_value(mocked_write): + TEST_INTERVAL = 3600 + with mock.patch("Updater.last_required_reboot_performed") as mocked_last: + mocked_last.return_value = True + with mock.patch("Updater.read_dom0_update_flag_from_disk") as mocked_read: + mocked_read.return_value = { + "last_status_update": str(datetime.now().strftime(updater.DATE_FORMAT)), + "status": "5", + } + # assuming that the tests won't take an hour to run! + assert updater.should_launch_updater(TEST_INTERVAL) is True diff --git a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec index 2c2f25a9..45dba765 100644 --- a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec +++ b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec @@ -94,7 +94,7 @@ find /srv/salt -maxdepth 1 -type f -iname '*.top' \ | xargs qubesctl top.enable > /dev/null %changelog -* Fri Feb 14 2020 Kevin O Gorman - 0.1.4 +* Fri Feb 14 2020 SecureDrop Team - 0.1.4 - Modifies updater to allow for a configurable interval between checks * Tue Feb 11 2020 SecureDrop Team - 0.1.3