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.

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

[QUIC] Certificate name validation #56175

Merged
merged 5 commits into from
Jul 23, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal static class CertificateValidation
{
private static readonly IdnMapping s_idnMapping = new IdnMapping();

internal static SslPolicyErrors BuildChainAndVerifyProperties(X509Chain chain, X509Certificate2 remoteCertificate, bool checkCertName, string? hostName)
internal static SslPolicyErrors BuildChainAndVerifyProperties(X509Chain chain, X509Certificate2 remoteCertificate, bool checkCertName, bool isServer, string? hostName)
{
SslPolicyErrors errors = chain.Build(remoteCertificate) ?
SslPolicyErrors.None :
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Win32.SafeHandles;
using System.Diagnostics;
using System.Net.Security;
using System.Runtime.InteropServices;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Security.Principal;

namespace System.Net
{
internal static partial class CertificateValidation
{
internal static SslPolicyErrors BuildChainAndVerifyProperties(X509Chain chain, X509Certificate2 remoteCertificate, bool checkCertName, bool isServer, string? hostName)
{
SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None;

bool chainBuildResult = chain.Build(remoteCertificate);
if (!chainBuildResult // Build failed on handle or on policy.
&& chain.SafeHandle!.DangerousGetHandle() == IntPtr.Zero) // Build failed to generate a valid handle.
{
throw new CryptographicException(Marshal.GetLastPInvokeError());
}

if (checkCertName)
{
unsafe
{
uint status = 0;

var eppStruct = new Interop.Crypt32.SSL_EXTRA_CERT_CHAIN_POLICY_PARA()
{
cbSize = (uint)sizeof(Interop.Crypt32.SSL_EXTRA_CERT_CHAIN_POLICY_PARA),
// Authenticate the remote party: (e.g. when operating in server mode, authenticate the client).
dwAuthType = isServer ? Interop.Crypt32.AuthType.AUTHTYPE_CLIENT : Interop.Crypt32.AuthType.AUTHTYPE_SERVER,
fdwChecks = 0,
pwszServerName = null
};

var cppStruct = new Interop.Crypt32.CERT_CHAIN_POLICY_PARA()
{
cbSize = (uint)sizeof(Interop.Crypt32.CERT_CHAIN_POLICY_PARA),
dwFlags = 0,
pvExtraPolicyPara = &eppStruct
};

fixed (char* namePtr = hostName)
{
eppStruct.pwszServerName = namePtr;
cppStruct.dwFlags |=
(Interop.Crypt32.CertChainPolicyIgnoreFlags.CERT_CHAIN_POLICY_IGNORE_ALL &
~Interop.Crypt32.CertChainPolicyIgnoreFlags.CERT_CHAIN_POLICY_IGNORE_INVALID_NAME_FLAG);

SafeX509ChainHandle chainContext = chain.SafeHandle!;
status = Verify(chainContext, ref cppStruct);
if (status == Interop.Crypt32.CertChainPolicyErrors.CERT_E_CN_NO_MATCH)
{
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateNameMismatch;
}
}
}
}

if (!chainBuildResult)
{
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors;
}

return sslPolicyErrors;
}

private static unsafe uint Verify(SafeX509ChainHandle chainContext, ref Interop.Crypt32.CERT_CHAIN_POLICY_PARA cpp)
{
Interop.Crypt32.CERT_CHAIN_POLICY_STATUS status = default;
status.cbSize = (uint)sizeof(Interop.Crypt32.CERT_CHAIN_POLICY_STATUS);

bool errorCode =
Interop.Crypt32.CertVerifyCertificateChainPolicy(
(IntPtr)Interop.Crypt32.CertChainPolicy.CERT_CHAIN_POLICY_SSL,
chainContext,
ref cpp,
ref status);

if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(chainContext, $"CertVerifyCertificateChainPolicy returned: {errorCode}. Status: {status.dwError}");
return status.dwError;
}
}
}
3 changes: 3 additions & 0 deletions src/libraries/System.Net.Quic/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -150,5 +150,8 @@
<data name="net_quic_writing_notallowed" xml:space="preserve">
<value>Writing is not allowed on stream.</value>
</data>
<data name="net_ssl_app_protocols_invalid" xml:space="preserve">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this? looks like extra.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we need this, it's used by one of the included interop source files:

throw new ArgumentException(SR.net_ssl_app_protocols_invalid, nameof(applicationProtocols));

https://github.com/dotnet/runtime/pull/56175/files#diff-1d4ce147edca0ce81f312b96fce76ac52eb02eb6d04e660817e3b47230edc794R64

<value>The application protocol list is invalid.</value>
</data>
</root>

46 changes: 46 additions & 0 deletions src/libraries/System.Net.Quic/src/System.Net.Quic.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,45 @@
<!-- Windows specific files -->
<ItemGroup Condition=" '$(TargetsWindows)' == 'true'">
<Compile Include="$(CommonPath)Interop\Windows\Interop.Libraries.cs" Link="Common\Interop\Windows\Interop.Libraries.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.CERT_CONTEXT.cs" Link="Common\Interop\Windows\Crypt32\Interop.CERT_CONTEXT.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.CERT_INFO.cs" Link="Common\Interop\Windows\Crypt32\Interop.CERT_INFO.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.CERT_PUBLIC_KEY_INFO.cs" Link="Common\Interop\Windows\Crypt32\Interop.CERT_PUBLIC_KEY_INFO.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.CRYPT_ALGORITHM_IDENTIFIER.cs" Link="Common\Interop\Windows\Crypt32\Interop.Interop.CRYPT_ALGORITHM_IDENTIFIER.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.CRYPT_BIT_BLOB.cs" Link="Common\Interop\Windows\Crypt32\Interop.Interop.CRYPT_BIT_BLOB.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.DATA_BLOB.cs" Link="Common\Interop\Windows\Crypt32\Interop.DATA_BLOB.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.certificates.cs" Link="Common\Interop\Windows\Crypt32\Interop.certificates.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.certificates_types.cs" Link="Common\Interop\Windows\Crypt32\Interop.certificates_types.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.CertEnumCertificatesInStore.cs" Link="Common\Interop\Windows\Crypt32\Interop.CertEnumCertificatesInStore.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Crypt32\Interop.MsgEncodingType.cs" Link="Common\Interop\Windows\Crypt32\Interop.Interop.MsgEncodingType.cs" />
<Compile Include="$(CommonPath)System\Net\Security\CertificateValidation.Windows.cs" Link="Common\System\Net\Security\CertificateValidation.Windows.cs" />
<Compile Include="System\Net\Quic\Implementations\MsQuic\Interop\MsQuicStatusCodes.Windows.cs" />
</ItemGroup>
<!-- Unix (OSX + Linux) specific files -->
<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
<Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs" Link="Common\Interop\Unix\Interop.Libraries.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.ASN1.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.ASN1.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.BIO.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.BIO.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.ERR.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.ERR.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.Initialization.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Initialization.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.Crypto.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Crypto.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.Ssl.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Ssl.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.SslCtx.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.SslCtx.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.SetProtocolOptions.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.SetProtocolOptions.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.X509.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.X509.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.X509Name.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.X509Name.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.X509Ext.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.X509Ext.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.X509Stack.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.X509Stack.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.X509StoreCtx.cs" Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.X509StoreCtx.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Net.Security.Native\Interop.Initialization.cs" Link="Common\Interop\Unix\System.Net.Security.Native\Interop.Initialization.cs" />
<Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeX509Handles.Unix.cs" Link="Common\Microsoft\Win32\SafeHandles\SafeX509Handles.Unix.cs" />
<Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\X509ExtensionSafeHandles.Unix.cs" Link="Common\Microsoft\Win32\SafeHandles\X509ExtensionSafeHandles.Unix.cs" />
<Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeInteriorHandle.cs" Link="Common\Microsoft\Win32\SafeHandles\SafeInteriorHandle.cs" />
<Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeBioHandle.Unix.cs" Link="Common\Microsoft\Win32\SafeHandles\SafeBioHandle.Unix.cs" />
<Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\Asn1SafeHandles.Unix.cs" Link="Common\Microsoft\Win32\SafeHandles\Asn1SafeHandles.Unix.cs" />
<Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeHandleCache.cs" Link="Common\Microsoft\Win32\SafeHandles\SafeHandleCache.cs" />
<Compile Include="$(CommonPath)System\Net\Security\CertificateValidation.Unix.cs" Link="Common\System\Net\Security\CertificateValidation.Unix.cs" />
</ItemGroup>
<!-- Linux specific files -->
<ItemGroup Condition="'$(TargetsLinux)' == 'true'">
<Compile Include="$(CommonPath)Interop\Linux\Interop.Libraries.cs" Link="Common\Interop\Linux\Interop.Libraries.cs" />
Expand All @@ -56,6 +93,7 @@
<Compile Include="$(CommonPath)Interop\OSX\Interop.Libraries.cs" Link="Common\Interop\OSX\Interop.Libraries.cs" />
<Compile Include="System\Net\Quic\Implementations\MsQuic\Interop\MsQuicStatusCodes.OSX.cs" />
</ItemGroup>

<!-- Project references -->

<ItemGroup>
Expand Down Expand Up @@ -83,6 +121,14 @@
<Reference Include="System.Threading.Channels" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.Cryptography.OpenSsl\src\System.Security.Cryptography.OpenSsl.csproj" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this with all the added interop?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we do, the interop uses public classes from this project. It doesn't build without it and the same it's referenced in S.N.Http.

</ItemGroup>

<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
<Reference Include="System.Diagnostics.StackTrace" />
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>

<!-- Support for deploying msquic -->
<ItemGroup Condition="'$(TargetsWindows)' == 'true' and
('$(TargetArchitecture)' == 'x64' or '$(TargetArchitecture)' == 'x86')">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ internal sealed class MsQuicConnection : QuicConnectionProvider
private bool _remoteCertificateRequired;
private X509RevocationMode _revocationMode = X509RevocationMode.Offline;
private RemoteCertificateValidationCallback? _remoteCertificateValidationCallback;
private string? _targetHost;

private string TargetHost => _targetHost ?? (_targetHost = _remoteEndPoint switch
wfurt marked this conversation as resolved.
Show resolved Hide resolved
{
DnsEndPoint dnsEp => dnsEp.Host,
IPEndPoint ipEp => ipEp.Address.ToString(),
_ => throw new Exception($"Unsupported remote endpoint type '{_remoteEndPoint.GetType()}'.")
});

internal sealed class State
{
Expand Down Expand Up @@ -183,6 +191,7 @@ public MsQuicConnection(QuicClientConnectionOptions options)
{
_revocationMode = options.ClientAuthenticationOptions.CertificateRevocationCheckMode;
_remoteCertificateValidationCallback = options.ClientAuthenticationOptions.RemoteCertificateValidationCallback;
_targetHost = options.ClientAuthenticationOptions.TargetHost;
}

_state.StateGCHandle = GCHandle.Alloc(_state);
Expand Down Expand Up @@ -401,10 +410,7 @@ private static uint HandleEventPeerCertificateReceived(State state, ref Connecti
chain.ChainPolicy.ExtraStore.AddRange(additionalCertificates);
}

if (!chain.Build(certificate))
{
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors;
}
sslPolicyErrors |= CertificateValidation.BuildChainAndVerifyProperties(chain, certificate, true, connection._isServer, connection.TargetHost);
}

if (!connection._remoteCertificateRequired)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@
<Compile Include="System\Net\Security\SslStreamPal.Windows.cs" />
<Compile Include="System\Net\Security\SslConnectionInfo.Windows.cs" />
<Compile Include="System\Net\Security\StreamSizes.Windows.cs" />
<Compile Include="$(CommonPath)System\Net\Security\CertificateValidation.Windows.cs"
Link="Common\System\Net\Security\CertificateValidation.Windows.cs" />
<Compile Include="$(CommonPath)System\Net\Security\SecurityBuffer.Windows.cs"
Link="Common\System\Net\Security\SecurityBuffer.Windows.cs" />
<Compile Include="$(CommonPath)System\Net\Security\SecurityBufferType.Windows.cs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(
bool isServer,
string? hostName)
{
return CertificateValidation.BuildChainAndVerifyProperties(chain, remoteCertificate, checkCertName, hostName);
return CertificateValidation.BuildChainAndVerifyProperties(chain, remoteCertificate, checkCertName, isServer, hostName);
}

//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,60 +21,7 @@ internal static SslPolicyErrors VerifyCertificateProperties(
bool isServer,
string? hostName)
{
SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None;

bool chainBuildResult = chain.Build(remoteCertificate);
if (!chainBuildResult // Build failed on handle or on policy.
&& chain.SafeHandle!.DangerousGetHandle() == IntPtr.Zero) // Build failed to generate a valid handle.
{
throw new CryptographicException(Marshal.GetLastPInvokeError());
}

if (checkCertName)
{
unsafe
{
uint status = 0;

var eppStruct = new Interop.Crypt32.SSL_EXTRA_CERT_CHAIN_POLICY_PARA()
{
cbSize = (uint)sizeof(Interop.Crypt32.SSL_EXTRA_CERT_CHAIN_POLICY_PARA),
// Authenticate the remote party: (e.g. when operating in server mode, authenticate the client).
dwAuthType = isServer ? Interop.Crypt32.AuthType.AUTHTYPE_CLIENT : Interop.Crypt32.AuthType.AUTHTYPE_SERVER,
fdwChecks = 0,
pwszServerName = null
};

var cppStruct = new Interop.Crypt32.CERT_CHAIN_POLICY_PARA()
{
cbSize = (uint)sizeof(Interop.Crypt32.CERT_CHAIN_POLICY_PARA),
dwFlags = 0,
pvExtraPolicyPara = &eppStruct
};

fixed (char* namePtr = hostName)
{
eppStruct.pwszServerName = namePtr;
cppStruct.dwFlags |=
(Interop.Crypt32.CertChainPolicyIgnoreFlags.CERT_CHAIN_POLICY_IGNORE_ALL &
~Interop.Crypt32.CertChainPolicyIgnoreFlags.CERT_CHAIN_POLICY_IGNORE_INVALID_NAME_FLAG);

SafeX509ChainHandle chainContext = chain.SafeHandle!;
status = Verify(chainContext, ref cppStruct);
if (status == Interop.Crypt32.CertChainPolicyErrors.CERT_E_CN_NO_MATCH)
{
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateNameMismatch;
}
}
}
}

if (!chainBuildResult)
{
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors;
}

return sslPolicyErrors;
return CertificateValidation.BuildChainAndVerifyProperties(chain, remoteCertificate, checkCertName, isServer, hostName);
}

//
Expand Down Expand Up @@ -185,21 +132,5 @@ internal static X509Store OpenStore(StoreLocation storeLocation)

return store;
}

private static unsafe uint Verify(SafeX509ChainHandle chainContext, ref Interop.Crypt32.CERT_CHAIN_POLICY_PARA cpp)
{
Interop.Crypt32.CERT_CHAIN_POLICY_STATUS status = default;
status.cbSize = (uint)sizeof(Interop.Crypt32.CERT_CHAIN_POLICY_STATUS);

bool errorCode =
Interop.Crypt32.CertVerifyCertificateChainPolicy(
(IntPtr)Interop.Crypt32.CertChainPolicy.CERT_CHAIN_POLICY_SSL,
chainContext,
ref cpp,
ref status);

if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(chainContext, $"CertVerifyCertificateChainPolicy returned: {errorCode}. Status: {status.dwError}");
return status.dwError;
}
}
}