Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove special casing for sd-whonix #618

Merged
merged 1 commit into from
Oct 14, 2020
Merged

Remove special casing for sd-whonix #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion dom0/sd-clean-whonix.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ remove-securedrop-log-package-from-whonix:
sd-cleanup-whonix-gw-15:
cmd.run:
- names:
- sudo rm -f /etc/rsyslog.d/sdlog.conf
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we use pkg.purged to remove the securedrop-log package, which removes config files:

Verify that a package is not installed, calling pkg.purge if necessary to purge the package. All configuration files are also removed.

Removing a configuration file on a Debian system by other means can have unpredictable consequences: the package manager will not reinstate it , because it will assume that the user removed it intentionally. For this reason, I think it's best to rely on pkg.purged alone here.

- sudo rm -f /etc/apt/sources.list.d/securedrop_workstation.list
- sudo systemctl restart rsyslog
- sudo apt-key del 4ED79CC3362D7D12837046024A3BE4A92211B03C
54 changes: 23 additions & 31 deletions dom0/sd-logging-setup.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,41 +62,33 @@ sd-gpg-remove-rsyslog-qubes-plugin:
- require:
- file: sd-gpg-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.
Copy link
Member Author

@eloquence eloquence Oct 6, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, this was added in 232c56f. @kushaldas, can you elaborate on what you meant at the time with "whonix-gw-15 template is not allowing to create the config file on package install time"? As far as I can tell, the config file /etc/rsyslog.d/sdlog.conf is installed just fine from the securedrop-log package, and I've not observed any behavior specific to whonix-gw-15 that would prevent this from being the case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#447 this is the PR and as I can see in my logs, the file was still missing even after the package was installed.

Here is my note from that work:

* https://github.com/freedomofpress/securedrop-workstation/pull/447  howt to remove the files?
  - how to remove gpg key? sudo apt-key del KEY_ID
  - `make clean`
  - remove works properly 
  - `make all` agagin.
  - most of the vms decided not to reply to Qubes calls :(
  - cleaned, and `make all` again 
  - Someday Qubes and Salt will play nicely together. Question is in which year?
  - this try, the package is there, but, no logs. 
  - because the sdlog.conf file is missing, WHY WHY WHY
  - I this before, i commented on the issue, dpkg did not create the sdlog.conf file at all on the system.
  - one idea, if the file is missing, just copy it over via rc.local file
  - testing again 
  - testing again, adding sdlog.conf only for sd-whonix if missing.
  - testing once more, things look better now
  - meeting with Mickael
  - need to test some more changes
  - make clean and make all again at super late night :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kushaldas, I experienced this once during testing with dpkg, and in my experience it happens when you've previously removed the file by hand and uninstalled (rather than purged) the package - regardless of the order of the two operations. It's not specific to Whonix, and is just how Debian handles configuration files. Because we purge the file correctly with our uninstall logic, I don't believe it should be an issue on production systems.

sd-rc-enable-logging-for-sd-whonix:
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/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: /rw/config/rc.local
- require:
- file: sd-rc-enable-logging-for-sd-whonix

{% 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 %}

# Remove outdated configuration that was previously used to configure the
# sd-whonix VM name for logging purposes, see:
# https://github.com/freedomofpress/securedrop-workstation/issues/583
#
# Can be removed in a future release once all production workstations have
# been updated.
{% if grains['id'] == "sd-whonix" %}
sd-whonix-cleanup-rc-local:
file.replace:
- names:
- /rw/config/rc.local
- pattern: '### BEGIN securedrop-workstation ###.*### END securedrop-workstation ###\s*'
- flags:
- MULTILINE
- DOTALL
- repl: ''
- backup: no

sd-whonix-cleanup-sdlog-conf:
file.absent:
- name: /rw/config/sdlog.conf
{% endif %}
8 changes: 1 addition & 7 deletions tests/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,9 @@ def _fileExists(self, remote_path):

return True

def logging_configured(self, vmname=False):
def logging_configured(self):
"""
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"))
Expand All @@ -120,10 +118,6 @@ def logging_configured(self, vmname=False):
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)
# Check for evidence of misconfigured logging in syslog,
# fail if matching events found
Expand Down
2 changes: 1 addition & 1 deletion tests/test_sd_whonix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def test_sd_whonix_repo_enabled(self):
assert self._fileExists(self.whonix_apt_list)

def test_logging_configured(self):
self.logging_configured(vmname=True)
self.logging_configured()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test_logging_configured shouldn't be removed, it should just have its call updated to pass no args (matching the modified test logic).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed (I misread the inheritance logic).

def test_sd_whonix_verify_tor_config(self):
# User must be debian-tor for v3 Onion, due to restrictive
Expand Down