Issue Details (XML | Word | Printable)

Key: JAVASERVERFACES-1028
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Ed Burns
Reporter: mojavelinux
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
javaserverfaces

build before restore no longer working

Created: 09/Mar/09 05:01 PM   Updated: 26/Jun/12 02:49 PM   Resolved: 14/Aug/09 09:07 AM
Component/s: None
Affects Version/s: current
Fix Version/s: 2.0.0-RC1

Time Tracking:
Not Specified

Environment:

Operating System: All
Platform: All


Issuezilla Id: 1,028
Tags:
Participants: driscoll, Ed Burns, Manfred Riem, mojavelinux, Ryan Lubke and syvalta


 Description  « Hide

The recent change to defer building the entire component tree until the render
response phase so only the metadata facet is built during the restore view has
caused the build during restore on postback feature of Facelets to be
deactivated. The plan was to have this feature be the default in JSF 2.0. When
this build occurs, there should be no special handling for the metadata facet
since the assumption is that the build is taking place because the existing one
could not be properly restored (either because it no longer exists in the
session or the session ended).

We have to be careful enabling this feature though, because it compromises the
basic contract of JSF that says the user must first see a view rendered by the
JSF application before being allowed to trigger an event. Once the build during
restore is enabled, it's possible to submit a form in a plain HTML page into the
JSF application without any verification of the source. It's crucial that the
enablement of this feature be accompanied by a secure token being exchanged in
the case of server-side state saving.

It should also be noted that we should really think about generating a more
secure token for the value of the javax.faces.ViewState token used in
server-side saving. The token is entirely too predictable, setting up a
situation where the user's session can be easily hijacked. (The token is
currently "j_id" + number of views in the session).



Ryan Lubke added a comment - 09/Mar/09 09:53 PM

Update target milestone.


driscoll added a comment - 26/Mar/09 03:59 PM

Change target milestone to post-beta.


syvalta added a comment - 27/May/09 12:23 AM

There seems to be good arguments against this feature in
http://www.seamframework.org/Documentation/CrossSiteRequestForgery.


Ryan Lubke added a comment - 27/May/09 07:39 AM

The portion of the document I believe you're referring to doesn't take into
consideration that Mojarra has a configuration option to detect stale client state.

The context init parameter, com.sun.faces.clientStateTimeout, when set, will
essentially provides the same functionality that the session timeout.

Here are the docs on the config option:

"This specifies the maximum time (in seconds) that client state will be
considered valid by the default StateManager/ResponseStateManager
implementations. If the time between requests exceeds the configured time, a
javax.faces.application.ViewExpiredException. will be thrown. It is important to
note that if this feature is enabled, and client requests are recieved with view
state produced from a previous version, the ViewExpiredException will be thrown
immediately."


driscoll added a comment - 09/Jul/09 03:26 PM

Move to new milestone


Ryan Lubke added a comment - 10/Aug/09 03:00 PM

Assigned


Ed Burns added a comment - 11/Aug/09 12:28 PM

Take ownership


Ed Burns added a comment - 11/Aug/09 12:51 PM

DA> Once the build during restore is enabled, it's
DA> possible to submit a form in a plain HTML page into the JSF
DA> application without any verification of the source.

I assert that this is not a problem because the only way to get a
value into JSF is by having a component in the tree that pulls the value
out of the query string or POST data. Furthermore, the component can't
do that until the data has been validated and converted.

DA> It's crucial that the enablement of this feature be accompanied by a
DA> secure token being exchanged in the case of server-side state
DA> saving.

DA> It should also be noted that we should really think about generating
DA> a more secure token for the value of the javax.faces.ViewState token
DA> used in server-side saving.

I assert these above two paragraphs constitute a separate issue each.
The former has been added to [spec-559], the latter [impl-1248].

Finally, I'll investigate that build before restore behaves as you state
and report back on this bug.

Dan, let me know your thoughts on my comments above.


mojavelinux added a comment - 11/Aug/09 01:14 PM

Sounds good to me. I believe that the spec issue report 559
(https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=559)
covers my main concern about security, so as soon as the defect is solved, I'm
content with calling this issue closed.


Ed Burns added a comment - 14/Aug/09 07:40 AM

I have a testcase that demonstrates that build before restore on postback DOES work as expected in
the default case, which is when you use the JSF 2.0 built in facelets.

Here is the gist of the testcase.

1. declare two system event listeners that give us insight into the timing of things:

<system-event-listener>
<system-event-listener-class>com.sun.faces.event.PostAddToViewListener</system-event-
listener-class>
<system-event-class>javax.faces.event.PostAddToViewEvent</system-event-class>
<source-class>javax.faces.component.html.HtmlInputText</source-class>
</system-event-listener>
<system-event-listener>
<system-event-listener-class>com.sun.faces.event.PostRestoreStateListener</system-event-
listener-class>
<system-event-class>javax.faces.event.PostRestoreStateEvent</system-event-class>
<source-class>javax.faces.component.html.HtmlInputText</source-class>
</system-event-listener>

2. Define the processEvent() implementations to store a message in request scope.

PostAddToViewListener:

public void processEvent(SystemEvent event) throws AbortProcessingException { Map<String, Object> requestMap = FacesContext.getCurrentInstance().getExternalContext().getRequestMap(); requestMap.put("message", event.getClass().getName()); }

PostRestoreStateListener

public void processEvent(SystemEvent event) throws AbortProcessingException { Map<String, Object> requestMap = FacesContext.getCurrentInstance().getExternalContext().getRequestMap(); String message = requestMap.get("message").toString(); requestMap.put("message", message + " " + event.getClass().getName()); }

3. make sure the events only get triggered on postback, by adding this isListenerForSource to both
listeners.

public boolean isListenerForSource(Object source) { Map<String, String> requestParameterMap = FacesContext.getCurrentInstance().getExternalContext().getRequestParameterMap(); return (requestParameterMap.containsKey("activatePostAddToViewListener") && source instanceof UIComponent); }

4. print out the message in the page. Note that because you wanted to know about postback we make
sure to only show the message on postback with our silly little f:param.

<h:form prependId="false">
<p>Enter first name: <h:inputText id="input" value="#{test1.stringProperty}" /></p>

<p>Message: #{message}</p>

<p><h:commandButton value="submit">
<f:param name="activatePostAddToViewListener" value="t" />
</h:commandButton></p>
</h:form>

Now, Dan, I did find a bug that caused the PostRestoreStateEvent to not be fired, which I've fixed, but
it's not checked in yet. I need to talk to Ryan first.


Ed Burns added a comment - 14/Aug/09 09:07 AM

Fix checked in.


Manfred Riem added a comment - 13/Mar/12 03:22 PM

Closing issue out