Rethink the EVP_PKEY cache of provider side keys

The role of this cache was two-fold:

1.  It was a cache of key copies exported to providers with which an
    operation was initiated.
2.  If the EVP_PKEY didn't have a legacy key, item 0 of the cache was
    the corresponding provider side origin, while the rest was the
    actual cache.

This dual role for item 0 made the code a bit confusing, so we now
make a separate keymgmt / keydata pair outside of that cache, which is
the provider side "origin" key.

A hard rule is that an EVP_PKEY cannot hold a legacy "origin" and a
provider side "origin" at the same time.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11148)

1 files changed, 87 insertions, 87 deletions

diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c
index cb30405166..a88d65dc5e 100644
--- a/crypto/evp/keymgmt_lib.c
+++ b/crypto/evp/keymgmt_lib.c
@@ -47,31 +47,48 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt)
 {
     void *keydata = NULL;
     struct import_data_st import_data;
-    size_t i, j;
+    size_t i = 0;
 
     /* Export to where? */
     if (keymgmt == NULL)
         return NULL;
 
     /* If we have an unassigned key, give up */
-    if (pk->pkeys[0].keymgmt == NULL)
+    if (pk->keymgmt == NULL)
         return NULL;
 
+    /* If |keymgmt| matches the "origin" |keymgmt|, no more to do */
+    if (pk->keymgmt == keymgmt)
+        return pk->keydata;
+
+    /* If this key is already exported to |keymgmt|, no more to do */
+    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;
+
+    /* If the "origin" |keymgmt| doesn't support exporting, give up */
     /*
-     * See if we have exported to this provider already.
-     * If we have, return immediately.
+     * TODO(3.0) consider an evp_keymgmt_export() return value that indicates
+     * that the method is unsupported.
      */
-    i = evp_keymgmt_util_find_pkey_cache_index(pk, keymgmt);
+    if (pk->keymgmt->export == NULL)
+        return NULL;
 
-    /* If we're already exported to the given keymgmt, no more to do */
-    if (keymgmt == pk->pkeys[i].keymgmt)
-        return pk->pkeys[i].keydata;
+    /* Check that we have found an empty slot in the export cache */
+    /*
+     * TODO(3.0) Right now, we assume we have ample space.  We will have to
+     * think about a cache aging scheme, though, if |i| indexes outside the
+     * array.
+     */
+    if (!ossl_assert(i < OSSL_NELEM(pk->operation_cache)))
+        return NULL;
 
     /*
      * Make sure that the type of the keymgmt to export to matches the type
-     * of already cached keymgmt
+     * of the "origin"
      */
-    if (!ossl_assert(match_type(pk->pkeys[0].keymgmt, keymgmt)))
+    if (!ossl_assert(match_type(pk->keymgmt, keymgmt)))
         return NULL;
 
     /* Create space to import data into */
@@ -89,109 +106,90 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt)
     import_data.keymgmt = keymgmt;
     import_data.selection = OSSL_KEYMGMT_SELECT_ALL;
 
-    for (j = 0; j < i && pk->pkeys[j].keymgmt != NULL; j++) {
-        EVP_KEYMGMT *exp_keymgmt = pk->pkeys[j].keymgmt;
-        void *exp_keydata = pk->pkeys[j].keydata;
-
-        /*
-         * TODO(3.0) consider an evp_keymgmt_export() return value that
-         * indicates that the method is unsupported.
-         */
-        if (exp_keymgmt->export == NULL)
-            continue;
-
-        /*
-         * The export function calls the callback (try_import), which does
-         * the import for us.  If successful, we're done.
-         */
-        if (evp_keymgmt_export(exp_keymgmt, exp_keydata,
-                               OSSL_KEYMGMT_SELECT_ALL,
-                               &try_import, &import_data))
-            break;
-
+    /*
+     * The export function calls the callback (try_import), which does the
+     * import for us.  If successful, we're done.
+     */
+    if (!evp_keymgmt_export(pk->keymgmt, pk->keydata, OSSL_KEYMGMT_SELECT_ALL,
+                            &try_import, &import_data)) {
         /* If there was an error, bail out */
         evp_keymgmt_freedata(keymgmt, keydata);
         return NULL;
     }
 
-    /*
-     * TODO(3.0) Right now, we assume we have ample space.  We will
-     * have to think about a cache aging scheme, though, if |i| indexes
-     * outside the array.
-     */
-    if (!ossl_assert(i < OSSL_NELEM(pk->pkeys)))
+    /* Add the new export to the operation cache */
+    if (!evp_keymgmt_util_cache_keydata(pk, i, keymgmt, keydata)) {
+        evp_keymgmt_freedata(keymgmt, keydata);
         return NULL;
-
-    evp_keymgmt_util_cache_pkey(pk, i, keymgmt, keydata);
+    }
 
     return keydata;
 }
 
-void evp_keymgmt_util_clear_pkey_cache(EVP_PKEY *pk)
+void evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk)
 {
-    size_t i, end = OSSL_NELEM(pk->pkeys);
+    size_t i, end = OSSL_NELEM(pk->operation_cache);
 
     if (pk != NULL) {
-        for (i = 0; i < end && pk->pkeys[i].keymgmt != NULL; i++) {
-            EVP_KEYMGMT *keymgmt = pk->pkeys[i].keymgmt;
-            void *keydata = pk->pkeys[i].keydata;
+        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;
 
-            pk->pkeys[i].keymgmt = NULL;
-            pk->pkeys[i].keydata = NULL;
+            pk->operation_cache[i].keymgmt = NULL;
+            pk->operation_cache[i].keydata = NULL;
             evp_keymgmt_freedata(keymgmt, keydata);
             EVP_KEYMGMT_free(keymgmt);
         }
-
-        pk->cache.size = 0;
-        pk->cache.bits = 0;
-        pk->cache.security_bits = 0;
     }
 }
 
-size_t evp_keymgmt_util_find_pkey_cache_index(EVP_PKEY *pk,
-                                              EVP_KEYMGMT *keymgmt)
+size_t evp_keymgmt_util_find_operation_cache_index(EVP_PKEY *pk,
+                                                   EVP_KEYMGMT *keymgmt)
 {
-    size_t i, end = OSSL_NELEM(pk->pkeys);
+    size_t i, end = OSSL_NELEM(pk->operation_cache);
 
-    for (i = 0; i < end && pk->pkeys[i].keymgmt != NULL; i++) {
-        if (keymgmt == pk->pkeys[i].keymgmt)
+    for (i = 0; i < end && pk->operation_cache[i].keymgmt != NULL; i++) {
+        if (keymgmt == pk->operation_cache[i].keymgmt)
             break;
     }
 
     return i;
 }
 
-void evp_keymgmt_util_cache_pkey(EVP_PKEY *pk, size_t index,
-                                 EVP_KEYMGMT *keymgmt, void *keydata)
+int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, size_t index,
+                                   EVP_KEYMGMT *keymgmt, void *keydata)
 {
     if (keydata != NULL) {
-        EVP_KEYMGMT_up_ref(keymgmt);
-        pk->pkeys[index].keydata = keydata;
-        pk->pkeys[index].keymgmt = keymgmt;
-
-        /*
-         * Cache information about the key object.  Only needed for the
-         * "original" provider side key.
-         *
-         * This services functions like EVP_PKEY_size, EVP_PKEY_bits, etc
-         */
-        if (index == 0) {
-            int bits = 0;
-            int security_bits = 0;
-            int size = 0;
-            OSSL_PARAM params[4];
-
-            params[0] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_BITS, &bits);
-            params[1] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_SECURITY_BITS,
-                                                 &security_bits);
-            params[2] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_MAX_SIZE,
-                                                 &size);
-            params[3] = OSSL_PARAM_construct_end();
-            if (evp_keymgmt_get_params(keymgmt, keydata, params)) {
-                pk->cache.size = size;
-                pk->cache.bits = bits;
-                pk->cache.security_bits = security_bits;
-            }
+        if (!EVP_KEYMGMT_up_ref(keymgmt))
+            return 0;
+        pk->operation_cache[index].keydata = keydata;
+        pk->operation_cache[index].keymgmt = keymgmt;
+    }
+    return 1;
+}
+
+void evp_keymgmt_util_cache_keyinfo(EVP_PKEY *pk)
+{
+    /*
+     * Cache information about the provider "origin" key.
+     *
+     * This services functions like EVP_PKEY_size, EVP_PKEY_bits, etc
+     */
+    if (pk->keymgmt != NULL) {
+        int bits = 0;
+        int security_bits = 0;
+        int size = 0;
+        OSSL_PARAM params[4];
+
+        params[0] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_BITS, &bits);
+        params[1] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_SECURITY_BITS,
+                                             &security_bits);
+        params[2] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_MAX_SIZE, &size);
+        params[3] = OSSL_PARAM_construct_end();
+        if (evp_keymgmt_get_params(pk->keymgmt, pk->keydata, params)) {
+            pk->cache.size = size;
+            pk->cache.bits = bits;
+            pk->cache.security_bits = security_bits;
         }
     }
 }
@@ -202,14 +200,16 @@ void *evp_keymgmt_util_fromdata(EVP_PKEY *target, EVP_KEYMGMT *keymgmt,
     void *keydata = evp_keymgmt_newdata(keymgmt);
 
     if (keydata != NULL) {
-        if (!evp_keymgmt_import(keymgmt, keydata, selection, params)) {
+        if (!evp_keymgmt_import(keymgmt, keydata, selection, params)
+            || !EVP_KEYMGMT_up_ref(keymgmt)) {
             evp_keymgmt_freedata(keymgmt, keydata);
             return NULL;
         }
 
-
-        evp_keymgmt_util_clear_pkey_cache(target);
-        evp_keymgmt_util_cache_pkey(target, 0, keymgmt, keydata);
+        evp_keymgmt_util_clear_operation_cache(target);
+        target->keymgmt = keymgmt;
+        target->keydata = keydata;
+        evp_keymgmt_util_cache_keyinfo(target);
     }
 
     return keydata;