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

Client-side support for VeraCrypt; use QWizard for export #1779

Merged
merged 24 commits into from
Feb 6, 2024

Conversation

rocodes
Copy link
Contributor

@rocodes rocodes commented Jan 30, 2024

Description

  • gui support for export to already-unlocked encrypted drives (luks, veracrypt)
  • Refactor: remove long-running singleton service Export Service thread
  • Refactor: combine Device and Export into single class
  • Refactor: consolidate all exports to one method signature. This allows for a single widget to handle (usb) exports, and consolidated logic in export.py
  • Refactor: use QProcess instead of subprocess + thread for managing qrexec commands
  • Use QWizard instead of modal dialog for export flow
  • Support more fine-grained status types to report to the user
  • Support Veracrypt
  • Tests WIP

Fixes #988
Fixes #1733
Fixes #1734
Towards #1729 (would be an easy addition with QWizard registeredField if desired)
Towards #1723 (on the way)
Fixes #1605

Currently there are still some styling issues that need to be resolved (button styling, animation):

export_wizard_1

Test Plan (when PR leaves draft mode)

  • Visual review makes sense
  • CI passing
  • Manual testing as below

Manual testing test plan

Setup

  • Prereq: Qubes SDW machine
  • Provision three USBs: a VeraCrypt-encrypted USB without a hidden volume (instructions), a LUKS-encrypted USB, and an invalid device of your choosing. (Invalid devices have != 1 encrypted partitions at the main device or first partition level).
  • Using build-debs.sh, build securedrop-client packages from the tip of this branch. You will need to check out this branch of the builder repo to pull in the pexpect wheel if it hasn't been merged yet.) Install the securedrop-export deb in sd-large-bullseye-template, or clone sd-large-bullseye-template and set the cloned template as template for sd-devices-dvm. if you don't want to mess with your sdw install. You will have to adjust rpc policies to copy the .deb into the template; prepend changes to /etc/qubes/policy.d/60-securedrop-workstation.policy. Shut down the template and sd-devices{-dvm).
  • Either install the securedrop-client deb in sd-app, and run LOGLEVEL=DEBUG securedrop-client, or give sd-app a netvm, install git, clone the client repo into sd-app, check out this branch, activate the venv, and then start the client with LOGLEVEL=DEBUG ./run.sh (observe log in ~/.securedrop_client/logs/client.log).

The business

USB detection
  • Clicking export with no USB inserted: preflight window shown, click "next", prompted to insert a usb.
  • >1 usbs attached to sd-devices yields "too many usbs" error
  • Invalid usb attached to sd-devices yields "invalid device" error
  • Can't proceed past "Insert a USB" page until exactly 1 valid device detected
  • Specific hint appears to differentiate error states
LUKS USB unlocking
  • 1 locked LUKS drive attached to sd-devices before export is initiated leads from preflight to passphrase prompt screen
  • 1 locked LUKS drive attached to sd-devices during export wizard ("please insert a usb drive") leads to passphrase prompt screen
  • Wrong unlock password does not allow you to pass passphrase screen, and an error message ("that passphrase didn't work") is shown
Successful export
  • Unlocked (or unlocked and mounted) Veracrypt drive yields successful export, "Export successful" page is shown ((When testing VeraCrypt devices, unlock them, eg using nautilus in sd-devices or using udisksctl unlock -b /path/to/device at commandline in sd-devices. Locked VC drives are not currently supported)) ( [securedrop-export] [spike] Support Veracrypt #1730)
  • Locked, unlocked, or unlocked and mounted LUKS drive yields successful export, as above
  • Drives that are already unlocked before starting the export go from the Preflight message to the Export Successful screen, skipping the intermediate prompts ( [securedrop-export] if device already unlocked, don't ask for passphrase #1734)
  • On successful exports, both luks and veracrypt drives are locked and unmounted at the end of export
Error behaviour
When good people do bad things
  • After initiating export and reaching passphrase prompt screen, removing USB drive from sd-devices, then clicking "next" returns you to "insert a device" screen
  • After initiating export and reaching the passphrase prompt screen, inserting another USB drive then clicking "next" returns you to "insert a device" screen with hint about too many USBs
  • Error "hints" are not shown after resolving the issue, pressing Next, then pressing the back button

Checklist

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

  • I have tested these changes in the appropriate Qubes environment
  • I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
  • These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

  • I have updated the AppArmor profile
  • No update to the AppArmor profile is required for these changes
  • I don't know and would appreciate guidance

If these changes modify the database schema, you should include a database migration. Please check as applicable:

  • I have written a migration and upgraded a test database based on main and confirmed that the migration is self-contained and applies cleanly
  • I have written a migration but have not upgraded a test database based on main and would like the reviewer to do so
  • I need help writing a database migration
  • No database schema changes are needed

@rocodes rocodes force-pushed the export-with-json-gui branch from 055e9a7 to 5e2c8c1 Compare January 31, 2024 02:01
@rocodes rocodes force-pushed the export-with-json-gui branch 2 times, most recently from 275b38d to f56b49e Compare February 3, 2024 17:43
@rocodes rocodes force-pushed the export-with-json-gui branch from f56b49e to 27525f5 Compare February 5, 2024 16:23
@rocodes
Copy link
Contributor Author

rocodes commented Feb 6, 2024

I'm planning to merge this into #1777 today for ease of complete review since the underlying export stuff has had a provisional review. I'll copy the test plan over as well.

@rocodes rocodes marked this pull request as ready for review February 6, 2024 17:22
@rocodes rocodes requested a review from a team as a code owner February 6, 2024 17:22
@rocodes rocodes merged commit 97ce094 into export-with-json Feb 6, 2024
17 of 47 checks passed
@rocodes rocodes deleted the export-with-json-gui branch February 6, 2024 17:23
@rocodes rocodes removed the request for review from a team February 6, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant