diff --git a/Dockerfile b/Dockerfile index b83befa..4f607e6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,7 +13,6 @@ RUN dotnet publish src/Bifrost.Web/Bifrost.Web.csproj \ FROM mcr.microsoft.com/dotnet/aspnet:8.0-bookworm-slim AS runtime WORKDIR /app ENV ASPNETCORE_URLS=http://0.0.0.0:5080 \ - ASPNETCORE_FORWARDEDHEADERS_ENABLED=true \ DOTNET_RUNNING_IN_CONTAINER=true EXPOSE 5080 diff --git a/src/Bifrost.Web/Features/Shared/StartupConfigurationValidator.cs b/src/Bifrost.Web/Features/Shared/StartupConfigurationValidator.cs index 41b1147..0ba0e10 100644 --- a/src/Bifrost.Web/Features/Shared/StartupConfigurationValidator.cs +++ b/src/Bifrost.Web/Features/Shared/StartupConfigurationValidator.cs @@ -66,6 +66,11 @@ public sealed class StartupConfigurationValidator( failures.Add("Connection string 'Bifrost' must be configured."); } + if (string.IsNullOrWhiteSpace(configuration["DataProtection:KeysPath"])) + { + failures.Add("DataProtection:KeysPath must be configured for production."); + } + if (failures.Count > 0) { throw new InvalidOperationException( diff --git a/src/Bifrost.Web/Program.cs b/src/Bifrost.Web/Program.cs index 695b478..218a7ff 100644 --- a/src/Bifrost.Web/Program.cs +++ b/src/Bifrost.Web/Program.cs @@ -17,9 +17,11 @@ using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Authentication.OAuth; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.HttpOverrides; using Microsoft.AspNetCore.HttpLogging; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; +using System.Net; var builder = WebApplication.CreateBuilder(args); @@ -110,14 +112,28 @@ builder.Services.AddHttpClient(client => client.BaseAddress = new Uri("https://api.stripe.com/"); }); +var dataProtectionBuilder = builder.Services + .AddDataProtection() + .SetApplicationName("Bifrost"); var dataProtectionKeysPath = builder.Configuration["DataProtection:KeysPath"]; if (!string.IsNullOrWhiteSpace(dataProtectionKeysPath)) { - builder.Services - .AddDataProtection() - .PersistKeysToFileSystem(new DirectoryInfo(dataProtectionKeysPath)); + dataProtectionBuilder.PersistKeysToFileSystem(new DirectoryInfo(dataProtectionKeysPath)); } +// nginx on this host is the only proxy, and it connects from loopback. Trusting exactly +// those addresses keeps a direct client from choosing its own address with X-Forwarded-For. +builder.Services.Configure(options => +{ + options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; + options.KnownNetworks.Clear(); + options.KnownProxies.Clear(); + options.KnownProxies.Add(IPAddress.Loopback); + options.KnownProxies.Add(IPAddress.IPv6Loopback); + options.KnownProxies.Add(IPAddress.Loopback.MapToIPv6()); + options.ForwardLimit = 1; +}); + var configuredGitHubOAuthOptions = builder.Configuration .GetSection(GitHubOAuthOptions.SectionName) .Get() ?? new GitHubOAuthOptions(); @@ -194,6 +210,8 @@ await using (var scope = app.Services.CreateAsyncScope()) .RunAsync(CancellationToken.None); } +app.UseForwardedHeaders(); + if (!app.Environment.IsDevelopment()) { app.UseExceptionHandler("/Error"); diff --git a/tests/Bifrost.Web.Tests/HostTrustTests.cs b/tests/Bifrost.Web.Tests/HostTrustTests.cs new file mode 100644 index 0000000..78f9057 --- /dev/null +++ b/tests/Bifrost.Web.Tests/HostTrustTests.cs @@ -0,0 +1,234 @@ +using System.Net; +using Bifrost.Web.Configuration; +using Bifrost.Web.Features.Shared; +using Bifrost.Web.Tests.Support; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.HttpOverrides; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; + +namespace Bifrost.Web.Tests; + +public sealed class HostTrustTests : IClassFixture +{ + private readonly TestWebApplicationFactory _factory; + + public HostTrustTests(TestWebApplicationFactory factory) + { + _factory = factory; + } + + [Theory] + [InlineData("127.0.0.1")] + [InlineData("::1")] + [InlineData("::ffff:127.0.0.1")] + public async Task Forwarded_for_from_loopback_peer_becomes_remote_ip(string peer) + { + var context = await RunForwardedHeaders(peer, "203.0.113.7", "https"); + + Assert.Equal(IPAddress.Parse("203.0.113.7"), context.Connection.RemoteIpAddress); + Assert.Equal("https", context.Request.Scheme); + } + + [Fact] + public async Task Forwarded_for_from_non_loopback_peer_is_ignored() + { + var context = await RunForwardedHeaders("198.51.100.9", "203.0.113.7", "https"); + + Assert.Equal(IPAddress.Parse("198.51.100.9"), context.Connection.RemoteIpAddress); + Assert.Equal("http", context.Request.Scheme); + } + + [Fact] + public async Task Only_the_rightmost_forwarded_entry_is_used() + { + var context = await RunForwardedHeaders("127.0.0.1", "6.6.6.6, 203.0.113.7", null); + + Assert.Equal(IPAddress.Parse("203.0.113.7"), context.Connection.RemoteIpAddress); + } + + [Fact] + public async Task A_loopback_forwarded_entry_does_not_chain_to_earlier_entries() + { + var context = await RunForwardedHeaders("127.0.0.1", "6.6.6.6, 127.0.0.1", null); + + Assert.Equal(IPAddress.Loopback, context.Connection.RemoteIpAddress); + } + + [Fact] + public async Task Only_the_exact_loopback_addresses_are_trusted_not_the_loopback_range() + { + var context = await RunForwardedHeaders("127.0.0.2", "203.0.113.7", null); + + Assert.Equal(IPAddress.Parse("127.0.0.2"), context.Connection.RemoteIpAddress); + } + + [Fact] + public void Payload_protected_under_one_content_root_unprotects_under_another() + { + var root = NewRoot(); + var keys = Path.Combine(root, "keys"); + try + { + using var hostA = WithContentRoot(MakeRelease(root, "release-a"), keys); + using var hostB = WithContentRoot(MakeRelease(root, "release-b"), keys); + + var sealedPayload = Protect(hostA, "payload"); + + Assert.Equal("payload", Unprotect(hostB, sealedPayload)); + Assert.NotEmpty(Directory.GetFiles(keys, "key-*.xml")); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + [Fact] + public void A_different_keys_path_does_not_unprotect_the_payload() + { + var root = NewRoot(); + try + { + using var hostA = WithContentRoot(MakeRelease(root, "release-a"), Path.Combine(root, "keys-a")); + using var hostB = WithContentRoot(MakeRelease(root, "release-b"), Path.Combine(root, "keys-b")); + + var sealedPayload = Protect(hostA, "payload"); + + Assert.ThrowsAny( + () => Unprotect(hostB, sealedPayload)); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + [Fact] + public void The_container_image_does_not_set_the_forwarded_headers_switch() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "Dockerfile"))) + { + directory = directory.Parent; + } + + Assert.NotNull(directory); + Assert.DoesNotContain( + "ASPNETCORE_FORWARDEDHEADERS_ENABLED", + File.ReadAllText(Path.Combine(directory!.FullName, "Dockerfile"))); + } + + [Fact] + public void Production_without_keys_path_fails_startup_validation() + { + var withoutKey = Assert.Throws(() => Validate(keysPath: null)); + Assert.Contains("DataProtection:KeysPath", withoutKey.Message); + + Validate(keysPath: "/not/echoed/anywhere"); + } + + [Fact] + public void Startup_refusal_names_the_key_and_never_echoes_configured_values() + { + var canary = "canary-secret-4f1e"; + var refusal = Assert.Throws( + () => Validate(keysPath: null, connectionString: canary)); + + Assert.Contains("DataProtection:KeysPath", refusal.Message); + Assert.DoesNotContain(canary, refusal.Message); + } + + [Fact] + public void Development_does_not_require_keys_path() + { + Validate(keysPath: null, environment: Environments.Development); + } + + private async Task RunForwardedHeaders(string peer, string forwardedFor, string? forwardedProto) + { + var options = _factory.Services.GetRequiredService>(); + var middleware = new ForwardedHeadersMiddleware( + _ => Task.CompletedTask, + NullLoggerFactory.Instance, + options); + var context = new DefaultHttpContext(); + context.Request.Scheme = "http"; + context.Connection.RemoteIpAddress = IPAddress.Parse(peer); + context.Request.Headers["X-Forwarded-For"] = forwardedFor; + if (forwardedProto is not null) + { + context.Request.Headers["X-Forwarded-Proto"] = forwardedProto; + } + + await middleware.Invoke(context); + return context; + } + + private static string NewRoot() => + Path.Combine(Path.GetTempPath(), "bifrost-dp-" + Guid.NewGuid().ToString("N")); + + private static string MakeRelease(string root, string name) => + Directory.CreateDirectory(Path.Combine(root, name)).FullName; + + private static string Protect(TestWebApplicationFactory host, string value) => + host.Services.GetRequiredService().CreateProtector("P").Protect(value); + + private static string Unprotect(TestWebApplicationFactory host, string value) => + host.Services.GetRequiredService().CreateProtector("P").Unprotect(value); + + private TestWebApplicationFactory WithContentRoot(string contentRoot, string keysPath) => + new KeyRingFactory(contentRoot, keysPath); + + private sealed class KeyRingFactory(string contentRoot, string keysPath) : TestWebApplicationFactory + { + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + base.ConfigureWebHost(builder); + builder.UseContentRoot(contentRoot); + builder.UseSetting("DataProtection:KeysPath", keysPath); + } + } + + private static void Validate( + string? keysPath, + string environment = "Production", + string? connectionString = "Host=db") + { + var values = new Dictionary + { + ["AllowedHosts"] = "bifrost.example", + ["ConnectionStrings:Bifrost"] = connectionString, + ["DataProtection:KeysPath"] = keysPath + }; + var configuration = new ConfigurationBuilder().AddInMemoryCollection(values).Build(); + var hostEnvironment = new StubHostEnvironment(environment); + + new StartupConfigurationValidator( + configuration, + hostEnvironment, + Options.Create(new GitHubOAuthOptions { ClientId = "id", ClientSecret = "secret" }), + Options.Create(new GitHubAppOptions { EnableWebhookSync = false }), + Options.Create(new HeimdallOptions { EnablePatronSupportIntake = false }), + Options.Create(new StripeOptions()), + Options.Create(new BifrostHostOptions + { + PublicBaseUrl = "https://bifrost.example", + ExpectedHost = "bifrost.example" + })).Validate(); + } + + private sealed class StubHostEnvironment(string environmentName) : IHostEnvironment + { + public string EnvironmentName { get; set; } = environmentName; + public string ApplicationName { get; set; } = "Bifrost.Web"; + public string ContentRootPath { get; set; } = string.Empty; + public Microsoft.Extensions.FileProviders.IFileProvider ContentRootFileProvider { get; set; } = null!; + } +} diff --git a/tests/Bifrost.Web.Tests/Support/TestWebApplicationFactory.cs b/tests/Bifrost.Web.Tests/Support/TestWebApplicationFactory.cs index fc8675d..ed3c4ed 100644 --- a/tests/Bifrost.Web.Tests/Support/TestWebApplicationFactory.cs +++ b/tests/Bifrost.Web.Tests/Support/TestWebApplicationFactory.cs @@ -14,7 +14,7 @@ using System.Net; namespace Bifrost.Web.Tests.Support; -public sealed class TestWebApplicationFactory : WebApplicationFactory +public class TestWebApplicationFactory : WebApplicationFactory { private readonly InMemoryDatabaseRoot _databaseRoot = new(); private readonly string _databaseName = $"bifrost-tests-{Guid.NewGuid():N}";