rework locking in SslStream to support TLS1.3 (#32925)

* initial locking * feedback from review * update _handshakeWaiter * feedback from review * feedback from review * feedback from review
dotnet · Mar 20, 2020 · 4bdf468 · 4bdf468
1 parent 7ae0add
commit 4bdf468
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 239 deletions.
diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
@@ -333,14 +333,11 @@ internal static int Encrypt(SafeSslHandle context, ReadOnlySpan<byte> input, ref
             int retVal;
             Exception? innerError = null;
 
-            lock (context)
-            {
-                retVal = Ssl.SslWrite(context, ref MemoryMarshal.GetReference(input), input.Length);
+            retVal = Ssl.SslWrite(context, ref MemoryMarshal.GetReference(input), input.Length);
 
-                if (retVal != input.Length)
-                {
-                    errorCode = GetSslError(context, retVal, out innerError);
-                }
+            if (retVal != input.Length)
+            {
+                errorCode = GetSslError(context, retVal, out innerError);
             }
 
             if (retVal != input.Length)
@@ -390,30 +387,27 @@ internal static int Decrypt(SafeSslHandle context, byte[] outBuffer, int offset,
             int retVal = BioWrite(context.InputBio!, outBuffer, offset, count);
             Exception? innerError = null;
 
-            lock (context)
+            if (retVal == count)
             {
-                if (retVal == count)
+                unsafe
                 {
-                    unsafe
+                    fixed (byte* fixedBuffer = outBuffer)
                     {
-                        fixed (byte* fixedBuffer = outBuffer)
-                        {
-                            retVal = Ssl.SslRead(context, fixedBuffer + offset, outBuffer.Length);
-                        }
-                    }
-
-                    if (retVal > 0)
-                    {
-                        count = retVal;
+                        retVal = Ssl.SslRead(context, fixedBuffer + offset, outBuffer.Length);
                     }
                 }
 
-                if (retVal != count)
+                if (retVal > 0)
                 {
-                    errorCode = GetSslError(context, retVal, out innerError);
+                        count = retVal;
                 }
             }
 
+            if (retVal != count)
+            {
+                errorCode = GetSslError(context, retVal, out innerError);
+            }
+
             if (retVal != count)
             {
                 retVal = 0;

diff --git a/src/libraries/Common/src/System/Net/SecurityStatusPal.cs b/src/libraries/Common/src/System/Net/SecurityStatusPal.cs
@@ -35,6 +35,7 @@ internal enum SecurityStatusPalErrorCode
         ContextExpired,
         CredentialsNeeded,
         Renegotiate,
+        TryAgain,
 
         // Errors
         OutOfMemory,

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
@@ -12,9 +12,8 @@ public partial class SslStream
         private interface ISslIOAdapter
         {
             ValueTask<int> ReadAsync(Memory<byte> buffer);
-            ValueTask<int> ReadLockAsync(Memory<byte> buffer);
-            Task WriteLockAsync();
             ValueTask WriteAsync(byte[] buffer, int offset, int count);
+            Task WaitAsync(TaskCompletionSource<bool> waiter);
             CancellationToken CancellationToken { get; }
         }
 
@@ -31,12 +30,10 @@ public AsyncSslIOAdapter(SslStream sslStream, CancellationToken cancellationToke
 
             public ValueTask<int> ReadAsync(Memory<byte> buffer) => _sslStream.InnerStream.ReadAsync(buffer, _cancellationToken);
 
-            public ValueTask<int> ReadLockAsync(Memory<byte> buffer) => _sslStream.CheckEnqueueReadAsync(buffer);
-
-            public Task WriteLockAsync() => _sslStream.CheckEnqueueWriteAsync();
-
             public ValueTask WriteAsync(byte[] buffer, int offset, int count) => _sslStream.InnerStream.WriteAsync(new ReadOnlyMemory<byte>(buffer, offset, count), _cancellationToken);
 
+            public Task WaitAsync(TaskCompletionSource<bool> waiter) => waiter.Task;
+
             public CancellationToken CancellationToken => _cancellationToken;
         }
 
@@ -48,17 +45,15 @@ public AsyncSslIOAdapter(SslStream sslStream, CancellationToken cancellationToke
 
             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));
-
             public ValueTask WriteAsync(byte[] buffer, int offset, int count)
             {
                 _sslStream.InnerStream.Write(buffer, offset, count);
                 return default;
             }
 
-            public Task WriteLockAsync()
+            public Task WaitAsync(TaskCompletionSource<bool> waiter)
             {
-                _sslStream.CheckEnqueueWrite();
+                waiter.Task.Wait();
                 return Task.CompletedTask;
             }