From bd9906c6676b247808d108170d8f97831c0a8e5b Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 21 Jul 2020 21:20:37 -0700 Subject: [PATCH 1/2] cleanup schannel pal --- .../Crypt32/Interop.certificates_types.cs | 10 ------ .../Interop/Windows/SspiCli/ISSPIInterface.cs | 2 +- .../Interop/Windows/SspiCli/Interop.SSPI.cs | 17 +++------- .../Interop/Windows/SspiCli/SSPIAuthType.cs | 4 +-- .../Windows/SspiCli/SSPISecureChannelType.cs | 4 +-- .../Interop/Windows/SspiCli/SSPIWrapper.cs | 4 +-- .../Windows/SspiCli/SecuritySafeHandles.cs | 24 +++----------- .../src/System.Net.Http.WinHttpHandler.csproj | 14 ++++++++ .../tests/UnitTests/FakeInterop.cs | 29 ++++++++++++++++ .../tests/UnitTests/FakeX509Certificates.cs | 19 +++++++++++ ....Net.Http.WinHttpHandler.Unit.Tests.csproj | 4 --- .../src/System.Net.Http.csproj | 30 +++++++++-------- .../System.Net.Http.Unit.Tests.csproj | 4 --- .../src/System.Net.HttpListener.csproj | 22 +++++++++---- .../src/System.Net.Mail.csproj | 22 +++++++++---- .../Unit/System.Net.Mail.Unit.Tests.csproj | 20 ++++++++--- .../src/System.Net.Security.csproj | 16 +++++++-- .../Net/Security/SslStreamPal.Windows.cs | 33 +++++++++---------- 18 files changed, 171 insertions(+), 107 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.certificates_types.cs b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.certificates_types.cs index 3a5d9d31baf8f8..2fa029d9d8f301 100644 --- a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.certificates_types.cs +++ b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.certificates_types.cs @@ -86,16 +86,6 @@ internal static partial class CertChainPolicyErrors internal const uint CERT_E_ROLE = 0x800B0103; } - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] - internal struct CERT_CONTEXT - { - internal uint dwCertEncodingType; - internal IntPtr pbCertEncoded; - internal uint cbCertEncoded; - internal IntPtr pCertInfo; - internal IntPtr hCertStore; - } - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] internal unsafe struct SSL_EXTRA_CERT_CHAIN_POLICY_PARA { diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs index 62daefd267e77f..7260037a70fd0d 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs @@ -13,7 +13,7 @@ internal interface ISSPIInterface SecurityPackageInfoClass[]? SecurityPackages { get; set; } 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); + unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCHANNEL_CRED* 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); 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 9556c5054533aa..93998b37ee7064 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs @@ -172,21 +172,14 @@ internal unsafe struct SecPkgContext_IssuerListInfoEx } [StructLayout(LayoutKind.Sequential)] - internal struct SCHANNEL_CRED + internal unsafe struct SCHANNEL_CRED { public const int CurrentVersion = 0x4; public int dwVersion; 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 Crypt32.CERT_CONTEXT** paCred; public IntPtr hRootStore; // == always null, OTHERWISE NOT RELIABLE public int cMappers; @@ -223,9 +216,7 @@ internal unsafe struct SCH_CREDENTIALS public int dwCredformat; public int cCreds; - // 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 Crypt32.CERT_CONTEXT** paCred; public IntPtr hRootStore; // == always null, OTHERWISE NOT RELIABLE public int cMappers; @@ -423,7 +414,7 @@ internal static extern unsafe int AcquireCredentialsHandleW( [In] string moduleName, [In] int usage, [In] void* logonID, - [In] ref SCHANNEL_CRED authData, + [In] SCHANNEL_CRED* 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 7d359791a0fe11..415ef9d9f7a24e 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs @@ -40,9 +40,9 @@ public int AcquireDefaultCredential(string moduleName, Interop.SspiCli.Credentia return SafeFreeCredentials.AcquireDefaultCredential(moduleName, usage, out outCredential); } - public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCHANNEL_CRED authdata, out SafeFreeCredentials outCredential) + public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCHANNEL_CRED* authdata, out SafeFreeCredentials outCredential) { - return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); + return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, authdata, out outCredential); } public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs index e30e53d27262a8..fb1d93d356d6af 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs @@ -40,9 +40,9 @@ public int AcquireDefaultCredential(string moduleName, Interop.SspiCli.Credentia return SafeFreeCredentials.AcquireDefaultCredential(moduleName, usage, out outCredential); } - public int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, ref Interop.SspiCli.SCHANNEL_CRED authdata, out SafeFreeCredentials outCredential) + public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCHANNEL_CRED* authdata, out SafeFreeCredentials outCredential) { - return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, ref authdata, out outCredential); + return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, authdata, out outCredential); } public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs index eaf912697a91ca..57ce6232943275 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs @@ -108,12 +108,12 @@ public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secMod return credentialsHandle; } - public static SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secModule, string package, Interop.SspiCli.CredentialUse intent, Interop.SspiCli.SCHANNEL_CRED scc) + public static unsafe SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface secModule, string package, Interop.SspiCli.CredentialUse intent, Interop.SspiCli.SCHANNEL_CRED* scc) { int errorCode = secModule.AcquireCredentialsHandle( package, intent, - ref scc, + scc, out SafeFreeCredentials 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 bef2693adb8823..bf366f896c45c7 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs @@ -257,40 +257,24 @@ public static unsafe int AcquireCredentialsHandle( public static unsafe int AcquireCredentialsHandle( string package, Interop.SspiCli.CredentialUse intent, - ref Interop.SspiCli.SCHANNEL_CRED authdata, + Interop.SspiCli.SCHANNEL_CRED* authdata, out SafeFreeCredentials outCredential) { 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, null, - ref authdata, + authdata, null, null, ref outCredential._handle, out timeStamp); - } - finally - { - authdata.paCred = copiedPtr; - } if (NetEventSource.Log.IsEnabled()) NetEventSource.Verbose(null, $"{nameof(Interop.SspiCli.AcquireCredentialsHandleW)} returns 0x{errorCode:x}, handle = {outCredential}"); diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj b/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj index 8c06bc26a682f8..be2dc41c111a62 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj @@ -17,6 +17,20 @@ + + + + + + + - - - - - + + + + + + + + - - - - - - - - - + + + + + + + + - - - + + + + + + + + - - + + + + + + + + + + + + + + + - Date: Tue, 21 Jul 2020 23:09:26 -0700 Subject: [PATCH 2/2] fix netfx --- .../Common/src/Interop/Windows/Crypt32/Interop.CERT_INFO.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CERT_INFO.cs b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CERT_INFO.cs index 26efa43871aa4a..21f010177ac183 100644 --- a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CERT_INFO.cs +++ b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CERT_INFO.cs @@ -3,7 +3,6 @@ using System; using System.Runtime.InteropServices; -using System.Runtime.InteropServices.ComTypes; internal static partial class Interop { @@ -16,8 +15,8 @@ internal struct CERT_INFO internal DATA_BLOB SerialNumber; internal CRYPT_ALGORITHM_IDENTIFIER SignatureAlgorithm; internal DATA_BLOB Issuer; - internal FILETIME NotBefore; - internal FILETIME NotAfter; + internal System.Runtime.InteropServices.ComTypes.FILETIME NotBefore; + internal System.Runtime.InteropServices.ComTypes.FILETIME NotAfter; internal DATA_BLOB Subject; internal CERT_PUBLIC_KEY_INFO SubjectPublicKeyInfo; internal CRYPT_BIT_BLOB IssuerUniqueId;