Permit whitelisting VMs for copy/paste & copying logs via tags #533

eloquence · 2020-04-10T21:39:34Z

Adds RPC policies that let tagged VMs send clipboard pastes to sd-app, receive clipboard pastes from sd-app, and receive files from sd-log.

Status

Ready for review; docs in progress at freedomofpress/securedrop-workstation-docs#35 and will need to be kept in sync with changes here.

Adds RPC policies for the following tags:

sd-receive-app-clipboard -- VMs with this tag will be able to receive clipboard contents from sd-app
sd-send-app-clipboard -- VMs with this tag will be able to send clipboard contents to sd-app
sd-receive-logs -- VMs with this tags will be able to receive any file sent via qvm-copy from sd-log

Adds a script in scripts/remove-tags that removes all tags, and runs it for make clean / securedrop-admin --uninstall operations.

Description of Changes

Fixes #526
Towards #529

Testing

Scenario: Basic use

Prerequisites

Check out the version of sd-dom0-qvm-rpc.sls from this branch and apply it to your /srv/salt directory in an up-to-date production or staging install, or provision a dev environment using make dev from this branch.
For production or staging environments, run securedrop-admin --apply to apply the change.

Test

Add the ability to receive clipboard contents from sd-app to a target VM, e.g. qvm-tags work add sd-receive-app-clipboard (qvm-tags <target VM name> add sd-receive-app-clipboard)
Add the ability to send clipboard contents from a single source VM to sd-app, e.g. qvm-tags vault add sd-send-app-clipboard (qvm-tags <source VM name> add sd-send-app-clipboard).
Add the ability to receive logs to a target VM, e.g., qvm-tags work add sd-receive-logs
Launch the client
- Observe that you are able to copy clipboard contents from the SecureDrop Client reply box to the target VM specified in step 1
- Observe that you are able to copy clipboard contents from the source VM specified in step 2 to an input field in the SecureDrop Client
- Observe that you are able to copy files from sd-log to the target VM specified in step 3 using qvm-copy-to-vm and qvm-copy.
- Observe that you are not able to perform copy/paste from any SecureDrop VM to any other VM, except from sd-app to target VM you configured in step 1
- Observe that you are not able to perform copy/paste to any SecureDrop VM from any VM except from the source VM you configured in step 4 to sd-app.
- Observe that you are not able to perform qvm-copy to any SecureDrop VMs except from sd-proxy to sd-app.
- Observe that you are not able to perform qvm-copy from any SecureDrop VMs except for sd-log as permitted by the sd-log rules you configured.
Remove the tags by running scripts/remove-tags from this branch.
- Observe via qvm-tags <name of VM> ls that tags were correctly removed.
- Observe that the copy & paste rules you configured have been disabled.
- Observe that the sd-log rules you configured have been disabled.
- Spot check that the rules otherwise still behave as before.

Scenario: Uninstall (prod)

Prerequisites

You must have a working staging or prod install you're willing to lose 😢
Re-tag select VMs as per the test plan above.
Make sure that the changes in this PR are applied to the /bin version of securedrop-admin, and to the files in /srv/salt touched by this PR (create remove-tags, update sd-clean-all.sls)

Test

Uninstall via securedrop-admin --uninstall.
- Observe that the tags you added are correctly removed as part of the uninstall.

Scenario: Uninstall (dev)

Prerequisites

You must have a working dev install you are willing to lose 😢
Re-tag select VMs as per the test plan above.
Ensure that your securedrop-workstation checkout directory in dom0 is using this branch.

Test

Uninstall via make clean
- Observe that the tags are correctly removed as part of the uninstall.

Checklist

RPM updated to include new files (release not bumped yet)
make test runs fine in dom0 (not tested yet; test for RPC policies has been updated, however).

Adds RPC policies that let tagged VMs send clipboard pastes, receive clipboard pastes, and receive files from sd-log.

eloquence · 2020-04-10T21:42:58Z

The use of qvm-copy-to-vm as indicated in the test plan works for me, but is deprecated; I was not able to use qvm-copy to copy files from sd-log, even after setting the policy to "allow" instead of "ask" -- I always get "Request refused". Not sure if the RPC policies require a refresh or if there's another reason those requests are still blocked.

eloquence · 2020-04-10T22:08:55Z

I was not able to use qvm-copy to copy files from sd-log

This appears to be due to the somewhat confusing behavior with empty selection prompts described in QubesOS/qubes-issues#4403 ; the @default rule in 846105e should resolve (and does not expand the scope of permissions).

conorsch

A few suggestions for maintenance over time, but largely on board with the proposals here. Let's flesh out that test plan as much as possible, in particular the "Observe that you are not able to perform copy/paste or copy files from SDW VMs to other VMs, or from other VMs to SDW VMs" step.

conorsch · 2020-04-10T22:30:19Z

dom0/sd-dom0-qvm-rpc.sls

@@ -17,6 +17,8 @@ dom0-rpc-qubes.ClipboardPaste:
    - marker_start: "### BEGIN securedrop-workstation ###"
    - marker_end: "### END securedrop-workstation ###"
    - content: |
+        @tag:send-clipboard-to-sd @tag:sd-workstation ask
+        @tag:sd-workstation @tag:receive-clipboard-from-sd ask


Pleased to report that the directionality appears to work as intended: it's possible to allow clipboard pastes into e.g. sd-app, while still forbidding pastes out. Testing is critical here, will likely take the bulk of review time. Fleshing out the test plan with specific targets, even more verbose than already written, is likely to help coordinate testing.

Test plan is plenty verbose now :)

conorsch · 2020-04-10T22:31:25Z

dom0/sd-dom0-qvm-rpc.sls

@@ -35,6 +37,8 @@ dom0-rpc-qubes.Filecopy:
    - marker_start: "### BEGIN securedrop-workstation ###"
    - marker_end: "### END securedrop-workstation ###"
    - content: |
+        sd-log @default ask
+        sd-log @tag:receive-sd-logs ask


Let's make sure all the tags added by the SDW config begin with sd-, for consistency. So: receive-sd-logs -> sd-receive-logs, etc. Will make it easier over time to reason about collections and interactions between RPC policies.

The requirement to include @default ask here is far from intuitive, but the citation you provided was informative, and functional testing locally does indeed line up with what's intended.

(This is done.)

conorsch · 2020-04-10T22:39:09Z

tests/vars/qubes-rpc.yml

@@ -1,6 +1,8 @@
 - policy: qubes.ClipboardPaste
  starts_with: |-
    ### BEGIN securedrop-workstation ###
+    @tag:send-clipboard-to-sd @tag:sd-workstation ask
+    @tag:sd-workstation @tag:receive-clipboard-from-sd ask


Good to have the tests updated. We also need a clean action, though: uninstalling should handle removal of these custom SDW-related tags. For example:

qvm-ls --tags sd-receive-logs --raw-list | xargs --no-run-if-empty -I {} qvm-tags {} del sd-receive-logs

for each custom tag.

Ended up adding a Python script leveraging qubesadmin for this purpose.

eloquence · 2020-04-11T00:08:31Z

Thanks @conorsch! Let's discuss as a team if/how we would ship those policies, which I think will influence both the naming & uninstall policy:

Do we want the user to use existing VMs like vault?
Do we want to provide instructions (or a dom0 utility) for setting up new VMs like sd-signal or sd-vault using our hardened base template?
Do we want to wait with making RPC changes until we've agreed what additional VMs may be needed?

I think it's worth considering putting together a small helper script for managing the creation of custom VMs, e.g.:

$ sd-vm create sd-signal --enable=receive-logs,custom-software,network
Created TemplateVM "sd-signal-buster-template". Install custom software here.
Created AppVM "sd-signal" with network access using "sys-firewall".
Permission "receive logs" has been granted for VM "sd-signal".

$ sd-vm update sd-signal --disable=receive-clipboard
Permission "receive clipboard" has been revoked for VM "sd-signal".

$ sd-vm create shiny-passwords 
Error: Cannot create VM "shiny-passwords".
VMs added to SecureDrop Workstation must begin with "sd-" prefix.

(The goal would be to keep this tool as thin of an abstraction as possible, of course.)

This would give us a supported customization path with some guardrails, abstracting tags and templates away from the user, and allowing us to add business logic as needed. In this model, I agree that the uninstall scenario should also remove the tags (or even the VMs!), as part of the managed configuration of the system.

On the other hand, if we ask users to create/tag VMs using standard Qubes tooling, I'm not sure our uninstall script should remove those tags, at least not without some explicit --purge type option.

Either way, I think some customization path is necessary so that we can use the pilot to determine which additional VMs we may want to ship by default.

eloquence · 2020-04-14T18:52:34Z

Talked to @emkll about this today. He's generally in favor of this approach during the pilot, until we have more data about what default VMs (and potential default policies for vault or others) we may want to add.

Main changes required:

Thorough documentation
sd- prefix for all tags
Cleanup logic

Once all that is done, I'll promote this PR from draft, provided we all agree this is the way to go.

For the cleanup logic, I'll also add a note to the docs and maybe the --uninstall command that custom tags will be removed.

Regarding the idea of a script for creating custom VMs with our base template, let's take that off the table -- getting grsec and our other security customizations to work right with custom apps is finicky, so we probably want to do this on behalf of our users as much as possible.

eloquence · 2020-04-17T18:01:44Z

I've tested the new tag removal logic via make clean and securedrop-admin --uninstall, and it seems to work as intended; quick visual feedback on the remove-tags script is appreciated.

I'll now add a parallel docs PR, flesh out the test plan, and then mark it as formally ready for review.

eloquence · 2020-04-18T01:01:01Z

I've significantly updated the test plan. I still want to add a section to the docs for managing the clipboard policies, but this should IMO be done in parallel with a first round review, just in case we make significant changes to this approach. The initial docs here should suffice as a starting point during review.

emkll

Thanks @eloquence . I have tested these changes in both dev and prod environments, and went through functional testing based on your excellent test plan. Please also see inline comments, for discussion.

Functional testing

Scenario: Uninstall (prod)

Installed via rpm moved to dom0

Observe that the tags you added are correctly removed as part of the uninstall.

Scenario: Uninstall (dev)

Observe that the tags are correctly removed as part of the uninstall.

emkll · 2020-04-20T14:52:20Z

dom0/sd-dom0-qvm-rpc.sls

@@ -17,6 +17,8 @@ dom0-rpc-qubes.ClipboardPaste:
    - marker_start: "### BEGIN securedrop-workstation ###"
    - marker_end: "### END securedrop-workstation ###"
    - content: |
+        @tag:sd-send-clipboard @tag:sd-workstation ask


There's an sd-client tag that we assign to sd-app, to allow gpg operations. Perhaps we can use that one here instead?

#526 states the three use cases we are looking to enable are as follows:

* pasting passphrases into the login window from a vault VM * copying messages from and to the conversation view (e.g., to another secure messaging app) * copying logs and error messages

With those requirements in mind, a policy scoped to @tag:sd-client and the ability to copy logs via qvm-copy and centralized logging, what use case have we documented that require enabling copy paste to all workstation VMs?

I was thinking about this as well a bit over the weekend. One use case is copying text out of disposable VMs used for viewing submissions. Right now this just works (with these changes) because the dispVMs have the sd-workstation tag. If we want to narrow the policies and still support that use case, we could perhaps use sd-client (two-directional copy/paste in and out) and sd-viewer-vm (one-directional copy/paste out).

Echoing @emkll, I recommend we scope to @tag:sd-client (i.e. exclude disposable VMs), see this note in the qubes documentation discouraging copy/paste from less trusted to more trusted domains:

one should keep in mind that performing a copy and paste operation from less trusted to more trusted domain can always be potentially insecure, because the data that we insert might potentially try to exploit some hypothetical bug in the destination VM (e.g. the seemingly innocent link that we copy from untrusted domain, might turn out to be, in fact, a large buffer of junk that, when pasted into the destination VM’s word processor could exploit a hypothetical bug in the undo buffer). This is a general problem and applies to any data transfer between less trusted to more trusted domains.... So, you should always copy clipboard and data only from more trusted to less trusted domains.

However, if we are finding (or otherwise think that) data transfer from disposable VMs into trusted VMs is an important workflow to support, we can file a followup issue for further investigation to either determine how to decrease risk when copy/pasting data from sd-viewer to sd-client or to provide an alternate method to handle that use case (some of the workflows we need to support for removing identifying metadata could handle this use case, e.g. adding an OCR step after we convert documents to trusted PDFs).

Sounds good; for now I'll call the permission sd-send-app-clipboard and sd-receive-app-clipboard to reflect its narrow scope.

This is done in d23dd8c (I used sd-app rather than a tag for the SecureDrop VM since that seems the best way to ensure we have at most a many-to-one/one-to-many relationship here). The test plan has been updated; I've not re-tested yet.

emkll · 2020-04-20T15:50:16Z

Makefile

@@ -110,6 +110,7 @@ clean: assert-dom0 prep-salt ## Destroys all SD VMs
 	$(MAKE) destroy-all
 	sudo qubesctl --show-output --skip-dom0 --targets whonix-gw-15 state.sls sd-clean-whonix
 	sudo qubesctl --show-output state.sls sd-clean-all
+	./scripts/remove-tags


do you think it would make sense to invoke this script/cleanup via sd-clean-all.sls? The challenge is that the path in dev environment will be different than the one in staging and production environments

I agree that seems like the better way to do it, will poke.

Definitely fold into clean-salt, as @emkll suggests. If we use a separate script, then the RPM spec will need to be updated, as well. Consider using the less elegant, but more simply xargs-based approach outlined above.

The RPM spec already picks up the script by wildcard.

What I would propose is this:

Move remove-tags from scripts to dom0

Add it to RPM (I think that's what you meant, Conor?)

Call it from sd-clean-all.sls as we do for update-xfce-settings resets.

Remove it from securedrop-admin and Makefile.

Does that make sense?

@eloquence Yes, those steps make sense. Moving the invocation to the salt clean-all state is sound at this point.

Ideally we'd call it from the scripts dir in /usr/share/, but before we can easily do that, we'll need to use the RPM in all environments, including make-dev, as described in #505 (comment)

Done in 49916a2, not tested yet.

Tested now, seems to work as intended (this is via securedrop-admin):

ID: remove-rpc-policy-tags Function: cmd.script Name: salt://remove-tags Result: True Comment: Command 'salt://remove-tags' run Started: 17:49:47.430389 Duration: 267.274 ms Changes: ---------- pid: 29831 retcode: 0 stderr: stdout: Removing tag 'sd-send-app-clipboard' from VM 'work'. Removing tag 'sd-receive-app-clipboard' from VM 'work'. ----------

emkll

Thanks for the changes @eloquence , pleased to report the test plan (including make clean in dev and securedrop-admin --uninstall in prod) work as expected with the recent changes.

As @redshiftzero suggested in #533 (comment) prior to merging this we should update the associated documentation in freedomofpress/securedrop-workstation-docs#35 linking out to the relevant Qubes disclaimers and clearly identifying the risks of using these RPC policies. Since, as of this review, the risks are not clearly presented in the docs, I propose we elaborate on the following three high-level risks:

Altering the RPC policies (adding a tag to a VM) to copy out introduces risks to data confidentiality: there is a risk that sensitive data can be obtained by an unauthorized third party, when copying from a more secure domain (sd-app, which has no network and is automatically managed by workstation code) to a less secure domain for example, a user-managed, networked domain), when the latter may contain vulnerabilities or be compromised. In this case, once the information leaves the sd-app VM, we have no controls in place to mitigate further risks.
Further risks to confidentiality through human error: when copying to or within sd-app, there is a risk end-users can accidentally paste sensitive information and send them to an anonymous source.
Altering the RPC policies to copy into sd-app introduces risks to VM integrity: there is a risk that data pasted in compromises or otherwise corrupts the domain, when copying from a less secure domain (for example, a user-managed, network domain) to a more secure domain (sd-app, which has no network and is automatically managed by workstation code). In this case, we do have some mitigations in place that may reduce this risk (AppArmor non-networked VM)

emkll · 2020-04-22T14:43:17Z

tests/vars/qubes-rpc.yml

@@ -1,6 +1,8 @@
 - policy: qubes.ClipboardPaste
  starts_with: |-
    ### BEGIN securedrop-workstation ###
+    @tag:sd-send-app-clipboard sd-app ask


I also think using sd-app here makes sense: the intent of the sd-client tag is to support dev environment for opening in dispVM and using split GPG (see 784076d)

We can revisit this later, should this be an issue in developer's environment. It should be a small diff that is easily applied on new and existing workstations

emkll

Thanks @eloquence the changes look good to me, one comment inline

I've rebased this PR locally on latest master, built the resulting RPM and tested in a production install:

emkll · 2020-04-28T16:34:02Z

dom0/sd-clean-all.sls

+remove-rpc-policy-tags:
+  cmd.script:
+    - name: salt://remove-tags
+
 # Removes files that are provisioned by the dom0 RPM, only for the development
 # environment, since dnf takes care of those provisioned in the RPM
 {% if d.environment == "dev" %}
 remove-dom0-sdw-config-files-dev:


We may want to require the remove-rpc-policy-tags here to avoid potential race conditions or precedence issues when applying this state on uninstall (though I did not observe any errors in local testing, this might happen if we add tasks in the future)

Good point, done in a04589a (I added the XFCE script because the same logic seems to apply to it); haven't re-tested yet. Can test in dev tomorrow if nobody beats me to it.

Re-tested in dev env w/ require logic, relevant output in https://gist.github.com/eloquence/9f9f8586034ccc4a57fe7bda900108cb, can confirm that files continue to be removed as expected.

emkll

Thanks for your patience on this one, @eloquence . I've reviewed the docs in freedomofpress/securedrop-workstation-docs#35 , and this PR should be merged. Successfully went though your test plan in 2 different scenarios:

In a dev environment to ensure the latest commit did not affect the clean . Ran into issues due to some local modifications, reverting them resolved. Opened make clean and sdw-admin --uninstall halt execution on error #545 to track what might cause configuration drift in the situation the workstation is uninstalled and then re-installed.
In a prod environment, simulated the in-place upgrade: Installed the rpm in dom0, and then ran the GUI updater, the RPC policies were added after the TemplateVM upgrades, during the dom0 state applied at the end of the update sequence.

emkll · 2020-05-05T21:32:39Z

tests/vars/qubes-rpc.yml

@@ -1,6 +1,8 @@
 - policy: qubes.ClipboardPaste
  starts_with: |-
    ### BEGIN securedrop-workstation ###
+    @tag:sd-send-app-clipboard sd-app ask


We can revisit this later, should this be an issue in developer's environment. It should be a small diff that is easily applied on new and existing workstations

Explains in detail how to set up a password manager, and how to configure tags for clipboard usage and logs. Depends on changes introduced in freedomofpress/securedrop-workstation#533

…g-rpc Permit whitelisting VMs for copy/paste & copying logs via tags

Permit whitelisting VMs for copy/paste & copying logs via tags

956cc10

Adds RPC policies that let tagged VMs send clipboard pastes, receive clipboard pastes, and receive files from sd-log.

Add @default rule so qvm-copy works

846105e

conorsch reviewed Apr 10, 2020

View reviewed changes

eloquence added 2 commits April 14, 2020 13:14

Prefix all tags with sd-

38dba62

Add script for removing SD tags from VMs

3951038

This was referenced Apr 17, 2020

Document copy & paste and log export via tagged VMs freedomofpress/securedrop-workstation-docs#33

Open

Add section about post-install config: passwords, clipboard & logs freedomofpress/securedrop-workstation-docs#35

Merged

eloquence marked this pull request as ready for review April 18, 2020 00:59

redshiftzero mentioned this pull request Apr 20, 2020

Provide method for sharing logs #529

Open

emkll reviewed Apr 20, 2020

View reviewed changes

Rescope clipboard permission to sd-app

d23dd8c

eloquence force-pushed the 526-529-copy-paste-and-log-rpc branch from d50d2a5 to d23dd8c Compare April 21, 2020 00:48

Run tag cleanup via Salt to avoid duplication

49916a2

emkll reviewed Apr 22, 2020

View reviewed changes

This was referenced Apr 22, 2020

Create staging release candidate #540

Closed

Clear clipboard after login screen freedomofpress/securedrop-client#1071

Merged

eloquence added this to the 0.3.0 milestone Apr 25, 2020

emkll reviewed Apr 28, 2020

View reviewed changes

Add require logic to clean-all state

a04589a

emkll mentioned this pull request May 5, 2020

make clean and sdw-admin --uninstall halt execution on error #545

Open

emkll approved these changes May 5, 2020

View reviewed changes

emkll merged commit 91c8eb5 into master May 5, 2020

emkll deleted the 526-529-copy-paste-and-log-rpc branch May 5, 2020 21:33

redshiftzero mentioned this pull request May 7, 2020

securedrop-client 0.2.0 freedomofpress/securedrop-client#1083

Merged

29 tasks

emkll mentioned this pull request May 20, 2020

Release securedrop-workstation 0.3.0 #548

Closed

19 tasks

zenmonkeykstop self-assigned this May 29, 2020

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

Merge pull request #533 from freedomofpress/526-529-copy-paste-and-lo…

2ef79f0

…g-rpc Permit whitelisting VMs for copy/paste & copying logs via tags

Permit whitelisting VMs for copy/paste & copying logs via tags #533

Permit whitelisting VMs for copy/paste & copying logs via tags #533

Conversation

eloquence commented Apr 10, 2020 • edited by emkll Loading

Status

Description of Changes

Testing

Scenario: Basic use

Prerequisites

Test

Scenario: Uninstall (prod)

Prerequisites

Test

Scenario: Uninstall (dev)

Prerequisites

Test

Checklist

eloquence commented Apr 10, 2020

eloquence commented Apr 10, 2020 • edited Loading

conorsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eloquence commented Apr 11, 2020 • edited Loading

eloquence commented Apr 14, 2020 • edited Loading

eloquence commented Apr 17, 2020

eloquence commented Apr 18, 2020

emkll left a comment

Choose a reason for hiding this comment

Functional testing

Scenario: Uninstall (prod)

Scenario: Uninstall (dev)

Choose a reason for hiding this comment

eloquence Apr 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eloquence Apr 20, 2020 • edited Loading

Choose a reason for hiding this comment

eloquence Apr 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emkll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emkll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emkll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eloquence commented Apr 10, 2020 •

edited by emkll

Loading

eloquence commented Apr 10, 2020 •

edited

Loading

eloquence commented Apr 11, 2020 •

edited

Loading

eloquence commented Apr 14, 2020 •

edited

Loading

eloquence Apr 20, 2020 •

edited

Loading

eloquence Apr 20, 2020 •

edited

Loading

eloquence Apr 21, 2020 •

edited

Loading