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 6c4ed6a..227f920 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 73035f6..dc66a69 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 881aaf9..8b0f542 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 47f4a24..60de260 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 ddafaee..64547dc 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);