From 901fd5a840f24d8cbedc244cb0e824300738dc1d Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sat, 6 Apr 2024 20:27:49 +0800 Subject: [PATCH 01/14] Use BCL ECDiffieHellman for KeyExchange (.NET 8.0 onward only) --- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 79 ++++++++++++++++--- .../Security/KeyExchangeECDH256.cs | 11 +-- .../Security/KeyExchangeECDH384.cs | 11 +-- .../Security/KeyExchangeECDH521.cs | 11 +-- 4 files changed, 78 insertions(+), 34 deletions(-) diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index c756fb6cb..a2b6442da 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -2,7 +2,7 @@ using Renci.SshNet.Common; using Renci.SshNet.Messages.Transport; -using Renci.SshNet.Security.Org.BouncyCastle.Asn1.X9; +using Renci.SshNet.Security.Org.BouncyCastle.Asn1.Sec; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Agreement; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Generators; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Parameters; @@ -13,13 +13,17 @@ namespace Renci.SshNet.Security { internal abstract class KeyExchangeECDH : KeyExchangeEC { +#if NET8_0_OR_GREATER + private System.Security.Cryptography.ECDiffieHellman _clientECDH; +#endif + /// - /// Gets the parameter of the curve. + /// Gets the name of the curve. /// /// - /// The parameter of the curve. + /// The name of the curve. /// - protected abstract X9ECParameters CurveParameter { get; } + protected abstract string CurveName { get; } private ECDHCBasicAgreement _keyAgreement; private ECDomainParameters _domainParameters; @@ -33,11 +37,30 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived; - _domainParameters = new ECDomainParameters(CurveParameter.Curve, - CurveParameter.G, - CurveParameter.N, - CurveParameter.H, - CurveParameter.GetSeed()); +#if NET8_0_OR_GREATER + if (IsNonWindowsOrWindowsVersionAtLeast(10)) + { + _clientECDH = System.Security.Cryptography.ECDiffieHellman.Create(); + _clientECDH.GenerateKey(System.Security.Cryptography.ECCurve.CreateFromFriendlyName(CurveName)); + + var q = _clientECDH.PublicKey.ExportParameters().Q; + + _clientExchangeValue = new byte[1 + q.X.Length + q.Y.Length]; + _clientExchangeValue[0] = 0x04; + Buffer.BlockCopy(q.X, 0, _clientExchangeValue, 1, q.X.Length); + Buffer.BlockCopy(q.Y, 0, _clientExchangeValue, q.X.Length + 1, q.Y.Length); + + SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); + + return; + } +#endif + var curveParameter = SecNamedCurves.GetByName(CurveName); + _domainParameters = new ECDomainParameters(curveParameter.Curve, + curveParameter.G, + curveParameter.N, + curveParameter.H, + curveParameter.GetSeed()); var g = new ECKeyPairGenerator(); g.Init(new ECKeyGenerationParameters(_domainParameters, new SecureRandom())); @@ -46,7 +69,6 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool _keyAgreement = new ECDHCBasicAgreement(); _keyAgreement.Init(aKeyPair.Private); _clientExchangeValue = ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded(); - SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); } @@ -91,6 +113,25 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b var y = new byte[cordSize]; Buffer.BlockCopy(serverExchangeValue, cordSize + 1, y, 0, y.Length); +#if NET8_0_OR_GREATER + if (IsNonWindowsOrWindowsVersionAtLeast(10)) + { + using var serverECDH = System.Security.Cryptography.ECDiffieHellman.Create(new System.Security.Cryptography.ECParameters + { + Curve = System.Security.Cryptography.ECCurve.CreateFromFriendlyName(CurveName), + Q = + { + X = x, + Y = y, + }, + }); + + var k = _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey); + SharedKey = k.ToBigInteger2().ToByteArray().Reverse(); + + return; + } +#endif 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); @@ -98,5 +139,23 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b var k1 = _keyAgreement.CalculateAgreement(publicKey); SharedKey = k1.ToByteArray().ToBigInteger2().ToByteArray().Reverse(); } + +#if NET8_0_OR_GREATER + + /// + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + if (disposing) + { + _clientECDH?.Dispose(); + } + } + + private static bool IsNonWindowsOrWindowsVersionAtLeast(int major) + { + return Environment.OSVersion.Platform != PlatformID.Win32NT || Environment.OSVersion.Version.Major >= major; + } +#endif } } diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH256.cs b/src/Renci.SshNet/Security/KeyExchangeECDH256.cs index b09652de8..29d3dd587 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH256.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH256.cs @@ -1,6 +1,4 @@ using Renci.SshNet.Abstractions; -using Renci.SshNet.Security.Org.BouncyCastle.Asn1.Sec; -using Renci.SshNet.Security.Org.BouncyCastle.Asn1.X9; namespace Renci.SshNet.Security { @@ -15,14 +13,11 @@ public override string Name } /// - /// Gets Curve Parameter. + /// Gets curve name. /// - protected override X9ECParameters CurveParameter + protected override string CurveName { - get - { - return SecNamedCurves.GetByName("P-256"); - } + get { return "secp256r1"; } } /// diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH384.cs b/src/Renci.SshNet/Security/KeyExchangeECDH384.cs index cba304305..c77ae8899 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH384.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH384.cs @@ -1,6 +1,4 @@ using Renci.SshNet.Abstractions; -using Renci.SshNet.Security.Org.BouncyCastle.Asn1.Sec; -using Renci.SshNet.Security.Org.BouncyCastle.Asn1.X9; namespace Renci.SshNet.Security { @@ -15,14 +13,11 @@ public override string Name } /// - /// Gets Curve Parameter. + /// Gets curve name. /// - protected override X9ECParameters CurveParameter + protected override string CurveName { - get - { - return SecNamedCurves.GetByName("P-384"); - } + get { return "secp384r1"; } } /// diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH521.cs b/src/Renci.SshNet/Security/KeyExchangeECDH521.cs index ce5b35515..389dd2073 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH521.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH521.cs @@ -1,6 +1,4 @@ using Renci.SshNet.Abstractions; -using Renci.SshNet.Security.Org.BouncyCastle.Asn1.Sec; -using Renci.SshNet.Security.Org.BouncyCastle.Asn1.X9; namespace Renci.SshNet.Security { @@ -15,14 +13,11 @@ public override string Name } /// - /// Gets Curve Parameter. + /// Gets curve name. /// - protected override X9ECParameters CurveParameter + protected override string CurveName { - get - { - return SecNamedCurves.GetByName("P-521"); - } + get { return "secp521r1"; } } /// From 4b8be402069efb4409ae44a5fe250038361d3228 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sat, 6 Apr 2024 22:17:08 +0800 Subject: [PATCH 02/14] Add back an empty line --- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index a2b6442da..4d64d5e3b 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -69,6 +69,7 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool _keyAgreement = new ECDHCBasicAgreement(); _keyAgreement.Init(aKeyPair.Private); _clientExchangeValue = ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded(); + SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); } From 61e44a48fbb609df531ee4c671b4970067c463b7 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sun, 7 Apr 2024 08:47:57 +0800 Subject: [PATCH 03/14] Remove the BouncyCastle dependency when target .NET 8.0 onward. --- src/Renci.SshNet/Renci.SshNet.csproj | 5 ++ src/Renci.SshNet/Security/KeyExchangeECDH.cs | 72 +++++++++----------- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/Renci.SshNet/Renci.SshNet.csproj b/src/Renci.SshNet/Renci.SshNet.csproj index a6cf61884..ae3db66bc 100644 --- a/src/Renci.SshNet/Renci.SshNet.csproj +++ b/src/Renci.SshNet/Renci.SshNet.csproj @@ -34,6 +34,11 @@ + + + + + True diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index 4d64d5e3b..790cd7c73 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -1,20 +1,30 @@ using System; + +#if NET8_0_OR_GREATER +using System.Security.Cryptography; +#endif + using Renci.SshNet.Common; using Renci.SshNet.Messages.Transport; +#if !NET8_0_OR_GREATER using Renci.SshNet.Security.Org.BouncyCastle.Asn1.Sec; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Agreement; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Generators; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Parameters; using Renci.SshNet.Security.Org.BouncyCastle.Math.EC; using Renci.SshNet.Security.Org.BouncyCastle.Security; +#endif namespace Renci.SshNet.Security { internal abstract class KeyExchangeECDH : KeyExchangeEC { #if NET8_0_OR_GREATER - private System.Security.Cryptography.ECDiffieHellman _clientECDH; + private ECDiffieHellman _clientECDH; +#else + private ECDHCBasicAgreement _keyAgreement; + private ECDomainParameters _domainParameters; #endif /// @@ -25,9 +35,6 @@ internal abstract class KeyExchangeECDH : KeyExchangeEC /// protected abstract string CurveName { get; } - private ECDHCBasicAgreement _keyAgreement; - private ECDomainParameters _domainParameters; - /// public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage) { @@ -38,23 +45,16 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived; #if NET8_0_OR_GREATER - if (IsNonWindowsOrWindowsVersionAtLeast(10)) - { - _clientECDH = System.Security.Cryptography.ECDiffieHellman.Create(); - _clientECDH.GenerateKey(System.Security.Cryptography.ECCurve.CreateFromFriendlyName(CurveName)); + _clientECDH = ECDiffieHellman.Create(); + _clientECDH.GenerateKey(ECCurve.CreateFromFriendlyName(CurveName)); - var q = _clientECDH.PublicKey.ExportParameters().Q; + var q = _clientECDH.PublicKey.ExportParameters().Q; - _clientExchangeValue = new byte[1 + q.X.Length + q.Y.Length]; - _clientExchangeValue[0] = 0x04; - Buffer.BlockCopy(q.X, 0, _clientExchangeValue, 1, q.X.Length); - Buffer.BlockCopy(q.Y, 0, _clientExchangeValue, q.X.Length + 1, q.Y.Length); - - SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); - - return; - } -#endif + _clientExchangeValue = new byte[1 + q.X.Length + q.Y.Length]; + _clientExchangeValue[0] = 0x04; + Buffer.BlockCopy(q.X, 0, _clientExchangeValue, 1, q.X.Length); + Buffer.BlockCopy(q.Y, 0, _clientExchangeValue, q.X.Length + 1, q.Y.Length); +#else var curveParameter = SecNamedCurves.GetByName(CurveName); _domainParameters = new ECDomainParameters(curveParameter.Curve, curveParameter.G, @@ -70,6 +70,7 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool _keyAgreement.Init(aKeyPair.Private); _clientExchangeValue = ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded(); +#endif SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); } @@ -115,30 +116,26 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b Buffer.BlockCopy(serverExchangeValue, cordSize + 1, y, 0, y.Length); #if NET8_0_OR_GREATER - if (IsNonWindowsOrWindowsVersionAtLeast(10)) + using var serverECDH = ECDiffieHellman.Create(new ECParameters { - using var serverECDH = System.Security.Cryptography.ECDiffieHellman.Create(new System.Security.Cryptography.ECParameters + Curve = ECCurve.CreateFromFriendlyName(CurveName), + Q = { - Curve = System.Security.Cryptography.ECCurve.CreateFromFriendlyName(CurveName), - Q = - { - X = x, - Y = y, - }, - }); - - var k = _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey); - SharedKey = k.ToBigInteger2().ToByteArray().Reverse(); - - return; - } -#endif + X = x, + Y = y, + }, + }); + + var k1 = _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey); + SharedKey = k1.ToBigInteger2().ToByteArray().Reverse(); +#else 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); var k1 = _keyAgreement.CalculateAgreement(publicKey); SharedKey = k1.ToByteArray().ToBigInteger2().ToByteArray().Reverse(); +#endif } #if NET8_0_OR_GREATER @@ -152,11 +149,6 @@ protected override void Dispose(bool disposing) _clientECDH?.Dispose(); } } - - private static bool IsNonWindowsOrWindowsVersionAtLeast(int major) - { - return Environment.OSVersion.Platform != PlatformID.Win32NT || Environment.OSVersion.Version.Major >= major; - } #endif } } From 4628a7336b391b32f10109f794cb041346bb475e Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 9 Apr 2024 19:43:14 +0800 Subject: [PATCH 04/14] Run KeyExchangeAlgorithmTests for .NET 6.0 --- appveyor.yml | 1 + .../Renci.SshNet.IntegrationTests.csproj | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 6638609c1..a7522fdcc 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -24,6 +24,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 net6.0 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_6_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_6_coverage.xml --filter KeyExchangeAlgorithmTests test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj - matrix: diff --git a/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj b/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj index 86e4cb0e6..a5e2adb76 100644 --- a/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj +++ b/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj @@ -1,7 +1,7 @@  - net48;net8.0 + net48;net6.0;net8.0 enable false true From 23a1dd367b6b45dd436dff51ce34e98955df3eca Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 9 Apr 2024 20:51:38 +0800 Subject: [PATCH 05/14] Build Renci.SshNet.IntegrationTests.csproj for net6.0 --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index a7522fdcc..77423fb20 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -18,6 +18,7 @@ for: - echo build - dotnet build -f net8.0 -c Debug test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj - dotnet build -f net8.0 -c Debug test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj + - dotnet build -f net6.0 -c Debug test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj test_script: - sh: echo "Run unit tests" From 182f586639b24ddf1610d60270aaf48a120c8914 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 9 Apr 2024 21:28:06 +0800 Subject: [PATCH 06/14] Update filter --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 77423fb20..bcb469cf8 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -25,7 +25,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 net6.0 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_6_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_6_coverage.xml --filter KeyExchangeAlgorithmTests test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj + - sh: dotnet test -f net6.0 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_6_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_6_coverage.xml --filter Name~Ecdh test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj - matrix: From 5502918dc62d42097e015fc9573d9fb4df33f58c Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Wed, 12 Jun 2024 21:49:44 +0800 Subject: [PATCH 07/14] Add back BouncyCastle as fallback --- src/Renci.SshNet/Renci.SshNet.csproj | 5 -- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 64 ++++++++++---------- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/src/Renci.SshNet/Renci.SshNet.csproj b/src/Renci.SshNet/Renci.SshNet.csproj index 2acc759ef..1fd630919 100644 --- a/src/Renci.SshNet/Renci.SshNet.csproj +++ b/src/Renci.SshNet/Renci.SshNet.csproj @@ -39,11 +39,6 @@ - - - - - True diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index 790cd7c73..f325d76f6 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -1,31 +1,24 @@ using System; -#if NET8_0_OR_GREATER -using System.Security.Cryptography; -#endif - using Renci.SshNet.Common; using Renci.SshNet.Messages.Transport; -#if !NET8_0_OR_GREATER using Renci.SshNet.Security.Org.BouncyCastle.Asn1.Sec; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Agreement; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Generators; using Renci.SshNet.Security.Org.BouncyCastle.Crypto.Parameters; using Renci.SshNet.Security.Org.BouncyCastle.Math.EC; using Renci.SshNet.Security.Org.BouncyCastle.Security; -#endif namespace Renci.SshNet.Security { internal abstract class KeyExchangeECDH : KeyExchangeEC { #if NET8_0_OR_GREATER - private ECDiffieHellman _clientECDH; -#else + private System.Security.Cryptography.ECDiffieHellman _clientECDH; +#endif private ECDHCBasicAgreement _keyAgreement; private ECDomainParameters _domainParameters; -#endif /// /// Gets the name of the curve. @@ -45,16 +38,21 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived; #if NET8_0_OR_GREATER - _clientECDH = ECDiffieHellman.Create(); - _clientECDH.GenerateKey(ECCurve.CreateFromFriendlyName(CurveName)); + if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10)) + { + _clientECDH = System.Security.Cryptography.ECDiffieHellman.Create(); + _clientECDH.GenerateKey(System.Security.Cryptography.ECCurve.CreateFromFriendlyName(CurveName)); - var q = _clientECDH.PublicKey.ExportParameters().Q; + var q = _clientECDH.PublicKey.ExportParameters().Q; - _clientExchangeValue = new byte[1 + q.X.Length + q.Y.Length]; - _clientExchangeValue[0] = 0x04; - Buffer.BlockCopy(q.X, 0, _clientExchangeValue, 1, q.X.Length); - Buffer.BlockCopy(q.Y, 0, _clientExchangeValue, q.X.Length + 1, q.Y.Length); -#else + _clientExchangeValue = new byte[1 + q.X.Length + q.Y.Length]; + _clientExchangeValue[0] = 0x04; + Buffer.BlockCopy(q.X, 0, _clientExchangeValue, 1, q.X.Length); + Buffer.BlockCopy(q.Y, 0, _clientExchangeValue, q.X.Length + 1, q.Y.Length); + + return; + } +#endif var curveParameter = SecNamedCurves.GetByName(CurveName); _domainParameters = new ECDomainParameters(curveParameter.Curve, curveParameter.G, @@ -70,7 +68,6 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool _keyAgreement.Init(aKeyPair.Private); _clientExchangeValue = ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded(); -#endif SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); } @@ -116,26 +113,31 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b Buffer.BlockCopy(serverExchangeValue, cordSize + 1, y, 0, y.Length); #if NET8_0_OR_GREATER - using var serverECDH = ECDiffieHellman.Create(new ECParameters + if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10)) { - Curve = ECCurve.CreateFromFriendlyName(CurveName), - Q = - { - X = x, - Y = y, - }, - }); - - var k1 = _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey); - SharedKey = k1.ToBigInteger2().ToByteArray().Reverse(); -#else + using var serverECDH = System.Security.Cryptography.ECDiffieHellman.Create( + new System.Security.Cryptography.ECParameters + { + Curve = System.Security.Cryptography.ECCurve.CreateFromFriendlyName(CurveName), + Q = + { + X = x, + Y = y, + }, + }); + + var k = _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey); + SharedKey = k.ToBigInteger2().ToByteArray().Reverse(); + + return; + } +#endif 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); var k1 = _keyAgreement.CalculateAgreement(publicKey); SharedKey = k1.ToByteArray().ToBigInteger2().ToByteArray().Reverse(); -#endif } #if NET8_0_OR_GREATER From a1b00e31d46b48b422e34ffc572b8bb03c2fd74b Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Thu, 13 Jun 2024 09:33:19 +0800 Subject: [PATCH 08/14] Add back the missing `SendMessage` --- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index f325d76f6..06b0b5075 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -50,6 +50,8 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool Buffer.BlockCopy(q.X, 0, _clientExchangeValue, 1, q.X.Length); Buffer.BlockCopy(q.Y, 0, _clientExchangeValue, q.X.Length + 1, q.Y.Length); + SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); + return; } #endif From 13dd7e13c09cb2328826f509ff83161254a8305b Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Thu, 25 Jul 2024 19:59:47 +0800 Subject: [PATCH 09/14] Run ECDH KEX integration tests under .NET48 --- appveyor.yml | 4 ++-- .../Renci.SshNet.IntegrationTests.csproj | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index e664646a6..22b8fb13d 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -21,14 +21,14 @@ for: - echo build - dotnet build -f net8.0 -c Debug test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj - dotnet build -f net8.0 -c Debug test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj - - dotnet build -f net6.0 -c Debug test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj + - dotnet build -f net48 -c Debug test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj test_script: - sh: echo "Run unit 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_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 net6.0 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_6_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_6_coverage.xml --filter Name~Ecdh 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~Ecdh|Name~Zlib" test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj - matrix: diff --git a/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj b/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj index abdc2b28d..0ad9f8ce8 100644 --- a/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj +++ b/test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj @@ -1,7 +1,7 @@  - net48;net6.0;net8.0 + net48;net8.0 enable true $(NoWarn);SYSLIB0021;SYSLIB1045;SYSLIB0014;IDE0220;IDE0010 From 385e087361ac26ee5abb09b30e46d418d14c461c Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sun, 28 Jul 2024 22:16:40 +0800 Subject: [PATCH 10/14] 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 --- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 4 ++-- src/Renci.SshNet/Security/KeyExchangeECDH256.cs | 2 +- src/Renci.SshNet/Security/KeyExchangeECDH384.cs | 2 +- src/Renci.SshNet/Security/KeyExchangeECDH521.cs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index 97cd5d1ec..8cacf09fb 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -1,9 +1,9 @@ using System; -using Org.BouncyCastle.Asn1.Sec; using Org.BouncyCastle.Crypto.Agreement; using Org.BouncyCastle.Crypto.Generators; using Org.BouncyCastle.Crypto.Parameters; +using Org.BouncyCastle.Crypto.Utilities; using Org.BouncyCastle.Math.EC; using Renci.SshNet.Abstractions; @@ -55,7 +55,7 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool return; } #endif - var curveParameter = SecNamedCurves.GetByName(CurveName); + var curveParameter = SshNamedCurves.GetByName(CurveName); _domainParameters = new ECDomainParameters(curveParameter.Curve, curveParameter.G, curveParameter.N, diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH256.cs b/src/Renci.SshNet/Security/KeyExchangeECDH256.cs index eaa4d6da2..0f9f552fc 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH256.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH256.cs @@ -17,7 +17,7 @@ public override string Name /// protected override string CurveName { - get { return "secp256r1"; } + get { return "nistp256"; } } /// diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH384.cs b/src/Renci.SshNet/Security/KeyExchangeECDH384.cs index 6a8f17f19..e8b9a5dc5 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH384.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH384.cs @@ -17,7 +17,7 @@ public override string Name /// protected override string CurveName { - get { return "secp384r1"; } + get { return "nistp384"; } } /// diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH521.cs b/src/Renci.SshNet/Security/KeyExchangeECDH521.cs index 9d1f105fa..634fee94e 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH521.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH521.cs @@ -17,7 +17,7 @@ public override string Name /// protected override string CurveName { - get { return "secp521r1"; } + get { return "nistp384"; } } /// From 387e6dafe4a764a8490c0acb1ef6309fef4c71b8 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sun, 28 Jul 2024 22:19:05 +0800 Subject: [PATCH 11/14] typo --- src/Renci.SshNet/Security/KeyExchangeECDH521.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH521.cs b/src/Renci.SshNet/Security/KeyExchangeECDH521.cs index 634fee94e..7b5bd7532 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH521.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH521.cs @@ -17,7 +17,7 @@ public override string Name /// protected override string CurveName { - get { return "nistp384"; } + get { return "nistp521"; } } /// From 73c94466f5cdb58d74d277823d7940e0360b7d48 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Mon, 29 Jul 2024 14:47:12 +0800 Subject: [PATCH 12/14] Fix build --- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 36 +++++++++++-------- .../Security/KeyExchangeECDH256.cs | 27 +++++++++++--- .../Security/KeyExchangeECDH384.cs | 27 +++++++++++--- .../Security/KeyExchangeECDH521.cs | 27 +++++++++++--- 4 files changed, 91 insertions(+), 26 deletions(-) diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index 8cacf09fb..5af8c440a 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -1,9 +1,9 @@ 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.Crypto.Utilities; using Org.BouncyCastle.Math.EC; using Renci.SshNet.Abstractions; @@ -14,19 +14,28 @@ namespace Renci.SshNet.Security { internal abstract class KeyExchangeECDH : KeyExchangeEC { + private ECDHCBasicAgreement _keyAgreement; + private ECDomainParameters _domainParameters; + #if NET8_0_OR_GREATER private System.Security.Cryptography.ECDiffieHellman _clientECDH; + + /// + /// Gets the curve. + /// + /// + /// The curve. + /// + protected abstract System.Security.Cryptography.ECCurve Curve { get; } #endif - private ECDHCBasicAgreement _keyAgreement; - private ECDomainParameters _domainParameters; /// - /// Gets the name of the curve. + /// Gets the parameter of the curve. /// /// - /// The name of the curve. + /// The parameter of the curve. /// - protected abstract string CurveName { get; } + protected abstract X9ECParameters CurveParameter { get; } /// public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage) @@ -41,7 +50,7 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10)) { _clientECDH = System.Security.Cryptography.ECDiffieHellman.Create(); - _clientECDH.GenerateKey(System.Security.Cryptography.ECCurve.CreateFromFriendlyName(CurveName)); + _clientECDH.GenerateKey(Curve); var q = _clientECDH.PublicKey.ExportParameters().Q; @@ -55,12 +64,11 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool return; } #endif - var curveParameter = SshNamedCurves.GetByName(CurveName); - _domainParameters = new ECDomainParameters(curveParameter.Curve, - curveParameter.G, - curveParameter.N, - curveParameter.H, - curveParameter.GetSeed()); + _domainParameters = new ECDomainParameters(CurveParameter.Curve, + CurveParameter.G, + CurveParameter.N, + CurveParameter.H, + CurveParameter.GetSeed()); var g = new ECKeyPairGenerator(); g.Init(new ECKeyGenerationParameters(_domainParameters, CryptoAbstraction.SecureRandom)); @@ -120,7 +128,7 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b using var serverECDH = System.Security.Cryptography.ECDiffieHellman.Create( new System.Security.Cryptography.ECParameters { - Curve = System.Security.Cryptography.ECCurve.CreateFromFriendlyName(CurveName), + Curve = Curve, Q = { X = x, diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH256.cs b/src/Renci.SshNet/Security/KeyExchangeECDH256.cs index 0f9f552fc..2a3be0af8 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH256.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH256.cs @@ -1,4 +1,7 @@ -using Renci.SshNet.Abstractions; +using Org.BouncyCastle.Asn1.Sec; +using Org.BouncyCastle.Asn1.X9; + +using Renci.SshNet.Abstractions; namespace Renci.SshNet.Security { @@ -12,12 +15,28 @@ public override string Name get { return "ecdh-sha2-nistp256"; } } +#if NET8_0_OR_GREATER + /// + /// Gets the curve. + /// + protected override System.Security.Cryptography.ECCurve Curve + { + get + { + return System.Security.Cryptography.ECCurve.NamedCurves.nistP256; + } + } +#endif + /// - /// Gets curve name. + /// Gets Curve Parameter. /// - protected override string CurveName + protected override X9ECParameters CurveParameter { - get { return "nistp256"; } + get + { + return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP256r1); + } } /// diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH384.cs b/src/Renci.SshNet/Security/KeyExchangeECDH384.cs index e8b9a5dc5..b9d440f7d 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH384.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH384.cs @@ -1,4 +1,7 @@ -using Renci.SshNet.Abstractions; +using Org.BouncyCastle.Asn1.Sec; +using Org.BouncyCastle.Asn1.X9; + +using Renci.SshNet.Abstractions; namespace Renci.SshNet.Security { @@ -12,12 +15,28 @@ public override string Name get { return "ecdh-sha2-nistp384"; } } +#if NET8_0_OR_GREATER + /// + /// Gets the curve. + /// + protected override System.Security.Cryptography.ECCurve Curve + { + get + { + return System.Security.Cryptography.ECCurve.NamedCurves.nistP384; + } + } +#endif + /// - /// Gets curve name. + /// Gets Curve Parameter. /// - protected override string CurveName + protected override X9ECParameters CurveParameter { - get { return "nistp384"; } + get + { + return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP384r1); + } } /// diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH521.cs b/src/Renci.SshNet/Security/KeyExchangeECDH521.cs index 7b5bd7532..07d993cee 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH521.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH521.cs @@ -1,4 +1,7 @@ -using Renci.SshNet.Abstractions; +using Org.BouncyCastle.Asn1.Sec; +using Org.BouncyCastle.Asn1.X9; + +using Renci.SshNet.Abstractions; namespace Renci.SshNet.Security { @@ -12,12 +15,28 @@ public override string Name get { return "ecdh-sha2-nistp521"; } } +#if NET8_0_OR_GREATER + /// + /// Gets the curve. + /// + protected override System.Security.Cryptography.ECCurve Curve + { + get + { + return System.Security.Cryptography.ECCurve.NamedCurves.nistP521; + } + } +#endif + /// - /// Gets curve name. + /// Gets Curve Parameter. /// - protected override string CurveName + protected override X9ECParameters CurveParameter { - get { return "nistp521"; } + get + { + return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP521r1); + } } /// From db0a98ef27b7a7070fd2714f861776e6c115de93 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Mon, 29 Jul 2024 21:32:26 +0800 Subject: [PATCH 13/14] Use System.Security.Cryptography namespace if NET8_0_OR_GREATER; Use one parameter constructor for class ECDomainParameters --- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 25 ++++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index 5af8c440a..a0b5a2027 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -1,10 +1,12 @@ using System; +#if NET8_0_OR_GREATER +using System.Security.Cryptography; +#endif 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; @@ -14,19 +16,20 @@ namespace Renci.SshNet.Security { internal abstract class KeyExchangeECDH : KeyExchangeEC { +#if NET8_0_OR_GREATER + private ECDiffieHellman _clientECDH; +#endif private ECDHCBasicAgreement _keyAgreement; private ECDomainParameters _domainParameters; #if NET8_0_OR_GREATER - private System.Security.Cryptography.ECDiffieHellman _clientECDH; - /// /// Gets the curve. /// /// /// The curve. /// - protected abstract System.Security.Cryptography.ECCurve Curve { get; } + protected abstract ECCurve Curve { get; } #endif /// @@ -49,7 +52,7 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool #if NET8_0_OR_GREATER if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10)) { - _clientECDH = System.Security.Cryptography.ECDiffieHellman.Create(); + _clientECDH = ECDiffieHellman.Create(); _clientECDH.GenerateKey(Curve); var q = _clientECDH.PublicKey.ExportParameters().Q; @@ -64,11 +67,7 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool return; } #endif - _domainParameters = new ECDomainParameters(CurveParameter.Curve, - CurveParameter.G, - CurveParameter.N, - CurveParameter.H, - CurveParameter.GetSeed()); + _domainParameters = new ECDomainParameters(CurveParameter); var g = new ECKeyPairGenerator(); g.Init(new ECKeyGenerationParameters(_domainParameters, CryptoAbstraction.SecureRandom)); @@ -125,8 +124,8 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b #if NET8_0_OR_GREATER if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10)) { - using var serverECDH = System.Security.Cryptography.ECDiffieHellman.Create( - new System.Security.Cryptography.ECParameters + using var serverECDH = ECDiffieHellman.Create( + new ECParameters { Curve = Curve, Q = @@ -142,7 +141,7 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b return; } #endif - var c = (FpCurve)_domainParameters.Curve; + var c = _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); From 2441f77e0d320d10c79d0a4aad829c72c903545e Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 30 Jul 2024 10:32:56 +0800 Subject: [PATCH 14/14] Separate BCL and BouncyCastle implementation --- .../Security/KeyExchangeECDH.BclImpl.cs | 77 +++++++++++++ .../KeyExchangeECDH.BouncyCastleImpl.cs | 44 ++++++++ src/Renci.SshNet/Security/KeyExchangeECDH.cs | 105 ++++++------------ 3 files changed, 156 insertions(+), 70 deletions(-) create mode 100644 src/Renci.SshNet/Security/KeyExchangeECDH.BclImpl.cs create mode 100644 src/Renci.SshNet/Security/KeyExchangeECDH.BouncyCastleImpl.cs diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.BclImpl.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.BclImpl.cs new file mode 100644 index 000000000..7c1a5fb51 --- /dev/null +++ b/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 diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.BouncyCastleImpl.cs new file mode 100644 index 000000000..4d0208efe --- /dev/null +++ b/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(); + } + } + } +} diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index a0b5a2027..e5ec28c5d 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -1,35 +1,26 @@ using System; -#if NET8_0_OR_GREATER -using System.Security.Cryptography; -#endif using Org.BouncyCastle.Asn1.X9; -using Org.BouncyCastle.Crypto.Agreement; -using Org.BouncyCastle.Crypto.Generators; -using Org.BouncyCastle.Crypto.Parameters; -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 ECDiffieHellman _clientECDH; -#endif - private ECDHCBasicAgreement _keyAgreement; - private ECDomainParameters _domainParameters; + private Impl _impl; -#if NET8_0_OR_GREATER /// /// Gets the curve. /// /// /// The curve. /// - protected abstract ECCurve Curve { get; } + protected abstract System.Security.Cryptography.ECCurve Curve { get; } +#else + private BouncyCastleImpl _impl; #endif /// @@ -52,30 +43,17 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool #if NET8_0_OR_GREATER if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10)) { - _clientECDH = ECDiffieHellman.Create(); - _clientECDH.GenerateKey(Curve); - - var q = _clientECDH.PublicKey.ExportParameters().Q; - - _clientExchangeValue = new byte[1 + q.X.Length + q.Y.Length]; - _clientExchangeValue[0] = 0x04; - Buffer.BlockCopy(q.X, 0, _clientExchangeValue, 1, q.X.Length); - Buffer.BlockCopy(q.Y, 0, _clientExchangeValue, q.X.Length + 1, q.Y.Length); - - SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); - - return; + _impl = new BclImpl(Curve); } + else + { + _impl = new BouncyCastleImpl(CurveParameter); + } +#else + _impl = new BouncyCastleImpl(CurveParameter); #endif - _domainParameters = new ECDomainParameters(CurveParameter); - 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(); + _clientExchangeValue = _impl.GenerateClientECPoint(); SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue)); } @@ -115,51 +93,38 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b _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); -#if NET8_0_OR_GREATER - if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10)) - { - using var serverECDH = ECDiffieHellman.Create( - new ECParameters - { - Curve = Curve, - Q = - { - X = x, - Y = y, - }, - }); - - var k = _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey); - SharedKey = k.ToBigInteger2().ToByteArray().Reverse(); - - return; - } -#endif - var c = _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); - - var k1 = _keyAgreement.CalculateAgreement(publicKey); - SharedKey = k1.ToByteArray().ToBigInteger2().ToByteArray().Reverse(); + SharedKey = agreement.ToBigInteger2().ToByteArray().Reverse(); } -#if NET8_0_OR_GREATER - /// protected override void Dispose(bool disposing) { base.Dispose(disposing); + if (disposing) { - _clientECDH?.Dispose(); + _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); } } -#endif } }