Use BCL Curve25519 for Windows 10+

[scott-xu]

Beginning in Windows 10, CNG provides support for Curve25519.
See https://learn.microsoft.com/en-us/windows/win32/seccng/cng-named-elliptic-curves

[Rob-Hague]

lgtm otherwise

[Rob-Hague]

actually, it is not working on my win10 22H2 machine (connected aborted). develop branch works fine, using curve25519-sha256

[scott-xu]

actually, it is not working on my win10 22H2 machine (connected aborted). develop branch works fine, using curve25519-sha256

Based on your description, it does not throw exception when get the named curve. May related to agreement derivation.

[Rob-Hague]

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

[Rob-Hague]

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);

[scott-xu]

That's really strange. Not sure why integration test passes.

[mus65]

That's really strange. Not sure why integration test passes.

CI only tests Windows with .NET Framework, not Windows with .NET . I guess we should add that?

[Rob-Hague]

Agreed. Unfortunately though, the tests still pass locally (on the earlier commit), so it seems like OpenSSH is more forgiving than this particular server.

On a similar note, we will need to consider that #1619 is set to leave a netstandard-only path without any coverage. And in #1659 it will be difficult to tell whether the BouncyCastle fallback is in use, based on the BCL's somewhat opaque MLKem.IsSupported. BouncyCastle addresses the former problem like this: https://github.com/bcgit/bc-csharp/blob/0c87b54b4b78e95eb80db716e1ac57f2e7875d21/crypto/test/BouncyCastle.Crypto.Tests.csproj#L42-L44. Potentially, the second problem could be addressed by running a test build using only the BouncyCastle fallbacks (putting the BCL paths under a compilation flag)

[mus65]

Agreed.

Opened #1704

On a similar note, we will need to consider that #1619 is set to leave a netstandard-only path without any coverage.

I wish MS would just deprecate .NET Standard. I'm personally not sure it's even worth adding coverage for Xamarin/Mono/UWP/Unity, most if which are EOL...

[Rob-Hague]

I am less concerned about those platforms and more concerned about being in a chain of dependencies where even a netcore app could be referencing the netstandard build. One of the highest listed "Used by" libraries on the SSH.NET page is netstandard only: https://www.nuget.org/packages/MongoDBMigrations/2.2.0#dependencies-body-tab

[mus65]

NuGet would still pick the most appropriate .NET assembly in this case, I just double-checked this locally. If you have a net48 project targeting MongoDBMigrations, the published app will contain the net40 assembly of SSH.NET, not the netstandard2.0 assembly.

If you target MongoDBMigrations from a net9.0 app, it would use netstandard2.0, but only because the SSH.NET version referenced by MongoDBMigrations doesn't even have targets for modern .NET . If you reference a newer version of SSH.NET in the app itself, it will also use the net9.0 assembly.

[scott-xu]

Agreed. Unfortunately though, the tests still pass locally (on the earlier commit), so it seems like OpenSSH is more forgiving than this particular server.

It fails if incorrectly implemented. See https://github.com/scott-xu/SSH.NET/actions/runs/18098643073/job/51495598766

[Rob-Hague]

It fails if incorrectly implemented.

Indeed, thanks. I think I must have run the net48 tests...

NuGet would still pick the most appropriate .NET assembly in this case

Indeed, thanks. I never realised that