Skip to content

Commit

Permalink
rebuild certificate context if we use client cert from credential cac…
Browse files Browse the repository at this point in the history 
…he (#47729)

* rebuild context if we use client cert from cache

* adjust expectation for windows

* add ITestOutputHelper

* use fixture to set up certificates

* add chain verification to the test

* adjust test

* fix assert

* disable test on macOS
  • Loading branch information
@wfurt
wfurt authored Feb 8, 2021
1 parent b8db68c commit aca1f01
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 16 deletions.
4 changes: 4 additions & 0 deletions src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,10 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
_credentialsHandle = cachedCredentialHandle;
_selectedClientCertificate = clientCertificate;
cachedCred = true;
if (selectedCert != null)
{
_sslAuthenticationOptions.CertificateContext = SslStreamCertificateContext.Create(selectedCert!);
}
}
else
{
Expand Down
136 changes: 122 additions & 14 deletions src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -11,20 +12,42 @@
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
public class CertificateSetup : IDisposable
{
private static readonly X509Certificate2 _serverCert;
private static readonly X509Certificate2Collection _serverChain;
public readonly X509Certificate2 serverCert;
public readonly X509Certificate2Collection serverChain;

static SslStreamNetworkStreamTest()
public CertificateSetup()
{
TestHelper.CleanupCertificates(nameof(SslStreamNetworkStreamTest));
(_serverCert, _serverChain) = TestHelper.GenerateCertificates("localhost", nameof(SslStreamNetworkStreamTest), longChain: true);
(serverCert, serverChain) = TestHelper.GenerateCertificates("localhost", nameof(SslStreamNetworkStreamTest), longChain: true);
}

public void Dispose()
{
serverCert.Dispose();
foreach (var c in serverChain)
{
c.Dispose();
}
}
}

public class SslStreamNetworkStreamTest : IClassFixture<CertificateSetup>
{
readonly ITestOutputHelper _output;
readonly CertificateSetup certificates;

public SslStreamNetworkStreamTest(ITestOutputHelper output, CertificateSetup setup)
{
_output = output;
certificates = setup;
}

[ConditionalFact]
Expand Down Expand Up @@ -220,22 +243,22 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout(
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]);
}
}

Expand All @@ -253,15 +276,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)
Expand Down Expand Up @@ -291,7 +314,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));
Expand All @@ -308,7 +331,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)
Expand All @@ -326,6 +349,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<SslStream> streams = new List<SslStream>();
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,
Expand Down
12 changes: 10 additions & 2 deletions src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,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);

Expand Down Expand Up @@ -109,7 +117,7 @@ internal static void CleanupCertificates(string testName)
catch { };
}

internal static (X509Certificate2 certificate, X509Certificate2Collection) GenerateCertificates(string targetName, [CallerMemberName] string? testName = null, bool longChain = false)
internal static (X509Certificate2 certificate, X509Certificate2Collection) GenerateCertificates(string targetName, [CallerMemberName] string? testName = null, bool longChain = false, bool serverCertificate = true)
{
const int keySize = 2048;
if (PlatformDetection.IsWindows && testName != null)
Expand All @@ -125,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,
Expand Down

0 comments on commit aca1f01

Please sign in to comment.