drop pbkdf2 dependency - #61
Conversation
| HashedPass = crypto:pbkdf2_hmac( | ||
| HashFunction, BinaryPass, Salt, HashIterations, ?KEY_LENGTH), | ||
| HexPass = to_hex(HashedPass), | ||
| HexPass == HashedPassword. |
There was a problem hiding this comment.
This is fine, but I'm wondering if we should do something closer to compare_secure to avoid shortcut logic for timing attacks.
| HexPass = to_hex(HashedPass), | ||
| HexPass == HashedPassword. | ||
|
|
||
| to_hex(Data) when is_binary(Data) -> |
There was a problem hiding this comment.
I'm guessing this hex representation is equivalent to pbkdf2:to_hex so passwords from previous versions will also match.
There was a problem hiding this comment.
Yes, this to_hex/1 was copied from pbkdf2.erl verbatim.
To verify, I built riak with erlang-pbkdf2 temporarily re-enabled, and ran:
(dev1@127.0.0.1)10> {ok, Hash, pbkdf2, sha, Salt, Iters} = riak_core_pw_auth:hash_password(<<"fafa">>). %% uses native OTP crypto:pbkdf2_hmac/5
{ok,<<"3a05b03656286ed258fd0df5fd0ddad63e5e20ed">>,pbkdf2,
sha,
<<131,5,17,87,63,138,121,45,48,19,72,134,171,100,202,11>>,
65536}
(dev1@127.0.0.1)11> {ok, Hash2} = pbkdf2:pbkdf2(sha, <<"fafa">>, Salt, Iters).
{ok,<<58,5,176,54,86,40,110,210,88,253,13,245,253,13,218,
214,62,94,32,237>>}
(dev1@127.0.0.1)12> pbkdf2:to_hex(Hash2) == Hash.
true
|
More generally, while we are at it, do we want to expose salt length, digest and other items in riak.conf, to honor Jared Morrow's TODO remarks from 2013? |
|
No, I would say if we do that, it should be in advanced.config and should be a separate ticket from this one. |
2 participants
Since OTP-24.2, crypto app has
pbkdf2_hmac/5, thus allowing us to drop an external dependency on pbkdf2. See OpenRiak/erlang-pbkdf2#1 for more context.