Skip to content
This repository has been archived by the owner on Sep 5, 2023. It is now read-only.

Specify CSRF Solution #869

Closed
glassfishrobot opened this issue Jul 23, 2010 · 54 comments
Closed

Specify CSRF Solution #869

glassfishrobot opened this issue Jul 23, 2010 · 54 comments

Comments

@glassfishrobot
Copy link

The specification should specify a solution for preventing CSRF (Cross Site
Request Forgery) for implementations to use. See:
https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=812

Environment

Operating System: All
Platform: Macintosh

Affected Versions

[2.0]

@glassfishrobot
Copy link
Author

Reported by rogerk

@glassfishrobot
Copy link
Author

Issue-Links:
depends on
JAVASERVERFACES-2269
JAVASERVERFACES-2204

@glassfishrobot
Copy link
Author

rogerk said:
Target

@glassfishrobot
Copy link
Author

rogerk said:
target

@glassfishrobot
Copy link
Author

rogerk said:
Kito has described potential solutions as well in:
https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=559

@glassfishrobot
Copy link
Author

rogerk said:
target MS6

@glassfishrobot
Copy link
Author

rogerk said:
The following attachment is for the "hidden field" approach for the CSRF
solution. Essentially a "javax.faces.ViewToken" hidden field is rendered
during Form rendering. This hidden field value is compared to the token
generated from a "secret key" stored in the session during Restore View Phase
processing.

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=282)
Changes For The Hidden Field CSRF Approach

@glassfishrobot
Copy link
Author

File: 869-hidden.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=283)
Revised Hidden Field Approach Change Bundle

@glassfishrobot
Copy link
Author

File: 869-hidden.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=284)
Revised Hidden Field Approach Change Bundle

@glassfishrobot
Copy link
Author

File: 869-hidden.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

File: 869-hidden.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=285)
Latest Revised Hidden Field Approach Change Bundle

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=286)
Latest Revised Action URL Approach Change Bundle

@glassfishrobot
Copy link
Author

File: 869-url.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

rogerk said:
changelog indicator

@glassfishrobot
Copy link
Author

@edburns said:
Ensure changelog_2_1 is in keywords list

@glassfishrobot
Copy link
Author

File: changebundle.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=294)
Latest Iteration Of Changes

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=297)
Latest Iteration Of Changes

@glassfishrobot
Copy link
Author

File: changebundle.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

@edburns said:
Created an attachment (id=298)
patch with fixes for session use and form id

@glassfishrobot
Copy link
Author

File: i869.patch
Attached By: @edburns

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=299)
Latest Iteration Of Changes

@glassfishrobot
Copy link
Author

File: changebundle.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

rogerk said:
Checked In.

@glassfishrobot
Copy link
Author

rogerk said:
Reopening temporarily as the default option for CSRF should be "none".
If we default to something other than "none" we would break backwards
compatibility of existing applications using 2.1 - especially applications
that have not yet implemented new Form Renderer requirements.
Changebundle forthcoming ...

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=300)
Latest Iteration Of Changes

@glassfishrobot
Copy link
Author

File: changebundle.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

rogerk said:
Checked in.

@glassfishrobot
Copy link
Author

rogerk said:
Created an attachment (id=303)
Revised Changes To Take Into Account Token Verification Exceptions Over Ajax (if responsewriter not available)

@glassfishrobot
Copy link
Author

File: changebundle.txt
Attached By: rogerk

@glassfishrobot
Copy link
Author

@edburns said:
Index: jsf-api/src/main/java/javax/faces/application/ViewHandler.java

==

Please put the new content in a element.

Otherwise, looks great.

r=edburns

@glassfishrobot
Copy link
Author

rogerk said:
Reopening due to late EG feedback. Also
see:https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1850

@glassfishrobot
Copy link
Author

rogerk said:
Target 2.2 (Unfortunately)

@glassfishrobot
Copy link
Author

@edburns said:
remove changelog_2_1 tag

@glassfishrobot
Copy link
Author

kellerapps said:
JSF components should support safe HTML.

@glassfishrobot
Copy link
Author

@glassfishrobot
Copy link
Author

dened said:
I wonder is there any reason for JSF to support the anti-CSRF token in URL in addition to (or instead of) POST parameter? Isn't POST parameter sufficient for the purpose? I think, support for the token in URL just adds unnecessary complications to a implementation and also a little weakens security. Please correct me if I'm wrong.

@glassfishrobot
Copy link
Author

@edburns said:
Adding jsf-api/doc/javaee_6.xsd
Adding jsf-api/doc/javaee_web_services_client_1_3.xsd
Adding jsf-api/doc/web-facesconfig_2_2.xsd
Sending jsf-ri/build.xml
Sending jsf-ri/src/main/java/com/sun/faces/config/ConfigManager.java
Sending jsf-ri/src/main/java/com/sun/faces/config/DbfFactory.java
Adding jsf-test/#869
Adding jsf-test/#869/build.xml
Adding jsf-test/#869/i_spec_869_war
Adding jsf-test/#869/i_spec_869_war/pom.xml
Adding jsf-test/#869/i_spec_869_war/src
Adding jsf-test/#869/i_spec_869_war/src/main
Adding jsf-test/#869/i_spec_869_war/src/main/java
Adding jsf-test/#869/i_spec_869_war/src/main/java/com
Adding jsf-test/#869/i_spec_869_war/src/main/java/com/sun
Adding jsf-test/#869/i_spec_869_war/src/main/java/com/sun/faces
Adding jsf-test/#869/i_spec_869_war/src/main/java/com/sun/faces/regression
Adding jsf-test/#869/i_spec_869_war/src/main/java/com/sun/faces/regression/i_spec_869_war
Adding jsf-test/#869/i_spec_869_war/src/main/webapp
Adding jsf-test/#869/i_spec_869_war/src/main/webapp/WEB-INF
Adding jsf-test/#869/i_spec_869_war/src/main/webapp/WEB-INF/beans.xml
Adding jsf-test/#869/i_spec_869_war/src/main/webapp/WEB-INF/faces-config.xml
Adding jsf-test/#869/i_spec_869_war/src/main/webapp/WEB-INF/web.xml
Adding jsf-test/#869/i_spec_869_war/src/main/webapp/i_spec_869_war.xhtml
Adding jsf-test/#869/i_spec_869_war/src/main/webapp/i_spec_869_war_protected.xhtml
Transmitting file data .............
Committed revision 9377.

@glassfishrobot
Copy link
Author

@edburns said:
snapshot

@glassfishrobot
Copy link
Author

File: 20110920-i_spec_869.patch
Attached By: @edburns

@glassfishrobot
Copy link
Author

@edburns said:
Committed to trunk.

Adding jsf-api/src/main/java/javax/faces/application/ProtectedViewException.java
Sending jsf-api/src/main/java/javax/faces/application/ViewHandler.java
Sending jsf-api/src/main/java/javax/faces/application/ViewHandlerWrapper.java
Sending jsf-api/src/main/java/javax/faces/component/UIViewAction.java
Sending jsf-api/src/main/java/javax/faces/render/ResponseStateManager.java
Sending jsf-api/src/main/java/javax/faces/render/package.html
Sending jsf-ri/conf/test/web.xml
Sending jsf-ri/src/main/java/com/sun/faces/application/view/MultiViewHandler.java
Sending jsf-ri/src/main/java/com/sun/faces/config/ConfigManager.java
Sending jsf-ri/src/main/java/com/sun/faces/config/WebConfiguration.java
Adding jsf-ri/src/main/java/com/sun/faces/config/processor/ProtectedViewsConfigProcessor.java
Sending jsf-ri/src/main/java/com/sun/faces/lifecycle/RestoreViewPhase.java
Sending jsf-ri/src/main/java/com/sun/faces/renderkit/ClientSideStateHelper.java
Sending jsf-ri/src/main/java/com/sun/faces/renderkit/ResponseStateManagerImpl.java
Sending jsf-ri/src/main/java/com/sun/faces/renderkit/StateHelper.java
Sending jsf-ri/src/main/java/com/sun/faces/util/FacesLogger.java
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_de.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_es.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_fr.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_ja.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_ko.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_pt_BR.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_zh_CN.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_zh_HK.properties
Sending jsf-ri/src/main/resources/com/sun/faces/LogStrings_zh_TW.properties
Sending jsf-test/#869/i_spec_869_war/src/main/webapp/WEB-INF/faces-config.xml
Sending jsf-test/#869/i_spec_869_war/src/main/webapp/WEB-INF/web.xml
Sending jsf-test/#869/i_spec_869_war/src/main/webapp/i_spec_869_war.xhtml
Transmitting file data .............................
Committed revision 9393.

@glassfishrobot
Copy link
Author

@edburns said:
Committed to trunk.

Sending applicationIntegration.fm
Sending preface.fm
Sending renderingModel.fm
Sending requestProcessingLifecycle.fm
Transmitting file data ....
Committed revision 1032.

@glassfishrobot
Copy link
Author

@arjantijms said:
Just wondering about one thing, when state is stored on the server and the value of javax.faces.ViewState is sufficiently random, isn't that also a protection against CSRF? If an attacker can not guess this value, then this also functions as a hidden token, doesn't it?

@glassfishrobot
Copy link
Author

@edburns said:
arjan_tijms: yes, it certainly is, and that is why most of the work of this issues is dealing with cases that are not postbacks. In other words, we need a way to protect GET requests within an app from CSRF attacks as well.

@glassfishrobot
Copy link
Author

Marked as fixed on Wednesday, December 14th 2011, 1:53:47 pm

@glassfishrobot
Copy link
Author

@manfredriem said:
Closing resolved issue out

@glassfishrobot
Copy link
Author

This issue was imported from java.net JIRA JAVASERVERFACES_SPEC_PUBLIC-869

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants