Use BCL Curve25519 for Windows 10+#1702
Merged
Merged
Conversation
f5486b5 to
9d15116
Compare
Rob-Hague
reviewed
Sep 26, 2025
Collaborator
|
actually, it is not working on my win10 22H2 machine (connected aborted). develop branch works fine, using curve25519-sha256 |
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
Collaborator
Author
Based on your description, it does not throw exception when get the named curve. May related to agreement derivation. |
Collaborator
|
Right, see wireshark.zip. I see the bad capture (on this branch) is sending ECDH init with a key length of 65 bytes (containing 32 trailing zeroes), and the server closes the connection. The good capture (on develop) is sending with 32 byte key, and everything works fine |
Collaborator
|
this works diff --git a/src/Renci.SshNet/Security/KeyExchangeEC.BclImpl.cs b/src/Renci.SshNet/Security/KeyExchangeEC.BclImpl.cs
index 54757719..571614f6 100644
--- a/src/Renci.SshNet/Security/KeyExchangeEC.BclImpl.cs
+++ b/src/Renci.SshNet/Security/KeyExchangeEC.BclImpl.cs
@@ -23,17 +23,15 @@ public override byte[] GenerateClientECPoint()
var q = _clientECDH.PublicKey.ExportParameters().Q;
- return EncodeECPoint(q);
+ return q.X;
}
public override byte[] CalculateAgreement(byte[] serverECPoint)
{
- var q = DecodeECPoint(serverECPoint);
-
var parameters = new ECParameters
{
Curve = _curve,
- Q = q,
+ Q = new ECPoint { X = serverECPoint, Y = new byte[serverECPoint.Length] },
};
using var serverECDH = ECDiffieHellman.Create(parameters); |
Collaborator
Author
|
That's really strange. Not sure why integration test passes. |
338e205 to
295b2b5
Compare
Rob-Hague
approved these changes
Sep 29, 2025
This reverts commit 3269626.
Rob-Hague
added a commit
that referenced
this pull request
Oct 4, 2025
* CI: add Windows Integration Tests for .NET see #1702 (comment) * fix podman setup with Windows and .NET * debug * x * x * x * revert * Run Windows .NET tests in separate job so they run in parallel and we avoid the Common_CreateMoreChannelsThanMaxSessions test failure. * fix coverlet artifacts * fix missing PermitTTY in RemoteSshdConfig Reset this fixes a test failure in Common_CreateMoreChannelsThanMaxSessions when running the tests multiple times against the same SSH server instance. see #1704 (comment) * speed up Windows tests turns out this is caused by DNS resolution taking about 2 seconds on every new connection... * add windows integration tests to Publish needs: --------- Co-authored-by: Rob Hague <rob.hague00@gmail.com> Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
This was referenced Oct 27, 2025
This was referenced Nov 3, 2025
This was referenced Mar 9, 2026
This was referenced Apr 10, 2026
This was referenced Jun 7, 2026
This was referenced Jun 14, 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.
Beginning in Windows 10, CNG provides support for Curve25519.
See https://learn.microsoft.com/en-us/windows/win32/seccng/cng-named-elliptic-curves