Hello Neil, Wouldn't it possible to just avoid adding the token when the application is inside of a portlet environment? --- Kito D. Mann | twitter: kito99 | Author, JSF in Action Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and consulting http://www.JSFCentral.com - JavaServer Faces FAQ, news, and info | twitter: jsfcentral +1 203-404-4848 x3 Sign up for the JSFCentral newsletter: http://oi.vresp.com/?fid=ac048d0e17 On Tue, Sep 21, 2010 at 4:20 PM, Neil Griffin <neil.griffin(a)portletfaces.org

On Tue, Sep 21, 2010 at 1:15 PM, Roger Kitain <roger.kitain(a)oracle.com&gt;wrote: Why? I think it's actually safer to allow both: http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prev.... According to this, appending the token works better when you can't guarantee that a server-side operation is occurring only through POST. Especially with the new capabilities of JSF 2 for parsing view parameters, I don't think we can make this guarantee. Also, the rewriting approach is what they're using for the generic filter in Tomcat 7: http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prev.... I was talking to Mark Thomas, the guy working on this feature, about this at JavaOne. As you can see, it doesn't even look like they're using the hidden field approach (probably because it'd be very hard to implement in a generic fashion). So, I think we should provide both features, but give the users to turn off either algorithm, with a context parameter such as: javax.faces.CSRF_ALGORITHM=NONE | FORM | URL | BOTH (or ALL) Thoughts?

On Mon, Sep 27, 2010 at 9:31 PM, Roger Kitain <roger.kitain(a)oracle.com&gt;wrote: This would use both algorithms -- hidden field and appending the token to the URL. To simplify things, we could probably use the same mechanism for generating the token internally for both, but that should probably be up to the implementation.

I disagree with suggested proposal. There are two pitfalls: a) additional parameter just increases size of generated html code and number of request parameters. JSF forms always contain viewId parameter, so adding encoded token to that field will have minimal side effects. That token should be checked by StateManager before restoring view. b) URL parameter does not make sense for forms, but more important for 'GET' access. To make this parameter compatible with portal environment, it can be passed trough ExternalContext.encodeNamespace method ( while I think that it should be portal vendor concern, where secure token has to protect whole portal page ). c) For token encoded as url parameter this proposal protects whole application, so no one can either got logged in to protected site because of circular dependencies: to open login page, visitor has to have secure token, which one he can get only from JSF login page... There should be per-page security configuration. On 09/30/2010 07:59 PM, Roger Kitain wrote:

Gang - Apologies for being so amazingly late in reviewing this - just haven't had a chance to look until now. Regarding: I think that this assumes that: 1. We are checking GET requests for the token. 2. We need a token for the initial request into the application. Looking at Mojarra's current implementation, these assumptions are not true. From RestoreViewPhase.execute(): Since we only verify the token for POST requests, if we arrive on the login page via a GET, we should be fine. However, if we only verify POST requests, I am not sure that I understand the value of CSRF_ALGORITHM=URL. Any POSTs from an h:form will already contain the token from the hidden form field. Including a query parameter in the form's action seems redundant/unnecessary. Speciyfing that ViewHandler.getActionURL() must include the token query parameter has other undesirable side effects. Both getRedirectURL() and getBookmarkableURL() call getActionURL(). As such, these URLs will end up with the query parameter even though we won't use this query parameter when handling the GET. We end up with noticeably uglier URLs with no gain in security. Earlier Kito mentioned the following justification for supporting the CSRF=URL approach: I think I see Kito's point. However, if we are only verifying the token on POST requests, the CSRF=URL approach doesn't address the view parameter use case. Again, we don't gain anything here. I get the feeling from both Alexander and Kito's comments that there was an assumption that we would be performing token verification for both POST and GET requests. I suppose we can consider doing this. However, as Alexander hinted, this gets tricky fast. If we only expose an option for globally securing all GETs, this means that the application can no longer support bookmarkable URLs (since every URL must contain a token which is only valid for a very limited lifetime). Any attempt to access the application without a valid token (such as a bookmark to the login page) will fail. As such, I agree with Alexander's comment: At least when it comes to the URL/GET-based solution, I don't see how enabling this globally is a useful solution. Based on the current implementation I fail to see the benefits of the CSRF=URL approach. I see at least one fairly significant drawback: unnecessarily uglier redirect/bookmarkable URLs. Unless I am missing something (which might well be the case), I am -1 on including CSRF=URL in the specification. Andy On 10/1/10 1:22 PM, Alexander Smirnov wrote: > I disagree with suggested proposal. There are two pitfalls: > a) additional parameter just increases size of generated html code and > number of request parameters. JSF forms always contain viewId parameter, > so adding encoded token to that field will have minimal side effects. > That token should be checked by StateManager before restoring view. > b) URL parameter does not make sense for forms, but more important for > 'GET' access. To make this parameter compatible with portal environment, > it can be passed trough ExternalContext.encodeNamespace method ( while I > think that it should be portal vendor concern, where secure token has to > protect whole portal page ). > c) For token encoded as url parameter this proposal protects whole > application, so no one can either got logged in to protected site > because of circular dependencies: to open login page, visitor has to > have secure token, which one he can get only from JSF login page... > > On 09/30/2010 07:59 PM, Roger Kitain wrote: > >> Folks - >> >> I have combined the previous two proposals into one based on valuable >> feed back from >> Kito Mann and Neil Griffin: >> >> On 9/27/10 5:12 PM, Kito Mann wrote: >> >>> On Tue, Sep 21, 2010 at 1:15 PM, Roger Kitain <roger.kitain(a)oracle.com >>> <mailto:roger.kitain@oracle.com>> wrote: >>> >>> >>> There are two proposals for enhancing CSRF attacks in JSF. We >>> need to pick one. >>> >>> >>> Why? I think it's actually safer to allow both: >>> http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prev.... >>> According to this, appending the token works better when you can't >>> guarantee that a server-side operation is occurring only through POST. >>> Especially with the new capabilities of JSF 2 for parsing view >>> parameters, I don't think we can make this guarantee. >>> >>> Also, the rewriting approach is what they're using for the generic >>> filter in Tomcat 7: >>> http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prev.... >>> I was talking to Mark Thomas, the guy working on this feature, about >>> this at JavaOne. As you can see, it doesn't even look like they're >>> using the hidden field approach (probably because it'd be very hard to >>> implement in a generic fashion). >>> >>> So, I think we should provide both features, but give the users to >>> turn off either algorithm, with a context parameter such as: >>> >>> javax.faces.CSRF_ALGORITHM=NONE | FORM | URL | BOTH (or ALL) >>> >>> >> On 9/21/10 4:20 PM, Neil Griffin wrote: >> >>> #1 is probably not compatible with portlets. Portals are in full >>> control of creation of URLs in general, and it is not possible to >>> simply append "&javax.faces.Token=XYZ" to a portal's ActionURL and >>> expect it to work. >>> >>> #2 is compatible with portlets, but it would be best to have the >>> hidden field namespaced. Otherwise it would be like >>> javax.faces.VIEW_ID which is not namespaced, and can cause problems in >>> portals. >>> Neil >>> >> ============================================== >> >> This proposal does the following: >> >> - Token is generated on the server consisting (minimally) of a randomly >> generated "secret key" (stored in session). The token can be generated >> upon session creation, or, at the time the token hidden field and/or >> token Url parameter is produced. >> >> - Standard context parameter javax.faces.CSRF_ALGORITHM controls where the >> token is produced by the following values: >> a. "form" : token is produced as the value of the hidden field with the name >> <form client id>:javax.faces.Token where <form client id> is the >> enclosing form's client identifier (produced with getClientId) - i.e. >> the hidden field is namespaced using the form's client identifier. >> b. "url" : token is produced and appended as a parameter to the form's >> action Url with the same name as in [a]. >> c. "all" : token is produced as in [a] *and* [b] >> d. "none" : no token is produced >> The default is [a] "form". >> >> - After render time, a subsequent action will send the token to the server. >> - Restore View Phase processing compares the incoming token request parameter >> value with the token value generated from the secret key in the session. >> >> Spec Document Modifications: >> >> Section 7.5.1: >> >> getActionURL: >> >> The returned URL must contain the >> parameter with a name consisting of the form's fully namespaced >> client identifier, the NamingContainer.SEPARATOR_CHAR and the constant >> defined by ResponseStateManager.VIEW_TOKEN_PARAM, >> if the configuration option javax.faces.CSRF_ALGORITHM >> is set to "url" or "all". The value of this parameter, known >> as the "token value" must be a cryptographically produced random generated >> value (known as the "secret key") retrieved from the session. If the >> "secret key" does not already exist in the session, create the random value >> and store it in the session. Implementations may choose to produce a more >> complex token value by combining the random "secret key" with other values. >> >> >> Section 2.2.1 >> >> "If the value of the configuration option javax.faces.CSRF_ALGORITHM >> is not "none" verify the "javax.faces.Token" request parameter value is >> the same as the token value generated from the "secret key" stored in the >> session. If the values do not match, throw a meaningful exception. >> The exception message must not include the token value." >> >> Standard RenderKit Docs >> >> - Form Rendering >> >> Render a hidden field named javax.faces.Token using the >> ResponseStateManager.VIEW_TOKEN_PARAM constant if the configuration option >> javax.faces.CSRF_ALGORITHM is set to the key words "form" or "all". >> The name must be namespaced with the enclosing form's client identifier. Render >> the value of this hidden field, known as the "token value", as a cryptographically >> produced random generated value (known as the "secret key") retrieved from the >> session. If the "secret key" does not already exist in the session, create the random value and >> store it in the session. Implementations may choose to produce a more complex token value by >> combining the random "secret key" with other values. >> >> Implementation Changes are attached to this issue: >> >> [1] https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=869 >> >> >> -roger >> >> >> >> >> >> >> >> >> >> >> >> -- >> roger.kitain(a)oracle.com >> https://twitter.com/rogerk09 >> http://www.java.net/blogs/rogerk >> >>

Hi Andy, is this the time to finally add a Form interface? please say so ;) best regards, Martin On 10/22/10, Andy Schwartz <andy.schwartz(a)oracle.com&gt; wrote: -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces

On 10/22/10 7:01 AM, Andy Schwartz wrote: Custom forms would need to follow the new rendering requirements for CSRF (described in renderkit docs). In 2.2, we could further simplify this by providing API "hooks". By default, CSRF is not enabled, so existing apps continue to work. -- roger.kitain(a)oracle.com https://twitter.com/rogerk09 http://www.java.net/blogs/rogerk

Hey Andy, all - On 10/21/10 9:53 PM, Andy Schwartz wrote: Understood. That is a correct assumption. Based on the circular dependency "chicken and egg" problem for GET requests, I agree. Based on this feedback I'm ok with rolling back the URL specification and implementation for CSRF. -- roger.kitain(a)oracle.com https://twitter.com/rogerk09 http://www.java.net/blogs/rogerk

Comments inline.. On 9/28/10 2:56 PM, Alexander Smirnov wrote: Yep - I agree that we could possibly augment the view state with some generated random value. Hmm... They are probably many ways to slice and dice this problem. I think we were looking for a more automatic solution then having to introduce more burden on page authors or application developers. -- roger.kitain(a)oracle.com https://twitter.com/rogerk09 http://www.java.net/blogs/rogerk

Roger, Di you answer Alexander's question regarding whether this feature is even necessary? In the current releases, with no explicit CSRF defenses, we have attacks against: 1) GET attack 2) POST attack against a page with token state saving 3) POST attack against a page with client state saving 1) For GET attacks, it appears that we are saying that we aren't going to defend against these anyway for this release. 2) For POST attacks against a page with token state saving. As long as the view state token is cryptographically strong, the attacker can't guess the token anyway. At that point, it comes down to what the behavior is for a request that has an invalid token--we can either return an error page or we can treat the request as a GET. If we treat the request as a GET, we are in 1 3) For POST attacks against a page with client state saving, as long as attackers can't forge the client state, they can't do anything. Since, they can't, we're safe here as well. Therefore, it isn't clear what the feature is buying us. If there is no clear need for the feature, it should not be added to the specification. -- Blake Sullivan On 9/21/10 10:15 AM, Roger Kitain wrote:

On 10/22/10 11:10 AM, Roger Kitain wrote: I think that it is trickier than that--you need to worry about things like your parameter leaking out through the referer header. At this point, I don't see how you would get the spec work done in time. Plus, a well written web application won't have side-effects on the GET anyway. The CSRF attacker can GET all day and not change the application state. Yep. Make it cryptographically strong, as it is in Trinidad and you are done with the server-side storage POST except for figuring out what to do if the token isn't valid. I see no need to add yet another token when the existing one will do. Yep Cool -- Blake Sullivan

On 10/22/10 1:32 PM, Martin Marinschek wrote: Martin, This is a pile of new functionality and the question is, and I'm curious what the application would do with this functionality. What is the problem that the application is trying to avoid? But I don't think it is a per-view. Isn't it potentially per action? It's what the application logic does in response to the request that determines whether the application is idempotent with regards to that particular request. On a side note. If the Mojarra server side state saving token isn't cryptographically strong, that's a security bug and needs to be fixed (and probably backported) regardless. Unless we are talking about adding additional functionality, and it seems late to be doing so for this release, this handles the POST case for server side state saving. The POST case for client side state saving is already handled by the fact that the state is carried in the POST and can't be forged. The main open issues in these cases have to do with how to handles cases where we get and invalid request and thinking about how a user who has backed up off of the backbutton stack would like this to behave versus how a token we have never seen before should behave. This leaves the ever popular GETs. I'm probably being lazy, but at this point I'm willing to punt on GETs because of potential problems with: 1) Worries about referer leakage if the secret is encoded in the URL 2) How to deal with bookmarking 3) General dislike for ugly URLs Admittedly, I think that 2) is the only one that really requires more thought, since I think that the solution to 1) is to a) Only worry about CSRF for pages served through a secure channel b) Require that pages served to authenticated users be served through a secure channel. For 3), I think it's gross but, that's just me :) -- Blake Sullivan

On 10/25/10 11:56 PM, Martin Marinschek wrote: By double submit, which cases: 1) User accidentally presses the button twice 2) User hits back to a POST 3) User hits reload on a POST For refresh handling, what is the use case where you want to deal with reload differently than the initial render? I agree that we need to decide how this should work as well as what should happen if we receive a token we've never seen and that this needs to be part of the 2.1 release. Why can't you do that today? I must admit that I haven't looked at the view-actions enough, but can't I have some view actions that are idempotent and others that aren't. I'm mostly worried about squeezing in something that we haven't fully thought out. I agree that if we added a new per-request token we should look at the rest of the cases. I just don't think we have time to do everything that we would need to for this release. -- Blake Sullivan

On 10/27/10 2:48 AM, Martin Marinschek wrote: OK. Though the first of these is better addressed (or additionally addressed) through the user interface--for example, disabling the button and potentially showing some kind of busy indicator on it. I'm just curious. What kind of message? We already have the issue that the specification should be saying how we handle a view state token for which the server has no view state. If we are saying that the view state token is also protecting POSTs from CSRF attacks, I think we need to make sure that our handling of the invalid token isn't too nice. We can be pretty nice in the case where it is a token that was once valid, but if it is a token we've never seen, we might want to go medieval on the response--for example send back a 400 error. They have been beaten into submission. But also the problem is hard for complicated applications. At a framework level we can tell the application to push undo/redo objects during the request, but I don't think that quite does the job in some of the weird cases. Yep. I agree. I fall on the "do no harm" side of the specification. When in doubt, do what you think is definitely right to make user's lives better. Keep track of the feedback for the next version. Hopefully get even better use cases for the next version and then get new versions out on a predictable and timely basis. -- Blake Sullivan

On Mon, Oct 25, 2010 at 8:14 PM, Blake Sullivan <blake.sullivan(a)oracle.com&gt; wrote: I think leaving out support for GETs is a bad idea. The OWSAP site states it quite well: The ideal solution is to only include the CSRF token in POST requests and modify server-side actions that have state changing affect to only respond to POST requests. If sensitive server-side actions are guaranteed to only ever respond to POST requests, then there is no need to include the token in GET requests. In most JavaEE web applications, however, HTTP method scoping is rarely ever utilized when retrieving HTTP parameters from a request. Calls to "HttpServletRequest.getParameter" will return a parameter value regardless if it was a GET or POST. See http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prev... for the entire page. Their sample filter handles GET requests. We can all talk about how updates should only occur during a POST, but that doesn't reflect reality -- there's nothing in JSF that enforces this. Also, with JSF 2's support for view parameters, it's even easier for developers to do unexpected things in GET requests. I don't see how this solution is any weaker than the algorithm proposed by OWSAP. Also, HDIV (http://www.hdiv.org/index.htm) which is used to secure different types of Java web apps, supports GET requests.

On Tue, Oct 26, 2010 at 2:41 PM, Andy Schwartz <andy.schwartz(a)oracle.com&gt; wrote: Perhaps I missed it earlier in the thread, but why? And what would you propose? -- Kito

On 10/26/10 11:01 AM, Kito Mann wrote: Thanks for the link Kito. I am actually familiar with this page. However, JSF is not "most JavaEE web applications." It is a specific framework and I believe that this section is talking about the problem where the servlet engine merges together the parameters from the URL with the POST body. However, since we won't have a problem with POSTs once we require that the state token is strengthened, why does this part apply? I agree that they can get into trouble here, but we can't do anything that makes things worse for existing developers. For example, how to you plan to deal with: 1) The initial request to the app 2) Bookmarked URLs 3) Referer leaking Were you planning to use a one use token or a per-page token? -- Blake Sullivan