WIP

bugsink/tests.py

@@ -7,6 +7,7 @@ from unittest import TestCase as RegularTestCase
 from django.test import TestCase as DjangoTestCase
 from django.test import override_settings
 from django.core.exceptions import SuspiciousOperation
+from .wsgi import allowed_hosts_fix_suggestions
 
 from .volume_based_condition import VolumeBasedCondition
 from .streams import (
@@ -376,3 +377,62 @@ class SetRemoteAddrMiddlewareTestCase(RegularTestCase):
 
         with self.assertRaises(SuspiciousOperation):
             SetRemoteAddrMiddleware.parse_x_forwarded_for("123.123.123.123,1.2.3.4")
+
+
+class AllowedHostsFixSuggestionsTestCase(RegularTestCase):
+
+    def test_allowed_hosts_fix_suggestions_allowed_hosts_unset(self):
+        self.assertEqual(
+            "Add 'testserver' to ALLOWED_HOSTS (currently empty).",
+            allowed_hosts_fix_suggestions("testserver", []))
+
+        # I _propably_ want to suggest another fix here.
+        # or nothing at all? Or just say "configure it"
+        '''
+        self.assertEqual(
+            "Add 'testserver' to ALLOWED_HOSTS (currently empty).",
+            allowed_hosts_fix_suggestions("localhost", []))
+        '''
+
+    def test_allowed_hosts_fix_suggestions_proxy_misconfiguration(self):
+        # Proxy misconfiguration: domain as per "typical server misconfiguration" with reasonable allowed_hosts set
+        # We first point to the proxy in this case, but still mention ALLOWED_HOSTS
+        self.assertEqual(
+            "Make sure your proxy (if any) forwards the 'Host: testserver' header or fix ALLOWED_HOSTS.",
+            allowed_hosts_fix_suggestions("localhost", ["testserver"]))
+
+        self.assertEqual(
+            "Make sure your proxy (if any) forwards the 'Host: testserver' header or fix ALLOWED_HOSTS.",
+            allowed_hosts_fix_suggestions("127.0.0.1", ["testserver"]))
+
+        self.assertEqual(
+            "Make sure your proxy (if any) forwards the 'Host: testserver' header or fix ALLOWED_HOSTS.",
+            allowed_hosts_fix_suggestions("123.123.123.123", ["testserver"]))
+
+
+    def test_allowed_hosts_fix_suggestions_allowed_hosts_simply_a_mismatch(self):
+        self.assertEqual(
+            "...",
+            allowed_hosts_fix_suggestions("testserver", ["teestserver"]))
+
+
+
+
+'''
+half-baked version:
+def allowed_hosts_fix_suggestions(domain, allowed_hosts):
+    if not allowed_hosts:
+        # If ALLOWED_HOSTS is not set at all (which takes quite some work in Bugsink, i.e. is unlikely) fix _that_
+        return "You may need to add %r to ALLOWED_HOSTS (currently empty)." % domain
+
+    # Candidate suggestions to add as 'Host' headers on a proxy; we generally don't want to suggest localhost-like
+    # values for this, and since there are various places where localhost-like values get added (`deduce_allowed_hosts`
+    # and `get_host`) they must be taken back out.
+    candidates = [host for host in allowed_hosts if host not in [".localhost", "localhost", "127.0.1", "[::1]"]]
+
+    if len(candidates) == 1:
+        single_candidate = candidates[0]
+        return "Make sure your proxy forwards the 'Host: %s' header." % single_candidate
+    message += " (currently set to %s), " % repr(allowed_hosts)
+
+'''

bugsink/wsgi.py

@@ -13,10 +13,28 @@ import django
 
 from django.core.handlers.wsgi import WSGIHandler, WSGIRequest
 from django.core.exceptions import DisallowedHost
+from django.http.request import split_domain_port, validate_host
 
 os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'bugsink_conf')
 
 
+def allowed_hosts_fix_suggestions(domain, allowed_hosts):
+    if not allowed_hosts:
+        # If ALLOWED_HOSTS is not set at all (which takes quite some work in Bugsink, i.e. is unlikely) fix _that_
+        return "You may need to add %r to ALLOWED_HOSTS (currently empty)." % domain
+
+    # Candidate suggestions to add as 'Host' headers on a proxy; we generally don't want to suggest localhost-like
+    # values for this, and since there are various places where localhost-like values get added (`deduce_allowed_hosts`
+    # and `get_host`) they must be taken back out.
+    candidates = [host for host in allowed_hosts if host not in [".localhost", "localhost", "127.0.1", "[::1]"]]
+
+    if len(candidates) == 1:
+        single_candidate = candidates[0]
+        return "Make sure your proxy forwards the 'Host: %s' header." % single_candidate
+    message += " (currently set to %s), " % repr(allowed_hosts)
+
+
+
 class CustomWSGIRequest(WSGIRequest):
     """
     Custom WSQIRequest subclass with 3 fixes/changes:
@@ -50,27 +68,35 @@ class CustomWSGIRequest(WSGIRequest):
         We're leaking a bit of information here, but I don't think it's too much TBH -- especially in the light of ssl
         certificates being specifically tied to the domain name.
         """
+        if self.path.startswith == "/health/":
+            # For /health/ endpoints, we skip the ALLOWED_HOSTS validation (see #140).
+            return self._get_raw_host()
 
-        # Import pushed down to make it absolutely clear we avoid circular importing/loading the wrong thing:
+        # copied from HttpRequest.get_host() in Django 4.2, with modifications.
+
+        host = self._get_raw_host()
+
+        # Allow variants of localhost if ALLOWED_HOSTS is empty and DEBUG=True.
         from django.conf import settings
+        allowed_hosts = settings.ALLOWED_HOSTS
+        if settings.DEBUG and not allowed_hosts:
+            allowed_hosts = [".localhost", "127.0.0.1", "[::1]"]
 
-        try:
-            return super().get_host()
-        except DisallowedHost as e:
-            if self.path.startswith == "/health/":
-                # For /health/ endpoints, we skip the ALLOWED_HOSTS validation (see #140).
-                return self._get_raw_host()
-
-            message = str(e)
-
-            if "ALLOWED_HOSTS" in message:
-                # The following 3 lines are copied from HttpRequest.get_host() in Django 4.2
-                allowed_hosts = settings.ALLOWED_HOSTS
-                if settings.DEBUG and not allowed_hosts:
-                    allowed_hosts = [".localhost", "127.0.0.1", "[::1]"]
-
-                message = message[:-1 * len(".")]
-                message += ", which is currently set to %s." % repr(allowed_hosts)
+        domain, port = split_domain_port(host)
+        if domain and validate_host(domain, allowed_hosts):
+            return host
+        else:
+            if domain:
+                # we _always_ start with the mismatch in general terms...
+                msg = "Header 'Host: %r' does not match any of allowed hosts: %s." % (domain, allowed_hosts)
+                # ...and follow up with specific suggestions (of which we are less sure) after that.
+                msg += " " + allowed_hosts_fix_suggestions(domain, allowed_hosts)
+            else:
+                msg = "Invalid HTTP_HOST header: %r." % host
+                msg += (
+                    " The domain name provided is not valid according to RFC 1034/1035."
+                )
+            raise DisallowedHost(msg)
 
             # from None, because our DisallowedHost is so directly caused by super()'s DisallowedHost that cause and
             # effect are the same, i.e. cause must be hidden from the stacktrace for the sake of clarity.