Ensure HTTPS handshake errors aren't logged on socket close

src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionFilter.cs

@@ -8,12 +8,15 @@ using System.Security.Cryptography.X509Certificates;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Server.Kestrel.Filter;
+using Microsoft.AspNetCore.Server.Kestrel.Https.Internal;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Https
 {
     public class HttpsConnectionFilter : IConnectionFilter
     {
+        private static readonly ClosedStream _closedStream = new ClosedStream();
+
         private readonly HttpsConnectionFilterOptions _options;
         private readonly IConnectionFilter _previous;
         private readonly ILogger _logger;
@@ -95,8 +98,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https
                     certificateRequired = true;
                 }
 
-                context.Connection = sslStream;
-
                 try
                 {
                     await sslStream.AuthenticateAsServerAsync(_options.ServerCertificate, certificateRequired,
@@ -105,6 +106,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https
                 catch (IOException ex)
                 {
                     _logger?.LogInformation(1, ex, "Failed to authenticate HTTPS connection.");
+
+                    sslStream.Dispose();
+                    context.Connection = _closedStream;
+
                     return;
                 }
 
@@ -121,6 +126,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https
 
                     features.Get<IHttpRequestFeature>().Scheme = "https";
                 };
+
+                context.Connection = sslStream;
             }
         }
 

src/Microsoft.AspNetCore.Server.Kestrel.Https/Internal/ClosedStream.cs

@@ -0,0 +1,66 @@
+// 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.IO;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal
+{
+    internal class ClosedStream : Stream
+    {
+        public override bool CanRead => true;
+        public override bool CanSeek => false;
+        public override bool CanWrite => false;
+
+        public override long Length
+        {
+            get
+            {
+                throw new NotSupportedException();
+            }
+        }
+
+        public override long Position
+        {
+            get
+            {
+                throw new NotSupportedException();
+            }
+            set
+            {
+                throw new NotSupportedException();
+            }
+        }
+
+        public override void Flush()
+        {
+        }
+
+        public override long Seek(long offset, SeekOrigin origin)
+        {
+            throw new NotSupportedException();
+        }
+
+        public override void SetLength(long value)
+        {
+            throw new NotSupportedException();
+        }
+
+        public override int Read(byte[] buffer, int offset, int count)
+        {
+            return 0;
+        }
+
+        public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
+        {
+            return Task.FromResult(0);
+        }
+
+        public override void Write(byte[] buffer, int offset, int count)
+        {
+            throw new NotSupportedException();
+        }
+    }
+}

test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/HttpsTests.cs

@@ -0,0 +1,157 @@
+// 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.Sockets;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Server.Kestrel.Https;
+using Microsoft.AspNetCore.Testing;
+using Microsoft.Extensions.Logging;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
+{
+    public class HttpsTests
+    {
+        [Fact]
+        public async Task EmptyRequestLoggedAsInformation()
+        {
+            var loggerFactory = new HandshakeErrorLoggerFactory();
+
+            var hostBuilder = new WebHostBuilder()
+                .UseKestrel(options =>
+                {
+                    options.UseHttps(@"TestResources/testCert.pfx", "testPassword");
+                })
+                .UseUrls("https://127.0.0.1:0/")
+                .UseLoggerFactory(loggerFactory)
+                .Configure(app => { });
+
+            using (var host = hostBuilder.Build())
+            {
+                host.Start();
+
+                using (await HttpClientSlim.GetSocket(new Uri($"http://127.0.0.1:{host.GetPort()}/")))
+                {
+                    // Close socket immediately
+                }
+            }
+
+            await loggerFactory.FilterLogger.LogTcs.Task;
+
+            Assert.Equal(1, loggerFactory.FilterLogger.LastEventId.Id);
+            Assert.Equal(LogLevel.Information, loggerFactory.FilterLogger.LastLogLevel);
+            Assert.Equal(0, loggerFactory.ErrorLogger.TotalErrorsLogged);
+        }
+
+
+        [Fact]
+        public async Task ClientHandshakeFailureLoggedAsInformation()
+        {
+            var loggerFactory = new HandshakeErrorLoggerFactory();
+
+            var hostBuilder = new WebHostBuilder()
+                .UseKestrel(options =>
+                {
+                    options.UseHttps(@"TestResources/testCert.pfx", "testPassword");
+                })
+                .UseUrls("https://127.0.0.1:0/")
+                .UseLoggerFactory(loggerFactory)
+                .Configure(app => { });
+
+            using (var host = hostBuilder.Build())
+            {
+                host.Start();
+
+                using (var socket = await HttpClientSlim.GetSocket(new Uri($"https://127.0.0.1:{host.GetPort()}/")))
+                using (var stream = new NetworkStream(socket))
+                {
+                    // Send null bytes and close socket
+                    await stream.WriteAsync(new byte[10], 0, 10);
+                }
+            }
+
+            await loggerFactory.FilterLogger.LogTcs.Task;
+
+            Assert.Equal(1, loggerFactory.FilterLogger.LastEventId.Id);
+            Assert.Equal(LogLevel.Information, loggerFactory.FilterLogger.LastLogLevel);
+            Assert.Equal(0, loggerFactory.ErrorLogger.TotalErrorsLogged);
+        }
+
+        private class HandshakeErrorLoggerFactory : ILoggerFactory
+        {
+            public HttpsConnectionFilterLogger FilterLogger { get; } = new HttpsConnectionFilterLogger();
+            public ApplicationErrorLogger ErrorLogger { get; } = new ApplicationErrorLogger();
+
+            public ILogger CreateLogger(string categoryName)
+            {
+                if (categoryName == nameof(HttpsConnectionFilter))
+                {
+                    return FilterLogger;
+                }
+                else
+                {
+                    return ErrorLogger;
+                }
+
+            }
+
+            public void AddProvider(ILoggerProvider provider)
+            {
+                throw new NotImplementedException();
+            }
+
+            public void Dispose()
+            {
+            }
+        }
+
+        private class HttpsConnectionFilterLogger : ILogger
+        {
+            public LogLevel LastLogLevel { get; set; }
+            public EventId LastEventId { get; set; }
+            public TaskCompletionSource<object> LogTcs { get; } = new TaskCompletionSource<object>();
+
+            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
+            {
+                LastLogLevel = logLevel;
+                LastEventId = eventId;
+                LogTcs.SetResult(null);
+            }
+
+            public bool IsEnabled(LogLevel logLevel)
+            {
+                throw new NotImplementedException();
+            }
+
+            public IDisposable BeginScope<TState>(TState state)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
+        private class ApplicationErrorLogger : ILogger
+        {
+            public int TotalErrorsLogged { get; set; }
+
+            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
+            {
+                if (logLevel == LogLevel.Error)
+                {
+                    TotalErrorsLogged++;
+                }
+            }
+
+            public bool IsEnabled(LogLevel logLevel)
+            {
+                return true;
+            }
+
+            public IDisposable BeginScope<TState>(TState state)
+            {
+                throw new NotImplementedException();
+            }
+        }
+    }
+}

test/Microsoft.AspNetCore.Server.KestrelTests/HttpsConnectionFilterTests.cs

@@ -331,34 +331,6 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             }
         }
 
-        [Fact]
-        public async Task ClientHandshakeFailureLoggedAsInformation()
-        {
-            var logger = new HandshakerErrorLogger();
-            var mockLoggerFactory = new Mock<ILoggerFactory>(MockBehavior.Strict);
-            mockLoggerFactory.Setup(m => m.CreateLogger(nameof(HttpsConnectionFilter))).Returns(logger);
-
-            var serviceContext = new TestServiceContext(new HttpsConnectionFilter(
-                new HttpsConnectionFilterOptions { ServerCertificate = _x509Certificate2 },
-                new NoOpConnectionFilter(),
-                mockLoggerFactory.Object)
-            );
-
-            using (var server = new TestServer(App, serviceContext, _serverAddress))
-            {
-                using (await HttpClientSlim.GetSocket(new Uri($"https://localhost:{server.Port}/")))
-                {
-                    // Close socket immediately
-                }
-
-                await logger.LogTcs.Task;
-
-                Assert.Equal(1, logger.LastEventId.Id);
-                Assert.Equal(LogLevel.Information, logger.LastLogLevel);
-                mockLoggerFactory.VerifyAll();
-            }
-        }
-
         private static async Task App(HttpContext httpContext)
         {
             var request = httpContext.Request;
@@ -405,29 +377,5 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
                 Assert.Null(line);
             }
         }
-
-        private class HandshakerErrorLogger : ILogger
-        {
-            public LogLevel LastLogLevel { get; set; }
-            public EventId LastEventId { get; set; }
-            public TaskCompletionSource<object> LogTcs { get; } = new TaskCompletionSource<object>();
-
-            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
-            {
-                LastLogLevel = logLevel;
-                LastEventId = eventId;
-                LogTcs.SetResult(null);
-            }
-
-            public bool IsEnabled(LogLevel logLevel)
-            {
-                throw new NotImplementedException();
-            }
-
-            public IDisposable BeginScope<TState>(TState state)
-            {
-                throw new NotImplementedException();
-            }
-        }
     }
 }