diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs index 81fdba8901bb3a..b13e217b7374e2 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs @@ -298,6 +298,9 @@ private static bool QueryCertContextAttribute(ISSPIInterface secModule, SafeDele public static bool QueryContextAttributes_SECPKG_ATTR_REMOTE_CERT_CONTEXT(ISSPIInterface secModule, SafeDeleteContext securityContext, out SafeFreeCertContext? certContext) => QueryCertContextAttribute(secModule, securityContext, Interop.SspiCli.ContextAttribute.SECPKG_ATTR_REMOTE_CERT_CONTEXT, out certContext); + public static bool QueryContextAttributes_SECPKG_ATTR_LOCAL_CERT_CONTEXT(ISSPIInterface secModule, SafeDeleteContext securityContext, out SafeFreeCertContext? certContext) + => QueryCertContextAttribute(secModule, securityContext, Interop.SspiCli.ContextAttribute.SECPKG_ATTR_LOCAL_CERT_CONTEXT, out certContext); + public static bool QueryContextAttributes_SECPKG_ATTR_REMOTE_CERT_CHAIN(ISSPIInterface secModule, SafeDeleteContext securityContext, out SafeFreeCertContext? certContext) => QueryCertContextAttribute(secModule, securityContext, Interop.SspiCli.ContextAttribute.SECPKG_ATTR_REMOTE_CERT_CHAIN, out certContext); diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs index 43716cd04367b7..dc0865382cdf20 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs @@ -91,6 +91,10 @@ internal static SslPolicyErrors VerifyCertificateProperties( return cert; } + // 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; + // // Used only by client SSL code, never returns null. // diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs index f9d3afe570af62..1f3fce1d7d8630 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs @@ -102,6 +102,10 @@ internal static SslPolicyErrors VerifyCertificateProperties( return result; } + // 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; + // // Used only by client SSL code, never returns null. // 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 2e6bbfd5e85d05..2e6ed0082192f4 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 @@ -95,6 +95,10 @@ internal static SslPolicyErrors VerifyCertificateProperties( return result; } + // 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; + // // Used only by client SSL code, never returns null. // diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs index 1ead906ec3585c..926cb646e79168 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs @@ -89,6 +89,28 @@ internal static SslPolicyErrors VerifyCertificateProperties( return result; } + // Check that local certificate was used by schannel. + internal static bool IsLocalCertificateUsed(SafeDeleteContext securityContext) + { + SafeFreeCertContext? localContext = null; + try + { + if (SSPIWrapper.QueryContextAttributes_SECPKG_ATTR_LOCAL_CERT_CONTEXT(GlobalSSPI.SSPISecureChannel, securityContext, out localContext) && + localContext != null) + { + return !localContext.IsInvalid; + } + } + finally + { + localContext?.Dispose(); + } + + // Some older Windows do not support that. This is only called when client certificate was provided + // so assume it was for a reason. + return true; + } + // // Used only by client SSL code, never returns null. // 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 ac8db67ba27753..b0221d467bd96f 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 @@ -518,7 +518,6 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint) bool cachedCred = false; // this is a return result from this method. X509Certificate2? selectedCert = SelectClientCertificate(out sessionRestartAttempt); - try { // Try to locate cached creds first. @@ -977,6 +976,12 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot } _remoteCertificate = certificate; + if (_selectedClientCertificate != null && !CertificateValidationPal.IsLocalCertificateUsed(_securityContext!)) + { + // We may slect client cert but it may not be used. + // This is primarily issue on Windows with credential caching + _selectedClientCertificate = null; + } if (_remoteCertificate == null) { diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationClientServer.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationClientServer.cs index 494bcf86fc226b..a5bc4f1ddd7ffb 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationClientServer.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationClientServer.cs @@ -165,11 +165,16 @@ public async Task CertificateValidationClientServer_EndToEnd_Ok(bool useClientSe clientCerts.Add(_clientCertificate); } - Task clientAuthentication = sslClientStream.AuthenticateAsClientAsync( - serverName, - clientCerts, - SslProtocolSupport.DefaultSslProtocols, - false); + // Connect to GUID to prevent TLS resume + var options = new SslClientAuthenticationOptions() + { + TargetHost = Guid.NewGuid().ToString("N"), + ClientCertificates = clientCerts, + EnabledSslProtocols = SslProtocolSupport.DefaultSslProtocols, + CertificateChainPolicy = new X509ChainPolicy(), + }; + options.CertificateChainPolicy.VerificationFlags = X509VerificationFlags.IgnoreInvalidName; + Task clientAuthentication = sslClientStream.AuthenticateAsClientAsync(options, default); Task serverAuthentication = sslServerStream.AuthenticateAsServerAsync( _serverCertificate, @@ -258,7 +263,6 @@ private bool ClientSideRemoteServerCertificateValidation(object sender, X509Cert Assert.Equal(expectedSslPolicyErrors, sslPolicyErrors); Assert.Equal(_serverCertificate, certificate); - return true; } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs index 2332a00e1779c2..698af3d28fee7b 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs @@ -4,9 +4,11 @@ using System.IO; using System.Threading.Tasks; using System.Net.Test.Common; +using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using Xunit; +using System.Runtime.InteropServices; namespace System.Net.Security.Tests { @@ -16,11 +18,13 @@ public class SslStreamMutualAuthenticationTest : IDisposable { private readonly X509Certificate2 _clientCertificate; private readonly X509Certificate2 _serverCertificate; + private readonly X509Certificate2 _selfSignedCertificate; public SslStreamMutualAuthenticationTest() { _serverCertificate = Configuration.Certificates.GetServerCertificate(); _clientCertificate = Configuration.Certificates.GetClientCertificate(); + _selfSignedCertificate = Configuration.Certificates.GetSelfSignedServerCertificate(); } public void Dispose() @@ -80,6 +84,47 @@ public async Task SslStream_RequireClientCert_IsMutuallyAuthenticated_ReturnsTru } } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [ClassData(typeof(SslProtocolSupport.SupportedSslProtocolsTestData))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/65563", TestPlatforms.Android)] + public async Task SslStream_CachedCredentials_IsMutuallyAuthenticatedCorrect( + SslProtocols protocol) + { + var clientOptions = new SslClientAuthenticationOptions + { + ClientCertificates = new X509CertificateCollection() { _clientCertificate }, + EnabledSslProtocols = protocol, + RemoteCertificateValidationCallback = delegate { return true; }, + TargetHost = Guid.NewGuid().ToString("N") + }; + + for (int i = 0; i < 5; i++) + { + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + { + bool expectMutualAuthentication = (i % 2) == 0; + + var serverOptions = new SslServerAuthenticationOptions + { + ClientCertificateRequired = expectMutualAuthentication, + ServerCertificate = expectMutualAuthentication ? _serverCertificate : _selfSignedCertificate, + RemoteCertificateValidationCallback = delegate { return true; }, + EnabledSslProtocols = protocol + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions), + server.AuthenticateAsServerAsync(serverOptions)); + + // mutual authentication should only be set if server required client cert + Assert.Equal(expectMutualAuthentication, server.IsMutuallyAuthenticated); + Assert.Equal(expectMutualAuthentication, client.IsMutuallyAuthenticated); + }; + } + } + private static bool AllowAnyCertificate( object sender, X509Certificate certificate,