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

header: implement v5 headers #111

Merged
merged 46 commits into from
Jul 13, 2022
Merged

header: implement v5 headers #111

merged 46 commits into from
Jul 13, 2022

Conversation

brxken128
Copy link
Owner

This is nowhere near done, at all.

I'm going to mark it as a draft for now - there's a lot to do. I need to:

  • Read the actual information itself from the header
  • Ensure that it can be done in a backwards-compatible manner
  • Handle serialization of the headers (and AAD creation)
  • Update the CLI app's functions (most notably the change password one)
  • Probably some more

There are a lot of things needed for this to work correctly - hopefully it's not too bad.

@brxken128 brxken128 marked this pull request as draft July 6, 2022 12:20
@pleshevskiy
Copy link
Contributor

To make everything work as expected at the end I can write tests)

@brxken128
Copy link
Owner Author

To make everything work as expected at the end I can write tests)

That works for me :)

Having a quick break and then going to get back to it.

@brxken128
Copy link
Owner Author

brxken128 commented Jul 6, 2022

As of my latest push, here's a rough outline of things I need to do before I'm happy with V5:

  • Serializing. This includes the first 32 bytes of the header, and every possible keyslot with the correct padding. For every absent keyslot, it needs to write 96 empty bytes in it's place. The total length should be 416 bytes, and the reasoning behind this is detailed in [FEATURE] Support for Multiple Keyslots #90.
  • Creating AAD (this will just be version/mode/algorithm/nonce)
  • Reworking the CLI app to adhere to the new formats
    • This will probably include the main encrypt/decrypt functions also, as they'll need to loop through the returned Vec<Keyslot> and attempt to decrypt the master key with the user-provided key. If there are no matches, reject the user's key as it is incorrect.
    • We'll also need to rework key changing functionality almost fully
    • Ensure that header stripping/restoring/etc is in working order, with the correct length. This is vital and will be catastrophic if there's any undefined behaviour.

@brxken128
Copy link
Owner Author

brxken128 commented Jul 8, 2022

https://github.com/brxken128/dexios/pull/111/commits/a642c74d4585662182e7dbfbfdc2db132e450f58 seems to have overwritten the recent changes in the master branch. I'll try to fix this lol

It went terribly wrong, and I blame that on my inexperience with git. Luckily I was able to find a local copy of header.rs, the file which contained most of this PR's work.

@brxken128 brxken128 reopened this Jul 8, 2022
@brxken128
Copy link
Owner Author

@pleshevskiy I've made some progress!

V5 headers work. I haven't tested it with multiple keys yet, as I need to add functionality for that within the header functions. I should also add support for argon2id, via -a.

It successfully encrypts, creates the AAD, deserializes and decrypts without a hitch.

@brxken128
Copy link
Owner Author

The changes in 5c7cf85 were made in order to separate the key functions from the header ones, but also in order to update the command syntax slightly.

I think dexios key change is worlds better than dexios header update-key.

I also plan to add a dexios key add function, which will be limited to V5+ headers and will only be available if < 4 keyslots are filled (as that is the hard limit within the standard).

@brxken128 brxken128 marked this pull request as ready for review July 11, 2022 16:00
@brxken128 brxken128 marked this pull request as draft July 11, 2022 16:01
@brxken128 brxken128 marked this pull request as ready for review July 13, 2022 14:57
@brxken128
Copy link
Owner Author

This is getting close to being finalised enough - I don't want to make too many changes, or else the merge conflicts will be a nightmare. key add functionality needs to be added, and key change functionality needs support for V5 headers, but aside from that things are pretty much done. I'll get to work on these in another PR.

@brxken128 brxken128 merged commit 6639139 into master Jul 13, 2022
@brxken128 brxken128 deleted the v5 branch July 13, 2022 15:02
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.

2 participants