Opened 6 months ago

Closed 6 months ago

Last modified 3 months ago

#30514 closed Bug (needsinfo)

Occasional CSRF cookie not set Django 2.1

Reported by: Yusuf Musleh Owned by: nobody
Component: CSRF Version: 2.1
Severity: Normal Keywords:
Cc: Rich Rauenzahn Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi all,

We recently noticed an increase in the number of 403 errors we were getting in our server, mainly the CSRF cookie not set. We added more logging and investigated it to see if they were legitimate errors (requests that should cause a 403 error), some are, however we discovered that a lot of them aren't. There were normal requests coming from normal users, we would get around 30 of these failed requests in a given day, it would happen randomly across random devices, browsers and urls and has been quite difficult to reproduce.

We Copy/Pasted the source code of middleware.csrf and added it to our code base as a custom middleware to add more logging and get a better traceback in sentry when the error occurs, to get more insights on issue, we learned the following:

  • For some reason the token was not set. csrf_token = request.META.get('CSRF_COOKIE’) returns None,
  • We know that if a user got the error, if they simply refresh the page things would work perfectly fine, this means that setting the token works, but sometimes it does not
  • We know that it is not a problem with our frontend since we also got this error in the django admin including non-login requests
  • We tried clearing all the expired sessions, and set CSRF_USE_SESSIONS = True, in hopes of it improving the situation, but nothing much has changed.

With all that being said, we narrowed down the problem to the _get_token(self, request) method, since we set CSRF_USE_SESSIONS = True it tries to fetch the token from the session using request.session.get(CSRF_SESSION_KEY), but that would return None. A very interesting thing we noticed is that the CSRF token is present in the actual request's header under HTTP_X_CSRFTOKEN. So we modified the method to fetch the CSRF token from the header as a fallback to see if it would fix the issue:

def _get_token(self, request):
    if settings.CSRF_USE_SESSIONS:
        try:
            token_from_session = 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%s.' % ('_CLASSES' if settings.MIDDLEWARE is None else '')
            )
    else:
        try:
            cookie_token = request.COOKIES[settings.CSRF_COOKIE_NAME]
        except KeyError:
            return None

        csrf_token = _sanitize_token(cookie_token)
        if csrf_token != cookie_token:
            # Cookie token needed to be replaced;
            # the cookie needs to be reset.
            request.csrf_cookie_needs_reset = True
        return csrf_token

    # Fallback code we added to fetch the token from the header
    if token_from_session is None:
        token_from_session = request.META.get(settings.CSRF_HEADER_NAME)

    return token_from_session

Fortunately after this change was made, we haven't gotten any CSRF cookie not set. errors since then because the _get_token method now returns a token, and the process_request method goes smoothly:

def process_request(self, request):
    csrf_token = self._get_token(request)
    if csrf_token is not None:
        # Use same token next time.
        request.META['CSRF_COOKIE'] = csrf_token

the request.META.get('CSRF_COOKIE’) no longer returns None.

We also tested sending bogus requests, ones with no csrf tokens set and ones with incorrect ones just to make sure that the CSRF functionality still works properly, and we get the CSRF cookie not set. and CSRF token missing or incorrect. respectively as expected.

The question still remains, we're not sure why this bug happens. The problem is since django would consider these requests as bogus and handles them accordingly, we were not aware of the issue for a while. Until it randomly happened to a few people on our team, where they would open a fresh page on our website and click on a button that would make a request, and the error would occur, thats when we began the investigation.

Could it be possible that due of the nature of this bug, it is occurring to others as well, but they are just not aware of it? I tried to include as much information as possible, we'd appreciate if someone can shed some light on matter.

Thanks.

Change History (2)

comment:1 Changed 6 months ago by Carlton Gibson

Resolution: needsinfo
Status: newclosed

Hi Yusuf,

Thanks for the report. Very well explained, and interesting.

To be honest, there's not much I can say immediately. There's two possible scenarios:

  1. There's a correct HTTP request, which Django is mishandling.
  2. Django's behaving, but the HTTP request is missing something.

If you could put a test case together showing 1 then we'd having something we could directly address.

Re 2, the only thing that immediately comes to mind is that we've seen Safari behaving strangely due to it's newer "Intelligent Tracking Protection". More or less, when using (multiple?) internal redirects, Safari will simply not send the cookies for the request, which would explain how your session lookup would be failing.

There's django-developers thread on this topic here: https://groups.google.com/d/topic/django-developers/RyDdt1TcH0c/discussion

Since you have the Sentry data available, can you see if perhaps Safari is the common theme here?
If it's not, what else do the requests have in common?

Maybe someone on django-users will have seen similar and can help to pin it down?

As it stands I'm going to close this as needsinfo. I'm really happy to re-open if can show Django is somehow at fault.

(If it is Safari that's at issue, it' worth continuing the django-developers discussion, because we should/could at least try to codify exactly what's happening and how to handle it...)

comment:2 Changed 3 months ago by Rich Rauenzahn

Cc: Rich Rauenzahn added
Note: See TracTickets for help on using tickets.
Back to Top