From d7e83dd56943c1d034558dcc3c8fbd86f7c898ca Mon Sep 17 00:00:00 2001 From: Jesper Louis Andersen Date: Fri, 17 Jan 2020 16:24:51 +0100 Subject: [PATCH] Track outlen inside the generichash wrapper --- CHANGELOG.md | 3 +++ c_src/enacl_nif.c | 4 ++-- c_src/generichash.c | 55 +++++++++++++++------------------------------ src/enacl.erl | 8 +++---- src/enacl_nif.erl | 8 +++---- 5 files changed, 31 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58371f0..30e5f4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. we clean up correctly and that we don't accidentally mis-ref-count data. The code is a bit more goto heavy, but this style is surprisingly common in C code. - The code now rejects updates to generichash states which were already finalized. +- We now track the desired outlen of a generichash operation in the opaque NIF + resource rather than on the Erlang side. This avoids some checks in the code, + and streamlines a good deal of the interface. ### Fixes - Fix a resource leak in generichash/sign init/update/final. diff --git a/c_src/enacl_nif.c b/c_src/enacl_nif.c index ed4f310..22bf062 100644 --- a/c_src/enacl_nif.c +++ b/c_src/enacl_nif.c @@ -1929,8 +1929,8 @@ static ErlNifFunc nif_funcs[] = { enif_crypto_generichash_KEYBYTES_MAX}, {"crypto_generichash", 3, enacl_crypto_generichash}, {"crypto_generichash_init", 2, enacl_crypto_generichash_init}, - {"crypto_generichash_update", 3, enacl_crypto_generichash_update}, - {"crypto_generichash_final", 2, enacl_crypto_generichash_final} + {"crypto_generichash_update", 2, enacl_crypto_generichash_update}, + {"crypto_generichash_final", 1, enacl_crypto_generichash_final} }; diff --git a/c_src/generichash.c b/c_src/generichash.c index a8a6642..f422578 100644 --- a/c_src/generichash.c +++ b/c_src/generichash.c @@ -6,10 +6,9 @@ #include "generichash.h" typedef struct enacl_generichash_ctx { - // The hash state - crypto_generichash_state *ctx; - // Is the context alive? - int alive; + crypto_generichash_state *ctx; // Underlying hash state from sodium + int alive; // Is the context still valid for updates/finalizes? + int outlen; // Final size of the hash } enacl_generichash_ctx; static ErlNifResourceType *enacl_generic_hash_ctx_rtype; @@ -24,12 +23,9 @@ int enacl_init_generic_hash_ctx(ErlNifEnv *env) { ERL_NIF_RT_CREATE | ERL_NIF_RT_TAKEOVER, NULL); if (enacl_generic_hash_ctx_rtype == NULL) - goto err; + return 0; return 1; - -err: - return 0; } static void enacl_generic_hash_ctx_dtor(ErlNifEnv *env, @@ -38,7 +34,9 @@ static void enacl_generic_hash_ctx_dtor(ErlNifEnv *env, return; } - sodium_free(obj->ctx); + if (obj->ctx) + sodium_free(obj->ctx); + return; } @@ -179,25 +177,23 @@ ERL_NIF_TERM enacl_crypto_generichash_init(ErlNifEnv *env, int argc, // And also protects the resource via guardpages obj->ctx = NULL; obj->alive = 0; + obj->outlen = 0; + obj->ctx = (crypto_generichash_state *)sodium_malloc( crypto_generichash_statebytes()); if (obj->ctx == NULL) { goto err; } obj->alive = 1; + obj->outlen = hash_size; // Call the library function - if (0 != crypto_generichash_init(obj->ctx, k, key.size, hash_size)) { + if (0 != crypto_generichash_init(obj->ctx, k, key.size, obj->outlen)) { ret = nacl_error_tuple(env, "hash_init_error"); goto err; } - // Create return values - ERL_NIF_TERM e1 = enif_make_atom(env, "hashstate"); - ERL_NIF_TERM e2 = argv[0]; - ERL_NIF_TERM e3 = enif_make_resource(env, obj); - - ret = enif_make_tuple3(env, e1, e2, e3); + ret = enif_make_resource(env, obj); goto done; bad_arg: return enif_make_badarg(env); @@ -225,15 +221,13 @@ ERL_NIF_TERM enacl_crypto_generichash_update(ErlNifEnv *env, int argc, enacl_generichash_ctx *obj = NULL; // Validate the arguments - if (argc != 3) + if (argc != 2) goto bad_arg; - if (!enif_get_uint(env, argv[0], &data_size)) - goto bad_arg; - if (!enif_get_resource(env, argv[1], + if (!enif_get_resource(env, argv[0], (ErlNifResourceType *)enacl_generic_hash_ctx_rtype, (void **)&obj)) goto bad_arg; - if (!enif_inspect_binary(env, argv[2], &data)) + if (!enif_inspect_binary(env, argv[1], &data)) goto bad_arg; if (!obj->alive) { @@ -247,11 +241,7 @@ ERL_NIF_TERM enacl_crypto_generichash_update(ErlNifEnv *env, int argc, goto done; } - ERL_NIF_TERM e1 = enif_make_atom(env, "hashstate"); - ERL_NIF_TERM e2 = argv[0]; - ERL_NIF_TERM e3 = argv[1]; - - ret = enif_make_tuple3(env, e1, e2, e3); + ret = argv[0]; goto done; bad_arg: @@ -264,12 +254,9 @@ ERL_NIF_TERM enacl_crypto_generichash_final(ErlNifEnv *env, int argc, ERL_NIF_TERM const argv[]) { ERL_NIF_TERM ret; ErlNifBinary hash; - unsigned int hash_size; enacl_generichash_ctx *obj = NULL; - if (argc != 2) - goto bad_arg; - if (!enif_get_uint(env, argv[0], &hash_size)) + if (argc != 1) goto bad_arg; if (!enif_get_resource(env, argv[1], enacl_generic_hash_ctx_rtype, (void **)&obj)) @@ -280,13 +267,7 @@ ERL_NIF_TERM enacl_crypto_generichash_final(ErlNifEnv *env, int argc, goto done; } - if ((hash_size <= crypto_generichash_BYTES_MIN) || - (hash_size >= crypto_generichash_BYTES_MAX)) { - ret = nacl_error_tuple(env, "invalid_hash_size"); - goto done; - } - - if (!enif_alloc_binary(hash_size, &hash)) { + if (!enif_alloc_binary(obj->outlen, &hash)) { ret = nacl_error_tuple(env, "alloc_failed"); goto done; } diff --git a/src/enacl.erl b/src/enacl.erl index 3a9daf8..d2ab386 100644 --- a/src/enacl.erl +++ b/src/enacl.erl @@ -354,11 +354,11 @@ generichash(HashSize, Message) -> generichash_init(HashSize, Key) -> enacl_nif:crypto_generichash_init(HashSize, Key). -generichash_update({hashstate, HashSize, HashState}, Message) -> - enacl_nif:crypto_generichash_update(HashSize, HashState, Message). +generichash_update(State, Message) -> + enacl_nif:crypto_generichash_update(State, Message). -generichash_final({hashstate, HashSize, HashState}) -> - enacl_nif:crypto_generichash_final(HashSize, HashState). +generichash_final(State) -> + enacl_nif:crypto_generichash_final(State). -type pwhash_limit() :: interactive | moderate | sensitive | pos_integer(). diff --git a/src/enacl_nif.erl b/src/enacl_nif.erl index b971c7f..a6d6ec5 100644 --- a/src/enacl_nif.erl +++ b/src/enacl_nif.erl @@ -162,8 +162,8 @@ crypto_generichash_KEYBYTES_MAX/0, crypto_generichash/3, crypto_generichash_init/2, - crypto_generichash_update/3, - crypto_generichash_final/2 + crypto_generichash_update/2, + crypto_generichash_final/1 ]). %% Access to the RNG @@ -202,8 +202,8 @@ crypto_generichash_KEYBYTES_MAX() -> erlang:nif_error(nif_not_loaded). crypto_generichash(_HashSize, _Message, _Key) -> erlang:nif_error(nif_not_loaded). crypto_generichash_init(_HashSize, _Key) -> erlang:nif_error(nif_not_loaded). -crypto_generichash_update(_HashSize, _HashState, _Message) -> erlang:nif_error(nif_not_loaded). -crypto_generichash_final(_HashSize, _HashState) -> erlang:nif_error(nif_not_loaded). +crypto_generichash_update(_HashState, _Message) -> erlang:nif_error(nif_not_loaded). +crypto_generichash_final(_HashState) -> erlang:nif_error(nif_not_loaded). crypto_pwhash(_Password, _Salt, _Ops, _Mem) -> erlang:nif_error(nif_not_loaded). crypto_pwhash_str(_Password, _Ops, _Mem) -> erlang:nif_error(nif_not_loaded).