[openssl] master update
Matt Caswell
matt at openssl.org
Tue Feb 2 14:00:22 UTC 2021
The branch master has been updated
via f94a91698b82a1986b553a1f46e4cd51219d0223 (commit)
via 0b07db6f56e0240de6cc2ea122eee6431459ef20 (commit)
via 40994605140b9fcbe98a786dc75bdc1b9e9fee3f (commit)
via 04b9435a991585d0f9a775a203cc3986d4872a6e (commit)
via b233ea82765e80038e4884564153f9c8543d9396 (commit)
via cd4e6a351201270cd2769e1e2af7e9fb875a3f80 (commit)
via a0134d293e907672e2717fe54ce6a4b3ae425388 (commit)
from 7ff9fdd4b31757f70080bd3fa2e633ca080408a4 (commit)
- Log -----------------------------------------------------------------
commit f94a91698b82a1986b553a1f46e4cd51219d0223
Author: Matt Caswell <matt at openssl.org>
Date: Wed Jan 27 17:23:13 2021 +0000
Add a CI job to run the threads test with threads sanitizer on
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
commit 0b07db6f56e0240de6cc2ea122eee6431459ef20
Author: Matt Caswell <matt at openssl.org>
Date: Wed Jan 27 17:18:27 2021 +0000
Ensure the EVP_PKEY operation_cache is appropriately locked
The EVP_PKEY operation_cache caches references to provider side key
objects that have previously been exported for this EVP_PKEY, and their
associated key managers. The cache may be updated from time to time as the
EVP_PKEY is exported to more providers. Since an EVP_PKEY may be shared by
multiple threads simultaneously we must be careful to ensure the cache
updates are locked.
Fixes #13818
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
commit 40994605140b9fcbe98a786dc75bdc1b9e9fee3f
Author: Matt Caswell <matt at openssl.org>
Date: Wed Jan 27 15:51:48 2021 +0000
Ensure access to FIPS_state and rate_limit is appropriately locked
These variables can be accessed concurrently from multiple threads so
we ensure that we properly lock them before read or write.
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
commit 04b9435a991585d0f9a775a203cc3986d4872a6e
Author: Matt Caswell <matt at openssl.org>
Date: Tue Jan 26 17:00:25 2021 +0000
Always ensure we hold ctx->lock when calling CRYPTO_get_ex_data()
Otherwise we can get data races.
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
commit b233ea82765e80038e4884564153f9c8543d9396
Author: Matt Caswell <matt at openssl.org>
Date: Tue Jan 26 15:23:19 2021 +0000
Avoid races by caching exported ciphers in the init function
TSAN was reporting a race of the exported ciphers cache that we create in
the default and fips providers. This was because we cached it in the query
function rather than the init function, so this would cause a race if multiple
threads queried at the same time. In practice it probably wouldn't make much
difference since different threads should come up with the same answer.
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
commit cd4e6a351201270cd2769e1e2af7e9fb875a3f80
Author: Matt Caswell <matt at openssl.org>
Date: Tue Jan 26 15:14:02 2021 +0000
Refactor RAND_get0_primary() locking
Make sure we never read or write to dgbl->primary outside of a lock.
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
commit a0134d293e907672e2717fe54ce6a4b3ae425388
Author: Matt Caswell <matt at openssl.org>
Date: Tue Jan 26 13:30:06 2021 +0000
Add a multi-thread test for shared EVP_PKEYs
EVP_PKEYs may be shared across mutliple threads. For example this is
common for users of libssl who provide a single EVP_PKEY private key for
an SSL_CTX, which is then shared between multiple threads for each SSL
object.
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
-----------------------------------------------------------------------
Summary of changes:
.github/workflows/ci.yml | 11 +++
crypto/context.c | 22 ++++--
crypto/evp/keymgmt_lib.c | 39 +++++++++-
crypto/evp/p_lib.c | 22 +++++-
crypto/ex_data.c | 13 +++-
crypto/rand/rand_lib.c | 64 ++++++++++-------
.../man3/evp_keymgmt_util_export_to_provider.pod | 16 +++--
include/crypto/cryptlib.h | 3 +
include/crypto/evp.h | 2 +-
providers/defltprov.c | 2 +-
providers/fips/fipsprov.c | 4 +-
providers/fips/self_test.c | 46 ++++++++----
test/recipes/90-test_threads.t | 6 +-
.../90-test_threads_data/rsakey.pem} | 0
test/threadstest.c | 82 +++++++++++++++++++++-
15 files changed, 267 insertions(+), 65 deletions(-)
copy test/{certs/serverkey.pem => recipes/90-test_threads_data/rsakey.pem} (100%)
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9e99a9b97b..b057eb1d5b 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -91,6 +91,17 @@ jobs:
- name: make test
run: make test HARNESS_JOBS=${HARNESS_JOBS:-4} OPENSSL_TEST_RAND_ORDER=0
+ threads_sanitizer:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout at v2
+ - name: config
+ run: CC=clang ./config --strict-warnings -fsanitize=thread && perl configdata.pm --dump
+ - name: make
+ run: make -s -j4
+ - name: make test
+ run: make TESTS=test_threads test HARNESS_JOBS=${HARNESS_JOBS:-4}
+
enable_non-default_options:
runs-on: ubuntu-latest
steps:
diff --git a/crypto/context.c b/crypto/context.c
index 7efe475b70..5a46921778 100644
--- a/crypto/context.c
+++ b/crypto/context.c
@@ -283,7 +283,9 @@ void *ossl_lib_ctx_get_data(OSSL_LIB_CTX *ctx, int index,
if (dynidx != -1) {
CRYPTO_THREAD_read_lock(ctx->index_locks[index]);
+ CRYPTO_THREAD_read_lock(ctx->lock);
data = CRYPTO_get_ex_data(&ctx->data, dynidx);
+ CRYPTO_THREAD_unlock(ctx->lock);
CRYPTO_THREAD_unlock(ctx->index_locks[index]);
return data;
}
@@ -293,8 +295,8 @@ void *ossl_lib_ctx_get_data(OSSL_LIB_CTX *ctx, int index,
dynidx = ctx->dyn_indexes[index];
if (dynidx != -1) {
- CRYPTO_THREAD_unlock(ctx->lock);
data = CRYPTO_get_ex_data(&ctx->data, dynidx);
+ CRYPTO_THREAD_unlock(ctx->lock);
CRYPTO_THREAD_unlock(ctx->index_locks[index]);
return data;
}
@@ -307,10 +309,22 @@ void *ossl_lib_ctx_get_data(OSSL_LIB_CTX *ctx, int index,
CRYPTO_THREAD_unlock(ctx->lock);
- /* The alloc call ensures there's a value there */
- if (CRYPTO_alloc_ex_data(CRYPTO_EX_INDEX_OSSL_LIB_CTX, NULL,
- &ctx->data, ctx->dyn_indexes[index]))
+ /*
+ * The alloc call ensures there's a value there. We release the ctx->lock
+ * for this, because the allocation itself may recursively call
+ * ossl_lib_ctx_get_data for other indexes (never this one). The allocation
+ * will itself aquire the ctx->lock when it actually comes to store the
+ * allocated data (see ossl_lib_ctx_generic_new() above). We call
+ * ossl_crypto_alloc_ex_data_intern() here instead of CRYPTO_alloc_ex_data().
+ * They do the same thing except that the latter calls CRYPTO_get_ex_data()
+ * as well - which we must not do without holding the ctx->lock.
+ */
+ if (ossl_crypto_alloc_ex_data_intern(CRYPTO_EX_INDEX_OSSL_LIB_CTX, NULL,
+ &ctx->data, ctx->dyn_indexes[index])) {
+ CRYPTO_THREAD_read_lock(ctx->lock);
data = CRYPTO_get_ex_data(&ctx->data, ctx->dyn_indexes[index]);
+ CRYPTO_THREAD_unlock(ctx->lock);
+ }
CRYPTO_THREAD_unlock(ctx->index_locks[index]);
diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c
index 763982e58f..0c643b3b49 100644
--- a/crypto/evp/keymgmt_lib.c
+++ b/crypto/evp/keymgmt_lib.c
@@ -102,10 +102,16 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt)
return pk->keydata;
/* If this key is already exported to |keymgmt|, no more to do */
+ CRYPTO_THREAD_read_lock(pk->lock);
i = evp_keymgmt_util_find_operation_cache_index(pk, keymgmt);
if (i < OSSL_NELEM(pk->operation_cache)
- && pk->operation_cache[i].keymgmt != NULL)
- return pk->operation_cache[i].keydata;
+ && pk->operation_cache[i].keymgmt != NULL) {
+ void *ret = pk->operation_cache[i].keydata;
+
+ CRYPTO_THREAD_unlock(pk->lock);
+ return ret;
+ }
+ CRYPTO_THREAD_unlock(pk->lock);
/* If the "origin" |keymgmt| doesn't support exporting, give up */
/*
@@ -153,20 +159,42 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt)
return NULL;
}
+ CRYPTO_THREAD_write_lock(pk->lock);
+ /* Check to make sure some other thread didn't get there first */
+ i = evp_keymgmt_util_find_operation_cache_index(pk, keymgmt);
+ if (i < OSSL_NELEM(pk->operation_cache)
+ && pk->operation_cache[i].keymgmt != NULL) {
+ void *ret = pk->operation_cache[i].keydata;
+
+ CRYPTO_THREAD_unlock(pk->lock);
+
+ /*
+ * Another thread seemms to have already exported this so we abandon
+ * all the work we just did.
+ */
+ evp_keymgmt_freedata(keymgmt, import_data.keydata);
+
+ return ret;
+ }
+
/* Add the new export to the operation cache */
if (!evp_keymgmt_util_cache_keydata(pk, i, keymgmt, import_data.keydata)) {
evp_keymgmt_freedata(keymgmt, import_data.keydata);
return NULL;
}
+ CRYPTO_THREAD_unlock(pk->lock);
+
return import_data.keydata;
}
-void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk)
+int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking)
{
size_t i, end = OSSL_NELEM(pk->operation_cache);
if (pk != NULL) {
+ if (locking && pk->lock != NULL && !CRYPTO_THREAD_write_lock(pk->lock))
+ return 0;
for (i = 0; i < end && pk->operation_cache[i].keymgmt != NULL; i++) {
EVP_KEYMGMT *keymgmt = pk->operation_cache[i].keymgmt;
void *keydata = pk->operation_cache[i].keydata;
@@ -176,7 +204,11 @@ void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk)
evp_keymgmt_freedata(keymgmt, keydata);
EVP_KEYMGMT_free(keymgmt);
}
+ if (locking && pk->lock != NULL)
+ CRYPTO_THREAD_unlock(pk->lock);
}
+
+ return 1;
}
size_t evp_keymgmt_util_find_operation_cache_index(EVP_PKEY *pk,
@@ -198,6 +230,7 @@ int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
if (keydata != NULL) {
if (!EVP_KEYMGMT_up_ref(keymgmt))
return 0;
+
pk->operation_cache[index].keydata = keydata;
pk->operation_cache[index].keymgmt = keymgmt;
}
diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index 5df9b19eae..21ce51d573 100644
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1621,7 +1621,7 @@ static void evp_pkey_free_it(EVP_PKEY *x)
{
/* internal function; x is never NULL */
- evp_keymgmt_util_clear_operation_cache(x);
+ evp_keymgmt_util_clear_operation_cache(x, 1);
#ifndef FIPS_MODULE
evp_pkey_free_legacy(x);
#endif
@@ -1735,6 +1735,8 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
* |i| remains zero, and we will clear the cache further down.
*/
if (pk->ameth->dirty_cnt(pk) == pk->dirty_cnt_copy) {
+ if (!CRYPTO_THREAD_read_lock(pk->lock))
+ goto end;
i = evp_keymgmt_util_find_operation_cache_index(pk, tmp_keymgmt);
/*
@@ -1746,8 +1748,10 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
if (i < OSSL_NELEM(pk->operation_cache)
&& pk->operation_cache[i].keymgmt != NULL) {
keydata = pk->operation_cache[i].keydata;
+ CRYPTO_THREAD_unlock(pk->lock);
goto end;
}
+ CRYPTO_THREAD_unlock(pk->lock);
}
/*
@@ -1782,12 +1786,22 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
keydata = NULL;
goto end;
}
- if (pk->ameth->dirty_cnt(pk) != pk->dirty_cnt_copy)
- evp_keymgmt_util_clear_operation_cache(pk);
+
+ if (!CRYPTO_THREAD_write_lock(pk->lock))
+ goto end;
+ if (pk->ameth->dirty_cnt(pk) != pk->dirty_cnt_copy
+ && !evp_keymgmt_util_clear_operation_cache(pk, 0)) {
+ CRYPTO_THREAD_unlock(pk->lock);
+ evp_keymgmt_freedata(tmp_keymgmt, keydata);
+ keydata = NULL;
+ EVP_KEYMGMT_free(tmp_keymgmt);
+ goto end;
+ }
EVP_KEYMGMT_free(tmp_keymgmt); /* refcnt-- */
/* Add the new export to the operation cache */
if (!evp_keymgmt_util_cache_keydata(pk, i, tmp_keymgmt, keydata)) {
+ CRYPTO_THREAD_unlock(pk->lock);
evp_keymgmt_freedata(tmp_keymgmt, keydata);
keydata = NULL;
goto end;
@@ -1795,6 +1809,8 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
/* Synchronize the dirty count */
pk->dirty_cnt_copy = pk->ameth->dirty_cnt(pk);
+
+ CRYPTO_THREAD_unlock(pk->lock);
goto end;
}
#endif /* FIPS_MODULE */
diff --git a/crypto/ex_data.c b/crypto/ex_data.c
index 5de99b4735..0d87ea7f0e 100644
--- a/crypto/ex_data.c
+++ b/crypto/ex_data.c
@@ -392,16 +392,23 @@ void CRYPTO_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad)
int CRYPTO_alloc_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad,
int idx)
{
- EX_CALLBACK *f;
- EX_CALLBACKS *ip;
void *curval;
- OSSL_EX_DATA_GLOBAL *global;
curval = CRYPTO_get_ex_data(ad, idx);
/* Already there, no need to allocate */
if (curval != NULL)
return 1;
+ return ossl_crypto_alloc_ex_data_intern(class_index, obj, ad, idx);
+}
+
+int ossl_crypto_alloc_ex_data_intern(int class_index, void *obj,
+ CRYPTO_EX_DATA *ad, int idx)
+{
+ EX_CALLBACK *f;
+ EX_CALLBACKS *ip;
+ OSSL_EX_DATA_GLOBAL *global;
+
global = ossl_lib_ctx_get_ex_data_global(ad->ctx);
if (global == NULL)
return 0;
diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index 4e561568cd..69afa9d2ea 100644
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -553,38 +553,52 @@ static EVP_RAND_CTX *rand_new_drbg(OSSL_LIB_CTX *libctx, EVP_RAND_CTX *parent,
EVP_RAND_CTX *RAND_get0_primary(OSSL_LIB_CTX *ctx)
{
RAND_GLOBAL *dgbl = rand_get_global(ctx);
+ EVP_RAND_CTX *ret;
if (dgbl == NULL)
return NULL;
- if (dgbl->primary == NULL) {
- if (!CRYPTO_THREAD_write_lock(dgbl->lock))
- return NULL;
+ if (!CRYPTO_THREAD_read_lock(dgbl->lock))
+ return NULL;
+
+ ret = dgbl->primary;
+ CRYPTO_THREAD_unlock(dgbl->lock);
+
+ if (ret != NULL)
+ return ret;
+
+ if (!CRYPTO_THREAD_write_lock(dgbl->lock))
+ return NULL;
+
+ ret = dgbl->primary;
+ if (ret != NULL) {
+ CRYPTO_THREAD_unlock(dgbl->lock);
+ return ret;
+ }
+
#ifndef FIPS_MODULE
- if (dgbl->seed == NULL) {
- ERR_set_mark();
- dgbl->seed = rand_new_seed(ctx);
- ERR_pop_to_mark();
- }
+ if (dgbl->seed == NULL) {
+ ERR_set_mark();
+ dgbl->seed = rand_new_seed(ctx);
+ ERR_pop_to_mark();
+ }
#endif
- if (dgbl->primary == NULL)
- dgbl->primary = rand_new_drbg(ctx, dgbl->seed,
- PRIMARY_RESEED_INTERVAL,
- PRIMARY_RESEED_TIME_INTERVAL);
- /*
- * The primary DRBG may be shared between multiple threads so we must
- * enable locking.
- */
- if (dgbl->primary != NULL && !EVP_RAND_enable_locking(dgbl->primary)) {
- ERR_raise(ERR_LIB_EVP, EVP_R_UNABLE_TO_ENABLE_LOCKING);
- EVP_RAND_CTX_free(dgbl->primary);
- dgbl->primary = NULL;
- CRYPTO_THREAD_lock_free(dgbl->lock);
- return NULL;
- }
- CRYPTO_THREAD_unlock(dgbl->lock);
+
+ ret = dgbl->primary = rand_new_drbg(ctx, dgbl->seed,
+ PRIMARY_RESEED_INTERVAL,
+ PRIMARY_RESEED_TIME_INTERVAL);
+ /*
+ * The primary DRBG may be shared between multiple threads so we must
+ * enable locking.
+ */
+ if (ret != NULL && !EVP_RAND_enable_locking(ret)) {
+ ERR_raise(ERR_LIB_EVP, EVP_R_UNABLE_TO_ENABLE_LOCKING);
+ EVP_RAND_CTX_free(ret);
+ ret = dgbl->primary = NULL;
}
- return dgbl->primary;
+ CRYPTO_THREAD_unlock(dgbl->lock);
+
+ return ret;
}
/*
diff --git a/doc/internal/man3/evp_keymgmt_util_export_to_provider.pod b/doc/internal/man3/evp_keymgmt_util_export_to_provider.pod
index bb2ad9ba8e..31f8b00e47 100644
--- a/doc/internal/man3/evp_keymgmt_util_export_to_provider.pod
+++ b/doc/internal/man3/evp_keymgmt_util_export_to_provider.pod
@@ -20,9 +20,9 @@ evp_keymgmt_util_fromdata
void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt);
size_t evp_keymgmt_util_find_operation_cache_index(EVP_PKEY *pk,
EVP_KEYMGMT *keymgmt);
- void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk);
- void evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
- EVP_KEYMGMT *keymgmt, void *keydata);
+ int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking);
+ int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
+ EVP_KEYMGMT *keymgmt, void *keydata);
void evp_keymgmt_util_cache_keyinfo(EVP_PKEY *pk);
void *evp_keymgmt_util_fromdata(EVP_PKEY *target, EVP_KEYMGMT *keymgmt,
int selection, const OSSL_PARAM params[]);
@@ -44,10 +44,13 @@ as this function ignores any legacy key data.
evp_keymgmt_util_find_operation_cache_index() finds the location if
I<keymgmt> in I<pk>'s cache of provided keys for operations. If
I<keymgmt> is NULL or couldn't be found in the cache, it finds the
-first empty slot instead if there is any.
+first empty slot instead if there is any. It should only be called while
+holding I<pk>'s lock (read or write).
evp_keymgmt_util_clear_operation_cache() can be used to explicitly
-clear the cache of operation key references.
+clear the cache of operation key references. If I<locking> is set to 1 then
+then I<pk>'s lock will be obtained while doing the clear. Otherwise it will be
+assumed that the lock has already been obtained or is not required.
evp_keymgmt_util_cache_keydata() can be used to assign a provider key
object to a specific cache slot in the given I<target>.
@@ -72,6 +75,9 @@ operation cache slot. If I<keymgmt> is NULL, or if there is no slot
with a match for I<keymgmt>, the index of the first empty slot is
returned, or the maximum number of slots if there isn't an empty one.
+evp_keymgmt_util_cache_keydata() and evp_keymgmt_util_clear_operation_cache()
+return 1 on success or 0 otherwise.
+
=head1 NOTES
"Legacy key" is the term used for any key that has been assigned to an
diff --git a/include/crypto/cryptlib.h b/include/crypto/cryptlib.h
index 69d94be54a..8fd04fa16f 100644
--- a/include/crypto/cryptlib.h
+++ b/include/crypto/cryptlib.h
@@ -29,3 +29,6 @@ void ossl_ctx_thread_stop(void *arg);
void ossl_trace_cleanup(void);
void ossl_malloc_setup_failures(void);
+
+int ossl_crypto_alloc_ex_data_intern(int class_index, void *obj,
+ CRYPTO_EX_DATA *ad, int idx);
diff --git a/include/crypto/evp.h b/include/crypto/evp.h
index 20335e9a32..bed75f406c 100644
--- a/include/crypto/evp.h
+++ b/include/crypto/evp.h
@@ -729,7 +729,7 @@ int evp_keymgmt_util_export(const EVP_PKEY *pk, int selection,
void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt);
size_t evp_keymgmt_util_find_operation_cache_index(EVP_PKEY *pk,
EVP_KEYMGMT *keymgmt);
-void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk);
+int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking);
int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
EVP_KEYMGMT *keymgmt, void *keydata);
void evp_keymgmt_util_cache_keyinfo(EVP_PKEY *pk);
diff --git a/providers/defltprov.c b/providers/defltprov.c
index 2a1ebb6218..c246ed42be 100644
--- a/providers/defltprov.c
+++ b/providers/defltprov.c
@@ -472,7 +472,6 @@ static const OSSL_ALGORITHM *deflt_query(void *provctx, int operation_id,
case OSSL_OP_DIGEST:
return deflt_digests;
case OSSL_OP_CIPHER:
- ossl_prov_cache_exported_algorithms(deflt_ciphers, exported_ciphers);
return exported_ciphers;
case OSSL_OP_MAC:
return deflt_macs;
@@ -570,6 +569,7 @@ int ossl_default_provider_init(const OSSL_CORE_HANDLE *handle,
ossl_prov_ctx_set0_core_bio_method(*provctx, corebiometh);
*out = deflt_dispatch_table;
+ ossl_prov_cache_exported_algorithms(deflt_ciphers, exported_ciphers);
return 1;
}
diff --git a/providers/fips/fipsprov.c b/providers/fips/fipsprov.c
index deffb88ba6..dc1bd7b472 100644
--- a/providers/fips/fipsprov.c
+++ b/providers/fips/fipsprov.c
@@ -434,8 +434,6 @@ static const OSSL_ALGORITHM *fips_query(void *provctx, int operation_id,
case OSSL_OP_DIGEST:
return fips_digests;
case OSSL_OP_CIPHER:
- ossl_prov_cache_exported_algorithms(fips_ciphers,
- exported_fips_ciphers);
return exported_fips_ciphers;
case OSSL_OP_MAC:
return fips_macs;
@@ -626,6 +624,8 @@ int OSSL_provider_init(const OSSL_CORE_HANDLE *handle,
fgbl->handle = handle;
+ ossl_prov_cache_exported_algorithms(fips_ciphers, exported_fips_ciphers);
+
selftest_params.libctx = libctx;
if (!SELF_TEST_post(&selftest_params, 0)) {
ERR_raise(ERR_LIB_PROV, PROV_R_SELF_TEST_POST_FAILURE);
diff --git a/providers/fips/self_test.c b/providers/fips/self_test.c
index 4d8e640c38..a3dd621262 100644
--- a/providers/fips/self_test.c
+++ b/providers/fips/self_test.c
@@ -47,18 +47,19 @@
static int FIPS_conditional_error_check = 1;
static int FIPS_state = FIPS_STATE_INIT;
static CRYPTO_RWLOCK *self_test_lock = NULL;
+static CRYPTO_RWLOCK *fips_state_lock = NULL;
static unsigned char fixed_key[32] = { FIPS_KEY_ELEMENTS };
static CRYPTO_ONCE fips_self_test_init = CRYPTO_ONCE_STATIC_INIT;
DEFINE_RUN_ONCE_STATIC(do_fips_self_test_init)
{
/*
- * This lock gets freed in platform specific ways that may occur after we
+ * These locks get freed in platform specific ways that may occur after we
* do mem leak checking. If we don't know how to free it for a particular
- * platform then we just leak it deliberately. So we temporarily disable the
- * mem leak checking while we allocate this.
+ * platform then we just leak it deliberately.
*/
self_test_lock = CRYPTO_THREAD_lock_new();
+ fips_state_lock = CRYPTO_THREAD_lock_new();
return self_test_lock != NULL;
}
@@ -150,6 +151,7 @@ DEP_INIT_ATTRIBUTE void init(void)
DEP_FINI_ATTRIBUTE void cleanup(void)
{
CRYPTO_THREAD_lock_free(self_test_lock);
+ CRYPTO_THREAD_lock_free(fips_state_lock);
}
#endif
@@ -212,6 +214,13 @@ err:
return ret;
}
+static void set_fips_state(int state)
+{
+ CRYPTO_THREAD_write_lock(fips_state_lock);
+ FIPS_state = state;
+ CRYPTO_THREAD_unlock(fips_state_lock);
+}
+
/* This API is triggered either on loading of the FIPS module or on demand */
int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test)
{
@@ -227,9 +236,9 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test)
if (!RUN_ONCE(&fips_self_test_init, do_fips_self_test_init))
return 0;
- CRYPTO_THREAD_read_lock(self_test_lock);
+ CRYPTO_THREAD_read_lock(fips_state_lock);
loclstate = FIPS_state;
- CRYPTO_THREAD_unlock(self_test_lock);
+ CRYPTO_THREAD_unlock(fips_state_lock);
if (loclstate == FIPS_STATE_RUNNING) {
if (!on_demand_test)
@@ -240,17 +249,23 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test)
}
CRYPTO_THREAD_write_lock(self_test_lock);
+ CRYPTO_THREAD_read_lock(fips_state_lock);
if (FIPS_state == FIPS_STATE_RUNNING) {
+ CRYPTO_THREAD_unlock(fips_state_lock);
if (!on_demand_test) {
CRYPTO_THREAD_unlock(self_test_lock);
return 1;
}
- FIPS_state = FIPS_STATE_SELFTEST;
+ set_fips_state(FIPS_STATE_SELFTEST);
} else if (FIPS_state != FIPS_STATE_SELFTEST) {
+ CRYPTO_THREAD_unlock(fips_state_lock);
CRYPTO_THREAD_unlock(self_test_lock);
ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_STATE);
return 0;
+ } else {
+ CRYPTO_THREAD_unlock(fips_state_lock);
}
+
if (st == NULL
|| st->module_checksum_data == NULL) {
ERR_raise(ERR_LIB_PROV, PROV_R_MISSING_CONFIG_DATA);
@@ -328,7 +343,7 @@ end:
(*st->bio_free_cb)(bio_module);
}
if (ok)
- FIPS_state = FIPS_STATE_RUNNING;
+ set_fips_state(FIPS_STATE_RUNNING);
else
ossl_set_error_state(OSSL_SELF_TEST_TYPE_NONE);
CRYPTO_THREAD_unlock(self_test_lock);
@@ -346,7 +361,7 @@ void ossl_set_error_state(const char *type)
int cond_test = (type != NULL && strcmp(type, OSSL_SELF_TEST_TYPE_PCT) == 0);
if (!cond_test || (FIPS_conditional_error_check == 1)) {
- FIPS_state = FIPS_STATE_ERROR;
+ set_fips_state(FIPS_STATE_ERROR);
ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_ENTERING_ERROR_STATE);
} else {
ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_CONDITIONAL_ERROR);
@@ -355,15 +370,20 @@ void ossl_set_error_state(const char *type)
int ossl_prov_is_running(void)
{
- const int res = FIPS_state == FIPS_STATE_RUNNING
- || FIPS_state == FIPS_STATE_SELFTEST;
+ int res;
static unsigned int rate_limit = 0;
- if (res) {
- rate_limit = 0;
- } else if (FIPS_state == FIPS_STATE_ERROR) {
+ if (!CRYPTO_THREAD_read_lock(fips_state_lock))
+ return 0;
+ res = FIPS_state == FIPS_STATE_RUNNING
+ || FIPS_state == FIPS_STATE_SELFTEST;
+ if (FIPS_state == FIPS_STATE_ERROR) {
+ CRYPTO_THREAD_unlock(fips_state_lock);
+ if (!CRYPTO_THREAD_write_lock(fips_state_lock))
+ return 0;
if (rate_limit++ < FIPS_ERROR_REPORTING_RATE_LIMIT)
ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_IN_ERROR_STATE);
}
+ CRYPTO_THREAD_unlock(fips_state_lock);
return res;
}
diff --git a/test/recipes/90-test_threads.t b/test/recipes/90-test_threads.t
index f46121a751..0410cd8007 100644
--- a/test/recipes/90-test_threads.t
+++ b/test/recipes/90-test_threads.t
@@ -8,7 +8,7 @@
use OpenSSL::Test::Simple;
-use OpenSSL::Test qw/:DEFAULT srctop_file srctop_dir bldtop_dir bldtop_file/;
+use OpenSSL::Test qw/:DEFAULT srctop_file srctop_dir bldtop_dir bldtop_file data_dir/;
use OpenSSL::Test::Utils;
use Cwd qw(abs_path);
@@ -35,8 +35,8 @@ if (!$no_fips) {
if ($no_fips) {
$ENV{OPENSSL_CONF} = abs_path(srctop_file("test", "default.cnf"));
- ok(run(test(["threadstest"])), "running test_threads");
+ ok(run(test(["threadstest", data_dir()])), "running test_threads");
} else {
$ENV{OPENSSL_CONF} = abs_path(srctop_file("test", "default-and-fips.cnf"));
- ok(run(test(["threadstest", "-fips"])), "running test_threads");
+ ok(run(test(["threadstest", "-fips", data_dir()])), "running test_threads");
}
diff --git a/test/certs/serverkey.pem b/test/recipes/90-test_threads_data/rsakey.pem
similarity index 100%
copy from test/certs/serverkey.pem
copy to test/recipes/90-test_threads_data/rsakey.pem
diff --git a/test/threadstest.c b/test/threadstest.c
index 2b9afa7d47..9c8e2181d0 100644
--- a/test/threadstest.c
+++ b/test/threadstest.c
@@ -19,6 +19,7 @@
#include "testutil.h"
static int do_fips = 0;
+static char *privkey;
#if !defined(OPENSSL_THREADS) || defined(CRYPTO_TDEBUG)
@@ -352,17 +353,66 @@ static void thread_multi_simple_fetch(void)
multi_success = 0;
}
+static EVP_PKEY *shared_evp_pkey = NULL;
+
+static void thread_shared_evp_pkey(void)
+{
+ char *msg = "Hello World";
+ unsigned char ctbuf[256];
+ unsigned char ptbuf[256];
+ size_t ptlen = sizeof(ptbuf), ctlen = sizeof(ctbuf);
+ EVP_PKEY_CTX *ctx = NULL;
+ int success = 0;
+ int i;
+
+ for (i = 0; i < 1 + do_fips; i++) {
+ if (i > 0)
+ EVP_PKEY_CTX_free(ctx);
+ ctx = EVP_PKEY_CTX_new_from_pkey(multi_libctx, shared_evp_pkey,
+ i == 0 ? "provider=default"
+ : "provider=fips");
+ if (!TEST_ptr(ctx))
+ goto err;
+
+ if (!TEST_int_ge(EVP_PKEY_encrypt_init(ctx), 0)
+ || !TEST_int_ge(EVP_PKEY_encrypt(ctx, ctbuf, &ctlen,
+ (unsigned char *)msg, strlen(msg)),
+ 0))
+ goto err;
+
+ EVP_PKEY_CTX_free(ctx);
+ ctx = EVP_PKEY_CTX_new_from_pkey(multi_libctx, shared_evp_pkey, NULL);
+
+ if (!TEST_ptr(ctx))
+ goto err;
+
+ if (!TEST_int_ge(EVP_PKEY_decrypt_init(ctx), 0)
+ || !TEST_int_ge(EVP_PKEY_decrypt(ctx, ptbuf, &ptlen, ctbuf, ctlen),
+ 0)
+ || !TEST_mem_eq(msg, strlen(msg), ptbuf, ptlen))
+ goto err;
+ }
+
+ success = 1;
+
+ err:
+ EVP_PKEY_CTX_free(ctx);
+ if (!success)
+ multi_success = 0;
+}
+
/*
* Do work in multiple worker threads at the same time.
* Test 0: General worker, using the default provider
* Test 1: General worker, using the fips provider
* Test 2: Simple fetch worker
+ * Test 3: Worker using a shared EVP_PKEY
*/
static int test_multi(int idx)
{
thread_t thread1, thread2;
int testresult = 0;
- OSSL_PROVIDER *prov = NULL;
+ OSSL_PROVIDER *prov = NULL, *prov2 = NULL;
void (*worker)(void);
if (idx == 1 && !do_fips)
@@ -384,6 +434,18 @@ static int test_multi(int idx)
case 2:
worker = thread_multi_simple_fetch;
break;
+ case 3:
+ /*
+ * If available we have both the default and fips providers for this
+ * test
+ */
+ if (do_fips
+ && !TEST_ptr(prov2 = OSSL_PROVIDER_load(multi_libctx, "fips")))
+ goto err;
+ if (!TEST_ptr(shared_evp_pkey = load_pkey_pem(privkey, multi_libctx)))
+ goto err;
+ worker = thread_shared_evp_pkey;
+ break;
default:
TEST_error("Invalid test index");
goto err;
@@ -404,7 +466,10 @@ static int test_multi(int idx)
err:
OSSL_PROVIDER_unload(prov);
+ OSSL_PROVIDER_unload(prov2);
OSSL_LIB_CTX_free(multi_libctx);
+ EVP_PKEY_free(shared_evp_pkey);
+ shared_evp_pkey = NULL;
return testresult;
}
@@ -428,6 +493,7 @@ const OPTIONS *test_get_options(void)
int setup_tests(void)
{
OPTION_CHOICE o;
+ char *datadir;
while ((o = opt_next()) != OPT_EOF) {
switch (o) {
@@ -441,10 +507,22 @@ int setup_tests(void)
}
}
+ if (!TEST_ptr(datadir = test_get_argument(0)))
+ return 0;
+
+ privkey = test_mk_file_path(datadir, "rsakey.pem");
+ if (!TEST_ptr(privkey))
+ return 0;
+
ADD_TEST(test_lock);
ADD_TEST(test_once);
ADD_TEST(test_thread_local);
ADD_TEST(test_atomic);
- ADD_ALL_TESTS(test_multi, 3);
+ ADD_ALL_TESTS(test_multi, 4);
return 1;
}
+
+void cleanup_tests(void)
+{
+ OPENSSL_free(privkey);
+}
More information about the openssl-commits
mailing list