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 Argon2 password hashing implementation to Standard Library #7676

Closed
elaine-jackson opened this issue Apr 16, 2019 · 10 comments
Closed

Add Argon2 password hashing implementation to Standard Library #7676

elaine-jackson opened this issue Apr 16, 2019 · 10 comments

Comments

@elaine-jackson
Copy link

The standard library for Crystal includes a few cryptographic functions including password hashing using the Bcrypt algorithm (See: https://github.com/crystal-lang/crystal/blob/60760a5460aa13a1f80c5a1a7410782dc4076b77/src/crypto/bcrypt.cr) this is very useful. The algorithm Argon2 (https://github.com/P-H-C/phc-winner-argon2) won the password hashing competition which was intended to find a successor to Bcrypt. Not that there is anything wrong with using Bcrypt but Argon2 builds upon what we learned with Bcrypt and makes password hashing even better for everyone. What do we think about adding Argon2 into the Crypto part of the Crystal Standard Library?

@asterite
Copy link
Member

I think it should be in a shard. The code won't be written by us so we won't have a way to maintain it in case the original maintainer disappears or has no time.

@j8r
Copy link
Contributor

j8r commented Apr 16, 2019

Having arbitrary algorithms in the stdlib because some particular core members have implemented them and take the responsibility to maintain them (forever?) is a bit flawed. What happens (s)he has no time to maintain it, and/or if there are no longer core members that have enough knowledge of this very algorithms?

I think this shouldn't be the responsibility of the core team to maintain Crypto, at all. This can have serious security implications and require a deep, specific understanding of how this algorithms works.
And they are quite simple compared to OpenSSL, which appears to be maintenance-heavy, as @RX14 point out

Rust has chosen to not support crypto.
On the other hand, Go has https://github.com/golang/crypto, but has the Google company behind.
Ruby uses only OpenSSL.

There is already https://github.com/ysbaddaden/scrypt-crystal, why won't each crypto algorithm owned by its implementer?

@ysbaddaden
Copy link
Contributor

Before even considering inclusion into stdlib there should at least be a working shard.

Also, bcrypt may be moved out, so it's kinda going circles to discuss to include an alternative.

Note: this has nothing to do with Argon2 which is an excellent password hashing toolset.

@j8r
Copy link
Contributor

j8r commented Apr 16, 2019

Another alternative is to work/wait for openssl/openssl#4091

@elaine-jackson
Copy link
Author

elaine-jackson commented Apr 16, 2019

I think it should be in a shard. The code won't be written by us so we won't have a way to maintain it in case the original maintainer disappears or has no time.

Understandable but you could say the same about any feature in the standard library. Should we stop maintaining HTTP::Client because one day the maintainer might quit?

Before even considering inclusion into stdlib there should at least be a working shard.

Also, bcrypt may be moved out, so it's kinda going circles to discuss to include an alternative.

Note: this has nothing to do with Argon2 which is an excellent password hashing toolset.

This worries me. Amber (https://amberframework.org) currently uses Bcrypt from the standard library for authentication. Please don't remove features that are in widespread use from the standard library.

Another alternative is to work/wait for openssl/openssl#4091

Honestly that may be a better idea that'll save the Crystal team time and effort otherwise needed to maintain our own implementation.

Having arbitrary algorithms in the stdlib because some particular core members have implemented them and take the responsibility to maintain them (forever?) is a bit flawed. What happens (s)he has no time to maintain it, and/or if there are no longer core members that have enough knowledge of this very algorithms?

That's a problem I'm not sure about the solution. I go back to should we remove HTTP::Client once the maintainers for it are gone.

I think this shouldn't be the responsibility of the core team to maintain Crypto, at all. This can have serious security implications and require a deep, specific understanding of how this algorithms works.

I agree the security implications are severe. Once you add something to the standard library it's problematic to remove as so many things will rely on it.

And they are quite simple compared to OpenSSL, which appears to be maintenance-heavy, as @RX14 point out

Rust has chosen to not support crypto.

Maybe like them we should offload to OpenSSL

On the other hand, Go has https://github.com/golang/crypto, but has the Google company behind.
Like you said Go has a huge corporate developer team backing it. Crystal does not.

Ruby uses only OpenSSL.

We're Ruby like, qw might as well copy the using OpenSSL part too 😛

There is already https://github.com/ysbaddaden/scrypt-crystal, why won't each crypto algorithm owned by its implementer?

Goodpoint. But including it in standard library would make it easier than having to find the name of and add the shard. Individuals could still maintain as part of the standard library. Is this the right approach?

@j8r
Copy link
Contributor

j8r commented Apr 16, 2019

As @ysbaddaden said, this issue can be considered stalled, waiting a Crystal implementation or an OpenSSL API before considering to include it to the stdlib - there is nothing yet!
For the time being, those interested can also create bindings for the official reference implementation.

@Sija
Copy link
Contributor

Sija commented Apr 16, 2019

Another alternative is to work/wait for openssl/openssl#4091

Honestly that may be a better idea that'll save the Crystal team time and effort otherwise needed to maintain our own implementation.

Given how's ecosystem fragmented, I'd say that waiting for openssl support is a road paved with frustration. Even if they'd release it tmrw (and they're not), it would become usable on major OS distributions after months, or years (what's visible for example in case of building nginx with TLSv1.3 support, which requires openssl 1.1.1, on currently newest, debian 9).

IMO building a shard, which might get sucked into the stdlib after some grace period sounds as the most sensible and realistic scenario to move things forward.

@konovod
Copy link
Contributor

konovod commented Apr 16, 2019

If someone desperately needs Argon2i, monocypher has it's implementation (with zero dependencies) and there are bindings for it.

@asterite
Copy link
Member

Understandable but you could say the same about any feature in the standard library. Should we stop maintaining HTTP::Client because one day the maintainer might quit?

Yes, I'm now in the opinion that things that are not core to the language should go into shards and be maintained by core team members plus community, or just community. You can see that HTTP::Client and HTTP::Server are already lacking some features and things don't move forward, and this is mainly because the core team member doesn't have the time and resources to tackle all of it. Move most things to shards and the community as a whole can evolve them.

@ysbaddaden
Copy link
Contributor

I'm closing. Please implement Argon2 and Blake2 as pure Crystal, or create a libsodium bindings (forget libssl). In any case, this should be a shard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants