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 d353e1c
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 5 deletions.
35 changes: 30 additions & 5 deletions oqsprov/oqsprov.c
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,21 @@ 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();
}

/*
__attribute__((destructor)) static void exit_obj_create_lock(void)
{
CRYPTO_THREAD_lock_free(lock_obj_create);
} */

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 +978,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 +1008,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 +1028,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 +1039,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 +1054,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
12 changes: 12 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,18 @@ 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
Expand Down
96 changes: 96 additions & 0 deletions test/oqs_test_multi_thread.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// 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 = 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, "oqsprovider",
OQS_PROVIDER_ENTRYPOINT_NAME)
!= 1) {
goto unload_default_provider;
}
if ((oqs_provider = OSSL_PROVIDER_load(lib_ctx, "oqsprovider")) == NULL) {
goto unload_default_provider;
}
ret = 0;
OSS_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;
}

0 comments on commit d353e1c

Please sign in to comment.