From e05b809b1556c0d8e6f9e917f8e01f0141a51461 Mon Sep 17 00:00:00 2001 From: thb-sb Date: Wed, 25 Oct 2023 01:03:38 +0200 Subject: [PATCH] Fix #272: run `c_obj_create` under a lock. See https://github.com/open-quantum-safe/oqs-provider/issues/272. This commit adds a lock to run `c_obj_create` and `OBJ_create` thread-safely in `OQS_PROVIDER_ENTRYPOINT_NAME`, --- oqsprov/oqsprov.c | 37 +++++++++-- test/CMakeLists.txt | 21 ++++++ test/oqs_test_multi_thread.c | 120 +++++++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 test/oqs_test_multi_thread.c diff --git a/oqsprov/oqsprov.c b/oqsprov/oqsprov.c index 615d00fe..a8f4c0be 100644 --- a/oqsprov/oqsprov.c +++ b/oqsprov/oqsprov.c @@ -900,6 +900,23 @@ static const OSSL_DISPATCH oqsprovider_dispatch_table[] # define OQS_PROVIDER_ENTRYPOINT_NAME OSSL_provider_init #endif // ifdef OQS_PROVIDER_STATIC +static CRYPTO_ONCE once_obj_create = CRYPTO_ONCE_STATIC_INIT; +static CRYPTO_RWLOCK *lock_obj_create; + +/* Initializes a lock for `OBJ_create`. */ +static void init_obj_create_lock(void) +{ + lock_obj_create = CRYPTO_THREAD_lock_new(); +} + +#ifndef _WIN32 +__attribute__((destructor)) static void exit_obj_create_lock(void) +{ + CRYPTO_THREAD_lock_free(lock_obj_create); + lock_obj_create = NULL; +} +#endif + int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, const OSSL_DISPATCH *in, const OSSL_DISPATCH **out, void **provctx) @@ -963,6 +980,13 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, ossl_versionp = *(void **)version_request[0].data; } + // initialize lock for `OBJ_create`. + if (!CRYPTO_THREAD_run_once(&once_obj_create, init_obj_create_lock) + || lock_obj_create == NULL) { + goto end_init; + } + CRYPTO_THREAD_write_lock(lock_obj_create); + // 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], @@ -970,7 +994,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, 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 @@ -986,7 +1010,7 @@ 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], "", @@ -994,7 +1018,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, 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) { @@ -1006,7 +1030,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; } } @@ -1017,7 +1041,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; @@ -1032,6 +1056,9 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, } rc = 1; +unlock_obj_create_lock: + CRYPTO_THREAD_unlock(lock_obj_create); + end_init: if (!rc) { if (ossl_versionp) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 1c5fd96a..e77b5454 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -110,11 +110,32 @@ 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" +) +# openssl under MSVC seems to have a bug registering NIDs: +# It only works when setting OPENSSL_CONF, not when loading the same cnf file: +if (MSVC) +set_tests_properties(oqs_multi_thread + PROPERTIES ENVIRONMENT "OPENSSL_MODULES=${OQS_PROV_BINARY_DIR};OPENSSL_CONF=${CMAKE_CURRENT_SOURCE_DIR}/openssl-ca.cnf" +) +else() +set_tests_properties(oqs_multi_thread + PROPERTIES ENVIRONMENT "OPENSSL_MODULES=${OQS_PROV_BINARY_DIR}" +) +endif() + 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_multi_thread ) endif() diff --git a/test/oqs_test_multi_thread.c b/test/oqs_test_multi_thread.c new file mode 100644 index 00000000..951a0552 --- /dev/null +++ b/test/oqs_test_multi_thread.c @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 AND MIT + +#ifdef _WIN32 + +# include + +#else + +# include + +#endif // ifdef _WIN32 + +#include +#include + +#include "test_common.h" + +static const size_t N_THREADS = 6; + +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 *libctx) +{ + OSSL_PROVIDER *default_provider = NULL; + OSSL_PROVIDER *oqs_provider = NULL; + int ret = -1; + +#ifdef OQS_PROVIDER_STATIC + if ((default_provider = OSSL_PROVIDER_load(libctx, "default")) == NULL) { + goto end; + } + if (OSSL_PROVIDER_add_builtin(libctx, "oqsprovider", + OQS_PROVIDER_ENTRYPOINT_NAME) + != 1) { + goto unload_default_provider; + } + if ((oqs_provider = OSSL_PROVIDER_load(libctx, "oqsprovider")) == NULL) { + goto unload_default_provider; + } + ret = 0; + OSS_PROVIDER_unload(oqs_provider); + +#else + + if (OSSL_LIB_CTX_load_config(libctx, kConfigFile) == 1 + && OSSL_PROVIDER_available(libctx, 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_libctx(void *arg) +{ + OSSL_LIB_CTX *libctx = NULL; + int ret = -1; + + (void)arg; + + if ((libctx = OSSL_LIB_CTX_new()) == NULL) { + goto end; + } + ret = load_oqs_provider_thread(libctx); + OSSL_LIB_CTX_free(libctx); + +end: + return (void *)(size_t)ret; +} + +int main(int argc, char **argv) +{ +#ifdef _WIN32 + HANDLE threads[N_THREADS]; +#else + pthread_t threads[N_THREADS]; +#endif // ifdef _WIN32 + size_t i; + void *result; + int ret = 0; + + T(argc == 3); + + kModuleName = argv[1]; + kConfigFile = argv[2]; + + for (i = 0; i < N_THREADS; ++i) { +#ifdef _WIN32 + threads[i] + = CreateThread(NULL, 0, thread_create_ossl_libctx, NULL, 0, NULL); +#else + pthread_create(threads + i, NULL, thread_create_ossl_libctx, NULL); +#endif // ifdef _WIN32 + } + +#ifdef _WIN32 + WaitForMultipleObjects(N_THREADS, threads, TRUE, INFINITE); +#endif // ifdef _WIN32 + + for (i = 0; i < N_THREADS; ++i) { + result = NULL; +#ifdef _WIN32 + ret |= CloseHandle(threads[i]); +#else + pthread_join(threads[i], &result); + ret |= (int)(size_t)result; +#endif // ifdef _WIN32 + } + + return ret; +}