Skip to content
This repository has been archived by the owner on Jan 5, 2024. It is now read-only.

Moves config to private volume #79

Merged
merged 1 commit into from
Oct 27, 2020
Merged

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Oct 5, 2020

The Qubes RPC file hardcodes the filepath to the YAML config file, which contains site-specific information such as the Onion URL for the Journalist Interface. As part of template consolidation [0], we're moving the config file out of the system/root partition and into the private (i.e. /home/) volume, so that the sd-proxy AppVM has the config information it needs while sharing a TemplateVM with other components.

[0] freedomofpress/securedrop-workstation#471

Closes #147.

Testing

A new package has been built based on this change and uploaded to apt-test via freedomofpress/securedrop-apt-test#65. You can check out the Workstation branch for freedomofpress/securedrop-workstation#619 to evaluate how the new package operates in tandem with the modified salt logic to ensure that the proxy config exists only in the private volume for sd-proxy.

@conorsch conorsch force-pushed the 77-read-config-from-private-volume branch from 438c04a to fdbd2b5 Compare October 5, 2020 23:38
conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 5, 2020
conorsch pushed a commit to freedomofpress/securedrop-builder that referenced this pull request Oct 6, 2020
Updates the RPC file in the code repo, then rebuilds based on new
version. See related PR in:

freedomofpress/securedrop-proxy#79
conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 6, 2020
@emkll emkll force-pushed the 77-read-config-from-private-volume branch from fdbd2b5 to 6ed71bc Compare October 9, 2020 13:25
@@ -1 +1 @@
/usr/bin/sd-proxy /etc/sd-proxy.yaml
/usr/bin/sd-proxy /home/user/.securedrop_proxy/sd-proxy.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, this path will only exist after running the template consolidation logic [1]. if we do not configure both the existing path and the new path here, the proxy will only work in either the non-consolidated or the consolidated template approach. In other words, this proxy change will break existing installs, until the dom0 logic is run again.

https://github.com/freedomofpress/securedrop-workstation/pull/619/files#diff-6665f59c22d94742e30d7bbf295194ceR7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In other words, this proxy change will break existing installs, until the dom0 logic is run again.

A critical assumption of the template consolidation plan in freedomofpress/securedrop-workstation#619 is that the SecureDrop Workstation components are updated only via the SDW GUI updater. As long as we accept that limitation, then the new updater logic will ensure that the config file is handled appropriately.

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

Rebased on latest main to get CI passing.

@conorsch
Copy link
Contributor Author

This PR is now "ready for review". I'm still leaving it in draft state, however, in order to block merge. As @emkll pointed out above, merging by itself without also merging the other template-consolidation PRs would cause the nightly builds to ship breaking changes.

conorsch pushed a commit to freedomofpress/securedrop-builder that referenced this pull request Oct 16, 2020
Updates the RPC file in the code repo, then rebuilds based on new
version. See related PR in:

freedomofpress/securedrop-proxy#79
emkll
emkll previously approved these changes Oct 19, 2020
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

Changes here look good to me.

After building a local package on this branch and comparing to the one on apt-test, observed some differences since this PR was rebased on top of #76 and at the time the package was merged to the apt-test server in https://github.com/freedomofpress/securedrop-dev-packages-lfs/pull/65/files .

We will need to either:

  1. Update the package on apt-test with these dependencies updated, in order to test pip / requests changes in dev/staging environments
  2. Build a new package for prod due to the updates to pip and requests, without testing these changes

Option 1 seems best to me, happy to build/upload the package if you agree @conorsch

@conorsch
Copy link
Contributor Author

Option 1 seems best to me

Good call! I'll rebuild and re-upload for your review. I won't change versions, since the "template-consolidation" channel already uses a bumped version for priority.

conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 19, 2020
Incorporated newer build dependencies, see discussion in
freedomofpress/securedrop-proxy#79 (review)
@conorsch
Copy link
Contributor Author

Rebuilt and submitted new package: freedomofpress/securedrop-apt-test#68

The Qubes RPC file hardcodes the filepath to the YAML config file,
which contains site-specific information such as the Onion URL for the
Journalist Interface. As part of template consolidation [0], we're
moving the config file out of the system/root partition and into the
private (i.e. /home/) volume, so that the `sd-proxy` AppVM has the
config information it needs while sharing a TemplateVM with other
components.

[0] https://github.com/freedomofpress/securedrop-workstation#471
@conorsch
Copy link
Contributor Author

Removed the version changes, since we'll follow up with changelog additions prior to release. Diff was

diff of removed files
diff --git a/changelog.md b/changelog.md
index bfded30..a4447bf 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.3.1
+
+  * Moves config file into private volume (#77).
+
 ## 0.3.0
 
   * Use incoming timeout value from JSON (#69).
diff --git a/securedrop_proxy/VERSION b/securedrop_proxy/VERSION
index 0d91a54..9e11b32 100644
--- a/securedrop_proxy/VERSION
+++ b/securedrop_proxy/VERSION
@@ -1 +1 @@
-0.3.0
+0.3.1

Ready for final review.

@conorsch conorsch force-pushed the 77-read-config-from-private-volume branch from 6ed71bc to 85e3241 Compare October 27, 2020 20:02
@conorsch conorsch marked this pull request as ready for review October 27, 2020 20:02
conorsch pushed a commit to freedomofpress/securedrop-builder that referenced this pull request Oct 27, 2020
Updates the RPC file in the code repo, then rebuilds based on new
version. See related PR in:

freedomofpress/securedrop-proxy#79
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

Changes look good to me here, and ci is passing. We will have to independently open a PR to bump versions and changelog in preparation for release, however nighlies should cover the dev/staging envs in the interim

@conorsch conorsch merged commit 155b977 into main Oct 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[securedrop-proxy] Move sd-proxy.yaml into private volume
2 participants