Problem: calling randombytes_close with libsodium can crash Contexts in other threads (#4242)

* add opt-out for randombytes_close

Problem: randombytes_close is either a no-op or unsafe when a Context is running.

Unfortunately, there does not appear to be a single always correct choice,
so let builders pick between two not-great options.

Opting out can leak an FD on /dev/urandom which may need to be closed explicitly.
However, with the default behavior,
using multiple contexts with CURVE can crash with no application-level workaround available.

randombytes_close is not threadsafe and calling it while still in use by a Context can cause a crash.

For implementations using /dev/[u]random, this can leave up to one leftover FD per process.

The libsodium docs suggest that this function rarely needs to be called explicitly.

CMakeLists.txt

@@ -258,6 +258,7 @@ endif()
 
 option(WITH_LIBSODIUM "Use libsodium instead of built-in tweetnacl" ON)
 option(WITH_LIBSODIUM_STATIC "Use static libsodium library" OFF)
+option(ENABLE_LIBSODIUM_RANDOMBYTES_CLOSE "Automatically close libsodium randombytes. Not threadsafe without getrandom()" ON)
 option(ENABLE_CURVE "Enable CURVE security" ON)
 
 if(ENABLE_CURVE)
@@ -271,6 +272,9 @@ if(ENABLE_CURVE)
       endif()
       set(ZMQ_USE_LIBSODIUM 1)
       set(ZMQ_HAVE_CURVE 1)
+      if (ENABLE_LIBSODIUM_RANDOMBYTES_CLOSE)
+        set(ZMQ_LIBSODIUM_RANDOMBYTES_CLOSE 1)
+      endif()
     else()
       message(
         WARNING

configure.ac

@@ -550,6 +550,20 @@ AS_IF([test "x$with_libsodium" = "xyes"], [
 AC_ARG_ENABLE([curve],
     [AS_HELP_STRING([--disable-curve], [disable CURVE security [default=no]])])
 
+AC_ARG_ENABLE(
+    [libsodium_randombytes_close],
+    [AS_HELP_STRING(
+        [--disable-libsodium_randombytes_close],
+        [Do not call libsodium randombytes_close() when terminating contexts.
+        If disabled, may leave one FD open on /dev/urandom
+        until randombytes_close() is called explicitly,
+        but fixes a crash when multiple contexts are used with CURVE.
+        Has no effect when getrandom() is available. [default=enabled]]
+    )],
+    [],
+    [enable_libsodium_randombytes_close=yes]
+)
+
 if test "x$enable_curve" = "xno"; then
     curve_library=""
     AC_MSG_NOTICE([CURVE security is disabled])
@@ -558,6 +572,12 @@ elif test "x$with_libsodium" = "xyes"; then
     AC_MSG_NOTICE([Using libsodium for CURVE security])
     AC_DEFINE(ZMQ_HAVE_CURVE, [1], [Using curve encryption])
     AC_DEFINE(ZMQ_USE_LIBSODIUM, [1], [Using libsodium for curve encryption])
+    if test "x$enable_libsodium_randombytes_close" = "xyes"; then
+        AC_DEFINE(ZMQ_LIBSODIUM_RANDOMBYTES_CLOSE, [1], [Automatically close libsodium randombytes. Not threadsafe without getrandom()])
+    else
+        AC_MSG_NOTICE([Disabling libsodium randombytes_close(). randombytes_close() may need to be called in applcation code.])
+    fi
+
     curve_library="libsodium"
     enable_curve="yes"
 

src/random.cpp

@@ -151,8 +151,13 @@ static void manage_random (bool init_)
     if (init_) {
         int rc = sodium_init ();
         zmq_assert (rc != -1);
+#if defined(ZMQ_LIBSODIUM_RANDOMBYTES_CLOSE)
     } else {
+        // randombytes_close either a no-op or not threadsafe
+        // doing this without refcounting can cause crashes
+        // if called while a context is active
         randombytes_close ();
+#endif
     }
 #else
     LIBZMQ_UNUSED (init_);