EVP_FETCH: deal with names without pre-defined NIDs
authorRichard Levitte <levitte@openssl.org>
Sun, 5 May 2019 06:42:21 +0000 (08:42 +0200)
committerRichard Levitte <levitte@openssl.org>
Sun, 12 May 2019 20:43:38 +0000 (13:43 -0700)
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 <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8878)

crypto/core_fetch.c
crypto/evp/evp_fetch.c
doc/internal/man3/evp_generic_fetch.pod
doc/internal/man3/ossl_method_construct.pod
include/internal/core.h

index 6c4ed6a30ad7041e7f7387657126fa9510e097dc..227f92071323d7cd36778d24a0a624280230cc0d 100644 (file)
@@ -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);
     }
 
index 73035f6fdee46796e6786e7e0c5e4759daa7eedd..dc66a694a28dcecc676306802120c7ba8f3c4d16 100644 (file)
@@ -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);
     }
index 881aaf954a5bbf87519d50a0b56df0b1b6ddbce9..8b0f5425154aec8c1f4956cfc019f0106f2ce040 100644 (file)
@@ -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<libctx>, C<operation_id>, C<algorithm>, and C<properties> and uses
+C<libctx>, C<operation_id>, C<name>, and C<properties> and uses
 it to create an EVP method with the help of the functions
 C<new_method>, C<upref_method>, and C<free_method>.
 
@@ -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);
     }
 
index 47f4a24e5cfbc0dc78ed5f5f7ce32e26f5738ef2..60de260e4ab93e61462c6a883d14437518adee2e 100644 (file)
@@ -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<store>.
 B<NULL> 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<libctx>.
 
-The method to be looked up should be identified with data from C<data>
+The method to be looked up should be identified with the given C<name> and
+data from C<data>
 (which is the C<mcm_data> that was passed to ossl_construct_method())
 and the provided property query C<propquery>.
 
@@ -100,15 +101,15 @@ B<NULL> 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<libctx>.
 
-The method should be associated with the given property definition
-C<propdef> and any identification data given through C<data> (which is
+The method should be associated with the given C<name> and property definition
+C<propdef> as well as any identification data given through C<data> (which is
 the C<mcm_data> that was passed to ossl_construct_method()).
 
 This function is expected to increment the C<method>'s reference count.
 
 =item construct()
 
-Constructs a sub-system method for the given C<algorithm_name> and the given
+Constructs a sub-system method for the given C<name> and the given
 dispatch table C<fns>.
 
 The associated I<provider object> C<prov> is passed as well, to make
@@ -133,7 +134,7 @@ B<NULL> on error.
 
 =head1 HISTORY
 
-This functionality was added to OpenSSL 3.0.0.
+This functionality was added to OpenSSL 3.0.
 
 =head1 COPYRIGHT
 
index ddafaeec09bc1136a090cfc00646bca21d0cebb3..64547dca400e191e8977d900a1348b9058c95756 100644 (file)
@@ -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);