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

Rack::Protection together with Rails >=5.2.4.3 & >= 6.0.3.1 - CSRF check is broken #1616

Closed
mobilutz opened this issue May 20, 2020 · 6 comments · Fixed by #1653
Closed

Rack::Protection together with Rails >=5.2.4.3 & >= 6.0.3.1 - CSRF check is broken #1616

mobilutz opened this issue May 20, 2020 · 6 comments · Fixed by #1653

Comments

@mobilutz
Copy link

mobilutz commented May 20, 2020

Because of a CVE, rails changed their CSRF handling a little with the version from above, see for example the change in 5.2.4.3:
rails/rails@d124f19

Now, Rack::Protection::AuthenticityToken and rails don't do the exact same thing anymore, which breaks some of our application as some they are Rack apps.

I already have a working prototype, I just need to polish this a bit and create a PR out of it.

I just wanted to make you aware, that the CSRF handling between rails and Rack::Protection is different now.

@jkowens
Copy link
Member

jkowens commented Aug 8, 2020

@mobilutz thanks for filing this report. We'll get work on getting this fixed.

@jkowens
Copy link
Member

jkowens commented Aug 8, 2020

Btw if you do have a PR in the works feel free to send it up 😉

@jkowens
Copy link
Member

jkowens commented Aug 10, 2020

@mobilutz after further investigation, it looks like Rack::Protection::AuthenticityToken based it's csrf protection on Rails methodology, but never guaranteed they would be interoperable. I'm interested to hear more about how you configured that since Rack::Protection::AuthenticityToken uses a different session key for the csrf token than Rails.

@mobilutz
Copy link
Author

@jkowens As we are using Rack-Protection for a grape integration in a bigger Rails apps, we can easily use Rails' code.
We are monkey-patching the code for now 😕

@jkowens
Copy link
Member

jkowens commented Aug 11, 2020

I think the question is should we add per form authenticity token capabilities. If we do that, I can see the justification in following suite and also adding the HMAC hashing. This PR describes why it was added to Rails CSRF protection:

rails/rails#22275

@namusyaka do you think we should consider adding per form authenticity tokens as an optional capability?

@jkowens
Copy link
Member

jkowens commented Oct 10, 2020

@mobilutz I've opened PR #1653 that should address this request. If you have a chance to test out my branch that would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants