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

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

Merged
merged 7 commits into from
May 5, 2020
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
956cc10
Permit whitelisting VMs for copy/paste & copying logs via tags
eloquence Apr 10, 2020
846105e
Add @default rule so qvm-copy works
eloquence Apr 10, 2020
38dba62
Prefix all tags with sd-
eloquence Apr 14, 2020
3951038
Add script for removing SD tags from VMs
eloquence Apr 17, 2020
d23dd8c
Rescope clipboard permission to sd-app
eloquence Apr 21, 2020
49916a2
Run tag cleanup via Salt to avoid duplication
eloquence Apr 22, 2020
a04589a
Add require logic to clean-all state
eloquence Apr 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

The RPM spec already picks up the script by wildcard.

Copy link
Member Author

Choose a reason for hiding this comment

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

What I would propose is this:

  1. Move remove-tags from scripts to dom0
  2. Add it to RPM (I think that's what you meant, Conor?)
  3. Call it from sd-clean-all.sls as we do for update-xfce-settings resets.
  4. Remove it from securedrop-admin and Makefile.

Does that make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

@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)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 49916a2, not tested yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

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'.
----------

sudo dnf -y -q remove securedrop-workstation-dom0-config 2>/dev/null || true
$(MAKE) clean-salt

Expand Down
4 changes: 4 additions & 0 deletions dom0/sd-dom0-qvm-rpc.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ dom0-rpc-qubes.ClipboardPaste:
- marker_start: "### BEGIN securedrop-workstation ###"
- marker_end: "### END securedrop-workstation ###"
- content: |
@tag:sd-send-app-clipboard sd-app ask
sd-app @tag:sd-receive-app-clipboard ask
@anyvm @tag:sd-workstation deny
@tag:sd-workstation @anyvm deny
dom0-rpc-qubes.FeaturesRequest:
Expand All @@ -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:sd-receive-logs ask
sd-proxy @tag:sd-client allow
@anyvm @tag:sd-workstation deny
@tag:sd-workstation @anyvm deny
Expand Down
32 changes: 32 additions & 0 deletions scripts/remove-tags
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/usr/bin/env python3
"""
Removes tags used for exempting VMs from default SecureDrop Workstation
RPC policies from all VMs (including non-SecureDrop ones).
"""
import qubesadmin

q = qubesadmin.Qubes()

TAGS_TO_REMOVE = ["sd-send-app-clipboard", "sd-receive-app-clipboard", "sd-receive-logs"]


def main():
tags_removed = False
for vm in q.domains:
for tag in TAGS_TO_REMOVE:
if tag in q.domains[vm].tags:
print("Removing tag '{}' from VM '{}'.".format(tag, vm))
try:
q.domains[vm].tags.remove(tag)
except Exception as error:
print("Error removing tag: '{}'".format(error))
print("Aborting.")
exit(1)
tags_removed = True

if tags_removed is False:
print("Tags {} not set on any VMs, nothing removed.".format(TAGS_TO_REMOVE))


if __name__ == "__main__":
main()
5 changes: 4 additions & 1 deletion scripts/securedrop-admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ def perform_uninstall():
subprocess.check_call(
["sudo", "dnf", "-y", "-q", "remove", "qubes-template-securedrop-workstation-buster"]
)
print("Removing SecureDrop tags from remaining VMs")
subprocess.check_call([os.path.join(SCRIPTS_PATH, "scripts/remove-tags")])
print("Uninstalling dom0 config package")
subprocess.check_call(
["sudo", "dnf", "-y", "-q", "remove", "securedrop-workstation-dom0-config"]
Expand All @@ -132,7 +134,8 @@ def main():
elif args.uninstall:
print(
"Uninstalling will remove all packages and destroy all VMs associated\n"
"with SecureDrop Workstation."
"with SecureDrop Workstation. It will also remove all SecureDrop tags\n"
"from other VMs on the system."
)
response = input("Are you sure you want to uninstall (y/N)? ")
if response.lower() != 'y':
Expand Down
4 changes: 4 additions & 0 deletions tests/vars/qubes-rpc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
- policy: qubes.ClipboardPaste
starts_with: |-
### BEGIN securedrop-workstation ###
@tag:sd-send-app-clipboard sd-app ask
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

Choose a reason for hiding this comment

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

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

sd-app @tag:sd-receive-app-clipboard ask
@anyvm @tag:sd-workstation deny
@tag:sd-workstation @anyvm deny
### END securedrop-workstation ###
Expand All @@ -15,6 +17,8 @@
- policy: qubes.Filecopy
starts_with: |-
### BEGIN securedrop-workstation ###
sd-log @default ask
sd-log @tag:sd-receive-logs ask
sd-proxy @tag:sd-client allow
@anyvm @tag:sd-workstation deny
@tag:sd-workstation @anyvm deny
Expand Down