Fix unbounded memory growth when using no-cached-fetch

When OpenSSL has been compiled with no-cached-fetch we do not cache
algorithms fetched from a provider. When we export an EVP_PKEY to a
provider we cache the details of that export in the operation cache for
that EVP_PKEY. Amoung the details we cache is the EVP_KEYMGMT that we used
for the export. When we come to reuse the key in the same provider that
we have previously exported the key to, we check the operation cache for
the cached key data. However because the EVP_KEYMGMT instance was not
cached then instance will be different every time and we were not
recognising that we had already exported the key to the provider.

This causes us to re-export the key to the same provider everytime the key
is used. Since this consumes memory we end up with unbounded memory growth.

The fix is to be more intelligent about recognising that we have already
exported key data to a given provider even if the EVP_KEYMGMT instance is
different.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/23841)

(cherry picked from commit dc9bc6c8e1bd329ead703417a2235ab3e97557ec)

diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c
index 8369d9578cbd0e7a68c3a8a2a87b9f4d7fcfa228..3226786bb5db5d6414bb46b72248d1a7be358536 100644 (file)
--- a/crypto/evp/keymgmt_lib.c
+++ b/crypto/evp/keymgmt_lib.c
@@ -243,10 +243,15 @@ OP_CACHE_ELEM *evp_keymgmt_util_find_operation_cache(EVP_PKEY *pk,
     /*
      * A comparison and sk_P_CACHE_ELEM_find() are avoided to not cause
      * problems when we've only a read lock.
+     * A keymgmt is a match if the |keymgmt| pointers are identical or if the
+     * provider and the name ID match
      */
     for (i = 0; i < end; i++) {
         p = sk_OP_CACHE_ELEM_value(pk->operation_cache, i);
-        if (keymgmt == p->keymgmt && (p->selection & selection) == selection)
+        if ((p->selection & selection) == selection
+                && (keymgmt == p->keymgmt
+                    || (keymgmt->name_id == p->keymgmt->name_id
+                        && keymgmt->prov == p->keymgmt->prov)))
             return p;
     }
     return NULL;

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index dcb359d106dea21cbb2140073d08dc191aec6e66..e85ccac2b15e4b892e1d1eb063b3f9cdaed58f65 100644 (file)
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1902,7 +1902,15 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
              * If |tmp_keymgmt| is present in the operation cache, it means
              * that export doesn't need to be redone.  In that case, we take
              * token copies of the cached pointers, to have token success
-             * values to return.
+             * values to return. It is possible (e.g. in a no-cached-fetch
+             * build), for op->keymgmt to be a different pointer to tmp_keymgmt
+             * even though the name/provider must be the same. In other words
+             * the keymgmt instance may be different but still equivalent, i.e.
+             * same algorithm/provider instance - but we make the simplifying
+             * assumption that the keydata can be used with either keymgmt
+             * instance. Not doing so introduces significant complexity and
+             * probably requires refactoring - since we would have to ripple
+             * the change in keymgmt instance up the call chain.
              */
             if (op != NULL && op->keymgmt != NULL) {
                 keydata = op->keydata;