From b87d87f9a528e093700d4728dadd9bdd68fb1644 Mon Sep 17 00:00:00 2001 From: Neil Williams Date: Tue, 3 Sep 2013 11:32:27 -0700 Subject: [PATCH] Determine c.secure status based on X-Forwarded-Proto header. Previously, c.secure status was determined based on the domain used. This allows for the status to vary independently of domain for greater flexibility. Note: it is critical that the load balancer strips any X-Forwarded-Proto headers that may've been sent by the client. --- install-reddit.sh | 2 ++ r2/r2/controllers/reddit_base.py | 13 +------------ r2/r2/lib/app_globals.py | 4 ---- r2/r2/lib/base.py | 9 +++++++++ 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/install-reddit.sh b/install-reddit.sh index 55ea62069..382490dbe 100755 --- a/install-reddit.sh +++ b/install-reddit.sh @@ -333,6 +333,8 @@ frontend frontend 0.0.0.0:80 option forwardfor except 127.0.0.1 option httpclose + reqidel ^X-Forwarded-Proto:.* + default_backend dynamic backend dynamic diff --git a/r2/r2/controllers/reddit_base.py b/r2/r2/controllers/reddit_base.py index 413946e50..4153f5222 100644 --- a/r2/r2/controllers/reddit_base.py +++ b/r2/r2/controllers/reddit_base.py @@ -741,18 +741,7 @@ class MinimalController(BaseController): c.domain_prefix = request.environ.get("reddit-domain-prefix", g.domain_prefix) - c.secure = request.host in g.secure_domains - - # wsgi.url_scheme is used in generating absolute urls, such as by webob - # for translating some of our relative-url redirects to rfc compliant - # absolute-url ones. TODO: consider using one of webob's methods of - # setting wsgi.url_scheme based on incoming request headers added by - # upstream things like stunnel/haproxy. - if c.secure: - request.environ["wsgi.url_scheme"] = "https" - # update request.fullurl since wsgi.url_scheme changed. - request.fullurl = request.host_url + request.fullpath - + c.secure = request.environ["wsgi.url_scheme"] == "https" c.request_origin = request.host_url #check if user-agent needs a dose of rate-limiting diff --git a/r2/r2/lib/app_globals.py b/r2/r2/lib/app_globals.py index 2538223ed..072ea66e5 100755 --- a/r2/r2/lib/app_globals.py +++ b/r2/r2/lib/app_globals.py @@ -327,16 +327,12 @@ class Globals(object): origin_prefix = self.domain_prefix + "." if self.domain_prefix else "" self.origin = "http://" + origin_prefix + self.domain - self.secure_domains = set([urlparse(self.payment_domain).netloc]) self.trusted_domains = set([self.domain]) self.trusted_domains.update(self.authorized_cnames) if self.https_endpoint: https_url = urlparse(self.https_endpoint) - self.secure_domains.add(https_url.netloc) self.trusted_domains.add(https_url.hostname) - if getattr(self, 'oauth_domain', None): - self.secure_domains.add(self.oauth_domain) # load the unique hashed names of files under static static_files = os.path.join(self.paths.get('static_files'), 'static') diff --git a/r2/r2/lib/base.py b/r2/r2/lib/base.py index 1363ac792..b53829ff8 100644 --- a/r2/r2/lib/base.py +++ b/r2/r2/lib/base.py @@ -76,6 +76,15 @@ class BaseController(WSGIController): self.post() def __call__(self, environ, start_response): + # we override this here to ensure that this header, and only this + # header, is trusted to reduce the number of potential + # misconfigurations between wsgi application servers (e.g. gunicorn + # which trusts three different headers out of the box for this) and + # haproxy (which won't clean out bad headers by default) + forwarded_proto = environ.get("HTTP_X_FORWARDED_PROTO", "http").lower() + assert forwarded_proto in ("http", "https") + request.environ["wsgi.url_scheme"] = forwarded_proto + true_client_ip = environ.get('HTTP_TRUE_CLIENT_IP') ip_hash = environ.get('HTTP_TRUE_CLIENT_IP_HASH') forwarded_for = environ.get('HTTP_X_FORWARDED_FOR', ())