diff --git a/CHANGELOG.md b/CHANGELOG.md index 87521074ab..c82e6a8cb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Native AOT: don't load SentryNative on unsupported platforms ([#4347](https://github.com/getsentry/sentry-dotnet/pull/4347)) - Fixed issue introduced in release 5.12.0 that might prevent other middleware or user code from reading request bodies ([#4373](https://github.com/getsentry/sentry-dotnet/pull/4373)) +- SentryTunnelMiddleware overwrites the X-Forwarded-For header ([#4375](https://github.com/getsentry/sentry-dotnet/pull/4375)) ### Dependencies diff --git a/src/Sentry.AspNetCore/SentryTunnelMiddleware.cs b/src/Sentry.AspNetCore/SentryTunnelMiddleware.cs index 2278f61a01..f65998d030 100644 --- a/src/Sentry.AspNetCore/SentryTunnelMiddleware.cs +++ b/src/Sentry.AspNetCore/SentryTunnelMiddleware.cs @@ -1,5 +1,6 @@ using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Primitives; using Sentry.Internal.Extensions; namespace Sentry.AspNetCore; @@ -98,10 +99,9 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) Method = HttpMethod.Post, Content = new StreamContent(memoryStream), }; - var clientIp = context.Connection?.RemoteIpAddress?.ToString(); - if (clientIp != null) + if (CreateXForwardedForHeader(context) is { } forwardedFor) { - sentryRequest.Headers.Add("X-Forwarded-For", context.Connection?.RemoteIpAddress?.ToString()); + sentryRequest.Headers.Add("X-Forwarded-For", forwardedFor); } var responseMessage = await client.SendAsync(sentryRequest).ConfigureAwait(false); // We send the response back to the client, whatever it was @@ -122,6 +122,25 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) } } + private static string? CreateXForwardedForHeader(HttpContext context) + { + var existingForwardedFor = context.Request.Headers["X-Forwarded-For"]; + var clientIp = context.Connection?.RemoteIpAddress?.ToString(); + if (clientIp is null) + { + return existingForwardedFor.Count > 0 ? existingForwardedFor.ToString() : null; + } + + if (existingForwardedFor.Count == 0) + { + return clientIp; + } + + return string.IsNullOrEmpty(existingForwardedFor) + ? clientIp + : $"{existingForwardedFor}, {clientIp}"; + } + private bool IsHostAllowed(string host) => host.EndsWith(".sentry.io", StringComparison.OrdinalIgnoreCase) || host.Equals("sentry.io", StringComparison.OrdinalIgnoreCase) || diff --git a/test/Sentry.AspNetCore.Tests/Tunnel/IntegrationsTests.cs b/test/Sentry.AspNetCore.Tests/Tunnel/IntegrationsTests.cs index 797b40b922..6b8741e186 100644 --- a/test/Sentry.AspNetCore.Tests/Tunnel/IntegrationsTests.cs +++ b/test/Sentry.AspNetCore.Tests/Tunnel/IntegrationsTests.cs @@ -1,4 +1,6 @@ +using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; @@ -9,6 +11,7 @@ public class IntegrationsTests : IDisposable private readonly TestServer _server; private HttpClient _httpClient; private MockHttpMessageHandler _httpMessageHandler; + private const string FakeRequestIp = "192.168.200.200"; public IntegrationsTests() { @@ -22,7 +25,16 @@ public IntegrationsTests() factory.CreateClient(Arg.Any()).Returns(_httpClient); s.AddSingleton(factory); }) - .Configure(app => { app.UseSentryTunneling(); }); + .Configure(app => + { + app.Use((context, next) => + { + // The context doesn't get sent by TestServer automatically... so we fake a remote request here + context.Connection.RemoteIpAddress = IPAddress.Parse(FakeRequestIp); + return next(); + }); + app.UseSentryTunneling(); + }); _server = new TestServer(builder); } @@ -35,9 +47,11 @@ public async Task TunnelMiddleware_CanForwardValidEnvelope(string host) var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel") { Content = new StringContent( - @"{""sent_at"":""2021-01-01T00:00:00.000Z"",""sdk"":{""name"":""sentry.javascript.browser"",""version"":""6.8.0""},""dsn"":""https://dns@" + host + @"/1""} -{""type"":""session""} -{""sid"":""fda00e933162466c849962eaea0cfaff""}") + $$""" + {"sent_at":"2021-01-01T00:00:00.000Z","sdk":{"name":"sentry.javascript.browser","version":"6.8.0"},"dsn":"https://dns@{{host}}/1"} + {"type":"session"} + {"sid":"fda00e933162466c849962eaea0cfaff"} + """) }; await _server.CreateClient().SendAsync(requestMessage); @@ -49,9 +63,12 @@ public async Task TunnelMiddleware_DoesNotForwardEnvelopeWithoutDsn() { var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel") { - Content = new StringContent(@"{} -{""type"":""session""} -{""sid"":""fda00e933162466c849962eaea0cfaff""}") + Content = new StringContent( + """ + {} + {"type":"session"} + {"sid":"fda00e933162466c849962eaea0cfaff"} + """) }; await _server.CreateClient().SendAsync(requestMessage); @@ -63,9 +80,11 @@ public async Task TunnelMiddleware_DoesNotForwardEnvelopeToArbitraryHost() { var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel"); requestMessage.Content = new StringContent( - @"{""sent_at"":""2021-01-01T00:00:00.000Z"",""sdk"":{""name"":""sentry.javascript.browser"",""version"":""6.8.0""},""dsn"":""https://dns@evil.com/1""} -{""type"":""session""} -{""sid"":""fda00e933162466c849962eaea0cfaff""}"); + """ + {"sent_at":"2021-01-01T00:00:00.000Z","sdk":{"name":"sentry.javascript.browser","version":"6.8.0"},"dsn":"https://dns@evil.com/1"} + {"type":"session"} + {"sid":"fda00e933162466c849962eaea0cfaff"} + """); await _server.CreateClient().SendAsync(requestMessage); Assert.Equal(0, _httpMessageHandler.NumberOfCalls); @@ -77,13 +96,46 @@ public async Task TunnelMiddleware_CanForwardEnvelopeToWhiteListedHost() var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel") { Content = new StringContent( - @"{""sent_at"":""2021-01-01T00:00:00.000Z"",""sdk"":{""name"":""sentry.javascript.browser"",""version"":""6.8.0""},""dsn"":""https://dns@sentry.mywebsite.com/1""} -{""type"":""session""} -{""sid"":""fda00e933162466c849962eaea0cfaff""}") + """ + {"sent_at":"2021-01-01T00:00:00.000Z","sdk":{"name":"sentry.javascript.browser","version":"6.8.0"},"dsn":"https://dns@sentry.mywebsite.com/1"} + {"type":"session"} + {"sid":"fda00e933162466c849962eaea0cfaff"} + """) + }; + await _server.CreateClient().SendAsync(requestMessage); + + Assert.Equal(1, _httpMessageHandler.NumberOfCalls); + } + + [Fact] + public async Task TunnelMiddleware_XForwardedFor_RetainsOriginIp() + { + // Arrange: Create a request with X-Forwarded-For header + var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel") + { + Content = new StringContent( + """ + {"sent_at":"2021-01-01T00:00:00.000Z","sdk":{"name":"sentry.javascript.browser","version":"6.8.0"},"dsn":"https://dns@sentry.io/1"} + {"type":"session"} + {"sid":"fda00e933162466c849962eaea0cfaff"} + """) }; + const string originalForwardedFor = "192.168.1.100, 10.0.0.1"; + requestMessage.Headers.Add("X-Forwarded-For", originalForwardedFor); + + // Act await _server.CreateClient().SendAsync(requestMessage); + // Assert Assert.Equal(1, _httpMessageHandler.NumberOfCalls); + + var forwardedRequest = _httpMessageHandler.LastRequest; + Assert.NotNull(forwardedRequest); + + Assert.True(forwardedRequest.Headers.Contains("X-Forwarded-For")); + var forwardedForHeader = forwardedRequest.Headers.GetValues("X-Forwarded-For").FirstOrDefault(); + Assert.NotNull(forwardedForHeader); + forwardedForHeader.Should().Be($"{originalForwardedFor}, {FakeRequestIp}"); } public void Dispose() diff --git a/test/Sentry.AspNetCore.Tests/Tunnel/MockHttpMessageHandler.cs b/test/Sentry.AspNetCore.Tests/Tunnel/MockHttpMessageHandler.cs index b55fd29498..2a81b29a56 100644 --- a/test/Sentry.AspNetCore.Tests/Tunnel/MockHttpMessageHandler.cs +++ b/test/Sentry.AspNetCore.Tests/Tunnel/MockHttpMessageHandler.cs @@ -7,6 +7,7 @@ public class MockHttpMessageHandler : DelegatingHandler public string Input { get; private set; } public int NumberOfCalls { get; private set; } + public HttpRequestMessage LastRequest { get; private set; } public MockHttpMessageHandler(string response, HttpStatusCode statusCode) { @@ -18,9 +19,10 @@ protected override async Task SendAsync(HttpRequestMessage CancellationToken cancellationToken) { NumberOfCalls++; + LastRequest = request; if (request.Content != null) // Could be a GET-request without a body { - Input = await request.Content.ReadAsStringAsync(); + Input = await request.Content.ReadAsStringAsync(cancellationToken); } return new HttpResponseMessage {