Skip to content

Commit 060d446

Browse files
fix: Sentry.Tunnel overwrites the X-Forwarded-For (#4375)
Resolves #1309: - #1309
1 parent 3947066 commit 060d446

File tree

4 files changed

+91
-17
lines changed

4 files changed

+91
-17
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- Source context for class libraries when running on Android in Release mode ([#4294](https://github.com/getsentry/sentry-dotnet/pull/4294))
1212
- Native AOT: don't load SentryNative on unsupported platforms ([#4347](https://github.com/getsentry/sentry-dotnet/pull/4347))
1313
- 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))
14+
- SentryTunnelMiddleware overwrites the X-Forwarded-For header ([#4375](https://github.com/getsentry/sentry-dotnet/pull/4375))
1415

1516
### Dependencies
1617

src/Sentry.AspNetCore/SentryTunnelMiddleware.cs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Microsoft.AspNetCore.Http;
22
using Microsoft.Extensions.DependencyInjection;
3+
using Microsoft.Extensions.Primitives;
34
using Sentry.Internal.Extensions;
45

56
namespace Sentry.AspNetCore;
@@ -98,10 +99,9 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next)
9899
Method = HttpMethod.Post,
99100
Content = new StreamContent(memoryStream),
100101
};
101-
var clientIp = context.Connection?.RemoteIpAddress?.ToString();
102-
if (clientIp != null)
102+
if (CreateXForwardedForHeader(context) is { } forwardedFor)
103103
{
104-
sentryRequest.Headers.Add("X-Forwarded-For", context.Connection?.RemoteIpAddress?.ToString());
104+
sentryRequest.Headers.Add("X-Forwarded-For", forwardedFor);
105105
}
106106
var responseMessage = await client.SendAsync(sentryRequest).ConfigureAwait(false);
107107
// We send the response back to the client, whatever it was
@@ -122,6 +122,25 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next)
122122
}
123123
}
124124

125+
private static string? CreateXForwardedForHeader(HttpContext context)
126+
{
127+
var existingForwardedFor = context.Request.Headers["X-Forwarded-For"];
128+
var clientIp = context.Connection?.RemoteIpAddress?.ToString();
129+
if (clientIp is null)
130+
{
131+
return existingForwardedFor.Count > 0 ? existingForwardedFor.ToString() : null;
132+
}
133+
134+
if (existingForwardedFor.Count == 0)
135+
{
136+
return clientIp;
137+
}
138+
139+
return string.IsNullOrEmpty(existingForwardedFor)
140+
? clientIp
141+
: $"{existingForwardedFor}, {clientIp}";
142+
}
143+
125144
private bool IsHostAllowed(string host) =>
126145
host.EndsWith(".sentry.io", StringComparison.OrdinalIgnoreCase) ||
127146
host.Equals("sentry.io", StringComparison.OrdinalIgnoreCase) ||

test/Sentry.AspNetCore.Tests/Tunnel/IntegrationsTests.cs

Lines changed: 65 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
using Microsoft.AspNetCore.Builder;
12
using Microsoft.AspNetCore.Hosting;
3+
using Microsoft.AspNetCore.Http;
24
using Microsoft.AspNetCore.TestHost;
35
using Microsoft.Extensions.DependencyInjection;
46

@@ -9,6 +11,7 @@ public class IntegrationsTests : IDisposable
911
private readonly TestServer _server;
1012
private HttpClient _httpClient;
1113
private MockHttpMessageHandler _httpMessageHandler;
14+
private const string FakeRequestIp = "192.168.200.200";
1215

1316
public IntegrationsTests()
1417
{
@@ -22,7 +25,16 @@ public IntegrationsTests()
2225
factory.CreateClient(Arg.Any<string>()).Returns(_httpClient);
2326
s.AddSingleton(factory);
2427
})
25-
.Configure(app => { app.UseSentryTunneling(); });
28+
.Configure(app =>
29+
{
30+
app.Use((context, next) =>
31+
{
32+
// The context doesn't get sent by TestServer automatically... so we fake a remote request here
33+
context.Connection.RemoteIpAddress = IPAddress.Parse(FakeRequestIp);
34+
return next();
35+
});
36+
app.UseSentryTunneling();
37+
});
2638
_server = new TestServer(builder);
2739
}
2840

@@ -35,9 +47,11 @@ public async Task TunnelMiddleware_CanForwardValidEnvelope(string host)
3547
var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel")
3648
{
3749
Content = new StringContent(
38-
@"{""sent_at"":""2021-01-01T00:00:00.000Z"",""sdk"":{""name"":""sentry.javascript.browser"",""version"":""6.8.0""},""dsn"":""https://dns@" + host + @"/1""}
39-
{""type"":""session""}
40-
{""sid"":""fda00e933162466c849962eaea0cfaff""}")
50+
$$"""
51+
{"sent_at":"2021-01-01T00:00:00.000Z","sdk":{"name":"sentry.javascript.browser","version":"6.8.0"},"dsn":"https://dns@{{host}}/1"}
52+
{"type":"session"}
53+
{"sid":"fda00e933162466c849962eaea0cfaff"}
54+
""")
4155
};
4256
await _server.CreateClient().SendAsync(requestMessage);
4357

@@ -49,9 +63,12 @@ public async Task TunnelMiddleware_DoesNotForwardEnvelopeWithoutDsn()
4963
{
5064
var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel")
5165
{
52-
Content = new StringContent(@"{}
53-
{""type"":""session""}
54-
{""sid"":""fda00e933162466c849962eaea0cfaff""}")
66+
Content = new StringContent(
67+
"""
68+
{}
69+
{"type":"session"}
70+
{"sid":"fda00e933162466c849962eaea0cfaff"}
71+
""")
5572
};
5673
await _server.CreateClient().SendAsync(requestMessage);
5774

@@ -63,9 +80,11 @@ public async Task TunnelMiddleware_DoesNotForwardEnvelopeToArbitraryHost()
6380
{
6481
var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel");
6582
requestMessage.Content = new StringContent(
66-
@"{""sent_at"":""2021-01-01T00:00:00.000Z"",""sdk"":{""name"":""sentry.javascript.browser"",""version"":""6.8.0""},""dsn"":""https://[email protected]/1""}
67-
{""type"":""session""}
68-
{""sid"":""fda00e933162466c849962eaea0cfaff""}");
83+
"""
84+
{"sent_at":"2021-01-01T00:00:00.000Z","sdk":{"name":"sentry.javascript.browser","version":"6.8.0"},"dsn":"https://[email protected]/1"}
85+
{"type":"session"}
86+
{"sid":"fda00e933162466c849962eaea0cfaff"}
87+
""");
6988
await _server.CreateClient().SendAsync(requestMessage);
7089

7190
Assert.Equal(0, _httpMessageHandler.NumberOfCalls);
@@ -77,13 +96,46 @@ public async Task TunnelMiddleware_CanForwardEnvelopeToWhiteListedHost()
7796
var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel")
7897
{
7998
Content = new StringContent(
80-
@"{""sent_at"":""2021-01-01T00:00:00.000Z"",""sdk"":{""name"":""sentry.javascript.browser"",""version"":""6.8.0""},""dsn"":""https://[email protected]/1""}
81-
{""type"":""session""}
82-
{""sid"":""fda00e933162466c849962eaea0cfaff""}")
99+
"""
100+
{"sent_at":"2021-01-01T00:00:00.000Z","sdk":{"name":"sentry.javascript.browser","version":"6.8.0"},"dsn":"https://[email protected]/1"}
101+
{"type":"session"}
102+
{"sid":"fda00e933162466c849962eaea0cfaff"}
103+
""")
104+
};
105+
await _server.CreateClient().SendAsync(requestMessage);
106+
107+
Assert.Equal(1, _httpMessageHandler.NumberOfCalls);
108+
}
109+
110+
[Fact]
111+
public async Task TunnelMiddleware_XForwardedFor_RetainsOriginIp()
112+
{
113+
// Arrange: Create a request with X-Forwarded-For header
114+
var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel")
115+
{
116+
Content = new StringContent(
117+
"""
118+
{"sent_at":"2021-01-01T00:00:00.000Z","sdk":{"name":"sentry.javascript.browser","version":"6.8.0"},"dsn":"https://[email protected]/1"}
119+
{"type":"session"}
120+
{"sid":"fda00e933162466c849962eaea0cfaff"}
121+
""")
83122
};
123+
const string originalForwardedFor = "192.168.1.100, 10.0.0.1";
124+
requestMessage.Headers.Add("X-Forwarded-For", originalForwardedFor);
125+
126+
// Act
84127
await _server.CreateClient().SendAsync(requestMessage);
85128

129+
// Assert
86130
Assert.Equal(1, _httpMessageHandler.NumberOfCalls);
131+
132+
var forwardedRequest = _httpMessageHandler.LastRequest;
133+
Assert.NotNull(forwardedRequest);
134+
135+
Assert.True(forwardedRequest.Headers.Contains("X-Forwarded-For"));
136+
var forwardedForHeader = forwardedRequest.Headers.GetValues("X-Forwarded-For").FirstOrDefault();
137+
Assert.NotNull(forwardedForHeader);
138+
forwardedForHeader.Should().Be($"{originalForwardedFor}, {FakeRequestIp}");
87139
}
88140

89141
public void Dispose()

test/Sentry.AspNetCore.Tests/Tunnel/MockHttpMessageHandler.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ public class MockHttpMessageHandler : DelegatingHandler
77

88
public string Input { get; private set; }
99
public int NumberOfCalls { get; private set; }
10+
public HttpRequestMessage LastRequest { get; private set; }
1011

1112
public MockHttpMessageHandler(string response, HttpStatusCode statusCode)
1213
{
@@ -18,9 +19,10 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
1819
CancellationToken cancellationToken)
1920
{
2021
NumberOfCalls++;
22+
LastRequest = request;
2123
if (request.Content != null) // Could be a GET-request without a body
2224
{
23-
Input = await request.Content.ReadAsStringAsync();
25+
Input = await request.Content.ReadAsStringAsync(cancellationToken);
2426
}
2527
return new HttpResponseMessage
2628
{

0 commit comments

Comments
 (0)