From 6d076dafae199ec0a8e7c55bc237bbf718385031 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Tue, 16 Oct 2018 15:25:27 -0700 Subject: [PATCH 1/3] Update certificate info for .zip and .mof files --- build/repo.targets | 3 ++- korebuild-lock.txt | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/build/repo.targets b/build/repo.targets index 7c93b5b9a8..5b63ccb152 100644 --- a/build/repo.targets +++ b/build/repo.targets @@ -64,8 +64,9 @@ noship - + + diff --git a/korebuild-lock.txt b/korebuild-lock.txt index 49afc832d9..e3cc145b03 100644 --- a/korebuild-lock.txt +++ b/korebuild-lock.txt @@ -1,2 +1,2 @@ -version:2.2.0-preview2-20181011.10 -commithash:224e1163a1145b60f324e0b432419ef4ba4880cc +version:2.2.0-preview2-20181016.3 +commithash:17ee7986dbe75408d9e4efd94eab1003adcc3ad7 From 8f99140f308fa2cef5f49360694fb924f87d2cfb Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Tue, 16 Oct 2018 15:30:19 -0700 Subject: [PATCH 2/3] Add client disconnect logs (#1521) --- .../Core/IISHttpContext.IO.cs | 8 +++- .../Core/IISHttpContext.Log.cs | 39 +++++++++++++++++++ .../Core/IISHttpContext.cs | 10 ++++- .../Core/IISHttpContextOfT.cs | 5 ++- .../Core/IISHttpServer.cs | 8 ++-- test/IIS.Tests/ClientDisconnectTests.cs | 18 ++++++++- 6 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.Log.cs diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.IO.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.IO.cs index 724d2e55e1..bfe44d634c 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.IO.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.IO.cs @@ -6,6 +6,7 @@ using System.Buffers; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Http.Features; namespace Microsoft.AspNetCore.Server.IIS.Core { @@ -121,6 +122,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core catch (Exception ex) { error = ex; + Log.UnexpectedError(_logger, nameof(IISHttpContext), ex); } finally { @@ -174,6 +176,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core catch (Exception ex) { error = ex; + Log.UnexpectedError(_logger, nameof(IISHttpContext), ex); } finally { @@ -199,9 +202,9 @@ namespace Microsoft.AspNetCore.Server.IIS.Core { cts.Cancel(); } - catch (Exception) + catch (Exception ex) { - // ignore + Log.ApplicationError(_logger, ((IHttpConnectionFeature)this).ConnectionId, TraceIdentifier, ex); } }); } @@ -219,6 +222,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core internal void ConnectionReset() { AbortIO(); + Log.ConnectionDisconnect(_logger, ((IHttpConnectionFeature)this).ConnectionId); } } } diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.Log.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.Log.cs new file mode 100644 index 0000000000..1056ac1479 --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.Log.cs @@ -0,0 +1,39 @@ +// 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; +using System.Runtime.CompilerServices; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Server.IIS.Core +{ + internal abstract partial class IISHttpContext + { + private static class Log + { + private static readonly Action _connectionDisconnect = + LoggerMessage.Define(LogLevel.Debug, new EventId(1, "ConnectionDisconnect"), @"Connection ID ""{ConnectionId}"" disconnecting."); + + private static readonly Action _applicationError = + LoggerMessage.Define(LogLevel.Error, new EventId(2, "ApplicationError"), @"Connection ID ""{ConnectionId}"", Request ID ""{TraceIdentifier}"": An unhandled exception was thrown by the application."); + + private static readonly Action _unexpectedError = + LoggerMessage.Define(LogLevel.Error, new EventId(3, "UnexpectedError"), @"Unexpected exception in ""{ClassName}.{MethodName}""."); + + public static void ConnectionDisconnect(ILogger logger, string connectionId) + { + _connectionDisconnect(logger, connectionId, null); + } + + public static void ApplicationError(ILogger logger, string connectionId, string traceIdentifier, Exception ex) + { + _applicationError(logger, connectionId, traceIdentifier, ex); + } + + public static void UnexpectedError(ILogger logger, string className, Exception ex, [CallerMemberName] string methodName = null) + { + _unexpectedError(logger, className, methodName, ex); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.cs index 01aeeebec6..09e478a989 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.cs @@ -17,9 +17,11 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.HttpSys.Internal; using Microsoft.AspNetCore.Server.IIS.Core.IO; using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.IIS.Core { @@ -50,6 +52,8 @@ namespace Microsoft.AspNetCore.Server.IIS.Core private readonly MemoryPool _memoryPool; private readonly IISHttpServer _server; + private readonly ILogger _logger; + private GCHandle _thisHandle; protected Task _readBodyTask; protected Task _writeBodyTask; @@ -69,13 +73,15 @@ namespace Microsoft.AspNetCore.Server.IIS.Core MemoryPool memoryPool, IntPtr pInProcessHandler, IISServerOptions options, - IISHttpServer server) + IISHttpServer server, + ILogger logger) : base((HttpApiTypes.HTTP_REQUEST*)NativeMethods.HttpGetRawRequest(pInProcessHandler)) { _memoryPool = memoryPool; _pInProcessHandler = pInProcessHandler; _options = options; _server = server; + _logger = logger; } public Version HttpVersion { get; set; } @@ -450,6 +456,8 @@ namespace Microsoft.AspNetCore.Server.IIS.Core { _applicationException = new AggregateException(_applicationException, ex); } + + Log.ApplicationError(_logger, ((IHttpConnectionFeature)this).ConnectionId, TraceIdentifier, ex); } public void PostCompletion(NativeMethods.REQUEST_NOTIFICATION_STATUS requestNotificationStatus) diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContextOfT.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContextOfT.cs index ec3fc006b2..bb104f5ac1 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContextOfT.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContextOfT.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.IIS.Core { @@ -14,8 +15,8 @@ namespace Microsoft.AspNetCore.Server.IIS.Core { private readonly IHttpApplication _application; - public IISHttpContextOfT(MemoryPool memoryPool, IHttpApplication application, IntPtr pInProcessHandler, IISServerOptions options, IISHttpServer server) - : base(memoryPool, pInProcessHandler, options, server) + public IISHttpContextOfT(MemoryPool memoryPool, IHttpApplication application, IntPtr pInProcessHandler, IISServerOptions options, IISHttpServer server, ILogger logger) + : base(memoryPool, pInProcessHandler, options, server, logger) { _application = application; } diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs index 7cde852570..bfabf61bd7 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs @@ -82,7 +82,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core { _httpServerHandle = GCHandle.Alloc(this); - _iisContextFactory = new IISContextFactory(_memoryPool, application, _options, this); + _iisContextFactory = new IISContextFactory(_memoryPool, application, _options, this, _logger); _nativeApplication.RegisterCallbacks(_requestHandler, _shutdownHandler, _onDisconnect, _onAsyncCompletion, (IntPtr)_httpServerHandle, (IntPtr)_httpServerHandle); return Task.CompletedTask; } @@ -260,18 +260,20 @@ namespace Microsoft.AspNetCore.Server.IIS.Core private readonly MemoryPool _memoryPool; private readonly IISServerOptions _options; private readonly IISHttpServer _server; + private readonly ILogger _logger; - public IISContextFactory(MemoryPool memoryPool, IHttpApplication application, IISServerOptions options, IISHttpServer server) + public IISContextFactory(MemoryPool memoryPool, IHttpApplication application, IISServerOptions options, IISHttpServer server, ILogger logger) { _application = application; _memoryPool = memoryPool; _options = options; _server = server; + _logger = logger; } public IISHttpContext CreateHttpContext(IntPtr pInProcessHandler) { - return new IISHttpContextOfT(_memoryPool, _application, pInProcessHandler, _options, _server); + return new IISHttpContextOfT(_memoryPool, _application, pInProcessHandler, _options, _server, _logger); } } } diff --git a/test/IIS.Tests/ClientDisconnectTests.cs b/test/IIS.Tests/ClientDisconnectTests.cs index 2731c5665e..d8c7d4ef89 100644 --- a/test/IIS.Tests/ClientDisconnectTests.cs +++ b/test/IIS.Tests/ClientDisconnectTests.cs @@ -17,7 +17,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests [OSSkipCondition(OperatingSystems.Windows, WindowsVersions.Win7, "https://github.com/aspnet/IISIntegration/issues/866")] public class ClientDisconnectTests : StrictTestServerTests { - [ConditionalFact] public async Task WritesSucceedAfterClientDisconnect() { @@ -48,6 +47,8 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests await requestCompletedCompletionSource.Task.DefaultTimeout(); } + + AssertConnectionDisconnectLog(); } [ConditionalFact] @@ -88,6 +89,8 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.IsType(exception); } + + AssertConnectionDisconnectLog(); } [ConditionalFact] @@ -125,6 +128,8 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.IsType(exception); Assert.Equal("The client has disconnected", exception.Message); + + AssertConnectionDisconnectLog(); } [ConditionalFact] @@ -202,6 +207,8 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } Assert.IsType(exception); } + + AssertConnectionDisconnectLog(); } [ConditionalFact] @@ -253,6 +260,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.IsType(exception); Assert.Equal("The client has disconnected", exception.Message); + AssertConnectionDisconnectLog(); } [ConditionalFact] @@ -275,7 +283,15 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } await requestAborted.Task; } + + AssertConnectionDisconnectLog(); } + + private void AssertConnectionDisconnectLog() + { + Assert.Contains(TestSink.Writes, w => w.EventId.Name == "ConnectionDisconnect"); + } + private static async Task SendContentLength1Post(TestConnection connection) { await connection.Send( From 48d40e0e3650e6dc23ba516203f316bb52c80b4c Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 18 Oct 2018 08:49:52 -0700 Subject: [PATCH 3/3] Add mitigations to HttpsClientCert_GetCertInformation flakiness (#1529) --- .../Properties/launchSettings.json | 10 +- .../ClientCertificateFixture.cs | 97 +++++++++---------- .../ClientCertificateTests.cs | 37 +++++-- 3 files changed, 83 insertions(+), 61 deletions(-) diff --git a/samples/NativeIISSample/Properties/launchSettings.json b/samples/NativeIISSample/Properties/launchSettings.json index f62697de39..246b7a0b47 100644 --- a/samples/NativeIISSample/Properties/launchSettings.json +++ b/samples/NativeIISSample/Properties/launchSettings.json @@ -14,8 +14,8 @@ "commandLineArgs": "$(IISExpressArguments)", "environmentVariables": { "IIS_SITE_PATH": "$(MSBuildThisFileDirectory)", - "ANCM_PATH": "$(AncmPath)", - "ANCMV2_PATH": "$(AncmV2Path)", + "ANCM_PATH": "$(AspNetCoreModuleV1ShimDll)", + "ANCMV2_PATH": "$(AspNetCoreModuleV2ShimDll)", "ANCM_OUTOFPROCESS_HANDLER": "$(AspNetCoreModuleV2OutOfProcessHandlerDll)", "LAUNCHER_ARGS": "$(TargetPath)", "ASPNETCORE_ENVIRONMENT": "Development", @@ -29,9 +29,9 @@ "commandLineArgs": "$(IISArguments)", "environmentVariables": { "IIS_SITE_PATH": "$(MSBuildThisFileDirectory)", - "ANCM_PATH": "$(AncmPath)", - "ANCMV2_PATH": "$(AncmV2Path)", - "ANCM_OUTOFPROCESS_HANDLER": "$(AspNetCoreModuleV2OutOfProcessHandlerDll)", + "ANCM_PATH": "$(AspNetCoreModuleV1ShimDll)", + "ANCMV2_PATH": "$(AspNetCoreModuleV2ShimDll)", + "ASPNETCORE_MODULE_OUTOFPROCESS_HANDLER": "$(AspNetCoreModuleV2OutOfProcessHandlerDll)", "LAUNCHER_ARGS": "$(TargetPath)", "ASPNETCORE_ENVIRONMENT": "Development", "LAUNCHER_PATH": "$(DotNetPath)", diff --git a/test/Common.FunctionalTests/ClientCertificateFixture.cs b/test/Common.FunctionalTests/ClientCertificateFixture.cs index 4979865e8b..40b6f2265e 100644 --- a/test/Common.FunctionalTests/ClientCertificateFixture.cs +++ b/test/Common.FunctionalTests/ClientCertificateFixture.cs @@ -11,68 +11,57 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests public class ClientCertificateFixture : IDisposable { private X509Certificate2 _certificate; + private const string _certIssuerPrefix = "CN=IISIntegrationTest_Root"; - public X509Certificate2 Certificate + public X509Certificate2 GetOrCreateCertificate() { - get + if (_certificate != null) { - if (_certificate != null) - { - return _certificate; - } + return _certificate; + } - using (var store = new X509Store(StoreName.Root, StoreLocation.LocalMachine)) - { - store.Open(OpenFlags.ReadWrite); + using (var store = new X509Store(StoreName.Root, StoreLocation.LocalMachine)) + { + store.Open(OpenFlags.ReadWrite); + var parentKey = CreateKeyMaterial(2048); - foreach (var cert in store.Certificates) - { - if (cert.Issuer != "CN=IISIntegrationTest_Root") - { - continue; - } - _certificate = cert; - store.Close(); - return cert; - } + // Create a cert name with a random guid to avoid name conflicts + var parentRequest = new CertificateRequest( + _certIssuerPrefix + Guid.NewGuid().ToString(), + parentKey, HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); - var parentKey = CreateKeyMaterial(2048); + parentRequest.CertificateExtensions.Add( + new X509BasicConstraintsExtension( + certificateAuthority: true, + hasPathLengthConstraint: false, + pathLengthConstraint: 0, + critical: true)); - // On first run of the test, creates the certificate in the trusted root certificate authorities. - var parentRequest = new CertificateRequest("CN=IISIntegrationTest_Root", parentKey, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + parentRequest.CertificateExtensions.Add( + new X509KeyUsageExtension(X509KeyUsageFlags.DigitalSignature | X509KeyUsageFlags.NonRepudiation, critical: true)); - parentRequest.CertificateExtensions.Add( - new X509BasicConstraintsExtension( - certificateAuthority: true, - hasPathLengthConstraint: false, - pathLengthConstraint: 0, - critical: true)); + parentRequest.CertificateExtensions.Add( + new X509SubjectKeyIdentifierExtension(parentRequest.PublicKey, false)); - parentRequest.CertificateExtensions.Add( - new X509KeyUsageExtension(X509KeyUsageFlags.DigitalSignature | X509KeyUsageFlags.NonRepudiation, critical: true)); + var notBefore = DateTimeOffset.Now.AddDays(-1); + var notAfter = DateTimeOffset.Now.AddYears(5); - parentRequest.CertificateExtensions.Add( - new X509SubjectKeyIdentifierExtension(parentRequest.PublicKey, false)); + var parentCert = parentRequest.CreateSelfSigned(notBefore, notAfter); - var notBefore = DateTimeOffset.Now.AddDays(-1); - var notAfter = DateTimeOffset.Now.AddYears(5); + // Need to export/import the certificate to associate the private key with the cert. + var imported = parentCert; - var parentCert = parentRequest.CreateSelfSigned(notBefore, notAfter); + var export = parentCert.Export(X509ContentType.Pkcs12, ""); + imported = new X509Certificate2(export, "", X509KeyStorageFlags.PersistKeySet | X509KeyStorageFlags.Exportable); + Array.Clear(export, 0, export.Length); - // Need to export/import the certificate to associate the private key with the cert. - var imported = parentCert; + // Add the cert to the cert store + _certificate = imported; - var export = parentCert.Export(X509ContentType.Pkcs12, ""); - imported = new X509Certificate2(export, "", X509KeyStorageFlags.PersistKeySet | X509KeyStorageFlags.Exportable); - Array.Clear(export, 0, export.Length); - - // Add the cert to the cert store - _certificate = imported; - - store.Add(certificate: imported); - store.Close(); - return imported; - } + store.Add(certificate: imported); + store.Close(); + return imported; } } @@ -86,7 +75,17 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests using (var store = new X509Store(StoreName.Root, StoreLocation.LocalMachine)) { store.Open(OpenFlags.ReadWrite); - store.Remove(Certificate); + store.Remove(_certificate); + + // Remove any extra certs that were left by previous tests. + for (var i = store.Certificates.Count - 1; i >= 0; i--) + { + var cert = store.Certificates[i]; + if (cert.Issuer.StartsWith(_certIssuerPrefix)) + { + store.Remove(cert); + } + } store.Close(); } } diff --git a/test/Common.FunctionalTests/ClientCertificateTests.cs b/test/Common.FunctionalTests/ClientCertificateTests.cs index 289fb18c2f..43ccc83eff 100644 --- a/test/Common.FunctionalTests/ClientCertificateTests.cs +++ b/test/Common.FunctionalTests/ClientCertificateTests.cs @@ -1,7 +1,9 @@ // 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; using System.Net.Http; +using System.Security.Cryptography.X509Certificates; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; using Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests; @@ -9,6 +11,7 @@ using Microsoft.AspNetCore.Server.IntegrationTesting; using Microsoft.AspNetCore.Server.IntegrationTesting.Common; using Microsoft.AspNetCore.Server.IntegrationTesting.IIS; using Microsoft.AspNetCore.Testing.xunit; +using Microsoft.Extensions.Logging; using Xunit; namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests @@ -54,31 +57,51 @@ namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests deploymentParameters.ApplicationBaseUriHint = $"https://localhost:{port}/"; deploymentParameters.AddHttpsToServerConfig(); - var deploymentResult = await DeployAsync(deploymentParameters); var handler = new HttpClientHandler { ServerCertificateCustomValidationCallback = (a, b, c, d) => true, ClientCertificateOptions = ClientCertificateOption.Manual, }; + X509Certificate2 cert = null; if (sendClientCert) { - Assert.NotNull(_certFixture.Certificate); - handler.ClientCertificates.Add(_certFixture.Certificate); + cert = _certFixture.GetOrCreateCertificate(); + handler.ClientCertificates.Add(cert); } + var deploymentResult = await DeployAsync(deploymentParameters); + var client = deploymentResult.CreateClient(handler); var response = await client.GetAsync("GetClientCert"); var responseText = await response.Content.ReadAsStringAsync(); - if (sendClientCert) + try { - Assert.Equal($"Enabled;{_certFixture.Certificate.GetCertHashString()}", responseText); + if (sendClientCert) + { + Assert.Equal($"Enabled;{cert.GetCertHashString()}", responseText); + } + else + { + Assert.Equal("Disabled", responseText); + } } - else + catch (Exception ex) { - Assert.Equal("Disabled", responseText); + Logger.LogError($"Certificate is invalid. Issuer name: {cert.Issuer}"); + using (var store = new X509Store(StoreName.Root, StoreLocation.LocalMachine)) + { + Logger.LogError($"List of current certificates in root store:"); + store.Open(OpenFlags.ReadWrite); + foreach (var otherCert in store.Certificates) + { + Logger.LogError(otherCert.Issuer); + } + store.Close(); + } + throw ex; } } }