diff --git a/src/BuildingBlocks/Web/Extensions.cs b/src/BuildingBlocks/Web/Extensions.cs index 2e4dce4459..b3c638da8b 100644 --- a/src/BuildingBlocks/Web/Extensions.cs +++ b/src/BuildingBlocks/Web/Extensions.cs @@ -77,6 +77,16 @@ public static IHostApplicationBuilder AddHeroPlatform(this IHostApplicationBuild .GetSection(nameof(TrustedProxyOptions)).Get() ?? new TrustedProxyOptions(); builder.Services.Configure(forwarded => { + // A hop count below 1 is never what an operator means, and neither bad value announces itself: + // 0 truncates the unwind loop to zero iterations, so forwarded headers stop being processed with + // no error, while a negative value overflows the middleware's buffer allocation and 500s every + // request - including requests carrying no forwarded headers at all. Fail the boot instead. + if (trustedProxy.ForwardLimit < 1) + { + throw new InvalidOperationException( + $"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.ForwardLimit)} is {trustedProxy.ForwardLimit}, which is not a valid proxy hop count: it must be at least 1 (one hop per proxy in front of the app)."); + } + forwarded.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; forwarded.ForwardLimit = trustedProxy.ForwardLimit; diff --git a/src/BuildingBlocks/Web/TrustedProxy/TrustedProxyOptions.cs b/src/BuildingBlocks/Web/TrustedProxy/TrustedProxyOptions.cs index 21d9b94e60..76ae517bf3 100644 --- a/src/BuildingBlocks/Web/TrustedProxy/TrustedProxyOptions.cs +++ b/src/BuildingBlocks/Web/TrustedProxy/TrustedProxyOptions.cs @@ -7,6 +7,13 @@ namespace FSH.Framework.Web.TrustedProxy; /// outside the proxy network cannot forge its own IP/scheme. When no proxies or networks are /// configured, the framework default (loopback only) stands and forwarded headers from any other /// source are ignored. +/// +/// Only X-Forwarded-For and X-Forwarded-Proto are honoured. X-Forwarded-Host is deliberately left +/// out: rewriting Request.Host from a header is a host-header injection primitive, and the endpoints +/// that build a public URL from the request (user registration and confirmation e-mails) would then +/// send links pointing wherever the header said. The trade-off is that Request.Host keeps the +/// internal host behind a proxy, and those links carry it. +/// /// public sealed class TrustedProxyOptions { @@ -20,6 +27,16 @@ public sealed class TrustedProxyOptions /// Number of proxy hops to unwind from X-Forwarded-For. Must match the real ingress hop count /// (cloudflared → Caddy → app is 2). The framework default of 1 reads only the rightmost hop, /// which yields the nearest proxy's IP (or an attacker-injected value) in a multi-hop topology. + /// Must be at least 1: anything lower is rejected at startup, since 0 would silently stop + /// forwarded-header processing and a negative value would fail every request. + /// + /// Setting it higher than the real hop count is what turns this into a vulnerability: the + /// middleware trusts one entry per hop, counting from the right, and only the peer itself is + /// checked against the trust list. A limit of 2 with a single proxy in front means the value the + /// proxy appended is discarded in favour of the one the client sent, so the caller picks its own + /// RemoteIpAddress and every IP-based rate limit and audit entry follows it. Count the proxies + /// that actually rewrite the header, not the ones in the diagram. + /// /// public int ForwardLimit { get; init; } = 1; } diff --git a/src/Tests/Framework.Tests/Web/TrustedProxyOptionsBindingTests.cs b/src/Tests/Framework.Tests/Web/TrustedProxyOptionsBindingTests.cs index d4204524d9..4c66fa4f11 100644 --- a/src/Tests/Framework.Tests/Web/TrustedProxyOptionsBindingTests.cs +++ b/src/Tests/Framework.Tests/Web/TrustedProxyOptionsBindingTests.cs @@ -93,5 +93,21 @@ public void ForwardedHeaders_Should_NameTheSetting_When_KnownNetworkMalformed() exception.Message.ShouldContain("10.0.0.0/999"); } + [Theory] + [InlineData("-1")] // negative — overflows the middleware's buffer allocation, 500s every request + [InlineData("0")] // zero — truncates the unwind loop, forwarded headers silently stop being read + public void ForwardedHeaders_Should_NameTheSetting_When_ForwardLimitBelowOne(string forwardLimit) + { + // Act - no proxies or networks configured, so this has to be rejected before the trust-boundary block. + var exception = Should.Throw(() => Resolve(new Dictionary + { + [$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.ForwardLimit)}"] = forwardLimit, + })); + + // Assert + exception.Message.ShouldContain("TrustedProxyOptions:ForwardLimit"); + exception.Message.ShouldContain($"is {forwardLimit}"); + } + #endregion }