Add a socket receive buffer (#1733)

Currently an array is allocated to read each packet from the socket, followed by
decryption which allocates another array for the plaintext payload. We can save one
of these two allocations by adding a persistent buffer for socket receives, and
allowing the cipher implementations to decrypt into the given payload array.

We can save the other allocation similarly, but in a separate change.

Author: Rob-Hague

src/Renci.SshNet/Abstractions/CryptoAbstraction.cs

@@ -1,3 +1,7 @@
+using System;
+#if !NET
+using System.Runtime.CompilerServices;
+#endif
 using System.Security.Cryptography;
 
 using Org.BouncyCastle.Crypto.Prng;
@@ -10,5 +14,37 @@ internal static class CryptoAbstraction
         internal static readonly RandomNumberGenerator Randomizer = RandomNumberGenerator.Create();
 
         internal static readonly SecureRandom SecureRandom = new SecureRandom(new CryptoApiRandomGenerator(Randomizer));
+
+#if !NET
+        [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
+#endif
+        public static bool FixedTimeEquals(ReadOnlySpan<byte> left, ReadOnlySpan<byte> right)
+        {
+#if NET
+            return CryptographicOperations.FixedTimeEquals(left, right);
+#else
+            // https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CryptographicOperations.cs
+
+            // NoOptimization because we want this method to be exactly as non-short-circuiting
+            // as written.
+            //
+            // NoInlining because the NoOptimization would get lost if the method got inlined.
+
+            if (left.Length != right.Length)
+            {
+                return false;
+            }
+
+            var length = left.Length;
+            var accum = 0;
+
+            for (var i = 0; i < length; i++)
+            {
+                accum |= left[i] - right[i];
+            }
+
+            return accum == 0;
+#endif
+        }
     }
 }

src/Renci.SshNet/Security/Cryptography/Cipher.cs

@@ -1,3 +1,6 @@
+#nullable enable
+using System;
+
 namespace Renci.SshNet.Security.Cryptography
 {
     /// <summary>
@@ -73,5 +76,25 @@ public virtual byte[] Decrypt(byte[] input)
         /// The decrypted data.
         /// </returns>
         public abstract byte[] Decrypt(byte[] input, int offset, int length);
+
+        /// <summary>
+        /// Decrypts the specified input into a given buffer.
+        /// </summary>
+        /// <param name="input">The input.</param>
+        /// <param name="offset">The zero-based offset in <paramref name="input"/> at which to begin decrypting.</param>
+        /// <param name="length">The number of bytes to decrypt from <paramref name="input"/>.</param>
+        /// <param name="output">The output buffer to write to.</param>
+        /// <param name="outputOffset">The zero-based offset in <paramref name="output"/> at which to write decrypted output.</param>
+        /// <returns>
+        /// The number of bytes written to <paramref name="output"/>.
+        /// </returns>
+        public virtual int Decrypt(byte[] input, int offset, int length, byte[] output, int outputOffset)
+        {
+            var plaintext = Decrypt(input, offset, length);
+
+            plaintext.AsSpan().CopyTo(output.AsSpan(outputOffset));
+
+            return plaintext.Length;
+        }
     }
 }

src/Renci.SshNet/Security/Cryptography/Ciphers/AesCipher.BclImpl.cs

@@ -1,4 +1,5 @@
-using System;
+#nullable enable
+using System;
 using System.Security.Cryptography;
 
 using Renci.SshNet.Common;
@@ -39,53 +40,45 @@ public BclImpl(
             }
 
             public override byte[] Encrypt(byte[] input, int offset, int length)
+            {
+                return Transform(_encryptor, input, offset, length, output: null, 0, out _);
+            }
+
+            public override byte[] Decrypt(byte[] input, int offset, int length)
+            {
+                return Transform(_decryptor, input, offset, length, output: null, 0, out _);
+            }
+
+            public override int Decrypt(byte[] input, int offset, int length, byte[] output, int outputOffset)
+            {
+                _ = Transform(_decryptor, input, offset, length, output, outputOffset, out var bytesWritten);
+
+                return bytesWritten;
+            }
+
+            private byte[] Transform(ICryptoTransform transform, byte[] input, int offset, int length, byte[]? output, int outputOffset, out int bytesWritten)
             {
                 if (_aes.Padding != PaddingMode.None)
                 {
                     // If padding has been specified, call TransformFinalBlock to apply
                     // the padding and reset the state.
-                    return _encryptor.TransformFinalBlock(input, offset, length);
-                }
 
-                var paddingLength = 0;
-                if (length % BlockSize > 0)
-                {
-                    if (_aes.Mode is System.Security.Cryptography.CipherMode.CFB or System.Security.Cryptography.CipherMode.OFB)
+                    var finalBlock = transform.TransformFinalBlock(input, offset, length);
+
+                    if (output is not null)
                     {
-                        // Manually pad the input for cfb and ofb cipher mode as BCL doesn't support partial block.
-                        // See https://github.com/dotnet/runtime/blob/e7d837da5b1aacd9325a8b8f2214cfaf4d3f0ff6/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SymmetricPadding.cs#L20-L21
-                        paddingLength = BlockSize - (length % BlockSize);
-                        input = input.Take(offset, length);
-                        length += paddingLength;
-                        Array.Resize(ref input, length);
-                        offset = 0;
+                        finalBlock.AsSpan().CopyTo(output.AsSpan(outputOffset));
                     }
+
+                    bytesWritten = finalBlock.Length;
+
+                    return finalBlock;
                 }
 
                 // Otherwise, (the most important case) assume this instance is
                 // used for one direction of an SSH connection, whereby the
                 // encrypted data in all packets are considered a single data
-                // stream i.e. we do not want to reset the state between calls to Encrypt.
-                var output = new byte[length];
-                _ = _encryptor.TransformBlock(input, offset, length, output, 0);
-
-                if (paddingLength > 0)
-                {
-                    // Manually unpad the output.
-                    Array.Resize(ref output, output.Length - paddingLength);
-                }
-
-                return output;
-            }
-
-            public override byte[] Decrypt(byte[] input, int offset, int length)
-            {
-                if (_aes.Padding != PaddingMode.None)
-                {
-                    // If padding has been specified, call TransformFinalBlock to apply
-                    // the padding and reset the state.
-                    return _decryptor.TransformFinalBlock(input, offset, length);
-                }
+                // stream i.e. we do not want to reset the state between calls to Decrypt.
 
                 var paddingLength = 0;
                 if (length % BlockSize > 0)
@@ -95,24 +88,33 @@ public override byte[] Decrypt(byte[] input, int offset, int length)
                         // Manually pad the input for cfb and ofb cipher mode as BCL doesn't support partial block.
                         // See https://github.com/dotnet/runtime/blob/e7d837da5b1aacd9325a8b8f2214cfaf4d3f0ff6/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SymmetricPadding.cs#L20-L21
                         paddingLength = BlockSize - (length % BlockSize);
-                        input = input.Take(offset, length);
-                        length += paddingLength;
-                        Array.Resize(ref input, length);
+
+                        var tmp = new byte[length + paddingLength];
+
+                        input.AsSpan(offset, length).CopyTo(tmp);
+
+                        input = tmp;
                         offset = 0;
+                        length = tmp.Length;
                     }
                 }
 
-                // Otherwise, (the most important case) assume this instance is
-                // used for one direction of an SSH connection, whereby the
-                // encrypted data in all packets are considered a single data
-                // stream i.e. we do not want to reset the state between calls to Decrypt.
-                var output = new byte[length];
-                _ = _decryptor.TransformBlock(input, offset, length, output, 0);
-
-                if (paddingLength > 0)
+                if (output is null)
                 {
+                    output = new byte[length];
+
+                    bytesWritten = transform.TransformBlock(input, offset, length, output, outputOffset);
+
+                    bytesWritten -= paddingLength;
+
                     // Manually unpad the output.
-                    Array.Resize(ref output, output.Length - paddingLength);
+                    Array.Resize(ref output, bytesWritten);
+                }
+                else
+                {
+                    bytesWritten = transform.TransformBlock(input, offset, length, output, outputOffset);
+
+                    bytesWritten -= paddingLength;
                 }
 
                 return output;

src/Renci.SshNet/Security/Cryptography/Ciphers/AesCipher.CtrImpl.cs

@@ -1,5 +1,7 @@
-using System;
+#nullable enable
+using System;
 using System.Buffers.Binary;
+using System.Diagnostics;
 using System.Numerics;
 using System.Security.Cryptography;
 
@@ -34,12 +36,32 @@ public CtrImpl(
 
             public override byte[] Encrypt(byte[] input, int offset, int length)
             {
-                return CTREncryptDecrypt(input, offset, length);
+                return Decrypt(input, offset, length);
             }
 
             public override byte[] Decrypt(byte[] input, int offset, int length)
             {
-                return CTREncryptDecrypt(input, offset, length);
+                ArgumentNullException.ThrowIfNull(input);
+
+                var buffer = CTREncryptDecrypt(input, offset, length, output: null, 0);
+
+                // adjust output for non-blocksized lengths
+                if (buffer.Length > length)
+                {
+                    Array.Resize(ref buffer, length);
+                }
+
+                return buffer;
+            }
+
+            public override int Decrypt(byte[] input, int offset, int length, byte[] output, int outputOffset)
+            {
+                ArgumentNullException.ThrowIfNull(input);
+                ArgumentNullException.ThrowIfNull(output);
+
+                _ = CTREncryptDecrypt(input, offset, length, output, outputOffset);
+
+                return length;
             }
 
             public override int DecryptBlock(byte[] inputBuffer, int inputOffset, int inputCount, byte[] outputBuffer, int outputOffset)
@@ -52,56 +74,67 @@ public override int EncryptBlock(byte[] inputBuffer, int inputOffset, int inputC
                 throw new NotImplementedException($"Invalid usage of {nameof(EncryptBlock)}.");
             }
 
-            private byte[] CTREncryptDecrypt(byte[] data, int offset, int length)
+            private byte[] CTREncryptDecrypt(byte[] data, int offset, int length, byte[]? output, int outputOffset)
             {
-                var count = length / BlockSize;
-                if (length % BlockSize != 0)
+                var blockSizedLength = length;
+                if (blockSizedLength % BlockSize != 0)
                 {
-                    count++;
+                    blockSizedLength += BlockSize - (blockSizedLength % BlockSize);
                 }
 
-                var buffer = new byte[count * BlockSize];
-                CTRCreateCounterArray(buffer);
-                _ = _encryptor.TransformBlock(buffer, 0, buffer.Length, buffer, 0);
-                ArrayXOR(buffer, data, offset, length);
+                Debug.Assert(blockSizedLength % BlockSize == 0);
 
-                // adjust output for non-blocksized lengths
-                if (buffer.Length > length)
+                if (output is null)
                 {
-                    Array.Resize(ref buffer, length);
+                    output = new byte[blockSizedLength];
+                    outputOffset = 0;
+                }
+                else if (data.AsSpan(offset, length).Overlaps(output.AsSpan(outputOffset, blockSizedLength)))
+                {
+                    throw new ArgumentException("Input and output buffers must not overlap");
                 }
 
-                return buffer;
+                CTRCreateCounterArray(output.AsSpan(outputOffset, blockSizedLength));
+
+                var bytesWritten = _encryptor.TransformBlock(output, outputOffset, blockSizedLength, output, outputOffset);
+
+                Debug.Assert(bytesWritten == blockSizedLength);
+
+                ArrayXOR(output, outputOffset, data, offset, length);
+
+                return output;
             }
 
             // creates the Counter array filled with incrementing copies of IV
-            private void CTRCreateCounterArray(byte[] buffer)
+            private void CTRCreateCounterArray(Span<byte> buffer)
             {
+                Debug.Assert(buffer.Length % 16 == 0);
+
                 for (var i = 0; i < buffer.Length; i += 16)
                 {
-                    BinaryPrimitives.WriteUInt64BigEndian(buffer.AsSpan(i + 8), _ivLower);
-                    BinaryPrimitives.WriteUInt64BigEndian(buffer.AsSpan(i), _ivUpper);
+                    BinaryPrimitives.WriteUInt64BigEndian(buffer.Slice(i + 8), _ivLower);
+                    BinaryPrimitives.WriteUInt64BigEndian(buffer.Slice(i), _ivUpper);
 
                     _ivLower += 1;
                     _ivUpper += (_ivLower == 0) ? 1UL : 0UL;
                 }
             }
 
             // XOR 2 arrays using Vector<byte>
-            private static void ArrayXOR(byte[] buffer, byte[] data, int offset, int length)
+            private static void ArrayXOR(byte[] buffer, int bufferOffset, byte[] data, int offset, int length)
             {
                 var i = 0;
 
                 var oneVectorFromEnd = length - Vector<byte>.Count;
                 for (; i <= oneVectorFromEnd; i += Vector<byte>.Count)
                 {
-                    var v = new Vector<byte>(buffer, i) ^ new Vector<byte>(data, offset + i);
-                    v.CopyTo(buffer, i);
+                    var v = new Vector<byte>(buffer, bufferOffset + i) ^ new Vector<byte>(data, offset + i);
+                    v.CopyTo(buffer, bufferOffset + i);
                 }
 
                 for (; i < length; i++)
                 {
-                    buffer[i] ^= data[offset + i];
+                    buffer[bufferOffset + i] ^= data[offset + i];
                 }
             }
 

src/Renci.SshNet/Security/Cryptography/Ciphers/AesCipher.cs

@@ -77,6 +77,12 @@ public override byte[] Decrypt(byte[] input, int offset, int length)
             return _impl.Decrypt(input, offset, length);
         }
 
+        /// <inheritdoc/>
+        public override int Decrypt(byte[] input, int offset, int length, byte[] output, int outputOffset)
+        {
+            return _impl.Decrypt(input, offset, length, output, outputOffset);
+        }
+
         /// <inheritdoc/>
         public void Dispose()
         {