Skip to content

Commit 2eb0717

Browse files
wfurtmdh1418
wfurt
authored and
mdh1418
committed
fix TLS resume with client certificates (dotnet#79898)
* fix TLS resume with client certificates * fix windows * fix unused warning * undo dotnet#79128 test change * remove dead code * fix resolve
1 parent 5a1fefa commit 2eb0717

15 files changed

Lines changed: 256 additions & 38 deletions

‎src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,8 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth
306306
if (!Interop.Ssl.Capabilities.Tls13Supported ||
307307
string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) ||
308308
sslAuthenticationOptions.CertificateContext != null ||
309-
sslAuthenticationOptions.CertSelectionDelegate != null)
309+
sslAuthenticationOptions.ClientCertificates?.Count > 0 ||
310+
sslAuthenticationOptions.CertSelectionDelegate != null)
310311
{
311312
cacheSslContext = false;
312313
}

‎src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ internal bool TryAddSession(IntPtr namePtr, IntPtr session)
134134
if (!string.IsNullOrEmpty(targetName))
135135
{
136136
// We do this only for lookup in RemoveSession.
137-
// Since this is part of chache manipulation and no function impact it is done here.
137+
// Since this is part of cache manipulation and no function impact it is done here.
138138
// This will use strdup() so it is safe to pass in raw pointer.
139139
Interop.Ssl.SessionSetHostname(session, namePtr);
140140

‎src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ internal enum ContextAttribute
6464
SECPKG_ATTR_ISSUER_LIST_EX = 0x59, // returns SecPkgContext_IssuerListInfoEx
6565
SECPKG_ATTR_CLIENT_CERT_POLICY = 0x60, // sets SecPkgCred_ClientCertCtlPolicy
6666
SECPKG_ATTR_CONNECTION_INFO = 0x5A, // returns SecPkgContext_ConnectionInfo
67+
SECPKG_ATTR_SESSION_INFO = 0x5D, // sets SecPkgContext_SessionInfo
6768
SECPKG_ATTR_CIPHER_INFO = 0x64, // returns SecPkgContext_CipherInfo
6869
SECPKG_ATTR_REMOTE_CERT_CHAIN = 0x67, // returns PCCERT_CONTEXT
6970
SECPKG_ATTR_UI_INFO = 0x68, // sets SEcPkgContext_UiInfo
@@ -331,6 +332,21 @@ internal unsafe struct SecPkgCred_ClientCertPolicy
331332
public char* pwszSslCtlIdentifier;
332333
}
333334

335+
[StructLayout(LayoutKind.Sequential)]
336+
internal unsafe struct SecPkgContext_SessionInfo
337+
{
338+
public uint dwFlags;
339+
public uint cbSessionId;
340+
public fixed byte rgbSessionId[32];
341+
342+
[Flags]
343+
public enum Flags
344+
{
345+
Zero = 0,
346+
SSL_SESSION_RECONNECT = 0x01,
347+
};
348+
}
349+
334350
[LibraryImport(Interop.Libraries.SspiCli, SetLastError = true)]
335351
internal static partial int EncryptMessage(
336352
ref CredHandle contextHandle,

‎src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Globalization;
66
using System.Runtime.InteropServices;
77
using System.Security.Authentication.ExtendedProtection;
8+
using System.Security.Cryptography.X509Certificates;
89
using Microsoft.Win32.SafeHandles;
910

1011
namespace System.Net.Security
@@ -310,10 +311,15 @@ public static unsafe int AcquireCredentialsHandle(
310311

311312
internal sealed class SafeFreeCredential_SECURITY : SafeFreeCredentials
312313
{
314+
#pragma warning disable 0649
315+
// This is used only by SslStream but it is included elsewhere
316+
public X509Certificate? LocalCertificate;
317+
#pragma warning restore 0649
313318
public SafeFreeCredential_SECURITY() : base() { }
314319

315320
protected override bool ReleaseHandle()
316321
{
322+
LocalCertificate?.Dispose();
317323
return Interop.SspiCli.FreeCredentialsHandle(ref _handle) == 0;
318324
}
319325
}

‎src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(
9292
}
9393

9494
// Check if the local certificate has been sent to the peer during the handshake.
95-
internal static bool IsLocalCertificateUsed(SafeDeleteContext? securityContext)
95+
internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _, SafeDeleteContext? securityContext)
9696
{
9797
SafeSslHandle? sslContext = ((SafeDeleteSslContext?)securityContext)?.SslContext;
9898
if (sslContext == null)

‎src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(
104104

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

109109
//
110110
// Used only by client SSL code, never returns null.

‎src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(
9797

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

102102
//
103103
// Used only by client SSL code, never returns null.

‎src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Security.Cryptography;
99
using System.Security.Cryptography.X509Certificates;
1010
using System.Security.Principal;
11+
using static Interop.SspiCli;
1112

1213
namespace System.Net
1314
{
@@ -90,8 +91,23 @@ internal static SslPolicyErrors VerifyCertificateProperties(
9091
}
9192

9293
// Check that local certificate was used by schannel.
93-
internal static bool IsLocalCertificateUsed(SafeDeleteContext securityContext)
94+
internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _credentialsHandle, SafeDeleteContext securityContext)
9495
{
96+
SecPkgContext_SessionInfo info = default;
97+
// fails on Server 2008 and older. We will fall-back to probing LOCAL_CERT_CONTEXT in that case.
98+
if (SSPIWrapper.QueryBlittableContextAttributes(
99+
GlobalSSPI.SSPISecureChannel,
100+
securityContext,
101+
Interop.SspiCli.ContextAttribute.SECPKG_ATTR_SESSION_INFO,
102+
ref info) &&
103+
((SecPkgContext_SessionInfo.Flags)info.dwFlags).HasFlag(SecPkgContext_SessionInfo.Flags.SSL_SESSION_RECONNECT))
104+
{
105+
// This is TLS Resumed session. Windows can fail to query the local cert bellow.
106+
// Instead, we will determine the usage form used credentials.
107+
SafeFreeCredential_SECURITY creds = (SafeFreeCredential_SECURITY)_credentialsHandle!;
108+
return creds.LocalCertificate != null;
109+
}
110+
95111
SafeFreeCertContext? localContext = null;
96112
try
97113
{

‎src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ private bool CompleteHandshake(ref ProtocolToken? alertToken, out SslPolicyError
512512
return true;
513513
}
514514

515-
if (_selectedClientCertificate != null && !CertificateValidationPal.IsLocalCertificateUsed(_securityContext!))
515+
if (_selectedClientCertificate != null && !CertificateValidationPal.IsLocalCertificateUsed(_credentialsHandle, _securityContext!))
516516
{
517517
// We may select client cert but it may not be used.
518518
// This is primarily an issue on Windows with credential caching.

‎src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs‎

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ public partial class SslStream
3030
private int _trailerSize = 16;
3131
private int _maxDataSize = 16354;
3232

33-
private bool _refreshCredentialNeeded = true;
34-
3533
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");
3634
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");
3735

@@ -104,11 +102,6 @@ internal bool RemoteCertRequired
104102
}
105103
}
106104

107-
internal void SetRefreshCredentialNeeded()
108-
{
109-
_refreshCredentialNeeded = true;
110-
}
111-
112105
internal void CloseContext()
113106
{
114107
if (!_remoteCertificateExposed)
@@ -489,6 +482,7 @@ private string[] GetRequestCertificateAuthorities()
489482
if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"Selected cert = {selectedCert}");
490483

491484
_selectedClientCertificate = clientCertificate;
485+
492486
return selectedCert;
493487
}
494488

@@ -529,7 +523,7 @@ This will not restart a session but helps minimizing the number of handles we cr
529523
530524
--*/
531525

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

@@ -594,7 +588,7 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
594588
_sslAuthenticationOptions.CertificateContext ??= SslStreamCertificateContext.Create(selectedCert!);
595589
}
596590

597-
_credentialsHandle = AcquireCredentialsHandle(_sslAuthenticationOptions);
591+
_credentialsHandle = AcquireCredentialsHandle(_sslAuthenticationOptions, newCredentialsRequested);
598592
thumbPrint = guessedThumbPrint; // Delay until here in case something above threw.
599593
}
600594
}
@@ -689,8 +683,11 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint)
689683
//
690684
byte[] guessedThumbPrint = selectedCert.GetCertHash();
691685
bool sendTrustedList = _sslAuthenticationOptions.CertificateContext!.Trust?._sendTrustInHandshake ?? false;
692-
SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy, sendTrustedList);
693-
686+
SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint,
687+
_sslAuthenticationOptions.EnabledSslProtocols,
688+
_sslAuthenticationOptions.IsServer,
689+
_sslAuthenticationOptions.EncryptionPolicy,
690+
sendTrustedList);
694691
if (cachedCredentialHandle != null)
695692
{
696693
_credentialsHandle = cachedCredentialHandle;
@@ -705,9 +702,9 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint)
705702
return cachedCred;
706703
}
707704

708-
private static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
705+
private static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions, bool newCredentialsRequested = false)
709706
{
710-
SafeFreeCredentials? cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions);
707+
SafeFreeCredentials? cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions, newCredentialsRequested);
711708

712709
if (sslAuthenticationOptions.CertificateContext != null && cred != null)
713710
{
@@ -761,16 +758,6 @@ internal ProtocolToken NextMessage(ReadOnlySpan<byte> incomingBuffer)
761758
{
762759
byte[]? nextmsg = null;
763760
SecurityStatusPal status = GenerateToken(incomingBuffer, ref nextmsg);
764-
765-
if (!_sslAuthenticationOptions.IsServer && status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
766-
{
767-
if (NetEventSource.Log.IsEnabled())
768-
NetEventSource.Info(this, "NextMessage() returned SecurityStatusPal.CredentialsNeeded");
769-
770-
SetRefreshCredentialNeeded();
771-
status = GenerateToken(incomingBuffer, ref nextmsg);
772-
}
773-
774761
ProtocolToken token = new ProtocolToken(nextmsg, status);
775762

776763
if (NetEventSource.Log.IsEnabled())
@@ -806,6 +793,10 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
806793
bool sendTrustList = false;
807794
byte[]? thumbPrint = null;
808795

796+
// We need to try get credentials at the beginning.
797+
// _credentialsHandle may be always null on some platforms but
798+
// _securityContext will be allocated on first call.
799+
bool refreshCredentialNeeded = _securityContext == null;
809800
//
810801
// Looping through ASC or ISC with potentially cached credential that could have been
811802
// already disposed from a different thread before ISC or ASC dir increment a cred ref count.
@@ -815,7 +806,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
815806
do
816807
{
817808
thumbPrint = null;
818-
if (_refreshCredentialNeeded)
809+
if (refreshCredentialNeeded)
819810
{
820811
cachedCreds = _sslAuthenticationOptions.IsServer
821812
? AcquireServerCredentials(ref thumbPrint)
@@ -865,15 +856,31 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
865856
_sslAuthenticationOptions,
866857
SelectClientCertificate
867858
);
859+
860+
if (status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
861+
{
862+
refreshCredentialNeeded = true;
863+
cachedCreds = AcquireClientCredentials(ref thumbPrint, newCredentialsRequested: true);
864+
865+
if (NetEventSource.Log.IsEnabled())
866+
NetEventSource.Info(this, "InitializeSecurityContext() returned 'CredentialsNeeded'.");
867+
868+
status = SslStreamPal.InitializeSecurityContext(
869+
ref _credentialsHandle!,
870+
ref _securityContext,
871+
_sslAuthenticationOptions.TargetHost,
872+
inputBuffer,
873+
ref result,
874+
_sslAuthenticationOptions,
875+
SelectClientCertificate);
876+
}
868877
}
869878
} while (cachedCreds && _credentialsHandle == null);
870879
}
871880
finally
872881
{
873-
if (_refreshCredentialNeeded)
882+
if (refreshCredentialNeeded)
874883
{
875-
_refreshCredentialNeeded = false;
876-
877884
//
878885
// Assuming the ISC or ASC has referenced the credential,
879886
// we want to call dispose so to decrement the effective ref count.

0 commit comments

Comments
 (0)