Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Reverts Cancellation on SslStream #25199

Closed
Drawaes wants to merge 1 commit into from
Closed

Reverts Cancellation on SslStream #25199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/System.Net.Security/src/System/Net/FixedSizeReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 3 additions & 8 deletions src/System.Net.Security/src/System/Net/HelperAsyncResults.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
{
Expand All @@ -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)
Expand All @@ -77,7 +72,7 @@ public void Reset(LazyAsyncResult userAsyncResult)
Offset = 0;
Count = 0;
#if DEBUG
_debugAsyncChain = 0;
_DebugAsyncChain = 0;
#endif
}

Expand Down
54 changes: 7 additions & 47 deletions src/System.Net.Security/src/System/Net/Security/SslState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -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();
Expand All @@ -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;
}
Expand All @@ -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())));
Expand Down Expand Up @@ -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))
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions src/System.Net.Security/src/System/Net/Security/SslStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -343,7 +343,7 @@ private void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthen
sslServerAuthenticationOptions._certValidationDelegate = _certValidationDelegate;

_sslState.ValidateCreateContext(sslServerAuthenticationOptions);
_sslState.ProcessAuthentication(null, CancellationToken.None);
_sslState.ProcessAuthentication(null);
}
#endregion

Expand Down
54 changes: 0 additions & 54 deletions src/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<OperationCanceledException>(() => client.AuthenticateAsClientAsync(clientOptions, cts.Token));
Task serverTask = Assert.ThrowsAsync<TimeoutException>(() => 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<TimeoutException>(() => client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None));
Task serverTask = Assert.ThrowsAnyAsync<OperationCanceledException>(() => server.AuthenticateAsServerAsync(serverOptions, cts.Token));

Assert.True(Task.WaitAll(new[] { clientTask, serverTask }, TestConfiguration.PassingTestTimeoutMilliseconds));
}
}

[Fact]
public void SslStream_StreamToStream_DuplicateOptions_Throws()
{
Expand Down
2 changes: 1 addition & 1 deletion src/System.Net.Security/tests/UnitTests/Fakes/FakeSslState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
}

Expand Down