From f6d35fd39df23181b60318b1d608f6ec03c4b774 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 18 May 2022 15:28:29 -0700 Subject: [PATCH 01/16] avoid allocation of SafeFreeSslCredentials and SafeDeleteSslContext on Linux --- .../Interop.OpenSsl.cs | 63 ++++++++++++++----- .../Interop.Ssl.cs | 8 ++- .../Net/Security/Unix/SafeDeleteContext.cs | 15 ++++- .../Net/Security/Unix/SafeDeleteSslContext.cs | 47 +------------- .../Net/CertificateValidationPal.Unix.cs | 6 +- .../System/Net/Security/SslStream.Protocol.cs | 8 ++- .../System/Net/Security/SslStreamPal.Unix.cs | 51 +++++++-------- 7 files changed, 100 insertions(+), 98 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index a3e872fe049627..4a104b6efc86c5 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -150,11 +150,8 @@ private static SslProtocols CalculateEffectiveProtocols(SslAuthenticationOptions } // This essentially wraps SSL_CTX* aka SSL_CTX_new + setting - internal static unsafe SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions, SslProtocols protocols, bool enableResume) + internal static unsafe SafeSslContextHandle AllocateSslContext(SslAuthenticationOptions sslAuthenticationOptions, SslProtocols protocols, bool enableResume) { - SafeX509Handle? certHandle = credential.CertHandle; - SafeEvpPKeyHandle? certKeyHandle = credential.CertKeyHandle; - // Always use SSLv23_method, regardless of protocols. It supports negotiating to the highest // mutually supported version and can thus handle any of the set protocols, and we then use // SetProtocolOptions to ensure we only allow the ones requested. @@ -225,20 +222,16 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SafeFreeSslCreden Interop.Ssl.SslCtxSetAlpnSelectCb(sslCtx, &AlpnServerSelectCallback, IntPtr.Zero); } - bool hasCertificateAndKey = - certHandle != null && !certHandle.IsInvalid - && certKeyHandle != null && !certKeyHandle.IsInvalid; - - if (hasCertificateAndKey) + if (sslAuthenticationOptions.CertificateContext != null) { - SetSslCertificate(sslCtx, certHandle!, certKeyHandle!); - } + SetSslCertificate(sslCtx, sslAuthenticationOptions.CertificateContext.Certificate); - if (sslAuthenticationOptions.CertificateContext != null && sslAuthenticationOptions.CertificateContext.IntermediateCertificates.Length > 0) - { - if (!Ssl.AddExtraChainCertificates(sslCtx, sslAuthenticationOptions.CertificateContext.IntermediateCertificates)) + if (sslAuthenticationOptions.CertificateContext.IntermediateCertificates.Length > 0) { - throw CreateSslException(SR.net_ssl_use_cert_failed); + if (!Ssl.AddExtraChainCertificates(sslCtx, sslAuthenticationOptions.CertificateContext.IntermediateCertificates)) + { + throw CreateSslException(SR.net_ssl_use_cert_failed); + } } } } @@ -291,7 +284,7 @@ internal static void UpdateClientCertiticate(SafeSslHandle ssl, SslAuthenticatio } // This essentially wraps SSL* SSL_new() - internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) + internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuthenticationOptions) { SafeSslHandle? sslHandle = null; SafeSslContextHandle? sslCtxHandle = null; @@ -344,7 +337,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (sslCtxHandle == null) { // We did not get SslContext from cache - sslCtxHandle = newCtxHandle = AllocateSslContext(credential, sslAuthenticationOptions, protocols, cacheSslContext); + sslCtxHandle = newCtxHandle = AllocateSslContext(sslAuthenticationOptions, protocols, cacheSslContext); if (cacheSslContext) { @@ -821,6 +814,42 @@ private static void BioWrite(SafeBioHandle bio, ReadOnlySpan buffer) return innerError; } + private static void SetSslCertificate(SafeSslContextHandle contextPtr, X509Certificate2 cert) + { + SafeEvpPKeyHandle? certKeyHandle = null; + + using (RSAOpenSsl? rsa = (RSAOpenSsl?)cert.GetRSAPrivateKey()) + { + if (rsa != null) + { + certKeyHandle = rsa.DuplicateKeyHandle(); + Interop.Crypto.CheckValidOpenSslHandle(certKeyHandle); + } + } + + if (certKeyHandle == null) + { + using (ECDsaOpenSsl? ecdsa = (ECDsaOpenSsl?)cert.GetECDsaPrivateKey()) + { + if (ecdsa != null) + { + certKeyHandle = ecdsa.DuplicateKeyHandle(); + Interop.Crypto.CheckValidOpenSslHandle(certKeyHandle); + } + } + } + + if (certKeyHandle == null) + { + throw new NotSupportedException(SR.net_ssl_io_no_server_cert); + } + + using (SafeX509Handle certHandle = Interop.Crypto.X509UpRef(cert.Handle)) + { + SetSslCertificate(contextPtr, certHandle, certKeyHandle); + } + } + private static void SetSslCertificate(SafeSslContextHandle contextPtr, SafeX509Handle certPtr, SafeEvpPKeyHandle keyPtr) { Debug.Assert(certPtr != null && !certPtr.IsInvalid, "certPtr != null && !certPtr.IsInvalid"); diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 0bda8b06d4b2c5..6f075d5a1c61d1 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -317,7 +317,7 @@ internal enum SslErrorCode namespace Microsoft.Win32.SafeHandles { - internal sealed class SafeSslHandle : SafeHandle + internal sealed class SafeSslHandle : SafeDeleteSslContext { private SafeBioHandle? _readBio; private SafeBioHandle? _writeBio; @@ -352,6 +352,12 @@ internal void MarkHandshakeCompleted() _handshakeCompleted = true; } + + public static SafeSslHandle Create(SslAuthenticationOptions sslAuthenticationOptions) + { + return Interop.OpenSsl.AllocateSslHandle(sslAuthenticationOptions); + } + public static SafeSslHandle Create(SafeSslContextHandle context, bool isServer) { SafeBioHandle readBio = Interop.Crypto.CreateMemoryBio(); diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs index eab627c6afd9c0..01f0f1c30b3965 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs @@ -13,7 +13,7 @@ internal abstract class SafeDeleteContext : DebugSafeHandle internal abstract class SafeDeleteContext : SafeHandle { #endif - private SafeFreeCredentials _credential; + private SafeFreeCredentials? _credential; protected SafeDeleteContext(SafeFreeCredentials credential) : base(IntPtr.Zero, true) @@ -29,6 +29,16 @@ protected SafeDeleteContext(SafeFreeCredentials credential) _credential.DangerousAddRef(ref ignore); } + public SafeDeleteContext(IntPtr handle) : base(handle, true) + { + _credential = null; + } + + public SafeDeleteContext(IntPtr handle, bool ownsHandle) : base(handle, ownsHandle) + { + _credential = null; + } + public override bool IsInvalid { get { return (null == _credential); } @@ -36,8 +46,7 @@ public override bool IsInvalid protected override bool ReleaseHandle() { - Debug.Assert((null != _credential), "Null credential in SafeDeleteContext"); - _credential.DangerousRelease(); + _credential?.DangerousRelease(); _credential = null!; return true; } diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteSslContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteSslContext.cs index 89ac9b6d8dc11c..9b1a5dd7af39db 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteSslContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteSslContext.cs @@ -15,55 +15,14 @@ namespace System.Net.Security { - internal sealed class SafeDeleteSslContext : SafeDeleteContext + internal class SafeDeleteSslContext : SafeDeleteContext { - private SafeSslHandle _sslContext; - - public SafeSslHandle SslContext + public SafeDeleteSslContext(IntPtr handle) : base(handle, true) { - get - { - return _sslContext; - } - } - - public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) - : base(credential) - { - Debug.Assert((null != credential) && !credential.IsInvalid, "Invalid credential used in SafeDeleteSslContext"); - - try - { - _sslContext = Interop.OpenSsl.AllocateSslHandle(credential, sslAuthenticationOptions); - } - catch (Exception ex) - { - Debug.Write("Exception Caught. - " + ex); - Dispose(); - throw; - } } - public override bool IsInvalid + public SafeDeleteSslContext(IntPtr handle, bool ownsHandle) : base(handle, ownsHandle) { - get - { - return (null == _sslContext) || _sslContext.IsInvalid; - } - } - - protected override void Dispose(bool disposing) - { - if (disposing) - { - if (null != _sslContext) - { - _sslContext.Dispose(); - _sslContext = null!; - } - } - - base.Dispose(disposing); } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs index 49f67eea22f5d9..e822c9315ac2a3 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs @@ -50,7 +50,7 @@ internal static SslPolicyErrors VerifyCertificateProperties( chain ??= new X509Chain(); using (SafeSharedX509StackHandle chainStack = - Interop.OpenSsl.GetPeerCertificateChain(((SafeDeleteSslContext)securityContext).SslContext)) + Interop.OpenSsl.GetPeerCertificateChain((SafeSslHandle)securityContext)) { if (!chainStack.IsInvalid) { @@ -98,7 +98,7 @@ internal static SslPolicyErrors VerifyCertificateProperties( // internal static string[] GetRequestCertificateAuthorities(SafeDeleteContext securityContext) { - using (SafeSharedX509NameStackHandle names = Interop.Ssl.SslGetClientCAList(((SafeDeleteSslContext)securityContext).SslContext)) + using (SafeSharedX509NameStackHandle names = Interop.Ssl.SslGetClientCAList((SafeSslHandle)securityContext)) { if (names.IsInvalid) { @@ -152,7 +152,7 @@ private static int QueryContextRemoteCertificate(SafeDeleteContext securityConte remoteCertContext = null; try { - SafeX509Handle remoteCertificate = Interop.OpenSsl.GetPeerCertificate(((SafeDeleteSslContext)securityContext).SslContext); + SafeX509Handle remoteCertificate = Interop.OpenSsl.GetPeerCertificate((SafeSslHandle)securityContext); // Note that cert ownership is transferred to SafeFreeCertContext remoteCertContext = new SafeFreeCertContext(remoteCertificate); return 0; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index b732dfe4d24a52..b45daeb9894d0f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -659,12 +659,12 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint) return cachedCred; } - private static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) + private static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) { - SafeFreeCredentials cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions.CertificateContext, sslAuthenticationOptions.EnabledSslProtocols, + SafeFreeCredentials? cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions.CertificateContext, sslAuthenticationOptions.EnabledSslProtocols, sslAuthenticationOptions.EncryptionPolicy, sslAuthenticationOptions.IsServer); - if (sslAuthenticationOptions.CertificateContext != null) + if (sslAuthenticationOptions.CertificateContext != null && cred != null) { // // Since the SafeFreeCredentials can be cached and reused, it may happen on long running processes that some cert on @@ -835,6 +835,8 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte internal SecurityStatusPal Renegotiate(out byte[]? output) { + ArgumentNullException.ThrowIfNull(_securityContext); + return SslStreamPal.Renegotiate( ref _credentialsHandle!, ref _securityContext, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 32ac2bfdbeba53..82302148e37705 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -31,7 +31,7 @@ public static SecurityStatusPal AcceptSecurityContext( ref byte[]? outputBuffer, SslAuthenticationOptions sslAuthenticationOptions) { - return HandshakeInternal(credential!, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, null); + return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, null); } public static SecurityStatusPal InitializeSecurityContext( @@ -43,20 +43,20 @@ public static SecurityStatusPal InitializeSecurityContext( SslAuthenticationOptions sslAuthenticationOptions, SelectClientCertificate? clientCertificateSelectionCallback) { - return HandshakeInternal(credential!, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, clientCertificateSelectionCallback); + return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, clientCertificateSelectionCallback); } - public static SafeFreeCredentials AcquireCredentialsHandle(SslStreamCertificateContext? certificateContext, + public static SafeFreeCredentials? AcquireCredentialsHandle(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { - return new SafeFreeSslCredentials(certificateContext, protocols, policy, isServer); + return null; } public static SecurityStatusPal EncryptMessage(SafeDeleteSslContext securityContext, ReadOnlyMemory input, int headerSize, int trailerSize, ref byte[] output, out int resultSize) { try { - resultSize = Interop.OpenSsl.Encrypt(securityContext.SslContext, input.Span, ref output, out Interop.Ssl.SslErrorCode errorCode); + resultSize = Interop.OpenSsl.Encrypt((SafeSslHandle)securityContext, input.Span, ref output, out Interop.Ssl.SslErrorCode errorCode); return MapNativeErrorCode(errorCode); } @@ -74,7 +74,7 @@ public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityCont try { - int resultSize = Interop.OpenSsl.Decrypt(securityContext.SslContext, buffer, out Interop.Ssl.SslErrorCode errorCode); + int resultSize = Interop.OpenSsl.Decrypt((SafeSslHandle)securityContext, buffer, out Interop.Ssl.SslErrorCode errorCode); SecurityStatusPal retVal = MapNativeErrorCode(errorCode); @@ -119,7 +119,7 @@ Interop.Ssl.SslErrorCode.SSL_ERROR_NONE or else { bindingHandle = Interop.OpenSsl.QueryChannelBinding( - securityContext.SslContext, + (SafeSslHandle)securityContext, attribute); } @@ -128,19 +128,18 @@ Interop.Ssl.SslErrorCode.SSL_ERROR_NONE or public static SecurityStatusPal Renegotiate( ref SafeFreeCredentials? credentialsHandle, - ref SafeDeleteSslContext? securityContext, + ref SafeDeleteSslContext context, SslAuthenticationOptions sslAuthenticationOptions, out byte[]? outputBuffer) { - var sslContext = ((SafeDeleteSslContext)securityContext!).SslContext; - SecurityStatusPal status = Interop.OpenSsl.SslRenegotiate(sslContext, out _); + SecurityStatusPal status = Interop.OpenSsl.SslRenegotiate((SafeSslHandle)context, out _); outputBuffer = Array.Empty(); if (status.ErrorCode != SecurityStatusPalErrorCode.OK) { return status; } - return HandshakeInternal(credentialsHandle!, ref securityContext, null, ref outputBuffer, sslAuthenticationOptions, null); + return HandshakeInternal(ref context!, null, ref outputBuffer, sslAuthenticationOptions, null); } public static void QueryContextStreamSizes(SafeDeleteContext? securityContext, out StreamSizes streamSizes) @@ -150,14 +149,12 @@ public static void QueryContextStreamSizes(SafeDeleteContext? securityContext, o public static void QueryContextConnectionInfo(SafeDeleteSslContext securityContext, ref SslConnectionInfo connectionInfo) { - connectionInfo.UpdateSslConnectionInfo(securityContext.SslContext); + connectionInfo.UpdateSslConnectionInfo((SafeSslHandle)securityContext); } - private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credential, ref SafeDeleteSslContext? context, + private static SecurityStatusPal HandshakeInternal(ref SafeDeleteSslContext? context, ReadOnlySpan inputBuffer, ref byte[]? outputBuffer, SslAuthenticationOptions sslAuthenticationOptions, SelectClientCertificate? clientCertificateSelectionCallback) { - Debug.Assert(!credential.IsInvalid); - byte[]? output = null; int outputSize = 0; @@ -165,10 +162,10 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia { if ((null == context) || context.IsInvalid) { - context = new SafeDeleteSslContext((credential as SafeFreeSslCredentials)!, sslAuthenticationOptions); + context = SafeSslHandle.Create(sslAuthenticationOptions); } - SecurityStatusPalErrorCode errorCode = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, inputBuffer, out output, out outputSize); + SecurityStatusPalErrorCode errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, inputBuffer, out output, out outputSize); if (errorCode == SecurityStatusPalErrorCode.CredentialsNeeded && clientCertificateSelectionCallback != null) { @@ -178,22 +175,22 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia sslAuthenticationOptions.CertificateContext = SslStreamCertificateContext.Create(clientCertificate); } - Interop.OpenSsl.UpdateClientCertiticate(((SafeDeleteSslContext)context).SslContext, sslAuthenticationOptions); - errorCode = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, null, out output, out outputSize); + Interop.OpenSsl.UpdateClientCertiticate((SafeSslHandle)context, sslAuthenticationOptions); + errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, null, out output, out outputSize); } // sometimes during renegotiation processing message does not yield new output. // That seems to be flaw in OpenSSL state machine and we have workaround to peek it and try it again. - if (outputSize == 0 && Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext)) + if (outputSize == 0 && Interop.Ssl.IsSslRenegotiatePending((SafeSslHandle)context)) { - errorCode = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, ReadOnlySpan.Empty, out output, out outputSize); + errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan.Empty, out output, out outputSize); } // When the handshake is done, and the context is server, check if the alpnHandle target was set to null during ALPN. // If it was, then that indicates ALPN failed, send failure. // We have this workaround, as openssl supports terminating handshake only from version 1.1.0, // whereas ALPN is supported from version 1.0.2. - SafeSslHandle sslContext = context.SslContext; + SafeSslHandle sslContext = (SafeSslHandle)context; if (errorCode == SecurityStatusPalErrorCode.OK && sslAuthenticationOptions.IsServer && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0 && sslContext.AlpnHandle.IsAllocated && sslContext.AlpnHandle.Target == null) @@ -228,22 +225,22 @@ public static SecurityStatusPal ApplyAlertToken(ref SafeFreeCredentials? credent return new SecurityStatusPal(SecurityStatusPalErrorCode.OK); } - public static SecurityStatusPal ApplyShutdownToken(ref SafeFreeCredentials? credentialsHandle, SafeDeleteSslContext sslContext) + public static SecurityStatusPal ApplyShutdownToken(ref SafeFreeCredentials? credentialsHandle, SafeDeleteSslContext context) { // Unset the quiet shutdown option initially configured. - Interop.Ssl.SslSetQuietShutdown(sslContext.SslContext, 0); + Interop.Ssl.SslSetQuietShutdown((SafeSslHandle)context, 0); - int status = Interop.Ssl.SslShutdown(sslContext.SslContext); + int status = Interop.Ssl.SslShutdown((SafeSslHandle)context); if (status == 0) { // Call SSL_shutdown again for a bi-directional shutdown. - status = Interop.Ssl.SslShutdown(sslContext.SslContext); + status = Interop.Ssl.SslShutdown((SafeSslHandle)context); } if (status == 1) return new SecurityStatusPal(SecurityStatusPalErrorCode.OK); - Interop.Ssl.SslErrorCode code = Interop.Ssl.SslGetError(sslContext.SslContext, status); + Interop.Ssl.SslErrorCode code = Interop.Ssl.SslGetError((SafeSslHandle)context, status); if (code == Interop.Ssl.SslErrorCode.SSL_ERROR_WANT_READ || code == Interop.Ssl.SslErrorCode.SSL_ERROR_WANT_WRITE) { From 8479866a63b1a79f4bc937fa07ca86dae245fc90 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 18 May 2022 18:17:21 -0700 Subject: [PATCH 02/16] fix quic --- .../Unix/System.Security.Cryptography.Native/Interop.Ssl.cs | 6 ------ src/libraries/System.Net.Quic/src/System.Net.Quic.csproj | 6 ++++++ .../src/System/Net/Security/SslStreamPal.Unix.cs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 6f075d5a1c61d1..e5054f04a94c13 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -352,12 +352,6 @@ internal void MarkHandshakeCompleted() _handshakeCompleted = true; } - - public static SafeSslHandle Create(SslAuthenticationOptions sslAuthenticationOptions) - { - return Interop.OpenSsl.AllocateSslHandle(sslAuthenticationOptions); - } - public static SafeSslHandle Create(SafeSslContextHandle context, bool isServer) { SafeBioHandle readBio = Interop.Crypto.CreateMemoryBio(); diff --git a/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj b/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj index 5a80b1dfb04228..c8d5c58ca9ba60 100644 --- a/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj +++ b/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj @@ -90,6 +90,12 @@ + + + + + + diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 82302148e37705..cf3e8559ca5558 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -162,7 +162,7 @@ private static SecurityStatusPal HandshakeInternal(ref SafeDeleteSslContext? con { if ((null == context) || context.IsInvalid) { - context = SafeSslHandle.Create(sslAuthenticationOptions); + context = Interop.OpenSsl.AllocateSslHandle(sslAuthenticationOptions); } SecurityStatusPalErrorCode errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, inputBuffer, out output, out outputSize); From 11d28212fc2e943a2a49a6deda8fdf5359e51110 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 18 May 2022 19:13:03 -0700 Subject: [PATCH 03/16] fix http --- src/libraries/System.Net.Http/src/System.Net.Http.csproj | 3 +++ 1 file changed, 3 insertions(+) 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 82d617b69e5e4f..e22eff73efc750 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -567,6 +567,7 @@ Link="Common\System\Threading\Tasks\TaskToApm.cs" /> + + Date: Thu, 19 May 2022 22:37:04 -0700 Subject: [PATCH 04/16] update quic --- src/libraries/System.Net.Quic/src/System.Net.Quic.csproj | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj b/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj index b262c1543723b6..39d6fbb45e798d 100644 --- a/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj +++ b/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj @@ -85,12 +85,6 @@ - - - - - - From 58980a20a7ff5bbd5163d261ac56a0f7c5c74592 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 8 Jun 2022 02:26:49 -0700 Subject: [PATCH 05/16] feedback from review --- .../Interop.OpenSsl.cs | 51 +------- .../Net/Security/Unix/SafeDeleteContext.cs | 25 ---- .../Security/Unix/SafeDeleteNegoContext.cs | 4 +- .../Security/Unix/SafeFreeSslCredentials.cs | 116 ------------------ .../src/System.Net.Security.csproj | 4 - .../Pal.Android/SafeDeleteSslContext.cs | 27 ++-- .../Pal.Managed/SafeFreeSslCredentials.cs | 49 -------- .../Security/Pal.OSX/SafeDeleteSslContext.cs | 24 ++-- .../SslStreamCertificateContext.Linux.cs | 36 ++++++ .../Net/Security/SslStreamPal.Android.cs | 6 +- .../System/Net/Security/SslStreamPal.OSX.cs | 6 +- 11 files changed, 72 insertions(+), 276 deletions(-) delete mode 100644 src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs delete mode 100644 src/libraries/System.Net.Security/src/System/Net/Security/Pal.Managed/SafeFreeSslCredentials.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index a7b52fa7dc26f7..0675565803dc12 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -224,7 +224,7 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SslAuthentication if (sslAuthenticationOptions.CertificateContext != null) { - SetSslCertificate(sslCtx, sslAuthenticationOptions.CertificateContext.Certificate); + SetSslCertificate(sslCtx, sslAuthenticationOptions.CertificateContext.CertificateHandle, sslAuthenticationOptions.CertificateContext.KeyHandle); if (sslAuthenticationOptions.CertificateContext.IntermediateCertificates.Length > 0) { @@ -259,20 +259,16 @@ internal static void UpdateClientCertiticate(SafeSslHandle ssl, SslAuthenticatio return; } - var credential = new SafeFreeSslCredentials(sslAuthenticationOptions.CertificateContext, sslAuthenticationOptions.EnabledSslProtocols, sslAuthenticationOptions.EncryptionPolicy, sslAuthenticationOptions.IsServer); - SafeX509Handle? certHandle = credential.CertHandle; - SafeEvpPKeyHandle? certKeyHandle = credential.CertKeyHandle; + Debug.Assert(sslAuthenticationOptions.CertificateContext.CertificateHandle != null); + Debug.Assert(sslAuthenticationOptions.CertificateContext.KeyHandle != null); - Debug.Assert(certHandle != null); - Debug.Assert(certKeyHandle != null); - - int retVal = Ssl.SslUseCertificate(ssl, certHandle); + int retVal = Ssl.SslUseCertificate(ssl, sslAuthenticationOptions.CertificateContext.CertificateHandle); if (1 != retVal) { throw CreateSslException(SR.net_ssl_use_cert_failed); } - retVal = Ssl.SslUsePrivateKey(ssl, certKeyHandle); + retVal = Ssl.SslUsePrivateKey(ssl, sslAuthenticationOptions.CertificateContext.KeyHandle); if (1 != retVal) { throw CreateSslException(SR.net_ssl_use_private_key_failed); @@ -285,7 +281,6 @@ internal static void UpdateClientCertiticate(SafeSslHandle ssl, SslAuthenticatio throw CreateSslException(SR.net_ssl_use_cert_failed); } } - } // This essentially wraps SSL* SSL_new() @@ -829,42 +824,6 @@ private static void BioWrite(SafeBioHandle bio, ReadOnlySpan buffer) return innerError; } - private static void SetSslCertificate(SafeSslContextHandle contextPtr, X509Certificate2 cert) - { - SafeEvpPKeyHandle? certKeyHandle = null; - - using (RSAOpenSsl? rsa = (RSAOpenSsl?)cert.GetRSAPrivateKey()) - { - if (rsa != null) - { - certKeyHandle = rsa.DuplicateKeyHandle(); - Interop.Crypto.CheckValidOpenSslHandle(certKeyHandle); - } - } - - if (certKeyHandle == null) - { - using (ECDsaOpenSsl? ecdsa = (ECDsaOpenSsl?)cert.GetECDsaPrivateKey()) - { - if (ecdsa != null) - { - certKeyHandle = ecdsa.DuplicateKeyHandle(); - Interop.Crypto.CheckValidOpenSslHandle(certKeyHandle); - } - } - } - - if (certKeyHandle == null) - { - throw new NotSupportedException(SR.net_ssl_io_no_server_cert); - } - - using (SafeX509Handle certHandle = Interop.Crypto.X509UpRef(cert.Handle)) - { - SetSslCertificate(contextPtr, certHandle, certKeyHandle); - } - } - private static void SetSslCertificate(SafeSslContextHandle contextPtr, SafeX509Handle certPtr, SafeEvpPKeyHandle keyPtr) { Debug.Assert(certPtr != null && !certPtr.IsInvalid, "certPtr != null && !certPtr.IsInvalid"); diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs index 01f0f1c30b3965..ebe3714b874c0b 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs @@ -13,41 +13,16 @@ internal abstract class SafeDeleteContext : DebugSafeHandle internal abstract class SafeDeleteContext : SafeHandle { #endif - private SafeFreeCredentials? _credential; - - protected SafeDeleteContext(SafeFreeCredentials credential) - : base(IntPtr.Zero, true) - { - Debug.Assert((null != credential), "Invalid credential passed to SafeDeleteContext"); - - // When a credential handle is first associated with the context we keep credential - // ref count bumped up to ensure ordered finalization. The credential properties - // are used in the SSL/NEGO data structures and should survive the lifetime of - // the SSL/NEGO context - bool ignore = false; - _credential = credential; - _credential.DangerousAddRef(ref ignore); - } - public SafeDeleteContext(IntPtr handle) : base(handle, true) { - _credential = null; } public SafeDeleteContext(IntPtr handle, bool ownsHandle) : base(handle, ownsHandle) { - _credential = null; - } - - public override bool IsInvalid - { - get { return (null == _credential); } } protected override bool ReleaseHandle() { - _credential?.DangerousRelease(); - _credential = null!; return true; } } diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs index 2e5a0b85600276..ac401e6d609e30 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs @@ -44,13 +44,13 @@ public SafeGssContextHandle? GssContext } public SafeDeleteNegoContext(SafeFreeNegoCredentials credential) - : base(credential) + : base(IntPtr.Zero) { Debug.Assert((null != credential), "Null credential in SafeDeleteNegoContext"); } public SafeDeleteNegoContext(SafeFreeNegoCredentials credential, string targetName) - : this(credential) + : base(IntPtr.Zero) { try { diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs deleted file mode 100644 index ae6bab3968d49e..00000000000000 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs +++ /dev/null @@ -1,116 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.Win32.SafeHandles; - -using System.Diagnostics; -using System.Runtime.InteropServices; -using System.Security.Authentication; -using System.Security.Authentication.ExtendedProtection; -using System.Security.Cryptography; -using System.Security.Cryptography.X509Certificates; - -#pragma warning disable CA1419 // TODO https://github.com/dotnet/roslyn-analyzers/issues/5232: not intended for use with P/Invoke - -namespace System.Net.Security -{ - internal sealed class SafeFreeSslCredentials : SafeFreeCredentials - { - private SafeX509Handle? _certHandle; - private SafeEvpPKeyHandle? _certKeyHandle; - private SslProtocols _protocols = SslProtocols.None; - private EncryptionPolicy _policy; - private bool _isInvalid; - private SslStreamCertificateContext? _context; - - internal SafeX509Handle? CertHandle - { - get { return _certHandle; } - } - - internal SafeEvpPKeyHandle? CertKeyHandle - { - get { return _certKeyHandle; } - } - - internal SslProtocols Protocols - { - get { return _protocols; } - } - - internal EncryptionPolicy Policy - { - get { return _policy; } - } - - public SafeFreeSslCredentials(SslStreamCertificateContext? context, SslProtocols protocols, EncryptionPolicy policy, bool isServer) - : base(IntPtr.Zero, true) - { - - Debug.Assert( - context == null || context.Certificate is X509Certificate2, - "Only X509Certificate2 certificates are supported at this time"); - - X509Certificate2? cert = context?.Certificate; - - if (cert != null) - { - Debug.Assert(cert.HasPrivateKey, "cert.HasPrivateKey"); - - using (RSAOpenSsl? rsa = (RSAOpenSsl?)cert.GetRSAPrivateKey()) - { - if (rsa != null) - { - _certKeyHandle = rsa.DuplicateKeyHandle(); - Interop.Crypto.CheckValidOpenSslHandle(_certKeyHandle); - } - } - - if (_certKeyHandle == null) - { - using (ECDsaOpenSsl? ecdsa = (ECDsaOpenSsl?)cert.GetECDsaPrivateKey()) - { - if (ecdsa != null) - { - _certKeyHandle = ecdsa.DuplicateKeyHandle(); - Interop.Crypto.CheckValidOpenSslHandle(_certKeyHandle); - } - } - } - - if (_certKeyHandle == null) - { - throw new NotSupportedException(SR.net_ssl_io_no_server_cert); - } - - _certHandle = Interop.Crypto.X509UpRef(cert.Handle); - Interop.Crypto.CheckValidOpenSslHandle(_certHandle); - } - - _protocols = protocols; - _policy = policy; - _context = context; - } - - public override bool IsInvalid - { - get { return _isInvalid; } - } - - protected override bool ReleaseHandle() - { - if (_certHandle != null) - { - _certHandle.Dispose(); - } - - if (_certKeyHandle != null) - { - _certKeyHandle.Dispose(); - } - - _isInvalid = true; - return true; - } - } -} 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 50a94c18ae0645..ecbfece640e31a 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -353,8 +353,6 @@ Link="Common\System\Net\Security\Unix\SafeDeleteSslContext.cs" /> - - @@ -403,7 +400,6 @@ - diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs index 417cfe317fbbc3..6b43554eb24958 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs @@ -38,15 +38,13 @@ internal sealed class SafeDeleteSslContext : SafeDeleteContext public SafeSslHandle SslContext => _sslContext; - public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions authOptions) - : base(credential) + public SafeDeleteSslContext(SslAuthenticationOptions authOptions) + : base(IntPtr.Zero) { - Debug.Assert((credential != null) && !credential.IsInvalid, "Invalid credential used in SafeDeleteSslContext"); - try { - _sslContext = CreateSslContext(credential); - InitializeSslContext(_sslContext, credential, authOptions); + _sslContext = CreateSslContext(authOptions); + InitializeSslContext(_sslContext, authOptions); } catch (Exception ex) { @@ -149,14 +147,14 @@ internal int ReadPendingWrites(byte[] buf, int offset, int count) return limit; } - private static SafeSslHandle CreateSslContext(SafeFreeSslCredentials credential) + private static SafeSslHandle CreateSslContext(SslAuthenticationOptions authOptions) { - if (credential.CertificateContext == null) + if (authOptions.CertificateContext == null) { return Interop.AndroidCrypto.SSLStreamCreate(); } - SslStreamCertificateContext context = credential.CertificateContext; + SslStreamCertificateContext context = authOptions.CertificateContext; X509Certificate2 cert = context.Certificate; Debug.Assert(context.Certificate.HasPrivateKey); @@ -201,10 +199,9 @@ private static AsymmetricAlgorithm GetPrivateKeyAlgorithm(X509Certificate2 cert, private unsafe void InitializeSslContext( SafeSslHandle handle, - SafeFreeSslCredentials credential, SslAuthenticationOptions authOptions) { - switch (credential.Policy) + switch (authOptions.EncryptionPolicy) { case EncryptionPolicy.RequireEncryption: #pragma warning disable SYSLIB0040 // NoEncryption and AllowNoEncryption are obsolete @@ -212,7 +209,7 @@ private unsafe void InitializeSslContext( break; #pragma warning restore SYSLIB0040 default: - throw new PlatformNotSupportedException(SR.Format(SR.net_encryptionpolicy_notsupported, credential.Policy)); + throw new PlatformNotSupportedException(SR.Format(SR.net_encryptionpolicy_notsupported, authOptions.EncryptionPolicy)); } bool isServer = authOptions.IsServer; @@ -228,12 +225,12 @@ private unsafe void InitializeSslContext( IntPtr managedContextHandle = GCHandle.ToIntPtr(GCHandle.Alloc(this, GCHandleType.Weak)); Interop.AndroidCrypto.SSLStreamInitialize(handle, isServer, managedContextHandle, &ReadFromConnection, &WriteToConnection, InitialBufferSize); - if (credential.Protocols != SslProtocols.None) + if (authOptions.EnabledSslProtocols != SslProtocols.None) { - SslProtocols protocolsToEnable = credential.Protocols & s_supportedSslProtocols.Value; + SslProtocols protocolsToEnable = authOptions.EnabledSslProtocols & s_supportedSslProtocols.Value; if (protocolsToEnable == 0) { - throw new PlatformNotSupportedException(SR.Format(SR.net_security_sslprotocol_notsupported, credential.Protocols)); + throw new PlatformNotSupportedException(SR.Format(SR.net_security_sslprotocol_notsupported, authOptions.EnabledSslProtocols)); } (int minIndex, int maxIndex) = protocolsToEnable.ValidateContiguous(s_orderedSslProtocols); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Managed/SafeFreeSslCredentials.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Managed/SafeFreeSslCredentials.cs deleted file mode 100644 index c047b88e5c5210..00000000000000 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Managed/SafeFreeSslCredentials.cs +++ /dev/null @@ -1,49 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.Net.Security; -using System.Security.Authentication; -using System.Security.Cryptography.X509Certificates; - -#pragma warning disable CA1419 // TODO https://github.com/dotnet/roslyn-analyzers/issues/5232: not intended for use with P/Invoke - -namespace System.Net -{ - internal sealed class SafeFreeSslCredentials : SafeFreeCredentials - { - public SafeFreeSslCredentials(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy) - : base(IntPtr.Zero, true) - { - if (certificateContext != null) - { - // Make a defensive copy of the certificate. In some async cases the - // certificate can have been disposed before being provided to the handshake. - // - // This meshes with the Unix (OpenSSL) PAL, because it extracts the private key - // and cert handle (which get up-reffed) to match the API expectations. - certificateContext = certificateContext.Duplicate(); - - Debug.Assert(certificateContext.Certificate.HasPrivateKey, "cert clone.HasPrivateKey"); - } - - CertificateContext = certificateContext; - Protocols = protocols; - Policy = policy; - } - - public EncryptionPolicy Policy { get; } - - public SslProtocols Protocols { get; } - - public SslStreamCertificateContext? CertificateContext { get; } - - public override bool IsInvalid => false; - - protected override bool ReleaseHandle() - { - CertificateContext?.Certificate.Dispose(); - return true; - } - } -} diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index 1b523ccb97a8d6..0fe89c28a0ec1e 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -27,16 +27,14 @@ internal sealed class SafeDeleteSslContext : SafeDeleteContext public SafeSslHandle SslContext => _sslContext; - public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) - : base(credential) + public SafeDeleteSslContext(SslAuthenticationOptions sslAuthenticationOptions) + : base(IntPtr.Zero) { - Debug.Assert((null != credential) && !credential.IsInvalid, "Invalid credential used in SafeDeleteSslContext"); - try { int osStatus; - _sslContext = CreateSslContext(credential, sslAuthenticationOptions.IsServer); + _sslContext = CreateSslContext(sslAuthenticationOptions); // Make sure the class instance is associated to the session and is provided // in the Read/Write callback connection parameter @@ -131,9 +129,9 @@ public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthentication } } - private static SafeSslHandle CreateSslContext(SafeFreeSslCredentials credential, bool isServer) + private static SafeSslHandle CreateSslContext(SslAuthenticationOptions options) { - switch (credential.Policy) + switch (options.EncryptionPolicy) { case EncryptionPolicy.RequireEncryption: #pragma warning disable SYSLIB0040 // NoEncryption and AllowNoEncryption are obsolete @@ -144,10 +142,10 @@ private static SafeSslHandle CreateSslContext(SafeFreeSslCredentials credential, break; #pragma warning restore SYSLIB0040 default: - throw new PlatformNotSupportedException(SR.Format(SR.net_encryptionpolicy_notsupported, credential.Policy)); + throw new PlatformNotSupportedException(SR.Format(SR.net_encryptionpolicy_notsupported, options.EncryptionPolicy)); } - SafeSslHandle sslContext = Interop.AppleCrypto.SslCreateContext(isServer ? 1 : 0); + SafeSslHandle sslContext = Interop.AppleCrypto.SslCreateContext(options.IsServer ? 1 : 0); try { @@ -159,14 +157,14 @@ private static SafeSslHandle CreateSslContext(SafeFreeSslCredentials credential, } // Let None mean "system default" - if (credential.Protocols != SslProtocols.None) + if (options.EnabledSslProtocols != SslProtocols.None) { - SetProtocols(sslContext, credential.Protocols); + SetProtocols(sslContext, options.EnabledSslProtocols); } - if (credential.CertificateContext != null) + if (options.CertificateContext != null) { - SetCertificate(sslContext, credential.CertificateContext); + SetCertificate(sslContext, options.CertificateContext); } Interop.AppleCrypto.SslBreakOnCertRequested(sslContext, true); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs index dedea329f240eb..a5916bd68e0b8b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Security.Authentication; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; using System.Threading.Tasks; @@ -17,6 +18,8 @@ public partial class SslStreamCertificateContext { private const bool TrimRootCertificate = true; internal readonly ConcurrentDictionary SslContexts; + internal readonly SafeX509Handle CertificateHandle; + internal readonly SafeEvpPKeyHandle KeyHandle; private bool _staplingForbidden; private byte[]? _ocspResponse; @@ -32,11 +35,44 @@ private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[] IntermediateCertificates = intermediates; Trust = trust; SslContexts = new ConcurrentDictionary(); + + + using (RSAOpenSsl? rsa = (RSAOpenSsl?)target.GetRSAPrivateKey()) + { + if (rsa != null) + { + KeyHandle = rsa.DuplicateKeyHandle(); + } + } + + if (KeyHandle== null) + { + using (ECDsaOpenSsl? ecdsa = (ECDsaOpenSsl?)target.GetECDsaPrivateKey()) + { + if (ecdsa != null) + { + KeyHandle = ecdsa.DuplicateKeyHandle(); + } + } + } + + if (KeyHandle== null) + { + throw new NotSupportedException(SR.net_ssl_io_no_server_cert); + } + + CertificateHandle = Interop.Crypto.X509UpRef(target.Handle); } internal static SslStreamCertificateContext Create(X509Certificate2 target) => Create(target, null, offline: false, trust: null, noOcspFetch: true); + ~SslStreamCertificateContext() + { + CertificateHandle?.Dispose(); + KeyHandle?.Dispose(); + } + internal bool OcspStaplingAvailable => _ocspUrls is not null; partial void SetNoOcspFetch(bool noOcspFetch) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index bbe3f1a43557b4..8f0c8b3cdaf76d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -55,13 +55,13 @@ public static SecurityStatusPal Renegotiate( throw new PlatformNotSupportedException(); } - public static SafeFreeCredentials AcquireCredentialsHandle( + public static SafeFreeCredentials? AcquireCredentialsHandle( SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { - return new SafeFreeSslCredentials(certificateContext, protocols, policy); + return null; } public static SecurityStatusPal EncryptMessage( @@ -186,7 +186,7 @@ private static SecurityStatusPal HandshakeInternal( if ((context == null) || context.IsInvalid) { - context = new SafeDeleteSslContext((credential as SafeFreeSslCredentials)!, sslAuthenticationOptions); + context = new SafeDeleteSslContext(sslAuthenticationOptions); sslContext = context; } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 73c80b20dab48d..59e7d0a2fa601f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -62,13 +62,13 @@ public static SecurityStatusPal Renegotiate( throw new PlatformNotSupportedException(); } - public static SafeFreeCredentials AcquireCredentialsHandle( + public static SafeFreeCredentials? AcquireCredentialsHandle( SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { - return new SafeFreeSslCredentials(certificateContext, protocols, policy); + return null; } public static SecurityStatusPal EncryptMessage( @@ -236,7 +236,7 @@ private static SecurityStatusPal HandshakeInternal( if ((null == context) || context.IsInvalid) { - sslContext = new SafeDeleteSslContext((credential as SafeFreeSslCredentials)!, sslAuthenticationOptions); + sslContext = new SafeDeleteSslContext(sslAuthenticationOptions); context = sslContext; } From b51f296d4681849ad74647eea4fec75049b988ab Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 8 Jun 2022 02:40:45 -0700 Subject: [PATCH 06/16] feedback from review --- .../src/System/Net/Security/SslStream.Protocol.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 3954146a5c4e00..dca52a0f9b026f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -835,7 +835,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte internal SecurityStatusPal Renegotiate(out byte[]? output) { - ArgumentNullException.ThrowIfNull(_securityContext); + Debug.Assert(_securityContext != null); return SslStreamPal.Renegotiate( ref _credentialsHandle!, From 9c2f79bd4b7ff9ff8da0c1df0e0b4343e23a56a5 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 8 Jun 2022 06:51:52 -0700 Subject: [PATCH 07/16] add IsInvalid --- .../Common/src/System/Net/Security/Unix/SafeDeleteContext.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs index ebe3714b874c0b..b26e691ea40380 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteContext.cs @@ -21,6 +21,11 @@ public SafeDeleteContext(IntPtr handle, bool ownsHandle) : base(handle, ownsHand { } + public override bool IsInvalid + { + get { return (IntPtr.Zero == handle); } + } + protected override bool ReleaseHandle() { return true; From 24a5cde25f0d516c7c7520aa869ea73efbeb3d3f Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 9 Jun 2022 10:52:36 +0200 Subject: [PATCH 08/16] remove obsolete assert --- .../src/System/Net/Security/SslStreamPal.Android.cs | 2 -- .../src/System/Net/Security/SslStreamPal.OSX.cs | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 8f0c8b3cdaf76d..f0176b708850ef 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -178,8 +178,6 @@ private static SecurityStatusPal HandshakeInternal( ref byte[]? outputBuffer, SslAuthenticationOptions sslAuthenticationOptions) { - Debug.Assert(!credential.IsInvalid); - try { SafeDeleteSslContext? sslContext = ((SafeDeleteSslContext?)context); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 59e7d0a2fa601f..9fd40511494bfa 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -228,8 +228,6 @@ private static SecurityStatusPal HandshakeInternal( SslAuthenticationOptions sslAuthenticationOptions, SelectClientCertificate? clientCertificateSelectionCallback) { - Debug.Assert(!credential.IsInvalid); - try { SafeDeleteSslContext? sslContext = ((SafeDeleteSslContext?)context); From 649d8a39aa8676000fd968a5d06acb291e179039 Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 17 Jun 2022 01:27:04 -0700 Subject: [PATCH 09/16] fix failing tests --- .../Net/Security/Unix/SafeDeleteNegoContext.cs | 13 +++++++++++++ .../Net/Security/Unix/SafeFreeNegoCredentials.cs | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs index ac401e6d609e30..1c7b700027c6c9 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs @@ -17,6 +17,7 @@ internal sealed class SafeDeleteNegoContext : SafeDeleteContext private SafeGssNameHandle? _targetName; private SafeGssContextHandle? _context; private bool _isNtlmUsed; + private SafeFreeNegoCredentials? _credential; public SafeGssCredHandle AcceptorCredential { @@ -47,6 +48,9 @@ public SafeDeleteNegoContext(SafeFreeNegoCredentials credential) : base(IntPtr.Zero) { Debug.Assert((null != credential), "Null credential in SafeDeleteNegoContext"); + _credential = credential; + bool ignore = false; + _credential.DangerousAddRef(ref ignore); } public SafeDeleteNegoContext(SafeFreeNegoCredentials credential, string targetName) @@ -61,6 +65,9 @@ public SafeDeleteNegoContext(SafeFreeNegoCredentials credential, string targetNa Dispose(); throw; } + _credential = credential; + bool ignore = false; + _credential.DangerousAddRef(ref ignore); } public void SetGssContext(SafeGssContextHandle context) @@ -95,6 +102,12 @@ protected override void Dispose(bool disposing) _acceptorCredential.Dispose(); _acceptorCredential = null; } + + if (_credential != null) + { + _credential.Dispose(); + _credential = null; + } } base.Dispose(disposing); } diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeNegoCredentials.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeNegoCredentials.cs index fef571d5f7dbd8..cf4f3b9df3d156 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeNegoCredentials.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeNegoCredentials.cs @@ -46,7 +46,7 @@ public SafeFreeNegoCredentials(bool isNtlmOnly, string username, string password const char At = '@'; const char Backwhack = '\\'; - // any invalid user format will not be mnipulated and passed as it is. + // any invalid user format will not be manipulated and passed as it is. int index = username.IndexOf(Backwhack); if (index > 0 && username.IndexOf(Backwhack, index + 1) < 0 && string.IsNullOrEmpty(domain)) { From b81616667ef33e463652a766069fecf9be7cb5f2 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 1 Jul 2022 11:53:57 +0000 Subject: [PATCH 10/16] add IsInvalid overload --- .../src/System/Net/Security/Unix/SafeDeleteNegoContext.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs index cb80598e22ae81..3c5f4b08848432 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs @@ -79,6 +79,11 @@ public void SetAuthenticationPackage(bool isNtlmUsed) _isNtlmUsed = isNtlmUsed; } + public override bool IsInvalid + { + get { return (null == _credential); } + } + protected override void Dispose(bool disposing) { if (disposing) From 83b89a39037ab58e9e674dccded6f80315433532 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 11 Jul 2022 02:13:23 -0700 Subject: [PATCH 11/16] use DangerousRelease instead of Dispose --- .../src/System/Net/Security/Unix/SafeDeleteNegoContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs index 3765d928fb2a1f..10a2b4ba689752 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs @@ -105,7 +105,7 @@ protected override void Dispose(bool disposing) if (_credential != null) { - _credential.Dispose(); + _credential.DangerousRelease(); _credential = null; } } From 1ffbbafe6ad52f7025d754d8d17abd2bb06ab5c9 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 11 Aug 2022 22:51:16 -0700 Subject: [PATCH 12/16] resolve --- .../src/System/Net/Security/SslStreamPal.Unix.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 67c19ae51beca8..1e1a0df55889e9 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -47,7 +47,6 @@ public static SecurityStatusPal InitializeSecurityContext( } public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) - SslProtocols protocols, EncryptionPolicy policy, bool isServer) { return null; } From 3349cc2f2228e24e25cd058082a3711cb96a0769 Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 12 Aug 2022 13:01:07 -0700 Subject: [PATCH 13/16] resolve merge --- .../src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index ae928490dfb74f..fd86353ed1fa58 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -129,7 +129,7 @@ public SafeDeleteSslContext(SslAuthenticationOptions sslAuthenticationOptions) private static SafeSslHandle CreateSslContext(SslAuthenticationOptions sslAuthenticationOptions) { - switch (options.EncryptionPolicy) + switch (sslAuthenticationOptions.EncryptionPolicy) { case EncryptionPolicy.RequireEncryption: #pragma warning disable SYSLIB0040 // NoEncryption and AllowNoEncryption are obsolete @@ -140,7 +140,7 @@ private static SafeSslHandle CreateSslContext(SslAuthenticationOptions sslAuthen break; #pragma warning restore SYSLIB0040 default: - throw new PlatformNotSupportedException(SR.Format(SR.net_encryptionpolicy_notsupported, options.EncryptionPolicy)); + throw new PlatformNotSupportedException(SR.Format(SR.net_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); } SafeSslHandle sslContext = Interop.AppleCrypto.SslCreateContext(sslAuthenticationOptions.IsServer ? 1 : 0); From 725305f54999ce904bc398d44a5e5f9203e94f8f Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 12 Aug 2022 16:39:27 -0700 Subject: [PATCH 14/16] resolve merge --- .../src/System/Net/Security/SslStreamPal.Android.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 5622133e75c473..f01dd68e294b2d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -56,10 +56,6 @@ public static SecurityStatusPal Renegotiate( } public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) - SslStreamCertificateContext? certificateContext, - SslProtocols protocols, - EncryptionPolicy policy, - bool isServer) { return null; } From c81d78c6c7f55bc557e9a8300707ac07598618e0 Mon Sep 17 00:00:00 2001 From: wfurt Date: Sat, 13 Aug 2022 21:16:37 -0700 Subject: [PATCH 15/16] feedback from review --- .../Security/Unix/SafeDeleteNegoContext.cs | 20 +++++++++++-------- .../SslStreamCertificateContext.Linux.cs | 16 +++++---------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs index 10a2b4ba689752..8f1e037f1f778c 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs @@ -46,9 +46,13 @@ public SafeDeleteNegoContext(SafeFreeNegoCredentials credential) : base(IntPtr.Zero) { Debug.Assert((null != credential), "Null credential in SafeDeleteNegoContext"); + bool added = false; + credential.DangerousAddRef(ref added); + if (!added) + { + throw new ObjectDisposedException(nameof(SafeFreeNegoCredentials)); + } _credential = credential; - bool ignore = false; - _credential.DangerousAddRef(ref ignore); _context = new SafeGssContextHandle(); } @@ -102,14 +106,14 @@ protected override void Dispose(bool disposing) _acceptorCredential.Dispose(); _acceptorCredential = null; } - - if (_credential != null) - { - _credential.DangerousRelease(); - _credential = null; - } } base.Dispose(disposing); } + + protected override bool ReleaseHandle() + { + _credential?.DangerousRelease(); + return true; + } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs index a5916bd68e0b8b..90cad74fbaecbd 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs @@ -45,7 +45,7 @@ private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[] } } - if (KeyHandle== null) + if (KeyHandle == null) { using (ECDsaOpenSsl? ecdsa = (ECDsaOpenSsl?)target.GetECDsaPrivateKey()) { @@ -54,11 +54,11 @@ private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[] KeyHandle = ecdsa.DuplicateKeyHandle(); } } - } - if (KeyHandle== null) - { - throw new NotSupportedException(SR.net_ssl_io_no_server_cert); + if (KeyHandle== null) + { + throw new NotSupportedException(SR.net_ssl_io_no_server_cert); + } } CertificateHandle = Interop.Crypto.X509UpRef(target.Handle); @@ -67,12 +67,6 @@ private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[] internal static SslStreamCertificateContext Create(X509Certificate2 target) => Create(target, null, offline: false, trust: null, noOcspFetch: true); - ~SslStreamCertificateContext() - { - CertificateHandle?.Dispose(); - KeyHandle?.Dispose(); - } - internal bool OcspStaplingAvailable => _ocspUrls is not null; partial void SetNoOcspFetch(bool noOcspFetch) From 2e7c5f3080b999161c78ab4de1e6f977ee2d9795 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Mon, 15 Aug 2022 13:00:22 -0700 Subject: [PATCH 16/16] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../src/System/Net/Security/Unix/SafeDeleteNegoContext.cs | 4 ---- .../System/Net/Security/SslStreamCertificateContext.Linux.cs | 1 - 2 files changed, 5 deletions(-) diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs index 8f1e037f1f778c..443e98e9de4b4f 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs @@ -48,10 +48,6 @@ public SafeDeleteNegoContext(SafeFreeNegoCredentials credential) Debug.Assert((null != credential), "Null credential in SafeDeleteNegoContext"); bool added = false; credential.DangerousAddRef(ref added); - if (!added) - { - throw new ObjectDisposedException(nameof(SafeFreeNegoCredentials)); - } _credential = credential; _context = new SafeGssContextHandle(); } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs index 90cad74fbaecbd..fdc98705d426b2 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs @@ -36,7 +36,6 @@ private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[] Trust = trust; SslContexts = new ConcurrentDictionary(); - using (RSAOpenSsl? rsa = (RSAOpenSsl?)target.GetRSAPrivateKey()) { if (rsa != null)