Bugfixes for the SymCrypt provider

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.13.0)
 
 project(SymCrypt-OpenSSL
-    VERSION 1.9.5
+    VERSION 1.9.6
     DESCRIPTION "The SymCrypt engine and provider for OpenSSL (SCOSSL)"
     HOMEPAGE_URL "https://github.com/microsoft/SymCrypt-OpenSSL")
 

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_dh.c

@@ -73,7 +73,7 @@ SCOSSL_DH_KEY_CTX *scossl_dh_dup_key_ctx(SCOSSL_DH_KEY_CTX *ctx, BOOL copyGroup)
                     NULL,
                     NULL);
 
-                if ((pDlgroupCopy = SymCryptDlgroupAllocate(pcbPrimeP, pcbPrimeQ)) == NULL)
+                if ((pDlgroupCopy = SymCryptDlgroupAllocate(pcbPrimeP * 8, pcbPrimeQ * 8)) == NULL)
                 {
                     goto cleanup;
                 }

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

@@ -149,6 +149,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 +210,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 +286,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 +342,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 +367,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 +387,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 +403,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/src/asymcipher/p_scossl_rsa_cipher.c

@@ -8,6 +8,7 @@
 
 #include <openssl/core_names.h>
 #include <openssl/proverr.h>
+#include <openssl/prov_ssl.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -79,7 +80,16 @@ static void p_scossl_rsa_cipher_freectx(_Inout_ SCOSSL_RSA_CIPHER_CTX *ctx)
 
 static SCOSSL_RSA_CIPHER_CTX *p_scossl_rsa_cipher_dupctx(_Inout_ SCOSSL_RSA_CIPHER_CTX *ctx)
 {
-    return OPENSSL_memdup(ctx, sizeof(SCOSSL_RSA_CIPHER_CTX));
+    SCOSSL_RSA_CIPHER_CTX *copyCtx = OPENSSL_memdup(ctx, sizeof(SCOSSL_RSA_CIPHER_CTX));
+
+    if (copyCtx != NULL && ctx->pbLabel != NULL &&
+        (copyCtx->pbLabel = OPENSSL_memdup(ctx->pbLabel, ctx->cbLabel)) == NULL)
+    {
+        OPENSSL_free(copyCtx);
+        copyCtx = NULL;
+    }
+
+    return copyCtx;
 }
 
 static SCOSSL_STATUS p_scossl_rsa_cipher_init(_Inout_ SCOSSL_RSA_CIPHER_CTX *ctx, _In_ SCOSSL_PROV_RSA_KEY_CTX *keyCtx,
@@ -141,11 +151,12 @@ static SCOSSL_STATUS p_scossl_rsa_decrypt_init(_Inout_ SCOSSL_RSA_CIPHER_CTX *ct
 }
 
 static SCOSSL_STATUS p_scossl_rsa_cipher_encrypt(_In_ SCOSSL_RSA_CIPHER_CTX *ctx,
-                                                 _Out_writes_bytes_(*outlen) unsigned char *out, _Out_ size_t *outlen, size_t outsize,
-                                                 _In_reads_bytes_(inlen) const unsigned char *in, size_t inlen)
+                                                  _Out_writes_bytes_(*outlen) unsigned char *out, _Out_ size_t *outlen, size_t outsize,
+                                                  _In_reads_bytes_(inlen) const unsigned char *in, size_t inlen)
 {
     int mdnid = 0;
     INT32 cbResult;
+    UINT32 cbModulus;
     SCOSSL_STATUS ret;
 
     if (ctx->keyCtx == NULL)
@@ -160,6 +171,25 @@ static SCOSSL_STATUS p_scossl_rsa_cipher_encrypt(_In_ SCOSSL_RSA_CIPHER_CTX *ctx
         return SCOSSL_FAILURE;
     }
 
+    cbModulus = SymCryptRsakeySizeofModulus(ctx->keyCtx->key);
+    if (cbModulus == 0)
+    {
+        ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_KEY);
+        return SCOSSL_FAILURE;
+    }
+
+    if (out == NULL)
+    {
+        *outlen = cbModulus;
+        return SCOSSL_SUCCESS;
+    }
+
+    if (outsize < cbModulus)
+    {
+        ERR_raise(ERR_LIB_PROV, PROV_R_OUTPUT_BUFFER_TOO_SMALL);
+        return SCOSSL_FAILURE;
+    }
+
     // Default to SHA1 for OAEP. Update md in context so this is
     // reflected in getparam
     if (ctx->padding == RSA_PKCS1_OAEP_PADDING)
@@ -187,11 +217,12 @@ static SCOSSL_STATUS p_scossl_rsa_cipher_encrypt(_In_ SCOSSL_RSA_CIPHER_CTX *ctx
 }
 
 static SCOSSL_STATUS p_scossl_rsa_cipher_decrypt(_In_ SCOSSL_RSA_CIPHER_CTX *ctx,
-                                                 _Out_writes_bytes_(*outlen) unsigned char *out, _Out_ size_t *outlen, size_t outsize,
-                                                 _In_reads_bytes_(inlen) const unsigned char *in, size_t inlen)
+                                                  _Out_writes_bytes_(*outlen) unsigned char *out, _Out_ size_t *outlen, size_t outsize,
+                                                  _In_reads_bytes_(inlen) const unsigned char *in, size_t inlen)
 {
     int mdnid = 0;
     INT32 cbResult;
+    UINT32 cbModulus;
     SCOSSL_STATUS ret;
 
     if (ctx->keyCtx == NULL)
@@ -206,6 +237,41 @@ static SCOSSL_STATUS p_scossl_rsa_cipher_decrypt(_In_ SCOSSL_RSA_CIPHER_CTX *ctx
         return SCOSSL_FAILURE;
     }
 
+    if (ctx->padding == RSA_PKCS1_WITH_TLS_PADDING)
+    {
+        if (out == NULL)
+        {
+            *outlen = SSL_MAX_MASTER_KEY_LENGTH;
+            return SCOSSL_SUCCESS;
+        }
+        if (outsize < SSL_MAX_MASTER_KEY_LENGTH)
+        {
+            ERR_raise(ERR_LIB_PROV, PROV_R_OUTPUT_BUFFER_TOO_SMALL);
+            return SCOSSL_FAILURE;
+        }
+    }
+    else
+    {
+        cbModulus = SymCryptRsakeySizeofModulus(ctx->keyCtx->key);
+        if (cbModulus == 0)
+        {
+            ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_KEY);
+            return SCOSSL_FAILURE;
+        }
+
+        if (out == NULL)
+        {
+            *outlen = cbModulus;
+            return SCOSSL_SUCCESS;
+        }
+
+        if (outsize < cbModulus)
+        {
+            ERR_raise(ERR_LIB_PROV, PROV_R_OUTPUT_BUFFER_TOO_SMALL);
+            return SCOSSL_FAILURE;
+        }
+    }
+
     // Default to SHA1 for OAEP. Update md in context so this is
     // reflected in getparam
     if (ctx->padding == RSA_PKCS1_OAEP_PADDING)

SymCryptProvider/src/ciphers/p_scossl_aes.c

@@ -774,7 +774,7 @@ static SCOSSL_STATUS p_scossl_aes_generic_set_ctx_params(_Inout_ SCOSSL_AES_CTX
             return SCOSSL_FAILURE;
         }
 
-        if (ctx->tlsMacSize > EVP_MAX_MD_SIZE)
+        if (tlsMacSize > EVP_MAX_MD_SIZE)
         {
             ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_MAC);
             return SCOSSL_FAILURE;

SymCryptProvider/src/keymgmt/p_scossl_dh_keymgmt.c

@@ -155,7 +155,7 @@ static SCOSSL_PROV_DH_KEY_CTX *p_scossl_dh_keymgmt_dup_key_ctx(_In_ const SCOSSL
                     NULL,
                     NULL);
 
-                if ((copyCtx->pDlGroup = SymCryptDlgroupAllocate(pcbPrimeP, pcbPrimeQ)) == NULL)
+                if ((copyCtx->pDlGroup = SymCryptDlgroupAllocate(pcbPrimeP * 8, pcbPrimeQ * 8)) == NULL)
                 {
                     OPENSSL_free(copyCtx);
                     return NULL;