From 6574c06db932c22b8732f8abba52c836ac425ccc Mon Sep 17 00:00:00 2001 From: thb-sb Date: Thu, 2 Nov 2023 20:37:17 +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 | 23 ++++++--- test/CMakeLists.txt | 13 +++++ test/oqs_test_multi_thread.c | 95 ++++++++++++++++++++++++++++++++++++ test/test_common.c | 6 --- test/test_common.h | 7 +++ 6 files changed, 132 insertions(+), 14 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..0a5a07b2 100644 --- a/oqsprov/oqsprov.c +++ b/oqsprov/oqsprov.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -917,6 +918,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, OSSL_PARAM version_request[] = {{"openssl-version", OSSL_PARAM_UTF8_PTR, &opensslv, sizeof(&opensslv), 0}, {NULL, 0, NULL, 0, 0}}; + unsigned long last_error = 0; OQS_init(); @@ -968,10 +970,14 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, 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; + last_error = ERR_peek_last_error(); + if ((last_error != ERR_PACK(ERR_LIB_OBJ, 0, OBJ_R_OID_EXISTS)) + || (OBJ_txt2nid(oqs_oid_alg_list[i]) == NID_undef)) { + 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; + } } /* create object (NID) again to avoid setup corner case problems @@ -1016,9 +1022,12 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, || ((libctx = OSSL_LIB_CTX_new_child(handle, orig_in)) == NULL) || ((*provctx = oqsx_newprovctx(libctx, handle, corebiometh)) == 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; + last_error = ERR_peek_last_error(); + if (last_error != ERR_PACK(ERR_LIB_OBJ, 0, OBJ_R_OID_EXISTS)) { + OQS_PROV_PRINTF("OQS PROV: error creating new provider context\n"); + ERR_raise(ERR_LIB_USER, OQSPROV_R_LIB_CREATE_ERR); + goto end_init; + } } *out = oqsprovider_dispatch_table; 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..5b295ebe --- /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 = 32; + +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);