From 58a4493daf6307eccd28feab3a71387aa9dc02ca Mon Sep 17 00:00:00 2001 From: wfurt Date: Sun, 14 Jun 2020 20:12:01 -0700 Subject: [PATCH 1/6] use new API on new windows to get TLS13 --- .../Interop/Windows/SspiCli/ISSPIInterface.cs | 1 + .../Interop/Windows/SspiCli/Interop.SSPI.cs | 105 ++++++++++++++++ .../Interop/Windows/SspiCli/SSPIAuthType.cs | 5 + .../Windows/SspiCli/SSPISecureChannelType.cs | 5 + .../Interop/Windows/SspiCli/SSPIWrapper.cs | 25 ++++ .../Windows/SspiCli/SecuritySafeHandles.cs | 54 ++++++++- .../src/System.Net.Http.csproj | 4 +- .../src/System.Net.HttpListener.csproj | 2 + .../src/System.Net.Mail.csproj | 2 + .../src/System.Net.Security.csproj | 4 + .../Net/Security/SslStreamPal.Windows.cs | 114 ++++++++++++++++++ .../FunctionalTests/SslStreamAlertsTest.cs | 23 ++-- .../tests/FunctionalTests/TestHelper.cs | 1 + 13 files changed, 336 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs index ee81875cee8354..71afaecff54458 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs @@ -15,6 +15,7 @@ internal interface ISSPIInterface int EnumerateSecurityPackages(out int pkgnum, out SafeFreeContextBuffer pkgArray); int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref SafeSspiAuthDataHandle authdata, out SafeFreeCredentials outCredential); int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCHANNEL_CRED authdata, out SafeFreeCredentials outCredential); + int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCH_CREDENTIALS authdata, out SafeFreeCredentials outCredential); int AcquireDefaultCredential(string moduleName, Interop.SspiCli.CredentialUse usage, out SafeFreeCredentials outCredential); int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags); int InitializeSecurityContext(ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, InputSecurityBuffers inputBuffers, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags); diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs index 61ca04f896cddd..a80333b159dc80 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs @@ -215,6 +215,97 @@ public enum Flags } } + [StructLayout(LayoutKind.Sequential)] + internal unsafe struct SCH_CREDENTIALS + { + public const int CurrentVersion = 0x5; + + public int dwVersion; + public int dwCredformat; + public int cCreds; + + // ptr to an array of pointers + // There is a hack done with this field. AcquireCredentialsHandle requires an array of + // certificate handles; we only ever use one. In order to avoid pinning a one element array, + // we copy this value onto the stack, create a pointer on the stack to the copied value, + // and replace this field with the pointer, during the call to AcquireCredentialsHandle. + // Then we fix it up afterwards. Fine as long as all the SSPI credentials are not + // supposed to be threadsafe. + public IntPtr paCred; + + public IntPtr hRootStore; // == always null, OTHERWISE NOT RELIABLE + public int cMappers; + public IntPtr aphMappers; // == always null, OTHERWISE NOT RELIABLE + + public int dwSessionLifespan; + public SCH_CREDENTIALS.Flags dwFlags; + public int cTlsParameters; + public TLS_PARAMETERS* pTlsParameters; + + [Flags] + public enum Flags + { + Zero = 0, + SCH_CRED_NO_SYSTEM_MAPPER = 0x02, + SCH_CRED_NO_SERVERNAME_CHECK = 0x04, + SCH_CRED_MANUAL_CRED_VALIDATION = 0x08, + SCH_CRED_NO_DEFAULT_CREDS = 0x10, + SCH_CRED_AUTO_CRED_VALIDATION = 0x20, + SCH_CRED_USE_DEFAULT_CREDS = 0x40, + SCH_DISABLE_RECONNECTS = 0x80, + SCH_CRED_REVOCATION_CHECK_END_CERT = 0x100, + SCH_CRED_REVOCATION_CHECK_CHAIN = 0x200, + SCH_CRED_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT = 0x400, + SCH_CRED_IGNORE_NO_REVOCATION_CHECK = 0x800, + SCH_CRED_IGNORE_REVOCATION_OFFLINE = 0x1000, + SCH_CRED_CACHE_ONLY_URL_RETRIEVAL_ON_CREATE = 0x2000, + SCH_SEND_ROOT_CERT = 0x40000, + SCH_SEND_AUX_RECORD = 0x00200000, + SCH_USE_STRONG_CRYPTO = 0x00400000, + SCH_USE_PRESHAREDKEY_ONLY = 0x800000, + SCH_ALLOW_NULL_ENCRYPTION = 0x02000000, + } + } + + [StructLayout(LayoutKind.Sequential)] + internal unsafe struct TLS_PARAMETERS + { + public int cAlpnIds; // Valid for server applications only. Must be zero otherwise. Number of ALPN IDs in rgstrAlpnIds; set to 0 if applies to all. + public IntPtr rgstrAlpnIds; // Valid for server applications only. Must be NULL otherwise. Array of ALPN IDs that the following settings apply to; set to NULL if applies to all. + public uint grbitDisabledProtocols; // List protocols you DO NOT want negotiated. + public int cDisabledCrypto; // Number of CRYPTO_SETTINGS structures; set to 0 if there are none. + public CRYPTO_SETTINGS* pDisabledCrypto; // Array of CRYPTO_SETTINGS structures; set to NULL if there are none; + public TLS_PARAMETERS.Flags dwFlags; // Optional flags to pass; set to 0 if there are none. + + [Flags] + public enum Flags + { + Zero = 0, + TLS_PARAMS_OPTIONAL = 0x01, // Valid for server applications only. Must be zero otherwise. + // TLS_PARAMETERS that will only be honored if they do not cause this server to terminate the handshake. + } + } + + [StructLayout(LayoutKind.Sequential)] + internal unsafe struct CRYPTO_SETTINGS + { + public TlsAlgorithmUsage eAlgorithmUsage; // How this algorithm is being used. + public UNICODE_STRING* strCngAlgId; // CNG algorithm identifier. + public int cChainingModes; // Set to 0 if CNG algorithm does not have a chaining mode. + public UNICODE_STRING* rgstrChainingModes; // Set to NULL if CNG algorithm does not have a chaining mode. + public int dwMinBitLength; // Blacklist key sizes less than this. Set to 0 if not defined or CNG algorithm implies bit length. + public int dwMaxBitLength; // Blacklist key sizes greater than this. Set to 0 if not defined or CNG algorithm implies bit length. + + public enum TlsAlgorithmUsage + { + TlsParametersCngAlgUsageKeyExchange, // Key exchange algorithm. RSA, ECHDE, DHE, etc. + TlsParametersCngAlgUsageSignature, // Signature algorithm. RSA, DSA, ECDSA, etc. + TlsParametersCngAlgUsageCipher, // Encryption algorithm. AES, DES, RC4, etc. + TlsParametersCngAlgUsageDigest, // Digest of cipher suite. SHA1, SHA256, SHA384, etc. + TlsParametersCngAlgUsageCertSig // Signature and/or hash used to sign certificate. RSA, DSA, ECDSA, SHA1, SHA256, etc. + } + } + [StructLayout(LayoutKind.Sequential)] internal unsafe struct SecBuffer { @@ -345,6 +436,20 @@ internal static extern unsafe int AcquireCredentialsHandleW( [Out] out long timeStamp ); + [DllImport(Interop.Libraries.SspiCli, ExactSpelling = true, CharSet = CharSet.Unicode, SetLastError = true)] + internal static extern unsafe int AcquireCredentialsHandleW( + [In] string? principal, + [In] string moduleName, + [In] int usage, + [In] void* logonID, + [In] ref SCH_CREDENTIALS authData, + [In] void* keyCallback, + [In] void* keyArgument, + ref CredHandle handlePtr, + [Out] out long timeStamp + ); + + [DllImport(Interop.Libraries.SspiCli, ExactSpelling = true, SetLastError = true)] internal static extern unsafe int InitializeSecurityContextW( ref CredHandle credentialHandle, diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs index f5a06cc9b65f72..3d86fd48a96062 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs @@ -46,6 +46,11 @@ public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.Credentia return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); } + public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCH_CREDENTIALS authdata, out SafeFreeCredentials outCredential) + { + return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); + } + public int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags) { return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, inputBuffers, ref outputBuffer, ref outFlags); diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs index ff73238161bab4..a44639e05a1701 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs @@ -46,6 +46,11 @@ public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.Credentia return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); } + public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCH_CREDENTIALS authdata, out SafeFreeCredentials outCredential) + { + return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); + } + public int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags) { return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, inputBuffers, ref outputBuffer, ref outFlags); diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs index a0d73e1a4bc4fb..7ce94e50764932 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs @@ -141,6 +141,31 @@ public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secMod return outCredential; } + public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secModule, string package, Interop.SspiCli.CredentialUse intent, Interop.SspiCli.SCH_CREDENTIALS scc) + { + if (NetEventSource.IsEnabled) + { + NetEventSource.Enter(null, package); + NetEventSource.Log.AcquireCredentialsHandle(package, intent, scc); + } + + SafeFreeCredentials? outCredential = null; + int errorCode = secModule.AcquireCredentialsHandle( + package, + intent, + ref scc, + out outCredential); + + if (errorCode != 0) + { + if (NetEventSource.IsEnabled) NetEventSource.Error(null, SR.Format(SR.net_log_operation_failed_with_error, nameof(AcquireCredentialsHandle), $"0x{errorCode:X}")); + throw new Win32Exception(errorCode); + } + + if (NetEventSource.IsEnabled) NetEventSource.Exit(null, outCredential); + return outCredential; + } + internal static int InitializeSecurityContext(ISSPIInterface secModule, ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness datarep, InputSecurityBuffers inputBuffers, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags) { if (NetEventSource.IsEnabled) NetEventSource.Log.InitializeSecurityContext(credential, context, targetName, inFlags); diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs index 382d1f6a2979ed..d1c5c24bf57c36 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs @@ -273,7 +273,6 @@ public static unsafe int AcquireCredentialsHandle( out SafeFreeCredentials outCredential) { if (NetEventSource.IsEnabled) NetEventSource.Enter(null, package, intent, authdata); - int errorCode = -1; long timeStamp; @@ -318,6 +317,59 @@ public static unsafe int AcquireCredentialsHandle( return errorCode; } + + public static unsafe int AcquireCredentialsHandle( + string package, + Interop.SspiCli.CredentialUse intent, + ref Interop.SspiCli.SCH_CREDENTIALS authdata, + out SafeFreeCredentials outCredential) + { + if (NetEventSource.IsEnabled) NetEventSource.Enter(null, package, intent, authdata); + + int errorCode = -1; + long timeStamp; + + // If there is a certificate, wrap it into an array. + // Not threadsafe. + IntPtr copiedPtr = authdata.paCred; + try + { + IntPtr certArrayPtr = new IntPtr(&copiedPtr); + if (copiedPtr != IntPtr.Zero) + { + authdata.paCred = certArrayPtr; + } + + outCredential = new SafeFreeCredential_SECURITY(); + + errorCode = Interop.SspiCli.AcquireCredentialsHandleW( + null, + package, + (int)intent, + null, + ref authdata, + null, + null, + ref outCredential._handle, + out timeStamp); + } + finally + { + authdata.paCred = copiedPtr; + } + +#if TRACE_VERBOSE + if (NetEventSource.IsEnabled) NetEventSource.Info(null, $"{nameof(Interop.SspiCli.AcquireCredentialsHandleW)} returns 0x{errorCode:x}, handle = {outCredential}"); +#endif + + if (errorCode != 0) + { + outCredential.SetHandleAsInvalid(); + } + + return errorCode; + } + } // diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index e7ebd8c441c0d2..cddb4e787c8b4c 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -423,6 +423,8 @@ + - \ No newline at end of file + diff --git a/src/libraries/System.Net.HttpListener/src/System.Net.HttpListener.csproj b/src/libraries/System.Net.HttpListener/src/System.Net.HttpListener.csproj index b27d2d05646c7f..6f8dce6cf43369 100644 --- a/src/libraries/System.Net.HttpListener/src/System.Net.HttpListener.csproj +++ b/src/libraries/System.Net.HttpListener/src/System.Net.HttpListener.csproj @@ -116,6 +116,8 @@ + + diff --git a/src/libraries/System.Net.Security/src/System.Net.Security.csproj b/src/libraries/System.Net.Security/src/System.Net.Security.csproj index d91dfa7d74a16d..cfd28c5df83d35 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -144,6 +144,8 @@ + + = 10 && osvi.dwBuildNumber >= 18836); + } + private const string SecurityPackage = "Microsoft Unified Security Protocol Provider"; private const Interop.SspiCli.ContextFlags RequiredFlags = @@ -107,6 +116,14 @@ public static SecurityStatusPal InitializeSecurityContext(ref SafeFreeCredential } public static SafeFreeCredentials AcquireCredentialsHandle(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) + { + // New crypto API does not allow to force NULL encryption. + return !UseNewCryptoApi || policy == EncryptionPolicy.NoEncryption ? + AcquireCredentialsHandleLegcy(certificate, protocols, policy, isServer) : + AcquireCredentialsHandleNew(certificate, protocols, policy, isServer); + } + + public static SafeFreeCredentials AcquireCredentialsHandleLegcy(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { int protocolFlags = GetProtocolFlagsFromSslProtocols(protocols, isServer); Interop.SspiCli.SCHANNEL_CRED.Flags flags; @@ -145,6 +162,82 @@ public static SafeFreeCredentials AcquireCredentialsHandle(X509Certificate? cert return AcquireCredentialsHandle(direction, secureCredential); } + public static unsafe SafeFreeCredentials AcquireCredentialsHandleNew(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) + { + int protocolFlags = GetProtocolFlagsFromSslProtocols(protocols, isServer); + Interop.SspiCli.SCH_CREDENTIALS.Flags flags; + Interop.SspiCli.CredentialUse direction; + + if (!isServer) + { + direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_OUTBOUND; + flags = + Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_MANUAL_CRED_VALIDATION | + Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_NO_DEFAULT_CREDS | + Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD; + } + else + { + direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND; + flags = Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD; + } + + if (policy == EncryptionPolicy.RequireEncryption) + { + // Always opt-in SCH_USE_STRONG_CRYPTO for TLS. + if (!isServer && ((protocolFlags & Interop.SChannel.SP_PROT_SSL3) == 0)) + { + flags |= Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_USE_STRONG_CRYPTO; + } + } + else if (policy == EncryptionPolicy.AllowNoEncryption) + { + // Allow null encryption cipher in addition to other ciphers. + flags |= Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_ALLOW_NULL_ENCRYPTION; + } + else + { + throw new ArgumentException(SR.Format(SR.net_invalid_enum, "EncryptionPolicy"), nameof(policy)); + } + + Interop.SspiCli.SCH_CREDENTIALS credential = default; + credential.dwVersion = Interop.SspiCli.SCH_CREDENTIALS.CurrentVersion; + credential.dwFlags = flags; + + if (certificate != null) + { + credential.paCred = certificate.Handle; + credential.cCreds = 1; + } + + if (NetEventSource.IsEnabled) NetEventSource.Info($"flags=({flags}), ProtocolFlags=({protocolFlags}), EncryptionPolicy={policy}"); + + if (protocolFlags != 0) + { + // If we were asked to do specific protocol we need to fill TLS_PARAMETERS. + Span tlsParameters = stackalloc Interop.SspiCli.TLS_PARAMETERS[1]; + + tlsParameters[0].cAlpnIds = 0; + tlsParameters[0].rgstrAlpnIds = IntPtr.Zero; + tlsParameters[0].grbitDisabledProtocols = 0; + tlsParameters[0].cDisabledCrypto = 0; + tlsParameters[0].pDisabledCrypto = null; + tlsParameters[0].dwFlags = Interop.SspiCli.TLS_PARAMETERS.Flags.Zero; + tlsParameters[0].grbitDisabledProtocols = (uint)protocolFlags ^ uint.MaxValue; + + fixed (Interop.SspiCli.TLS_PARAMETERS* ptr = tlsParameters) + { + credential.cTlsParameters = 1; + credential.pTlsParameters = ptr; + return AcquireCredentialsHandle(direction, credential); + } + } + else + { + return AcquireCredentialsHandle(direction, credential); + } + } + internal static byte[]? GetNegotiatedApplicationProtocol(SafeDeleteContext context) { Interop.SecPkgContext_ApplicationProtocol alpnContext = default; @@ -437,5 +530,26 @@ private static SafeFreeCredentials AcquireCredentialsHandle(Interop.SspiCli.Cred return SSPIWrapper.AcquireCredentialsHandle(GlobalSSPI.SSPISecureChannel, SecurityPackage, credUsage, secureCredential); } } + + private static unsafe SafeFreeCredentials AcquireCredentialsHandle(Interop.SspiCli.CredentialUse credUsage, Interop.SspiCli.SCH_CREDENTIALS secureCredential) + { + // First try without impersonation, if it fails, then try the process account. + // I.E. We don't know which account the certificate context was created under. + try + { + // + // For app-compat we want to ensure the credential are accessed under >>process<< account. + // + return WindowsIdentity.RunImpersonated(SafeAccessTokenHandle.InvalidHandle, () => + { + return SSPIWrapper.AcquireCredentialsHandle(GlobalSSPI.SSPISecureChannel, SecurityPackage, credUsage, secureCredential); + }); + } + catch + { + return SSPIWrapper.AcquireCredentialsHandle(GlobalSSPI.SSPISecureChannel, SecurityPackage, credUsage, secureCredential); + } + } + } } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlertsTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlertsTest.cs index 909abbe7571b75..262cca6f3fab2c 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlertsTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlertsTest.cs @@ -88,13 +88,14 @@ public async Task SslStream_StreamToStream_ServerInitiatedCloseNotify_Ok() } } - [Fact] - public async Task SslStream_StreamToStream_ClientInitiatedCloseNotify_Ok() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SslStream_StreamToStream_ClientInitiatedCloseNotify_Ok(bool sendData) { - VirtualNetwork network = new VirtualNetwork(); - - using (var clientStream = new VirtualNetworkStream(network, isServer: false)) - using (var serverStream = new VirtualNetworkStream(network, isServer: true)) + (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); + using (clientStream) + using (serverStream) using (var client = new SslStream(clientStream, true, AllowAnyServerCertificate)) using (var server = new SslStream(serverStream)) using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) @@ -106,13 +107,21 @@ public async Task SslStream_StreamToStream_ClientInitiatedCloseNotify_Ok() await Task.WhenAll(handshake).TimeoutAfter(TestConfiguration.PassingTestTimeoutMilliseconds); + var readBuffer = new byte[1024]; + if (sendData) + { + // Send some data before shutting down. This may matter for TLS13. + handshake[0] = server.WriteAsync(readBuffer, 0, 1); + handshake[1] = client.ReadAsync(readBuffer, 0, 1); + await Task.WhenAll(handshake).TimeoutAfter(TestConfiguration.PassingTestTimeoutMilliseconds); + } + await client.ShutdownAsync(); int bytesRead = await server.ReadAsync(readBuffer, 0, readBuffer.Length); // close_notify received by the server. Assert.Equal(0, bytesRead); - await server.ShutdownAsync(); bytesRead = await client.ReadAsync(readBuffer, 0, readBuffer.Length); // close_notify received by the client. diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index fb8d77d5bb4e3e..74a1c947df60ef 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -16,6 +16,7 @@ public static (Stream ClientStream, Stream ServerStream) GetConnectedStreams() { if (Capability.SecurityForceSocketStreams()) { + // DOTNET_TEST_NET_SECURITY_FORCE_SOCKET_STREAMS is set. return GetConnectedTcpStreams(); } From 881e77da6c52bc2ef179cfca9b877307ae1c4821 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 15 Jun 2020 00:17:40 -0700 Subject: [PATCH 2/6] use unmanaged ref to avoid copy --- .../Interop/Windows/SspiCli/ISSPIInterface.cs | 2 +- .../Interop/Windows/SspiCli/Interop.SSPI.cs | 2 +- .../Interop/Windows/SspiCli/SSPIAuthType.cs | 4 +-- .../Windows/SspiCli/SSPISecureChannelType.cs | 4 +-- .../Interop/Windows/SspiCli/SSPIWrapper.cs | 6 ++--- .../Windows/SspiCli/SecuritySafeHandles.cs | 12 ++++----- .../Net/Security/SslStreamPal.Windows.cs | 27 +++++-------------- 7 files changed, 22 insertions(+), 35 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs index 71afaecff54458..4fdb25e5b5da3e 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs @@ -15,7 +15,7 @@ internal interface ISSPIInterface int EnumerateSecurityPackages(out int pkgnum, out SafeFreeContextBuffer pkgArray); int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref SafeSspiAuthDataHandle authdata, out SafeFreeCredentials outCredential); int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCHANNEL_CRED authdata, out SafeFreeCredentials outCredential); - int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCH_CREDENTIALS authdata, out SafeFreeCredentials outCredential); + unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential); int AcquireDefaultCredential(string moduleName, Interop.SspiCli.CredentialUse usage, out SafeFreeCredentials outCredential); int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags); int InitializeSecurityContext(ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, InputSecurityBuffers inputBuffers, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags); diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs index a80333b159dc80..6b17e81ad7c75e 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs @@ -442,7 +442,7 @@ internal static extern unsafe int AcquireCredentialsHandleW( [In] string moduleName, [In] int usage, [In] void* logonID, - [In] ref SCH_CREDENTIALS authData, + [In] SCH_CREDENTIALS* authData, [In] void* keyCallback, [In] void* keyArgument, ref CredHandle handlePtr, diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs index 3d86fd48a96062..25ff9caf12f446 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs @@ -46,9 +46,9 @@ public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.Credentia return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); } - public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCH_CREDENTIALS authdata, out SafeFreeCredentials outCredential) + public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential) { - return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); + return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, authdata, out outCredential); } public int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs index a44639e05a1701..cf05a64bef21dd 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs @@ -46,9 +46,9 @@ public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.Credentia return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); } - public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCH_CREDENTIALS authdata, out SafeFreeCredentials outCredential) + public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential) { - return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); + return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, authdata, out outCredential); } public int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref SecurityBuffer outputBuffer, ref Interop.SspiCli.ContextFlags outFlags) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs index 7ce94e50764932..0a8139e03e4295 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs @@ -141,19 +141,19 @@ public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secMod return outCredential; } - public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secModule, string package, Interop.SspiCli.CredentialUse intent, Interop.SspiCli.SCH_CREDENTIALS scc) + public static unsafe SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secModule, string package, Interop.SspiCli.CredentialUse intent, Interop.SspiCli.SCH_CREDENTIALS* scc) { if (NetEventSource.IsEnabled) { NetEventSource.Enter(null, package); - NetEventSource.Log.AcquireCredentialsHandle(package, intent, scc); + NetEventSource.Log.AcquireCredentialsHandle(package, intent, (IntPtr)scc); } SafeFreeCredentials? outCredential = null; int errorCode = secModule.AcquireCredentialsHandle( package, intent, - ref scc, + scc, out outCredential); if (errorCode != 0) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs index d1c5c24bf57c36..ce3461c2006dea 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs @@ -321,23 +321,23 @@ public static unsafe int AcquireCredentialsHandle( public static unsafe int AcquireCredentialsHandle( string package, Interop.SspiCli.CredentialUse intent, - ref Interop.SspiCli.SCH_CREDENTIALS authdata, + Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential) { - if (NetEventSource.IsEnabled) NetEventSource.Enter(null, package, intent, authdata); + if (NetEventSource.IsEnabled) NetEventSource.Enter(null, package, intent, (IntPtr)authdata); int errorCode = -1; long timeStamp; // If there is a certificate, wrap it into an array. // Not threadsafe. - IntPtr copiedPtr = authdata.paCred; + IntPtr copiedPtr = authdata->paCred; try { IntPtr certArrayPtr = new IntPtr(&copiedPtr); if (copiedPtr != IntPtr.Zero) { - authdata.paCred = certArrayPtr; + authdata->paCred = certArrayPtr; } outCredential = new SafeFreeCredential_SECURITY(); @@ -347,7 +347,7 @@ public static unsafe int AcquireCredentialsHandle( package, (int)intent, null, - ref authdata, + authdata, null, null, ref outCredential._handle, @@ -355,7 +355,7 @@ public static unsafe int AcquireCredentialsHandle( } finally { - authdata.paCred = copiedPtr; + authdata->paCred = copiedPtr; } #if TRACE_VERBOSE diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 28580417969c42..1854e700bbba29 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -215,27 +215,14 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleNew(X509Certifi if (protocolFlags != 0) { // If we were asked to do specific protocol we need to fill TLS_PARAMETERS. - Span tlsParameters = stackalloc Interop.SspiCli.TLS_PARAMETERS[1]; + Interop.SspiCli.TLS_PARAMETERS tlsParameters = default; + tlsParameters.grbitDisabledProtocols = (uint)protocolFlags ^ uint.MaxValue; - tlsParameters[0].cAlpnIds = 0; - tlsParameters[0].rgstrAlpnIds = IntPtr.Zero; - tlsParameters[0].grbitDisabledProtocols = 0; - tlsParameters[0].cDisabledCrypto = 0; - tlsParameters[0].pDisabledCrypto = null; - tlsParameters[0].dwFlags = Interop.SspiCli.TLS_PARAMETERS.Flags.Zero; - tlsParameters[0].grbitDisabledProtocols = (uint)protocolFlags ^ uint.MaxValue; - - fixed (Interop.SspiCli.TLS_PARAMETERS* ptr = tlsParameters) - { - credential.cTlsParameters = 1; - credential.pTlsParameters = ptr; - return AcquireCredentialsHandle(direction, credential); - } - } - else - { - return AcquireCredentialsHandle(direction, credential); + credential.cTlsParameters = 1; + credential.pTlsParameters = &tlsParameters; } + + return AcquireCredentialsHandle(direction, &credential); } internal static byte[]? GetNegotiatedApplicationProtocol(SafeDeleteContext context) @@ -531,7 +518,7 @@ private static SafeFreeCredentials AcquireCredentialsHandle(Interop.SspiCli.Cred } } - private static unsafe SafeFreeCredentials AcquireCredentialsHandle(Interop.SspiCli.CredentialUse credUsage, Interop.SspiCli.SCH_CREDENTIALS secureCredential) + private static unsafe SafeFreeCredentials AcquireCredentialsHandle(Interop.SspiCli.CredentialUse credUsage, Interop.SspiCli.SCH_CREDENTIALS* secureCredential) { // First try without impersonation, if it fails, then try the process account. // I.E. We don't know which account the certificate context was created under. From 28fd68855f90c6645acc6366673a8e9b73fd2368 Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 16 Jun 2020 23:14:16 -0700 Subject: [PATCH 3/6] feedback from review --- .../Interop/Windows/SspiCli/Interop.SSPI.cs | 11 +++----- .../Windows/SspiCli/SecuritySafeHandles.cs | 25 +++---------------- .../Unit/System.Net.Mail.Unit.Tests.csproj | 2 ++ .../Net/Security/SslStreamPal.Windows.cs | 23 ++++++++++------- 4 files changed, 23 insertions(+), 38 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs index 6b17e81ad7c75e..91dc0198e99878 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs @@ -224,14 +224,9 @@ internal unsafe struct SCH_CREDENTIALS public int dwCredformat; public int cCreds; - // ptr to an array of pointers - // There is a hack done with this field. AcquireCredentialsHandle requires an array of - // certificate handles; we only ever use one. In order to avoid pinning a one element array, - // we copy this value onto the stack, create a pointer on the stack to the copied value, - // and replace this field with the pointer, during the call to AcquireCredentialsHandle. - // Then we fix it up afterwards. Fine as long as all the SSPI credentials are not - // supposed to be threadsafe. - public IntPtr paCred; + // This is pointer to arry of CERT_CONTEXT* + // We do not use it directly in .NET. Instead, we wrap returned OS pointer in safe handle. + public void* paCred; public IntPtr hRootStore; // == always null, OTHERWISE NOT RELIABLE public int cMappers; diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs index f6925c5af613e4..d3579dbe9b9f4c 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs @@ -271,6 +271,7 @@ public static unsafe int AcquireCredentialsHandle( out SafeFreeCredentials outCredential) { if (NetEventSource.IsEnabled) NetEventSource.Enter(null, package, intent, authdata); + int errorCode = -1; long timeStamp; @@ -325,20 +326,9 @@ public static unsafe int AcquireCredentialsHandle( int errorCode = -1; long timeStamp; - // If there is a certificate, wrap it into an array. - // Not threadsafe. - IntPtr copiedPtr = authdata->paCred; - try - { - IntPtr certArrayPtr = new IntPtr(&copiedPtr); - if (copiedPtr != IntPtr.Zero) - { - authdata->paCred = certArrayPtr; - } - - outCredential = new SafeFreeCredential_SECURITY(); + outCredential = new SafeFreeCredential_SECURITY(); - errorCode = Interop.SspiCli.AcquireCredentialsHandleW( + errorCode = Interop.SspiCli.AcquireCredentialsHandleW( null, package, (int)intent, @@ -348,15 +338,8 @@ public static unsafe int AcquireCredentialsHandle( null, ref outCredential._handle, out timeStamp); - } - finally - { - authdata->paCred = copiedPtr; - } -#if TRACE_VERBOSE - if (NetEventSource.IsEnabled) NetEventSource.Info(null, $"{nameof(Interop.SspiCli.AcquireCredentialsHandleW)} returns 0x{errorCode:x}, handle = {outCredential}"); -#endif + if (NetEventSource.IsEnabled) NetEventSource.Verbose(null, $"{nameof(Interop.SspiCli.AcquireCredentialsHandleW)} returns 0x{errorCode:x}, handle = {outCredential}"); if (errorCode != 0) { diff --git a/src/libraries/System.Net.Mail/tests/Unit/System.Net.Mail.Unit.Tests.csproj b/src/libraries/System.Net.Mail/tests/Unit/System.Net.Mail.Unit.Tests.csproj index d4da9df33459e6..c6c2f54be3fe0d 100644 --- a/src/libraries/System.Net.Mail/tests/Unit/System.Net.Mail.Unit.Tests.csproj +++ b/src/libraries/System.Net.Mail/tests/Unit/System.Net.Mail.Unit.Tests.csproj @@ -252,5 +252,7 @@ Link="Common\Interop\Windows\SspiCli\SecuritySafeHandles.cs" /> + diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 1854e700bbba29..511b9c71ee441a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -16,8 +16,8 @@ namespace System.Net.Security { internal static class SslStreamPal { - private static bool UseNewCryptoApi = CheckWindowsVersion(); - private static bool CheckWindowsVersion() + private static readonly bool UseNewCryptoApi = Is1809OrNewer(); + private static bool Is1809OrNewer() { // On newer Windows version we use new API to get TLS1.3. // API is supported since Windows 10 1809 (17763)but there is no reason to use at the moment. @@ -168,7 +168,12 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleNew(X509Certifi Interop.SspiCli.SCH_CREDENTIALS.Flags flags; Interop.SspiCli.CredentialUse direction; - if (!isServer) + if (isServer) + { + direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND; + flags = Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD; + } + else { direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_OUTBOUND; flags = @@ -176,11 +181,6 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleNew(X509Certifi Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_NO_DEFAULT_CREDS | Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD; } - else - { - direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND; - flags = Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD; - } if (policy == EncryptionPolicy.RequireEncryption) { @@ -206,8 +206,13 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleNew(X509Certifi if (certificate != null) { - credential.paCred = certificate.Handle; credential.cCreds = 1; + Span certificates = stackalloc IntPtr[1]; + certificates[0] = certificate.Handle; + fixed (void* ptr = &certificates[0]) + { + credential.paCred = ptr; + } } if (NetEventSource.IsEnabled) NetEventSource.Info($"flags=({flags}), ProtocolFlags=({protocolFlags}), EncryptionPolicy={policy}"); From ad23a53404f3bde54e8a623a4bc90a3431f8b38d Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 6 Jul 2020 20:14:27 -0700 Subject: [PATCH 4/6] feedback from review --- .../Interop/Windows/SspiCli/SSPIWrapper.cs | 20 ++---------- .../Net/Security/SslStreamPal.Windows.cs | 31 +++++++++---------- 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs index 0a8139e03e4295..a8032e25952dcf 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs @@ -118,18 +118,11 @@ public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secMod public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secModule, string package, Interop.SspiCli.CredentialUse intent, Interop.SspiCli.SCHANNEL_CRED scc) { - if (NetEventSource.IsEnabled) - { - NetEventSource.Enter(null, package); - NetEventSource.Log.AcquireCredentialsHandle(package, intent, scc); - } - - SafeFreeCredentials? outCredential = null; int errorCode = secModule.AcquireCredentialsHandle( package, intent, ref scc, - out outCredential); + out SafeFreeCredentials outCredential); if (errorCode != 0) { @@ -137,24 +130,16 @@ public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secMod throw new Win32Exception(errorCode); } - if (NetEventSource.IsEnabled) NetEventSource.Exit(null, outCredential); return outCredential; } public static unsafe SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secModule, string package, Interop.SspiCli.CredentialUse intent, Interop.SspiCli.SCH_CREDENTIALS* scc) { - if (NetEventSource.IsEnabled) - { - NetEventSource.Enter(null, package); - NetEventSource.Log.AcquireCredentialsHandle(package, intent, (IntPtr)scc); - } - - SafeFreeCredentials? outCredential = null; int errorCode = secModule.AcquireCredentialsHandle( package, intent, scc, - out outCredential); + out SafeFreeCredentials outCredential); if (errorCode != 0) { @@ -162,7 +147,6 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface throw new Win32Exception(errorCode); } - if (NetEventSource.IsEnabled) NetEventSource.Exit(null, outCredential); return outCredential; } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 511b9c71ee441a..af8055d1d6d15f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -16,14 +16,11 @@ namespace System.Net.Security { internal static class SslStreamPal { - private static readonly bool UseNewCryptoApi = Is1809OrNewer(); - private static bool Is1809OrNewer() - { + private static readonly bool UseNewCryptoApi = // On newer Windows version we use new API to get TLS1.3. // API is supported since Windows 10 1809 (17763)but there is no reason to use at the moment. - return (Interop.NtDll.RtlGetVersionEx(out Interop.NtDll.RTL_OSVERSIONINFOEX osvi) == 0 && - osvi.dwMajorVersion >= 10 && osvi.dwBuildNumber >= 18836); - } + Interop.NtDll.RtlGetVersionEx(out Interop.NtDll.RTL_OSVERSIONINFOEX osvi) == 0 && + osvi.dwMajorVersion >= 10 && osvi.dwBuildNumber >= 18836; private const string SecurityPackage = "Microsoft Unified Security Protocol Provider"; @@ -117,13 +114,15 @@ public static SecurityStatusPal InitializeSecurityContext(ref SafeFreeCredential public static SafeFreeCredentials AcquireCredentialsHandle(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { - // New crypto API does not allow to force NULL encryption. + // New crypto API supports TLS1.3 but it does not allow to force NULL encryption. return !UseNewCryptoApi || policy == EncryptionPolicy.NoEncryption ? - AcquireCredentialsHandleLegcy(certificate, protocols, policy, isServer) : - AcquireCredentialsHandleNew(certificate, protocols, policy, isServer); + AcquireCredentialsHandleSchannelCred(certificate, protocols, policy, isServer) : + AcquireCredentialsHandleSchCredentials(certificate, protocols, policy, isServer); } - public static SafeFreeCredentials AcquireCredentialsHandleLegcy(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) + // This is legacy crypto API used on .NET Framework and older Windows versions. + // It only supports TLS up to 1.2 + public static SafeFreeCredentials AcquireCredentialsHandleSchannelCred(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { int protocolFlags = GetProtocolFlagsFromSslProtocols(protocols, isServer); Interop.SspiCli.SCHANNEL_CRED.Flags flags; @@ -162,7 +161,8 @@ public static SafeFreeCredentials AcquireCredentialsHandleLegcy(X509Certificate? return AcquireCredentialsHandle(direction, secureCredential); } - public static unsafe SafeFreeCredentials AcquireCredentialsHandleNew(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) + // This function uses new crypto API to support TLS 1.3 and beyond. + public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchCredentials(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { int protocolFlags = GetProtocolFlagsFromSslProtocols(protocols, isServer); Interop.SspiCli.SCH_CREDENTIALS.Flags flags; @@ -204,15 +204,12 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleNew(X509Certifi credential.dwVersion = Interop.SspiCli.SCH_CREDENTIALS.CurrentVersion; credential.dwFlags = flags; + IntPtr certificateHandle = IntPtr.Zero; if (certificate != null) { credential.cCreds = 1; - Span certificates = stackalloc IntPtr[1]; - certificates[0] = certificate.Handle; - fixed (void* ptr = &certificates[0]) - { - credential.paCred = ptr; - } + certificateHandle = certificate.Handle; + credential.paCred = &certificateHandle; } if (NetEventSource.IsEnabled) NetEventSource.Info($"flags=({flags}), ProtocolFlags=({protocolFlags}), EncryptionPolicy={policy}"); From 60c550ae5c71d3b63887e86c93bdcceebf3fcca3 Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 14 Jul 2020 18:42:26 -0700 Subject: [PATCH 5/6] feedback from review --- .../Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs | 3 +-- .../src/System/Net/Security/SslStreamPal.Windows.cs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs index 00795abde15d89..bef2693adb8823 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs @@ -308,12 +308,11 @@ public static unsafe int AcquireCredentialsHandle( Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential) { - int errorCode = -1; long timeStamp; outCredential = new SafeFreeCredential_SECURITY(); - errorCode = Interop.SspiCli.AcquireCredentialsHandleW( + int errorCode = Interop.SspiCli.AcquireCredentialsHandleW( null, package, (int)intent, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 454c78fff12e10..140ad50fefe063 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -18,8 +18,7 @@ internal static class SslStreamPal private static readonly bool UseNewCryptoApi = // On newer Windows version we use new API to get TLS1.3. // API is supported since Windows 10 1809 (17763)but there is no reason to use at the moment. - Interop.NtDll.RtlGetVersionEx(out Interop.NtDll.RTL_OSVERSIONINFOEX osvi) == 0 && - osvi.dwMajorVersion >= 10 && osvi.dwBuildNumber >= 18836; + Environment.OSVersion.Version.Major >= 10 && Environment.OSVersion.Version.Build >= 18836; private const string SecurityPackage = "Microsoft Unified Security Protocol Provider"; From fcb81d131c327e38b3c4e68825096c9ddbbb944f Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 15 Jul 2020 10:04:33 -0700 Subject: [PATCH 6/6] kick the build --- .../src/System/Net/Security/SslStreamPal.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 511b9c71ee441a..d71837b3e387ea 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -20,7 +20,7 @@ internal static class SslStreamPal private static bool Is1809OrNewer() { // On newer Windows version we use new API to get TLS1.3. - // API is supported since Windows 10 1809 (17763)but there is no reason to use at the moment. + // API is supported since Windows 10 1809 (17763) but there is no reason to use at the moment. return (Interop.NtDll.RtlGetVersionEx(out Interop.NtDll.RTL_OSVERSIONINFOEX osvi) == 0 && osvi.dwMajorVersion >= 10 && osvi.dwBuildNumber >= 18836); }