Ensure the EVP_PKEY operation_cache is appropriately locked

The EVP_PKEY operation_cache caches references to provider side key
objects that have previously been exported for this EVP_PKEY, and their
associated key managers. The cache may be updated from time to time as the
EVP_PKEY is exported to more providers. Since an EVP_PKEY may be shared by
multiple threads simultaneously we must be careful to ensure the cache
updates are locked.

Fixes #13818

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)

diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c
index 763982e58f7e18247761f0bff9443942fbeebbf7..0c643b3b49fc6f57b81a5a782cfe0acb60ecd4fb 100644 (file)
--- a/crypto/evp/keymgmt_lib.c
+++ b/crypto/evp/keymgmt_lib.c
@@ -102,10 +102,16 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt)
         return pk->keydata;
 
     /* If this key is already exported to |keymgmt|, no more to do */
+    CRYPTO_THREAD_read_lock(pk->lock);
     i = evp_keymgmt_util_find_operation_cache_index(pk, keymgmt);
     if (i < OSSL_NELEM(pk->operation_cache)
-        && pk->operation_cache[i].keymgmt != NULL)
-        return pk->operation_cache[i].keydata;
+        && pk->operation_cache[i].keymgmt != NULL) {
+        void *ret = pk->operation_cache[i].keydata;
+
+        CRYPTO_THREAD_unlock(pk->lock);
+        return ret;
+    }
+    CRYPTO_THREAD_unlock(pk->lock);
 
     /* If the "origin" |keymgmt| doesn't support exporting, give up */
     /*
@@ -153,20 +159,42 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt)
         return NULL;
     }
 
+    CRYPTO_THREAD_write_lock(pk->lock);
+    /* Check to make sure some other thread didn't get there first */
+    i = evp_keymgmt_util_find_operation_cache_index(pk, keymgmt);
+    if (i < OSSL_NELEM(pk->operation_cache)
+        && pk->operation_cache[i].keymgmt != NULL) {
+        void *ret = pk->operation_cache[i].keydata;
+
+        CRYPTO_THREAD_unlock(pk->lock);
+
+        /*
+         * Another thread seemms to have already exported this so we abandon
+         * all the work we just did.
+         */
+        evp_keymgmt_freedata(keymgmt, import_data.keydata);
+
+        return ret;
+    }
+
     /* Add the new export to the operation cache */
     if (!evp_keymgmt_util_cache_keydata(pk, i, keymgmt, import_data.keydata)) {
         evp_keymgmt_freedata(keymgmt, import_data.keydata);
         return NULL;
     }
 
+    CRYPTO_THREAD_unlock(pk->lock);
+
     return import_data.keydata;
 }
 
-void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk)
+int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking)
 {
     size_t i, end = OSSL_NELEM(pk->operation_cache);
 
     if (pk != NULL) {
+        if (locking && pk->lock != NULL && !CRYPTO_THREAD_write_lock(pk->lock))
+            return 0;
         for (i = 0; i < end && pk->operation_cache[i].keymgmt != NULL; i++) {
             EVP_KEYMGMT *keymgmt = pk->operation_cache[i].keymgmt;
             void *keydata = pk->operation_cache[i].keydata;
@@ -176,7 +204,11 @@ void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk)
             evp_keymgmt_freedata(keymgmt, keydata);
             EVP_KEYMGMT_free(keymgmt);
         }
+        if (locking && pk->lock != NULL)
+            CRYPTO_THREAD_unlock(pk->lock);
     }
+
+    return 1;
 }
 
 size_t evp_keymgmt_util_find_operation_cache_index(EVP_PKEY *pk,
@@ -198,6 +230,7 @@ int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
     if (keydata != NULL) {
         if (!EVP_KEYMGMT_up_ref(keymgmt))
             return 0;
+
         pk->operation_cache[index].keydata = keydata;
         pk->operation_cache[index].keymgmt = keymgmt;
     }

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index 5df9b19eaeaee75ad6c1849602ca06f61faaf851..21ce51d5737f9c32420a5ff6e2e26a1e16cd8710 100644 (file)
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1621,7 +1621,7 @@ static void evp_pkey_free_it(EVP_PKEY *x)
 {
     /* internal function; x is never NULL */
 
-    evp_keymgmt_util_clear_operation_cache(x);
+    evp_keymgmt_util_clear_operation_cache(x, 1);
 #ifndef FIPS_MODULE
     evp_pkey_free_legacy(x);
 #endif
@@ -1735,6 +1735,8 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
          * |i| remains zero, and we will clear the cache further down.
          */
         if (pk->ameth->dirty_cnt(pk) == pk->dirty_cnt_copy) {
+            if (!CRYPTO_THREAD_read_lock(pk->lock))
+                goto end;
             i = evp_keymgmt_util_find_operation_cache_index(pk, tmp_keymgmt);
 
             /*
@@ -1746,8 +1748,10 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
             if (i < OSSL_NELEM(pk->operation_cache)
                 && pk->operation_cache[i].keymgmt != NULL) {
                 keydata = pk->operation_cache[i].keydata;
+                CRYPTO_THREAD_unlock(pk->lock);
                 goto end;
             }
+            CRYPTO_THREAD_unlock(pk->lock);
         }
 
         /*
@@ -1782,12 +1786,22 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
             keydata = NULL;
             goto end;
         }
-        if (pk->ameth->dirty_cnt(pk) != pk->dirty_cnt_copy)
-            evp_keymgmt_util_clear_operation_cache(pk);
+
+        if (!CRYPTO_THREAD_write_lock(pk->lock))
+            goto end;
+        if (pk->ameth->dirty_cnt(pk) != pk->dirty_cnt_copy
+                && !evp_keymgmt_util_clear_operation_cache(pk, 0)) {
+            CRYPTO_THREAD_unlock(pk->lock);
+            evp_keymgmt_freedata(tmp_keymgmt, keydata);
+            keydata = NULL;
+            EVP_KEYMGMT_free(tmp_keymgmt);
+            goto end;
+        }
         EVP_KEYMGMT_free(tmp_keymgmt); /* refcnt-- */
 
         /* Add the new export to the operation cache */
         if (!evp_keymgmt_util_cache_keydata(pk, i, tmp_keymgmt, keydata)) {
+            CRYPTO_THREAD_unlock(pk->lock);
             evp_keymgmt_freedata(tmp_keymgmt, keydata);
             keydata = NULL;
             goto end;
@@ -1795,6 +1809,8 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
 
         /* Synchronize the dirty count */
         pk->dirty_cnt_copy = pk->ameth->dirty_cnt(pk);
+    
+        CRYPTO_THREAD_unlock(pk->lock);
         goto end;
     }
 #endif  /* FIPS_MODULE */

diff --git a/doc/internal/man3/evp_keymgmt_util_export_to_provider.pod b/doc/internal/man3/evp_keymgmt_util_export_to_provider.pod
index bb2ad9ba8eb5b02d9c6827102881baea585b0efb..31f8b00e47d984b04578a149e7e84490f9219c74 100644 (file)
--- a/doc/internal/man3/evp_keymgmt_util_export_to_provider.pod
+++ b/doc/internal/man3/evp_keymgmt_util_export_to_provider.pod
@@ -20,9 +20,9 @@ evp_keymgmt_util_fromdata
  void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt);
  size_t evp_keymgmt_util_find_operation_cache_index(EVP_PKEY *pk,
                                                     EVP_KEYMGMT *keymgmt);
- void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk);
- void evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
-                                     EVP_KEYMGMT *keymgmt, void *keydata);
+ int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking);
+ int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
+                                    EVP_KEYMGMT *keymgmt, void *keydata);
  void evp_keymgmt_util_cache_keyinfo(EVP_PKEY *pk);
  void *evp_keymgmt_util_fromdata(EVP_PKEY *target, EVP_KEYMGMT *keymgmt,
                                  int selection, const OSSL_PARAM params[]);
@@ -44,10 +44,13 @@ as this function ignores any legacy key data.
 evp_keymgmt_util_find_operation_cache_index() finds the location if
 I<keymgmt> in I<pk>'s cache of provided keys for operations.  If
 I<keymgmt> is NULL or couldn't be found in the cache, it finds the
-first empty slot instead if there is any.
+first empty slot instead if there is any. It should only be called while
+holding I<pk>'s lock (read or write).
 
 evp_keymgmt_util_clear_operation_cache() can be used to explicitly
-clear the cache of operation key references.
+clear the cache of operation key references. If I<locking> is set to 1 then
+then I<pk>'s lock will be obtained while doing the clear. Otherwise it will be
+assumed that the lock has already been obtained or is not required.
 
 evp_keymgmt_util_cache_keydata() can be used to assign a provider key
 object to a specific cache slot in the given I<target>.
@@ -72,6 +75,9 @@ operation cache slot.  If I<keymgmt> is NULL, or if there is no slot
 with a match for I<keymgmt>, the index of the first empty slot is
 returned, or the maximum number of slots if there isn't an empty one.
 
+evp_keymgmt_util_cache_keydata() and evp_keymgmt_util_clear_operation_cache()
+return 1 on success or 0 otherwise.
+
 =head1 NOTES
 
 "Legacy key" is the term used for any key that has been assigned to an

diff --git a/include/crypto/evp.h b/include/crypto/evp.h
index 20335e9a321cf671f7c9ca2c1f494ba24f8ab22a..bed75f406cce4df10c470d119fd46d3224457bae 100644 (file)
--- a/include/crypto/evp.h
+++ b/include/crypto/evp.h
@@ -729,7 +729,7 @@ int evp_keymgmt_util_export(const EVP_PKEY *pk, int selection,
 void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt);
 size_t evp_keymgmt_util_find_operation_cache_index(EVP_PKEY *pk,
                                                    EVP_KEYMGMT *keymgmt);
-void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk);
+int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking);
 int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
                                    EVP_KEYMGMT *keymgmt, void *keydata);
 void evp_keymgmt_util_cache_keyinfo(EVP_PKEY *pk);