diff --git a/CHANGELOG.md b/CHANGELOG.md index d104d59..5096bda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,6 @@ Under errors, the current code can leak binaries. I don't want that to happen, s are going to use a better cleanup scheme in the code base. - Play the mutex game with: - - Generic Hash multi-part types - Sign multi-part types The resource-variant structs need to be mutex-protected. Otherwise we run the risk of @@ -81,6 +80,7 @@ sections applied. The current code is subtly in error! - Clang static analysis warnings (Thomas Arts). - Replace a constant 31 with a computation from libsodium (Thomas Arts, from a security review). - Some subtle memory leaks in the error path for kx operations were plugged. +- The multi-part generichash interface is now properly process/thread safe. ## [0.17.2] diff --git a/c_src/enacl_nif.c b/c_src/enacl_nif.c index 5e33df3..6993db9 100644 --- a/c_src/enacl_nif.c +++ b/c_src/enacl_nif.c @@ -445,8 +445,10 @@ static ErlNifFunc nif_funcs[] = { enacl_crypto_generichash_KEYBYTES_MAX}, {"crypto_generichash", 3, enacl_crypto_generichash}, {"crypto_generichash_init", 2, enacl_crypto_generichash_init}, - {"crypto_generichash_update", 2, enacl_crypto_generichash_update}, - {"crypto_generichash_final", 1, enacl_crypto_generichash_final} + erl_nif_dirty_job_cpu_bound_macro("crypto_generichash_update", 2, + enacl_crypto_generichash_update), + erl_nif_dirty_job_cpu_bound_macro("crypto_generichash_final", 1, + enacl_crypto_generichash_final) }; diff --git a/c_src/generichash.c b/c_src/generichash.c index 5909ad6..222dd82 100644 --- a/c_src/generichash.c +++ b/c_src/generichash.c @@ -6,9 +6,11 @@ #include "generichash.h" typedef struct enacl_generichash_ctx { + ErlNifMutex *mtx; 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; @@ -37,6 +39,9 @@ static void enacl_generic_hash_ctx_dtor(ErlNifEnv *env, if (obj->ctx) sodium_free(obj->ctx); + if (obj->mtx != NULL) + enif_mutex_destroy(obj->mtx); + return; } @@ -175,6 +180,7 @@ ERL_NIF_TERM enacl_crypto_generichash_init(ErlNifEnv *env, int argc, // Allocate the state context via libsodium // Note that this ensures a 64byte alignment for the resource // And also protects the resource via guardpages + obj->mtx = NULL; obj->ctx = NULL; obj->alive = 0; obj->outlen = 0; @@ -187,6 +193,11 @@ ERL_NIF_TERM enacl_crypto_generichash_init(ErlNifEnv *env, int argc, obj->alive = 1; obj->outlen = hash_size; + if ((obj->mtx = enif_mutex_create("enacl.generichash")) == NULL) { + ret = enacl_error_tuple(env, "generichash_mutex_error"); + goto err; + } + // Call the library function if (0 != crypto_generichash_init(obj->ctx, k, key.size, obj->outlen)) { ret = enacl_error_tuple(env, "hash_init_error"); @@ -230,6 +241,8 @@ ERL_NIF_TERM enacl_crypto_generichash_update(ErlNifEnv *env, int argc, if (!enif_inspect_binary(env, argv[1], &data)) goto bad_arg; + enif_mutex_lock(obj->mtx); + if (!obj->alive) { ret = enacl_error_tuple(env, "finalized"); goto done; @@ -247,6 +260,7 @@ ERL_NIF_TERM enacl_crypto_generichash_update(ErlNifEnv *env, int argc, bad_arg: return enif_make_badarg(env); done: + enif_mutex_unlock(obj->mtx); return ret; } @@ -262,6 +276,7 @@ ERL_NIF_TERM enacl_crypto_generichash_final(ErlNifEnv *env, int argc, (void **)&obj)) goto bad_arg; + enif_mutex_lock(obj->mtx); if (!obj->alive) { ret = enacl_error_tuple(env, "finalized"); goto done; @@ -293,5 +308,6 @@ bad_arg: release: enif_release_binary(&hash); done: + enif_mutex_unlock(obj->mtx); return ret; }