technion / ruby-argon2

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

Cannot specify memory costs that aren't `2^N` #63

Closed f3ndot closed 1 year ago

f3ndot commented 1 year ago

The current OWASP guidance recommends memory costs (46 MiB, 19 MiB, 12 MiB, 9 MiB, or 7 MiB) that are not expressible in the current notation of m_cost being the N that 2 can be raised to the power of. This is troublesome for those who want to adhere to a recommended memory cost expressed as an absolute value (eg 9,216 KiB).

I understand this lib is adhering to the reference implementation, thinly exposing the m_cost param of 2^N, but perhaps for the sake of greater compatibility with continually evolving recommended values, the m_cost: option should be an absolute value expressed in KiB? This is consistent with Python's argon2-cffi.

Previously:

Argon2::Password.create('password', {m_cost: 16})
    # => "$argon2id$v=19$m=65536,t=2,p=1$pgUSydEmCXecEWFkz2P9iw$8iB2ipvGkQXyuvoJYDz5Ppwy7V2WSEdF7+/XlZYRYAY"

Suggested:

Argon2::Password.create('password', {m_cost: 64 * 1024})
    # => "$argon2id$v=19$m=65536,t=2,p=1$pgUSydEmCXecEWFkz2P9iw$8iB2ipvGkQXyuvoJYDz5Ppwy7V2WSEdF7+/XlZYRYAY"
f3ndot commented 1 year ago

Alternatively exposing maybe a m_cost_literal alongside for backwards compatibility.

Dovetailed with https://github.com/technion/ruby-argon2/pull/62 additional profile costs for the current OWASP values can be added for maximum flexibility.

technion commented 1 year ago

Thanks for raising this issue. I've been thinking about how to address this. I'm aware of OWASP's guidance on the matter, but I'm also aware the PR you recently had merged allows several definitions of "standard" values to be profiled. I don't currently feel there's a heavy need to use a value outside this range. I agree in hindsight it should have been coded this way, but it's a fairly drastic change and departs from, for example, the way bcrypt works with work orders which was a heavy inspiration at a time when I don't believe anyone was trying to write Argon2 bindings:

https://github.com/bcrypt-ruby/bcrypt-ruby/blob/master/README.md?plain=1#L87

If there's a heavier push in future I'll reevaluate, but I'll close this off for now.