[openssl] master update

Richard Levitte levitte at openssl.org
Wed Aug 21 23:51:41 UTC 2019


The branch master has been updated
       via  85d09e8848012d0dfdacf827d9d56730fa5daf16 (commit)
       via  c1d56231ef6385b557ec72eec508e55ea26ca8b0 (commit)
       via  b1d40ddfe23fd9551b89cdcfa52d1c23fc667f8a (commit)
      from  d32d304836caaca475c21a82b94e494898cb60c5 (commit)


- Log -----------------------------------------------------------------
commit 85d09e8848012d0dfdacf827d9d56730fa5daf16
Author: Richard Levitte <levitte at openssl.org>
Date:   Wed Aug 21 10:12:05 2019 +0200

    Fix drbg_ossl_ctx_free() and drbg_nonce_ossl_ctx_free() to handle NULL
    
    If these were passed NULL, the crashed with a SIGSEGV, when they
    should do like all other freeing functions and become a no-op.
    
    Reviewed-by: Shane Lontis <shane.lontis at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/9650)

commit c1d56231ef6385b557ec72eec508e55ea26ca8b0
Author: Richard Levitte <levitte at openssl.org>
Date:   Wed Aug 21 10:08:44 2019 +0200

    Modify ossl_method_store_add() to accept an OSSL_PROVIDER and check for it
    
    If ossl_method_store_add() gets called with a method that already exists
    (i.e. the store has one with matching provider, nid and properties), that
    method should not be stored.  We do this check inside ossl_method_store_add()
    because it has all the locking required to do so safely.
    
    Fixes #9561
    
    Reviewed-by: Shane Lontis <shane.lontis at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/9650)

commit b1d40ddfe23fd9551b89cdcfa52d1c23fc667f8a
Author: Richard Levitte <levitte at openssl.org>
Date:   Wed Aug 21 09:58:10 2019 +0200

    Modify ossl_method_store_add() to handle reference counting
    
    Because this function affects the reference count on failure (the call
    to impl_free() does this), it may as well handle incrementing it as
    well to indicate the extra reference in the method store.
    
    Reviewed-by: Shane Lontis <shane.lontis at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/9650)

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

Summary of changes:
 crypto/core_fetch.c                     | 10 +++++-----
 crypto/evp/evp_fetch.c                  | 14 ++++++--------
 crypto/property/property.c              | 26 +++++++++++++++++++++-----
 crypto/rand/drbg_lib.c                  |  6 ++++++
 doc/internal/man3/OSSL_METHOD_STORE.pod | 20 +++++++++++++-------
 include/internal/core.h                 |  4 ++--
 include/internal/property.h             | 11 ++++++-----
 test/property_test.c                    | 13 ++++++++-----
 8 files changed, 67 insertions(+), 37 deletions(-)

diff --git a/crypto/core_fetch.c b/crypto/core_fetch.c
index c1c81588bb..6e4414d831 100644
--- a/crypto/core_fetch.c
+++ b/crypto/core_fetch.c
@@ -52,14 +52,14 @@ static void ossl_method_construct_this(OSSL_PROVIDER *provider,
          * If we haven't been told not to store,
          * add to the global store
          */
-        data->mcm->put(data->libctx, NULL, method, data->operation_id,
-                       algo->algorithm_name,
+        data->mcm->put(data->libctx, NULL, method, provider,
+                       data->operation_id, algo->algorithm_name,
                        algo->property_definition, data->mcm_data);
     }
 
-    data->mcm->put(data->libctx, data->store, method, data->operation_id,
-                   algo->algorithm_name, algo->property_definition,
-                   data->mcm_data);
+    data->mcm->put(data->libctx, data->store, method, provider,
+                   data->operation_id, algo->algorithm_name,
+                   algo->property_definition, data->mcm_data);
 
     /* refcnt-- because we're dropping the reference */
     data->mcm->destruct(method, data->mcm_data);
diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
index 5c100dd1eb..fa85178a7e 100644
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -114,9 +114,9 @@ static void *get_method_from_store(OPENSSL_CTX *libctx, void *store,
 }
 
 static int put_method_in_store(OPENSSL_CTX *libctx, void *store,
-                               void *method, int operation_id,
-                               const char *name, const char *propdef,
-                               void *data)
+                               void *method, const OSSL_PROVIDER *prov,
+                               int operation_id, const char *name,
+                               const char *propdef, void *data)
 {
     struct method_data_st *methdata = data;
     OSSL_NAMEMAP *namemap;
@@ -132,11 +132,9 @@ static int put_method_in_store(OPENSSL_CTX *libctx, void *store,
         && (store = get_default_method_store(libctx)) == NULL)
         return 0;
 
-    if (methdata->refcnt_up_method(method)
-        && ossl_method_store_add(store, methid, propdef, method,
-                                 methdata->destruct_method))
-        return 1;
-    return 0;
+    return ossl_method_store_add(store, prov, methid, propdef, method,
+                                 methdata->refcnt_up_method,
+                                 methdata->destruct_method);
 }
 
 static void *construct_method(const char *name, const OSSL_DISPATCH *fns,
diff --git a/crypto/property/property.c b/crypto/property/property.c
index c3fa8df9c6..182ea6454b 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -25,6 +25,7 @@
 #define IMPL_CACHE_FLUSH_THRESHOLD  500
 
 typedef struct {
+    const OSSL_PROVIDER *provider;
     OSSL_PROPERTY_LIST *properties;
     void *method;
     void (*method_destruct)(void *);
@@ -173,13 +174,15 @@ static int ossl_method_store_insert(OSSL_METHOD_STORE *store, ALGORITHM *alg)
         return ossl_sa_ALGORITHM_set(store->algs, alg->nid, alg);
 }
 
-int ossl_method_store_add(OSSL_METHOD_STORE *store,
-                          int nid, const char *properties,
-                          void *method, void (*method_destruct)(void *))
+int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
+                          int nid, const char *properties, void *method,
+                          int (*method_up_ref)(void *),
+                          void (*method_destruct)(void *))
 {
     ALGORITHM *alg = NULL;
     IMPLEMENTATION *impl;
     int ret = 0;
+    int i;
 
     if (nid <= 0 || method == NULL || store == NULL)
         return 0;
@@ -190,6 +193,11 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store,
     impl = OPENSSL_malloc(sizeof(*impl));
     if (impl == NULL)
         return 0;
+    if (method_up_ref != NULL && !method_up_ref(method)) {
+        OPENSSL_free(impl);
+        return 0;
+    }
+    impl->provider = prov;
     impl->method = method;
     impl->method_destruct = method_destruct;
 
@@ -219,8 +227,16 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store,
             goto err;
     }
 
-    /* Push onto stack */
-    if (sk_IMPLEMENTATION_push(alg->impls, impl))
+    /* Push onto stack if there isn't one there already */
+    for (i = 0; i < sk_IMPLEMENTATION_num(alg->impls); i++) {
+        const IMPLEMENTATION *tmpimpl = sk_IMPLEMENTATION_value(alg->impls, i);
+
+        if (tmpimpl->provider == impl->provider
+            && tmpimpl->properties == impl->properties)
+            break;
+    }
+    if (i == sk_IMPLEMENTATION_num(alg->impls)
+        && sk_IMPLEMENTATION_push(alg->impls, impl))
         ret = 1;
     ossl_property_unlock(store);
     if (ret == 0)
diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index 825e90d48e..f8b58d7245 100644
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -191,6 +191,9 @@ static void drbg_ossl_ctx_free(void *vdgbl)
 {
     DRBG_GLOBAL *dgbl = vdgbl;
 
+    if (dgbl == NULL)
+        return;
+
     RAND_DRBG_free(dgbl->master_drbg);
     CRYPTO_THREAD_cleanup_local(&dgbl->private_drbg);
     CRYPTO_THREAD_cleanup_local(&dgbl->public_drbg);
@@ -230,6 +233,9 @@ static void drbg_nonce_ossl_ctx_free(void *vdngbl)
 {
     DRBG_NONCE_GLOBAL *dngbl = vdngbl;
 
+    if (dngbl == NULL)
+        return;
+
     CRYPTO_THREAD_lock_free(dngbl->rand_nonce_lock);
 
     OPENSSL_free(dngbl);
diff --git a/doc/internal/man3/OSSL_METHOD_STORE.pod b/doc/internal/man3/OSSL_METHOD_STORE.pod
index abe7ebe317..afd1dd5982 100644
--- a/doc/internal/man3/OSSL_METHOD_STORE.pod
+++ b/doc/internal/man3/OSSL_METHOD_STORE.pod
@@ -19,9 +19,10 @@ ossl_method_store_cache_get, ossl_method_store_cache_set
  void ossl_method_store_free(OSSL_METHOD_STORE *store);
  int ossl_method_store_init(OPENSSL_CTX *ctx);
  void ossl_method_store_cleanup(OPENSSL_CTX *ctx);
- int ossl_method_store_add(OSSL_METHOD_STORE *store,
-                           int nid, const char *properties,
-                           void *method, void (*method_destruct)(void *));
+ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
+                           int nid, const char *properties, void *method,
+                           int (*method_up_ref)(void *),
+                           void (*method_destruct)(void *));
  int ossl_method_store_remove(OSSL_METHOD_STORE *store,
                               int nid, const void *method);
  int ossl_method_store_fetch(OSSL_METHOD_STORE *store,
@@ -62,10 +63,15 @@ B<ctx> to allow access to the required underlying property data.
 
 ossl_method_store_free() frees resources allocated to B<store>.
 
-ossl_method_store_add() adds the B<method> to the B<store> as an instance of an
-algorithm indicated by B<nid> and the property definition B<properties>.
-The optional B<method_destruct> function is called when B<method> is being
-released from B<store>.
+ossl_method_store_add() adds the B<method> constructed from an implementation in
+the provider B<prov> to the B<store> as an instance of an algorithm indicated by
+B<nid> and the property definition B<properties>, unless the B<store> already
+has a method from the same provider with the same B<nid> and B<properties>.
+If the B<method_up_ref> function is given, it's called to increment the
+reference count of the method.
+If the B<method_destruct> function is given, it's called when this function
+fails to add the method to the store, or later on when it is being released from
+the B<store>.
 
 ossl_method_store_remove() removes the B<method> identified by B<nid> from the
 B<store>.
diff --git a/include/internal/core.h b/include/internal/core.h
index bd2f9a0989..a40d3c69af 100644
--- a/include/internal/core.h
+++ b/include/internal/core.h
@@ -37,8 +37,8 @@ typedef struct ossl_method_construct_method_st {
                  void *data);
     /* Store a method in a store */
     int (*put)(OPENSSL_CTX *libctx, void *store, void *method,
-               int operation_id, const char *name, const char *propdef,
-               void *data);
+               const OSSL_PROVIDER *prov, 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);
diff --git a/include/internal/property.h b/include/internal/property.h
index a916be3cc5..3c6d6a9002 100644
--- a/include/internal/property.h
+++ b/include/internal/property.h
@@ -18,11 +18,12 @@ typedef struct ossl_method_store_st OSSL_METHOD_STORE;
 /* Implementation store functions */
 OSSL_METHOD_STORE *ossl_method_store_new(OPENSSL_CTX *ctx);
 void ossl_method_store_free(OSSL_METHOD_STORE *store);
-int ossl_method_store_add(OSSL_METHOD_STORE *store, int nid,
-                          const char *properties, void *implementation,
-                          void (*implementation_destruct)(void *));
-int ossl_method_store_remove(OSSL_METHOD_STORE *store,
-                             int nid, const void *implementation);
+int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
+                          int nid, const char *properties, void *method,
+                          int (*method_up_ref)(void *),
+                          void (*method_destruct)(void *));
+int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid,
+                             const void *method);
 int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
                             const char *prop_query, void **result);
 int ossl_method_store_set_global_properties(OSSL_METHOD_STORE *store,
diff --git a/test/property_test.c b/test/property_test.c
index 765416a11d..29e8ac3f51 100644
--- a/test/property_test.c
+++ b/test/property_test.c
@@ -240,8 +240,9 @@ static int test_register_deregister(void)
         goto err;
 
     for (i = 0; i < OSSL_NELEM(impls); i++)
-        if (!TEST_true(ossl_method_store_add(store, impls[i].nid, impls[i].prop,
-                                             impls[i].impl, NULL))) {
+        if (!TEST_true(ossl_method_store_add(store, NULL, impls[i].nid,
+                                             impls[i].prop, impls[i].impl,
+                                             NULL, NULL))) {
             TEST_note("iteration %zd", i + 1);
             goto err;
         }
@@ -307,8 +308,9 @@ static int test_property(void)
         goto err;
 
     for (i = 0; i < OSSL_NELEM(impls); i++)
-        if (!TEST_true(ossl_method_store_add(store, impls[i].nid, impls[i].prop,
-                                             impls[i].impl, NULL))) {
+        if (!TEST_true(ossl_method_store_add(store, NULL, impls[i].nid,
+                                             impls[i].prop, impls[i].impl,
+                                             NULL, NULL))) {
             TEST_note("iteration %zd", i + 1);
             goto err;
         }
@@ -347,7 +349,8 @@ static int test_query_cache_stochastic(void)
     for (i = 1; i <= max; i++) {
         v[i] = 2 * i;
         BIO_snprintf(buf, sizeof(buf), "n=%d\n", i);
-        if (!TEST_true(ossl_method_store_add(store, i, buf, "abc", NULL))
+        if (!TEST_true(ossl_method_store_add(store, NULL, i, buf, "abc",
+                                             NULL, NULL))
                 || !TEST_true(ossl_method_store_cache_set(store, i, buf, v + i))
                 || !TEST_true(ossl_method_store_cache_set(store, i, "n=1234",
                                                           "miss"))) {


More information about the openssl-commits mailing list