Template consolidation via GUI updater #619

conorsch · 2020-10-08T15:41:55Z

Status

Ready for review

Description of Changes

Closes #471.

Consolidations the SDW templates into two: sd-small-buster-template and sd-large-buster-template.
Handles migration via GUI updater
Updates tests throughout

Depends on multiple closely related PRs:

Template consolidation test plan

The procedure for upgrading prod machines will be as follows:

Proactively contact pilot participants to notify about special case update.
Release all packages, both RPMs (for dom0) and debs (for SDW VMs), at same time.
When GUI updater appears, users should close it, open a dom0 terminal, and run sudo qubes-dom0-update -y. Doing so will obtain the latest GUI updater code.
Users should then double-click the SecureDrop desktop icon to rerun the new GUI updater, which will handle the consolidation steps as part of the upgrade.

Given the special case of requesting user action, we'll need to test both the "happy" path, where users follow our advice exactly, and the "sad" path, where special action is not taken.

See detailed test plan for the upcoming 0.5.0 release: https://github.com/freedomofpress/securedrop-workstation/wiki/0.5.0-Test-Plan Complete only the sections under Scenario: Template consolidation. The other scenarios we'll handle as part of QA, after merging this and related PRs.

QA

Writing a separate "QA" checklist because merging this and its related PR requires careful orchestration. Prior to merge of this PR, there are test-only commits marked "TEMPORARY" that should be removed. Doing so is required to use the "main" channel of the apt-test repository. Right now, packages are only present in the "template-consolidation" channel to facilitate testing.

The following PRs must be reviewed, then merged together:

Moves config to private volume securedrop-proxy#79
Reorganizes mimetype declarations for template consolidation securedrop-builder#198
PR moving the built packages to apt-test main channel

Checklist

If you have made code changes

Linter (make flake8) passes in the development environment (this box may
be left unchecked, as flake8 also runs in CI)

If you have made changes to the provisioning logic

All tests (make test) pass in dom0 of a Qubes install
This PR adds/removes files, and includes required updates to the packaging
logic in MANIFEST.in and rpm-build/SPECS/securedrop-workstation-dom0-config.spec

Switching a prod install to update from QA repositories

For testing purposes it may be necessary to modify a prod installation to use QA repos. The process of doing so is threefold:

update /etc/apt/source.list.d/securedrop_workstation.list to use apt-test.freedom.press and add the testing key in all Debian-based SD templates
update the repo definition in dom0 from yum.securedrop.org to yum-test.securedrop.org
update /srv/salt/sd-default-config.yml to point the prod env definition at dev env variables.

To do so safely using Salt, as root:

untar /home/user/securedrop-workstation/scripts/qa-switch.tar.gz in /srv/salt
run bash /srv/salt/consolidation-qa-switch.sh

conorsch · 2020-10-09T16:17:13Z

dom0/sd-logging-setup.sls

@@ -14,15 +14,12 @@ install-securedrop-log-package:
      - sls: fpf-apt-test-repo
 {% endif %}

-{% if grains['id'] == "sd-log-buster-template" %}
+{% if grains['id'] in ["sd-small-buster-template", "sd-large-buster-template"] %}


Do we need to install redis in large template? Should only be required for receiver of logs.

conorsch · 2020-10-09T16:21:03Z

dom0/sd-workstation.top

    - sd-devices-files
+    - sd-viewer-files


TODO: add mime handling to small & large templates, also to sd-gpg appvm

zenmonkeykstop · 2020-10-09T18:49:46Z

Test plan above working well, seeing expected result after updates. Couple of issues though:

full install takes a while, throwing off the progress bar. This could be confusing to users, messaging or redoing progress bar would help
first attempt hit a libxenlight failure creating sd-proxy. It's not immediately clear to me how a production user would recover from this, and it is more likely because of said full install (vs just template updates in usual scenario)
two error seen in make test. The first is expected, the second not (and is mime-type related):

======================================================================
FAIL: test_log_dirs_properly_named (test_log_vm.SD_Log_Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kog/securedrop-workstation/tests/test_log_vm.py", line 50, in test_log_dirs_properly_named
    self.assertFalse("host" in log_dirs)
AssertionError: True is not false

======================================================================
FAIL: test_mime_types (test_viewer.SD_Viewer_Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kog/securedrop-workstation/tests/test_viewer.py", line 37, in test_mime_types
    self.assertEqual(actual_app, expected_app)
AssertionError: 'open-in-dvm.desktop' != 'org.gnome.gedit.desktop'
- open-in-dvm.desktop
+ org.gnome.gedit.desktop


----------------------------------------------------------------------

conorsch · 2020-10-16T23:54:58Z

Rebased to stay current with "main". To proceed with the securedrop-export changes, we'll need to review and merge freedomofpress/securedrop-apt-test#67, then use the template-consolidation channel to evaluate provisioning, upgrade, and config test coverage here.

After that, I plan to document the /tmp/-flag rationale in a detailed comment, and tell bandit not to worry about it.

conorsch · 2020-10-19T17:14:14Z

On latest revisions, still seeing two failing tests:

FAIL: test_mime_types (test_sd_devices.SD_Devices_Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/securedrop-workstation/tests/test_sd_devices.py", line 37, in test_mime_types
    self.assertEqual(actual_app, expected_app)
AssertionError: '' != 'open-in-dvm.desktop'
+ open-in-dvm.desktop

======================================================================
FAIL: test_mime_types (test_viewer.SD_Viewer_Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/securedrop-workstation/tests/test_viewer.py", line 37, in test_mime_types
    self.assertEqual(actual_app, expected_app)
AssertionError: 'vim.desktop' != 'org.gnome.gedit.desktop'
- vim.desktop
+ org.gnome.gedit.desktop


----------------------------------------------------------------------
Ran 78 tests in 225.856s

FAILED (failures=2)
Makefile:109: recipe for target 'test' failed
make: *** [test] Error 1

Those are surely related to the recent export changes in freedomofpress/securedrop-export#65, so let's investigate.

…pdate logic doesn't break)

The sd-proxy mimetype handlers will be configured via the `securedrop-workstation-config` package, same as others.

emkll

So far, testing has been successful in both the new install case and the upgrade case (in the dev environment):

upgrade testing from main based on the test plan, with all tests passing
provisioning from scratch on this branch, with all tests passing

I did notice however a minor deviation /usr/share/applications/mimeapps.list is present on the small template but not the large template. this is because 1. freedomofpress/securedrop-client#1160 is not yet merged and 2. https://github.com/freedomofpress/securedrop-dev-packages-lfs/tree/main/workstation/template-consolidation does not create a version of the package greater than 2.1 with these changes (ie removing mimeapps.list in the template in which securedrop-client is installed).

The presence of this file is helpful to ensure defense-in-depth against mime handling misconfiguration (defaulting to a safe, open-in-dvm option), but it may be best handled in securedrop-workstation-config package (or the salt logic) to ensure converge accross both templates

We'll be posting the rc1 package to yum-test to aid in QA. Not using a tag, as described in docs [0], since we're postponing merge of feature branch [1] until QA is finished, given the scope of changes. [0] https://github.com/freedomofpress/securedrop-workstation/wiki/Building-securedrop-workstation-dom0-config-RPM-package [1] #619

Based on changes presented in freedomofpress/securedrop-workstation#619 Note that no tag is used here, since QA is happening against the feature branch. In lieu of a tag, I verified a commit in the build logs, but that commit hash will be lost with subsequent rebases. Still, the sig verification is useful for review of the package.

For QA during review of [0]. See build logs at [0] freedomofpress/securedrop-workstation#619 [1] freedomofpress/build-logs@ef65668

emkll · 2020-10-27T15:45:41Z

Ran into some issues when running make dev (from scratch) locally on this branch, specifically with logging: only sd-whonix and securedrop-workstation-buster are receiving logs. The packages and apt configuration appea3er to be correctly configured, and rsyslog is running on the VMs, but the logs are not being stored (and did not see any errors in dom0 logs). By bumping the version in the rpmspec, the logging is now working when running make dev, and now all tests are passing

I propose we:

Update the rpmspec version/changelog
drop the temporary commits
consider removing the switch logic from this repository (or removing them from the production RPM, to minimize the likelihood of miconfiguration), requiring devs to manually shuttle the files to dom0 i think is fine, since they don't need regular updating
merge this PR and all associated packaging PRs, and run nightly tasks + add packages with no nightly builds to the main branch
Observe what happens to developers and follow up with fixes as required

EDIT: (In the original comment, I had not pulled latest changes) After pulling latest changes, i can confirm make dev is working as expected, and all tests are passing. This is good to merge once we remove the commits described above.

conorsch · 2020-10-27T17:49:24Z

Great, thanks for confirming @emkll! In standup today we agreed it's time to starting merging. @zenmonkeykstop is going to move the qa-switch material out of the RPM, then I'll drop the temporary commits—both here and in the related PRs—and promote to final "ready-for-review" status, so we can get approvals in.

With this change, all SDW VMs are using the new consolidated small/large templates. Reuses salt state files in top file The new small & large templates don't need to be configured in their own state files, we can just add references to the top file to assign the old states per vm and consolidate them. Removes unconsolidated templates from config Don't bother to create them if we're not planning on using them. Updates make-dev target to use new consolidated templates sd-gpg: wait for consolidated templates The consolidated templates spike was handled in a different SLS file, which sd-gpg wasn't depending on it. That makes for a race, so let's update the dependency list to ensure proper ordering. Removed old templates and updated tests. Removed definitions for sd-{app,log,proxy}-buster-template VMs. Added back dom0/mimeapps.list, as it's used by config tests. Updated tests to refer to new templates. Updated logging setup to ensure correct config present on non sd-log VMs Consolidates templates via handle-upgrade script The "securedrop-handle-upgrade" script was added in order to migrate from Stretch-based to Buster-based templates. With some small changes to the expected template pattern, we can leverage the same script to handle consolidating the templates. This only handles *creation* of the VMs, it doesn't sort out the apt packages and config logic specific to each Template/App combination. Additional logic will need to be added to the updater to support hands-off consolidation. Uses new pkg securedrop-workstation-viewer Supersedes the securedrop-workstation-sd-svs-disp package, to make the transition to consolidated templates a bit easier, by avoiding dpkg conflicts altogether. Tests for securedrop-workstation-svs-disp absence The package "securedrop-workstation-svs-disp" has been superseded by "securedrop-workstation-viewer", so in testing for presence of one, ensure the absence of the other. Updates the config tests to permit testing package absence; before it was showing a confusing stack trace if package was missing (so error rather than failure). Forces template setting on sd-log Using "prefs" rather than "present" so that the value is updated, not merely set at VM creation time. Sets default mimetype handler for sd-proxy Uses the new sd-mime-handling state and applies it to sd-proxy, so that symlinks for mimetypes are handled appropriately post-template- consolidation. Requires packaging changes that are for now only in a feature branch on the dev packages repo, served via apt-test. Includes a small fix to ensure that the files in private volume are owned by normal user, not root. Removes unused sd-proxy files The do-not-open here files are left over from a prototype of the workstation, no longer required. Tests sd-proxy: use open-in-dvm for mimetypes We've ditched the do-not-open-here logic, so sd-proxy should now default to using open-in-dvm for all filetypes. The config tests now reflect this. Copy/pasted from the sd-app tests, didn't bother to refactor to make it DRY. Fix mimetype private volume perms Using "mode" and "makedirs" together for a symlink led to a broken config: Salt was creating the parent directories with mode 644, so they weren't traversable, so the mimeapps.list file couldn't be read by normal user. Fixed. Handle upgrade script: remove old templates Leverages the "remove" subcommand for the securedrop-handle-upgrade script to purge the old, non-consolidated TemplateVMs after migrating. Moves sd-proxy config to private volume Requires package update to securedrop-proxy, so that the RPC config at /etc/qubes-rpc/securedrop.Proxy references the new filepath. Includes a new test for that file, since we weren't explicitly examining its contents before. Removes legacy sd-proxy config file It shouldn't ever be present post-consolidated, given that the newly created VM never had the /etc/ path configured, but adding anyway as a defensive measure, and also to get the tests passing sooner. Moves consolidated templates into single state We originally had the consolidated templates in their own salt state file, to aid in development, so we could target just that one component. Folding into the pre-existing state file for the base template now, and cleaning up the extra references, mostly to minimize the diff and aid in review.

Follow up to [PR]. When running "make clone && make prep-dev" repeatedly in dom0 during development, the dnf cache in dom0 gets out of sync, and gets confused about whether the "securedrop-workstation-dom0-config" package is installed. Running "sudo dnf clean all" purges the cache and keeps things working smoothly.

Adds a "securedrop-check-migration" script that will drop a flag in /tmp/, for the updater's consumption, if the updater should rerun all salt states, rather than just package upgrades. Required for significant changes like template consolidation.

As explained in the comment, we accept the risk of a hardcoded tempfile in dom0, because it's a single-user system, isolated from the other VM components. At a later date, we'll likely port the check-migration logic to pure python, but unfortunately 'import qubesadmin' would break the mocked CI tests for the launcher and updater.

Follow-up to [0]. Post-consolidation, we can expect the /etc/profile.d/ path to be present on all systems, but only on sd-app should it return "sd-gpg" rather than an empty string. [0] #623

Ensuring that we're able to ship unattended migrations in the future. It's important that *first* we update the dom0 RPM, if required, then re-apply the dom0 state. After that, we'll check whether a migration is requested (that's the clincher), then either run a full-state apply or upgrade packages within each TemplateVM, depending on the check. Tinkered with the progress bars accordingly. It's not ideal, since the full-state run blocks the progress bar.

Squashes a few related commits for the qa-switcher logic, including the final change prior to merge that moved the files to a separate location so they wouldn't be included in the rc RPMs.

We'll be posting the rc1 package to yum-test to aid in QA. Not using a tag, as described in docs [0], since we're postponing merge of feature branch [1] until QA is finished, given the scope of changes. [0] https://github.com/freedomofpress/securedrop-workstation/wiki/Building-securedrop-workstation-dom0-config-RPM-package [1] #619

Including a "0.5.0" message as a start, even though we're only on rc1.

conorsch · 2020-10-27T19:46:59Z

Removed temporary commits for final review. Preserved the utils/ subdir for the qa-switcher logic, since that's required for QA as described in https://github.com/freedomofpress/securedrop-workstation/wiki/0.5.0-Test-Plan Squashed some of fix-it commits together.

Ready for final review!

emkll

diff from the previous review looks good to me https://github.com/freedomofpress/securedrop-workstation/compare/3d16c1e..fc35345

With the associated packages changes merged in other repos, this should be good to merge

emkll · 2020-10-27T22:32:33Z

All nightlies/packages and changes required for this branch have been updated on apt-test https://apt-test.freedom.press/pool/main/

As part of review, we've moved candidate packages from the "template-consolidation" channel on apt-test to "main" on apt-test. Therefore the qa-switch tool must be updated accordingly.

emkll

Changes to qa-switcher look good to me

Now that we've merged the template-consolidation PR [0], we can resume the normal rc build procedure documented in the wiki [1]. [0] #619 [1] https://github.com/freedomofpress/securedrop-workstation/wiki/Building-securedrop-workstation-dom0-config-RPM-package

We'll be posting the rc1 package to yum-test to aid in QA. Not using a tag, as described in docs [0], since we're postponing merge of feature branch [1] until QA is finished, given the scope of changes. [0] https://github.com/freedomofpress/securedrop-workstation/wiki/Building-securedrop-workstation-dom0-config-RPM-package [1] freedomofpress/securedrop-workstation#619

Now that we've merged the template-consolidation PR [0], we can resume the normal rc build procedure documented in the wiki [1]. [0] freedomofpress/securedrop-workstation#619 [1] https://github.com/freedomofpress/securedrop-workstation/wiki/Building-securedrop-workstation-dom0-config-RPM-package

…y-rebased Template consolidation via GUI updater

conorsch mentioned this pull request Oct 9, 2020

Moves config to private volume freedomofpress/securedrop-proxy#79

Merged

conorsch commented Oct 9, 2020

View reviewed changes

This was referenced Oct 9, 2020

[0.2.3-rpm] libxenlight failed to create new domain sd-log #498

Closed

Consolidate SDW templates wherever possible #471

Closed

zenmonkeykstop added 2 commits October 16, 2020 16:20

initial stab at small/large templates

7c4f7f9

moved template package installs to their own state files

0bd7493

conorsch force-pushed the 608-import-export-security-rebased branch from c69db7e to 6768811 Compare October 16, 2020 23:53

conorsch mentioned this pull request Oct 19, 2020

Remove mimetype associations from package freedomofpress/securedrop-client#1160

Merged

zenmonkeykstop and others added 5 commits October 20, 2020 17:20

updating sd-app, sd-log, sd-proxy to use small template

9fcd3cf

adding the old templates back in for comparison's sake (also so the u…

eaaf21c

…pdate logic doesn't break)

include sd-templates in appVM state files that use them

cd0d92b

update small templateVM references in Makefile and provisioning script

4f17c1b

Removes sd-proxy mimeapps.list handling

9f8cf7c

The sd-proxy mimetype handlers will be configured via the `securedrop-workstation-config` package, same as others.

conorsch force-pushed the 608-import-export-security-rebased branch from e3ae0ae to 3671717 Compare October 21, 2020 00:22

conorsch mentioned this pull request Oct 21, 2020

Support enforcing state for other VMs #567

Closed

conorsch force-pushed the 608-import-export-security-rebased branch from 3671717 to 63b42bf Compare October 21, 2020 00:26

emkll reviewed Oct 21, 2020

View reviewed changes

zenmonkeykstop force-pushed the 608-import-export-security-rebased branch 2 times, most recently from bd2f888 to ff2a599 Compare October 22, 2020 15:50

conorsch requested review from sssoleileraaa and zenmonkeykstop October 22, 2020 19:25

conorsch pushed a commit to freedomofpress/securedrop-yum-test that referenced this pull request Oct 22, 2020

Adds sdw-dom0-config 0.5.0-rc1

0dcc0a5

For QA during review of [0]. See build logs at [0] freedomofpress/securedrop-workstation#619 [1] freedomofpress/build-logs@ef65668

This was referenced Oct 22, 2020

Adds sdw-dom0-config 0.5.0-rc1 freedomofpress/securedrop-yum-test#16

Merged

Prod uninstall doesn't reset power menu #625

Closed

conorsch changed the title ~~[WIP] Template consolidation via GUI updater~~ Template consolidation via GUI updater Oct 27, 2020

Conor Schaefer and others added 10 commits October 27, 2020 12:42

Updates sd-app gpg config tests

b00a0ca

Follow-up to [0]. Post-consolidation, we can expect the /etc/profile.d/ path to be present on all systems, but only on sd-app should it return "sd-gpg" rather than an empty string. [0] #623

updated launcher to update each templateVM only once per run

43a3fc8

Added qa switcher script for consolidation testing

40caff5

Squashes a few related commits for the qa-switcher logic, including the final change prior to merge that moved the files to a separate location so they wouldn't be included in the rc RPMs.

Updates changelog for RPM

fc35345

Including a "0.5.0" message as a start, even though we're only on rc1.

conorsch force-pushed the 608-import-export-security-rebased branch from 3d16c1e to fc35345 Compare October 27, 2020 19:45

conorsch marked this pull request as ready for review October 27, 2020 19:47

conorsch added this to Under Review in SecureDrop Team Board Oct 27, 2020

emkll previously approved these changes Oct 27, 2020

View reviewed changes

Removes template-consolidation from qa-switch

c40aa38

As part of review, we've moved candidate packages from the "template-consolidation" channel on apt-test to "main" on apt-test. Therefore the qa-switch tool must be updated accordingly.

conorsch dismissed emkll’s stale review via c40aa38 October 27, 2020 22:41

emkll approved these changes Oct 27, 2020

View reviewed changes

conorsch merged commit d4a72f0 into main Oct 27, 2020

SecureDrop Team Board automation moved this from Under Review to Done Oct 27, 2020

conorsch mentioned this pull request Oct 27, 2020

Bumps RPM version 0.5.0-rc1 -> 0.5.0-rc2 #626

Merged

3 tasks

cfm pushed a commit that referenced this pull request Apr 1, 2024

Merge pull request #619 from freedomofpress/608-import-export-securit…

766a2f5

…y-rebased Template consolidation via GUI updater

legoktm deleted the 608-import-export-security-rebased branch May 28, 2024 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Template consolidation via GUI updater #619

Template consolidation via GUI updater #619

conorsch commented Oct 8, 2020 •

edited

Loading

conorsch Oct 9, 2020

conorsch Oct 9, 2020

zenmonkeykstop commented Oct 9, 2020

conorsch commented Oct 16, 2020

conorsch commented Oct 19, 2020

emkll left a comment •

edited

Loading

emkll commented Oct 27, 2020 •

edited

Loading

conorsch commented Oct 27, 2020

conorsch commented Oct 27, 2020

emkll left a comment

emkll commented Oct 27, 2020

emkll left a comment

Template consolidation via GUI updater #619

Template consolidation via GUI updater #619

Conversation

conorsch commented Oct 8, 2020 • edited Loading

Status

Description of Changes

Template consolidation test plan

QA

Checklist

If you have made code changes

If you have made changes to the provisioning logic

Switching a prod install to update from QA repositories

conorsch Oct 9, 2020

Choose a reason for hiding this comment

conorsch Oct 9, 2020

Choose a reason for hiding this comment

zenmonkeykstop commented Oct 9, 2020

conorsch commented Oct 16, 2020

conorsch commented Oct 19, 2020

emkll left a comment • edited Loading

Choose a reason for hiding this comment

emkll commented Oct 27, 2020 • edited Loading

conorsch commented Oct 27, 2020

conorsch commented Oct 27, 2020

emkll left a comment

Choose a reason for hiding this comment

emkll commented Oct 27, 2020

emkll left a comment

Choose a reason for hiding this comment

conorsch commented Oct 8, 2020 •

edited

Loading

emkll left a comment •

edited

Loading

emkll commented Oct 27, 2020 •

edited

Loading