From 316b8d49746933d1845d600314b002d9b64d3e3d Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 10 Sep 2014 11:06:19 -0600 Subject: [PATCH] Stripped headers containing underscores to prevent spoofing in WSGI environ. This is a security fix. Disclosure following shortly. Thanks to Jedediah Smith for the report. --- django/core/servers/basehttp.py | 8 +++++ docs/howto/auth-remote-user.txt | 16 ++++++++++ docs/releases/1.4.18.txt | 24 ++++++++++++++ docs/releases/1.6.10.txt | 24 ++++++++++++++ docs/releases/1.7.3.txt | 22 +++++++++++++ tests/servers/test_basehttp.py | 55 +++++++++++++++++++++++++++++++++ 6 files changed, 149 insertions(+) diff --git a/django/core/servers/basehttp.py b/django/core/servers/basehttp.py index 600d9eb32a..a6dd4d2d0e 100644 --- a/django/core/servers/basehttp.py +++ b/django/core/servers/basehttp.py @@ -139,6 +139,14 @@ class WSGIRequestHandler(simple_server.WSGIRequestHandler, object): sys.stderr.write(msg) def get_environ(self): + # Strip all headers with underscores in the name before constructing + # the WSGI environ. This prevents header-spoofing based on ambiguity + # between underscores and dashes both normalized to underscores in WSGI + # env vars. Nginx and Apache 2.4+ both do this as well. + for k, v in self.headers.items(): + if '_' in k: + del self.headers[k] + env = super(WSGIRequestHandler, self).get_environ() path = self.path diff --git a/docs/howto/auth-remote-user.txt b/docs/howto/auth-remote-user.txt index 30aad24293..07389c1fac 100644 --- a/docs/howto/auth-remote-user.txt +++ b/docs/howto/auth-remote-user.txt @@ -64,6 +64,22 @@ If your authentication mechanism uses a custom HTTP header and not class CustomHeaderMiddleware(RemoteUserMiddleware): header = 'HTTP_AUTHUSER' +.. warning:: + + Be very careful if using a ``RemoteUserMiddleware`` subclass with a custom + HTTP header. You must be sure that your front-end web server always sets or + strips that header based on the appropriate authentication checks, never + permitting an end-user to submit a fake (or "spoofed") header value. Since + the HTTP headers ``X-Auth-User`` and ``X-Auth_User`` (for example) both + normalize to the ``HTTP_X_AUTH_USER`` key in ``request.META``, you must + also check that your web server doesn't allow a spoofed header using + underscores in place of dashes. + + This warning doesn't apply to ``RemoteUserMiddleware`` in its default + configuration with ``header = 'REMOTE_USER'``, since a key that doesn't + start with ``HTTP_`` in ``request.META`` can only be set by your WSGI + server, not directly from an HTTP request header. + If you need more control, you can create your own authentication backend that inherits from :class:`~django.contrib.auth.backends.RemoteUserBackend` and override one or more of its attributes and methods. diff --git a/docs/releases/1.4.18.txt b/docs/releases/1.4.18.txt index e5df185cfb..55256cfdf3 100644 --- a/docs/releases/1.4.18.txt +++ b/docs/releases/1.4.18.txt @@ -7,6 +7,30 @@ Django 1.4.18 release notes Django 1.4.18 fixes several security issues in 1.4.17 as well as a regression on Python 2.5 in the 1.4.17 release. +WSGI header spoofing via underscore/dash conflation +=================================================== + +When HTTP headers are placed into the WSGI environ, they are normalized by +converting to uppercase, converting all dashes to underscores, and prepending +`HTTP_`. For instance, a header ``X-Auth-User`` would become +``HTTP_X_AUTH_USER`` in the WSGI environ (and thus also in Django's +``request.META`` dictionary). + +Unfortunately, this means that the WSGI environ cannot distinguish between +headers containing dashes and headers containing underscores: ``X-Auth-User`` +and ``X-Auth_User`` both become ``HTTP_X_AUTH_USER``. This means that if a +header is used in a security-sensitive way (for instance, passing +authentication information along from a front-end proxy), even if the proxy +carefully strips any incoming value for ``X-Auth-User``, an attacker may be +able to provide an ``X-Auth_User`` header (with underscore) and bypass this +protection. + +In order to prevent such attacks, both Nginx and Apache 2.4+ strip all headers +containing underscores from incoming requests by default. Django's built-in +development server now does the same. Django's development server is not +recommended for production use, but matching the behavior of common production +servers reduces the surface area for behavior changes during deployment. + Bugfixes ======== diff --git a/docs/releases/1.6.10.txt b/docs/releases/1.6.10.txt index 72eb21e137..dafee70c8c 100644 --- a/docs/releases/1.6.10.txt +++ b/docs/releases/1.6.10.txt @@ -5,3 +5,27 @@ Django 1.6.10 release notes *Under development* Django 1.6.10 fixes several security issues in 1.6.9. + +WSGI header spoofing via underscore/dash conflation +=================================================== + +When HTTP headers are placed into the WSGI environ, they are normalized by +converting to uppercase, converting all dashes to underscores, and prepending +`HTTP_`. For instance, a header ``X-Auth-User`` would become +``HTTP_X_AUTH_USER`` in the WSGI environ (and thus also in Django's +``request.META`` dictionary). + +Unfortunately, this means that the WSGI environ cannot distinguish between +headers containing dashes and headers containing underscores: ``X-Auth-User`` +and ``X-Auth_User`` both become ``HTTP_X_AUTH_USER``. This means that if a +header is used in a security-sensitive way (for instance, passing +authentication information along from a front-end proxy), even if the proxy +carefully strips any incoming value for ``X-Auth-User``, an attacker may be +able to provide an ``X-Auth_User`` header (with underscore) and bypass this +protection. + +In order to prevent such attacks, both Nginx and Apache 2.4+ strip all headers +containing underscores from incoming requests by default. Django's built-in +development server now does the same. Django's development server is not +recommended for production use, but matching the behavior of common production +servers reduces the surface area for behavior changes during deployment. diff --git a/docs/releases/1.7.3.txt b/docs/releases/1.7.3.txt index 5e3cfcd84a..20d0b59457 100644 --- a/docs/releases/1.7.3.txt +++ b/docs/releases/1.7.3.txt @@ -6,7 +6,29 @@ Django 1.7.3 release notes Django 1.7.3 fixes several security issues and bugs in 1.7.2. +WSGI header spoofing via underscore/dash conflation +=================================================== +When HTTP headers are placed into the WSGI environ, they are normalized by +converting to uppercase, converting all dashes to underscores, and prepending +`HTTP_`. For instance, a header ``X-Auth-User`` would become +``HTTP_X_AUTH_USER`` in the WSGI environ (and thus also in Django's +``request.META`` dictionary). + +Unfortunately, this means that the WSGI environ cannot distinguish between +headers containing dashes and headers containing underscores: ``X-Auth-User`` +and ``X-Auth_User`` both become ``HTTP_X_AUTH_USER``. This means that if a +header is used in a security-sensitive way (for instance, passing +authentication information along from a front-end proxy), even if the proxy +carefully strips any incoming value for ``X-Auth-User``, an attacker may be +able to provide an ``X-Auth_User`` header (with underscore) and bypass this +protection. + +In order to prevent such attacks, both Nginx and Apache 2.4+ strip all headers +containing underscores from incoming requests by default. Django's built-in +development server now does the same. Django's development server is not +recommended for production use, but matching the behavior of common production +servers reduces the surface area for behavior changes during deployment. Bugfixes ======== diff --git a/tests/servers/test_basehttp.py b/tests/servers/test_basehttp.py index 41e9258bf5..7352adf287 100644 --- a/tests/servers/test_basehttp.py +++ b/tests/servers/test_basehttp.py @@ -6,6 +6,11 @@ from django.test.utils import captured_stderr from django.utils.six import BytesIO +class Stub(object): + def __init__(self, **kwargs): + self.__dict__.update(kwargs) + + class WSGIRequestHandlerTestCase(TestCase): def test_https(self): request = WSGIRequest(RequestFactory().get('/').environ) @@ -20,3 +25,53 @@ class WSGIRequestHandlerTestCase(TestCase): "but it only supports HTTP.", stderr.getvalue() ) + + def test_strips_underscore_headers(self): + """WSGIRequestHandler ignores headers containing underscores. + + This follows the lead of nginx and Apache 2.4, and is to avoid + ambiguity between dashes and underscores in mapping to WSGI environ, + which can have security implications. + """ + def test_app(environ, start_response): + """A WSGI app that just reflects its HTTP environ.""" + start_response('200 OK', []) + http_environ_items = sorted( + '%s:%s' % (k, v) for k, v in environ.items() + if k.startswith('HTTP_') + ) + yield (','.join(http_environ_items)).encode('utf-8') + + rfile = BytesIO() + rfile.write(b"GET / HTTP/1.0\r\n") + rfile.write(b"Some-Header: good\r\n") + rfile.write(b"Some_Header: bad\r\n") + rfile.write(b"Other_Header: bad\r\n") + rfile.seek(0) + + # WSGIRequestHandler closes the output file; we need to make this a + # no-op so we can still read its contents. + class UnclosableBytesIO(BytesIO): + def close(self): + pass + + wfile = UnclosableBytesIO() + + def makefile(mode, *a, **kw): + if mode == 'rb': + return rfile + elif mode == 'wb': + return wfile + + request = Stub(makefile=makefile) + server = Stub(base_environ={}, get_app=lambda: test_app) + + # We don't need to check stderr, but we don't want it in test output + with captured_stderr(): + # instantiating a handler runs the request as side effect + WSGIRequestHandler(request, '192.168.0.2', server) + + wfile.seek(0) + body = list(wfile.readlines())[-1] + + self.assertEqual(body, b'HTTP_SOME_HEADER:good')