Skip to content

Remove SafeEcKeyHandle#129565

Draft
PranavSenthilnathan wants to merge 1 commit into
dotnet:mainfrom
PranavSenthilnathan:remove-safeeckeyhandle
Draft

Remove SafeEcKeyHandle#129565
PranavSenthilnathan wants to merge 1 commit into
dotnet:mainfrom
PranavSenthilnathan:remove-safeeckeyhandle

Conversation

@PranavSenthilnathan

Copy link
Copy Markdown
Member

Remove SafeEcKeyHandle. Any EC_KEY operations are handled in native now. The last remaining uses were parameter export (which used SafeEcKeyHandle in the fallback path for EC_KEY-backed EVP_PKeys) and the IntPtr constructors for ECDsaOpenSsl/ECDiffieHellmanOpenSsl.

…yHandle

Restructure EC key/curve parameter export to use a native driver pattern
that dispatches between OpenSSL 3.0 provider-backed APIs and legacy
EC_KEY-based APIs. This eliminates the managed version check and fallback
logic, moving it entirely into native code.

- Add native driver functions CryptoNative_EvpPKeyGetEcKeyParameters and
  CryptoNative_EvpPKeyGetEcCurveParameters that try the 3.0 API first,
  then fall back to the legacy EC_KEY API.
- Add CryptoNative_CreateEvpPkeyFromEcKey that creates an EVP_PKEY from
  a raw EC_KEY pointer and returns the key size in a single call.
- Update ECDsaOpenSsl(IntPtr) and ECDiffieHellmanOpenSsl(IntPtr) legacy
  constructors to use the new native function directly.
- Remove SafeEcKeyHandle from Unix production code entirely.
- Un-export CryptoNative_GetECKeyParameters, CryptoNative_GetECCurveParameters,
  CryptoNative_EvpPkeyGetEcKey, and CryptoNative_EvpPkeySetEcKey (now
  internal-only or removed).
- Simplify managed ECOpenSsl.ImportExport.cs to always use EvpPKey-based APIs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the managed SafeEcKeyHandle usage on Unix by moving the remaining EC_KEY*-based operations behind native entrypoints, and updates managed EC import/export and OpenSSL-based constructors to operate purely via EVP_PKEY* handles.

Changes:

  • Replaces CryptoNative_EvpPkeyGetEcKey / CryptoNative_EvpPkeySetEcKey with a new native helper that creates an EVP_PKEY* from an EC_KEY* and returns the computed key size.
  • Updates native EC parameter export to dispatch between OpenSSL 3 provider-backed keys and legacy EC_KEY keys internally (removing managed-side branching).
  • Removes the Unix SafeEcKeyHandle implementation and updates ECDsaOpenSsl / ECDiffieHellmanOpenSsl IntPtr constructors to use the new native entrypoint.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_eckey.h Replaces old EC_KEY get/set shims with a single “create EVP_PKEY from EC_KEY” export.
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_eckey.c Implements CryptoNative_CreateEvpPkeyFromEcKey and returns key size as an out param.
src/native/libs/System.Security.Cryptography.Native/pal_ecc_import_export.h Removes public EC_KEY-parameter exports; adjusts EVP_PKEY export signatures/docs for mixed OpenSSL 3/legacy handling.
src/native/libs/System.Security.Cryptography.Native/pal_ecc_import_export.c Makes EC_KEY parameter helpers private and adds provider-first/legacy-fallback driver functions for parameter export.
src/native/libs/System.Security.Cryptography.Native/entrypoints.c Removes obsolete EC_KEY-related exports and adds CryptoNative_CreateEvpPkeyFromEcKey.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsaOpenSsl.cs Switches IntPtr constructor to use the new native “EC_KEY → EVP_PKEY + keySize” path.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.cs Switches IntPtr constructor to use the new native “EC_KEY → EVP_PKEY + keySize” path.
src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj Removes compilation of the Unix SafeEcKeyHandle file.
src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.ImportExport.cs Removes managed EC_KEY fallback path; always exports via EVP_PKEY helpers.
src/libraries/Common/src/Microsoft/Win32/SafeHandles/SafeEcKeyHandle.Unix.cs Removes Unix SafeEcKeyHandle implementation.
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.EcKey.cs Updates interop to call CryptoNative_CreateEvpPkeyFromEcKey and return key size.
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcKey.cs Removes Unix EC_KEY P/Invokes, leaving only NID→OID helper.
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs Removes EC_KEY-based parameter exports; relies on EVP_PKEY parameter exports.
Comments suppressed due to low confidence (1)

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.EcKey.cs:29

  • CryptoNative_CreateEvpPkeyFromEcKey can return a null SafeEvpPKeyHandle when the native function returns NULL. The current code unconditionally dereferences pkey (pkey.IsInvalid) which can throw NullReferenceException instead of a CryptographicException. Also, this disposes the handle before capturing the OpenSSL error, which can lose/alter error queue state; other interop wrappers in this repo capture the exception first, then dispose.
            SafeEvpPKeyHandle pkey = CryptoNative_CreateEvpPkeyFromEcKey(ecKeyHandle, out keySize);

            if (pkey.IsInvalid)
            {
                pkey.Dispose();
                throw Interop.Crypto.CreateOpenSslCryptographicException();
            }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants