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

[docs] Overhaul Transfer Device and export recommendations #4657

Merged
merged 3 commits into from Sep 17, 2019

Conversation

@eloquence
Copy link
Contributor

commented Aug 2, 2019

Status

Ready for review

Description of Changes

  • More clearly prioritizes printing as the recommended method to get documents off the SVS whenever feasible;

  • Introduces a recommended workflow using a VeraCrypt-encrypted Export Device intended to be clear and consistent, while still closely resembling current practices;

  • Removes instructions to set up per-journalist GPG keys;

  • Updates diagrams consistent with the above.

Resolves #4620
Resolves #4646
Resolves #4434
Resolves #4670
Resolves #3598

Review plan

Review the documentation against the issues below. Does it now provide instructions to administrators that are clear, sensible, and consistent? Please note the significant updates to the diagrams in the documentation.

The biggest change for end users is the recommendation to provision a USB drive encrypted with VeraCrypt as the Export Device. I have done this and used it both in Tails and on my regular Linux workstation. I would recommend that the reviewer similarly provision a VeraCrypt drive and attempt to access it in a recent version of Tails and on their everyday workstation (bonus points for testing on Mac or Windows).

I have tested that VeraCrypt successfully mounts read-only drives, both using the application, and using the graphical integration in Tails. On the command line, cryptsetup must be invoked with the --readonly option for this to work.

@eloquence

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2019

Still a lot to do here, but I wanted to get the current WIP up for any early feedback. Most of this should be consistent with our recent discussions. Couple of areas where I think it could be useful for FPF to do a bit more hardware testing, if we haven't already done so:

  • USB write blockers like this one seem like a relatively cost effective way to prevent infecting drives that may be taken back to the SVS.

  • VeraCrypt is still pretty cumbersome and slow, and a hardware encrypted drive honestly seems more user-friendly and practical for SecureDrop uses. If we can recommend such an option, it would seem to potentially bypass a lot of usability and cross-platform issues. There are hardware-encrypted NitroKeys, which are advertised as fully open source & open hardware w/ third party audits. If these hold up under scrutiny, it would be nice to be able to officially recommend them.

@eloquence eloquence force-pushed the docs-transfer-to-export branch from f11bb56 to 697ac00 Aug 2, 2019
@eloquence eloquence added this to In Development in SecureDrop Team Board Aug 2, 2019
@eloquence eloquence force-pushed the docs-transfer-to-export branch 2 times, most recently from b544d0f to 8dc3913 Aug 2, 2019
@eloquence

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

As a side note, the recommendation in this PR (to use separate USB drives for Export and Transfer Device) is in fact exactly identical to the architecture in this diagram where these are called "USB Key A" and "USB Key B". The only problem is that "USB Key A" and "USB Key B" are only referred to in this diagram and nowhere else in the docs.

Copy link
Contributor

left a comment

Great clarifications throughout, @eloquence. Left a few comments for further improvements to readability, but overall these changes are already quite strong.

docs/checklists/pre_install_hardware.rst Show resolved Hide resolved
docs/hardware.rst Outdated Show resolved Hide resolved
docs/hardware.rst Outdated Show resolved Hide resolved
docs/passphrases.rst Outdated Show resolved Hide resolved
docs/set_up_transfer_and_export_device.rst Outdated Show resolved Hide resolved
eloquence added a commit that referenced this pull request Aug 9, 2019
Detailed justification in #4657
@eloquence

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

9a5349a is potentially the single most contentious part of this PR. It removes all references to setting up individual GPG keys for journalists, and instead provides instructions for using VeraCrypt.

As a reminder, VeraCrypt replaces LUKS as the previous recommendation -- a recommendation which was impossible to follow in practice. The documentation effectively instructed journalists to double-encrypt files, with LUKS and with their personal keys, and to do so in a manner that is impossible to decrypt on their everyday workstation (due to LUKS not being cross-platform). That said, whether to copy to Tails or to their everyday workstation -- even that part was not clearly spelled out.

Our docs provided minimal instructions for working with GPG keys (instead linking out to the EFF guides), and suggested that the private key be stored on the Journalist Workstation -- which is defined in the glossary as the Tails-based workstation used to access the journalist interface. That makes little sense in the real world, where journalists will need to leave Tails behind pretty quickly.

An easier option than introducing VeraCrypt would be to make the previous GPG instructions a bit more sensible, and to ask journalists to use an ordinary, unencrypted USB drive to copy files off the SVS, encrypting them first to their public key. This may well be current practice in at least some organizations.

The downsides of that approach are significant, IMO:

  • Relying on GPG as the standard way to encrypt data signs organizations not already committed to GPG up for a lot of complexity, compared with using an encrypted drive.

  • Encrypting individual submissions while keeping the drive unencrypted keeps metadata (time/date of export, file size, etc.) in unencrypted form. There's a high risk of accidentally decrypting submissions directly on the Export Device, or accidentally copying data in unencrypted form.

  • An adversary gaining access to an unencrypted Export Device may be able to successfully recover "deleted" files without any additional credentials.

For these reasons, I don't think it's even a good idea to suggest a GPG workflow as an alternative to the VeraCrypt approach in the docs.

If all of this leaves you with a bad feeling, I share the same concern. I'm not a fan of any of these solutions. VeraCrypt is non-free software (due to legacy TrueCrypt bits), difficult to use (a lot of complexity in the UI that should be hidden behind progressive disclosure patterns and sensible defaults), and a bit flaky: I've had it randomly freeze on me twice during testing.

In terms of cross-platform usability, I've not tested them personally, but it seems hard to beat an integrated hardware-encrypted solution like the Kingston DT-2000 with on-device PIN entry. And of course there are more open solutions like the NitroKey encrypted drives that may be worth considering.

But if we have to choose between the devil and the deep blue sea, I think just recommending VeraCrypt and purging GPG from the docs is our best long term bet, and consistent with our current plan of record to use VeraCrypt on the SecureDrop Workstation. Really, much of this complexity is inherent in working with an air-gapped system, and the Workstation will greatly reduce the end user complexity here.

Finally, I'll mention Jen's suggestion to get files off the SVS by way of Tails+OnionShare. Tails does have pretty nice built-in OnionShare integration, and this would allow us to use LUKS for both the Transfer Device and the Export Device. It does introduce a new layer of complexity -- downloading files from Tor on the journalist's everyday workstation, and keeping the OnionShare on Tails running while that's happening, plus the usual concerns about Tor speed and reliability. This makes me think that OnionShare is always a good candidate as a secondary option (both here and on Qubes), but not so much as the primary recommendation.

@eloquence eloquence force-pushed the docs-transfer-to-export branch from 6eb69ff to d91a52f Aug 15, 2019
eloquence added a commit that referenced this pull request Aug 15, 2019
Detailed justification in #4657
@eloquence eloquence force-pushed the docs-transfer-to-export branch from d91a52f to 1d4c8c0 Aug 15, 2019
eloquence added a commit that referenced this pull request Aug 15, 2019
Detailed justification in #4657
@eloquence eloquence requested review from emkll and harlo Aug 15, 2019
@eloquence

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2019

OK, I'm going to call this ready for review! I've tried to be as thorough as possible in updating all existing references in the docs to the workflow that's being described here, but it's possible that I have missed some. I will take a couple more passes myself.

Because this PR impacts our training practices, it should not be considered ready for merge unless and until @harlo gives her +1.

@eloquence eloquence marked this pull request as ready for review Aug 15, 2019
@eloquence eloquence changed the title [WIP] Overhaul Transfer Device and export recommendations Overhaul Transfer Device and export recommendations Aug 15, 2019
@eloquence eloquence moved this from In Development to Ready for review in SecureDrop Team Board Aug 15, 2019
Copy link
Contributor

left a comment

Thanks @eloquence for the changes, I've made a first pass review and left a few comments inline. Regarding the write protection, If we are to recommend it on the export device, it might also be worth recommending it on the transfer device for completeness, preventing files to be written by the svs to the transfer device

docs/hardware.rst Outdated Show resolved Hide resolved
docs/hardware.rst Outdated Show resolved Hide resolved
docs/hardware.rst Show resolved Hide resolved
docs/hardware.rst Outdated Show resolved Hide resolved
docs/hardware.rst Outdated Show resolved Hide resolved
docs/journalist.rst Outdated Show resolved Hide resolved
docs/journalist.rst Outdated Show resolved Hide resolved
docs/diagrams/SecureDrop_DataFlow.draw Show resolved Hide resolved
docs/journalist.rst Show resolved Hide resolved
docs/passphrases.rst Show resolved Hide resolved
@eloquence eloquence force-pushed the docs-transfer-to-export branch from 282b406 to 82c9436 Aug 19, 2019
eloquence added a commit that referenced this pull request Aug 20, 2019
Detailed justification in #4657
@eloquence eloquence force-pushed the docs-transfer-to-export branch from 33bb90b to 4c5c739 Aug 20, 2019
@eloquence eloquence moved this from Ready for review to In Development in SecureDrop Team Board Aug 20, 2019
Copy link
Contributor

left a comment

I'm comfortable with this, @eloquence; thanks for strongly enumerating the caveats up-front. 👍

@huertanix

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

I read through the VeraCrypt updates and I think they look great; Only two minor things that might be 🆒 to add in the future.

  • Adding something along the lines of "read out guide to learn about how password managers work and how to pick one" linking out to our guide when mentioning using a personal/phone password manager to keep the Export key passphrase in.
  • Mentioning, either in the instruction bullet point asking the user to open VeraCrypt on their everyday workstation or wherever it makes more sense, that they need to first install VeraCrypt on their everyday workstation. This also might be a place to link to the VeraCrypt downloads page and our guide on verifying open source software.
@eloquence

This comment has been minimized.

Copy link
Contributor Author

commented Sep 5, 2019

Thanks @huertanix, @harlo and @olivemartini (who sent comments privately), much appreciated :)

Adding something along the lines of "read out guide to learn about how password managers work and how to pick one" linking out to our guide when mentioning using a personal/phone password manager to keep the Export key passphrase in.

This is currently linked to, here:

https://github.com/freedomofpress/securedrop/blame/docs-transfer-to-export/docs/set_up_transfer_and_export_device.rst#L89-L93

I tried to address this in a single place ("Decide how to manage passphrases") since it comes up a few times.

I agree re: the VeraCrypt requirement, will clarify.

@eloquence

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

@huertanix VeraCrypt docs clarified in 920ac10; feedback appreciated if you have time :)

@eloquence eloquence changed the title Overhaul Transfer Device and export recommendations [docs] Overhaul Transfer Device and export recommendations Sep 6, 2019
@eloquence eloquence added this to the 1.0.0 milestone Sep 10, 2019
eloquence added a commit that referenced this pull request Sep 11, 2019
Detailed justification in #4657
@eloquence eloquence force-pushed the docs-transfer-to-export branch from 920ac10 to 05679b9 Sep 11, 2019
eloquence added a commit that referenced this pull request Sep 11, 2019
Detailed justification in #4657
@eloquence eloquence force-pushed the docs-transfer-to-export branch from 05679b9 to 54301d8 Sep 11, 2019
Copy link
Contributor

left a comment

Thanks for the changes and for your patience @eloquence . I've taken another pass through your changes, and the content looks great, I have no further comments to provide, and think this is good to merge.
However, I've included two comments regarding the commit history. From the GitHub review menu, it seems like we will be losing the git history for two files, due to a file name change. Are those file name changes strictly required? If so, would it be possible to use git mvinstead?

docs/includes/pre-install-hardware.txt Show resolved Hide resolved
docs/set_up_transfer_device.rst Show resolved Hide resolved
eloquence added 2 commits Jul 31, 2019
Resolves #4620
Resolves #4646
Resolves #4434
Resolves #4670

In addition to introducing the Transfer and Export Device,
this commit clearly breaks out optional hardware into its
own checklist, more strongly recommends purchase of a
printer, and strengthens recommendations for malware
mitigation.

It removes some outdated recommendations and a reference to
storing the journalist's GPG passphrase in KeePassX
(not mentioned anywhere else).

It removes instructions for journalists to set up individual
GPG keys, as they are unlikely to be followed, and the more
critical recommendation is to ensure tha the Export Device
is encrypted.

It updates the overview diagram and data flow diagram
consistent with the above changes. This update also makes
the diagrams more consistent with terminology and current
practices. It removes OnionShare from the data flow
diagram, as it is not currently mentioned elsewhere in the
docs.
@eloquence eloquence force-pushed the docs-transfer-to-export branch from 54301d8 to 14bdc72 Sep 12, 2019
@eloquence

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

I squashed this PR into a single commit, and split the rename operations into a follow-up commit. GitHub's combined "Files" view still shows the operations as deletions, but if you git log --follow on the renamed files, you should now get the full history.

Copy link
Contributor

left a comment

Changes identical to the ones reviewed previously in 54301d8 (https://github.com/freedomofpress/securedrop/compare/54301d89a200a5c98e814cec905f1e6e1407dc33..14bdc72f946387ac79c155e6816d5590a280f949)

Confirming that git log --follow now works for those files, 🎉 thanks @eloquence

Copy link
Contributor

left a comment

The technical procedures for use of a Veracrypt-encrypted Export USB look good. I'm concerned about the practicality of some of the recommendations around hardware choices and solutions to prevent malware being copied to the SVS, but there's nothing there that should block approval. The updates to the threat model document are incomplete, however.

docs/set_up_transfer_and_export_device.rst Outdated Show resolved Hide resolved
docs/set_up_transfer_and_export_device.rst Outdated Show resolved Hide resolved
VeraCrypt-encrypted media can be opened in the Tails operating system and on
common Linux distributions without installing additional software. To open
VeraCrypt media on Windows or Mac workstations, or to create VeraCrypt drives,
you need to install the VeraCrypt software. The `guide by Freedom of the Press

This comment has been minimized.

Copy link
@zenmonkeykstop

zenmonkeykstop Sep 16, 2019

Contributor

It might be worth being explicit here and saying that you need to create the Veracrypt volumes off Tails. It's possible that someone would read this as saying you should install additional software on Tails to create Veracrypt volumes. We don't want to encourage this IMO. Users shouldn't be customizing their Tails distros without good reason.

This comment has been minimized.

Copy link
@eloquence

eloquence Sep 16, 2019

Author Contributor

Added a note to the effect that it should not be installed on the SecureDrop drives in 60cc802. (IMO another Tails drive may be a fine choice.)

@@ -473,7 +473,6 @@ What a Compromise of the Journalist's Property Can Achieve
- Access the Hidden Service values used by the *Journalist Interface*.
- Access SSH keys and passphrases for the *Application Server* and the
*Monitor Server*.
- Access the journalist's personal GPG key.

This comment has been minimized.

Copy link
@zenmonkeykstop

zenmonkeykstop Sep 16, 2019

Contributor

Threat model doc refers to SD 0.3 - unclear if it should be changed piecemeal, or edited in its entirety to include references to other assets. This change adds an Export USB and recommends the use of a password manager on the journalist's mobile device, neither of which are covered here.

This comment has been minimized.

Copy link
@eloquence

eloquence Sep 16, 2019

Author Contributor

Per discussion earlier today, I've made minimal updates to the threat model consistent with the new terminology and updated data flow in 60cc802.

Copy link
Contributor

left a comment

Changes in the threat model document look good to me. Not a blocker for merge, but I think it might be useful to track the differences with the implications of compromise of the transfer device (which should have only gpg-encrypted submissions, on a luks-encrypted device) vs export device (veracrypt encrypted device, without necessarily having per-file encryption).

@eloquence eloquence force-pushed the docs-transfer-to-export branch from 60cc802 to 86db461 Sep 17, 2019
@eloquence

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

@emkll I've tried to clarify the potential use of write protection for the Transfer Device as a defense-in-depth strategy in 86db461 (amended existing commit, which seems more readable in this case).

Regarding the threat model, note that it speaks already to the point you're mentioning, as it notes that an attacker will have access to SecureDrop submissions if the Export Device is in use:

View, modify, and delete decrypted submissions, if they are stored in decrypted form on the Secure Viewing Station, or if the Export Device is in use.

If the journalist follows the recommended procedure, this will not be the case even if the Transfer Device is in use.

I think the Export Device / Transfer Device risks could be more clearly separated from the workstation-related risks, but I also think the way this information is organized right now could be improved, so I would recommend leaving that to a follow-up PR.

The hardware recommendations more clearly prioritize choices we
can support and are familiar with (VeraCrypt, USB w/ write switch)
over ones we cannot currently support well (hardware-encrypted
drives, write blockers).

The threat model has been updated consistent with the data flow
updates; note that the clarified data flow means that the Transfer
Device never stores _decrypted_ submissions.

The use of write blockers for the Transfer Device has been
clarified, as well.
@eloquence eloquence force-pushed the docs-transfer-to-export branch from 86db461 to 3b886df Sep 17, 2019
@emkll
emkll approved these changes Sep 17, 2019
Copy link
Contributor

left a comment

thanks for the changes @eloquence , looks good to me. I won't immediately merge however to ensure @zenmonkeykstop is satisfied with the changes that were requested.

Copy link
Contributor

left a comment

LGTM, thanks for updates.

@zenmonkeykstop zenmonkeykstop merged commit 5fc5ed5 into develop Sep 17, 2019
3 checks passed
3 checks passed
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: static-analysis-and-no-known-cves Your tests passed on CircleCI!
Details
ci/circleci: translation-tests Your tests passed on CircleCI!
Details
SecureDrop Team Board automation moved this from Ready for review to Done Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.