From 827474bb3280adbdcc325bbec74e64ad266ada14 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Thu, 22 Apr 2021 08:59:09 -0700 Subject: [PATCH] fix quic cert validation with OpenSSL (#51015) * fix quic cert validation with OpenSSL * feedback from review * update certificate conditions --- .../src/System.Net.Quic.csproj | 1 + .../MsQuic/Interop/MsQuicEnums.cs | 1 + .../MsQuic/Interop/MsQuicNativeMethods.cs | 1 + .../Interop/SafeMsQuicConfigurationHandle.cs | 6 ++ .../MsQuic/MsQuicConnection.cs | 57 +++++++++++++++---- .../tests/FunctionalTests/MsQuicTestBase.cs | 15 ++++- 6 files changed, 66 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj b/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj index 8ef23c8a93b..9534d921282 100644 --- a/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj +++ b/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj @@ -45,6 +45,7 @@ + diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/MsQuicEnums.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/MsQuicEnums.cs index 33ffea25d1c..f380fa712b2 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/MsQuicEnums.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/MsQuicEnums.cs @@ -34,6 +34,7 @@ internal enum QUIC_CREDENTIAL_FLAGS : uint DEFER_CERTIFICATE_VALIDATION = 0x00000020, // Schannel only currently. REQUIRE_CLIENT_AUTHENTICATION = 0x00000040, // Schannel only currently. USE_TLS_BUILTIN_CERTIFICATE_VALIDATION = 0x00000080, + USE_PORTABLE_CERTIFICATES = 0x00004000, } internal enum QUIC_CERTIFICATE_HASH_STORE_FLAGS diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/MsQuicNativeMethods.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/MsQuicNativeMethods.cs index f3ea17bd036..cebcdaf46f0 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/MsQuicNativeMethods.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/MsQuicNativeMethods.cs @@ -426,6 +426,7 @@ internal struct ConnectionEventPeerCertificateReceived internal IntPtr PlatformCertificateHandle; internal uint DeferredErrorFlags; internal uint DeferredStatus; + internal IntPtr PlatformCertificateChainHandle; } [StructLayout(LayoutKind.Explicit)] diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/SafeMsQuicConfigurationHandle.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/SafeMsQuicConfigurationHandle.cs index cc3d21140ec..1c8e173f919 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/SafeMsQuicConfigurationHandle.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/Interop/SafeMsQuicConfigurationHandle.cs @@ -72,6 +72,12 @@ private static unsafe SafeMsQuicConfigurationHandle Create(QuicOptions options, flags |= QUIC_CREDENTIAL_FLAGS.INDICATE_CERTIFICATE_RECEIVED | QUIC_CREDENTIAL_FLAGS.NO_CERTIFICATE_VALIDATION; } + if (!OperatingSystem.IsWindows()) + { + // Use certificate handles on Windows, fall-back to ASN1 otherwise. + flags |= QUIC_CREDENTIAL_FLAGS.USE_PORTABLE_CERTIFICATES; + } + Debug.Assert(!MsQuicApi.Api.Registration.IsInvalid); var settings = new QuicSettings diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs index fe38d8f8ac8..6d6431fc728 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs @@ -204,12 +204,7 @@ private static uint HandleEventPeerCertificateReceived(State state, ref Connecti SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None; X509Chain? chain = null; X509Certificate2? certificate = null; - - if (!OperatingSystem.IsWindows()) - { - // TODO fix validation with OpenSSL - return MsQuicStatusCodes.Success; - } + X509Certificate2Collection? additionalCertificates = null; MsQuicConnection? connection = state.Connection; if (connection == null) @@ -217,16 +212,46 @@ private static uint HandleEventPeerCertificateReceived(State state, ref Connecti return MsQuicStatusCodes.InvalidState; } - if (connectionEvent.Data.PeerCertificateReceived.PlatformCertificateHandle != IntPtr.Zero) - { - certificate = new X509Certificate2(connectionEvent.Data.PeerCertificateReceived.PlatformCertificateHandle); - } - try { + if (connectionEvent.Data.PeerCertificateReceived.PlatformCertificateHandle != IntPtr.Zero) + { + if (OperatingSystem.IsWindows()) + { + certificate = new X509Certificate2(connectionEvent.Data.PeerCertificateReceived.PlatformCertificateHandle); + } + else + { + unsafe + { + ReadOnlySpan quicBuffer; + if (connectionEvent.Data.PeerCertificateReceived.PlatformCertificateChainHandle != IntPtr.Zero) + { + quicBuffer = new ReadOnlySpan((void*)connectionEvent.Data.PeerCertificateReceived.PlatformCertificateChainHandle, sizeof(QuicBuffer)); + if (quicBuffer[0].Length != 0 && quicBuffer[0].Buffer != null) + { + ReadOnlySpan asn1 = new ReadOnlySpan(quicBuffer[0].Buffer, (int)quicBuffer[0].Length); + additionalCertificates = new X509Certificate2Collection(); + additionalCertificates.Import(asn1); + if (additionalCertificates.Count > 0) + { + certificate = additionalCertificates[0]; + } + } + } + else + { + quicBuffer = new ReadOnlySpan((void*)connectionEvent.Data.PeerCertificateReceived.PlatformCertificateHandle, sizeof(QuicBuffer)); + ReadOnlySpan asn1 = new ReadOnlySpan(quicBuffer[0].Buffer, (int)quicBuffer[0].Length); + certificate = new X509Certificate2(asn1); + } + } + } + } + if (certificate == null) { - if (NetEventSource.Log.IsEnabled() && connection._remoteCertificateRequired) NetEventSource.Error(state.Connection, $"Remote certificate required, but no remote certificate received"); + if (NetEventSource.Log.IsEnabled() && connection._remoteCertificateRequired) NetEventSource.Error(state.Connection, "Remote certificate required, but no remote certificate received"); sslPolicyErrors |= SslPolicyErrors.RemoteCertificateNotAvailable; } else @@ -236,6 +261,14 @@ private static uint HandleEventPeerCertificateReceived(State state, ref Connecti chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; chain.ChainPolicy.ApplicationPolicy.Add(connection._isServer ? s_clientAuthOid : s_serverAuthOid); + if (additionalCertificates != null && additionalCertificates.Count > 1) + { + for (int i = 1; i < additionalCertificates.Count; i++) + { + chain.ChainPolicy.ExtraStore.Add(additionalCertificates[i]); + } + } + if (!chain.Build(certificate)) { sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors; diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTestBase.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTestBase.cs index aea4d003503..bbd697050b2 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTestBase.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTestBase.cs @@ -3,21 +3,30 @@ using System.Collections.Generic; using System.Net.Security; +using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Tasks; +using Xunit; namespace System.Net.Quic.Tests { // TODO: why do we have 2 base classes with some duplicated methods? public class MsQuicTestBase { + public X509Certificate2 ServerCertificate = System.Net.Test.Common.Configuration.Certificates.GetServerCertificate(); + + public bool RemoteCertificateValidationCallback(object sender, X509Certificate? certificate, X509Chain? chain, SslPolicyErrors sslPolicyErrors) + { + Assert.Equal(ServerCertificate.GetCertHash(), certificate?.GetCertHash()); + return true; + } + public SslServerAuthenticationOptions GetSslServerAuthenticationOptions() { return new SslServerAuthenticationOptions() { ApplicationProtocols = new List() { new SslApplicationProtocol("quictest") }, - // TODO: use a cert. MsQuic currently only allows certs that are trusted. - ServerCertificate = System.Net.Test.Common.Configuration.Certificates.GetServerCertificate() + ServerCertificate = ServerCertificate }; } @@ -26,7 +35,7 @@ public SslClientAuthenticationOptions GetSslClientAuthenticationOptions() return new SslClientAuthenticationOptions() { ApplicationProtocols = new List() { new SslApplicationProtocol("quictest") }, - RemoteCertificateValidationCallback = (sender, certificate, chain, errors) => { return true; } + RemoteCertificateValidationCallback = RemoteCertificateValidationCallback }; } -- GitLab