diff --git a/KestrelHttpServer.sln b/KestrelHttpServer.sln index 56420c09fa..a5e8db2e49 100644 --- a/KestrelHttpServer.sln +++ b/KestrelHttpServer.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 15 -VisualStudioVersion = 15.0.26621.2 +VisualStudioVersion = 15.0.26706.0 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{7972A5D6-3385-4127-9277-428506DD44FF}" ProjectSection(SolutionItems) = preProject @@ -52,9 +52,20 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server.Kestrel.Performance", "test\Microsoft.AspNetCore.Server.Kestrel.Performance\Microsoft.AspNetCore.Server.Kestrel.Performance.csproj", "{EBFE9719-A44B-4978-A71F-D5C254E7F35A}" EndProject -Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "TestResources", "TestResources", "{2822C132-BFFB-4D53-AC5B-E7E47DD81A6E}" +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "TestCertificates", "TestCertificates", "{2822C132-BFFB-4D53-AC5B-E7E47DD81A6E}" ProjectSection(SolutionItems) = preProject - test\shared\TestResources\testCert.pfx = test\shared\TestResources\testCert.pfx + test\shared\TestCertificates\eku.client.ini = test\shared\TestCertificates\eku.client.ini + test\shared\TestCertificates\eku.client.pfx = test\shared\TestCertificates\eku.client.pfx + test\shared\TestCertificates\eku.code_signing.ini = test\shared\TestCertificates\eku.code_signing.ini + test\shared\TestCertificates\eku.code_signing.pfx = test\shared\TestCertificates\eku.code_signing.pfx + test\shared\TestCertificates\eku.multiple_usages.ini = test\shared\TestCertificates\eku.multiple_usages.ini + test\shared\TestCertificates\eku.multiple_usages.pfx = test\shared\TestCertificates\eku.multiple_usages.pfx + test\shared\TestCertificates\eku.server.ini = test\shared\TestCertificates\eku.server.ini + test\shared\TestCertificates\eku.server.pfx = test\shared\TestCertificates\eku.server.pfx + test\shared\TestCertificates\make-test-certs.sh = test\shared\TestCertificates\make-test-certs.sh + test\shared\TestCertificates\no_extensions.ini = test\shared\TestCertificates\no_extensions.ini + test\shared\TestCertificates\no_extensions.pfx = test\shared\TestCertificates\no_extensions.pfx + test\shared\TestCertificates\testCert.pfx = test\shared\TestCertificates\testCert.pfx EndProjectSection EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv", "src\Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv\Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.csproj", "{A76B8C8C-0DC5-4DD3-9B1F-02E51A0915F4}" diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapter.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapter.cs index 5f17cfaf63..4cf33b1832 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapter.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapter.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Linq; using System.Net.Security; using System.Security.Cryptography.X509Certificates; using System.Threading.Tasks; @@ -15,9 +16,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https { public class HttpsConnectionAdapter : IConnectionAdapter { + // See http://oid-info.com/get/1.3.6.1.5.5.7.3.1 + // Indicates that a certificate can be used as a SSL server certificate + private const string ServerAuthenticationOid = "1.3.6.1.5.5.7.3.1"; + private static readonly ClosedAdaptedConnection _closedAdaptedConnection = new ClosedAdaptedConnection(); private readonly HttpsConnectionAdapterOptions _options; + private readonly X509Certificate2 _serverCertificate; private readonly ILogger _logger; public HttpsConnectionAdapter(HttpsConnectionAdapterOptions options) @@ -37,6 +43,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https throw new ArgumentException(HttpsStrings.ServiceCertificateRequired, nameof(options)); } + // capture the certificate now so it can be switched after validation + _serverCertificate = options.ServerCertificate; + + EnsureCertificateIsAllowedForServerAuth(_serverCertificate); + _options = options; _logger = loggerFactory?.CreateLogger(nameof(HttpsConnectionAdapter)); } @@ -100,7 +111,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https try { - await sslStream.AuthenticateAsServerAsync(_options.ServerCertificate, certificateRequired, + await sslStream.AuthenticateAsServerAsync(_serverCertificate, certificateRequired, _options.SslProtocols, _options.CheckCertificateRevocation); } catch (IOException ex) @@ -119,6 +130,43 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https return new HttpsAdaptedConnection(sslStream); } + private static void EnsureCertificateIsAllowedForServerAuth(X509Certificate2 certificate) + { + /* If the Extended Key Usage extension is included, then we check that the serverAuth usage is included. (http://oid-info.com/get/1.3.6.1.5.5.7.3.1) + * If the Extended Key Usage extension is not included, then we assume the certificate is allowed for all usages. + * + * See also https://blogs.msdn.microsoft.com/kaushal/2012/02/17/client-certificates-vs-server-certificates/ + * + * From https://tools.ietf.org/html/rfc3280#section-4.2.1.13 "Certificate Extensions: Extended Key Usage" + * + * If the (Extended Key Usage) extension is present, then the certificate MUST only be used + * for one of the purposes indicated. If multiple purposes are + * indicated the application need not recognize all purposes indicated, + * as long as the intended purpose is present. Certificate using + * applications MAY require that a particular purpose be indicated in + * order for the certificate to be acceptable to that application. + */ + + var hasEkuExtension = false; + + foreach (var extension in certificate.Extensions.OfType()) + { + hasEkuExtension = true; + foreach (var oid in extension.EnhancedKeyUsages) + { + if (oid.Value.Equals(ServerAuthenticationOid, StringComparison.Ordinal)) + { + return; + } + } + } + + if (hasEkuExtension) + { + throw new InvalidOperationException(HttpsStrings.FormatInvalidServerCertificateEku(certificate.Thumbprint)); + } + } + private static X509Certificate2 ConvertToX509Certificate2(X509Certificate certificate) { if (certificate == null) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapterOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapterOptions.cs index d10fdbf23b..f09ea10bc1 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapterOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapterOptions.cs @@ -23,7 +23,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https } /// + /// /// Specifies the server certificate used to authenticate HTTPS connections. + /// + /// + /// If the server certificate has an Extended Key Usage extension, the usages must include Server Authentication (OID 1.3.6.1.5.5.7.3.1). + /// /// public X509Certificate2 ServerCertificate { get; set; } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsStrings.resx b/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsStrings.resx index d0745abab8..b7ab3141db 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsStrings.resx +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsStrings.resx @@ -120,6 +120,9 @@ Failed to authenticate HTTPS connection. + + Certificate {thumbprint} cannot be used as an SSL server certificate. It has an Extended Key Usage extension but the usages do not include Server Authentication (OID 1.3.6.1.5.5.7.3.1). + The server certificate parameter is required. diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Https/Properties/AssemblyInfo.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Https/Properties/AssemblyInfo.cs new file mode 100644 index 0000000000..3bb5150c92 --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Https/Properties/AssemblyInfo.cs @@ -0,0 +1,6 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Server.Kestrel.FunctionalTests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Https/Properties/HttpsStrings.Designer.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Https/Properties/HttpsStrings.Designer.cs index 1b86a18e5d..03676ba6a7 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Https/Properties/HttpsStrings.Designer.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Https/Properties/HttpsStrings.Designer.cs @@ -24,6 +24,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https internal static string FormatAuthenticationFailed() => GetString("AuthenticationFailed"); + /// + /// Certificate {thumbprint} cannot be used as an SSL server certificate. It has an Extended Key Usage extension but the usages do not include Server Authentication (OID 1.3.6.1.5.5.7.3.1). + /// + internal static string InvalidServerCertificateEku + { + get => GetString("InvalidServerCertificateEku"); + } + + /// + /// Certificate {thumbprint} cannot be used as an SSL server certificate. It has an Extended Key Usage extension but the usages do not include Server Authentication (OID 1.3.6.1.5.5.7.3.1). + /// + internal static string FormatInvalidServerCertificateEku(object thumbprint) + => string.Format(CultureInfo.CurrentCulture, GetString("InvalidServerCertificateEku", "thumbprint"), thumbprint); + /// /// The server certificate parameter is required. /// diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/Microsoft.AspNetCore.Server.Kestrel.Core.Tests.csproj b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/Microsoft.AspNetCore.Server.Kestrel.Core.Tests.csproj index d4f68d8023..5289c063ab 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/Microsoft.AspNetCore.Server.Kestrel.Core.Tests.csproj +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/Microsoft.AspNetCore.Server.Kestrel.Core.Tests.csproj @@ -17,7 +17,7 @@ - + diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsConnectionAdapterTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsConnectionAdapterTests.cs index 0f6b48097e..d75728f8f0 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsConnectionAdapterTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsConnectionAdapterTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Net; using System.Net.Http; using System.Net.Security; @@ -18,12 +19,19 @@ using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Testing; using Xunit; +using Xunit.Abstractions; namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { public class HttpsConnectionAdapterTests { private static X509Certificate2 _x509Certificate2 = new X509Certificate2(TestResources.TestCertificatePath, "testPassword"); + private readonly ITestOutputHelper _output; + + public HttpsConnectionAdapterTests(ITestOutputHelper output) + { + _output = output; + } // https://github.com/aspnet/KestrelHttpServer/issues/240 // This test currently fails on mono because of an issue with SslStream. @@ -367,6 +375,60 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } + [Theory] + [InlineData("no_extensions.pfx")] + public void AcceptsCertificateWithoutExtensions(string testCertName) + { + var certPath = TestResources.GetCertPath(testCertName); + _output.WriteLine("Loading " + certPath); + var cert = new X509Certificate2(certPath, "testPassword"); + Assert.Empty(cert.Extensions.OfType()); + + new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + { + ServerCertificate = cert, + }); + } + + [Theory] + [InlineData("eku.server.pfx")] + [InlineData("eku.multiple_usages.pfx")] + public void ValidatesEnhancedKeyUsageOnCertificate(string testCertName) + { + var certPath = TestResources.GetCertPath(testCertName); + _output.WriteLine("Loading " + certPath); + var cert = new X509Certificate2(certPath, "testPassword"); + Assert.NotEmpty(cert.Extensions); + var eku = Assert.Single(cert.Extensions.OfType()); + Assert.NotEmpty(eku.EnhancedKeyUsages); + + new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + { + ServerCertificate = cert, + }); + } + + [Theory] + [InlineData("eku.code_signing.pfx")] + [InlineData("eku.client.pfx")] + public void ThrowsForCertificatesMissingServerEku(string testCertName) + { + var certPath = TestResources.GetCertPath(testCertName); + _output.WriteLine("Loading " + certPath); + var cert = new X509Certificate2(certPath, "testPassword"); + Assert.NotEmpty(cert.Extensions); + var eku = Assert.Single(cert.Extensions.OfType()); + Assert.NotEmpty(eku.EnhancedKeyUsages); + + var ex = Assert.Throws(() => + new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + { + ServerCertificate = cert, + })); + + Assert.Equal(HttpsStrings.FormatInvalidServerCertificateEku(cert.Thumbprint), ex.Message); + } + private static async Task App(HttpContext httpContext) { var request = httpContext.Request; diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.csproj b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.csproj index e96a61c766..9265ff3a69 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.csproj +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.csproj @@ -16,7 +16,7 @@ - + diff --git a/test/shared/TestCertificates/eku.client.ini b/test/shared/TestCertificates/eku.client.ini new file mode 100644 index 0000000000..e2f2d8ab74 --- /dev/null +++ b/test/shared/TestCertificates/eku.client.ini @@ -0,0 +1,12 @@ +# See https://www.openssl.org/docs/man1.0.2/apps/req.html for details on file format + +[ req ] +prompt = no +distinguished_name = testdn + +[ testdn ] +commonName = testcertonly + +# see https://www.openssl.org/docs/man1.0.2/apps/x509v3_config.html +[ req_extensions ] +extendedKeyUsage = clientAuth diff --git a/test/shared/TestCertificates/eku.client.pfx b/test/shared/TestCertificates/eku.client.pfx new file mode 100644 index 0000000000..32c76a1928 Binary files /dev/null and b/test/shared/TestCertificates/eku.client.pfx differ diff --git a/test/shared/TestCertificates/eku.code_signing.ini b/test/shared/TestCertificates/eku.code_signing.ini new file mode 100644 index 0000000000..d6a6c118f6 --- /dev/null +++ b/test/shared/TestCertificates/eku.code_signing.ini @@ -0,0 +1,12 @@ +# See https://www.openssl.org/docs/man1.0.2/apps/req.html for details on file format + +[ req ] +prompt = no +distinguished_name = testdn + +[ testdn ] +commonName = testcertonly + +# see https://www.openssl.org/docs/man1.0.2/apps/x509v3_config.html +[ req_extensions ] +extendedKeyUsage = codeSigning diff --git a/test/shared/TestCertificates/eku.code_signing.pfx b/test/shared/TestCertificates/eku.code_signing.pfx new file mode 100644 index 0000000000..050dfd05fe Binary files /dev/null and b/test/shared/TestCertificates/eku.code_signing.pfx differ diff --git a/test/shared/TestCertificates/eku.multiple_usages.ini b/test/shared/TestCertificates/eku.multiple_usages.ini new file mode 100644 index 0000000000..128af9a6fb --- /dev/null +++ b/test/shared/TestCertificates/eku.multiple_usages.ini @@ -0,0 +1,12 @@ +# See https://www.openssl.org/docs/man1.0.2/apps/req.html for details on file format + +[ req ] +prompt = no +distinguished_name = req_distinguished_name + +[ req_distinguished_name ] +commonName = testcertonly + +# see https://www.openssl.org/docs/man1.0.2/apps/x509v3_config.html +[ req_extensions ] +extendedKeyUsage = serverAuth,clientAuth diff --git a/test/shared/TestCertificates/eku.multiple_usages.pfx b/test/shared/TestCertificates/eku.multiple_usages.pfx new file mode 100644 index 0000000000..3bbbd9c0d4 Binary files /dev/null and b/test/shared/TestCertificates/eku.multiple_usages.pfx differ diff --git a/test/shared/TestCertificates/eku.server.ini b/test/shared/TestCertificates/eku.server.ini new file mode 100644 index 0000000000..a3f07ef543 --- /dev/null +++ b/test/shared/TestCertificates/eku.server.ini @@ -0,0 +1,12 @@ +# See https://www.openssl.org/docs/man1.0.2/apps/req.html for details on file format + +[ req ] +prompt = no +distinguished_name = testdn + +[ testdn ] +commonName = testcertonly + +# see https://www.openssl.org/docs/man1.0.2/apps/x509v3_config.html +[ req_extensions ] +extendedKeyUsage = serverAuth diff --git a/test/shared/TestCertificates/eku.server.pfx b/test/shared/TestCertificates/eku.server.pfx new file mode 100644 index 0000000000..8ac3ad5bdb Binary files /dev/null and b/test/shared/TestCertificates/eku.server.pfx differ diff --git a/test/shared/TestCertificates/make-test-certs.sh b/test/shared/TestCertificates/make-test-certs.sh new file mode 100755 index 0000000000..e489408b1c --- /dev/null +++ b/test/shared/TestCertificates/make-test-certs.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash + +# +# Should be obvious, but don't use the certs created here for anything real. This is just meant for our testing. +# + +set -euo pipefail + +__machine_has() { + hash "$1" > /dev/null 2>&1 + return $? +} + +# +# Main +# + +if ! __machine_has openssl; then + echo 'OpenSSL is required to create the test certificates.' 1>&2 + exit 1 +fi + +# See https://www.openssl.org/docs/man1.0.2/apps/x509.html for more details on the openssl conf file + +if [[ $# == 0 ]]; then + echo "Usage: ${BASH_SOURCE[0]} ..." + echo "" + echo "Arguments:" + echo " Multiple allowed. Path to the *.ini file that configures a cert." +fi + +# loop over all arguments +while [[ $# > 0 ]]; do + # bashism for trimming the extension + config=$1 + shift + cert_name="${config%.*}" + key="$cert_name.pem" + cert="$cert_name.crt" + pfx="$cert_name.pfx" + + echo "Creating cert $cert_name" + + # see https://www.openssl.org/docs/man1.0.2/apps/req.html + openssl req -x509 \ + -days 1 \ + -config $config \ + -nodes \ + -newkey rsa:2048 \ + -keyout $key \ + -extensions req_extensions \ + -out $cert + + # See https://www.openssl.org/docs/man1.0.2/apps/pkcs12.html + openssl pkcs12 -export \ + -in $cert \ + -inkey $key \ + -out $pfx \ + -password pass:testPassword # so secure ;) + + rm $key $cert +done diff --git a/test/shared/TestCertificates/no_extensions.ini b/test/shared/TestCertificates/no_extensions.ini new file mode 100644 index 0000000000..df234f06a6 --- /dev/null +++ b/test/shared/TestCertificates/no_extensions.ini @@ -0,0 +1,13 @@ +# See https://www.openssl.org/docs/man1.0.2/apps/req.html for details on file format + +[ req ] +prompt = no +distinguished_name = testdn + +[ testdn ] +commonName = testcertonly + +# see https://www.openssl.org/docs/man1.0.2/apps/x509v3_config.html +[ req_extensions ] +# keyUsages = +# extendedKeyUsage = diff --git a/test/shared/TestCertificates/no_extensions.pfx b/test/shared/TestCertificates/no_extensions.pfx new file mode 100644 index 0000000000..b4be4b5eda Binary files /dev/null and b/test/shared/TestCertificates/no_extensions.pfx differ diff --git a/test/shared/TestResources/testCert.pfx b/test/shared/TestCertificates/testCert.pfx similarity index 100% rename from test/shared/TestResources/testCert.pfx rename to test/shared/TestCertificates/testCert.pfx diff --git a/test/shared/TestResources.cs b/test/shared/TestResources.cs index 30e495bd2e..6b0cf70fec 100644 --- a/test/shared/TestResources.cs +++ b/test/shared/TestResources.cs @@ -7,8 +7,9 @@ namespace Microsoft.AspNetCore.Testing { public static class TestResources { - private static readonly string _testCertificatePath = Path.Combine(Directory.GetCurrentDirectory(), "testCert.pfx"); + private static readonly string _baseDir = Directory.GetCurrentDirectory(); - public static string TestCertificatePath => _testCertificatePath; + public static string TestCertificatePath { get; } = Path.Combine(_baseDir, "testCert.pfx"); + public static string GetCertPath(string name) => Path.Combine(_baseDir, name); } }