| Age | Commit message (Collapse) | Author |
|---|
|
We refactor ossl_provider_doall_activated() so that we only need to take
a read lock instead of a write lock for the flag_lock. This should improve
performance by avoiding the lock contention. We achieve this by protecting
the activatecnt via atomics rather than via a lock and by avoiding the full
provider activation/deactivation procedure where it is not needed.
Partial fix for #20286
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20927)
|
|
Adding all providers is quadratic time because each provider is checked
for being in the stack before adding it. However, this is an improvement
over the older O(n^2 log n) code where each find also sorted the stack.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from https://github.com/openssl/openssl/pull/20842)
|
|
Fixes #20710
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20745)
|
|
Reviewed-by: Todd Short <todd.short@me.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19752)
|
|
We were taking a read lock. It should have been a write lock.
Fixes #19474
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19481)
|
|
When a provider is activated, these three cases would record that the
provider init function failed (implying that it was called):
- failure to load the provider module (in case it's a dynamically
loadable module)
- the init function not being present (i.e. being NULL)
- the init function being called and returning an error indication
(i.e. returning a false value)
This is confusing.
Separating the three cases so that they record different errors will
make it easier to determine causes of failure.
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19419)
|
|
Since OPENSSL_malloc() and friends report ERR_R_MALLOC_FAILURE, and
at least handle the file name and line number they are called from,
there's no need to report ERR_R_MALLOC_FAILURE where they are called
directly, or when SSLfatal() and RLAYERfatal() is used, the reason
`ERR_R_MALLOC_FAILURE` is changed to `ERR_R_CRYPTO_LIB`.
There were a number of places where `ERR_R_MALLOC_FAILURE` was reported
even though it was a function from a different sub-system that was
called. Those places are changed to report ERR_R_{lib}_LIB, where
{lib} is the name of that sub-system.
Some of them are tricky to get right, as we have a lot of functions
that belong in the ASN1 sub-system, and all the `sk_` calls or from
the CRYPTO sub-system.
Some extra adaptation was necessary where there were custom OPENSSL_malloc()
wrappers, and some bugs are fixed alongside these changes.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19301)
|
|
The earlier fix being inadequate
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/18822)
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18799)
|
|
When assigning pointers to functions in an OSSL_DISPATCH table, we try
to ensure that those functions are properly defined or declared with
an extra declaration using the corresponding function typedefs that
are defined by include/openssl/core_dispatch.h.
For the core dispatch table, found in crypto/provider_core.c, it seems
we forgot this habit, and thus didn't ensure well enough that the
function pointers that are assigned in the table can actually be used
for those dispatch table indexes.
This change adds all the missing declarations, and compensates for
differences with functions that do the necessary casting, making those
explicit rather than implicit, thereby trying to assure that we know
what we're doing.
One function is not fixed in this change, because there's a controversy,
a clash between the signature of BIO_ctrl() and OSSL_FUNC_BIO_ctrl_fn.
They have different return types.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18198)
|
|
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18641)
|
|
Fixes #18618
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18630)
(cherry picked from commit b07db93f6b38aaea8b131f7dcfba7ad7386196d2)
|
|
The function ossl_provider_free() decrements the refcnt of the
provider and frees it if it has reached 0. This only works if the
refcnt has already been initialised. We must only call
ossl_provider_free() after this initialisation - otherwise it will fail
to free the provider correctly.
Addresses the issue mentioned here:
https://github.com/openssl/openssl/pull/18355#issuecomment-1138741857
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18417)
|
|
If the callback fails then we don't correctly free providers that were
already in our stack and that we up-refed earlier.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18413)
|
|
evp_method_store_flush() and evp_method_store_remove_all_provided()
only cover EVP operations, but not encoders, decoders and store loaders.
This adds corresponding methods for those as well. Without this, their
method stores are never cleaned up when the corresponding providers are
deactivated or otherwise modified.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18151)
|
|
This adds ossl_method_store_remove_all_provided(), which selectively
removes methods from the given store that are provided by the given
provider.
This also adds the EVP specific evp_method_store_remove_all_provided(),
which matches ossl_method_store_remove_all_provided() but can also
retrieve the correct store to manipulate for EVP functions.
This allows us to modify ossl_provider_self_test() to do the job it's
supposed to do, but through clearly defined functions instead of a
cache flushing call that previously did more than that.
ossl_provider_deactivate() is also modified to remove methods associated
with the deactivated provider, and not just clearing the cache.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18151)
|
|
When evp_method_store_flush() flushed the query cache, it also freed
all methods in the EVP method store, through an unfortunate call of
ossl_method_store_flush_cache() with an argument saying that all
methods should indeed be dropped.
To undo some of the confusion, ossl_method_store_flush_cache() is
renamed to ossl_method_store_cache_flush_all(), and limited to do
only that. Some if the items in the internal ALGORITHM structure are
also renamed and commented to clarify what they are for.
Fixes #18150
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18151)
|
|
This is a misused function, as it was called during query cache flush,
when the provider operation bits were meant to record if methods for a
certain operation has already been added to the method store.
Fixes #18150
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18151)
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Release: yes
|
|
This refactors OSSL_LIB_CTX to avoid using CRYPTO_EX_DATA. The assorted
objects to be managed by OSSL_LIB_CTX are hardcoded and are initialized
eagerly rather than lazily, which avoids the need for locking on access
in most cases.
Fixes #17116.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17881)
|
|
Avoid freeing a provider that was not up-ref-ed before.
Fixes #17292
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17295)
|
|
Not holding the flag lock when creating/removing child providers can
confuse the activation counts if the parent provider is loaded/unloaded
at the same time.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
|
|
A "find" operation on a stack can end up sorting the underlying stack. In
this case it is necessary to use a "write" lock to synchronise access to
the stack across multiple threads.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
|
|
A provider may have been activated, but failed when being added to
the store. At this point we still need to deactivate it.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
|
|
We leave it to the caller to confirm that the provider does not exist
in the store. If it does exist then later adding it to the store will
fail.
It is possible that the provider could be added to the store in
between the caller checking, and the caller calling ossl_provider_new.
We leave it to the caller to properly handle the failure when it
attempts to add the provider to the store. This is simpler than
having ossl_provider_new try to handle it.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
|
|
If a provider doesn't have any child providers then there is no need
to attempt to remove them - so we should not do so. This removes some
potentialy thread races.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
|
|
`ossl_provider_set_fallback`
These are legacy of older versions of the code and are currently not used
anywhere.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16985)
|
|
Also add digest parameter documentation for add_sigid and
permit NULL as digest name in the provider upcall.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16770)
|
|
The previous commit provided some guidelines and some rules for using
locking in order to avoid deadlocks. This commit refactors the code in
order to adhere to those guidelines and rules.
Fixes #16312
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16469)
|
|
Provide some guidelines, as well as some rules for using the locks in
provider_core.c, in order to avoid the introduction of deadlocks.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16469)
|
|
If two threads both attempt to load the same provider at the same time,
they will first both check to see if the provider already exists. If it
doesn't then they will both then create new provider objects and call the
init function. However only one of the threads will be successful in adding
the provider to the store. For the "losing" thread we should still return
"success", but we should deinitialise and free the no longer required
provider object, and return the object that exists in the store.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
An earlier stage of the refactor in the last few commits moved this
function out of provider.c because it needed access to the provider
structure internals. The final version however no longer needs this so
it is moved back again.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
Create the OSSL_PROVIDER_INFO to replace struct provider_info_st.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
This restriction was in place to avoid problems with recursive attempts
to aquire the flag lock/store lock from within a provider's init function.
Since those locks are no longer held when calling the init function there
is no reason for the restriction any more.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
Previously providers were added to the store first, and then subsequently
initialised. This meant that during initialisation the provider object
could be shared between multiple threads and hence the locks needed to be
held. However this causes problems because the provider init function is
essentially a user callback and could do virtually anything. There are
many API calls that could be invoked that could subsequently attempt to
acquire the locks. This will fail because the locks are already held.
However, now we have refactored things so that the provider is created and
initialised before being added to the store. Therefore at the point of
initialisation the provider object is not shared with other threads and so
no locks need to be held.
Fixes #15793
Fixes #15712
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
This means we can distinguish providers that have been added to the
store, and those which haven't yet been.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
These 2 functions have become so close to each other that they may as well
be just one function.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
Update use_fallbacks to zero when we add a provider to the store rather
than when we activate it. Its only at the point that we add it to the store
that it is actually usable and visible to other threads.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
Now that a provider is no longer put into the store until after it has
been activated we don't need flag_couldbechild any more. This flag was
used to indicate whether a provider was eligible for conversion into a
child provider or not. This was only really interesting for predefined
providers that were automatically created.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
Rather than creating the provider, adding to the store and then activating
it, we do things the other way around, i.e. activate first and then add to
the store. This means that the activation should occur before other threads
are aware of the provider.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
If provider specified in a config file are not "activated" then we defer
instantiating the provider object until it is actually needed.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
Previously we created the provider object for builtin providers at the
point that OPENSSL_add_builtin() was called. Instead we delay that until
the provider is actually loaded.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
Previously we instantiated all the predefined providers at the point that
we create the provider store. Instead we move them to be instantiated as we
need them.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
|
|
This enables providers to register new OIDs in the same libcrypto instance
as is used by the application.
Fixes #15624
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15681)
|
|
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15680)
|
|
Fixes #15451
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15539)
|
|
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15405)
|
|
If the DRBG is used within the scope of the FIPS OSSL_provider_init
function then it attempts to register a thread callback via c_thread_start.
However the implementation of c_thread_start assumed that the provider's
provctx was already present. However because OSSL_provider_init is still
running it was actually NULL. This means the thread callback fail to work
correctly and a memory leak resulted.
Instead of having c_thread_start use the provctx as the callback argument
we change the definition of c_thread_start to have an explicit callback
argument to use.
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15278)
|
|
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15242)
|
|
Where a child libctx is in use it needs to know what the current global
properties are.
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15242)
|
