EVP: evp_keymgmt_util_try_import() should clean up on failed import
authorRichard Levitte <levitte@openssl.org>
Fri, 23 Apr 2021 13:40:30 +0000 (15:40 +0200)
committerRichard Levitte <levitte@openssl.org>
Fri, 23 Apr 2021 18:22:49 +0000 (20:22 +0200)
If evp_keymgmt_util_try_import() allocated keydata, and the import
itself fails, it should deallocate keydata.

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/15008)

crypto/evp/keymgmt_lib.c

index f3118a76c937b5a826285841eebf200d44df1f1a..301e1a8a2f0c3020478155809ce7669b39726134 100644 (file)
@@ -31,12 +31,15 @@ static int match_type(const EVP_KEYMGMT *keymgmt1, const EVP_KEYMGMT *keymgmt2)
 int evp_keymgmt_util_try_import(const OSSL_PARAM params[], void *arg)
 {
     struct evp_keymgmt_util_try_import_data_st *data = arg;
+    int delete_on_error = 0;
 
     /* Just in time creation of keydata */
-    if (data->keydata == NULL
-        && (data->keydata = evp_keymgmt_newdata(data->keymgmt)) == NULL) {
-        ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE);
-        return 0;
+    if (data->keydata == NULL) {
+        if ((data->keydata = evp_keymgmt_newdata(data->keymgmt)) == NULL) {
+            ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE);
+            return 0;
+        }
+        delete_on_error = 1;
     }
 
     /*
@@ -46,8 +49,14 @@ int evp_keymgmt_util_try_import(const OSSL_PARAM params[], void *arg)
     if (params[0].key == NULL)
         return 1;
 
-    return evp_keymgmt_import(data->keymgmt, data->keydata, data->selection,
-                              params);
+    if (evp_keymgmt_import(data->keymgmt, data->keydata, data->selection,
+                           params))
+        return 1;
+    if (delete_on_error) {
+        evp_keymgmt_freedata(data->keymgmt, data->keydata);
+        data->keydata = NULL;
+    }
+    return 0;
 }
 
 int evp_keymgmt_util_assign_pkey(EVP_PKEY *pkey, EVP_KEYMGMT *keymgmt,
@@ -149,11 +158,9 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt)
      * which does the import for us.  If successful, we're done.
      */
     if (!evp_keymgmt_util_export(pk, OSSL_KEYMGMT_SELECT_ALL,
-                                 &evp_keymgmt_util_try_import, &import_data)) {
+                                 &evp_keymgmt_util_try_import, &import_data))
         /* If there was an error, bail out */
-        evp_keymgmt_freedata(keymgmt, import_data.keydata);
         return NULL;
-    }
 
     if (!CRYPTO_THREAD_write_lock(pk->lock)) {
         evp_keymgmt_freedata(keymgmt, import_data.keydata);