From 7722606b369e569529ac30130f04abe17cae57cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Mon, 8 Jul 2019 11:32:38 +0100
Subject: [PATCH] rpc: ensure thread safe initialization of SASL library
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Neither the sasl_client_init or sasl_server_init methods are even
remotely threadsafe. They do a bunch of one-time initialization and
merely use a simple integer counter to avoid repeated work, not even
using atomic increment/reads on the counter. This can easily race in a
threaded program. Protect the calls using a virOnce initializer function
which is guaranteed threadsafe at least from libvirt's POV.

If the application using libvirt also uses another library that makes
use of SASL then the race still exists. It is impossible to fix that
fully except in SASL code itself.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/rpc/virnetsaslcontext.c | 54 ++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c
index f49340f033..62b103a428 100644
--- a/src/rpc/virnetsaslcontext.c
+++ b/src/rpc/virnetsaslcontext.c
@@ -77,22 +77,46 @@ VIR_ONCE_GLOBAL_INIT(virNetSASLContext);
 VIR_WARNINGS_NO_DEPRECATED
 #endif
 
-virNetSASLContextPtr virNetSASLContextNewClient(void)
+static int virNetSASLContextClientOnceInit(void)
 {
-    virNetSASLContextPtr ctxt;
-    int err;
-
-    if (virNetSASLContextInitialize() < 0)
-        return NULL;
-
-    err = sasl_client_init(NULL);
+    int err = sasl_client_init(NULL);
     if (err != SASL_OK) {
         virReportError(VIR_ERR_AUTH_FAILED,
                        _("failed to initialize SASL library: %d (%s)"),
                        err, sasl_errstring(err, NULL, NULL));
-        return NULL;
+        return -1;
     }
 
+    return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virNetSASLContextClient);
+
+
+static int virNetSASLContextServerOnceInit(void)
+{
+    int err = sasl_server_init(NULL, "libvirt");
+    if (err != SASL_OK) {
+        virReportError(VIR_ERR_AUTH_FAILED,
+                       _("failed to initialize SASL library: %d (%s)"),
+                       err, sasl_errstring(err, NULL, NULL));
+        return -1;
+    }
+
+    return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virNetSASLContextServer);
+
+
+virNetSASLContextPtr virNetSASLContextNewClient(void)
+{
+    virNetSASLContextPtr ctxt;
+
+    if (virNetSASLContextInitialize() < 0 ||
+        virNetSASLContextClientInitialize() < 0)
+        return NULL;
+
     if (!(ctxt = virObjectLockableNew(virNetSASLContextClass)))
         return NULL;
 
@@ -102,19 +126,11 @@ virNetSASLContextPtr virNetSASLContextNewClient(void)
 virNetSASLContextPtr virNetSASLContextNewServer(const char *const*usernameWhitelist)
 {
     virNetSASLContextPtr ctxt;
-    int err;
 
-    if (virNetSASLContextInitialize() < 0)
+    if (virNetSASLContextInitialize() < 0 ||
+        virNetSASLContextServerInitialize() < 0)
         return NULL;
 
-    err = sasl_server_init(NULL, "libvirt");
-    if (err != SASL_OK) {
-        virReportError(VIR_ERR_AUTH_FAILED,
-                       _("failed to initialize SASL library: %d (%s)"),
-                       err, sasl_errstring(err, NULL, NULL));
-        return NULL;
-    }
-
     if (!(ctxt = virObjectLockableNew(virNetSASLContextClass)))
         return NULL;