From efc19434b5956006aa4854c2747adc5aa0320805 Mon Sep 17 00:00:00 2001 From: Erik Moeller Date: Fri, 5 Feb 2021 16:44:26 -0800 Subject: [PATCH 1/4] Prevent sd-viewer VMs from launching dispVMs - Ensures no default disposable VM is configured - Prevents qubes.GetImageRGBA calls, which could be used to open disposable VMs. This policy is applied to all SecureDrop Workstation VMs, as this RPC call is not required by any current VMs. --- dom0/sd-dom0-qvm-rpc.sls | 9 +++++++++ dom0/sd-viewer.sls | 1 + tests/test_vms_exist.py | 2 ++ tests/vars/qubes-rpc.yml | 17 +++++++---------- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/dom0/sd-dom0-qvm-rpc.sls b/dom0/sd-dom0-qvm-rpc.sls index b6fc2fcb..99edc4bd 100644 --- a/dom0/sd-dom0-qvm-rpc.sls +++ b/dom0/sd-dom0-qvm-rpc.sls @@ -42,6 +42,15 @@ dom0-rpc-qubes.Filecopy: sd-proxy @tag:sd-client allow @anyvm @tag:sd-workstation deny @tag:sd-workstation @anyvm deny +dom0-rpc-qubes.GetImageRGBA: + file.blockreplace: + - name: /etc/qubes-rpc/policy/qubes.GetImageRGBA + - prepend_if_not_found: True + - marker_start: "### BEGIN securedrop-workstation ###" + - marker_end: "### END securedrop-workstation ###" + - content: | + @anyvm @tag:sd-workstation deny + @tag:sd-workstation @anyvm deny dom0-rpc-qubes.OpenInVM: file.blockreplace: - name: /etc/qubes-rpc/policy/qubes.OpenInVM diff --git a/dom0/sd-viewer.sls b/dom0/sd-viewer.sls index dc078967..414422c9 100644 --- a/dom0/sd-viewer.sls +++ b/dom0/sd-viewer.sls @@ -25,6 +25,7 @@ sd-viewer: - template: sd-large-buster-template - netvm: "" - template_for_dispvms: True + - default_dispvm: "" - tags: - add: - sd-workstation diff --git a/tests/test_vms_exist.py b/tests/test_vms_exist.py index 070ad06f..7ca8730f 100644 --- a/tests/test_vms_exist.py +++ b/tests/test_vms_exist.py @@ -99,6 +99,8 @@ def test_sd_viewer_config(self): self.assertTrue(vm.template == "sd-large-buster-template") self.assertFalse(vm.provides_network) self.assertTrue(vm.template_for_dispvms) + # sd-viewer should not be able to create other disposable VMs + self.assertIsNone(vm.default_dispvm) self._check_kernel(vm) self._check_service_running(vm, "paxctld") self.assertTrue("sd-workstation" in vm.tags) diff --git a/tests/vars/qubes-rpc.yml b/tests/vars/qubes-rpc.yml index 8b963eb9..c8065615 100644 --- a/tests/vars/qubes-rpc.yml +++ b/tests/vars/qubes-rpc.yml @@ -34,24 +34,21 @@ $tag:anon-vm $anyvm deny $anyvm $anyvm allow,target=dom0 -- policy: qubes.GetRandomizedTime +- policy: qubes.GetImageRGBA starts_with: |- - ## Note that policy parsing stops at the first match, - ## so adding anything below "$anyvm $anyvm action" line will have no effect - - ## Please use a single # to start your custom comments - - $anyvm dom0 allow + ### BEGIN securedrop-workstation ### + @anyvm @tag:sd-workstation deny + @tag:sd-workstation @anyvm deny + ### END securedrop-workstation ### -- policy: qubes.GetImageRGBA +- policy: qubes.GetRandomizedTime starts_with: |- ## Note that policy parsing stops at the first match, ## so adding anything below "$anyvm $anyvm action" line will have no effect ## Please use a single # to start your custom comments - $anyvm $dispvm allow - $anyvm $anyvm ask + $anyvm dom0 allow - policy: qubes.Gpg starts_with: |- From fa60a26a86642a91ed161fb372a8391a1d78871e Mon Sep 17 00:00:00 2001 From: Erik Moeller Date: Thu, 25 Feb 2021 17:34:25 -0800 Subject: [PATCH 2/4] Provision default mailcap rules and update tests --- dom0/sd-mime-handling.sls | 7 +++++++ tests/base.py | 29 +++++++++++++++++++++++++++++ tests/test_app.py | 5 ++++- tests/test_proxy_vm.py | 3 +++ tests/test_sd_devices.py | 5 ++++- tests/test_viewer.py | 3 +++ 6 files changed, 50 insertions(+), 2 deletions(-) diff --git a/dom0/sd-mime-handling.sls b/dom0/sd-mime-handling.sls index ac827340..664ef3a5 100644 --- a/dom0/sd-mime-handling.sls +++ b/dom0/sd-mime-handling.sls @@ -43,3 +43,10 @@ sd-private-volume-mimeapps-handling: - file: sd-private-volume-mimeapps-config-dir {% endif %} + +sd-private-volume-mailcap-handling: + file.symlink: + - name: /home/user/.mailcap + - target: /opt/sdw/mailcap.default + - user: user + - group: user diff --git a/tests/base.py b/tests/base.py index 7f8c994e..a6c8cec7 100644 --- a/tests/base.py +++ b/tests/base.py @@ -135,6 +135,35 @@ def logging_configured(self): # cmd_output = self._run("sudo grep -F \"action 'action-0-omprog' suspended (module 'omprog')\" /var/log/syslog | wc -l").strip() # noqa # self.assertTrue(cmd_output == "0") + def mailcap_hardened(self): + """ + Ensure that mailcap rules are not used as a fallback when looking up + appropriate viewer applications for files of a given MIME type. + """ + + # Ensure that mailcap configuration files are present in the expected + # locations and contain the expected contents. Rules in `~/.mailcap` + # take precedence over those in `/etc/mailcap`. + self.assertTrue(self._fileExists("/home/user/.mailcap")) + self.assertTrue(self._fileExists("/opt/sdw/mailcap.default")) + self.assertFileHasLine("/home/user/.mailcap", '*/*; logger "Mailcap is disabled."') + + # Because we target an AppVM, we cannot easily use the Pyhton tempfile + # module here without relying on a helper script. Instead, we use the + # mktemp utility to create a test file with the OpenDocument extension. + tmpfile_name = self._run("mktemp -t XXXXXX.odt") + + # The --norun argument ensures that we do not launch any application, + # regardless of the result of this invocation. + mailcap_result = self._run("run-mailcap --norun {}".format(tmpfile_name)) + + # For simplicity, we remove the tempfile here instead of in a separate + # teardown method. + self._run("rm {}".format(tmpfile_name)) + + # Ensure that the wildcard rule worked as expected. + self.assertEqual(mailcap_result, 'logger "Mailcap is disabled." <{}'.format(tmpfile_name)) + def qubes_gpg_domain_configured(self, vmname=False): """ Ensure the QUBES_GPG_DOMAIN is properly set for a given AppVM. This diff --git a/tests/test_app.py b/tests/test_app.py index 809b34c8..ca5f3ee5 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -16,7 +16,7 @@ def test_open_in_dvm_desktop(self): contents = self._get_file_contents("/usr/share/applications/open-in-dvm.desktop") expected_contents = [ "TryExec=/usr/bin/qvm-open-in-vm", - "Exec=/usr/bin/qvm-open-in-vm --view-only '@dispvm:sd-viewer' %f", + "Exec=/usr/bin/qvm-open-in-vm --view-only @dispvm:sd-viewer %f", ] for line in expected_contents: self.assertTrue(line in contents) @@ -35,6 +35,9 @@ def test_mimeapps_functional(self): actual_app = self._run("xdg-mime query default {}".format(line)) self.assertEqual(actual_app, "open-in-dvm.desktop") + def test_mailcap_hardened(self): + self.mailcap_hardened() + def test_sd_client_package_installed(self): self.assertTrue(self._package_is_installed("securedrop-client")) diff --git a/tests/test_proxy_vm.py b/tests/test_proxy_vm.py index 13b01e56..37e84c87 100644 --- a/tests/test_proxy_vm.py +++ b/tests/test_proxy_vm.py @@ -76,6 +76,9 @@ def test_mime_types(self): actual_app = self._run("xdg-mime query default {}".format(line)) self.assertEqual(actual_app, "open-in-dvm.desktop") + def test_mailcap_hardened(self): + self.mailcap_hardened() + def test_gpg_domain_configured(self): self.qubes_gpg_domain_configured(self.vm_name) diff --git a/tests/test_sd_devices.py b/tests/test_sd_devices.py index 94df94ff..a4e00bc8 100644 --- a/tests/test_sd_devices.py +++ b/tests/test_sd_devices.py @@ -36,11 +36,14 @@ def test_mime_types(self): actual_app = self._run("xdg-mime query default {}".format(mime_type)) self.assertEqual(actual_app, expected_app) + def test_mailcap_hardened(self): + self.mailcap_hardened() + def test_open_in_dvm_desktop(self): contents = self._get_file_contents("/usr/share/applications/open-in-dvm.desktop") expected_contents = [ "TryExec=/usr/bin/qvm-open-in-vm", - "Exec=/usr/bin/qvm-open-in-vm --view-only '@dispvm:sd-viewer' %f", + "Exec=/usr/bin/qvm-open-in-vm --view-only @dispvm:sd-viewer %f", ] for line in expected_contents: self.assertTrue(line in contents) diff --git a/tests/test_viewer.py b/tests/test_viewer.py index 38e7ecc0..71cf3947 100644 --- a/tests/test_viewer.py +++ b/tests/test_viewer.py @@ -44,6 +44,9 @@ def test_mime_types(self): actual_app = self._run("xdg-mime query default {}".format(mime_type)) self.assertEqual(actual_app, expected_app) + def test_mailcap_hardened(self): + self.mailcap_hardened() + def test_gpg_domain_configured(self): self.qubes_gpg_domain_configured(self.vm_name) From 6cf625ce3f02dbce0b7516e2059b6c345a82956c Mon Sep 17 00:00:00 2001 From: Conor Schaefer Date: Mon, 1 Mar 2021 13:23:44 -0500 Subject: [PATCH 3/4] Adds migration flag for mailcap-hardening The updater must perform a full state run in order to apply the mailcap hardening inside AppVM private volumes. Create the necessary run-everything flag via RPM postinst. The updater logic will automatically clean up the flags are it runs. We'll need to remove this logic from postinst in the subsequent release of 'securedrop-workstation-dom0-config', so it doesn't recreate the flag. --- dom0/securedrop-check-migration | 3 --- rpm-build/SPECS/securedrop-workstation-dom0-config.spec | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dom0/securedrop-check-migration b/dom0/securedrop-check-migration index 6a2abffe..375efde0 100755 --- a/dom0/securedrop-check-migration +++ b/dom0/securedrop-check-migration @@ -26,7 +26,4 @@ if [[ -n "$(qvm-ls --tags sd-workstation --raw-list | perl -nE '/sd-(?!small|lar reason="template-consolidation" echo "Migration required for ${reason}, will re-run Salt states." request_migration "$reason" -else - echo "No migration required, skipping full state run against all VMs." - rm -rf "${flag_dir}" fi diff --git a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec index 11fefe25..d5fb0609 100644 --- a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec +++ b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec @@ -105,6 +105,8 @@ find /srv/salt -maxdepth 1 -type f -iname '*.top' \ | xargs -n1 basename \ | sed -e 's/\.top$$//g' \ | xargs qubesctl top.enable > /dev/null +mkdir -p /tmp/sdw-migrations +touch /tmp/sdw-migrations/mailcap-hardening %changelog * Fri Nov 20 2020 SecureDrop Team - 0.5.2 From 57d7672b3f816974101828d8bb214edf238791a3 Mon Sep 17 00:00:00 2001 From: Erik Moeller Date: Tue, 9 Mar 2021 22:21:26 -0800 Subject: [PATCH 4/4] Bump version to 0.5.3; update changelog --- VERSION | 2 +- .../SPECS/securedrop-workstation-dom0-config.spec | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/VERSION b/VERSION index cb0c939a..be14282b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.5.2 +0.5.3 diff --git a/rpm-build/SPECS/securedrop-workstation-dom0-config.spec b/rpm-build/SPECS/securedrop-workstation-dom0-config.spec index d5fb0609..c43f418b 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.5.2 +Version: 0.5.3 Release: 1%{?dist} Summary: SecureDrop Workstation Group: Library License: GPLv3+ URL: https://github.com/freedomofpress/securedrop-workstation -Source0: securedrop-workstation-dom0-config-0.5.2.tar.gz +Source0: securedrop-workstation-dom0-config-0.5.3.tar.gz BuildArch: noarch BuildRequires: python3-setuptools @@ -28,7 +28,7 @@ configuration over time. %undefine py_auto_byte_compile %prep -%setup -n securedrop-workstation-dom0-config-0.5.2 +%setup -n securedrop-workstation-dom0-config-0.5.3 %build %{__python3} setup.py build @@ -109,6 +109,10 @@ mkdir -p /tmp/sdw-migrations touch /tmp/sdw-migrations/mailcap-hardening %changelog +* Wed Mar 10 2021 SecureDrop Team - 0.5.3 +- Prevents sd-viewer from launching disposable VMs +- Provisions default mailcap rules to enforce Fail Closed behavior + * Fri Nov 20 2020 SecureDrop Team - 0.5.2 - Fixes updater, ensuring dom0 packages are updated