4

I'm Using MyFaces 2.2.3 with client side state saving + PrimeFaces

After asking how to prevent the re-use of a ViewState in different sessions I was told by BalusC , that I can inject my own CSRF token by override the from renderer to let the value be a CSRF token ,

I'm looking for a solution that wont force me to modify my xhtml pages at all :)


BalusC has suggested a better way to prevent CSRF attack by extending ViewHandlerWrapper , and it works great, I only had to modify a bit the restoreView in the following way

public UIViewRoot restoreView(FacesContext context, String viewId) {
    UIViewRoot view = super.restoreView(context, viewId);
    if (getCsrfToken(context).equals(view.getAttributes().get(CSRF_TOKEN_KEY))) {
        return view;
    } else {
        HttpSession session = (HttpSession) context.getExternalContext().getSession(false);
        if (session != null) {
            session.invalidate(); //invalidate session so (my custom and unrelated) PhaseListener will notice that its a bad session now
        }
        try {
            FacesContext.getCurrentInstance().getExternalContext().redirect("CSRF detected and blocked"); //better looking user feedback
        } catch (IOException e) {
            e.printStackTrace();
        }
        return null;
    }
}

Old solution

I tried without success so far,

Added to faces-config.xml

<render-kit>
    <renderer>
        <component-family>javax.faces.Form</component-family>
        <renderer-type>javax.faces.Form</renderer-type>
        <renderer-class>com.communitake.mdportal.renderers.CTFormRenderer</renderer-class>
    </renderer>
</render-kit>   

Then in CTFormRenderer.java

@Override
public void encodeEnd(FacesContext context, UIComponent arg1) throws IOException {
    //how to set form value be a CSRF token?
}

@Override
public void decode(FacesContext context, UIComponent component) {
    HttpSession session = (HttpSession) context.getExternalContext().getSession(false);
    String token = (String) session.getAttribute(CSRFTOKEN_NAME);
    String tokenFromForm = //how to get the value stored in form value attribute because (String) component.getAttributes().get("value"); return null
    //check token against tokenFromForm...
}

I don't want to add a custom component to each and every h:form, instead I want to extend the form renderer so all my form will have the csrf token.

Community
  • 1
  • 1
Daniel
  • 36,833
  • 10
  • 119
  • 200
  • does it work without PrimeFaces, in plain jsf (just to be sure)? If it does not, does it work with Mojarra? – Kukeltje May 29 '15 at 14:48
  • I'm not entirely clear on your direction here: "I can inject my own CSRF token by override the from renderer to let the value be a CSRF token " does this mean you want to specify the CSRF token by hand? You know it has to correspond to what the server generated originally right? – kolossus May 29 '15 at 15:16
  • @kolossus , you can read more about why I need this solution in my previous question and BalusC answer http://stackoverflow.com/questions/30373089/reusing-viewstate-value-in-other-session-csrf – Daniel May 29 '15 at 21:33
  • Are you planning to generate this token also? Or you want to retrieve the stock JSF value? – kolossus Jun 01 '15 at 13:08
  • I'm planning to generate it by myself – Daniel Jun 01 '15 at 13:13
  • How is what you're planning to do any different from what JSF2.2 provides by way of protected views and [`ResponseStateManager.NON_POSTBACK_VIEW_TOKEN_PARAM`](http://docs.oracle.com/javaee/7/api/javax/faces/render/ResponseStateManager.html#NON_POSTBACK_VIEW_TOKEN_PARAM) – kolossus Jun 01 '15 at 17:40
  • Then a viewhandler would be a cleaner place do this IMHO @BalusC – kolossus Jun 01 '15 at 19:44
  • But won't that (a combined viewstate-csrftoken) leave him where he started, i.e. having to pass the token back and forth between client and server, where anyone can just copy-paste @BalusC? – kolossus Jun 01 '15 at 20:14
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/79363/discussion-between-kolossus-and-balusc). – kolossus Jun 01 '15 at 21:04
  • @kolossus But my custom csrf token will be accepted by that session in which it was created, while the ViewState token can be accepted by any session – Daniel Jun 02 '15 at 06:42
  • Check https://stackoverflow.com/questions/26886121/how-to-enable-csrf-protection-in-jsf-spring-integrated-application/68196626#68196626. I hope it will helpful. – Samet nargül Jun 30 '21 at 14:28

1 Answers1

7

This <h:form> renderer override approach is not safe to PrimeFaces partialSubmit="true". Also, reusing its hidden field identifying the submitted form would be JSF implementation specific as this is not part of JSF API.

On a second thought, it's much simpler to store the CSRF token just directly in the JSF view state itself. You can achieve that with a custom ViewHandler as below which sets an attribute in UIViewRoot (which get automatically saved in JSF view state):

public class CsrfViewHandler extends ViewHandlerWrapper {

    private static final String CSRF_TOKEN_KEY = CsrfViewHandler.class.getName();

    private ViewHandler wrapped;

    public CsrfViewHandler(ViewHandler wrapped) {
        this.wrapped = wrapped;
    }

    @Override
    public UIViewRoot restoreView(FacesContext context, String viewId) {
        UIViewRoot view = super.restoreView(context, viewId);
        return getCsrfToken(context).equals(view.getAttributes().get(CSRF_TOKEN_KEY)) ? view : null;
    }

    @Override
    public void renderView(FacesContext context, UIViewRoot view) throws IOException, FacesException {
        view.getAttributes().put(CSRF_TOKEN_KEY, getCsrfToken(context));
        super.renderView(context, view);
    }

    private String getCsrfToken(FacesContext context) {
        String csrfToken = (String) context.getExternalContext().getSessionMap().get(CSRF_TOKEN_KEY);

        if (csrfToken == null) {
            csrfToken = UUID.randomUUID().toString();
            context.getExternalContext().getSessionMap().put(CSRF_TOKEN_KEY, csrfToken);
        }

        return csrfToken;
    }

    @Override
    public ViewHandler getWrapped() {
        return wrapped;
    }

}

Do note that when restoreView() returns null, JSF will throw a ViewExpiredException "as usual".

To get it to run, register as below in faces-config.xml:

<application>
    <view-handler>com.example.CsrfViewHandler</view-handler>    
</application>

Because it has no additional value with server side state saving, you can if necessary detect as below in the constructor of the view handler if the current JSF application is configured with client side state saving:

FacesContext context = FacesContext.getCurrentInstance();

if (!context.getApplication().getStateManager().isSavingStateInClient(context)) {
    throw new IllegalStateException("This view handler is only applicable when JSF is configured with "
        + StateManager.STATE_SAVING_METHOD_PARAM_NAME + "=" + StateManager.STATE_SAVING_METHOD_CLIENT);
}
BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • Thanks! looks exactly what I needed, Will test it tomorrow :) b.t.w is it ok to redirect to some custom error page in case of not equals instead of returning null and showing the ViewExpiredException page? – Daniel Jun 02 '15 at 20:14
  • Checked it and despite the fact that the `getCsrfToken(context).equals(view.getAttributes().get(CSRF_TOKEN_KEY))` is false and a null is returned in the restoreView , jsf **still proceed to the `action` method** of the `f:ajax` and the extenral html still manages to post its data, But if I replace the `return null;` with `FacesContext.getCurrentInstance().getExternalContext().redirect("somePage.jsf"); return null` it **will not proceed to action method** – Daniel Jun 03 '15 at 10:33
  • Hm, are there more view handlers in the application? – BalusC Jun 03 '15 at 10:40
  • No, its the only one , and actually the redirection serve me as a `flow hijack` its not that it actually redirects because the external form submission returns an xml and the browser shows: `This XML file does not appear to have any style information associated with it. The document tree is shown below. ` – Daniel Jun 03 '15 at 10:47
  • Can't reproduce that JSF still proceeds to action method with MyFaces 2.2.8. Which context param settings do you use? – BalusC Jun 03 '15 at 11:25
  • Here is my *context-param* pastebin.com/1wDxVaMu and b.t.w with 2.2.8 it still proceeds to the action method for me :| – Daniel Jun 03 '15 at 11:38
  • Nothing weird there. Also can't reproduce with MyFaces 2.2.3. Are you using some authentication which might conflict with that ajax redirect? In any case, you could manually `throw new ViewExpiredException()` instead of returning `null`. – BalusC Jun 03 '15 at 11:38
  • I got `AuthenticationPhaseListener` that `implements PhaseListener` that checks the session / logged in user validity, but how come the returning `null` from `restoreView` is not enough in my case? – Daniel Jun 03 '15 at 11:50
  • b.t.w `throw new ViewExpiredException()` does not prevent from action to happen, it does print a short stacktrace of *ViewExpiredException* but still proceeds to the action method , so the only thing that worked is to *redirect* :| – Daniel Jun 03 '15 at 12:13
  • I'd put a debug breakpoint in `restoreView()` and explore what exactly the code does after leaving the method with `null`. – BalusC Jun 03 '15 at 12:50
  • In my case, it returns into `org.apache.myfaces.lifecycle.RestoreViewExecutor` where it immediately throws VEE after determining it's `null`: http://grepcode.com/file/repo1.maven.org/maven2/org.apache.myfaces.core/myfaces-impl/2.2.3/org/apache/myfaces/lifecycle/RestoreViewExecutor.java#168 – BalusC Jun 03 '15 at 13:03
  • Did some debugging and in my case it also goes to the `RestoreViewExecutor` and throw a `VEE` , then it goes to `finally{facesContext.setProcessingEvents(true);}` and then in `LifyCycleImpl.java` it goes to `catch (Throwable e){ // JSF 2.0: publish the executor's exception (if any). publishException (e, currentPhaseId, context);}` and then `finally{phaseListenerMgr.informPhaseListenersAfter(currentPhaseId);flash.doPostPhaseActions(context);}` and right after that goes to my PhaseListener and its `afterPhase` method... is it an *OK* flow ? – Daniel Jun 04 '15 at 10:00
  • If that phase listener is performing navigation, then that's fishy. A servlet filter should be used instead. – BalusC Jun 04 '15 at 10:02