CORE: query for operations only once per provider (unless no_store is true)

When a desired algorithm wasn't available, we didn't register anywhere
that an attempt had been made, with the result that next time the same
attempt was made, the whole process would be done again.

To avoid this churn, we register a bit for each operation that has
been queried in the libcrypto provider object, and test it before
trying the same query and method construction loop again.

If course, if the provider has told us not to cache, we don't register
this bit.

Fixes #11814

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11842)

diff --git a/crypto/core_algorithm.c b/crypto/core_algorithm.c
index 79625fdea62b00421c64e631e186ef60a665c052..5c019f0405d58a4ac23c708ca005471095bf7134 100644 (file)
--- a/crypto/core_algorithm.c
+++ b/crypto/core_algorithm.c
@@ -16,8 +16,11 @@
 struct algorithm_data_st {
     OPENSSL_CTX *libctx;
     int operation_id;            /* May be zero for finding them all */
+    int (*pre)(OSSL_PROVIDER *, int operation_id, void *data, int *result);
     void (*fn)(OSSL_PROVIDER *, const OSSL_ALGORITHM *, int no_store,
                void *data);
+    int (*post)(OSSL_PROVIDER *, int operation_id, int no_store, void *data,
+                int *result);
     void *data;
 };
 
@@ -36,19 +39,48 @@ static int algorithm_do_this(OSSL_PROVIDER *provider, void *cbdata)
     for (cur_operation = first_operation;
          cur_operation <= last_operation;
          cur_operation++) {
-        const OSSL_ALGORITHM *map =
-            ossl_provider_query_operation(provider, cur_operation,
-                                          &no_store);
+        const OSSL_ALGORITHM *map = NULL;
+        int ret;
 
+        /* Do we fulfill pre-conditions? */
+        if (data->pre == NULL) {
+            /* If there is no pre-condition function, assume "yes" */
+            ret = 1;
+        } else {
+            if (!data->pre(provider, cur_operation, data->data, &ret))
+                /* Error, bail out! */
+                return 0;
+        }
+
+        /* If pre-condition not fulfilled, go to the next operation */
+        if (!ret)
+            continue;
+
+        map = ossl_provider_query_operation(provider, cur_operation,
+                                            &no_store);
         if (map == NULL)
             continue;
 
-        ok = 1;                  /* As long as we've found *something* */
         while (map->algorithm_names != NULL) {
             const OSSL_ALGORITHM *thismap = map++;
 
             data->fn(provider, thismap, no_store, data->data);
         }
+
+        /* Do we fulfill post-conditions? */
+        if (data->post == NULL) {
+            /* If there is no post-condition function, assume "yes" */
+            ret = 1;
+        } else {
+            if (!data->post(provider, cur_operation, no_store, data->data,
+                            &ret))
+                /* Error, bail out! */
+                return 0;
+        }
+
+        /* If post-condition fulfilled, set general success */
+        if (ret)
+            ok = 1;
     }
 
     return ok;
@@ -56,16 +88,22 @@ static int algorithm_do_this(OSSL_PROVIDER *provider, void *cbdata)
 
 void ossl_algorithm_do_all(OPENSSL_CTX *libctx, int operation_id,
                            OSSL_PROVIDER *provider,
+                           int (*pre)(OSSL_PROVIDER *, int operation_id,
+                                      void *data, int *result),
                            void (*fn)(OSSL_PROVIDER *provider,
                                       const OSSL_ALGORITHM *algo,
                                       int no_store, void *data),
+                           int (*post)(OSSL_PROVIDER *, int operation_id,
+                                       int no_store, void *data, int *result),
                            void *data)
 {
-    struct algorithm_data_st cbdata;
+    struct algorithm_data_st cbdata = { 0, };
 
     cbdata.libctx = libctx;
     cbdata.operation_id = operation_id;
+    cbdata.pre = pre;
     cbdata.fn = fn;
+    cbdata.post = post;
     cbdata.data = data;
 
     if (provider == NULL)

diff --git a/crypto/core_fetch.c b/crypto/core_fetch.c
index 7f815a50acb06106dd582b183d3aecb591d08394..51ae4011dc5c3ce4c141e901bcfab24c49f13f4b 100644 (file)
--- a/crypto/core_fetch.c
+++ b/crypto/core_fetch.c
@@ -24,6 +24,42 @@ struct construct_data_st {
     void *mcm_data;
 };
 
+static int ossl_method_construct_precondition(OSSL_PROVIDER *provider,
+                                              int operation_id, void *cbdata,
+                                              int *result)
+{
+    if (!ossl_assert(result != NULL)) {
+        ERR_raise(ERR_LIB_CRYPTO, ERR_R_PASSED_NULL_PARAMETER);
+        return 0;
+    }
+
+    if (!ossl_provider_test_operation_bit(provider, operation_id, result))
+        return 0;
+
+    /*
+     * The result we get tells if methods have already been constructed.
+     * However, we want to tell whether construction should happen (true)
+     * or not (false), which is the opposite of what we got.
+     */
+    *result = !*result;
+
+    return 1;
+}
+
+static int ossl_method_construct_postcondition(OSSL_PROVIDER *provider,
+                                               int operation_id, int no_store,
+                                               void *cbdata, int *result)
+{
+    if (!ossl_assert(result != NULL)) {
+        ERR_raise(ERR_LIB_CRYPTO, ERR_R_PASSED_NULL_PARAMETER);
+        return 0;
+    }
+
+    *result = 1;
+    return no_store != 0
+        || ossl_provider_set_operation_bit(provider, operation_id);
+}
+
 static void ossl_method_construct_this(OSSL_PROVIDER *provider,
                                        const OSSL_ALGORITHM *algo,
                                        int no_store, void *cbdata)
@@ -86,7 +122,10 @@ void *ossl_method_construct(OPENSSL_CTX *libctx, int operation_id,
         cbdata.mcm = mcm;
         cbdata.mcm_data = mcm_data;
         ossl_algorithm_do_all(libctx, operation_id, NULL,
-                              ossl_method_construct_this, &cbdata);
+                              ossl_method_construct_precondition,
+                              ossl_method_construct_this,
+                              ossl_method_construct_postcondition,
+                              &cbdata);
 
         method = mcm->get(libctx, cbdata.store, mcm_data);
         mcm->dealloc_tmp_store(cbdata.store);

diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
index be5ab111aaf01fdb35003b89bb652496a8565340..596f5925350236ec5f3d0fc6d05ab0d659cd0560 100644 (file)
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -444,7 +444,13 @@ void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
     data.free_method = free_method;
     data.user_fn = user_fn;
     data.user_arg = user_arg;
-    ossl_algorithm_do_all(libctx, operation_id, NULL, do_one, &data);
+
+    /*
+     * No pre- or post-condition for this call, as this only creates methods
+     * temporarly and then promptly destroys them.
+     */
+    ossl_algorithm_do_all(libctx, operation_id, NULL, NULL, do_one, NULL,
+                          &data);
 }
 
 const char *evp_first_name(const OSSL_PROVIDER *prov, int name_id)

diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 662576cd7b1dfff13e44832764438742a881ad5e..0c2166008047e268500ebf015007e12acaf4fcd2 100644 (file)
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -71,6 +71,13 @@ struct ossl_provider_st {
     OSSL_provider_get_params_fn *get_params;
     OSSL_provider_query_operation_fn *query_operation;
 
+    /*
+     * Cache of bit to indicate of query_operation() has been called on
+     * a specific operation or not.
+     */
+    unsigned char *operation_bits;
+    size_t operation_bits_sz;
+
     /* Provider side data */
     void *provctx;
 };
@@ -317,6 +324,9 @@ void ossl_provider_free(OSSL_PROVIDER *prov)
             }
 # endif
 #endif
+            OPENSSL_free(prov->operation_bits);
+            prov->operation_bits = NULL;
+            prov->operation_bits_sz = 0;
             prov->flag_initialized = 0;
         }
 
@@ -782,6 +792,42 @@ const OSSL_ALGORITHM *ossl_provider_query_operation(const OSSL_PROVIDER *prov,
     return prov->query_operation(prov->provctx, operation_id, no_cache);
 }
 
+int ossl_provider_set_operation_bit(OSSL_PROVIDER *provider, size_t bitnum)
+{
+    size_t byte = bitnum / 8;
+    unsigned char bit = (1 << (bitnum % 8)) & 0xFF;
+
+    if (provider->operation_bits_sz <= byte) {
+        provider->operation_bits = OPENSSL_realloc(provider->operation_bits,
+                                                   byte + 1);
+        if (provider->operation_bits == NULL) {
+            ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
+            return 0;
+        }
+        memset(provider->operation_bits + provider->operation_bits_sz,
+               '\0', byte + 1 - provider->operation_bits_sz);
+    }
+    provider->operation_bits[byte] |= bit;
+    return 1;
+}
+
+int ossl_provider_test_operation_bit(OSSL_PROVIDER *provider, size_t bitnum,
+                                     int *result)
+{
+    size_t byte = bitnum / 8;
+    unsigned char bit = (1 << (bitnum % 8)) & 0xFF;
+
+    if (!ossl_assert(result != NULL)) {
+        ERR_raise(ERR_LIB_CRYPTO, ERR_R_PASSED_NULL_PARAMETER);
+        return 0;
+    }
+
+    *result = 0;
+    if (provider->operation_bits_sz > byte)
+        *result = ((provider->operation_bits[byte] & bit) != 0);
+    return 1;
+}
+
 /*-
  * Core functions for the provider
  * ===============================

diff --git a/crypto/serializer/serializer_meth.c b/crypto/serializer/serializer_meth.c
index a098ffb07b09f3a80f611b42f11259954b2d62e7..bc30c96befd0c4bdc79491ecac3ad027b9292820 100644 (file)
--- a/crypto/serializer/serializer_meth.c
+++ b/crypto/serializer/serializer_meth.c
@@ -417,8 +417,13 @@ void OSSL_SERIALIZER_do_all_provided(OPENSSL_CTX *libctx,
 
     data.user_fn = (void (*)(void *, void *))fn;
     data.user_arg = arg;
-    ossl_algorithm_do_all(libctx, OSSL_OP_SERIALIZER, NULL,
-                          serializer_do_one, &data);
+
+    /*
+     * No pre- or post-condition for this call, as this only creates methods
+     * temporarly and then promptly destroys them.
+     */
+    ossl_algorithm_do_all(libctx, OSSL_OP_SERIALIZER, NULL, NULL,
+                          serializer_do_one, NULL, &data);
 }
 
 void OSSL_SERIALIZER_names_do_all(const OSSL_SERIALIZER *ser,

diff --git a/doc/internal/man3/ossl_provider_new.pod b/doc/internal/man3/ossl_provider_new.pod
index 36fe6301bf4c9c96866a3b43b994551e43feeba4..d5d732d4158b774b8cd14e8b07722e78c483f08c 100644 (file)
--- a/doc/internal/man3/ossl_provider_new.pod
+++ b/doc/internal/man3/ossl_provider_new.pod
@@ -13,7 +13,8 @@ ossl_provider_name, ossl_provider_dso,
 ossl_provider_module_name, ossl_provider_module_path,
 ossl_provider_library_context,
 ossl_provider_teardown, ossl_provider_gettable_params,
-ossl_provider_get_params, ossl_provider_query_operation
+ossl_provider_get_params, ossl_provider_query_operation,
+ossl_provider_set_operation_bit, ossl_provider_test_operation_bit
 - internal provider routines
 
 =head1 SYNOPSIS
@@ -63,6 +64,10 @@ ossl_provider_get_params, ossl_provider_query_operation
                                                      int operation_id,
                                                      int *no_cache);
 
+ int ossl_provider_set_operation_bit(OSSL_PROVIDER *provider, size_t bitnum);
+ int ossl_provider_test_operation_bit(OSSL_PROVIDER *provider, size_t bitnum,
+                                      int *result);
+
 =head1 DESCRIPTION
 
 I<OSSL_PROVIDER> is a type that holds all the necessary information
@@ -208,6 +213,13 @@ I<query_operation> function, if the provider has one.
 It should return an array of I<OSSL_ALGORITHM> for the given
 I<operation_id>.
 
+ossl_provider_set_operation_bit() registers a 1 for operation I<bitnum>
+in a bitstring that's internal to I<provider>.
+
+ossl_provider_tests_operation_bit() checks if the bit operation I<bitnum>
+is set (1) or not (0) in the internal I<provider> bitstring, and sets
+I<*result> to 1 or 0 accorddingly. 
+
 =head1 NOTES
 
 Locating a provider module happens as follows:
@@ -270,6 +282,9 @@ otherwise NULL.
 ossl_provider_get_params() returns 1 on success, or 0 on error.
 If this function isn't available in the provider, 0 is returned.
 
+ossl_provider_set_operation_bit() and ossl_provider_test_operation_bit()
+return 1 on success, or 0 on error.
+
 =head1 SEE ALSO
 
 L<OSSL_PROVIDER(3)>, L<provider(7)>, L<openssl(1)>

diff --git a/include/internal/core.h b/include/internal/core.h
index ca043334867642e67e838abf5200b1b21d72ab5a..e2d2b28e8c07133d0d76eeae9a86f8602e0fafd7 100644 (file)
--- a/include/internal/core.h
+++ b/include/internal/core.h
@@ -50,9 +50,13 @@ void *ossl_method_construct(OPENSSL_CTX *ctx, int operation_id,
 
 void ossl_algorithm_do_all(OPENSSL_CTX *libctx, int operation_id,
                            OSSL_PROVIDER *provider,
+                           int (*pre)(OSSL_PROVIDER *, int operation_id,
+                                      void *data, int *result),
                            void (*fn)(OSSL_PROVIDER *provider,
                                       const OSSL_ALGORITHM *algo,
                                       int no_store, void *data),
+                           int (*post)(OSSL_PROVIDER *, int operation_id,
+                                       int no_store, void *data, int *result),
                            void *data);
 
 #endif

diff --git a/include/internal/provider.h b/include/internal/provider.h
index e39a5eae82242cf4b649e2896f57ce0a30736d68..135b660f49639c1e1bd91f3b801ba8e9b5834b22 100644 (file)
--- a/include/internal/provider.h
+++ b/include/internal/provider.h
@@ -74,6 +74,11 @@ const OSSL_ALGORITHM *ossl_provider_query_operation(const OSSL_PROVIDER *prov,
                                                     int operation_id,
                                                     int *no_cache);
 
+/* Cache of bits to see if we already queried an operation */
+int ossl_provider_set_operation_bit(OSSL_PROVIDER *provider, size_t bitnum);
+int ossl_provider_test_operation_bit(OSSL_PROVIDER *provider, size_t bitnum,
+                                     int *result);
+
 /* Configuration */
 void ossl_provider_add_conf_module(void);