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.

Sign up for GitHub

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

Ask SChannel for stapled OCSP when the client will do revocation #71570

Merged
merged 2 commits into from
Jul 12, 2022
Merged

Ask SChannel for stapled OCSP when the client will do revocation #71570

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2760242
Ask SChannel for stapled OCSP when the client will do revocation
bartonjs Jul 2, 2022
7959398
Use ConditonalTheory in the new test for late skip reasons
bartonjs Jul 11, 2022
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
1 change: 1 addition & 0 deletions src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CertContextPropId.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ internal enum CertContextPropId : int
CERT_ARCHIVED_PROP_ID = 19,
CERT_KEY_IDENTIFIER_PROP_ID = 20,
CERT_PUBKEY_ALG_PARA_PROP_ID = 22,
CERT_OCSP_RESPONSE_PROP_ID = 70,
CERT_NCRYPT_KEY_HANDLE_PROP_ID = 78,
CERT_DELETE_KEYSET_PROP_ID = 101,
CERT_CLR_DELETE_KEY_PROP_ID = 125,
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ public enum Flags
SCH_CRED_MANUAL_CRED_VALIDATION = 0x08,
SCH_CRED_NO_DEFAULT_CREDS = 0x10,
SCH_CRED_AUTO_CRED_VALIDATION = 0x20,
SCH_CRED_REVOCATION_CHECK_END_CERT = 0x100,
SCH_CRED_IGNORE_NO_REVOCATION_CHECK = 0x800,
SCH_CRED_IGNORE_REVOCATION_OFFLINE = 0x1000,
SCH_SEND_AUX_RECORD = 0x00200000,
SCH_USE_STRONG_CRYPTO = 0x00400000,
}
Expand Down
34 changes: 29 additions & 5 deletions src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Security.Authentication;
using System.Security.Cryptography.X509Certificates;

namespace System.Net.Security
{
Expand All @@ -26,18 +27,26 @@ internal static class SslSessionsCache
private readonly EncryptionPolicy _encryptionPolicy;
private readonly bool _isServerMode;
private readonly bool _sendTrustList;
private readonly bool _checkRevocation;

//
// SECURITY: X509Certificate.GetCertHash() is virtual hence before going here,
// the caller of this ctor has to ensure that a user cert object was inspected and
// optionally cloned.
//
internal SslCredKey(byte[]? thumbPrint, int allowedProtocols, bool isServerMode, EncryptionPolicy encryptionPolicy, bool sendTrustList)
internal SslCredKey(
byte[]? thumbPrint,
int allowedProtocols,
bool isServerMode,
EncryptionPolicy encryptionPolicy,
bool sendTrustList,
bool checkRevocation)
{
_thumbPrint = thumbPrint ?? Array.Empty<byte>();
_allowedProtocols = allowedProtocols;
_encryptionPolicy = encryptionPolicy;
_isServerMode = isServerMode;
_checkRevocation = checkRevocation;
_sendTrustList = sendTrustList;
}

Expand Down Expand Up @@ -68,6 +77,7 @@ public override int GetHashCode()
hashCode ^= (int)_encryptionPolicy;
hashCode ^= _isServerMode ? 0x10000 : 0x20000;
hashCode ^= _sendTrustList ? 0x40000 : 0x80000;
hashCode ^= _checkRevocation ? 0x100000 : 0x200000;

return hashCode;
}
Expand All @@ -86,6 +96,7 @@ public bool Equals(SslCredKey other)
_allowedProtocols == other._allowedProtocols &&
_isServerMode == other._isServerMode &&
_sendTrustList == other._sendTrustList &&
_checkRevocation == other._checkRevocation &&
thumbPrint.AsSpan().SequenceEqual(otherThumbPrint);
}
}
Expand All @@ -96,15 +107,21 @@ public bool Equals(SslCredKey other)
// ATTN: The returned handle can be invalid, the callers of InitializeSecurityContext and AcceptSecurityContext
// must be prepared to execute a back-out code if the call fails.
//
internal static SafeFreeCredentials? TryCachedCredential(byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy, bool sendTrustList = false)
internal static SafeFreeCredentials? TryCachedCredential(
byte[]? thumbPrint,
SslProtocols sslProtocols,
bool isServer,
EncryptionPolicy encryptionPolicy,
bool checkRevocation,
bool sendTrustList = false)
{
if (s_cachedCreds.IsEmpty)
{
if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, $"Not found, Current Cache Count = {s_cachedCreds.Count}");
return null;
}

var key = new SslCredKey(thumbPrint, (int)sslProtocols, isServer, encryptionPolicy, sendTrustList);
var key = new SslCredKey(thumbPrint, (int)sslProtocols, isServer, encryptionPolicy, sendTrustList, checkRevocation);

//SafeCredentialReference? cached;
SafeFreeCredentials? credentials = GetCachedCredential(key);
Expand All @@ -129,7 +146,14 @@ public bool Equals(SslCredKey other)
//
// ATTN: The thumbPrint must be from inspected and possibly cloned user Cert object or we get a security hole in SslCredKey ctor.
//
internal static void CacheCredential(SafeFreeCredentials creds, byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy, bool sendTrustList = false)
internal static void CacheCredential(
SafeFreeCredentials creds,
byte[]? thumbPrint,
SslProtocols sslProtocols,
bool isServer,
EncryptionPolicy encryptionPolicy,
bool checkRevocation,
bool sendTrustList = false)
{
Debug.Assert(creds != null, "creds == null");

Expand All @@ -139,7 +163,7 @@ internal static void CacheCredential(SafeFreeCredentials creds, byte[]? thumbPri
return;
}

SslCredKey key = new SslCredKey(thumbPrint, (int)sslProtocols, isServer, encryptionPolicy, sendTrustList);
SslCredKey key = new SslCredKey(thumbPrint, (int)sslProtocols, isServer, encryptionPolicy, sendTrustList, checkRevocation);

SafeFreeCredentials? credentials = GetCachedCredential(key);

Expand Down
19 changes: 15 additions & 4 deletions src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,12 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
// SECURITY: selectedCert ref if not null is a safe object that does not depend on possible **user** inherited X509Certificate type.
//
byte[]? guessedThumbPrint = selectedCert?.GetCertHash();
SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy);
SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(
guessedThumbPrint,
_sslAuthenticationOptions.EnabledSslProtocols,
_sslAuthenticationOptions.IsServer,
_sslAuthenticationOptions.EncryptionPolicy,
_sslAuthenticationOptions.CertificateRevocationCheckMode != X509RevocationMode.NoCheck);

// We can probably do some optimization here. If the selectedCert is returned by the delegate
// we can always go ahead and use the certificate to create our credential
Expand Down Expand Up @@ -657,8 +662,7 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint)

private static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
{
SafeFreeCredentials cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions.CertificateContext, sslAuthenticationOptions.EnabledSslProtocols,
sslAuthenticationOptions.EncryptionPolicy, sslAuthenticationOptions.IsServer);
SafeFreeCredentials cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions);

if (sslAuthenticationOptions.CertificateContext != null)
{
Expand Down Expand Up @@ -819,7 +823,14 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
//
if (!cachedCreds && _securityContext != null && !_securityContext.IsInvalid && _credentialsHandle != null && !_credentialsHandle.IsInvalid)
{
SslSessionsCache.CacheCredential(_credentialsHandle, thumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy, sendTrustList);
SslSessionsCache.CacheCredential(
_credentialsHandle,
thumbPrint,
_sslAuthenticationOptions.EnabledSslProtocols,
_sslAuthenticationOptions.IsServer,
_sslAuthenticationOptions.EncryptionPolicy,
_sslAuthenticationOptions.CertificateRevocationCheckMode != X509RevocationMode.NoCheck,
sendTrustList);
}
}
}
Expand Down
11 changes: 5 additions & 6 deletions src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,12 @@ public static SecurityStatusPal Renegotiate(
throw new PlatformNotSupportedException();
}

public static SafeFreeCredentials AcquireCredentialsHandle(
SslStreamCertificateContext? certificateContext,
SslProtocols protocols,
EncryptionPolicy policy,
bool isServer)
public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
{
return new SafeFreeSslCredentials(certificateContext, protocols, policy);
return new SafeFreeSslCredentials(
sslAuthenticationOptions.CertificateContext,
sslAuthenticationOptions.EnabledSslProtocols,
sslAuthenticationOptions.EncryptionPolicy);
}

public static SecurityStatusPal EncryptMessage(
Expand Down
11 changes: 5 additions & 6 deletions src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,12 @@ public static SecurityStatusPal Renegotiate(
throw new PlatformNotSupportedException();
}

public static SafeFreeCredentials AcquireCredentialsHandle(
SslStreamCertificateContext? certificateContext,
SslProtocols protocols,
EncryptionPolicy policy,
bool isServer)
public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
{
return new SafeFreeSslCredentials(certificateContext, protocols, policy);
return new SafeFreeSslCredentials(
sslAuthenticationOptions.CertificateContext,
sslAuthenticationOptions.EnabledSslProtocols,
sslAuthenticationOptions.EncryptionPolicy);
}

public static SecurityStatusPal EncryptMessage(
Expand Down
9 changes: 6 additions & 3 deletions src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ public static SecurityStatusPal InitializeSecurityContext(
return HandshakeInternal(credential!, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, clientCertificateSelectionCallback);
}

public static SafeFreeCredentials AcquireCredentialsHandle(SslStreamCertificateContext? certificateContext,
SslProtocols protocols, EncryptionPolicy policy, bool isServer)
public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
{
return new SafeFreeSslCredentials(certificateContext, protocols, policy, isServer);
return new SafeFreeSslCredentials(
sslAuthenticationOptions.CertificateContext,
sslAuthenticationOptions.EnabledSslProtocols,
sslAuthenticationOptions.EncryptionPolicy,
sslAuthenticationOptions.IsServer);
}

public static SecurityStatusPal EncryptMessage(SafeDeleteSslContext securityContext, ReadOnlyMemory<byte> input, int headerSize, int trailerSize, ref byte[] output, out int resultSize)
Expand Down
53 changes: 42 additions & 11 deletions src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,21 @@ public static SecurityStatusPal Renegotiate(
return status;
}

public static SafeFreeCredentials AcquireCredentialsHandle(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer)
public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions)
{
try
{
EncryptionPolicy policy = sslAuthenticationOptions.EncryptionPolicy;

// New crypto API supports TLS1.3 but it does not allow to force NULL encryption.
#pragma warning disable SYSLIB0040 // NoEncryption and AllowNoEncryption are obsolete
SafeFreeCredentials cred = !UseNewCryptoApi || policy == EncryptionPolicy.NoEncryption ?
AcquireCredentialsHandleSchannelCred(certificateContext, protocols, policy, isServer) :
AcquireCredentialsHandleSchCredentials(certificateContext, protocols, policy, isServer);
AcquireCredentialsHandleSchannelCred(sslAuthenticationOptions) :
AcquireCredentialsHandleSchCredentials(sslAuthenticationOptions);
#pragma warning restore SYSLIB0040

SslStreamCertificateContext? certificateContext = sslAuthenticationOptions.CertificateContext;

if (certificateContext != null && certificateContext.Trust != null && certificateContext.Trust._sendTrustInHandshake)
{
AttachCertificateStore(cred, certificateContext.Trust._store!);
Expand Down Expand Up @@ -182,10 +187,11 @@ private static unsafe void AttachCertificateStore(SafeFreeCredentials cred, X509

// This is legacy crypto API used on .NET Framework and older Windows versions.
// It only supports TLS up to 1.2
public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer)
public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(SslAuthenticationOptions authOptions)
{
X509Certificate2? certificate = certificateContext?.Certificate;
int protocolFlags = GetProtocolFlagsFromSslProtocols(protocols, isServer);
X509Certificate2? certificate = authOptions.CertificateContext?.Certificate;
bool isServer = authOptions.IsServer;
int protocolFlags = GetProtocolFlagsFromSslProtocols(authOptions.EnabledSslProtocols, isServer);
Interop.SspiCli.SCHANNEL_CRED.Flags flags;
Interop.SspiCli.CredentialUse direction;

Expand All @@ -196,17 +202,29 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(Ss
Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_CRED_MANUAL_CRED_VALIDATION |
Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_CRED_NO_DEFAULT_CREDS |
Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_SEND_AUX_RECORD;

// Request OCSP Stapling from the server
if (authOptions.CertificateRevocationCheckMode != X509RevocationMode.NoCheck)
{
flags |=
Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_CRED_REVOCATION_CHECK_END_CERT |
Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_CRED_IGNORE_NO_REVOCATION_CHECK |
Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_CRED_IGNORE_REVOCATION_OFFLINE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the mode is X509RevocationMode.Online should we ignore failures? While that may be convenient I'm wondering if that meets security expectations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're ignoring any failures out of SChannel anyways, and we rebuild the chain using the data they provide us.

}
}
else
{
direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND;
flags = Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_SEND_AUX_RECORD;
if (certificateContext?.Trust?._sendTrustInHandshake == true)

if (authOptions.CertificateContext?.Trust?._sendTrustInHandshake == true)
{
flags |= Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_CRED_NO_SYSTEM_MAPPER;
}
}

EncryptionPolicy policy = authOptions.EncryptionPolicy;

#pragma warning disable SYSLIB0040 // NoEncryption and AllowNoEncryption are obsolete
// Always opt-in SCH_USE_STRONG_CRYPTO for TLS.
if (((protocolFlags == 0) ||
Expand Down Expand Up @@ -234,17 +252,19 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(Ss
}

// This function uses new crypto API to support TLS 1.3 and beyond.
public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchCredentials(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer)
public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchCredentials(SslAuthenticationOptions authOptions)
{
X509Certificate2? certificate = certificateContext?.Certificate;
int protocolFlags = GetProtocolFlagsFromSslProtocols(protocols, isServer);
X509Certificate2? certificate = authOptions.CertificateContext?.Certificate;
bool isServer = authOptions.IsServer;
int protocolFlags = GetProtocolFlagsFromSslProtocols(authOptions.EnabledSslProtocols, isServer);
Interop.SspiCli.SCH_CREDENTIALS.Flags flags;
Interop.SspiCli.CredentialUse direction;

if (isServer)
{
direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND;
flags = Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD;
if (certificateContext?.Trust?._sendTrustInHandshake == true)
if (authOptions.CertificateContext?.Trust?._sendTrustInHandshake == true)
{
flags |= Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_NO_SYSTEM_MAPPER;
}
Expand All @@ -256,8 +276,19 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchCredentials(
Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_MANUAL_CRED_VALIDATION |
Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_NO_DEFAULT_CREDS |
Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD;

// Request OCSP Stapling from the server
if (authOptions.CertificateRevocationCheckMode != X509RevocationMode.NoCheck)
{
flags |=
Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_REVOCATION_CHECK_END_CERT |
Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_IGNORE_NO_REVOCATION_CHECK |
Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_IGNORE_REVOCATION_OFFLINE;
}
}

EncryptionPolicy policy = authOptions.EncryptionPolicy;

if (policy == EncryptionPolicy.RequireEncryption)
{
// Always opt-in SCH_USE_STRONG_CRYPTO for TLS.
Expand Down
Loading