From notifications-return-30431-apmail-ofbiz-notifications-archive=ofbiz.apache.org@ofbiz.apache.org Fri Feb 7 09:11:03 2020 Return-Path: X-Original-To: apmail-ofbiz-notifications-archive@minotaur.apache.org Delivered-To: apmail-ofbiz-notifications-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by minotaur.apache.org (Postfix) with SMTP id 8B32019E38 for ; Fri, 7 Feb 2020 09:11:03 +0000 (UTC) Received: (qmail 11583 invoked by uid 500); 7 Feb 2020 09:11:02 -0000 Delivered-To: apmail-ofbiz-notifications-archive@ofbiz.apache.org Received: (qmail 11502 invoked by uid 500); 7 Feb 2020 09:11:02 -0000 Mailing-List: contact notifications-help@ofbiz.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ofbiz.apache.org Delivered-To: mailing list notifications@ofbiz.apache.org Received: (qmail 11480 invoked by uid 99); 7 Feb 2020 09:11:02 -0000 Received: from mailrelay1-us-west.apache.org (HELO mailrelay1-us-west.apache.org) (209.188.14.139) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 07 Feb 2020 09:11:02 +0000 Received: from jira-he-de.apache.org (static.172.67.40.188.clients.your-server.de [188.40.67.172]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id A032CE3158 for ; Fri, 7 Feb 2020 09:11:01 +0000 (UTC) Received: from jira-he-de.apache.org (localhost.localdomain [127.0.0.1]) by jira-he-de.apache.org (ASF Mail Server at jira-he-de.apache.org) with ESMTP id 6DBDC780735 for ; Fri, 7 Feb 2020 09:11:00 +0000 (UTC) Date: Fri, 7 Feb 2020 09:11:00 +0000 (UTC) From: "Jacques Le Roux (Jira)" To: notifications@ofbiz.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Comment Edited] (OFBIZ-11306) POC for CSRF Token MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 [ https://issues.apache.org/jira/browse/OFBIZ-11306?page=3Dcom.atlassia= n.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D170= 32224#comment-17032224 ]=20 Jacques Le Roux edited comment on OFBIZ-11306 at 2/7/20 9:10 AM: ----------------------------------------------------------------- Hi Michael, Nothing directly related to this issue is commited. There is a reference to= this issue when I committed OFBIZ-11329 which was a blocking subtask but n= ot directly related to this work, hence it's committed. It was discovered w= rong while working on this issue and had been fixed. Then the Jira-Git rela= tion makes these commits appears here. [As I already explained|https://s.apache.org/72j22] I'm not a fan of featur= e branches. If we want to adopt a specific Git workflow we need a consensus= and I guess a vote. If we do a such thing I urge all participants to serio= uyly read the articles I cited. Anyway there is not need here, the last pat= ch is the reference.=20 bq. Regarding the store of the tokens inside the OFBiz cache: this seems to= be an invalid approach to me. The tokens should be hold in the session or = a cookie. As I (actually James) already explained in my last answer to you, the sessi= on can't be used because of inter webapps requests. Could you please explai= n how you envision to use cookies for that?=20 You might be interested to read https://stackoverflow.com/questions/2050484= 6/why-is-it-common-to-put-csrf-prevention-tokens-in-cookies.=20 I also recommend you to read the exchanges we already had with James. For i= nstance we don't want a sole token value (in a cookie for instance) but hav= e a strategy where the tokens are generated for each request. Though allowi= ng the back and forth buttons which is normally an issue with tokens by req= uests. Still, this is a WIP. At the moment our main remaining issue is related wit= h the introduction of REST requests with OFBIZ-11007 (James began before th= is was committed).=20 Of course all ideas are welcome, but please backed up with reviews and stro= ng support. was (Author: jacques.le.roux): Hi Michael, Nothing directly related to this issue is commited. There is a reference to= this issue when I committed OFBIZ-11329 which was a blocking subtask but n= ot directly related to this work, hence it's committed. It was discovered w= rong while working on this issue and had been fixed. Then the Jira-Git rela= tion makes these commits appears here. [As I already explainedhttps://s.apache.org/72j22] I'm not a fan of feature= branches. If we want to adopt a specific Git workflow we need a consensus = and I guess a vote. If we do a such thing I urge all participants to seriou= yly read the articles I cited. Anyway there is not need here, the last patc= h is the reference.=20 bq. Regarding the store of the tokens inside the OFBiz cache: this seems to= be an invalid approach to me. The tokens should be hold in the session or = a cookie. As I (actually James) already explained in my last answer to you, the sessi= on can't be used because of inter webapps requests. Could you please explai= n how you envision to use cookies for that?=20 You might be interested to read https://stackoverflow.com/questions/2050484= 6/why-is-it-common-to-put-csrf-prevention-tokens-in-cookies.=20 I also recommend you to read the exchanges we already had with James. For i= nstance we don't want a sole token value (in a cookie for instance) but hav= e a strategy where the tokens are generated for each request. Though allowi= ng the back and forth buttons which is normally an issue with tokens by req= uests. Still, this is a WIP. At the moment our main remaining issue is related wit= h the introduction of REST requests with OFBIZ-11007 (James began before th= is was committed).=20 Of course all ideas are welcome, but please backed up with reviews and stro= ng support. > POC for CSRF Token > ------------------ > > Key: OFBIZ-11306 > URL: https://issues.apache.org/jira/browse/OFBIZ-11306 > Project: OFBiz > Issue Type: Improvement > 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-v2.patch, OFBIZ-11306.patch, OFBIZ-11306.patch= , OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patc= h, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.pat= ch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.pa= tch, OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plug= ins.patch, OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch > > > CRSF tokens are generated using SecureRandom class (maybe later a JWT wit= h a "time out").=20 > 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 ta= g to exempt or force CSRF token check.=20 > # 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 fil= es. It will save time for users doing custom implementation and maintenance= . While there is CSRF token in the form URL, the token is invalidated duri= ng form submission. So it's uniqueand harmless even though the CSRF token o= f the form submission is shown in the browser address bar. > # For *Ajax calls* an ajaxPrefilter function (observer on DOM ready) is a= dded through OfbizUtil.js (itself called at start in decorators and such) > # The html metadata is storing the csrf token used by JQuery AJAX. This t= oken will not change to another value after it is consumed > # Csrf tokens for the user are removed from the UtilCache when the user l= ogs out or session invalidated. > The general rule are as follows: > * RequestMap configured with 'get' method will be exempted from CSRF toke= n check. > * RequestMap configured with 'post' or 'all' method will be subjected to = CSRF token check. (Note there are discussions that RequestMap with =E2=80= =98all=E2=80=99 method should also not be subjected to CSRF token check. Th= is 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 th= e general rules above. > To implement: > * -Allow token map size to be configurable in properties.- OK that's done= locally > To Discuss: > * Invalidate authenticated user session when CSRF token check fails. > * Configure the general rules in a Service method (which will be run insi= de the constructor of RequestMap class) when determining the final security= CsrfToken value. -- This message was sent by Atlassian Jira (v8.3.4#803005)