Browse Source

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.
Carl Meyer 10 years ago
parent
commit
316b8d4974

+ 8 - 0
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

+ 16 - 0
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.

+ 24 - 0
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
 ========
 

+ 24 - 0
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.

+ 23 - 1
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
 ========

+ 55 - 0
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')