[ 
https://issues.apache.org/jira/browse/OFBIZ-11306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17064786#comment-17064786
 ] 

Jacques Le Roux commented on OFBIZ-11306:
-----------------------------------------

Hi James,

I had still some notes not verified or answered.

You asked:
bq.  My question is whether it is correct to allow one web app to ajax call 
another web app?  Allowing one web app to ajax call another web app, with the 
former web app knowing the csrf token of the latter web app, is only possible 
if we convert the static js files to ftl files. But I don't think there is many 
use case for it.For now, I have set the security token check to false for 
/getAssociatedStateList in Catalog app, to allow the eCommerce app to call the 
uri.

With OFBIZ-11470 we are secured, in a CSRF perspective, with the same-site 
cookie attribute. It's not perfect, but according to OWASP, is the perfect duo 
for CSRF defense when associated with CSRF tokens. So I think we should not 
worry too much about such cases. Actually we should even extend that to other 
similar cases. I stumbled upon that while working on OFBIZ-4956 when [Deepak 
spotted an issue with getAssociatedStateList when auth="true" was 
required|https://markmail.org/message/moquflgvs7yclwid]. Like for 
getAssociatedStateList, we need to check the remaining 195 cases where 
auth="false" and decide if we should change to "true", with the CSRF defense 
then used by default. In other cases (auth="false") we need to decide if should 
set the CSRF token check to false.

BTW, you speak about /getAssociatedStateList in Catalog app when you actually 
changed in common-controller. There is no need to change it there because , 
apart the ecommerce application, there are no application that requires an 
anonymous flow is required. It should be only changed in ecommerce controller.

At some point I wondered if we should not use a JWT rather than a (pseudo) 
random value for the CSRF token, this for timeout reason. Don't get me wrong 
I'm sure that the random values generated by java.security.SecureRandom are 
safe enough. It's just that I wonder about the timeout, you are never too 
cautious. The situation in Europe with the Covid-19 dramatically shows it. I'll 
ask the community about that, it's not a decision we should take alone.

I have no other remaining notes :)

> POC for CSRF Token
> ------------------
>
>                 Key: OFBIZ-11306
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-11306
>             Project: OFBiz
>          Issue Type: Sub-task
>          Components: ALL APPLICATIONS
>    Affects Versions: Upcoming Branch
>            Reporter: James Yong
>            Assignee: Jacques Le Roux
>            Priority: Minor
>              Labels: CSRF
>             Fix For: Upcoming Branch
>
>         Attachments: CsrfTokenAjaxTransform.java, CsrfTokenTransform.java, 
> CsrfUtil.java, OFBIZ-11306-alternative merged with James's.patch, 
> OFBIZ-11306-alternative merged with James's.patch, OFBIZ-11306-alternative 
> merged with James's.patch, OFBIZ-11306-alternative.patch, 
> OFBIZ-11306-alternative.patch, OFBIZ-11306-alternative.patch, 
> OFBIZ-11306-alternative.patch, OFBIZ-11306-alternative.patch, 
> OFBIZ-11306-alternative.patch, OFBIZ-11306-alternative.patch, 
> OFBIZ-11306-alternative.patch, OFBIZ-11306-alternative.patch, 
> OFBIZ-11306-v2.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, 
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, 
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, 
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, 
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, 
> OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch, 
> OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch, 
> OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch, 
> partyTokenMap.webtools.txt
>
>
> CRSF tokens are generated using SecureRandom class (maybe later a JWT with a 
> "time out"). 
> They are stored in the user sessions (for AJAX calls and unauthenticated HTTP 
> calls) or OFBiz UtilCache (for authenticated HTTP calls), and verified during 
> POST request.
> # In *controllers* a new csrf-token attribute is added to the security tag to 
> exempt or force CSRF token check. 
> # In *Widget Forms* a hidden token field is auto-generated.
> # In *FTL form* a CSRF token is passed through <@ofbizUrl> to automatise the 
> change. Using <@ofbizUrl> macro to generate the CSRF token means there is no 
> need to manually add the CSRF token field to each form in the ftl files. It 
> will save time for users doing custom implementation and maintenance.  While 
> there is CSRF token in the form URL, the token is invalidated during form 
> submission. So it's unique and harmless even though the CSRF token of the 
> form submission is shown in the browser address bar.
> # For *Ajax calls* an ajaxPrefilter function (observer on DOM ready) is added 
> through OfbizUtil.js (itself called at start in decorators and such)
> # The html metadata is storing the csrf token used by JQuery AJAX. This token 
> will not change to another value after it is consumed
> # Csrf tokens for the user are removed from the UtilCache when the user logs 
> out or session invalidated.
> The general rule are as follows:
> * RequestMap configured with 'get' method will be exempted from CSRF token 
> check.
> * RequestMap configured with 'post' or 'all' method will be subjected to CSRF 
> token check. (Note there are discussions that RequestMap with ‘all’ method 
> should also not be subjected to CSRF token check. This will be done after 
> ensuring a separate uri is used when posting changes.)
> * "main" request URIs are exempted from CSRF token check.
> * Setting csrf-token to false or true on the Request Map will override the 
> general rules above.
> To Discuss:
> * Invalidate authenticated user session when CSRF token check fails.
> * Configure the general rules in a Service method (which will be run inside 
> the constructor of RequestMap class) when determining the final 
> securityCsrfToken value.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to