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.

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;
     }

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);

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,18 +26,17 @@ 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));
 
+    if (skey == NULL || !ossl_assert(skeymgmt != NULL))
+        return NULL;
+
     if (!CRYPTO_NEW_REF(&skey->references, 1))
         goto err;
 
@@ -45,6 +45,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 +55,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 +71,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,42 +97,25 @@ 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;
 }
 
 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;
 
     return skey;
 
  err:
-    EVP_SKEYMGMT_free(skeymgmt);
     EVP_SKEY_free(skey);
     return NULL;
 }
@@ -196,8 +192,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 +234,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);
 }
 
@@ -294,12 +286,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;
 

providers/implementations/skeymgmt/generic.c

@@ -45,6 +45,9 @@ void *generic_import(void *provctx, int selection, const OSSL_PARAM params[])
         return NULL;
 
     generic = OPENSSL_zalloc(sizeof(PROV_SKEY));
+    if (generic == NULL)
+        return NULL;
+
     generic->libctx = libctx;
 
     generic->type = SKEY_TYPE_GENERIC;