Commit Graph

140 Commits

Author SHA1 Message Date
Pauli
f80910390c Coverity 1503321 & 1503327: dereference after null check
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)
2022-07-17 17:51:13 +10:00
Richard Levitte
9574842e90 Pre-declare all core dispatch table functions, and fix the internal ones
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)
2022-07-13 07:56:51 +02:00
Peiwei Hu
61f510600e crypto/provider_core.c: Avoid calling unlock two times
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18641)
2022-06-28 17:48:19 +02:00
Tomas Mraz
b8fd15a8dc Avoid including decoder/encoder/store headers into fips module
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 b07db93f6b)
2022-06-24 08:26:42 +02:00
Matt Caswell
c4ed6f6f0e Don't call ossl_provider_free() without first setting refcnt
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)
2022-06-02 10:31:12 +01:00
Matt Caswell
b4be10dfcd Fix a memory leak is ossl_provider_doall_activated
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)
2022-06-02 10:31:12 +01:00
Richard Levitte
32e3c07137 Add method store cache flush and method removal to non-EVP operations
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)
2022-05-05 15:06:12 +02:00
Richard Levitte
2e4d0677ea Make it possible to remove methods by the provider that provides them
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)
2022-05-05 15:05:55 +02:00
Richard Levitte
60640d79ca Don't empty the method store when flushing the query cache
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)
2022-05-05 15:05:54 +02:00
Richard Levitte
20b6d85ab2 Drop ossl_provider_clear_all_operation_bits() and all uses of it
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)
2022-05-05 15:05:54 +02:00
Matt Caswell
fecb3aae22 Update copyright year
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Release: yes
2022-05-03 13:34:51 +01:00
Hugo Landau
927d0566de Refactor OSSL_LIB_CTX to avoid using CRYPTO_EX_DATA
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)
2022-04-01 10:49:19 +11:00
Tomas Mraz
33df7cbe5e ossl_provider_add_to_store: Avoid use-after-free
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)
2021-12-17 17:33:49 +01:00
Matt Caswell
addbd7c9d7 Hold the flag_lock when calling child callbacks
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)
2021-11-12 17:16:14 +00:00
Matt Caswell
4aced11785 Use a write lock during ossl_provider_find()
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)
2021-11-12 17:16:14 +00:00
Matt Caswell
e39bd62151 Don't bail out during provider deactivation if we don't have store
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)
2021-11-12 17:16:14 +00:00
Matt Caswell
dc6d9ede62 Don't try and do ossl_provider_find in ossl_provider_new
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)
2021-11-12 17:16:14 +00:00
Matt Caswell
c59fc87b33 Don't attempt to deactive child providers if we don't need to
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)
2021-11-12 17:16:14 +00:00
Pauli
90c311315c prov: remove unused field flag_fallback and function 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)
2021-11-10 09:26:11 +10:00
Michael Baentsch
4f71624964 Permit no/empty digest in core_obj_add_sigid
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)
2021-10-22 16:26:46 +02:00
Matt Caswell
2b4a611ef1 Refactor provider_core.c to adhere to the locking rules
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)
2021-08-31 20:44:16 +10:00
Matt Caswell
03c137de97 Add commentary about lock usage in provider_core.c
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)
2021-08-31 20:44:16 +10:00
Matt Caswell
59a783d05a Fix a race in ossl_provider_add_to_store()
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)
2021-06-24 14:48:15 +01:00
Matt Caswell
d382c46525 Move OPENSSL_add_builtin back into provider.c
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)
2021-06-24 14:48:15 +01:00
Matt Caswell
b7248964a4 make struct provider_info_st a full type
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)
2021-06-24 14:48:15 +01:00
Matt Caswell
549b5cb4ed Don't skip the current provider in ossl_provider_register_child_cb
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)
2021-06-24 14:48:15 +01:00
Matt Caswell
f109e96559 Don't hold any locks while calling the provider init function
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)
2021-06-24 14:48:15 +01:00
Matt Caswell
b91687c567 Only associate a provider with a store once it has been added to it
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)
2021-06-24 14:48:15 +01:00
Matt Caswell
814c2018e1 Merge ossl_provider_activate() and ossl_provider_activate_child()
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)
2021-06-24 14:48:14 +01:00
Matt Caswell
eb2263da9a Set use_fallbacks to zero when we add a provider to the store
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)
2021-06-24 14:48:14 +01:00
Matt Caswell
d5fbd5b4ed Remove flag_couldbechild
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)
2021-06-24 14:48:14 +01:00
Matt Caswell
29aff65315 Add a new provider to the store only after we activate it
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)
2021-06-24 14:48:14 +01:00
Matt Caswell
352d482a29 Instantiate configuration supplied providers when we need them
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)
2021-06-24 14:48:14 +01:00
Matt Caswell
1d74203cf5 Instantiate user-added builtin providers when we need them
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)
2021-06-24 14:48:14 +01:00
Matt Caswell
8d4dec0d4b Instantiate predefined providers just-in-time
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)
2021-06-24 14:48:14 +01:00
Matt Caswell
97abae6a9d Add various OBJ functions as callbacks
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)
2021-06-16 15:04:50 +10:00
Tomas Mraz
6cf811e867 ossl_provider_set_module_path: Prevent potential UAF
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15680)
2021-06-11 08:52:14 +10:00
Pauli
71653965b3 crypto: remove TODOs
Fixes #15451

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15539)
2021-06-02 16:30:15 +10:00
Tomas Mraz
c4e9167437 Rename also the OSSL_PROVIDER_name() function
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15405)
2021-06-01 12:43:57 +02:00
Matt Caswell
c9732f0953 Fix a memleak in the FIPS provider
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)
2021-05-24 09:39:15 +10:00
Matt Caswell
b195677073 Update documentation for global properties mirroring
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15242)
2021-05-20 09:35:41 +01:00
Matt Caswell
447588b69a Add a callback for providers to know about global properties changes
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)
2021-05-20 09:32:40 +01:00
Pauli
634da876e0 provider: fix coverity 1484884: uninitialised lock use
This actually fixes a more subtle problem that wasn't detected which could
cause memory leaks.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/15300)
2021-05-18 13:24:41 +10:00
Matt Caswell
36a89c0439 Init the child providers immediately on creation of the child libctx
We were deferring the initial creation of the child providers until the
first fetch. This is a carry over from an earlier iteration of the child
lib ctx development and is no longer necessary. In fact we need to init
the child providers immediately otherwise not all providers quite init
correctly.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15270)
2021-05-15 10:33:42 +01:00
Matt Caswell
c1fb5e072f Exclude child provider code from the FIPS module
We don't need the child provider code in the FIPS module so we exclude
it.

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14991)
2021-05-11 15:03:13 +01:00
Matt Caswell
abaa2dd298 Don't convert pre-existing providers into children
If a provider explicitly loads another provider into a child libctx where
it wasn't previously loaded then we don't start treating it like a child
if the parent libctx subsequently loads the same provider.

Fixes #14925

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14991)
2021-05-11 15:03:13 +01:00
Matt Caswell
8c62707565 Add support for child provider to up_ref/free their parent
If the ref counts on a child provider change, then this needs to be
reflected in the parent so we add callbacks to do this.

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14991)
2021-05-11 14:59:43 +01:00
Matt Caswell
7b88c184b6 Register callbacks with core for child provider creation/deletion
By adding callbacks to the core this will enable (in future commits) the
ability to add/remove child providers as the providers are added/removed
from the parent libctx.

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14991)
2021-05-11 14:59:43 +01:00
Matt Caswell
f12a5690de Add the concept of a child OSSL_LIB_CTX
Add a child OSSL_LIB_CTX that will mirror the providers loaded into the
parent libctx. This is useful for providers that want to use algorithms
from other providers and just need to inherit the providers used by the
application.

Fixes #14925

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14991)
2021-05-11 14:56:55 +01:00
Matt Caswell
a16d21744d Add the ability for ex_data to have a priority
Where an object has multiple ex_data associated with it, then we free that
ex_data in order of priority (high priority first).

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14991)
2021-05-11 14:56:55 +01:00