Skip to content

Commit

Permalink
Fix #272: run c_obj_create under a lock.
Browse files Browse the repository at this point in the history
See #272.

This commit adds a lock to run `c_obj_create` and `OBJ_create` thread-safely in
`OQS_PROVIDER_ENTRYPOINT_NAME`,
  • Loading branch information
thb-sb committed Oct 24, 2023
1 parent 8a96fed commit 1970d83
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 5 deletions.
37 changes: 32 additions & 5 deletions oqsprov/oqsprov.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -963,14 +980,21 @@ 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],
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
Expand All @@ -986,15 +1010,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) {
Expand All @@ -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;
}
}

Expand All @@ -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;
Expand All @@ -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)
Expand Down
26 changes: 26 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,37 @@ 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} ${OQS_ADDL_SOCKET_LIBS})
if (NOT MSVC)
find_package(Threads REQUIRED)
target_link_libraries(oqs_test_multi_thread PRIVATE Threads::Threads)
endif()

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()
120 changes: 120 additions & 0 deletions test/oqs_test_multi_thread.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// SPDX-License-Identifier: Apache-2.0 AND MIT

#ifdef _WIN32

# include <Windows.h>

#else

# include <pthread.h>

#endif // ifdef _WIN32

#include <openssl/crypto.h>
#include <openssl/provider.h>

#include "test_common.h"

#define 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;
}

0 comments on commit 1970d83

Please sign in to comment.