From 9682c04d8d3651d5393973ae5549af9456933d35 Mon Sep 17 00:00:00 2001 From: thb-sb Date: Thu, 2 Nov 2023 23:12:11 +0100 Subject: [PATCH] Fix #272: check `c_obj_create` error code for `OBJ_R_OID_EXISTS`. See https://github.com/open-quantum-safe/oqs-provider/issues/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 | 2 +- oqsprov/oqsprov.c | 45 ++++++++++++++--- test/CMakeLists.txt | 13 +++++ test/oqs_test_multi_thread.c | 95 ++++++++++++++++++++++++++++++++++++ test/test_common.c | 6 --- test/test_common.h | 7 +++ 6 files changed, 155 insertions(+), 13 deletions(-) create mode 100644 test/oqs_test_multi_thread.c diff --git a/.circleci/config.yml b/.circleci/config.yml index 1cfbca05..d5f32217 100644 --- a/.circleci/config.yml +++ b/.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 diff --git a/oqsprov/oqsprov.c b/oqsprov/oqsprov.c index dba438c0..cad09812 100644 --- a/oqsprov/oqsprov.c +++ b/oqsprov/oqsprov.c @@ -18,6 +18,11 @@ #include #include +#ifdef _WIN32 +# include +#endif +#include + #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,6 +982,18 @@ 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], @@ -971,7 +1001,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 @@ -987,7 +1017,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], "", @@ -995,7 +1025,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) { @@ -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) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 1c5fd96a..8c5371ff 100644 --- a/test/CMakeLists.txt +++ b/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() diff --git a/test/oqs_test_multi_thread.c b/test/oqs_test_multi_thread.c new file mode 100644 index 00000000..e0375754 --- /dev/null +++ b/test/oqs_test_multi_thread.c @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: Apache-2.0 AND MIT + +#include + +#include +#include + +#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; +} diff --git a/test/test_common.c b/test/test_common.c index 19382c9c..9ab0b71e 100644 --- a/test/test_common.c +++ b/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). */ diff --git a/test/test_common.h b/test/test_common.h index 844796a0..c2539939 100644 --- a/test/test_common.h +++ b/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);