HttpPlatformHandler/Kestrel: HttpContext.Connection.RemoteIpAddress not working

[guardrex]

Following: #16

I'm using Google RECAPTCHA on this contact form, and I think I used to have ...

string gIp = Context.Connection.RemoteIpAddress.ToString();

... in the beta7 version of this app. For the beta8 upgrade, that Context went away, but it seemed cool with HttpContext ...

string gIp = HttpContext.Connection.RemoteIpAddress.ToString();

... but that's throwing a null reference exception. Is this a "Luke not handling the contexts correctly" problem or a "HttpPlatformHandler/Kestrel" problem?

[Tratcher]

app.UseIISPlatformHandler(); should set RemoteIpAddress.

[guardrex]

I have the app.UseIISPlatformHandler(); in the app with the package in place. Is there anything else I rundown on this for you?

[Tratcher]

You could show your startup class.

[guardrex]

namespace MyApp
{
    public class Startup
    {
        public IConfiguration Configuration { get; set; }

        public Startup(IHostingEnvironment env)
        {
            var configurationBuilder = new ConfigurationBuilder().AddEnvironmentVariables();
            Configuration = configurationBuilder.Build();
            string machineName = Environment.GetEnvironmentVariable("COMPUTERNAME");
            if(machineName.StartsWith("~~~~~~", StringComparison.OrdinalIgnoreCase))
            {
                Configuration["ASPNET_ENV"] = "Production";
                ... other config stuff here ...
            }
            else if (machineName.StartsWith("~~~~~~", StringComparison.OrdinalIgnoreCase))
            {
                Configuration["ASPNET_ENV"] = "Production";
                ... other config stuff here ...
            }
            else
            {
                Configuration["ASPNET_ENV"] = "Development";
            }
        }

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddAntiforgery();
            services.AddSingleton(_ => Configuration);
            services.AddMvc(options => 
            {
                string machineName = Environment.GetEnvironmentVariable("COMPUTERNAME");
                if (machineName.StartsWith("~~~~~~", StringComparison.OrdinalIgnoreCase))
                {
                    options.CacheProfiles.Add("PublicCache", new CacheProfile()
                    {
                        Duration = 2592000,
                        Location = ResponseCacheLocation.Any
                    });
                }
                else
                {
                    options.CacheProfiles.Add("PublicCache", new CacheProfile()
                    {
                        Duration = 0,
                        Location = ResponseCacheLocation.None
                    });
                }
            });
        }

        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            app.UseIISPlatformHandler();
            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
                app.UseBrowserLink();
            }
            else
            {
                app.UseExceptionHandler("/error");
            }
            app.UseStaticFiles();
            app.Use(next => 
            {
                return ctx => 
                {
                    ctx.Response.Headers.Remove("Server");
                    return next(ctx);
                };
            });
            app.UseMvcWithDefaultRoute();
        }
    }
}

[Tratcher]

Looks fine. I recommend setting a break point here: ctx.Response.Headers.Remove("Server"); where it should be easy to verify the state of your request. E.g. dump the request headers and Connection properties to see what's available.

[guardrex]

Here's what I see stopping at that line and inspecting headers and connection:

Cache-Control:max-age=0
Connection:Keep-Alive Accept:text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
Accept-Encoding:gzip, deflate, sdch
Accept-Language:en-US,en;q=0.8
Cookie:_ga=GA1.2.162218639.1444775105; i4hHD23M7PY=CfDJ8OtjC ... 4TxFw
Host:www.XXXXXXX.com:8001
User-Agent:Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.71 Safari/537.36
Upgrade-Insecure-Requests:1
X-Forwarded-For:XXX.XXX.XXX.XXX:60982
X-Forwarded-Proto:http
X-Original-Proto:http

... but both ctx.Connection.LocalIpAddress and ctx.Connection.RemoteIpAddress were null.

On IIS Express, ctx.Connection.RemoteIpAddress correctly shows {::1}.

[Tratcher]

Ah, IPAddress.TryParse is probably choking on the port. https://github.com/aspnet/IISIntegration/blob/dev/src/Microsoft.AspNet.IISPlatformHandler/IISPlatformHandlerMiddleware.cs#L80-L96

It should probably split out the port and set it to Connection.RemotePort.

[guardrex]

Cool. I'll just drop the RECAPTCHA stuff for now and watch for an update here.

[gamoody]

Is the fix for this likely to make it into RC1? We have a site which we would like to take live when RC1 is released but it uses this for geo-location purposes.

[Tratcher]

No, this didn't make it into RC1. However it should be pretty easy to work around. Here's the existing code: https://github.com/aspnet/IISIntegration/blob/dev/src/Microsoft.AspNet.IISPlatformHandler/IISPlatformHandlerMiddleware.cs#L80-L96
The fix should only be a few lines. If you get to it before we do feel free to send a PR.

[guardrex]

@Tratcher Until the team gets to it (unless, as you say, someone jumps in with a PR), isn't the best practice when only having the headers to look for HTTP_X_FORWARDED_FOR. If that isn't there, then just take whatever REMOTE_ADDR has?

[EDIT] Nevermind ... I just looked at that code block, and I see what you mean about the port. No worries.

[guardrex]

@Tratcher btw - Maybe this changed when the move to Kestrel took place, but I'm not seeing ::1 running locally. Is that normal/expected?

[EDIT] ... and HttpContext.Connection.IsLocal is coming back false locally.

[guardrex]

[REMOVED]

[Tratcher]

IPv6? [::1]:2345
Also note that the header HttpPlatformHandler sets is X-Forwarded-For.

[guardrex]

@Tratcher Ah, yes, thanks for mentioning the X-Forwarded-For.

I didn't check the spec yet on Request.Headers[] and contend with casing. I'll make a mod to it if needed later tonight. I'm just trying to hack-up some crap to make things work ok for testing.

I wasn't getting any IP header in Fiddler for local requests. Maybe Fiddler is the wrong way to look, but I thought when I did this earlier the ::1 was showing in Fiddler when running locally (IISExpress, beta8, IPv4).

[guardrex]

@Tratcher So casing doesn't matter ... null header does ... then there's a mess of non-standard ones. I didn't look into great detail or know if I have them all. I saw https://tools.ietf.org/html/rfc7239 and a few SO posts. I think this hacks it pretty good ... until you cats do a real number on it.

NOTE TO ALL: I didn't fully test this ... use at your own risk!

private string GetRemoteIP()
{
    string[] remoteIpHeaders = 
    {
        "X-FORWARDED-FOR",
        "REMOTE_ADDR",
        "HTTP_X_FORWARDED_FOR",
        "HTTP_CLIENT_IP",
        "HTTP_X_FORWARDED",
        "HTTP_X_CLUSTER_CLIENT_IP",
        "HTTP_FORWARDED_FOR",
        "HTTP_FORWARDED",
        "X_FORWARDED_FOR",
        "CLIENT_IP",
        "X_FORWARDED",
        "X_CLUSTER_CLIENT_IP",
        "FORWARDED_FOR",
        "FORWARDED"
    };
    string value;
    foreach (string remoteIpHeader in remoteIpHeaders)
    {
        if (Request.Headers[remoteIpHeader] != null)
        {
            value = Request.Headers[remoteIpHeader];
            if (!string.IsNullOrEmpty(value))
            {
                value = value.Split(',')[0].Split(';')[0];
                if (value.Contains("="))
                {
                    value = value.Split('=')[1];
                }
                value = value.Trim('"');
                if (value.Contains(":"))
                {
                    value = value.Substring(0, value.LastIndexOf(':'));
                }
                return value.TrimStart('[').TrimEnd(']');
            }
            else
            {
                break;
            }
        }
    }
    return "Whatever you want ... no header found or empty value found";
}

[JonghoL]

tiny fix

private static void UpdateRemoteIp(HttpContext httpContext)
        {
            var xForwardedForHeaderValue = httpContext.Request.Headers.GetCommaSeparatedValues(XForwardedForHeaderName);
            if (xForwardedForHeaderValue != null && xForwardedForHeaderValue.Length > 0)
            {
                IPAddress ipFromHeader;
                var portSeparateLength = xForwardedForHeaderValue[0].LastIndexOf(':');
                var ipAddr = xForwardedForHeaderValue[0].Substring(0, portSeparateLength);
                if (IPAddress.TryParse(ipAddr, out ipFromHeader))
                {
                    var remoteIPString = httpContext.Connection.RemoteIpAddress?.ToString();
                    if (!string.IsNullOrEmpty(remoteIPString))
                    {
                        httpContext.Request.Headers[XOriginalIPName] = remoteIPString;
                    }
                    httpContext.Connection.RemoteIpAddress = ipFromHeader;
                    int remotePortFromHeader;
                    if (int.TryParse(xForwardedForHeaderValue[0].Substring(portSeparateLength + 1, xForwardedForHeaderValue[0].Length - portSeparateLength - 1), out remotePortFromHeader))
                    {
                        httpContext.Connection.RemotePort = remotePortFromHeader;
                    }
                }
            }
        }

[guardrex]

@JonghoL Did you test? Does it work for IPv6 and local (::1)? This one is on SO ...

var arr = clientId.Split(':');
clientId = arr.Length <= 2 ? arr[0] : string.Join(":", arr.Take(8).ToArray());
if (IPAddress.TryParse(clientId, out clientIpAddr))

http://stackoverflow.com/questions/22596037/ipaddress-tryparse-returns-false/22596060#22596060

[muratg]

@JonghoL do you plan to send a PR? Otherwise @pakrym should be able to look into this.

[guardrex]

Is X-Forwarded-For going to become Forwarded now because that's the RFC? It's probably more of an HttpPlatformHandler team question ... just curious.

[Tratcher]

@guardrex, interesting, we'll take a look.

[aholmes]

This appears to still be an issue. I have an empty ASP.NET project for which I started debugging by clicking "IIS Express" in Visual Studio. When accessing the local URL in Chrome, context.Connection.RemoteIpAddress is set correctly to ::1. When I test with curl (curl -i http://localhost:50588), the address is null.

With Chrome, the X-Forwarded-For header is set to [::1]:51760. With Curl, it is set to 127.0.0.1:51861.

I'm not certain what information may be relevant to this, so I've attached two screenshots of the request headers from Chrome and Curl.

One other thing that may or may not be relevant; when making the request with Chrome, I noticed that my Run lambda executes twice. Here's my code.

    public void Configure(IApplicationBuilder app)
    {
        app.UseIISPlatformHandler();

        app.Run(async (context) =>
        {
            var ipAddress = context.Connection.RemoteIpAddress.IsIPv4MappedToIPv6
                ? context.Connection.RemoteIpAddress.MapToIPv4().ToString()
                : context.Connection.RemoteIpAddress.ToString();

            await context.Response.WriteAsync(ipAddress);
        });
    }

Chrome:

Curl:

Edit:

When selecting "web" for debugging and executing DNX, the address is null for both Chrome and Curl.

[Tratcher]

RC1 or RC2? In RC2 the IP forwarders logic has been moved from UseIISPlatformHandler to UseForwardedHeaders.
https://github.com/aspnet/BasicMiddleware/blob/dev/samples/HttpOverridesSample/Startup.cs#L13

[aholmes]

I see ... is this possibly an issue with the ASP.NET template for Visual Studio? The code linked is for AspNetCore, which is not present in my project.

[Tratcher]

VS contains RC1 templates and you should use RC1 packages with them. Converting to RC2 is more than just changing the package references. aspnet/Announcements#144

[aholmes]

Thanks, I appreciate the explanation.

[sharabiania]

@Tratcher The link provided for the work around is broken:

https://github.com/aspnet/IISIntegration/blob/dev/src/Microsoft.AspNet.IISPlatformHandler/IISPlatformHandlerMiddleware.cs#L80-L96

Page not found

[Tratcher]

@guardrex That's way overkill

[guardrex]

@Tratcher I know ... HPH (under RC1?) doesn't forward them anyway, right? The list was from a survey of possible IP headers from various platforms. I'll delete the comment.

[Tratcher]

HPH 1.2 with RC1 forwards x-forwarded-for and x-forwarded-proto. There was a bug that we couldn't parses the x-forwarded-for if it had a port, which is most of what your workaround code was for.