Modify ossl_method_store_add() to accept an OSSL_PROVIDER and check for it

If ossl_method_store_add() gets called with a method that already exists
(i.e. the store has one with matching provider, nid and properties), that
method should not be stored.  We do this check inside ossl_method_store_add()
because it has all the locking required to do so safely.

Fixes #9561

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9650)
This commit is contained in:
Richard Levitte 2019-08-21 10:08:44 +02:00
parent b1d40ddfe2
commit c1d56231ef
7 changed files with 47 additions and 29 deletions

View File

@ -52,14 +52,14 @@ static void ossl_method_construct_this(OSSL_PROVIDER *provider,
* If we haven't been told not to store, * If we haven't been told not to store,
* add to the global store * add to the global store
*/ */
data->mcm->put(data->libctx, NULL, method, data->operation_id, data->mcm->put(data->libctx, NULL, method, provider,
algo->algorithm_name, data->operation_id, algo->algorithm_name,
algo->property_definition, data->mcm_data); algo->property_definition, data->mcm_data);
} }
data->mcm->put(data->libctx, data->store, method, data->operation_id, data->mcm->put(data->libctx, data->store, method, provider,
algo->algorithm_name, algo->property_definition, data->operation_id, algo->algorithm_name,
data->mcm_data); algo->property_definition, data->mcm_data);
/* refcnt-- because we're dropping the reference */ /* refcnt-- because we're dropping the reference */
data->mcm->destruct(method, data->mcm_data); data->mcm->destruct(method, data->mcm_data);

View File

@ -114,9 +114,9 @@ static void *get_method_from_store(OPENSSL_CTX *libctx, void *store,
} }
static int put_method_in_store(OPENSSL_CTX *libctx, void *store, static int put_method_in_store(OPENSSL_CTX *libctx, void *store,
void *method, int operation_id, void *method, const OSSL_PROVIDER *prov,
const char *name, const char *propdef, int operation_id, const char *name,
void *data) const char *propdef, void *data)
{ {
struct method_data_st *methdata = data; struct method_data_st *methdata = data;
OSSL_NAMEMAP *namemap; OSSL_NAMEMAP *namemap;
@ -132,7 +132,7 @@ static int put_method_in_store(OPENSSL_CTX *libctx, void *store,
&& (store = get_default_method_store(libctx)) == NULL) && (store = get_default_method_store(libctx)) == NULL)
return 0; return 0;
return ossl_method_store_add(store, methid, propdef, method, return ossl_method_store_add(store, prov, methid, propdef, method,
methdata->refcnt_up_method, methdata->refcnt_up_method,
methdata->destruct_method); methdata->destruct_method);
} }

View File

@ -25,6 +25,7 @@
#define IMPL_CACHE_FLUSH_THRESHOLD 500 #define IMPL_CACHE_FLUSH_THRESHOLD 500
typedef struct { typedef struct {
const OSSL_PROVIDER *provider;
OSSL_PROPERTY_LIST *properties; OSSL_PROPERTY_LIST *properties;
void *method; void *method;
void (*method_destruct)(void *); void (*method_destruct)(void *);
@ -173,7 +174,7 @@ static int ossl_method_store_insert(OSSL_METHOD_STORE *store, ALGORITHM *alg)
return ossl_sa_ALGORITHM_set(store->algs, alg->nid, alg); return ossl_sa_ALGORITHM_set(store->algs, alg->nid, alg);
} }
int ossl_method_store_add(OSSL_METHOD_STORE *store, int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
int nid, const char *properties, void *method, int nid, const char *properties, void *method,
int (*method_up_ref)(void *), int (*method_up_ref)(void *),
void (*method_destruct)(void *)) void (*method_destruct)(void *))
@ -181,6 +182,7 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store,
ALGORITHM *alg = NULL; ALGORITHM *alg = NULL;
IMPLEMENTATION *impl; IMPLEMENTATION *impl;
int ret = 0; int ret = 0;
int i;
if (nid <= 0 || method == NULL || store == NULL) if (nid <= 0 || method == NULL || store == NULL)
return 0; return 0;
@ -191,8 +193,11 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store,
impl = OPENSSL_malloc(sizeof(*impl)); impl = OPENSSL_malloc(sizeof(*impl));
if (impl == NULL) if (impl == NULL)
return 0; return 0;
if (method_up_ref != NULL && !method_up_ref(method)) if (method_up_ref != NULL && !method_up_ref(method)) {
OPENSSL_free(impl);
return 0; return 0;
}
impl->provider = prov;
impl->method = method; impl->method = method;
impl->method_destruct = method_destruct; impl->method_destruct = method_destruct;
@ -222,8 +227,16 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store,
goto err; goto err;
} }
/* Push onto stack */ /* Push onto stack if there isn't one there already */
if (sk_IMPLEMENTATION_push(alg->impls, impl)) for (i = 0; i < sk_IMPLEMENTATION_num(alg->impls); i++) {
const IMPLEMENTATION *tmpimpl = sk_IMPLEMENTATION_value(alg->impls, i);
if (tmpimpl->provider == impl->provider
&& tmpimpl->properties == impl->properties)
break;
}
if (i == sk_IMPLEMENTATION_num(alg->impls)
&& sk_IMPLEMENTATION_push(alg->impls, impl))
ret = 1; ret = 1;
ossl_property_unlock(store); ossl_property_unlock(store);
if (ret == 0) if (ret == 0)

View File

@ -19,7 +19,7 @@ ossl_method_store_cache_get, ossl_method_store_cache_set
void ossl_method_store_free(OSSL_METHOD_STORE *store); void ossl_method_store_free(OSSL_METHOD_STORE *store);
int ossl_method_store_init(OPENSSL_CTX *ctx); int ossl_method_store_init(OPENSSL_CTX *ctx);
void ossl_method_store_cleanup(OPENSSL_CTX *ctx); void ossl_method_store_cleanup(OPENSSL_CTX *ctx);
int ossl_method_store_add(OSSL_METHOD_STORE *store, int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
int nid, const char *properties, void *method, int nid, const char *properties, void *method,
int (*method_up_ref)(void *), int (*method_up_ref)(void *),
void (*method_destruct)(void *)); void (*method_destruct)(void *));
@ -63,8 +63,10 @@ B<ctx> to allow access to the required underlying property data.
ossl_method_store_free() frees resources allocated to B<store>. ossl_method_store_free() frees resources allocated to B<store>.
ossl_method_store_add() adds the B<method> to the B<store> as an instance of an ossl_method_store_add() adds the B<method> constructed from an implementation in
algorithm indicated by B<nid> and the property definition B<properties>. the provider B<prov> to the B<store> as an instance of an algorithm indicated by
B<nid> and the property definition B<properties>, unless the B<store> already
has a method from the same provider with the same B<nid> and B<properties>.
If the B<method_up_ref> function is given, it's called to increment the If the B<method_up_ref> function is given, it's called to increment the
reference count of the method. reference count of the method.
If the B<method_destruct> function is given, it's called when this function If the B<method_destruct> function is given, it's called when this function

View File

@ -37,8 +37,8 @@ typedef struct ossl_method_construct_method_st {
void *data); void *data);
/* Store a method in a store */ /* Store a method in a store */
int (*put)(OPENSSL_CTX *libctx, void *store, void *method, int (*put)(OPENSSL_CTX *libctx, void *store, void *method,
int operation_id, const char *name, const char *propdef, const OSSL_PROVIDER *prov, int operation_id, const char *name,
void *data); const char *propdef, void *data);
/* Construct a new method */ /* Construct a new method */
void *(*construct)(const char *name, const OSSL_DISPATCH *fns, void *(*construct)(const char *name, const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov, void *data); OSSL_PROVIDER *prov, void *data);

View File

@ -18,12 +18,12 @@ typedef struct ossl_method_store_st OSSL_METHOD_STORE;
/* Implementation store functions */ /* Implementation store functions */
OSSL_METHOD_STORE *ossl_method_store_new(OPENSSL_CTX *ctx); OSSL_METHOD_STORE *ossl_method_store_new(OPENSSL_CTX *ctx);
void ossl_method_store_free(OSSL_METHOD_STORE *store); void ossl_method_store_free(OSSL_METHOD_STORE *store);
int ossl_method_store_add(OSSL_METHOD_STORE *store, int nid, int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
const char *properties, void *implementation, int nid, const char *properties, void *method,
int (*implementation_up_ref)(void *), int (*method_up_ref)(void *),
void (*implementation_destruct)(void *)); void (*method_destruct)(void *));
int ossl_method_store_remove(OSSL_METHOD_STORE *store, int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid,
int nid, const void *implementation); const void *method);
int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid, int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
const char *prop_query, void **result); const char *prop_query, void **result);
int ossl_method_store_set_global_properties(OSSL_METHOD_STORE *store, int ossl_method_store_set_global_properties(OSSL_METHOD_STORE *store,

View File

@ -240,8 +240,9 @@ static int test_register_deregister(void)
goto err; goto err;
for (i = 0; i < OSSL_NELEM(impls); i++) for (i = 0; i < OSSL_NELEM(impls); i++)
if (!TEST_true(ossl_method_store_add(store, impls[i].nid, impls[i].prop, if (!TEST_true(ossl_method_store_add(store, NULL, impls[i].nid,
impls[i].impl, NULL, NULL))) { impls[i].prop, impls[i].impl,
NULL, NULL))) {
TEST_note("iteration %zd", i + 1); TEST_note("iteration %zd", i + 1);
goto err; goto err;
} }
@ -307,8 +308,9 @@ static int test_property(void)
goto err; goto err;
for (i = 0; i < OSSL_NELEM(impls); i++) for (i = 0; i < OSSL_NELEM(impls); i++)
if (!TEST_true(ossl_method_store_add(store, impls[i].nid, impls[i].prop, if (!TEST_true(ossl_method_store_add(store, NULL, impls[i].nid,
impls[i].impl, NULL, NULL))) { impls[i].prop, impls[i].impl,
NULL, NULL))) {
TEST_note("iteration %zd", i + 1); TEST_note("iteration %zd", i + 1);
goto err; goto err;
} }
@ -347,7 +349,8 @@ static int test_query_cache_stochastic(void)
for (i = 1; i <= max; i++) { for (i = 1; i <= max; i++) {
v[i] = 2 * i; v[i] = 2 * i;
BIO_snprintf(buf, sizeof(buf), "n=%d\n", i); BIO_snprintf(buf, sizeof(buf), "n=%d\n", i);
if (!TEST_true(ossl_method_store_add(store, i, buf, "abc", NULL, NULL)) if (!TEST_true(ossl_method_store_add(store, NULL, i, buf, "abc",
NULL, NULL))
|| !TEST_true(ossl_method_store_cache_set(store, i, buf, v + i)) || !TEST_true(ossl_method_store_cache_set(store, i, buf, v + i))
|| !TEST_true(ossl_method_store_cache_set(store, i, "n=1234", || !TEST_true(ossl_method_store_cache_set(store, i, "n=1234",
"miss"))) { "miss"))) {