Remove XSRF Cookie

[domenkozar]

I'd like to argue that it's a misfeature at this point due to:

• cookie is always present, which makes things like caching harder
• it's really hard to get started for newcomers as GET/POST requests need to send XSRF back on the client side
• seamless CSRF protection is now done using https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite

Are there any users of XSRF cookie that would miss it?

[domenkozar]

cc @jkarni

[jkarni]

I don't think it's a misfeature; though it is annoying how it's implemented, and how much it affects. To quote OWASP about SameSite:

It is important to note that this attribute should be implemented as an additional layer defense in depth concept. This attribute protects the user through the browsers supporting it, and it contains as well 2 ways to bypass it as mentioned in the following section. This attribute should not replace having a CSRF Token. Instead, it should co-exist with that token in order to protect the user in a more robust way.

Regarding "cookie is always present, which makes things like caching harder" - won't this be the case with any protected endpoint?

I think it's pretty normal to have this enabled by default (both Django and Rails do, for example).

[domenkozar]

Regarding "cookie is always present, which makes things like caching harder" - won't this be the case with any protected endpoint?

It's possible to have a protected endpoint where auth is optional. In that case servant-auth always issues a dummy cookie for XSRF, while session cookie just doesn't exist.

[domenkozar]

I think it's pretty normal to have this enabled by default (both Django and Rails do, for example).

There are two differences that make a difference for the user:

• xsrfExcludeGet defaults to True for Django
• no cookie is issued for GET requests

[jkarni]

Fair points. So in summary:

• If we default to excludeGet, and
• only issue the cookies when needed

Then we'd be okay? I defaulted to not excluding GET because the number of times I see GET endpoints doing actual state-changing things is enormous, but if it's annoying we can remove it.

There are other approaches that OWASP suggests, which might also be alternatives. All of them seem to have problems, or involve at least as much input from the users as sending the token back. I myself don't feel super comfortable removing the XSRF token and doing nothing else.

[domenkozar]

Fair points. So in summary:

• If we default to excludeGet, and
• only issue the cookies when needed

Then we'd be okay? I defaulted to not excluding GET because the number of times I see GET endpoints doing actual state-changing things is enormous, but if it's annoying we can remove it.

There are other approaches that OWASP suggests, which might also be alternatives. All of them seem to have problems, or involve at least as much input from the users as sending the token back. I myself don't feel super comfortable removing the XSRF token and doing nothing else.

Yeah, if we resolve those two and align with Django/Rails that would be fine.

I agree about stateful GETs, but hopefully with Cloudflare and other CDN services those are making them less common.

[domenkozar]

I've writte the following middleware for those wanting to improve their cache-ability of servant responses:

module Cachix.Server.RemoveXSRFHeaderMiddleware where 

import Network.HTTP.Types   (Header)
import Network.Wai          (Middleware, modifyResponse, mapResponseHeaders)
import Data.ByteString (take)
import Protolude

middleware :: Middleware
middleware = modifyResponse $ mapResponseHeaders deleteXsrfCookie

deleteXsrfCookie :: [Header] -> [Header]
deleteXsrfCookie [] = []
deleteXsrfCookie (header : headers) = 
  let
    isXSRF ("set-cookie", value) = Data.ByteString.take 13 value == "NO-XSRF-TOKEN"
    isXSRF _ = False
  in if isXSRF header then headers else header : deleteXsrfCookie headers