From fe83e69b1a731323d0a9c8364ef72daaf462e15b Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Thu, 1 Jun 2017 17:51:23 -0700 Subject: [PATCH] Add a startup filter which initializes the key ring before the server starts --- ...taProtectionServiceCollectionExtensions.cs | 2 + .../Internal/DataProtectionStartupFilter.cs | 43 ++++++++ .../LoggingExtensions.cs | 24 +++- .../HostingTests.cs | 104 ++++++++++++++++++ ...soft.AspNetCore.DataProtection.Test.csproj | 1 + 5 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 src/Microsoft.AspNetCore.DataProtection/Internal/DataProtectionStartupFilter.cs create mode 100644 test/Microsoft.AspNetCore.DataProtection.Test/HostingTests.cs diff --git a/src/Microsoft.AspNetCore.DataProtection/DataProtectionServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.DataProtection/DataProtectionServiceCollectionExtensions.cs index 04e68303d0..0df59732cf 100644 --- a/src/Microsoft.AspNetCore.DataProtection/DataProtectionServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.DataProtection/DataProtectionServiceCollectionExtensions.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.DataProtection.Internal; using Microsoft.AspNetCore.DataProtection.KeyManagement; using Microsoft.AspNetCore.DataProtection.KeyManagement.Internal; using Microsoft.AspNetCore.DataProtection.XmlEncryption; +using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -77,6 +78,7 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); services.TryAddSingleton(); + services.TryAddEnumerable(ServiceDescriptor.Singleton()); // Internal services services.TryAddSingleton(); diff --git a/src/Microsoft.AspNetCore.DataProtection/Internal/DataProtectionStartupFilter.cs b/src/Microsoft.AspNetCore.DataProtection/Internal/DataProtectionStartupFilter.cs new file mode 100644 index 0000000000..f2abbae5be --- /dev/null +++ b/src/Microsoft.AspNetCore.DataProtection/Internal/DataProtectionStartupFilter.cs @@ -0,0 +1,43 @@ +// 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 Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.DataProtection.KeyManagement.Internal; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.DataProtection.Internal +{ + internal class DataProtectionStartupFilter : IStartupFilter + { + private readonly IKeyRingProvider _keyRingProvider; + private readonly ILogger _logger; + + public DataProtectionStartupFilter(IKeyRingProvider keyRingProvider, ILoggerFactory loggerFactory) + { + _keyRingProvider = keyRingProvider; + _logger = loggerFactory.CreateLogger(); + } + + public Action Configure(Action next) + { + try + { + // It doesn't look like much, but this preloads the key ring, + // which in turn may load data from remote stores like Redis or Azure. + var keyRing = _keyRingProvider.GetCurrentKeyRing(); + + _logger.KeyRingWasLoadedOnStartup(keyRing.DefaultKeyId); + } + catch (Exception ex) + { + // This should be non-fatal, so swallow, log, and allow server startup to continue. + // The KeyRingProvider may be able to try again on the first request. + _logger.KeyRingFailedToLoadOnStartup(ex); + } + + return next; + } + } +} diff --git a/src/Microsoft.AspNetCore.DataProtection/LoggingExtensions.cs b/src/Microsoft.AspNetCore.DataProtection/LoggingExtensions.cs index b7667b503c..a2cc325f46 100644 --- a/src/Microsoft.AspNetCore.DataProtection/LoggingExtensions.cs +++ b/src/Microsoft.AspNetCore.DataProtection/LoggingExtensions.cs @@ -129,6 +129,10 @@ namespace Microsoft.Extensions.Logging private static Action _policyResolutionStatesThatANewKeyShouldBeAddedToTheKeyRing; + private static Action _keyRingWasLoadedOnStartup; + + private static Action _keyRingFailedToLoadOnStartup; + private static Action _usingEphemeralKeyRepository; private static Action _usingRegistryAsKeyRepositoryWithDPAPI; @@ -388,6 +392,14 @@ namespace Microsoft.Extensions.Logging _usingAzureAsKeyRepository = LoggerMessage.Define(eventId: 0, logLevel: LogLevel.Information, formatString: "Azure Web Sites environment detected. Using '{FullName}' as key repository; keys will not be encrypted at rest."); + _keyRingWasLoadedOnStartup = LoggerMessage.Define( + eventId: 0, + logLevel: LogLevel.Debug, + formatString: "Key ring with default key {KeyId:B} was loaded during application startup."); + _keyRingFailedToLoadOnStartup = LoggerMessage.Define( + eventId: 0, + logLevel: LogLevel.Information, + formatString: "Key ring failed to load during application startup."); } /// @@ -760,5 +772,15 @@ namespace Microsoft.Extensions.Logging { _usingAzureAsKeyRepository(logger, fullName, null); } + + public static void KeyRingWasLoadedOnStartup(this ILogger logger, Guid defaultKeyId) + { + _keyRingWasLoadedOnStartup(logger, defaultKeyId, null); + } + + public static void KeyRingFailedToLoadOnStartup(this ILogger logger, Exception innerException) + { + _keyRingFailedToLoadOnStartup(logger, innerException); + } } -} \ No newline at end of file +} diff --git a/test/Microsoft.AspNetCore.DataProtection.Test/HostingTests.cs b/test/Microsoft.AspNetCore.DataProtection.Test/HostingTests.cs new file mode 100644 index 0000000000..cd43effe37 --- /dev/null +++ b/test/Microsoft.AspNetCore.DataProtection.Test/HostingTests.cs @@ -0,0 +1,104 @@ +// 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.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.DataProtection.KeyManagement.Internal; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.DataProtection.Test +{ + public class HostingTests + { + [Fact] + public async Task LoadsKeyRingBeforeServerStarts() + { + var tcs = new TaskCompletionSource(); + var mockKeyRing = new Mock(); + mockKeyRing.Setup(m => m.GetCurrentKeyRing()) + .Returns(Mock.Of()) + .Callback(() => tcs.TrySetResult(null)); + + var builder = new WebHostBuilder() + .UseStartup() + .ConfigureServices(s => + s.AddDataProtection() + .Services + .Replace(ServiceDescriptor.Singleton(mockKeyRing.Object)) + .AddSingleton( + new FakeServer(onStart: () => tcs.TrySetException(new InvalidOperationException("Server was started before key ring was initialized"))))); + + using (var host = builder.Build()) + { + await host.StartAsync(); + } + + await tcs.Task.TimeoutAfter(TimeSpan.FromSeconds(10)); + mockKeyRing.VerifyAll(); + } + + [Fact] + public async Task StartupContinuesOnFailureToLoadKey() + { + var mockKeyRing = new Mock(); + mockKeyRing.Setup(m => m.GetCurrentKeyRing()) + .Throws(new NotSupportedException("This mock doesn't actually work, but shouldn't kill the server")) + .Verifiable(); + + var builder = new WebHostBuilder() + .UseStartup() + .ConfigureServices(s => + s.AddDataProtection() + .Services + .Replace(ServiceDescriptor.Singleton(mockKeyRing.Object)) + .AddSingleton(Mock.Of())); + + using (var host = builder.Build()) + { + await host.StartAsync(); + } + + mockKeyRing.VerifyAll(); + } + + private class TestStartup + { + public void Configure(IApplicationBuilder app) + { + } + } + + public class FakeServer : IServer + { + private readonly Action _onStart; + + public FakeServer(Action onStart) + { + _onStart = onStart; + } + + public IFeatureCollection Features => new FeatureCollection(); + + public Task StartAsync(IHttpApplication application, CancellationToken cancellationToken) + { + _onStart(); + return Task.CompletedTask; + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + public void Dispose() + { + } + } + } +} diff --git a/test/Microsoft.AspNetCore.DataProtection.Test/Microsoft.AspNetCore.DataProtection.Test.csproj b/test/Microsoft.AspNetCore.DataProtection.Test/Microsoft.AspNetCore.DataProtection.Test.csproj index 065d45985a..6dd58b3e0b 100644 --- a/test/Microsoft.AspNetCore.DataProtection.Test/Microsoft.AspNetCore.DataProtection.Test.csproj +++ b/test/Microsoft.AspNetCore.DataProtection.Test/Microsoft.AspNetCore.DataProtection.Test.csproj @@ -17,6 +17,7 @@ +