Remove a lot of allocation from SslStream's handshake (dotnet/corefx#35091)
authorStephen Toub <stoub@microsoft.com>
Fri, 8 Feb 2019 20:09:50 +0000 (15:09 -0500)
committerGitHub <noreply@github.com>
Fri, 8 Feb 2019 20:09:50 +0000 (15:09 -0500)
commit8ec5a72b49d44274becd9aea7a4db9eda39ad206
treeae295561dbbbe9053b5abb1e455648c0ff51c65e
parent21f7a38f6ed56a53da0518df973152b718842611
Remove a lot of allocation from SslStream's handshake (dotnet/corefx#35091)

* Remove a lot of allocation from SslStream's handshake

This makes a sizeable dent in the allocation incurred as part of SslStream's handshake (AuthenticateAsClient/Server{Async}), though mainly on Windows; many of these changes also impact Unix, but on Unix there's also substantially more allocation, so the relevant percentage impact is less.

Main changes:
- X509Certificate2.RawData.  This was being accessed by SslStream in order to store the byte[] away in case the RemoteCertificate property was accessed, in which case it would use it to instantiate an X509Certificate2.  That, however, was resulting in lots of sizeable byte[]s being allocated.  We can instead just instantiate an X509Certificate2 from the original, and while that entails a few additional small allocations, it's a sizeable reduction, and we'd eventually need the cert anyway if the property is accessed.
- X509ChainPolicy.  Every time Reset was called, it would allocate several new collections, but they often weren't used.  Made them lazy.
- X509 object finalization.  A bunch of X509-related objects were being left for finalization.  Those are now aggressively disposed.
- ChainPal.GetChainStatusInformation: every call was P/Invoke'ing to get a string error message for a handful of prechosen error codes... we can just get the strings once upfront and avoid allocating them each time.
- OidCollections.  A bunch of these are created but not actually populated, and each time one is created it was allocating a new List.  Changed to just have it be built around an array.
- SecPkgContext classes.  These have been converted to blittable structs, avoiding the associated allocations and marshaling costs, e.g. SecPkgContext_ApplicationProtocol would end up allocating a byte[] along with the instance itself.
- PtrToStructure.  When QueryContextAttributes was being called, in many cases PtrToStructure was being used.  This has been cleaned up, to avoid the marshaling and just use spans / casting.
- SecurityBuffer[] arrays.  Lots of code was written in terms of SecurityBuffer[], which meant lots of operations allocating these.  However, they were never larger than three items.  We now just stack allocate spans for them.
- Overloads taking both SecurityBuffer and SecurityBuffer[].  I believe because of the arrays, there were some cases where methods would take both a SecurityBuffer argument and a SecurityBuffer[] argument, with one and only one non-null.  Now that we're just passing around allocation-free spans, this has been cleaned up.
- GCHandles and GCHandle[].  Multiple methods were allocating arrays of GCHandles, and then one GCHandle.Alloc (not a managed allocation, but also not free) per SecurityBuffer.  Now that we're known to be limited to at most three, we just used fixed (even if we don't actually have three), avoiding the need for the arrays and for the GCHandle.Alloc, which is more expensive than fixed.
- SecBuffer[] arrays.  Several places were allocating a SecBuffer[]; we can just stackalloc a struct.  These are all limited by the number of SecurityBuffers passed around, which is inherently small (<= 3 in the current code).
- SecureChannel.EnsurePrivateKey.  It was always forcing a certificate's Thumbpring into existence, even though it was rarely needed.
- SecureChannel.AcquireClientCredentials.  It was always forcing a List into existence, even if it wasn't needed.
- SslAuthenticationOptions.  This internal type had several fields for delegates that were never used.  Removed.
- Span.ToArray.  In several places utilize span to create an array from a pointer/length rather than allocating the array and Marshal.Copy'ing.  This is mostly for readability.
- Also cleaned up a few things, like using ?.Dispose across System.Net.Security.
- Protocol version strings.  On Unix we were getting the protocol version constant string from the native string, allocating a managed string for it, and then comparing it against the set of known constant strings.  We don't need to do that.

There's still more that can be done.  On Windows, the two big remaining sources of allocation are for byte[]s created for data to send/receive in the handshake, and that will hopefully be addressed by work to combine SslStream/SslStreamInternal/SslState and to rewrite them to use async/await, and the allocation of the actual SecurityBuffers, which should be fairly straightforward to convert to structs (albeit with some care, as they're mutated as they're passed around).  On Unix, there's a ton more allocation, in particular as part of X509Chain building, and that should be investigated separately as part of an effort to improve performance there.

* Move SecurityBuffer to be Windows-only and make it a struct

* Rename several files

* Fix netstandard build of SqlClient

Commit migrated from https://github.com/dotnet/corefx/commit/3c30357d51b96339021b4ee52f38445c31248bb2
50 files changed:
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs
src/libraries/Common/src/Interop/Windows/SChannel/Interop.SecPkgContext_ApplicationProtocol.cs
src/libraries/Common/src/Interop/Windows/SChannel/SecPkgContext_ConnectionInfo.cs
src/libraries/Common/src/Interop/Windows/SspiCli/GlobalSSPI.cs
src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs
src/libraries/Common/src/Interop/Windows/SspiCli/NegotiationInfoClass.cs
src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs
src/libraries/Common/src/Interop/Windows/SspiCli/SSPIInterface.cs
src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs
src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs
src/libraries/Common/src/Interop/Windows/SspiCli/SecPkgContext_Sizes.cs
src/libraries/Common/src/Interop/Windows/SspiCli/SecPkgContext_StreamSizes.cs
src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs
src/libraries/Common/src/System/Net/LazyAsyncResult.cs
src/libraries/Common/src/System/Net/NTAuthentication.Common.cs
src/libraries/Common/src/System/Net/NegotiationInfoClass.cs
src/libraries/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs
src/libraries/Common/src/System/Net/Security/NegotiateStreamPal.Windows.cs
src/libraries/Common/src/System/Net/Security/SSPIHandleCache.cs
src/libraries/Common/src/System/Net/Security/SecurityBuffer.Windows.cs [moved from src/libraries/Common/src/System/Net/Security/SecurityBuffer.cs with 58% similarity]
src/libraries/Common/src/System/Net/Security/SecurityBufferType.Windows.cs [moved from src/libraries/Common/src/System/Net/Security/SecurityBufferType.cs with 100% similarity]
src/libraries/System.Data.SqlClient/src/System.Data.SqlClient.csproj
src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIProxy.cs
src/libraries/System.Net.Http/src/System.Net.Http.csproj
src/libraries/System.Net.HttpListener/src/System.Net.HttpListener.csproj
src/libraries/System.Net.Mail/src/System.Net.Mail.csproj
src/libraries/System.Net.Security/src/System.Net.Security.csproj
src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs
src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs
src/libraries/System.Net.Security/src/System/Net/FixedSizeReader.cs
src/libraries/System.Net.Security/src/System/Net/Security/InternalNegotiateStream.cs
src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStreamPal.Windows.cs
src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslConnectionInfo.Unix.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslConnectionInfo.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslState.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamInternal.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs
src/libraries/System.Security.Cryptography.Encoding/src/System/Security/Cryptography/OidCollection.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/ChainPal.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/ChainPal.GetChainStatusInformation.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509Certificate.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509ChainPolicy.cs