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

[securedrop-export] [spike] Support Veracrypt #1730

Closed
sssoleileraaa opened this issue Aug 25, 2021 · 3 comments · Fixed by #1777
Closed

[securedrop-export] [spike] Support Veracrypt #1730

sssoleileraaa opened this issue Aug 25, 2021 · 3 comments · Fixed by #1777
Assignees

Comments

@sssoleileraaa
Copy link
Contributor

Description

Use the veracrypt cli to implement the export workflow (check encryption, unlock device, dismount, etc), similar to how we do this for a luks-encrypted device using cryptsetup (see https://github.com/freedomofpress/securedrop-export/blob/main/securedrop_export/disk/actions.py for implementation details).

@rocodes rocodes self-assigned this Apr 28, 2022
@rocodes
Copy link
Contributor

rocodes commented May 4, 2022

Some preliminary notes:

  • VeraCrypt cli is one option that we could consider, but hopefully we won't need it: cryptsetup can already open truecrypt or veracrypt drives via the --type tcrypt --veracrypt parameter (e.g. sudo cryptsetup open --type tcrypt --veracrypt /dev/sdX enc_volume (this will prompt you for the passphrase, and if successful, the drive will be mapped to /dev/mapper/enc_volume and the function will return 0).
  • Note that there is a slightly different syntax for TrueCrypt vs VeraCrypt drives (the extra --veracrypt parameter). I don't think we need to support legacy tc drives but I'm just mentioning this for completeness.
  • cryptsetup provides a nice isLuks method, but the same is not true for VeraCrypt volumes. This is because VeraCrypt/tc volumes don't have a cleartext header like LUKS-encrypted volumes do, meaning that our preliminary 'is this a veracrypt drive?' disk-test won't really work. (There are other ways of making an educated guess about whether a volume is a Veracrypt drive--basically, looking at volume size, randomness/entropy and other metadata, but this approach probably is a lot of overhead and not worth it in our case).
  • TailsOS implements a "this might be VeraCrypt" (basically using some of the above heuristics) and contains a custom veracrypt mounter: read more here, code here.

I think that the simplest thing to do will be to modify our DiskTestAction and DiskExportAction. During DiskTest, we currently return success if the user has exactly 1 usb connected, with no more than 1 partition, that is a LUKS-encrypted volume. Then we prompt for the passphrase, then we attempt the export.

What we will do now is simplify disk-test to see if there's exactly 1 connected USB with 1 partition. Then, if so, return a success code, prompt for the passphrase, and in the subsequent step, ascertain whether the disk is encrypted correctly (LUKS, VeraCrypt), or not (USB_ENCRYPTION_NOT_SUPPORTED).

The con of this approach is

  • We won't know if a USB is misconfigured until we've already prompted the user for the passphrase (because we'll first see if it's a LUKS drive with isLuks, but in in order to see if it's a VeraCrypt drive, we'll try mapping it and passing the passphrase and see what happens). This also means that afaict we won't be able to tell the difference between a user typing a bad passphrase on a VeraCrypt drive, and a user using the wrong kind of USB entirely, unless we implement some of the heuristics that Tails uses. (The cryptsetup return code for "entered a password for a non-veracrypt-encrypted drive" and "entered a password for a veracrypt encrypted drive, but wrong password" are both 1).

It may get a little bit more complicated when we add support for already-unlocked drives, but I think we can figure it out and still avoid duplicating any code.

@rocodes
Copy link
Contributor

rocodes commented May 11, 2022

Update on detecting VeraCrypt drives, and overall update:

We can detect already-mapped VeraCrypt drives by looking through the logical volumes in /dev/mapper/ and seeing if any of them are TCRYPT drives whose device ID matches the USB device we detected. (So part of my initial comment of prompting earlier for a passphrase can be amended to "if there's exactly 1 connected USB with 1 partition, AND it's not an already-mapped LUKS or VeraCrypt drive, then prompt for passphrase." This is more like the current implementation, although we do still lose the ability to immediately tell the user "you inserted the wrong USB--this one's not the right format.").

The earlier part of my comment is still true-- after prompting for passphrase, we can then report back to the user one of the following states:

  • luks success!
  • veracrypt success!
  • it's a luks drive with the wrong passphrase entered
  • something else is wrong (wrong veracrypt password OR not a veracrypt drive at all).
    That's as granular as it will get on the veracrypt front, unfortunately.

Now re: other updates, I've been talking to @gonzalo-bulnes about some changes we may want to make to the export code (client and this repo) in order to facilitate adding further features and making the code more testable. I'm working on a PR to refactor out the commandline wrapper code (all the cryptsetup stuff), to separate it from the export actions and make it independently testable. I'm also going to include some changes that will make it harder to use the export workflow wrong--for example, right now, the export actions all return None and set class variables as a side-effect, which makes it harder to look at the Export Actions and reason about what's happening or where something has gone wrong, and also make adding new export functionality a bit error-prone.

I'm partway through these changes and will put up a draft PR soon. I think this will make it a lot easier to perform error-handling and add additional export functionality in the future.

(cc @creviera since this is a little bit out of scope of the original discussion of this issue)

@sssoleileraaa
Copy link
Contributor Author

I'm working on a PR to refactor out the commandline wrapper code (all the cryptsetup stuff), to separate it from the export actions and make it independently testable.

After talking more to @rocodes in person about this, my understanding is that this is a large enough refactor that veracrypt support might need to be pushed back until the next release, which unfortunately isn't until August. There's still three weeks before we need to make that decision though!

@zenmonkeykstop zenmonkeykstop changed the title [spike] Support Veracrypt [securedrop-export] [spike] Support Veracrypt Dec 13, 2023
@zenmonkeykstop zenmonkeykstop transferred this issue from freedomofpress/securedrop-export Dec 13, 2023
@rocodes rocodes mentioned this issue Jan 23, 2024
34 tasks
@rocodes rocodes moved this to In Progress in SecureDrop dev cycle Jan 24, 2024
@zenmonkeykstop zenmonkeykstop moved this from In Progress to Ready For Review in SecureDrop dev cycle Feb 13, 2024
@cfm cfm closed this as completed in #1777 Feb 22, 2024
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in SecureDrop dev cycle Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants