From 4dbbf7514fb93583cf935ecb0fa9b33604b03e4e Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Wed, 12 Nov 2014 20:45:43 +0000 Subject: [PATCH] Create output buffer after padding to ensure its length matches the padded length. Fixes issue #2547. --- Renci.SshClient/Build/nuget/SSH.NET.nuspec | 1 + .../Security/Cryptography/BlockCipherTest.cs | 76 +++++++++++++++++-- .../Security/Cryptography/BlockCipher.cs | 5 +- 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/Renci.SshClient/Build/nuget/SSH.NET.nuspec b/Renci.SshClient/Build/nuget/SSH.NET.nuspec index 9f5cec31..187f841f 100644 --- a/Renci.SshClient/Build/nuget/SSH.NET.nuspec +++ b/Renci.SshClient/Build/nuget/SSH.NET.nuspec @@ -35,6 +35,7 @@ * Signing key is missing from source download (issue #2455) * Forwarded Port channels can get stuck waiting (issue #1558) * NullReferenceException when SftpFileStream is finalized after dispose of session (issue #2013) + * BlockCipher.Encrypt fails if input message is not padded (issue #2547) 2014.4.6-beta1 ============== diff --git a/Renci.SshClient/Renci.SshNet.Tests/Classes/Security/Cryptography/BlockCipherTest.cs b/Renci.SshClient/Renci.SshNet.Tests/Classes/Security/Cryptography/BlockCipherTest.cs index 666de1ac..6442b4ea 100644 --- a/Renci.SshClient/Renci.SshNet.Tests/Classes/Security/Cryptography/BlockCipherTest.cs +++ b/Renci.SshClient/Renci.SshNet.Tests/Classes/Security/Cryptography/BlockCipherTest.cs @@ -1,15 +1,77 @@ -using Renci.SshNet.Security.Cryptography; +using System; +using System.Linq; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Renci.SshNet.Security.Cryptography; +using Renci.SshNet.Security.Cryptography.Ciphers; +using Renci.SshNet.Security.Cryptography.Ciphers.Paddings; using Renci.SshNet.Tests.Common; -namespace Renci.SshNet.Tests +namespace Renci.SshNet.Tests.Classes.Security.Cryptography { - /// - ///This is a test class for BlockCipherTest and is intended - ///to contain all BlockCipherTest Unit Tests - /// - [TestClass()] + [TestClass] public class BlockCipherTest : TestBase { + [TestMethod] + public void EncryptShouldTakeIntoAccountPaddingForLengthOfOutputBufferPassedToEncryptBlock() + { + var input = new byte[] { 0x2c, 0x1a, 0x05, 0x00, 0x68 }; + var output = new byte[] { 0x0a, 0x00, 0x03, 0x02, 0x06, 0x08, 0x07, 0x05 }; + var key = new byte[] { 0x17, 0x78, 0x56, 0xe1, 0x3e, 0xbd, 0x3e, 0x50, 0x1d, 0x79, 0x3f, 0x0f, 0x55, 0x37, 0x45, 0x54 }; + var blockCipher = new BlockCipherStub(key, 8, null, new PKCS5Padding()) + { + EncryptBlockDelegate = (inputBuffer, inputOffset, inputCount, outputBuffer, outputOffset) => + { + Assert.AreEqual(8, outputBuffer.Length); + Buffer.BlockCopy(output, 0, outputBuffer, 0, output.Length); + return inputBuffer.Length; + } + }; + + var actual = blockCipher.Encrypt(input); + + Assert.IsTrue(output.SequenceEqual(actual)); + } + + [TestMethod] + public void DecryptShouldTakeIntoAccountPaddingForLengthOfOutputBufferPassedToDecryptBlock() + { + var input = new byte[] { 0x2c, 0x1a, 0x05, 0x00, 0x68 }; + var output = new byte[] { 0x0a, 0x00, 0x03, 0x02, 0x06, 0x08, 0x07, 0x05 }; + var key = new byte[] { 0x17, 0x78, 0x56, 0xe1, 0x3e, 0xbd, 0x3e, 0x50, 0x1d, 0x79, 0x3f, 0x0f, 0x55, 0x37, 0x45, 0x54 }; + var blockCipher = new BlockCipherStub(key, 8, null, new PKCS5Padding()) + { + DecryptBlockDelegate = (inputBuffer, inputOffset, inputCount, outputBuffer, outputOffset) => + { + Assert.AreEqual(8, outputBuffer.Length); + Buffer.BlockCopy(output, 0, outputBuffer, 0, output.Length); + return inputBuffer.Length; + } + }; + + var actual = blockCipher.Decrypt(input); + + Assert.IsTrue(output.SequenceEqual(actual)); + } + + + private class BlockCipherStub : BlockCipher + { + public Func EncryptBlockDelegate; + public Func DecryptBlockDelegate; + + public BlockCipherStub(byte[] key, byte blockSize, CipherMode mode, CipherPadding padding) : base(key, blockSize, mode, padding) + { + } + + public override int EncryptBlock(byte[] inputBuffer, int inputOffset, int inputCount, byte[] outputBuffer, int outputOffset) + { + return EncryptBlockDelegate(inputBuffer, inputOffset, inputCount, outputBuffer, outputOffset); + } + + public override int DecryptBlock(byte[] inputBuffer, int inputOffset, int inputCount, byte[] outputBuffer, int outputOffset) + { + return DecryptBlockDelegate(inputBuffer, inputOffset, inputCount, outputBuffer, outputOffset); + } + } } } diff --git a/Renci.SshClient/Renci.SshNet/Security/Cryptography/BlockCipher.cs b/Renci.SshClient/Renci.SshNet/Security/Cryptography/BlockCipher.cs index 5c5b1b45..fdf0567d 100644 --- a/Renci.SshClient/Renci.SshNet/Security/Cryptography/BlockCipher.cs +++ b/Renci.SshClient/Renci.SshNet/Security/Cryptography/BlockCipher.cs @@ -71,8 +71,6 @@ namespace Renci.SshNet.Security.Cryptography /// Encrypted data public override byte[] Encrypt(byte[] data) { - var output = new byte[data.Length]; - if (data.Length % _blockSize > 0) { if (_padding == null) @@ -82,9 +80,10 @@ namespace Renci.SshNet.Security.Cryptography data = _padding.Pad(_blockSize, data); } + var output = new byte[data.Length]; var writtenBytes = 0; - for (int i = 0; i < data.Length / _blockSize; i++) + for (var i = 0; i < data.Length / _blockSize; i++) { if (_mode == null) {