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

Encrypted at rest tagging #1611

Closed
wants to merge 31 commits into from
Closed

Conversation

psivesely
Copy link
Contributor

This is far from ready. Creating this PR to get early feedback on my implementation of database column encryption.

Database column encryption works as follows. Upon creation of the first admin, a single, shared database key is generated server-side. This key is encrypted to a key derived from the admin's password and a salt and stored in a new column in their Journalist record. Upon login to the JI, the admin's password and salt are used to derive the key that decrypts this shared database key. The database key plaintext is then stored in the client-side session. Each time a request is made that involves label text, the database key, sent to the server via the signed client-side session, is used to decrypt and encrypt label text. I created a transformer and comparator to allow searching the encrypted records, and ensure uniqueness of label text. When a new user is created, the database key is also taken from the client-side session of the admin creating the new user, and encrypted to the "personal_key" derived from their password and a new salt, the encrypted database key and salt being stored as new columns in this new users Journalist record.

The concepts here are somewhat similar to what we do with source codenames. Since we can avoid the nightmare that is GPG and use much faster AES-CTR, the implementation is fairly simple, and in manual testing I don't experience a slow down (more on this in a bullet point below). In the future, I'd like to expand and improve on the ideas here and apply them to additional database columns. If we were to add authentication and create a server-side client, this could be one way de-privilege the app server (although I have some other ideas to this regard that might be better).

There's a lot left to implement and investigate, including:

  • One of the impetuses for creating this was to offload the UX burden of only allowing admins to create labels (and putting up the big opsec warning on the page to do it). I still haven't changed the code to allow all journalists to do so.
  • @ninavizz is going to get back to me sometime next week with some wireframes regarding a rework of how the labels will work and how journalists will be able to interact with them. Once we settle on a design, I'll have to implement it.
  • Changes to the manage module snuck into one of Jen's commits, and I'll need to revert that to scope the PR appropriately.
  • I want to get some better benchmarks of what it's like to add this feature to a busy SecureDrop that's getting 50-100 tips a day and labeling them all with 1-3 labels out of a collection of a couple dozen. Also, I want to add authentication, and evaluate that as well.
  • Investigate use of flash (re DRY out flashed templates #1602), and see if we're unnecessarily creating too many categories.
  • I still need to finish implementing the logic in the manage module that handles the case where an admin is created who is not the first admin. This will require another user (not even admin) to enter their credentials in order to decrypt the database key so it can be encrypted to the new admin. This we can assume is a pretty rare scenario, so hopefully it won't be too burdensome when it does have to happen.
  • I have yet to consider all possible credentials lost scenarios, and try to come up with the best resolution to them all. As long as one user has their credentials, the database key will be decryptable, and the labels will be salvageable. I think I'm going to actually resurrect Prevent admin journalists from deleting themselves #1562 because it would help prevent loss of the database key.
  • I have some ideas about metadata obfuscation. With the way this is currently implemented you can see which label ids are applied to each submission and source, even though you can't see the text corresponding with each label id. I suspect that I can only partially ameliorate this situation without a heavy performance penalty, but even if my solution is not information-theoretic, some coverage at a minimal price is probably the balance of obfuscation and performance we want to aim for here.
  • I wrote some additional tests, but I really want to test this thing to death before we consider shipping it. So I'll be writing more.
  • When everything else above is checked off, I'll need to rebase, and write some good commit messages.

One more thing I'll touch on as a later idea is that if #204 is implemented we could store the database side in sources' client-side sessions. The reason for this is that it would allow us to encrypt sensitive columns such as submission times and could pose an alternative or addition to #822.

Closes #1170. Closes #1422 (supersedes). Closes #1562.

redshiftzero and others added 30 commits March 2, 2017 18:31
…using tags

Tests verify that an admin is able to add and remove tag categories from the
SecureDrop server, that a journalist is able to add and remove labels from a particular
source or submission, and that they are able to filter sources by particular labels.

This also modifies the templates to add ids to some elements for ease of testing.
* DRY up utility functions
* Use SQLAlchemy ORM more effectively (i.e. relationships)
…for rebase

As part of a rebase of @redshiftzero's `tagging` branch onto `develop`, I had to
skip an unmergeable patch, 3f3bbb8 because it modified a no longer existing
file. The commit in question defined some CSS styles for the tags in Jen's
implementation of tagging support. Since we switched to SASS, I also couldn't
just run `git show 3f3bbb8 | patch securedrop/sass/journalist.sass`--there was
some refactoring involved anyway, which I only discovered once digging into the
SASS. I ended up manually converting the CSS to SASS and placing it in the
appropriate files. This committ is the result of that work.
@psivesely
Copy link
Contributor Author

Closing for now because I know this won't get considered for merge for a really long time and will be out of date by then.

@psivesely psivesely closed this May 19, 2017
@legoktm legoktm deleted the encrypted-at-rest-tagging branch January 9, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants