Skip to content

Commit

Permalink
Port rest of the changes from #79898
Browse files Browse the repository at this point in the history
  • Loading branch information
rzikm committed Dec 7, 2023
1 parent 453a97a commit 8db62ef
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 30 deletions.
18 changes: 17 additions & 1 deletion src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ internal enum ContextAttribute
SECPKG_ATTR_ISSUER_LIST_EX = 0x59, // returns SecPkgContext_IssuerListInfoEx
SECPKG_ATTR_CLIENT_CERT_POLICY = 0x60, // sets SecPkgCred_ClientCertCtlPolicy
SECPKG_ATTR_CONNECTION_INFO = 0x5A, // returns SecPkgContext_ConnectionInfo
SECPKG_ATTR_SESSION_INFO = 0x5D, // sets SecPkgContext_SessionInfo
SECPKG_ATTR_CIPHER_INFO = 0x64, // returns SecPkgContext_CipherInfo
SECPKG_ATTR_REMOTE_CERT_CHAIN = 0x67, // returns PCCERT_CONTEXT
SECPKG_ATTR_UI_INFO = 0x68, // sets SEcPkgContext_UiInfo
Expand Down Expand Up @@ -249,7 +250,7 @@ public enum Flags
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_SEND_AUX_RECORD = 0x00200000,
SCH_USE_STRONG_CRYPTO = 0x00400000,
SCH_USE_PRESHAREDKEY_ONLY = 0x800000,
SCH_ALLOW_NULL_ENCRYPTION = 0x02000000,
Expand Down Expand Up @@ -334,6 +335,21 @@ internal unsafe struct SecPkgCred_ClientCertPolicy
public char* pwszSslCtlIdentifier;
}

[StructLayout(LayoutKind.Sequential)]
internal unsafe struct SecPkgContext_SessionInfo
{
public uint dwFlags;
public uint cbSessionId;
public fixed byte rgbSessionId[32];

[Flags]
public enum Flags
{
Zero = 0,
SSL_SESSION_RECONNECT = 0x01,
};
}

[LibraryImport(Interop.Libraries.SspiCli, SetLastError = true)]
internal static partial int EncryptMessage(
ref CredHandle contextHandle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Diagnostics;
using System.Globalization;
using System.Runtime.InteropServices;
using System.Security.Cryptography.X509Certificates;
using System.Security.Authentication.ExtendedProtection;
using Microsoft.Win32.SafeHandles;

Expand Down Expand Up @@ -310,10 +311,15 @@ public static unsafe int AcquireCredentialsHandle(

internal sealed class SafeFreeCredential_SECURITY : SafeFreeCredentials
{
#pragma warning disable 0649
// This is used only by SslStream but it is included elsewhere
public X509Certificate? LocalCertificate;
#pragma warning restore 0649
public SafeFreeCredential_SECURITY() : base() { }

protected override bool ReleaseHandle()
{
LocalCertificate?.Dispose();
return Interop.SspiCli.FreeCredentialsHandle(ref _handle) == 0;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(
string? hostName)
{
if (remoteCertificate == null)
return SslPolicyErrors.RemoteCertificateNotAvailable;
return SslPolicyErrors.RemoteCertificateNotAvailable;

SslPolicyErrors errors = chain.Build(remoteCertificate)
? SslPolicyErrors.None
Expand Down Expand Up @@ -93,7 +93,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(

// This is only called when we selected local client certificate.
// Currently this is only when Java crypto asked for it.
internal static bool IsLocalCertificateUsed(SafeDeleteContext? _) => true;
internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _1, SafeDeleteContext? _2) => true;

//
// Used only by client SSL code, never returns null.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(

// This is only called when we selected local client certificate.
// Currently this is only when Apple crypto asked for it.
internal static bool IsLocalCertificateUsed(SafeDeleteContext? _) => true;
internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _1, SafeDeleteContext? _2) => true;

//
// Used only by client SSL code, never returns null.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(

// This is only called when we selected local client certificate.
// Currently this is only when OpenSSL needs it because peer asked.
internal static bool IsLocalCertificateUsed(SafeDeleteContext? _) => true;
internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _1, SafeDeleteContext? _2) => true;

//
// Used only by client SSL code, never returns null.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Security.Principal;
using static Interop.SspiCli;

namespace System.Net
{
Expand Down Expand Up @@ -90,8 +91,23 @@ internal static SslPolicyErrors VerifyCertificateProperties(
}

// Check that local certificate was used by schannel.
internal static bool IsLocalCertificateUsed(SafeDeleteContext securityContext)
internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _credentialsHandle, SafeDeleteContext securityContext)
{
SecPkgContext_SessionInfo info = default;
// fails on Server 2008 and older. We will fall-back to probing LOCAL_CERT_CONTEXT in that case.
if (SSPIWrapper.QueryBlittableContextAttributes(
GlobalSSPI.SSPISecureChannel,
securityContext,
Interop.SspiCli.ContextAttribute.SECPKG_ATTR_SESSION_INFO,
ref info) &&
((SecPkgContext_SessionInfo.Flags)info.dwFlags).HasFlag(SecPkgContext_SessionInfo.Flags.SSL_SESSION_RECONNECT))
{
// This is TLS Resumed session. Windows can fail to query the local cert bellow.
// Instead, we will determine the usage form used credentials.
SafeFreeCredential_SECURITY creds = (SafeFreeCredential_SECURITY)_credentialsHandle!;
return creds.LocalCertificate != null;
}

SafeFreeCertContext? localContext = null;
try
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ public partial class SslStream
private int _trailerSize = 16;
private int _maxDataSize = 16354;

private bool _refreshCredentialNeeded = true;

private static readonly Oid s_serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1");
private static readonly Oid s_clientAuthOid = new Oid("1.3.6.1.5.5.7.3.2", "1.3.6.1.5.5.7.3.2");

Expand Down Expand Up @@ -109,11 +107,6 @@ internal bool RemoteCertRequired
}
}

internal void SetRefreshCredentialNeeded()
{
_refreshCredentialNeeded = true;
}

internal void CloseContext()
{
if (!_remoteCertificateExposed)
Expand Down Expand Up @@ -515,7 +508,7 @@ This will not restart a session but helps minimizing the number of handles we cr
--*/

private bool AcquireClientCredentials(ref byte[]? thumbPrint)
private bool AcquireClientCredentials(ref byte[]? thumbPrint, bool newCredentialsRequested = false)
{
// Acquire possible Client Certificate information and set it on the handle.

Expand Down Expand Up @@ -580,7 +573,7 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
_sslAuthenticationOptions.CertificateContext = SslStreamCertificateContext.Create(selectedCert!);
}

_credentialsHandle = AcquireCredentialsHandle(_sslAuthenticationOptions);
_credentialsHandle = AcquireCredentialsHandle(_sslAuthenticationOptions, newCredentialsRequested);
thumbPrint = guessedThumbPrint; // Delay until here in case something above threw.
}
}
Expand Down Expand Up @@ -691,9 +684,9 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint)
return cachedCred;
}

private static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
private static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions, bool newCredentialsRequested = false)
{
SafeFreeCredentials? cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions);
SafeFreeCredentials? cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions, newCredentialsRequested);

if (sslAuthenticationOptions.CertificateContext != null && cred != null)
{
Expand Down Expand Up @@ -753,7 +746,6 @@ internal ProtocolToken NextMessage(ReadOnlySpan<byte> incomingBuffer)
if (NetEventSource.Log.IsEnabled())
NetEventSource.Info(this, "NextMessage() returned SecurityStatusPal.CredentialsNeeded");

SetRefreshCredentialNeeded();
status = GenerateToken(incomingBuffer, ref nextmsg);
}

Expand Down Expand Up @@ -792,6 +784,11 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
bool sendTrustList = false;
byte[]? thumbPrint = null;

// We need to try get credentials at the beginning.
// _credentialsHandle may be always null on some platforms but
// _securityContext will be allocated on first call.
bool refreshCredentialNeeded = _securityContext == null;

//
// Looping through ASC or ISC with potentially cached credential that could have been
// already disposed from a different thread before ISC or ASC dir increment a cred ref count.
Expand All @@ -801,7 +798,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
do
{
thumbPrint = null;
if (_refreshCredentialNeeded)
if (refreshCredentialNeeded)
{
cachedCreds = _sslAuthenticationOptions.IsServer
? AcquireServerCredentials(ref thumbPrint)
Expand Down Expand Up @@ -830,15 +827,31 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
_sslAuthenticationOptions,
SelectClientCertificate
);

if (status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
{
refreshCredentialNeeded = true;
cachedCreds = AcquireClientCredentials(ref thumbPrint, newCredentialsRequested: true);

if (NetEventSource.Log.IsEnabled())
NetEventSource.Info(this, "InitializeSecurityContext() returned 'CredentialsNeeded'.");

status = SslStreamPal.InitializeSecurityContext(
ref _credentialsHandle!,
ref _securityContext,
_sslAuthenticationOptions.TargetHost,
inputBuffer,
ref result,
_sslAuthenticationOptions,
SelectClientCertificate);
}
}
} while (cachedCreds && _credentialsHandle == null);
}
finally
{
if (_refreshCredentialNeeded)
if (refreshCredentialNeeded)
{
_refreshCredentialNeeded = false;

//
// Assuming the ISC or ASC has referenced the credential,
// we want to call dispose so to decrement the effective ref count.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public static SecurityStatusPal Renegotiate(
throw new PlatformNotSupportedException();
}

public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions _1, bool _2)
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public static SecurityStatusPal Renegotiate(
throw new PlatformNotSupportedException();
}

public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions _1, bool _2)
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static SecurityStatusPal InitializeSecurityContext(
return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, clientCertificateSelectionCallback);
}

public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions _1, bool _2)
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public static SecurityStatusPal Renegotiate(
return status;
}

public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions, bool newCredentialsRequested)
{
try
{
Expand All @@ -156,6 +156,16 @@ public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOpti
AttachCertificateStore(cred, certificateContext.Trust._store!);
}

// Windows can fail to get local credentials in case of TLS Resume.
// We will store associated certificate in credentials and use it in case
// of TLS resume. It will be disposed when the credentials are.
if (newCredentialsRequested && sslAuthenticationOptions.CertificateContext != null)
{
SafeFreeCredential_SECURITY handle = (SafeFreeCredential_SECURITY)cred;
// We need to create copy to avoid Disposal issue.
handle.LocalCertificate = new X509Certificate2(sslAuthenticationOptions.CertificateContext.Certificate);
}

return cred;
}
catch (Win32Exception e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public enum ClientCertSource
{
ClientCertificate,
SelectionCallback,
CertificateContext
}

public static TheoryData<ClientCertSource> CertSourceData()
Expand Down Expand Up @@ -184,9 +183,6 @@ public async Task SslStream_NegotiateClientCertificate_IsMutuallyAuthenticatedCo
case ClientCertSource.SelectionCallback:
clientOptions.LocalCertificateSelectionCallback = ClientCertSelectionCallback;
break;
case ClientCertSource.CertificateContext:
clientOptions.ClientCertificateContext = SslStreamCertificateContext.Create(_clientCertificate, new());
break;
}

Task t2 = client.AuthenticateAsClientAsync(clientOptions);
Expand Down

0 comments on commit 8db62ef

Please sign in to comment.