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

Add digest as parameter to crypt routines #10

Merged
merged 3 commits into from Apr 25, 2018

Conversation

nickgsc
Copy link
Contributor

@nickgsc nickgsc commented Apr 24, 2018

Fixes #9

@hjanuschka
Copy link
Owner

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
Copy link
Contributor Author

nickgsc commented Apr 25, 2018

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
Copy link
Owner

hjanuschka commented Apr 25, 2018

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

@hjanuschka
Copy link
Owner

hey @nickgsc i backported your changes to fastlane/fastlane#12390 match - where we forked cryptex initially

@hjanuschka
Copy link
Owner

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

@nickgsc
Copy link
Contributor Author

nickgsc commented Apr 26, 2018

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!

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.

None yet

2 participants