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.

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

Supports opening submissions in DispVMs from Qubes dev env #490

Merged
merged 7 commits into from Jul 30, 2019

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Jul 24, 2019

Description

Adds support for running the client code (via ./run.sh) within a Qubes AppVM, and opening submissions in DispVMs from the client interface. Related issues:

Test Plan

Throughout these instructions, sd-dev is assumed to be the name of the developer's AppVM where code is written. If you use a different VM name, substitute that below.

  • Clone/checkout Uses tags for RPC grants denoting Client privileges securedrop-workstation#299 and run make all
  • In dom0, run qvm-tags sd-dev add sd-client
  • In sd-dev, clone/checkout the "securedrop" repository and run make dev to start the containers
  • In sd-dev, clone/checkout this PR branch, and run ./run.sh to start the client interface
  • Submit a PDF to the Source Interface, confirm visible in the Client
  • Open the PDF submission in the Client, confirm it opens in a DispVM
  • Send a reply to a source, confirm no errors

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs, network (via the RPC service) traffic, or fine tuning of the graphical user interface, Qubes testing is required. Please check as applicable:

  • I have tested these changes in Qubes
  • I do not have a Qubes OS workstation (the reviewer will need to test these changes in Qubes)

Conor Schaefer added 5 commits July 24, 2019 09:39
Includes required files for the securedrop-client to run inside Qubes,
and references those files in the package MANIFEST, so they are included
the tarball emitted by the sdist prep process. These files were
previously put in place by Salt, but here we fold them into the
packaging logic.

In addition to achieving slightly cleaner packaging workflows, having
the mime handler files in this repo exposes them to the run.sh script
for spinning up a local dev environment within a Qubes AppVM.
Similar to the --no-proxy flag, here we add a --no-qubes flag, off by
default, to denote that the Client is running on a non-Qubes OS
(typically macOS, used by developers).
Infers whether the developer's host OS is Qubes, based on the presence
of env vars matching pattern `^QUBES_`. If Qubes, we can permit the use
of DispVMs for opening submissions.

The logic assumes that the Qubes AppVM running the client (e.g.
`sd-dev`) has the tag `sd-client` applied. We cannot inspect this from
within the AppVM, so we'll lean on documentation to instruct developers
about this.
The `wait` call must ensure that the dev env setup steps complete
*prior* to executing the local app code. We need not background running
the app code, since that's the final action in the run.sh script.
The recent changes provide improved Qubes dev env support in the
`run.sh` script. Modified the possible scenarios to explain the
different environments now possible. These changes make the dev env docs
rather verbose, so some consolidation may be warranted.
Copy link
Contributor

@sssoleileraaa sssoleileraaa left a comment

Choose a reason for hiding this comment

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

This works for me. I had to do a little post docker install setup to make it so I could run docker as a non-root user, but otherwise, everything worked as advertised.

I like how this change makes it so you can run all the client developer workflows on one computer. I think this is ready to merge (after ci jobs show all green) whether or not we recommend it, but let's wait until freedomofpress/securedrop-workstation#301 is finished with documentation to make sure everyone is on the same page.

@sssoleileraaa
Copy link
Contributor

Clone/checkout freedomofpress/securedrop-workstation#299 and run make all

freedomofpress/securedrop-workstation#299 was merged into master branch so this step one could clone master and run make all (in case there are others testing this)

Ensure the `qubes` variable is false when checking the function call.
@eloquence eloquence added this to Ready for review in SecureDrop Team Board Jul 30, 2019
@eloquence eloquence moved this from Ready for review to In Development in SecureDrop Team Board Jul 30, 2019
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.

Thanks @conorsch it seems to be working as expected in my local Qubes environment. Thanks for the great test plan:

  • Clone/checkout Uses tags for RPC grants denoting Client privileges securedrop-workstation#299 and run make all (ran on latest on master since that PR was merged already_
  • In dom0, run qvm-tags sd-dev add sd-client
  • In sd-dev, clone/checkout the "securedrop" repository and run make dev to start the containers
  • In sd-dev, clone/checkout this PR branch, and run ./run.sh to start the client interface
  • Submit a PDF to the Source Interface, confirm visible in the Client
  • Open the PDF submission in the Client, confirm it opens in a DispVM
  • Send a reply to a source, confirm no errors
  • I have tested these changes in Qubes

Pushed 00532a2 to fix a failing test, otherwise LGTM.

dev-dvm

Intentionally not merging this in case @creviera would like to make some changes (docs or otherwise)

@sssoleileraaa sssoleileraaa force-pushed the support-dispvms-in-qubes-dev-env branch from 0e90461 to cae3685 Compare July 30, 2019 19:37
@sssoleileraaa
Copy link
Contributor

@emkll I updated the README to include a link to the securedrop docs, mentioned the requirement to run docker as non-root user in Qubes, and an extra bit about end-to-end client testing. This is ready once again for your 👀

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.

Thanks @creviera for the great docs addition, if you don't mind addressing a couple of typos and good to merge (some of which weren't introduced by your diff)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@sssoleileraaa sssoleileraaa force-pushed the support-dispvms-in-qubes-dev-env branch from 6c64a3e to 63ae2f4 Compare July 30, 2019 20:05
@sssoleileraaa sssoleileraaa merged commit 2499dbc into master Jul 30, 2019
SecureDrop Team Board automation moved this from In Development to Done Jul 30, 2019
@sssoleileraaa sssoleileraaa deleted the support-dispvms-in-qubes-dev-env branch July 30, 2019 20:25
@emkll
Copy link
Contributor

emkll commented Jul 30, 2019

Opened #497 to track the use of split-gpg in dev env as a potential follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

client fails to send replies in qubes when using run.sh Dev script fails on first run in Qubes
3 participants