From cda1bdcc1091241b2255f106491012decdded2b0 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Wed, 10 Feb 2021 06:00:56 -0800 Subject: [PATCH] port #47729 to 5.0 (#48042) --- .../src/System/Net/Security/SecureChannel.cs | 4 + .../SslStreamNetworkStreamTest.cs | 128 ++++++++++++++++-- .../tests/FunctionalTests/TestHelper.cs | 13 +- 3 files changed, 129 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs index c8cdcb780a26..87c7f4d334b2 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs @@ -599,6 +599,10 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint) _credentialsHandle = cachedCredentialHandle; _selectedClientCertificate = clientCertificate; cachedCred = true; + if (selectedCert != null) + { + _sslAuthenticationOptions.CertificateContext = SslStreamCertificateContext.Create(selectedCert!); + } } else { diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index a7d33494dd47..7fdf298cd0ac 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.IO; using System.Net.Sockets; using System.Net.Test.Common; @@ -11,25 +12,39 @@ using System.Threading.Tasks; using Microsoft.DotNet.XUnitExtensions; using Xunit; +using Xunit.Abstractions; namespace System.Net.Security.Tests { using Configuration = System.Net.Test.Common.Configuration; - public class SslStreamNetworkStreamTest : IDisposable + public class CertificateSetup : IDisposable { - private readonly X509Certificate2 _serverCert; - private readonly X509Certificate2Collection _serverChain; + public readonly X509Certificate2 serverCert; + public readonly X509Certificate2Collection serverChain; - public SslStreamNetworkStreamTest() + public CertificateSetup() { - (_serverCert, _serverChain) = TestHelper.GenerateCertificates("localhost", this.GetType().Name); + TestHelper.CleanupCertificates(nameof(SslStreamNetworkStreamTest)); + (serverCert, serverChain) = TestHelper.GenerateCertificates("localhost", nameof(SslStreamNetworkStreamTest), longChain: true); } public void Dispose() { TestHelper.CleanupCertificates(this.GetType().Name); } + } + + public class SslStreamNetworkStreamTest : IClassFixture + { + readonly ITestOutputHelper _output; + readonly CertificateSetup certificates; + + public SslStreamNetworkStreamTest(ITestOutputHelper output, CertificateSetup setup) + { + _output = output; + certificates = setup; + } [Fact] public async Task SslStream_SendReceiveOverNetworkStream_Ok() @@ -280,22 +295,22 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) public async Task SslStream_UntrustedCaWithCustomCallback_OK(bool usePartialChain) { var rnd = new Random(); - int split = rnd.Next(0, _serverChain.Count - 1); + int split = rnd.Next(0, certificates.serverChain.Count - 1); var clientOptions = new SslClientAuthenticationOptions() { TargetHost = "localhost" }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => { // add our custom root CA - chain.ChainPolicy.CustomTrustStore.Add(_serverChain[_serverChain.Count - 1]); + chain.ChainPolicy.CustomTrustStore.Add(certificates.serverChain[certificates.serverChain.Count - 1]); chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; // Add only one CA to verify that peer did send intermediate CA cert. // In case of partial chain, we need to make missing certs available. if (usePartialChain) { - for (int i = split; i < _serverChain.Count - 1; i++) + for (int i = split; i < certificates.serverChain.Count - 1; i++) { - chain.ChainPolicy.ExtraStore.Add(_serverChain[i]); + chain.ChainPolicy.ExtraStore.Add(certificates.serverChain[i]); } } @@ -313,15 +328,15 @@ public async Task SslStream_UntrustedCaWithCustomCallback_OK(bool usePartialChai serverChain = new X509Certificate2Collection(); for (int i = 0; i < split; i++) { - serverChain.Add(_serverChain[i]); + serverChain.Add(certificates.serverChain[i]); } } else { - serverChain = _serverChain; + serverChain = certificates.serverChain; } - serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(_serverCert, serverChain); + serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(certificates.serverCert, certificates.serverChain); (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); using (clientStream) @@ -350,7 +365,7 @@ public async Task SslStream_UntrustedCaWithCustomCallback_Throws(bool customCall (sender, certificate, chain, sslPolicyErrors) => { // Add only root CA to verify that peer did send intermediate CA cert. - chain.ChainPolicy.CustomTrustStore.Add(_serverChain[_serverChain.Count -1]); + chain.ChainPolicy.CustomTrustStore.Add(certificates.serverChain[certificates.serverChain.Count - 1]); chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; // This should work and we should be able to trust the chain. Assert.True(chain.Build((X509Certificate2)certificate)); @@ -367,7 +382,7 @@ public async Task SslStream_UntrustedCaWithCustomCallback_Throws(bool customCall } var serverOptions = new SslServerAuthenticationOptions(); - serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(_serverCert, _serverChain); + serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(certificates.serverCert, certificates.serverChain); (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); using (clientStream) @@ -385,6 +400,91 @@ public async Task SslStream_UntrustedCaWithCustomCallback_Throws(bool customCall } } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/46837", TestPlatforms.OSX)] + public async Task SslStream_ClientCertificate_SendsChain() + { + List streams = new List(); + TestHelper.CleanupCertificates("SslStream_ClinetCertificate_SendsChain"); + (X509Certificate2 clientCertificate, X509Certificate2Collection clientChain) = TestHelper.GenerateCertificates("SslStream_ClinetCertificate_SendsChain", serverCertificate: false); + using (X509Store store = new X509Store(StoreName.CertificateAuthority, StoreLocation.CurrentUser)) + { + // add chain certificate so we can construct chain since there is no way how to pass intermediates directly. + store.Open(OpenFlags.ReadWrite); + store.AddRange(clientChain); + store.Close(); + } + + // There is race between adding certificate to the store and building chain + // Make sure we can build chain before proceeding to ssl handshale + using (var chain = new X509Chain()) + { + int count = 25; + chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags; + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chain.ChainPolicy.DisableCertificateDownloads = false; + bool chainStatus = chain.Build(clientCertificate); + while (chain.ChainElements.Count != (clientChain.Count + 1) && count > 0) + { + Thread.Sleep(100); + count--; + chainStatus = chain.Build(clientCertificate); + } + + // Verify we can construct full chain + Assert.True(chain.ChainElements.Count > clientChain.Count, "chain cannot be built"); + } + + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = "localhost", }; + clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + clientOptions.LocalCertificateSelectionCallback = (sender, target, certificates, remoteCertificate, issuers) => clientCertificate; + + var serverOptions = new SslServerAuthenticationOptions() { ClientCertificateRequired = true }; + serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(Configuration.Certificates.GetServerCertificate(), null); + serverOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => + { + // Client should send chain without root CA. There is no good way how to know if the chain was built from certificates + // from wire or from system store. However, SslStream adds certificates from wire to ExtraStore in RemoteCertificateValidationCallback. + // So we verify the operation by checking the ExtraStore. On Windows, that includes leaf itself. + _output.WriteLine("RemoteCertificateValidationCallback called with {0} and {1} extra certificates", sslPolicyErrors, chain.ChainPolicy.ExtraStore.Count); + foreach (X509Certificate c in chain.ChainPolicy.ExtraStore) + { + _output.WriteLine("received {0}", c.Subject); + } + + Assert.True(chain.ChainPolicy.ExtraStore.Count >= clientChain.Count - 1, "client did not sent expected chain"); + return true; + }; + + // run the test multiple times while holding established SSL so we could hit credential cache. + for (int i = 0; i < 3; i++) + { + (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); + SslStream client = new SslStream(clientStream); + SslStream server = new SslStream(serverStream); + + Task t1 = client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); + Task t2 = server.AuthenticateAsServerAsync(serverOptions, CancellationToken.None); + await Task.WhenAll(t1, t2).TimeoutAfter(TestConfiguration.PassingTestTimeoutMilliseconds); + + // hold to the streams so they stay in credential cache + streams.Add(client); + streams.Add(server); + } + + TestHelper.CleanupCertificates("SslStream_ClinetCertificate_SendsChain"); + clientCertificate.Dispose(); + foreach (X509Certificate c in clientChain) + { + c.Dispose(); + } + + foreach (SslStream s in streams) + { + s.Dispose(); + } + } + private static bool ValidateServerCertificate( object sender, X509Certificate retrievedServerPublicCertificate, diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index 446882e5e6b6..8cbf66faf3b8 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -26,6 +26,14 @@ public static class TestHelper }, false); + private static readonly X509EnhancedKeyUsageExtension s_tlsClientEku = + new X509EnhancedKeyUsageExtension( + new OidCollection + { + new Oid("1.3.6.1.5.5.7.3.2", null) + }, + false); + private static readonly X509BasicConstraintsExtension s_eeConstraints = new X509BasicConstraintsExtension(false, false, 0, false); @@ -107,7 +115,8 @@ internal static void CleanupCertificates(string testName) } catch { }; } - internal static (X509Certificate2 certificate, X509Certificate2Collection) GenerateCertificates(string targetName, string? testName = null, bool longChain = false) + + internal static (X509Certificate2 certificate, X509Certificate2Collection) GenerateCertificates(string targetName, string? testName = null, bool longChain = false, bool serverCertificate = true) { const int keySize = 2048; @@ -124,7 +133,7 @@ internal static (X509Certificate2 certificate, X509Certificate2Collection) Gener extensions.Add(builder.Build()); extensions.Add(s_eeConstraints); extensions.Add(s_eeKeyUsage); - extensions.Add(s_tlsServerEku); + extensions.Add(serverCertificate ? s_tlsServerEku : s_tlsClientEku); CertificateAuthority.BuildPrivatePki( PkiOptions.IssuerRevocationViaCrl,