Sentry SDK is overwriting the User.IpAddress on ASP.NET

Package

Sentry

.NET Flavor

.NET

.NET Version

4.7.2

OS

Windows

SDK Version

4.4.0

Self-Hosted Sentry Version

No response

Steps to Reproduce

Set SendDefaultPii to true during SentrySdk.Init call
Create new SentryEvent - and set the event's User.IpAddress to a specific value.
Call SentrySdk.CaptureEvent and pass in the event.

Expected Result

The Sentry dashboard should show the User's IP as the one we set.

Actual Result

It appears to always contain an internal/server IP instead. I believe this may be because of this code?

sentry-dotnet/src/Sentry.AspNet/Internal/SystemWebRequestEventProcessor.cs

Line 75 in 5bcb9ca

@event.User.IpAddress = context.Request.UserHostAddress;

But I am honestly not certain.

If I am doing something wrong here, would love guidance. But we have our own logic that fires off before a Sentry event sends - and if it's inside a web requests - gets the current user context (who's logged in) and IP address data, then stamps it onto the SentryEvent. We get the IP address ourselves because we're parsing CloudFlare/proxy IPs in the HTTP headers.

@nicholashead

Hi @nicholashead , thanks for raising this.

SystemWebRequestEventProcessor

At first glance, I think you're right about the SystemWebRequestEventProcessor.

I did notice we have this test which is supposed to avoid the situation you're describing (you set the User.IpAddress explicitly and then our processor overwrites it). That functionality is in the MainSentryEventProcessor (ultimately the Enricher):

sentry-dotnet/src/Sentry/Internal/MainSentryEventProcessor.cs

Line 157 in 777d664

_enricher.Apply(@event);

sentry-dotnet/src/Sentry/Internal/Enricher.cs

Line 83 in bd62cf0

eventLike.User.IpAddress ??= DefaultIpAddress;

That event processor gets set on the SentryClient so I believe it runs for both ASP.NET Core and ASP.NET. I think it's getting overridden in ASP.NET by the code you highlighted in SystemWebRequestEventProcessor

REMOTE_ADDR

@nicholashead this won't affect you as you're setting the IP Address explicitly yourselves, but for ASP.NET Core Sentry also has these (which ensure Automatic IP address resolution works):

sentry-dotnet/src/Sentry.AspNetCore/ScopeExtensions.cs

Line 124 in 749c9a7

var host = context.Request.Host.Host;

sentry-dotnet/src/Sentry.AspNetCore/ScopeExtensions.cs

Lines 152 to 157 in 749c9a7

    
           // these might be reported in a non CGI, old-school way 
        
           if (options.SendDefaultPii 
        
               && context.Connection.RemoteIpAddress?.ToString() is { } ipAddress) 
        
           { 
        
               scope.Request.Env["REMOTE_ADDR"] = ipAddress; 
        
           }

When building classic ASP.NET (framework) applications none of the mechanics to populate information in the scope seems to exist. So we probably need to wire up Scope.Request.Env["REMOTE_ADDR"] in ASP.NET too, for the Auto IP Address resolution to work.

@bruno-garcia / @bitsandfoxes is there any additional context for whether/how this should work on ASP.NET? I can't see any equivalent in ASP.NET of the ASP.NET Core scope extensions, which populate this information. It's slightly more work to obtain this information in ASP.NET as we'd need to read it from headers ourselves but I think we could at the very least check for an X-Forwarded-For.

Thanks for the research! Really appreciate it. The only reason we’re having issues with this is because of the proxied traffic, so if you were able to improve the default logic to look at the forwarded for header that’d be great. We’re still stuck on older .NET framework for one of our apps for now…

There is something off with either the order of processor execution here or the processor is not supposed to simply overwrite data that has already been set.

When building classic ASP.NET (framework) applications none of the mechanics to populate information in the scope seems to exist.

Wdym? Doesn't the application of the scope be taken care of by the client? As part of the regular event processing in the core package?

@jamescrosswell

@jamescrosswell, since you already looked into this, do you have an idea of how much effort it'll be to add support to our ASP.NET package?

@jamescrosswell

@jamescrosswell, since you already looked into this, do you have an idea of how much effort it'll be to add support to our ASP.NET package?

I think we can fix the original problem that was reported fairly quickly (1 day maybe???).

Creating some equivalent of the ASP.NET Core Scope extensions for ASP.NET feels more like a week.

Both wild guesstimates though.

Navigation Menu

Repository navigation

Sentry SDK is overwriting the User.IpAddress on ASP.NET #3337

Package

.NET Flavor

.NET Version

OS

SDK Version

Self-Hosted Sentry Version

Steps to Reproduce

Expected Result

Actual Result

SystemWebRequestEventProcessor

REMOTE_ADDR

3 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	// these might be reported in a non CGI, old-school way
	if (options.SendDefaultPii
	&& context.Connection.RemoteIpAddress?.ToString() is { } ipAddress)
	{
	scope.Request.Env["REMOTE_ADDR"] = ipAddress;
	}

Sentry SDK is overwriting the User.IpAddress on ASP.NET #3337

Description

Package

.NET Flavor

.NET Version

OS

SDK Version

Self-Hosted Sentry Version

Steps to Reproduce

Expected Result

Actual Result

Activity

jamescrosswell commented on Apr 30, 2024

SystemWebRequestEventProcessor

REMOTE_ADDR

nicholashead commented on Apr 30, 2024

bitsandfoxes commented on May 8, 2024

3 remaining items

bitsandfoxes commented on Jun 28, 2024

jamescrosswell commented on Jul 4, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions