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

Renames sd-journalist to sd-proxy #206

Merged
merged 3 commits into from
Nov 12, 2018
Merged

Renames sd-journalist to sd-proxy #206

merged 3 commits into from
Nov 12, 2018

Conversation

emkll
Copy link
Contributor

@emkll emkll commented Nov 11, 2018 •
edited by conorsch
Loading

Closes #138
🚨 This requires changes in securedrop-client which are not live, merging this branch must be coordinated with the release of securedrop-client as to not break master. This is because securedrop-sdk hardcodes the proxyvm name, and it must be changed from sd-journalist to sd-proxy (see freedomofpress/securedrop-sdk#43)

Test plan:

  1. Do not use this branch; check out master and make clone
  2. Make clean (to remove old VMs such as sd-journalist)
  3. Check out this branch and make clone
  4. Make all
  5. Make test
  6. Sideload securedrop-client 0.0.5
  • Make test does not return any errors
  • There are no sd-journalist or sd-journalist-template VMs present
  • Documents download and decrypt in sd-client

Merge this at the same time as SecureDrop-client 0.0.5 is deployed to apt-test-qubes

@emkll emkll requested a review from conorsch November 11, 2018 19:37
@emkll emkll force-pushed the rename-sd-journalist branch from c195eb9 to a5c95df Compare November 11, 2018 21:01
@conorsch conorsch changed the title Rename sd journalist Renames sd-journalist to sd-proxy Nov 11, 2018
@emkll emkll force-pushed the rename-sd-journalist branch from 4787028 to 5c06a9b Compare November 11, 2018 22:46
conorsch
conorsch reviewed Nov 11, 2018
View reviewed changes
dom0/sd-proxy.sls Outdated
- label: blue
- prefs:
- netvm: sd-whonix
- kernelopts: "nopat apparmor=1 security=apparmor"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this kernelopts line is a holdover from when the VM was based on Whonix (WS), and isn't necessary anymore. As long as it doesn't break, OK to merge, we can circle back to clean up these configs, addressing the unwanted kernelopts line then.

@emkll
Copy link
Contributor Author

emkll commented Nov 11, 2018

Thanks for the changes, @conorsch . Rebased on latest master, and did a clean build, all tests pass and I can decrypt/view videos, photos and messgaes in dispvms. 👍

@conorsch
Copy link
Contributor

  • Make test does not return any errors
  • There are no sd-journalist or sd-journalist-template VMs present
  • Documents download and decrypt in sd-client

Works for me, with the manually sideloaded deb for securedrop-client v0.0.5. Let's coordinate on version bumping to keep things in sync, @emkll.

conorsch
conorsch approved these changes Nov 12, 2018
View reviewed changes
Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

Approved, no further changes requested; we'll want to coordinate merge with package updates in the repo, to keep things humming along smoothly across dev workstations.

@conorsch
Copy link
Contributor

no further changes requested

I lied; there's one more change. Summarizing in-person discussion with @emkll, @joshuathayer, and @redshiftzero , we suspect that the change from Whonix WS to the SDW Debian-9-based template for sd-proxy-template is increasing timeouts for the proxy, effectively blocking the client from connecting to the server after ~5m of idle time. The plan of record is to update the client code to refresh more frequently, but in the meantime, we'll stick with the whonix-ws-14 base for sd-proxy-template, since that's a more well known quantity.

Removes GUI feedback tooling from sd-proxy
dbfdd5c 
This logic was a holdover from the sd-journalist days, in which we had
custom notification for GUI feedback in dom0. No need to continue to
configure the dependencies for those scripts, since they're not used
post conversion of the sd-proxy.

Also consolidated all sd-proxy files into the sd-proxy-template, mainly
by moving the mimetypes into a system path, as has already been done
with other VMs.
@conorsch conorsch force-pushed the rename-sd-journalist branch from 5c06a9b to dbfdd5c Compare November 12, 2018 00:29
@conorsch
Copy link
Contributor

Done. Backed up the old branch in rename-sd-journalist-backup, for reference.

@conorsch
Copy link
Contributor

Merging; @emkll is standing by to release new packages, and we'll run a make clean && make all to validate the new logic.

@conorsch conorsch merged commit 95dffef into master Nov 12, 2018
@emkll emkll deleted the rename-sd-journalist branch November 13, 2018 22:18
@conorsch conorsch mentioned this pull request Mar 5, 2020
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@conorsch conorsch conorsch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emkll @conorsch