New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs for UseForwardedHeaders, working with reverse proxies and load balancers #2384

Closed
Tratcher opened this Issue Dec 13, 2016 · 90 comments

Comments

Projects
@Tratcher
Member

Tratcher commented Dec 13, 2016

@Tratcher

This comment has been minimized.

Member

Tratcher commented Dec 13, 2016

@spboyer spboyer changed the title from Docs for UseForwardedHeaders, working with reverse proxies and load ballancers to Docs for UseForwardedHeaders, working with reverse proxies and load balancers Dec 14, 2016

@Rick-Anderson

This comment has been minimized.

Contributor

Rick-Anderson commented Dec 15, 2016

@Tratcher Please clarify the ask and outline the solution.

@Tratcher

This comment has been minimized.

Member

Tratcher commented Jan 3, 2017

Most sites are hosted behind a reverse proxy, especially our recommended configurations using IIS/ANCM or NGinx. When the request is being proxied some information may be lost like the original scheme (http/https), the client IP address, etc.. You may need these values to properly generate links, evaluate polices, geolocate clients, etc.. There is a convention for the proxies to forward these values as HTTP headers (x-forwarded-*). The UseForwardedHeaders middleware reads these headers and fills in the associated fields on HttpContext.

UseForwardedHeaders has pretty complicated settings due to trust concerns with these forwarded headers (e.g. spoofing). Explain how the middleware works overall and each of the settings.

UseForwardedHeaders is enabled by default by UseIISIntegration, but with a very restricted configuration specific to ANCM.

@spboyer

This comment has been minimized.

Contributor

spboyer commented Jan 5, 2017

@Tratcher if UseIISIntegration is not enabled and/or IIS is not used (i.e. nginx, apache, etc) it is important to not only have the reverse proxy setup properly for forwarding the requests and headers, but to also add the UseForwardedHeaders middleware.

Is that what you are asking here to document?

and is more that the following needed:

            app.UseForwardedHeaders(new ForwardedHeadersOptions
            {
                ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto
            });

@Tratcher

This comment has been minimized.

Member

Tratcher commented Jan 6, 2017

@spboyer yes that's part of it. There are several other settings related to identifying/trusting the proxies. You also need to make sure your reverse proxy is adding the headers.

@Tratcher

This comment has been minimized.

Member

Tratcher commented Jan 6, 2017

@spboyer

This comment has been minimized.

Contributor

spboyer commented Jan 6, 2017

@Tratcher so are you looking for a specific document targeted at just the setting for UseForwardedHeaders?

UseForwardedHeaders has pretty complicated settings due to trust concerns with these forwarded headers (e.g. spoofing). Explain how the middleware works overall and each of the settings.

@Tratcher

This comment has been minimized.

Member

Tratcher commented Jan 6, 2017

Not quite. We need to explain the end-to-end scenarios and how the various UseForwardedHeaders settings apply (e.g. IIS+ANCM, Nginx, multiple proxies, etc.).

@mitja-p

This comment has been minimized.

mitja-p commented Mar 7, 2017

We had some problems with forwarding and lost 2 days due to this. It would be nice to mention in the documentation that only headers for KnownProxies / KnownNetworks are taken into account and that the default is only IPAddress.IPv6Loopback / IPAddress.IPLoopback. So changing forwarding limit has basically no effect without modifying KnownProxies / KnownNetworks. And the right way to setup forwarding when there is Proxy -> IIS -> ASPNET situation is:

  • increase ForwardingLimit > 1
  • depending on the type of proxy also set RequireHeaderSymmetry = false
  • add the proxy IP to KnownProxies / KnownNetworks.
@pksorensen

This comment has been minimized.

pksorensen commented Mar 13, 2017

I wanted to pitch in here as I failed to find documentation for what I am doing.

I write my nginx configuration in service fabric dynamic from the following code:

                sb.AppendLine($"{tabs}proxy_set_header Host                          $host;");
                sb.AppendLine($"{tabs}proxy_set_header X-Real-IP                   $remote_addr;");
                sb.AppendLine($"{tabs}proxy_set_header X-Forwarded-For      $proxy_add_x_forwarded_for;");
                sb.AppendLine($"{tabs}proxy_set_header X-Forwarded-Host    $host;");
                sb.AppendLine($"{tabs}proxy_set_header X-Forwarded-Server  $host;");
                sb.AppendLine($"{tabs}proxy_set_header X-Forwarded-Proto   $scheme;");
                sb.AppendLine($"{tabs}proxy_set_header X-Forwarded-Path     $request_uri;");

and have a few senaries that the UseForwardedHeaders dont cover or I am missing documentation on how to make it work.

 app.UseForwardedHeaders(new ForwardedHeadersOptions
            {
                 ForwardedHeaders = ForwardedHeaders.All
            });

basically I have to run the following also to make it work

        private const string XForwardedPathBase = "X-Forwarded-PathBase";
        private const string XForwardedProto = "X-Forwarded-Proto";
 
        app.Use((context, next) =>
            {
                if (context.Request.Headers.TryGetValue(XForwardedPathBase, out StringValues pathBase))
                {
                    context.Request.PathBase = new PathString(pathBase);
                    
                }

                if (context.Request.Headers.TryGetValue(XForwardedProto, out StringValues proto))
                {
                    context.Request.Protocol = proto;
                }

                return next();
            });

To update the protocol in those cases where nginx do the ssl offloading and the backend app just uses http. And to set the PathBase, for those cases when nginx has removed part of the path when passing the request forward.

If UseForwardedHeaders can do this already, how do i do it?

@Tratcher

This comment has been minimized.

Member

Tratcher commented Mar 13, 2017

X-forwarded-proto support is built in, but it maps to scheme, not protocol (http/1.1).

With PathBase make sure you're creating your PathString with the unescaped value.

@pksorensen

This comment has been minimized.

pksorensen commented Mar 13, 2017

thanks for the added info. i will try a few thing then to find out why my identity server generates wrong urls. https://www.earthml.com/identity/.well-known/openid-configuration (the urls it generate should also be https )

@pksorensen

This comment has been minimized.

pksorensen commented Mar 13, 2017

context.Request.Protocol = proto; was a typo

Here is my working solution, so somehow the forward middleware do not work:
image

Next step is properly to check some case sensitive stuff, i properly made a mistake there

@Tratcher

This comment has been minimized.

Member

Tratcher commented Mar 13, 2017

Can you log your headers as received by the app? The middleware is pretty strict.

@pksorensen

This comment has been minimized.

pksorensen commented Mar 13, 2017

Ye, i am off for a few hours- then i will try to log out some requests to see what actually happens. Thanks for helping

@pksorensen

This comment has been minimized.

pksorensen commented Mar 13, 2017

Here are the headers.
Will start investigate the code to see where it can go wrong.

["X-Forwarded-Server"]=["www.earthml.com"],
["X-Forwarded-Proto"]=["https"]
["X-Forwarded-Path"]=["/identity/connect/authorize/consent?client_id=EarthML.Mapify&redirect_uri=https%3A%2F%2Flocalhost%3A44377%2F&response_type=id_token%20token&scope=openid%20profile&state=621bc11cd0674059994828b401561a8c&nonce=7f2766fd2c3f4393abebf1d3c0630e93"]
["X-Forwarded-PathBase"]=["/identity/"]
@Tratcher

This comment has been minimized.

Member

Tratcher commented Mar 13, 2017

You have it set to ForwardedHeaders.All, but you don't have an x-Forwarded-For header, so it freaks out and won't process Proto either. Scope it down to just the header you need.

@Rick-Anderson

This comment has been minimized.

Contributor

Rick-Anderson commented Mar 15, 2017

@pksorensen would you be willing to draft the document for this issue?

@Rick-Anderson Rick-Anderson added the P1 label Mar 15, 2017

@pksorensen

This comment has been minimized.

pksorensen commented Mar 16, 2017

@Rick-Anderson Sure, not sure what exactly that means and what I should do :) The problem was resolved based on @Tratcher comment, about changing ForwardedHeaders.All to the headers actually used by my setup.

@Tratcher Is it ideal that it is this strict? Since the config lays within the application, meaning that if the guy responsible for the nginx setup changes the config, the the app breaks. Worth considering to use thoes headers present when set to all?

@Tratcher

This comment has been minimized.

Member

Tratcher commented Mar 16, 2017

We have an open bug for relaxing some of the defaults to make it easier to use (aspnet/BasicMiddleware#190).

However, we still need a writeup explaining the usage scenarios. See #2384 (comment)

@Tratcher

This comment has been minimized.

@V1ct0rV

This comment has been minimized.

V1ct0rV commented Sep 4, 2018

Hi @Tratcher, this is the output, I masked IP addresses and domains:

Request Method: GET
Request Scheme: https
Request Path: /headers
Request Headers:
Cache-Control: max-age=0
Connection: close
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9,es;q=0.8
Cookie: __cfduid=dfb239a4ce70a0a5bc821ada409fcb1761535638746; gsScrollPos-1878=; .AspNetCore.Antiforgery.9TtSrW0ssss=CfDJ8LPdmgbQkBpGkdbE_rXr0sqZk2-fRfbOvBKWpVqKiUlJr2hST7d7n_fPY-737AeNqCNl4vYf0FWn4TKFKxHgjFi26J2gKAuWvM54Rhm2VIwmi9bI
Host: xxx.something.com
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36
X-Request-ID: 69679e55cc863857c677f4d806d460e7
X-Real-IP: xx.xx.xx.xx
X-Forwarded-For: xx.xx.xx.xx
X-Forwarded-Host: xxx.something.com
X-Forwarded-Port: 443
X-Forwarded-Proto: https
X-Original-URI: /headers
X-Scheme: https
upgrade-insecure-requests: 1
dnt: 1

Request RemoteIp: ::ffff:456.675.0.5
@Tratcher

This comment has been minimized.

Member

Tratcher commented Sep 4, 2018

That RemoteIp is your problem, see ForwardedHeadersOptions.KnownNetworks or KnownProxies.

@OsmondJiang

This comment has been minimized.

OsmondJiang commented Sep 4, 2018

@V1ct0rV ,check my last comment, open the hidden comments in this thread and you will find your answers.

@V1ct0rV

This comment has been minimized.

V1ct0rV commented Sep 4, 2018

@OsmondJiang Thanks! I applied @nrandell Solution:

var options = new ForwardedHeadersOptions
	        {
		        ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto
	        };
	        options.KnownNetworks.Clear();
	        options.KnownProxies.Clear();
	        app.UseForwardedHeaders(options);

I spent one day trying to solve this issue, the documentation should be more clear or at least do not add default values to KnownNetworks and KnownProxies

Thanks!!!

@guardrex

This comment has been minimized.

Collaborator

guardrex commented Sep 4, 2018

@Tratcher Can you specify what, if anything, from #2384 (comment) that you'd like me to address in the topic? I'm a bit worried about clearing networks/proxies given the guidance on header spoofing.

@Tratcher

This comment has been minimized.

Member

Tratcher commented Sep 4, 2018

Yeah, clearing isn't an acceptable answer either. You need to actually supply a new value or your app is vulnerable to spoofed requests.

@nrandell

This comment has been minimized.

nrandell commented Sep 4, 2018

I think what would be really useful for people is to explain how to do this in various scenarios. So for my example where the debug message said Unknown proxy: 10.0.1.4:53690, I know I could probably get data from anything on 10.0.1.x, or possibly even 10.0.x.x.

Making it obvious how some of these scenarios work would make things a lot easier. I'm sure there are some standard scenarios, such as Azure app service running docker containers on Linux that would benefit from some of these examples. I'm pretty sure I've copied and pasted the clearing of KnownNetworks and KnownProxies a number of times for various projects I've worked on.

Is header spoofing really possible if you are sitting on an azure app service (I'm sure I will regret writing this comment …)

@Tratcher

This comment has been minimized.

Member

Tratcher commented Sep 4, 2018

For important fields like x-forwarded-XXX you always need to assume spoofing is possible and guard against it. Yes the risk varies by environment, but it never goes away.

@ygoe

This comment has been minimized.

ygoe commented Sep 4, 2018

Isn't there a design issue in "examples" for "standard scenarios"? Do you want to change your application code and rebuild when you switch the hosting environment? Say from Azure to a local server, or whatever. Do you even have the code for all web apps you run? I feel these things should be covered by the hosting environment in a way that doesn't require code changes.

@Tratcher

This comment has been minimized.

Member

Tratcher commented Sep 4, 2018

They should certainly be extracted to config to facilitate environment portability.

@V1ct0rV

This comment has been minimized.

V1ct0rV commented Sep 4, 2018

Hi @Tratcher

My nginx reverse proxy Ipaddress is: 100.116.0.5 and the local network CIDR is 100.64.0.0/10 I tried with options.KnownProxies.Add(IPAddress.Parse("100.116.0.5")); with no luck, then tried with options.KnownNetworks.Add(new IPNetwork(IPAddress.Parse("100.64.0.0"), 10)); and did not work.

I really want to have configured it in a proper way, what I'm doing wrong?

@Tratcher

This comment has been minimized.

Member

Tratcher commented Sep 4, 2018

Your debug output said the reverse proxy address was ::ffff:456.675.0.5. Note that's an IPv4 address nested in an IPv6 address. Your KnownProxies or KnownNetworks would need to be represented as IPv6 addresses as well.

@guardrex

This comment has been minimized.

Collaborator

guardrex commented Sep 4, 2018

Note that's an IPv4 address nested in an IPv6 address. Your KnownProxies or KnownNetworks would need to be represented as IPv6 addresses as well.

Now, that might be something documentable™️.

@buvinghausen

This comment has been minimized.

buvinghausen commented Sep 4, 2018

@V1ct0rV please read my post above search for ::ffff: this was explained 4 months ago..

@V1ct0rV

This comment has been minimized.

V1ct0rV commented Sep 5, 2018

Hi All,

I finally get working my app, the configuration at the end is:

services.Configure<ForwardedHeadersOptions>(options =>
	        {
		        options.ForwardedHeaders =
			        ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto;
		        options.KnownNetworks.Add(new IPNetwork(IPAddress.Parse("::ffff:100.64.0.0"), 106));
			});

I think this should be documented on the official documentation, not always we deploy the reverse proxy in the same server, about the ipv6 this should be documented too.

Thanks for you help.

@Tratcher

This comment has been minimized.

Member

Tratcher commented Sep 5, 2018

@guardrex you got this?

@guardrex

This comment has been minimized.

Collaborator

guardrex commented Sep 5, 2018

Yes, I'll open an issue.

Note that's an IPv4 address nested in an IPv6 address. Your KnownProxies or KnownNetworks would need to be represented as IPv6 addresses as well.

We're going for that ☝️ coverage.

Anything else?

@Tratcher

This comment has been minimized.

Member

Tratcher commented Sep 5, 2018

And general discussion around setting KnownNetworks/Proxies. #2384 (comment)

@mattnewell

This comment has been minimized.

mattnewell commented Sep 11, 2018

With due respect, this API is the pit of failure.

If you call UseForwardedHeaders with no arguments, it does nothing and throws no exception. See aspnet/BasicMiddleware#296

If you call UseForwardedHeaders(new ForwardedHeadersOptions { ForwardedHeaders = ForwardedHeaders.All }) it does nothing and throws no exception. See #2384 (comment)

I would suggest this needs improvement at the API level. If this isn't the appropriate repository, I'm happy to do the legwork of logging additional issues in the correct location.

@danroth27

This comment has been minimized.

Member

danroth27 commented Sep 14, 2018

@mattnewell Thanks for the feedback! The preferred repo for feedback on the UseForwardedHeaders API would be the https://github.com/aspnet/BasicMiddleware repo (looks like you've already made some comments over there).

@pksorensen

This comment has been minimized.

pksorensen commented Nov 23, 2018

This keeps hunting me.

I want to get my X-Forwarded-For to work (disabled it long time ago to get thigns working).

But when i add it things break again due to ForwardedHeaders.XForwardedHost | ForwardedHeaders.XForwardedProto not being mappend if somehting going wrong.

Reading other comments, its properly something to do with those knownproxies settings.

So is there a way to identify from my setup what the correct setttings is supposed to be?
I am running nginx as loadbalancer on service fabric. Each node will have ips 10.0.0.4-10.0.0.6 (for 3 node system).

Should i add these ips to the known proxies list?

@Tratcher

This comment has been minimized.

Member

Tratcher commented Nov 23, 2018

Comments on closed issues are not tracked, please open a new issue with the details for your scenario.

@ygoe

This comment has been minimized.

ygoe commented Nov 23, 2018

Stupid question, because I see this comment a lot: What does that mean “not tracked”? Somebody does get notifications.

@Tratcher

This comment has been minimized.

Member

Tratcher commented Nov 24, 2018

Notifications are the lowest form of tracking, we may or may not notice them in the flood. Open issues get regularly reviewed, put in milestones, assigned, etc..

@jondmcelroy

This comment has been minimized.

jondmcelroy commented Dec 11, 2018

To sum up the 90+ comments here:

// In the Configure function in Startup.cs

// Add the ForwardedHeadersOptions that you want.
// By default the options are empty, so you MUST specify what you want.
var options = new ForwardedHeadersOptions
{
    ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto
};

// Clear the forward headers networks so any ip can forward headers
// Should ONLY do this in dev/testing
// options.KnownNetworks.Clear();
// options.KnownProxies.Clear();

// For security you should limit the networks that can forward headers
// Adding a network with a mask
options.KnownNetworks.Add(new IPNetwork(IPAddress.Parse("::ffff:111.12.0.0"), 16));
// OR Adding specific ips
options.KnownProxies.Add(IPAddress.Parse("::ffff:10.0.1.2"));
options.KnownProxies.Add(IPAddress.Parse("::ffff:10.0.1.3"));

app.UseForwardedHeaders(options);
@Tratcher

This comment has been minimized.

Member

Tratcher commented Dec 11, 2018

@jondmcelroy the IPv6 restriction no longer applies in 2.2.

Clearing the networks and proxies is strongly discouraged.

@buvinghausen

This comment has been minimized.

buvinghausen commented Dec 11, 2018

@Tratcher strongly discouraged may not be sufficient.

You should never ever ever ever consider doing it ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment