Fix #272: check c_obj_create error code for OBJ_R_OID_EXISTS.

See #272.

This commit checks that if `c_obj_create` fails, the error code is `OBJ_R_OID_EXISTS`
and the OID has been successfully added (using `OBJ_txt2nid`). If it's the case,
then the error is skipped.

.circleci/config.yml

@@ -84,7 +84,7 @@ jobs:
           name: Run tests
           command: |
              if << parameters.OQS_PROVIDER_BUILD_STATIC >> ; then
-               ctest --test-dir _build/
+               ctest --output-on-failure --test-dir _build/
              else
                ./scripts/runtests.sh -V
              fi

oqsprov/oqsprov.c

@@ -18,6 +18,11 @@
 #include <stdio.h>
 #include <string.h>
 
+#ifdef _WIN32
+#    include <windows.h>
+#endif
+#include <openssl/crypto.h>
+
 #ifdef NDEBUG
 #    define OQS_PROV_PRINTF(a)
 #    define OQS_PROV_PRINTF2(a, b)
@@ -850,7 +855,6 @@ static int oqsprovider_get_params(void *provctx, OSSL_PARAM params[])
     p = OSSL_PARAM_locate(params, OSSL_PROV_PARAM_STATUS);
     if (p != NULL && !OSSL_PARAM_set_int(p, 1)) // provider is always running
         return 0;
-    // not passing in params to respond to is no error; response is empty then
     return 1;
 }
 
@@ -884,6 +888,20 @@ static void oqsprovider_teardown(void *provctx)
     OQS_destroy();
 }
 
+static CRYPTO_ONCE kOnceLockForObjCreate = CRYPTO_ONCE_STATIC_INIT;
+static CRYPTO_RWLOCK *kLockForObjCreate = NULL;
+
+/* Initializes the rwlock `kLockForObjCreate`. */
+#ifdef _MSC_VER
+__declspec(no_sanitize("address"))
+#else
+__attribute__((no_sanitize("address")))
+#endif
+    static void init_lock_for_obj_create(void)
+{
+    kLockForObjCreate = CRYPTO_THREAD_lock_new();
+}
+
 /* Functions we provide to the core */
 static const OSSL_DISPATCH oqsprovider_dispatch_table[]
     = {{OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))oqsprovider_teardown},
@@ -964,14 +982,26 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle,
         ossl_versionp = *(void **)version_request[0].data;
     }
 
+    // Initializes the lock for obj create.
+    if (!CRYPTO_THREAD_run_once(&kOnceLockForObjCreate,
+                                init_lock_for_obj_create)
+        || kLockForObjCreate == NULL) {
+        return 0;
+    }
+
+    // Lock the write lock for creating objects.
+    if (!CRYPTO_THREAD_write_lock(kLockForObjCreate)) {
+        return 0;
+    }
+
     // insert all OIDs to the global objects list
     for (i = 0; i < OQS_OID_CNT; i += 2) {
         if (!c_obj_create(handle, oqs_oid_alg_list[i], oqs_oid_alg_list[i + 1],
                           oqs_oid_alg_list[i + 1])) {
             ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR);
             fprintf(stderr, "error registering NID for %s\n",
                     oqs_oid_alg_list[i + 1]);
-            goto end_init;
+            goto unlock_obj_create_lock;
         }
 
         /* create object (NID) again to avoid setup corner case problems
@@ -987,15 +1017,15 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle,
         if (!oqs_set_nid((char *)oqs_oid_alg_list[i + 1],
                          OBJ_sn2nid(oqs_oid_alg_list[i + 1]))) {
             ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR);
-            goto end_init;
+            goto unlock_obj_create_lock;
         }
 
         if (!c_obj_add_sigid(handle, oqs_oid_alg_list[i + 1], "",
                              oqs_oid_alg_list[i + 1])) {
             fprintf(stderr, "error registering %s with no hash\n",
                     oqs_oid_alg_list[i + 1]);
             ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR);
-            goto end_init;
+            goto unlock_obj_create_lock;
         }
 
         if (OBJ_sn2nid(oqs_oid_alg_list[i + 1]) != 0) {
@@ -1007,7 +1037,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle,
                     "OQS PROV: Impossible error: NID unregistered for %s.\n",
                     oqs_oid_alg_list[i + 1]);
             ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR);
-            goto end_init;
+            goto unlock_obj_create_lock;
         }
     }
 
@@ -1018,7 +1048,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle,
             == NULL)) {
         OQS_PROV_PRINTF("OQS PROV: error creating new provider context\n");
         ERR_raise(ERR_LIB_USER, OQSPROV_R_LIB_CREATE_ERR);
-        goto end_init;
+        goto unlock_obj_create_lock;
     }
 
     *out = oqsprovider_dispatch_table;
@@ -1033,6 +1063,9 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle,
     }
     rc = 1;
 
+unlock_obj_create_lock:
+    CRYPTO_THREAD_unlock(kLockForObjCreate);
+
 end_init:
     if (!rc) {
         if (ossl_versionp)

test/CMakeLists.txt

@@ -110,11 +110,24 @@ set_tests_properties(oqs_endecode
 )
 endif()
 
+add_executable(oqs_test_multi_thread oqs_test_multi_thread.c)
+target_link_libraries(oqs_test_multi_thread PRIVATE ${OPENSSL_CRYPTO_LIBRARY})
+add_test(
+  NAME oqs_multi_thread
+  COMMAND oqs_test_multi_thread
+          "oqsprovider"
+          "${CMAKE_CURRENT_SOURCE_DIR}/openssl-ca.cnf"
+)
+set_tests_properties(oqs_multi_thread
+    PROPERTIES ENVIRONMENT "OPENSSL_MODULES=${OQS_PROV_BINARY_DIR}"
+)
+
 if (OQS_PROVIDER_BUILD_STATIC)
   targets_set_static_provider(oqs_test_signatures
     oqs_test_kems
     oqs_test_groups
     oqs_test_tlssig
     oqs_test_endecode
+    oqs_test_multi_thread
   )
 endif()

test/oqs_test_multi_thread.c

@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: Apache-2.0 AND MIT
+
+#include <pthread.h>
+
+#include <openssl/crypto.h>
+#include <openssl/provider.h>
+
+#include "test_common.h"
+
+static const size_t N_THREADS = 8;
+
+static const char *kModuleName = NULL;
+static const char *kConfigFile = NULL;
+
+/** \brief Loads the oqs-provider in an `OSSL_LIB_CTX` object. */
+static int load_oqs_provider_thread(OSSL_LIB_CTX *lib_ctx)
+{
+    OSSL_PROVIDER *default_provider = NULL;
+    OSSL_PROVIDER *oqs_provider = NULL;
+    int ret = -1;
+
+#ifdef OQS_PROVIDER_STATIC
+    if ((default_provider = OSSL_PROVIDER_load(lib_ctx, "default")) == NULL) {
+        goto end;
+    }
+    if (OSSL_PROVIDER_add_builtin(lib_ctx, kModuleName,
+                                  OQS_PROVIDER_ENTRYPOINT_NAME)
+        != 1) {
+        goto unload_default_provider;
+    }
+    if ((oqs_provider = OSSL_PROVIDER_load(lib_ctx, kModuleName)) == NULL) {
+        goto unload_default_provider;
+    }
+    ret = 0;
+    OSSL_PROVIDER_unload(oqs_provider);
+
+#else
+
+    if (OSSL_LIB_CTX_load_config(lib_ctx, kConfigFile) == 1
+        && OSSL_PROVIDER_available(lib_ctx, kModuleName)) {
+        ret = 0;
+    }
+    goto end;
+
+#endif // ifdef OQS_PROVIDER_STATIC
+
+unload_default_provider:
+    OSSL_PROVIDER_unload(default_provider);
+
+end:
+    return ret;
+}
+
+/** \brief Creates an OSSL_LIB_CTX object and loads oqs-provider. */
+static void *thread_create_ossl_lib_ctx(void *arg)
+{
+    OSSL_LIB_CTX *lib_ctx = NULL;
+    int ret = -1;
+
+    (void)arg;
+
+    if ((lib_ctx = OSSL_LIB_CTX_new()) == NULL) {
+        goto end;
+    }
+    ret = load_oqs_provider_thread(lib_ctx);
+    OSSL_LIB_CTX_free(lib_ctx);
+
+end:
+    return (void *)(size_t)ret;
+}
+
+int main(int argc, char **argv)
+{
+    size_t i;
+    pthread_t threads[N_THREADS];
+    void *result;
+    int ret = 0;
+
+    T(argc == 3);
+
+    kModuleName = argv[1];
+    kConfigFile = argv[2];
+
+    for (i = 0; i < N_THREADS; ++i) {
+        pthread_create(threads + i, NULL, thread_create_ossl_lib_ctx, NULL);
+    }
+
+    for (i = 0; i < N_THREADS; ++i) {
+        result = NULL;
+        pthread_join(threads[i], &result);
+        ret |= (int)(size_t)result;
+    }
+
+    return ret;
+}

test/test_common.c

@@ -35,12 +35,6 @@ int alg_is_enabled(const char *algname)
     return strstr(algname, alglist) == NULL;
 }
 
-#ifdef OQS_PROVIDER_STATIC
-#    define OQS_PROVIDER_ENTRYPOINT_NAME oqs_provider_init
-#else
-#    define OQS_PROVIDER_ENTRYPOINT_NAME OSSL_provider_init
-#endif // ifdef OQS_PROVIDER_STATIC
-
 #ifndef OQS_PROVIDER_STATIC
 
 /* Loads the oqs-provider from a shared module (.so). */

test/test_common.h

@@ -36,6 +36,13 @@
 void hexdump(const void *ptr, size_t len);
 int alg_is_enabled(const char *algname);
 
+#ifdef OQS_PROVIDER_STATIC
+#    define OQS_PROVIDER_ENTRYPOINT_NAME oqs_provider_init
+#else
+#    define OQS_PROVIDER_ENTRYPOINT_NAME OSSL_provider_init
+#endif // ifdef OQS_PROVIDER_STATIC
+extern OSSL_provider_init_fn OQS_PROVIDER_ENTRYPOINT_NAME;
+
 /* Loads the oqs-provider. */
 void load_oqs_provider(OSSL_LIB_CTX *libctx, const char *modulename,
                        const char *configfile);