[WIP] Prefer using non-deprecated EC OSSL APIs where possible#127190
[WIP] Prefer using non-deprecated EC OSSL APIs where possible#127190PranavSenthilnathan wants to merge 2 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
There was a problem hiding this comment.
Pull request overview
This PR introduces new OpenSSL 3.0+ EVP_PKEY-based EC key generation/import paths (to avoid deprecated EC_KEY APIs where possible), with managed code updated to prefer these paths and fall back to legacy behavior when needed.
Changes:
- Add native CryptoNative exports to generate/import EC keys via EVP_PKEY (named curves and explicit parameters).
- Update managed ECOpenSsl/ECDH code to use the new EVP_PKEY paths first, with legacy EC_KEY fallback.
- Extend the OpenSSL shim to light up additional OpenSSL 3.0 param-building/keygen APIs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Security.Cryptography.Native/pal_ecc_import_export.h | Adds new native API declarations for EVP_PKEY EC key generation/import. |
| src/native/libs/System.Security.Cryptography.Native/pal_ecc_import_export.c | Implements EVP_PKEY-based EC keygen and fromdata import (named + explicit). |
| src/native/libs/System.Security.Cryptography.Native/opensslshim.h | Lights up OpenSSL 3.0 param_build and related functions used by new native code. |
| src/native/libs/System.Security.Cryptography.Native/entrypoints.c | Exposes the new native functions via the CryptoNative entrypoint table. |
| src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.cs | Prefers EVP_PKEY EC keygen/import for OpenSSL 3.0 with fallback to EC_KEY. |
| src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSslPublicKey.cs | Stores/uses EVP_PKEY handles directly instead of wrapping EC_KEY. |
| src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs | Uses EVP_PKEY curve-name detection and imports via new ECOpenSsl helpers. |
| src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs | Adds P/Invoke wrappers for the new CryptoNative EVP_PKEY EC APIs. |
| #ifdef NEED_OPENSSL_3_0 | ||
| if (!API_EXISTS(EVP_PKEY_keygen)) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| int nid = OBJ_txt2nid(oid); | ||
| if (!nid) | ||
| { | ||
| return -1; | ||
| } | ||
|
|
||
| const char* groupName = OBJ_nid2sn(nid); | ||
| if (!groupName) | ||
| { | ||
| return -1; | ||
| } | ||
|
|
||
| EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new_from_name(NULL, "EC", NULL); | ||
| if (ctx == NULL) | ||
| { | ||
| goto error; | ||
| } | ||
|
|
||
| if (EVP_PKEY_keygen_init(ctx) <= 0) | ||
| { | ||
| goto error; | ||
| } | ||
|
|
||
| if (EVP_PKEY_CTX_set_group_name(ctx, groupName) <= 0) | ||
| { |
There was a problem hiding this comment.
The OpenSSL 3.0 keygen path only checks API_EXISTS(EVP_PKEY_keygen) but then unconditionally calls other lightup APIs (EVP_PKEY_CTX_new_from_name, EVP_PKEY_CTX_set_group_name). Since these are LIGHTUP_FUNCTIONs, a missing symbol would result in calling through a null function pointer. Please extend the guard to include all lightup functions used in this block (or switch them to REQUIRED if they are truly mandatory when NEED_OPENSSL_3_0).
| #ifdef NEED_OPENSSL_3_0 | ||
| if (!API_EXISTS(EVP_PKEY_fromdata) || !API_EXISTS(OSSL_PARAM_BLD_new)) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
CryptoNative_EvpPKeyCreateByEcKeyParameters guards only EVP_PKEY_fromdata and OSSL_PARAM_BLD_new, but then uses several other lightup functions (OSSL_PARAM_BLD_push_*, OSSL_PARAM_BLD_to_param, OSSL_PARAM_BLD_free, OSSL_PARAM_free, and EVP_PKEY_CTX_new_from_name). If any of these are unavailable at runtime, this will dereference a null function pointer. Please either add API_EXISTS(...) checks for the full set of required lightup APIs, or restructure to use non-lightup APIs / mark them REQUIRED where appropriate.
| ERR_clear_error(); | ||
|
|
||
| #ifdef NEED_OPENSSL_3_0 | ||
| if (!API_EXISTS(EVP_PKEY_fromdata) || !API_EXISTS(OSSL_PARAM_BLD_new)) |
There was a problem hiding this comment.
CryptoNative_EvpPKeyCreateByEcExplicitParameters has the same lightup-safety issue as the named-curve create path: it checks only EVP_PKEY_fromdata and OSSL_PARAM_BLD_new, but then calls multiple other LIGHTUP_FUNCTIONs (OSSL_PARAM_BLD helpers, OSSL_PARAM_free, EVP_PKEY_CTX_new_from_name). Please ensure all lightup functions used are verified via API_EXISTS before use to avoid null function pointer calls.
| if (!API_EXISTS(EVP_PKEY_fromdata) || !API_EXISTS(OSSL_PARAM_BLD_new)) | |
| if (!API_EXISTS(EVP_PKEY_CTX_new_from_name) || | |
| !API_EXISTS(EVP_PKEY_fromdata_init) || | |
| !API_EXISTS(EVP_PKEY_fromdata) || | |
| !API_EXISTS(OSSL_PARAM_BLD_new) || | |
| !API_EXISTS(OSSL_PARAM_BLD_free) || | |
| !API_EXISTS(OSSL_PARAM_BLD_to_param) || | |
| !API_EXISTS(OSSL_PARAM_BLD_push_utf8_string) || | |
| !API_EXISTS(OSSL_PARAM_BLD_push_BN) || | |
| !API_EXISTS(OSSL_PARAM_BLD_push_octet_string) || | |
| !API_EXISTS(OSSL_PARAM_free)) |
| internal static SafeEvpPKeyHandle GenerateECKey(ECCurve curve, out int keySize) | ||
| { | ||
| return ImportECKeyCore(new ECOpenSsl(curve), out keySize); | ||
| if (curve.IsNamed) | ||
| { | ||
| string oid = !string.IsNullOrEmpty(curve.Oid.Value) ? curve.Oid.Value : curve.Oid.FriendlyName!; | ||
|
|
||
| SafeEvpPKeyHandle? pkey = Interop.Crypto.EvpPKeyGenerateByEcKeyOid(oid); |
There was a problem hiding this comment.
GenerateECKey(ECCurve curve, out int keySize) uses curve.Oid.Value / curve.Oid.FriendlyName! without calling curve.Validate() first. Since ECCurve is a struct, callers can pass a default/invalid named curve (e.g., CurveType = Named with Oid == null), which would currently throw a NullReferenceException instead of the expected crypto/PlatformNotSupported exception. Consider calling curve.Validate() up front (matching ECOpenSsl.GenerateKey) before trying the EVP_PKEY path/fallback.
No description provided.