Can't get CSRF protection to work on CXF

[mthmulders]

Setup:

• OpenLiberty 18.0.0.2
• Java EE 8
• Ozark 1.0.0-m04 SNAPSHOT (20180903.132704-47)
• MVC 1.0 SNAPSHOT (20180902.064053-77)

I'm trying to get CSRF protection to work on my controller. It looks like this:

    @CsrfProtected
    @POST
    @Path("/configure")
    @View("redirect:/hello")
    public void configure(@FormParam("locale") final String locale) {
        // left out
    }

If I read the spec correctly, the default for javax.mvc.security.CsrfProtection is EXPLICIT, saying that any method annotated with @CsrfProtected is to be protected.

I have a page that submits an HTML form to this controller method using POST, and it lacks the CSRF token parameter. I would expect the form submission to be rejected, but my controller method is hit.

Browsing through Ozark source code, I expect it is the responsibility of the CsrfValidateInterceptor class to perform this check. Using the debugger, I can see it is being instantiated, but the breakpoint in aroundReadFrom(...) is never hit.

Again, reproduction repo is https://github.com/mthmulders/openliberty-mvc. Any clues?

[chkal]

Yes, you are correct. The method should actually be processed by CsrfValidateInterceptor and the controller should not be invoked if the CSRF token is missing. That's really weird.

CsrfValidateInterceptor is registered via a name binding, which means that it should actually be invoked for all requests to controller methods (annotated with @Controller).

Could you please check if the interceptor registration works by confirming that this code gets executed:

https://github.com/mvc-spec/ozark/blob/master/core/src/main/java/org/mvcspec/ozark/bootstrap/DefaultConfigProvider.java#L45

There is also some special handling for CXF here:

https://github.com/mvc-spec/ozark/blob/master/core/src/main/java/org/mvcspec/ozark/bootstrap/DefaultConfigProvider.java#L56

But my guess is that this CXF workaround isn't the cause of your issues.

My only other guess is that the name binding isn't working as expected for some reason. I ran into issues regarding the name binding before, so this is just a wild guess, but it may be possible.

[mthmulders]

I can confirm that DefaultConfigProvider.java#L45 is being hit. Also, inside register(FeatureContext, Class), isCxf equals true, and providerInstances is an ArrayList with one element in it, an instance of CsrfValidateInterceptor. This object is then registered with the feature context.

[chkal]

Thanks for confirming. Looks like we need to do some more debugging to solve this. Unfortunately there are a few weird issues with CXF like #168 for example.

I'll try to setup some test environment with plain Tomcat + CXF + Ozark to see if I can find the root cause for this.

Of course any kind of help is welcome!

[mthmulders]

Yesterday I've tried to debug OpenLiberty itself - to no avail. I was just about to resort to plan B, which is exactly what you've proposed. I'm far from an expert in CXF, but I'll see what I can find...

[chkal]

Ok, great! Any kind of help is welcome.

My current guess is that the name binding @Controller on the CsrfValidateInterceptor doesn't work as expected for some reason. Maybe because CDI is involved. But that's just a wild guess. Anyway, it is worth a try to just remove @Controller from CsrfValidateInterceptor which should execute the interceptor for all JAX-RS requests, not just for MVC ones.

[mthmulders]

Just to make sure... does it work in other JAX-RS implementations? I've found RESTfu­­l Jav­a­ wit­h ­JAX­-­­RS 2.­0­ (Second Edition), that says:

These interceptors are only triggered when a MessageBodyReader or MessageBodyWriter is needed to unmarshal or marshal a Java object to and from the HTTP message body.

If that was true, I'd expect that other JAX-RS implementations also suffer from this. In this particular case, the controller method isn't reading Java objects directly from the HTTP message body.

[chkal]

It is definitely working on Glassfish and AFAIK also on Wildfly. My interpretation of this spec requirement is that the reader is needed because you are actually using @FormParam in your code to access form parameters which got submitted in the body. No?

[mthmulders]

Well, of course I hoped that it would work in other JAX-RS implementations :-). The document I referred to isn't a specification, so I don't know it's status. I interpreted it as "when a message body is read to a POJO" - like when a JSON structure is sent and the controller method has a method argument of a corresponding Java structure.

I'll try a bit more debugging when/how these interceptors are invoked in CXF in the coming days.

[chkal]

Thanks for the clarification. Well, actually it may be possible that different JAX-RS implementations handle ReaderInterceptor invocation differently.

The exact wording in the JAX-RS spec is:

An entity interceptor implementing ReaderInterceptor wraps around calls to MessageBodyReader ’s
method readFrom . An entity interceptor implementing WriterInterceptor wraps around calls to
MessageBodyWriter ’s method writeTo . JAX-RS implementations are REQUIRED to call registered
interceptors when mapping representations to Java types and vice versa.

It would be interesting to know how CXF handles this requirement.

Also, it may be worth a try to add a parameter of type javax.ws.rs.core.Form to your controller method. If CXF doesn't execute the ReaderInterceptor because there is no Java type to map, this may force it to do so.

[mthmulders]

Bingo! Adding an javax.ws.rs.core.Form parameter to the controller method fires the CsrfValidateInterceptor.

By the way, paragraph 3.3.2.1 of the JAX-RS 2.1 spec suggests that a ReaderInterceptor should be executed for @FormParam method arguments:

The value of a parameter not annotated with @FormParam or any of the annotations listed in in Section 3.2, called the entity parameter, is mapped from the request entity body. Conversion between an entity body and a Java type is the responsibility of an entity provider, see Section 4.2. Resource methods MUST have at most one entity parameter.

I did a quick search in the CXF issue tracker, but can't find anything related to this issue.

What would be the way to go here? File an issue with CXF and see if they agree upon our interpretation of the spec at this point?

[chkal]

Ok, thanks a lot! That's interesting!

I just asked for clarification about this on the jaxrs-dev list. But I agree that CXF is most likely doing it wrong. However, as always, such statements in the spec are up to interpretation. 😁