elasticdog / transcrypt

transparently encrypt files within a git repository
MIT License
1.44k stars 102 forks source link

Add support for pbkdf2 key derivation with iterations, and make this the default #126

Open jmurty opened 2 years ago

jmurty commented 2 years ago

This is some initial, and very rough, progress towards adding support for pbkdf2 key derivation as available in (and strongly recommended by) newer versions of OpenSSL and LibreSSL, aiming to solve lingering issues #55 and #59

This branch/PR is taking an ugly but easy approach where the existing transcrypt.cipher configuration setting stores compound values where a key derivation function and iteration count can be set as cipher:key-derivation:iterations — i.e. aes-256-cbc:pbkdf2:1024 to replace the prior aes-256-cbc

As of now:

TODO:

brianmay commented 2 years ago

What implications does this have, if any, for existing repos?

I am assuming that it will be possible to upgrade the cipher used for a repo, and at that point you will need a version with this change. Is that correct?

jmurty commented 2 years ago

The main implication for the improved crypto approach is that, for updated repos, transcrypt will no longer work out-of-the-box on systems with outdated versions of OpenSSL: in particular, on Macs.

So the necessary steps would be:

Erotemic commented 2 years ago

Is there still momentum here? I have a fork where I enable pbkdf2, I can help with tasks to get this landed sooner if that is helpful.

jmurty commented 2 years ago

Hi @Erotemic although there isn't much momentum at present – I've had a busy few months – I'd appreciate your help to push things forward.

The main outstanding tasks are listed in the TODOs above, but I think the most difficult thing to get right will be the user experience: helping people to upgrade existing repos, recognising when the user's system isn't compatible with the new approach (especially macOS) and helping them to fix that.

The discussion over in #55 about whether simply switching to pbkdf2 is sufficiently secure given how the salt is handled is also worth resolving. If we're going to break backwards compatibility with a new version we should make sure the approach is as strong as we expect. I'm no cryptographer however, so am out of my depth on that discussion.

Erotemic commented 2 years ago

@jmurty my thought is that a good first pass would be to simply make all of these new options configurable, while keeping existing defaults. If we restrict the scope of the PR to just allowing the user to configure a more secure transcrypt repo rather than defaulting to those secure settings, then it will be a lot easier to write a stable patch. Then once that is merged and users have the option of creating new repos with secure settings, we can think more about the process of helping users seamlessly upgrade.

I have a pass at this in my fork, but I've hard-coded all of the config settings that I like. To port those changes here I'd like to know: what is the best way for storing "transcrypt state"? Just add new git configuration variables with a transcrypt prefix?

Erotemic commented 2 years ago

Another though that I had. Instead of creating a custom cipher scheme that we need to use awk to parse (which imo is somewhat awkward), would it make more sense to simply let the user pass custom options to openssl? Or should we lock specific options that are supported to prevent foot-shooting? In any case, I think the cipher variable should be left alone and we should use new state variables to store if we are using a KDF and any options to that KDF (I'm also thinking if we don't support custom options, maybe don't allow the user to change iters - at least in this PR for simplicity? If a user really wants that they will make a patch, and we can discuss it there).

Something like this:


# Configurable Sandalone OpenSSL Demo
password="12345"
kdf_options="-pbkdf2 -iter 1000"
cipher=aes-256-cbc
md_digest="sha256"
salt=deadbeaf00000bad

read -ra kdf_option_array <<<"${kdf_options}"
echo "secret_text" | SECRET_PASSWORD=${password} openssl enc -"${cipher}" -md "${md_digest}" "${kdf_option_array[@]}" -pass env:SECRET_PASSWORD -S "${salt}" -e -a 
echo "U2FsdGVkX1/erb6vAAALrcaDZJ0Ixb0qwGA4VBjTQS4=" | SECRET_PASSWORD=${password} openssl enc -"${cipher}" -md "${md_digest}" "${kdf_option_array[@]}" -pass env:SECRET_PASSWORD -d -a