Improve Libsodium API usage and use better xchacha20 version#12279
Improve Libsodium API usage and use better xchacha20 version#12279chrisbra wants to merge 15 commits intovim:masterfrom
Conversation
21b5866 to
2e4bc5b
Compare
Codecov Report
@@ Coverage Diff @@
## master #12279 +/- ##
==========================================
+ Coverage 72.15% 81.96% +9.81%
==========================================
Files 164 164
Lines 190725 194247 +3522
Branches 43573 43856 +283
==========================================
+ Hits 137611 159214 +21603
+ Misses 40929 22199 -18730
- Partials 12185 12834 +649
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 140 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Refactor the crypt_create functions and instead of passing separate arguments for salt, salt_len, seed, seed_len into a single struct and pass that around. This makes it easier, to later enhance the struct and add an additional parameter to pass around.
This adds a new Xchacha20v2 Encryption algorithm. This is basically the same as the existing xchacha20 algorithm, except that it will store the pwhashing parameters in the encrypted file header. The libsodium API strongely suggests to store those parameters alongside the encrypted to file to be able to decrypt files later on. This is required, because the libsodium API could change those parameters and in that case it would no longer be possible to decrypt files that were created before those constants change. For now, this is just a copy of the xchacha20 definition. We will make implementation changes in a later commit
Amend the cryptmethod_T struct and store the size of additional data that needs to be stored in the crypt header. This will be used later on to store the libsodium xchachav20 pwhashing parameters in the enrypted file header. For all previous methods, this is 0, except for xchacha20v2 version, which needs to store: - long long opslimit (8 bytes) + - size_t memlimit (8 bytes) + - int algorithm (4 bytes) which means we need 20 bytes additional storage.
Mention explicitly, that the swap file encryption is not supported when using the xchacha20 encryption methods. This is because those are no fixed size encryption and would therefore change the swap file format.
Instead of checking for crypt_method == CRYPT_M_SOD add a new function crypt_method_is_sodium, that will check for the 2 sodium encryption methods, CRYPT_M_SOD or CRYPT_M_SOD2. Most of the time, when checking for xchacha20 they need to be handled similarly (e.g. disable swapfile, disable undofile, etc), so encapsulate the actual check in a separate function
Instead of using the libsodium API constants for the password hashing, use variables. Later on, depending on whether the encryption is initialized from a file or not, we may need to pass either the default values or the values found from the header.
In order to allow the user to use the new xchacha20v2 cryptmethod, we need to allow to set the 'cryptmethod' option. So now add the new value 'xchacha20v2' for the new version fixed libsodium encryption value
Store the password hashing parameters opslimit, memlimit and algorithm in the encrypted file header for the xchacha20v2 version and use those values when decrypting the file contents. Note: I am not sure, if this works correctly when reading/writing across different architectures
Now that we have xchacha20v2 working and storing the relevant parameters in the file header, we can mark the previous version as deprecated (even so it is not really weak).
Amend the documentation for the new xchacha20v2 encryption method. While at it, also be explicit and directly mention for each weak encryption to no longer use it in the description of the option value 'cryptmethod'
We need to be careful when changing the header format. Enryption and Decryption of file contents depends on the same block size. Unfortunately, with xchacha20v2, the file header changes and adds additional 20 bytes and we will use that block size to read the first block. However, when starting to read an enrypted file, we do not know yet, whether it uses the xchacha20 or xchacha20v2 encryption method. We will initially use the slightly larger block size of the xchacha20v2 version to read the file, but when detecting xchacha20 format and the file did not fit into the 8K + plus metadata, we need to rewind the file-descriptor by those additional 20 bytes (add_len bytes of xchacha20v2), to allow to properly decode it. Also when decoding the block using xchacha20, we need to adjust the blocksize and remove the 20 bytes again, since those are only there for the new xchacha20v2 version, but not the old version.
output the different hashing parameters when using ':verbose' to open a xchacha20v2 enrypted file.
|
Tests looked good, except for one failure in macos huge which seemed unrelated: rebased to latest master now |
|
I have the habit of using a prefix for struct members, so that it's easy to change them and avoid mistakes with using the wrong name. The "cat_" prefix should work well for "crypt_arg_T". The "#ifdef FEAT_EVAL" before using p_verbose doesn't make sense. Since this appears to be for debugging, putting the #ifdef around the use of crypt_sodium_report_hash_params() should work better (and make the tiny build a bit smaller). |
Move the #ifdef FEAT_SODIUM around the whole function definition.
|
I pushed 2 commits to address those two points. |
This is the PR for fixing #12204
I have tried to split up the changes to make reviewing easier. It starts by refactoring some of the crypt code to make integrating the new libsodium method easier.
There are 2 gotchas:
I have added some tests to verify that the current and old encryption works when more than 1 block is read. Also added one test to make sure decrypting works with custom hashing parameters used, to verify that reading those parameters from the file work.