Skip to content

Commit

Permalink
Fixed #35328 - Improved debug messaging behind proxies.
Browse files Browse the repository at this point in the history
  • Loading branch information
ryanhiebert committed Mar 25, 2024
1 parent b6e2b83 commit 0df060c
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 16 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ answer newbie questions, and generally made Django that much better:
Russ Webber
Ryan Hall <ryanhall989@gmail.com>
Ryan Heard <ryanwheard@gmail.com>
Ryan Hiebert <ryan@ryanhiebert.com>
ryankanno
Ryan Kelly <ryan@rfk.id.au>
Ryan Niemeyer <https://profiles.google.com/ryan.niemeyer/about>
Expand Down
29 changes: 21 additions & 8 deletions django/middleware/csrf.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
# This matches if any character is not in CSRF_ALLOWED_CHARS.
invalid_token_chars_re = _lazy_re_compile("[^a-zA-Z0-9]")

REASON_BAD_ORIGIN = "Origin checking failed - %s does not match any trusted origins."
REASON_BAD_ORIGIN = (
"Origin checking failed - %s does not match any trusted origins to server %s."
)
REASON_NO_REFERER = "Referer checking failed - no Referer."
REASON_BAD_REFERER = "Referer checking failed - %s does not match any trusted origins."
REASON_NO_CSRF_COOKIE = "CSRF cookie not set."
Expand Down Expand Up @@ -268,19 +270,25 @@ def _set_csrf_cookie(self, request, response):
# Set the Vary header since content varies with the CSRF cookie.
patch_vary_headers(response, ("Cookie",))

def _origin_verified(self, request):
request_origin = request.META["HTTP_ORIGIN"]
def _host_origin(self, request):
"""
Find the host origin that is automatically trusted.
"""
try:
good_host = request.get_host()
except DisallowedHost:
pass
return
else:
good_origin = "%s://%s" % (
return "%s://%s" % (
"https" if request.is_secure() else "http",
good_host,
)
if request_origin == good_origin:
return True

def _origin_verified(self, request):
request_origin = request.META["HTTP_ORIGIN"]
host_origin = self._host_origin(request)
if host_origin and request_origin == host_origin:
return True
if request_origin in self.allowed_origins_exact:
return True
try:
Expand Down Expand Up @@ -436,7 +444,12 @@ def process_view(self, request, callback, callback_args, callback_kwargs):
if "HTTP_ORIGIN" in request.META:
if not self._origin_verified(request):
return self._reject(
request, REASON_BAD_ORIGIN % request.META["HTTP_ORIGIN"]
request,
REASON_BAD_ORIGIN
% (
request.META["HTTP_ORIGIN"],
self._host_origin(request) or '<disallowed host>',
),
)
elif request.is_secure():
# If the Origin header wasn't provided, reject HTTPS requests if
Expand Down
16 changes: 15 additions & 1 deletion django/views/csrf.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ def csrf_failure(request, reason="", template_name=CSRF_FAILURE_TEMPLATE_NAME):
"""
Default view used when request fails CSRF protection
"""
from django.middleware.csrf import REASON_NO_CSRF_COOKIE, REASON_NO_REFERER
from django.middleware.csrf import (
REASON_BAD_ORIGIN,
REASON_NO_CSRF_COOKIE,
REASON_NO_REFERER,
)

c = {
"title": _("Forbidden"),
Expand Down Expand Up @@ -61,6 +65,16 @@ def csrf_failure(request, reason="", template_name=CSRF_FAILURE_TEMPLATE_NAME):
"re-enable them, at least for this site, or for “same-origin” "
"requests."
),
"bad_origin": reason.split(" - ")[0] == REASON_BAD_ORIGIN.split(" - ")[0],
"forwarded_may_fix": (
request.headers.get("X-Forwarded-Proto", "") == "https"
and not request.is_secure()
or request.headers.get("Origin", "").endswith(
f'://{request.headers.get("X-Forwarded-Host", "")}'
)
),
"x_forwarded_proto": request.headers.get("X-Forwarded-Proto"),
"x_forwarded_host": request.headers.get("X-Forwarded-Host"),
"DEBUG": settings.DEBUG,
"docs_version": get_docs_version(),
"more": _("More information is available with DEBUG=True."),
Expand Down
29 changes: 28 additions & 1 deletion django/views/templates/csrf_403.html
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,32 @@ <h2>Help</h2>
</pre>
{% endif %}

{% if bad_origin %}
{% if forwarded_may_fix %}
<p>The <code>Origin</code> header does not match
the expected server origin,
but common proxy headers are present in the request
and may include parts of the <code>Origin</code> header.</p>
<ul>
<li><strong><code>X-Forwarded-Proto</code></strong>:
<code>{{x_forwarded_proto}}</code></li>
<li><strong><code>X-Forwarded-Host</code></strong>:
<code>{{x_forwarded_host}}</code></li>
</ul>
<p>After you understand the
<a href="https://docs.djangoproject.com/en/{{ docs_version }}/ref/settings/#secure-proxy-ssl-header">important security considerations</a>,
you may wish to add one or more of the following
settings to permit Django to trust these headers.</p>
<p><pre>
SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https")
USE_X_FORWARDED_HOST = True
</pre></p>
{% else %}
If the expected server origin looks correct,
you may wish to add the origin to the CSRF_TRUSTED_ORIGINS setting.
{% endif %}
{% else %}

<p>In general, this can occur when there is a genuine Cross Site Request Forgery, or when
<a
href="https://docs.djangoproject.com/en/{{ docs_version }}/ref/csrf/">Django’s
Expand All @@ -53,7 +79,7 @@ <h2>Help</h2>
<li>Your browser is accepting cookies.</li>

<li>The view function passes a <code>request</code> to the template’s <a
href="https://docs.djangoproject.com/en/dev/topics/templates/#django.template.backends.base.Template.render"><code>render</code></a>
href="https://docs.djangoproject.com/en/{{ docs_version }}/topics/templates/#django.template.backends.base.Template.render"><code>render</code></a>
method.</li>

<li>In the template, there is a <code>{% templatetag openblock %} csrf_token
Expand All @@ -68,6 +94,7 @@ <h2>Help</h2>
tab or hitting the back button after a login, you may need to reload the
page with the form, because the token is rotated after a login.</li>
</ul>
{% endif %}

<p>You’re seeing the help section of this page because you have <code>DEBUG =
True</code> in your Django settings file. Change that to <code>False</code>,
Expand Down
3 changes: 2 additions & 1 deletion docs/releases/5.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ Cache
CSRF
~~~~

* ...
* The error messaging is improved when ``Origin`` checking fails,
including better hints when likely proxy headers are detected.

Database backends
~~~~~~~~~~~~~~~~~
Expand Down
61 changes: 56 additions & 5 deletions tests/csrf_tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ def test_bad_origin_bad_domain(self):
with self.assertLogs("django.security.csrf", "WARNING") as cm:
response = mw.process_view(req, post_form_view, (), {})
self.assertEqual(response.status_code, 403)
msg = REASON_BAD_ORIGIN % req.META["HTTP_ORIGIN"]
msg = REASON_BAD_ORIGIN % (req.META["HTTP_ORIGIN"], "http://" + req.META["HTTP_HOST"])

Check failure on line 910 in tests/csrf_tests/tests.py

View workflow job for this annotation

GitHub Actions / flake8

line too long (94 > 88 characters)
self.assertEqual(cm.records[0].getMessage(), "Forbidden (%s): " % msg)

@override_settings(ALLOWED_HOSTS=["www.example.com"])
Expand All @@ -922,7 +922,7 @@ def test_bad_origin_null_origin(self):
with self.assertLogs("django.security.csrf", "WARNING") as cm:
response = mw.process_view(req, post_form_view, (), {})
self.assertEqual(response.status_code, 403)
msg = REASON_BAD_ORIGIN % req.META["HTTP_ORIGIN"]
msg = REASON_BAD_ORIGIN % (req.META["HTTP_ORIGIN"], "http://" + req.META["HTTP_HOST"])

Check failure on line 925 in tests/csrf_tests/tests.py

View workflow job for this annotation

GitHub Actions / flake8

line too long (94 > 88 characters)
self.assertEqual(cm.records[0].getMessage(), "Forbidden (%s): " % msg)

@override_settings(ALLOWED_HOSTS=["www.example.com"])
Expand All @@ -938,7 +938,7 @@ def test_bad_origin_bad_protocol(self):
with self.assertLogs("django.security.csrf", "WARNING") as cm:
response = mw.process_view(req, post_form_view, (), {})
self.assertEqual(response.status_code, 403)
msg = REASON_BAD_ORIGIN % req.META["HTTP_ORIGIN"]
msg = REASON_BAD_ORIGIN % (req.META["HTTP_ORIGIN"], "https://" + req.META["HTTP_HOST"])

Check failure on line 941 in tests/csrf_tests/tests.py

View workflow job for this annotation

GitHub Actions / flake8

line too long (95 > 88 characters)
self.assertEqual(cm.records[0].getMessage(), "Forbidden (%s): " % msg)

@override_settings(
Expand All @@ -965,7 +965,7 @@ def test_bad_origin_csrf_trusted_origin_bad_protocol(self):
with self.assertLogs("django.security.csrf", "WARNING") as cm:
response = mw.process_view(req, post_form_view, (), {})
self.assertEqual(response.status_code, 403)
msg = REASON_BAD_ORIGIN % req.META["HTTP_ORIGIN"]
msg = REASON_BAD_ORIGIN % (req.META["HTTP_ORIGIN"], "https://" + req.META["HTTP_HOST"])

Check failure on line 968 in tests/csrf_tests/tests.py

View workflow job for this annotation

GitHub Actions / flake8

line too long (95 > 88 characters)
self.assertEqual(cm.records[0].getMessage(), "Forbidden (%s): " % msg)
self.assertEqual(mw.allowed_origins_exact, {"http://no-match.com"})
self.assertEqual(
Expand All @@ -991,7 +991,7 @@ def test_bad_origin_cannot_be_parsed(self):
with self.assertLogs("django.security.csrf", "WARNING") as cm:
response = mw.process_view(req, post_form_view, (), {})
self.assertEqual(response.status_code, 403)
msg = REASON_BAD_ORIGIN % req.META["HTTP_ORIGIN"]
msg = REASON_BAD_ORIGIN % (req.META["HTTP_ORIGIN"], "http://" + req.META["HTTP_HOST"])

Check failure on line 994 in tests/csrf_tests/tests.py

View workflow job for this annotation

GitHub Actions / flake8

line too long (94 > 88 characters)
self.assertEqual(cm.records[0].getMessage(), "Forbidden (%s): " % msg)

@override_settings(ALLOWED_HOSTS=["www.example.com"])
Expand Down Expand Up @@ -1057,6 +1057,57 @@ def test_good_origin_wildcard_csrf_trusted_origin_allowed(self):
self.assertEqual(mw.allowed_origins_exact, set())
self.assertEqual(mw.allowed_origin_subdomains, {"https": [".example.com"]})

@override_settings(ALLOWED_HOSTS=["www.example.com", "localhost"], DEBUG=True)
def test_bad_origin_x_forwarded_host_with_port(self):
"""Give a helpful message if we see a likely X-Forwarded-Host header."""
req = self._get_POST_request_with_token()
req.META["HTTP_HOST"] = "localhost:8000"
req.META["HTTP_ORIGIN"] = "http://www.example.com:8080"
req.META["HTTP_X_FORWARDED_HOST"] = "www.example.com:8080"
mw = CsrfViewMiddleware(post_form_view)
self._check_referer_rejects(mw, req)
self.assertIs(mw._origin_verified(req), False)
with self.assertLogs("django.security.csrf", "WARNING") as cm:
response = mw.process_view(req, post_form_view, (), {})
self.assertEqual(response.status_code, 403)
msg = REASON_BAD_ORIGIN % (req.META["HTTP_ORIGIN"], "http://" + req.META["HTTP_HOST"])

Check failure on line 1073 in tests/csrf_tests/tests.py

View workflow job for this annotation

GitHub Actions / flake8

line too long (94 > 88 characters)
self.assertEqual(cm.records[0].getMessage(), "Forbidden (%s): " % msg)
self.assertContains(response, "X-Forwarded-Host", status_code=403)

@override_settings(ALLOWED_HOSTS=["www.example.com", "localhost"], DEBUG=True)
def test_bad_origin_x_forwarded_host_no_port(self):
"""Give a helpful message if we see a likely X-Forwarded-Host header."""
req = self._get_POST_request_with_token()
req.META["HTTP_HOST"] = "localhost"
req.META["HTTP_ORIGIN"] = "http://www.example.com"
req.META["HTTP_X_FORWARDED_HOST"] = "www.example.com"
mw = CsrfViewMiddleware(post_form_view)
self._check_referer_rejects(mw, req)
self.assertIs(mw._origin_verified(req), False)
with self.assertLogs("django.security.csrf", "WARNING") as cm:
response = mw.process_view(req, post_form_view, (), {})
self.assertEqual(response.status_code, 403)
msg = REASON_BAD_ORIGIN % (req.META["HTTP_ORIGIN"], "http://" + req.META["HTTP_HOST"])

Check failure on line 1090 in tests/csrf_tests/tests.py

View workflow job for this annotation

GitHub Actions / flake8

line too long (94 > 88 characters)
self.assertEqual(cm.records[0].getMessage(), "Forbidden (%s): " % msg)
self.assertContains(response, "X-Forwarded-Host", status_code=403)

@override_settings(ALLOWED_HOSTS=["www.example.com"], DEBUG=True)
def test_bad_origin_x_forwarded_proto(self):
"""Give a helpful message if we see a likely X-Forwarded-Proto header."""
req = self._get_POST_request_with_token()
req.META["HTTP_HOST"] = "www.example.com"
req.META["HTTP_ORIGIN"] = "https://www.example.com"
req.META["HTTP_X_FORWARDED_PROTO"] = "https"
mw = CsrfViewMiddleware(post_form_view)
self._check_referer_rejects(mw, req)
self.assertIs(mw._origin_verified(req), False)
with self.assertLogs("django.security.csrf", "WARNING") as cm:
response = mw.process_view(req, post_form_view, (), {})
self.assertEqual(response.status_code, 403)
msg = REASON_BAD_ORIGIN % (req.META["HTTP_ORIGIN"], "http://" + req.META["HTTP_HOST"])

Check failure on line 1107 in tests/csrf_tests/tests.py

View workflow job for this annotation

GitHub Actions / flake8

line too long (94 > 88 characters)
self.assertEqual(cm.records[0].getMessage(), "Forbidden (%s): " % msg)
self.assertContains(response, "X-Forwarded-Proto", status_code=403)


class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase):
def _set_csrf_cookie(self, req, cookie):
Expand Down

0 comments on commit 0df060c

Please sign in to comment.