From b07532be752668be5eb5dbd0a8303abf5c219c99 Mon Sep 17 00:00:00 2001 From: Randall Leeds Date: Wed, 10 Jan 2018 11:44:56 -0800 Subject: [PATCH] Forbid contradictory secure scheme headers When a request specifies contradictory secure scheme headers, raise a parse error. --- docs/source/deploy.rst | 11 ++++++----- examples/nginx.conf | 3 +-- gunicorn/http/errors.py | 5 +++++ gunicorn/http/message.py | 27 +++++++++++++++++++++++++++ gunicorn/http/wsgi.py | 17 +++-------------- gunicorn/workers/base.py | 4 ++++ tests/requests/invalid/019.http | 4 ++++ tests/requests/invalid/019.py | 6 ++++++ 8 files changed, 56 insertions(+), 21 deletions(-) create mode 100644 tests/requests/invalid/019.http create mode 100644 tests/requests/invalid/019.py diff --git a/docs/source/deploy.rst b/docs/source/deploy.rst index 289ff727..a731179f 100644 --- a/docs/source/deploy.rst +++ b/docs/source/deploy.rst @@ -37,11 +37,12 @@ To turn off buffering, you only need to add ``proxy_buffering off;`` to your } ... -When Nginx is handling SSL it is helpful to pass the protocol information -to Gunicorn. Many web frameworks use this information to generate URLs. -Without this information, the application may mistakenly generate 'http' -URLs in 'https' responses, leading to mixed content warnings or broken -applications. In this case, configure Nginx to pass an appropriate header:: +It is recommended to pass protocol information to Gunicorn. Many web +frameworks use this information to generate URLs. Without this +information, the application may mistakenly generate 'http' URLs in +'https' responses, leading to mixed content warnings or broken +applications. To configure Nginx to pass an appropriate header, add +a ``proxy_set_header`` directive to your ``location`` block:: ... proxy_set_header X-Forwarded-Proto $scheme; diff --git a/examples/nginx.conf b/examples/nginx.conf index 6fc9115b..b06cccf8 100644 --- a/examples/nginx.conf +++ b/examples/nginx.conf @@ -57,8 +57,7 @@ http { location @proxy_to_app { proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - # enable this if and only if you use HTTPS - # proxy_set_header X-Forwarded-Proto https; + proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header Host $http_host; # we don't want nginx trying to do something clever with # redirects, we set the Host: header above already. diff --git a/gunicorn/http/errors.py b/gunicorn/http/errors.py index bab7856f..d284a059 100644 --- a/gunicorn/http/errors.py +++ b/gunicorn/http/errors.py @@ -107,3 +107,8 @@ class ForbiddenProxyRequest(ParseException): def __str__(self): return "Proxy request from %r not allowed" % self.host + + +class InvalidSchemeHeaders(ParseException): + def __str__(self): + return "Contradictory scheme headers" diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py index acd73873..96e3adc5 100644 --- a/gunicorn/http/message.py +++ b/gunicorn/http/message.py @@ -14,6 +14,7 @@ from gunicorn.http.errors import (InvalidHeader, InvalidHeaderName, NoMoreData, InvalidRequestLine, InvalidRequestMethod, InvalidHTTPVersion, LimitRequestLine, LimitRequestHeaders) from gunicorn.http.errors import InvalidProxyLine, ForbiddenProxyRequest +from gunicorn.http.errors import InvalidSchemeHeaders from gunicorn.six import BytesIO from gunicorn.util import split_request_uri @@ -34,6 +35,7 @@ class Message(object): self.headers = [] self.trailers = [] self.body = None + self.scheme = "https" if cfg.is_ssl else "http" # set headers limits self.limit_request_fields = cfg.limit_request_fields @@ -57,11 +59,24 @@ class Message(object): raise NotImplementedError() def parse_headers(self, data): + cfg = self.cfg headers = [] # Split lines on \r\n keeping the \r\n on each line lines = [bytes_to_str(line) + "\r\n" for line in data.split(b"\r\n")] + # handle scheme headers + scheme_header = False + secure_scheme_headers = {} + if '*' in cfg.forwarded_allow_ips: + secure_scheme_headers = cfg.secure_scheme_headers + elif isinstance(self.unreader, SocketUnreader): + remote_addr = self.unreader.sock.getpeername() + if isinstance(remote_addr, tuple): + remote_host = remote_addr[0] + if remote_host in cfg.forwarded_allow_ips: + secure_scheme_headers = cfg.secure_scheme_headers + # Parse headers into key/value pairs paying attention # to continuation lines. while lines: @@ -92,7 +107,19 @@ class Message(object): if header_length > self.limit_request_field_size > 0: raise LimitRequestHeaders("limit request headers fields size") + + if name in secure_scheme_headers: + secure = value == secure_scheme_headers[name] + scheme = "https" if secure else "http" + if scheme_header: + if scheme != self.scheme: + raise InvalidSchemeHeaders() + else: + scheme_header = True + self.scheme = scheme + headers.append((name, value)) + return headers def set_body_reader(self): diff --git a/gunicorn/http/wsgi.py b/gunicorn/http/wsgi.py index 63ff3d33..13b96eea 100644 --- a/gunicorn/http/wsgi.py +++ b/gunicorn/http/wsgi.py @@ -121,25 +121,14 @@ def create(req, sock, client, server, cfg): # default variables host = None - url_scheme = "https" if cfg.is_ssl else "http" script_name = os.environ.get("SCRIPT_NAME", "") - # set secure_headers - secure_headers = cfg.secure_scheme_headers - if client and not isinstance(client, string_types): - if ('*' not in cfg.forwarded_allow_ips - and client[0] not in cfg.forwarded_allow_ips): - secure_headers = {} - # add the headers to the environ for hdr_name, hdr_value in req.headers: if hdr_name == "EXPECT": # handle expect if hdr_value.lower() == "100-continue": sock.send(b"HTTP/1.1 100 Continue\r\n\r\n") - elif secure_headers and (hdr_name in secure_headers and - hdr_value == secure_headers[hdr_name]): - url_scheme = "https" elif hdr_name == 'HOST': host = hdr_value elif hdr_name == "SCRIPT_NAME": @@ -157,7 +146,7 @@ def create(req, sock, client, server, cfg): environ[key] = hdr_value # set the url scheme - environ['wsgi.url_scheme'] = url_scheme + environ['wsgi.url_scheme'] = req.scheme # set the REMOTE_* keys in environ # authors should be aware that REMOTE_HOST and REMOTE_ADDR @@ -182,9 +171,9 @@ def create(req, sock, client, server, cfg): if host: server = host.split(':') if len(server) == 1: - if url_scheme == "http": + if req.scheme == "http": server.append(80) - elif url_scheme == "https": + elif req.scheme == "https": server.append(443) else: server.append('') diff --git a/gunicorn/workers/base.py b/gunicorn/workers/base.py index f16c0e95..ce40796f 100644 --- a/gunicorn/workers/base.py +++ b/gunicorn/workers/base.py @@ -21,6 +21,7 @@ from gunicorn.http.errors import ( InvalidHTTPVersion, LimitRequestLine, LimitRequestHeaders, ) from gunicorn.http.errors import InvalidProxyLine, ForbiddenProxyRequest +from gunicorn.http.errors import InvalidSchemeHeaders from gunicorn.http.wsgi import default_environ, Response from gunicorn.six import MAXSIZE @@ -201,6 +202,7 @@ class Worker(object): InvalidHTTPVersion, InvalidHeader, InvalidHeaderName, LimitRequestLine, LimitRequestHeaders, InvalidProxyLine, ForbiddenProxyRequest, + InvalidSchemeHeaders, SSLError)): status_int = 400 @@ -226,6 +228,8 @@ class Worker(object): reason = "Forbidden" mesg = "Request forbidden" status_int = 403 + elif isinstance(exc, InvalidSchemeHeaders): + mesg = "%s" % str(exc) elif isinstance(exc, SSLError): reason = "Forbidden" mesg = "'%s'" % str(exc) diff --git a/tests/requests/invalid/019.http b/tests/requests/invalid/019.http new file mode 100644 index 00000000..2f793bfa --- /dev/null +++ b/tests/requests/invalid/019.http @@ -0,0 +1,4 @@ +GET /test HTTP/1.1\r\n +X-Forwarded-Proto: https\r\n +X-Forwarded-Ssl: off\r\n +\r\n diff --git a/tests/requests/invalid/019.py b/tests/requests/invalid/019.py new file mode 100644 index 00000000..4779f7e6 --- /dev/null +++ b/tests/requests/invalid/019.py @@ -0,0 +1,6 @@ +from gunicorn.config import Config +from gunicorn.http.errors import InvalidSchemeHeaders + +request = InvalidSchemeHeaders +cfg = Config() +cfg.set('forwarded_allow_ips', '*')