Skip to content

Commit

Permalink
Add support for X509ChainPolicy for QUIC (#73056)
Browse files Browse the repository at this point in the history 
* Add support for CertificateChainPolicy to Quic

* Add link to issue

* Fix use of partial chain in tests

* Code review feedback

* Revert change in SslStream tests
  • Loading branch information
@rzikm
rzikm committed Aug 4, 2022
1 parent 209c040 commit 1f0582e
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 21 deletions.
32 changes: 28 additions & 4 deletions src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.SslConnectionOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,23 @@ private readonly struct SslConnectionOptions
/// </summary>
private readonly RemoteCertificateValidationCallback? _validationCallback;

public SslConnectionOptions(QuicConnection connection, bool isClient, string? targetHost, bool certificateRequired, X509RevocationMode revocationMode, RemoteCertificateValidationCallback? validationCallback)
/// <summary>
/// Configured via <see cref="SslServerAuthenticationOptions.CertificateChainPolicy"/> or <see cref="SslClientAuthenticationOptions.CertificateChainPolicy"/>.
/// </summary>
private readonly X509ChainPolicy? _certificateChainPolicy;

public SslConnectionOptions(QuicConnection connection, bool isClient,
string? targetHost, bool certificateRequired, X509RevocationMode
revocationMode, RemoteCertificateValidationCallback? validationCallback,
X509ChainPolicy? certificateChainPolicy)
{
_connection = connection;
_isClient = isClient;
_targetHost = targetHost;
_certificateRequired = certificateRequired;
_revocationMode = revocationMode;
_validationCallback = validationCallback;
_certificateChainPolicy = certificateChainPolicy;
}

public unsafe int ValidateCertificate(QUIC_BUFFER* certificatePtr, QUIC_BUFFER* chainPtr, out X509Certificate2? certificate)
Expand All @@ -65,9 +74,24 @@ public unsafe int ValidateCertificate(QUIC_BUFFER* certificatePtr, QUIC_BUFFER*
if (certificatePtr is not null)
{
chain = new X509Chain();
chain.ChainPolicy.RevocationMode = _revocationMode;
chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot;
chain.ChainPolicy.ApplicationPolicy.Add(_isClient ? s_serverAuthOid : s_clientAuthOid);
if (_certificateChainPolicy != null)
{
chain.ChainPolicy = _certificateChainPolicy;
}
else
{
chain.ChainPolicy.RevocationMode = _revocationMode;
chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot;

// TODO: configure chain.ChainPolicy.CustomTrustStore to mirror behavior of SslStream.VerifyRemoteCertificate (https://github.com/dotnet/runtime/issues/73053)
}

// set ApplicationPolicy unless already provided.
if (chain.ChainPolicy.ApplicationPolicy.Count == 0)
{
// Authenticate the remote party: (e.g. when operating in server mode, authenticate the client).
chain.ChainPolicy.ApplicationPolicy.Add(_isClient ? s_serverAuthOid : s_clientAuthOid);
}

if (MsQuicApi.UsesSChannelBackend)
{
Expand Down
6 changes: 4 additions & 2 deletions src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,8 @@ private async ValueTask FinishConnectAsync(QuicClientConnectionOptions options,
options.ClientAuthenticationOptions.TargetHost,
certificateRequired: true,
options.ClientAuthenticationOptions.CertificateRevocationCheckMode,
options.ClientAuthenticationOptions.RemoteCertificateValidationCallback);
options.ClientAuthenticationOptions.RemoteCertificateValidationCallback,
options.ClientAuthenticationOptions.CertificateChainPolicy?.Clone());
_configuration = MsQuicConfiguration.Create(options);

IntPtr targetHostPtr = Marshal.StringToCoTaskMemUTF8(options.ClientAuthenticationOptions.TargetHost ?? host ?? address?.ToString());
Expand Down Expand Up @@ -327,7 +328,8 @@ internal ValueTask FinishHandshakeAsync(QuicServerConnectionOptions options, str
targetHost: null,
options.ServerAuthenticationOptions.ClientCertificateRequired,
options.ServerAuthenticationOptions.CertificateRevocationCheckMode,
options.ServerAuthenticationOptions.RemoteCertificateValidationCallback);
options.ServerAuthenticationOptions.RemoteCertificateValidationCallback,
options.ServerAuthenticationOptions.CertificateChainPolicy?.Clone());
_configuration = MsQuicConfiguration.Create(options, targetHost);

unsafe
Expand Down
89 changes: 87 additions & 2 deletions src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,38 @@

namespace System.Net.Quic.Tests
{
public class CertificateSetup : IDisposable
{
public readonly X509Certificate2 serverCert;
public readonly X509Certificate2Collection serverChain;

public CertificateSetup()
{
System.Net.Security.Tests.TestHelper.CleanupCertificates(nameof(MsQuicTests));
(serverCert, serverChain) = System.Net.Security.Tests.TestHelper.GenerateCertificates("localhost", nameof(MsQuicTests), longChain: true);
}

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

[Collection(nameof(DisableParallelization))]
[ConditionalClass(typeof(QuicTestBase), nameof(QuicTestBase.IsSupported))]
public class MsQuicTests : QuicTestBase
public class MsQuicTests : QuicTestBase, IClassFixture<CertificateSetup>
{
private static byte[] s_data = "Hello world!"u8.ToArray();
readonly CertificateSetup _certificates;

public MsQuicTests(ITestOutputHelper output) : base(output) { }
public MsQuicTests(ITestOutputHelper output, CertificateSetup setup) : base(output)
{
_certificates = setup;
}

[Fact]
public async Task ConnectWithCertificateChain()
Expand Down Expand Up @@ -103,6 +128,66 @@ public async Task ConnectWithCertificateChain()
}
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task ConnectWithUntrustedCaWithCustomTrust_OK(bool usePartialChain)
{
int split = Random.Shared.Next(0, _certificates.serverChain.Count - 1);

X509Certificate2Collection serverChain;
if (usePartialChain)
{
// give first few certificates without root CA
serverChain = new X509Certificate2Collection();
for (int i = 0; i < split; i++)
{
serverChain.Add(_certificates.serverChain[i]);
}
}
else
{
serverChain = _certificates.serverChain;
}

var listenerOptions = CreateQuicListenerOptions();
listenerOptions.ConnectionOptionsCallback = (_, _, _) =>
{
var serverOptions = CreateQuicServerOptions();
serverOptions.ServerAuthenticationOptions.ServerCertificateContext = SslStreamCertificateContext.Create(_certificates.serverCert, serverChain);
serverOptions.ServerAuthenticationOptions.RemoteCertificateValidationCallback = null;
return ValueTask.FromResult(serverOptions);
};

await using QuicListener listener = await CreateQuicListener(listenerOptions);

var clientOptions = CreateQuicClientOptions(listener.LocalEndPoint);
var clientSslOptions = clientOptions.ClientAuthenticationOptions;
clientSslOptions.TargetHost = "localhost";
clientSslOptions.RemoteCertificateValidationCallback = null;
clientSslOptions.CertificateChainPolicy = new X509ChainPolicy()
{
RevocationMode = X509RevocationMode.NoCheck,
TrustMode = X509ChainTrustMode.CustomRootTrust
};
clientSslOptions.CertificateChainPolicy.CustomTrustStore.Add(_certificates.serverChain[_certificates.serverChain.Count - 1]);
// 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 < _certificates.serverChain.Count - 1; i++)
{
clientSslOptions.CertificateChainPolicy.ExtraStore.Add(_certificates.serverChain[i]);
}
}

// should connect successfully
(QuicConnection clientConnection, QuicConnection serverConnection) = await CreateConnectedQuicConnection(clientOptions, listener);
await clientConnection.DisposeAsync();
await serverConnection.DisposeAsync();
}


[ConditionalFact]
public async Task UntrustedClientCertificateFails()
{
Expand Down
32 changes: 19 additions & 13 deletions src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ public class SslStreamNetworkStreamTest : IClassFixture<CertificateSetup>
private static bool SupportsRenegotiation => TestConfiguration.SupportsRenegotiation;

readonly ITestOutputHelper _output;
readonly CertificateSetup certificates;
readonly CertificateSetup _certificates;

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

[ConditionalFact]
Expand Down Expand Up @@ -756,19 +756,22 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout(
[SkipOnPlatform(TestPlatforms.Android, "Self-signed certificates are rejected by Android before the .NET validation is reached")]
public async Task SslStream_UntrustedCaWithCustomTrust_OK(bool usePartialChain)
{
int split = Random.Shared.Next(0, certificates.serverChain.Count - 1);
int split = Random.Shared.Next(0, _certificates.serverChain.Count - 1);

var clientOptions = new SslClientAuthenticationOptions() { TargetHost = "localhost" };
clientOptions.CertificateChainPolicy = new X509ChainPolicy() { RevocationMode = X509RevocationMode.NoCheck,
TrustMode = X509ChainTrustMode.CustomRootTrust };
clientOptions.CertificateChainPolicy.CustomTrustStore.Add(certificates.serverChain[certificates.serverChain.Count - 1]);
clientOptions.CertificateChainPolicy = new X509ChainPolicy()
{
RevocationMode = X509RevocationMode.NoCheck,
TrustMode = X509ChainTrustMode.CustomRootTrust
};
clientOptions.CertificateChainPolicy.CustomTrustStore.Add(_certificates.serverChain[_certificates.serverChain.Count - 1]);
// 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 < certificates.serverChain.Count - 1; i++)
for (int i = split; i < _certificates.serverChain.Count - 1; i++)
{
clientOptions.CertificateChainPolicy.ExtraStore.Add(certificates.serverChain[i]);
clientOptions.CertificateChainPolicy.ExtraStore.Add(_certificates.serverChain[i]);
}
}

Expand All @@ -780,15 +783,18 @@ public async Task SslStream_UntrustedCaWithCustomTrust_OK(bool usePartialChain)
serverChain = new X509Certificate2Collection();
for (int i = 0; i < split; i++)
{
serverChain.Add(certificates.serverChain[i]);
serverChain.Add(_certificates.serverChain[i]);
}
}
else
{
serverChain = certificates.serverChain;
serverChain = _certificates.serverChain;
}

serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(certificates.serverCert, certificates.serverChain);
// TODO: line below is wrong, but it breaks on Mac, it should be
// serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(_certificates.serverCert, serverChain);
// [ActiveIssue("https://github.com/dotnet/runtime/issues/73295")]
serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(_certificates.serverCert, _certificates.serverChain);

(Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams();
using (clientStream)
Expand Down Expand Up @@ -818,7 +824,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(certificates.serverChain[certificates.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 @@ -835,7 +841,7 @@ public async Task SslStream_UntrustedCaWithCustomCallback_Throws(bool customCall
}

var serverOptions = new SslServerAuthenticationOptions();
serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(certificates.serverCert, certificates.serverChain);
serverOptions.ServerCertificateContext = SslStreamCertificateContext.Create(_certificates.serverCert, _certificates.serverChain);

(Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams();
using (clientStream)
Expand Down

0 comments on commit 1f0582e

Please sign in to comment.