From 2e49c05472ab76cee4e30c2eaa4fa576b9ae92c6 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Sun, 5 May 2019 08:42:21 +0200 Subject: [PATCH] EVP_FETCH: deal with names without pre-defined NIDs We didn't deal very well with names that didn't have pre-defined NIDs, as the NID zero travelled through the full process and resulted in an inaccessible method. By consequence, we need to refactor the method construction callbacks to rely more on algorithm names. We must, however, still store the legacy NID with the method, for the sake of other code that depend on it (for example, CMS). Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/8878) --- crypto/core_fetch.c | 17 +++---- crypto/evp/evp_fetch.c | 51 +++++++++++---------- doc/internal/man3/evp_generic_fetch.pod | 8 ++-- doc/internal/man3/ossl_method_construct.pod | 23 +++++----- include/internal/core.h | 10 ++-- 5 files changed, 57 insertions(+), 52 deletions(-) diff --git a/crypto/core_fetch.c b/crypto/core_fetch.c index 6c4ed6a30a..227f920713 100644 --- a/crypto/core_fetch.c +++ b/crypto/core_fetch.c @@ -56,14 +56,14 @@ static int ossl_method_construct_this(OSSL_PROVIDER *provider, void *cbdata) * If we haven't been told not to store, * add to the global store */ - data->mcm->put(data->libctx, NULL, - thismap->property_definition, - method, data->mcm_data); + data->mcm->put(data->libctx, NULL, method, + thismap->algorithm_name, + thismap->property_definition, data->mcm_data); } - data->mcm->put(data->libctx, data->store, - thismap->property_definition, - method, data->mcm_data); + data->mcm->put(data->libctx, data->store, method, + thismap->algorithm_name, thismap->property_definition, + data->mcm_data); /* refcnt-- because we're dropping the reference */ data->mcm->destruct(method, data->mcm_data); @@ -79,7 +79,8 @@ void *ossl_method_construct(OPENSSL_CTX *libctx, int operation_id, { void *method = NULL; - if ((method = mcm->get(libctx, NULL, propquery, mcm_data)) == NULL) { + if ((method = + mcm->get(libctx, NULL, name, propquery, mcm_data)) == NULL) { struct construct_data_st cbdata; /* @@ -97,7 +98,7 @@ void *ossl_method_construct(OPENSSL_CTX *libctx, int operation_id, ossl_provider_forall_loaded(libctx, ossl_method_construct_this, &cbdata); - method = mcm->get(libctx, cbdata.store, propquery, mcm_data); + method = mcm->get(libctx, cbdata.store, name, propquery, mcm_data); mcm->dealloc_tmp_store(cbdata.store); } diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c index 73035f6fde..dc66a694a2 100644 --- a/crypto/evp/evp_fetch.c +++ b/crypto/evp/evp_fetch.c @@ -69,16 +69,23 @@ static OSSL_METHOD_STORE *get_default_method_store(OPENSSL_CTX *libctx) } static void *get_method_from_store(OPENSSL_CTX *libctx, void *store, - const char *propquery, void *data) + const char *name, const char *propquery, + void *data) { struct method_data_st *methdata = data; void *method = NULL; + OSSL_NAMEMAP *namemap; + int id; if (store == NULL && (store = get_default_method_store(libctx)) == NULL) return NULL; - (void)ossl_method_store_fetch(store, methdata->nid, propquery, &method); + if ((namemap = ossl_namemap_stored(libctx)) == NULL + || (id = ossl_namemap_add(namemap, name)) == 0) + return NULL; + + (void)ossl_method_store_fetch(store, id, propquery, &method); if (method != NULL && !methdata->refcnt_up_method(method)) { @@ -88,13 +95,15 @@ static void *get_method_from_store(OPENSSL_CTX *libctx, void *store, } static int put_method_in_store(OPENSSL_CTX *libctx, void *store, - const char *propdef, - void *method, void *data) + void *method, const char *name, + const char *propdef, void *data) { struct method_data_st *methdata = data; - int nid = methdata->nid_method(method); + OSSL_NAMEMAP *namemap; + int id; - if (nid == 0) + if ((namemap = ossl_namemap_stored(methdata->libctx)) == NULL + || (id = ossl_namemap_add(namemap, name)) == 0) return 0; if (store == NULL @@ -102,25 +111,20 @@ static int put_method_in_store(OPENSSL_CTX *libctx, void *store, return 0; if (methdata->refcnt_up_method(method) - && ossl_method_store_add(store, nid, propdef, method, + && ossl_method_store_add(store, id, propdef, method, methdata->destruct_method)) return 1; return 0; } -static void *construct_method(const char *algorithm_name, - const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov, - void *data) +static void *construct_method(const char *name, const OSSL_DISPATCH *fns, + OSSL_PROVIDER *prov, void *data) { struct method_data_st *methdata = data; - OSSL_NAMEMAP *namemap; - int nid; + /* TODO(3.0) get rid of the need for legacy NIDs */ + int legacy_nid = OBJ_sn2nid(name); - if ((namemap = ossl_namemap_stored(methdata->libctx)) == NULL - || (nid = ossl_namemap_add(namemap, algorithm_name)) == 0) - return NULL; - - return methdata->method_from_dispatch(nid, fns, prov); + return methdata->method_from_dispatch(legacy_nid, fns, prov); } static void destruct_method(void *method, void *data) @@ -131,7 +135,7 @@ static void destruct_method(void *method, void *data) } void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id, - const char *algorithm, const char *properties, + const char *name, const char *properties, void *(*new_method)(int nid, const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov), int (*upref_method)(void *), @@ -140,14 +144,14 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id, { OSSL_METHOD_STORE *store = get_default_method_store(libctx); OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx); - int nid; + int id; void *method = NULL; if (store == NULL || namemap == NULL) return NULL; - if ((nid = ossl_namemap_number(namemap, algorithm)) == 0 - || !ossl_method_store_cache_get(store, nid, properties, &method)) { + if ((id = ossl_namemap_number(namemap, name)) == 0 + || !ossl_method_store_cache_get(store, id, properties, &method)) { OSSL_METHOD_CONSTRUCT_METHOD mcm = { alloc_tmp_method_store, dealloc_tmp_method_store, @@ -158,7 +162,6 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id, }; struct method_data_st mcmdata; - mcmdata.nid = nid; mcmdata.mcm = &mcm; mcmdata.libctx = libctx; mcmdata.method_from_dispatch = new_method; @@ -166,10 +169,10 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id, mcmdata.refcnt_up_method = upref_method; mcmdata.destruct_method = free_method; mcmdata.nid_method = nid_method; - method = ossl_method_construct(libctx, operation_id, algorithm, + method = ossl_method_construct(libctx, operation_id, name, properties, 0 /* !force_cache */, &mcm, &mcmdata); - ossl_method_store_cache_set(store, nid, properties, method); + ossl_method_store_cache_set(store, id, properties, method); } else { upref_method(method); } diff --git a/doc/internal/man3/evp_generic_fetch.pod b/doc/internal/man3/evp_generic_fetch.pod index 881aaf954a..8b0f542515 100644 --- a/doc/internal/man3/evp_generic_fetch.pod +++ b/doc/internal/man3/evp_generic_fetch.pod @@ -10,7 +10,7 @@ evp_generic_fetch - generic algorithm fetcher and method creator for EVP #include "evp_locl.h" void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id, - const char *algorithm, const char *properties, + const char *name, const char *properties, void *(*new_method)(int nid, const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov), int (*upref_method)(void *), @@ -20,7 +20,7 @@ evp_generic_fetch - generic algorithm fetcher and method creator for EVP =head1 DESCRIPTION evp_generic_fetch() calls ossl_method_construct() with the given -C, C, C, and C and uses +C, C, C, and C and uses it to create an EVP method with the help of the functions C, C, and C. @@ -159,10 +159,10 @@ And here's the implementation of the FOO method fetcher: } EVP_FOO *EVP_FOO_fetch(OPENSSL_CTX *ctx, - const char *algorithm, + const char *name, const char *properties) { - return evp_generic_fetch(ctx, OSSL_OP_FOO, algorithm, properties, + return evp_generic_fetch(ctx, OSSL_OP_FOO, name, properties, foo_from_dispatch, foo_upref, foo_free); } diff --git a/doc/internal/man3/ossl_method_construct.pod b/doc/internal/man3/ossl_method_construct.pod index 47f4a24e5c..60de260e4a 100644 --- a/doc/internal/man3/ossl_method_construct.pod +++ b/doc/internal/man3/ossl_method_construct.pod @@ -15,13 +15,13 @@ OSSL_METHOD_CONSTRUCT_METHOD, ossl_method_construct /* Remove a store */ void (*dealloc_tmp_store)(void *store); /* Get an already existing method from a store */ - void *(*get)(OPENSSL_CTX *libctx, void *store, const char *propquery, - void *data); + void *(*get)(OPENSSL_CTX *libctx, void *store, const char *name, + const char *propquery, void *data); /* Store a method in a store */ - int (*put)(OPENSSL_CTX *libctx, void *store, const char *propdef, - void *method, void *data); + int (*put)(OPENSSL_CTX *libctx, void *store, void *method, + const char *name, const char *propdef, void *data); /* Construct a new method */ - void *(*construct)(const char *algorithm_name, const OSSL_DISPATCH *fns, + void *(*construct)(const char *name, const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov, void *data); /* Destruct a method */ void (*destruct)(void *method); @@ -77,14 +77,15 @@ Remove a temporary store. =item get() -Look up an already existing method from a store. +Look up an already existing method from a store by name. The store may be given with C. B is a valid value and means that a sub-system default store must be used. This default store should be stored in the library context C. -The method to be looked up should be identified with data from C +The method to be looked up should be identified with the given C and +data from C (which is the C that was passed to ossl_construct_method()) and the provided property query C. @@ -100,15 +101,15 @@ B is a valid value and means that a sub-system default store must be used. This default store should be stored in the library context C. -The method should be associated with the given property definition -C and any identification data given through C (which is +The method should be associated with the given C and property definition +C as well as any identification data given through C (which is the C that was passed to ossl_construct_method()). This function is expected to increment the C's reference count. =item construct() -Constructs a sub-system method for the given C and the given +Constructs a sub-system method for the given C and the given dispatch table C. The associated I C is passed as well, to make @@ -133,7 +134,7 @@ B on error. =head1 HISTORY -This functionality was added to OpenSSL 3.0.0. +This functionality was added to OpenSSL 3.0. =head1 COPYRIGHT diff --git a/include/internal/core.h b/include/internal/core.h index ddafaeec09..64547dca40 100644 --- a/include/internal/core.h +++ b/include/internal/core.h @@ -32,13 +32,13 @@ typedef struct ossl_method_construct_method_st { /* Remove a store */ void (*dealloc_tmp_store)(void *store); /* Get an already existing method from a store */ - void *(*get)(OPENSSL_CTX *libctx, void *store, const char *propquery, - void *data); + void *(*get)(OPENSSL_CTX *libctx, void *store, const char *name, + const char *propquery, void *data); /* Store a method in a store */ - int (*put)(OPENSSL_CTX *libctx, void *store, const char *propdef, - void *method, void *data); + int (*put)(OPENSSL_CTX *libctx, void *store, void *method, + const char *name, const char *propdef, void *data); /* Construct a new method */ - void *(*construct)(const char *algorithm_name, const OSSL_DISPATCH *fns, + void *(*construct)(const char *name, const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov, void *data); /* Destruct a method */ void (*destruct)(void *method, void *data); -- 2.25.1