From 0211740fcc47a954be19cceb65fb57a6f7deb797 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Wed, 8 May 2019 14:00:31 +0200 Subject: [PATCH] EVP_FETCH: remove the need to transport the legacy NID through construction Now that the legacy NID isn't used as a main index for fetched algorithms, the legacy NID was just transported around unnecessarily. This is removed, and the legacy NID is simply set by EVP_{API}_fetch() after the construction process is done. Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/8878) --- crypto/evp/digest.c | 35 +++++++++++++++--------- crypto/evp/evp_enc.c | 36 ++++++++++++++++--------- crypto/evp/evp_fetch.c | 16 ++++------- crypto/evp/evp_locl.h | 5 ++-- doc/internal/man3/evp_generic_fetch.pod | 31 +++++++++++---------- 5 files changed, 70 insertions(+), 53 deletions(-) diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c index 9b10a7f3fe..dc50528607 100644 --- a/crypto/evp/digest.c +++ b/crypto/evp/digest.c @@ -494,13 +494,14 @@ int EVP_MD_CTX_ctrl(EVP_MD_CTX *ctx, int cmd, int p1, void *p2) return 0; } -static void *evp_md_from_dispatch(int mdtype, const OSSL_DISPATCH *fns, - OSSL_PROVIDER *prov) +static void *evp_md_from_dispatch(const OSSL_DISPATCH *fns, + OSSL_PROVIDER *prov) { EVP_MD *md = NULL; int fncnt = 0; - if ((md = EVP_MD_meth_new(mdtype, NID_undef)) == NULL) + /* EVP_MD_fetch() will set the legacy NID if available */ + if ((md = EVP_MD_meth_new(NID_undef, NID_undef)) == NULL) return NULL; for (; fns->function_id != 0; fns++) { @@ -587,17 +588,25 @@ static void evp_md_free(void *md) EVP_MD_meth_free(md); } -static int evp_md_nid(void *vmd) -{ - EVP_MD *md = vmd; - - return md->type; -} - EVP_MD *EVP_MD_fetch(OPENSSL_CTX *ctx, const char *algorithm, const char *properties) { - return evp_generic_fetch(ctx, OSSL_OP_DIGEST, algorithm, properties, - evp_md_from_dispatch, evp_md_upref, - evp_md_free, evp_md_nid); + EVP_MD *md = + evp_generic_fetch(ctx, OSSL_OP_DIGEST, algorithm, properties, + evp_md_from_dispatch, evp_md_upref, + evp_md_free); + +#ifndef FIPS_MODE + /* TODO(3.x) get rid of the need for legacy NIDs */ + if (md != NULL) { + /* + * FIPS module note: since internal fetches will be entirely + * provider based, we know that none of its code depends on legacy + * NIDs or any functionality that use them. + */ + md->type = OBJ_sn2nid(algorithm); + } +#endif + + return md; } diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c index 29b707a026..c05d48ea02 100644 --- a/crypto/evp/evp_enc.c +++ b/crypto/evp/evp_enc.c @@ -1044,13 +1044,17 @@ int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in) return 1; } -static void *evp_cipher_from_dispatch(int nid, const OSSL_DISPATCH *fns, +static void *evp_cipher_from_dispatch(const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov) { EVP_CIPHER *cipher = NULL; int fnciphcnt = 0, fnctxcnt = 0; - if ((cipher = EVP_CIPHER_meth_new(nid, 0, 0)) == NULL) + /* + * The legacy NID is set by EVP_CIPHER_fetch() if the name exists in + * the object database. + */ + if ((cipher = EVP_CIPHER_meth_new(0, 0, 0)) == NULL) return NULL; for (; fns->function_id != 0; fns++) { @@ -1167,17 +1171,25 @@ static void evp_cipher_free(void *cipher) EVP_CIPHER_meth_free(cipher); } -static int evp_cipher_nid(void *vcipher) -{ - EVP_CIPHER *cipher = vcipher; - - return cipher->nid; -} - EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm, const char *properties) { - return evp_generic_fetch(ctx, OSSL_OP_CIPHER, algorithm, properties, - evp_cipher_from_dispatch, evp_cipher_upref, - evp_cipher_free, evp_cipher_nid); + EVP_CIPHER *cipher = + evp_generic_fetch(ctx, OSSL_OP_CIPHER, algorithm, properties, + evp_cipher_from_dispatch, evp_cipher_upref, + evp_cipher_free); + +#ifndef FIPS_MODE + /* TODO(3.x) get rid of the need for legacy NIDs */ + if (cipher != NULL) { + /* + * FIPS module note: since internal fetches will be entirely + * provider based, we know that none of its code depends on legacy + * NIDs or any functionality that use them. + */ + cipher->nid = OBJ_sn2nid(algorithm); + } +#endif + + return cipher; } diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c index dc66a694a2..fdd6209bc2 100644 --- a/crypto/evp/evp_fetch.c +++ b/crypto/evp/evp_fetch.c @@ -39,13 +39,11 @@ static const OPENSSL_CTX_METHOD default_method_store_method = { struct method_data_st { OPENSSL_CTX *libctx; const char *name; - int nid; + int id; OSSL_METHOD_CONSTRUCT_METHOD *mcm; - void *(*method_from_dispatch)(int nid, const OSSL_DISPATCH *, - OSSL_PROVIDER *); + void *(*method_from_dispatch)(const OSSL_DISPATCH *, OSSL_PROVIDER *); int (*refcnt_up_method)(void *method); void (*destruct_method)(void *method); - int (*nid_method)(void *method); }; /* @@ -121,10 +119,8 @@ static void *construct_method(const char *name, const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov, void *data) { struct method_data_st *methdata = data; - /* TODO(3.0) get rid of the need for legacy NIDs */ - int legacy_nid = OBJ_sn2nid(name); - return methdata->method_from_dispatch(legacy_nid, fns, prov); + return methdata->method_from_dispatch(fns, prov); } static void destruct_method(void *method, void *data) @@ -136,11 +132,10 @@ static void destruct_method(void *method, void *data) void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id, const char *name, const char *properties, - void *(*new_method)(int nid, const OSSL_DISPATCH *fns, + void *(*new_method)(const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov), int (*upref_method)(void *), - void (*free_method)(void *), - int (*nid_method)(void *)) + void (*free_method)(void *)) { OSSL_METHOD_STORE *store = get_default_method_store(libctx); OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx); @@ -168,7 +163,6 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id, mcmdata.destruct_method = free_method; mcmdata.refcnt_up_method = upref_method; mcmdata.destruct_method = free_method; - mcmdata.nid_method = nid_method; method = ossl_method_construct(libctx, operation_id, name, properties, 0 /* !force_cache */, &mcm, &mcmdata); diff --git a/crypto/evp/evp_locl.h b/crypto/evp/evp_locl.h index 0bb6d15afe..8876b061af 100644 --- a/crypto/evp/evp_locl.h +++ b/crypto/evp/evp_locl.h @@ -91,8 +91,7 @@ int is_partially_overlapping(const void *ptr1, const void *ptr2, int len); void *evp_generic_fetch(OPENSSL_CTX *ctx, int operation_id, const char *algorithm, const char *properties, - void *(*new_method)(int nid, const OSSL_DISPATCH *fns, + void *(*new_method)(const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov), int (*upref_method)(void *), - void (*free_method)(void *), - int (*nid_method)(void *)); + void (*free_method)(void *)); diff --git a/doc/internal/man3/evp_generic_fetch.pod b/doc/internal/man3/evp_generic_fetch.pod index 8b0f542515..2679a7eba2 100644 --- a/doc/internal/man3/evp_generic_fetch.pod +++ b/doc/internal/man3/evp_generic_fetch.pod @@ -11,11 +11,10 @@ evp_generic_fetch - generic algorithm fetcher and method creator for EVP void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id, const char *name, const char *properties, - void *(*new_method)(int nid, const OSSL_DISPATCH *fns, + void *(*new_method)(const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov), int (*upref_method)(void *), - void (*free_method)(void *), - int (*nid_method)(void *)); + void (*free_method)(void *)); =head1 DESCRIPTION @@ -42,10 +41,6 @@ one. frees the given method. -=item nid_method() - -returns the nid associated with the given method. - =back =head1 RETURN VALUES @@ -80,7 +75,6 @@ And here's the implementation of the FOO method fetcher: /* typedef struct evp_foo_st EVP_FOO */ struct evp_foo_st { OSSL_PROVIDER *prov; - int nid; CRYPTO_REF_COUNT refcnt; OSSL_OP_foo_newctx_fn *newctx; OSSL_OP_foo_init_fn *init; @@ -93,7 +87,7 @@ And here's the implementation of the FOO method fetcher: * In this example, we have a public method creator and destructor. * It's not absolutely necessary, but is in the spirit of OpenSSL. */ - EVP_FOO *EVP_FOO_meth_from_dispatch(int foo_type, const OSSL_DISPATCH *fns, + EVP_FOO *EVP_FOO_meth_from_dispatch(const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov) { EVP_FOO *foo = NULL; @@ -120,7 +114,6 @@ And here's the implementation of the FOO method fetcher: break; } } - foo->nid = foo_type; foo->prov = prov; if (prov) ossl_provider_upref(prov); @@ -138,10 +131,10 @@ And here's the implementation of the FOO method fetcher: } } - static void *foo_from_dispatch(int nid, const OSSL_DISPATCH *fns, + static void *foo_from_dispatch(const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov) { - return EVP_FOO_meth_from_dispatch(nid, fns, prov); + return EVP_FOO_meth_from_dispatch(fns, prov); } static int foo_upref(void *vfoo) @@ -162,8 +155,18 @@ And here's the implementation of the FOO method fetcher: const char *name, const char *properties) { - return evp_generic_fetch(ctx, OSSL_OP_FOO, name, properties, - foo_from_dispatch, foo_upref, foo_free); + EVP_FOO *foo = + evp_generic_fetch(ctx, OSSL_OP_FOO, name, properties, + foo_from_dispatch, foo_upref, foo_free); + + /* + * If this method exists in legacy form, with a constant NID for the + * given |name|, this is the spot to find that NID and set it in + * the newly constructed EVP_FOO instance. + */ + + return foo; + } And finally, the library functions: