Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AesGcm] Falls back to use BouncyCastle if BCL doesn't support #1450

Merged
merged 23 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7df5108
[AesGcmCipher] Use BouncyCastle as a fallback if BCL does not support.
scott-xu Jul 21, 2024
dfe5b22
Switch back to collection initializer
scott-xu Jul 22, 2024
aff0b4d
Merge branch 'develop' into bc_aesgcm
WojciechNagorski Jul 23, 2024
a905b34
Merge branch 'develop' into bc_aesgcm
WojciechNagorski Jul 24, 2024
f1a92d3
Merge branch 'develop' into bc_aesgcm
WojciechNagorski Jul 25, 2024
55f38c4
Merge branch 'develop' into bc_aesgcm
scott-xu Jul 27, 2024
f3121f4
Remove conditional compilation
scott-xu Jul 27, 2024
1152c9d
Throw SshConnectionException with Reason MacError when authentication…
scott-xu Jul 27, 2024
75f9bce
Separate BCL and BouncyCastle implementation
scott-xu Jul 30, 2024
8bdf511
Update AesGcmCipher.BclImpl.cs
scott-xu Jul 30, 2024
013741e
Merge branch 'develop' into bc_aesgcm
scott-xu Aug 1, 2024
11ab2ee
Merge branch 'develop' into bc_aesgcm
scott-xu Aug 2, 2024
e36823b
Naming enhancement
scott-xu Aug 4, 2024
b12e3b6
Remove empty line
scott-xu Aug 5, 2024
a324a8c
Merge branch 'develop' of https://github.com/scott-xu/SSH.NET into bc…
scott-xu Aug 6, 2024
1751ed7
Disable S1199. See https://github.com/sshnet/SSH.NET/pull/1371#discus…
scott-xu Aug 9, 2024
6f47e03
Set InnerException when MAC error. Remove Message check.
scott-xu Aug 10, 2024
432d6b6
Store KeyParameter as private field
scott-xu Aug 10, 2024
1e9a0b4
Use GcmCipher.ProcessAadBytes to avoid the copy of associated data
scott-xu Aug 10, 2024
fb68303
Move nonce to constructor to avoid creating AeadParameters each packet
scott-xu Aug 10, 2024
b4dc013
Use const int for tag size
scott-xu Aug 10, 2024
2e9228d
Merge branch 'develop' into bc_aesgcm
scott-xu Aug 11, 2024
ab5989d
Merge branch 'develop' into bc_aesgcm
scott-xu Aug 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ dotnet_diagnostic.S1144.severity = none
# This is a duplicate of IDE0060.
dotnet_diagnostic.S1172.severity = none

# S1199: Nested code blocks should not be used
# https://rules.sonarsource.com/csharp/RSPEC-1199
dotnet_diagnostic.S1199.severity = none

# S1481: Unused local variables should be removed
# https://rules.sonarsource.com/csharp/RSPEC-1481
#
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ The main types provided by this library are:
* aes128-ctr
* aes192-ctr
* aes256-ctr
* aes128-gcm<span></span>@openssh.com (.NET 6 and higher)
* aes256-gcm<span></span>@openssh.com (.NET 6 and higher)
* aes128-gcm<span></span>@openssh.com
* aes256-gcm<span></span>@openssh.com
* chacha20-poly1305<span></span>@openssh.com
* aes128-cbc
* aes192-cbc
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ for:
# some coverage until a proper solution for running the .NET Framework integration tests in CI is found.
# Running all the tests causes problems which are not worth solving in this rare configuration.
# See https://github.com/sshnet/SSH.NET/pull/1462 and related links
- 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
- 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|Name~Gcm" test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj

-
matrix:
Expand Down
29 changes: 13 additions & 16 deletions src/Renci.SshNet/ConnectionInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,22 +381,19 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy
{ "diffie-hellman-group1-sha1", () => new KeyExchangeDiffieHellmanGroup1Sha1() },
};

Encryptions = new Dictionary<string, CipherInfo>();
Encryptions.Add("aes128-ctr", new CipherInfo(128, (key, iv) => new AesCipher(key, iv, AesCipherMode.CTR, pkcs7Padding: false)));
Encryptions.Add("aes192-ctr", new CipherInfo(192, (key, iv) => new AesCipher(key, iv, AesCipherMode.CTR, pkcs7Padding: false)));
Encryptions.Add("aes256-ctr", new CipherInfo(256, (key, iv) => new AesCipher(key, iv, AesCipherMode.CTR, pkcs7Padding: false)));
#if NET6_0_OR_GREATER
if (AesGcm.IsSupported)
{
Encryptions.Add("aes128-gcm@openssh.com", new CipherInfo(128, (key, iv) => new AesGcmCipher(key, iv), isAead: true));
Encryptions.Add("aes256-gcm@openssh.com", new CipherInfo(256, (key, iv) => new AesGcmCipher(key, iv), isAead: true));
}
#endif
Encryptions.Add("chacha20-poly1305@openssh.com", new CipherInfo(512, (key, iv) => new ChaCha20Poly1305Cipher(key), isAead: true));
Encryptions.Add("aes128-cbc", new CipherInfo(128, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)));
Encryptions.Add("aes192-cbc", new CipherInfo(192, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)));
Encryptions.Add("aes256-cbc", new CipherInfo(256, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)));
Encryptions.Add("3des-cbc", new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CbcCipherMode(iv), padding: null)));
Encryptions = new Dictionary<string, CipherInfo>
{
{ "aes128-ctr", new CipherInfo(128, (key, iv) => new AesCipher(key, iv, AesCipherMode.CTR, pkcs7Padding: false)) },
{ "aes192-ctr", new CipherInfo(192, (key, iv) => new AesCipher(key, iv, AesCipherMode.CTR, pkcs7Padding: false)) },
{ "aes256-ctr", new CipherInfo(256, (key, iv) => new AesCipher(key, iv, AesCipherMode.CTR, pkcs7Padding: false)) },
{ "aes128-gcm@openssh.com", new CipherInfo(128, (key, iv) => new AesGcmCipher(key, iv), isAead: true) },
{ "aes256-gcm@openssh.com", new CipherInfo(256, (key, iv) => new AesGcmCipher(key, iv), isAead: true) },
{ "chacha20-poly1305@openssh.com", new CipherInfo(512, (key, iv) => new ChaCha20Poly1305Cipher(key), isAead: true) },
{ "aes128-cbc", new CipherInfo(128, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)) },
{ "aes192-cbc", new CipherInfo(192, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)) },
{ "aes256-cbc", new CipherInfo(256, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)) },
{ "3des-cbc", new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CbcCipherMode(iv), padding: null)) },
};

HmacAlgorithms = new Dictionary<string, HashInfo>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
#if NET6_0_OR_GREATER
using System;
using System.Security.Cryptography;

using Renci.SshNet.Common;
using Renci.SshNet.Messages.Transport;

namespace Renci.SshNet.Security.Cryptography.Ciphers
{
internal partial class AesGcmCipher
{
private sealed class BclImpl : Impl
{
private readonly AesGcm _aesGcm;
private readonly byte[] _nonce;

public BclImpl(byte[] key, byte[] nonce)
{
#if NET8_0_OR_GREATER
_aesGcm = new AesGcm(key, TagSizeInBytes);
#else
_aesGcm = new AesGcm(key);
#endif
_nonce = nonce;
}

public override void Encrypt(byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset)
{
var cipherTextLength = plainTextLength;
var plainText = new ReadOnlySpan<byte>(input, plainTextOffset, plainTextLength);
var cipherText = new Span<byte>(output, cipherTextOffset, cipherTextLength);
var tag = new Span<byte>(output, cipherTextOffset + cipherTextLength, TagSizeInBytes);
var associatedData = new ReadOnlySpan<byte>(input, associatedDataOffset, associatedDataLength);

_aesGcm.Encrypt(_nonce, plainText, cipherText, tag, associatedData);
}

public override void Decrypt(byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset)
{
var plainTextLength = cipherTextLength;
var cipherText = new ReadOnlySpan<byte>(input, cipherTextOffset, cipherTextLength);
var tag = new ReadOnlySpan<byte>(input, cipherTextOffset + cipherTextLength, TagSizeInBytes);
var plainText = new Span<byte>(output, plainTextOffset, plainTextLength);
var associatedData = new ReadOnlySpan<byte>(input, associatedDataOffset, associatedDataLength);

try
{
_aesGcm.Decrypt(_nonce, cipherText, tag, output, associatedData);
}
#if NET8_0_OR_GREATER
catch (AuthenticationTagMismatchException ex)
#else
catch (CryptographicException ex)
#endif
{
throw new SshConnectionException("MAC error", DisconnectReason.MacError, ex);
}
}

protected override void Dispose(bool disposing)
{
base.Dispose(disposing);

if (disposing)
{
_aesGcm.Dispose();
}
}
}
}
}
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
using Org.BouncyCastle.Crypto;
using Org.BouncyCastle.Crypto.Engines;
using Org.BouncyCastle.Crypto.Modes;
using Org.BouncyCastle.Crypto.Parameters;

using Renci.SshNet.Common;
using Renci.SshNet.Messages.Transport;

namespace Renci.SshNet.Security.Cryptography.Ciphers
{
internal partial class AesGcmCipher
{
private sealed class BouncyCastleImpl : Impl
{
private readonly GcmBlockCipher _cipher;
private readonly AeadParameters _parameters;

public BouncyCastleImpl(byte[] key, byte[] nonce)
{
_cipher = new GcmBlockCipher(new AesEngine());
_parameters = new AeadParameters(new KeyParameter(key), TagSizeInBytes * 8, nonce);
}

public override void Encrypt(byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset)
{
_cipher.Init(forEncryption: true, _parameters);
_cipher.ProcessAadBytes(input, associatedDataOffset, associatedDataLength);
var cipherTextLength = _cipher.ProcessBytes(input, plainTextOffset, plainTextLength, output, cipherTextOffset);
_ = _cipher.DoFinal(output, cipherTextOffset + cipherTextLength);
}

public override void Decrypt(byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset)
{
_cipher.Init(forEncryption: false, _parameters);
_cipher.ProcessAadBytes(input, associatedDataOffset, associatedDataLength);
var plainTextLength = _cipher.ProcessBytes(input, cipherTextOffset, cipherTextLength + TagSizeInBytes, output, plainTextOffset);
try
{
_ = _cipher.DoFinal(output, plainTextLength);
}
catch (InvalidCipherTextException ex)
{
throw new SshConnectionException("MAC error", DisconnectReason.MacError, ex);
}
}
}
}
}
80 changes: 55 additions & 25 deletions src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#if NET6_0_OR_GREATER
using System;
using System;
using System.Buffers.Binary;
using System.Diagnostics;
using System.Security.Cryptography;

using Renci.SshNet.Common;

Expand All @@ -12,10 +10,16 @@ namespace Renci.SshNet.Security.Cryptography.Ciphers
/// AES GCM cipher implementation.
/// <see href="https://datatracker.ietf.org/doc/html/rfc5647"/>.
/// </summary>
internal sealed class AesGcmCipher : SymmetricCipher, IDisposable
internal sealed partial class AesGcmCipher : SymmetricCipher, IDisposable
{
private const int PacketLengthFieldLength = 4;
private const int TagSizeInBytes = 16;
private readonly byte[] _iv;
private readonly AesGcm _aesGcm;
#if NET6_0_OR_GREATER
private readonly Impl _impl;
#else
private readonly BouncyCastleImpl _impl;
#endif
Rob-Hague marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets the minimun block size.
Expand All @@ -42,7 +46,7 @@ public override int TagSize
{
get
{
return 16;
return TagSizeInBytes;
}
}

Expand All @@ -56,11 +60,16 @@ public AesGcmCipher(byte[] key, byte[] iv)
{
// SSH AES-GCM requires a 12-octet Initial IV
_iv = iv.Take(12);
#if NET8_0_OR_GREATER
_aesGcm = new AesGcm(key, TagSize);
#else
_aesGcm = new AesGcm(key);
#if NET6_0_OR_GREATER
if (System.Security.Cryptography.AesGcm.IsSupported)
{
_impl = new BclImpl(key, _iv);
}
else
#endif
{
_impl = new BouncyCastleImpl(key, _iv);
}
}

/// <summary>
Expand All @@ -84,15 +93,17 @@ public AesGcmCipher(byte[] key, byte[] iv)
/// </returns>
public override byte[] Encrypt(byte[] input, int offset, int length)
{
var packetLengthField = new ReadOnlySpan<byte>(input, offset, 4);
var plainText = new ReadOnlySpan<byte>(input, offset + 4, length - 4);

var output = new byte[length + TagSize];
packetLengthField.CopyTo(output);
var cipherText = new Span<byte>(output, 4, length - 4);
var tag = new Span<byte>(output, length, TagSize);
Buffer.BlockCopy(input, offset, output, 0, PacketLengthFieldLength);

_aesGcm.Encrypt(nonce: _iv, plainText, cipherText, tag, associatedData: packetLengthField);
_impl.Encrypt(
input,
plainTextOffset: offset + PacketLengthFieldLength,
plainTextLength: length - PacketLengthFieldLength,
associatedDataOffset: offset,
associatedDataLength: PacketLengthFieldLength,
output,
cipherTextOffset: PacketLengthFieldLength);

IncrementCounter();

Expand Down Expand Up @@ -122,14 +133,16 @@ public override byte[] Decrypt(byte[] input, int offset, int length)
{
Debug.Assert(offset == 8, "The offset must be 8");

var packetLengthField = new ReadOnlySpan<byte>(input, 4, 4);
var cipherText = new ReadOnlySpan<byte>(input, offset, length);
var tag = new ReadOnlySpan<byte>(input, offset + length, TagSize);

var output = new byte[length];
var plainText = new Span<byte>(output);

_aesGcm.Decrypt(nonce: _iv, cipherText, tag, plainText, associatedData: packetLengthField);
_impl.Decrypt(
input,
cipherTextOffset: offset,
cipherTextLength: length,
associatedDataOffset: offset - PacketLengthFieldLength,
associatedDataLength: PacketLengthFieldLength,
output,
plainTextOffset: 0);

IncrementCounter();

Expand Down Expand Up @@ -158,7 +171,7 @@ public void Dispose(bool disposing)
{
if (disposing)
{
_aesGcm.Dispose();
_impl.Dispose();
}
}

Expand All @@ -169,6 +182,23 @@ public void Dispose()
Dispose(disposing: true);
GC.SuppressFinalize(this);
}

private abstract class Impl : IDisposable
{
public abstract void Encrypt(byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset);

public abstract void Decrypt(byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset);

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
4 changes: 1 addition & 3 deletions src/Renci.SshNet/Security/KeyExchangeECDH.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,10 @@ public override void Start(Session session, KeyExchangeInitMessage message, bool
_impl = new BclImpl(Curve);
}
else
#endif
{
_impl = new BouncyCastleImpl(CurveParameter);
}
#else
_impl = new BouncyCastleImpl(CurveParameter);
#endif

_clientExchangeValue = _impl.GenerateClientECPoint();

Expand Down
6 changes: 1 addition & 5 deletions src/Renci.SshNet/Session.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,11 +1258,7 @@ private Message ReceiveMessage(Socket socket)
var plainFirstBlock = firstBlock;

// First block is not encrypted in AES GCM mode.
if (_serverCipher is not null
#if NET6_0_OR_GREATER
and not Security.Cryptography.Ciphers.AesGcmCipher
#endif
)
if (_serverCipher is not null and not Security.Cryptography.Ciphers.AesGcmCipher)
{
_serverCipher.SetSequenceNumber(_inboundPacketSequence);

Expand Down
3 changes: 1 addition & 2 deletions test/Renci.SshNet.IntegrationTests/CipherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ public void Aes256Ctr()
DoTest(Cipher.Aes256Ctr);
}

#if NET6_0_OR_GREATER
[TestMethod]
public void Aes128Gcm()
{
Expand All @@ -76,7 +75,7 @@ public void Aes256Gcm()
{
DoTest(Cipher.Aes256Gcm);
}
#endif

[TestMethod]
public void ChaCha20Poly1305()
{
Expand Down