From b532a1e30e9fea9198de72fd186d59eeb7ba3afd Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:38:41 -0400 Subject: [PATCH] fix(security): trusted-proxy ForwardedHeaders so LoginThrottle keys on the real client IP (plan R2-07 T2) --- src/ZB.MOM.WW.ScadaBridge.Host/Program.cs | 12 ++++ .../ForwardedHeadersSetup.cs | 64 +++++++++++++++++++ .../ForwardedHeadersSetupTests.cs | 36 +++++++++++ 3 files changed, 112 insertions(+) create mode 100644 src/ZB.MOM.WW.ScadaBridge.Security/ForwardedHeadersSetup.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Security.Tests/ForwardedHeadersSetupTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index 69fb8252..2be86d1d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -337,6 +337,18 @@ try } // Middleware pipeline + + // Trusted-proxy forwarded headers (arch-review R2 N2): behind Traefik every client + // shares the proxy's IP, collapsing the LoginThrottle's {username}|{ip} keys onto + // one address (per-IP isolation gone + cheap username-lockout DoS). When enabled, + // X-Forwarded-For from the CONFIGURED proxies only is honored. MUST run first — + // everything downstream keys on Connection.RemoteIpAddress. + var forwardedOptions = builder.Configuration + .GetSection(ZB.MOM.WW.ScadaBridge.Security.ForwardedHeadersSecurityOptions.SectionName) + .Get() ?? new(); + if (ZB.MOM.WW.ScadaBridge.Security.ForwardedHeadersSetup.Build(forwardedOptions) is { } fho) + app.UseForwardedHeaders(fho); + app.UseWebSockets(); app.UseRouting(); app.UseAuthentication(); diff --git a/src/ZB.MOM.WW.ScadaBridge.Security/ForwardedHeadersSetup.cs b/src/ZB.MOM.WW.ScadaBridge.Security/ForwardedHeadersSetup.cs new file mode 100644 index 00000000..8e85de61 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Security/ForwardedHeadersSetup.cs @@ -0,0 +1,64 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.HttpOverrides; +using System.Net; + +namespace ZB.MOM.WW.ScadaBridge.Security; + +/// +/// Config shape for trusted-proxy forwarded-header handling +/// (ScadaBridge:Security:ForwardedHeaders). Disabled by default: a deployment +/// with no reverse proxy must NOT honor X-Forwarded-For (a client could spoof its +/// throttle key). The documented Traefik topologies enable it with the docker +/// network range so LoginThrottle keys on the REAL client IP (arch-review R2 N2). +/// +public sealed class ForwardedHeadersSecurityOptions +{ + /// Configuration section binding this options class. + public const string SectionName = "ScadaBridge:Security:ForwardedHeaders"; + + /// Whether X-Forwarded-For from trusted proxies is honored. + public bool Enabled { get; set; } + + /// Exact proxy IPs allowed to supply X-Forwarded-For. + public string[] KnownProxies { get; set; } = []; + + /// CIDR networks allowed to supply X-Forwarded-For (e.g. "172.16.0.0/12"). + public string[] KnownNetworks { get; set; } = []; +} + +/// +/// Builds the for app.UseForwardedHeaders. +/// Pure and fail-fast: malformed entries throw at startup rather than silently +/// trusting nothing/everything. Defaults (loopback) are CLEARED — only the +/// explicitly configured proxies/networks are trusted, and ForwardLimit=1 means +/// only the entry the trusted proxy itself appended is honored. +/// +public static class ForwardedHeadersSetup +{ + /// + /// Builds the forwarded-headers options, or when disabled. + /// + /// The bound . + /// The configured , or when disabled. + public static ForwardedHeadersOptions? Build(ForwardedHeadersSecurityOptions options) + { + if (!options.Enabled) return null; + var built = new ForwardedHeadersOptions + { + ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto, + ForwardLimit = 1, + }; + built.KnownProxies.Clear(); + // KnownIPNetworks (System.Net.IPNetwork) is the net10 successor to the now-deprecated + // KnownNetworks; UseForwardedHeaders reads it identically. + built.KnownIPNetworks.Clear(); + foreach (var proxy in options.KnownProxies) + built.KnownProxies.Add(IPAddress.Parse(proxy)); + foreach (var network in options.KnownNetworks) + { + var parts = network.Split('/'); + built.KnownIPNetworks.Add(new(IPAddress.Parse(parts[0]), int.Parse(parts[1]))); + } + return built; + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Security.Tests/ForwardedHeadersSetupTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Security.Tests/ForwardedHeadersSetupTests.cs new file mode 100644 index 00000000..e5107601 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Security.Tests/ForwardedHeadersSetupTests.cs @@ -0,0 +1,36 @@ +using Microsoft.AspNetCore.HttpOverrides; +using ZB.MOM.WW.ScadaBridge.Security; +using Xunit; + +namespace ZB.MOM.WW.ScadaBridge.Security.Tests; + +public class ForwardedHeadersSetupTests +{ + [Fact] + public void Build_Disabled_ReturnsNull() + => Assert.Null(ForwardedHeadersSetup.Build(new ForwardedHeadersSecurityOptions { Enabled = false })); + + [Fact] + public void Build_Enabled_ParsesProxiesAndNetworks_AndClearsDefaults() + { + var built = ForwardedHeadersSetup.Build(new ForwardedHeadersSecurityOptions + { + Enabled = true, + KnownProxies = ["10.5.0.9"], + KnownNetworks = ["172.16.0.0/12"], + })!; + Assert.Equal(ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto, built.ForwardedHeaders); + Assert.Equal(1, built.ForwardLimit); + Assert.Single(built.KnownProxies); // ONLY the configured proxy — + Assert.DoesNotContain(System.Net.IPAddress.IPv6Loopback, // the loopback default is cleared + built.KnownProxies); + Assert.Single(built.KnownIPNetworks); // net10 successor to the deprecated KnownNetworks + } + + [Theory] + [InlineData("not-an-ip", "172.16.0.0/12")] + [InlineData("10.5.0.9", "not-a-cidr")] + public void Build_MalformedEntry_ThrowsAtStartup(string proxy, string network) + => Assert.ThrowsAny(() => ForwardedHeadersSetup.Build( + new ForwardedHeadersSecurityOptions { Enabled = true, KnownProxies = [proxy], KnownNetworks = [network] })); +}