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

Jump to bottom

[release/5.0] rebuild certificate context if we use client cert from credential cache #48042

Merged
merged 1 commit into from Feb 10, 2021
Merged

[release/5.0] rebuild certificate context if we use client cert from credential cache #48042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs
View file
Open in desktop
Expand Up @@ -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
{
Expand Down
128 changes: 114 additions & 14 deletions src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs
View file
Open in desktop
@@ -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,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<CertificateSetup>
{
readonly ITestOutputHelper _output;
readonly CertificateSetup certificates;

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

[Fact]
public async Task SslStream_SendReceiveOverNetworkStream_Ok()
Expand Down Expand Up @@ -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]);
}
}

Expand All @@ -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)
Expand Down Expand Up @@ -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));
Expand All @@ -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)
Expand All @@ -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<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
13 changes: 11 additions & 2 deletions src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs
View file
Open in desktop
Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand All @@ -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,
Expand Down