Harden parameter checks in provider functions

ScosslCommon/inc/scossl_helpers.h

@@ -119,8 +119,10 @@ typedef enum {
     SCOSSL_ERR_F_GET_SYMCRYPT_MAC_ALGORITHM,
     SCOSSL_ERR_F_HKDF_DERIVE,
     SCOSSL_ERR_F_MAC_DUPCTX,
+    SCOSSL_ERR_F_MAC_FINAL,
     SCOSSL_ERR_F_MAC_INIT,
     SCOSSL_ERR_F_MAC_SET_HMAC_MD,
+    SCOSSL_ERR_F_MAC_UPDATE,
     SCOSSL_ERR_F_RSA_DECRYPT,
     SCOSSL_ERR_F_RSA_ENCRYPT,
     SCOSSL_ERR_F_RSA_EXPORT_KEY,

ScosslCommon/inc/scossl_mac.h

@@ -32,6 +32,7 @@ typedef struct
     PCSCOSSL_MAC_EX pMacEx;
     PBYTE pbKey;
     SIZE_T cbKey;
+    BOOL initialized;
 
     // Provider specific fields
     PVOID libctx;

ScosslCommon/src/scossl_aes_aead.c

@@ -56,6 +56,7 @@ SCOSSL_STATUS scossl_aes_gcm_init_key(SCOSSL_CIPHER_GCM_CTX *ctx,
             return SCOSSL_FAILURE;
         }
     }
+
     if (key != NULL)
     {
         scError = SymCryptGcmExpandKey(&ctx->key, SymCryptAesBlockCipher, key, keylen);
@@ -64,6 +65,7 @@ SCOSSL_STATUS scossl_aes_gcm_init_key(SCOSSL_CIPHER_GCM_CTX *ctx,
             return SCOSSL_FAILURE;
         }
     }
+
     return SCOSSL_SUCCESS;
 }
 
@@ -299,7 +301,7 @@ SCOSSL_STATUS scossl_aes_gcm_set_iv_len(SCOSSL_CIPHER_GCM_CTX *ctx, size_t ivlen
     if (ivlen != ctx->ivlen)
     {
         ctx->ivlen = ivlen;
-    
+
         if (ctx->iv != NULL)
         {
             OPENSSL_free(ctx->iv);
@@ -455,7 +457,7 @@ SCOSSL_STATUS scossl_aes_ccm_init_key(SCOSSL_CIPHER_CCM_CTX *ctx,
 
     ctx->ccmStage = SCOSSL_CCM_STAGE_INIT;
     ctx->cbData = 0;
-    if (iv)
+    if (iv != NULL)
     {
         if (!scossl_aes_ccm_set_iv_len(ctx, ivlen))
         {
@@ -466,14 +468,16 @@ SCOSSL_STATUS scossl_aes_ccm_init_key(SCOSSL_CIPHER_CCM_CTX *ctx,
         memcpy(ctx->iv, iv, ctx->ivlen);
         ctx->ivSet = 1;
     }
-    if (key)
+
+    if (key != NULL)
     {
         scError = SymCryptAesExpandKey(&ctx->key, key, keylen);
         if (scError != SYMCRYPT_NO_ERROR)
         {
             return SCOSSL_FAILURE;
         }
     }
+
     return SCOSSL_SUCCESS;
 }
 

ScosslCommon/src/scossl_helpers.c

@@ -79,8 +79,10 @@ static ERR_STRING_DATA SCOSSL_ERR_function_strings[] = {
     {ERR_PACK(0, SCOSSL_ERR_F_GET_SYMCRYPT_MAC_ALGORITHM, 0), "scossl_get_symcrypt_hmac_algorithm"},
     {ERR_PACK(0, SCOSSL_ERR_F_HKDF_DERIVE, 0), "scossl_hkdf_derive"},
     {ERR_PACK(0, SCOSSL_ERR_F_MAC_DUPCTX, 0), "scossl_mac_dupctx"},
+    {ERR_PACK(0, SCOSSL_ERR_F_MAC_FINAL, 0), "scossl_mac_final"},
     {ERR_PACK(0, SCOSSL_ERR_F_MAC_INIT, 0), "scossl_mac_init"},
     {ERR_PACK(0, SCOSSL_ERR_F_MAC_SET_HMAC_MD, 0), "scossl_mac_set_hmac_md"},
+    {ERR_PACK(0, SCOSSL_ERR_F_MAC_UPDATE, 0), "scossl_mac_update"},
     {ERR_PACK(0, SCOSSL_ERR_F_RSA_DECRYPT, 0), "scossl_rsa_decrypt"},
     {ERR_PACK(0, SCOSSL_ERR_F_RSA_ENCRYPT, 0), "scossl_rsa_encrypt"},
     {ERR_PACK(0, SCOSSL_ERR_F_RSA_EXPORT_KEY, 0), "scossl_rsa_export_key"},

ScosslCommon/src/scossl_mac.c

@@ -86,6 +86,9 @@ SCOSSL_MAC_CTX *scossl_mac_dupctx(SCOSSL_MAC_CTX *ctx)
     SCOSSL_STATUS success = SCOSSL_FAILURE;
     SCOSSL_MAC_CTX *copyCtx = NULL;
 
+    if (ctx == NULL)
+        return NULL;
+
     if ((copyCtx = OPENSSL_zalloc(sizeof(SCOSSL_MAC_CTX))) != NULL)
     {
         if (ctx->pbKey != NULL)
@@ -149,6 +152,8 @@ SCOSSL_MAC_CTX *scossl_mac_dupctx(SCOSSL_MAC_CTX *ctx)
             }
         }
 
+        copyCtx->initialized = ctx->initialized;
+
         if (ctx->mdName != NULL &&
             (copyCtx->mdName = OPENSSL_strdup(ctx->mdName)) == NULL)
         {
@@ -208,6 +213,8 @@ SCOSSL_STATUS scossl_mac_set_hmac_md(SCOSSL_MAC_CTX *ctx, int mdNid)
         ctx->expandedKey = NULL;
     }
 
+    ctx->initialized = FALSE;
+
     switch (mdNid)
     {
     case NID_sha1:
@@ -282,6 +289,8 @@ SCOSSL_STATUS scossl_mac_set_cmac_cipher(SCOSSL_MAC_CTX *ctx, const EVP_CIPHER *
         ctx->expandedKey = NULL;
     }
 
+    ctx->initialized = FALSE;
+
     switch (EVP_CIPHER_nid(cipher))
     {
     case NID_aes_128_cbc:
@@ -336,21 +345,23 @@ SCOSSL_STATUS scossl_mac_init(SCOSSL_MAC_CTX *ctx,
         return SCOSSL_FAILURE;
     }
 
-    if (ctx->expandedKey == NULL)
+    if (pbKey != NULL)
     {
-        SCOSSL_COMMON_ALIGNED_ALLOC_EX(expandedKey, OPENSSL_malloc, SCOSSL_MAC_EXPANDED_KEY, ctx->pMac->expandedKeySize);
-        if (expandedKey == NULL)
+        ctx->initialized = FALSE;
+
+        if (ctx->expandedKey == NULL)
         {
-            SCOSSL_LOG_ERROR(SCOSSL_ERR_F_MAC_INIT, ERR_R_MALLOC_FAILURE,
-                "Failed to aligned allocate expanded key");
-            return SCOSSL_FAILURE;
-        }
+            SCOSSL_COMMON_ALIGNED_ALLOC_EX(expandedKey, OPENSSL_malloc, SCOSSL_MAC_EXPANDED_KEY, ctx->pMac->expandedKeySize);
+            if (expandedKey == NULL)
+            {
+                SCOSSL_LOG_ERROR(SCOSSL_ERR_F_MAC_INIT, ERR_R_MALLOC_FAILURE,
+                    "Failed to aligned allocate expanded key");
+                return SCOSSL_FAILURE;
+            }
 
-        ctx->expandedKey = expandedKey;
-    }
+            ctx->expandedKey = expandedKey;
+        }
 
-    if (pbKey != NULL)
-    {
         scError = ctx->pMac->expandKeyFunc(ctx->expandedKey, pbKey, cbKey);
 
         if (scError != SYMCRYPT_NO_ERROR)
@@ -359,6 +370,15 @@ SCOSSL_STATUS scossl_mac_init(SCOSSL_MAC_CTX *ctx,
                 "SymCryptMacExpandKey failed", scError);
             return SCOSSL_FAILURE;
         }
+
+        ctx->initialized = TRUE;
+    }
+
+    if (!ctx->initialized)
+    {
+        SCOSSL_LOG_ERROR(SCOSSL_ERR_F_MAC_INIT, SCOSSL_ERR_R_MISSING_CTX_DATA,
+            "MAC key has not been initialized");
+        return SCOSSL_FAILURE;
     }
 
     ctx->pMac->initFunc(ctx->macState, ctx->expandedKey);
@@ -370,6 +390,13 @@ _Use_decl_annotations_
 SCOSSL_STATUS scossl_mac_update(SCOSSL_MAC_CTX *ctx,
                                 PCBYTE pbData, SIZE_T cbData)
 {
+    if (!ctx->initialized)
+    {
+        SCOSSL_LOG_ERROR(SCOSSL_ERR_F_MAC_UPDATE, SCOSSL_ERR_R_MISSING_CTX_DATA,
+            "MAC key has not been initialized");
+        return SCOSSL_FAILURE;
+    }
+
     ctx->pMac->appendFunc(ctx->macState, pbData, cbData);
 
     return SCOSSL_SUCCESS;
@@ -379,6 +406,13 @@ _Use_decl_annotations_
 SCOSSL_STATUS scossl_mac_final(SCOSSL_MAC_CTX *ctx,
                                PBYTE pbResult, SIZE_T *cbResult, SIZE_T outsize)
 {
+    if (!ctx->initialized)
+    {
+        SCOSSL_LOG_ERROR(SCOSSL_ERR_F_MAC_FINAL, SCOSSL_ERR_R_MISSING_CTX_DATA,
+            "MAC key has not been initialized");
+        return SCOSSL_FAILURE;
+    }
+
     if (pbResult != NULL)
     {
         if (outsize < ctx->pMac->resultSize)

SymCryptProvider/inc/p_scossl_base.h.in

@@ -34,12 +34,12 @@ static const OSSL_PARAM p_scossl_param_types[] = {
     OSSL_PARAM_int(OSSL_PROV_PARAM_STATUS, NULL),
     OSSL_PARAM_END};
 
-// EVP_MD_CTX_dup is a helpful function for the provider, but was not added until OpenSSL 3.1
-// This function is copied from 3.1 to allow its use when the provider is built against 3.0
-#if OPENSSL_VERSION_MAJOR == 3 && OPENSSL_VERSION_MINOR == 0
-EVP_MD_CTX *EVP_MD_CTX_dup(const EVP_MD_CTX *in);
-
-#endif // OPENSSL_VERSION_MAJOR == 3 && OPENSSL_VERSION_MINOR == 0
+// Helper function from the default provider that is used by get/set
+// parameter functions to avoid iterating through an empty parameter array.
+static inline BOOL p_scossl_is_params_empty(_In_ const OSSL_PARAM params[])
+{
+    return params == NULL || params->key == NULL;
+}
 
 #ifdef __cplusplus
 }

SymCryptProvider/src/asymcipher/p_scossl_rsa_cipher.c

@@ -246,6 +246,12 @@ static SCOSSL_STATUS p_scossl_rsa_cipher_get_ctx_params(_In_ SCOSSL_RSA_CIPHER_C
 {
     OSSL_PARAM *p;
 
+    if (ctx == NULL)
+    {
+        ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_NULL_PARAMETER);
+        return SCOSSL_FAILURE;
+    }
+
     if ((p = OSSL_PARAM_locate(params, OSSL_ASYM_CIPHER_PARAM_PAD_MODE)) != NULL)
     {
         int i = 0;
@@ -307,6 +313,17 @@ static SCOSSL_STATUS p_scossl_rsa_cipher_set_ctx_params(_Inout_ SCOSSL_RSA_CIPHE
     const OSSL_PARAM *param_propq;
     const char *mdName, *mdProps;
 
+    if (ctx == NULL)
+    {
+        ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_NULL_PARAMETER);
+        return SCOSSL_FAILURE;
+    }
+
+    if (p_scossl_is_params_empty(params))
+    {
+        return SCOSSL_SUCCESS;
+    }
+
     if ((p = OSSL_PARAM_locate_const(params, OSSL_ASYM_CIPHER_PARAM_PAD_MODE)) != NULL)
     {
         // Padding mode may be passed as legacy NID or string, and is

SymCryptProvider/src/ciphers/p_scossl_aes.c

@@ -74,6 +74,9 @@ static void p_scossl_aes_generic_freectx(SCOSSL_AES_CTX *ctx)
 
 static SCOSSL_AES_CTX *p_scossl_aes_generic_dupctx(SCOSSL_AES_CTX *ctx)
 {
+    if (ctx == NULL)
+        return NULL;
+
     SCOSSL_COMMON_ALIGNED_ALLOC(copyCtx, OPENSSL_malloc, SCOSSL_AES_CTX);
     if (copyCtx != NULL)
     {
@@ -153,14 +156,42 @@ static SCOSSL_STATUS p_scossl_aes_generic_skey_encrypt_init(_Inout_ SCOSSL_AES_C
                                                             _In_reads_bytes_opt_(ivlen) const unsigned char *iv, size_t ivlen,
                                                             _In_ const OSSL_PARAM params[])
 {
-    return p_scossl_aes_generic_init_internal(ctx, TRUE, skey->pbKey, skey->cbKey, iv, ivlen, params);
+    PBYTE pbKey;
+    SIZE_T cbKey;
+
+    if (skey != NULL)
+    {
+        pbKey = skey->pbKey;
+        cbKey = skey->cbKey;
+    }
+    else
+    {
+        pbKey = NULL;
+        cbKey = 0;
+    }
+
+    return p_scossl_aes_generic_init_internal(ctx, 1, pbKey, cbKey, iv, ivlen, params);
 }
 
 static SCOSSL_STATUS p_scossl_aes_generic_skey_decrypt_init(_Inout_ SCOSSL_AES_CTX *ctx, _In_ SCOSSL_SKEY *skey,
                                                             _In_reads_bytes_opt_(ivlen) const unsigned char *iv, size_t ivlen,
                                                             _In_ const OSSL_PARAM params[])
 {
-    return p_scossl_aes_generic_init_internal(ctx, FALSE, skey->pbKey, skey->cbKey, iv, ivlen, params);
+    PBYTE pbKey;
+    SIZE_T cbKey;
+
+    if (skey != NULL)
+    {
+        pbKey = skey->pbKey;
+        cbKey = skey->cbKey;
+    }
+    else
+    {
+        pbKey = NULL;
+        cbKey = 0;
+    }
+
+    return p_scossl_aes_generic_init_internal(ctx, 0, pbKey, cbKey, iv, ivlen, params);
 }
 
 #define SYMCRYPT_OPENSSL_MASK8_SELECT( _mask, _a, _b ) (SYMCRYPT_FORCE_READ8(&_mask) & _a) | (~(SYMCRYPT_FORCE_READ8(&_mask)) & _b)
@@ -702,7 +733,13 @@ SCOSSL_STATUS p_scossl_aes_generic_get_params(_Inout_ OSSL_PARAM params[],
 
 static SCOSSL_STATUS p_scossl_aes_generic_get_ctx_params(_In_ SCOSSL_AES_CTX *ctx, _Inout_ OSSL_PARAM params[])
 {
-    OSSL_PARAM *p = NULL;
+    OSSL_PARAM *p;
+
+    if (ctx == NULL)
+    {
+        ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_NULL_PARAMETER);
+        return SCOSSL_FAILURE;
+    }
 
     if ((p = OSSL_PARAM_locate(params, OSSL_CIPHER_PARAM_KEYLEN)) != NULL &&
         !OSSL_PARAM_set_size_t(p, ctx->keylen))
@@ -746,7 +783,18 @@ static SCOSSL_STATUS p_scossl_aes_generic_get_ctx_params(_In_ SCOSSL_AES_CTX *ct
 
 static SCOSSL_STATUS p_scossl_aes_generic_set_ctx_params(_Inout_ SCOSSL_AES_CTX *ctx, _In_ const OSSL_PARAM params[])
 {
-    const OSSL_PARAM *p = NULL;
+    const OSSL_PARAM *p;
+
+    if (ctx == NULL)
+    {
+        ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_NULL_PARAMETER);
+        return SCOSSL_FAILURE;
+    }
+
+    if (p_scossl_is_params_empty(params))
+    {
+        return SCOSSL_SUCCESS;
+    }
 
     if ((p = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_PADDING)) != NULL)
     {