Skip to content

Commit 58c85ee

Browse files
scott-xuWojciechNagorski
scott-xu
and
WojciechNagorski
authored
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 <[email protected]>
1 parent 252c732 commit 58c85ee

7 files changed

+226
-39
lines changed

‎appveyor.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ for:
2828
- 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
2929
- sh: echo "Run integration tests"
3030
- 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
31-
- 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
31+
- 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
3232

3333
-
3434
matrix:

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

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
#if NET8_0_OR_GREATER
2+
using System;
3+
using System.Security.Cryptography;
4+
5+
namespace Renci.SshNet.Security
6+
{
7+
internal abstract partial class KeyExchangeECDH
8+
{
9+
private sealed class BclImpl : Impl
10+
{
11+
private readonly ECCurve _curve;
12+
private readonly ECDiffieHellman _clientECDH;
13+
14+
public BclImpl(ECCurve curve)
15+
{
16+
_curve = curve;
17+
_clientECDH = ECDiffieHellman.Create();
18+
}
19+
20+
public override byte[] GenerateClientECPoint()
21+
{
22+
_clientECDH.GenerateKey(_curve);
23+
24+
var q = _clientECDH.PublicKey.ExportParameters().Q;
25+
26+
return EncodeECPoint(q);
27+
}
28+
29+
public override byte[] CalculateAgreement(byte[] serverECPoint)
30+
{
31+
var q = DecodeECPoint(serverECPoint);
32+
33+
var parameters = new ECParameters
34+
{
35+
Curve = _curve,
36+
Q = q,
37+
};
38+
39+
using var serverECDH = ECDiffieHellman.Create(parameters);
40+
41+
return _clientECDH.DeriveRawSecretAgreement(serverECDH.PublicKey);
42+
}
43+
44+
private static byte[] EncodeECPoint(ECPoint point)
45+
{
46+
var q = new byte[1 + point.X.Length + point.Y.Length];
47+
q[0] = 0x04;
48+
Buffer.BlockCopy(point.X, 0, q, 1, point.X.Length);
49+
Buffer.BlockCopy(point.Y, 0, q, point.X.Length + 1, point.Y.Length);
50+
51+
return q;
52+
}
53+
54+
private static ECPoint DecodeECPoint(byte[] q)
55+
{
56+
var cordSize = (q.Length - 1) / 2;
57+
var x = new byte[cordSize];
58+
var y = new byte[cordSize];
59+
Buffer.BlockCopy(q, 1, x, 0, x.Length);
60+
Buffer.BlockCopy(q, cordSize + 1, y, 0, y.Length);
61+
62+
return new ECPoint { X = x, Y = y };
63+
}
64+
65+
protected override void Dispose(bool disposing)
66+
{
67+
base.Dispose(disposing);
68+
69+
if (disposing)
70+
{
71+
_clientECDH.Dispose();
72+
}
73+
}
74+
}
75+
}
76+
}
77+
#endif

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

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
using Org.BouncyCastle.Asn1.X9;
2+
using Org.BouncyCastle.Crypto.Agreement;
3+
using Org.BouncyCastle.Crypto.Generators;
4+
using Org.BouncyCastle.Crypto.Parameters;
5+
6+
using Renci.SshNet.Abstractions;
7+
8+
namespace Renci.SshNet.Security
9+
{
10+
internal abstract partial class KeyExchangeECDH
11+
{
12+
private sealed class BouncyCastleImpl : Impl
13+
{
14+
private readonly ECDomainParameters _domainParameters;
15+
private readonly ECDHCBasicAgreement _keyAgreement;
16+
17+
public BouncyCastleImpl(X9ECParameters curveParameters)
18+
{
19+
_domainParameters = new ECDomainParameters(curveParameters);
20+
_keyAgreement = new ECDHCBasicAgreement();
21+
}
22+
23+
public override byte[] GenerateClientECPoint()
24+
{
25+
var g = new ECKeyPairGenerator();
26+
g.Init(new ECKeyGenerationParameters(_domainParameters, CryptoAbstraction.SecureRandom));
27+
28+
var aKeyPair = g.GenerateKeyPair();
29+
_keyAgreement.Init(aKeyPair.Private);
30+
31+
return ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded();
32+
}
33+
34+
public override byte[] CalculateAgreement(byte[] serverECPoint)
35+
{
36+
var c = _domainParameters.Curve;
37+
var q = c.DecodePoint(serverECPoint);
38+
var publicKey = new ECPublicKeyParameters("ECDH", q, _domainParameters);
39+
40+
return _keyAgreement.CalculateAgreement(publicKey).ToByteArray();
41+
}
42+
}
43+
}
44+
}

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

+59-32
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,28 @@
11
using System;
22

33
using Org.BouncyCastle.Asn1.X9;
4-
using Org.BouncyCastle.Crypto.Agreement;
5-
using Org.BouncyCastle.Crypto.Generators;
6-
using Org.BouncyCastle.Crypto.Parameters;
7-
using Org.BouncyCastle.Math.EC;
84

9-
using Renci.SshNet.Abstractions;
105
using Renci.SshNet.Common;
116
using Renci.SshNet.Messages.Transport;
127

138
namespace Renci.SshNet.Security
149
{
15-
internal abstract class KeyExchangeECDH : KeyExchangeEC
10+
internal abstract partial class KeyExchangeECDH : KeyExchangeEC
1611
{
12+
#if NET8_0_OR_GREATER
13+
private Impl _impl;
14+
15+
/// <summary>
16+
/// Gets the curve.
17+
/// </summary>
18+
/// <value>
19+
/// The curve.
20+
/// </value>
21+
protected abstract System.Security.Cryptography.ECCurve Curve { get; }
22+
#else
23+
private BouncyCastleImpl _impl;
24+
#endif
25+
1726
/// <summary>
1827
/// Gets the parameter of the curve.
1928
/// </summary>
@@ -22,9 +31,6 @@ internal abstract class KeyExchangeECDH : KeyExchangeEC
2231
/// </value>
2332
protected abstract X9ECParameters CurveParameter { get; }
2433

25-
private ECDHCBasicAgreement _keyAgreement;
26-
private ECDomainParameters _domainParameters;
27-
2834
/// <inheritdoc/>
2935
public override void Start(Session session, KeyExchangeInitMessage message, bool sendClientInitMessage)
3036
{
@@ -34,19 +40,20 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool
3440

3541
Session.KeyExchangeEcdhReplyMessageReceived += Session_KeyExchangeEcdhReplyMessageReceived;
3642

37-
_domainParameters = new ECDomainParameters(CurveParameter.Curve,
38-
CurveParameter.G,
39-
CurveParameter.N,
40-
CurveParameter.H,
41-
CurveParameter.GetSeed());
42-
43-
var g = new ECKeyPairGenerator();
44-
g.Init(new ECKeyGenerationParameters(_domainParameters, CryptoAbstraction.SecureRandom));
45-
46-
var aKeyPair = g.GenerateKeyPair();
47-
_keyAgreement = new ECDHCBasicAgreement();
48-
_keyAgreement.Init(aKeyPair.Private);
49-
_clientExchangeValue = ((ECPublicKeyParameters)aKeyPair.Public).Q.GetEncoded();
43+
#if NET8_0_OR_GREATER
44+
if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10))
45+
{
46+
_impl = new BclImpl(Curve);
47+
}
48+
else
49+
{
50+
_impl = new BouncyCastleImpl(CurveParameter);
51+
}
52+
#else
53+
_impl = new BouncyCastleImpl(CurveParameter);
54+
#endif
55+
56+
_clientExchangeValue = _impl.GenerateClientECPoint();
5057

5158
SendMessage(new KeyExchangeEcdhInitMessage(_clientExchangeValue));
5259
}
@@ -86,18 +93,38 @@ private void HandleServerEcdhReply(byte[] hostKey, byte[] serverExchangeValue, b
8693
_hostKey = hostKey;
8794
_signature = signature;
8895

89-
var cordSize = (serverExchangeValue.Length - 1) / 2;
90-
var x = new byte[cordSize];
91-
Buffer.BlockCopy(serverExchangeValue, 1, x, 0, x.Length); // first byte is format. should be checked and passed to bouncy castle?
92-
var y = new byte[cordSize];
93-
Buffer.BlockCopy(serverExchangeValue, cordSize + 1, y, 0, y.Length);
96+
var agreement = _impl.CalculateAgreement(serverExchangeValue);
97+
98+
SharedKey = agreement.ToBigInteger2().ToByteArray().Reverse();
99+
}
100+
101+
/// <inheritdoc/>
102+
protected override void Dispose(bool disposing)
103+
{
104+
base.Dispose(disposing);
105+
106+
if (disposing)
107+
{
108+
_impl?.Dispose();
109+
}
110+
}
111+
112+
private abstract class Impl : IDisposable
113+
{
114+
public abstract byte[] GenerateClientECPoint();
115+
116+
public abstract byte[] CalculateAgreement(byte[] serverECPoint);
94117

95-
var c = (FpCurve)_domainParameters.Curve;
96-
var q = c.CreatePoint(new Org.BouncyCastle.Math.BigInteger(1, x), new Org.BouncyCastle.Math.BigInteger(1, y));
97-
var publicKey = new ECPublicKeyParameters("ECDH", q, _domainParameters);
118+
protected virtual void Dispose(bool disposing)
119+
{
120+
}
98121

99-
var k1 = _keyAgreement.CalculateAgreement(publicKey);
100-
SharedKey = k1.ToByteArray().ToBigInteger2().ToByteArray().Reverse();
122+
public void Dispose()
123+
{
124+
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
125+
Dispose(disposing: true);
126+
GC.SuppressFinalize(this);
127+
}
101128
}
102129
}
103130
}

‎src/Renci.SshNet/Security/KeyExchangeECDH256.cs

+15-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using Org.BouncyCastle.Asn1.Sec;
1+
using Org.BouncyCastle.Asn1.Sec;
22
using Org.BouncyCastle.Asn1.X9;
33

44
using Renci.SshNet.Abstractions;
@@ -15,14 +15,27 @@ public override string Name
1515
get { return "ecdh-sha2-nistp256"; }
1616
}
1717

18+
#if NET8_0_OR_GREATER
19+
/// <summary>
20+
/// Gets the curve.
21+
/// </summary>
22+
protected override System.Security.Cryptography.ECCurve Curve
23+
{
24+
get
25+
{
26+
return System.Security.Cryptography.ECCurve.NamedCurves.nistP256;
27+
}
28+
}
29+
#endif
30+
1831
/// <summary>
1932
/// Gets Curve Parameter.
2033
/// </summary>
2134
protected override X9ECParameters CurveParameter
2235
{
2336
get
2437
{
25-
return SecNamedCurves.GetByName("secp256r1");
38+
return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP256r1);
2639
}
2740
}
2841

‎src/Renci.SshNet/Security/KeyExchangeECDH384.cs

+15-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using Org.BouncyCastle.Asn1.Sec;
1+
using Org.BouncyCastle.Asn1.Sec;
22
using Org.BouncyCastle.Asn1.X9;
33

44
using Renci.SshNet.Abstractions;
@@ -15,14 +15,27 @@ public override string Name
1515
get { return "ecdh-sha2-nistp384"; }
1616
}
1717

18+
#if NET8_0_OR_GREATER
19+
/// <summary>
20+
/// Gets the curve.
21+
/// </summary>
22+
protected override System.Security.Cryptography.ECCurve Curve
23+
{
24+
get
25+
{
26+
return System.Security.Cryptography.ECCurve.NamedCurves.nistP384;
27+
}
28+
}
29+
#endif
30+
1831
/// <summary>
1932
/// Gets Curve Parameter.
2033
/// </summary>
2134
protected override X9ECParameters CurveParameter
2235
{
2336
get
2437
{
25-
return SecNamedCurves.GetByName("secp384r1");
38+
return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP384r1);
2639
}
2740
}
2841

‎src/Renci.SshNet/Security/KeyExchangeECDH521.cs

+15-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using Org.BouncyCastle.Asn1.Sec;
1+
using Org.BouncyCastle.Asn1.Sec;
22
using Org.BouncyCastle.Asn1.X9;
33

44
using Renci.SshNet.Abstractions;
@@ -15,14 +15,27 @@ public override string Name
1515
get { return "ecdh-sha2-nistp521"; }
1616
}
1717

18+
#if NET8_0_OR_GREATER
19+
/// <summary>
20+
/// Gets the curve.
21+
/// </summary>
22+
protected override System.Security.Cryptography.ECCurve Curve
23+
{
24+
get
25+
{
26+
return System.Security.Cryptography.ECCurve.NamedCurves.nistP521;
27+
}
28+
}
29+
#endif
30+
1831
/// <summary>
1932
/// Gets Curve Parameter.
2033
/// </summary>
2134
protected override X9ECParameters CurveParameter
2235
{
2336
get
2437
{
25-
return SecNamedCurves.GetByName("secp521r1");
38+
return SecNamedCurves.GetByOid(SecObjectIdentifiers.SecP521r1);
2639
}
2740
}
2841

0 commit comments

Comments
 (0)