Use BCL Curve25519 for Windows 10+ (#1702)

* Use BCL Curve25519 when possible

* Update KeyExchangeMLKem768X25519Sha256 and KeyExchangeSNtruP761X25519Sha512

* Split Start and Finish methods for inheritance

* Some refactor

* Update src/Renci.SshNet/Security/KeyExchangeEC.BclImpl.cs

Co-authored-by: Rob Hague <rob.hague00@gmail.com>

* revert

* Create dedicated KeyExchangeECCurve25519 BclImpl

* cleanup

* minor code refactor

* integration test

* Revert "integration test"

This reverts commit 326962664c70b148b71f80063a227d9e5430ba3b.

---------

Co-authored-by: Rob Hague <rob.hague00@gmail.com>

src/Renci.SshNet/Security/KeyExchangeEC.cs

@@ -1,4 +1,6 @@
-using Renci.SshNet.Messages.Transport;
+using System;
+
+using Renci.SshNet.Messages.Transport;
 
 namespace Renci.SshNet.Security
 {
@@ -76,5 +78,23 @@ namespace Renci.SshNet.Security
             _serverPayload = message.GetBytes();
             _clientPayload = Session.ClientInitMessage.GetBytes();
         }
+
+        protected abstract class Impl : IDisposable
+        {
+            public abstract byte[] GenerateClientPublicKey();
+
+            public abstract byte[] CalculateAgreement(byte[] serverPublicKey);
+
+            protected virtual void Dispose(bool disposing)
+            {
+            }
+
+            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/KeyExchangeECCurve25519.BclImpl.cs

@@ -0,0 +1,57 @@
+#if NET
+using System.Security.Cryptography;
+
+namespace Renci.SshNet.Security
+{
+    internal partial class KeyExchangeECCurve25519
+    {
+        protected sealed class BclImpl : Impl
+        {
+            private readonly ECCurve _curve;
+            private readonly ECDiffieHellman _clientECDH;
+
+            public BclImpl()
+            {
+                _curve = ECCurve.CreateFromFriendlyName("Curve25519");
+                _clientECDH = ECDiffieHellman.Create();
+            }
+
+            public override byte[] GenerateClientPublicKey()
+            {
+                _clientECDH.GenerateKey(_curve);
+
+                var q = _clientECDH.PublicKey.ExportParameters().Q;
+
+                return q.X;
+            }
+
+            public override byte[] CalculateAgreement(byte[] serverPublicKey)
+            {
+                var parameters = new ECParameters
+                {
+                    Curve = _curve,
+                    Q = new ECPoint
+                    {
+                        X = serverPublicKey,
+                        Y = new byte[serverPublicKey.Length]
+                    },
+                };
+
+                using var serverECDH = ECDiffieHellman.Create(parameters);
+
+                return _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey);
+            }
+
+            protected override void Dispose(bool disposing)
+            {
+                base.Dispose(disposing);
+
+                if (disposing)
+                {
+                    _clientECDH.Dispose();
+                }
+            }
+        }
+    }
+}
+#endif

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

@@ -0,0 +1,38 @@
+using Org.BouncyCastle.Crypto.Agreement;
+using Org.BouncyCastle.Crypto.Generators;
+using Org.BouncyCastle.Crypto.Parameters;
+
+using Renci.SshNet.Abstractions;
+
+namespace Renci.SshNet.Security
+{
+    internal partial class KeyExchangeECCurve25519
+    {
+        protected sealed class BouncyCastleImpl : Impl
+        {
+            private X25519Agreement _keyAgreement;
+
+            public override byte[] GenerateClientPublicKey()
+            {
+                var g = new X25519KeyPairGenerator();
+                g.Init(new X25519KeyGenerationParameters(CryptoAbstraction.SecureRandom));
+
+                var aKeyPair = g.GenerateKeyPair();
+                _keyAgreement = new X25519Agreement();
+                _keyAgreement.Init(aKeyPair.Private);
+
+                return ((X25519PublicKeyParameters)aKeyPair.Public).GetEncoded();
+            }
+
+            public override byte[] CalculateAgreement(byte[] serverPublicKey)
+            {
+                var publicKey = new X25519PublicKeyParameters(serverPublicKey);
+
+                var k1 = new byte[_keyAgreement.AgreementSize];
+                _keyAgreement.CalculateAgreement(publicKey, k1, 0);
+
+                return k1;
+            }
+        }
+    }
+}

src/Renci.SshNet/Security/KeyExchangeECCurve25519.cs

@@ -1,16 +1,18 @@
-using Org.BouncyCastle.Crypto.Agreement;
-using Org.BouncyCastle.Crypto.Generators;
-using Org.BouncyCastle.Crypto.Parameters;
-
-using Renci.SshNet.Abstractions;
+using Renci.SshNet.Abstractions;
 using Renci.SshNet.Common;
 using Renci.SshNet.Messages.Transport;
 
 namespace Renci.SshNet.Security
 {
-    internal sealed class KeyExchangeECCurve25519 : KeyExchangeEC
+    internal partial class KeyExchangeECCurve25519 : KeyExchangeEC
     {
-        private X25519Agreement _keyAgreement;
+#pragma warning disable SA1401 // Fields should be private
+#if NET
+        protected Impl _impl;
+#else
+        protected BouncyCastleImpl _impl;
+#endif
+#pragma warning restore SA1401 // Fields should be private
 
         /// <summary>
         /// Gets algorithm name.
@@ -35,29 +37,46 @@ namespace Renci.SshNet.Security
         public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage)
         {
             base.Start(session, message, sendClientInitMessage);
+#if NET
+            if (System.OperatingSystem.IsWindowsVersionAtLeast(10))
+            {
+                _impl = new BclImpl();
+            }
+            else
+#endif
+            {
+                _impl = new BouncyCastleImpl();
+            }
+
+            StartImpl();
+        }
 
+        /// <summary>
+        /// The implementation of start key exchange algorithm.
+        /// </summary>
+        protected virtual void StartImpl()
+        {
             Session.RegisterMessage("SSH_MSG_KEX_ECDH_REPLY");
 
             Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived;
 
-            var g = new X25519KeyPairGenerator();
-            g.Init(new X25519KeyGenerationParameters(CryptoAbstraction.SecureRandom));
-
-            var aKeyPair = g.GenerateKeyPair();
-            _keyAgreement = new X25519Agreement();
-            _keyAgreement.Init(aKeyPair.Private);
-            _clientExchangeValue = ((X25519PublicKeyParameters)aKeyPair.Public).GetEncoded();
+            _clientExchangeValue = _impl.GenerateClientPublicKey();
 
             SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue));
         }
 
-        /// <summary>
-        /// Finishes key exchange algorithm.
-        /// </summary>
+        /// <inheritdoc/>
         public override void Finish()
         {
             base.Finish();
+            FinishImpl();
+        }
 
+        /// <summary>
+        /// The implementation of finish key exchange algorithm.
+        /// </summary>
+        protected virtual void FinishImpl()
+        {
             Session.KeyExchangeEcdhReplyMessageReceived -= Session_KeyExchangeEcdhReplyMessageReceived;
         }
 
@@ -98,11 +117,19 @@ namespace Renci.SshNet.Security
             _hostKey = hostKey;
             _signature = signature;
 
-            var publicKey = new X25519PublicKeyParameters(serverExchangeValue);
-
-            var k1 = new byte[_keyAgreement.AgreementSize];
-            _keyAgreement.CalculateAgreement(publicKey, k1, 0);
+            var k1 = _impl.CalculateAgreement(serverExchangeValue);
             SharedKey = k1.ToBigInteger2().ToByteArray(isBigEndian: true);
         }
+
+        /// <inheritdoc/>
+        protected override void Dispose(bool disposing)
+        {
+            base.Dispose(disposing);
+
+            if (disposing)
+            {
+                _impl?.Dispose();
+            }
+        }
     }
 }

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

@@ -17,7 +17,7 @@ namespace Renci.SshNet.Security
                 _clientECDH = ECDiffieHellman.Create();
             }
 
-            public override byte[] GenerateClientECPoint()
+            public override byte[] GenerateClientPublicKey()
             {
                 _clientECDH.GenerateKey(_curve);
 
@@ -26,9 +26,9 @@ namespace Renci.SshNet.Security
                 return EncodeECPoint(q);
             }
 
-            public override byte[] CalculateAgreement(byte[] serverECPoint)
+            public override byte[] CalculateAgreement(byte[] serverPublicKey)
             {
-                var q = DecodeECPoint(serverECPoint);
+                var q = DecodeECPoint(serverPublicKey);
 
                 var parameters = new ECParameters
                 {

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

@@ -20,7 +20,7 @@ namespace Renci.SshNet.Security
                 _keyAgreement = new ECDHCBasicAgreement();
             }
 
-            public override byte[] GenerateClientECPoint()
+            public override byte[] GenerateClientPublicKey()
             {
                 var g = new ECKeyPairGenerator();
                 g.Init(new ECKeyGenerationParameters(_domainParameters, CryptoAbstraction.SecureRandom));
@@ -31,10 +31,10 @@ namespace Renci.SshNet.Security
                 return ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded();
             }
 
-            public override byte[] CalculateAgreement(byte[] serverECPoint)
+            public override byte[] CalculateAgreement(byte[] serverPublicKey)
             {
                 var c = _domainParameters.Curve;
-                var q = c.DecodePoint(serverECPoint);
+                var q = c.DecodePoint(serverPublicKey);
                 var publicKey = new ECPublicKeyParameters("ECDH", q, _domainParameters);
 
                 return _keyAgreement.CalculateAgreement(publicKey).ToByteArray();

src/Renci.SshNet/Security/KeyExchangeECDH.cs

@@ -1,6 +1,4 @@
-using System;
-
-using Org.BouncyCastle.Asn1.X9;
+using Org.BouncyCastle.Asn1.X9;
 
 using Renci.SshNet.Common;
 using Renci.SshNet.Messages.Transport;
@@ -41,7 +39,7 @@ namespace Renci.SshNet.Security
             Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived;
 
 #if NET
-            if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10))
+            if (!System.OperatingSystem.IsWindows() || System.OperatingSystem.IsWindowsVersionAtLeast(10))
             {
                 _impl = new BclImpl(Curve);
             }
@@ -51,7 +49,7 @@ namespace Renci.SshNet.Security
                 _impl = new BouncyCastleImpl(CurveParameter);
             }
 
-            _clientExchangeValue = _impl.GenerateClientECPoint();
+            _clientExchangeValue = _impl.GenerateClientPublicKey();
 
             SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue));
         }
@@ -106,23 +104,5 @@ namespace Renci.SshNet.Security
                 _impl?.Dispose();
             }
         }
-
-        private abstract class Impl : IDisposable
-        {
-            public abstract byte[] GenerateClientECPoint();
-
-            public abstract byte[] CalculateAgreement(byte[] serverECPoint);
-
-            protected virtual void Dispose(bool disposing)
-            {
-            }
-
-            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/KeyExchangeMLKem768X25519Sha256.cs

@@ -1,7 +1,6 @@
 using System.Globalization;
 using System.Linq;
 
-using Org.BouncyCastle.Crypto.Agreement;
 using Org.BouncyCastle.Crypto.Generators;
 using Org.BouncyCastle.Crypto.Kems;
 using Org.BouncyCastle.Crypto.Parameters;
@@ -12,10 +11,9 @@ using Renci.SshNet.Messages.Transport;
 
 namespace Renci.SshNet.Security
 {
-    internal sealed class KeyExchangeMLKem768X25519Sha256 : KeyExchangeEC
+    internal sealed class KeyExchangeMLKem768X25519Sha256 : KeyExchangeECCurve25519
     {
         private MLKemDecapsulator _mlkemDecapsulator;
-        private X25519Agreement _x25519Agreement;
 
         /// <summary>
         /// Gets algorithm name.
@@ -37,10 +35,8 @@ namespace Renci.SshNet.Security
         }
 
         /// <inheritdoc/>
-        public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage)
+        protected override void StartImpl()
         {
-            base.Start(session, message, sendClientInitMessage);
-
             Session.RegisterMessage("SSH_MSG_KEX_HYBRID_REPLY");
 
             Session.KeyExchangeHybridReplyMessageReceived += Session_KeyExchangeHybridReplyMessageReceived;
@@ -52,28 +48,18 @@ namespace Renci.SshNet.Security
             _mlkemDecapsulator = new MLKemDecapsulator(MLKemParameters.ml_kem_768);
             _mlkemDecapsulator.Init(mlkem768KeyPair.Private);
 
-            var x25519KeyPairGenerator = new X25519KeyPairGenerator();
-            x25519KeyPairGenerator.Init(new X25519KeyGenerationParameters(CryptoAbstraction.SecureRandom));
-            var x25519KeyPair = x25519KeyPairGenerator.GenerateKeyPair();
-
-            _x25519Agreement = new X25519Agreement();
-            _x25519Agreement.Init(x25519KeyPair.Private);
-
             var mlkem768PublicKey = ((MLKemPublicKeyParameters)mlkem768KeyPair.Public).GetEncoded();
-            var x25519PublicKey = ((X25519PublicKeyParameters)x25519KeyPair.Public).GetEncoded();
+
+            var x25519PublicKey = _impl.GenerateClientPublicKey();
 
             _clientExchangeValue = mlkem768PublicKey.Concat(x25519PublicKey);
 
             SendMessage(new KeyExchangeHybridInitMessage(_clientExchangeValue));
         }
 
-        /// <summary>
-        /// Finishes key exchange algorithm.
-        /// </summary>
-        public override void Finish()
+        /// <inheritdoc/>
+        protected override void FinishImpl()
         {
-            base.Finish();
-
             Session.KeyExchangeHybridReplyMessageReceived -= Session_KeyExchangeHybridReplyMessageReceived;
         }
 
@@ -114,21 +100,20 @@ namespace Renci.SshNet.Security
             _hostKey = hostKey;
             _signature = signature;
 
-            if (serverExchangeValue.Length != _mlkemDecapsulator.EncapsulationLength + _x25519Agreement.AgreementSize)
+            if (serverExchangeValue.Length != _mlkemDecapsulator.EncapsulationLength + X25519PublicKeyParameters.KeySize)
             {
                 throw new SshConnectionException(
                     string.Format(CultureInfo.CurrentCulture, "Bad S_Reply length: {0}.", serverExchangeValue.Length),
                     DisconnectReason.KeyExchangeFailed);
             }
 
-            var secret = new byte[_mlkemDecapsulator.SecretLength + _x25519Agreement.AgreementSize];
+            var mlkemSecret = new byte[_mlkemDecapsulator.SecretLength];
 
-            _mlkemDecapsulator.Decapsulate(serverExchangeValue, 0, _mlkemDecapsulator.EncapsulationLength, secret, 0, _mlkemDecapsulator.SecretLength);
+            _mlkemDecapsulator.Decapsulate(serverExchangeValue, 0, _mlkemDecapsulator.EncapsulationLength, mlkemSecret, 0, _mlkemDecapsulator.SecretLength);
 
-            var x25519PublicKey = new X25519PublicKeyParameters(serverExchangeValue, _mlkemDecapsulator.EncapsulationLength);
-            _x25519Agreement.CalculateAgreement(x25519PublicKey, secret, _mlkemDecapsulator.SecretLength);
+            var x25519Agreement = _impl.CalculateAgreement(serverExchangeValue.Take(_mlkemDecapsulator.EncapsulationLength, X25519PublicKeyParameters.KeySize));
 
-            SharedKey = CryptoAbstraction.HashSHA256(secret);
+            SharedKey = CryptoAbstraction.HashSHA256(mlkemSecret.Concat(x25519Agreement));
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeSNtruP761X25519Sha512.cs

@@ -1,9 +1,6 @@
-using System;
-using System.Globalization;
+using System.Globalization;
 using System.Linq;
 
-using Org.BouncyCastle.Crypto.Agreement;
-using Org.BouncyCastle.Crypto.Generators;
 using Org.BouncyCastle.Crypto.Parameters;
 using Org.BouncyCastle.Pqc.Crypto.NtruPrime;
 
@@ -13,10 +10,9 @@ using Renci.SshNet.Messages.Transport;
 
 namespace Renci.SshNet.Security
 {
-    internal sealed class KeyExchangeSNtruP761X25519Sha512 : KeyExchangeEC
+    internal sealed class KeyExchangeSNtruP761X25519Sha512 : KeyExchangeECCurve25519
     {
         private SNtruPrimeKemExtractor _sntrup761Extractor;
-        private X25519Agreement _x25519Agreement;
 
         /// <summary>
         /// Gets algorithm name.
@@ -38,10 +34,8 @@ namespace Renci.SshNet.Security
         }
 
         /// <inheritdoc/>
-        public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage)
+        protected override void StartImpl()
         {
-            base.Start(session, message, sendClientInitMessage);
-
             Session.RegisterMessage("SSH_MSG_KEX_ECDH_REPLY");
 
             Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived;
@@ -52,28 +46,18 @@ namespace Renci.SshNet.Security
 
             _sntrup761Extractor = new SNtruPrimeKemExtractor((SNtruPrimePrivateKeyParameters)sntrup761KeyPair.Private);
 
-            var x25519KeyPairGenerator = new X25519KeyPairGenerator();
-            x25519KeyPairGenerator.Init(new X25519KeyGenerationParameters(CryptoAbstraction.SecureRandom));
-            var x25519KeyPair = x25519KeyPairGenerator.GenerateKeyPair();
-
-            _x25519Agreement = new X25519Agreement();
-            _x25519Agreement.Init(x25519KeyPair.Private);
-
             var sntrup761PublicKey = ((SNtruPrimePublicKeyParameters)sntrup761KeyPair.Public).GetEncoded();
-            var x25519PublicKey = ((X25519PublicKeyParameters)x25519KeyPair.Public).GetEncoded();
+
+            var x25519PublicKey = _impl.GenerateClientPublicKey();
 
             _clientExchangeValue = sntrup761PublicKey.Concat(x25519PublicKey);
 
             SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue));
         }
 
-        /// <summary>
-        /// Finishes key exchange algorithm.
-        /// </summary>
-        public override void Finish()
+        /// <inheritdoc/>
+        protected override void FinishImpl()
         {
-            base.Finish();
-
             Session.KeyExchangeEcdhReplyMessageReceived -= Session_KeyExchangeEcdhReplyMessageReceived;
         }
 
@@ -122,14 +106,12 @@ namespace Renci.SshNet.Security
             }
 
             var sntrup761CipherText = serverExchangeValue.Take(_sntrup761Extractor.EncapsulationLength);
-            var secret = _sntrup761Extractor.ExtractSecret(sntrup761CipherText);
-            var sntrup761SecretLength = secret.Length;
 
-            var x25519PublicKey = new X25519PublicKeyParameters(serverExchangeValue, _sntrup761Extractor.EncapsulationLength);
-            Array.Resize(ref secret, sntrup761SecretLength + _x25519Agreement.AgreementSize);
-            _x25519Agreement.CalculateAgreement(x25519PublicKey, secret, sntrup761SecretLength);
+            var sntrup761Secret = _sntrup761Extractor.ExtractSecret(sntrup761CipherText);
+
+            var x25519Agreement = _impl.CalculateAgreement(serverExchangeValue.Take(_sntrup761Extractor.EncapsulationLength, X25519PublicKeyParameters.KeySize));
 
-            SharedKey = CryptoAbstraction.HashSHA512(secret);
+            SharedKey = CryptoAbstraction.HashSHA512(sntrup761Secret.Concat(x25519Agreement));
         }
     }
 }