Use BCL ECDiffieHellman for KeyExchange instead of BouncyCastle (.NET 8.0 onward only) (#1371)

* Use BCL ECDiffieHellman for KeyExchange (.NET 8.0 onward only)

* Add back an empty line

* Remove the BouncyCastle dependency when target .NET 8.0 onward.

* Run KeyExchangeAlgorithmTests for .NET 6.0

* Build Renci.SshNet.IntegrationTests.csproj for net6.0

* Update filter

* Add back BouncyCastle as fallback

* Add back the missing `SendMessage`

* Run ECDH KEX integration tests under .NET48

* Use SshNamedCurves instead of SecNamedCurves for BouncyCastle.
BCL supports both names. See https://github.com/dotnet/runtime/blob/main/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/OidLookup.cs#L200-L202

* typo

* Fix build

* Use System.Security.Cryptography namespace if NET8_0_OR_GREATER;
Use one parameter constructor for class ECDomainParameters

* Separate BCL and BouncyCastle implementation

---------

Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com>

appveyor.yml

@@ -28,7 +28,7 @@ for:
     - sh: dotnet test -f net8.0 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_unit_test_net_8_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_unit_test_net_8_coverage.xml test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj
     - sh: echo "Run integration tests"
     - sh: dotnet test -f net8.0 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_8_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_8_coverage.xml test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj
-    - sh: dotnet test -f net48 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_48_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_48_coverage.xml --filter "Name=ChaCha20Poly1305|Name~Zlib" test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj
+    - sh: dotnet test -f net48 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_48_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_48_coverage.xml --filter "Name=ChaCha20Poly1305|Name~Ecdh|Name~Zlib" test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj
 
 -
   matrix:

src/Renci.SshNet/Security/KeyExchangeECDH.BclImpl.cs

@@ -0,0 +1,77 @@
+#if NET8_0_OR_GREATER
+using System;
+using System.Security.Cryptography;
+
+namespace Renci.SshNet.Security
+{
+    internal abstract partial class KeyExchangeECDH
+    {
+        private sealed class BclImpl : Impl
+        {
+            private readonly ECCurve _curve;
+            private readonly ECDiffieHellman _clientECDH;
+
+            public BclImpl(ECCurve curve)
+            {
+                _curve = curve;
+                _clientECDH = ECDiffieHellman.Create();
+            }
+
+            public override byte[] GenerateClientECPoint()
+            {
+                _clientECDH.GenerateKey(_curve);
+
+                var q = _clientECDH.PublicKey.ExportParameters().Q;
+
+                return EncodeECPoint(q);
+            }
+
+            public override byte[] CalculateAgreement(byte[] serverECPoint)
+            {
+                var q = DecodeECPoint(serverECPoint);
+
+                var parameters = new ECParameters
+                {
+                    Curve = _curve,
+                    Q = q,
+                };
+
+                using var serverECDH = ECDiffieHellman.Create(parameters);
+
+                return _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey);
+            }
+
+            private static byte[] EncodeECPoint(ECPoint point)
+            {
+                var q = new byte[1 + point.X.Length + point.Y.Length];
+                q[0] = 0x04;
+                Buffer.BlockCopy(point.X, 0, q, 1, point.X.Length);
+                Buffer.BlockCopy(point.Y, 0, q, point.X.Length + 1, point.Y.Length);
+
+                return q;
+            }
+
+            private static ECPoint DecodeECPoint(byte[] q)
+            {
+                var cordSize = (q.Length - 1) / 2;
+                var x = new byte[cordSize];
+                var y = new byte[cordSize];
+                Buffer.BlockCopy(q, 1, x, 0, x.Length);
+                Buffer.BlockCopy(q, cordSize + 1, y, 0, y.Length);
+
+                return new ECPoint { X = x, Y = y };
+            }
+
+            protected override void Dispose(bool disposing)
+            {
+                base.Dispose(disposing);
+
+                if (disposing)
+                {
+                    _clientECDH.Dispose();
+                }
+            }
+        }
+    }
+}
+#endif

src/Renci.SshNet/Security/KeyExchangeECDH.BouncyCastleImpl.cs

@@ -0,0 +1,44 @@
+using Org.BouncyCastle.Asn1.X9;
+using Org.BouncyCastle.Crypto.Agreement;
+using Org.BouncyCastle.Crypto.Generators;
+using Org.BouncyCastle.Crypto.Parameters;
+
+using Renci.SshNet.Abstractions;
+
+namespace Renci.SshNet.Security
+{
+    internal abstract partial class KeyExchangeECDH
+    {
+        private sealed class BouncyCastleImpl : Impl
+        {
+            private readonly ECDomainParameters _domainParameters;
+            private readonly ECDHCBasicAgreement _keyAgreement;
+
+            public BouncyCastleImpl(X9ECParameters curveParameters)
+            {
+                _domainParameters = new ECDomainParameters(curveParameters);
+                _keyAgreement = new ECDHCBasicAgreement();
+            }
+
+            public override byte[] GenerateClientECPoint()
+            {
+                var g = new ECKeyPairGenerator();
+                g.Init(new ECKeyGenerationParameters(_domainParameters, CryptoAbstraction.SecureRandom));
+
+                var aKeyPair = g.GenerateKeyPair();
+                _keyAgreement.Init(aKeyPair.Private);
+
+                return ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded();
+            }
+
+            public override byte[] CalculateAgreement(byte[] serverECPoint)
+            {
+                var c = _domainParameters.Curve;
+                var q = c.DecodePoint(serverECPoint);
+                var publicKey = new ECPublicKeyParameters("ECDH", q, _domainParameters);
+
+                return _keyAgreement.CalculateAgreement(publicKey).ToByteArray();
+            }
+        }
+    }
+}

src/Renci.SshNet/Security/KeyExchangeECDH.cs

@@ -1,19 +1,28 @@
 using System;
 
 using Org.BouncyCastle.Asn1.X9;
-using Org.BouncyCastle.Crypto.Agreement;
-using Org.BouncyCastle.Crypto.Generators;
-using Org.BouncyCastle.Crypto.Parameters;
-using Org.BouncyCastle.Math.EC;
 
-using Renci.SshNet.Abstractions;
 using Renci.SshNet.Common;
 using Renci.SshNet.Messages.Transport;
 
 namespace Renci.SshNet.Security
 {
-    internal abstract class KeyExchangeECDH : KeyExchangeEC
+    internal abstract partial class KeyExchangeECDH : KeyExchangeEC
     {
+#if NET8_0_OR_GREATER
+        private Impl _impl;
+
+        /// <summary>
+        /// Gets the curve.
+        /// </summary>
+        /// <value>
+        /// The curve.
+        /// </value>
+        protected abstract System.Security.Cryptography.ECCurve Curve { get; }
+#else
+        private BouncyCastleImpl _impl;
+#endif
+
         /// <summary>
         /// Gets the parameter of the curve.
         /// </summary>
@@ -22,9 +31,6 @@ namespace Renci.SshNet.Security
         /// </value>
         protected abstract X9ECParameters CurveParameter { get; }
 
-        private ECDHCBasicAgreement _keyAgreement;
-        private ECDomainParameters _domainParameters;
-
         /// <inheritdoc/>
         public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage)
         {
@@ -34,19 +40,20 @@ namespace Renci.SshNet.Security
 
             Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived;
 
-            _domainParameters = new ECDomainParameters(CurveParameter.Curve,
-                                                      CurveParameter.G,
-                                                      CurveParameter.N,
-                                                      CurveParameter.H,
-                                                      CurveParameter.GetSeed());
-
-            var g = new ECKeyPairGenerator();
-            g.Init(new ECKeyGenerationParameters(_domainParameters, CryptoAbstraction.SecureRandom));
-
-            var aKeyPair = g.GenerateKeyPair();
-            _keyAgreement = new ECDHCBasicAgreement();
-            _keyAgreement.Init(aKeyPair.Private);
-            _clientExchangeValue = ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded();
+#if NET8_0_OR_GREATER
+            if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10))
+            {
+                _impl = new BclImpl(Curve);
+            }
+            else
+            {
+                _impl = new BouncyCastleImpl(CurveParameter);
+            }
+#else
+            _impl = new BouncyCastleImpl(CurveParameter);
+#endif
+
+            _clientExchangeValue = _impl.GenerateClientECPoint();
 
             SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue));
         }
@@ -86,18 +93,38 @@ namespace Renci.SshNet.Security
             _hostKey = hostKey;
             _signature = signature;
 
-            var cordSize = (serverExchangeValue.Length - 1) / 2;
-            var x = new byte[cordSize];
-            Buffer.BlockCopy(serverExchangeValue, 1, x, 0, x.Length); // first byte is format. should be checked and passed to bouncy castle?
-            var y = new byte[cordSize];
-            Buffer.BlockCopy(serverExchangeValue, cordSize + 1, y, 0, y.Length);
+            var agreement = _impl.CalculateAgreement(serverExchangeValue);
+
+            SharedKey = agreement.ToBigInteger2().ToByteArray().Reverse();
+        }
+
+        /// <inheritdoc/>
+        protected override void Dispose(bool disposing)
+        {
+            base.Dispose(disposing);
+
+            if (disposing)
+            {
+                _impl?.Dispose();
+            }
+        }
+
+        private abstract class Impl : IDisposable
+        {
+            public abstract byte[] GenerateClientECPoint();
+
+            public abstract byte[] CalculateAgreement(byte[] serverECPoint);
 
-            var c = (FpCurve)_domainParameters.Curve;
-            var q = c.CreatePoint(new Org.BouncyCastle.Math.BigInteger(1, x), new Org.BouncyCastle.Math.BigInteger(1, y));
-            var publicKey = new ECPublicKeyParameters("ECDH", q, _domainParameters);
+            protected virtual void Dispose(bool disposing)
+            {
+            }
 
-            var k1 = _keyAgreement.CalculateAgreement(publicKey);
-            SharedKey = k1.ToByteArray().ToBigInteger2().ToByteArray().Reverse();
+            public void Dispose()
+            {
+                // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
+                Dispose(disposing: true);
+                GC.SuppressFinalize(this);
+            }
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeECDH256.cs

@@ -1,4 +1,4 @@
-using Org.BouncyCastle.Asn1.Sec;
+using Org.BouncyCastle.Asn1.Sec;
 using Org.BouncyCastle.Asn1.X9;
 
 using Renci.SshNet.Abstractions;
@@ -15,6 +15,19 @@ namespace Renci.SshNet.Security
             get { return "ecdh-sha2-nistp256"; }
         }
 
+#if NET8_0_OR_GREATER
+        /// <summary>
+        /// Gets the curve.
+        /// </summary>
+        protected override System.Security.Cryptography.ECCurve Curve
+        {
+            get
+            {
+                return System.Security.Cryptography.ECCurve.NamedCurves.nistP256;
+            }
+        }
+#endif
+
         /// <summary>
         /// Gets Curve Parameter.
         /// </summary>
@@ -22,7 +35,7 @@ namespace Renci.SshNet.Security
         {
             get
             {
-                return SecNamedCurves.GetByName("secp256r1");
+                return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP256r1);
             }
         }
 

src/Renci.SshNet/Security/KeyExchangeECDH384.cs

@@ -1,4 +1,4 @@
-using Org.BouncyCastle.Asn1.Sec;
+using Org.BouncyCastle.Asn1.Sec;
 using Org.BouncyCastle.Asn1.X9;
 
 using Renci.SshNet.Abstractions;
@@ -15,6 +15,19 @@ namespace Renci.SshNet.Security
             get { return "ecdh-sha2-nistp384"; }
         }
 
+#if NET8_0_OR_GREATER
+        /// <summary>
+        /// Gets the curve.
+        /// </summary>
+        protected override System.Security.Cryptography.ECCurve Curve
+        {
+            get
+            {
+                return System.Security.Cryptography.ECCurve.NamedCurves.nistP384;
+            }
+        }
+#endif
+
         /// <summary>
         /// Gets Curve Parameter.
         /// </summary>
@@ -22,7 +35,7 @@ namespace Renci.SshNet.Security
         {
             get
             {
-                return SecNamedCurves.GetByName("secp384r1");
+                return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP384r1);
             }
         }
 

src/Renci.SshNet/Security/KeyExchangeECDH521.cs

@@ -1,4 +1,4 @@
-using Org.BouncyCastle.Asn1.Sec;
+using Org.BouncyCastle.Asn1.Sec;
 using Org.BouncyCastle.Asn1.X9;
 
 using Renci.SshNet.Abstractions;
@@ -15,6 +15,19 @@ namespace Renci.SshNet.Security
             get { return "ecdh-sha2-nistp521"; }
         }
 
+#if NET8_0_OR_GREATER
+        /// <summary>
+        /// Gets the curve.
+        /// </summary>
+        protected override System.Security.Cryptography.ECCurve Curve
+        {
+            get
+            {
+                return System.Security.Cryptography.ECCurve.NamedCurves.nistP521;
+            }
+        }
+#endif
+
         /// <summary>
         /// Gets Curve Parameter.
         /// </summary>
@@ -22,7 +35,7 @@ namespace Renci.SshNet.Security
         {
             get
             {
-                return SecNamedCurves.GetByName("secp521r1");
+                return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP521r1);
             }
         }