From 7df5108fd9f1afb959bf55314f5043a076799da8 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sun, 21 Jul 2024 21:15:13 +0800 Subject: [PATCH 01/14] [AesGcmCipher] Use BouncyCastle as a fallback if BCL does not support. --- README.md | 4 +- appveyor.yml | 2 + src/Renci.SshNet/ConnectionInfo.cs | 9 +- .../Cryptography/Ciphers/AesGcmCipher.cs | 82 +++++++++++++++---- .../CipherTests.cs | 2 - 5 files changed, 71 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index a6c09ecc8..f89c0c340 100644 --- a/README.md +++ b/README.md @@ -70,8 +70,8 @@ The main types provided by this library are: * aes128-ctr * aes192-ctr * aes256-ctr -* aes128-gcm@openssh.com (.NET 6 and higher) -* aes256-gcm@openssh.com (.NET 6 and higher) +* aes128-gcm@openssh.com +* aes256-gcm@openssh.com * aes128-cbc * aes192-cbc * aes256-cbc diff --git a/appveyor.yml b/appveyor.yml index f8b7db9fa..2a843aa01 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -21,12 +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 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 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~Gcm test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj - matrix: diff --git a/src/Renci.SshNet/ConnectionInfo.cs b/src/Renci.SshNet/ConnectionInfo.cs index 4c2e9b375..0f3bb2bd2 100644 --- a/src/Renci.SshNet/ConnectionInfo.cs +++ b/src/Renci.SshNet/ConnectionInfo.cs @@ -385,13 +385,8 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy 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("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)); 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))); diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs index cd673d04a..09db2b7d9 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs @@ -1,8 +1,13 @@ -#if NET6_0_OR_GREATER -using System; +using System; using System.Buffers.Binary; using System.Diagnostics; +#if NET6_0_OR_GREATER using System.Security.Cryptography; +#endif + +using Org.BouncyCastle.Crypto.Engines; +using Org.BouncyCastle.Crypto.Modes; +using Org.BouncyCastle.Crypto.Parameters; using Renci.SshNet.Common; @@ -12,10 +17,18 @@ namespace Renci.SshNet.Security.Cryptography.Ciphers /// AES GCM cipher implementation. /// . /// - internal sealed class AesGcmCipher : SymmetricCipher, IDisposable + internal sealed class AesGcmCipher +#if NET6_0_OR_GREATER + : SymmetricCipher, IDisposable +#else + : SymmetricCipher +#endif { private readonly byte[] _iv; +#if NET6_0_OR_GREATER private readonly AesGcm _aesGcm; +#endif + private readonly GcmBlockCipher _cipher; /// /// Gets the minimun block size. @@ -56,11 +69,18 @@ public AesGcmCipher(byte[] key, byte[] iv) { // SSH AES-GCM requires a 12-octet Initial IV _iv = iv.Take(12); +#if NET6_0_OR_GREATER + if (AesGcm.IsSupported) + { #if NET8_0_OR_GREATER - _aesGcm = new AesGcm(key, TagSize); + _aesGcm = new AesGcm(key, TagSize); #else - _aesGcm = new AesGcm(key); + _aesGcm = new AesGcm(key); #endif + return; + } +#endif + _cipher = new GcmBlockCipher(new AesEngine()); } /// @@ -85,14 +105,27 @@ public AesGcmCipher(byte[] key, byte[] iv) public override byte[] Encrypt(byte[] input, int offset, int length) { var packetLengthField = new ReadOnlySpan(input, offset, 4); - var plainText = new ReadOnlySpan(input, offset + 4, length - 4); - var output = new byte[length + TagSize]; packetLengthField.CopyTo(output); - var cipherText = new Span(output, 4, length - 4); - var tag = new Span(output, length, TagSize); +#if NET6_0_OR_GREATER + if (AesGcm.IsSupported) + { + var plainText = new ReadOnlySpan(input, offset + 4, length - 4); + var cipherText = new Span(output, 4, length - 4); + var tag = new Span(output, length, TagSize); - _aesGcm.Encrypt(nonce: _iv, plainText, cipherText, tag, associatedData: packetLengthField); + _aesGcm.Encrypt(nonce: _iv, plainText, cipherText, tag, associatedData: packetLengthField); + + IncrementCounter(); + + return output; + } +#endif + var parameters = new AeadParameters(new KeyParameter(Key), TagSize * 8, nonce: _iv, associatedText: packetLengthField.ToArray()); + _cipher.Init(forEncryption: true, parameters); + + var len = _cipher.ProcessBytes(input, offset + 4, length - 4, output, 4); + _cipher.DoFinal(output, len + 4); IncrementCounter(); @@ -123,13 +156,27 @@ public override byte[] Decrypt(byte[] input, int offset, int length) Debug.Assert(offset == 8, "The offset must be 8"); var packetLengthField = new ReadOnlySpan(input, 4, 4); - var cipherText = new ReadOnlySpan(input, offset, length); - var tag = new ReadOnlySpan(input, offset + length, TagSize); - var output = new byte[length]; - var plainText = new Span(output); +#if NET6_0_OR_GREATER + if (AesGcm.IsSupported) + { + var cipherText = new ReadOnlySpan(input, offset, length); + var tag = new ReadOnlySpan(input, offset + length, TagSize); + + var plainText = new Span(output); + + _aesGcm.Decrypt(nonce: _iv, cipherText, tag, plainText, associatedData: packetLengthField); - _aesGcm.Decrypt(nonce: _iv, cipherText, tag, plainText, associatedData: packetLengthField); + IncrementCounter(); + + return output; + } +#endif + var parameters = new AeadParameters(new KeyParameter(Key), TagSize * 8, nonce: _iv, associatedText: packetLengthField.ToArray()); + _cipher.Init(forEncryption: false, parameters); + + var len = _cipher.ProcessBytes(input, offset, length + TagSize, output, 0); + _cipher.DoFinal(output, len); IncrementCounter(); @@ -150,6 +197,7 @@ private void IncrementCounter() BinaryPrimitives.WriteUInt64BigEndian(invocationCounter, count + 1); } +#if NET6_0_OR_GREATER /// /// Dispose the instance. /// @@ -158,7 +206,7 @@ public void Dispose(bool disposing) { if (disposing) { - _aesGcm.Dispose(); + _aesGcm?.Dispose(); } } @@ -169,6 +217,6 @@ public void Dispose() Dispose(disposing: true); GC.SuppressFinalize(this); } +#endif } } -#endif diff --git a/test/Renci.SshNet.IntegrationTests/CipherTests.cs b/test/Renci.SshNet.IntegrationTests/CipherTests.cs index b2b3fd4c1..2332699d0 100644 --- a/test/Renci.SshNet.IntegrationTests/CipherTests.cs +++ b/test/Renci.SshNet.IntegrationTests/CipherTests.cs @@ -64,7 +64,6 @@ public void Aes256Ctr() DoTest(Cipher.Aes256Ctr); } -#if NET6_0_OR_GREATER [TestMethod] public void Aes128Gcm() { @@ -76,7 +75,6 @@ public void Aes256Gcm() { DoTest(Cipher.Aes256Gcm); } -#endif private void DoTest(Cipher cipher) { _remoteSshdConfig.ClearCiphers() From dfe5b224a1594e44672b07551568920794288836 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Mon, 22 Jul 2024 13:30:00 +0800 Subject: [PATCH 02/14] Switch back to collection initializer --- src/Renci.SshNet/ConnectionInfo.cs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Renci.SshNet/ConnectionInfo.cs b/src/Renci.SshNet/ConnectionInfo.cs index 0f3bb2bd2..704278a91 100644 --- a/src/Renci.SshNet/ConnectionInfo.cs +++ b/src/Renci.SshNet/ConnectionInfo.cs @@ -381,16 +381,18 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy { "diffie-hellman-group1-sha1", () => new KeyExchangeDiffieHellmanGroup1Sha1() }, }; - Encryptions = new Dictionary(); - 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))); - 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)); - 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 + { + { "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) }, + { "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 { From f3121f4d888afc11557630ab2aebbf5f414ddfa1 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sat, 27 Jul 2024 20:40:42 +0800 Subject: [PATCH 03/14] Remove conditional compilation --- src/Renci.SshNet/Session.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index 7ff95e27b..0dbd03738 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -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); From 1152c9da32862ebdc41549eefbdd56b40833dc49 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sat, 27 Jul 2024 23:26:46 +0800 Subject: [PATCH 04/14] Throw SshConnectionException with Reason MacError when authentication tag mismatch --- .../Cryptography/Ciphers/AesGcmCipher.cs | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs index 09db2b7d9..6a00f9f47 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs @@ -4,12 +4,13 @@ #if NET6_0_OR_GREATER using System.Security.Cryptography; #endif - +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 { @@ -165,7 +166,18 @@ public override byte[] Decrypt(byte[] input, int offset, int length) var plainText = new Span(output); - _aesGcm.Decrypt(nonce: _iv, cipherText, tag, plainText, associatedData: packetLengthField); + try + { + _aesGcm.Decrypt(nonce: _iv, cipherText, tag, plainText, associatedData: packetLengthField); + } +#if NET8_0_OR_GREATER + catch (AuthenticationTagMismatchException) +#else + catch (CryptographicException ex) when (ex.Message == "The computed authentication tag did not match the input authentication tag.") +#endif + { + throw new SshConnectionException("MAC error", DisconnectReason.MacError); + } IncrementCounter(); @@ -176,7 +188,14 @@ public override byte[] Decrypt(byte[] input, int offset, int length) _cipher.Init(forEncryption: false, parameters); var len = _cipher.ProcessBytes(input, offset, length + TagSize, output, 0); - _cipher.DoFinal(output, len); + try + { + _cipher.DoFinal(output, len); + } + catch (InvalidCipherTextException) + { + throw new SshConnectionException("MAC error", DisconnectReason.MacError); + } IncrementCounter(); From 75f9bce49c0275a970e287b699938c9264721cc6 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 30 Jul 2024 19:19:27 +0800 Subject: [PATCH 05/14] Separate BCL and BouncyCastle implementation --- .../Ciphers/AesGcmCipher.BclImpl.cs | 70 ++++++++++ .../Ciphers/AesGcmCipher.BouncyCastleImpl.cs | 52 ++++++++ .../Cryptography/Ciphers/AesGcmCipher.cs | 120 +++++------------- 3 files changed, 157 insertions(+), 85 deletions(-) create mode 100644 src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs create mode 100644 src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs new file mode 100644 index 000000000..0e160541d --- /dev/null +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs @@ -0,0 +1,70 @@ +#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 int _tagSize; + + public BclImpl(byte[] key, int tagSize) + { +#if NET8_0_OR_GREATER + _aesGcm = new AesGcm(key, tagSize); +#else + _aesGcm = new AesGcm(key); +#endif + _tagSize = tagSize; + } + + public override void Encrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) + { + var plainText = new Span(input, offset, length); + var cipherText = new Span(output, outOffset, length); + var tag = new Span(output, outOffset + length, _tagSize); + var associatedData = new Span(input, aadOffset, aadLength); + + _aesGcm.Encrypt(nonce, plainText, cipherText, tag, associatedData); + } + + public override void Decrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) + { + var cipherText = new ReadOnlySpan(input, offset, length); + var tag = new ReadOnlySpan(input, offset + length, _tagSize); + var plainText = new Span(output, outOffset, length); + var associatedData = new ReadOnlySpan(input, aadOffset, aadLength); + + try + { + _aesGcm.Decrypt(nonce, cipherText, tag, output, associatedData); + } +#if NET8_0_OR_GREATER + catch (AuthenticationTagMismatchException) +#else + catch (CryptographicException ex) when (ex.Message == "The computed authentication tag did not match the input authentication tag.") +#endif + { + throw new SshConnectionException("MAC error", DisconnectReason.MacError); + } + } + + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + + if (disposing) + { + _aesGcm.Dispose(); + } + } + } + } +} +#endif diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs new file mode 100644 index 000000000..5dea343f0 --- /dev/null +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs @@ -0,0 +1,52 @@ +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 byte[] _key; + private readonly int _tagSize; + private readonly GcmBlockCipher _cipher; + + public BouncyCastleImpl(byte[] key, int tagSize) + { + _key = key; + _tagSize = tagSize; + _cipher = new GcmBlockCipher(new AesEngine()); + } + + public override void Encrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) + { + var parameters = new AeadParameters(new KeyParameter(_key), _tagSize * 8, nonce, input.Take(aadOffset, aadLength)); + _cipher.Init(forEncryption: true, parameters); + + var len = _cipher.ProcessBytes(input, offset, length, output, outOffset); + _ = _cipher.DoFinal(output, outOffset + len); + } + + public override void Decrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) + { + var parameters = new AeadParameters(new KeyParameter(_key), _tagSize * 8, nonce, input.Take(aadOffset, aadLength)); + _cipher.Init(forEncryption: false, parameters); + + var len = _cipher.ProcessBytes(input, offset, length + _tagSize, output, outOffset); + try + { + _ = _cipher.DoFinal(output, len); + } + catch (InvalidCipherTextException) + { + throw new SshConnectionException("MAC error", DisconnectReason.MacError); + } + } + } + } +} diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs index 6a00f9f47..b3435df1f 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs @@ -1,16 +1,8 @@ using System; using System.Buffers.Binary; using System.Diagnostics; -#if NET6_0_OR_GREATER -using System.Security.Cryptography; -#endif -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 { @@ -18,18 +10,15 @@ namespace Renci.SshNet.Security.Cryptography.Ciphers /// AES GCM cipher implementation. /// . /// - internal sealed class AesGcmCipher -#if NET6_0_OR_GREATER - : SymmetricCipher, IDisposable -#else - : SymmetricCipher -#endif + internal sealed partial class AesGcmCipher : SymmetricCipher, IDisposable { private readonly byte[] _iv; #if NET6_0_OR_GREATER - private readonly AesGcm _aesGcm; + private readonly Impl _impl; + +#else + private readonly BouncyCastleImpl _impl; #endif - private readonly GcmBlockCipher _cipher; /// /// Gets the minimun block size. @@ -71,17 +60,17 @@ public AesGcmCipher(byte[] key, byte[] iv) // SSH AES-GCM requires a 12-octet Initial IV _iv = iv.Take(12); #if NET6_0_OR_GREATER - if (AesGcm.IsSupported) + if (System.Security.Cryptography.AesGcm.IsSupported) { -#if NET8_0_OR_GREATER - _aesGcm = new AesGcm(key, TagSize); -#else - _aesGcm = new AesGcm(key); -#endif - return; + _impl = new BclImpl(key, TagSize); } + else + { + _impl = new BouncyCastleImpl(key, TagSize); + } +#else + _impl = new BouncyCastleImpl(key, TagSize); #endif - _cipher = new GcmBlockCipher(new AesEngine()); } /// @@ -105,28 +94,10 @@ public AesGcmCipher(byte[] key, byte[] iv) /// public override byte[] Encrypt(byte[] input, int offset, int length) { - var packetLengthField = new ReadOnlySpan(input, offset, 4); var output = new byte[length + TagSize]; - packetLengthField.CopyTo(output); -#if NET6_0_OR_GREATER - if (AesGcm.IsSupported) - { - var plainText = new ReadOnlySpan(input, offset + 4, length - 4); - var cipherText = new Span(output, 4, length - 4); - var tag = new Span(output, length, TagSize); - - _aesGcm.Encrypt(nonce: _iv, plainText, cipherText, tag, associatedData: packetLengthField); + Buffer.BlockCopy(input, offset, output, 0, 4); - IncrementCounter(); - - return output; - } -#endif - var parameters = new AeadParameters(new KeyParameter(Key), TagSize * 8, nonce: _iv, associatedText: packetLengthField.ToArray()); - _cipher.Init(forEncryption: true, parameters); - - var len = _cipher.ProcessBytes(input, offset + 4, length - 4, output, 4); - _cipher.DoFinal(output, len + 4); + _impl.Encrypt(_iv, input, offset + 4, length - 4, offset, 4, output, 4); IncrementCounter(); @@ -156,46 +127,9 @@ public override byte[] Decrypt(byte[] input, int offset, int length) { Debug.Assert(offset == 8, "The offset must be 8"); - var packetLengthField = new ReadOnlySpan(input, 4, 4); var output = new byte[length]; -#if NET6_0_OR_GREATER - if (AesGcm.IsSupported) - { - var cipherText = new ReadOnlySpan(input, offset, length); - var tag = new ReadOnlySpan(input, offset + length, TagSize); - var plainText = new Span(output); - - try - { - _aesGcm.Decrypt(nonce: _iv, cipherText, tag, plainText, associatedData: packetLengthField); - } -#if NET8_0_OR_GREATER - catch (AuthenticationTagMismatchException) -#else - catch (CryptographicException ex) when (ex.Message == "The computed authentication tag did not match the input authentication tag.") -#endif - { - throw new SshConnectionException("MAC error", DisconnectReason.MacError); - } - - IncrementCounter(); - - return output; - } -#endif - var parameters = new AeadParameters(new KeyParameter(Key), TagSize * 8, nonce: _iv, associatedText: packetLengthField.ToArray()); - _cipher.Init(forEncryption: false, parameters); - - var len = _cipher.ProcessBytes(input, offset, length + TagSize, output, 0); - try - { - _cipher.DoFinal(output, len); - } - catch (InvalidCipherTextException) - { - throw new SshConnectionException("MAC error", DisconnectReason.MacError); - } + _impl.Decrypt(_iv, input, offset, length, offset - 4, 4, output, 0); IncrementCounter(); @@ -216,7 +150,6 @@ private void IncrementCounter() BinaryPrimitives.WriteUInt64BigEndian(invocationCounter, count + 1); } -#if NET6_0_OR_GREATER /// /// Dispose the instance. /// @@ -225,7 +158,7 @@ public void Dispose(bool disposing) { if (disposing) { - _aesGcm?.Dispose(); + _impl.Dispose(); } } @@ -236,6 +169,23 @@ public void Dispose() Dispose(disposing: true); GC.SuppressFinalize(this); } -#endif + + private abstract class Impl : IDisposable + { + public abstract void Encrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset); + + public abstract void Decrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset); + + 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); + } + } } } From 8bdf511f9ad97ac112aeaaedfa894c928d31bbfb Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 30 Jul 2024 22:38:31 +0800 Subject: [PATCH 06/14] Update AesGcmCipher.BclImpl.cs --- .../Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs index 0e160541d..b5bc8409e 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs @@ -1,4 +1,4 @@ -#if NET6_0_OR_GREATER +#if NET6_0_OR_GREATER using System; using System.Security.Cryptography; @@ -26,10 +26,10 @@ public BclImpl(byte[] key, int tagSize) public override void Encrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) { - var plainText = new Span(input, offset, length); + var plainText = new ReadOnlySpan(input, offset, length); var cipherText = new Span(output, outOffset, length); var tag = new Span(output, outOffset + length, _tagSize); - var associatedData = new Span(input, aadOffset, aadLength); + var associatedData = new ReadOnlySpan(input, aadOffset, aadLength); _aesGcm.Encrypt(nonce, plainText, cipherText, tag, associatedData); } From e36823b3cdbbb01b722645ba0d4ed8fcc08d6737 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Mon, 5 Aug 2024 00:35:25 +0800 Subject: [PATCH 07/14] Naming enhancement --- .../Ciphers/AesGcmCipher.BclImpl.cs | 22 +++++++------- .../Ciphers/AesGcmCipher.BouncyCastleImpl.cs | 16 +++++----- .../Cryptography/Ciphers/AesGcmCipher.cs | 29 +++++++++++++++---- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs index b5bc8409e..81545b906 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs @@ -24,22 +24,24 @@ public BclImpl(byte[] key, int tagSize) _tagSize = tagSize; } - public override void Encrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) + public override void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset) { - var plainText = new ReadOnlySpan(input, offset, length); - var cipherText = new Span(output, outOffset, length); - var tag = new Span(output, outOffset + length, _tagSize); - var associatedData = new ReadOnlySpan(input, aadOffset, aadLength); + var cipherTextLength = plainTextLength; + var plainText = new ReadOnlySpan(input, plainTextOffset, plainTextLength); + var cipherText = new Span(output, cipherTextOffset, cipherTextLength); + var tag = new Span(output, cipherTextOffset + cipherTextLength, _tagSize); + var associatedData = new ReadOnlySpan(input, associatedDataOffset, associatedDataLength); _aesGcm.Encrypt(nonce, plainText, cipherText, tag, associatedData); } - public override void Decrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) + public override void Decrypt(byte[] nonce, byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset) { - var cipherText = new ReadOnlySpan(input, offset, length); - var tag = new ReadOnlySpan(input, offset + length, _tagSize); - var plainText = new Span(output, outOffset, length); - var associatedData = new ReadOnlySpan(input, aadOffset, aadLength); + var plainTextLength = cipherTextLength; + var cipherText = new ReadOnlySpan(input, cipherTextOffset, cipherTextLength); + var tag = new ReadOnlySpan(input, cipherTextOffset + cipherTextLength, _tagSize); + var plainText = new Span(output, plainTextOffset, plainTextLength); + var associatedData = new ReadOnlySpan(input, associatedDataOffset, associatedDataLength); try { diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs index 5dea343f0..129030df2 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs @@ -23,24 +23,24 @@ public BouncyCastleImpl(byte[] key, int tagSize) _cipher = new GcmBlockCipher(new AesEngine()); } - public override void Encrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) + public override void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset) { - var parameters = new AeadParameters(new KeyParameter(_key), _tagSize * 8, nonce, input.Take(aadOffset, aadLength)); + var parameters = new AeadParameters(new KeyParameter(_key), _tagSize * 8, nonce, input.Take(associatedDataOffset, associatedDataLength)); _cipher.Init(forEncryption: true, parameters); - var len = _cipher.ProcessBytes(input, offset, length, output, outOffset); - _ = _cipher.DoFinal(output, outOffset + len); + var cipherTextLength = _cipher.ProcessBytes(input, plainTextOffset, plainTextLength, output, cipherTextOffset); + _ = _cipher.DoFinal(output, cipherTextOffset + cipherTextLength); } - public override void Decrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset) + public override void Decrypt(byte[] nonce, byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset) { - var parameters = new AeadParameters(new KeyParameter(_key), _tagSize * 8, nonce, input.Take(aadOffset, aadLength)); + var parameters = new AeadParameters(new KeyParameter(_key), _tagSize * 8, nonce, input.Take(associatedDataOffset, associatedDataLength)); _cipher.Init(forEncryption: false, parameters); - var len = _cipher.ProcessBytes(input, offset, length + _tagSize, output, outOffset); + var plainTextLength = _cipher.ProcessBytes(input, cipherTextOffset, cipherTextLength + _tagSize, output, plainTextOffset); try { - _ = _cipher.DoFinal(output, len); + _ = _cipher.DoFinal(output, plainTextLength); } catch (InvalidCipherTextException) { diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs index b3435df1f..4cc90d01e 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs @@ -12,6 +12,7 @@ namespace Renci.SshNet.Security.Cryptography.Ciphers /// internal sealed partial class AesGcmCipher : SymmetricCipher, IDisposable { + private const int PacketLengthFieldLength = 4; private readonly byte[] _iv; #if NET6_0_OR_GREATER private readonly Impl _impl; @@ -95,9 +96,17 @@ public AesGcmCipher(byte[] key, byte[] iv) public override byte[] Encrypt(byte[] input, int offset, int length) { var output = new byte[length + TagSize]; - Buffer.BlockCopy(input, offset, output, 0, 4); - - _impl.Encrypt(_iv, input, offset + 4, length - 4, offset, 4, output, 4); + Buffer.BlockCopy(input, offset, output, 0, PacketLengthFieldLength); + + _impl.Encrypt( + nonce: _iv, + input, + plainTextOffset: offset + PacketLengthFieldLength, + plainTextLength: length - PacketLengthFieldLength, + associatedDataOffset: offset, + associatedDataLength: PacketLengthFieldLength, + output, + cipherTextOffset: PacketLengthFieldLength); IncrementCounter(); @@ -129,7 +138,15 @@ public override byte[] Decrypt(byte[] input, int offset, int length) var output = new byte[length]; - _impl.Decrypt(_iv, input, offset, length, offset - 4, 4, output, 0); + _impl.Decrypt( + nonce: _iv, + input, + cipherTextOffset: offset, + cipherTextLength: length, + associatedDataOffset: offset - PacketLengthFieldLength, + associatedDataLength: PacketLengthFieldLength, + output, + plainTextOffset: 0); IncrementCounter(); @@ -172,9 +189,9 @@ public void Dispose() private abstract class Impl : IDisposable { - public abstract void Encrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset); + public abstract void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset); - public abstract void Decrypt(byte[] nonce, byte[] input, int offset, int length, int aadOffset, int aadLength, byte[] output, int outOffset); + public abstract void Decrypt(byte[] nonce, byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset); protected virtual void Dispose(bool disposing) { From b12e3b630a89a48737969a56b51b7d12903aa814 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Mon, 5 Aug 2024 23:22:21 +0800 Subject: [PATCH 08/14] Remove empty line --- src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs index 4cc90d01e..53ae0c1a7 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs @@ -16,7 +16,6 @@ internal sealed partial class AesGcmCipher : SymmetricCipher, IDisposable private readonly byte[] _iv; #if NET6_0_OR_GREATER private readonly Impl _impl; - #else private readonly BouncyCastleImpl _impl; #endif From 1751ed77ce91807e028e90891df19250caaa30fd Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Fri, 9 Aug 2024 20:31:02 +0800 Subject: [PATCH 09/14] Disable S1199. See https://github.com/sshnet/SSH.NET/pull/1371#discussion_r1704293356 --- .editorconfig | 4 ++++ .../Security/Cryptography/Ciphers/AesGcmCipher.cs | 4 +--- src/Renci.SshNet/Security/KeyExchangeECDH.cs | 4 +--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.editorconfig b/.editorconfig index 05a2600d0..20029dbf9 100644 --- a/.editorconfig +++ b/.editorconfig @@ -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 # diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs index 53ae0c1a7..9d4279b9c 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs @@ -65,12 +65,10 @@ public AesGcmCipher(byte[] key, byte[] iv) _impl = new BclImpl(key, TagSize); } else +#endif { _impl = new BouncyCastleImpl(key, TagSize); } -#else - _impl = new BouncyCastleImpl(key, TagSize); -#endif } /// diff --git a/src/Renci.SshNet/Security/KeyExchangeECDH.cs b/src/Renci.SshNet/Security/KeyExchangeECDH.cs index e5ec28c5d..b5e887f60 100644 --- a/src/Renci.SshNet/Security/KeyExchangeECDH.cs +++ b/src/Renci.SshNet/Security/KeyExchangeECDH.cs @@ -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(); From 6f47e0317b27ae78a863c2c9ebbeb49446ccbb06 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sat, 10 Aug 2024 20:55:13 +0800 Subject: [PATCH 10/14] Set InnerException when MAC error. Remove Message check. --- .../Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs | 6 +++--- .../Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs index 81545b906..0e80de736 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs @@ -48,12 +48,12 @@ public override void Decrypt(byte[] nonce, byte[] input, int cipherTextOffset, i _aesGcm.Decrypt(nonce, cipherText, tag, output, associatedData); } #if NET8_0_OR_GREATER - catch (AuthenticationTagMismatchException) + catch (AuthenticationTagMismatchException ex) #else - catch (CryptographicException ex) when (ex.Message == "The computed authentication tag did not match the input authentication tag.") + catch (CryptographicException ex) #endif { - throw new SshConnectionException("MAC error", DisconnectReason.MacError); + throw new SshConnectionException("MAC error", DisconnectReason.MacError, ex); } } diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs index 129030df2..f3a64e0b3 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs @@ -42,9 +42,9 @@ public override void Decrypt(byte[] nonce, byte[] input, int cipherTextOffset, i { _ = _cipher.DoFinal(output, plainTextLength); } - catch (InvalidCipherTextException) + catch (InvalidCipherTextException ex) { - throw new SshConnectionException("MAC error", DisconnectReason.MacError); + throw new SshConnectionException("MAC error", DisconnectReason.MacError, ex); } } } From 432d6b6b0543d0b818520407dd096741eeabc999 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sat, 10 Aug 2024 21:05:12 +0800 Subject: [PATCH 11/14] Store KeyParameter as private field --- .../Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs index f3a64e0b3..fcf1c6ec5 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs @@ -12,20 +12,20 @@ internal partial class AesGcmCipher { private sealed class BouncyCastleImpl : Impl { - private readonly byte[] _key; + private readonly KeyParameter _keyParameter; private readonly int _tagSize; private readonly GcmBlockCipher _cipher; public BouncyCastleImpl(byte[] key, int tagSize) { - _key = key; + _keyParameter = new KeyParameter(key); _tagSize = tagSize; _cipher = new GcmBlockCipher(new AesEngine()); } public override void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset) { - var parameters = new AeadParameters(new KeyParameter(_key), _tagSize * 8, nonce, input.Take(associatedDataOffset, associatedDataLength)); + var parameters = new AeadParameters(_keyParameter, _tagSize * 8, nonce, input.Take(associatedDataOffset, associatedDataLength)); _cipher.Init(forEncryption: true, parameters); var cipherTextLength = _cipher.ProcessBytes(input, plainTextOffset, plainTextLength, output, cipherTextOffset); @@ -34,7 +34,7 @@ public override void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, in public override void Decrypt(byte[] nonce, byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset) { - var parameters = new AeadParameters(new KeyParameter(_key), _tagSize * 8, nonce, input.Take(associatedDataOffset, associatedDataLength)); + var parameters = new AeadParameters(_keyParameter, _tagSize * 8, nonce, input.Take(associatedDataOffset, associatedDataLength)); _cipher.Init(forEncryption: false, parameters); var plainTextLength = _cipher.ProcessBytes(input, cipherTextOffset, cipherTextLength + _tagSize, output, plainTextOffset); From 1e9a0b4ab9e2b9301afd080cedeaf101a4829403 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sat, 10 Aug 2024 22:54:29 +0800 Subject: [PATCH 12/14] Use GcmCipher.ProcessAadBytes to avoid the copy of associated data --- .../Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs index fcf1c6ec5..d50420701 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs @@ -25,18 +25,18 @@ public BouncyCastleImpl(byte[] key, int tagSize) public override void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset) { - var parameters = new AeadParameters(_keyParameter, _tagSize * 8, nonce, input.Take(associatedDataOffset, associatedDataLength)); + var parameters = new AeadParameters(_keyParameter, _tagSize * 8, nonce); _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[] nonce, byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset) { - var parameters = new AeadParameters(_keyParameter, _tagSize * 8, nonce, input.Take(associatedDataOffset, associatedDataLength)); + var parameters = new AeadParameters(_keyParameter, _tagSize * 8, nonce); _cipher.Init(forEncryption: false, parameters); - + _cipher.ProcessAadBytes(input, associatedDataOffset, associatedDataLength); var plainTextLength = _cipher.ProcessBytes(input, cipherTextOffset, cipherTextLength + _tagSize, output, plainTextOffset); try { From fb683037570402401505b28fd7f42d3a3aa7b407 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sun, 11 Aug 2024 00:45:22 +0800 Subject: [PATCH 13/14] Move nonce to constructor to avoid creating AeadParameters each packet --- .../Ciphers/AesGcmCipher.BclImpl.cs | 22 ++++++++++--------- .../Ciphers/AesGcmCipher.BouncyCastleImpl.cs | 22 +++++++++---------- .../Cryptography/Ciphers/AesGcmCipher.cs | 10 ++++----- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs index 0e80de736..618530cac 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs @@ -12,40 +12,42 @@ internal partial class AesGcmCipher private sealed class BclImpl : Impl { private readonly AesGcm _aesGcm; - private readonly int _tagSize; + private readonly int _tagSizeInBytes; + private readonly byte[] _nonce; - public BclImpl(byte[] key, int tagSize) + public BclImpl(byte[] key, int tagSizeInBytes, byte[] nonce) { #if NET8_0_OR_GREATER - _aesGcm = new AesGcm(key, tagSize); + _aesGcm = new AesGcm(key, tagSizeInBytes); #else _aesGcm = new AesGcm(key); #endif - _tagSize = tagSize; + _nonce = nonce; + _tagSizeInBytes = tagSizeInBytes; } - public override void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset) + 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(input, plainTextOffset, plainTextLength); var cipherText = new Span(output, cipherTextOffset, cipherTextLength); - var tag = new Span(output, cipherTextOffset + cipherTextLength, _tagSize); + var tag = new Span(output, cipherTextOffset + cipherTextLength, _tagSizeInBytes); var associatedData = new ReadOnlySpan(input, associatedDataOffset, associatedDataLength); - _aesGcm.Encrypt(nonce, plainText, cipherText, tag, associatedData); + _aesGcm.Encrypt(_nonce, plainText, cipherText, tag, associatedData); } - public override void Decrypt(byte[] nonce, byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset) + 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(input, cipherTextOffset, cipherTextLength); - var tag = new ReadOnlySpan(input, cipherTextOffset + cipherTextLength, _tagSize); + var tag = new ReadOnlySpan(input, cipherTextOffset + cipherTextLength, _tagSizeInBytes); var plainText = new Span(output, plainTextOffset, plainTextLength); var associatedData = new ReadOnlySpan(input, associatedDataOffset, associatedDataLength); try { - _aesGcm.Decrypt(nonce, cipherText, tag, output, associatedData); + _aesGcm.Decrypt(_nonce, cipherText, tag, output, associatedData); } #if NET8_0_OR_GREATER catch (AuthenticationTagMismatchException ex) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs index d50420701..8bcb214ae 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs @@ -12,32 +12,30 @@ internal partial class AesGcmCipher { private sealed class BouncyCastleImpl : Impl { - private readonly KeyParameter _keyParameter; - private readonly int _tagSize; private readonly GcmBlockCipher _cipher; + private readonly AeadParameters _parameters; + private readonly int _tagSizeInBytes; - public BouncyCastleImpl(byte[] key, int tagSize) + public BouncyCastleImpl(byte[] key, int tagSizeInBytes, byte[] nonce) { - _keyParameter = new KeyParameter(key); - _tagSize = tagSize; _cipher = new GcmBlockCipher(new AesEngine()); + _parameters = new AeadParameters(new KeyParameter(key), tagSizeInBytes * 8, nonce); + _tagSizeInBytes = tagSizeInBytes; } - public override void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset) + public override void Encrypt(byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset) { - var parameters = new AeadParameters(_keyParameter, _tagSize * 8, nonce); - _cipher.Init(forEncryption: true, parameters); + _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[] nonce, byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset) + public override void Decrypt(byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset) { - var parameters = new AeadParameters(_keyParameter, _tagSize * 8, nonce); - _cipher.Init(forEncryption: false, parameters); + _cipher.Init(forEncryption: false, _parameters); _cipher.ProcessAadBytes(input, associatedDataOffset, associatedDataLength); - var plainTextLength = _cipher.ProcessBytes(input, cipherTextOffset, cipherTextLength + _tagSize, output, plainTextOffset); + var plainTextLength = _cipher.ProcessBytes(input, cipherTextOffset, cipherTextLength + _tagSizeInBytes, output, plainTextOffset); try { _ = _cipher.DoFinal(output, plainTextLength); diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs index 9d4279b9c..7ab2f053c 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs @@ -62,12 +62,12 @@ public AesGcmCipher(byte[] key, byte[] iv) #if NET6_0_OR_GREATER if (System.Security.Cryptography.AesGcm.IsSupported) { - _impl = new BclImpl(key, TagSize); + _impl = new BclImpl(key, TagSize, _iv); } else #endif { - _impl = new BouncyCastleImpl(key, TagSize); + _impl = new BouncyCastleImpl(key, TagSize, _iv); } } @@ -96,7 +96,6 @@ public override byte[] Encrypt(byte[] input, int offset, int length) Buffer.BlockCopy(input, offset, output, 0, PacketLengthFieldLength); _impl.Encrypt( - nonce: _iv, input, plainTextOffset: offset + PacketLengthFieldLength, plainTextLength: length - PacketLengthFieldLength, @@ -136,7 +135,6 @@ public override byte[] Decrypt(byte[] input, int offset, int length) var output = new byte[length]; _impl.Decrypt( - nonce: _iv, input, cipherTextOffset: offset, cipherTextLength: length, @@ -186,9 +184,9 @@ public void Dispose() private abstract class Impl : IDisposable { - public abstract void Encrypt(byte[] nonce, byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset); + public abstract void Encrypt(byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset); - public abstract void Decrypt(byte[] nonce, byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset); + public abstract void Decrypt(byte[] input, int cipherTextOffset, int cipherTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int plainTextOffset); protected virtual void Dispose(bool disposing) { From b4dc0134c15af42b7dc4da0bd7f5924c6969607b Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Sun, 11 Aug 2024 01:19:41 +0800 Subject: [PATCH 14/14] Use const int for tag size --- .../Cryptography/Ciphers/AesGcmCipher.BclImpl.cs | 10 ++++------ .../Ciphers/AesGcmCipher.BouncyCastleImpl.cs | 8 +++----- .../Security/Cryptography/Ciphers/AesGcmCipher.cs | 7 ++++--- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs index 618530cac..4ed11cf51 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BclImpl.cs @@ -12,18 +12,16 @@ internal partial class AesGcmCipher private sealed class BclImpl : Impl { private readonly AesGcm _aesGcm; - private readonly int _tagSizeInBytes; private readonly byte[] _nonce; - public BclImpl(byte[] key, int tagSizeInBytes, byte[] nonce) + public BclImpl(byte[] key, byte[] nonce) { #if NET8_0_OR_GREATER - _aesGcm = new AesGcm(key, tagSizeInBytes); + _aesGcm = new AesGcm(key, TagSizeInBytes); #else _aesGcm = new AesGcm(key); #endif _nonce = nonce; - _tagSizeInBytes = tagSizeInBytes; } public override void Encrypt(byte[] input, int plainTextOffset, int plainTextLength, int associatedDataOffset, int associatedDataLength, byte[] output, int cipherTextOffset) @@ -31,7 +29,7 @@ public override void Encrypt(byte[] input, int plainTextOffset, int plainTextLen var cipherTextLength = plainTextLength; var plainText = new ReadOnlySpan(input, plainTextOffset, plainTextLength); var cipherText = new Span(output, cipherTextOffset, cipherTextLength); - var tag = new Span(output, cipherTextOffset + cipherTextLength, _tagSizeInBytes); + var tag = new Span(output, cipherTextOffset + cipherTextLength, TagSizeInBytes); var associatedData = new ReadOnlySpan(input, associatedDataOffset, associatedDataLength); _aesGcm.Encrypt(_nonce, plainText, cipherText, tag, associatedData); @@ -41,7 +39,7 @@ public override void Decrypt(byte[] input, int cipherTextOffset, int cipherTextL { var plainTextLength = cipherTextLength; var cipherText = new ReadOnlySpan(input, cipherTextOffset, cipherTextLength); - var tag = new ReadOnlySpan(input, cipherTextOffset + cipherTextLength, _tagSizeInBytes); + var tag = new ReadOnlySpan(input, cipherTextOffset + cipherTextLength, TagSizeInBytes); var plainText = new Span(output, plainTextOffset, plainTextLength); var associatedData = new ReadOnlySpan(input, associatedDataOffset, associatedDataLength); diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs index 8bcb214ae..4c271211f 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.BouncyCastleImpl.cs @@ -14,13 +14,11 @@ private sealed class BouncyCastleImpl : Impl { private readonly GcmBlockCipher _cipher; private readonly AeadParameters _parameters; - private readonly int _tagSizeInBytes; - public BouncyCastleImpl(byte[] key, int tagSizeInBytes, byte[] nonce) + public BouncyCastleImpl(byte[] key, byte[] nonce) { _cipher = new GcmBlockCipher(new AesEngine()); - _parameters = new AeadParameters(new KeyParameter(key), tagSizeInBytes * 8, nonce); - _tagSizeInBytes = tagSizeInBytes; + _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) @@ -35,7 +33,7 @@ public override void Decrypt(byte[] input, int cipherTextOffset, int cipherTextL { _cipher.Init(forEncryption: false, _parameters); _cipher.ProcessAadBytes(input, associatedDataOffset, associatedDataLength); - var plainTextLength = _cipher.ProcessBytes(input, cipherTextOffset, cipherTextLength + _tagSizeInBytes, output, plainTextOffset); + var plainTextLength = _cipher.ProcessBytes(input, cipherTextOffset, cipherTextLength + TagSizeInBytes, output, plainTextOffset); try { _ = _cipher.DoFinal(output, plainTextLength); diff --git a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs index 7ab2f053c..fb0926c5c 100644 --- a/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs +++ b/src/Renci.SshNet/Security/Cryptography/Ciphers/AesGcmCipher.cs @@ -13,6 +13,7 @@ namespace Renci.SshNet.Security.Cryptography.Ciphers internal sealed partial class AesGcmCipher : SymmetricCipher, IDisposable { private const int PacketLengthFieldLength = 4; + private const int TagSizeInBytes = 16; private readonly byte[] _iv; #if NET6_0_OR_GREATER private readonly Impl _impl; @@ -45,7 +46,7 @@ public override int TagSize { get { - return 16; + return TagSizeInBytes; } } @@ -62,12 +63,12 @@ public AesGcmCipher(byte[] key, byte[] iv) #if NET6_0_OR_GREATER if (System.Security.Cryptography.AesGcm.IsSupported) { - _impl = new BclImpl(key, TagSize, _iv); + _impl = new BclImpl(key, _iv); } else #endif { - _impl = new BouncyCastleImpl(key, TagSize, _iv); + _impl = new BouncyCastleImpl(key, _iv); } }