Hi Cris, First I want to mention that I don't have a problem with any of those approaches if it solves a problem for you and doesn't introduce any dangerous side effects into the sling framework. As far as I am concerned, the sling components are part of a framework that you are free to assemble in any way you see fit. And if a security component implemented as a filter that performs CRSF protection for all requests is what satisfies your requirements, then it isn't my place to tell you to do something else.
I'll try to give my thoughts for some of your questions below: I think a CSRF framework should check all requests based on the request's > REST semantics and framework configuration. Sling is a REST framework, and > the definition of POST requests (as I understand) it implies posting a new > resource to persist in the system. I’m particularly interested in adding > CSRF before the Sling POST Servlet. Such that if the CSRF filter is enabled > for particular path and http method, then it won't matter what servlet > handles the request down the line. You could make the case that the base Sling is primarily promoted as a REST framework, but it is also capable of doing other things and often does. A POST request may do things other than persisting resources to the content repository. There could be many servlets in a sling environment that are entirely read-only (by design) and may have no need for the overhead of CSRF checking. If that isn't the case for your project, then that can inform your decision. I’m familiar with the Synchronizer Token Pattern, which is I’ve seen in > WordPress and Spring. But it might have a few downsides: > > 1) It is stateful. Sling seems to value being a stateless architecture. > Well I suppose if you want a strict interpretation, but I would argue that it would barely require any state. In the simplest implementation, I believe we are talking about a single string stored per user. I could make the case that this is exactly the same amount of state that is required in your Double Submit Cookie approach. The state would just be stored in a property of the server-side user profile instead of in a cookie value (+ request parameter?) and save a few bytes passing cookies around. Additionally, storing this small amount of "cluster-wide shared" state in the content repository may help you avoid requiring sticky sessions in your load balancer if you have a cluster of sling instances. Also, if I understand the Double Submit Cookie technique correctly, it would involve refactoring every page that renders a UI form (or javascript that does ajax) to inject an additional request parameter into the request (to compare to the cookie value) and the value of that parameter that you render in your UI pages (or javascript) would have to come from state stored somewhere, right? 2) CSRF protection should extend to the Sling POST Servlet but I don’t want > modify that or require devs for all the endpoints that might change state > to use CSRF token checking annotations. Chances are greater that an older > servlets might be open in that circumstance. I would rather have a CSRF > framework offering broad protection even for legacy servlets. > Ok, but it doesn't seem like much extra work to me and it should be a one-time task. Plus, the exercise of doing the work to discover what servlet endpoints are accessible could be an educational experience that could reveal other problems. But that would be your choice to make. In any case, comprehensive testing would be needed to verify that all the possible scenarios are covered without introducing new side effects. 3) Other Java systems that have implemented CSRF use Java Filters [2]. The > Spring and the OWASP example filters are two examples. > Yes that is true, but those systems also do not have the built-in advantage that you have with sling of having a content repository that makes it trivial to get/set/store the "cluster-wide shared" state that would be required. That's all I have for now. Best Regards, -Eric On Wed, Sep 22, 2021 at 8:54 AM Cris Rockwell <[email protected]> wrote: > Hi Eric > > Thank you for the thoughtful reply. > > > Do you think that CSRF protection would be necessary for all POST > requests > > directed toward your resources? > > I think a CSRF framework should check all requests based on the request's > REST semantics and framework configuration. Sling is a REST framework, and > the definition of POST requests (as I understand) it implies posting a new > resource to persist in the system. I’m particularly interested in adding > CSRF before the Sling POST Servlet. Such that if the CSRF filter is enabled > for particular path and http method, then it won't matter what servlet > handles the request down the line. > > > Perhaps some solution that uses the Synchronizer Token Pattern would be a > > more surgical option for specific forms and not require cookies? > > I’m familiar with the Synchronizer Token Pattern, which is I’ve seen in > WordPress and Spring. But it might have a few downsides: > > 1) It is stateful. Sling seems to value being a stateless architecture. > > 2) CSRF protection should extend to the Sling POST Servlet but I don’t > want modify that or require devs for all the endpoints that might change > state to use CSRF token checking annotations. Chances are greater that an > older servlets might be open in that circumstance. I would rather have a > CSRF framework offering broad protection even for legacy servlets. > > 3) Other Java systems that have implemented CSRF use Java Filters [2]. The > Spring and the OWASP example filters are two examples. > > Given this, do you think the stateful servlet solution as described is > better than a stateless filter solution? If so please elaborate. > > Thanks! > Cris > > > [1] > https://events.static.linuxfound.org/sites/events/files/slides/ApacheConNA-ApacheSling.pdf > < > https://events.static.linuxfound.org/sites/events/files/slides/ApacheConNA-ApacheSling.pdf> > > [2] > https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/web/csrf/CsrfFilter.html > < > https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/web/csrf/CsrfFilter.html > > > > > > > > On Sep 21, 2021, at 3:16 PM, Eric Norman <[email protected]> wrote: > > > > Do you think that CSRF protection would be necessary for all POST > requests > > directed toward your resources? I would expect that CSRF protection > would > > typically only be required for actions that could change server side > > state. I guess my question is whether the determination about whether to > > check the CSRF token would be determined by the target resource or the > > target servlet handling the request. > > > > Perhaps some solution that uses the Synchronizer Token Pattern would be a > > more surgical option for specific forms and not require cookies? I've > > used something like that in the past (not directly related to sling) and > > the implementation is pretty straightforward. > > > > For example, something like this: > > 1. Create a CSRFTokenFactory service that gets or generates a unique CSRF > > token for the current user (perhaps the expirable token value state > stored > > in the user profile for checking later?) > > 2. Annotate (or set a property via configuration?) each servlet that > > requires CSRF checking > > 3. Include a hidden CSRF field in your UI forms (or add a custom request > > header via javascript) for all requests directed toward those servlets > > 4. Then the CSRF enabled servlets simply need to do a check before doing > > work that validates the submitted CSRF token is valid before doing > anything > > 5. (optionally) a GetCSRFToken GET servlet could be made available that > > returns the current CSRF token. That can be useful if you want the same > > CSRF protection when using curl (or equivalent) calls from shell scripts > as > > you can GET the token into a variable and then pass that value along on > > subsequent POST calls within your script. > > > > Regards, > > -Eric > > > > On Thu, Sep 16, 2021 at 8:48 AM Cris Rockwell <[email protected]> > wrote: > > > >> Hi > >> > >> I found an older ticket SLING-3469 [1] requesting CSRF protection. > >> > >> Most examples of CSRF frameworks that I’ve seen go beyond checking the > >> referrer header. These frameworks offer either stateful or stateless > >> solutions. I guess stateless is would be the more preferred option, if > so > >> the OWASP guideline suggests a 'Double Submit Cookie’ approach [2]. > >> > >> To implement that in Sling, I’m proposing a new CSRF Filter with a > >> sling.filter.scope=REQUEST such that it runs after the resource is > >> resolved. The filter checks whether the resource is under a CSRF enabled > >> parent, and if so it does a process similar to what is shown on lines > 104 > >> thru 142 [3]. Setting a header and token for all requests, and rejecting > >> requests having the protected configured methods (i.e. POST) where the > >> header value does not match the cookie value. It’s also possible to > allow > >> certain user agents (such as curl) as this intended to provide > protection > >> from browser-based attacks. > >> > >> There may be a separate issue regarding the SlingReferrerFilter. It has > an > >> exception that always allows referrer.startsWith("app:/“). It allows any > >> AIR apps to bypass the filter. As there’s already a way to add 'Allowed > >> regexp referrers’ allowedRegexReferrers the hardcoded “app:/“ exception > >> could be removed. If that seems fine, I’ll post a new ticket and PR. > >> > >> Let me know what you think about either of these ideas. > >> > >> Thanks > >> Cris R > >> > >> [1] https://issues.apache.org/jira/browse/SLING-3469 < > >> https://issues.apache.org/jira/browse/SLING-3469> > >> [2] > >> > https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html > >> < > >> > https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html > > > >> > >> [3] > >> > https://github.com/righettod/poc-csrf/blob/master/src/main/java/eu/righettod/poccsrf/filter/CSRFValidationFilter.java > >> < > >> > https://github.com/righettod/poc-csrf/blob/master/src/main/java/eu/righettod/poccsrf/filter/CSRFValidationFilter.java > > > >> > >> > >> > >> > >
