salcho commented on pull request #439: URL: https://github.com/apache/wicket/pull/439#issuecomment-670427420
Hi @papegaaij , @svenmeier, @martin-g & @eozmen410 I notice that different reviewers have conflicting designs in mind. This conflict comes from the fact that @papegaaij's applications use the current `CsrfPreventionRequestCycleListener` not so much as a mitigation against CSRF attacks, but rather as an authorization or ACL component that exposes applications/pages/handlers to a set of whitelisted origins. Unfortunately, this is not what Fetch Metadata was designed for (and @svenmeier correctly points this out), but rather to simply reject cross-site requests to endpoints that aren't meant to be used cross-site, regardless of the source origin. At this point, we can make changes to use Fetch Metadata as both a CSRF protection and an ACL component, but I believe we would be making Wicket a disservice by creating a security module that is: trivially bypassable (send a [CORS-safelisted request](https://fetch.spec.whatwg.org/#simple-header) to avoid setting the Origin header, which makes the check undecidable) and very hard to maintain, since the policies would be order-dependent and inter-dependent. We are very happy to contribute to Wicket, but unfortunately we only have a limited number of resources to do so. Having said that, I have a proposal that I think can reconciliate both of these approaches: - Revert all changes made to `CsrfPreventionRequestCycleListener` - Contribute to Wicket with a pure `FetchMetadataRequestCycleListener` that is completely independent from the `Origin` header, providing effective protection against CSRF attacks. - Allow other listeners to compose this new protection (either by composing the new listener or by picking indvidual resource isolation policies). This way Edmond can have full freedom to integrate Fetch Metadata into his module with all the nuances of his use case. Our main goal is to provide easy to maintain security mitigations that are effective at keeping users safe in the web platform ([here's an example of this in another Apache project](https://github.com/apache/struts/pull/426)) and I believe this proposal achieves just that. Would Wicket developers be interested in seeing something like this? We would be happy to push another set of changes if so. Thank you all for your patience and I'm looking forward to reading your thoughts! :) ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
