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

[ENG-440, ENG-366] Header format improvements & crypto refactor #610

Closed
wants to merge 191 commits into from

Conversation

brxken128
Copy link
Contributor

This PR adds encrypted file headers, advanced versioning and schemas (built on top of bincode).

We use:

  • a trait to define a header
  • a macro to reduce the amount of manual changes
  • a header interface which probably isn't required but definitely makes the API nicer
  • bundles to "wrap" certain items so that we're able to deserialize the correct version with ease

Header metadata and preview media objects have been scrapped entirely for a new, generic object-based system (with enum type tagging).

We write the header's size, and then the header itself to the stream - I don't know if this is required at all. There's a high probability that bincode will handle it just fine, so I may drop this.

We do need a better way to interface with the header objects directly, and we need the functionality to add/remove keyslots from the header (and update it, but that's the easy part). We should also probably limit the total size of the header objects, as there's no limit (there's only a limit on the amount of objects).

This PR breaks quite a lot of the current encryption/decryption features, but most of that is going to change anyway with the UX work that's planned so it's not major at all.

@vercel
Copy link

vercel bot commented Mar 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
spacedrive-landing ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 26, 2023 1:46pm
spacedrive-web ❌ Failed (Inspect) May 26, 2023 1:46pm

@utkubakir utkubakir changed the title [ENG-440] Header format improvements [ENG-440, ENG-366] Header format improvements & crypto refactor Jun 10, 2023
@linear
Copy link

linear bot commented Jun 10, 2023

ENG-440 Encrypted header upgrades

Currently, we have some versioning in place for our encrypted headers. It is not robust, nor is it (too) easy to update to a new version.

These changes should completely resolve that issue - and more. I plan to use bincode, a trait, a macro and a schema-based system which should allow for us to support new versions while also maintaining perfect backwards-compatibility.

The plan is subject to change, and it will be entirely incompatible with our current header format - but hopefully any future changes will be extremely easy to implement (and won't break previous versions).

I am also going to make the header objects extremely generic, and we can add new specific types as we please. The current preview media and metadata objects will be dropped in favor of this design.

ENG-366 Crypto UX overhaul

The UX of our crypto system isn't the most ideal. It's littered with confusing terminology, unnecessary actions and isn't the easiest for the everyday user to understand.

I have a series of changes that will bring it to the next level:

  • Key mounting will remain as a concept, but the meaning will be shifted. Users will need to unlock their key manager with their master password, and then they will need to "mount" keys before they are available for use. Mounting meaning, re-enter the key. It is critical that we don't store anything about the key directly. We can verify the key is correct by attempting to decrypt a "dummy" value (will require a few extra DB columns, nothing major). A bad client wouldn't gain an advantage in this situation, and users would really just be shooting themselves in the foot if they altered this functionality manually.
  • For the above to function, long/memory-hard hashing will have to be done on each key mount attempt.
  • I think key deletion as a concept should be removed, as it will offer near-zero security benefit, and will only complicate key-content salt associations
  • In relation to removing key deletion, I'm unsure how we can approach the issue where the exact same key being added to the key manager twice will result in entirely different functionality. If we're sticking with this form of mounting/this key manager design as a whole, there's no viable solution. I originally planned to hash all key values with the content salt from the file until we found a match, but now that we won't actually know the key values this is impossible. The only option I can see for users who want the exact same key between two different libraries will be to use the key to encrypt a file, and then use the "decrypt with password" functionality. We can offset this moderately by allowing users to enter the content salt manually in a hidden away input box, but it is still not the most ideal.
  • The key viewer will have to be modified to remove the key value itself
  • Decryption with the key manager can no longer automatically mount the associated key
  • Auto-mounting will have to be removed, as we don't know the key itself

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.

None yet

2 participants