obj: add locking to the OBJ sigid calls

This is done using a single global lock.  The premise for this is that new
objects will most frequently be added at start up and never added subsequently.
Thus, the locking will be for read most of the time.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15713)

diff --git a/crypto/objects/obj_xref.c b/crypto/objects/obj_xref.c
index da1035112f4f06feffa5f6b1df6950500269d40f..3a6ae02bf02c8239ddc04accbf4a9064751ff035 100644 (file)
--- a/crypto/objects/obj_xref.c
+++ b/crypto/objects/obj_xref.c
@@ -10,9 +10,11 @@
 #include <openssl/objects.h>
 #include "obj_xref.h"
 #include "internal/nelem.h"
+#include "internal/thread_once.h"
 #include <openssl/err.h>
 
 static STACK_OF(nid_triple) *sig_app, *sigx_app;
+static CRYPTO_RWLOCK *sig_lock;
 
 static int sig_cmp(const nid_triple *a, const nid_triple *b)
 {
@@ -32,62 +34,104 @@ DECLARE_OBJ_BSEARCH_CMP_FN(const nid_triple *, const nid_triple *, sigx);
 static int sigx_cmp(const nid_triple *const *a, const nid_triple *const *b)
 {
     int ret;
+
     ret = (*a)->hash_id - (*b)->hash_id;
-    if (ret)
+    if (ret != 0)
         return ret;
     return (*a)->pkey_id - (*b)->pkey_id;
 }
 
 IMPLEMENT_OBJ_BSEARCH_CMP_FN(const nid_triple *, const nid_triple *, sigx);
 
-int OBJ_find_sigid_algs(int signid, int *pdig_nid, int *ppkey_nid)
+static CRYPTO_ONCE sig_init = CRYPTO_ONCE_STATIC_INIT;
+
+DEFINE_RUN_ONCE_STATIC(o_sig_init)
+{
+    sig_lock = CRYPTO_THREAD_lock_new();
+    return sig_lock != NULL;
+}
+
+static ossl_inline int obj_sig_init(void)
+{
+    return RUN_ONCE(&sig_init, o_sig_init);
+}
+
+static int ossl_obj_find_sigid_algs(int signid, int *pdig_nid, int *ppkey_nid,
+                                    int lock)
 {
     nid_triple tmp;
-    const nid_triple *rv = NULL;
-    tmp.sign_id = signid;
+    const nid_triple *rv;
+    int idx;
 
-    if (sig_app != NULL) {
-        int idx = sk_nid_triple_find(sig_app, &tmp);
-        rv = sk_nid_triple_value(sig_app, idx);
-    }
-#ifndef OBJ_XREF_TEST2
+    if (signid == NID_undef)
+        return 0;
+
+    tmp.sign_id = signid;
+    rv = OBJ_bsearch_sig(&tmp, sigoid_srt, OSSL_NELEM(sigoid_srt));
     if (rv == NULL) {
-        rv = OBJ_bsearch_sig(&tmp, sigoid_srt, OSSL_NELEM(sigoid_srt));
+        if (!obj_sig_init())
+            return 0;
+        if (lock && !CRYPTO_THREAD_read_lock(sig_lock)) {
+            ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_READ_LOCK);
+            return 0;
+        }
+        if (sig_app != NULL) {
+            idx = sk_nid_triple_find(sig_app, &tmp);
+            if (idx >= 0)
+                rv = sk_nid_triple_value(sig_app, idx);
+        }
+        if (lock)
+            CRYPTO_THREAD_unlock(sig_lock);
+        if (rv == NULL)
+            return 0;
     }
-#endif
-    if (rv == NULL)
-        return 0;
-    if (pdig_nid)
+
+    if (pdig_nid != NULL)
         *pdig_nid = rv->hash_id;
-    if (ppkey_nid)
+    if (ppkey_nid != NULL)
         *ppkey_nid = rv->pkey_id;
     return 1;
 }
 
+int OBJ_find_sigid_algs(int signid, int *pdig_nid, int *ppkey_nid)
+{
+    return ossl_obj_find_sigid_algs(signid, pdig_nid, ppkey_nid, 1);
+}
+
 int OBJ_find_sigid_by_algs(int *psignid, int dig_nid, int pkey_nid)
 {
     nid_triple tmp;
     const nid_triple *t = &tmp;
-    const nid_triple **rv = NULL;
+    const nid_triple **rv;
+    int idx;
+
+    if (dig_nid == NID_undef || pkey_nid == NID_undef)
+        return 0;
 
     tmp.hash_id = dig_nid;
     tmp.pkey_id = pkey_nid;
 
-    if (sigx_app) {
-        int idx = sk_nid_triple_find(sigx_app, &tmp);
-        if (idx >= 0) {
-            t = sk_nid_triple_value(sigx_app, idx);
-            rv = &t;
-        }
-    }
-#ifndef OBJ_XREF_TEST2
+    rv = OBJ_bsearch_sigx(&t, sigoid_srt_xref, OSSL_NELEM(sigoid_srt_xref));
     if (rv == NULL) {
-        rv = OBJ_bsearch_sigx(&t, sigoid_srt_xref, OSSL_NELEM(sigoid_srt_xref));
+        if (!obj_sig_init())
+            return 0;
+        if (!CRYPTO_THREAD_read_lock(sig_lock)) {
+            ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_READ_LOCK);
+            return 0;
+        }
+        if (sigx_app != NULL) {
+            idx = sk_nid_triple_find(sigx_app, &tmp);
+            if (idx >= 0) {
+                t = sk_nid_triple_value(sigx_app, idx);
+                rv = &t;
+            }
+        }
+        CRYPTO_THREAD_unlock(sig_lock);
+        if (rv == NULL)
+            return 0;
     }
-#endif
-    if (rv == NULL)
-        return 0;
-    if (psignid)
+
+    if (psignid != NULL)
         *psignid = (*rv)->sign_id;
     return 1;
 }
@@ -95,14 +139,14 @@ int OBJ_find_sigid_by_algs(int *psignid, int dig_nid, int pkey_nid)
 int OBJ_add_sigid(int signid, int dig_id, int pkey_id)
 {
     nid_triple *ntr;
-    if (sig_app == NULL)
-        sig_app = sk_nid_triple_new(sig_sk_cmp);
-    if (sig_app == NULL)
+    int dnid = NID_undef, pnid = NID_undef, ret = 0;
+
+    if (signid == NID_undef || dig_id == NID_undef || pkey_id == NID_undef)
         return 0;
-    if (sigx_app == NULL)
-        sigx_app = sk_nid_triple_new(sigx_cmp);
-    if (sigx_app == NULL)
+
+    if (!obj_sig_init())
         return 0;
+
     if ((ntr = OPENSSL_malloc(sizeof(*ntr))) == NULL) {
         ERR_raise(ERR_LIB_OBJ, ERR_R_MALLOC_FAILURE);
         return 0;
@@ -111,18 +155,49 @@ int OBJ_add_sigid(int signid, int dig_id, int pkey_id)
     ntr->hash_id = dig_id;
     ntr->pkey_id = pkey_id;
 
-    if (!sk_nid_triple_push(sig_app, ntr)) {
+    if (!CRYPTO_THREAD_write_lock(sig_lock)) {
+        ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_WRITE_LOCK);
         OPENSSL_free(ntr);
         return 0;
     }
 
-    if (!sk_nid_triple_push(sigx_app, ntr))
-        return 0;
+    /* Check that the entry doesn't exist or exists as desired */
+    if (ossl_obj_find_sigid_algs(signid, &dnid, &pnid, 0)) {
+        ret = dnid == dig_id && pnid == pkey_id;
+        goto err;
+    }
+
+    if (sig_app == NULL) {
+        sig_app = sk_nid_triple_new(sig_sk_cmp);
+        if (sig_app == NULL)
+            goto err;
+    }
+    if (sigx_app == NULL) {
+        sigx_app = sk_nid_triple_new(sigx_cmp);
+        if (sigx_app == NULL)
+            goto err;
+    }
+
+    /*
+     * Better might be to find where to insert the element and insert it there.
+     * This would avoid the sorting steps below.
+     */
+    if (!sk_nid_triple_push(sig_app, ntr))
+        goto err;
+    if (!sk_nid_triple_push(sigx_app, ntr)) {
+        ntr = NULL;             /* This is referenced by sig_app still */
+        goto err;
+    }
 
     sk_nid_triple_sort(sig_app);
     sk_nid_triple_sort(sigx_app);
 
-    return 1;
+    ntr = NULL;
+    ret = 1;
+ err:
+    OPENSSL_free(ntr);
+    CRYPTO_THREAD_unlock(sig_lock);
+    return ret;
 }
 
 static void sid_free(nid_triple *tt)
@@ -133,7 +208,9 @@ static void sid_free(nid_triple *tt)
 void OBJ_sigid_free(void)
 {
     sk_nid_triple_pop_free(sig_app, sid_free);
-    sig_app = NULL;
     sk_nid_triple_free(sigx_app);
+    CRYPTO_THREAD_lock_free(sig_lock);
+    sig_app = NULL;
     sigx_app = NULL;
+    sig_lock = NULL;
 }