Hi Jacques

I second your points. However, I have the following question -

Since you have explored and followed OWASP very extensively, do you think
with the introduction of same-site attribute, the whole concept of CSRF
token becomes somewhat redundant, provided almost every browser has the
support for this attribute now?
I haven't gone into too much detail, so my understanding on this is
limited. However, from what I understood, same-site has the ability to
become an all-in-one solution for CSRF attacks provided browsers honour it.

Best,
Girish


On Sat, Mar 28, 2020 at 2:39 PM Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> Hi,
>
> Of course, I have my own opinion. Here are my answers to these questions.
>
>  1. By default in OFBiz the session timeout is 1 hour. After that, OFBiz
> generates a new CSRF token before you sign in. I think for OFBiz
> applications
>     it's enough security. Of course we could have more fancy defenses like
> banks which are using random numeric pads for authentication and two-factor
>     authentication for important operations. Or companies like GitHub
> which use two-factor authentication in case of machine or browser change. I
>     don't think it's needed OOTB for OFBiz applications. Some users may
> need it but it's then to them to implement what they specifically need. So
>     random values generated by java.security.SecureRandom are safe enough
> in my opinion.
>  2. If someone tries to use a not auth protected request the CSRF defenses
> (token + same-site) will not allow it from another domain if csrf-token is
>     not set to false. That's already reassuring and we maybe not need to
> worry much about the remaining 195 cases where auth="false". Because there
>     are some obviously needed, like all those related to login or password
> change. For the others it may turn out that they are also needed for other
>     reasons. For them we need to test them one by one and in some case
> need to set csrf-token to false, for instance in case of requests in an
>     anonymous flow. So finally, despite the remaining 195 cases, it should
> not be too hard and too long to decide on this.
>
> Also note that with OFBIZ-11470 <
> https://issues.apache.org/jira/browse/OFBIZ-11470> we are more secured,
> in a CSRF perspective, with the same-site
> cookie attribute. It's not perfect in itself, but according to OWASP, it's
> the perfect duo for CSRF defense when associated with CSRF tokens.
>
> I continue to work on the remaining 195 cases where auth="false"...
>
> HTH
>
> Jacques
>
> Le 27/03/2020 à 19:16, Jacques Le Roux a écrit :
> > Hi All,
> >
> > Before I create a PR as a last opportunity to allow reviews and tests,
> I'd like to ask 2 last questions:
> >
> > 1. should we 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, as currently
> used, are safe enough. It's just that I wonder about the timeout. Should we
> > care?
> > 2. In relation with OFBIZ-4956, 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" must
> remain) we need to decide if should set the CSRF token check to false.
> >
> > Apart that my
> https://github.com/JacquesLeRoux/ofbiz-framework/tree/POC-for-CSRF-Token-OFBIZ-11306
> branch is ready to create a PR. We can't wait too
> > long about those 2 points, even if the 2nd needs a "bit" of work.
> Anyway, for now I'll wait answers, and hopefully help for OFBIZ-4956.
> >
> > Thanks
> >
> > Jacques
> >
> >
> > Le 26/03/2020 à 07:39, James Yong a écrit :
> >> +1 with CSRF defense enabled in Demo
> >>> Hi,
> >>>
> >>> I thought about that a bit more. I suggest to let the stable version
> (soon, R17) as is, ie with  CSRF defense enabled. This way users, mostly
> >>> interested in stable, would  see the real situation.
> >>>
> >>> And to use the NoCsrfDefenseStrategy in trunk. So developers, often
> brought to use the trunk for development reasons, would have more latitude;
> as
> >>> they certainly will do locally.
> >>>
> >>> If nobody disagree we will do so at
> https://issues.apache.org/jira/browse/OFBIZ-11472 with Swapnil
> >>>
> >>> If we do so, the link
> https://demo-stable.ofbiz.apache.org/ordermgr/control/main?USERNAME=admin&PASSWORD=ofbiz&JavaScriptEnabled=Y
> will no longer
> >>> work.
> >>>
> >>> https://demo-stable.ofbiz.apache.org/ordermgr should be used and we
> need to update https://ofbiz.apache.org/ofbiz-demos.html for that.
> >>>
> >>> Jacques
> >>>
> >>>
>

Reply via email to