Improvements and bug fixes after a personal code review, as well as independent professional audit (Cure53 team).

-AesCtrCryptoTransform improvements.
-CryptoRandom bug fixes.
  "NextDouble()" now generates 52 bits of entropy.
  "NextBytes()" is rewritten to avoid a possible serious concurrency flaw.
-Minor improvements and fixes throughout the codebase.
-Version bump to 1.4.

Author: sdrapkin

.gitignore

@@ -238,3 +238,5 @@ _Doc/*
 /Mac/HMAC2*.cs
 /.vs/config/applicationhost.config
 /Inferno.1.0.0.nuspec
+/COMMITS.md
+*.snk

Cipher/AesCtrCryptoTransform.cs

@@ -13,15 +13,16 @@ internal static class AesConstants
 
 	public class AesCtrCryptoTransform : ICryptoTransform
 	{
-		byte[] counterBuffer = new byte[AesConstants.AES_BLOCK_SIZE];
+		byte[] counterBuffer_KeyStreamBuffer = new byte[AesConstants.AES_BLOCK_SIZE * 2];
+		int keyStreamBytesRemaining;
 
 		Aes aes;
 		readonly ICryptoTransform cryptoTransform;
 
 		public bool CanReuseTransform { get { return false; } }
 		public bool CanTransformMultipleBlocks { get { return true; } }
-		public int InputBlockSize { get { return AesConstants.AES_BLOCK_SIZE; } }
-		public int OutputBlockSize { get { return AesConstants.AES_BLOCK_SIZE; } }
+		public int InputBlockSize { get { return 1; } }
+		public int OutputBlockSize { get { return 1; } }
 
 		/// <summary>ctor</summary>
 		public AesCtrCryptoTransform(byte[] key, ArraySegment<byte> counterBufferSegment, Func<Aes> aesFactory = null)
@@ -33,39 +34,61 @@ public AesCtrCryptoTransform(byte[] key, ArraySegment<byte> counterBufferSegment
 			this.aes.Mode = CipherMode.ECB;
 			this.aes.Padding = PaddingMode.None;
 
-			Utils.BlockCopy(counterBufferSegment.Array, counterBufferSegment.Offset, counterBuffer, 0, AesConstants.AES_BLOCK_SIZE);
+			Utils.BlockCopy(counterBufferSegment.Array, counterBufferSegment.Offset, counterBuffer_KeyStreamBuffer, 0, AesConstants.AES_BLOCK_SIZE);
 			this.cryptoTransform = aes.CreateEncryptor(rgbKey: key, rgbIV: null);
 		}// ctor
 
 		#region public
 		public int TransformBlock(byte[] inputBuffer, int inputOffset, int inputCount, byte[] outputBuffer, int outputOffset)
 		{
-			int partialBlockSize = inputCount % AesConstants.AES_BLOCK_SIZE;
-			int fullBlockSize = inputCount & (-AesConstants.AES_BLOCK_SIZE);//inputCount - partialBlockSize;
-			int i, j;
-			byte[] counterBuffer = this.counterBuffer; // looks dumb, but local-access is faster than field-access
+			if (inputCount == 0) return 0;
 
-			for (i = outputOffset, /* reusing inputCount as iMax */ inputCount = outputOffset + fullBlockSize; i < inputCount; i += AesConstants.AES_BLOCK_SIZE)
+			int i, j, k, remainingInputCount = inputCount;
+			byte[] counterBuffer_KeyStreamBuffer = this.counterBuffer_KeyStreamBuffer; // looks dumb, but local-access is faster than field-access
+
+			// process any available key stream first
+			if (this.keyStreamBytesRemaining > 0)
 			{
-				Utils.BlockCopy(counterBuffer, 0, outputBuffer, i, AesConstants.AES_BLOCK_SIZE);
-				for (j = AesConstants.AES_BLOCK_SIZE - 1; j >= AesConstants.AES_BLOCK_SIZE - AesConstants.COUNTER_SIZE; --j)
-					if (++counterBuffer[j] != 0) break;
+				j = inputCount > this.keyStreamBytesRemaining ? this.keyStreamBytesRemaining : inputCount;
+				for (i = 0; i < j; ++i)
+					outputBuffer[outputOffset + i] = (byte)(counterBuffer_KeyStreamBuffer[AesConstants.AES_BLOCK_SIZE * 2 - this.keyStreamBytesRemaining + i] ^ inputBuffer[inputOffset + i]);
+
+				inputOffset += j;
+				outputOffset += j;
+				this.keyStreamBytesRemaining -= j;
+				remainingInputCount -= j;
+
+				if (remainingInputCount == 0) return inputCount;
 			}
 
+			int partialBlockSize = remainingInputCount % AesConstants.AES_BLOCK_SIZE;
+			int fullBlockSize = remainingInputCount & (-AesConstants.AES_BLOCK_SIZE); // remainingInputCount - partialBlockSize;
+
+			// process full blocks, if any
 			if (fullBlockSize > 0)
 			{
+				for (i = outputOffset, /* reusing k as iMax */ k = outputOffset + fullBlockSize; i < k; i += AesConstants.AES_BLOCK_SIZE)
+				{
+					Utils.BlockCopy(counterBuffer_KeyStreamBuffer, 0, outputBuffer, i, AesConstants.AES_BLOCK_SIZE);
+					for (j = AesConstants.AES_BLOCK_SIZE - 1; j >= AesConstants.AES_BLOCK_SIZE - AesConstants.COUNTER_SIZE; --j) if (++counterBuffer_KeyStreamBuffer[j] != 0) break;
+				}
+
 				fullBlockSize = this.cryptoTransform.TransformBlock(outputBuffer, outputOffset, fullBlockSize, outputBuffer, outputOffset);
-				//for (i = 0; i < fullBlockSize; ++i) outputBuffer[outputOffset + i] ^= inputBuffer[inputOffset + i];
 				Utils.Xor(outputBuffer, outputOffset, inputBuffer, inputOffset, fullBlockSize);
 			}
 
+			// process the remaining partial block, if any
 			if (partialBlockSize > 0)
 			{
-				outputOffset += fullBlockSize;
 				inputOffset += fullBlockSize;
-				this.cryptoTransform.TransformBlock(counterBuffer, 0, AesConstants.AES_BLOCK_SIZE, counterBuffer, 0);
-				for (i = 0; i < partialBlockSize; ++i) outputBuffer[outputOffset + i] = (byte)(counterBuffer[i] ^ inputBuffer[inputOffset + i]);
+				outputOffset += fullBlockSize;
+
+				this.cryptoTransform.TransformBlock(counterBuffer_KeyStreamBuffer, 0, AesConstants.AES_BLOCK_SIZE, counterBuffer_KeyStreamBuffer, AesConstants.AES_BLOCK_SIZE);
+				for (j = AesConstants.AES_BLOCK_SIZE - 1; j >= AesConstants.AES_BLOCK_SIZE - AesConstants.COUNTER_SIZE; --j) if (++counterBuffer_KeyStreamBuffer[j] != 0) break;
+				for (i = 0; i < partialBlockSize; ++i) outputBuffer[outputOffset + i] = (byte)(counterBuffer_KeyStreamBuffer[AesConstants.AES_BLOCK_SIZE + i] ^ inputBuffer[inputOffset + i]);
+				this.keyStreamBytesRemaining = AesConstants.AES_BLOCK_SIZE - partialBlockSize;
 			}
+
 			return inputCount;
 		}// TransformBlock()
 
@@ -88,7 +111,7 @@ public void Dispose()
 				}
 				finally
 				{
-					Array.Clear(counterBuffer, 0, AesConstants.AES_BLOCK_SIZE);
+					Array.Clear(this.counterBuffer_KeyStreamBuffer, 0, AesConstants.AES_BLOCK_SIZE * 2);
 					this.aes = null;
 				}
 			}

CryptoRandom.cs

@@ -74,7 +74,7 @@ public long NextLong()
 		public long NextLong(long maxValue)
 		{
 			if (maxValue < 0)
-				throw new ArgumentOutOfRangeException("maxValue");
+				throw new ArgumentOutOfRangeException(nameof(maxValue));
 
 			return NextLong(0, maxValue);
 		}//NextLong()
@@ -96,7 +96,7 @@ public long NextLong(long minValue, long maxValue)
 				return minValue;
 
 			if (minValue > maxValue)
-				throw new ArgumentOutOfRangeException("minValue");
+				throw new ArgumentOutOfRangeException(nameof(minValue));
 
 			ulong diff = decimal.ToUInt64((decimal)maxValue - minValue);
 			ulong upperBound = ulong.MaxValue / diff * diff;
@@ -136,7 +136,7 @@ public override int Next()
 		public override int Next(int maxValue)
 		{
 			if (maxValue < 0)
-				throw new ArgumentOutOfRangeException("maxValue");
+				throw new ArgumentOutOfRangeException(nameof(maxValue));
 
 			return Next(0, maxValue);
 		}//Next()
@@ -158,7 +158,7 @@ public override int Next(int minValue, int maxValue)
 				return minValue;
 
 			if (minValue > maxValue)
-				throw new ArgumentOutOfRangeException("minValue");
+				throw new ArgumentOutOfRangeException(nameof(minValue));
 
 			long diff = (long)maxValue - minValue;
 			long upperBound = uint.MaxValue / diff * diff;
@@ -180,8 +180,8 @@ public override int Next(int minValue, int maxValue)
 		/// </returns>
 		public override double NextDouble()
 		{
-			const double max = 1.0 + uint.MaxValue;
-			return GetRandomUInt() / max;
+			const double max = 1L << 53; // https://en.wikipedia.org/wiki/Double-precision_floating-point_format
+			return (GetRandomULong() >> 11) / max;
 		}//NextDouble()
 
 		/// <summary>
@@ -220,7 +220,7 @@ public void NextBytes(byte[] buffer, int offset, int count)
 			var checkedBufferSegment = new ArraySegment<byte>(buffer, offset, count); // bounds-validation happens here
 			if (count == 0) return;
 			NextBytesInternal(checkedBufferSegment);
-		}
+		}//NextBytes()
 
 		void NextBytesInternal(ArraySegment<byte> bufferSegment)
 		{
@@ -236,68 +236,50 @@ void NextBytesInternal(ArraySegment<byte> bufferSegment)
 				throw new CryptographicException((int)status);
 			}
 
-			while (true)
+			lock (_byteCache)
 			{
-				int currentByteCachePosition = Interlocked.Add(ref _byteCachePosition, count);
-				if (currentByteCachePosition <= BYTE_CACHE_SIZE && currentByteCachePosition > 0)
+				if (_byteCachePosition + count <= BYTE_CACHE_SIZE)
 				{
-					Utils.BlockCopy(_byteCache, currentByteCachePosition - count, buffer, 0, count);
+					Utils.BlockCopy(_byteCache, _byteCachePosition, buffer, offset, count);
+					_byteCachePosition += count;
 					return;
 				}
 
-				lock (_byteCache)
+				status = BCrypt.BCryptGenRandom(_byteCache, BYTE_CACHE_SIZE);
+				if (status == BCrypt.NTSTATUS.STATUS_SUCCESS)
 				{
-					currentByteCachePosition = _byteCachePosition; // atomic read
-					if (currentByteCachePosition > (BYTE_CACHE_SIZE - count) || currentByteCachePosition <= 0)
-					{
-						status = BCrypt.BCryptGenRandom(_byteCache, BYTE_CACHE_SIZE);
-						if (status == BCrypt.NTSTATUS.STATUS_SUCCESS)
-						{
-							_byteCachePosition = count; // atomic write
-							Utils.BlockCopy(_byteCache, 0, buffer, 0, count);
-							return;
-						}
-
-						// defensive logic to prevent _byteCachePosition from wrapping into valid range due to BCryptGenRandom failures
-						if (currentByteCachePosition > BYTE_CACHE_SIZE || currentByteCachePosition < 0) _byteCachePosition = BYTE_CACHE_SIZE;
-
-						throw new CryptographicException((int)status);
-					}// if outside the valid range
-				}// lock
-			}// while(true)
-		}//NextBytes()
+					_byteCachePosition = count;
+					Utils.BlockCopy(_byteCache, 0, buffer, offset, count);
+					return;
+				}
+				throw new CryptographicException((int)status);
+			}// lock
+		}//NextBytesInternal()
 
 		/// <summary>
 		/// Gets one random unsigned 32bit integer in a thread safe manner.
 		/// </summary>
 		uint GetRandomUInt()
 		{
 			BCrypt.NTSTATUS status;
-			while (true)
+
+			lock (_byteCache)
 			{
-				int currentByteCachePosition = Interlocked.Add(ref _byteCachePosition, sizeof(uint));
-				if (currentByteCachePosition <= BYTE_CACHE_SIZE && currentByteCachePosition > 0)
-					return BitConverter.ToUInt32(_byteCache, currentByteCachePosition - sizeof(uint));
+				if (_byteCachePosition + sizeof(uint) <= BYTE_CACHE_SIZE)
+				{
+					var result = BitConverter.ToUInt32(_byteCache, _byteCachePosition);
+					_byteCachePosition += sizeof(uint);
+					return result;
+				}
 
-				lock (_byteCache)
+				status = BCrypt.BCryptGenRandom(_byteCache, BYTE_CACHE_SIZE);
+				if (status == BCrypt.NTSTATUS.STATUS_SUCCESS)
 				{
-					currentByteCachePosition = _byteCachePosition; // atomic read
-					if (currentByteCachePosition > (BYTE_CACHE_SIZE - sizeof(uint)) || currentByteCachePosition <= 0)
-					{
-						status = BCrypt.BCryptGenRandom(_byteCache, BYTE_CACHE_SIZE);
-						if (status == BCrypt.NTSTATUS.STATUS_SUCCESS)
-						{
-							_byteCachePosition = sizeof(uint); // atomic write
-							return BitConverter.ToUInt32(_byteCache, 0);
-						}
-
-						// defensive logic to prevent _byteCachePosition from wrapping into valid range due to BCryptGenRandom failures
-						if (currentByteCachePosition > BYTE_CACHE_SIZE || currentByteCachePosition < 0) _byteCachePosition = BYTE_CACHE_SIZE;
-
-						throw new CryptographicException((int)status);
-					}// if outside the valid range
-				}// lock
-			}// while(true)
+					_byteCachePosition = sizeof(uint);
+					return BitConverter.ToUInt32(_byteCache, 0);
+				}
+				throw new CryptographicException((int)status);
+			}// lock
 		}//GetRandomUInt()
 
 		/// <summary>
@@ -306,31 +288,24 @@ uint GetRandomUInt()
 		ulong GetRandomULong()
 		{
 			BCrypt.NTSTATUS status;
-			while (true)
+
+			lock (_byteCache)
 			{
-				int currentByteCachePosition = Interlocked.Add(ref _byteCachePosition, sizeof(ulong));
-				if (currentByteCachePosition <= BYTE_CACHE_SIZE && currentByteCachePosition > 0)
-					return BitConverter.ToUInt64(_byteCache, currentByteCachePosition - sizeof(ulong));
+				if (_byteCachePosition + sizeof(ulong) <= BYTE_CACHE_SIZE)
+				{
+					var result = BitConverter.ToUInt64(_byteCache, _byteCachePosition);
+					_byteCachePosition += sizeof(ulong);
+					return result;
+				}
 
-				lock (_byteCache)
+				status = BCrypt.BCryptGenRandom(_byteCache, BYTE_CACHE_SIZE);
+				if (status == BCrypt.NTSTATUS.STATUS_SUCCESS)
 				{
-					currentByteCachePosition = _byteCachePosition; // atomic read
-					if (currentByteCachePosition > (BYTE_CACHE_SIZE - sizeof(ulong)) || currentByteCachePosition <= 0)
-					{
-						status = BCrypt.BCryptGenRandom(_byteCache, BYTE_CACHE_SIZE);
-						if (status == BCrypt.NTSTATUS.STATUS_SUCCESS)
-						{
-							_byteCachePosition = sizeof(ulong); // atomic write
-							return BitConverter.ToUInt64(_byteCache, 0);
-						}
-
-						// defensive logic to prevent _byteCachePosition from wrapping into valid range due to BCryptGenRandom failures
-						if (currentByteCachePosition > BYTE_CACHE_SIZE || currentByteCachePosition < 0) _byteCachePosition = BYTE_CACHE_SIZE;
-
-						throw new CryptographicException((int)status);
-					}// if outside the valid range
-				}// lock
-			}// while(true)
+					_byteCachePosition = sizeof(ulong);
+					return BitConverter.ToUInt64(_byteCache, 0);
+				}
+				throw new CryptographicException((int)status);
+			}// lock
 		}//GetRandomULong()
 	}//class CryptoRandom
 

EtM_CBC.cs

@@ -52,7 +52,7 @@ public static void Encrypt(byte[] masterKey, ArraySegment<byte> plaintext, byte[
 			int finalBlockLength = plaintext.Count % Cipher.AesConstants.AES_BLOCK_SIZE;
 			int paddingLength = Cipher.AesConstants.AES_BLOCK_SIZE - finalBlockLength;
 			int ciphertextLength = CONTEXT_BUFFER_LENGTH + plaintext.Count + paddingLength + MAC_LENGTH;
-			if (output.Length - outputOffset < ciphertextLength) throw new ArgumentOutOfRangeException("output", "'output' array segment is not big enough for the ciphertext");
+			if (output.Length - outputOffset < ciphertextLength) throw new ArgumentOutOfRangeException(nameof(output), $"'{nameof(output)}' array segment is not big enough for the ciphertext");
 
 			try
 			{

EtM_CTR.cs

@@ -35,7 +35,7 @@ static void ClearKeyMaterial()
 		public static void Encrypt(byte[] masterKey, ArraySegment<byte> plaintext, byte[] output, int outputOffset, ArraySegment<byte>? salt = null, uint counter = 1)
 		{
 			int ciphertextLength = CONTEXT_BUFFER_LENGTH + plaintext.Count + MAC_LENGTH;
-			if (output.Length - outputOffset < ciphertextLength) throw new ArgumentOutOfRangeException("output", "'output' array segment is not big enough for the ciphertext");
+			if (output.Length - outputOffset < ciphertextLength) throw new ArgumentOutOfRangeException(nameof(output), $"'{nameof(output)}' array segment is not big enough for the ciphertext");
 			try
 			{
 				_cryptoRandom.NextBytes(_contextBuffer.Value);

EtM_Transforms.cs

@@ -28,7 +28,7 @@ public class EtM_EncryptTransform : ICryptoTransform
 		/// <summary>ctor</summary>
 		public EtM_EncryptTransform(byte[] key, ArraySegment<byte>? salt = null, uint chunkNumber = 1)
 		{
-			if (key == null) throw new ArgumentNullException("key", "key cannot be null.");
+			if (key == null) throw new ArgumentNullException(nameof(key), nameof(key) + " cannot be null.");
 			this.key = key;
 			this.salt = salt;
 			this.currentChunkNumber = chunkNumber;
@@ -53,7 +53,7 @@ public int TransformBlock(byte[] inputBuffer, int inputOffset, int inputCount, b
 						output: outputBuffer,
 						outputOffset: outputOffset + j,
 						salt: this.salt,
-						counter: this.currentChunkNumber++);
+						counter: checked(this.currentChunkNumber++));
 				}
 			}
 			return j;
@@ -104,7 +104,7 @@ public class EtM_DecryptTransform : ICryptoTransform
 		/// <summary>ctor</summary>
 		public EtM_DecryptTransform(byte[] key, ArraySegment<byte>? salt = null, uint chunkNumber = 1, bool authenticateOnly = false)
 		{
-			if (key == null) throw new ArgumentNullException("key", "key cannot be null.");
+			if (key == null) throw new ArgumentNullException(nameof(key), nameof(key) + " cannot be null.");
 			this.key = key;
 			this.salt = salt;
 			this.currentChunkNumber = chunkNumber;
@@ -152,7 +152,7 @@ public int TransformBlock(byte[] inputBuffer, int inputOffset, int inputCount, b
 						this.key = null;
 						throw new CryptographicException("Decryption failed for block " + this.currentChunkNumber.ToString() + ".");
 					}
-					++this.currentChunkNumber;
+					this.currentChunkNumber = checked(this.currentChunkNumber + 1);
 				}
 			}
 			return j;

Extensions/B64Extensions.cs

@@ -84,7 +84,7 @@ public static string ToB64(this ArraySegment<byte> inputSegment)
 		public static byte[] FromB64(this string input)
 		{
 			if (input == null)
-				throw new ArgumentNullException("input");
+				throw new ArgumentNullException(nameof(input));
 
 			int len = input.Length;
 			if (len < 1)

Extensions/Base16Config.cs

@@ -21,7 +21,7 @@ public Base16Config(char[] alphabet = null)
 			}
 
 			if (alphabet.Length != BASE)
-				throw new ArgumentOutOfRangeException("alphabet", "'alphabet' array must have exactly " + BASE + " characters.");
+				throw new ArgumentOutOfRangeException(nameof(alphabet), $"'{nameof(alphabet)}' array must have exactly {BASE.ToString()} characters.");
 
 			this.Base16table = alphabet;