Skip to content

Commit

Permalink
Fix #272: check c_obj_create error code for OBJ_R_OID_EXISTS.
Browse files Browse the repository at this point in the history
See #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.
  • Loading branch information
thb-sb authored and Thomas Bailleux committed Nov 2, 2023
1 parent 4dac252 commit 564ee11
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 12 deletions.
47 changes: 41 additions & 6 deletions oqsprov/oqsprov.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
#include <stdio.h>
#include <string.h>

#ifdef _WIN32
# include <Windows.h>
#endif
#include <openssl/crypto.h>

#ifdef NDEBUG
# define OQS_PROV_PRINTF(a)
# define OQS_PROV_PRINTF2(a, b)
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -884,6 +888,22 @@ static void oqsprovider_teardown(void *provctx)
OQS_destroy();
}

static CRYPTO_ONCE kOnceLockForObjCreate = CRYPTO_ONCE_STATIC_INIT;
static CRYPTO_RWLOCK *kLockForObjCreate = NULL;

/* Frees the rwlock `kLockForObjCreate`. */
static void free_lock_for_obj_create(void)
{
CRYPTO_THREAD_lock_free(kLockForObjCreate);
}

/* Initializes the rwlock `kLockForObjCreate`. */
static void init_lock_for_obj_create(void)
{
kLockForObjCreate = CRYPTO_THREAD_lock_new();
atexit(free_lock_for_obj_create);
}

/* Functions we provide to the core */
static const OSSL_DISPATCH oqsprovider_dispatch_table[]
= {{OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))oqsprovider_teardown},
Expand Down Expand Up @@ -964,14 +984,26 @@ 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],
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 @@ -987,15 +1019,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 @@ -1007,7 +1039,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 @@ -1018,7 +1050,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 @@ -1033,6 +1065,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)
Expand Down
17 changes: 17 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,23 @@ set_tests_properties(oqs_endecode
)
endif()

if (NOT MSVC)
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_multi_thread)
endif()
endif()

if (OQS_PROVIDER_BUILD_STATIC)
targets_set_static_provider(oqs_test_signatures
oqs_test_kems
Expand Down
97 changes: 97 additions & 0 deletions test/oqs_test_multi_thread.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// SPDX-License-Identifier: Apache-2.0 AND MIT

#ifndef _WIN32
# include <pthread.h>
#endif

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

#include "test_common.h"

static const size_t N_THREADS = 16;

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;
}
6 changes: 0 additions & 6 deletions test/test_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand Down
7 changes: 7 additions & 0 deletions test/test_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit 564ee11

Please sign in to comment.