search

hjanuschka / fastlane-plugin-cryptex

MIT License
36 stars 16 forks source link

Add digest as parameter to crypt routines #10

Closed nickgsc closed 6 years ago

nickgsc commented 6 years ago

Fixes #9

hjanuschka commented 6 years ago

hey @nickgsc 🙌

that loooks great, however, is md5 not supported et-all anymore or is it just not the default anymore?

if it is safe to rely on its availble, i would prefer to use md5 as a default, to not break existing cryptex repo's, or do i get it wrong?

implemenation is great, just a matter of default sha256 (which would be better, i agree) vs md5

nickgsc commented 6 years ago

I had that thought too. I suppose defaulting it to md5 would be better for backwards compatibility of existing repos. It is definitely still a supported digest, it's just no longer the default on OpenSSL 1.1 (when it was on older versions of OpenSSL).

For reference, here are the available digests I was able to identify in the environments I have at my disposal right now:

OpenSSL 1.1.0f (on Debian Jessie):

Message Digest commands (see the `dgst' command for more details)
blake2b512        blake2s256        gost              md4
md5               rmd160            sha1              sha224
sha256            sha384            sha512

LibreSSL 2.2.7 (MacOS High Sierra):

Message Digest commands (see the `dgst' command for more details)
gost-mac          md4               md5               md_gost94
ripemd160         sha               sha1              sha224
sha256            sha384            sha512            streebog256
streebog512       whirlpool

OpenSSL 1.0.2o (MacOS via Brew):

Message Digest commands (see the `dgst' command for more details)
md4               md5               mdc2              rmd160
sha               sha1
hjanuschka commented 6 years ago

awesome - many thx for your work ❤️ and your top-notch explanation 💯 going to merge it, and push a new release soon

hjanuschka commented 6 years ago

hey @nickgsc i backported your changes to https://github.com/fastlane/fastlane/pull/12390 match - where we forked cryptex initially

hjanuschka commented 6 years ago

ohhh i see it already was there :( my fault.

nickgsc commented 6 years ago

I didn't realize that this had been forked from match, so I didn't even check that repo for a solution. I just rolled the fix directly here.

In any case, glad we have parity on that now between the two!