Skip to content

Bugfixes for the SymCrypt provider #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mamckee wants to merge 11 commits into
base: scossl-1.9
Choose a base branch
from
Open

Bugfixes for the SymCrypt provider #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d1fb8f1
Compare correct values in rsa match
mamckee Apr 13, 2026
8cc3749
Use correct bit size for DH P and Q in dup
mamckee Apr 13, 2026
6a2114c
Properly validate x25519 key length
mamckee Apr 14, 2026
8b18bf2
Update RSA encrypt/decrypt input validation to match default implemen…
mamckee Apr 14, 2026
0d032e9
Use correct bit lengths in copy group only case for DH
mamckee Apr 14, 2026
d9b1807
Add mac initialized flag
mamckee Apr 14, 2026
ba805c0
Compare to correct mac stize in aes
mamckee Apr 14, 2026
22d77cc
Fix RSA size check
mamckee Apr 14, 2026
cd2490f
Update the version
mamckee Apr 21, 2026
1cf36dc
Check whether RSA key is valid for non-TLS case if out is non-null
mamckee Apr 23, 2026
46cc4a6
Use secure memory for x25519 private key import scratch space
mamckee Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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")

Expand Down
2 changes: 2 additions & 0 deletions ScosslCommon/inc/scossl_helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions ScosslCommon/inc/scossl_mac.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ typedef struct
PCSCOSSL_MAC_EX pMacEx;
PBYTE pbKey;
SIZE_T cbKey;
BOOL initialized;

// Provider specific fields
PVOID libctx;
Expand Down
2 changes: 1 addition & 1 deletion ScosslCommon/src/scossl_dh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions ScosslCommon/src/scossl_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
49 changes: 38 additions & 11 deletions ScosslCommon/src/scossl_mac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -336,21 +342,21 @@ 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)
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)
Expand All @@ -359,6 +365,13 @@ SCOSSL_STATUS scossl_mac_init(SCOSSL_MAC_CTX *ctx,
"SymCryptMacExpandKey failed", scError);
return SCOSSL_FAILURE;
}

ctx->initialized = TRUE;
Copy link
Copy Markdown
Contributor

@MS-megliu MS-megliu Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (nit): Consider setting ctx->initialized = FALSE at the entry of the if (pbKey != NULL) block (around line 345), before attempting key expansion.

Currently, if a previously-initialized context calls init with a new key and expandKeyFunc fails (early return at line 367), ctx->initialized remains TRUE from the prior successful init. Subsequent update/final calls would then silently operate with the stale old expanded key rather than failing with the ""not initialized"" error.

Poisoning at entry and setting TRUE only on success (as you already do at this line) would close that window.

}

if (!ctx->initialized)
{
Comment thread
mamckee marked this conversation as resolved.
return SCOSSL_FAILURE;
}

ctx->pMac->initFunc(ctx->macState, ctx->expandedKey);
Expand All @@ -370,6 +383,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;
Expand All @@ -379,6 +399,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;
}
Comment thread
mamckee marked this conversation as resolved.

if (pbResult != NULL)
{
if (outsize < ctx->pMac->resultSize)
Expand Down
65 changes: 61 additions & 4 deletions SymCryptProvider/src/asymcipher/p_scossl_rsa_cipher.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <openssl/core_names.h>
#include <openssl/proverr.h>
#include <openssl/prov_ssl.h>

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -141,11 +142,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)
Expand All @@ -160,6 +162,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)
Expand Down Expand Up @@ -187,11 +208,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)
Expand All @@ -206,6 +228,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;
}
}
Comment thread
mamckee marked this conversation as resolved.
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;
}
Comment thread
mamckee marked this conversation as resolved.
}

// Default to SHA1 for OAEP. Update md in context so this is
// reflected in getparam
if (ctx->padding == RSA_PKCS1_OAEP_PADDING)
Expand Down
2 changes: 1 addition & 1 deletion SymCryptProvider/src/ciphers/p_scossl_aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion SymCryptProvider/src/keymgmt/p_scossl_dh_keymgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading