inveniosoftware / invenio-rest

REST API support for Invenio.
https://invenio-rest.readthedocs.io
MIT License
2 stars 41 forks source link

Misleading authorisation token error messages #132

Open bootsa opened 2 years ago

bootsa commented 2 years ago

Package version (if known): v1.2.7 & current (2022-02-16) InvenioRDM demo install

Describe the bug

Posting to the REST api (e.g. https://inveniordm.web.cern.ch/api/records) with an invalid or missing token returns Referer errors and is a generic 400 response rather than a more specific error - probably 401 rather than 403 as the user is not known without a valid token.

Steps to Reproduce

  1. Post to REST api with missing or invalid token
  2. Response:
    {
    "message": "Referer checking failed - no Referer.",
    "status": 400
    }

Expected behavior

Error message related to actual issue and a more specific error code so it can be better handled by client applications.

Additional context

A small issue but it caused some confusion in figuring out the actual cause.

lnielsen commented 2 years ago

Can you provide the REST API calls that you're making? This looks like a CSRF token validation error, so I assume you're using session based authentication rather than access token?

slint commented 9 months ago

TLDR: This is indeed a bug, that affects non-browser REST API clients (found also by a ZenodoRDM testing partner). I think the solution is to ignore CSRF if there was no session cookie passed in the request, which boils down to simply checking if session.new.


The issue is about the fact that our REST API endpoints accept authentication using both cookie-based sessions and "stateless" private access tokens (passed via ?access_token={TOKEN} or Authorization: Bearer {TOKEN} header).

To replicate the issue:

# POST without headers or querystring param
$ curl -X POST "https://inveniordm.web.cern.ch/api"
{"message": "Referer checking failed - no Referer.", "status": 400}

# POST without an invalid access token
$ curl -X POST "https://inveniordm.web.cern.ch/api?access_token=invalid"
{"message": "Referer checking failed - no Referer.", "status": 400}

What I would expect is:

# 405, since no need for CSRF, and we don't support POST on `/api`
$ curl -X POST "https://inveniordm.web.cern.ch/api"
405 Method Not Allowed

# Not sure here... 
$ curl -X POST "https://inveniordm.web.cern.ch/api?access_token=invalid"
400 Bad Request

# Endpoint requires auth, and token is invalid, so it's a 401
$ curl -X POST "https://inveniordm.web.cern.ch/api/records?access_token=invalid"
401 Unauthorized

For the 2nd request, I'm not sure since there are many ways to approach this:

Solution

The case here is that if the request is not coming from the browser, there's no point in checking for CSRF... The real question now is: "How do we know the request is not coming from the browser?". I feel that this SO answer gets it right:

While cookies are the primary attack vector for CSRF attacks, you are also vulnerable if you use HTTP/Basic authentication. More generally, if the browser is able automatically pass along login credentials for your app, then CSRF matters. ... Either way, the overall answer is simple: if you are using cookies (or other authentication methods that the browser can do automatically) then you need CSRF protection. If you aren't using cookies then you don't.

In our case, the only authentication methods we're using in Invenio(RDM) which the browser performs automatically are cookie/session-based. So the solution is to disable CSRF in csrf_validate if there is a session cookie passed, which boils down to simply checking if session.new.

I would also look for a 2nd opinion, possibly from the security team at CERN.

phette23 commented 4 months ago

FYI for others running into this bug, you can set REST_CSRF_ENABLED = False in invenio.cfg to disable CSRF checking on API requests. There's a warning in ext.py that "CSRF validation will be enabled by default in the version 1.3.x" though.

I am testing locally so security is not a concern, but this obviously has security implications if used in production.