Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid allocation of SafeFreeSslCredentials and SafeDeleteSslContext on Linux #69527

Merged
merged 22 commits into from
Aug 15, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -225,17 +222,10 @@ 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)
{
SetSslCertificate(sslCtx, certHandle!, certKeyHandle!);
}

if (sslAuthenticationOptions.CertificateContext != null)
{
SetSslCertificate(sslCtx, sslAuthenticationOptions.CertificateContext.CertificateHandle, sslAuthenticationOptions.CertificateContext.KeyHandle);

if (sslAuthenticationOptions.CertificateContext.IntermediateCertificates.Length > 0)
{
if (!Ssl.AddExtraChainCertificates(sslCtx, sslAuthenticationOptions.CertificateContext.IntermediateCertificates))
Expand Down Expand Up @@ -269,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);
Expand All @@ -295,11 +281,10 @@ internal static void UpdateClientCertiticate(SafeSslHandle ssl, SslAuthenticatio
throw CreateSslException(SR.net_ssl_use_cert_failed);
}
}

}

// 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;
Expand Down Expand Up @@ -352,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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ internal enum SslErrorCode

namespace Microsoft.Win32.SafeHandles
{
internal sealed class SafeSslHandle : SafeHandle
internal sealed class SafeSslHandle : SafeDeleteSslContext
{
private SafeBioHandle? _readBio;
private SafeBioHandle? _writeBio;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,21 @@ internal abstract class SafeDeleteContext : DebugSafeHandle
internal abstract class SafeDeleteContext : SafeHandle
{
#endif
private SafeFreeCredentials _credential;

protected SafeDeleteContext(SafeFreeCredentials credential)
: base(IntPtr.Zero, true)
public SafeDeleteContext(IntPtr handle) : base(handle, 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, bool ownsHandle) : base(handle, ownsHandle)
{
}

public override bool IsInvalid
{
get { return (null == _credential); }
get { return (IntPtr.Zero == handle); }
}

protected override bool ReleaseHandle()
{
Debug.Assert((null != _credential), "Null credential in SafeDeleteContext");
_credential.DangerousRelease();
_credential = null!;
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ internal sealed class SafeDeleteNegoContext : SafeDeleteContext
private SafeGssNameHandle? _targetName;
private SafeGssContextHandle _context;
private bool _isNtlmUsed;
private SafeFreeNegoCredentials? _credential;

public SafeGssCredHandle AcceptorCredential
{
Expand Down Expand Up @@ -42,14 +43,21 @@ public SafeGssContextHandle GssContext
}

public SafeDeleteNegoContext(SafeFreeNegoCredentials credential)
: base(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));
}
wfurt marked this conversation as resolved.
Show resolved Hide resolved
_credential = credential;
wfurt marked this conversation as resolved.
Show resolved Hide resolved
_context = new SafeGssContextHandle();
}

public SafeDeleteNegoContext(SafeFreeNegoCredentials credential, string targetName)
: this(credential)
: base(IntPtr.Zero)
{
try
{
Expand All @@ -61,6 +69,9 @@ public SafeDeleteNegoContext(SafeFreeNegoCredentials credential, string targetNa
Dispose();
throw;
}
_credential = credential;
bool ignore = false;
_credential.DangerousAddRef(ref ignore);
wfurt marked this conversation as resolved.
Show resolved Hide resolved
}

public void SetGssContext(SafeGssContextHandle context)
Expand All @@ -73,6 +84,11 @@ public void SetAuthenticationPackage(bool isNtlmUsed)
_isNtlmUsed = isNtlmUsed;
}

public override bool IsInvalid
{
get { return (null == _credential); }
}

protected override void Dispose(bool disposing)
{
if (disposing)
Expand All @@ -93,5 +109,11 @@ protected override void Dispose(bool disposing)
}
base.Dispose(disposing);
}

protected override bool ReleaseHandle()
{
_credential?.DangerousRelease();
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,55 +13,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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public SafeFreeNegoCredentials(Interop.NetSecurityNative.PackageType packageType
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))
{
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,6 @@
Link="Common\System\Net\Security\Unix\SafeDeleteSslContext.cs" />
<Compile Include="$(CommonPath)System\Net\Security\Unix\SafeFreeCertContext.cs"
Link="Common\System\Net\Security\Unix\SafeFreeCertContext.cs" />
<Compile Include="$(CommonPath)System\Net\Security\Unix\SafeFreeSslCredentials.cs"
Link="Common\System\Net\Security\Unix\SafeFreeSslCredentials.cs" />
</ItemGroup>
<ItemGroup Condition="'$(UseAndroidCrypto)' == 'true'">
<Compile Include="$(CommonPath)Interop\Android\Interop.Libraries.cs"
Expand All @@ -382,7 +380,6 @@
Link="Common\Interop\Android\System.Security.Cryptography.Native.Android\Interop.X509.cs" />
<Compile Include="System\Net\CertificateValidationPal.Android.cs" />
<Compile Include="System\Net\Security\Pal.Android\SafeDeleteSslContext.cs" />
<Compile Include="System\Net\Security\Pal.Managed\SafeFreeSslCredentials.cs" />
<Compile Include="System\Net\Security\Pal.Managed\SslProtocolsValidation.cs" />
<Compile Include="System\Net\Security\SslConnectionInfo.Android.cs" />
<Compile Include="System\Net\Security\SslStreamCertificateContext.Android.cs" />
Expand Down Expand Up @@ -416,7 +413,6 @@
<Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeCreateHandle.OSX.cs"
Link="Common\Microsoft\Win32\SafeHandles\SafeCreateHandle.OSX.cs" />
<Compile Include="System\Net\CertificateValidationPal.OSX.cs" />
<Compile Include="System\Net\Security\Pal.Managed\SafeFreeSslCredentials.cs" />
<Compile Include="System\Net\Security\Pal.Managed\SslProtocolsValidation.cs" />
<Compile Include="System\Net\Security\Pal.OSX\SafeDeleteSslContext.cs" />
<Compile Include="System\Net\Security\SslConnectionInfo.OSX.cs" />
Expand Down
Loading