simplify SslStream.AuthenticateAs*Async() #453

wfurt · 2019-12-02T20:47:28Z

This implements task based processing for Async authentication functions instead of using APM wrapper. With this change we do less allocations but benchmarks from performance repo show only small improvement. Main benefit is simplification for now. For example, this is old https exception for certificate validation:

System.Net.Http.HttpRequestException: The SSL connection could not be established, see inner exception.
 ---> System.Security.Authentication.AuthenticationException: The remote certificate is invalid according to the validation procedure.
   at System.Net.Security.SslStream.StartSendAuthResetSignal(ProtocolToken message, AsyncProtocolRequest asyncRequest, ExceptionDispatchInfo exception) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 710
   at System.Net.Security.SslStream.CheckCompletionBeforeNextReceive(ProtocolToken message, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 546
   at System.Net.Security.SslStream.StartSendBlob(Byte[] incoming, Int32 count, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 527
   at System.Net.Security.SslStream.ProcessReceivedBlob(Byte[] buffer, Int32 count, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 696
   at System.Net.Security.SslStream.StartReadFrame(Byte[] buffer, Int32 readBytes, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 651
   at System.Net.Security.SslStream.StartReceiveBlob(Byte[] buffer, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 599
   at System.Net.Security.SslStream.CheckCompletionBeforeNextReceive(ProtocolToken message, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 556
   at System.Net.Security.SslStream.StartSendBlob(Byte[] incoming, Int32 count, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 527
   at System.Net.Security.SslStream.ProcessReceivedBlob(Byte[] buffer, Int32 count, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 696
   at System.Net.Security.SslStream.StartReadFrame(Byte[] buffer, Int32 readBytes, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 651
   at System.Net.Security.SslStream.PartialFrameCallback(AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 831
--- End of stack trace from previous location where exception was thrown ---
   at System.Net.Security.SslStream.ThrowIfExceptional() in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 82
   at System.Net.Security.SslStream.InternalEndProcessAuthentication(LazyAsyncResult lazyResult) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 472
   at System.Net.Security.SslStream.EndProcessAuthentication(IAsyncResult result) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 442
   at System.Net.Security.SslStream.EndAuthenticateAsClient(IAsyncResult asyncResult) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.cs:line 237
   at System.Net.Security.SslStream.<>c.<AuthenticateAsClientAsync>b__65_1(IAsyncResult iar) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.cs:line 404
   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location where exception was thrown ---
   at System.Net.Http.ConnectHelper.EstablishSslConnectionAsyncCore(Stream stream, SslClientAuthenticationOptions sslOptions, CancellationToken cancellationToken) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs:line 145
   --- End of inner exception stack trace ---
   at System.Net.Http.ConnectHelper.EstablishSslConnectionAsyncCore(Stream stream, SslClientAuthenticationOptions sslOptions, CancellationToken cancellationToken) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs:line 161
   at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage request, Boolean allowHttp2, CancellationToken cancellationToken) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs:line 651
   at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs:line 666
   at System.Net.Http.HttpConnectionPool.GetHttpConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs:line 332
   at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs:line 524
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs:line 33
   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/HttpClient.cs:line 521
   at proxy_test.Program.fetch2(String uri) in /Users/furt/proxy-test/Program.cs:line 39
   at proxy_test.Program.Main(String[] args) in /Users/furt/proxy-test/Program.cs:line 55INNED ==== System.Security.Authentication.AuthenticationException: The remote certificate is invalid according to the validation procedure.
   at System.Net.Security.SslStream.StartSendAuthResetSignal(ProtocolToken message, AsyncProtocolRequest asyncRequest, ExceptionDispatchInfo exception) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 710
   at System.Net.Security.SslStream.CheckCompletionBeforeNextReceive(ProtocolToken message, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 546
   at System.Net.Security.SslStream.StartSendBlob(Byte[] incoming, Int32 count, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 527
   at System.Net.Security.SslStream.ProcessReceivedBlob(Byte[] buffer, Int32 count, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 696
   at System.Net.Security.SslStream.StartReadFrame(Byte[] buffer, Int32 readBytes, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 651
   at System.Net.Security.SslStream.StartReceiveBlob(Byte[] buffer, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 599
   at System.Net.Security.SslStream.CheckCompletionBeforeNextReceive(ProtocolToken message, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 556
   at System.Net.Security.SslStream.StartSendBlob(Byte[] incoming, Int32 count, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 527
   at System.Net.Security.SslStream.ProcessReceivedBlob(Byte[] buffer, Int32 count, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 696
   at System.Net.Security.SslStream.StartReadFrame(Byte[] buffer, Int32 readBytes, AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 651
   at System.Net.Security.SslStream.PartialFrameCallback(AsyncProtocolRequest asyncRequest) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 831
--- End of stack trace from previous location where exception was thrown ---
   at System.Net.Security.SslStream.ThrowIfExceptional() in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 82
   at System.Net.Security.SslStream.InternalEndProcessAuthentication(LazyAsyncResult lazyResult) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 472
   at System.Net.Security.SslStream.EndProcessAuthentication(IAsyncResult result) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 442
   at System.Net.Security.SslStream.EndAuthenticateAsClient(IAsyncResult asyncResult) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.cs:line 237
   at System.Net.Security.SslStream.<>c.<AuthenticateAsClientAsync>b__65_1(IAsyncResult iar) in /Users/furt/github/corefx-30/src/System.Net.Security/src/System/Net/Security/SslStream.cs:line 404
   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location where exception was thrown ---
   at System.Net.Http.ConnectHelper.EstablishSslConnectionAsyncCore(Stream stream, SslClientAuthenticationOptions sslOptions, CancellationToken cancellationToken) in /Users/furt/github/corefx-30/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs:line 145
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at proxy_test.Program.Main(String[] args) in /Users/furt/proxy-test/Program.cs:line 73

same situation with new code it looks like:

System.Net.Http.HttpRequestException: The SSL connection could not be established, see inner exception.
 ---> System.Security.Authentication.AuthenticationException: The remote certificate is invalid according to the validation procedure.
   at System.Net.Security.SslStream.StartSendAuthResetSignal(ProtocolToken message, AsyncProtocolRequest asyncRequest, ExceptionDispatchInfo exception) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 794
   at System.Net.Security.SslStream.ForceAuthenticationAsync(Boolean receiveFirst, Byte[] buffer, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 426
   at System.Net.Http.ConnectHelper.EstablishSslConnectionAsyncCore(Stream stream, SslClientAuthenticationOptions sslOptions, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs:line 146
   --- End of inner exception stack trace ---
   at System.Net.Http.ConnectHelper.EstablishSslConnectionAsyncCore(Stream stream, SslClientAuthenticationOptions sslOptions, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs:line 162
   at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage request, Boolean allowHttp2, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs:line 650
   at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs:line 665
   at System.Net.Http.HttpConnectionPool.GetHttpConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs:line 331
   at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs:line 523
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs:line 33
   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs:line 521
   at proxy_test.Program.fetch2(String uri) in /Users/furt/proxy-test/Program.cs:line 39
   at proxy_test.Program.Main(String[] args) in /Users/furt/proxy-test/Program.cs:line 55INNED ==== System.Security.Authentication.AuthenticationException: The remote certificate is invalid according to the validation procedure.
   at System.Net.Security.SslStream.StartSendAuthResetSignal(ProtocolToken message, AsyncProtocolRequest asyncRequest, ExceptionDispatchInfo exception) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 794
   at System.Net.Security.SslStream.ForceAuthenticationAsync(Boolean receiveFirst, Byte[] buffer, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 426
   at System.Net.Http.ConnectHelper.EstablishSslConnectionAsyncCore(Stream stream, SslClientAuthenticationOptions sslOptions, CancellationToken cancellationToken) in /Users/furt/github/wfurt-runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs:line 146
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at proxy_test.Program.Main(String[] args) in /Users/furt/proxy-test/Program.cs:line 73

there is more cleanup we can do but this is starting point to simplify processing.
As general improvements I updated handshake to avoid reading 5 byte chunks and context switch and use Span when possible. I plan to update 1 byte queue for OSX but I did not want to do it as part of this change.

Early feedback would be appreciated.

When working on this I bump to few test failures. Most notably WaitForSecureConnection used in CipherSuiteTest makes assumption that certain failures will happen synchronously.
I don't know if that is artifact of old implementation or if that is something we have to preserve. For now I updated test to handle asynchronous behavior. cc: @krwq

While looking at test failures I bump to few using memory stream. That makes it impossible to use Wireshark and decode and display message exchange. I added helper method and re-factored some existing tests so it is possible on-demand to use stream on top of TCP.

fixes https://github.com/dotnet/corefx/issues/35075

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs

src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs

src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs

src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs

…35074

…35075

wfurt · 2019-12-03T22:12:54Z

I removed more dead code and all tests should be passing now.
I think it would be worth of looking at locking logic - I think we may be able to simplify it but I would prefer to do it as separate change.

…sl_35075

wfurt · 2019-12-03T22:17:15Z

/azp run runtime-libraries outerloop

azure-pipelines · 2019-12-03T22:17:41Z

Azure Pipelines successfully started running 1 pipeline(s).

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs

src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs

src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs

wfurt · 2019-12-12T06:02:40Z

/azp run runtime-libraries outerloop

azure-pipelines · 2019-12-12T06:02:55Z

Azure Pipelines successfully started running 1 pipeline(s).

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs

wfurt · 2019-12-12T18:49:38Z

/azp run runtime-libraries stress-ssl

azure-pipelines · 2019-12-12T18:49:48Z

Azure Pipelines successfully started running 1 pipeline(s).

wfurt · 2019-12-12T21:07:58Z

/azp run runtime-libraries stress-ssl

azure-pipelines · 2019-12-12T21:08:08Z

Azure Pipelines successfully started running 1 pipeline(s).

…35075

wfurt · 2019-12-16T21:00:07Z

/azp run runtime-libraries stress-ssl

azure-pipelines · 2019-12-16T21:00:17Z

Azure Pipelines successfully started running 1 pipeline(s).

…35075

wfurt · 2019-12-17T22:35:24Z

/azp run runtime-libraries stress-ssl

azure-pipelines · 2019-12-17T22:35:33Z

Azure Pipelines successfully started running 1 pipeline(s).

wfurt · 2019-12-18T18:52:48Z

Windows_NT failures seems like infrastructure.

port from corefx

2df9af9

wfurt added the area-System.Net.Security label Dec 2, 2019

wfurt requested review from krwq and stephentoub December 2, 2019 20:47

add TestHelper.cs

266d05c

stephentoub reviewed Dec 2, 2019

View reviewed changes

capture handshake exception

6423ab7

wfurt mentioned this pull request Dec 3, 2019

improve renegotiation ssl tests #470

Merged

wfurt added 5 commits December 3, 2019 10:27

Merge branch 'master' of https://github.com/dotnet/runtime into assl_…

26d19dc

…35074

feedback from review

206b3a2

cleanup more apm and renegotiation

afac8d4

fix unit test

dac0457

Merge branch 'master' of https://github.com/dotnet/runtime into assl_…

ce380c2

…35075

wfurt changed the title ~~WIP: simplify SslStream.AuthenticateAs*Async()~~ simplify SslStream.AuthenticateAs*Async() Dec 3, 2019

wfurt requested review from bartonjs and a team December 3, 2019 22:05

wfurt added 2 commits December 3, 2019 14:15

feedback from review

bdda9f7

Merge branch 'assl_35075' of https://github.com/wfurt/runtime into as…

1a10d3c

…sl_35075

krwq reviewed Dec 3, 2019

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs Outdated Show resolved Hide resolved

feedback from review

abf2513