Add __Host- prefix to CSRF and session cookie, remove cookie_domain (#…

…3831)

* Add __Host- prefix to CSRF and session cookie, remove cookie_domain

* Fix tests

doc/admin/config.rst

@@ -42,7 +42,6 @@ Example::
     currency=EUR
     datadir=/data
     plugins_default=pretix.plugins.sendmail,pretix.plugins.statistics
-    cookie_domain=.pretix.de
 
 ``instance_name``
     The name of this installation. Default: ``pretix.de``
@@ -71,9 +70,6 @@ Example::
 ``auth_backends``
     A comma-separated list of available auth backends. Defaults to ``pretix.base.auth.NativeAuthBackend``.
 
-``cookie_domain``
-    The cookie domain to be set. Defaults to ``None``.
-
 ``registration``
     Enables or disables the registration of new admin users. Defaults to ``off``.
 

src/pretix/multidomain/middlewares.py

@@ -40,10 +40,12 @@
     SessionMiddleware as BaseSessionMiddleware,
 )
 from django.core.cache import cache
-from django.core.exceptions import DisallowedHost
+from django.core.exceptions import DisallowedHost, ImproperlyConfigured
 from django.http.request import split_domain_port
 from django.middleware.csrf import (
-    CSRF_SESSION_KEY, CsrfViewMiddleware as BaseCsrfMiddleware,
+    CSRF_SESSION_KEY, CSRF_TOKEN_LENGTH,
+    CsrfViewMiddleware as BaseCsrfMiddleware, _check_token_format,
+    _unmask_cipher_token,
 )
 from django.shortcuts import render
 from django.urls import set_urlconf
@@ -144,6 +146,13 @@ class SessionMiddleware(BaseSessionMiddleware):
     a custom domain.
     """
 
+    def process_request(self, request):
+        session_key = request.COOKIES.get(
+            '__Host-' + settings.SESSION_COOKIE_NAME,
+            request.COOKIES.get(settings.SESSION_COOKIE_NAME)
+        )
+        request.session = self.SessionStore(session_key)
+
     def process_response(self, request, response):
         try:
             accessed = request.session.accessed
@@ -154,7 +163,10 @@ def process_response(self, request, response):
         else:
             # First check if we need to delete this cookie.
             # The session should be deleted only if the session is entirely empty
-            if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
+            is_secure = request.scheme == 'https'
+            if '__Host-' + settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
+                response.delete_cookie('__Host-' + settings.SESSION_COOKIE_NAME)
+            elif settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
                 response.delete_cookie(settings.SESSION_COOKIE_NAME)
             else:
                 if accessed:
@@ -171,12 +183,14 @@ def process_response(self, request, response):
                     # Skip session save for 500 responses, refs #3881.
                     if response.status_code != 500:
                         request.session.save()
+                        if is_secure and settings.SESSION_COOKIE_NAME in request.COOKIES:  # remove legacy cookie
+                            response.delete_cookie(settings.SESSION_COOKIE_NAME)
+                            response.delete_cookie(settings.SESSION_COOKIE_NAME, samesite="None")
                         set_cookie_without_samesite(
                             request, response,
-                            settings.SESSION_COOKIE_NAME,
+                            '__Host-' + settings.SESSION_COOKIE_NAME if is_secure else settings.SESSION_COOKIE_NAME,
                             request.session.session_key, max_age=max_age,
                             expires=expires,
-                            domain=get_cookie_domain(request),
                             path=settings.SESSION_COOKIE_PATH,
                             secure=request.scheme == 'https',
                             httponly=settings.SESSION_COOKIE_HTTPONLY or None
@@ -191,38 +205,52 @@ class CsrfViewMiddleware(BaseCsrfMiddleware):
     a custom domain.
     """
 
+    def _get_secret(self, request):
+        if settings.CSRF_USE_SESSIONS:
+            try:
+                csrf_secret = request.session.get(CSRF_SESSION_KEY)
+            except AttributeError:
+                raise ImproperlyConfigured(
+                    "CSRF_USE_SESSIONS is enabled, but request.session is not "
+                    "set. SessionMiddleware must appear before CsrfViewMiddleware "
+                    "in MIDDLEWARE."
+                )
+        else:
+            try:
+                csrf_secret = request.COOKIES.get('__Host-' + settings.CSRF_COOKIE_NAME)
+                if not csrf_secret:
+                    csrf_secret = request.COOKIES[settings.CSRF_COOKIE_NAME]
+            except KeyError:
+                csrf_secret = None
+            else:
+                # This can raise InvalidTokenFormat.
+                _check_token_format(csrf_secret)
+        if csrf_secret is None:
+            return None
+        # Django versions before 4.0 masked the secret before storing.
+        if len(csrf_secret) == CSRF_TOKEN_LENGTH:
+            csrf_secret = _unmask_cipher_token(csrf_secret)
+        return csrf_secret
+
     def _set_csrf_cookie(self, request, response):
         if settings.CSRF_USE_SESSIONS:
             if request.session.get(CSRF_SESSION_KEY) != request.META["CSRF_COOKIE"]:
                 request.session[CSRF_SESSION_KEY] = request.META["CSRF_COOKIE"]
         else:
+            is_secure = request.scheme == 'https'
             # Set the CSRF cookie even if it's already set, so we renew
             # the expiry timer.
+            if is_secure and settings.CSRF_COOKIE_NAME in request.COOKIES:  # remove legacy cookie
+                response.delete_cookie(settings.CSRF_COOKIE_NAME)
+                response.delete_cookie(settings.CSRF_COOKIE_NAME, samesite="None")
             set_cookie_without_samesite(
                 request, response,
-                settings.CSRF_COOKIE_NAME,
+                '__Host-' + settings.CSRF_COOKIE_NAME if is_secure else settings.CSRF_COOKIE_NAME,
                 request.META["CSRF_COOKIE"],
                 max_age=settings.CSRF_COOKIE_AGE,
-                domain=get_cookie_domain(request),
                 path=settings.CSRF_COOKIE_PATH,
-                secure=request.scheme == 'https',
+                secure=is_secure,
                 httponly=settings.CSRF_COOKIE_HTTPONLY
             )
             # Content varies with the CSRF cookie, so set the Vary header.
             patch_vary_headers(response, ('Cookie',))
-
-
-def get_cookie_domain(request):
-    if '.' not in request.host:
-        # As per spec, browsers do not accept cookie domains without dots in it,
-        # e.g. "localhost", see http://curl.haxx.se/rfc/cookie_spec.html
-        return None
-    default_domain, default_port = split_domain_port(urlparse(settings.SITE_URL).netloc)
-    if request.host == default_domain:
-        # We are on our main domain, set the cookie domain the user has chosen
-        return settings.SESSION_COOKIE_DOMAIN
-    else:
-        # We are on an organizer's custom domain, set no cookie domain, as we do not want
-        # the cookies to be present on any other domain. Setting an explicit value can be
-        # dangerous, see http://erik.io/blog/2014/03/04/definitive-guide-to-cookie-domains/
-        return None

src/pretix/presale/views/__init__.py

@@ -54,7 +54,6 @@
 from pretix.base.services.cart import get_fees
 from pretix.base.templatetags.money import money_filter
 from pretix.helpers.cookies import set_cookie_without_samesite
-from pretix.multidomain.middlewares import get_cookie_domain
 from pretix.multidomain.urlreverse import eventreverse
 from pretix.presale.signals import question_form_fields
 
@@ -469,7 +468,6 @@ def wrapped_view(request, *args, **kwargs):
                 locale,
                 max_age=max_age,
                 expires=(datetime.utcnow() + timedelta(seconds=max_age)).strftime('%a, %d-%b-%Y %H:%M:%S GMT'),
-                domain=get_cookie_domain(request)
             )
             return resp
 

src/pretix/presale/views/locale.py

@@ -40,7 +40,6 @@
 from django.views.generic import View
 
 from pretix.helpers.cookies import set_cookie_without_samesite
-from pretix.multidomain.middlewares import get_cookie_domain
 
 from .robots import NoSearchIndexViewMixin
 
@@ -63,7 +62,6 @@ def get(self, request, *args, **kwargs):
                 max_age=max_age,
                 expires=(datetime.utcnow() + timedelta(seconds=max_age)).strftime(
                     '%a, %d-%b-%Y %H:%M:%S GMT'),
-                domain=get_cookie_domain(request)
             )
 
         return resp

src/pretix/settings.py

@@ -335,8 +335,6 @@
 else:
     CELERY_TASK_ALWAYS_EAGER = True
 
-SESSION_COOKIE_DOMAIN = config.get('pretix', 'cookie_domain', fallback=None)
-
 CACHE_TICKETS_HOURS = config.getint('cache', 'tickets', fallback=24 * 3)
 
 ENTROPY = {

src/tests/multidomain/test_middlewares.py

@@ -165,11 +165,10 @@ def test_cookie_domain_on_event_domain(env, client):
 
 @pytest.mark.django_db
 def test_cookie_domain_on_main_domain(env, client):
-    with override_settings(SESSION_COOKIE_DOMAIN='example.com'):
-        client.post('/mrmcd/2015/cart/add', HTTP_HOST='example.com')
-        r = client.get('/mrmcd/2015/', HTTP_HOST='example.com')
-        assert r.client.cookies['pretix_csrftoken']['domain'] == 'example.com'
-        assert r.client.cookies['pretix_session']['domain'] == 'example.com'
+    client.post('/mrmcd/2015/cart/add', HTTP_HOST='example.com')
+    r = client.get('/mrmcd/2015/', HTTP_HOST='example.com')
+    assert r.client.cookies['pretix_csrftoken']['domain'] == ''
+    assert r.client.cookies['pretix_session']['domain'] == ''
 
 
 @pytest.mark.django_db
@@ -200,8 +199,8 @@ def test_cookie_samesite_none(env, client, agent):
     client.post('/mrmcd/2015/cart/add', HTTP_HOST='example.com', HTTP_USER_AGENT=agent,
                 secure=True)
     r = client.get('/mrmcd/2015/', HTTP_HOST='example.com', HTTP_USER_AGENT=agent, secure=True)
-    assert r.client.cookies['pretix_csrftoken']['samesite'] == 'None'
-    assert r.client.cookies['pretix_session']['samesite'] == 'None'
+    assert r.client.cookies['__Host-pretix_csrftoken']['samesite'] == 'None'
+    assert r.client.cookies['__Host-pretix_session']['samesite'] == 'None'
 
 
 @pytest.mark.django_db
@@ -220,4 +219,4 @@ def test_cookie_samesite_none(env, client, agent):
 def test_cookie_samesite_none_only_on_compatible_browsers(env, client, agent):
     client.post('/mrmcd/2015/cart/add', HTTP_HOST='example.com', HTTP_USER_AGENT=agent, secure=True)
     r = client.get('/mrmcd/2015/', HTTP_HOST='example.com', HTTP_USER_AGENT=agent, secure=True)
-    assert not r.client.cookies['pretix_csrftoken'].get('samesite')
+    assert not r.client.cookies['__Host-pretix_csrftoken'].get('samesite')