CSRF Vulnerability on Login (and other unauthenticated POST endpoints) for SessionAuthenticaiton

[ajbeach2]

Django Rest Framework by default will make APIView csrf excempt for ApiView. CSRF handling in DRF is done at the SessionAuthenticaiton class level.

Here is the relevant code: https://github.com/encode/django-rest-framework/blob/master/rest_framework/views.py#L144 All ApiView(s) will by default will be csrf_exempt.

When requests are authenticated with the SessionAuthentication class, unauthenticated users csrf tokens are not enforced https://github.com/encode/django-rest-framework/blob/master/rest_framework/authentication.py#L124

This library uses the @api_view decorator which warps the ApiView class in DRF. Therefore. this library is vulnerable to csrf for logins and other endpoints which do not require authentication.

This test case, added to the test_login.py, demonstrates the failure:

def test_csrf(self):
        from rest_framework.test import APIRequestFactory
        factory = APIRequestFactory(enforce_csrf_checks=True)
        request = factory.post("/login", {
            'login': self.user.username,
            'password': self.password,
        })
        self.add_session_to_request(request)
        response = self.view_func(request)
        self.assert_invalid_response(response, status.HTTP_403_FORBIDDEN)

I am working on a possible fix, but I am not sure the best approach. In my personable projects, I don't use Token Authentication, I only use session. If this is the case, you can create a custom authentication class and add it to the @authentication_class decorator from rest_framework.decorators. This approach will break the Token Authentication however.

from rest_framework.authentication import SessionAuthentication

class SessionAuthAll(SessionAuthentication):

    def authenticate(self, request):
        """
        Returns a `User` and force CSRF even if
        the user is Unauthenticated
        """

        # Get the session-based user from the underlying HttpRequest object
        user = getattr(request._request, 'user', None)

        self.enforce_csrf(request)

        # CSRF passed with authenticated user
        return (user, None)

[peterthomassen]

CSRF tokens are to mitigate abuse of already existing sessions. An API that is intended to be open to all clients, however, should accept logins from everywhere. How should the API know whether the web site initiating the login is not a legitimate client? - Also, if you log in from curl, CSRF tokens will not be presented.

Once login has happened and a token has been created, you of course have to control which sites can use the token. The right approach to handle that are CORS headers. For details of what to configure, take a look at https://www.sjoerdlangkemper.nl/2018/09/12/authorization-header-and-cors/. You can use https://pypi.org/project/django-cors-headers/ to plug this into your Django project.

tl;dr: I do not think this is a vulnerability.

[ajbeach2]

If you have strictly auth token based api that doesn't use cookies that you are correct, that CSRF tokens are not needed because you aren't using cookies.

It only really matters when you are using only session based auth that uses cookies. Login csrf is a legitimate attack vector where, a forged request could cause the user to login to an account they don't own. There are more details here on the attack: https://stackoverflow.com/questions/6412813/do-login-forms-need-tokens-against-csrf-attacks

The django login forms provided by contrib.auth enforce csrf tokens for logins: https://github.com/django/django/blob/master/django/contrib/auth/views.py#L50.

The django rest framework recommends using django's login forms for this reason: https://www.django-rest-framework.org/api-guide/authentication/

Warning: Always use Django's standard login view when creating login pages. This will ensure your login views are properly protected.

CSRF validation in REST framework works slightly differently to standard Django due to the need to support both session and non-session based authentication to the same views. This means that only authenticated requests require CSRF tokens, and anonymous requests may be sent without CSRF tokens. This behaviour is not suitable for login views, which should always have CSRF validation applied.

I brought up this issue: https://github.com/encode/django-rest-framework/issues/6795

But it boils down to that: if you have both session + token backends enabled, and only enforce csrf for session, it wouldn't matter as django would try the next auth backend.

I am not sure there is a real solution here other than: if you only use session cookie based auth (not token auth without cookies) you should enforce csrf tokens.

[ajbeach2]

You could potentially address this, you would have to dynamically look at which auth backends are enabled, and only enforce csrf if the only backend is cookie based.

[apragacz]

@psibean I wanted to address your comment, but you deleted it (or at least it looks like so). Is there any reason for this?

[psibean]

Sorry! I had not meant to delete my comment here - apologies. And sorry for the delay getting back onto this - I've been caught up in some other work. I just wanted to say that I absolutely love how this package is setup and configured (especially for development) - I got the development environment up and running pretty quickly in WSL2!

I was really happy to find this package as most other auth based django apps completely ignore the "http only session" auth being the only authentication option as a use case. Other libraries are heavily riddled with token auth coupling. So to see this as an issue is the only disappointment - and given it can be worked around, it isn't a huge one!

For me, this also has a bit of overhead for things I don't need, I was considering adapting small parts of this into my own code (providing credit and watching / contributing here as appropriate), or taking this and updating it to be class based, removing all of the token authentication out and strictly supporting a session only rest authentication

[kris7ian]

I am considering using this package for my new project, any tips or guides on how to address this issue? I'm also planning to use session only rest authentication (why is this so uncommon? :) ) Thanks

[adel-ht]

@kris7ian A fix is given by ajbeach2 in the issue original post.

@apragacz What's up with the branch issue-65-session-login-with-csrf Any plan to merge that ? Thanks !

[apragacz]

@adel-ht that change was not working correctly as there was some limitation regarding using @api_view, but now this is actually unblocked. I wasn't sure if the approach there could help with the actual issue being reported so it wasn't merged yet.

[adel-ht]

Not sure how to push this forward, any thoughts @apragacz ?