From 509fd8687387334dd18f18fef9b3933e0f2f6b3e Mon Sep 17 00:00:00 2001
From: Conor Schaefer <conor@freedom.press>
Date: Thu, 20 Feb 2020 10:02:06 -0800
Subject: [PATCH 1/7] Consolidates log aggregation into single state file

We were adding multiple tasks, customized per-VM, to several state
files. Let's instead intelligently handle all logging-related config
in a single state file, and include that in the various VM
configurations as necessary.

Requires a small change to the "securedrop-log" package (managed
separately), to infer the hostname from the logclient, rather than
hardcoding it in a config file on each VM. That way, we can reuse the
same config file for all machines.

There's still an exception
---
 dom0/sd-app-files.sls                  | 13 +----
 dom0/sd-devices-files.sls              | 15 +----
 dom0/sd-log-disable-plugin.sls         | 23 --------
 dom0/sd-log-template-files.sls         | 21 -------
 dom0/sd-log.sls                        |  2 +
 dom0/sd-logging-setup.sls              | 80 ++++++++++++++++++++++++++
 dom0/sd-proxy-template-files.sls       | 12 +---
 dom0/sd-rsyslog.conf.j2                |  1 -
 dom0/sd-viewer-files.sls               |  9 +--
 dom0/sd-whonix-rsyslog-enable.sls      | 43 --------------
 dom0/sd-whonix-template-files.sls      | 22 -------
 dom0/sd-workstation-template-files.sls |  8 ---
 dom0/sd-workstation.top                |  8 +--
 13 files changed, 92 insertions(+), 165 deletions(-)
 delete mode 100644 dom0/sd-log-disable-plugin.sls
 delete mode 100644 dom0/sd-log-template-files.sls
 create mode 100644 dom0/sd-logging-setup.sls
 delete mode 100644 dom0/sd-whonix-rsyslog-enable.sls
 delete mode 100644 dom0/sd-whonix-template-files.sls

diff --git a/dom0/sd-app-files.sls b/dom0/sd-app-files.sls
index 420f7889..9828d4ed 100644
--- a/dom0/sd-app-files.sls
+++ b/dom0/sd-app-files.sls
@@ -10,21 +10,12 @@
 ##
 include:
   - fpf-apt-test-repo
+  - sd-logging-setup
 
 # FPF repo is setup in "securedrop-workstation" template
-install-securedrop-client-and-securedrop-log-package:
+install-securedrop-client-package:
   pkg.installed:
     - pkgs:
       - securedrop-client
-      - securedrop-log
     - require:
       - sls: fpf-apt-test-repo
-
-
-sd-rsyslog-for-sd-app:
-  file.managed:
-    - name: /etc/sd-rsyslog.conf
-    - source: "salt://sd-rsyslog.conf.j2"
-    - template: jinja
-    - context:
-        vmname: sd-app
\ No newline at end of file
diff --git a/dom0/sd-devices-files.sls b/dom0/sd-devices-files.sls
index b7320154..8d8b3e9a 100644
--- a/dom0/sd-devices-files.sls
+++ b/dom0/sd-devices-files.sls
@@ -10,6 +10,7 @@
 ##
 include:
   - fpf-apt-test-repo
+  - sd-logging-setup
 
 # Libreoffice needs to be installed here to convert to pdf to allow printing
 sd-devices-install-libreoffice:
@@ -24,19 +25,5 @@ sd-devices-install-libreoffice:
 sd-devices-install-package:
   pkg.installed:
     - name: securedrop-export
-
-
-sd-devices-install-securedrop-log-package:
-  pkg.installed:
-    - pkgs:
-      - securedrop-log
     - require:
       - sls: fpf-apt-test-repo
-
-sd-rsyslog-for-sd-devices:
-  file.managed:
-    - name: /etc/sd-rsyslog.conf
-    - source: "salt://sd-rsyslog.conf.j2"
-    - template: jinja
-    - context:
-        vmname: sd-devices
diff --git a/dom0/sd-log-disable-plugin.sls b/dom0/sd-log-disable-plugin.sls
deleted file mode 100644
index 34251790..00000000
--- a/dom0/sd-log-disable-plugin.sls
+++ /dev/null
@@ -1,23 +0,0 @@
-# -*- coding: utf-8 -*-
-# vim: set syntax=yaml ts=2 sw=2 sts=2 et :
-
-##
-# Disable securedrop-log rsyslog plugin in sd-log vm
-# Fixes https://github.com/freedomofpress/securedrop-log/issues/15
-# Due to a single Debian package for both logging and also sd-log vm
-# we need to do the following step.
-##
-
-
-sd-log-remove-rsyslog-qubes-plugin:
-  file.blockreplace:
-    - name: /rw/config/rc.local
-    - append_if_not_found: True
-    - marker_start: "### BEGIN securedrop-workstation ###"
-    - marker_end: "### END securedrop-workstation ###"
-    - content: |
-        # Removes sdlog.conf file for rsyslog
-        rm -f /etc/rsyslog.d/sdlog.conf
-        systemctl restart rsyslog
-  cmd.run:
-    - name: rm -f /etc/rsyslog.d/sdlog.conf && systemctl restart rsyslog
diff --git a/dom0/sd-log-template-files.sls b/dom0/sd-log-template-files.sls
deleted file mode 100644
index 04ebe571..00000000
--- a/dom0/sd-log-template-files.sls
+++ /dev/null
@@ -1,21 +0,0 @@
-# -*- coding: utf-8 -*-
-# vim: set syntax=yaml ts=2 sw=2 sts=2 et :
-include:
-  - fpf-apt-test-repo
-
-sd-log-install-securedrop-log-package:
-  pkg.installed:
-    - pkgs:
-      - redis-server
-      - redis
-      - securedrop-log
-    - require:
-      - sls: fpf-apt-test-repo
-
-redis:
-  service.running:
-    - enable: True
-
-securedrop-log:
-  service.running:
-    - enable: True
diff --git a/dom0/sd-log.sls b/dom0/sd-log.sls
index b88de2e4..50e77fca 100644
--- a/dom0/sd-log.sls
+++ b/dom0/sd-log.sls
@@ -37,6 +37,8 @@ sd-log:
     - features:
       - enable:
         - service.paxctld
+        - service.redis
+        - service.securedrop-log
     - require:
       - qvm: sd-log-buster-template
 
diff --git a/dom0/sd-logging-setup.sls b/dom0/sd-logging-setup.sls
new file mode 100644
index 00000000..4302f5b8
--- /dev/null
+++ b/dom0/sd-logging-setup.sls
@@ -0,0 +1,80 @@
+# -*- coding: utf-8 -*-
+# vim: set syntax=yaml ts=2 sw=2 sts=2 et :
+
+include:
+  - fpf-apt-test-repo
+
+{% if "template" in grains['id'] or grains['id'] in ["securedrop-workstation-buster", "whonix-gw-15"] %}
+# Install securedrop-log package in TemplateVMs only
+install-securedrop-log-package:
+  pkg.installed:
+    - pkgs:
+      - securedrop-log
+    - require:
+      - sls: fpf-apt-test-repo
+{% endif %}
+
+{% if grains['id'] == "sd-log-buster-template" %}
+install-redis-for-sd-log-template:
+  pkg.installed:
+    - pkgs:
+      - redis-server
+      - redis
+remove-sd-rsyslog-config-for-logserver:
+  file.absent:
+    - name: /etc/rsyslog.d/sdlog.conf
+
+{% elif grains['id'] == "sd-log" %}
+# Only for the "sd-log" AppVM, configure /rw/config to disable
+# custom log config, and also start the necessary services.
+sd-log-remove-rsyslog-qubes-plugin:
+  file.blockreplace:
+    - name: /rw/config/rc.local
+    - append_if_not_found: True
+    - marker_start: "### BEGIN securedrop-workstation ###"
+    - marker_end: "### END securedrop-workstation ###"
+    - content: |
+        # Removes sdlog.conf file for rsyslog
+        rm -f /etc/rsyslog.d/sdlog.conf
+        systemctl restart rsyslog
+        systemctl start redis
+        systemctl start securedrop-log
+  cmd.run:
+    - name: /rw/config/rc.local
+    - require:
+      - file: sd-log-remove-rsyslog-qubes-plugin
+
+{% elif grains['id'] == "sd-whonix" %}
+# We can not place the file on the template under /etc/rsyslog.d/ because of whonix
+# template. This sdlog.conf file is the same from the securedrop-log package, to
+# make sure that rsyslogd use our logging plugin.
+sd-rsyslog-sdlog-conf-for-sd-whonix:
+  file.managed:
+    - name: /rw/config/sdlog.conf
+    - source: "salt://sdlog.conf"
+
+# Because whonix-gw-15 template is not allowing to create the config file on
+# package install time, we do it via rc.local call.
+sd-rc-enable-logging:
+  file.blockreplace:
+    - name: /rw/config/rc.local
+    - append_if_not_found: True
+    - marker_start: "### BEGIN securedrop-workstation ###"
+    - marker_end: "### END securedrop-workstation ###"
+    - content: |
+        # Add sd-rsyslog.conf file for syslog
+        ln -sf /rw/config/sd-rsyslog.conf /etc/sd-rsyslog.conf
+        if [ ! -f /etc/rsyslog.d/sdlog.conf ]; then
+            ln -sf /rw/config/sdlog.conf /etc/rsyslog.d/sdlog.conf
+        fi
+        systemctl restart rsyslog
+  cmd.run:
+    - name: ln -sf /rw/config/sd-rsyslog.conf /etc/sd-rsyslog.conf && systemctl restart rsyslog
+
+{% else %}
+# For all other VMs, configure to send to sd-log
+configure-rsyslog-for-sd:
+  file.managed:
+    - name: /etc/sd-rsyslog.conf
+    - source: "salt://sd-rsyslog.conf.j2"
+{% endif %}
diff --git a/dom0/sd-proxy-template-files.sls b/dom0/sd-proxy-template-files.sls
index 8128ff3b..286ee7c6 100644
--- a/dom0/sd-proxy-template-files.sls
+++ b/dom0/sd-proxy-template-files.sls
@@ -2,6 +2,7 @@
 # vim: set syntax=yaml ts=2 sw=2 sts=2 et :
 include:
   - fpf-apt-test-repo
+  - sd-logging-setup
 
 sd-proxy-do-not-open-here-script:
   file.managed:
@@ -41,11 +42,10 @@ sd-proxy-configure-mimetypes:
 
 # Depends on FPF-controlled apt repo, already present
 # in underlying "securedrop-workstation" base template.
-install-securedrop-proxy-and-securedrop-log-package:
+install-securedrop-proxy-package:
   pkg.installed:
     - pkgs:
       - securedrop-proxy
-      - securedrop-log
     - require:
       - sls: fpf-apt-test-repo
 
@@ -60,11 +60,3 @@ install-securedrop-proxy-yaml-config:
     - context:
         hostname: {{ d.hidserv.hostname }}
     - mode: 0644
-
-sd-rsyslog-for-sd-proxy:
-  file.managed:
-    - name: /etc/sd-rsyslog.conf
-    - source: "salt://sd-rsyslog.conf.j2"
-    - template: jinja
-    - context:
-        vmname: sd-proxy
\ No newline at end of file
diff --git a/dom0/sd-rsyslog.conf.j2 b/dom0/sd-rsyslog.conf.j2
index b93ab64f..eabb5fc9 100644
--- a/dom0/sd-rsyslog.conf.j2
+++ b/dom0/sd-rsyslog.conf.j2
@@ -1,3 +1,2 @@
 [sd-rsyslog]
 remotevm = sd-log
-localvm = {{ vmname }}
diff --git a/dom0/sd-viewer-files.sls b/dom0/sd-viewer-files.sls
index 50ff7135..4c5baac8 100644
--- a/dom0/sd-viewer-files.sls
+++ b/dom0/sd-viewer-files.sls
@@ -29,17 +29,10 @@ sd-viewer-install-libreoffice:
         interval: 60
     - install_recommends: False
 
-sd-viewer-install-logging:
-  pkg.installed:
-    - pkgs:
-      - securedrop-log
-    - require:
-      - sls: fpf-apt-test-repo
-
 sd-rsyslog-for-sd-viewer:
   file.managed:
     - name: /etc/sd-rsyslog.conf
     - source: "salt://sd-rsyslog.conf.j2"
     - template: jinja
     - context:
-        vmname: sd-viewer
\ No newline at end of file
+        vmname: sd-viewer
diff --git a/dom0/sd-whonix-rsyslog-enable.sls b/dom0/sd-whonix-rsyslog-enable.sls
deleted file mode 100644
index c668a7da..00000000
--- a/dom0/sd-whonix-rsyslog-enable.sls
+++ /dev/null
@@ -1,43 +0,0 @@
-# -*- coding: utf-8 -*-
-# vim: set syntax=yaml ts=2 sw=2 sts=2 et :
-
-##
-# Enables securedrop-log rsyslog plugin in sd-whonix
-##
-
-sd-rsyslog-for-sd-whonix:
-  file.managed:
-    - name: /rw/config/sd-rsyslog.conf
-    - source: "salt://sd-rsyslog.conf.j2"
-    - template: jinja
-    - context:
-        vmname: sd-whonix
-
-
-# We can not place the file on the template under /etc/rsyslog.d/ because of whonix
-# template. This sdlog.conf file is the same from the securedrop-log package, to
-# make sure that rsyslogd use our logging plugin.
-sd-rsyslog-sdlog-conf-for-sd-whonix:
-  file.managed:
-    - name: /rw/config/sdlog.conf
-    - source: "salt://sdlog.conf"
-
-# Because whonix-gw-15 template is not allowing to create the config file on
-# package install time, we do it via rc.local call.
-sd-rc-enable-logging:
-  file.blockreplace:
-    - name: /rw/config/rc.local
-    - append_if_not_found: True
-    - marker_start: "### BEGIN securedrop-workstation ###"
-    - marker_end: "### END securedrop-workstation ###"
-    - content: |
-        # Add sd-rsyslog.conf file for syslog
-        ln -sf /rw/config/sd-rsyslog.conf /etc/sd-rsyslog.conf
-        if [ ! -f /etc/rsyslog.d/sdlog.conf ]; then
-            ln -sf /rw/config/sdlog.conf /etc/rsyslog.d/sdlog.conf
-        fi
-        systemctl restart rsyslog
-  cmd.run:
-    - name: ln -sf /rw/config/sd-rsyslog.conf /etc/sd-rsyslog.conf && systemctl restart rsyslog
-
-
diff --git a/dom0/sd-whonix-template-files.sls b/dom0/sd-whonix-template-files.sls
deleted file mode 100644
index adb7cc60..00000000
--- a/dom0/sd-whonix-template-files.sls
+++ /dev/null
@@ -1,22 +0,0 @@
-# -*- coding: utf-8 -*-
-# vim: set syntax=yaml ts=2 sw=2 sts=2 et :
-
-##
-# sd-whonix-template-files
-# ========
-#
-# Installs configuration packages specific to the sd-whonix
-# used for network calls.
-#
-##
-
-include:
-  - fpf-apt-test-repo
-
-sd-whonix-install-logging:
-  pkg.installed:
-    - pkgs:
-      - securedrop-log
-    - require:
-      - sls: fpf-apt-test-repo
-
diff --git a/dom0/sd-workstation-template-files.sls b/dom0/sd-workstation-template-files.sls
index da127baa..70efef85 100644
--- a/dom0/sd-workstation-template-files.sls
+++ b/dom0/sd-workstation-template-files.sls
@@ -11,14 +11,6 @@ sd-workstation-template-install-kernel-config-packages:
     - require:
       - sls: fpf-apt-test-repo
 
-
-sd-workstation-install-securedrop-log-package:
-  pkg.installed:
-    - pkgs:
-      - securedrop-log
-    - require:
-      - sls: fpf-apt-test-repo
-
 # Ensure that paxctld starts immediately. For AppVMs,
 # use qvm.features.enabled = ["paxctld"] to ensure service start.
 sd-workstation-template-enable-paxctld:
diff --git a/dom0/sd-workstation.top b/dom0/sd-workstation.top
index 548b289b..6e36b875 100644
--- a/dom0/sd-workstation.top
+++ b/dom0/sd-workstation.top
@@ -21,7 +21,7 @@ base:
     - sd-remove-unused-templates
 
   sd-log-buster-template:
-    - sd-log-template-files
+    - sd-logging-setup
   sd-devices-buster-template:
     - sd-devices-files
   sd-gpg:
@@ -38,15 +38,15 @@ base:
     - sd-sys-firewall-files
   sd-whonix:
     - sd-whonix-hidserv-key
-    - sd-whonix-rsyslog-enable
   securedrop-workstation-buster:
     - sd-workstation-template-files
+    - sd-logging-setup
   sys-usb:
     - sd-usb-autoattach-add
   whonix-gw-15:
-    - sd-whonix-template-files
+    - sd-logging-setup
   sd-log:
-    - sd-log-disable-plugin
+    - sd-logging-setup
 
   # "Placeholder" config to trigger TemplateVM boots,
   # so upgrades can be applied automatically via cron.

From b6406f182dc6fe3c5aa5b15b11a74fbf9b1749e8 Mon Sep 17 00:00:00 2001
From: Conor Schaefer <conor@freedom.press>
Date: Tue, 3 Mar 2020 15:22:18 -0800
Subject: [PATCH 2/7] Expands config tests for logging

Ensures services are running in 'sd-log', since log aggregation can't
work if they're not. These checks are added to make the overall test
suite more durable.

In addition, updates some of the test logic to stop expecting a
hardcoded VM name in the config file. This way, we can reuse the same
config logic on all VMs.
---
 tests/base.py            |  7 ++-----
 tests/test_log_vm.py     | 15 +++++++++++++++
 tests/test_sd_devices.py |  2 +-
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/tests/base.py b/tests/base.py
index 39f46a2a..6f5445d7 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -113,18 +113,15 @@ def _fileExists(self, remote_path):
 
         return True
 
-    def logging_configured(self, vmname=""):
+    def logging_configured(self):
         """
         Make sure rsyslog is configured to send in data to sd-log vm.
         """
-        if not vmname:
-            vmname = self.vm_name
         self.assertTrue(self._fileExists("/etc/sd-rsyslog.conf"))
         # Then we check the configuration inside of the file.
         file_content = self._get_file_contents("/etc/sd-rsyslog.conf")
         static_content = """[sd-rsyslog]
 remotevm = sd-log
-localvm = {0}
-""".format(vmname)
+"""
         self.assertEqual(file_content, static_content)
         self.assertTrue(self._package_is_installed("securedrop-log"))
diff --git a/tests/test_log_vm.py b/tests/test_log_vm.py
index 9d6a09c2..55823f19 100644
--- a/tests/test_log_vm.py
+++ b/tests/test_log_vm.py
@@ -11,10 +11,25 @@ def setUp(self):
     def test_sd_log_package_installed(self):
         self.assertTrue(self._package_is_installed("securedrop-log"))
 
+    def test_sd_log_redis_is_installed(self):
+        self.assertTrue(self._package_is_installed("redis"))
+        self.assertTrue(self._package_is_installed("redis-server"))
+
     def test_log_utility_installed(self):
         self.assertTrue(self._fileExists("/usr/sbin/securedrop-log"))
         self.assertTrue(self._fileExists("/etc/qubes-rpc/securedrop.Log"))
 
+    def test_sd_log_has_no_custom_rsyslog(self):
+        self.assertFalse(self._fileExists("/etc/rsyslog.d/sdlog.conf"))
+
+    def test_sd_log_service_running(self):
+        results = self._run("sudo systemctl is-active securedrop-log")
+        assert results == "active"
+
+    def test_redis_service_running(self):
+        results = self._run("sudo systemctl is-active redis")
+        assert results == "active"
+
 
 def load_tests(loader, tests, pattern):
     suite = unittest.TestLoader().loadTestsFromTestCase(SD_Log_Tests)
diff --git a/tests/test_sd_devices.py b/tests/test_sd_devices.py
index a638f0c5..b3e96435 100644
--- a/tests/test_sd_devices.py
+++ b/tests/test_sd_devices.py
@@ -21,7 +21,7 @@ def test_sd_export_package_installed(self):
         self.assertTrue(self._package_is_installed("securedrop-export"))
 
     def test_logging_configured(self):
-        self.logging_configured(vmname="sd-devices")
+        self.logging_configured()
 
     def test_mime_types(self):
         filepath = os.path.join(os.path.dirname(os.path.abspath(__file__)),

From fa2b53b0bc52998e0149cdd73a7a0e8e35d693d7 Mon Sep 17 00:00:00 2001
From: Conor Schaefer <conor@freedom.press>
Date: Tue, 3 Mar 2020 14:40:11 -0800
Subject: [PATCH 3/7] Removes duplicate sd-log RPC policy

We had the same task writing twice in dom0, so removing the duplicate as
unnecessary. Also updating the tests to ensure the right policy is
configured. In updating the config tests, discovered that we didn't have
any checks for the custom (i.e. `securedrop.*`) RPC policies, so added
those. Had to adjust the testing path assumptions somewhat in order to
accommodate.
---
 dom0/sd-log.sls          |  8 ------
 tests/test_qubes_rpc.py  |  4 +--
 tests/vars/qubes-rpc.yml | 58 +++++++++++++++++++++++-----------------
 3 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/dom0/sd-log.sls b/dom0/sd-log.sls
index 50e77fca..bf93f04c 100644
--- a/dom0/sd-log.sls
+++ b/dom0/sd-log.sls
@@ -59,11 +59,3 @@ sd-log-private-volume-size:
         qvm-volume resize sd-log:private {{ d.vmsizes.sd_log }}GiB
     - require:
       - qvm: sd-log
-
-# Permit the SecureDrop Proxy to manage Client connections
-sd-dom-dom0-securedrop.Log:
-  file.prepend:
-    - name: /etc/qubes-rpc/policy/securedrop.Log
-    - text: |
-        @tag:sd-workstation sd-log allow
-        @anyvm @anyvm deny
diff --git a/tests/test_qubes_rpc.py b/tests/test_qubes_rpc.py
index 37144ebf..59241533 100644
--- a/tests/test_qubes_rpc.py
+++ b/tests/test_qubes_rpc.py
@@ -3,7 +3,7 @@
 import unittest
 import yaml
 
-QUBES_POLICY_PREFIX = "/etc/qubes-rpc/policy/qubes."
+QUBES_POLICY_DIR = "/etc/qubes-rpc/policy"
 
 
 class SD_Qubes_Rpc_Tests(unittest.TestCase):
@@ -25,7 +25,7 @@ def test_Policies(self):
         self.assertFalse(fail)
 
     def _startsWith(self, filename, expectedPolicy):
-        filePath = os.path.join(QUBES_POLICY_PREFIX + filename)
+        filePath = os.path.join(QUBES_POLICY_DIR, filename)
         with io.open(filePath, 'r') as f:
             actualPolicy = f.read()
             if actualPolicy.startswith(expectedPolicy):
diff --git a/tests/vars/qubes-rpc.yml b/tests/vars/qubes-rpc.yml
index db1d9a64..256f84f3 100644
--- a/tests/vars/qubes-rpc.yml
+++ b/tests/vars/qubes-rpc.yml
@@ -1,18 +1,18 @@
-- policy: ClipboardPaste
+- policy: qubes.ClipboardPaste
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @anyvm @tag:sd-workstation deny
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: FeaturesRequest
+- policy: qubes.FeaturesRequest
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @anyvm @tag:sd-workstation deny
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: Filecopy
+- policy: qubes.Filecopy
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     sd-proxy @tag:sd-client allow
@@ -20,7 +20,7 @@
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: GetDate
+- policy: qubes.GetDate
   starts_with: |-
     ## Note that policy parsing stops at the first match,
     ## so adding anything below "$anyvm $anyvm action" line will have no effect
@@ -30,7 +30,7 @@
     $tag:anon-vm	$anyvm	deny
     $anyvm	$anyvm	allow,target=dom0
 
-- policy: GetRandomizedTime
+- 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
@@ -39,7 +39,7 @@
 
     $anyvm	dom0	allow
 
-- policy: GetImageRGBA
+- 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
@@ -49,7 +49,7 @@
     $anyvm $dispvm allow
     $anyvm $anyvm ask
 
-- policy: Gpg
+- policy: qubes.Gpg
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @tag:sd-client sd-gpg allow
@@ -57,7 +57,7 @@
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: GpgImportKey
+- policy: qubes.GpgImportKey
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @tag:sd-client sd-gpg allow
@@ -65,16 +65,16 @@
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: InputKeyboard
+- policy: qubes.InputKeyboard
   starts_with: |-
     $anyvm $anyvm deny
 
-- policy: InputMouse
+- policy: qubes.InputMouse
   starts_with: |-
     sys-usb dom0 allow,user=root
     $anyvm $anyvm deny
 
-- policy: NotifyTools
+- policy: qubes.NotifyTools
   starts_with: |-
     ## Note that policy parsing stops at the first match,
     ## so adding anything below "$anyvm $anyvm action" line will have no effect
@@ -83,7 +83,7 @@
 
     $anyvm	dom0	allow
 
-- policy: NotifyUpdates
+- policy: qubes.NotifyUpdates
   starts_with: |-
     ## Note that policy parsing stops at the first match,
     ## so adding anything below "$anyvm $anyvm action" line will have no effect
@@ -92,7 +92,7 @@
 
     $anyvm	dom0	allow
 
-- policy: OpenInVM
+- policy: qubes.OpenInVM
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @tag:sd-client @dispvm:sd-viewer allow
@@ -102,21 +102,21 @@
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: OpenURL
+- policy: qubes.OpenURL
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @anyvm @tag:sd-workstation deny
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: PdfConvert
+- policy: qubes.PdfConvert
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @anyvm @tag:sd-workstation deny
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: ReceiveUpdates
+- policy: qubes.ReceiveUpdates
   starts_with: |-
     ## Note that policy parsing stops at the first match,
     ## so adding anything below "$anyvm $anyvm action" line will have no effect
@@ -125,14 +125,24 @@
 
     $anyvm	dom0	allow
 
-- policy: StartApp
+- policy: securedrop.Log
+  starts_with: |-
+    @tag:sd-workstation sd-log allow
+    @anyvm @anyvm deny
+
+- policy: securedrop.Proxy
+  starts_with: |-
+    sd-app sd-proxy allow
+    @anyvm @anyvm deny
+
+- policy: qubes.StartApp
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @anyvm @tag:sd-workstation deny
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: SyncAppMenus
+- policy: qubes.SyncAppMenus
   starts_with: |-
     ## Note that policy parsing stops at the first match,
     ## so adding anything below "$anyvm $anyvm action" line will have no effect
@@ -141,7 +151,7 @@
 
     $anyvm	dom0	allow
 
-- policy: UpdatesProxy
+- policy: qubes.UpdatesProxy
   starts_with: |-
     ## Note that policy parsing stops at the first match,
     ## so adding anything below "$anyvm $anyvm action" line will have no effect
@@ -162,7 +172,7 @@
 
     $anyvm $anyvm deny
 
-- policy: USB
+- policy: qubes.USB
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     sd-devices sys-usb allow
@@ -170,7 +180,7 @@
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: USBAttach
+- policy: qubes.USBAttach
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     sys-usb sd-devices allow,user=root
@@ -178,20 +188,20 @@
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: VMRootShell
+- policy: qubes.VMRootShell
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @anyvm @tag:sd-workstation deny
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: VMShell
+- policy: qubes.VMShell
   starts_with: |-
     ### BEGIN securedrop-workstation ###
     @anyvm @tag:sd-workstation deny
     @tag:sd-workstation @anyvm deny
     ### END securedrop-workstation ###
 
-- policy: WindowIconUpdater
+- policy: qubes.WindowIconUpdater
   starts_with: |-
     $anyvm dom0 allow

From caa77af4bfc9a918a8d0cc142fa575d506932228 Mon Sep 17 00:00:00 2001
From: Conor Schaefer <conor@freedom.press>
Date: Thu, 5 Mar 2020 15:14:32 -0800
Subject: [PATCH 4/7] Sets log hostname for Whonix & Viewer

The sd-viewer config didn't have the logging setup included, which was
an oversight. The Whonix-related VMs, sd-whonix, sd-proxy, and
corresponding templates, require special treatment that's now accounted
for in the single log-config state file.

Significantly expanded test coverage in order to verify the end state.
---
 dom0/sd-logging-setup.sls | 16 ++++++++++------
 dom0/sd-viewer-files.sls  |  9 +--------
 dom0/sd-workstation.top   |  3 +++
 tests/base.py             | 18 ++++++++++++++++--
 tests/test_log_vm.py      | 19 +++++++++++++++++++
 tests/test_proxy_vm.py    |  2 +-
 tests/test_sd_whonix.py   |  2 +-
 7 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/dom0/sd-logging-setup.sls b/dom0/sd-logging-setup.sls
index 4302f5b8..8e3bc01d 100644
--- a/dom0/sd-logging-setup.sls
+++ b/dom0/sd-logging-setup.sls
@@ -44,7 +44,7 @@ sd-log-remove-rsyslog-qubes-plugin:
     - require:
       - file: sd-log-remove-rsyslog-qubes-plugin
 
-{% elif grains['id'] == "sd-whonix" %}
+{% elif grains['id'] in ["sd-whonix", "sd-proxy", "sd-proxy-buster-template"] %}
 # We can not place the file on the template under /etc/rsyslog.d/ because of whonix
 # template. This sdlog.conf file is the same from the securedrop-log package, to
 # make sure that rsyslogd use our logging plugin.
@@ -63,13 +63,17 @@ sd-rc-enable-logging:
     - marker_end: "### END securedrop-workstation ###"
     - content: |
         # Add sd-rsyslog.conf file for syslog
-        ln -sf /rw/config/sd-rsyslog.conf /etc/sd-rsyslog.conf
-        if [ ! -f /etc/rsyslog.d/sdlog.conf ]; then
-            ln -sf /rw/config/sdlog.conf /etc/rsyslog.d/sdlog.conf
-        fi
+        ln -sf /rw/config/sdlog.conf /etc/rsyslog.d/sdlog.conf
+        cat <<EOF > /etc/sd-rsyslog.conf
+        [sd-rsyslog]
+        remotevm = sd-log
+        localvm = {{ grains['id'] }}
+        EOF
         systemctl restart rsyslog
   cmd.run:
-    - name: ln -sf /rw/config/sd-rsyslog.conf /etc/sd-rsyslog.conf && systemctl restart rsyslog
+    - name: /rw/config/rc.local
+    - require:
+      - file: sd-rc-enable-logging-for-sd-whonix
 
 {% else %}
 # For all other VMs, configure to send to sd-log
diff --git a/dom0/sd-viewer-files.sls b/dom0/sd-viewer-files.sls
index 4c5baac8..3503b7c6 100644
--- a/dom0/sd-viewer-files.sls
+++ b/dom0/sd-viewer-files.sls
@@ -12,6 +12,7 @@
 
 include:
   - fpf-apt-test-repo
+  - sd-logging-setup
 
 sd-viewer-install-mimetype-handler-package:
   pkg.installed:
@@ -28,11 +29,3 @@ sd-viewer-install-libreoffice:
         attempts: 3
         interval: 60
     - install_recommends: False
-
-sd-rsyslog-for-sd-viewer:
-  file.managed:
-    - name: /etc/sd-rsyslog.conf
-    - source: "salt://sd-rsyslog.conf.j2"
-    - template: jinja
-    - context:
-        vmname: sd-viewer
diff --git a/dom0/sd-workstation.top b/dom0/sd-workstation.top
index 6e36b875..14326dd7 100644
--- a/dom0/sd-workstation.top
+++ b/dom0/sd-workstation.top
@@ -28,6 +28,8 @@ base:
     - sd-gpg-files
   sd-proxy-buster-template:
     - sd-proxy-template-files
+  sd-proxy:
+    - sd-logging-setup
   sd-app:
     - sd-app-config
   sd-viewer-buster-template:
@@ -38,6 +40,7 @@ base:
     - sd-sys-firewall-files
   sd-whonix:
     - sd-whonix-hidserv-key
+    - sd-logging-setup
   securedrop-workstation-buster:
     - sd-workstation-template-files
     - sd-logging-setup
diff --git a/tests/base.py b/tests/base.py
index 6f5445d7..5bef4749 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -113,15 +113,29 @@ def _fileExists(self, remote_path):
 
         return True
 
-    def logging_configured(self):
+    def logging_configured(self, vmname=False):
         """
         Make sure rsyslog is configured to send in data to sd-log vm.
+        Takes an optional 'vmname' argument, in case hostname
+        returned by system is an insufficient identifier, e.g. Whonix.
         """
+        self.assertTrue(self._package_is_installed("securedrop-log"))
+        self.assertTrue(self._fileExists("/usr/sbin/sd-rsyslog"))
+        self.assertTrue(self._fileExists("/etc/rsyslog.d/sdlog.conf"))
         self.assertTrue(self._fileExists("/etc/sd-rsyslog.conf"))
         # Then we check the configuration inside of the file.
         file_content = self._get_file_contents("/etc/sd-rsyslog.conf")
         static_content = """[sd-rsyslog]
 remotevm = sd-log
 """
+        # A hardcoded vmname should only be present if required,
+        # since securedrop-log will default to value of `hostname`.
+        if vmname:
+            static_content += "localvm = {}\n".format(self.vm_name)
         self.assertEqual(file_content, static_content)
-        self.assertTrue(self._package_is_installed("securedrop-log"))
+        # Check for evidence of misconfigured logging in syslog,
+        # fail if matching events found
+        # Several VMs show this error message even though they're shipping logs,
+        # so let's investigate further.
+        # 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")
diff --git a/tests/test_log_vm.py b/tests/test_log_vm.py
index 55823f19..ab87016a 100644
--- a/tests/test_log_vm.py
+++ b/tests/test_log_vm.py
@@ -30,6 +30,25 @@ def test_redis_service_running(self):
         results = self._run("sudo systemctl is-active redis")
         assert results == "active"
 
+    def test_logs_are_flowing(self):
+        cmd_output = self._run("ls -1 /home/user/QubesIncomingLogs")
+        log_dirs = cmd_output.split("\n")
+        # Confirm AppVMs are sending logs
+        self.assertTrue("sd-app" in log_dirs)
+        self.assertTrue("sd-whonix" in log_dirs)
+        # The following will only have logs if the machine has booted,
+        # which is not guaranteed given that we randomize test order.
+        # self.assertTrue("sd-devices" in log_dirs)
+        # self.assertTrue("sd-proxy" in log_dirs)
+        # self.assertTrue("sd-viewer" in log_dirs)
+
+    def test_log_dirs_properly_named(self):
+        # Rerunning this command to keep test output readable
+        cmd_output = self._run("ls -1 /home/user/QubesIncomingLogs")
+        log_dirs = cmd_output.split("\n")
+        # Confirm we don't have 'host' entries from Whonix VMs
+        self.assertFalse("host" in log_dirs)
+
 
 def load_tests(loader, tests, pattern):
     suite = unittest.TestLoader().loadTestsFromTestCase(SD_Log_Tests)
diff --git a/tests/test_proxy_vm.py b/tests/test_proxy_vm.py
index 9ecdf9fa..21e2f4ca 100644
--- a/tests/test_proxy_vm.py
+++ b/tests/test_proxy_vm.py
@@ -41,7 +41,7 @@ def test_whonix_ws_repo_absent(self):
         assert not self._fileExists("/etc/apt/sources.list.d/whonix.list")
 
     def test_logging_configured(self):
-        self.logging_configured()
+        self.logging_configured(vmname=True)
 
     def test_mime_types(self):
         with open("sd-proxy/mimeapps.list", "r") as f:
diff --git a/tests/test_sd_whonix.py b/tests/test_sd_whonix.py
index 3edd0c74..15bccb8e 100644
--- a/tests/test_sd_whonix.py
+++ b/tests/test_sd_whonix.py
@@ -59,7 +59,7 @@ def test_sd_whonix_repo_enabled(self):
         assert self._fileExists(self.whonix_apt_list)
 
     def test_logging_configured(self):
-        self.logging_configured()
+        self.logging_configured(vmname=True)
 
 
 def load_tests(loader, tests, pattern):

From be2796895f84c87a55a013e698603ff392f4944d Mon Sep 17 00:00:00 2001
From: Conor Schaefer <conor@freedom.press>
Date: Mon, 9 Mar 2020 08:26:02 -0700
Subject: [PATCH 5/7] Ensures sd-gpg does not log, adds tests

During the consolidation of the logging config, `sd-gpg` started trying
to log again, but it wasn't working correctly. Here we disable the
logging config explicitly, to match what's on master, and add a config test
to confirm.
---
 dom0/sd-logging-setup.sls | 18 ++++++++++++++++++
 dom0/sd-workstation.top   |  1 +
 tests/test_gpg.py         |  4 ++++
 3 files changed, 23 insertions(+)

diff --git a/dom0/sd-logging-setup.sls b/dom0/sd-logging-setup.sls
index 8e3bc01d..4bb33bd6 100644
--- a/dom0/sd-logging-setup.sls
+++ b/dom0/sd-logging-setup.sls
@@ -44,6 +44,24 @@ sd-log-remove-rsyslog-qubes-plugin:
     - require:
       - file: sd-log-remove-rsyslog-qubes-plugin
 
+{% elif grains['id'] == "sd-gpg" %}
+# For sd-gpg, we disable logging altogether, since access
+# to the keyring will be logged in sd-app
+sd-gpg-remove-rsyslog-qubes-plugin:
+  file.blockreplace:
+    - name: /rw/config/rc.local
+    - append_if_not_found: True
+    - marker_start: "### BEGIN securedrop-workstation ###"
+    - marker_end: "### END securedrop-workstation ###"
+    - content: |
+        # Removes sdlog.conf file for rsyslog
+        rm -f /etc/rsyslog.d/sdlog.conf
+        systemctl restart rsyslog
+  cmd.run:
+    - name: /rw/config/rc.local
+    - require:
+      - file: sd-gpg-remove-rsyslog-qubes-plugin
+
 {% elif grains['id'] in ["sd-whonix", "sd-proxy", "sd-proxy-buster-template"] %}
 # We can not place the file on the template under /etc/rsyslog.d/ because of whonix
 # template. This sdlog.conf file is the same from the securedrop-log package, to
diff --git a/dom0/sd-workstation.top b/dom0/sd-workstation.top
index 14326dd7..b0b09a66 100644
--- a/dom0/sd-workstation.top
+++ b/dom0/sd-workstation.top
@@ -26,6 +26,7 @@ base:
     - sd-devices-files
   sd-gpg:
     - sd-gpg-files
+    - sd-logging-setup
   sd-proxy-buster-template:
     - sd-proxy-template-files
   sd-proxy:
diff --git a/tests/test_gpg.py b/tests/test_gpg.py
index cb8df361..10705722 100644
--- a/tests/test_gpg.py
+++ b/tests/test_gpg.py
@@ -50,6 +50,10 @@ def test_sd_gpg_timeout(self):
     def test_we_have_the_key(self):
         self.assertEqual(get_local_fp(), get_remote_fp())
 
+    def test_logging_disabled(self):
+        # Logging to sd-log should be disabled on sd-gpg
+        self.assertFalse(self._fileExists("/etc/rsyslog.d/sdlog.conf"))
+
 
 def load_tests(loader, tests, pattern):
     suite = unittest.TestLoader().loadTestsFromTestCase(SD_GPG_Tests)

From e73aceb3e3d77a1a8dfd64efebd1dbf6521bd3c6 Mon Sep 17 00:00:00 2001
From: Conor Schaefer <conor@freedom.press>
Date: Mon, 9 Mar 2020 10:59:46 -0700
Subject: [PATCH 6/7] Removes special case logging setup for proxy

The `sd-proxy` machine was previously based on a Whonix Workstation
template, which required special handling of the hostname, which is
"host" on all Whonix VMs. We've since ported the proxy to the SDW base
template, which returns accurate hostnames, so we can drop the custom
logic here.
---
 dom0/sd-logging-setup.sls | 4 ++--
 dom0/sd-workstation.top   | 2 --
 tests/test_proxy_vm.py    | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/dom0/sd-logging-setup.sls b/dom0/sd-logging-setup.sls
index 4bb33bd6..64db8ae9 100644
--- a/dom0/sd-logging-setup.sls
+++ b/dom0/sd-logging-setup.sls
@@ -62,7 +62,7 @@ sd-gpg-remove-rsyslog-qubes-plugin:
     - require:
       - file: sd-gpg-remove-rsyslog-qubes-plugin
 
-{% elif grains['id'] in ["sd-whonix", "sd-proxy", "sd-proxy-buster-template"] %}
+{% elif grains['id'] == "sd-whonix" %}
 # We can not place the file on the template under /etc/rsyslog.d/ because of whonix
 # template. This sdlog.conf file is the same from the securedrop-log package, to
 # make sure that rsyslogd use our logging plugin.
@@ -73,7 +73,7 @@ sd-rsyslog-sdlog-conf-for-sd-whonix:
 
 # Because whonix-gw-15 template is not allowing to create the config file on
 # package install time, we do it via rc.local call.
-sd-rc-enable-logging:
+sd-rc-enable-logging-for-sd-whonix:
   file.blockreplace:
     - name: /rw/config/rc.local
     - append_if_not_found: True
diff --git a/dom0/sd-workstation.top b/dom0/sd-workstation.top
index b0b09a66..3acf2040 100644
--- a/dom0/sd-workstation.top
+++ b/dom0/sd-workstation.top
@@ -29,8 +29,6 @@ base:
     - sd-logging-setup
   sd-proxy-buster-template:
     - sd-proxy-template-files
-  sd-proxy:
-    - sd-logging-setup
   sd-app:
     - sd-app-config
   sd-viewer-buster-template:
diff --git a/tests/test_proxy_vm.py b/tests/test_proxy_vm.py
index 21e2f4ca..9ecdf9fa 100644
--- a/tests/test_proxy_vm.py
+++ b/tests/test_proxy_vm.py
@@ -41,7 +41,7 @@ def test_whonix_ws_repo_absent(self):
         assert not self._fileExists("/etc/apt/sources.list.d/whonix.list")
 
     def test_logging_configured(self):
-        self.logging_configured(vmname=True)
+        self.logging_configured()
 
     def test_mime_types(self):
         with open("sd-proxy/mimeapps.list", "r") as f:

From adc6dd4b62ae89b7db7fd77b41a4c9cd431d711d Mon Sep 17 00:00:00 2001
From: Conor Schaefer <conor@freedom.press>
Date: Mon, 9 Mar 2020 18:55:38 -0700
Subject: [PATCH 7/7] Reboots logging VMs during install

A side-effect of consolidating the log collection logic is that all VMs,
even the TemplateVM for sd-log, begins shipping logs immediately upon
configuration. That's a problem for sd-log AppVM & TemplateVM because
the RPC calls to send logs will cause the AppVM to boot. If the AppVM
doesn't have the final config from the TemplateVM, because of an early
boot, log aggregation won't work. Let's configure both the TemplateVM &
AppVM, then shut down the TemplateVM (automatic as part of salt disp
mgmt vm logic), then reboot the AppVM. All tests passing on a clean
install.
---
 scripts/provision-all | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/provision-all b/scripts/provision-all
index 2955af37..cb595c09 100755
--- a/scripts/provision-all
+++ b/scripts/provision-all
@@ -17,11 +17,12 @@ sudo qubesctl --show-output --skip-dom0 --targets sys-firewall state.sls sd-sys-
 echo "Set up dom0 config files, including RPC policies, and create VMs"
 sudo qubesctl --show-output state.highstate
 
-echo "Setup sd-log-buster-template vm first"
-sudo qubesctl --show-output --skip-dom0 --targets sd-log-buster-template state.highstate
+echo "Set up logging VMs early"
+sudo qubesctl --show-output --skip-dom0 --targets sd-log,sd-log-buster-template state.highstate
+# Reboot sd-log so it's ready to receive logs from other VMs about to be configured
+qvm-shutdown --wait sd-log && qvm-start sd-log
 # Provision whonix-gw-15 with log additions because it isn't tagged with sd-workstation (we don't want it removed after a make clean)
 sudo qubesctl --show-output --skip-dom0 --targets whonix-gw-15 state.highstate
-#sudo qubesctl --show-output --skip-dom0 --targets whonix-gw-15 state.sls sd-whonix-template-files
 qvm-shutdown --wait whonix-gw-15
 
 # Format list of all VMs comma-separated, for use as qubesctl target