Refactor EVP_SKEY initialization

Enforce that skeymgmt cannot ever be NULL in EVP_SKEY.

Also add missing allocation checks.

Fixes multiple issues found by Coverity.

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from https://github.com/openssl/openssl/pull/26795)

diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c
index 44189604bd1fc15d8e0b0173838e2b93bd329d2c..b0060ec9e5b20a0af167dbea1ddbbf457c9b97ad 100644 (file)
--- a/crypto/evp/evp_enc.c
+++ b/crypto/evp/evp_enc.c
@@ -559,8 +559,7 @@ static int evp_cipher_init_skey_internal(EVP_CIPHER_CTX *ctx,
         }
     }
 
-    if (skey != NULL && skey->skeymgmt != NULL
-        && ctx->cipher->prov != skey->skeymgmt->prov) {
+    if (skey != NULL && ctx->cipher->prov != skey->skeymgmt->prov) {
         ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
         return 0;
     }

diff --git a/crypto/evp/evp_local.h b/crypto/evp/evp_local.h
index e1d5f3213b657e515161ec867710117bae97fb51..2954d57bd5229388c1dbddc1b4920f45638e9ecc 100644 (file)
--- a/crypto/evp/evp_local.h
+++ b/crypto/evp/evp_local.h
@@ -414,5 +414,3 @@ int evp_names_do_all(OSSL_PROVIDER *prov, int number,
                      void (*fn)(const char *name, void *data),
                      void *data);
 int evp_cipher_cache_constants(EVP_CIPHER *cipher);
-
-EVP_SKEY *evp_skey_alloc(void);

diff --git a/crypto/evp/s_lib.c b/crypto/evp/s_lib.c
index 3738f2e5fc91c6a9759643623ec2f9cfb2d39df7..85480b5b151d6d861c142f1bda2ba82ec8e5ac9b 100644 (file)
--- a/crypto/evp/s_lib.c
+++ b/crypto/evp/s_lib.c
@@ -13,6 +13,7 @@
 #include <openssl/evp.h>
 #include <openssl/core_names.h>
 
+#include "internal/common.h"
 #include "internal/provider.h"
 #include "crypto/evp.h"
 #include "evp_local.h"
@@ -25,17 +26,18 @@ int EVP_SKEY_export(const EVP_SKEY *skey, int selection,
         return 0;
     }
 
-    if (skey->skeymgmt == NULL) {
-        ERR_raise(ERR_LIB_EVP, ERR_R_PASSED_INVALID_ARGUMENT);
-        return 0;
-    }
-
     return evp_skeymgmt_export(skey->skeymgmt, skey->keydata, selection, export_cb, export_cbarg);
 }
 
-EVP_SKEY *evp_skey_alloc(void)
+static EVP_SKEY *evp_skey_alloc(EVP_SKEYMGMT *skeymgmt)
 {
-    EVP_SKEY *skey = OPENSSL_zalloc(sizeof(EVP_SKEY));
+    EVP_SKEY *skey;
+
+    if (!ossl_assert(skeymgmt != NULL))
+        return NULL;
+
+    if ((skey = OPENSSL_zalloc(sizeof(*skey))) == NULL)
+        return NULL;
 
     if (!CRYPTO_NEW_REF(&skey->references, 1))
         goto err;
@@ -45,6 +47,7 @@ EVP_SKEY *evp_skey_alloc(void)
         ERR_raise(ERR_LIB_EVP, ERR_R_CRYPTO_LIB);
         goto err;
     }
+    skey->skeymgmt = skeymgmt;
     return skey;
 
  err:
@@ -54,14 +57,12 @@ EVP_SKEY *evp_skey_alloc(void)
     return NULL;
 }
 
-EVP_SKEY *EVP_SKEY_import(OSSL_LIB_CTX *libctx, const char *skeymgmtname, const char *propquery,
-                          int selection, const OSSL_PARAM *params)
+static EVP_SKEY *evp_skey_alloc_fetch(OSSL_LIB_CTX *libctx,
+                                      const char *skeymgmtname,
+                                      const char *propquery)
 {
-    EVP_SKEYMGMT *skeymgmt = NULL;
-    EVP_SKEY *skey = evp_skey_alloc();
-
-    if (skey == NULL)
-        return NULL;
+    EVP_SKEYMGMT *skeymgmt;
+    EVP_SKEY *skey;
 
     skeymgmt = EVP_SKEYMGMT_fetch(libctx, skeymgmtname, propquery);
     if (skeymgmt == NULL) {
@@ -72,10 +73,24 @@ EVP_SKEY *EVP_SKEY_import(OSSL_LIB_CTX *libctx, const char *skeymgmtname, const
         skeymgmt = EVP_SKEYMGMT_fetch(libctx, OSSL_SKEY_TYPE_GENERIC, propquery);
         if (skeymgmt == NULL) {
             ERR_raise(ERR_LIB_EVP, ERR_R_FETCH_FAILED);
-            goto err;
+            return NULL;
         }
     }
-    skey->skeymgmt = skeymgmt;
+
+    skey = evp_skey_alloc(skeymgmt);
+    if (skey == NULL)
+        EVP_SKEYMGMT_free(skeymgmt);
+
+    return skey;
+}
+
+EVP_SKEY *EVP_SKEY_import(OSSL_LIB_CTX *libctx, const char *skeymgmtname, const char *propquery,
+                          int selection, const OSSL_PARAM *params)
+{
+    EVP_SKEY *skey = evp_skey_alloc_fetch(libctx, skeymgmtname, propquery);
+
+    if (skey == NULL)
+        return NULL;
 
     skey->keydata = evp_skeymgmt_import(skey->skeymgmt, selection, params);
     if (skey->keydata == NULL)
@@ -84,7 +99,6 @@ EVP_SKEY *EVP_SKEY_import(OSSL_LIB_CTX *libctx, const char *skeymgmtname, const
     return skey;
 
  err:
-    EVP_SKEYMGMT_free(skeymgmt);
     EVP_SKEY_free(skey);
     return NULL;
 }
@@ -92,26 +106,11 @@ EVP_SKEY *EVP_SKEY_import(OSSL_LIB_CTX *libctx, const char *skeymgmtname, const
 EVP_SKEY *EVP_SKEY_generate(OSSL_LIB_CTX *libctx, const char *skeymgmtname,
                             const char *propquery, const OSSL_PARAM *params)
 {
-    EVP_SKEYMGMT *skeymgmt = NULL;
-    EVP_SKEY *skey = evp_skey_alloc();
+    EVP_SKEY *skey = evp_skey_alloc_fetch(libctx, skeymgmtname, propquery);
 
     if (skey == NULL)
         return NULL;
 
-    skeymgmt = EVP_SKEYMGMT_fetch(libctx, skeymgmtname, propquery);
-    if (skeymgmt == NULL) {
-        /*
-         * if the specific key_type is unkown, attempt to use the generic
-         * key management
-         */
-        skeymgmt = EVP_SKEYMGMT_fetch(libctx, OSSL_SKEY_TYPE_GENERIC, propquery);
-        if (skeymgmt == NULL) {
-            ERR_raise(ERR_LIB_EVP, ERR_R_FETCH_FAILED);
-            goto err;
-        }
-    }
-    skey->skeymgmt = skeymgmt;
-
     skey->keydata = evp_skeymgmt_generate(skey->skeymgmt, params);
     if (skey->keydata == NULL)
         goto err;
@@ -119,7 +118,6 @@ EVP_SKEY *EVP_SKEY_generate(OSSL_LIB_CTX *libctx, const char *skeymgmtname,
     return skey;
 
  err:
-    EVP_SKEYMGMT_free(skeymgmt);
     EVP_SKEY_free(skey);
     return NULL;
 }
@@ -196,8 +194,7 @@ void EVP_SKEY_free(EVP_SKEY *skey)
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
-    if (skey->keydata && skey->skeymgmt)
-        evp_skeymgmt_freedata(skey->skeymgmt, skey->keydata);
+    evp_skeymgmt_freedata(skey->skeymgmt, skey->keydata);
 
     EVP_SKEYMGMT_free(skey->skeymgmt);
 
@@ -239,9 +236,6 @@ int EVP_SKEY_is_a(const EVP_SKEY *skey, const char *name)
     if (skey == NULL)
         return 0;
 
-    if (skey->skeymgmt == NULL)
-        return 0;
-
     return EVP_SKEYMGMT_is_a(skey->skeymgmt, name);
 }
 
@@ -266,15 +260,26 @@ EVP_SKEY *EVP_SKEY_to_provider(EVP_SKEY *skey, OSSL_LIB_CTX *libctx,
     EVP_SKEYMGMT *skeymgmt = NULL;
     EVP_SKEY *ret = NULL;
 
-    if (prov != NULL) {
-        skeymgmt = evp_skeymgmt_fetch_from_prov(prov, skey->skeymgmt->type_name,
-                                                propquery);
+    if (skey == NULL) {
+        ERR_raise(ERR_LIB_EVP, ERR_R_PASSED_NULL_PARAMETER);
+        return NULL;
+    }
 
+    if (prov != NULL) {
+        if (skey->skeymgmt->prov == prov)
+            skeymgmt = skey->skeymgmt;
+        else
+            skeymgmt = evp_skeymgmt_fetch_from_prov(prov, skey->skeymgmt->type_name,
+                                                    propquery);
     } else {
         /* If no provider, get the default skeymgmt */
         skeymgmt = EVP_SKEYMGMT_fetch(libctx, skey->skeymgmt->type_name,
                                       propquery);
     }
+    if (skeymgmt == NULL) {
+        ERR_raise(ERR_LIB_EVP, ERR_R_FETCH_FAILED);
+        return NULL;
+    }
 
     /* Short-circuit if destination provider is the same as origin */
     if (skey->skeymgmt->name_id == skeymgmt->name_id
@@ -294,12 +299,11 @@ EVP_SKEY *EVP_SKEY_to_provider(EVP_SKEY *skey, OSSL_LIB_CTX *libctx,
     if (ctx.keydata == NULL)
         goto err;
 
-    ret = evp_skey_alloc();
+    ret = evp_skey_alloc(skeymgmt);
     if (ret == NULL)
         goto err;
 
     ret->keydata = ctx.keydata;
-    ret->skeymgmt = skeymgmt;
 
     return ret;