Harden parameter checks in provider functions by mamckee · Pull Request #168 · microsoft/SymCrypt-OpenSSL

mamckee · 2026-05-04T21:11:03Z

Multiple places in the provider, especially ctx get/set param functions, implicitly rely on the context parameter supplied by the EVP layer being non-NULL. The default provider is inconsistent with these checks, but some interfaces do check whether the context is NULL.

This PR addresses the behavior gap with the default provider and hardens additional functions to check whether the supplied context is NULL. This makes the SymCrypt provider more resilient towards applications that call the OpenSSL APIs in unconventional ways.

This PR also adds a check in set context param functions to exit early if the params array is NULL. This is common for context initialization, where set context param functions are always called, but frequently don't pass any params.

Harden provider interface functions with extra NULL checks
- Check context is not NULL before all get/set param functions. This is checked by some of the default provider interfaces. I've added the check to all of these functions for consistency. If a caller calls one of these functions with an uninitialized EVP context, it could lead to a crash. While technically a bad calling pattern and bug on the caller, our provider should behave the same as the default provider and fail instead of crashing.
- Added NULL check to context duplication functions to return early if the supplied context for duplication is NULL.
- Added NULL check to SKEY init functions, that gracefully handles a NULL skey parameter rather than crashing.
Added check to set param functions to see if the param array is empty. This is a slight optimization for the init path, since *set_ctx_params functions can often be called with no params. *get_ctx_params is very unlikely to be called with an empty parameter array.
Cleaned up styling in affected functions in older files to match the style used elsewhere in the provider. Particularly in the AES and rand get/set parameter functions.

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/digests/. - p_scossl_shake_set_ctx_params: check ctx and params for NULL - p_scossl_cshake_set_ctx_params: check ctx and params for NULL - p_scossl_cshake_settable_ctx_params: tolerate NULL ctx - p_scossl_digest_get_state_internal: check ctx and params for NULL - p_scossl_digest_set_state_internal: check ctx and params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/macs/. - p_scossl_hmac_set_ctx_params: check ctx and params for NULL - p_scossl_cmac_set_ctx_params: check ctx and params for NULL - p_scossl_kmac_set_ctx_params: check ctx and params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/kdfs/. - p_scossl_hkdf_set_ctx_params: check ctx and params for NULL - p_scossl_tls13kdf_set_ctx_params: check ctx and params for NULL - p_scossl_kbkdf_set_ctx_params: check ctx and params for NULL - p_scossl_pbkdf2_set_ctx_params: check ctx and params for NULL - p_scossl_srtpkdf_set_ctx_params: check ctx and params for NULL - p_scossl_sshkdf_set_ctx_params: check ctx and params for NULL - p_scossl_sskdf_set_ctx_params: check ctx and params for NULL - p_scossl_tls1prf_set_ctx_params: check ctx and params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/ciphers/. - p_scossl_aes_generic_set_ctx_params: check ctx and params for NULL - p_scossl_aes_gcm_set_ctx_params: check ctx and params for NULL - p_scossl_aes_ccm_set_ctx_params: check ctx and params for NULL - p_scossl_aes_xts_set_ctx_params: check ctx and params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/kem/. - p_scossl_mlkem_set_ctx_params: check params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/exchange/. - p_scossl_dh_set_ctx_params: check ctx and params for NULL - p_scossl_kdf_keyexch_set_ctx_params: check ctx and params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/asymciphers/. - p_scossl_rsa_cipher_get_ctx_params: check ctx for NULL - p_scossl_rsa_cipher_set_ctx_params: check ctx and params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/signature/. - p_scossl_rsa_set_ctx_params: check ctx and params for NULL - p_scossl_rsa_get_ctx_params: check ctx for NULL - p_scossl_ecdsa_set_ctx_params: check ctx and params for NULL - p_scossl_ecdsa_get_ctx_params: check ctx for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/keymgmt/. - p_scossl_rsa_keygen_set_params: check genCtx and params for NULL - p_scossl_dh_keygen_set_params: check genCtx and params for NULL - p_scossl_dh_keymgmt_set_params: check ctx and params for NULL - p_scossl_ecc_keygen_set_params: check genCtx and params for NULL - p_scossl_ecc_keymgmt_set_params: check keyCtx and params for NULL - p_scossl_mlkem_keymgmt_set_params: check keyCtx and params for NULL - p_scossl_mlkem_hybrid_keymgmt_set_params: check keyCtx and params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <copilot@github.com>

Copilot

Pull request overview

This PR aims to harden SymCrypt provider entry points against unconventional EVP-layer usage by adding NULL-context checks, short-circuiting empty parameter arrays in many set-param paths, and cleaning up some compatibility/style code around the provider surface.

Changes:

Added NULL ctx checks to many provider dup/get/set-param helpers across signatures, MACs, KDFs, ciphers, key exchange, KEM, digests, encoder, and key management code.
Introduced p_scossl_is_params_empty() and used it in many set-param handlers to return early when no parameters are supplied.
Removed obsolete OpenSSL 3.0 compatibility declarations/definitions and did small include/style cleanups in affected files.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
SymCryptProvider/src/signature/p_scossl_rsa_signature.c	Hardened RSA signature ctx duplication and ctx/md param handling.
SymCryptProvider/src/signature/p_scossl_ecdsa_signature.c	Hardened ECDSA signature ctx duplication and ctx/md param handling.
SymCryptProvider/src/p_scossl_rand.c	Minor RAND ctx-param style cleanup and shared header include.
SymCryptProvider/src/p_scossl_ecc.c	Added NULL check in ECC key ctx duplication.
SymCryptProvider/src/p_scossl_base.c	Removed old OpenSSL 3.0 compatibility shims.
SymCryptProvider/src/mac/p_scossl_kmac.c	Added NULL/empty-param guards for KMAC ctx operations.
SymCryptProvider/src/mac/p_scossl_hmac.c	Added NULL/empty-param guards for HMAC ctx operations.
SymCryptProvider/src/mac/p_scossl_cmac.c	Added NULL/empty-param guards for CMAC ctx operations.
SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c	Added extra NULL checks in RSA keymgmt duplication, keygen, get, has, and match paths.
SymCryptProvider/src/keymgmt/p_scossl_mlkem_keymgmt.c	Added NULL checks in ML-KEM keymgmt duplication and param accessors.
SymCryptProvider/src/keymgmt/p_scossl_mlkem_hybrid_keymgmt.c	Added NULL checks in hybrid ML-KEM keymgmt duplication and param accessors.
SymCryptProvider/src/keymgmt/p_scossl_ecc_keymgmt.c	Added NULL checks in ECC keygen/get/set/match helpers and reordered dependent locals.
SymCryptProvider/src/keymgmt/p_scossl_dh_keymgmt.c	Added NULL checks in DH key duplication, keygen, get/set, and match helpers.
SymCryptProvider/src/keyexch/p_scossl_kdf_keyexch.c	Added NULL/empty-param guards around wrapped KDF keyexch ctx helpers.
SymCryptProvider/src/keyexch/p_scossl_ecdh.c	Added NULL check in ECDH ctx duplication.
SymCryptProvider/src/keyexch/p_scossl_dh.c	Added NULL/empty-param guards in DH key exchange ctx helpers.
SymCryptProvider/src/kem/p_scossl_mlkem.c	Added NULL checks in ML-KEM KEM ctx duplication and set-param path.
SymCryptProvider/src/kem/p_scossl_mlkem_hybrid.c	Added NULL check in hybrid ML-KEM KEM ctx duplication.
SymCryptProvider/src/kdf/p_scossl_tls1prf.c	Added NULL/empty-param guards in TLS1 PRF ctx helpers.
SymCryptProvider/src/kdf/p_scossl_sskdf.c	Added NULL/empty-param guards in SSKDF ctx helpers plus whitespace cleanup.
SymCryptProvider/src/kdf/p_scossl_sshkdf.c	Added NULL/empty-param guards in SSHKDF ctx helpers plus formatting cleanup.
SymCryptProvider/src/kdf/p_scossl_srtpkdf.c	Added NULL/empty-param guards in SRTP KDF ctx helpers and shared header include.
SymCryptProvider/src/kdf/p_scossl_pbkdf2.c	Added NULL/empty-param guards in PBKDF2 ctx helpers.
SymCryptProvider/src/kdf/p_scossl_kbkdf.c	Added NULL/empty-param guards in KBKDF ctx helpers.
SymCryptProvider/src/kdf/p_scossl_hkdf.c	Added NULL/empty-param guards in HKDF/TLS13-KDF ctx helpers.
SymCryptProvider/src/encoder/p_scossl_encode_common.c	Added NULL/empty-param guards in encoder ctx param setter.
SymCryptProvider/src/digests/p_scossl_shake.c	Added NULL/empty-param guards for SHAKE ctx params.
SymCryptProvider/src/digests/p_scossl_digest_generic.c	Added NULL checks in generic digest state import/export helpers.
SymCryptProvider/src/digests/p_scossl_digest_common.c	Added digest dupctx NULL guard and empty-param fast path for generic get_params.
SymCryptProvider/src/digests/p_scossl_cshake.c	Added NULL/empty-param guards in cSHAKE ctx helpers and shared header include.
SymCryptProvider/src/ciphers/p_scossl_aes.c	Added NULL/empty-param guards in generic AES ctx helpers.
SymCryptProvider/src/ciphers/p_scossl_aes_xts.c	Added NULL guards in AES-XTS dup/init/get/set helpers and shared header include.
SymCryptProvider/src/ciphers/p_scossl_aes_aead.c	Added NULL/empty-param guards in AES-GCM/CCM ctx helpers and minor style cleanup.
SymCryptProvider/src/asymcipher/p_scossl_rsa_cipher.c	Added NULL/empty-param guards in RSA asymcipher ctx param helpers.
SymCryptProvider/inc/p_scossl_base.h.in	Added shared empty-params helper used by provider ctx param code.
ScosslCommon/src/scossl_mac.c	Added NULL guard in common MAC ctx duplication.
ScosslCommon/src/scossl_aes_aead.c	Minor style-only cleanup in common AES AEAD helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (4)

SymCryptProvider/src/signature/p_scossl_rsa_signature.c:1

This function previously handled params == NULL by returning SCOSSL_SUCCESS, but that guard was removed. OSSL_PARAM_locate(params, ...) will dereference params, so calls that legitimately pass NULL (or an empty array) will now crash. Consider restoring the early return (or using p_scossl_is_params_empty(params) like the set_ctx_params paths) before attempting any OSSL_PARAM_locate calls.
SymCryptProvider/src/signature/p_scossl_ecdsa_signature.c:1
Similar to the RSA signature path: the existing params == NULL early-return was removed. If OpenSSL (or a caller) invokes get_ctx_params with params == NULL, OSSL_PARAM_locate will dereference NULL. Restoring the previous guard or using p_scossl_is_params_empty(params) at the top would avoid a potential crash.
SymCryptProvider/src/signature/p_scossl_rsa_signature.c:1
The new combined check reports PROV_R_MISSING_MESSAGE_DIGEST even when ctx == NULL. That error code is accurate for ctx->mdctx == NULL, but misleading for a NULL context pointer. Consider splitting the checks so ctx == NULL raises ERR_R_PASSED_NULL_PARAMETER, and reserve PROV_R_MISSING_MESSAGE_DIGEST for ctx->mdctx == NULL.
SymCryptProvider/src/signature/p_scossl_rsa_signature.c:1
When ctx == NULL, this now returns the PSS gettable param list. In OpenSSL provider APIs, OpenSSL may query gettable/settable ctx params before a context is fully configured, so returning a mode-specific list for the NULL-ctx case can change observable behavior. If the PSS list is not a strict superset of the non-PSS list, callers could see missing params (or unexpected ones). Consider returning a union/superset list when ctx == NULL, or defaulting to the generic (non-PSS) list for the NULL-ctx query.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (4)

SymCryptProvider/src/signature/p_scossl_rsa_signature.c:1

p_scossl_rsa_get_ctx_params no longer handles params == NULL (the previous early-return was removed). This will dereference NULL via OSSL_PARAM_locate(params, ...) and regress behavior for callers that pass NULL/empty params. Reintroduce an early return using p_scossl_is_params_empty(params) (returning SCOSSL_SUCCESS), similar to the updated set_ctx_params functions.
SymCryptProvider/src/signature/p_scossl_ecdsa_signature.c:1
p_scossl_ecdsa_get_ctx_params removed the prior params == NULL fast-path but still calls OSSL_PARAM_locate(params, ...), which will crash if params is NULL. Add an early return using p_scossl_is_params_empty(params) and return SCOSSL_SUCCESS to preserve the old behavior and align with the PR’s “empty params” hardening theme.
SymCryptProvider/src/signature/p_scossl_rsa_signature.c:1
This raises PROV_R_MISSING_MESSAGE_DIGEST even when ctx itself is NULL, which makes diagnosing caller misuse harder (and is inconsistent with many other functions in this PR that raise ERR_R_PASSED_NULL_PARAMETER for NULL ctx). Split the checks so ctx == NULL raises ERR_R_PASSED_NULL_PARAMETER, and only ctx->mdctx == NULL raises PROV_R_MISSING_MESSAGE_DIGEST.
SymCryptProvider/src/p_scossl_ecc.c:1
This changes the non-X25519 pointFormat from the previously fixed SYMCRYPT_ECPOINT_FORMAT_XY to (SYMCRYPT_ECPOINT_FORMAT)keyCtx->conversionFormat. If conversionFormat is not guaranteed to be a valid SYMCRYPT_ECPOINT_FORMAT value (or uses a different enum space), this can lead to incorrect key export/import behavior or SymCrypt errors. Consider keeping the prior XY default here, or add an explicit mapping/validation from conversionFormat to SYMCRYPT_ECPOINT_FORMAT before casting.

Copilot

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.

        if ((selection & OSSL_KEYMGMT_SELECT_KEYPAIR) != 0 && keyCtx->initialized)
        {
+            pointFormat = keyCtx->isX25519 ? SYMCRYPT_ECPOINT_FORMAT_X : (SYMCRYPT_ECPOINT_FORMAT)keyCtx->conversionFormat;
+
            if (copyCtx->curve == NULL)


 static SCOSSL_ECDSA_CTX *p_scossl_ecdsa_dupctx(_In_ SCOSSL_ECDSA_CTX *ctx)
 {
-    SCOSSL_ECDSA_CTX *copyCtx = OPENSSL_zalloc(sizeof(SCOSSL_ECDSA_CTX));
+    SCOSSL_ECDSA_CTX *copyCtx;
+
+    if (ctx == NULL)


 static SCOSSL_STATUS p_scossl_rsa_digest_signverify_update(_In_ SCOSSL_RSA_SIGN_CTX *ctx,
                                                           _In_reads_bytes_(datalen) const unsigned char *data, size_t datalen)
 {
-    if (ctx->mdctx == NULL)
+    if (ctx == NULL || ctx->mdctx == NULL)
    {
+        ERR_raise(ERR_LIB_PROV, PROV_R_MISSING_MESSAGE_DIGEST);
        return SCOSSL_FAILURE;


mamckee and others added 21 commits May 1, 2026 17:32

kem: add missing NULL parameter checks

d1116bf

Mirror NULL-parameter checks performed by the OpenSSL default provider in providers/implementations/kem/. - p_scossl_mlkem_set_ctx_params: check params for NULL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Clean up asym cipher

e98ef12

Cleanup AES AEAD

812f21b

Co-authored-by: Copilot <copilot@github.com>

Cleanup ciphers

2105df3

Co-authored-by: Copilot <copilot@github.com>

Cleanup digests

42776e0

Co-authored-by: Copilot <copilot@github.com>

Cleanup kdf

6a7f21c

Co-authored-by: Copilot <copilot@github.com>

Cleanup kem

05c92b8

Co-authored-by: Copilot <copilot@github.com>

Cleanup keyexch

9a7f0fe

Co-authored-by: Copilot <copilot@github.com>

Cleanup keymgmt

6a3823e

Co-authored-by: Copilot <copilot@github.com>

Cleanup mac

92f2cc5

Co-authored-by: Copilot <copilot@github.com>

Cleanup signature

27cea8d

Co-authored-by: Copilot <copilot@github.com>

Cleanup encode

143ce52

Co-authored-by: Copilot <copilot@github.com>

Add NULL param check to set_ctx_param functions

e6a7398

Co-authored-by: Copilot <copilot@github.com>

mamckee requested a review from Copilot May 4, 2026 21:11

Copilot started reviewing on behalf of mamckee May 4, 2026 21:11 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

PR comments

6ce1ab0

mamckee requested a review from Copilot May 5, 2026 17:37

Copilot AI reviewed May 5, 2026

View reviewed changes

Copilot started reviewing on behalf of mamckee May 5, 2026 18:29 View session

mamckee requested a review from Copilot May 6, 2026 22:33

Copilot AI reviewed May 6, 2026

View reviewed changes

Comment thread SymCryptProvider/src/ciphers/p_scossl_aes_xts.c

Comment thread SymCryptProvider/inc/p_scossl_base.h.in Outdated

Comment thread SymCryptProvider/src/kdf/p_scossl_hkdf.c

Copilot started reviewing on behalf of mamckee May 6, 2026 22:48 View session

Fix typo in comment

3aff983

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot stopped work on behalf of mamckee due to an error May 7, 2026 19:27
Before you can use Copilot cloud agent, you need to pick a "Usage billed to" option in your Copilot settings. Update your settings at https://github.com/settings/copilot/features, then try again.

Raise error in tls 1_3 kdf set params

312661c

mamckee requested a review from Copilot May 7, 2026 19:44

Copilot AI reviewed May 7, 2026

View reviewed changes

Comment thread SymCryptProvider/src/keyexch/p_scossl_kdf_keyexch.c

Comment thread SymCryptProvider/inc/p_scossl_base.h.in

Copilot started reviewing on behalf of mamckee May 7, 2026 20:07 View session

mamckee added 2 commits May 7, 2026 21:33

Allow empty init case for SKEYs

d49cd69

Cleanup and add missing checks

90f7b67

mamckee requested a review from Copilot May 7, 2026 22:58

mamckee marked this pull request as ready for review May 7, 2026 22:58

mamckee requested a review from samuel-lee-msft May 7, 2026 23:00

Copilot started reviewing on behalf of mamckee May 7, 2026 23:00 View session

mamckee requested a review from MS-megliu May 7, 2026 23:00

Copilot AI reviewed May 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Harden parameter checks in provider functions#168

Harden parameter checks in provider functions#168
mamckee wants to merge 26 commits intomainfrom
mamckee-parameter-safety-checks

mamckee commented May 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mamckee commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mamckee commented May 4, 2026 •

edited

Loading