search

technion / ruby-argon2

A Ruby gem offering bindings for Argon2 password hashing
MIT License
229 stars 30 forks source link

Argon2id binding #24

Closed midnight-wonderer closed 5 years ago

midnight-wonderer commented 6 years ago

Quote from the IETF draft.
https://tools.ietf.org/html/draft-irtf-cfrg-argon2-03#section-9.4

Recommendations The Argon2id variant with t=1 and maximum available memory is recommended as a default setting for all environments. This setting is secure against side-channel attacks and maximizes adversarial costs on dedicated brute-force hardware.

I wonder if we can get the variant to be the default of ruby-argon2 too?

technion commented 6 years ago

Definitely a good idea. It's probably a better default based on the above - which is a change from the earlier recommendation.

Give me a bit of time to work through this.

technion commented 6 years ago

Plot twist: The reference C library has no tests for Argon 2id. Let me get a PR for that first.

technion commented 5 years ago

@MidnightWonderer Just updating to note I've sent https://github.com/P-H-C/phc-winner-argon2/pull/261 in to address the upstream issue, waiting on merge or feedback.

technion commented 5 years ago

@midnight-wonderer Thanks for your patience with this. As you may have seen, my PR has been taken upstream.

I've pushed some commits to integrate support for 2id in our C bindings, and I'll be integrating verify support soon. Changing the actual default will take a major version bump, but we'll get there.

ankane commented 5 years ago

Thanks for all the updates @technion 👍 I'm excited to see this as well (and may make it the default for blind index in the future).

midnight-wonderer commented 5 years ago

@ankane are you planning to upgrade from argon2i to argon2id for your indexes?
Does this actually a bad thing for indexing use cases?

ankane commented 5 years ago

@midnight-wonderer Planning to add a new algorithm type for it (to mirror CipherSweet blind indexes). What are you thinking?

technion commented 5 years ago

I have tagged v1.2.0 which now supports verifying Argon2id hashes.

I will shift the default in master shortly but I'll take my time with tagging 2.0 and ensure it's correct.

ankane commented 5 years ago

Thanks @technion 💯 Are there plans to add support for creating Argon2id hashes (hash_argon2id)?

midnight-wonderer commented 5 years ago

@ankane I think that changing the algorithm would have the same result as changing salt (or key).
Not many people would decide to do it in their production environment.

This will be a bit off topic but just my 2 cents:
The indexing is a misuse of Argon2 in the first place IMHO. (There are parameters apart from salt stored in the encrypted data. e.g., algorithm version)
Encrypting just the password with Argon2 with a proper randomized salt, and never index any password, is what I would recommend.

ankane commented 5 years ago

I appreciate the feedback.

  1. As you mention, current users would need to rotate the blind index to use the new algorithm, but I think it's a good choice for new users.

  2. Two common use cases for Argon2 are password storage and key derivation. Blind indexing uses the second. https://libsodium.gitbook.io/doc/password_hashing/the_argon2i_function

technion commented 5 years ago

@ankane absolutely, I'll be making Argon2id the default in 2.0.

technion commented 5 years ago

Update: master now hashes to Argon2_id, whilst maintaining full backward compat. This feature needs better tests before its'fully baked.

ankane commented 5 years ago

@technion Awesome! I think it'd be good to have a separate function like hash_argon2id_encode so Argon2i can still be used when desired.

technion commented 5 years ago

@ankane I've had a goal here of not giving someone enough rope to hang themselves.

I certainly appreciate the need to support verifying all formats, but I can refer to the amount of in production code I've been told apparently uses the _salt_do_not_supply parameter in describing why just following current best practice is a better proposal.

Edit: To be clear though, this is a change that will necessitate a v2.0 version, and I'll keep 1.x supported as long as feasible. So if this matters, there is that option.

ankane commented 5 years ago

I completely agree that cryptography libraries should be designed to be hard to misuse, but don't think this falls into that category.

Regardless, thanks for adding Argon2id support 🎉

technion commented 5 years ago

I'll close this off as master has covered everything I believe is necessary. I'll tag 2.0 shortly for release.