Trust loopback forwarded headers and pin the Data Protection key ring
UseForwardedHeaders runs first with KnownProxies exactly loopback and ForwardLimit 1; Data Protection always uses application name "Bifrost"; the startup validator refuses production without DataProtection:KeysPath. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
837bc11a15
commit
2e0de9601c
4 changed files with 207 additions and 4 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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<StripeCheckoutService>(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<ForwardedHeadersOptions>(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<GitHubOAuthOptions>() ?? 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");
|
||||
|
|
|
|||
180
tests/Bifrost.Web.Tests/HostTrustTests.cs
Normal file
180
tests/Bifrost.Web.Tests/HostTrustTests.cs
Normal file
|
|
@ -0,0 +1,180 @@
|
|||
using System.Net;
|
||||
using Bifrost.Web.Configuration;
|
||||
using Bifrost.Web.Features.Shared;
|
||||
using Bifrost.Web.Tests.Support;
|
||||
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<TestWebApplicationFactory>
|
||||
{
|
||||
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 void Payload_protected_under_one_content_root_unprotects_under_another()
|
||||
{
|
||||
var root = Path.Combine(Path.GetTempPath(), "bifrost-dp-" + Guid.NewGuid().ToString("N"));
|
||||
var keys = Path.Combine(root, "keys");
|
||||
var releaseA = Directory.CreateDirectory(Path.Combine(root, "release-a")).FullName;
|
||||
var releaseB = Directory.CreateDirectory(Path.Combine(root, "release-b")).FullName;
|
||||
try
|
||||
{
|
||||
using var hostA = WithContentRoot(releaseA, keys);
|
||||
using var hostB = WithContentRoot(releaseB, keys);
|
||||
|
||||
var protectedPayload = hostA.Services
|
||||
.GetRequiredService<IDataProtectionProvider>()
|
||||
.CreateProtector("P")
|
||||
.Protect("payload");
|
||||
var opened = hostB.Services
|
||||
.GetRequiredService<IDataProtectionProvider>()
|
||||
.CreateProtector("P")
|
||||
.Unprotect(protectedPayload);
|
||||
|
||||
Assert.Equal("payload", opened);
|
||||
}
|
||||
finally
|
||||
{
|
||||
Directory.Delete(root, recursive: true);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Production_without_keys_path_fails_startup_validation()
|
||||
{
|
||||
var withoutKey = Assert.Throws<InvalidOperationException>(() => 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_its_value()
|
||||
{
|
||||
var canary = "canary-secret-path-4f1e";
|
||||
var refusal = Assert.Throws<InvalidOperationException>(
|
||||
() => Validate(keysPath: canary, connectionString: null));
|
||||
|
||||
Assert.DoesNotContain(canary, refusal.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Development_does_not_require_keys_path()
|
||||
{
|
||||
Validate(keysPath: null, environment: Environments.Development);
|
||||
}
|
||||
|
||||
private async Task<HttpContext> RunForwardedHeaders(string peer, string forwardedFor, string? forwardedProto)
|
||||
{
|
||||
var options = _factory.Services.GetRequiredService<IOptions<ForwardedHeadersOptions>>();
|
||||
var middleware = new ForwardedHeadersMiddleware(
|
||||
_ => Task.CompletedTask,
|
||||
NullLoggerFactory.Instance,
|
||||
options);
|
||||
var context = new DefaultHttpContext();
|
||||
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 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.ConfigureAppConfiguration((_, configuration) =>
|
||||
configuration.AddInMemoryCollection(new Dictionary<string, string?>
|
||||
{
|
||||
["DataProtection:KeysPath"] = keysPath
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
||||
private static void Validate(
|
||||
string? keysPath,
|
||||
string environment = "Production",
|
||||
string? connectionString = "Host=db")
|
||||
{
|
||||
var values = new Dictionary<string, string?>
|
||||
{
|
||||
["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!;
|
||||
}
|
||||
}
|
||||
|
|
@ -14,7 +14,7 @@ using System.Net;
|
|||
|
||||
namespace Bifrost.Web.Tests.Support;
|
||||
|
||||
public sealed class TestWebApplicationFactory : WebApplicationFactory<Program>
|
||||
public class TestWebApplicationFactory : WebApplicationFactory<Program>
|
||||
{
|
||||
private readonly InMemoryDatabaseRoot _databaseRoot = new();
|
||||
private readonly string _databaseName = $"bifrost-tests-{Guid.NewGuid():N}";
|
||||
|
|
|
|||
Loading…
Reference in a new issue