From 6acf09baba8a612a389140febee57b9a3abfa0ae Mon Sep 17 00:00:00 2001 From: drawaes Date: Sun, 12 Nov 2017 00:32:11 +0000 Subject: [PATCH] Reverts cancelation on SslStream Revert "Implement cancellation policy for sslstream." This reverts commit f2c8a559f52fdc3bd771d39744f86c6e42844c27. --- .../src/System/Net/FixedSizeReader.cs | 2 +- .../src/System/Net/HelperAsyncResults.cs | 11 ++-- .../src/System/Net/Security/SslState.cs | 54 +++---------------- .../src/System/Net/Security/SslStream.cs | 8 +-- .../FunctionalTests/SslStreamAlpnTests.cs | 54 ------------------- .../tests/UnitTests/Fakes/FakeSslState.cs | 2 +- 6 files changed, 16 insertions(+), 115 deletions(-) diff --git a/src/System.Net.Security/src/System/Net/FixedSizeReader.cs b/src/System.Net.Security/src/System/Net/FixedSizeReader.cs index b6faae6fcc06..94dc81225e9f 100644 --- a/src/System.Net.Security/src/System/Net/FixedSizeReader.cs +++ b/src/System.Net.Security/src/System/Net/FixedSizeReader.cs @@ -53,7 +53,7 @@ public static async void ReadPacketAsync(Stream transport, AsyncProtocolRequest int remainingCount = request.Count, offset = request.Offset; do { - int bytes = await transport.ReadAsync(request.Buffer, offset, remainingCount, request.CancellationToken).ConfigureAwait(false); + int bytes = await transport.ReadAsync(request.Buffer, offset, remainingCount, CancellationToken.None).ConfigureAwait(false); if (bytes == 0) { if (remainingCount != request.Count) diff --git a/src/System.Net.Security/src/System/Net/HelperAsyncResults.cs b/src/System.Net.Security/src/System/Net/HelperAsyncResults.cs index 6d7c87addf78..736bd98e1b35 100644 --- a/src/System.Net.Security/src/System/Net/HelperAsyncResults.cs +++ b/src/System.Net.Security/src/System/Net/HelperAsyncResults.cs @@ -21,7 +21,7 @@ namespace System.Net internal class AsyncProtocolRequest { #if DEBUG - internal object _debugAsyncChain; // Optionally used to track chains of async calls. + internal object _DebugAsyncChain; // Optionally used to track chains of async calls. #endif private AsyncProtocolCallback _callback; @@ -34,15 +34,12 @@ internal class AsyncProtocolRequest public LazyAsyncResult UserAsyncResult; public int Result; public object AsyncState; - public readonly CancellationToken CancellationToken; public byte[] Buffer; // Temporary buffer reused by a protocol. public int Offset; public int Count; - public AsyncProtocolRequest(LazyAsyncResult userAsyncResult) : this(userAsyncResult, CancellationToken.None) { } - - public AsyncProtocolRequest(LazyAsyncResult userAsyncResult, CancellationToken cancellationToken) + public AsyncProtocolRequest(LazyAsyncResult userAsyncResult) { if (userAsyncResult == null) { @@ -52,9 +49,7 @@ public AsyncProtocolRequest(LazyAsyncResult userAsyncResult, CancellationToken c { NetEventSource.Fail(this, "userAsyncResult is already completed."); } - UserAsyncResult = userAsyncResult; - CancellationToken = cancellationToken; } public void Reset(LazyAsyncResult userAsyncResult) @@ -77,7 +72,7 @@ public void Reset(LazyAsyncResult userAsyncResult) Offset = 0; Count = 0; #if DEBUG - _debugAsyncChain = 0; + _DebugAsyncChain = 0; #endif } diff --git a/src/System.Net.Security/src/System/Net/Security/SslState.cs b/src/System.Net.Security/src/System/Net/Security/SslState.cs index 33e8f5a82b34..2ef9b71add8a 100644 --- a/src/System.Net.Security/src/System/Net/Security/SslState.cs +++ b/src/System.Net.Security/src/System/Net/Security/SslState.cs @@ -547,7 +547,7 @@ internal int CheckOldKeyDecryptedData(byte[] buffer, int offset, int count) // This method assumes that a SSPI context is already in a good shape. // For example it is either a fresh context or already authenticated context that needs renegotiation. // - internal void ProcessAuthentication(LazyAsyncResult lazyResult, CancellationToken cancellationToken) + internal void ProcessAuthentication(LazyAsyncResult lazyResult) { if (Interlocked.Exchange(ref _nestedAuth, 1) == 1) { @@ -560,7 +560,7 @@ internal void ProcessAuthentication(LazyAsyncResult lazyResult, CancellationToke AsyncProtocolRequest asyncRequest = null; if (lazyResult != null) { - asyncRequest = new AsyncProtocolRequest(lazyResult, cancellationToken); + asyncRequest = new AsyncProtocolRequest(lazyResult); asyncRequest.Buffer = null; #if DEBUG lazyResult._debugAsyncChain = asyncRequest; @@ -639,13 +639,6 @@ internal void ReplyOnReAuthentication(byte[] buffer) // private void ForceAuthentication(bool receiveFirst, byte[] buffer, AsyncProtocolRequest asyncRequest) { - if (asyncRequest != null && asyncRequest.CancellationToken.IsCancellationRequested) - { - // Cancel async operation, before I/O starts. - asyncRequest.CompleteUserWithError(new OperationCanceledException(asyncRequest.CancellationToken)); - return; - } - if (CheckEnqueueHandshake(buffer, asyncRequest)) { // Async handshake is enqueued and will resume later. @@ -750,13 +743,6 @@ internal void InternalEndProcessAuthentication(LazyAsyncResult lazyResult) // private void StartSendBlob(byte[] incoming, int count, AsyncProtocolRequest asyncRequest) { - if (asyncRequest != null && asyncRequest.CancellationToken.IsCancellationRequested) - { - // Cancel async operation if cancellation requested. - asyncRequest.CompleteUserWithError(new OperationCanceledException(asyncRequest.CancellationToken)); - return; - } - ProtocolToken message = Context.NextMessage(incoming, 0, count); _securityStatus = message.Status; @@ -783,7 +769,7 @@ private void StartSendBlob(byte[] incoming, int count, AsyncProtocolRequest asyn else { asyncRequest.AsyncState = message; - Task t = InnerStream.WriteAsync(message.Payload, 0, message.Size, asyncRequest.CancellationToken); + Task t = InnerStream.WriteAsync(message.Payload, 0, message.Size); if (t.IsCompleted) { t.GetAwaiter().GetResult(); @@ -794,7 +780,7 @@ private void StartSendBlob(byte[] incoming, int count, AsyncProtocolRequest asyn if (!ar.CompletedSynchronously) { #if DEBUG - asyncRequest._debugAsyncChain = ar; + asyncRequest._DebugAsyncChain = ar; #endif return; } @@ -811,13 +797,6 @@ private void StartSendBlob(byte[] incoming, int count, AsyncProtocolRequest asyn // private void CheckCompletionBeforeNextReceive(ProtocolToken message, AsyncProtocolRequest asyncRequest) { - if (asyncRequest != null && asyncRequest.CancellationToken.IsCancellationRequested) - { - // Cancel async operation if cancellation requested. - asyncRequest.CompleteUserWithError(new OperationCanceledException(asyncRequest.CancellationToken)); - return; - } - if (message.Failed) { StartSendAuthResetSignal(null, asyncRequest, ExceptionDispatchInfo.Capture(new AuthenticationException(SR.net_auth_SSPI, message.GetException()))); @@ -847,12 +826,6 @@ private void CheckCompletionBeforeNextReceive(ProtocolToken message, AsyncProtoc // private void StartReceiveBlob(byte[] buffer, AsyncProtocolRequest asyncRequest) { - if (asyncRequest != null && asyncRequest.CancellationToken.IsCancellationRequested) - { - asyncRequest.CompleteUserWithError(new OperationCanceledException(asyncRequest.CancellationToken)); - return; - } - if (_pendingReHandshake) { if (CheckEnqueueHandshakeRead(ref buffer, asyncRequest)) @@ -868,7 +841,7 @@ private void StartReceiveBlob(byte[] buffer, AsyncProtocolRequest asyncRequest) } } - // This is first server read. + //This is first server read. buffer = EnsureBufferSize(buffer, 0, SecureChannel.ReadHeaderSize); int readBytes = 0; @@ -891,14 +864,9 @@ private void StartReceiveBlob(byte[] buffer, AsyncProtocolRequest asyncRequest) StartReadFrame(buffer, readBytes, asyncRequest); } + // private void StartReadFrame(byte[] buffer, int readBytes, AsyncProtocolRequest asyncRequest) { - if (asyncRequest != null && asyncRequest.CancellationToken.IsCancellationRequested) - { - asyncRequest.CompleteUserWithError(new OperationCanceledException(asyncRequest.CancellationToken)); - return; - } - if (readBytes == 0) { // EOF received @@ -945,18 +913,11 @@ private void StartReadFrame(byte[] buffer, int readBytes, AsyncProtocolRequest a readBytes = 0; } } - ProcessReceivedBlob(buffer, readBytes + restBytes, asyncRequest); } private void ProcessReceivedBlob(byte[] buffer, int count, AsyncProtocolRequest asyncRequest) { - if (asyncRequest != null && asyncRequest.CancellationToken.IsCancellationRequested) - { - asyncRequest.CompleteUserWithError(new OperationCanceledException(asyncRequest.CancellationToken)); - return; - } - if (count == 0) { // EOF received. @@ -1021,7 +982,7 @@ private void StartSendAuthResetSignal(ProtocolToken message, AsyncProtocolReques else { asyncRequest.AsyncState = exception; - Task t = InnerStream.WriteAsync(message.Payload, 0, message.Size, asyncRequest.CancellationToken); + Task t = InnerStream.WriteAsync(message.Payload, 0, message.Size); if (t.IsCompleted) { t.GetAwaiter().GetResult(); @@ -1109,7 +1070,6 @@ private static void WriteCallback(IAsyncResult transportResult) exception.Throw(); } - // Not allowing cancellation in the callback. sslState.CheckCompletionBeforeNextReceive((ProtocolToken)asyncState, asyncRequest); } catch (Exception e) diff --git a/src/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/System.Net.Security/src/System/Net/Security/SslStream.cs index 7c64b7e6cd72..aa30e5e3ca20 100644 --- a/src/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -185,7 +185,7 @@ internal virtual IAsyncResult BeginAuthenticateAsClient(SslClientAuthenticationO _sslState.ValidateCreateContext(sslClientAuthenticationOptions); LazyAsyncResult result = new LazyAsyncResult(_sslState, asyncState, asyncCallback); - _sslState.ProcessAuthentication(result, cancellationToken); + _sslState.ProcessAuthentication(result); return result; } @@ -239,7 +239,7 @@ private IAsyncResult BeginAuthenticateAsServer(SslServerAuthenticationOptions ss _sslState.ValidateCreateContext(sslServerAuthenticationOptions); LazyAsyncResult result = new LazyAsyncResult(_sslState, asyncState, asyncCallback); - _sslState.ProcessAuthentication(result, cancellationToken); + _sslState.ProcessAuthentication(result); return result; } @@ -307,7 +307,7 @@ private void AuthenticateAsClient(SslClientAuthenticationOptions sslClientAuthen sslClientAuthenticationOptions._certSelectionDelegate = _certSelectionDelegate; _sslState.ValidateCreateContext(sslClientAuthenticationOptions); - _sslState.ProcessAuthentication(null, CancellationToken.None); + _sslState.ProcessAuthentication(null); } public virtual void AuthenticateAsServer(X509Certificate serverCertificate) @@ -343,7 +343,7 @@ private void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthen sslServerAuthenticationOptions._certValidationDelegate = _certValidationDelegate; _sslState.ValidateCreateContext(sslServerAuthenticationOptions); - _sslState.ProcessAuthentication(null, CancellationToken.None); + _sslState.ProcessAuthentication(null); } #endregion diff --git a/src/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs b/src/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs index 07e02c440c3c..169eaa00deca 100644 --- a/src/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs +++ b/src/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs @@ -60,60 +60,6 @@ protected bool AllowAnyServerCertificate( } } - [Fact] - public void SslStream_StreamToStream_ClientCancellation_Throws() - { - VirtualNetwork network = new VirtualNetwork(); - using (var clientStream = new VirtualNetworkStream(network, isServer: false)) - using (var serverStream = new VirtualNetworkStream(network, isServer: true)) - using (var client = new SslStream(clientStream)) - using (var server = new SslStream(serverStream)) - using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) - { - SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions(); - clientOptions.RemoteCertificateValidationCallback = AllowAnyServerCertificate; - clientOptions.TargetHost = certificate.GetNameInfo(X509NameType.SimpleName, false); - - SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions(); - serverOptions.ServerCertificate = certificate; - - CancellationTokenSource cts = new CancellationTokenSource(); - cts.Cancel(); - - Task clientTask = Assert.ThrowsAnyAsync(() => client.AuthenticateAsClientAsync(clientOptions, cts.Token)); - Task serverTask = Assert.ThrowsAsync(() => server.AuthenticateAsServerAsync(serverOptions, CancellationToken.None)); - - Assert.True(Task.WaitAll(new[] { clientTask, serverTask }, TestConfiguration.PassingTestTimeoutMilliseconds)); - } - } - - [Fact] - public void SslStream_StreamToStream_ServerCancellation_Throws() - { - VirtualNetwork network = new VirtualNetwork(); - using (var clientStream = new VirtualNetworkStream(network, isServer: false)) - using (var serverStream = new VirtualNetworkStream(network, isServer: true)) - using (var client = new SslStream(clientStream)) - using (var server = new SslStream(serverStream)) - using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) - { - SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions(); - clientOptions.RemoteCertificateValidationCallback = AllowAnyServerCertificate; - clientOptions.TargetHost = certificate.GetNameInfo(X509NameType.SimpleName, false); - - SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions(); - serverOptions.ServerCertificate = certificate; - - CancellationTokenSource cts = new CancellationTokenSource(); - cts.Cancel(); - - Task clientTask = Assert.ThrowsAsync(() => client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None)); - Task serverTask = Assert.ThrowsAnyAsync(() => server.AuthenticateAsServerAsync(serverOptions, cts.Token)); - - Assert.True(Task.WaitAll(new[] { clientTask, serverTask }, TestConfiguration.PassingTestTimeoutMilliseconds)); - } - } - [Fact] public void SslStream_StreamToStream_DuplicateOptions_Throws() { diff --git a/src/System.Net.Security/tests/UnitTests/Fakes/FakeSslState.cs b/src/System.Net.Security/tests/UnitTests/Fakes/FakeSslState.cs index bb32bb901a2f..98f481df35cb 100644 --- a/src/System.Net.Security/tests/UnitTests/Fakes/FakeSslState.cs +++ b/src/System.Net.Security/tests/UnitTests/Fakes/FakeSslState.cs @@ -183,7 +183,7 @@ internal void Close() // This method assumes that a SSPI context is already in a good shape. // For example it is either a fresh context or already authenticated context that needs renegotiation. // - internal void ProcessAuthentication(LazyAsyncResult lazyResult, CancellationToken cancellationToken) + internal void ProcessAuthentication(LazyAsyncResult lazyResult) { }