Merge PR #331: Change ZipAESStream to handle reads of less data than the AES block size

* Change ZipAESStream to better handle reads of less than the crypto block size
* Add unit test for reading AES Encrypted/Stored zip entries
* Fixes #324

Author: Numpsy

src/ICSharpCode.SharpZipLib/Encryption/ZipAESStream.cs

@@ -27,8 +27,6 @@ public ZipAESStream(Stream stream, ZipAESTransform transform, CryptoStreamMode m
 			_transform = transform;
 			_slideBuffer = new byte[1024];
 
-			_blockAndAuth = CRYPTO_BLOCK_SIZE + AUTH_CODE_LENGTH;
-
 			// mode:
 			//  CryptoStreamMode.Read means we read from "stream" and pass decrypted to our Read() method.
 			//  Write bypasses this stream and uses the Transform directly.
@@ -41,33 +39,72 @@ public ZipAESStream(Stream stream, ZipAESTransform transform, CryptoStreamMode m
 		// The final n bytes of the AES stream contain the Auth Code.
 		private const int AUTH_CODE_LENGTH = 10;
 
+		// Blocksize is always 16 here, even for AES-256 which has transform.InputBlockSize of 32.
+		private const int CRYPTO_BLOCK_SIZE = 16;
+
+		// total length of block + auth code
+		private const int BLOCK_AND_AUTH = CRYPTO_BLOCK_SIZE + AUTH_CODE_LENGTH;
+
 		private Stream _stream;
 		private ZipAESTransform _transform;
 		private byte[] _slideBuffer;
 		private int _slideBufStartPos;
 		private int _slideBufFreePos;
 
-		// Blocksize is always 16 here, even for AES-256 which has transform.InputBlockSize of 32.
-		private const int CRYPTO_BLOCK_SIZE = 16;
+		// Buffer block transforms to enable partial reads
+		private byte[] _transformBuffer = null;// new byte[CRYPTO_BLOCK_SIZE];
+		private int _transformBufferFreePos;
+		private int _transformBufferStartPos;
 
-		private int _blockAndAuth;
+		// Do we have some buffered data available?
+		private bool HasBufferedData =>_transformBuffer != null && _transformBufferStartPos < _transformBufferFreePos;
 
 		/// <summary>
 		/// Reads a sequence of bytes from the current CryptoStream into buffer,
 		/// and advances the position within the stream by the number of bytes read.
 		/// </summary>
 		public override int Read(byte[] buffer, int offset, int count)
+		{
+			// Nothing to do
+			if (count == 0)
+				return 0;
+
+			// If we have buffered data, read that first
+			int nBytes = 0;
+			if (HasBufferedData)
+			{
+				nBytes = ReadBufferedData(buffer, offset, count);
+
+				// Read all requested data from the buffer
+				if (nBytes == count)
+					return nBytes;
+
+				offset += nBytes;
+				count -= nBytes;
+			}
+
+			// Read more data from the input, if available
+			if (_slideBuffer != null)
+				nBytes += ReadAndTransform(buffer, offset, count);
+
+			return nBytes;
+		}
+
+		// Read data from the underlying stream and decrypt it
+		private int ReadAndTransform(byte[] buffer, int offset, int count)
 		{
 			int nBytes = 0;
 			while (nBytes < count)
 			{
+				int bytesLeftToRead = count - nBytes;
+
 				// Calculate buffer quantities vs read-ahead size, and check for sufficient free space
 				int byteCount = _slideBufFreePos - _slideBufStartPos;
 
 				// Need to handle final block and Auth Code specially, but don't know total data length.
 				// Maintain a read-ahead equal to the length of (crypto block + Auth Code).
 				// When that runs out we can detect these final sections.
-				int lengthToRead = _blockAndAuth - byteCount;
+				int lengthToRead = BLOCK_AND_AUTH - byteCount;
 				if (_slideBuffer.Length - _slideBufFreePos < lengthToRead)
 				{
 					// Shift the data to the beginning of the buffer
@@ -84,17 +121,11 @@ public override int Read(byte[] buffer, int offset, int count)
 
 				// Recalculate how much data we now have
 				byteCount = _slideBufFreePos - _slideBufStartPos;
-				if (byteCount >= _blockAndAuth)
+				if (byteCount >= BLOCK_AND_AUTH)
 				{
-					// At least a 16 byte block and an auth code remains.
-					_transform.TransformBlock(_slideBuffer,
-											  _slideBufStartPos,
-											  CRYPTO_BLOCK_SIZE,
-											  buffer,
-											  offset);
-					nBytes += CRYPTO_BLOCK_SIZE;
-					offset += CRYPTO_BLOCK_SIZE;
-					_slideBufStartPos += CRYPTO_BLOCK_SIZE;
+					var read = TransformAndBufferBlock(buffer, offset, bytesLeftToRead, CRYPTO_BLOCK_SIZE);
+					nBytes += read;
+					offset += read;
 				}
 				else
 				{
@@ -103,14 +134,7 @@ public override int Read(byte[] buffer, int offset, int count)
 					{
 						// At least one byte of data plus auth code
 						int finalBlock = byteCount - AUTH_CODE_LENGTH;
-						_transform.TransformBlock(_slideBuffer,
-												  _slideBufStartPos,
-												  finalBlock,
-												  buffer,
-												  offset);
-
-						nBytes += finalBlock;
-						_slideBufStartPos += finalBlock;
+						nBytes += TransformAndBufferBlock(buffer, offset, bytesLeftToRead, finalBlock);
 					}
 					else if (byteCount < AUTH_CODE_LENGTH)
 						throw new Exception("Internal error missed auth code"); // Coding bug
@@ -125,12 +149,62 @@ public override int Read(byte[] buffer, int offset, int count)
 						}
 					}
 
+					// don't need this any more, so use it as a 'complete' flag
+					_slideBuffer = null;
+
 					break;  // Reached the auth code
 				}
 			}
 			return nBytes;
 		}
 
+		// read some buffered data
+		private int ReadBufferedData(byte[] buffer, int offset, int count)
+		{
+			int copyCount = Math.Min(count, _transformBufferFreePos - _transformBufferStartPos);
+
+			Array.Copy(_transformBuffer, _transformBufferStartPos, buffer, offset, count);
+			_transformBufferStartPos += copyCount;
+
+			return copyCount;
+		}
+
+		// Perform the crypto transform, and buffer the data if less than one block has been requested.
+		private int TransformAndBufferBlock(byte[] buffer, int offset, int count, int blockSize)
+		{
+			// If the requested data is greater than one block, transform it directly into the output
+			// If it's smaller, do it into a temporary buffer and copy the requested part
+			bool bufferRequired = (blockSize > count);
+
+			if (bufferRequired && _transformBuffer == null)
+				_transformBuffer = new byte[CRYPTO_BLOCK_SIZE];
+
+			var targetBuffer = bufferRequired ? _transformBuffer : buffer;
+			var targetOffset = bufferRequired ? 0 : offset;
+
+			// Transform the data
+			_transform.TransformBlock(_slideBuffer,
+									  _slideBufStartPos,
+									  blockSize,
+									  targetBuffer,
+									  targetOffset);
+
+			_slideBufStartPos += blockSize;
+
+			if (!bufferRequired)
+			{
+				return blockSize;
+			}
+			else
+			{
+				Array.Copy(_transformBuffer, 0, buffer, offset, count);
+				_transformBufferStartPos = count;
+				_transformBufferFreePos = blockSize;
+
+				return count;
+			}
+		}
+
 		/// <summary>
 		/// Writes a sequence of bytes to the current stream and advances the current position within this stream by the number of bytes written.
 		/// </summary>

test/ICSharpCode.SharpZipLib.Tests/Zip/ZipEncryptionHandling.cs

@@ -1,4 +1,5 @@
-using ICSharpCode.SharpZipLib.Zip;
+using ICSharpCode.SharpZipLib.Core;
+using ICSharpCode.SharpZipLib.Zip;
 using NUnit.Framework;
 using System;
 using System.Diagnostics;
@@ -145,6 +146,66 @@ public void ZipFileStoreAes()
 			}
 		}
 
+		/// <summary>
+		/// Test using AES encryption on a file whose contents are Stored rather than deflated
+		/// </summary>
+		[Test]
+		[Category("Encryption")]
+		[Category("Zip")]
+		public void ZipFileStoreAesPartialRead()
+		{
+			string password = "password";
+
+			using (var memoryStream = new MemoryStream())
+			{
+				// Try to create a zip stream
+				WriteEncryptedZipToStream(memoryStream, password, 256, CompressionMethod.Stored);
+
+				// reset
+				memoryStream.Seek(0, SeekOrigin.Begin);
+
+				// try to read it
+				var zipFile = new ZipFile(memoryStream, leaveOpen: true)
+				{
+					Password = password
+				};
+
+				foreach (ZipEntry entry in zipFile)
+				{
+					if (!entry.IsFile) continue;
+
+					// Should be stored rather than deflated
+					Assert.That(entry.CompressionMethod, Is.EqualTo(CompressionMethod.Stored), "Entry should be stored");
+
+					using (var ms = new MemoryStream())
+					{
+						using (var zis = zipFile.GetInputStream(entry))
+						{
+							byte[] buffer = new byte[1];
+
+							while (true)
+							{
+								int b = zis.ReadByte();
+
+								if (b == -1)
+									break;
+
+								ms.WriteByte((byte)b);
+							}
+						}
+
+						ms.Seek(0, SeekOrigin.Begin);
+
+						using (var sr = new StreamReader(ms, Encoding.UTF8))
+						{
+							var content = sr.ReadToEnd();
+							Assert.That(content, Is.EqualTo(DummyDataString), "Decompressed content does not match input data");
+						}
+					}
+				}
+			}
+		}
+
 		private static readonly string[] possible7zPaths = new[] {
 			// Check in PATH
 			"7z", "7za",