Suppress warnings for OpenSSL 1.1.1

[dirkhas]

Using OpenSSL 1.1.1 (in my case, in Git for Windows, which uses OpenSSL 1.1.1a 20 Nov 2018), the clean script constantly leads to git commands printing out the following warning:

*** WARNING : deprecated key derivation used.
Using -iter or -pbkdf2 would be better.

[dirkhas]

I'll create a pull request to fix it.

[ghost]

in transcrypt 1.1.0, I still get this warning,it recommend 'Using -iter or -pbkdf2 would be better.'

[elasticdog]

This will be a bit tricky to address, as OpenSSL 1.1.1 was released in Sep 2018 and I want to make sure that older versions also work properly. The situation is further complicated by macOS using LibreSSL 2.6.5 these days, which does not support the -pbkdf2 option, as far as I know. I'll have to do a bit more research here to know how far back the -iter option has been around in both projects to see if it makes sense to update the default behavior and introduce a breaking change.

I don't think suppressing the warning is the best way forward in this case since it did bubble up a legit deprecation that will bite us eventually.

[Erotemic]

Perhaps it is possible to do a version check and then actually use -pbkdf2 and -iter when calling openssl, as long as you have at least 1.1.1?

EDIT: After looking into this I realize why its tricky. You couldn't do a checkout on an older machine if you encrypted on a newer machine.

It might be reasonable to add a flag the enables -pbkdf2 to transcrypt when you either setup or authenticate a repository. Then each repo would have the option of forbidding the older versions of openssl.

[andreineculau]

I'm wondering if anyone has a clear path going forward.

Both -pbkdf2 and -iter have landed in 1.1.1 afaik and libressl 2.9.1, so in order to support previous versions, you need to check for the support of the flags. But that is not enough because it should be an active decision of transcrypt not to support openssl@<1.1.1/libressl@<2.9.1, or of the transcrypt user not to support openssl@<1.1.1/libressl@<2.9.1, otherwise you risk weird behavior on two different machines, one with, and the other without these flags.

If someone decides, I can file a PR to test out the decision, no problem.

[jmurty]

I think the best approach for handling this situation would be for transcrypt to switch to using the more secure -pbkdf2 and -iter options by default, but in a way that minimises breaking changes as much as possible.

Here are some rough ideas on how to do this, and a longwinded (sorry) brain-dump about how it could all work.

Additional features

• add support for a new (optional?) transcrypt.openssl-path config setting that points to the OpenSSL binary to be used by the clean / smudge / textconv scripts. With this setting, users on platforms with outdated OpenSSL like macOS could tell transcrypt to use a newer versions installed in a custom location, e.g. with Homebrew.

• add a crypt-cipher or similar attribute to .gitattributes to capture the encryption method transcrypt will use for that file, and support a specific set of values represented as alias like v1 / v2 etc or maybe something more explicit like aes-256-cbc:md5. Again, the clean / smudge / textconv scripts would check and respect this attribute.

• before attempting to run OpenSSL the clean / smudge / textconv scripts could check the required encryption method vs what is supported by OpenSSL and the configured path, and do something sensible if there's a mismatch. Ideally print helpful warnings, if that's possible from within those scripts.

Usage

With the above, on init transcrypt could:

• check the OpenSSL version available on the platform, and if an outdated version is present recommend that the user install and configure a newer OpenSSL version for transcrypt to use.

• prompt the user to choose an encryption method as one of the settings, with an explanation of pros and cons. E.g. the newer method is more secure, but not supported on some platforms without extra work. If a user has an old OpenSSL they wouldn't have the option to use a newer encryption method.

  • The only real effect of this choice would be to use the chosen method in the example line in the generated .gitattributes file, since that is where each file's encryption method should be captured.

Scenarios

If a user chooses to use a newer method for some or all of their secret files:

• other users with the latest transcypt and newer OpenSSL will have things work seamlessly
• other users with the latest transcrypt but older OpenSSL could not decrypt the files, but would at least get the most sensible behaviour possible (e.g. warning messages) for repos that use an encryption method their system doesn't support
• other users with old OpenSSL versions and old transcrypt versions are just out of luck if they encounter a repo using a newer encryption method that their system doesn't support. I don't think there is anything much we can do about that?

If a user chooses to use an older encryption method for maximum compatibility this should be handled transparently for other users in all cases:

• the latest transcrypt will check the file's attributes and apply the older encryption method
• older transcrypt versions would be oblivious to all this and just do their thing, which would happen to be the right thing.

[elasticdog]

I think most of that sounds sane, but I'm not sure if setting an explicit path to the binary (versus just relying on the user's $PATH) would be worth the trouble versus transparently testing and warning about incompatibility issues. Is there a scenario you're imagining where the user would install a more modern version of openssl, but not want it to be their environment's default?

[jmurty]

That is a good point that we should expect users to simply add a newer version of OpenSSL to their path once they have gone to the trouble of installing it.

I myself don't do this on macOS despite installing later versions with Homebrew, mainly because I'm scared of breaking things in subtle ways which is surprisingly easy to do with macOS + OpenSSL + Homebrew. The fact I don't see the ugly OpenSSL warnings is a nice side-benefit. But that whole idea of making transcrypt learn the OpenSSL path is probably just me projecting my own poor practices.

Regarding my suggestions about adding cipher definition attributes to .gitattributes I'm still undecided about whether it really makes sense to do that, or whether it would be better to add explicit transcrypt version headers to the actual encrypted files instead as discussed briefly here, or some combination of the two. I might need to do some rough prototyping in code to try out different things and learn what tends to work best.

[jwatt]

I'll have to do a bit more research here to know how far back the -iter option has been around in both projects

andreineculau already gave "as far as I know" version numbers, but to confirm them I went digging for the relevant info.

From this commit it looks like -pbkdf2 and -iter were added to LibreSSL at the same time (although the commit message and subsequent release notes only mention -pbkdf2 explicitly). Expanding the tags on that commit it looks like they first shipped in libressl-v2.9.1 (which was released April 21st, 2019). That's confirmed by the 2.9.1 release notes.

From this commit it seems that -pbkdf2 and -iter were also added to OpenSSL together at the same time. Expanding the tags it looks like it was first landed in the original 1.1.1 (released 11 Sep 2018 as you say). It doesn't look like they were backported to 1.1.0.

[jwatt]

that whole idea of making transcrypt learn the OpenSSL path is probably just me projecting my own poor practices.

No, I agree with you. I have also previously had unexpected things break when adding Homebrew's keg-only OpenSSL to $PATH on macOS, and therefore for the same reason would benefit from being able to tell transcrypt where to find the openssl binary if -pbkdf2 and -iter support is added.

I'd note that yadm added the config option yadm.openssl-program for its basic built-in encryption (that option is currently a no-op for it's transcrypt support for obvious reasons). That was presumably for the same reason, although I don't see any explicit rational in the PR.

[jwatt]

Regarding my suggestions about adding cipher definition attributes to .gitattributes I'm still undecided about whether it really makes sense to do that, or whether it would be better to add explicit transcrypt version headers to the actual encrypted files

Could it just live in .git/config like the other settings in the [transcrypt] section, and be transported between repos via the command that's output by git transcrypt --display? That's perhaps simpler than having a per-file mechanism (and the possibility of files diverging?).

[jmurty]

That's a very good point. A single repo-wide cipher setting in the Git config's [transcrypt] section would indeed be much simpler than per-file settings.

Thinking through the repercussions, the possible downsides are:

1. All the repo's secret files would need to use the same cipher version settings (this is where the simplicity comes from).
2. Secret files would all need to be be upgraded (re-encrypted) to the newer cipher version — and maybe also downgraded? — when the user changes the repo-wide cipher version setting.
3. The cipher version settings are somewhat buried in one user's Git config, instead of being explicit in .gitattributes files. It may not be clear to other repo users when version settings are changed by another user, or what they have been changed to.

I think all these downsides are reasonable trade-offs for the much simpler approach of having a single repo-wide cipher version setting. For 2. the main challenge will be keeping the user experience clear and understandable, but that is a challenge regardless of how exactly cipher changes are implemented.

So overall 👍 to having a single repo-wide Get config setting like transcrypt.cipher-version, alongside the existing transcrypt.cipher and transcrypt.password settings, that is included in the transcrypt --display output.

Adding in a transcrypt.openssl-path setting also seems reasonable since the PATH problem affects others, not just me. This would need to be a user/system-specific setting though, and not be included in the transcrypt --display output.

[anvandaren]

Have you thought any more about a way forward for this issue?
I'm evaluating using transcrypt, and although the message is just a warning it would be easier to get others in my team to accept transcrypt if the message was not there.

[jmurty]

Since we haven't any progress on this issue in months or years, maybe it's time to consider a simple but brutal solution instead of anything that's more elegant but complicated and therefore keeps getting deferred?

The best way to unblock this could be to start a branch & PR working towards an update which would explicitly break compatibility with older OpenSSL versions (including the default that comes on Macs) for repos configured to use the recommended and default pbkdf2 command. This might need to be version 3, depending on how backwards-compatible it ends up being.

We already have the openssl-path config option in the main branch (set with transcrypt --set-openssl-path) which is a reasonable workaround for those on systems with outdated OpenSSL.

This leaves at least the following missing pieces:

• update encrypt/decrypt commands to use -pbkdf2 and -iter arguments when the cipher config is pbkdf2
• make sure existing repos can be upgraded to pbkdf2 easily with the --upgrade command
• detect old or incompatible OpenSSL versions and give useful tips for fixing it.
• maybe take the chance to make other breaking changes, like move away from MD5 for hashing?

[Erotemic]

I've been using my fork which blindly enables -pbkdf2 and uses SHA512 for hashing for 2 years now: https://github.com/Erotemic/transcrypt

Using MD5 and no -pbkdf2 is simply not secure in my use case. So from my perspective this repo (and the default on OSX) is broken without it.

@elasticdog

Is there a scenario you're imagining where the user would install a more modern version of openssl, but not want it to be their environment's default?

This could be the case for something like CUDA, not sure if it would be the case for openssl, but with the dodgy software practices used by some, I imaging it might be the case.

[andreineculau]

To put gas on fire - openssl 1.1.1 was released november 2018, libressl 2.9.1 in april 2019. If we go ahead requiring these versions, that's how new software we require.

[andersjel]

I've been using my fork which blindly enables -pbkdf2 and uses SHA512 for hashing for 2 years now: https://github.com/Erotemic/transcrypt

Using MD5 and no -pbkdf2 is simply not secure in my use case. So from my perspective this repo (and the default on OSX) is broken without it.

@elasticdog

Is there a scenario you're imagining where the user would install a more modern version of openssl, but not want it to be their environment's default?

This could be the case for something like CUDA, not sure if it would be the case for openssl, but with the dodgy software practices used by some, I imaging it might be the case.

Just a concern related to this:
Is the salt derived from the password stored in the repository? If so, could it be used to brute-force the password regardless of the used -pbkdf2 argument? I.e. could the attacker try different passwords to get the correct salt without applying the pbkdf2 function?

[Erotemic]

@andersjel the salt is determined via: salt=$("${openssl_path}" dgst -hmac "${filename}:${password}" -sha256 "$tempfile" | tr -d '\r\n' | tail -c16) which includes the filename, password, and the hash of the raw file itself. Then this is passed to openssl via -S $salt. So, I don't believe an attacker has easy access to this.

The end-to-end process works like this:

encrypted_filename=encrypted_secret.txt
filename=plaintext_secret.txt
echo "super duper secret" > $filename

password=12345
cipher=aes-256-cbc

# Encrypt with salt
salt=$(openssl dgst -hmac "${filename}:${password}" -sha512 "$filename" | tr -d '\r\n' | tail -c 16)
ENC_PASS=$password openssl enc -$cipher -md SHA512 -pass env:ENC_PASS -pbkdf2 -e -a -S "$salt" -in "$filename" > "$encrypted_filename"
cat "$encrypted_filename"

# Decrypt
ENC_PASS=$password openssl enc -$cipher -md SHA512 -pass env:ENC_PASS -pbkdf2 -d -a -in "$encrypted_filename"

If you can figure out a way to munge the value of ENC_PASS in the decrypt stage such that -pbkdf2 does not need to be passed, then that would be a vulnerability.

For reference, if you do know the salt you can run pbkdf2 standalone via:

sudo apt install nettle-bin
echo "$password" | nettle-pbkdf2 "$salt"

I think it is secure, but I won't make any strong claims.

[andersjel]

@Erotemic The issue is that the salt is prepended to the encrypted file:

$ ENC_PASS=test openssl enc -aes-256-cbc -in /dev/null -md SHA512 -pass env:ENC_PASS -pbkdf2 -e -a -S deadbeaf00000bad | base64 -d | hd
00000000  53 61 6c 74 65 64 5f 5f  de ad be af 00 00 0b ad  |Salted__........|
00000010  a5 00 38 f1 09 2e 22 84  08 dd 78 0e 1d 4a 7b 11  |..8..."...x..J{.|
00000020

So to brute-force the password, the attacker only has to do SHA512 HMAC operations until the attacker finds the correct salt, and not the much more expensive pbkdf2.

[jmurty]

Hi, this is an interesting discussion especially given the (slow) work in #126 to adopt PBKDF2 for a future update to Transcrypt.

I'm not a cryptographer so am out of my depth, but from @andersjel's code examples the use of the known filename + unknown password as inputs to generate the salt does seem like a potential problem.

What would be a better approach? Should/could the salt be generated from random data instead of "${filename}:${password}" e.g. with something like openssl rand -hex 20?

I found this discussion helpful: https://crypto.stackexchange.com/questions/3484/pbkdf2-and-salt

[jmurty]

Ah, random data won't work:

transcrypt/transcrypt

Lines 115 to 120 in fdf81c5

	# The `decryption -> encryption` process on an unchanged file must be
	# deterministic for everything to work transparently. To do that, the same
	# salt must be used each time we encrypt the same file. An HMAC has been
	# proven to be a PRF, so we generate an HMAC-SHA256 for each decrypted file
	# (keyed with a combination of the filename and transcrypt password), and
	# then use the last 16 bytes of that HMAC for the file's unique salt.

[jmurty]

I think the salt generation process is secure (enough) because the salt isn't derived from just "${filename}:${password}" it also hashes the contents of the plaintext file.

To brute-force the password an attacker would only need to do SHA512 HMAC operations, but they would need to guess both the password and the plaintext file contents. If the attacker can guess the plaintext, then they don't really need the password.

However, this does still leave the password vulnerable to brute-force attacks if the repository includes encrypted files with easily-guessable contents. The attacker would use the easily guessed file to reverse engineer the password, then use the password to access other unguessable encrypted files.

[LuisBL]

any news on this one ?

[jmurty]

Hi, we don't yet have a solution to support PBKDF2.

The most progress is in this PR, which is in turn based on other PRs and discussions: #136

But there are unresolved issues with incompatibility between different systems and/or versions of OpenSSL.

I do plan to take another go at it but can't make any guarantees about when I will be able to get to it.

[dawidmachon]

I'm coming back to see if there is any news for suppressing this warning in Windows env.

[andreineculau]

@jmurty @elasticdog 👋

we have this issue and #134 , and then 3 PRs #126 #132 #136 around openssl@3 and PBKDF2. I most probably miss context, but while I understand that we want to maintain backwards compatibility, we (I mean you! 🙏 ) are maintaining transcrypt for systems with 4-5 years old software, while providing a suboptimal experience for the rest that are running newer (that have had access to PBKDF2 for 4-5 years now).

At the same time, best practice states that you commit the transcrypt script in each transcrypted repo, in order to ensure encryption compatibility. That is to say: those that want their repos to be decrypted on systems running old software, can't depend on system-wide newer transcrypt versions.

It would be great to provide a timeline when enough is enough, and transcrypt v3 is released with breaking changes no matter what. For the time being, [reading] a 4 year old (#55 (comment))

I want to make sure that older versions also work properly

in a situation where the simplest changeset is adding one argument to 4 openssl calls (and yes, breaking backwards compatibility), sounds like parallel universes.

transcrypt is one of those "best thing since bread and butter", but onboarding experience is even critical to actually using it.

[jmurty]

Hi @andreineculau, @dawidmachon, @LuisBL, @andersjel and anyone else who might be interested in being a guinea pig, I'd appreciate help testing and proving the new PR #162 which adds support for PBKDF2.

It's early and rough, but I think there's a good chance this PR will move us towards a much-improved transcrypt version 3 before too much longer.

[dbarnett]

Is there any workaround to ack and suppress the warnings, short of all this complexity to fix the fundamental security smell openssl is warning about?

IIUC I could choose to just ignore the warning and everything would work fine, but it's constantly printing this junk warning output to the screen from most git commands, often more than once:

$ git stash
*** WARNING : deprecated key derivation used.
Using -iter or -pbkdf2 would be better.
*** WARNING : deprecated key derivation used.
Using -iter or -pbkdf2 would be better.
*** WARNING : deprecated key derivation used.
Using -iter or -pbkdf2 would be better.
*** WARNING : deprecated key derivation used.
Using -iter or -pbkdf2 would be better.
Saved working directory and index state WIP on main: 1798016 .local/bin: Install transcrypt

[jmurty]

Hi @dbarnett if you want to just suppress the warning messages the tweak in this branch should do the job.

I don't want to suppress these warnings in the official release however, on the theory that they will (eventually) guilt me into releasing full PBKDF2 support, and also annoy other users enough that they will help me test the support in PR #162.

[Edit: Updated commit link to point to branch instead, with a follow-on fix]

[dbarnett]

Thanks! I wasn't suggesting to categorically silence it, but I'd hoped transcrypt or openssl would have a way to ACK the warnings case-by-case.

It might still be worth pushing a modified version of that in the meantime that instead of outright swallowing the warning wraps it and points new users to context, like "warning is expected, configure xyz option to suppress, see issue #nn for details".

And there wouldn't at least be a way to roll together duplicates of the same warning from a single git command invocation, would there?

[Erotemic]

@jmurty It would be nice if we could come up with a concrete list of action items / requirements for #162

I love transcrypt, but I also feel uncomfortable recommending it due to its weaker KDF defaults. I'm not even sure pbkdf2 is enough, but argon2 support in openssl is not widespread, and I'm not sure when it will be.