Replace CRSF token with SameSite=strict

[silverwind]

SameSite=strict effectively prevents Cookie-based CRSF attacks and it also brings the benefit of simplifying our code. From Wikipedia:

An additional "SameSite" attribute can be included when the server sets a cookie, instructing the browser on whether to attach the cookie to cross-site requests. If this attribute is set to "strict", then the cookie will only be sent on same-origin requests, making CSRF ineffective.

Browser support is pretty good on it. It also means cookies will never be send to other domains like when STATIC_URL_PREFIX is set differently, but as far as I'm aware, cookies are not needed for static assets.

Related: #5583

[silverwind]

There is one concerning bit in the RFC draft which distinguishes a "registrable domain":

A request is "same-site" if its target's URI's origin's registrable domain is an exact match for the request's initiator's "site for cookies", and "cross-site" otherwise.

Unless I'm missing something, CRSF from sub1.example.com to sub2.example.com would still possible with that, thought I guess the risk is much lower in such a scenario.

[lafriks]

There is still problem if Gitea is hosted in subdir with other apps

[silverwind]

@lafriks indeed, but I'd say that's a rare scenario and most of today's web's security relies on origins so they would be vulnerable to other attack vectors as well. Maybe we should just add a note that running on subdirectory is dangerous and should be avoided.

[guillep2k]

I'd rather not rely on draft recommendations for security enforcement. Especially since older browsers will just not implement it.

[silverwind]

I don't know why it's still draft but it's implemented since 4 years so it should be rather stable. Browsers are in process of enabling the lax setting for all sites (Chrome Status) which further reinforces the stability of that feature.

[CirnoT]

There is a very limiting factor to Strict cookies - if a link to Gitea issue or PR is provided on external site, clicking it will be seen as cross-site request and user will be logged out upon following it.

Using Lax will ensure they are sent on cross-site request, but only for protocols deemed safe, so usually GET only (note that this does include forms if their methods is GET)

The subdomains are also an issue, imagine a situation where smaller company sets up web email and Gitea on two subdomains and user receives phishing email.

[mohe2015]

According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie subdomains don't get the cookies if no domain is set.

I think explicitly setting SameSite=Lax should be enough. According to https://web.dev/samesite-cookies-explained/: "The default behaviour applied by Chrome is slightly more permissive than an explicit SameSite=Lax as it will allow certain cookies to be sent on top-level POST requests"

[CirnoT]

Lax is certainly a good idea to implement alone, but not as direct replacement for proper CSRF tokens as it still allows for misuse of GET forms via cross-site requests and depends on user-agent support.

[mohe2015]

According to https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html: "Do not use GET requests for state changing operations."

So I think these occurrences should be changed then.

Non state-changing should be fine AFAIK.

[silverwind]

Pretty sure we have some non-semantic GETs which may block us to some degree here. One such example is #12403.

[DuckDuckWhale]

#12403 and its original issue #10725 seems to be fixed. Any updates?

[silverwind]

Some work had been done to get SameSite support into macaron but I'm not sure where it stands currently as there's also the migration to chi ongoing.

[lunny]

https://gitea.com/macaron/csrf/commit/9889eb38af52ef888ac7c44ebf739b728647ca75 hasn't been ported to Gitea based chi.

[zeripath]

#14900 will make all our cookies SameSite lax by default

[zeripath]

I guess the next step would be then to add configuration to drop the csrf but leave it on unless a server admin wants it off.

[CirnoT]

CSRF tokens should still be observed along with cookie configuration. Removal (or even adding option for it) of it would be a very premature action considering browsers are still not yet fully enforcing new cookie configuration in various edge scenarios as to not break the hell out of everything and we have to deal with multiple LTS releases such as Firefox ESR being used.

ref. https://medium.com/@renwa/bypass-samesite-cookies-default-to-lax-and-get-csrf-343ba09b9f2b

[mohe2015]

What about SameSite strict or a combination (lax for GET and both for POST as far as I can remember)?

[silverwind]

CSRF removal would be nice to have to eliminate a few bugs we have around it. I'm not sure how those bugs exactly but there are ways CRSF tokens will be rejected by legimitate clients. I guess one way to reproduce may be to open form, restart server, send form. I guess it only is feasible once we enforce strict mode.

[jub0bs]

I'm not part of the Gitea community, but I urge you not to rely solely on SameSite to the detriment of anti-CSRF tokens. The SameSite cookie attribute is powerless against cross-origin, same-site attacks, and many orgs do not scrutinize the security level of their subdomains.

@silverwind That section from Wikipedia that you quoted was incorrect and has since been corrected. It now reads:

An additional "SameSite" attribute can be included when the server sets a cookie, instructing the browser on whether to attach the cookie to cross-site requests. If this attribute is set to "strict", then the cookie will only be sent on same-site requests, making CSRF ineffective.

(my emphasis)

@mohe2015 Whether the Domain attribute was set or not in the Set-Cookie header is irrelevant. Requests issued from same-site origins to the target origin will carry all relevant cookies associated with that origin domain.

[mohe2015]

I'm not part of the Gitea community, but I urge you not to rely solely on SameSite to the detriment of anti-CSRF tokens. The SameSite cookie attribute is powerless against cross-origin, same-site attacks, and many orgs do not scrutinize the security level of their subdomains.

Great so now we have an RFC with a security feature that is not a security feature. Why is there no SameOrigin cookie-option...