Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC][Security] Built-in CSRF protection for firewalls #34004

Closed
ro0NL opened this issue Oct 17, 2019 · 8 comments
Closed

[RFC][Security] Built-in CSRF protection for firewalls #34004

ro0NL opened this issue Oct 17, 2019 · 8 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed) Security

Comments

@ro0NL
Copy link
Contributor

ro0NL commented Oct 17, 2019

This is an alternative proposal for #13464 and #33171, based on #33171 (comment) and #33171 (comment)

Long story short:

security:
  firewalls:
    some:
      csrf:
        allow_self: true # default
        allow_origin: [ trusted.com ]
        allow_session: true # default, used if session is available

I think it could deprecate the form integration, and probably the entire storage/token concept currently provided by Security/Csrf.

The new CSRF protection would be based on the Origin/Referer header.

Imagine restoring a prod DB in your local dev application, which contains links to the prod environment (or e.g. hardcoded URLs in code), based on user navigation you can go from dev to prod without really noticing. Mostly im logged in on both environments and both applications look exactly the same. In this case, we've successfully CSRF attacked ourselfves using GET requests 😓

A global firewall protection would solve it out-of-the-box.

Thoughts?

@xabbuh xabbuh added RFC RFC = Request For Comments (proposals about features that you want to be discussed) Security labels Oct 17, 2019
@rpkamp
Copy link
Contributor

rpkamp commented Oct 17, 2019

Imagine restoring a prod DB in your local dev application, which contains links to the prod environment (or e.g. hardcoded URLs in code), based on user navigation you can go from dev to prod without really noticing. Mostly im logged in on both environments and both applications look exactly the same. In this case, we've successfully CSRF attacked ourselfves using GET requests sweat

This is a different problem, which could be solved using something like sub filter in NGiNX (by replacing the prod domain with the dev domain on all html output) and Apache probably has something similar. You could even do it with onResponse listener that changes prod urls to dev urls in the HTML in the response using preg_replace, and then enable that listener in dev mode only.

The new CSRF protection would be based on the Origin/Referer header.

Those are user provided values, which are easy to temper with. Once a bad actor finds that some domain is exempt from CSRF your door is wide open.

Lastly, CSRF is dead 😉

@ro0NL
Copy link
Contributor Author

ro0NL commented Oct 17, 2019

AFAIK tempering with refererer header means a tempered browser, which is outside CSRF scope.

I agree samesite cookies solve 99% cases perhaps, but having a default protection is still valueable and is stateless.

My point is i dont want to filter etc., but have default protection.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Oct 17, 2019

My 2cts: we deprecate security-csrf in 5.1, that will give 5 more years for samesite to propagate.

@ro0NL
Copy link
Contributor Author

ro0NL commented Oct 17, 2019

remains the question if referral protection is a user concern yes/no

perhaps the config is too much, but a core listener for us to wire would be much appreciated (to keep security details outside as much as possible :))

@rpkamp
Copy link
Contributor

rpkamp commented Oct 18, 2019

What might also help is a browser plugin that makes it clear which environment you're in. I have written one for Chrome and configured such that when I am on a production site it shows a big red div with PRODUCTION and some more interesting information on it (like IP etc).

https://chrome.google.com/webstore/detail/requestinfo-display/acjdjhebifgcpdfefkijjeejjckfahoi

(the div in the screenshot is gray, but you can override all css properties on a per-configuration basis)

@stof
Copy link
Member

stof commented Oct 18, 2019

Hacking Referer indeed requires a tempered browser. But modern browsers allow to restrict the info submitting in the Referer header, including disabling it entirely. So a CSRF solution built on the Referer header could actually be excluding users enabling such privacy feature in their browser. Deprecating cookie-based a bad idea IMO.

@ro0NL
Copy link
Contributor Author

ro0NL commented Oct 19, 2019

@stof agree, from the config pov it could be like allow_empty: true + allow_from_cookie/session: false

@rpkamp nice :) but it's a dev-tool somewhat, i.e. i cant enforce it on e.g. content management department.

@ro0NL
Copy link
Contributor Author

ro0NL commented Sep 10, 2021

the long-term goal is to remove the component

@ro0NL ro0NL closed this as completed Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed) Security
Projects
None yet
Development

No branches or pull requests

6 participants