From 7d8fdf69c0f8d0f58318e9ee22bd23b6d8b8aa60 Mon Sep 17 00:00:00 2001
From: Jesper Louis Andersen <jesper.louis.andersen@gmail.com>
Date: Fri, 24 Jan 2020 14:48:21 +0100
Subject: [PATCH] Protect generichash by a mutex

While sodium is thread-safe, our
resources are not. Furthermore,
we might have an update call going
when someone decides to call
finalize and so on. It is not clever
to do so, but on the other hand
I want to protect against this.

While here, mark the mutexed
calls as dirty CPU. This avoids them
blocking the main scheduler and
only messes with the background
dirty threads, which is somewhat
more safe.

The consequence is that order
access to the resource is now
serialized. I don't think you should
do it, but it is now possible.
---
 CHANGELOG.md        |  2 +-
 c_src/enacl_nif.c   |  6 ++++--
 c_src/generichash.c | 16 ++++++++++++++++
 3 files changed, 21 insertions(+), 3 deletions(-)

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;
 }