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)

(cherry picked from commit c568900c9ac02e92c54bd3168773d54d7350a580)
Reviewed-by: Hugo Landau <hlandau@openssl.org>

1 files changed, 117 insertions, 40 deletions

diff --git a/crypto/objects/obj_xref.c b/crypto/objects/obj_xref.c
index da1035112f..8b4980d5b5 100644
--- 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 || 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;
 }