Fix pwhash_str* functions.

The API for pwhash_str returns a cstring in the output buffer. These
are null terminated. However, we return the full buffer as a binary
back to Erlang. This means that we have a buffer with 0'es in the end.

The tests take this buffer and passes it back in as is. Hence all the
tests pass. However, it is conceivable that if we write said buffer to
disk somewhere, we are not going to write those 0's out.

When we then load the ASCII-armored Argon2 string into memory again,
it is not 0-terminated as a cstring should be, and this produces
errors all over the place.

The fix is twofold:

* Return the full buffer to Erlang, but use binary:split/2 to create a
  subbinary with the relevant part.
* Add a 0 in the end of ASCII Argon2 string before passing it to
  libsodium

Since we are looking at pwhashing, and Argon2, we expect the
computational problem to be memory bound. Thus, spending a bit more
work in memory is not going to have any considerable impact on the
speed of this system.
This commit is contained in:
Jesper Louis Andersen 2018-05-21 15:21:09 +02:00
parent e77aca8ecb
commit edd95498d1
3 changed files with 26 additions and 14 deletions

View File

@ -1327,7 +1327,7 @@ ERL_NIF_TERM enif_crypto_pwhash_str_verify(ErlNifEnv *env, int argc, ERL_NIF_TER
// Validate the arguments
if( (argc != 2) ||
(!enif_inspect_binary(env, argv[0], &h)) ||
(!enif_inspect_iolist_as_binary(env, argv[0], &h)) ||
(!enif_inspect_iolist_as_binary(env, argv[1], &p)) ) {
return enif_make_badarg(env);
}

View File

@ -721,17 +721,17 @@ pwhash_str_verify(PasswdHash, Passwd) ->
prop_pwhash_str_verify() ->
?FORALL({Passwd},
{?FAULT_RATE(1, 40, g_iodata())},
begin
case v_iodata(Passwd) of
true ->
{ok, Ascii} = enacl:pwhash_str(Passwd),
S = enacl:pwhash_str_verify(Ascii, Passwd),
equals(S, true);
false ->
badargs(fun() -> enacl:pwhash_str(Passwd) end),
badargs(fun() -> enacl:pwhash_str_verify("", Passwd) end)
end
end).
begin
case v_iodata(Passwd) of
true ->
{ok, Ascii} = enacl:pwhash_str(Passwd),
S = enacl:pwhash_str_verify(Ascii, Passwd),
equals(S, true);
false ->
badargs(fun() -> enacl:pwhash_str(Passwd) end),
badargs(fun() -> enacl:pwhash_str_verify("", Passwd) end)
end
end).
%% SUBTLE HASHING
%% ---------------------------

View File

@ -350,7 +350,19 @@ pwhash(Password, Salt) ->
%% @end
-spec pwhash_str(iodata()) -> {ok, iodata()} | {error, term()}.
pwhash_str(Password) ->
enacl_nif:crypto_pwhash_str(Password).
case enacl_nif:crypto_pwhash_str(Password) of
{ok, ASCII} ->
{ok, strip_null_terminate(ASCII)};
{error, Reason} ->
{error, Reason}
end.
strip_null_terminate(Binary) ->
[X, _] = binary:split(Binary, <<0>>),
X.
null_terminate(ASCII) ->
iolist_to_binary([ASCII, 0]).
%% @doc pwhash_str_verify/2 compares a password with a hash
%%
@ -359,7 +371,7 @@ pwhash_str(Password) ->
%% @end
-spec pwhash_str_verify(binary(), iodata()) -> boolean().
pwhash_str_verify(HashPassword, Password) ->
enacl_nif:crypto_pwhash_str_verify(HashPassword, Password).
enacl_nif:crypto_pwhash_str_verify(null_terminate(HashPassword), Password).
%% Public Key Crypto
%% ---------------------