Skip to content
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 · 92 comments
Closed
Assignees
Labels

Comments

@Tratcher
Copy link
Member

Tratcher commented Dec 13, 2016

https://docs.microsoft.com/en-us/aspnet/core/publishing/linuxproduction
aspnet/Security#1070
aspnet/Security#929
aspnet/Security#757
aspnet/Security#853
aspnet/IISIntegration#140
aspnet/Security#1620
aspnet/Security#1702
Enabled by default by UseIISIntegration, but with limited settings (ANCM reverse proxy on loopback)

@Tratcher
Copy link
Member Author

@spboyer spboyer changed the title Docs for UseForwardedHeaders, working with reverse proxies and load ballancers Docs for UseForwardedHeaders, working with reverse proxies and load balancers Dec 14, 2016
@Rick-Anderson
Copy link
Contributor

@Tratcher Please clarify the ask and outline the solution.

@Tratcher
Copy link
Member Author

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
Copy link
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
Copy link
Member Author

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
Copy link
Member Author

Tratcher commented Jan 6, 2017

https://docs.microsoft.com/en-us/aspnet/core/publishing/linuxproduction captures the nginx specific scenario.

@spboyer
Copy link
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
Copy link
Member Author

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
Copy link

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
Copy link

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
Copy link
Member Author

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
Copy link

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
Copy link

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
Copy link
Member Author

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

@pksorensen
Copy link

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
Copy link

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
Copy link
Member Author

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
Copy link
Contributor

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

@pksorensen
Copy link

@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
Copy link
Member Author

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)

@v1ct0rv
Copy link

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
Copy link
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
Copy link
Member Author

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
Copy link

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
Copy link
Member Author

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
Copy link

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
Copy link
Member Author

Tratcher commented Sep 4, 2018

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

@v1ct0rv
Copy link

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
Copy link
Member Author

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
Copy link
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:tm:.

@buvinghausen
Copy link

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

@v1ct0rv
Copy link

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
Copy link
Member Author

Tratcher commented Sep 5, 2018

@guardrex you got this?

@guardrex
Copy link
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
Copy link
Member Author

Tratcher commented Sep 5, 2018

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

@mattnewell
Copy link

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 https://github.com/aspnet/BasicMiddleware/issues/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
Copy link
Member

@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
Copy link

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
Copy link
Member Author

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

@ygoe
Copy link

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
Copy link
Member Author

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
Copy link

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
Copy link
Member Author

@jondmcelroy the IPv6 restriction no longer applies in 2.2.

Clearing the networks and proxies is strongly discouraged.

@buvinghausen
Copy link

@Tratcher strongly discouraged may not be sufficient.

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

@andycmaj
Copy link

andycmaj commented Dec 18, 2018

key point here that f'ed me over: #2384 (comment)

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.

Do NOT use ALL. Use only the headers your proxy ACTUALLY forwards.

Edit

I also ran into the Message: "Unknown proxy: "[::ffff:10.12.1.90]:55122"" issue. Given the guidance:

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

I'm not sure what the right answer is here... there's no way i can/should-have-to know my network ip addresses in advance. Are you really suggesting that we should be modifying these values (via config or otherwise) for every environment we deploy to?

@guardrex
Copy link
Collaborator

@andycmaj Please open a new issue using the Content feedback button at the bottom of the Configure ASP.NET Core to work with proxy servers and load balancers topic.

@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests