aspnet / MetaPackages

[Archived] NuGet meta packages. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
211 stars 109 forks source link

Add Microsoft.AspNetCore.HttpOverrides to CreateDefaultBuilder() #253

Closed jchannon closed 6 years ago

jchannon commented 6 years ago

CreateDefaultBuilder calls UseIISIntegraiton but obviously if this isn't used on a OSX/Linux UseHttpMethodOverride isn't invoked to handle headers like X-Forwarded-Proto.

UseHttpMethodOverride should not be a default feature just for IIS users only.

Reference : https://github.com/aspnet/BasicMiddleware/tree/dev/src/Microsoft.AspNetCore.HttpOverrides

Tratcher commented 6 years ago

You're referring to UseForwardedHeaders, not UseHttpMethodOverride, correct?

This is intentional. UseForwardedHeaders is actually quite dangerous as it allows local values like scheme and remote IP to be overridden by remote data. This must not be used unless you have some level of trust in said data, e.g. you know you have a reverse proxy that sets these specific headers. This is why it's added by UseIISIntegraiton only when running behind IIS, it knows what headers will be provided by AspNetCoreModule.

There's an acknowledged gap where we have no UseNginxIntegration to mirror UseIISIntegraiton. We'd want this to provide much of the same functionality such as lighting up only when behind nginx, adding UseForwardedHeaders, handling path base, etc.. This would be paired with docs on the recommended nginx config file to use to match the server settings.

jchannon commented 6 years ago

Yeah exactly right. We have nginx and assumed the UseForwardHeaders was a default behaviour in the runtime somehow.

Out of interest even with IIS how do you know that iis send the forwarded header and not some cheeky hacker.

On Wed, 3 Jan 2018 at 18:23, Chris Ross notifications@github.com wrote:

You're referring to UseForwardedHeaders, not UseHttpMethodOverride, correct?

This is intentional. UseForwardedHeaders is actually quite dangerous as it allows local values like scheme and remote IP to be overridden by remote data. This must not be used unless you have some level of trust in said data, e.g. you know you have a reverse proxy that sets these specific headers. This is why it's added by UseIISIntegraiton only when running behind IIS, it knows what headers will be provided by AspNetCoreModule.

There's an acknowledged gap where we have no UseNginxIntegration to mirror UseIISIntegraiton. We'd want this to provide much of the same functionality such as lighting up only when behind nginx, adding UseForwardedHeaders, handling path base, etc.. This would be paired with docs on the recommended nginx config file to use to match the server settings.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/aspnet/MetaPackages/issues/253#issuecomment-355087080, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGapuNbQBQa4RrRr99xplNPYgSfw8Lfks5tG8W6gaJpZM4RQ_rC .

Tratcher commented 6 years ago

We use environment variables at startup to detect that we're running behind IIS/ANCM, and in ANCM we always have it set those.

aspnet-hello commented 6 years ago

We periodically close 'discussion' issues that have not been updated in a long period of time.

We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.