From 2c429bbe190b163398f196655f627296d38e7f1a Mon Sep 17 00:00:00 2001 From: Brennan <brecon@microsoft.com> Date: Tue, 22 Apr 2025 15:43:34 -0700 Subject: [PATCH] [release/2.3] Forwarded Headers Middleware: Ignore XForwardedHeaders from Unknown Proxy --- eng/PatchConfig.props | 5 ++ .../src/ForwardedHeadersMiddleware.cs | 17 +++--- .../test/ForwardedHeadersMiddlewareTest.cs | 57 +++++++++++++++++++ 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index 2d90909c1984..c335258d253d 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -22,4 +22,9 @@ Later on, this will be checked using this condition: <PackagesInPatch> </PackagesInPatch> </PropertyGroup> + <PropertyGroup Condition=" '$(VersionPrefix)' == '2.3.3' "> + <PackagesInPatch> + Microsoft.AspNetCore.HttpOverrides; + </PackagesInPatch> + </PropertyGroup> </Project> diff --git a/src/Middleware/HttpOverrides/src/ForwardedHeadersMiddleware.cs b/src/Middleware/HttpOverrides/src/ForwardedHeadersMiddleware.cs index decda81d58ec..88317c455109 100644 --- a/src/Middleware/HttpOverrides/src/ForwardedHeadersMiddleware.cs +++ b/src/Middleware/HttpOverrides/src/ForwardedHeadersMiddleware.cs @@ -227,16 +227,17 @@ public void ApplyForwarders(HttpContext context) for ( ; entriesConsumed < sets.Length; entriesConsumed++) { var set = sets[entriesConsumed]; - if (checkFor) + + // For the first instance, allow remoteIp to be null for servers that don't support it natively. + if (currentValues.RemoteIpAndPort != null && checkKnownIps && !CheckKnownAddress(currentValues.RemoteIpAndPort.Address)) { - // For the first instance, allow remoteIp to be null for servers that don't support it natively. - if (currentValues.RemoteIpAndPort != null && checkKnownIps && !CheckKnownAddress(currentValues.RemoteIpAndPort.Address)) - { - // Stop at the first unknown remote IP, but still apply changes processed so far. - _logger.LogDebug(1, $"Unknown proxy: {currentValues.RemoteIpAndPort}"); - break; - } + // Stop at the first unknown remote IP, but still apply changes processed so far. + _logger.LogDebug(1, $"Unknown proxy: {currentValues.RemoteIpAndPort}"); + break; + } + if (checkFor) + { IPEndPoint parsedEndPoint; if (IPEndPointParser.TryParse(set.IpAndPortText, out parsedEndPoint)) { diff --git a/src/Middleware/HttpOverrides/test/ForwardedHeadersMiddlewareTest.cs b/src/Middleware/HttpOverrides/test/ForwardedHeadersMiddlewareTest.cs index f88243c9e77f..8140e6ba5895 100644 --- a/src/Middleware/HttpOverrides/test/ForwardedHeadersMiddlewareTest.cs +++ b/src/Middleware/HttpOverrides/test/ForwardedHeadersMiddlewareTest.cs @@ -799,6 +799,63 @@ public async Task AllOptionsDisabledRequestDoesntChange() Assert.Equal("http", context.Request.Scheme); } + [Theory] + [InlineData(ForwardedHeaders.XForwardedFor, false)] + [InlineData(ForwardedHeaders.XForwardedFor, true)] + [InlineData(ForwardedHeaders.XForwardedHost, false)] + [InlineData(ForwardedHeaders.XForwardedHost, true)] + [InlineData(ForwardedHeaders.XForwardedProto, false)] + [InlineData(ForwardedHeaders.XForwardedProto, true)] + public async Task IgnoreXForwardedHeadersFromUnknownProxy(ForwardedHeaders forwardedHeaders, bool unknownProxy) + { + var builder = new WebHostBuilder() + .Configure(app => + { + var options = new ForwardedHeadersOptions + { + ForwardedHeaders = forwardedHeaders + }; + if (!unknownProxy) + { + var proxy = IPAddress.Parse("10.0.0.1"); + options.KnownProxies.Add(proxy); + } + app.UseForwardedHeaders(options); + }); + var server = new TestServer(builder); + + var context = await server.SendAsync(c => + { + c.Request.Headers["X-Forwarded-For"] = "11.111.111.11"; + c.Request.Headers["X-Forwarded-Host"] = "testhost"; + c.Request.Headers["X-Forwarded-Proto"] = "Protocol"; + c.Connection.RemoteIpAddress = IPAddress.Parse("10.0.0.1"); + c.Connection.RemotePort = 99; + }); + + if (unknownProxy) + { + Assert.Equal("10.0.0.1", context.Connection.RemoteIpAddress.ToString()); + Assert.Equal("localhost", context.Request.Host.ToString()); + Assert.Equal("http", context.Request.Scheme); + } + else + { + if (forwardedHeaders.HasFlag(ForwardedHeaders.XForwardedFor)) + { + Assert.Equal("11.111.111.11", context.Connection.RemoteIpAddress.ToString()); + } + if (forwardedHeaders.HasFlag(ForwardedHeaders.XForwardedHost)) + { + Assert.Equal("testhost", context.Request.Host.ToString()); + } + if (forwardedHeaders.HasFlag(ForwardedHeaders.XForwardedProto)) + { + Assert.Equal("Protocol", context.Request.Scheme); + } + } + } + [Fact] public async Task PartiallyEnabledForwardsPartiallyChangesRequest() {