[openssl] master update

Richard Levitte levitte at openssl.org
Mon Jun 10 06:03:30 UTC 2019


The branch master has been updated
       via  b4d3f203da6210f148b2a0c7bf5a802b55ba0e65 (commit)
       via  2ccb1b4ecab2c3ac1dc2ff81a48869a79afa7839 (commit)
      from  a08714e18131b1998faa0113e5bd4024044654ac (commit)


- Log -----------------------------------------------------------------
commit b4d3f203da6210f148b2a0c7bf5a802b55ba0e65
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Jun 7 12:30:01 2019 +0200

    doc/internal/man3/ossl_method_construct.pod: follow common conventions
    
    Reviewed-by: Shane Lontis <shane.lontis at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/9109)

commit 2ccb1b4ecab2c3ac1dc2ff81a48869a79afa7839
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Jun 7 11:44:08 2019 +0200

    EVP fetching: make operation_id part of the method identity
    
    Because the operation identity wasn't integrated with the created
    methods, the following code would give unexpected results:
    
        EVP_MD *md = EVP_MD_fetch(NULL, "MD5", NULL);
        EVP_CIPHER *cipher = EVP_CIPHER_fetch(NULL, "MD5", NULL);
    
        if (md != NULL)
            printf("MD5 is a digest\n");
        if (cipher != NULL)
            printf("MD5 is a cipher\n");
    
    The message is that MD5 is both a digest and a cipher.
    
    Partially fixes #9106
    
    Reviewed-by: Shane Lontis <shane.lontis at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/9109)

-----------------------------------------------------------------------

Summary of changes:
 crypto/core_fetch.c                         | 10 ++--
 crypto/evp/evp_fetch.c                      | 86 +++++++++++++++++++++++------
 doc/internal/man3/ossl_method_construct.pod | 65 +++++++++++++---------
 include/internal/core.h                     |  8 ++-
 4 files changed, 119 insertions(+), 50 deletions(-)

diff --git a/crypto/core_fetch.c b/crypto/core_fetch.c
index a99f092..56a3c5c 100644
--- a/crypto/core_fetch.c
+++ b/crypto/core_fetch.c
@@ -59,12 +59,12 @@ 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, method,
+            data->mcm->put(data->libctx, NULL, method, data->operation_id,
                            thismap->algorithm_name,
                            thismap->property_definition, data->mcm_data);
         }
 
-        data->mcm->put(data->libctx, data->store, method,
+        data->mcm->put(data->libctx, data->store, method, data->operation_id,
                        thismap->algorithm_name, thismap->property_definition,
                        data->mcm_data);
 
@@ -83,7 +83,8 @@ void *ossl_method_construct(OPENSSL_CTX *libctx, int operation_id,
     void *method = NULL;
 
     if ((method =
-         mcm->get(libctx, NULL, name, propquery, mcm_data)) == NULL) {
+         mcm->get(libctx, NULL, operation_id, name, propquery, mcm_data))
+        == NULL) {
         struct construct_data_st cbdata;
 
         /*
@@ -101,7 +102,8 @@ 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, name, propquery, mcm_data);
+        method = mcm->get(libctx, cbdata.store, operation_id, 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 d3b5bca..1c9e27d 100644
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -39,7 +39,6 @@ static const OPENSSL_CTX_METHOD default_method_store_method = {
 struct method_data_st {
     OPENSSL_CTX *libctx;
     const char *name;
-    int id;
     OSSL_METHOD_CONSTRUCT_METHOD *mcm;
     void *(*method_from_dispatch)(const OSSL_DISPATCH *, OSSL_PROVIDER *);
     int (*refcnt_up_method)(void *method);
@@ -66,24 +65,45 @@ static OSSL_METHOD_STORE *get_default_method_store(OPENSSL_CTX *libctx)
                                 &default_method_store_method);
 }
 
+/*
+ * To identity the method in the method store, we mix the name identity
+ * with the operation identity, with the assumption that we don't have
+ * more than 2^24 names or more than 2^8 operation types.
+ *
+ * The resulting identity is a 32-bit integer, composed like this:
+ *
+ * +---------24 bits--------+-8 bits-+
+ * |      name identity     | op id  |
+ * +------------------------+--------+
+ */
+static uint32_t method_id(unsigned int operation_id, unsigned int name_id)
+{
+    if (!ossl_assert(name_id < (1 << 24) || operation_id < (1 << 8))
+        || !ossl_assert(name_id > 0 && operation_id > 0))
+        return 0;
+    return ((name_id << 8) & 0xFFFFFF00) | (operation_id & 0x000000FF);
+}
+
 static void *get_method_from_store(OPENSSL_CTX *libctx, void *store,
-                                   const char *name, const char *propquery,
-                                   void *data)
+                                   int operation_id, const char *name,
+                                   const char *propquery, void *data)
 {
     struct method_data_st *methdata = data;
     void *method = NULL;
     OSSL_NAMEMAP *namemap;
-    int id;
+    int nameid;
+    uint32_t methid;
 
     if (store == NULL
         && (store = get_default_method_store(libctx)) == NULL)
         return NULL;
 
     if ((namemap = ossl_namemap_stored(libctx)) == NULL
-        || (id = ossl_namemap_add(namemap, name)) == 0)
+        || (nameid = ossl_namemap_add(namemap, name)) == 0
+        || (methid = method_id(operation_id, nameid)) == 0)
         return NULL;
 
-    (void)ossl_method_store_fetch(store, id, propquery, &method);
+    (void)ossl_method_store_fetch(store, methid, propquery, &method);
 
     if (method != NULL
         && !methdata->refcnt_up_method(method)) {
@@ -93,15 +113,18 @@ static void *get_method_from_store(OPENSSL_CTX *libctx, void *store,
 }
 
 static int put_method_in_store(OPENSSL_CTX *libctx, void *store,
-                               void *method, const char *name,
-                               const char *propdef, void *data)
+                               void *method, int operation_id,
+                               const char *name, const char *propdef,
+                               void *data)
 {
     struct method_data_st *methdata = data;
     OSSL_NAMEMAP *namemap;
-    int id;
+    int nameid;
+    uint32_t methid;
 
     if ((namemap = ossl_namemap_stored(methdata->libctx)) == NULL
-        || (id = ossl_namemap_add(namemap, name)) == 0)
+        || (nameid = ossl_namemap_add(namemap, name)) == 0
+        || (methid = method_id(operation_id, nameid)) == 0)
         return 0;
 
     if (store == NULL
@@ -109,7 +132,7 @@ static int put_method_in_store(OPENSSL_CTX *libctx, void *store,
         return 0;
 
     if (methdata->refcnt_up_method(method)
-        && ossl_method_store_add(store, id, propdef, method,
+        && ossl_method_store_add(store, methid, propdef, method,
                                  methdata->destruct_method))
         return 1;
     return 0;
@@ -139,14 +162,32 @@ 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 id;
+    int nameid = 0;
+    uint32_t methid = 0;
     void *method = NULL;
 
     if (store == NULL || namemap == NULL)
         return NULL;
 
-    if ((id = ossl_namemap_number(namemap, name)) == 0
-        || !ossl_method_store_cache_get(store, id, properties, &method)) {
+    /*
+     * If there's ever an operation_id == 0 passed, we have an internal
+     * programming error.
+     */
+    if (!ossl_assert(operation_id > 0))
+        return NULL;
+
+    /*
+     * method_id returns 0 if we have too many operations (more than
+     * about 2^8) or too many names (more than about 2^24).  In that
+     * case, we can't create any new method.
+     */
+    if ((nameid = ossl_namemap_number(namemap, name)) != 0
+        && (methid = method_id(operation_id, nameid)) == 0)
+        return NULL;
+
+    if (nameid == 0
+        || !ossl_method_store_cache_get(store, methid, properties,
+                                        &method)) {
         OSSL_METHOD_CONSTRUCT_METHOD mcm = {
             alloc_tmp_method_store,
             dealloc_tmp_method_store,
@@ -164,10 +205,19 @@ 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;
-        method = ossl_method_construct(libctx, operation_id, name,
-                                       properties, 0 /* !force_cache */,
-                                       &mcm, &mcmdata);
-        ossl_method_store_cache_set(store, id, properties, method);
+        if ((method = ossl_method_construct(libctx, operation_id, name,
+                                            properties, 0 /* !force_cache */,
+                                            &mcm, &mcmdata)) == NULL) {
+            /*
+             * If construction did create a method for us, we know that
+             * there is a correct nameid and methodid, since those have
+             * already been calculated in get_method_from_store() and
+             * put_method_in_store() above.
+             */
+            nameid = ossl_namemap_number(namemap, name);
+            methid = method_id(operation_id, nameid);
+            ossl_method_store_cache_set(store, methid, properties, method);
+        }
     } else {
         upref_method(method);
     }
diff --git a/doc/internal/man3/ossl_method_construct.pod b/doc/internal/man3/ossl_method_construct.pod
index 60de260..ecb99e0 100644
--- a/doc/internal/man3/ossl_method_construct.pod
+++ b/doc/internal/man3/ossl_method_construct.pod
@@ -15,11 +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 *name,
-                  const char *propquery, void *data);
+     void *(*get)(OPENSSL_CTX *libctx, void *store,
+                  int operation_id, const char *name, const char *propquery,
+                  void *data);
      /* Store a method in a store */
      int (*put)(OPENSSL_CTX *libctx, void *store, void *method,
-                const char *name, const char *propdef, void *data);
+                int operation_id, const char *name, const char *propdef,
+                void *data);
      /* Construct a new method */
      void *(*construct)(const char *name, const OSSL_DISPATCH *fns,
                         OSSL_PROVIDER *prov, void *data);
@@ -41,13 +43,25 @@ on provider dispatch tables need to do so in exactly the same way.
 ossl_method_construct() does this while leaving it to the sub-systems
 to define more precisely how the methods are created, stored, etc.
 
+It's important to keep in mind that a method is identified by three things:
+
+=over 4
+
+=item The operation identity
+
+=item The name of the algorithm
+
+=item The properties associated with the algorithm implementation
+
+=back
+
 =head2 Functions
 
 ossl_method_construct() creates a method by asking all available
-providers for a dispatch table given an C<operation_id>, an algorithm
-C<name> and a set of C<properties>, and then calling appropriate
+providers for a dispatch table given an I<operation_id>, an algorithm
+I<name> and a set of I<properties>, and then calling the appropriate
 functions given by the sub-system specific method creator through
-C<mcm> and the data in C<mcm_data> (which is passed by
+I<mcm> and the data in I<mcm_data> (which is passed by
 ossl_method_construct()).
 
 This function assumes that the sub-system method creator implements
@@ -59,14 +73,14 @@ appropriate).
 
 A central part of constructing a sub-system specific method is to give
 ossl_method_construct a set of functions, all in the
-C<OSSL_METHOD_CONSTRUCT_METHOD> structure, which holds the following
+B<OSSL_METHOD_CONSTRUCT_METHOD> structure, which holds the following
 function pointers:
 
 =over 4
 
 =item alloc_tmp_store()
 
-Create a temporary method store in the scope of the library context C<ctx>.
+Create a temporary method store in the scope of the library context I<ctx>.
 This store is used to temporarily store methods for easier lookup, for
 when the provider doesn't want its dispatch table stored in a longer
 term cache.
@@ -79,40 +93,41 @@ Remove a temporary store.
 
 Look up an already existing method from a store by name.
 
-The store may be given with C<store>.
+The store may be given with I<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>.
+This default store should be stored in the library context I<libctx>.
 
-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>.
+The method to be looked up should be identified with the given
+I<operation_id>, I<name>, the provided property query I<propquery>
+and data from I<data> (which is the I<mcm_data> that was passed to
+ossl_construct_method()).
 
 This function is expected to increment the method's reference count.
 
 =item put()
 
-Places the C<method> created by the construct() function (see below)
+Places the I<method> created by the construct() function (see below)
 in a store.
 
-The store may be given with C<store>.
+The store may be given with I<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>.
+This default store should be stored in the library context I<libctx>.
 
-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()).
+The method should be associated with the given I<operation_id>,
+I<name> and property definition I<propdef> as well as any
+identification data given through I<data> (which is the I<mcm_data>
+that was passed to ossl_construct_method()).
 
-This function is expected to increment the C<method>'s reference count.
+This function is expected to increment the I<method>'s reference count.
 
 =item construct()
 
-Constructs a sub-system method for the given C<name> and the given
-dispatch table C<fns>.
+Constructs a sub-system method for the given I<name> and the given
+dispatch table I<fns>.
 
-The associated I<provider object> C<prov> is passed as well, to make
+The associated provider object I<prov> is passed as well, to make
 it possible for the sub-system constructor to keep a reference, which
 is recommended.
 If such a reference is kept, the I<provider object> reference counter
@@ -122,7 +137,7 @@ This function is expected to set the method's reference count to 1.
 
 =item desctruct()
 
-Decrement the C<method>'s reference count, and destruct it when
+Decrement the I<method>'s reference count, and destruct it when
 the reference count reaches zero.
 
 =back
diff --git a/include/internal/core.h b/include/internal/core.h
index 64547dc..3f0cdfa 100644
--- a/include/internal/core.h
+++ b/include/internal/core.h
@@ -32,11 +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 *name,
-                 const char *propquery, void *data);
+    void *(*get)(OPENSSL_CTX *libctx, void *store,
+                 int operation_id, const char *name, const char *propquery,
+                 void *data);
     /* Store a method in a store */
     int (*put)(OPENSSL_CTX *libctx, void *store, void *method,
-               const char *name, const char *propdef, void *data);
+               int operation_id, const char *name, const char *propdef,
+               void *data);
     /* Construct a new method */
     void *(*construct)(const char *name, const OSSL_DISPATCH *fns,
                        OSSL_PROVIDER *prov, void *data);


More information about the openssl-commits mailing list