Some X25519 fixes for OpenSSL key handling#129338
Merged
Merged
Conversation
The "Does this EVP_PKEY contain an exportable private key" check for X25519 was slightly wrong for OpenSSL 1.1.1.
…enSsl during the secret agreement. _hasPrivate tells us if the key contains an exportable private key. For the implementation methods, this is generally analogous to has "any" private key since they will go in to the default provider. However with X25519DiffieHellmanOpenSsl, we don't know if the private key is exportable because it might be in a provider that does not permit exporting the private key at all. All we can do in that case is let the provider tell us if the secret agreement worked or not.
Contributor
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens X25519 key handling on the OpenSSL-backed implementation by making the “has private key” detection robust across OpenSSL versions (notably 1.1.1) and by letting OpenSSL be the source of truth for whether secret agreement is possible when the key originates from an external handle.
Changes:
- Fix OpenSSL X25519 “has private” detection by always requesting the raw private key into a correctly-sized buffer (and cleansing it afterward), avoiding OpenSSL 1.1.1’s misleading NULL-buffer success behavior.
- Update
X25519DiffieHellmanOpenSslto stop pre-guarding secret derivation on “exportable private” and only enforce that check for private-key export operations. - Relax public-key-only derive tests to accept any
CryptographicExceptionsubtype (to accommodate OpenSSL-specific exception types).
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_x25519.c | Makes private-key presence detection robust on OpenSSL 1.1.1 by probing with a real buffer and cleansing it. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X25519DiffieHellmanOpenSsl.OpenSsl.cs | Removes the derive-time exportability guard; keeps the check for export APIs via _hasExportablePrivate. |
| src/libraries/System.Security.Cryptography/tests/X25519DiffieHellmanBaseTests.cs | Updates expectations for public-key-only derive to allow OpenSSL-specific CryptographicException subclasses. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 0
bartonjs
approved these changes
Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes a couple of fixes to the X25519 implementation.
Our "Does this X25519 instance have an exportable key" check for OpenSSL did not work correct on OpenSSL 1.1.1. This is explained in the comments. This doesn't functionally fix anything, developers just get a better error.
With X25519DiffieHellmanOpenSsl, we were guarding DeriveRawSecretAgreementCore with _hasPrivate. This is not right for the X25519DiffieHellmanOpenSsl implementation because we got the key handle from somewhere else, so we don't know if the private key is exportable or not. Instead we just have to let the provider tell us if the key agreement worked or not. We can still use this check for the PKCS#8 and private key exports.