fix buffer handling in Tls handshake (dotnet#32267)

* fix buffer handling in Tls handshake * feedback from review
shoter · Feb 14, 2020 · f5874b0 · f5874b0
1 parent 0b32c1a
commit f5874b0
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 72 deletions.
diff --git a/src/libraries/System.Net.Security/src/System.Net.Security.csproj b/src/libraries/System.Net.Security/src/System.Net.Security.csproj
@@ -66,18 +66,21 @@
       <Link>Common\System\Net\DebugCriticalHandleZeroOrMinusOneIsInvalid.cs</Link>
     </Compile>
     <!-- System.Net common -->
+    <Compile Include="$(CommonPath)System\Net\ArrayBuffer.cs">
+       <Link>Common\System\Net\ArrayBuffer.cs</Link>
+    </Compile>
     <Compile Include="$(CommonPath)System\Net\ExceptionCheck.cs">
       <Link>Common\System\Net\ExceptionCheck.cs</Link>
     </Compile>
     <Compile Include="$(CommonPath)System\Net\LazyAsyncResult.cs">
       <Link>Common\System\Net\LazyAsyncResult.cs</Link>
     </Compile>
-    <Compile Include="$(CommonPath)System\Net\UriScheme.cs">
-      <Link>Common\System\Net\UriScheme.cs</Link>
-    </Compile>
     <Compile Include="$(CommonPath)System\Net\SecurityProtocol.cs">
       <Link>Common\System\Net\SecurityProtocol.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)System\Net\UriScheme.cs">
+      <Link>Common\System\Net\UriScheme.cs</Link>
+    </Compile>
     <!-- Common -->
     <Compile Include="$(CommonPath)System\NotImplemented.cs">
       <Link>Common\System\NotImplemented.cs</Link>
@@ -471,4 +474,4 @@
     <Reference Include="System.Security.Cryptography.OpenSsl" />
     <Reference Include="System.Security.Cryptography.Primitives" />
   </ItemGroup>
-</Project>
+</Project>
diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs
@@ -717,21 +717,21 @@ private bool AcquireServerCredentials(ref byte[] thumbPrint, ReadOnlySpan<byte>
         }
 
         //
-        internal ProtocolToken NextMessage(byte[] incoming, int offset, int count)
+        internal ProtocolToken NextMessage(ReadOnlySpan<byte> incomingBuffer)
         {
             if (NetEventSource.IsEnabled)
                 NetEventSource.Enter(this);
 
             byte[] nextmsg = null;
-            SecurityStatusPal status = GenerateToken(incoming, offset, count, ref nextmsg);
+            SecurityStatusPal status = GenerateToken(incomingBuffer, ref nextmsg);
 
             if (!_sslAuthenticationOptions.IsServer && status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
             {
                 if (NetEventSource.IsEnabled)
                     NetEventSource.Info(this, "NextMessage() returned SecurityStatusPal.CredentialsNeeded");
 
                 SetRefreshCredentialNeeded();
-                status = GenerateToken(incoming, offset, count, ref nextmsg);
+                status = GenerateToken(incomingBuffer, ref nextmsg);
             }
 
             ProtocolToken token = new ProtocolToken(nextmsg, status);
@@ -763,27 +763,14 @@ server in response
             Return:
                 status - error information
         --*/
-        private SecurityStatusPal GenerateToken(byte[] input, int offset, int count, ref byte[] output)
+        private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte[] output)
         {
             if (NetEventSource.IsEnabled) NetEventSource.Enter(this, $"_refreshCredentialNeeded = {_refreshCredentialNeeded}");
 
-            if (offset < 0 || offset > (input == null ? 0 : input.Length))
-            {
-                NetEventSource.Fail(this, "Argument 'offset' out of range.");
-                throw new ArgumentOutOfRangeException(nameof(offset));
-            }
-
-            if (count < 0 || count > (input == null ? 0 : input.Length - offset))
-            {
-                NetEventSource.Fail(this, "Argument 'count' out of range.");
-                throw new ArgumentOutOfRangeException(nameof(count));
-            }
-
             byte[] result = Array.Empty<byte>();
             SecurityStatusPal status = default;
             bool cachedCreds = false;
             byte[] thumbPrint = null;
-            ReadOnlySpan<byte> inputBuffer = new ReadOnlySpan<byte>(input, offset, count);
 
             //
             // Looping through ASC or ISC with potentially cached credential that could have been
@@ -1155,7 +1142,7 @@ private ProtocolToken GenerateAlertToken()
             byte[] nextmsg = null;
 
             SecurityStatusPal status;
-            status = GenerateToken(null, 0, 0, ref nextmsg);
+            status = GenerateToken(default, ref nextmsg);
 
             ProtocolToken token = new ProtocolToken(nextmsg, status);
 

diff --git a/...ibraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.Adapters.cs b/...ibraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.Adapters.cs
@@ -11,7 +11,7 @@ public partial class SslStream
     {
         private interface ISslIOAdapter
         {
-            ValueTask<int> ReadAsync(byte[] buffer, int offset, int count);
+            ValueTask<int> ReadAsync(Memory<byte> buffer);
             ValueTask<int> ReadLockAsync(Memory<byte> buffer);
             Task WriteLockAsync();
             ValueTask WriteAsync(byte[] buffer, int offset, int count);
@@ -29,7 +29,7 @@ public AsyncSslIOAdapter(SslStream sslStream, CancellationToken cancellationToke
                 _sslStream = sslStream;
             }
 
-            public ValueTask<int> ReadAsync(byte[] buffer, int offset, int count) => _sslStream.InnerStream.ReadAsync(new Memory<byte>(buffer, offset, count), _cancellationToken);
+            public ValueTask<int> ReadAsync(Memory<byte> buffer) => _sslStream.InnerStream.ReadAsync(buffer, _cancellationToken);
 
             public ValueTask<int> ReadLockAsync(Memory<byte> buffer) => _sslStream.CheckEnqueueReadAsync(buffer);
 
@@ -46,7 +46,7 @@ public AsyncSslIOAdapter(SslStream sslStream, CancellationToken cancellationToke
 
             public SyncSslIOAdapter(SslStream sslStream) => _sslStream = sslStream;
 
-            public ValueTask<int> ReadAsync(byte[] buffer, int offset, int count) => new ValueTask<int>(_sslStream.InnerStream.Read(buffer, offset, count));
+            public ValueTask<int> ReadAsync(Memory<byte> buffer) => new ValueTask<int>(_sslStream.InnerStream.Read(buffer.Span));
 
             public ValueTask<int> ReadLockAsync(Memory<byte> buffer) => new ValueTask<int>(_sslStream.CheckEnqueueRead(buffer));