URL Scheme should be looked up from wsgi #3

Open
mitsuhiko opened this Issue Apr 30, 2012 · 7 comments

Comments

Projects
None yet
7 participants
@mitsuhiko

This code:

request.headers.get('X-Forwarded-Proto', 'http') == 'https'

Really should be checking against the wsgi.url_scheme. Just trusting HTTP headers from the client is generally not the best idea. If yuo want to use X-Forwarded-Proto you can still remap them as necessary in a WSGI middleware.

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Apr 30, 2012

Owner

I need to use X-Forwarded-Proto for reverse proxied SSL termination. Seems like there's little benefit to moving to middleware?

Owner

kennethreitz commented Apr 30, 2012

I need to use X-Forwarded-Proto for reverse proxied SSL termination. Seems like there's little benefit to moving to middleware?

@nvie

This comment has been minimized.

Show comment
Hide comment
@nvie

nvie Oct 26, 2012

Contributor

The only clean way to handle this is to check the wsgi environ, specifically wsgi.url_scheme (which is exactly what's done by request.is_secure). If requests are actually secure, but wsgi.url_scheme == 'http', something is configured wrong.

Other places inside Flask/Werkzeug also rely solely on wsgi.url_scheme, like for example redirect('/some/relative/url'), which will send an https://-based Location header iff wsgi.url_scheme == 'https'.

Using Flask-SSLify on Heroku for me led to the following odd case:

I hadn't configured gunicorn correctly to set the wsgi.url_scheme based on the X-Forwarded-Proto header (as received from the Heroku reverse proxy). Therefore, all my requests weren't secure, according to request.is_secure. But since Flask-SSLify had this explicit header check in place, it would auto-redirect me to https:// URLs nicely. The effect: on each redirect('/relative/url'), Werkzeug was generating http-based redirect URLs. So I was flip-flopping between HTTP and HTTPS continuously.

If Flask-SSLify didn't do this check, it would've been clear immediately that my wsgi environ setup was wrong, so I'm with @mitsuhiko on this one.

Contributor

nvie commented Oct 26, 2012

The only clean way to handle this is to check the wsgi environ, specifically wsgi.url_scheme (which is exactly what's done by request.is_secure). If requests are actually secure, but wsgi.url_scheme == 'http', something is configured wrong.

Other places inside Flask/Werkzeug also rely solely on wsgi.url_scheme, like for example redirect('/some/relative/url'), which will send an https://-based Location header iff wsgi.url_scheme == 'https'.

Using Flask-SSLify on Heroku for me led to the following odd case:

I hadn't configured gunicorn correctly to set the wsgi.url_scheme based on the X-Forwarded-Proto header (as received from the Heroku reverse proxy). Therefore, all my requests weren't secure, according to request.is_secure. But since Flask-SSLify had this explicit header check in place, it would auto-redirect me to https:// URLs nicely. The effect: on each redirect('/relative/url'), Werkzeug was generating http-based redirect URLs. So I was flip-flopping between HTTP and HTTPS continuously.

If Flask-SSLify didn't do this check, it would've been clear immediately that my wsgi environ setup was wrong, so I'm with @mitsuhiko on this one.

@kljensen

This comment has been minimized.

Show comment
Hide comment
@kljensen

kljensen Mar 17, 2014

I had the same problem as @nvie on Heroku. When a trailing slash is missing, the redirect to the same path with slash present happens as expected; however, the protocol changes from https to http. I used the ProxyFix fixer from Werkzeug to remedy.

import os
from flask import Flask
from werkzeug.contrib.fixers import ProxyFix
from flask_sslify import SSLify


app = Flask(__name__)
app.wsgi_app = ProxyFix(app.wsgi_app)
SSLify(app, age=300, permanent=True)

@app.route('/hello/')
def hello():
    return 'Hello World!'

With that setup, a request to https://domain.tld/hello correctly redirects to https://domain.tld/hello/ instead of http://domain.tld/hello.

I had the same problem as @nvie on Heroku. When a trailing slash is missing, the redirect to the same path with slash present happens as expected; however, the protocol changes from https to http. I used the ProxyFix fixer from Werkzeug to remedy.

import os
from flask import Flask
from werkzeug.contrib.fixers import ProxyFix
from flask_sslify import SSLify


app = Flask(__name__)
app.wsgi_app = ProxyFix(app.wsgi_app)
SSLify(app, age=300, permanent=True)

@app.route('/hello/')
def hello():
    return 'Hello World!'

With that setup, a request to https://domain.tld/hello correctly redirects to https://domain.tld/hello/ instead of http://domain.tld/hello.

@jonafato

This comment has been minimized.

Show comment
Hide comment
@jonafato

jonafato May 11, 2015

Just to reiterate what others have said here, explicitly checking the X-Forwarded-Proto here should not be needed and may cause confusion when parts of the application appear to be secure and others do not.

  • When not behind a proxy, the header should be harmlessly irrelevant or potentially incorrect.
  • When behind a proxy, ProxyFix should be used to automatically use this header when returning request.is_secure.
  • If ProxyFix is not used and Flask-SSLify properly detects that the site is being served over HTTPS via X-Forwarded-Proto, links that require HTTPS (e.g. password reset links) may be generated using HTTP instead resulting in unwanted information leaks / security holes.

If this change would be accepted, I'm happy to make it and add some documentation regarding proxies to accompany it.

Just to reiterate what others have said here, explicitly checking the X-Forwarded-Proto here should not be needed and may cause confusion when parts of the application appear to be secure and others do not.

  • When not behind a proxy, the header should be harmlessly irrelevant or potentially incorrect.
  • When behind a proxy, ProxyFix should be used to automatically use this header when returning request.is_secure.
  • If ProxyFix is not used and Flask-SSLify properly detects that the site is being served over HTTPS via X-Forwarded-Proto, links that require HTTPS (e.g. password reset links) may be generated using HTTP instead resulting in unwanted information leaks / security holes.

If this change would be accepted, I'm happy to make it and add some documentation regarding proxies to accompany it.

@coreybrett

This comment has been minimized.

Show comment
Hide comment
@coreybrett

coreybrett Jun 1, 2015

So If I were to use the code sample given by kljensen that includes ProxyFix, would that work properly on Heroku?

So If I were to use the code sample given by kljensen that includes ProxyFix, would that work properly on Heroku?

thisismyrobot added a commit to thisismyrobot/dnstwister that referenced this issue Feb 1, 2016

@wodow

This comment has been minimized.

Show comment
Hide comment
@wodow

wodow May 19, 2016

In the absence of this change, how about a warning in the README? (I have recently encountered the same issue at @nvie and @kljensen - running on Heroku - and it was not obvious to detect.)

wodow commented May 19, 2016

In the absence of this change, how about a warning in the README? (I have recently encountered the same issue at @nvie and @kljensen - running on Heroku - and it was not obvious to detect.)

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Jun 10, 2016

Owner

@wodow latest version of gunicorn fixes this, with auto-configuration on Heroku, thanks to me :)

Owner

kennethreitz commented Jun 10, 2016

@wodow latest version of gunicorn fixes this, with auto-configuration on Heroku, thanks to me :)

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